Skip to content

Commit ec150c6

Browse files
committed
fix(plugin-security)!: on the write doors, a row the caller cannot read answers what a nonexistent id answers
The by-id write pre-image check (security-plugin.ts, step 2.7) now asks every principal class whether it can read the row its by-id update or delete addressed, before it asks whether it may write it. The question is the read door's own: a caller-context by-id read, so every data middleware's read visibility counts. A row that read does not return is answered by the read door's not-found producer (recordNotFoundError), the answer a nonexistent id gets. A caller who can read the row but may not write it keeps its 403. - By-id versus predicate comes from the engine's own dispatch predicates (resolveEngineUpdateDispatch / resolveEngineDeleteDispatch). - Only absence is "cannot read": a store fault propagates as raised, and a read-time policy refusal keeps the answer the write had before. - Writes the platform issues under the caller's context keep their previous answer: the engine's cascade delete, hook writes, and the referential clear of a lookup. An async scope owned by the plugin marks them; the engine is not edited. - security/explain models the same answer: an update or delete of a record the principal cannot read reports the missing-record shape. Turned pins keep every expect, flipped to the ruled answer. New pins: a real-engine unit file and a door-level dogfood file covering both principal classes, both verbs and both parent-derived objects. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
1 parent e83c9f6 commit ec150c6

26 files changed

Lines changed: 977 additions & 78 deletions
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@objectstack/plugin-security': minor
3+
---
4+
5+
fix(plugin-security)!: on the write doors, a row the caller cannot read answers what a nonexistent id answers
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) a write-door answer and a write-door verdict, no authorable surface: the by-id write pre-image check now asks every principal whether it can read the row it addressed, and answers a row it cannot read with the read door's existing not-found. No authorable key, spelling, export, error code or stored shape is added, retired or renamed; every stored row and every metadata item keeps parsing and is read the same way, so `objectstack migrate meta` has nothing to rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a write door's answer (not already-registered); and the change is a door verdict, not a TypeScript declaration (not runtime-interface-only or type-surface-only). -->
10+
11+
**BREAKING**: a by-id update or delete of a row the caller cannot read now answers `404 RECORD_NOT_FOUND`, with exactly the body an id that names no row gets, for every principal class. On the write doors, "hidden" and "gone" are now one answer to a caller who cannot read the row. It ships as `minor` under the launch-window convention for accept-set narrowings. No export is added or removed, and no error code is new.
12+
13+
**What changed.** The answer used to depend on which gate saw the row first. Where a write-class row filter binds the caller, the by-id write pre-image check answered `403 PERMISSION_DENIED`. Where none binds it, a later gate answered with its own 403: `FORBIDDEN` from record sharing, or a parent-derived gate's code on attachments and comments. Meanwhile a nonexistent id answered `404`. So the write door could tell a hidden row apart from a missing one. The pre-image check now asks the read door's own question first, for the by-id write the caller addressed: a by-id read in the caller's context, every data middleware's visibility included. A row that read does not return gets the read door's not-found producer. A store fault propagates as raised, and a read-time policy refusal is not treated as absence.
14+
15+
**What is refused now that was not.** A principal that no write-class row filter binds could have its by-id write admitted on a row the read door hides from it. One example is the uploader of an attachment, or the author of a comment, whose parent record they can no longer read. That write is now refused with the not-found answer, as it already was for every principal a row filter binds.
16+
17+
**FROM → TO.** A by-id update or delete of a row hidden from the caller: FROM a `403` (`PERMISSION_DENIED`, `FORBIDDEN`, or a parent-derived gate's code) → TO `404 RECORD_NOT_FOUND`, the body a nonexistent id gets.
18+
19+
**If you are affected.** A client that read a by-id write's `403` as "the row exists, but you may not change it" should read `404 RECORD_NOT_FOUND` the way the read door means it: no row you can see has this id.
20+
21+
**Unchanged.**
22+
- A caller who can read the row but may not write it keeps its 403. They already see the row.
23+
- By-id writes the platform issues under the caller's context keep their previous answer, because the caller never named their target: the engine's cascade delete of a dependent row, a hook's write, and the referential clear of a lookup.
24+
- Writes that are not routed by id are unchanged.
25+
26+
`security/explain` follows enforcement. Its record verdict for an update or delete of a record the principal cannot read is now the missing-record shape: `visible: false`, with no decider.

‎content/docs/permissions/attachments-access.mdx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,12 @@ editable by design). A multi-delete requires *every* matched row to pass.
7171
| Code | Status | When |
7272
| --- | --- | --- |
7373
| `ATTACHMENT_DELETE_DENIED` | 403 | The caller can read the attachment, but is neither the uploader nor able to edit the parent record |
74-
| `PERMISSION_DENIED` | 403 | The caller cannot read the parent record, so the attachment is not visible to them. This is the platform's not-visible refusal whichever layer gives it — the row-level write check that runs before the gate, or the gate itself for a caller that check does not cover — and it names neither the parent nor the attachment's link to it |
74+
| `RECORD_NOT_FOUND` | 404 | A delete or update **by id** of an attachment the caller cannot read — its parent record is not readable to them. On the write doors a row the caller cannot read is a row that does not exist: the answer is exactly what an id that names no attachment gets, for every caller, so a hidden attachment and a missing one cannot be told apart. It applies even to the uploader once the parent is out of their sight, because the read door no longer returns the attachment to them either |
75+
| `PERMISSION_DENIED` | 403 | The gate's own not-visible refusal, where the gate answers before that check. It names neither the parent nor the attachment's link to it |
7576

7677
An update of another user's attachment follows the same rule — the uploader or
77-
a parent editor — and a caller who cannot read the parent gets the same
78-
not-visible refusal.
78+
a parent editor — and a by-id update of an attachment the caller cannot read
79+
gets the same not-found answer.
7980

8081
The platform baseline also ships a parent-blind row-level delete floor
8182
(`owner_only_deletes`: you may delete only the rows you created, for members

‎content/docs/permissions/permissions-matrix.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ flowchart TD
360360
| 7 | **Field-Level Security** | Which fields is the user allowed to see/edit? |
361361

362362
<Callout type="tip">
363-
**Performance:** For reads, steps 2–6 are compiled into a query filter (owner-match ∪ materialized shares, AND-ed with RLS) at query time, not evaluated record-by-record. By-id writes are verified with a pre-image check: the target row is re-read through the write-scope filter before the mutation. This keeps security checks efficient even on tables with millions of rows.
363+
**Performance:** For reads, steps 2–6 are compiled into a query filter (owner-match ∪ materialized shares, AND-ed with RLS) at query time, not evaluated record-by-record. By-id writes are verified with a pre-image check: the target row is re-read through the write-scope filter before the mutation, and a row the caller cannot read answers exactly what a missing id answers (`404 RECORD_NOT_FOUND`). This keeps security checks efficient even on tables with millions of rows.
364364
</Callout>
365365

366366
## See also

‎content/docs/protocol/kernel/error-handling.mdx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,12 @@ Content-Type: application/json
907907
}
908908
```
909909
910+
⚠️ **This is the answer for a row the caller can read.** A by-id update or delete of a row
911+
the caller **cannot read** answers exactly what an id that names no row answers —
912+
`404 RECORD_NOT_FOUND` — whichever rule hides the row, so the write door never tells
913+
"hidden" apart from "gone". A caller who can read the row but may not write it gets the 403
914+
shown here.
915+
910916
⚠️ **`FORBIDDEN`, not `PERMISSION_DENIED`.** A by-id write the sharing rules refuse carries
911917
`FORBIDDEN`; `PERMISSION_DENIED` is what the capability and identity guards carry. Both are
912918
403s in the same envelope on this door, so branch on either — ⛔ but do not assume one code

‎packages/plugins/plugin-audit/src/comment-access-hooks.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,12 @@ function forbid(message: string, object?: string): never {
200200
* The code of the platform's not-visible refusal: the one plugin-security's
201201
* by-id write pre-image check throws when the caller's own read visibility
202202
* does not reach the target row (`PermissionDeniedError`, 403).
203+
*
204+
* [#21771] No longer the pre-image check's answer for a row the caller cannot
205+
* read: under the write doors' ruling A that check asks the caller's read
206+
* visibility for every principal, and answers the by-id update or delete the
207+
* caller addressed with what a nonexistent id answers, before this gate runs.
208+
* This refusal stays the gate's own, for a write the gate answers first.
203209
*/
204210
const NOT_VISIBLE_CODE: StandardErrorCode = 'PERMISSION_DENIED';
205211
const NOT_VISIBLE_STATUS = 403;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,9 @@ describe('sys_user self-service — the four pins, each attributed to a layer',
350350
// with `refusedBy === 'object-gate'` — the object bit was false, so the row
351351
// scope never ran and this pin proved nothing about it. Now the pre-image
352352
// re-read HAPPENED, was scoped to the caller, and came back empty.
353-
expect(r.preImageWheres).toEqual([{ $and: [{ id: PEER }, { id: ME }] }]);
353+
// …and, the row being refused, the read question that keeps the 403: the
354+
// 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 }]);
354356
expect(r.error?.name).toBe('PermissionDeniedError');
355357
expect(r.error?.code).toBe('PERMISSION_DENIED');
356358
});

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,18 +205,26 @@ async function readFilter(cell: any, roleCtx: any): Promise<unknown> {
205205
*/
206206
async function writeFilter(cell: any, roleCtx: any): Promise<unknown> {
207207
const plugin = new SecurityPlugin();
208-
const h = makeHarness({ ...cell, orgScoping: cell.orgScoping ?? true, findOneImpl: () => null });
208+
// The write-class re-read finds nothing; [#21771] the addressed write's
209+
// read question (a plain by-id read) finds the row, so the matrix keeps
210+
// measuring the WRITE filter alone.
211+
const h = makeHarness({
212+
...cell,
213+
orgScoping: cell.orgScoping ?? true,
214+
findOneImpl: (q: any) => (q?.where?.$and ? null : { id: 'r1' }),
215+
});
209216
await plugin.init(h.ctx); await plugin.start(h.ctx);
210217
const opCtx: any = {
211218
object: cell.objectName, operation: 'update',
212219
data: { id: 'r1', name: 'x' }, options: { where: { id: 'r1' } }, context: roleCtx,
213220
};
214221
let threw: any = null;
215222
try { await h.run(opCtx); } catch (e: any) { threw = e; }
216-
if (h.findOne.mock.calls.length === 0) {
223+
const reRead = h.findOne.mock.calls.find((call: any[]) => call[1]?.where?.$and);
224+
if (!reRead) {
217225
return threw ? `CRUD_DENY:${threw?.name ?? 'err'}` : 'BYPASS(no-write-filter)';
218226
}
219-
return h.findOne.mock.calls[0][1].where.$and.slice(1);
227+
return reRead[1].where.$and.slice(1);
220228
}
221229

222230
/**

0 commit comments

Comments
 (0)