Skip to content

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

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/sec-hardening-v
Nov 4, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/sec-hardening-v

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: updated here — content/docs/permissions/attachments-access.mdx now states that a sys_file created through the data API records the signed-in user as owner_id, and that such a create with no signed-in user is refused. No published skill teaches writing sys_file.owner_id (git grep over skills/ finds none).

维护者速读

  • 改了什么:非系统上下文创建 sys_file 时,owner_id 一律记为当前会话用户(覆盖请求里带的值),与附件 uploaded_by、评论 author_id 同一规则;会话没有登录用户的创建被拒绝(403 RECORD_NOT_ACCESSIBLE)。
  • 为什么改:一个文件的上传者就是创建它的人;出处只在创建时记录一次,之后只读(更新侧已是只读)。
  • 风险与回滚:收窄(BREAKING)。经数据接口创建 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.

  • Create. A non-system create of sys_file records the session's user as owner_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 existing sys_file create guard (installSysFileStorageOwnedColumnsGuard), beside the storage-owned column strip, so sys_file keeps one beforeInsert registration.
  • Refused. A non-system create whose session names no signed-in user is refused with 403 RECORD_NOT_ACCESSIBLE, the code and status the attachment and comment hooks answer their own no-user creates with.
  • Unchanged. The system context keeps the owner_id it 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_id is already read-only after creation, so an update drops a non-system caller's value and reports it in droppedFields.
  • One difference from the attachment hook, by design. The attachment hook passes a context-less call entirely. Here the carve-out covers the uploader only: the storage-owned and file-reference columns are still dropped from a context-less create, because that strip judges every write that is not isSystem (as the update strip does) and this rule does not loosen it.
  • Silent, as the attachment and comment stamps are. The engine reports droppedFields / onFieldsDropped only for strips it runs itself, it runs none on a sys_ 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 as uploaded_by and author_id are not.
  • Docs. content/docs/permissions/attachments-access.mdx, and the owner_id comment in objects/system-file.object.ts.

Tests

  • Unit, at the guard (new 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 403 RECORD_NOT_ACCESSIBLE with 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.
  • Integration, real engine over sqlite (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.
  • Dogfood, at the data door (attachments-permission-matrix.dogfood.test.ts, uploader block): an administrator's POST /data/sys_file naming 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 of owner_id answers 200 with owner_id in droppedFields, and (e) the upload door stamps the uploading member.
  • Ablation, one shot (fix committed first; mutation through scripts/ablation-replace.mjs, which restored on exit): the stamp line replaced by a no-op, service-storage rebuilt, ablation-dist-preflight proved the line absent from dist/. 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 HEAD empty, rebuilt, preflight proved the mutation absent and the line present in dist/, both runs green again.

Acceptance notes

  • sys_file.owner_id has 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's owner_id, the holder of the ownership-transfer grant included (measured on origin/main 917f024: PATCH 200, droppedFields names owner_id, stored value unchanged). This PR keeps that; only the system context records a different uploader.
  • No UI or service creates sys_file through the data API (git grep over ui/ and packages/ finds the one writer, StorageMetadataStore.createFile, under the system context), so no caller changes.

Verification

Readings on 3aa77b9a7f (origin/main 232d3bb merged in; it touches no package) unless stated. The head 89c379ba6e adds one comment-only commit (the guard's docblock on ctx.notice). At that head, the guard tests read Test 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=2 under scripts/pm/os-verify-lock.sh: Test Files 72 passed (72), Tests 1341 passed (1341), lock VERDICT command-exit 0.
  • pnpm --filter @objectstack/service-storage typecheck: exit 0, including check:test-typecheck: OK; tsc --listFiles on tsconfig.test.json lists the guard and both test files.
  • pnpm --filter @objectstack/dogfood typecheck: exit 0; its program lists attachments-permission-matrix.dogfood.test.ts.
Derived gates (dispatch-gates --commands, 94 families)
  • 91 exit 0, each exit code captured before any pipe.
  • 3 NOT MEASURED, each answered PREREQUISITE NOT MET (exit 3) and measured nothing: check:skill-examples (no built client-react declarations in this worktree), check:dual-build-cjs-loads (seven packages outside the dogfood build closure have no dist/), check:type-check-debt (its closure build never got a turn on the shared verify lock). CI's Lint & Repo Gates runs all three on a full build.
  • dispatch-gates --ran: 94 derived famil(ies) accounted for — 91 run, 3 NOT-MEASURED, exit 0.
Dogfood battery
  • Full pnpm --filter @objectstack/dogfood test on 3aa77b9 under os-verify-lock: VERDICT command-exit 0; Test Files 417 passed | 3 skipped (420); Tests 3464 passed | 28 skipped (3492)

…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>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

35 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 9a53fc9feac04d44f40fffc62f93d04433de1227.

⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 103 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 10 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 9a53fc9feac04d44f40fffc62f93d04433de1227 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 169a5cc19f9a5bd6c6750044994fc6dbd78d5e23 — the merge of head 89c379ba6e21e5650ad7e8900a5fdc361942d8c9 into base 9a53fc9feac04d44f40fffc62f93d04433de1227, 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 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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

Contract-review-claim: session_01QSeKYn5CmmdShCJifm6WYj · head 3aa77b9 · 2026-11-04T19:46Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 3aa77b9a7f45449166602cfaa2ea550eaf40fad5
Local-runs: none
Scope: packages/services/service-storage/src/sys-file-storage-owned-columns.ts, packages/services/service-storage/src/sys-file-storage-owned-columns.test.ts, packages/services/service-storage/src/sys-file-storage-owned-columns.integration.test.ts, packages/services/service-storage/src/objects/system-file.object.ts, packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts, content/docs/permissions/attachments-access.mdx, .changeset/sys-file-uploader-is-its-creator.md + packages/services/service-storage/src/attachment-access-hooks.ts, packages/plugins/plugin-audit/src/comment-access-hooks.ts, packages/services/service-storage/src/storage-service-plugin.ts, packages/services/service-storage/src/metadata-store.ts, packages/services/service-storage/src/file-reference-lifecycle.ts, packages/objectql/src/engine-execution-plumbing.ts, packages/objectql/src/validation/rule-validator.ts, packages/core/src/security/assemble-execution-context.ts, packages/spec/src/data/hook.zod.ts

Face: Clause-②: no (narrowing), !. Net diff read against merge-base 232d3bb5ef (7 files, +280/-10); all cited code read on origin/main at 9a53fc9fea. Every non-skipped check-run on the head is success, the ten required contexts included (Lint UI, Type Check UI and Build & E2E UI skipped by path filter). Precedent #29868 (merged 6f35b746f0) read.

① Derived judgments

  • Placement. The stamp sits inside the existing beforeInsert handler of installSysFileStorageOwnedColumnsGuard, after the isSystem return and the data-shape guard, before the lifecycle-column strip. On main, sys_file has exactly one registration naming object: 'sys_file' (installed once in storage-service-plugin.ts at kernel:ready); the only other beforeInsert registrations in service-storage target sys_attachment or are wildcard by packageId. The PR adds no registration, so sys_file keeps one. The unit test pins the registration list as [['beforeInsert', 'sys_file']]. Holds.
  • Match to the attachment hook. Same three legs as attachment-access-hooks.ts's create hook and comment-access-hooks.ts's: isSystem keeps the supplied value; a non-system session records session.userId over whatever the create carried; a session whose userId is not a non-empty string is refused 403 with the same catalog code the two siblings refuse theirs with. The bare-kernel carve-out (no session at all keeps the supplied value) is the siblings' if (!ctx?.session) return, expressed here as if (ctx?.session) recordUploader(...). Holds.
  • The deliberate difference. Right. The lifecycle-column strip was already judged "every write whose context is not isSystem, a context-less one included" on main, and that is the update strip's rule too; the uploader rule has nobody to record on a context-less call, so it keeps what was supplied, as the siblings do. Narrowing the uploader rule did not have to widen the strip, and the PR does not. The unit test pins both halves on one context-less call (value kept, status reset to pending, key dropped).
  • Silence. The overwrite is silent, as uploaded_by and author_id are. The engine's insert-side readonly strip returns a null subject for any sys_ object (staticReadonlyInsertSubject), so no droppedFields entry is produced on this create and nothing is reported. The dev's supporting claim is overstated, though: the hook contract does carry a member a hook can speak to the requester through, ctx.notice (the write's onNotice listener, bound on every insert dispatch). It carries a free-text save notice, not a dropped-field report, and neither sibling stamp uses it, so silence stays the right reading of the ruling ("as the attachment and comment create hooks stamp"). The sentence in the module docblock and PR body saying the contract declares no such member should be corrected (should-fix, below).
  • Unchanged surfaces. The upload doors write through StorageMetadataStore.createFile under systemInsertOptionsFor (isSystem: true), which the hook returns on first; the diff touches none of presign, chunked or complete, and the integration test pins both doors still recording the uploading user. The update path is untouched: owner_id is readonly: true on main and the engine's update strip handles it; the integration test pins an update leaving the create's uploader and a system update changing it. git grep on main finds one data-API writer of sys_file outside tests, that same store under the system context, so no caller in packages/ or ui/ changes.

② Semver level

  • patch on @objectstack/service-storage with ! and a BREAKING banner: correct. Metadata-protocol: none is right (no key, value or type an author writes moves), and under the level rule a none kind takes patch whatever the breakage, keeping its banner and ADR-0087 disposition.
  • Clause-②: no (narrowing): correct arm. A runtime accept set narrows (a supplied owner_id no longer lands on a non-system create; a no-user create is refused); nothing widens.
  • ADR-0087 not-required (no-migration-prescription): correct. No stored row is read, rewritten or dropped; no spec key, export or stored shape moves; the remedy for a writer that needs a different uploader recorded is a system-context write, which no ledger entry can derive. The changeset closes the other categories on facts, as the marker form asks.
  • The BREAKING banner names who moves (a client creating sys_file through the data API; a server-side caller creating with no signed-in user) and the fix for each (the row is recorded under the caller; write from a system context, or run as the system principal or name the user acted for). The console and upload doors are named as needing nothing. Check Changeset and the PR-body guards are green on the head. Holds.

③ Boundary flags

  • The order's update-time transfer assumption. The dev is right and A is the answer. Since fix(plugin-audit, service-storage)!: a row's author or uploader is read-only after creation, like the comment's display name #29868, sys_file.owner_id is readonly: true, so the engine's update strip drops a non-system caller's value, the holder of the ownership-transfer grant included (pinned on main by the dogfood block's PATCH case: 200 with owner_id in droppedFields, stored value unchanged; the docs page says so in words). An update-time transfer under allowTransfer is not a path that exists on main, and this ruling's provenance is fix(plugin-audit, service-storage)!: a row's author or uploader is read-only after creation, like the comment's display name #29868's rule that authorship is read-only after creation. Nothing in the PR changes; the seat corrects its own order wording.
  • The dev's private lead (2). Read at the plumbing, buildSession copies userId, tenantId, positions, accessToken, isSystem, actor and three opt-out flags, then returns undefined when every one is undefined; principalKind is not copied. So a context carrying only principalKind would indeed read as context-less to every sibling carve-out. But no guest door can produce that shape: the one producer of principalKind on main is assembleExecutionContextOrGuest, whose guest envelope always carries positions: ['guest'] (emitted, since defined), so buildSession yields a session with no userId, and the create is refused, not admitted. No non-test source constructs principalKind by hand (every other hit is a comment). An agent envelope keeps userId as the human. Verdict: not a must-measure follow-up; the measurement is done by reading. The carve-out's safety is keyed on the guest envelope carrying a session-bearing field rather than on principalKind, which is worth one pin in the plumbing's own test (should-fix follow-up, not in this PR).
  • The Verification block. The body says the dogfood battery is NOT MEASURED on this head and points to the dev report; the battery has since run green at the head (417 files / 3464 tests). A reading cited is a count plus the tree it was taken against, and the body is the record a later reader greps, so the seat corrects that block to carry the reading before landing. Should-fix, not blocking: the body states a conservative truth as of writing, not a false one, and the head's Dogfood Regression Gate check-run is independent green evidence.

④ Design judgment

Judged against the published surface on main and the ruling: the narrowing makes a provenance rule real at the one door it was missing from (a sys_file created through the data API), aligned with the two sibling stamps and the read-only-after-creation rule of #29868. Attempts to break it:

  • owner_id null or empty on a create. The stamp assigns session.userId unconditionally after the no-user check, so a null, empty or absent value is overwritten the same as another user's id; the unit test pins the absent case. Holds.
  • Bulk create. Both batch doors reach the engine's insert/insertMany, whose beforeInsert runs once per row with that row as input.data (pinned on main by the engine's insert-many test), so the guard's early return on an array input is never the path a batch takes. The integration test pins a two-row batch recording the caller on each row. Holds.
  • Impersonating or on-behalf-of caller. Impersonation mints a session for the target user, so userId is the impersonated user and the row records them, as uploaded_by and author_id would. An agent acting on behalf of a human keeps userId as the human, with the client on provenance.performedBy; the row records the human, as the siblings do. Consistent with the ruling; no finding.
  • Guest or service-principal caller. A guest envelope and a service-principal context (actor only) both yield a session with no userId and are refused; the integration test pins the service-principal case writing no row. Fail-closed. Holds.
  • Docs page claims. Each claim on attachments-access.mdx matches the code: data-API create records the signed-in user; no-user create refused 403 RECORD_NOT_ACCESSIBLE; update drops a non-system caller's value and reports it. The closing sentence "Only a system-context write records a different uploader" omits the context-less bare-kernel case the changeset names; the page addresses the platform doors, where no such call exists, so no change is required (note only).
  • §6 rule-level check. Title, body, changeset, commit messages, docs page and every test name state the hardening rule and its controls, none an attack; the module docblock likewise. No breach.

Findings:

  • Should-fix (this PR, comment-only): the sentence "the hook contract declares no member through which a hook reports one" in the sys-file-storage-owned-columns.ts docblock and the PR body. The contract carries ctx.notice; what it lacks is a dropped-field report, and the sibling stamps use no notice either. Say that instead.
  • Should-fix (PR body, before landing): replace the NOT MEASURED dogfood line with the reading at the head.
  • Should-fix (follow-up, not this PR): a pin that buildSession of a guest envelope is defined, so the context-less carve-out never admits an anonymous request.
  • Must-fix: none.

Implemented-by: claude/sec-hardening-v
Reviewed-by: session_01QSeKYn5CmmdShCJifm6WYj

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>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 89c379ba6e21e5650ad7e8900a5fdc361942d8c9
Local-runs: none
Scope: packages/services/service-storage/src/sys-file-storage-owned-columns.ts + packages/spec/src/data/hook.zod.ts

Delta review: the prior PASS stands at 3aa77b9a7f; this record judges git diff 3aa77b9a7f..89c379ba6e, the rewritten PR body and the new head's check-runs. The delta is one commit, comment-only, in the guard's module docblock (5 insertions, 4 deletions, no code line moved). Face unchanged: Clause-②: no (narrowing), !.

① Derived judgments

  • The one hunk rewrites the "silence" paragraph of the docblock: it now says the engine reports a dropped field only for the strips it runs itself and runs none on a sys_ object's create, that the hook contract carries no dropped-field report, that its caller-facing member ctx.notice is a free-text save notice, and that neither sibling stamp uses it. Each clause matches the contract on main (HookContext.notice, a HookNoticeSetter bound at every insert/update dispatch and answered as the write's notice; no droppedFields member on the context) and the two sibling hooks (no notice call in either create stamp). The prior should-fix 1 is applied as asked.
  • No behaviour moves: the handler, recordUploader, refuseNoUploader, the strip and the registration are byte-identical to the reviewed head. The reviewed verdicts on placement, sibling match, the deliberate difference and the unchanged surfaces carry over.

② Semver level

Unchanged and still correct: patch on @objectstack/service-storage with !, Clause-②: no (narrowing), Metadata-protocol: none, ADR-0087 not-required (no-migration-prescription); the changeset is untouched by the delta.

③ Boundary flags

  • PR body. The same sentence is corrected in the body ("the hook contract carries no dropped-field report: its one caller-facing member, ctx.notice, is a free-text save notice"), so body and docblock agree with each other and with the contract.
  • Verification reading. The body now carries the dogfood battery as a count plus the tree it was taken against (on 3aa77b9a7f under the verify lock: command-exit 0, Test Files 417 passed and 3 skipped of 420, Tests 3464 passed and 28 skipped of 3492), and states that 89c379ba6e adds one comment-only commit on which the guard tests (2 files, 38 tests) and the 49 derived gate families for the changed file were re-read green. A comment-only delta needs no battery re-run; the body says which tree each reading is from. Prior should-fix 2 is applied.
  • Check-runs on the new head. All ten required contexts are green: Lint & Repo Gates, TypeScript Type Check, Test Core, Build Core, Governed Surface Queue Guard, Test UI and Live E2E UI success; Lint UI, Type Check UI and Build & E2E UI skipped by path filter. Dogfood Regression Gate success. Two advisory runs (Check Changeset, PR-body guards) show a second in_progress instance started after the body edit; their first instances on this same head completed success, and neither is in the required set.
  • Still open, not for this PR: the follow-up pin that buildSession of a guest envelope is defined (prior should-fix 3).

④ Design judgment

The 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 (api is data access, notice the only answer-bound member). No finding; no must-fix.

Implemented-by: claude/sec-hardening-v
Reviewed-by: session_01QSeKYn5CmmdShCJifm6WYj

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

席位核对

ACCEPT · head 89c379ba6e21e5650ad7e8900a5fdc361942d8c9 · 2026-11-04T20:33Z

  • Shape: 7 files, +281/−10. Not governed. Under 3000. Patch for service-storage, with !, Clause-②: no (narrowing), Metadata-protocol: none, ADR-0087 not-required (no-migration-prescription).
  • Rule held (security-channel ruling of 2026-11-04, batch 4 item 7, under fix(plugin-audit, service-storage)!: a row's author or uploader is read-only after creation, like the comment's display name #29868): a stored file's uploader is the user who created it, the same rule as an attachment's and a comment's author.
    • A non-system create of sys_file records the session user over whatever it supplied.
    • A create with no signed-in user is refused.
    • System context and the upload doors are unchanged.
  • Contract review: full record PASS at 3aa77b9a7f. The delta record on this head is comment 6641419007: Served-tier: CONTRACT_REVIEW_TIER, Head-sha equals this head, VERDICT PASS, no must-fix. Two of the review's should-fixes are applied (the docblock sentence and the Verification reading).
  • Check-runs at this head (latest per name): 14 skipped, 36 success. mergeable_state: clean.
  • Gates (dev): 49 derived families run, 0 NOT MEASURED. Guard tests: 38 passed. Full dogfood battery at the code head: 417 files, 3464 tests. Ablation: red, then restored.
  • Seat correction: the dispatch order assumed an update-time transfer of this column under the transfer grant. There is no such path: fix(plugin-audit, service-storage)!: a row's author or uploader is read-only after creation, like the comment's display name #29868 made the column read-only. The order's wording was wrong; the PR is unchanged.
  • Follow-up (separate order G2, running): a pin that a guest request always reaches create hooks with a session.
  • Landing: ready, then auto-merge.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 4, 2026 20:35
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit 59e6463 Nov 4, 2026
55 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/sec-hardening-v branch November 4, 2026 20:57
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant