control-plane-api: require_unmasked guards credential and admin surfaces - #3427
Open
bbartman wants to merge 2 commits into
Open
control-plane-api: require_unmasked guards credential and admin surfaces#3427bbartman wants to merge 2 commits into
bbartman wants to merge 2 commits into
Conversation
This was referenced Aug 27, 2026
bbartman
force-pushed
the
bmb/3376-stack-8-unmasked-guards
branch
2 times, most recently
from
August 28, 2026 12:17
a9c1cb1 to
9a83348
Compare
bbartman
force-pushed
the
bmb/3376-stack-8-unmasked-guards
branch
2 times, most recently
from
August 28, 2026 16:59
ed2a716 to
58810c1
Compare
bbartman
force-pushed
the
bmb/3376-stack-8-unmasked-guards
branch
from
August 31, 2026 12:45
58810c1 to
54ed52f
Compare
Task 6 of #3376: close the remaining surfaces a masked bearer must not reach. Forbidden::require_unmasked is the one definition of the masked-bearer refusal - keyed on the capability_mask claim's presence, never its value - consumed by requirement evaluation at extraction, by the capability_token mint, and directly by GraphQL resolvers, which an axum extractor cannot reach. createRefreshToken refuses masked bearers ahead of its service-account lookup: a refresh token exchanges for a full-authority access token, which would escape the mask. Revocation never widens authority and stays open to masked bearers, with a test pinning both. /admin/create-data-plane and /admin/update-l2-reporting take Authority<RequireUnmasked>: their SQL internal.user_roles authorization cannot bind the capability ceiling, so they fail closed for masked bearers, byte-identical for unmasked callers. Each gains its first tests: the structured 403 for a masked bearer, and the handler's own ops/-admin refusal for an unmasked one. The RequireUnmasked doc carries the audited inventory of unmasked-only surfaces and why everything else deliberately stays open; the SQL functions reachable through PostgREST remain the documented mask-bypass boundary tracked under #3376 task 8. The GraphQL schema regen also picks up the userCapability description from the legacy-decision migration, which had not been regenerated.
revokeApiKey authorizes RevokeApiKey through the mask-filtered grant walk; it is not an identity-gated revocation, and grouping it with revokeRefreshToken misstated why it needs no unmasked guard.
bbartman
force-pushed
the
bmb/3376-stack-8-unmasked-guards
branch
from
September 1, 2026 13:08
54ed52f to
2e8cdaa
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.
Task 6 of #3376 (stacked on #3424): the
require_unmaskedguards. Masked bearers are refused on every surface where holding one must not mint a wider credential or slip past mask enforcement entirely.What this does
Forbidden::require_unmasked(&ControlClaims)is the single definition of the masked-bearer refusal — keyed on thecapability_maskclaim's presence, never its value, because a mask which happens to enable everything is still a deliberately-reduced credential. Requirement evaluation consumes it at extraction, thecapability_tokenmint consumes it in place of its inline check, and GraphQL resolvers consume it directly, where an axum extractor cannot reach.GraphQL
createRefreshTokenrefuses masked bearers with the structuredunmasked_token_requiredbody (carried in error extensions, identical to the REST shape). A refresh token exchanges for a full-authority access token, so a masked bearer minting one would escape its mask. The refusal precedes the service-account lookup: it's a pure function of the verified claims, so a masked caller costs no DB round-trip — the same ordering as the mint.revokeRefreshTokendeliberately stays open to masked bearers: revocation never widens authority. A test pins both sides./admin/create-data-planeand/admin/update-l2-reportingtakeAuthority<RequireUnmasked>. Their authorization is SQLinternal.user_rolesrather than the snapshot walk, so the capability ceiling structurally cannot bind there; fail-closed is the safe posture until the wider refactor retires their SQL checks (decision 12 on the ticket). Unmasked callers are byte-identical.The identity-gated mutation audit (decision 8) is re-verified against the current mutation surface and documented as the
RequireUnmaskeddoc comment, where the next person adding a credential mutation will find it: the unmasked-only inventory (the mint,createRefreshToken, both/adminroutes), and why everything else deliberately stays open — revocations never widen, credential-adjacent operations (createApiKey,createServiceAccount) authorize through the mask-filtered grant walk, and invite redemption widens the user's grants while the bearer still exercises them only through its mask.What this does not close
The SQL twins reachable through PostgREST —
public.create_refresh_tokenandpublic.gateway_auth_token— remain an open bypass for a masked token presented directly to Supabase, part of the documented PostgREST mask-evaporation boundary whose resolution is the #2877 migration (task 8 on the ticket). This PR closes the control-plane-API escapes only.Plan as executed
Forbiddenbeside therequired_coveredprecedent, so "masked = claim presence" has exactly one definition; the mint and requirement evaluation are refactored onto it.createRefreshToken: mask check first — pure claims check before any I/O, matching the mint./admincomments rewritten in the present tense: what binds, what cannot, and where the refactor is tracked.RequireUnmaskeddoc) rather than only in ticket/PR prose./adminroute gets its first tests — a masked bearer draws the structured 403 at extraction, and an unmasked non-ops/-admin draws the handler's ownpermission_denied, proving unmasked callers still reach the unchanged handler logic. Full success-path provisioning fixtures are deliberately out of scope.userCapabilitydescription text from control-plane-api, tables: legacy decisions migrate to effective capability bits #3422, which had not been regenerated there — doc-only drift, caught up here rather than churning the stack.Tests
authority::test::*—require_unmaskedbehavior unchanged under the refactor (presence-keyed refusal, HTTP-level 403 bodies).refresh_tokens::test::test_create_refresh_token_requires_unmasked— masked create refused withunmasked_token_requiredand emptymissing_capabilitiesin extensions; unmasked create succeeds; masked revocation succeeds.create_data_plane::test::test_create_data_plane_requires_unmasked,update_l2_reporting::test::test_update_l2_reporting_requires_unmasked— an Admin-enabling mask is still refused (presence, not value) with the structured body; an unmasked non-admin reaches the handler's ownops/gate.token_exchange::test::test_capability_token_mint— the mint's refusals, now through the shared helper.