Repository navigation
fix(service-storage)!: a stored file's uploader is the user who created it, as an attachment's and a comment's author are - #30170
Conversation
…ed it, as an attachment's and a comment's author are A non-system create of sys_file records the session's user as owner_id, over whatever the create carried, and a create whose session names no signed-in user is refused with 403 RECORD_NOT_ACCESSIBLE. System-context writes, the upload doors and a context-less call on a bare kernel keep the uploader they supply. Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude <noreply@anthropic.com>
…stored file's uploader rule Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 35 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 10 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 169a5cc19f9a5bd6c6750044994fc6dbd78d5e23 && git checkout 169a5cc19f9a5bd6c6750044994fc6dbd78d5e23
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9a53fc9feac04d44f40fffc62f93d04433de1227 89c379ba6e21e5650ad7e8900a5fdc361942d8c9 && git checkout -B drift-repro 9a53fc9feac04d44f40fffc62f93d04433de1227 && git merge --no-ff 89c379ba6e21e5650ad7e8900a5fdc361942d8c9
node scripts/docs-audit/affected-docs.mjs --json 9a53fc9feac04d44f40fffc62f93d04433de1227
|
|
security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj Contract-review-claim: session_01QSeKYn5CmmdShCJifm6WYj · head 3aa77b9 · 2026-11-04T19:46Z |
|
security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj Contract reviewServed-tier: Face: ① Derived judgments
② Semver level
③ Boundary flags
④ Design judgmentJudged against the published surface on
Findings:
Implemented-by: VERDICT: PASS |
… as a save notice, not a dropped-field report Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude <noreply@anthropic.com>
|
security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj Contract reviewServed-tier: Delta review: the prior PASS stands at ① Derived judgments
② Semver levelUnchanged and still correct: ③ Boundary flags
④ Design judgmentThe narrowing is unchanged from the reviewed head and holds for the same reasons. §6 check on the delta: the new docblock paragraph, the commit message ("the sys_file guard's docblock names ctx.notice as a save notice, not a dropped-field report") and the rewritten body sentences state the rule and the contract, nothing of an attack; no breach. The docblock's "one caller-facing member" is accurate for what a hook can say to the requester ( Implemented-by: VERDICT: PASS |
|
security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj 席位核对ACCEPT · head
|
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: updated here —
content/docs/permissions/attachments-access.mdxnow states that asys_filecreated through the data API records the signed-in user asowner_id, and that such a create with no signed-in user is refused. No published skill teaches writingsys_file.owner_id(git grepoverskills/finds none).维护者速读
sys_file时,owner_id一律记为当前会话用户(覆盖请求里带的值),与附件uploaded_by、评论author_id同一规则;会话没有登录用户的创建被拒绝(403RECORD_NOT_ACCESSIBLE)。sys_file时请求里的owner_id不再生效;上传接口、系统上下文写入、无会话的裸内核调用都不变;控制台不经数据接口创建sys_file,无需改动。回滚即 revert 本 PR。packages/services/service-storage/src/sys-file-storage-owned-columns.ts(recordUploader与钩子里if (ctx?.session)那一行)、packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts(数据接口上的新钉)、.changeset/sys-file-uploader-is-its-creator.md。pnpm --filter @objectstack/service-storage exec vitest run src/sys-file-storage-owned-columns.test.ts src/sys-file-storage-owned-columns.integration.test.ts,预期Test Files 2 passed (2)、Tests 38 passed (38)。What changes
A stored file's uploader is the user who created it, as an attachment's and a comment's author are.
sys_filerecords the session's user asowner_id, over whatever the request carries: the stamp the attachment create hook (sys_attachment.uploaded_by) and the comment create hook (sys_comment.author_id) already apply. It lives in the existingsys_filecreate guard (installSysFileStorageOwnedColumnsGuard), beside the storage-owned column strip, sosys_filekeeps onebeforeInsertregistration.RECORD_NOT_ACCESSIBLE, the code and status the attachment and comment hooks answer their own no-user creates with.owner_idit supplies: the upload doors (presigned, chunked, complete) record the session's user themselves under it, as do imports and migrations. A call with no session at all (a bare kernel) keeps its value too, which is the attachment hook's carve-out. Updates are unchanged:owner_idis already read-only after creation, so an update drops a non-system caller's value and reports it indroppedFields.isSystem(as the update strip does) and this rule does not loosen it.droppedFields/onFieldsDroppedonly for strips it runs itself, it runs none on asys_object's create, and the hook contract carries no dropped-field report: its one caller-facing member,ctx.notice, is a free-text save notice, and neither sibling stamp uses it. So the overwrite is not reported, exactly asuploaded_byandauthor_idare not.content/docs/permissions/attachments-access.mdx, and theowner_idcomment inobjects/system-file.object.ts.Tests
sys-file-storage-owned-columns.test.ts, 8 cases): the session's user is recorded over another user's id and when none is named; three no-user sessions (a service principal, an organization only, an empty user id) are refused 403RECORD_NOT_ACCESSIBLEwith nothing logged; a system create keeps its value; a bare-kernel call keeps its value and still loses the storage-owned columns; the stamp sits beside the strip.sys-file-storage-owned-columns.integration.test.ts, 6 new cases): single and batch creates record the caller; a no-user create is refused and writes no row; the upload doors (presign, complete) still record the uploading user; a system create and a system update keep the uploader they write; an update leaves the uploader the create recorded.attachments-permission-matrix.dogfood.test.ts, uploader block): an administrator'sPOST /data/sys_filenaming another user answers 201 and records the administrator; control, a system create keeps the uploader it supplies. The block's existing pins stay the update-side and upload-door controls: an administrator's PATCH ofowner_idanswers 200 withowner_idindroppedFields, and (e) the upload door stamps the uploading member.scripts/ablation-replace.mjs, which restored on exit): the stamp line replaced by a no-op,service-storagerebuilt,ablation-dist-preflightproved the line absent fromdist/. Red as predicted: 5 of 38 in the two guard files (the three unit and two integration cases that read the stamp) and the new dogfood pin; every control stayed green. Restore leg: blob equal to HEAD,git diff HEADempty, rebuilt, preflight proved the mutation absent and the line present indist/, both runs green again.Acceptance notes
sys_file.owner_idhas no update-time ownership-transfer path: since the rule that made a row's uploader read-only after creation, the update strip drops a non-system caller'sowner_id, the holder of the ownership-transfer grant included (measured onorigin/main917f024: PATCH 200,droppedFieldsnamesowner_id, stored value unchanged). This PR keeps that; only the system context records a different uploader.sys_filethrough the data API (git grepoverui/andpackages/finds the one writer,StorageMetadataStore.createFile, under the system context), so no caller changes.Verification
Readings on
3aa77b9a7f(origin/main232d3bb merged in; it touches no package) unless stated. The head89c379ba6eadds one comment-only commit (the guard's docblock onctx.notice). At that head, the guard tests readTest Files 2 passed (2),Tests 38 passed (38), and the 49 gate families derived for the changed file all exit 0 (dispatch-gates --ran:49 run, 0 NOT-MEASURED).service-storage: full suite and typecheck
pnpm --filter @objectstack/service-storage exec vitest run --maxWorkers=2underscripts/pm/os-verify-lock.sh:Test Files 72 passed (72),Tests 1341 passed (1341), lockVERDICT command-exit 0.pnpm --filter @objectstack/service-storage typecheck: exit 0, includingcheck:test-typecheck: OK;tsc --listFilesontsconfig.test.jsonlists the guard and both test files.pnpm --filter @objectstack/dogfood typecheck: exit 0; its program listsattachments-permission-matrix.dogfood.test.ts.Derived gates (dispatch-gates --commands, 94 families)
check:skill-examples(no builtclient-reactdeclarations in this worktree),check:dual-build-cjs-loads(seven packages outside the dogfood build closure have nodist/),check:type-check-debt(its closure build never got a turn on the shared verify lock). CI'sLint & Repo Gatesruns all three on a full build.dispatch-gates --ran:94 derived famil(ies) accounted for — 91 run, 3 NOT-MEASURED, exit 0.Dogfood battery
pnpm --filter @objectstack/dogfood teston 3aa77b9 under os-verify-lock: VERDICT command-exit 0; Test Files 417 passed | 3 skipped (420); Tests 3464 passed | 28 skipped (3492)