Repository navigation
fix(service-storage)!: the upload commit, chunked-completion and progress doors act only for the uploader - #22170
Conversation
…ress doors act only for the uploader One ownership rule, `isFileUploader` in a new `upload-ownership.ts`, declared once and consulted by the three upload doors that act on an id the caller names. Each door calls it through `requireUploader` right after its by-id read and before any write or disclosure: the caller's user id must equal the file's `owner_id`; an upload session reaches its uploader through `file_id`; an empty owner or a missing file row is refused; no administrator exception. The refusal is one body, `403 PERMISSION_DENIED`, whoever is refused. Open mode (no session resolver wired) is unchanged: there is no caller identity to compare. The download doors, the start doors, the tenant stamp and scope, and the not-found answers are unchanged. Pins per door: the uploader passes; a same-organization non-uploader and a caller acting in another organization get the identical refusal with the rows untouched; the rule precedes the expiry write. Three route fixtures in the tenant-audit suite gain the uploader their doors now require, and the envelope conformance suite gains the new refusal. Clause-②: no (narrowing) Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
…oted organization wall The ruling's addendum asks, per upload door, for a caller acting in another organization to be refused with the same body as a caller in the uploader's own organization, measured on a booted wall. The wall is raised by this package's OrganizationsPlugin, which no other workspace package may declare (ADR-0132), so the measurement lives here: the smallest package that boots it. `bootStack` with this package's OrganizationsPlugin and the real StorageServicePlugin, the isolated posture requested and read back off the tenancy service; a colleague joins the uploader's organization through the invitation doors and an outsider creates an organization of their own. Per door (commit, chunked completion, progress): a positive pin, and the colleague and the outsider answered the identical `403 PERMISSION_DENIED` with the rows read back whole and unchanged. The two new devDependencies (`@objectstack/service-storage`, `@objectstack/verify`) are aliased to source, as `check:test-source-alias` prescribes. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
…wall The same-organization and the cross-organization refusals become separate pins, so each one fails on its own assertion when its door's ownership call is removed. The cross-organization pin asserts the outsider's answer and the unchanged rows first, then that the body equals the colleague's. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
…oots a stack `check:registry-log-declared` requires every suite that boots a stack through `bootStack` to quiet the registry's per-item registration chatter through the engine's own seam; the upload-door wall test made this package one. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 24 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. 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): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ac1ee4b6045999e967ba71bbd3daf09b736382b5 && git checkout ac1ee4b6045999e967ba71bbd3daf09b736382b5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 959c209d56a8b9ab6f7e561e87700721194e4678 e4eb04d602ca9f8e797d515fc5d93c336f633a7c && git checkout -B drift-repro 959c209d56a8b9ab6f7e561e87700721194e4678 && git merge --no-ff e4eb04d602ca9f8e797d515fc5d93c336f633a7c
node scripts/docs-audit/affected-docs.mjs --json 959c209d56a8b9ab6f7e561e87700721194e4678
|
Brings in PR #22138, so this PR's OSV scan judges only what it adds (main's next@16.3.6 advisories, anchor #22148, are the base's). No file is changed on both sides. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
… RESOURCE_CONFLICT, not an outage 500 (objectstack-ai#22216) Fixes objectstack-ai#22175 Clause-②: no The commit door and the chunked-completion door of `@objectstack/service-storage` now answer an uploader whose active organization changed after starting an upload with `409 RESOURCE_CONFLICT`, in the ADR-0112 envelope, with a message that names the organization change. They used to answer `500 INTERNAL` with the store's data-engine outage text. The operator log gets one warning that names the cause. Which organization an upload belongs to does not change, and the tenant scope of every write is unchanged. ## Rulings carried (triage `6052856225`, verbatim) - "answer with a worded 4xx that names the organization change, in the ADR-0112 envelope" - "the operator log names the cause" - "⛔ No change to which organization an upload belongs to: objectstack-ai#22046's ruling keeps the tenant scope as it is." - "`Clause-②: no`. Patch changeset for the storage service." - From the card: "⛔ No catch-all that hides a real engine failure." ## What changed - **`metadata-store.ts`**: a new module-level function, `organizationOutOfWriteReach(store, row, context)`. It is not re-exported from the package entry, and the public face is unchanged. It answers one question about the by-id write the store would issue under `context`: can that write reach `row`? If it cannot, the function returns the organization the row was started in. Otherwise it returns `null`. - It returns `null` for the engine-absent stand-in, which does not scope its writes. A module-private `WeakSet` filled in the constructor records which stores are engine-backed. - It returns `null` when no organization is threaded. It reuses `writeOptionsFor`, the same rule that decides whether the write carries a `tenantId`. - It returns `null` for a row stamped with no organization, because the driver keeps org-less rows in reach. - Otherwise, a row is in reach exactly when its organization is the acting one. - It composes no predicate. The statement's tenant term stays the driver's. - **`storage-routes.ts`**: a new gate, `requireStartingOrganization(writeContext, rows, …)`. It runs right after the ownership rule from objectstack-ai#22170 and before any write, so only the uploader ever reaches it. It asks the store about every row the door is about to write. When a row is out of reach, the gate logs one `warn` line naming the door, the upload id, the starting organization and the active one. It then answers `409` / `RESOURCE_CONFLICT` with a constant message that names no organization. - **Commit door** (`POST …/upload/complete`): it asks about the `sys_file` row. The write context is now bound once and passed both to the question and to `updateFile`, so the two cannot differ. - **Chunked-completion door** (`POST …/upload/chunked/:uploadId/complete`): it asks about the upload session and its file, and it asks before the expiry stamp, which is the first scoped write. The file row that the ownership rule reads is now bound to a variable rather than read inline. - **Changeset**: `.changeset/22175-upload-org-change-4xx.md`, a `patch` for `@objectstack/service-storage`. **Code choice.** `RESOURCE_CONFLICT` is the standard-catalog member that HTTP 409 derives, so no ledger entry and no `packages/spec` change were needed. The request conflicts with where the upload stands, and the caller can resolve it: the same call succeeds after switching back, and the wall pin shows this. `PERMISSION_DENIED` would merge this answer with the ownership rule's one refusal, and this caller is the uploader. `PRECONDITION_REQUIRED` (428) means the request is missing a precondition, and nothing is missing here. ## Before and after, on the booted organization wall Measured with `packages/plugins/organizations`: `bootStack` with `OrganizationsPlugin` and `StorageServicePlugin`, posture `isolated`, and the uploader switched through the product's own `set-active` door. | door | before (`1e5d322c`) | after (`af2811c51c`) | |---|---|---| | commit, organization changed | `500 INTERNAL`, "StorageMetadataStore: sys_file update failed against the data engine … Restore the data engine …", cause "Record … not found in sys_file" | `409 RESOURCE_CONFLICT`, "This upload was started in a different organization than your active one. …"; row unchanged; switching back commits it (`200`) | | chunked completion, organization changed | `500 INTERNAL`, the same text for `sys_upload_session` | `409 RESOURCE_CONFLICT`; session and file rows unchanged; switching back completes it (`200`) | | commit / chunked completion, same organization | `200` | `200` (control) | | progress, organization changed | `200` | `200` (unchanged) | H1, measured: the scoped miss is a **throw**, not zero rows. The SQL driver's scoped `UPDATE … WHERE id` touches no row and its scoped read-back returns `null`. The engine then raises "Record ID not found in OBJECT", and `engineOp` wraps that as `StorageMetadataStoreError`. ## Why the question lives in the store, not as a bare comparison at the door A comparison written only at the door would also refuse on the engine-absent stand-in. The stand-in records the organization on insert but does not scope its writes, and it finishes this upload today (`200`). Refusing there would narrow an accept set that is published (`registerStorageRoutes` and `StorageMetadataStore` are both public), and that reads as `Clause-②: no (narrowing)` against the ruled `Clause-②: no`. A public getter on `StorageMetadataStore` would widen the public face instead. The module-level function changes neither, and it keeps the stand-in's answers unchanged, which is H4. ## Tests - `storage-routes.metadata-outage.test.ts`: a new block, `an organization change is not an outage`, with 14 cases on that file's existing fake engine. The double was reused and no new double was pinned. - At both doors: `409` with the code and first sentence, rows unchanged, and the warn line naming the upload and both organizations. - Same-organization control (`200`). - Same-organization **real engine failure** still answers `500 INTERNAL` with "Restore the data engine", at both doors. - No active organization (`200`), an org-less row (`200`), an org-less session whose file is out of reach (`409`), and the question asked before the expiry write. - The ownership rule runs first: a non-uploader in another organization still gets `403 PERMISSION_DENIED`, which says nothing about organizations. - The progress door is unchanged, and the stand-in is unchanged (`200` from either organization). - `metadata-store.test.ts`: 6 cases on `organizationOutOfWriteReach`. - `error-envelope.conformance.test.ts`: one case for the new code, parsed against the declared envelope. - `packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts` (new): the premise, both doors' negative pins with switch-back, both same-organization controls, and the progress door unchanged. ## Ablation (from committed `af2811c51c`) The mutation went through `scripts/ablation-replace.mjs` in wrap mode, run under the verify lock. It replaced the question with `const started = null` (marker `ABLATION-22175`). - On-disk evidence: anchor count went 1 to 0, marker 0 to 1, blob `fd950d45ec2a` to `eba43b779bd2`. - Both suites resolve the subject from `src`: the organizations package's vitest alias maps `@objectstack/service-storage` to `src/index.ts`, and the unit tests import relatively. So no `dist` leg applied. - Wall: the 2 negative pins turned red. Each received `500 INTERNAL` with the outage text, the original defect reproduced. The other 4 stayed green. - Unit: 5 cases turned red, the conformance case among them. The fake engine does not scope, so the doors answered `200`, or `410` on the expiry case. - Restore: proven by the tool (blob equals HEAD `fd950d45ec2a`, `git diff HEAD` empty) and re-read afterwards (anchor 1, marker 0). ## Gates (at `af2811c51c`) - **Derivation.** Run from this worktree with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, after `git fetch origin main` (origin/main `13aea18959`, merge base `1e5d322c1`). It derived 68 commands. That is the seat's 66 plus `check:dispatcher-error-vocabulary` and `check-dev-prereqs.mjs --self-test`. - **Reconciliation.** `--ran` was run with every exit code recorded. It printed: "68 derived famil(ies) accounted for — 67 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)". - **Exit 0: 67 commands.** One of them, `check:i18n`, first answered PREREQUISITE NOT MET (exit 3). After its stated build closure, it printed: "check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys)". - **NOT MEASURED: `check:dual-build-cjs-loads`.** Its prerequisite is a whole-workspace build (33 packages have no `dist/`). In its place I ran a narrow probe on the only package whose source this diff touches: the rebuilt `service-storage` loads through `require('./packages/services/service-storage/dist/index.cjs')` (exit 0). The new function is absent from `dist/index.d.ts`. CI runs the full gate. - **Error-envelope and error-code families.** - From the derivation: `check:route-envelope` passes, and `check:dispatcher-error-vocabulary` answered "OK — 54 unregistered code-stamping site(s), all classified". - From the roster families the derivation lists but cannot place: `check:error-code-casing` ("no unlisted lowercase error codes in 7791 scanned file(s)"), `check:error-status-conformance` ("every derivable runtime status is documented, and every documented status is reachable") and `@objectstack/spec check:error-code-provenance` (OK). - Also run beyond the derivation, all exit 0: `check:authz-resolver`, `check:filter-alias-parity`, `check-changeset-fixed.mjs`, `check:tenant-chokepoint` and `check:durability-log-level`. - **Package suites at `af2811c51c`.** `@objectstack/service-storage`: 43 files, 704 tests pass. `@objectstack/organizations`: 11 files, 147 tests pass. `typecheck` exits 0 for both, with `check:test-typecheck: OK` for each. - **Lint, narrowed.** I ran `eslint --no-inline-config --format json` over the 6 touched TypeScript files: files=6, errors=0, warnings=0. Population: there was no "file ignored" warning, so all 6 were linted under `eslint.config.mjs`. Invariance: that config never enables type-aware linting (no `parserOptions.project`, no typed rules), so this diff cannot change the verdict on an untouched file. The full `pnpm lint` is CI's. ## Acceptance notes - **Same family, outside this card's two doors.** These were measured on the same wall with a throwaway probe that was deleted after the run: - the chunk door (`PUT …/upload/chunked/:uploadId/chunk/:chunkIndex`), after the uploader switches organization, answers `500 INTERNAL` with the `sys_upload_session` outage text; - the progress door, for a session past its `expires_at`, answers the same `500`, because its expiry stamp is a scoped write. From the starting organization it answers `200` with `expired`. - The store question this PR adds would answer both of these scoped writes. They are reported to the seat as one finding and are not fixed here, because the dispatch fenced the two doors. - The dispatch said the card had 3 comments. The REST read shows 2, the triage and the claim. The newest `Claim:` names this branch. --- _Generated by [Claude Code](https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #22046
Clause-②: no (narrowing)
Ruling B on the card, with its addendum: one ownership rule, declared once and checked at the three storage upload doors that act on an id the caller names. Classes, positions and functions only.
What changed
packages/services/service-storage/src/upload-ownership.ts(new).isFileUploader(callerUserId, file)is the one rule. The caller's user id must equal the file'sowner_id. An emptyowner_idand a missing file row both answerfalse, so a door never guesses an owner. There is no administrator exception. The module also declares the refusal:403,PERMISSION_DENIED, and one constant message.storage-routes.ts.requireUploader(authSession, file, res)calls the predicate and sends the refusal. Each door calls it right after its by-id read and before any write or disclosure:POST …/upload/complete): aftergetFile, beforeupdateFile;POST …/upload/chunked/:uploadId/complete): aftergetSessionit reads the session's file byfile_id, because the session row has no user column, then checks. This comes before the expiry stamp, the status writes and the answer;GET …/upload/chunked/:uploadId/progress): the same, before the expiry write and the progress answer.authorizeDownload, the two start doors, the tenant stamp and the tenant scope, the not-found answers, and the chunk door with its resume-token check.authservice):requireUploaderpasses. There is no caller identity to compare, and the routes stay open as theresolveSessionoption documents. See the acceptance notes below..changeset/22046-upload-doors-uploader-only.md:@objectstack/service-storageminor. It is an accept-set narrowing, marked BREAKING under the launch-window convention, with ADR-0087 dispositionnot-required (no-migration-prescription).Pins
service-storage·src/upload-door-ownership.test.ts(23 cases). Per door:403 PERMISSION_DENIED, the rows are unchanged, and the body names nothing about the row;It also covers: an empty owner is refused; a session whose file row is gone is refused; there is no administrator exception; the rule runs before the expiry write and before the expiry disclosure; the not-found answers are unchanged; open mode is unchanged; and the predicate's own cases.
Booted organization wall ·
packages/plugins/organizations/src/storage-upload-door-ownership.wall.test.ts(10 cases). Cross-lane files, declared here:packages/plugins/organizations/package.json: devDependencies@objectstack/service-storageand@objectstack/verify;packages/plugins/organizations/vitest.config.ts: both aliased to source, ascheck:test-source-aliasprescribes;pnpm-lock.yaml: 6 lines.The wall is raised by
OrganizationsPlugin. ADR-0132's boundary (no-framework-dependents.pin.test.ts) lets no other workspace package declare it, which is why the dogfood multi-organization blocks skip. That makes this package the smallest one that boots the wall. The ADR-0132 pin allows the dependency in this direction.What the test runs:
bootStackwith this package'sOrganizationsPluginand the realStorageServicePlugin,OS_TENANCY_POSTURE=isolatedandOS_AUTH_MEMBERSHIP_POLICY=invite-only;tenancyservice: postureisolated,isolationActive: true,degraded: false;Per door:
403 PERMISSION_DENIEDwith a body equal to the colleague's.The rows the door names are read back whole, as system, before and after each refused call, and must be equal.
Fixture triage. The route block in
tenant-audit-update-delete-half-repairs.test.tsseeded rows with no uploader. The commit and progress doors now refuse such rows before the write that block pins, so its seeds gainowner_id(the session's user) and the progress case gains its file row. Envelope conformance gains the new refusal's case.Evidence (head
67e5a1ef)033e5c53plus the pin file) gaveTests 12 failed | 11 passed (23). Every negative pin was red and every positive or unchanged pin was green.@objectstack/service-storagevitestTest Files 43 passed (43) · Tests 683 passed (683).@objectstack/organizationsvitestTest Files 10 passed (10) · Tests 141 passed (141). The dogfood suites that drive these doors on the wire (attachments-permission-matrix,field-file-collection,predicate-write-unreadable-not-matched,write-door-unreadable-is-not-found) gaveTests 56 passed | 1 skipped (57); the skip is the existing multi-organization block, which skips in this repository. Typecheck is green for both packages, and both test-layer programs list the new files (--listFiles).scripts/ablation-replace.mjs --delete, the door'srequireUploadercall removed; each leg restored to the HEAD blob withgit diff HEADempty):@objectstack/service-storageto source (the relative import in the storage suite, the vitest alias in the wall suite).eslint --no-inline-config --format jsonon the 7 touched lintable files: 7 files, 0 errors, 0 warnings. The population is every touched.tsfile, and eslint reports none as ignored. This repo's config enables no type-aware rules, so the diff cannot move a verdict on an untouched file.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat67e5a1efderived 79 gate commands. All 79 were run on that head and each exited 0.--ranwith the recorded exit codes reports79 run, 0 NOT-MEASURED (a DERIVED zero). One gate was red on the first pass and is fixed in this diff:check:registry-log-declaredrequired the organizations suite, now booting a stack, to declareOS_REGISTRY_LOG.Acceptance notes (boundary readings, not changed here)
404(FILE_NOT_FOUND/UPLOAD_SESSION_NOT_FOUND). An id that exists but belongs to someone else answers403. The pair therefore tells a caller whether an id they hold exists. The ruling fixes403and leaves the not-found answer alone, so neither is touched. Before this change the same distinction existed through the door's success answer.PUT …/chunk/:chunkIndex) is outside the ruling. It keeps its resume-token check.@objectstack/organizationstests now depend on@objectstack/verifyand its closure (test only; nothing ships).Generated by Claude Code