Repository navigation
Commit 5210acf
security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:服务端附件动词 `attachFileAs` 以调用者身份插入 `sys_attachment`
时,为它在同一次调用里刚创建的那一个 `sys_file`
担保(`vouchForReference`,`@objectstack/core`);写入时的引用校验对这一个对象/字段/id
不再问调用者的读权限,但仍探测它存在。其余一切仍按调用者判定。
- 为什么改:#31031 之后 `lookup` 引用只能指向调用者读得到的记录,而成员一般读不到 `sys_file`,于是成员经
`attachFileAs` 上传附件、以及打印存档到附件(`printRecordToFile`)一律失败(500,"File: no
sys_file record has id …")。本 PR 恢复这两条路径,#31031 的通用规则不变。
- 风险与回滚:风险低。担保是 symbol 键的上下文属性,请求体、查询、存储行、流程定义都拼不出来;只对该次插入、该字段、该 id
生效。任何数据门(REST、MCP、流程、导入)送来的 `sys_file` id 照旧按调用者读权限判。回滚即撤本 PR,成员附件会重新失败。
- 先看:`packages/objectql/src/engine-referential-integrity.ts` 中
`callerReads`
一行;`packages/services/service-storage/src/file-attachment-service.ts` 步骤
⑤ 的插入;`packages/core/src/security/vouched-reference.ts`。
- 两分钟验证:`pnpm --filter @objectstack/service-storage exec vitest run
src/file-attachment-service.test.ts`,预期 32/32 绿,其中成员上传附件一组 7 条。
- 需要你做的:请决定下方 Acceptance notes 第一条(浏览器附件面板的两步上传同样被 #31031 拦下)走哪条路。
## What changes
Restores member attachments after #31031. That PR made a non-system
write's `lookup` / `master_detail` / `user` reference resolve only
through the caller's own read door, and that rule stands. The
server-side attach verb writes its `sys_attachment` row as the caller,
and that row's `file_id` names the `sys_file` the same call has just
created for the caller. A member who may attach files need not read
`sys_file`. So the reference check refused the verb's own row, and both
`attachFileAs` and print-to-attachment failed for every such member.
**The rule.** A service verb that has created a record in the same call
may vouch, on its own write, for the one reference to that record. For
exactly that object, field and id, the write-time reference check does
not ask the caller's read door, and it still runs the elevated,
tenant-kept existence probe. Nothing else on the write is admitted by
it.
- `packages/core/src/security/vouched-reference.ts` adds
`vouchForReference` / `isVouchedReference` and the `VOUCHED_REFERENCES`
key. It is the same shape as the flow-write mark
(`FLOW_WRITE_REFERENCE_CHECK`): a `Symbol.for` key that sits in no
schema, does not survive `ExecutionContextSchema.parse`, and cannot be
spelled by any request body, query string, stored row or flow
definition.
- `packages/objectql/src/engine-referential-integrity.ts`:
`assertReferencesResolve` asks the caller's read door for every `lookup`
/ `user` / `master_detail` value except a vouched one, which goes to the
existence probe alone.
- `packages/services/service-storage/src/file-attachment-service.ts`:
the caller-scoped `sys_attachment` insert vouches for
`sys_attachment.file_id` = the file id created in step ③. The system
path (`attachFile`) carries no mark, because system writes are already
exempt.
**Why this seam.** I looked at three other shapes and did not use them:
- An `ExecutionContext` key or an engine option. Either would add to the
spec's Zod contract.
- A system-elevated insert of the link. That drops the caller's RLS,
FLS, the `uploaded_by` stamp, the parent-edit attach rule and the object
permission on `sys_attachment`.
- Admitting a `sys_file` the caller uploaded (`owner_id`) inside the
generic rule. That special-cases one object in the generic engine, and
it widens every data door.
The mark keeps the insert as the caller's own write. Every other gate
still judges the caller: object permission to create an attachment,
parent edit (`installAttachmentAccessHooks`), `enable.files`, the
file-reference gate (`installAttachmentFileReferenceGate`, the download
door's verdict on the file), RLS, FLS, and the owner and audit stamps.
The caller's other references, the parent included, are judged as
before. No read is widened.
## Tests
Final-head local runs are in the Verification section below.
-
`packages/services/service-storage/src/file-attachment-service.test.ts`
gets a new top-level describe. It runs a real ObjectQL over sqlite with
an engine middleware standing in for the object-permission layer: a
non-system read of `sys_file` is refused `PERMISSION_DENIED` / 403.
- Pins: a member attaches through `attachFileAs`. The attachment is
`uploaded_by` the member and the file is owned by them. The print
sequence (`assertMayAttach`, then `attachFileAs`) also attaches as the
member.
- Controls: `assertMayAttach` still refuses a member the attach rule
refuses. A parent the member cannot read is refused `RECORD_NOT_FOUND` /
404. The member's own data-engine insert of `sys_attachment` naming a
file they cannot read is refused `VALIDATION_FAILED` /
`reference_not_found`, and so is the same insert naming the very file
the verb attached.
- `packages/objectql/src/engine-unreadable-reference-target.test.ts`
gets a new top-level describe.
- The vouched value lands and only the elevated probe runs.
- A vouched id that names no row is still refused.
- Another unreadable id in the same field, the same id in another field,
and the same id on another object are each refused.
- A multi-valued field admits only the vouched member.
- A context without the mark is refused, and so is one carrying the
symbol's description as a plain string key.
- `packages/qa/dogfood/test/print-service-archive.dogfood.test.ts` runs
on a real showcase boot.
- A member holding the contributor set and a `sys_attachment`
read/create grant, and nothing on `sys_file`, prints their own invoice
to its attachments. The attachment names them, and `sys_file` stays
unreadable to them afterwards.
- Control: the same member's own `sys_attachment` insert naming a file
they own and cannot read is refused `reference_not_found`.
**Ablations.** Each was run on a committed fix, with the restore proved
by blob hash against HEAD.
- Storage side, with the vouch removed from the insert. The suite reads
storage source directly, so no build was needed. The two member-attach
pins and the same-file pin go red (3 failed, 29 passed). The four
controls stay green.
- Engine side, with `isVouchedReference` removed from `callerReads`.
objectql was rebuilt and `ablation-dist-preflight` confirmed the marker
absent from all 14 built files.
- objectql pins: 4 failed, 117 passed. The 4 red are the admitting pins;
the nowhere, other-id, other-object and unmarked controls stay green.
- storage pins: 3 failed, 29 passed.
- print dogfood: the member print is red; 10 passed, the control
included.
- The restore leg was rebuilt and the preflight found the marker present
in 4 built files.
- #31009's `showcase-mcp-record-files.dogfood.test.ts` lives only on
that PR's branch. It was run in a scratch tree: this branch merged with
`origin/claude/project-thread-s2vcdk` (head `c2a3229be1`) and not
pushed. Result: 13 of 13 pass. With the engine-side ablation in that
same tree, exactly the three attachment cases go red (3 failed, 10
passed), matching the 3-of-13 regression measured after `eb96480dd6`.
## Acceptance notes
1. **The browser attachments panel's two-step flow is refused the same
way, and this PR does not change that.**
- What fails: the panel uploads through the presigned door, then posts
`POST /data/sys_attachment` with the returned `file_id` through the
generic data door. That post answers `400 VALIDATION_FAILED`, `file_id`
`reference_not_found`, for a member who holds the `sys_attachment`
create grant and nothing on `sys_file`.
- Measured on this branch: a showcase boot with a member holding
`showcase_contributor` plus a `sys_attachment` read/create grant.
Presign 200, PUT 200, complete 200, attach 400. The probe file was not
committed.
- Why main stays green: `attachments-permission-matrix.dogfood.test.ts`
runs on a fixture baseline that grants `'*'` read, `sys_file` included.
- Why this PR leaves it: per the order, the generic rule is not loosened
for any data door here. The decision is the seat's.
2. **Audit of other service-internal writes on `main`.** I scanned every
insert/update in `packages/services/*/src` and `packages/plugins/*/src`
whose options carry a context that is not obviously the system's, and
found no second instance of this regression shape to fix.
- `admin-user-endpoints.ts` (plugin-auth). The admin create door grants
a permission set as the administrator: `sys_user_permission_set.user_id`
names the `sys_user` the same call created. The door's platform-admin
gate admits only callers who read every user. Measured:
`create-user-permission-set.dogfood.test.ts` passes 4 of 4 on this
branch, G1 (user and grant land together) included. Not changed.
- `approval-service.ts` `mirrorStatusField`. It updates the record's
status mirror with an attributed context. That writes a status value,
not a reference. Not affected.
- `service-automation` `crud-nodes.ts` (a flow writing as the user).
This is a generic door by design, so the caller's references are judged.
Not changed.
- Everything else matched in those paths — device registry, connector
connections/credentials, sharing, share links, approvals' rows,
datasource secrets, settings audit, the storage metadata store and the
backfills — writes as the system.
## Verification
Head `b018a55aff` (this branch with `main` merged in).
- `pnpm --filter @objectstack/core typecheck`, `@objectstack/objectql`,
`@objectstack/service-storage` and `@objectstack/dogfood`: each exit 0.
- `pnpm --filter @objectstack/core test`, `@objectstack/service-storage
test` and `@objectstack/objectql test`: each exit 0, at `78b1698943`.
The commits after it change the two pin files' option typing and merge
`main`; `main`'s five commits touch none of these packages. The two pin
files were re-run after the typing change: objectql 121/121, storage
32/32.
- Dogfood at `98c5826af7`: `print-service-archive` with
`create-user-permission-set`, 15/15.
- `pnpm --filter @objectstack/spec check:generated` after merging
`main`: every gate up to date.
- Derived gate union (`dispatch-gates.mjs --commands`, 71 commands): 70
of 71 exit 0 in one sweep. `check:type-check-debt` lost its turn on the
shared verify lock (exit 3, PREREQUISITE NOT MET, nothing measured);
re-run under the lock it exits 0. Reconciliation: `dispatch-gates --ran`
reports 71 derived, 71 run, 0 NOT-MEASURED, all with recorded exit
codes.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent cad764d commit 5210acf
8 files changed
Lines changed: 425 additions & 5 deletions
File tree
- .changeset
- packages
- core/src/security
- objectql/src
- qa/dogfood/test
- services/service-storage/src
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
537 | 537 | | |
538 | 538 | | |
539 | 539 | | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
540 | 550 | | |
541 | 551 | | |
542 | 552 | | |
| |||
| 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 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
55 | | - | |
| 55 | + | |
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
| |||
219 | 219 | | |
220 | 220 | | |
221 | 221 | | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
222 | 232 | | |
223 | 233 | | |
224 | 234 | | |
| |||
301 | 311 | | |
302 | 312 | | |
303 | 313 | | |
304 | | - | |
305 | | - | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
306 | 320 | | |
307 | 321 | | |
308 | 322 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
34 | | - | |
| 34 | + | |
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| |||
498 | 498 | | |
499 | 499 | | |
500 | 500 | | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
0 commit comments