Skip to content

control-plane-api: require_unmasked guards credential and admin surfaces - #3427

Open
bbartman wants to merge 2 commits into
bmb/3376-stack-7-capability-mintfrom
bmb/3376-stack-8-unmasked-guards
Open

control-plane-api: require_unmasked guards credential and admin surfaces#3427
bbartman wants to merge 2 commits into
bmb/3376-stack-7-capability-mintfrom
bmb/3376-stack-8-unmasked-guards

Conversation

@bbartman

Copy link
Copy Markdown
Contributor

Task 6 of #3376 (stacked on #3424): the require_unmasked guards. 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 the capability_mask claim'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, the capability_token mint consumes it in place of its inline check, and GraphQL resolvers consume it directly, where an axum extractor cannot reach.

GraphQL createRefreshToken refuses masked bearers with the structured unmasked_token_required body (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. revokeRefreshToken deliberately stays open to masked bearers: revocation never widens authority. A test pins both sides.

/admin/create-data-plane and /admin/update-l2-reporting take Authority<RequireUnmasked>. Their authorization is SQL internal.user_roles rather 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 RequireUnmasked doc comment, where the next person adding a credential mutation will find it: the unmasked-only inventory (the mint, createRefreshToken, both /admin routes), 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_token and public.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

  • Shared guard: a pure helper on Forbidden beside the required_covered precedent, so "masked = claim presence" has exactly one definition; the mint and requirement evaluation are refactored onto it.
  • Guard ordering in createRefreshToken: mask check first — pure claims check before any I/O, matching the mint.
  • /admin comments rewritten in the present tense: what binds, what cannot, and where the refactor is tracked.
  • Audit documentation lives with the mechanism (RequireUnmasked doc) rather than only in ticket/PR prose.
  • Tests: denial paths plus a SQL-gate probe. Each /admin route gets its first tests — a masked bearer draws the structured 403 at extraction, and an unmasked non-ops/-admin draws the handler's own permission_denied, proving unmasked callers still reach the unchanged handler logic. Full success-path provisioning fixtures are deliberately out of scope.
  • The GraphQL schema regen also picks up the userCapability description 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_unmasked behavior unchanged under the refactor (presence-keyed refusal, HTTP-level 403 bodies).
  • refresh_tokens::test::test_create_refresh_token_requires_unmasked — masked create refused with unmasked_token_required and empty missing_capabilities in 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 own ops/ gate.
  • token_exchange::test::test_capability_token_mint — the mint's refusals, now through the shared helper.

@bbartman
bbartman force-pushed the bmb/3376-stack-8-unmasked-guards branch 2 times, most recently from a9c1cb1 to 9a83348 Compare August 28, 2026 12:17
@bbartman
bbartman force-pushed the bmb/3376-stack-8-unmasked-guards branch 2 times, most recently from ed2a716 to 58810c1 Compare August 28, 2026 16:59
@bbartman
bbartman force-pushed the bmb/3376-stack-8-unmasked-guards branch from 58810c1 to 54ed52f Compare August 31, 2026 12:45
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
bbartman force-pushed the bmb/3376-stack-8-unmasked-guards branch from 54ed52f to 2e8cdaa Compare September 1, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant