Repository navigation
Commit ecd944f
fix(objectql)!: a non-system write to a secret field may only seal a new value or keep its own record's ref (#27695)
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 #27582
Clause-②: no (narrowing) — a non-system write to a secret field refuses
a value already spelled `secret:` unless it is the reference that
record's column already holds (so a create always refuses one), where it
stored such a value as given before. Nothing is widened.
Metadata-protocol: none
Skills-docs: none — nothing a metadata author writes changes; the rule
is inside the engine's credential write path, and the system-context
page gains the row CI requires for the new elevation read.
## 维护者速读
- 改了什么:`secret` 字段收到形如 `secret:`
的已存引用时,非系统写入只能保留本记录这一列已存的那个引用(保留即不改写该列);新建记录不接受任何引用;其他引用一律以
`VALIDATION_ERROR` / 400 拒绝,且无论引用指向什么,答复都相同。系统上下文写入保持原样放行。
- 为什么改:分诊裁定"失败即拒绝:非系统写入只能封存新值或保留本记录自己的引用"。此前已是引用的值对所有调用方原样放行。
- 风险与回滚:拒绝发生在任何凭据行或记录写入之前;判断只读本次写入自己的已存行(按 id
更新用已读的前像,按条件更新用已有的匹配行备忘),不查 `sys_secret`。界面回传的掩码、`null`
清空、新值加密、读取掩码都不变。回滚只需 revert 本 PR。
-
先看:`packages/objectql/src/engine-secret-internal-fields.ts`(`encryptSecretFields`
的引用分支与
`SecretRefWriteError`),`packages/objectql/src/engine.ts`(三处调用点各自传入本次写入的已存行),`packages/objectql/src/secret-field-own-ref-write.test.ts`。
- 两分钟验证:`pnpm --filter @objectstack/objectql exec vitest run
src/secret-field-own-ref-write.test.ts`,预期 19/19 通过。
- 需要你做的:无,席位落地。
## What changes
The rule, in the secret-fields path #27572 already uses
(`encryptSecretFields`, whose body lives in
`engine-secret-internal-fields.ts`): **a non-system write may only seal
a new value or keep its own record's ref.**
- **The ref branch.** A `secret` value that is already a `secret:` ref
used to be left as it was for every caller. Now:
- The system principal (`context.isSystem === true`, the engine's
existing marker) keeps that pass-through, unchanged.
- Any other caller's ref is kept only when every row the write lands on
already holds exactly that value in that column. Then the key is
dropped, as the echoed read mask is, so the statement does not rewrite
the column at all.
- Anything else is refused with the new `SecretRefWriteError`: ADR-0112
`VALIDATION_ERROR` / 400, the standard-catalog pair the sibling
credential refusal (`EmptyCredentialWriteError`) and the connector
credential door already use. It is thrown before any credential row or
record is written. The message names the column and never the value, and
nothing about the handle is looked up, so the answer does not depend on
what the ref names.
- **Whose rows are "its own".** `encryptSecretFields` takes a
`StoredRowsReader`, and each of its three call sites in `engine.ts` says
what its own rows are:
- the insert passes none (a new record has no ref of its own);
- the by-id update passes the pre-image it has already read;
- the predicate update passes the matched-row memo (`readPriorRows`).
The reader is called only when a non-system payload carries a ref, so
every other write reads exactly what it did before.
- **Every engine write door.** Single and batch insert, `insertMany`
(the refused row is reported on its own and the others land), the by-id
update and the predicate update. So every REST door that writes through
them is covered too: create, `createMany`, import, PATCH.
- **Docs.** `content/docs/permissions/system-context.mdx` gains row 23e
for the new elevation read, and its site register is regenerated
(`check:system-context-census` requires both).
- **Not in this change.** Values stored before this change are not
re-examined. The write preview (`ObjectQL.validate()`) runs no
credential pass, as before.
## Tests
- `packages/objectql/src/secret-field-own-ref-write.test.ts` (new, 19
cases): a real engine over an in-memory driver, on every door listed
above.
- Refused: a non-system create, batch create, `insertMany` row, or
`beforeInsert`-hook value naming another record's ref; the same with no
context at all; a by-id update naming another record's ref; a column
that holds nothing; a predicate update where one matched row does not
hold the ref; a predicate that matches no row. Each refusal is asserted
on `code`, `status`, `httpStatus`, `object` and `field`. A ref naming an
existing row and one naming nothing get an identical message, which does
not contain the handle.
- Accepted: keeping the record's own ref, as an extra key or as the only
key, and the statement does not carry the column; the echoed mask; a
`beforeUpdate` hook that writes the stored row back; a predicate ref
that every matched row holds; a new value; and the system principal's
pass-through on create, by-id and predicate writes.
- `packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts`
(new, 5 cases): the same rule at the REST data door of a booted stack
with a provider wired.
- Refused: `POST /data/:object`, `/createMany` and `/import`, each 400
`VALIDATION_ERROR` or a per-row import error, and nothing written. A
PATCH naming another record's ref is refused with the same status, code
and message as a PATCH naming a ref that resolves to nothing, and the
stored value stays.
- Accepted: a PATCH echoing the mask the GET returned (what the form
sends back), and a PATCH carrying the record's own ref, leave the stored
ref and the credential unchanged.
- Unchanged: a new value still seals, and a system-context write still
takes a ref as given.
- `error-http-status-spelling.test.ts` registers the new refusal (both
status spellings agree).
## Acceptance notes
- **The `engine.ts` edit is wider than the claim's file surface.** The
claim allows `engine.ts` only to pass the system-context flag. That flag
was already in reach (`context` is an argument). What was not in reach
is the record's own stored row, which the ruling's "its own record's
ref" needs, so the three call sites now pass it. Nothing else in
`engine.ts` changed.
- **What the UI sends.** The form's password widget sends back the value
it read, which is the mask. The mask is dropped before the ref branch,
so the form round trip is unaffected; the dogfood file pins it.
- **Overlap.** #27657 (the same two objectql files, in the
delete-release region), #27477 and #27424 (`system-context.mdx`, plus
`engine.ts` for #27477) all landed on `main` while this was in progress.
`origin/main` 5e2f386 is merged in, and every result below was taken
after that merge. None of the 39 open PRs touches any of this PR's seven
files, so there is nothing left to `merge-tree` against. Against today's
`origin/main` (86f2afe) the merge is still clean, and none of the newer
commits touches these files.
## Verification
Every reading below is at head `75e18b19`, after `origin/main` 5e2f386
was merged in, unless it names another commit.
<details><summary><b>Red first</b>: the tests were committed before the
fix (bf7eff9)</summary>
- `secret-field-own-ref-write.test.ts`: `Tests 11 failed | 8 passed
(19)`.
- `secret-field-own-ref-write.dogfood.test.ts`: `Tests 3 failed | 2
passed (5)`.
</details>
<details><summary><b>Green</b>: build, typecheck, the whole objectql
suite, and the credential-channel dogfood files</summary>
The first four rows ran under one verify-lock hold; the two eslint rows
ran after it. Lock verdict: `command-exit 0`.
| Step | Result |
|---|---|
| `turbo run build --filter='@objectstack/dogfood^...'` | exit 0 |
| `pnpm --filter @objectstack/objectql typecheck` | exit 0 |
| `pnpm --filter @objectstack/objectql exec vitest run` | `Test Files
491 passed (491)` · `Tests 9224 passed (9224)` |
| dogfood, 9 credential-channel files: this PR's file plus
`secret-field-delete-release`, `secret-field-orphan-rows`,
`write-response-credential-mask`, `flow-credential-channel`,
`flow-trigger-record-credential-mask`,
`approval-snapshot-credential-field`, `admin-credential-lifecycle` and
`webhook-meta-door-credential-redaction` | `Test Files 9 passed (9)` ·
`Tests 60 passed (60)` |
| `eslint --no-inline-config packages/objectql` | exit 0 |
| `eslint --no-inline-config` on the new dogfood file | exit 0 |
</details>
<details><summary><b>Ablations</b>: each clause of the rule goes red
when removed, and each restore is proven blob == HEAD</summary>
Each leg runs `scripts/ablation-replace.mjs` on one clause, then runs
the new unit file (19 cases). The restore is then proven: blob
`8796f4077121` (`engine-secret-internal-fields.ts`) and `0af248405c8f`
(`engine.ts`) == HEAD, and `git diff HEAD` is empty.
| Leg | Mutation | Result |
|---|---|---|
| A1 | the system-principal check passes every caller | `13 failed \| 6
passed (19)` |
| A2 | the "no stored row" clause removed (`--delete`) | `7 failed \| 12
passed (19)` |
| A3 | "every matched row holds it" weakened to "some row" | `1 failed
\| 18 passed (19)` |
| A4 | any stored ref counts, not only the same value | `3 failed \| 16
passed (19)` |
| A5 | a kept ref is not dropped from the statement | `2 failed \| 17
passed (19)` |
| A6 | the by-id update passes no stored row | `3 failed \| 16 passed
(19)` |
| A7 | the refusal message carries the value | `2 failed \| 17 passed
(19)` |
| D (dist) | A1 held in source, `objectql` rebuilt, then the REST
dogfood file run | `ablation-dist-preflight` marker present (exit 0) ·
`3 failed \| 2 passed (5)` · after restore and rebuild, preflight marker
absent (exit 0) · `5 passed (5)` |
Two first attempts did nothing and were redone; neither counts above. On
an earlier head, A2 used an empty replacement where the tool requires
`--delete`, and A5 used a replacement that was a substring of its
anchor. The tool refused both, so no mutation was applied. The legs
above are the corrected forms, all re-measured at `75e18b19`.
</details>
<details><summary><b>Gates</b>: 94 derived, 94 run, each exit code
recorded, 0 NOT-MEASURED</summary>
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derives 94 commands at `75e18b19`, the same
list as before the merge.
- Every command ran, and each exit code was captured before any pipe.
`--ran` reconcile: `94 derived, 94 run, 0 NOT-MEASURED, 0 UNRUN`, a
derived zero (all 94 carry an exit code and none is 3). All 94 exit 0.
- `check:type-check-debt` first answered 3 (prerequisite not met): its
closure build timed out on the shared lock queue, so nothing was
measured. It was re-run inside this run's own lock hold and exited 0 (`0
raw tsc error(s) total`).
- Before the merge, at 2f31768, `check:skill-examples` and
`check:dual-build-cjs-loads` first answered 3 (dist missing for packages
outside this diff). They were built and both exited 0. After a container
restart, `check:dual-build-cjs-loads` answered 1 once, on four unrelated
packages whose interrupted builds had left no `.d.ts`. After the full
closure build it exited 0 (`112 published require entry point(s) across
69 package(s) load`).
- `check:system-context-census` was red until row 23e and its register
line were added (`--write`). It exits 0 at this head.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F)_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 525ea36 commit ecd944f
7 files changed
Lines changed: 597 additions & 6 deletions
File tree
- .changeset
- content/docs/permissions
- packages
- objectql/src
- qa/dogfood/test
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
136 | 136 | | |
137 | 137 | | |
138 | 138 | | |
| 139 | + | |
139 | 140 | | |
140 | 141 | | |
141 | 142 | | |
| |||
532 | 533 | | |
533 | 534 | | |
534 | 535 | | |
| 536 | + | |
535 | 537 | | |
536 | 538 | | |
537 | 539 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
26 | 32 | | |
27 | 33 | | |
28 | 34 | | |
| |||
106 | 112 | | |
107 | 113 | | |
108 | 114 | | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
109 | 162 | | |
110 | 163 | | |
111 | 164 | | |
112 | 165 | | |
113 | 166 | | |
114 | 167 | | |
115 | 168 | | |
| 169 | + | |
116 | 170 | | |
117 | 171 | | |
118 | 172 | | |
119 | 173 | | |
| 174 | + | |
120 | 175 | | |
121 | 176 | | |
122 | 177 | | |
| |||
141 | 196 | | |
142 | 197 | | |
143 | 198 | | |
144 | | - | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
145 | 215 | | |
146 | 216 | | |
147 | 217 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
125 | 125 | | |
126 | 126 | | |
127 | 127 | | |
| 128 | + | |
128 | 129 | | |
129 | 130 | | |
130 | 131 | | |
| |||
10575 | 10576 | | |
10576 | 10577 | | |
10577 | 10578 | | |
10578 | | - | |
| 10579 | + | |
| 10580 | + | |
| 10581 | + | |
| 10582 | + | |
| 10583 | + | |
| 10584 | + | |
| 10585 | + | |
| 10586 | + | |
| 10587 | + | |
| 10588 | + | |
10579 | 10589 | | |
10580 | 10590 | | |
10581 | 10591 | | |
| |||
10585 | 10595 | | |
10586 | 10596 | | |
10587 | 10597 | | |
| 10598 | + | |
10588 | 10599 | | |
10589 | 10600 | | |
10590 | 10601 | | |
| |||
10594 | 10605 | | |
10595 | 10606 | | |
10596 | 10607 | | |
10597 | | - | |
| 10608 | + | |
10598 | 10609 | | |
10599 | 10610 | | |
10600 | 10611 | | |
| |||
15374 | 15385 | | |
15375 | 15386 | | |
15376 | 15387 | | |
15377 | | - | |
| 15388 | + | |
| 15389 | + | |
15378 | 15390 | | |
15379 | 15391 | | |
15380 | 15392 | | |
| |||
17029 | 17041 | | |
17030 | 17042 | | |
17031 | 17043 | | |
17032 | | - | |
| 17044 | + | |
| 17045 | + | |
17033 | 17046 | | |
17034 | 17047 | | |
17035 | 17048 | | |
| |||
17426 | 17439 | | |
17427 | 17440 | | |
17428 | 17441 | | |
17429 | | - | |
| 17442 | + | |
| 17443 | + | |
| 17444 | + | |
17430 | 17445 | | |
17431 | 17446 | | |
17432 | 17447 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
| 40 | + | |
40 | 41 | | |
41 | 42 | | |
42 | 43 | | |
| |||
52 | 53 | | |
53 | 54 | | |
54 | 55 | | |
| 56 | + | |
55 | 57 | | |
56 | 58 | | |
57 | 59 | | |
| |||
0 commit comments