Skip to content

fix(service-storage): derive the tenancy posture at the file-read admission seam - #16017

Draft
zhuangjianguo wants to merge 3 commits into
mainfrom
claude/issue-15352-storage-tenancy-posture
Draft

fix(service-storage): derive the tenancy posture at the file-read admission seam#16017
zhuangjianguo wants to merge 3 commits into
mainfrom
claude/issue-15352-storage-tenancy-posture

Conversation

@zhuangjianguo

Copy link
Copy Markdown
Collaborator

Fixes #15352

buildFileReadAuthorizer (packages/services/service-storage/src/storage-service-plugin.ts) resolved every gated download with resolveAuthzContext({ ql: engine, headers, getSession }) and supplied no tenancyPosture. Both posture-conditional API-key refusals are gated on the CALLER supplying one — organization_required (core/security/api-key.ts, if (!tenantId && tenancyPosture)) and organization_membership_ended (core/security/resolve-authz-context.ts, if (keyPrincipal?.tenantId && input.tenancyPosture)) — so at this door neither ran. Its headers are the real request's (toWebHeaders), so x-api-key is accepted, and an API key's tenant is sys_api_key.active_organization_id copied verbatim: the caller's own stored claim, never vetted against current membership. Under a wall-enforcing posture a key stamped with an organization its owner had LEFT resolved a userId here and was then judged by the ownership and record-reachability checks — checks evaluated for a principal the wall should have refused at the door.

The posture is now read off the kernel's tenancy service, per download, and classified rather than swallowed (#13906 decision 1 option A): never registered ⇒ branded ⇒ quiet undefined; registered and unable to answer ⇒ AuthzStoreUnavailableError. Raised inside the authorizer's own try, so it takes the #13279 relay already there rather than adding a second net.

Measured, on this tree

Driven through the real routes (GET /api/v1/storage/files/:fileId/url and its 302 sibling GET /api/v1/storage/files/:fileId), booted from a real StorageServicePlugin, with a real ObjectKernel carrying the wiring fact. This door performs no write, so the second fact — the analogue of the sibling cards' "read the write back from the store" — is read off the storage ADAPTER: every arm asserts whether getPresignedDownload minted a download capability at all. A door that answered 401 after minting a URL would pass a status-only suite and still have issued the capability.

wiring ex-member's org-stamped key organization-less key current member (control) no credential (control) unreachable parent (control)
isolated 401, 0 minted 401, 0 minted 200 + URL, 1 minted 401 403 FILE_DOWNLOAD_DENIED
group 401, 0 minted 200 + URL, 1 minted (union scope, by design) 200 401 403
single 200 200 200 401 403
tenancy never registered 200 200 200 401 403
ABLATION — no getKernel 200 + live signed URL, 1 minted 200 200 401 403

The card's body says two things that are false on the current tree, both re-measured here rather than repeated: the census of resolveAuthzContext callers supplying a posture is 4 of 8 on origin/main (rest-server.ts, runtime/security/resolve-execution-context.ts, mcp/src/plugin.ts, cloud-connection/src/marketplace-install-local-plugin.ts), not 2; and group is not unaffected — the ex-member refusal fires there exactly as under isolated, because organization_membership_ended keys on postureEnforcesWall, which group satisfies. The organization-less row deliberately differs: organization_required additionally requires NOT postureUsesUnionScope, which group fails. Both directions are pinned.

The ablation is held permanently

Its handle is the wiring this package shipped before this card: a PluginContext with no getKernel at all, which is byte-identically what every other suite here still mounts (storage-service-plugin.test.ts's makeCtx). It removes exactly the one input this change added, and with it removed the ex-member downloads again — status and a freshly minted signed URL. A pin that cannot go red has measured nothing.

The outage arm, MEASURED rather than assumed — and it is NOT 503

⚠️ On this door a tenancy service that was REGISTERED and FAILED to build renders 403 FILE_DOWNLOAD_DENIED, not the 503 SERVICE_UNAVAILABLE the AuthzStoreUnavailableError brand declares, and not the bare 500 measured next door on service-datasource. Measured on this branch, all three principals, both download routes, 0 capabilities minted.

The mechanism is one frame ABOVE the authorizer and is pre-existing. The authorizer does re-raise the brand (isAuthzStoreUnavailableError(err) ⇒ throw, #13279), but registerStorageRoutes' authorizeDownload wraps the whole call in catch { verdict = 'deny' }, so the re-raise is absorbed and rendered as the gate's own refusal. That catch has swallowed the #13279 permission-store outage at this door since that card landed; this change adds a second producer of the identical brand at the identical seam, it does not create the path. ⛔ Not repaired here.

This is the datum #15999 explicitly asks for: that card names storage-service-plugin.ts:921 in its census of six re-raise sites and records the other five as UNMEASURED. The measurement is posted there. It also bears on that card's "two directions" question: for this door neither repair direction is sufficient on its own, because the throw never escapes the handler in the first place.

Fail-closed either way, which is why the pin asserts the outage CLASS ([403, 500, 503], never 200, never 302, never a minted capability) rather than the digits — a later status repair must not have to redden a security test.

Clause-2: no — re-derived, not inherited

The claim comment's no was explicitly non-binding. Re-derived with the instrument this family settled on, because api-surface/ artifacts exist only for packages/spec and prove nothing about a service package: build the package, swap in the base commit's copy of the one changed source file, rebuild, compare the built dist/index.d.ts.

BASE                     f7db8f4fd268a86a08c62ae4894cf7417720f8c9
source blob WITH         b08838c3a2b5130fcc3f65898f2aeed53d25bf07
source blob WITHOUT      b1d817d3b3e9152139a9a0faecd1e92af186e27d   (differ -- positive control)
mutation on disk         resolveAdmissionTenancyPosture 3 -> 0 occurrences
                         'getSession, tenancyPosture'   1 -> 0 occurrences
                         hash-object == BASE blob

dist/index.d.ts WITH     sha256 c4fb9b3dd402b03cf542bb5e27429f47caa60c9e5c3bf192da11571e1f05045c   500960 bytes
dist/index.d.ts WITHOUT  sha256 c4fb9b3dd402b03cf542bb5e27429f47caa60c9e5c3bf192da11571e1f05045c   500960 bytes
                         => IDENTICAL

restore proved           hash-object == HEAD blob, `git diff HEAD` 0 lines,
                         marker counts back to 3 / 1, leg-3 rebuild reproduces leg 1

The other limb is unchanged too: the diff touches no packages/spec/src/**, no Zod schema and no error-code ledger. No accept set widens; a declared guard returns to enforced. Clause-2: no.

Verification

Everything below was run at final commit bf004da92.

pnpm --filter @objectstack/service-storage test        36 files / 562 tests passed
pnpm --filter @objectstack/service-storage typecheck   exit 0 (tsc + scripts + test-layer, 0 errors)
pnpm lint                                              exit 0 -- the REPO-WIDE run, not a narrowing

Derived gate family via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (which reads its own change set from the merge base; 2 paths, 263 families discovered). Run locally, all exit 0:

check:authz-resolver           check:engine-double-contract   check:where-matcher
check:objectql-double-limit    check:nul-bytes                check:cross-package-test-inputs
check:test-source-alias        check:slot-lookup              check:error-status-conformance
check:tenant-chokepoint        check:changeset-gate-self-tests check:objectui-changeset
check:single-claim-paths       check:doc-authoring            check:auth-mount-ledger

The ratchet-bearing subset was re-run at bf004da92 after the final commit, not before it. The rest of the farm is CI's.

⚠️ Lint & Repo Gates may red on check:merge-driver with ✗ self-test failed -- 1 failure(s) (cases and floor). That is #15990 (a Corepack store holding an unpinned pnpm), not this change; fix open as PR #16009, hardening as #16012.

Deliberately not done

⛔ No shared helper extracted. resolveAdmissionTenancyPosture is a local copy, as mcp and cloud-connection each wrote one, because sibling cards (#15349, #15350, #15351) are live on this same seam in other packages and an extraction by one collides with the rest. The docblock's justification for keeping the async accessor's ABSENCE quiet is written for THIS door — this gate already returns undefined (downloads ungated) when the auth service or the engine is missing, so degrading through an incompletely composed host is its declared contract — rather than copied from a sibling whose reason is not true here. Family-wide KernelBase/LiteKernel behaviour is ruled and filed as #15997.

🤖 Generated with Claude Code


Generated by Claude Code

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 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, 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 — 6 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 f7db8f4fd268a86a08c62ae4894cf7417720f8c9packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json f7db8f4fd268a86a08c62ae4894cf7417720f8c9

⚠️ 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 — and the fix is already on main, so a merge is in flight rather than a re-run.

check:merge-driverscripts/check-regen-pending.mjs --self-test, job 101360648238, on head bf004da92. Same case set and the same ✗ self-test failed -- 1 failure(s) (cases and floor) summary seen on five other PRs today.

Why it is not this PR's: the self-test's fixture wrote a stub package.json into a tmpdir carrying no packageManager, so Corepack resolved the registry's latest — pnpm 12.3.4, whose Rust CLI rejects -s with exit 2. Every stub then read as "gate exited non-zero" ⇒ stale, reddening exactly the cases expecting a non-stale verdict. Corepack wrote a sticky lastKnownGood.json and actions/cache@v6 saved COREPACK_HOME back, so jobs restored a store holding either a -s-accepting pnpm or a pnpm 12. origin/main's own push build went red on it (run 33981169123, 17:51Z), and commit f7db8f4fd carries two Lint & Repo Gates runs — one failure, one success, same tree.

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

Fix landed: #16002 merged as f50c394da ("pin the regen self-test fixture to the root's packageManager"). Verified by content on origin/main, not the API's merged field — packageManager now matches in scripts/check-regen-pending.mjs, with fixtureSelfTest firing as a control on the same command and scope.

⛔ This PR's one permitted re-run is NOT being spent, deliberately. A re-run replays the original merge ref against the old base, so it would re-measure the same poisoned condition — and 5 of 5 re-runs on the other PRs reproduced. With the fix on main, the CI rule's own preferred path applies: "port the same change into this PR now and push; it no-ops once the base carries it." The base carries it, so the port is a main merge, and that is in progress.

⛔ No test skipped, disabled or quarantined; ⛔ no empty commit; ⛔ no registry widened.


Generated by Claude Code

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

2 participants