Skip to content

fix(service-datasource): the admin routes derive the tenancy posture — an ex-member's org-stamped API key is no longer admitted - #16011

Queued
zhuangjianguo wants to merge 5 commits into
mainfrom
claude/issue-15350-datasource-tenancy-posture
Queued

fix(service-datasource): the admin routes derive the tenancy posture — an ex-member's org-stamped API key is no longer admitted#16011
zhuangjianguo wants to merge 5 commits into
mainfrom
claude/issue-15350-datasource-tenancy-posture

Conversation

@zhuangjianguo

Copy link
Copy Markdown
Collaborator

Fixes #15350

The hole

packages/services/service-datasource/src/admin-routes.ts resolved every request with resolveAuthzContext({ ql, headers, getSession }) and supplied no tenancyPosture. Both posture-conditional API-key refusals are gated on the caller supplying one:

  • organization_requiredpackages/core/src/security/api-key.ts, if (!tenantId && tenancyPosture)
  • organization_membership_endedpackages/core/src/security/resolve-authz-context.ts, if (keyPrincipal?.tenantId && input.tenancyPosture)

These are real request headers on the datasource ADMIN routes, so x-api-key is accepted there. A key's tenantId is sys_api_key.active_organization_id copied verbatim — the caller's own stored claim, never vetted against current membership — so under a wall-enforcing posture a key stamped with an organization its owner had left was admitted, and the family then gated it on authz.systemPermissions alone.

Severity, stated as the card states it. This family gates on system capabilities rather than on organization-scoped rows, so the measured consequence is an admitted principal — an ex-member whose platform grants outlive their membership creates, patches and deletes this deployment's datasources — not a cross-organization row read. Less severe than #15256's data door. Not correct either.

The fix

One local resolver, resolveAdmissionTenancyPosture, reading the tenancy service off the kernel's async accessor and classifying the result per #13906 decision 1 option A:

The throw is raised inside requireDatasourceAdmin's own try, so it takes the relay that block already runs for the identical fault one seam over (isAuthzStoreUnavailableError(err) re-raises rather than laundering an outage into a denial, #13279). No new relay.

The async accessor is used because the brand exists only on that path: PluginContext.getService throws two unbranded plain Errors, so the synchronous read this file uses for objectql cannot tell the two facts apart. Its absence stays quiet, as on the two sibling seams — a KernelBase-shaped host exposes getKernel() but has no getServiceAsync, and dereferencing it would turn "this host shape has no async registry" into an outage.

No shared helper was extracted, deliberately: #15349 is in flight and #15351 / #15352 are queued on this same seam in other packages, so a helper extracted by one of the four collides with the other three. The extraction looks correct to do once, as its own card, after they land — noted in the report for the PM rather than done here.

Acceptance — #15365's shape, copied

src/__tests__/admin-routes-tenancy-posture-admission.test.ts, 29 cases on one registrar:

  • Controls in both directions, before any subject arm. A current member reads and writes; an anonymous caller is 401 and nothing lands; a key whose owner holds no permission set is 403 PERMISSION_DENIED. A door that authenticates nobody cannot "pass" here by refusing for the wrong reason.
  • The subject rows under isolated. The ex-member's stamped key and the organization-less key are 401 on list, create and delete; the refusal names its reason on the server side and never the credential.
  • Writes are read back FROM THE DATASOURCE STORE, never from the response body — the service double mutates a real array and the assertions count rows in it.
  • A permanently held wiring ablation. Its handle is the wiring this package shipped before this card: a PluginContext with no getKernel at all, which is byte-identically what the other suites here still mount. Ablated, the ex-member reads again (200), writes again (201, row lands) and deletes again (204, row gone), while the member and capability controls are unchanged.
  • Both halves of option A. Never registered resolves quietly and runs no refusal; registered-and-failed is never answered as an admission and never as a capability denial.
  • RBAC is symmetric across the three key principals — one shared permission set — so only the tenancy posture can separate the arms.
  • A real ObjectKernel carries the wiring fact, because the classification under measurement is the registry's and a double imitating both brands would be asserting about itself.

group was MEASURED, not assumed

The card records the group write path as never measured and forbids asserting it unaffected. Measured here, both directions:

posture ex-member's stamped key organization-less key
isolated 401 on read and write, nothing lands 401
group 401 on read and write, nothing lands admitted (200 / 201)
single admitted admitted

organization_membership_ended keys on postureEnforcesWall, which group satisfies, so the ex-member is repaired under group too. organization_required additionally requires !postureUsesUnionScope, which group fails, so an organization-less key stays admitted there by design — it already reads the union of its owner's organizations. Both rows are pinned so the asymmetry is a recorded decision rather than an accident.

One thing this PR measured and did NOT repair

The outage arm asserts the outage class — status in the set 500 or 503, never 200 and never 403 — rather than the digits. Measured on this tree, the family answers 500 INTERNAL_ERROR ("No response from handler"), not the 503 SERVICE_UNAVAILABLE the brand declares: requireDatasourceAdmin re-raises out of route handlers that have no catch, and the Hono adapter renders any escaped throw as a bare 500. That escape is pre-existing — the same catch has re-raised the identical brand for the ql permission-store outage since #13279 — so this PR does not open it and does not repair it. Filed separately as #15999. Pinning the class keeps this suite measuring the security property, so repairing the status later does not have to redden a security pin.

Tier

Clause-② re-derived from this diff rather than inherited, and it is no, on two measurements:

  • git diff origin/main...HEAD -- packages/spec/api-surface/ is empty (0 lines), and the diff adds and removes no export line.
  • Stronger: the built dist/index.d.ts of @objectstack/service-datasource is byte-identical with and without the change — sha256 15a4858494…, 109773 bytes both ways, compared by building the package, checking out origin/main's copy of the one changed source file, rebuilding, and diffing (restored byte-exact afterwards, blob a978a47f, and the restore leg rebuilt).

The change is closure-local code plus type-only imports inside an existing function. No published contract face moves; a runtime authorization decision does.

Verification

Measured on 42d89418d, on a fully built tree.

  • pnpm --filter @objectstack/service-datasource test32 files, 672 tests passed
  • pnpm --filter @objectstack/service-datasource typecheck — clean, and tsc --noEmit --listFiles confirms the new test file is in the program (1 hit), so that green really covers it
  • The 55 gate families scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack derives for this diff — all 55 exit 0, exit codes captured before any pipe. Two of them (check:dual-build-cjs-loads, check:type-check-debt) first answered PREREQUISITE NOT MET (exit 3, NOT MEASURED); the full package closure was built and both were then measured green.

Generated by Claude Code

…es admission seam

The datasource ADMIN routes accept real request headers, so `x-api-key` is
admitted there — but `requireDatasourceAdmin` called `resolveAuthzContext`
with no `tenancyPosture`, and both posture-conditional API-key refusals are
gated on the caller supplying one. Neither ran, so under a wall-enforcing
posture an API key stamped with an organization its owner has left was
admitted and the family then gated it on `authz.systemPermissions` alone.

The posture is classified per #13906 decision 1 option A: a `tenancy` service
that was NEVER REGISTERED is branded and resolves quietly to "no posture"; one
that was registered and FAILED to build raises `AuthzStoreUnavailableError`,
which `requireDatasourceAdmin`'s existing catch re-raises rather than
laundering into a denial (#13279).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…its permanent wiring ablation and the two option-A arms

29 cases on one registrar: controls in both directions (a current member reads
and writes, an anonymous caller is refused, an entitled-less key gets the
capability refusal), the ex-member and organization-less subject rows under
`isolated`, and a permanently-held ablation whose handle is the wiring this
package shipped before this card — a PluginContext with no `getKernel`.

Writes are read back FROM THE DATASOURCE STORE, never from the response body.
`group` is MEASURED rather than assumed: the ex-member is refused there too
(`postureEnforcesWall('group')`), while the organization-less key is admitted
(union scope).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ge class rather than its digits

Sized as #15365 was: a fail-closed bug fix, no accept set widens.

The outage arm now asserts the outage CLASS. Measured on this tree, the family
answers 500 INTERNAL_ERROR rather than the 503 SERVICE_UNAVAILABLE the brand
carries — a pre-existing relay gap this card did not open (the same escape has
existed for the `ql` permission-store outage since #13279) and does not repair.
Asserting the class keeps the pin on the security property.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…accessor

`tsc --noEmit` reds on a hand-written `{ fetch: (req: Request) => Promise<Response> }`:
Hono's `fetch` takes an env and an execution context after the request, so the
narrower signature is a type error rather than a simplification.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added the size/l label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 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 2da2901efa2708d8895e436867838cbeeda191cbpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 51c0f020c6e918ab19ef37c999b7d64e767f2fc9 — the merge of head 2e7238dbc72124b3b73b433832e6922b06cc9832 into base 2da2901efa2708d8895e436867838cbeeda191cb, 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 51c0f020c6e918ab19ef37c999b7d64e767f2fc9 && git checkout 51c0f020c6e918ab19ef37c999b7d64e767f2fc9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2da2901efa2708d8895e436867838cbeeda191cb 2e7238dbc72124b3b73b433832e6922b06cc9832 && git checkout -B drift-repro 2da2901efa2708d8895e436867838cbeeda191cb && git merge --no-ff 2e7238dbc72124b3b73b433832e6922b06cc9832

node scripts/docs-audit/affected-docs.mjs --json 2da2901efa2708d8895e436867838cbeeda191cb

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

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Standing down on the red Lint & Repo Gates: not this PR's — the fifth PR today hit by the gate defect filed as #15990 / #15992.

check:merge-driverscripts/check-regen-pending.mjs --self-test, job 101355767791, on the current head 42d89418d.

Mechanism, measured (and my earlier summary of it was wrong — corrected on #15978/#15998): the launcher does not fail to resolve. .github/actions/setup-pnpm materialises only the pinned pnpm 10.31.0 and writes no lastKnownGood.json; the fixture's tmpdir pins nothing, so Corepack resolves the registry's latest dist-tag — now pnpm 12.3.4, whose Rust CLI rejects -s with exit 2. Every stub then reads as "gate exited non-zero" ⇒ stale, reddening exactly the cases expecting a non-stale verdict. Corepack writes a sticky lastKnownGood.json, and actions/cache@v6 saves COREPACK_HOME back, so a job restores a store holding either a -s-accepting pnpm or a pnpm 12.

Not tree-dependent, and the cleanest proof is on main itself: commit f7db8f4fd carries two Lint & Repo Gates runs — one failure, one success. Same tree, opposite verdicts. origin/main's own push build (run 33981169123) concluded failure at 17:51Z.

Severity bound: false-RED only, never false-green — a store-poisoned run makes every artifact look stale, and stale is the refusing verdict.

Fix status: PR #16002 (from the domain:spec seat, pinning the fixture to the root's packageManager) is queued and is the one landing. PR #16009 exists from this seat and is being held deliberately — ⛔ two edits of the same function racing into the queue is worse than either; once #16002 lands, #16009 is re-scoped as a delta over it or closed. Hardening of the cache channel itself is #16012.

Nothing to port into this PR before #16002 lands.

⛔ No test skipped, disabled or quarantined; ⛔ no empty commit. Spending this PR's one permitted re-run, which is unspent. ⚠️ All four earlier PRs' re-runs reproduced, so a pass here would be luck, not a verdict — if it reproduces, this PR waits on #16002.


Generated by Claude Code

…y-posture

Picks up #16002, which repairs the check:merge-driver / check-regen-pending.mjs
--self-test failure that reddened this PR, origin/main's own push build and five
other PRs. Not a change to this card's diff.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 5, 2026 20:32
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 5, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

service-datasource: the admin routes supply no tenancyPosture to resolveAuthzContext — an ex-member's org-stamped API key is admitted

2 participants