Repository navigation
Commit a0413fc
fix(service-storage): sys_file's storage-owned columns (key, mime_type, size, status, etag) are not writable outside the storage service (#27765)
domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F
<!-- claude-projects-attribution: {"github_login":"os-sales"} -->
_Requested by **Sales** · [project
thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuPMZ8BHWtPf6nBTp51qSM64)_
Fixes #27399
domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F (card
thread session_01L6bhgWddm8zjbee2kUdmmA)
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — no file under `skills/**` and no hand-written
`content/docs` page teaches writing `sys_file`'s `key`, `mime_type`,
`size`, `status` or `etag` through the data door (`git grep` for those
columns beside `sys_file` outside `content/docs/references/` and the
release notes: no such sentence); every taught upload path goes through
the storage upload doors, which this change keeps.
Claim: 6508175120 (seat `domain:server#2`, branch
`claude/project-thread-36yup5`, the card thread's harness-pinned
branch). Triage direction: 6489236866. Ruling: none.
## 维护者速读
- **改了什么**:`sys_file` 里由存储服务记账的五列(存储键
`key`、`mime_type`、`size`、`status`、`etag`)对数据入口变成只读:非系统的更新(按 id
或按条件)不再能改它们,其余字段照常落地;非系统的新建会丢掉调用方给这五列的值,并以 `pending`
起步。存储服务自己的写入(预签名、上传完成、分片上传、文件字段认领/释放/复制、附件墓碑)全部走系统上下文,不受影响。
- **为什么改**:文件字段靠 `status` 拒绝没传完的文件,靠 `size` / `mime_type` 判 `accept` /
`maxSize`。之前管理员可以 `PATCH /api/v1/data/sys_file/ID` 把一个从没收到字节的上传改成
`committed`,再被记录的文件字段认领;或事后改大小和类型骗过字段约束。
- **风险与回滚**:收窄(narrowing),但不新增拒绝:更新照样回 200,只是这五列保持存储服务写的值(引擎按惯例记一条
warn);新建照样成功,只是以 `pending` 起步、五列为空。只影响直接改 `sys_file` 这几列的管理脚本。回滚 =
revert 本 PR,无数据迁移。
- **先看**:①
`packages/services/service-storage/src/sys-file-storage-owned-columns.ts`(新建侧守卫,`isSystem`
豁免那一行决定存储服务自己的写入不被拦);②
`packages/services/service-storage/src/objects/system-file.object.ts`(五列
`readonly` 与列清单);③
`packages/qa/dogfood/test/sys-file-metadata-write-refusal.dogfood.test.ts`(#11076
管理员对照钉子改为「请求到达、`name` 落地、两列保持原值」)。
- **两分钟验证**:`pnpm --filter @objectstack/service-storage exec vitest run
src/sys-file-storage-owned-columns.integration.test.ts`,预期 12
个用例全绿(卡片复现、五列逐一更新、条件更新、单条与批量新建、上传→完成→认领全链路、系统新建保值)。
- **需要你做的**:无,席位落地(合入队列前由席位安排契约复核)。
## What changes
- **The column list.** `SYS_FILE_STORAGE_OWNED_COLUMNS` in
`system-file.object.ts` names the five columns the storage service owns:
`key` (where the bytes live), `mime_type`, `size` and `etag` (what the
backend was told and answered) and `status` (how far the upload got).
`key` and `etag` are in the same class as the three the card names: a
forged `key` points a row at bytes it never uploaded, and `etag` is the
backend's own answer at `/upload/complete`. The ownership columns
(`ref_*`, `released_ref_*`), `name`, `acl`, `scope`, `owner_id` and
`metadata` are not in it and stay as they were. The constant is
module-internal (not re-exported from `src/index.ts`), so the published
surface does not move.
- **Update.** Each of the five is declared `readonly: true`. Measured:
the engine's static readonly strip runs on an UPDATE of every object,
`sys_` included, for every context that is not `isSystem`
(`staticReadonlyStripRuns = !opCtx.context?.isSystem`), by id and by
predicate. It STRIPS, it does not refuse: the caller's value is taken
off, the rest of the payload lands, the write answers as before and
`this.logger` gets the readonly warn. Only a caller passing
`strictReadonlyWrites` is refused (`ERR_READONLY_FIELD_REJECTED`), as
for every readonly column; the data doors do not pass it.
- **Create.** Measured: on an INSERT, `staticReadonlyInsertSubject`
(`@objectstack/objectql`) returns `null` for every `sys_` object,
leaving its columns to the object's own guard, so `readonly` alone does
not touch a non-system create. The new
`installSysFileStorageOwnedColumnsGuard`
(`sys-file-storage-owned-columns.ts`) is that guard: a `beforeInsert`
hook on `sys_file`, installed by `StorageServicePlugin` at
`kernel:ready` beside the attachment lifecycle hooks. For a write whose
context is not `isSystem` (context-less included, as the update strip
judges), it deletes any of the five the payload carries, sets `status:
'pending'` and logs one `warn` naming the dropped columns. It does not
refuse: the row is written with the caller's other columns, a file field
refuses it (it is `pending`), and the lifecycle's `pending` retention
collects it. ⛔ `packages/objectql`'s `sys_` insert exemption is not
touched.
- **The system-context census.** The guard's `isSystem` return is one
elevation read, so `content/docs/permissions/system-context.mdx` gains
row 47g for it (`installSysFileStorageOwnedColumnsGuard`) and its
generated site register is rewritten by `check-system-context-census.mjs
--write`.
- **Every writer of these columns is already system context** (measured,
nothing changed): `StorageMetadataStore.createFile` / `updateFile` /
`deleteFile` (`systemInsertOptionsFor`, `{ isSystem: true, tenantId?
}`), which the presigned, PUT, `/upload/complete`, chunked upload
(`storage-routes.ts`), `field-file-service.ts` and
`file-attachment-service.ts` doors all go through;
`file-reference-lifecycle.ts` (claim, release, copy insert:
`SYSTEM_CTX`); `attachment-lifecycle.ts` (tombstone, re-attach, reap
guard: `SYSTEM_CTX`); `backfill-file-references.ts` (`SYSTEM_CTX`). The
two backfills (`backfill-sys-file-organizations.ts`,
`backfill-sys-file-public-scope.ts`) write only `organization_id` /
`scope`. No `sys_file` write outside `service-storage` (`git grep` for
an insert / update of `'sys_file'` in `packages/**` source).
## Tests
- New
`service-storage/src/sys-file-storage-owned-columns.integration.test.ts`,
the #26479 rig (real `ObjectQL` over sqlite `:memory:`,
`StorageServicePlugin` on the local adapter, the upload doors called as
routes, data writes as a signed-in caller with no system context), 12
cases:
- the list is exactly the five, each `readonly` on `SystemFile`;
- the card's steps 1 to 5: presign, PATCH `{ status: 'committed', size:
999 }`, the row keeps every storage-owned value (still `pending`), and a
file field still refuses the file with the #26479 envelope (`400
VALIDATION_FAILED`, `invalid_value` on `poster`);
- an update by id cannot change each column (`it.each` over the five),
and `name` in the same payload lands;
- a predicate update cannot change any of them, and `acl` in the same
payload lands;
- a create cannot set any of them: the row is `pending` with all five
null, `name` kept, and a file field refuses it; a batch create the same
on every row;
- the service's own presign → PUT → `/upload/complete` → claim path
still writes every column (`mime_type`, `size`, `status: committed`,
`etag` from complete, a `key` holding the file id) and a record's file
field claims it;
- a system create keeps every value it writes.
- Moved pin, dogfood: `sys-file-metadata-write-refusal.dogfood.test.ts`
(the #11076 RBAC refusal pin). Its non-admin cases (403
`PERMISSION_DENIED`, columns unchanged) are untouched. Its
platform-admin contrast control asserted the admin's PATCH of
`mime_type` / `size` LANDS, the exact behaviour this card reverses (the
full battery's one red on the first run). The control now sends `name`
in the same payload and asserts 200, that `name` landed (so the request
still reaches the row) and that `mime_type` / `size` keep the seeded
values. The header's sentences that said the columns carry no `readonly`
are updated; the file always said it was written to survive this posture
call.
- Moved pin: `storage-service-plugin.test.ts`'s wiring case asserted
every scoped hook names `sys_attachment`; it now counts the guard's one
`beforeInsert` on `sys_file` separately and pins it.
## Acceptance notes
- **Ablation, three legs, each red then restored**
(`scripts/ablation-replace.mjs`, fix committed first; restore blob ==
HEAD and `git diff HEAD` empty on each). The suite imports
`service-storage`'s `src/` directly, so no `dist/` rebuild is in any
leg.
- Leg 1, every `readonly: true,` in `system-file.object.ts` to `false`
(6 hits, `id` included): 10 failed / 2 passed. Red: the list pin, the
card's steps, all five update-by-id cases, the predicate update, both
creates. Green: the end-to-end path and the system create.
- Leg 2, delete the `installSysFileStorageOwnedColumnsGuard(...)` call
in the plugin: 2 failed / 10 passed (the single and the batch create).
- Leg 3, delete the guard's `isSystem` exemption: 8 failed / 4 passed.
Red: the end-to-end path and the system create (the presign's own insert
lost its `key` and `size`), and the update cases whose committed fixture
is built by that path.
- A first run of leg 1 left the status-by-id case green because the
forged value equalled the committed one; the fixture now forges
`pending` over a committed file, and the leg above is the rerun.
- Dispatch-order conflicts, stated: the order asked for `pnpm lint`;
`.claude/agents/os-dev.md` makes the repo-wide sweep CI's run, so eslint
ran on the changed package (and `Lint & Repo Gates` runs the sweep). The
order asked for "no new refusal": none is added on either verb.
- Not handled, noted: the create guard does not read
`strictReadonlyWrites` (the engine's insert strip would refuse under it
on a non-`sys_` object); no data door passes that option.
- Out of scope, noted (code reading, no door measured): the ownership
columns `ref_object` / `ref_id` / `ref_field` and `released_ref_*` are
also plain writable fields to a caller allowed to write `sys_file`;
whether that is a defect is a separate question from this card's
lifecycle columns.
- Branch name is the card thread's harness-pinned
`claude/project-thread-36yup5`, not the `claude/issue-N-slug`
convention; the claim above is its identity record.
## Verification
All on head `aebcc2cd` (fix and tests `886ec313`..`f886aad7`, merged
with `origin/main` at `6f79d563`, then the ADR-0087 marker, the dogfood
control and the census row).
<details><summary>Package suite, typecheck, eslint</summary>
- `pnpm --filter @objectstack/service-storage test && … typecheck` under
`os-verify-lock.sh` on `aebcc2cd`: `VERDICT command-exit 0`, Test Files
71 passed (71), Tests 1299 passed (1299); the test-layer typecheck
reports 0 errors.
- `eslint --no-inline-config packages/services/service-storage
packages/qa/dogfood/test/sys-file-metadata-write-refusal.dogfood.test.ts`:
exit 0. The repo-wide `pnpm lint` sweep is left to `Lint & Repo Gates`.
- The new suite alone: 12 passed (12).
</details>
<details><summary>Dogfood battery</summary>
- First run (head `6f79d563`, closure built with `turbo run build
--filter='@objectstack/dogfood^...'`): 1 failed / 353 passed / 3 skipped
files, Tests 1 failed / 2857 passed / 20 skipped:
`sys-file-metadata-write-refusal.dogfood.test.ts` › the platform-admin
contrast control (received the seeded `application/octet-stream` / 11,
expected the admin's values). Moved as described under Tests; that file
alone then 6 passed (6).
- Final run on `aebcc2cd`, closure rebuilt then `pnpm --filter
@objectstack/dogfood test` under `os-verify-lock.sh`, no shard and no
path: `VERDICT command-exit 0`, Test Files 354 passed / 3 skipped (357),
Tests 2858 passed / 20 skipped (2878).
</details>
<details><summary>Derived gates</summary>
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 94 families derived at `aebcc2cd`; all 94
run on that head, each `exit 0`.
- Along the way: `check:adr-0087-registration` first red (no disposition
marker; `no-migration-prescription` added);
`check:system-context-census` first red (the guard's `isSystem` read had
no row; row 47g added); `check:plugin-teardown-shape --self-test` first
exit 3 (shallow clone; its pinned fixture commit fetched);
`check:dual-build-cjs-loads` first exit 3 (7 dists missing; built).
- `--ran` reconcile: `94 derived famil(ies) accounted for — 94 run, 0
NOT-MEASURED (a DERIVED zero — all 94 recorded an exit code and none of
them is 3)`.
- No open PR edits these files (name-only diffs of the 17 open PRs'
heads against `origin/main`), so no `merge-tree` pair was owed.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01L6bhgWddm8zjbee2kUdmmA)_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01L6bhgWddm8zjbee2kUdmmA
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 64599ec commit a0413fc
8 files changed
Lines changed: 485 additions & 23 deletions
File tree
- .changeset
- content/docs/permissions
- packages
- qa/dogfood/test
- services/service-storage/src
- objects
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
184 | 184 | | |
185 | 185 | | |
186 | 186 | | |
| 187 | + | |
187 | 188 | | |
188 | 189 | | |
189 | 190 | | |
| |||
659 | 660 | | |
660 | 661 | | |
661 | 662 | | |
| 663 | + | |
662 | 664 | | |
663 | 665 | | |
664 | 666 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
| 4 | + | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
27 | 29 | | |
28 | 30 | | |
29 | 31 | | |
30 | 32 | | |
31 | 33 | | |
32 | 34 | | |
33 | 35 | | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | | - | |
38 | | - | |
39 | | - | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
48 | | - | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
49 | 51 | | |
50 | 52 | | |
51 | 53 | | |
| |||
232 | 234 | | |
233 | 235 | | |
234 | 236 | | |
235 | | - | |
| 237 | + | |
236 | 238 | | |
237 | | - | |
| 239 | + | |
| 240 | + | |
238 | 241 | | |
239 | | - | |
| 242 | + | |
240 | 243 | | |
241 | 244 | | |
242 | | - | |
243 | | - | |
244 | | - | |
245 | | - | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
246 | 253 | | |
247 | 254 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
27 | 48 | | |
28 | 49 | | |
29 | 50 | | |
| |||
57 | 78 | | |
58 | 79 | | |
59 | 80 | | |
| 81 | + | |
60 | 82 | | |
61 | 83 | | |
62 | 84 | | |
63 | 85 | | |
| 86 | + | |
64 | 87 | | |
65 | 88 | | |
66 | 89 | | |
| |||
69 | 92 | | |
70 | 93 | | |
71 | 94 | | |
| 95 | + | |
72 | 96 | | |
73 | 97 | | |
| 98 | + | |
74 | 99 | | |
75 | 100 | | |
| 101 | + | |
76 | 102 | | |
77 | 103 | | |
| 104 | + | |
78 | 105 | | |
79 | 106 | | |
80 | 107 | | |
| |||
104 | 131 | | |
105 | 132 | | |
106 | 133 | | |
| 134 | + | |
107 | 135 | | |
108 | 136 | | |
109 | 137 | | |
| 138 | + | |
110 | 139 | | |
111 | 140 | | |
112 | 141 | | |
113 | 142 | | |
114 | 143 | | |
115 | 144 | | |
116 | 145 | | |
| 146 | + | |
117 | 147 | | |
118 | 148 | | |
| 149 | + | |
119 | 150 | | |
120 | 151 | | |
121 | 152 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
432 | 432 | | |
433 | 433 | | |
434 | 434 | | |
| 435 | + | |
435 | 436 | | |
436 | 437 | | |
437 | 438 | | |
438 | 439 | | |
439 | | - | |
440 | | - | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
441 | 443 | | |
442 | 444 | | |
443 | 445 | | |
444 | 446 | | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
445 | 451 | | |
446 | 452 | | |
447 | 453 | | |
| |||
488 | 494 | | |
489 | 495 | | |
490 | 496 | | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
491 | 500 | | |
492 | 501 | | |
493 | 502 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
| 66 | + | |
66 | 67 | | |
67 | 68 | | |
68 | 69 | | |
| |||
488 | 489 | | |
489 | 490 | | |
490 | 491 | | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
491 | 496 | | |
492 | 497 | | |
493 | 498 | | |
| |||
0 commit comments