control-plane-api: create_data_plane gates ops admin via Snapshot - #3435
Open
bbartman wants to merge 4 commits into
Open
control-plane-api: create_data_plane gates ops admin via Snapshot#3435bbartman wants to merge 4 commits into
bbartman wants to merge 4 commits into
Conversation
This was referenced Aug 28, 2026
bbartman
force-pushed
the
bmb/strangle-1-create-data-plane
branch
from
August 28, 2026 17:48
6d447cd to
534f215
Compare
bbartman
force-pushed
the
bmb/strangle-1-create-data-plane
branch
from
August 31, 2026 14:05
534f215 to
0a1a568
Compare
bbartman
force-pushed
the
bmb/strangle-1-create-data-plane
branch
from
September 1, 2026 12:02
0a1a568 to
fc54bc0
Compare
The /admin/create-data-plane and /admin/update-l2-reporting endpoints gate on the user being an admin of 'ops/', each via an identical SQL query over internal.user_roles(). As part of strangling that function out of reachable Rust paths, introduce a Snapshot-based policy helper shaped for Envelope::authorization_outcome, alongside the existing evaluate_names_authorization. The helper deliberately changes semantics: user_roles() walks the grant graph only downward from roles the user holds, while the Snapshot walk also reaches grants held by ancestor subject roles — the same gap tracked by #2848. An admin chain into 'ops/' through an ancestor subject was wrongly denied by the SQL gate and is authorized here. The unit test pins this case by name. Endpoints are switched to the helper in follow-up commits.
Swap the endpoint's internal.user_roles() SQL gate for evaluate_ops_admin over the request's pinned Snapshot, evaluated through Envelope::authorization_outcome. Beyond strangling the SQL function, this upgrades a denial from a terminal 403 to the standard retry protocol: a freshly-granted ops admin triggers an early Snapshot refresh and a 307 retry rather than a hard failure, matching the authorize_* endpoints. The identical SQL gate in update_l2_reporting is migrated separately, in the next change of this stack. Endpoint tests cover the denial paths only: an authorized request proceeds into data-plane provisioning, which authorization tests have no business exercising. Allowed cases are pinned by evaluate_ops_admin unit tests.
Review of the stack's first change asked that the deliberate semantic divergence from internal.user_roles() — authorizing admin chains reached through ancestor subject roles — be pinned by a named test, rather than a commented section inside the combined case list.
Review flagged evaluate_ops_admin as a possible Middle Man: it was evaluate_names_authorization with a fixed Admin capability, a fixed 'ops/' prefix, and a bespoke error message. The bespoke message isn't worth a second walk implementation — and the generic message is the denial text every other endpoint already emits — so the endpoint now calls evaluate_names_authorization directly. The named tests keep pinning the ops-gate semantics, including the ancestor-subject chain divergence from internal.user_roles(). Review also flagged the test-side grant-building duplication: the new snapshot_of_grants helper repeated make_grants from the authorized_prefixes tests. Both now share one fixture in test_server.
bbartman
force-pushed
the
bmb/strangle-1-create-data-plane
branch
from
September 2, 2026 12:00
fc54bc0 to
e167eca
Compare
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
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.
Part 1 of a 3-PR stack strangling the last reachable Rust call sites of
internal.user_roles(), continuing the Snapshot-authorization work of #2781. Stacked on the publications-spec-authz branch.Plan
After the publications migration, exactly three reachable Rust paths still authorize via
internal.user_roles()SQL:/admin/create-data-plane— this PR./admin/update-l2-reporting— next PR, an identical swap.user_has_admin_capability) — final PR, which must thread the agent'ssnapshot_watchintoDirectiveHandler.Everything else that references
user_roles/auth_rolesis SQL-land reachable only via PostgREST/RLS (live_specs_ext,combined_grants_ext,gateway_auth_token, …) and is out of scope for this stack. In particular, the #2848 workaroundensure_private_data_plane_grantsstays: its removal is gated on the RLS-side migration, not on these three.This change
create_data_plane's SQL gate forevaluate_names_authorization(snapshot, claims, Admin, ["ops/"]), evaluated throughEnvelope::authorization_outcome. Denials now follow the standard authorization-retry protocol (early Snapshot refresh + 307 retry when the Snapshot may be stale) instead of failing terminally, matching theauthorize_*endpoints. This is an HTTP-envelope mechanism; GraphQL resolvers and the (upcoming) directive handler instead take one-shot pinned-Snapshot checks.evaluate_ops_adminhelper with a bespoke message; review flagged it as a Middle Man overevaluate_names_authorizationand it was folded away.)make_grants/snapshot_of_grantstotest_server, deduplicating the grant-table construction previously repeated in theauthorized_prefixestests.Deliberate semantic change
internal.user_roles()walks the grant graph only downward from roles the user holds. The Snapshot walk (tables::UserGrant::is_authorized) also reaches grants held by ancestor subject roles — the same gap tracked by #2848. An admin chain intoops/through an ancestor subject (e.g. admin ofestuary/support/with a(estuary/ → ops/, admin)grant) was wrongly denied by the SQL gate and is authorized here. The named testtest_evaluate_ops_admin_ancestor_subject_chainpins this.Tests
server::testspin the ops-gate semantics: directops/admin, unrelated-tenant admin denied (with the exact denial message),read≠ admin, the ancestor-subject chain allowed, and the same chain denied without admin capability.Review
An
xhightwo-axis review (standards + spec) ran against this PR; its findings — a named-test granularity nit, the Middle Man helper, and the duplicated test fixture — are addressed in the two follow-up commits.