Skip to content

docs(adr-0104): a write references only a file its writer may read - #25971

Merged
os-zhuang merged 2 commits into
mainfrom
claude/project-thread-n8sx5h
Oct 26, 2026
Merged

os-zhuang merged 2 commits into
mainfrom
claude/project-thread-n8sx5h

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Elon · project thread

Follow-up to #25692, which #25951 fixed (merge commit 3761710927). This PR changes no code.

维护者速读

改了什么:给 ADR-0104 D3 补一条修订,记明记录写入时的「认领」和「复制认领」只对写入者能读的文件发生,判断用的就是下载接口那一套规则。具体有三处:Status 加一条修订记录;在 2026-07-27 增补的最坏情况表下加注;末尾新增 2026-10-26 增补。按你在设计审阅后的选择(先改措辞再批),增补又补了三处准确性说明:保留的 ID 通过这条记录对任何读者都不可用,后续由 #26043 改成明确报错;流程和钩子按各自的 runAs 身份判断,只有系统身份免检;「分不清无权和不存在」只在保存这一步成立。

为什么改:#25951 已经把这条规则写进代码。但 ADR 里「独占加复制认领,最坏只是多存一份字节」这一行,隐含了一个前提:复制的人本来就能读原文件。代码之前没有检查这个前提,所以决策记录不完整。把前提写进 ADR,以后改这块的人搜 copy-on-claim 就能读到这条规则。

风险与代价(含回滚):只改文档,不动代码和数据。回滚就是 revert 这个 PR,对运行时没有影响。

席位意见:建议批准。增补只把 #25951 已经落地的规则写进决策记录,没有新增或放宽任何行为,措辞只写机制,符合本卡的披露要求。逐句核对增补与合入代码是否一致,交给契约复审(CONTRACT_REVIEW_TIER)记录,记录会贴在本 PR 上,批准前请先看它的结论(措辞修改后的 head 会重新复审)。有一个相邻的问题由席位在线下另行转告维护者,不写在这里。

你要做的:审阅并批准这个 PR。它改的是受管面 docs/adr/**,需要你本人批准才能落地。

What changes

Per the card's disclosure rule, the text states the mechanism only.

Gates

Run on head 13d9a1ad (3d14f9ae plus one wording commit), whose base is main 3761710927.

  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derives 20 gates. All 20 exit 0, and --ran reconciles them: 20 derived, 20 run, 0 NOT-MEASURED.
  • pnpm check:pm-governed-prose: exit 0.
  • check:commit-card-trailers --range origin/main..HEAD: the 1 commit passes.
  • No changeset: the diff publishes nothing from any released package (skip-changeset).

Acceptance notes

  • Tier H (docs/adr/**): this lands only on an approval by a governed approver.

Generated by Claude Code

Amends ADR-0104 D3 wave 2: claim and copy-on-claim run only for a file the
writing caller may read, by the download door's own verdict. Adds the
2026-10-26 addendum, a Status bullet, and a note under the 2026-07-27
worst-case row for exclusive ownership.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016DTRymn12to8YSGMcTzS7h
@objectstack-fleet objectstack-fleet Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Oct 26, 2026
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation labels Oct 26, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 3d14f9ae5c7220890b0c42dfa335b469e617b7fa
Local-runs: none
Scope: docs/adr/0104-field-runtime-value-shape-contract.md + packages/services/service-storage/src/file-reference-lifecycle.ts, packages/services/service-storage/src/storage-service-plugin.ts, packages/services/service-storage/src/attachment-access-hooks.ts, packages/services/service-storage/src/storage-routes.ts (the download door's own arms), all read on main at 3761710927

Face: rules (docs/adr/**), read against the merged code the amendment describes (PR #25951, merge commit 3761710927), card #25692 and its comments, and the check-runs on this head. Adversarial read: each normative sentence of the +60 is judged on its own against the merged code.

① Derived judgments

  • Status bullet (2026-10-26). Right. The claim path's single lookup (findReferenceableFile) feeds both copy-on-claim (applyCopyOnClaim, the before-hooks) and the claim (claimFile, the after-hooks); with a bound verdict and a caller, a refused file is answered as null, the same answer the organization-scoped lookup gives a row it cannot see. #25682 (e2b40a264b) is the commit that introduced that organization-scoped lookup, so the precedent citation is correct.
  • Note under the 2026-07-27 worst-case table. Right. Steps 1 and 2 of the resequencing list are ownership bookkeeping and governed download, as the addendum names them; before 3761710927 the claim path asked the organization-scoped lookup and nothing else, which the merged PR body states in the same words.
  • The rule ("claims or copies only a file its caller may read, by the download door's own verdict"). Right. The plugin binds buildFileReferenceReadAuthorizer into installFileReferenceHooks from the live registry; the lifecycle module decides nothing itself and both halves of the claim path ask through one function.
  • "One judgement, two callers ... neither restates it." Right for the judgement: judgeFileRead is one function, called by the download door under its request-resolved context and by the claim path under the write's caller context. Its four arms are as listed: uploader (owner_id), a reader of the one owning record via readableFieldOwners (delegate or raw read, fail-closed), the organization arm for a file no record owns (#25693), any readable sys_attachment parent for an attachments-scope file. One precision point, non-blocking: the fifth item, "anyone for a public_read file", is not an arm of the shared function. The door serves a public_read file in storage-routes.ts before it asks its authorizer, and the claim path's binding mirrors that pre-judgement arm (and the door's no-user answer) in its own two lines. The per-file outcome is the same at both doors, which is what the rule promises; the sentence slightly overstates where that one arm lives. The organization arm is a summary of the wall's verdict for the posture in force, the same wording the shipped changeset uses.
  • "The quiet answer." Right. A refused file leaves the id as written; nothing is claimed or copied; no new code or status is introduced. The cross-organization precedent is correctly attributed.
  • "A write's own copy." Right for the stated path: a copy minted in the before-phase is remembered per write and claimed without a second question. The addendum omits the fail-safe the merged code carries when a write's two phases share no key (the copy is then judged as any unclaimed file and, if refused, left unclaimed). An omission, not a misstatement.
  • "Outage." Right at the before-write gate, which every write referencing a file passes: AuthzStoreUnavailableError is re-raised by the binding, not caught by the lookup, and fails the write under the error's own code and status. Any other fault in the judgement answers as a refusal.
  • "No caller, no question." Right. referencingCallerOf returns no caller for a system write or a session-less programmatic call; an anonymous session is a caller and is refused for every file but a public_read one. The system-write exemption is registered as row 47f on the system-context census. The "deliberate server-side copy runs as a system write" line is the shipped changeset's prescribed fix, restated.
  • "What it qualifies" / "Why this stays inside ADR-0104." Right. The exclusive row's "duplicated bytes, recoverable" rating is left intact and gains the precondition; exclusivity and copy-on-claim are not revisited; step 1 of the resequencing still deletes nothing.
  • Omission, non-blocking: the merged binding asks nothing on a kernel with no auth service, where the door binds no gate and serves every file. The addendum does not say so, but "by the download door's own verdict" covers it, since the reference rule is exactly as strong as the door there.
  • No sentence of the +60 is false of the merged code. Disclosure: the text states mechanism only, and nothing in it goes beyond what the merged PR body and shipped changeset already say publicly.

② Semver level

Docs-only diff, one file under docs/adr/, nothing from any released package. skip-changeset is the right level; the changeset check on this head is green.

③ Boundary flags

  • Tier H. The one path is docs/adr/**. This record is the contract-tier reading on the rules face; it does not lift the landing. The PR lands only on an authorized APPROVED review by a GOVERNED_APPROVERS account, then by the owning seat, as the PR body's acceptance note says. The PR is correctly still a draft.
  • Cross-reference, optional. D3's "Authorization" bullet ("possession of a URL stops being possession of the bytes") is the clause this rule extends to a file id; the addendum does not cite it. A one-line pointer there would land a grep from that bullet on the rule, but nothing conflicts and no clause of the 2026-07-27, 2026-08-07 or 2026-09-05 addenda is contradicted.
  • Precision, optional. The two non-blocking points above (where the public_read arm lives; the no-auth-service kernel) could each take one clause if the maintainer wants the ADR to match the binding line for line. Neither changes the rule.
  • Check-runs on this head, read at 2026-10-26T07:02Z: 57 check-runs, all completed; 38 success, 19 skipped, 0 failed, 0 pending. Governed Surface Queue Guard, Check Changeset, Check Documentation Links and the card-claim checks are among the green ones.

Implemented-by: claude/project-thread-n8sx5h
Reviewed-by: session_01TFnDdepdwyDJeM2kJ4GtMy

VERDICT: PASS


Generated by Claude Code

The 2026-10-26 addendum's quiet answer is scoped to the write door, says
what a kept id gives a reader and how step 5 classifies it, and names
#26043. The no-caller exemption names the runAs defaults of flows and
hooks and the API-key caller.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016DTRymn12to8YSGMcTzS7h
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 13d9a1adf7d301e437e4b4a650827270cf37079b
Local-runs: none
Scope: docs/adr/0104-field-runtime-value-shape-contract.md + packages/services/service-storage/src/file-reference-lifecycle.ts, packages/services/service-storage/src/storage-service-plugin.ts, packages/services/service-storage/src/attachment-access-hooks.ts, packages/services/service-storage/src/storage-routes.ts, packages/services/service-storage/src/verify-file-references.ts, packages/objectql/src/engine.ts, packages/objectql/src/hook-wrappers.ts, packages/spec/src/data/hook.zod.ts, packages/spec/src/automation/flow.zod.ts, packages/services/service-automation/src/runtime-identity.ts, packages/core/src/security/resolve-authz-context.ts, all read on main at 431f065177 (the lifecycle and plugin unchanged since 3761710927)

Face: rules (docs/adr/**). Follow-up record: 6410217722 PASSed head 3d14f9ae; this head is that head plus one wording commit (git diff 3d14f9ae..13d9a1ad, +18/-7 in the 2026-10-26 addendum). The whole +71 against main was re-read; the sentences the prior record judged are unchanged, and the judgments there stand. What follows judges the new sentences, each against the merged code. Inputs: the PR body, the diff, card #25692 and its comments, issue #26043, and the check-runs on this head.

① Derived judgments

"At the write door" (the qualifier). Right. The quiet answer is given in the lifecycle's beforeInsert / beforeUpdate and afterInsert / afterUpdate hooks, which is the record write; findReferenceableFile answers a file the caller may not read as null, the same answer the organization-scoped lookup gives a row it cannot see, and only there. The read side and the download door each answer differently (the refused marker; 403 FILE_DOWNLOAD_DENIED), so scoping the "cannot tell the two apart" statement to the write door is the precise statement, not a weaker one.

"Unless the record already owned the file, the kept id gives no reader anything through it." Right. On hydration (resolveFileReferences), an id is served by the record only through readParentDerivedFiles, whose lookup is ref_object = this object and ref_id in this result's record ids, and the hit is applied per record (hit.ownerId must equal the record's own id). A kept id names a file this record does not own, so that arm never serves it. A reader admitted to sys_file reads the row by sys_file's own rule, which is not through the record.

"A reader refused sys_file read gets the refused marker (metadataRefused)." Right. serveRefusedFileHydration marks every id its record does not own as { id, metadataRefused: true } (FileRefusedValueSchema), on both the admission-refused arm (#24335) and the read-refused arm (#6116).

"The download door judges the file by its own rule, never by this record." Right. judgeFileRead reads the file row's owner_id, then its own ref_object / ref_id, then its scope; the record holding the kept id is not an input. The door's public_read and no-user arms precede it in storage-routes.ts, as the prior record noted.

"A slot holding a file it does not own is a state that step 5's verify-references classifies as blocking." Right. Step 5's text names "a file held by a slot that does not own it" among the blocking kinds, and verify-file-references.ts puts both shapes the kept id can take, unowned_reference and foreign_owner, in BLOCKING_ISSUE_KINDS. The ADR is quoting its own step 5 accurately.

"#26043 tracks replacing this answer with a field-level refusal." Right. #26043 is open, asks for exactly that (a 400 VALIDATION_ERROR with an ADR-0114 invalid_value entry naming the field only), and names this addendum's paragraph as the text it will amend. An issue citation in an ADR is the ADR convention this file already follows.

"Everything else has a caller." Right. referencingCallerOf returns null only for a context with no session or a session marked isSystem; every other write, an anonymous session included, is a caller. This restates the merged code's own doc line.

"A flow runs as runAs: 'user' by default and is judged as the user it acts for." Right. FlowSchema.runAs is z.enum(['system', 'user']).default('user'); runtime-identity.ts builds a runAs: 'user' run's data context as the triggering user's principal (the decider after an approval decision, which "the user it acts for" covers), and its writes reach the lifecycle with that user's session.

"A hook's data operations take the context of the write that fired them by default (runAs: 'inherit'), or the triggering user's (runAs: 'user')." Right. HookSchema.runAs is z.enum(['system', 'user', 'inherit']).default('inherit'); hook-wrappers.ts installs nothing under 'inherit' (the engine-built ctx.api carries the firing write's context) and derives the triggering user's api under 'user'. The ADR's two clauses are the schema's describe text, condensed.

"An API-key caller is judged as the key's user." Right. resolveAuthzContext step 1 sets userId = keyPrincipal.userId and credential = 'api_key'; the write's session carries that user, and judgeFileRead reads context.userId. An API key resolves to no other identity.

"Only a declared runAs: 'system' run, or a write already made as the system, is exempt." Right. A 'system' flow run or hook writes under isSystem: true, which referencingCallerOf answers with no caller; so does an engine self-write, a migration or a seed. The exemption's extent is exactly referencingCallerOf's null arm, which the prior record checked against the system-context census (row 47f).

Disclosure. The new text states mechanism only: what a kept id yields on read and at the door, which identities count as callers, and a public follow-up card. Nothing in it goes beyond the merged code's own comments, the shipped changeset, and the public text of #26043.

Omissions, non-blocking. The prior record's two precision points stand unchanged (where the public_read arm lives; the kernel with no auth service). The new paragraph does not say that a sys_file-admitted reader hydrates the kept id's metadata by sys_file's own rule; "gives no reader anything through it" is true as worded, since that reader gets nothing the record adds.

No new sentence is false of the merged code.

② Semver level

Docs-only diff, one file under docs/adr/, nothing from any released package. skip-changeset holds; the Check Changeset runs on this head are skipped by that label.

③ Boundary flags

Implemented-by: claude/project-thread-n8sx5h
Reviewed-by: session_01TFnDdepdwyDJeM2kJ4GtMy

VERDICT: PASS


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review October 26, 2026 10:26
@os-zhuang
os-zhuang added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit b2971aa Oct 26, 2026
54 checks passed
@os-zhuang
os-zhuang deleted the claude/project-thread-n8sx5h branch October 26, 2026 11:22
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/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants