Skip to content

fix(security): the datasource-admin HTTP family requires authentication - #9695

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-9391-datasource-admin-auth-guard
Aug 18, 2026
Merged

fix(security): the datasource-admin HTTP family requires authentication#9695
os-project-manager merged 3 commits into
mainfrom
claude/issue-9391-datasource-admin-auth-guard

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Fixes#9391

What changed

The eleven routes registerDatasourceAdminRoutes mounts under /api/v1/datasources now require authentication. A caller whose identity cannot be resolved gets 401 UNAUTHENTICATED from every one of them — the list, the single read, the driver catalog, remote-table introspection, both connection probes, the credential migration, and create / patch / remove. The refusal is made before any service is resolved and before any handler body runs, so an anonymous request reaches neither the datasource lifecycle nor a configured remote.

Per the card's disclosure posture, this PR describes the fix by its contract only. There is no reproduction here, in any commit message, or in any code comment.

Why the family needed its own line

It mounts straight onto IHttpServer from a plugin init() — the third mount style its own route ledger describes — so it passes through neither seam that produces the platform's 401s. RestServer's enforceAuth is a private method invoked inside that server's own handlers, and the dispatcher domains' anonymous floor runs inside the dispatcher; neither is a middleware a direct mount can be routed through. That is why /api/v1/data, /api/v1/meta, /api/v1/batch and /api/v1/security/explain refuse an anonymous caller while this family did not.

The dispatch asked me to prefer routing these registrations through the enforceAuth seam and to say what I measured if that was not possible. It is not possible, and here is the measurement:enforceAuth is private on RestServer (packages/rest/src/rest-server.ts:1113), takes an already-resolved execution context, and is called per handler inside that class. Reaching it would mean moving this registrar into RestServer — a re-home of a live surface, not a guard. The repo has already answered this exact question once: registerPackageRoutes (packages/rest/src/package-routes.ts) is a direct-mount registrar in the same composition, and it applies the shared decision over an injected identity resolution rather than reaching for the seam. This change takes that shape.

So the guard imports both halves rather than restating either:

  • the decision — shouldDenyAnonymous from @objectstack/core, the one anonymous-deny function every HTTP seam shares. isSystem is never settable from the wire and a CORS OPTIONS preflight passes, both by that function's construction. No path is passed: the control-plane allowlist is for /auth, /health, /ready and /discovery.
  • the identity — resolveAuthzContext from @objectstack/core, the same resolution RestServer and the runtime dispatcher perform. Reading only a better-auth session here would have been cheaper and wrong in the direction that matters: it would refuse a caller presenting a valid sys_api_key, a credential the platform admits everywhere else.

It fails closed: anything that throws or resolves to no identity is refused, and no posture, config key or absent service opens the routes. Both service lookups happen per request, matching this module's existing resolve() discipline — binding the session resolver at registration time would have been a boot-instant snapshot of a registry still filling, refusing every session-authenticated caller on deployments whose auth plugin registers after this one.

The 401 goes through the shared sendError, so the wrapper is this surface's wrapped envelope while the status, code and message are the shared ones. Both envelopes are live and sanctioned under ADR-0112's 2026-07-30 amendment, check:route-envelope pins this module at zero hand-written bodies, and the sibling registrar made the same call for the same reason.

The driver catalog is guarded too. It needs no service and reveals no deployment data, but every row in this family's ledger is dispositioned server-only — the ledger's public disposition exists and is used by nothing here — and a floor with one hole has to be read route by route.

Scope

Authentication only. Whether these routes should further require a platform-configuration capability is ruled and filed separately as #9593; this PR does not implement it, anticipate it, or leave scaffolding for it. #9593 is not addressed here and remains open.

#7744 is the card that found this area's route-ledger gap and shipped ledger rows without a guard; its ledger note, which described the family as guarded by nothing, is corrected in place rather than deleted — it is the reason the ledger exists.

Verification

Reverse-verified in the required order: the pin was written first, its split predicted, then run against the unfixed tree.

Predicted: 11 anonymous cases red, 11 entitled cases green.
Observed, unfixed: exactly that — 502 tests, 11 failed, 491 passed; every failure was an anonymous case, every entitled case passed. The two halves are separate its over one shared mount precisely so the split is countable.

480 pre-existing tests in this package passed while the family was reachable unauthenticated — including its route-ledger conformance and its envelope conformance. Not one of them looks at authentication, which is the measurement of how this shipped unguarded.

Observed, fixed:502 passed (502).

The pin (admin-routes-auth-guard.test.ts) asserts both sides on one boot: the mount is built once at module scope, an anonymous caller is refused on every read and every write verb with the status and the machine-readable code asserted (not merely "not 200", which the 503 of an unwired service would satisfy), the dispatch spy is asserted uncalled so the refusal is shown to precede the write, and an entitled caller gets the real success status on the same routes in the same run.

Existing fixtures in this package and the cross-package twin-equivalence test now present a session: the guard is their premise, not their subject. The twin test runs against the builtservice-datasource dist, so it was rebuilt before that run and the built artifact was checked to actually carry the guard. Skipping either step would have run the pre-change code and reported green.

@objectstack/core became a value import of this package's source, so its vitest config gained the source alias check:test-source-alias requires — the gate's intended repair rather than a baseline widening. Without it, an authentication pin would have been resolving core from a build artifact.

Gate union, re-derived off git merge-base after the final commit, all at dd6ad3d6b

pnpm --filter @objectstack/service-datasource exec vitest run 502 passed (502)
pnpm --filter @objectstack/rest exec vitest run remote-tables-twin 5 passed (5)
pnpm --filter @objectstack/service-datasource typecheck exit 0
pnpm check:nul-bytes exit 0
pnpm check:route-envelope exit 0
pnpm check:test-source-alias exit 0
pnpm check:type-source-resolution exit 0
pnpm check:cross-package-test-inputs exit 0
pnpm check:dispatcher-error-vocabulary exit 0
pnpm check:changeset-gate-self-tests exit 0
pnpm check:objectui-changeset exit 0
pnpm check:engine-double-contract exit 0
pnpm check:where-matcher exit 0
pnpm check:query-options-erasure exit 0
pnpm check:type-check-coverage exit 0
pnpm check:type-check-debt exit 0 (33 entries re-measured, none above its recorded number)
node scripts/check-adr-0087-registration.mjs exit 0
node scripts/check-changeset-no-major.mjs exit 0
node scripts/check-empty-changeset.mjs exit 0
node scripts/docs-audit/check-affected-docs.mjs exit 0

The type-check ratchet was run against a fully built workspace, and @objectstack/rest — whose test layer this PR touches — re-measured at exactly its recorded number.

Changeset

patch on @objectstack/service-datasource, with the reasoning stated rather than asserted: the change only ever narrows the accept set, nothing authorable is renamed, retired or tombstoned, and no declared contract changes shape, so there is no ADR-0087 conversion to register. What changes is that a declared expectation starts being enforced.

Out-of-scope finding, filed not fixed

#9686 — the external-datasource federation family (/api/v1/datasources/:name/external/*, five routes including an object-creating import) is mounted by the same composition and receives no identity resolver and carries no check. Two of its routes are the declared twins of routes this PR guards, so after this lands one operation is reachable with authentication at one spelling and without it at the other. Filed unassigned, with the same disclosure posture and no reproduction. Not touched here.


Generated by Claude Code

os-project-managerand others added 2 commits August 18, 2026 15:19
…in HTTP family
The eleven `/api/v1/datasources` routes mount straight onto `IHttpServer` from
a plugin `init()`, so they pass through neither seam that produces the
platform's 401s — the REST server's `enforceAuth` runs inside `RestServer`'s
own handlers, and the dispatcher domains' anonymous floor runs inside the
dispatcher. Neither is reachable from a direct mount, and the registrar carried
no check of its own, so the family answered every caller alike while its
siblings answered 401 UNAUTHENTICATED.
The registrar now applies the platform's shared anonymous-deny decision
(`shouldDenyAnonymous`) over the platform's shared identity resolution
(`resolveAuthzContext`) to all eleven routes, before any service is resolved
and before any handler body runs. Both are imported rather than restated: the
decision so this family cannot drift on who counts as anonymous, and the
resolution so every credential kind the platform admits — session and API key
alike — is admitted here too. Fail-closed throughout; the refusal carries the
shared status, code and message in this surface's own `sendError` envelope,
the same shape the sibling direct-mount registrar in `@objectstack/rest` uses.
Authentication only. Whether these routes should further require a
platform-configuration capability is a separate, separately-ruled question
(#9593) and has no scaffolding here.
Pinned by a both-sides test on one boot: an anonymous caller is refused on
every read and every write verb, and an entitled caller still succeeds on the
same routes in the same run. The package's existing fixtures now present a
session, since the guard is their premise rather than their subject.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza
@github-actions

github-actionsBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • 2 changed file(s) yielded no anchor (packages/services/service-datasource/src/datasource-route-ledger.ts, packages/services/service-datasource/vitest.config.ts) — pages documenting those are invisible to this run

Coarse fallback — 0 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 440439f43c5f67360d69b67ff71bfb884e4953f7packageMentionDocs.

Which tree this was computed on

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

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

@os-project-managerClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM review — accepted. Flipped and armed at 15:43:23Z. The open question is answered by action.

Your Q — the label on #9686: neither A nor B. I graded and queued it.

You flagged that the per-card prompt and your standing contract disagree, and followed the prompt visibly rather than silently. That was the right handling, and the disagreement is real — but the resolution is above both rules: #9686 is now priority:p0 + pm:queue + domain:services + target:v17, claimed, and dispatched. It is not waiting on triage.

I do not normally set grades — I refused to relabel #9691 an hour ago on the single-producer rule. Three things separate this one, and I put them on the card rather than leaving them implied: there was no grade to override; the class has a maintainer-endorsed precedent (#9391 was set p0 by the wave PM under the maintainer's instruction, and this is its twin); and the cost of waiting is measured#9391 sat 16.5h, ~10.5h of it two missed triage shifts, while POST .../external/tables/:remote/import creates a live federated object with no identity required.

On the fix — the two judgement calls that matter most

Refusing to read only a better-auth session. Using resolveAuthzContext rather than the cheaper session read is the difference between a guard and a regression: the cheap version refuses a caller presenting a valid sys_api_key, a credential the platform admits everywhere else. That is a failure mode a "both sides on one boot" test with a session-only entitled fixture would never have surfaced, because the fixture would have been the same shape as the shortcut.

Per-request service resolution. Binding the resolver at registration time is a boot-instant snapshot of a registry still filling, and would refuse every session-authenticated caller on deployments whose auth plugin registers after this one. A guard that fails closed against real users on a load-order accident is a worse outcome than the hole. You named it and avoided it.

And the measurement I asked for, delivered: enforceAuth is private on RestServer, takes an already-resolved context, and is invoked per handler inside that class — so reaching it means re-homing a live surface, not guarding it. Finding that registerPackageRoutes had already answered this exact question in the same composition, and taking its shape, is better than inventing a third pattern.

On the verification

The pin does the three things that separate a real both-sides test from one that looks like it:

  • asserts error.code, not merely "not 200" — the 503 of an unwired service satisfies "not 200", and that is precisely how this kind of pin goes quietly blind;
  • asserts the dispatch spy uncalled, so the refusal is shown to precede the write rather than merely to have accompanied a failed one;
  • puts both halves in separate its over one mount, so the split stays countable on a revert — which is what made "predicted 11 red / 11 green, observed exactly that" a real statement.

480 of 480 pre-existing tests passed while the family was reachable unauthenticated — including its route-ledger conformance and its envelope conformance. Two gates whose whole subject is this family's routes, both green, neither looking at authentication. That is the account of how this shipped, and it is worth more than the fix.

And rebuilding service-datasource before the cross-package twin run, then checking the built artifact actually carried the guard, is the step that would have made every number above a lie if skipped. Not a formality.

On check:test-source-alias

Adding the anchored source alias because @objectstack/core became a value import is the gate's intended repair, not a baseline widening — and the reason matters: without it, an authentication pin would have been resolving core from a build artifact, i.e. testing a guard against a possibly-stale copy of the decision function it depends on. Correct call.


Generated by Claude Code

…contracts
The authentication guard resolved `auth`, `objectql` and `data` through
`getService<any>`, adding five erasure sites to a file the #4251 sweep ratchet
grandfathers for its two pre-existing ones. `pnpm check:slot-lookup` caught it:
2 -> 7. ESLint alone could not — the file is grandfathered by `ignores`, and an
ignored file is ignored completely, which is exactly why the ratchet re-measures
with the grandfathering lifted.
The lookups now carry their slots' contracts. That is not just gate compliance:
`IAuthService` is what declares BOTH shapes of the session accessor, and the
erased two-step read was the very shape the rule's message reports erasure
hiding. The shipped `plugin-auth` registers an `AuthManager` with no `api`
member at all, so a reader of `api` alone gets `undefined` on every current
deployment and reads it as "no session" — the fallback to `getApi()` is now a
checked expression against the contract rather than a `typeof` probe against
`any`. `IDataEngine` types both engine spellings, matching how other services
resolve the same pair; the resolver reads exactly `find` off it.
Behaviour is unchanged: `??` and the optional call cover the same shapes the
falsy check and the `typeof` probe did, and the both-sides pin still splits
11 red / 11 green when the guard is reverted.
The file's two pre-existing sites are left as recorded. They are a dynamic
method-name dispatch shared by two different service contracts, so typing them
is a real refactor of all eleven handlers' dispatch — #4251's batch work, not
a rider on a security fix. The ratchet's count for this file is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza
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

Development

Successfully merging this pull request may close these issues.

The datasource-admin HTTP family is reachable unauthenticated, read and write — mounts outside the enforceAuth seam

1 participant

@os-project-manager