fix(platform): close cross-org isolation leaks (server + release-execution reads)#160
Merged
Merged
Conversation
The ReleaseExecutionStatus and ReleaseRequestStatus enums both include rollback_failed, but the CHECK constraints from 0008 never did. A failed rollback therefore could not persist its terminal status: the UPDATE hit the constraint, the error was swallowed, the execution stayed rollback_in_progress, and the worker re-claimed and re-ran the failing rollback every 2s poll, forever. Rebuild both CHECKs with the full status set the code writes.
The 0020 unique index guarantees at most one active execution per release request, but it cannot see across requests: every direct publish/redeploy mints a fresh internal request, so two concurrent publishes of the same ruleset (or a publish racing a governed execute) would still both roll out. Add a target-level guard - any active execution for the same project + ruleset + environment now returns 409 from both execute_release_request and the direct-publish enqueue path. Also map the 0020 unique-index violation to 409 instead of 500 for the loser of two concurrent executes of the same request.
The old safety net (fail_stuck_active_executions) was dead code: both bootstrap callers passed false, and enabling it would have been wrong anyway - it had no grace period and would kill in-flight rollouts on every restart, and its rollback_failed write predates the 0022 CHECK fix. Remove it. The poll loop already self-heals claimable executions (the session advisory lock releases when a dead worker's connection drops), but a terminal execution is not claimable, so a worker crash between settling the execution and settling the surrounding state stranded rows forever: - dispatched deployments whose execution already finished (studio polls them indefinitely) - executing requests whose latest execution already finished - executing requests that never got an execution row Add a 60s reconciliation task in the worker that repairs all three, with grace periods so it never races the worker's own settle writes. Completed-via-rollback executions are told apart from completed rollouts by their rollback_in_progress -> completed history row, so requests settle to rolled_back exactly as the worker would have written. Repairs are recorded in the request history and are idempotent, healing rows stranded before this task existed.
A rolling_out / rollback_in_progress execution is worker-claimable, and build_rolling_context / build_rollback_context hard-errored when a row they depend on was gone (a bound server pruned as stale >30min or deleted by an admin, the request, environment, draft, or rollback snapshot). The spawned worker task only logged that error and dropped its advisory lock without writing any status, so the execution stayed claimable and was re-claimed and re-failed every 2s poll forever. The reconciler could not help — it only repairs executions that already reached a terminal status. Distinguish a permanent missing dependency (SetupDependencyMissing, from a None lookup) from a transient store error (propagated, retried next poll). On a permanent failure, settle the execution terminally (Failed, or RollbackFailed for a rollback) along with its request and any direct-publish deployment row, instead of looping.
Three handlers took an id from the path and fetched by that id alone, after only a coarse auth check — and server ids are derivable (sha256 of the engine URL), so the ids are guessable across orgs: - get_server / get_server_metrics / get_server_health ignored the caller's claims entirely. Any authenticated user could read another org's engine (url, version, capabilities) and, via metrics/health, trigger a control-plane NATS RPC to it. Now an org-owned server requires the caller to be a member of that org; global (unowned) servers stay readable, matching delete_server's gating. - get_release_execution_for_request / list_release_execution_events checked the permission on the path org/project but then fetched by the unscoped release_id / execution_id. An admin in their own org could read another org's execution state and event stream with a guessed id. Both now resolve the release through the org+project scoped getter (404 on mismatch), and the events handler additionally verifies the execution belongs to that release. Also scope the engine-proxy fallback: when a project's environment has no bound server (the auto-created production env has none, so this is common), the fallback pool is now restricted to the caller's org plus deliberately-global servers, never another org's engine — previously it picked the most-recently-registered server anywhere, routing org A's eval traffic and ruleset payloads to org B's engine.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the cross-org read leaks from the 2026-07-09 auth audit. Stacked on #158 (base =
fix/platform-release-correctness, because the release-execution fix touchesexecutions.rswhich #158 also changes); GitHub will retarget this tomainonce #158 merges.Org membership is the isolation boundary the product sells, and
server_id = sha256(normalized_url)is derivable — an attacker who guesses a victim engine's URL (http://ordo-server:8080,http://localhost:8080, …) computes its id. Three handlers trusted that id after only a coarse auth check.1. Server detail / metrics / health — claims ignored (High)
get_server,get_server_metrics,get_server_healthtookExtension(_claims)and fetched by id with no org check. Any authenticated user (even in zero orgs) could:Fix: an org-owned server now requires the caller to be a member of that org (
load_org_and_check_role(Role::Viewer)); a global (unowned) server stays readable by any authenticated user, matching howdelete_serveralready gates only when an org owns the server.2. Release execution / events — fetched by unscoped id (Medium)
get_release_execution_for_requestandlist_release_execution_eventschecked the permission on the path org/project, then fetched by the rawrelease_id/execution_id. An admin/owner in their own org could read another org's execution detail and event stream (instance/server names, versions, rollout progress) with a guessed id. Both now resolve the release through the org+project-scopedget_release_request(→ 404 on mismatch), and the events handler additionally verifies the execution belongs to that release. Mirrors howlist_release_request_historyalready does it correctly.3. Engine-proxy fallback routed to the global pool (High)
resolve_engine_urlignored itsorg_id;resolve_fallback_engine_urlpicked the most-recently-registered online server from all orgs. The auto-createdproductionenv has no bound server, so this fallback is commonly hit — meaning org A's eval traffic (itsX-Tenant-IDand, for ruleset writes, the full payload) could be shipped to org B's engine. Fix: the fallback pool is now restricted to the caller's org plus deliberately-global (unowned) servers; the configuredengine_urlremains the final fallback. A legitimately-shared global engine still works; another org's engine is never borrowed.Not included (from the same audit, deferred)
create_role/assign_member_rolehas no permission ceiling / self-assign guard (privilege escalation, needs a policy decision).org_idunder the shared secret; connect tokens never expire.Verification
cargo check/clippy -p ordo-platformclean. Read-authorization changes; the fallback change is data-plane and should be smoke-tested on a cluster with an unbound-env project before relying on it (per the repo's "data-plane changes get cluster-verified" convention).