Skip to content

security(rest): require a capability on the external-datasource federation family - #10254

Merged
os-elon merged 2 commits into
mainfrom
claude/issue-9901-federation-family-capability-gate
Aug 20, 2026
Merged

security(rest): require a capability on the external-datasource federation family#10254
os-elon merged 2 commits into
mainfrom
claude/issue-9901-federation-family-capability-gate

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#9901

Maintainer ruling, 2026-08-20 (issue comment 5353919655), verbatim: 「其他接受你的建议。」 — the external-datasource federation family is not deliberately the lower-privilege door, and converges with its declared admin twin on the capability axis.

What changed

packages/rest/src/external-datasource-routes.ts gated on authentication alone (#9686, which ruled the capability question out of its own scope and pointed it here). It now applies a capability above that floor, from the same single resolveExecutionContext call:

routeSDK callrequires
GET /:name/external/tablesdatasources.external.listTablesmanage_platform_settings
POST /:name/external/tables/:remote/draftdatasources.external.draftmanage_platform_settings
POST /:name/external/tables/:remote/importdatasources.external.importmanage_metadata
POST /:name/external/refresh-catalogdatasources.external.refreshCatalogmanage_metadata
POST /:name/external/validatedatasources.external.validate(unchanged — authentication only; see below)

One resolution, two decisions, anonymous first — the shape the declared admin twin took, for the same reason: two resolutions of one request can disagree, because the second is a fresh set of sys_* reads.

The refusal is 403 PERMISSION_DENIED, not the FORBIDDEN the sibling package-routes.ts emits. FORBIDDEN is a grandfathered pre-gate synonym under ADR-0112 D3's STANDARD_SYNONYM_WAIVERS, whose own schema says it "keeps a WIRE VALUE registered; it does not endorse the spelling for new code" — and PERMISSION_DENIED is what the admin twin answers, which the twin-equivalence suite now compares directly rather than checking each side against a literal.

isSystem is deliberately not a capability bypass: it is read for the anonymous decision only (shouldDenyAnonymous reads it), exactly as the admin twin does. Since isSystem is never resolved from inbound HTTP, an isSystem arm would be a second policy no wire caller could take — unfalsifiable divergence from the twin.

The known-gap case was folded back, not deleted

remote-tables-twin.equivalence.test.ts carried a case added by #9887 recording the divergence this ruling settles, with a block-header instruction to fold the row back into the agreement when the federation spelling grew its gate. That is what happened: the middle row of the admission axis (authenticated but UNENTITLED) is now asserted as an equivalence — both spellings' status and code compared to each other — alongside the three rows that already agreed. It keeps its own case rather than merging into the anonymous one, because 401 and 403 are different facts about a caller and a single "both refuse" case would be satisfied by either.

A second case was added there: a caller holding a real but irrelevant permission set (manage_org_users, a declared capability from the spec catalog). Without it, "unentitled" in that file only ever meant "holds no grant at all", and a gate asking merely whether the caller holds anything would pass every case.

POST /external/validate — named, not silently included

The ruling enumerates four routes. validate is the fifth: no admin twin, creates no metadata, and not one of the four. It keeps the #9686 authentication floor and says so through its own explicit 'authenticated' kind rather than inheriting a neighbour's gate — an un-ruled route quietly acquiring a capability is a change nobody decided. Pinned as such, and filed separately for triage rather than decided here.

Pin sweep (whole repo, one pass)

Changed — all assert the substance of the new semantics (unentitled ⇒ refused, asserting codeandstatus; entitled ⇒ still served):

  • external-datasource-routes-auth-guard.test.ts — five new cases: unentitled refused 403 PERMISSION_DENIED on all four ruled routes with the service never reached (so the refusal provably precedes the two writes); both directions of the read/write split (manage_platform_settings alone clears reads and is refused writes, and the reverse — only the crossed pair can tell "two capabilities" from "one capability spelled twice"); validate still served to a caller holding nothing; an unentitled api-key holder refused, so the gate cannot be read off key scopes instead of systemPermissions. The fixture engine grew sys_user_permission_set / sys_permission_set rows so grants come off the real resolution path.
  • remote-tables-twin.equivalence.test.ts — the fold-back described above.
  • external-datasource-envelope.conformance.test.ts — its CREDENTIALED stub now holds both capabilities. Its subject is the envelope of the success/400/503 arms; without the grants every case would silently become a reading of the 403 body.
  • discovery-advertised-direct-mounts.parity.test.tsmanage_platform_settings joins the stub's set for the same reason.

Found and deliberately not changed:

  • direct-mount-base-follows-apipath.test.ts — drives …/external/tables with no credential and expects 401. The anonymous floor is unchanged, so this pin is still true and still measures base placement. Kept verbatim — it is a refusal assertion for a genuinely illegal shape.
  • The three anonymous/bad-credential cases in external-datasource-routes-auth-guard.test.ts — likewise kept verbatim.
  • direct-mount-introspection.test.ts — compares the returned route array against the registration calls; never drives a handler.
  • packages/client/src/client.test.ts (datasources.external.* URL pins) and service-route-ledger-coverage.test.ts — mock fetch and assert the URL the SDK builds. The client is unchanged; a server-side gate is invisible to them.
  • packages/rest/src/rest-route-ledger.ts and service-datasource/src/datasource-route-ledger.ts — record route/family/disposition, carry no capability column. (rest-route-ledger.ts is also under the serial fence for in-flight fix(rest,client): the /meta FSM state route is singular — retire the plural registration (#10077) #10177 and was not touched.)
  • packages/cli/src/commands/datasource/{list-tables,introspect,validate}.ts — raw fetch callers that surface whatever the server answers; behaviour changes for unentitled operators, no code does.
  • scripts/check-route-envelope.mjs — its ratchet for this module is { responses: 0, ok: 0, err: 0 } and stays at zero: the new refusal goes through the shared sendError, so the module still builds no body itself.
  • packages/services/service-datasource/src/admin-routes.ts and its pins — the admin half already measured this in security(service-datasource): datasource-admin routes require a platform-settings capability beyond authentication #9593; this card converges the other spelling onto it and leaves it alone.

Published-surface note

datasources.external.* is SDK surface, so this is a behaviour change for existing credentialed callers. The changeset states the requirement plainly, per route, with the migration (grant the capability the route needs; admin_full_access already carries both).

⛔ Not for the merge queue

This card carries needs:contract-review (Clause-② yes — the ruling says so in as many words). Draft only; not ready, no auto-merge, label untouched. Green CI does not change that.

Verification

Gates were run at f83ec2cf (final commit; working tree clean, git status --porcelain empty). Ablation legs and their rebuild reasoning are in the dev report on #9901.


Generated by Claude Code

…apability
Read routes require manage_platform_settings (converging with the #9593
admin twin); the two write routes require manage_metadata.
Refs #9901
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 21 documentable anchor(s).

21 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 318f96ae5cc25d04a2650a6ef6d4c2b930619b96.

3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 318f96ae5cc25d04a2650a6ef6d4c2b930619b96packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5d50dfb4918acc571ee273fdb0b380683d50535e — the merge of head f83ec2cff8b022274e7956e7dd301ee63bad470a into base 318f96ae5cc25d04a2650a6ef6d4c2b930619b96, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5d50dfb4918acc571ee273fdb0b380683d50535e && git checkout 5d50dfb4918acc571ee273fdb0b380683d50535e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 318f96ae5cc25d04a2650a6ef6d4c2b930619b96 f83ec2cff8b022274e7956e7dd301ee63bad470a && git checkout -B drift-repro 318f96ae5cc25d04a2650a6ef6d4c2b930619b96 && git merge --no-ff f83ec2cff8b022274e7956e7dd301ee63bad470a
node scripts/docs-audit/affected-docs.mjs --json 318f96ae5cc25d04a2650a6ef6d4c2b930619b96

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 318f96ae5cc25d04a2650a6ef6d4c2b930619b96 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-elon@claude