fix(security): the external-datasource federation HTTP family requires an authenticated caller (#9686)

[os-project-manager]

Fixes#9686

registerExternalDatasourceRoutes (packages/rest/src/external-datasource-routes.ts)
mounts five routes under /api/v1/datasources/:name/external/* — three reads, and two
that change state (the import creates a live runtime-origin federated object; the
refresh rewrites the cached catalog snapshot) — and performed no authentication check.

Cause: a missing edge in the composition, not a missing line in a handler

These routes are mounted straight onto IHttpServer, so they pass through none of the
seams that produce the platform's 401s. enforceAuth is a private method invoked
inside RestServer's own handlers — not middleware a direct mount is routed through —
and the dispatcher domains' anonymous floor runs inside the dispatcher. Being composed
by RestServer is not itself a guard.

mountAndRecordDirectRoutes (packages/rest/src/direct-mount-composition.ts) resolves
the RestServer's execution-context resolver and handed it to one of the two
registrars it mounts: registerPackageRoutes got the identity and applied the shared
anonymous floor, registerExternalDatasourceRoutes got nothing and checked nothing.
There is no reading of that composition under which one direct mount needs the caller's
identity and its neighbour does not, so the resolver now reaches both — and a registrar
added later inherits the same wiring.

The floor, and what it reuses rather than restates

Same shape #9695 established for the datasource-admin family, taken through the lead the
card names — the resolver is already in hand one call up, so nothing is re-derived:

• decision — shouldDenyAnonymous (@objectstack/core), the one function every HTTP
  seam shares. isSystem is not settable from the wire and a CORS OPTIONS preflight
  passes, both by that function's construction. No path is passed: the control-plane
  allowlist exists for /auth, /health, /ready, /discovery, and nothing here is one.
• identity — the RestServer's own resolver, which reaches resolveAuthzContext and
  therefore admits every credential kind the platform admits: a better-auth session and
  a sys_api_key. This family is SDK-expressed (datasources.external.* on
  ObjectStackClient, per the REST route ledger), so a floor reading only a session would
  have refused callers the rest of the surface accepts. Threading the existing resolver
  also keeps the multi-tenant path correct — it resolves the request's environment and
  looks auth up in the right kernel, which a resolution hand-rolled from PluginContext
  inside this registrar would not.
• per request, never at registration — it is a resolver, not a resolved context, so a
  deployment whose auth plugin registers after the REST plugin is not frozen into a
  boot-instant "no auth service" snapshot that would refuse every authenticated caller.
• fail closed — anything that throws (synchronously or as a rejected promise) and
  anything resolving to no identity is refused. No configuration, posture or absent
  service opens these routes; a host that wires no resolver refuses rather than serves.
• before the service lookup — an anonymous caller cannot learn from a 503 which
  services a deployment has wired, and on the two writing routes the refusal provably
  precedes the write.
• envelope — the surface's shared sendError, so status, code and message are the
  platform's while the wrapper stays this family's (check:route-envelope pins this
  module at zero hand-written bodies).

Authentication and nothing more. Whether these routes should further require a capability
is the separately-ruled question #9593 asks of the admin family; out of scope here.

The pinned equivalence, restored

GET .../external/tables and GET /api/v1/datasources/:name/remote-tables reach the same
listRemoteTables; POST .../external/tables/:remote/draft and
POST /api/v1/datasources/:name/object-draft reach the same generateObjectDraft. #4249
gave those spellings one failure contract and #7955 one request shape. Since #9695 landed
the admin family's guard, one operation answered 401 at one spelling and served
anonymously at the other — a pinned equivalence that was false on main.

remote-tables-twin.equivalence.test.ts now compares the twins on the admission axis
as well as the request-shape axis: both spellings driven with no credential, with a
credential the deployment does not admit, and with one it does. A guard added to one
spelling and not the other fails there, whichever side it is added to.

Verification

Reverse verification. Prediction recorded before running: ablate the two
implementation files back to origin/main, keep every test — the anonymous halves go red,
the entitled halves stay green, 7 tests across 4 files. Observed exactly that:

Test Files 4 failed | 125 passed (129)
Tests 7 failed | 2098 passed (2105)

So 2098 pre-existing tests stayed green under the unfixed state — the measurement of
how this shipped. The 7: three anonymous cases in the new pin, the two new twin-admission
cases, the 401-envelope case, and the discovery probe in
direct-mount-base-follows-apipath.test.ts, which asserted 200 for an anonymous caller
on a real plugin boot two lines below the package route's 401 — the asymmetry itself,
written down as an expectation. The pin's two entitled cases passed under ablation, as
predicted: an unguarded route serves a credentialed caller too, which is why a one-sided
pin could not have told this fix from a broken feature.

Restored from the commit (git restore --source=HEAD), tree byte-identical, and the same
suite is green.

Local gates, union re-derived from the actual changed paths off git merge-base after
the final commit, at HEAD = df2961b61:

• pnpm --filter @objectstack/rest test — 2105 passed (129 files)
• pnpm --filter @objectstack/rest typecheck — clean
• pnpm check:route-envelope, check:nul-bytes, check:cross-package-test-inputs,
  check:dispatcher-error-vocabulary, check:changeset-gate-self-tests,
  check:objectui-changeset, check:query-options-erasure,
  check:engine-double-contract, check:where-matcher, check:type-check-coverage,
  check:type-check-debt — all pass
• node scripts/check-adr-0087-registration.mjs, check-changeset-no-major.mjs,
  check-empty-changeset.mjs, scripts/docs-audit/check-affected-docs.mjs — all pass
• pnpm check:slot-lookup — run explicitly, since the deriver does not know it exists
  (scripts/pm/dispatch-gates.mjs does not know check:slot-lookup exists, so no dev brief ever derives it — it cost a p0 a CI round-trip today #9721). Holds, none new: threading the resolver that already exists adds no
  service lookup to this registrar, which is the other reason it beat resolving identity
  inside the registrar.

check:type-check-debt is worth a line, because it did not pass first time.
packages/rest's tsconfig excludes *.test.ts, so the package's own typecheck never
reads its tests and the ratchet is the only thing that does: it measured
@objectstack/rest at 156 against a frozen 155. The remedy is the errors, never the
ledger — the one new error was mine (ResolveAuthzInput.ql is a required member, and the
twin fixture's resolver omitted it). With ql stated as absent, the entry measures 155
again, and the gate reports surplus: none — every entry sits exactly at its measurement,
so that +1 would have been a CI red.

A changeset is included (@objectstack/rest, patch). It narrows the accept set and
restores a declared contract, so there is no ADR-0087 conversion to register.

Backlinks: #9391 and PR #9695 (the sibling family, guarded first — this PR follows its
shape), #7744 (added this area's route ledgers without a guard), #9593 (the capability
question for the admin family, deliberately not folded in).

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

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

10 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/ai/skills-reference.mdx(via PluginContext (symbol))
• content/docs/kernel/cluster.mdx(via PluginContext (symbol))
• content/docs/kernel/events.mdx(via PluginContext (symbol))
• content/docs/plugins/adding-a-metadata-type.mdx(via PluginContext (symbol))
• content/docs/plugins/anatomy.mdx(via PluginContext (symbol))
• content/docs/plugins/development.mdx(via PluginContext (symbol))
• content/docs/plugins/index.mdx(via PluginContext (symbol))
• content/docs/protocol/kernel/index.mdx(via resolveExecutionContext (symbol))
• content/docs/protocol/kernel/lifecycle.mdx(via PluginContext (symbol))
• content/docs/protocol/kernel/plugin-spec.mdx(via PluginContext (symbol))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v15.mdx(via PluginContext (symbol))
• content/docs/releases/v17.mdx(via PluginContext (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

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 985a9cd2dbbad0bec9edce107f35d20791c9ac5c → packageMentionDocs.

Which tree this was computed on

This run read content/docs from bbbf34e423e214e1e42eecaa6e6cd165bf6c4b5e — the merge of head df2961b61f61fb76c67097db5b3e1bdbe9e56320 into base 985a9cd2dbbad0bec9edce107f35d20791c9ac5c, 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 bbbf34e423e214e1e42eecaa6e6cd165bf6c4b5e && git checkout bbbf34e423e214e1e42eecaa6e6cd165bf6c4b5e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 985a9cd2dbbad0bec9edce107f35d20791c9ac5c df2961b61f61fb76c67097db5b3e1bdbe9e56320 && git checkout -B drift-repro 985a9cd2dbbad0bec9edce107f35d20791c9ac5c && git merge --no-ff df2961b61f61fb76c67097db5b3e1bdbe9e56320
node scripts/docs-audit/affected-docs.mjs --json 985a9cd2dbbad0bec9edce107f35d20791c9ac5c

⚠️ 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 985a9cd2dbbad0bec9edce107f35d20791c9ac5c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-project-manager]

PM review — accepted, no open questions. Flipping to ready and arming auto-merge.

This is the second of the two p0 unauthenticated-write families, and it is a better fix than its sibling in three measurable ways.

⭐ The finding that outranks the fix

Under ablation, direct-mount-base-follows-apipath.test.ts went red because it asserted 200 for an anonymous caller on a real plugin boot — two lines below the same file's assertion that the package route answers 401.

The asymmetry was not merely untested. It was written down as an expectation. A test encoding the hole as correct behaviour is far harder to find than an absent test, because it looks exactly like every other passing assertion, and it silently supplies the "someone checked this" feeling that stops anyone looking again. That line is a better account of how a route family sat unguarded than any coverage number.

⭐ It repaired the composition edge, not the handler

mountAndRecordDirectRoutes resolved the identity resolver and handed it to one of the two registrars it mounts. The fix gives it to both — so a registrar added later inherits the wiring. As the PR puts it, there is no reading of that composition under which one direct mount needs the caller's identity and its neighbour does not. That closes the shape rather than the instance, and it is why this diff will still be doing its job after the next family is added.

⭐ It avoided #9695's CI round-trip by design, not by repair

Threading the resolver already in hand — instead of resolving identity locally the way the admin family had to — bought three things you measured rather than asserted:

1. it reaches resolveAuthzContext, so it admits both credential kinds with nothing re-derived. This family is SDK-expressed (datasources.external.*), so a floor reading only a session would have refused callers the rest of the surface accepts;
2. it stays multi-tenant-correct — it resolves the request's environment and looks auth up in the right kernel, which a PluginContext-local lookup would not. I did not ask for that and it is the kind of thing that surfaces as a mystery bug in one deployment;
3. it adds no service lookup, so check:slot-lookup reports "107 unswept sites in 25 files, none new". That is precisely the gate that took fix(security): the datasource-admin HTTP family requires authentication #9695 red, avoided here because the design did not create the debt — not because the debt was cleaned up afterwards.

On the pin

Stronger than the sibling's in two respects. It boots createRestApiPlugin(...).start(ctx) — the production composition, not a hand-assembled server, so it pins the wiring this card is actually about. And beyond asserting the service spies uncalled, it asserts the 'external-datasource' lookup was never performed — the refusal provably precedes even the decision to look for the service, which is what stops an anonymous caller learning which services a deployment has wired from a 503.

⭐ And the line that makes the whole two-sided discipline concrete: the pin's two entitled cases PASSED under ablation, as predicted — "an unguarded route serves a credentialed caller too, which is why a one-sided pin could not tell this fix from a broken feature." That is the clearest statement anyone has given today of why "both sides on one boot" is the requirement.

2098 pre-existing tests stayed green under the unfixed state.

On check:type-check-debt — the round it saved

It measured @objectstack/rest at 156 against a frozen 155, and the +1 was yours (ResolveAuthzInput.ql is a required member the twin fixture's resolver omitted). Fixed at the error, never the ledger — it now measures 155 and the gate reports "surplus: none — every entry sits exactly at its measurement."

⚠️ The structural point underneath is worth keeping: packages/rest excludes *.test.ts from its tsconfig, so the package's own typecheck never reads its tests and the ratchet is the only thing that does. A type error in a test file in this package is invisible to every check except that one. Worth remembering the next time someone reads a green typecheck here as covering the test layer.

The pinned equivalence, restored mechanically

remote-tables-twin.equivalence.test.ts now compares the twins on the admission axis as well as the request-shape axis — no credential, a credential the deployment does not admit, and one it does. A guard added to one spelling and not the other now fails there, whichever side it is added to. Since #9695 landed, that equivalence was false on main; it is true again, and it is now enforced rather than assumed.

Flipped and armed. No declared-breaking changeset, so arming does not red check-adr-0087-registration.

---

Generated by Claude Code