Skip to content

Commit 70981e6

Browse files
fix(plugin-security): a by-id write's row re-read propagates a store fault, and judges the platform's own predicates without the caller-predicate guard (#30307)
Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - 改了什么:按 id 的更新/删除在写前复读该行时,存储故障按原样抛出;平台自己拼进复读的租户墙与行级写谓词标记为平台所有,"调用方谓词守卫"只审调用方自己的过滤条件;主从(controlled_by_parent)写对主记录的复读同样处理。 - 为什么改:复读只有声明式拒绝才算记录级 403,存储故障按其本身报告;调用方谓词守卫只审调用方自己写的谓词,平台拼出并标记为平台所有的谓词(写复读、主记录复读、分析读范围、共享读范围)不在其列。 - 风险与回滚:放宽面只限于"平台拼出并标记的谓词不由守卫审";调用方自己的谓词照审,读不到的行与不存在的 id 答法与其他调用方一致;squash 落地后单 commit 可整体 revert。主从写对主记录的复读遇存储故障仍答 403(沿用该处注释所记裁决);它是否也应像 step 2.7 那样按原样抛出存储故障,是安全席列在其清单上、待维护者裁决的一项。 - 先看:`security-plugin-authz-middleware.ts`(step 2.7 复读处)、`predicate-guard.ts`(`collectConditionFields` 跳过 policy 子树那一行)、`security-plugin-cbp-write-gate.ts`(主记录复读)。 - 两分钟验证:`pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 src/by-id-write-row-reread.posture.test.ts`,预期 19 passed;分析面 `pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2 src/analytics-platform-scope-predicate-guard.test.ts`,预期 2 passed。 - 需要你做的:无,席位落地。 ## What changes Before an update or delete by id, the security middleware re-reads the addressed row under the caller's context together with the platform's write predicates for that row (the tenancy wall and the row-level write predicates). Two rules hold for that re-read (step 2.7), and the second also for the controlled-by-parent master read a detail write asks (step 2.8): 1. **A store fault propagates as raised; only a declared refusal keeps the record-level answer.** The re-read's outcome is classified by `readUnlessRefused`, the one classification the write doors already use for a caller-context read: a row or no row; a declared 4xx keeps the record-level `403 PERMISSION_DENIED`, one answer whatever the id names; anything else is the write's answer, unchanged (for `ERR_DATASOURCE_UNAVAILABLE` the transports answer `503`). The step 2.8 master read keeps its `403` for every failure (its own comment records why); this PR does not change that catch. 2. **The platform's own predicates are not the caller's predicate.** The write predicates ride into the re-read under one fresh `$and` stamped `'policy'` with the spec's filter-subtree provenance mark (`markFilterSubtreeProvenance`, the mark step 3 already stamps on every read scope it injects), and the field-level predicate guard (`collectConditionFields`) does not judge a subtree so marked. An unmarked subtree, or one with any other mark, is judged as the caller's, and the mark does not survive a copy, a serialization or the wire. No second guard and no second read: the one re-read is asked once, and the one guard reads the existing mark. Where the rule reaches: every `'policy'`-marked subtree that arrives at the guard inside a caller-context read — the step 2.7 re-read and the step 2.8 master read above, the analytics read-scope merge (`withReadScope` and its foreign-key expand) handed to an aggregate under the caller's context, and a record-sharing read scope composed before this middleware. Only platform-composed predicates are exempt; nothing the caller writes is. Unchanged: a caller's own filter on a field it cannot read is refused (read, analytics query and predicate write); a row the caller cannot read and an id naming no row answer as they do for every other caller; a row the caller reads but may not write keeps its `403`; a system write is not the gate's; the predicate write's read question still guards the caller's verbatim predicate. ## Tests - `by-id-write-row-reread.posture.test.ts` (new; real `ObjectQL` over SQLite, real `SecurityPlugin`, postures `group` and `isolated`, `single` as control): - a store fault raised by the re-read is the write's answer, the same error object, nothing written; a declared 4xx keeps the record-level `403`, identical for a row and for no row; with the store answering, the same writes land. - a caller whose field rules hide the organization column updates and deletes a row of its organization by id; a row of another organization and an id naming no row answer that caller exactly as they answer a caller who reads the column (`404 RECORD_NOT_FOUND`), nothing written; the caller's own predicate on the column is still refused on a read and on a predicate write; a system write lands. - a controlled-by-parent detail insert by a caller whose field rules hide the master's organization column is admitted under a master of its organization; a master of another organization answers it what it answers a caller who reads the column. - an organization administrator whose field rules hide both assignment tables' organization column removes an in-boundary assignment by id (`sys_user_position`, `sys_user_permission_set`), admitted as for an administrator who reads the column. - `predicate-guard.test.ts`: a `'policy'` subtree is not collected nor anything beneath it; the caller's own predicate beside it is still refused; an unmarked subtree, an `'author'` subtree, a spread copy and a JSON copy of a `'policy'` subtree are all judged. - `packages/rest/src/analytics-platform-scope-predicate-guard.test.ts` (new; real `ObjectQL`, real `SecurityPlugin` under `group` and `isolated`, real `AnalyticsServicePlugin` on the engine-aggregate strategy): a caller whose field rules hide the scoped organization column counts the records of its organization, the same count as a caller who reads the column; its own filter on that column is refused `403 PERMISSION_DENIED`. - `authz-matrix-gate.test.ts`: the Layer 0 matrix helper reads the write predicates from the re-read's platform subtree (the re-read is `{ $and: [{ id }, PLATFORM] }`); every matrix expectation is unchanged. ## Acceptance notes - The organization administrator's assignment-removal pin depends on the stored-row verdict that landed in #30248 (merged into this branch from `main`); the delegated-administration gate itself is not touched here. - The step 2.8 master read keeps its catch-all `403` (its own comment records the ruling it follows), so the two differ for a store fault only; whether it should follow step 2.7 is left to the seat. ## Verification Readings are on head `6c7ff71e5e` unless named otherwise. `6c7ff71e5e` changes no source file over `8a92cab832` (a merge of `main` into `cc1fbac302`): it brings the three `sys_user` self-service pins that assert the by-id re-read's `where` verbatim to the re-read's shape (the platform's write predicates as one nested `$and` beside `{ id }`), with that file's row-scope sentence and two plugin-security test comments; each pin's verdict half (the refusing layer, the error code) is unchanged. The PR's source, changeset and earlier test files are byte-identical across `cc1fbac302`, `8a92cab832` and `6c7ff71e5e`, so the earlier ablation readings and the dogfood reading on `b6b340a3db` stand for that source. <details><summary>Suites and type checks</summary> - `pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/sys-user-self-service-route.test.ts`, under the verify lock: on `8a92cab832` `Tests 3 failed | 9 passed (12)`, the same three pins `Test Core (8/8)` read red on that head, each diff the nested `$and`; on `6c7ff71e5e` `Test Files 1 passed (1)`, `Tests 12 passed (12)`, VERDICT command-exit 0. - On `6c7ff71e5e`, under the verify lock: the full plugin-auth suite, `Test Files 224 passed (224)`, `Tests 3954 passed | 10 skipped (3964)`, exit 0; the full plugin-security suite, `Test Files 244 passed (244)`, `Tests 5378 passed | 45 skipped (5423)`, exit 0; `pnpm --filter @objectstack/plugin-auth typecheck` and `pnpm --filter @objectstack/plugin-security typecheck`, VERDICT command-exit 0 (`check:test-typecheck` OK on both). - CI on `6c7ff71e5e`: `Test Core` success, every shard success; `Test Core (8/8)` reads plugin-auth `Test Files 224 passed (224)`, `Tests 3954 passed | 10 skipped (3964)` and plugin-security `Test Files 244 passed (244)`, `Tests 5379 passed | 45 skipped (5424)`; `TypeScript Type Check`, `Lint & Repo Gates`, `Build Core`, `Dogfood Regression Gate`, `Live E2E UI`, `Test UI` and `Governed Surface Queue Guard` success. - On `cc1fbac302`, under the verify lock: `pnpm --filter @objectstack/plugin-security typecheck` exit 0 (`check:test-typecheck` OK); `by-id-write-row-reread.posture.test.ts`, `predicate-guard.test.ts`, `authz-matrix-gate.test.ts`: `Test Files 3 passed (3)`, `Tests 111 passed (111)`; `packages/rest` `analytics-platform-scope-predicate-guard.test.ts` and `analytics-masked-field-gate.test.ts`: `Test Files 2 passed (2)`, `Tests 12 passed (12)`; `pnpm --filter @objectstack/rest typecheck` exit 0 (`check:test-typecheck` OK). - On `b6b340a3db`: the full plugin-security suite, `Test Files 243 passed (243)`, `Tests 5349 passed | 45 skipped (5394)`, VERDICT command-exit 0. </details> <details><summary>One-shot ablations (each restored, blob equal to HEAD, `git diff HEAD` empty)</summary> - Rule 1, the re-read's classification replaced: 2 failed, red, then restored. - Rule 2, step 2.7 mark removed: 4 failed, red, then restored. - Rule 2, step 2.8 mark removed: 2 failed, red, then restored. - Rule 2, guard skip removed: 7 failed, red, then restored. - Rule 2, step 2.7 mark removed, against the organization administrator's pin: 2 failed, red, then restored. - Rule 2, guard skip removed, against the analytics pin (plugin-security rebuilt, the removal confirmed absent from `dist/` by `ablation-dist-preflight`): 2 failed, red, then restored and rebuilt. </details> <details><summary>Gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on `6c7ff71e5e` derived 62 commands; all 62 run, every exit code captured before any pipe, every one `0`; `--ran` reconciliation: 62 derived, 62 run, 0 NOT-MEASURED (a derived zero). `check:dual-build-cjs-loads`, `check:i18n` and `check:type-check-debt` read exit 3 (prerequisite: built output) in the first pass and `0` once run with the package tree built under the lock. On `cc1fbac302`: 61 derived, all 61 `0`. </details> <details><summary>Dogfood</summary> No `!` in the title and no narrowing arm, so not the full battery: the ten dogfood files that drive by-id and predicate writes (`write-door-unreadable-is-not-found`, `update-by-id-refusal-order`, `occ-stale-token-organization-wall`, `owner-anchor-and-bulk-writes`, `showcase-invoice-cbp`, `authored-row-write-scope`, `showcase-fls-read-mask-strip`, `delegated-admin-assignment-write-unreadable-row`, `predicate-write-unreadable-not-matched`, `cbp-parent-attachment-comment-gates`), closure built, under the verify lock, on `b6b340a3db`: `Test Files 10 passed (10)`, `Tests 107 passed (107)`, VERDICT command-exit 0. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj)_ <!-- signed: security#1 · os-sales · session_01L6bhgVfjk2i8T2D4RmxsKg --> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: objectstack-fleet[bot] <332303061+objectstack-fleet[bot]@users.noreply.github.com>
1 parent 48231b0 commit 70981e6

11 files changed

Lines changed: 717 additions & 29 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security): a by-id write's row re-read propagates a store fault, and judges the platform's own predicates without the caller-predicate guard
6+
7+
Clause-②: yes (widening)
8+
Metadata-protocol: none
9+
10+
Before an update or delete by id, the security middleware re-reads the addressed row under the caller's context with the platform's write predicates for that row (the tenancy wall and the row-level write predicates). Two rules hold for that re-read, and the second for the controlled-by-parent master read a detail write asks as well:
11+
12+
- **A store fault propagates as raised.** A re-read the store cannot answer (an unavailable datasource, a driver failure) is the write's answer, unchanged — for `ERR_DATASOURCE_UNAVAILABLE` the transports answer `503` — and nothing is written. Only a declared refusal (a 4xx) keeps the record-level `403 PERMISSION_DENIED`, one answer whatever the id names. The controlled-by-parent master read keeps its `403` for every failure.
13+
- **The platform's own predicates are not the caller's predicate.** The field-level predicate guard refuses a caller's filter on a field its field rules do not let it read, and judges only the caller's predicate: a predicate subtree the platform composed and marked as its own is not judged. That holds for every such subtree that reaches the guard inside a caller-context read — the by-id write re-read and the controlled-by-parent master read, the analytics read-scope merge handed to an aggregate under the caller's context, and a record-sharing read scope composed before the security middleware. Only platform-composed predicates are exempt; nothing the caller writes is. Under the `group` and `isolated` tenancy postures, a caller whose field rules hide the organization column updates and deletes the records of its organization by id, writes a controlled-by-parent detail under a master of its organization, and aggregates over the records of its organization. A record of another organization and an id that names no record answer that caller exactly as they answer any other caller.
14+
15+
Unchanged: a caller's own filter on a field it cannot read is refused, on a read, an analytics query and a predicate write; a record the caller reads but may not write keeps its `403`; a system write is not the gate's.

‎packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
*
2323
* 1. the CRUD **object gate** — `member_default.objects.sys_user.allowEdit`;
2424
* 2. the **row scope** — the by-id write pre-image check, which re-reads the
25-
* target row through the caller's compiled write-RLS filter and denies when
26-
* it comes back empty (`sys_user_self`);
25+
* target row with its `{ id }` address AND-ed with one nested `$and` of the
26+
* caller's compiled write-RLS parts (the platform's own predicate, never
27+
* the caller's) and denies when it comes back empty (`sys_user_self`);
2728
* 3. the ADR-0092 D2 **identity write guard** — an engine `beforeUpdate` hook
2829
* that strips non-whitelisted columns and throws when nothing editable
2930
* survives.
@@ -298,8 +299,9 @@ describe('sys_user self-service — the row scope the write actually composes',
298299
const r = await route(ME, { locale: 'ja-JP' });
299300
expect(r.refusedBy).toBeNull();
300301
// One pre-image re-read, and its `where` is the `{id}` address ANDed with
301-
// the compiled RLS parts. Asserted verbatim because every other assertion in
302-
// this file leans on `matchesFilter` reading it correctly, and because it is
302+
// one nested `$and` of the compiled RLS parts. Asserted verbatim because
303+
// every other assertion in this file leans on `matchesFilter` reading it
304+
// correctly (nested `$and` included), and because it is
303305
// the security property in one line: the caller's id, and nothing wider.
304306
//
305307
// Then exactly one more read: the caller's OWN row by id, which step 3.6
@@ -308,7 +310,7 @@ describe('sys_user self-service — the row scope the write actually composes',
308310
// already admitted above, never a scope: it cannot widen what the member may
309311
// write, and the check it feeds can only refuse more.
310312
expect(r.preImageWheres).toHaveLength(2);
311-
expect(r.preImageWheres[0]).toEqual({ $and: [{ id: ME }, { id: ME }] });
313+
expect(r.preImageWheres[0]).toEqual({ $and: [{ id: ME }, { $and: [{ id: ME }] }] });
312314
expect(r.preImageWheres[1]).toEqual({ id: ME });
313315
});
314316

@@ -352,7 +354,7 @@ describe('sys_user self-service — the four pins, each attributed to a layer',
352354
// re-read HAPPENED, was scoped to the caller, and came back empty.
353355
// …and, the row being refused, the read question that keeps the 403: the
354356
// member reads the peer's row, so it is not answered as a missing one.
355-
expect(r.preImageWheres).toEqual([{ $and: [{ id: PEER }, { id: ME }] }, { id: PEER }]);
357+
expect(r.preImageWheres).toEqual([{ $and: [{ id: PEER }, { $and: [{ id: ME }] }] }, { id: PEER }]);
356358
expect(r.error?.name).toBe('PermissionDeniedError');
357359
expect(r.error?.code).toBe('PERMISSION_DENIED');
358360
});
@@ -400,7 +402,7 @@ describe('sys_user self-service — the four pins, each attributed to a layer',
400402
// The pre-image re-read ran and SUCCEEDED (the row was visible), and the
401403
// post-image check read the caller's own row and passed — proof the first
402404
// two layers admitted the write before the guard refused it.
403-
expect(r.preImageWheres).toEqual([{ $and: [{ id: ME }, { id: ME }] }, { id: ME }]);
405+
expect(r.preImageWheres).toEqual([{ $and: [{ id: ME }, { $and: [{ id: ME }] }] }, { id: ME }]);
404406
// ADR-0112 envelope, both discriminators (the REST boundary derives 403 from
405407
// `status`, and `mapDataError` keys on `code`).
406408
expect(r.error?.code).toBe('PERMISSION_DENIED');

‎packages/plugins/plugin-security/src/authz-matrix-gate.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,11 @@ async function writeFilter(cell: any, roleCtx: any): Promise<unknown> {
244244
if (!reRead) {
245245
return threw ? `CRUD_DENY:${threw?.name ?? 'err'}` : 'BYPASS(no-write-filter)';
246246
}
247-
return reRead[1].where.$and.slice(1);
247+
// The re-read is `{ $and: [{ id }, PLATFORM] }`: PLATFORM is one `$and` of
248+
// the write predicates, marked as the platform's own (never the caller's
249+
// predicate), and its members are the matrix this gate locks.
250+
const [, platform] = reRead[1].where.$and;
251+
return platform.$and;
248252
}
249253

250254
/**

0 commit comments

Comments
 (0)