fix(service-storage): scope the sys_file / sys_upload_session update and delete doors to the acting organization (#13178)

[claude]

Fixes #13178

Repairs the four half-repairs the maintainer's 2026-08-30 ruling (第 5 场总监席决裁批 #7, verbatim 「批 #7 同意」) narrowed this card to: the update and delete siblings of the two inserts #12745 and #12928 already fixed. Nothing else in the census's 24 context-less call sites, and nothing in the 175, is touched.

How the four sites were located

The ruling's own execution note forbids copying its line numbers, so they were re-derived against today's tree. Input: the census os-dev-report (comment 5468253235), which names each site by file, verb and object — never relied on for a line number. Resolution, run in the worktree at merge base bcef1cfac:

grep -nE "\.(insert|update|delete|upsert|create|bulkCreate|bulkDelete|updateMany|deleteMany)\(" \
  packages/services/service-storage/src/metadata-store.ts

That enumerates all eight engine calls the store issues, which is what makes the pairing checkable rather than asserted: the two insert calls carry an options bag and the four update/delete calls did not.

site at HEAD	verb	object	state before this PR
metadata-store.ts updateFile	update	sys_file	no context — the un-repaired half of #12745
metadata-store.ts deleteFile	delete	sys_file	no context — same
metadata-store.ts updateSession	update	sys_upload_session	no context — the un-repaired half of #12928
metadata-store.ts deleteSession	delete	sys_upload_session	no context — same

All four were still unfixed at merge base — verified by that enumeration, not assumed from the census.

What #12745 / #12928 did, and how this matches

Both landed the same three-part shape (c94be6291d, 2ff01cf7fa): an optional StorageWriteContext parameter on the store method; writeOptionsFor(context) folded onto the engine options bag; and the acting organization threaded at the route. This PR is that shape on the remaining four verbs — same helper, same { context: { tenantId } } spelling, same "absent means absent" rule where a caller with no organization produces the byte-identical pre-repair call.

One thing genuinely differs, and it is documented rather than glossed. Write-side tenancy in the SQL driver is two mechanisms wearing one option: on insert injectTenantOnInsert STAMPS the tenant column from options.tenantId; on update and delete applyTenantScope SCOPES the statement with it. So the same bag buys a value on one verb and a reach on the other. Consequences, both measured in the new suite:

• a row belonging to another organization stops being reachable through these doors on a walled deployment;
• an organization_id IS NULL row stays reachable, because the driver's term is (organization_id = :tenantId OR organization_id IS NULL) — Fresh objectstack dev boot: tenant admin sees ZERO rows in sys_position / sys_permission_set / sys_business_unit over REST (Setup Access Control renders empty) #2734's global-row fail-open. That is load-bearing here: multi-org: sys_upload_session is never org-stamped either — StorageMetadataStore.createSession inserts without a context #12928 was ruled forward-stamp-only, so the entire pre-multi-org: sys_upload_session is never org-stamped either — StorageMetadataStore.createSession inserts without a context #12928 session population is NULL by decision and strict equality would have stranded it mid-upload.

Because the meaning differs, the engine-absent Map stand-in is deliberately NOT changed. createFile's stand-in records a value so a later read can return it; there is no value to record on these verbs, only a reach to enforce, and enforcing a tenant wall over a process-local Map would stand up a second isolation mechanism outside the driver that owns the real one.

Scope fences, discharged

• ⛔ The guard ORDER in auditMissingTenant is untouched — packages/drivers/driver-sql/** has no diff in this PR at all. The reading that isSystem writes return at the bypassTenantAudit guard one step before the posture gate is the class-level diagnosis, and it is not acted on here.
• ⛔ No class-level device was added or prepared. No CI gate walking write call sites, no throwing audit under a test env var, no per-boot report. Those three are 暂缓 behind the design card, and whether isSystem writes belong in this control scope at all is a question this PR neither answers nor leans on.
• ⛔ No packages/spec change, no ADR, no release notes, no test skipped or re-baselined.

Paths touched, declared in full

path	why
packages/services/service-storage/src/metadata-store.ts	the four sites
packages/services/service-storage/src/storage-routes.ts	the production callers — without it the store's new parameter has no producer
packages/services/service-storage/src/tenant-audit-update-delete-half-repairs.test.ts	new suite, 21 cases
scripts/engine-double-contract.pinned.json	+3 rows. The gate asked for exactly this (New pinned coverage is GOOD … the ledger just has to learn about it); written with --write, 0 lost
.changeset/tenant-audit-update-delete-half-repairs.md	minor — argued in the changeset body

Docs-drift advisory — read, and NOT falsified

The drift run flags content/docs/permissions/attachments-access.mdx as carrying both sys_file and sys_upload_session. I read the page in full (141 lines). It is accurate as written and nothing was changed.

Its three subjects are the sys_attachment parent-derived access model (create / read / delete on the join row), the download gate, and the ADR-0057 storage-byte lifecycle. This PR touches none of them: no sys_attachment path, no download endpoint, no reap guard. The two doors it changes are upload-side writes on the metadata rows.

The decisive reading is an absence, so it is measured rather than asserted: grep -icE "organization|tenant|multi-tenant|org " over that page returns 1, and the single hit is the phrase "org logos" in a list of non-attachments file examples. The page makes no tenancy claim at all, so there is nothing on it for an organization-scoping change to contradict. Its nearest sentence — the upload entry points "stamp owner_id on the new sys_file" — is about the insert and about owner_id, both untouched.

The four release-owned pages in the same advisory were read but not edited (they are read-only in a code PR). A targeted cross-grep for any claim pairing these two tables with organization or tenant scoping returns zero rows across content/docs/releases/*.mdx. The one v17 line naming a sys_file / sys_upload_session write is the #5216 durability correction ("a write that never landed stops reporting success"), which this PR does not touch. No release page is wrong, so there is nothing to file as a docs-only card.

Verification

Union re-run after the final commit, at e02648ef1b. Every heavy run went through scripts/pm/os-verify-lock.sh; the verdicts quoted are the lines that entry point printed, never a bare $? behind a pipe.

• pnpm --filter @objectstack/service-storage test — 32 files, 506 tests, all pass (21 of them new).
• tsc --noEmit --pretty false -p packages/services/service-storage/tsconfig.json — 51 errors, exactly the check-type-check-coverage ledger entry, and 0 of them in any of the three touched files. That entry's note says the margin is gone and the next new error reds on arrival, so this is the load-bearing number. --listFiles confirms all three files are inside the program — the count is a measurement of them, not a program that excludes them.
• Gate family derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (change set taken from the merge base by the script itself). Green: engine-double-contract, where-matcher, objectql-double-limit, query-options-erasure, type-check-coverage, cross-package-test-inputs (both invocations), test-source-alias, type-source-resolution, route-envelope, undeclared-dep-imports, logger-receiver-detach, nul-bytes, i18n, i18n-stale-fill, doc-authoring, published-files, slot-lookup, page-declaration-shape, ci-filter-parity, comment-mask-adoption, keyed-text-bounds, plugin-teardown-shape, shard-attestation, changeset-gate-self-tests, objectui-changeset, pm-half-states, adr-0087-registration, changeset-no-major, empty-changeset, release-rehearsal-clone, docs-audit/affected-docs.
• pnpm lint (eslint . --no-inline-config, repo-wide) — exit 0, no findings, held the lock 67s.
• pnpm check:i18n — OK, 9 packages, all bundles in sync, service-storage among them, after its ten-package build closure.
• Two gates first went red and were repaired, not baselined: engine-double-contract (ledger taught the new pins) and query-options-erasure (a findOne query bag was erased to any; typed as DriverQuery instead, test surface back to 240 at the ceiling).

Ablation

Implementation committed first; the mutation stripped ...options from all four sites, under an EXIT INT TERM trap with an absolute repo root.

• Mutation proven on disk by anchored counts in both directions: repaired-anchor 4 → 0, ablated-anchor 0 → 4, and the blob hash moved.
• Direction predicted before running: the store, route and engine-leg cases go red; the driver-leg cases stay green because they drive the driver directly and never reach the store. Observed: 10 failed / 11 passed, and the ten are exactly the ten predicted.
• No rebuild involved, and none is claimed: the mutated file is a same-package relative import (./metadata-store.js), so the suite reads it from source. @objectstack/driver-sql is source-aliased by this package's vitest config; nothing mutated resolves through dist/.
• Restore proven by observation, not by exit code: blob hash back to the HEAD blob, git diff HEAD empty, git status --porcelain empty over the whole tree.

Not measured — stated as not-measured, not folded into green

• check:type-check-debt (the ratchet invocation) was not run — it re-runs tsc for every ledger entry and needs the whole workspace closure built, which does not fit the container's foreground ceiling under lock contention. Declared narrowing: the only ledger entry a three-file service-storage diff can move was measured directly, with the gate's own invocation shape, and is the 51 above. The rest of the ledger has no path in this diff.
• check:dual-build-cjs-loads needs the Build Core artifacts; left to CI.
• check-test-completeness and scripts/pm/check-half-states.mjs both exited with their own PREREQUISITE NOT MET code (3) — no test log, no token. Neither is a finding.
• Not measured end-to-end through a real ObjectQL over a real SqlDriver in one process. The chain is pinned as two legs that meet: real ObjectQL proves context.tenantId reaches DriverOptions.tenantId on both verbs, and a real SqlDriver on live SQLite proves what that option then does. A single full-stack fixture would be stronger and is not here.
• No claim about a licensed walled install. Auto-stamping organization_id lives in the out-of-tree enterprise middleware, so "the write now carries its tenant" is a statement about the call site.

---

Authored by Claude Code, session session_01F3jdziLbAPGeceVNmSox5L (recorded in prose because a body edit normalises the footer link).

Generated by Claude Code

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-storage, touching 8 documentable anchor(s).

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

• content/docs/api/plugin-endpoints.mdx (via sys_file (literal))
• content/docs/automation/approvals.mdx (via sys_file (literal))
• content/docs/deployment/cli.mdx (via sys_file (literal))
• content/docs/kernel/contracts/storage-service.mdx (via sys_file (literal))
• content/docs/permissions/attachments-access.mdx (via sys_file (literal), sys_upload_session (literal))
• content/docs/protocol/objectql/types.mdx (via sys_file (literal))
• content/docs/ui/translations.mdx (via sys_file (literal), sys_upload_session (literal))

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

• content/docs/releases/implementation-status.mdx (via sys_file (literal))
• content/docs/releases/index.mdx (via sys_file (literal))
• content/docs/releases/v15.mdx (via sys_file (literal), sys_upload_session (literal))
• content/docs/releases/v16.mdx (via sys_file (literal), sys_upload_session (literal))
• content/docs/releases/v17.mdx (via sys_file (literal), sys_upload_session (literal))

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

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 57827b617e1f6f38a684b5b1daf56c8a7455cb81 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 57827b617e1f6f38a684b5b1daf56c8a7455cb81

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

[zhuangjianguo]

PM review — ACCEPT, with landing held until CI converges (25 success · 2 skipped · 6 still running · 0 red at time of writing; Lint & Repo Gates, Test Core 1–2/6 and the three Dogfood shards are outstanding). My bar is every check green, not the required subset. ⛔ I do not merge it.

What I verified myself, on the diff and on origin/main — not from the report

1. Fences, on the actual 5-file diff. 0 lines in content/docs/releases/, packages/spec/, docs/adr/, .claude/, skills/.
2. The guard order is untouched, by construction rather than by claim — driver-sql has zero files in this PR. Ruling 1.4's deferral to design card design: isSystem 写入是否在租户审计控制范围内?——#13178 类级装置(A/B/C)的共同前置,从未被裁过 #13491 held without needing to be argued.
3. scripts/engine-double-contract.pinned.json is +15 / −0 — purely additive rows registering the new test file's own delete / findOne / update calls. Nothing removed, no existing expectation loosened. That is registration, and it is the file where re-baselining would have hidden.
4. The docs-drift pointer I relayed, re-measured independently with a control. The report claims content/docs/permissions/attachments-access.mdx makes no tenancy claim, so there is nothing for an organization-scoping change to contradict. Re-run on origin/main (57827b617e): grep -icE "organization|tenant|\borg\b" → 1, and that one hit is line 93's "org logos". Positive control sys_file → 5, so the file was read and the pattern discriminates. ⇒ an absence, measured, not assumed. The four release-owned pages were read and correctly not edited, and the cross-grep for a claim pairing these tables with organization scoping returned zero — so there is genuinely nothing to file, and none was invented.

The three Zone 2 assumptions, and the one that matters

• A — BROKE, and it was mine. I assumed the four sites were in driver-sql; they are in service-storage. Zone 2 is exactly where that belonged: as a falsifiable hypothesis it cost nothing, and as a Zone 1 specification it would have pointed the work at the wrong package.
• C — BENT, and this is the substantive finding of the run. I assumed "same table, same class" made the repair a mechanical copy of the insert halves. It is not: the same option stamps on insert (injectTenantOnInsert) but scopes on update/delete (applyTenantScope), so it buys a reach rather than a value. ⭐ The consequence was handled the conservative way — an organization_id IS NULL row deliberately stays reachable, so the pre-multi-org: sys_upload_session is never org-stamped either — StorageMetadataStore.createSession inserts without a context #12928 population the ruling explicitly declined to backfill is not stranded. That is the right call and it is the kind of thing that would have been silently wrong under a literal copy.
• D — HELD, and upgraded from weak to measured. I declined to open the open PRs' file lists and said so, marking my clearance a weak title-level reading. The executor did the work: all 10 open PRs enumerated, only fix(core,rest,services): fail loud when a permission-store read fails #13475 touches service-storage, and only storage-service-plugin.ts — disjoint from this PR's metadata-store.ts / storage-routes.ts. My weak reading was right, and it is no longer weak.

Two gates went red first and were repaired, not baselined

engine-double-contract (the ledger taught the 3 new pins through its own --write, "0 lost") and query-options-erasure (a findOne query bag erased to any, typed as DriverQuery instead). ⛔ Neither was silenced.

The out-of-scope finding is the honest kind

#13528 — deleteFile / deleteSession have no shipped-source caller; only tests reach them, with a positive control showing 7 shipped callers for the update pair. So half the repaired reach is exercised only by tests today. The repair stays correct — the two verbs are the ruled scope and the parameter is the store's contract — and disclosing it rather than quietly shipping a reach nobody exercises is the right handling. Routed pm:retriage; grading is triage's.

⇒ On full green I flip ready and enqueue. If any of the six outstanding checks reds, it is diagnosed and relayed — ⛔ no assertion relaxed to reach green.

---

Generated by Claude Code