Repository navigation
fix(service-storage): a member attaches a file from the browser again — the upload's own completion attaches it as them, after #31031 - #31072
Conversation
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…ys_file stays unreadable Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…r own write Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
… pass Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…s it as them Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…he request names attachTo Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…ouches only for an unheld upload Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
… mark Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
… upload, and refuses every other file Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
… only after its own gates Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
… completion Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…pload Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…nel's flow on the showcase boot Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…e from the browser Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…r guard; the tenant audit census counts the attach verb's insert Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
# Conflicts: # packages/core/src/security/index.ts # packages/core/src/security/vouched-reference.ts # packages/objectql/src/engine-unreadable-reference-target.test.ts # packages/services/service-storage/src/file-attachment-service.test.ts # packages/services/service-storage/src/file-attachment-service.ts
The census script, its root alias, the core repo pin, the Test Core input wiring and the lint workflow step move to a separate change. The attach on upload completion, the one-shot vouch and their pins stay. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
📓 Docs Drift CheckThis PR changes 4 package(s): 57 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 15 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 182 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 35e85c0824c4e910e1e037261a62ca069be01426 && git checkout 35e85c0824c4e910e1e037261a62ca069be01426
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3f61e4f8fb567160f839c3269ee95aa89ee4f1db cb31937dcc5e30cb340f9bb565236a87d0c5da1b && git checkout -B drift-repro 3f61e4f8fb567160f839c3269ee95aa89ee4f1db && git merge --no-ff cb31937dcc5e30cb340f9bb565236a87d0c5da1b
node scripts/docs-audit/affected-docs.mjs --json 3f61e4f8fb567160f839c3269ee95aa89ee4f1db
|
…protocol line is added and spec takes minor Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…— the upload's own completion attaches it Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
|
security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc Contract reviewServed-tier: Read at 2026-11-07T03:35Z: the PR body as rewritten, its 26-file list, the three-dot diff against the merge base Check-runs on this head. All ten required contexts are ① Derived judgmentsThe accept set moves in two published places and one door. Each change, judged:
Two things the rule leaves open, neither a widening of any caller's rights:
② Semver level
③ Boundary flags
④ Design judgmentThe seam. Putting the attach on the complete door is the right seam, and the body's three reasons hold against the code: The mark. The PR reuses the one-file vouch without widening it: no new signature, no new field on the context, one new call site vouching the same triple on the same kind of insert (the caller's own Consumer hunt — every caller of the complete door and every reader of its answer, on
Nothing breaks; the one behaviour change for an existing caller is the panel's, which is the fix. Implemented-by: VERDICT: PASS |
|
security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc 席位核对ACCEPT · 2026-11-07T03:36Z · head
Generated by Claude Code |
…s2vcdk Five files conflicted. The three service-storage files keep both sides: main's attachUploadAs and commit-door attacher, and this branch's record-file verbs and upload-link door (registerStorageFamily takes attachUpload sixth, then getUploadLinkDoor). The tenant-audit census takes main's artefacts, regenerated: 268 write call sites. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibzuTqmB5hqFYmMATiY
security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc
Clause-②: yes (widening)
Metadata-protocol: added
Skills-docs: updated here —
content/docs/permissions/attachments-access.mdxandcontent/docs/api/plugin-endpoints.mdxstate theattachTokey of the upload-complete request and its refusals; no file underskills/**teaches the upload-complete request (agit grepofskills/finds noupload/completeand noCompleteUploadRequest).维护者速读
attachTo),不再走第二步通用数据接口。sys_file读权限的成员从浏览器挂附件,第二步POST /data/sys_attachment被拒(400reference_not_found)。file-attachment-service.ts的attachUploadAs(担保前的各道检查)、storage-routes.ts完成上传门的attachTo分支、engine-defaults-and-autonumbers.ts的buildHookApi(剥除担保)。pnpm --filter @objectstack/dogfood exec vitest run test/attachments-browser-attach.dogfood.test.ts(先构建依赖闭包),预期 8 passed。What changes
The rule. The upload-complete door (
POST /api/v1/storage/upload/complete) takes an optionalattachTo: { parentObject, parentId }. With it, the door first runs every gate it already ran: the session, a malformedattachTo(400), an id that names no file (404), the upload ownership rule (403), the starting organization, and the held bytes (409). It then hands the upload to the file-attachments service's newattachUploadAs, running as the caller. The answer is the committed file plus its newattachmentId.attachUploadAsvouches for onesys_fileand nothing else. Every condition below must hold, in this order, before the vouch is spent:pending.attachmentsscope.findFileHolder: nosys_attachmentrow and noref_*column).sys_attachmentcreate permission, the parent-edit attach rule, andenable.files.The verb then writes the
sys_attachmentrow (vouching for thatfile_id) and commits the file, as one unit. If the write fails, the upload stayspending.One-shot vouch. The vouch is spent on the verb's own write.
buildHookApibuilds every hook'sctx.api, pluswithRunAs('user')andsudoderived from it, from a context with the mark stripped (withoutVouchedReferences,@objectstack/core). A hook running under the vouched insert that names the same file is judged through the caller's read door.Protocol. The spec's
CompleteUploadRequestSchemagains the optionalattachTokey, andFileUploadResponseSchemagains an optionalattachmentId. SoMetadata-protocol:isadded, and@objectstack/specis gradedminor.Unchanged.
sys_file.POST /api/v1/data/sys_attachmentnaming asys_filethe caller cannot read is still refusedreference_not_found.attachTocommits exactly as before.attachTowith 501.Console.
attachToand makes no second call. It keeps its refusal messages.attachToand returns theattachmentId.Why the complete door, not a new route.
pendingis the proof that the file came from this flow and was never attached.Other paths checked.
storage.uploadhas no attach step.sys_attachment.None of them needed a change.
Docs.
content/docs/permissions/attachments-access.mdxcontent/docs/api/plugin-endpoints.mdxRecounted. The tenant audit census counts the verb's new
sys_attachmentinsert (265 to 266 write call sites), and the strictness ledger counts the newattachToobject.Tests
file-attachment-service.test.ts, 13 newattachUploadAscases.sys_fileread attaches. The file is committed,uploaded_byis the member, andsys_filestays unreadable to them.userscope, missing bytes,enable.files, an unreadable parent,reference_not_found.storage-routes.complete-attach.test.ts(new, 15 cases).attachToshapes.VALIDATION_FAILED. An undeclared fault is a generic 500.attachTois unchanged.engine-unreadable-reference-target.test.ts, 3 new cases. A hook writing throughctx.api, or throughctx.api.withRunAs('user'), under the vouched insert, naming the same unreadable target, is refused. The control: under a system write, the same hook's write lands.attachments-browser-attach.dogfood.test.ts(new, 8 cases), at the HTTP level on the showcase boot, as a member holdingshowcase_contributorplussys_attachmentread/create and nothing onsys_file.attachTogives 200. One committed file, one attachment in the member's name. The panel's list read and the download URL work, andsys_file/:idstays 403.reference_not_found;'*'reader works through both flows.RecordAttachmentsPanel.test.tsxuses the real adapter with a stubbed fetch. The complete body carriesattachTo, a refusal maps to friendly copy, and a missingattachmentIdis an error.affordanceGrant-12047control asserts the adapter getsattachToand thatdataSource.createis never called.affordanceGrantMap-12082.test.tsxdrops the panel'shandleFiles :: createsite, since the panel no longer callscreate. ItsattachmentUploadrow is still read where the upload is offered, and its delete site is unchanged.Acceptance notes
'*'reader working.requireUploaderpath already makes it on completion withoutattachTo. The verb's own answer is identical for both.Follow-up. The call-site census gate (
check:vouch-for-reference-callers) has moved out of this PR. It pins the code that sets the vouched-reference mark to an allowlist:file-attachment-service.ts, 2 calls. Its parts:repovitest pin,lint.ymlstep.It lands in its own PR after this one merges. That PR is Tier H, through the maintainer, because of the workflow step. This PR touches no governed surface.
Verification
Ablation: the vouch in attachUploadAs
The verb's insert context
vouchForReference(unitContext as ExecutionContext, …)was replaced with a plainunitContextthroughscripts/ablation-replace.mjs. service-storage was rebuilt, andablation-dist-preflightconfirmed the marker is present in 2 built files.-t attachUploadAs): 2 failed, 11 passed. Red: the member attach, and the case that first attaches through the verb and then tries a direct insert. Its setup attach now fails.'*'reader through both flows.24e738f31e20), source clean. Rebuilt, and preflight--absentconfirmed the marker is absent from all 6 built files.Ablation: the strip in buildHookApi
withoutVouchedReferences(...)was removed frombuildHookApi. Result:engine-unreadable-reference-target.test.ts2 failed, 122 passed. Red: thectx.apiandwithRunAs('user')hook cases. Green: the system-write control. Restore: blob == HEAD.Suites at head cb31937 (main merged at 5210acf), under the verify lock
pnpm install --frozen-lockfile && pnpm build: 119 of 119 tasks.@objectstack/core: test 3143 passed;test:repo94 passed.@objectstack/objectql:engine-unreadable-reference-target.test.ts124 passed. The whole package (11372 passed) was run before the census gate was split out; that commit does not touch objectql.@objectstack/service-storage: test 1388 passed; typecheck clean.attachments-browser-attach,attachments-permission-matrix,print-service-archive.skipIf(!organizationsAvailable)block.affordanceGrantMap-12082.test.tsx: 55 passed.plugin-form,app-shellanddata-objectstackwhole packages: 23858 passed, 10 skipped.turbo run type-checkfor@object-ui/app-shelland@object-ui/data-objectstack: 68 of 68 tasks.@objectstack/speccheck:generated: all 14 artifacts current.!and Clause-② carries no(narrowing)arm.Gates
dispatch-gates --commandsat cb31937 derives 117 commands, and all 117 were run and exited 0.--ranreconciles them: 117 derived, 117 run, 0 not measured, 0 unrun.pnpm --dir ui exec vitest run scripts/,spec check:generatedandcheck:type-check-debt.check:self-test-wired: green.check-governed-merges.mjs --pr 31072: NOT governed. 0 of 26 paths hit the register, andgit diff origin/main...HEAD -- .github/is empty.check-changeset-no-major,check-adr-0087-registration(non-breaking) andcheck-empty-changeset, all green.check-changeset-no-majorwas also fed apull_requestpayload carrying this body: its level axis passes, reading clause-②yes (widening), metadata-protocoladded, and@objectstack/spec: minor.https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc