Skip to content

Commit 7778115

Browse files
claude[bot]claude
andauthored
feat(objectql): find() guarantees its array — an afterFind that replaces the container is refused (#16232)
* feat(objectql): refuse an afterFind that replaces find()'s array container `ObjectQL.find` declares `Promise<any[]>` but ended its hook path with `return hookContext.result`, with nothing between the `afterFind` dispatch and that return re-checking the value. A handler assigning `ctx.result = { records: [ ... ] }` therefore made a `find()` declared to resolve to an array resolve to an envelope, silently. Ruled 2026-09-06 (direction 1): `find()` guarantees the array, and a hook that replaces the container is refused loudly. The check sits immediately after `triggerHooks('afterFind', ...)` and BEFORE `maskSecretFields` / `stripSearchCompanionFromRead`, both of which already assume the array. Shaping stays legal: mutating rows in place, dropping keys, filtering rows out and assigning a different ARRAY are all untouched. `Array.isArray` is the whole predicate. Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ Co-authored-by: Claude <noreply@anthropic.com> * test(plugin-auth): the #15597 control now asserts the refusal, not the envelope #15823 closes the seam this control drove: an `afterFind` assigning a non-array no longer produces an envelope at the fourteen real reads, it produces `FIND_HOOK_RESULT_NOT_ARRAY`. The control keeps both of its jobs — the mechanism is driven on every block and the refusal asserted, and `expectBareArray`'s discrimination is now checked directly, since no engine can hand it an envelope any more. #15597's own conclusion is untouched: the fourteen limbs were removed on the argument that they were right to remove even given an open seam. Also fixes the refusal message's article (`a object` -> `an object`) and pins it. Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ Co-authored-by: Claude <noreply@anthropic.com> * chore: re-anchor the system-context census and pay the spec load at module top Two gate repairs the guard's diff owes: - `check:system-context-census` reads DOC anchors keyed by LINE NUMBER, and the 21 lines the guard adds (plus the import) shifted fourteen of them in `content/docs/permissions/system-context.mdx`. Repaired with the script's own `--fix`; line numbers only, nothing semantic. Measured green at the merge base first, so the rot is this branch's. - `check:test-source-alias` refuses a first module load paid inside a clocked test body: the new pin's `await import('@objectstack/spec/api')` moves to a module-top import, so the transform is paid during collection. Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ Co-authored-by: Claude <noreply@anthropic.com> * Merge origin/main into claude/issue-15823-find-afterfind-array-guard `main` moved onto three of this branch's files (engine.ts, objectql/index.ts and the system-context census page). Landed through `scripts/pm/os-regen-merge.sh` so the os-regen-driven artifact is regenerated from the MERGED tree rather than text-merged: the driver merges those paths at exit 0 while silently keeping one side, and only a regeneration exposes it. The one artifact the pre-commit hook held the merge for — `content/docs/permissions/system-context.mdx` — is regenerated here with `pnpm gen:system-context-census` (15 anchors re-derived against the merged engine.ts; line numbers only). Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 60c0f61 commit 7778115

10 files changed

Lines changed: 608 additions & 35 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@objectstack/objectql": minor
3+
"@objectstack/spec": minor
4+
---
5+
6+
`ObjectQL.find()` now guarantees the array it declares: an `afterFind` hook that replaces the result container is refused with `FIND_HOOK_RESULT_NOT_ARRAY`.
7+
8+
`find()` is declared `Promise<any[]>`, but on the hook path it returned `hookContext.result` with nothing re-checking the value after the `afterFind` dispatch. A handler assigning `ctx.result = { records: [ … ] }` therefore made a read declared to resolve to an array resolve to an envelope instead — silently, with no throw, no diagnostic and no log, while roughly 140 call sites read the answer as an array on the strength of the declaration.
9+
10+
The engine now refuses that, immediately after the `afterFind` dispatch and ahead of the two consumers that already assume the array (secret-field masking and the `__search` companion strip). The refusal is a named error, `FindHookResultNotArrayError`, carrying the registered ADR-0112 code `FIND_HOOK_RESULT_NOT_ARRAY` and HTTP `500`; its message names the hook event and the object, and `developerMessage` carries the remedy.
11+
12+
**Shaping stays legal, and nothing about it changes.** A handler may still mutate rows in place, delete keys, filter rows out, or assign a *different array* built from them — `Array.isArray` is the whole predicate, deliberately, so that `ctx.result = ctx.result.map(…)` keeps working. Only the container is protected.
13+
14+
What to do if this refusal fires:
15+
16+
- to answer no rows, assign `[]`;
17+
- to refuse the read, `throw` from the handler — the supported way for any hook guard to say no;
18+
- to hand a caller a different structure, build it in the caller, not in the hook.
19+
20+
`@objectstack/spec` widens by one member: `FIND_HOOK_RESULT_NOT_ARRAY` joins `ERROR_CODE_LEDGER` under `@objectstack/objectql`, so the generated `ErrorCode` union — and therefore `ApiErrorSchema.code` — accepts it. Additive: no existing code is removed or renamed.
21+
22+
Scope: this closes the one `return hookContext.result` site in the engine with a concrete declared shape to violate. `findOne`, `update` and `delete` declare `Promise<any>` and carry no enforceable declaration; that is a separate question about those declarations and is deliberately not answered here.

‎content/docs/permissions/system-context.mdx‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,17 +109,17 @@ that silently does not happen.
109109

110110
| # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
111111
|:--|:---|:---|:---|:---|
112-
| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11746` |
113-
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11929` |
114-
| 20 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped | `objectql/src/engine.ts:10394` |
115-
| 21 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10527`, `readonly-strict-errors.ts:66` |
116-
| 22 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:6252` |
117-
| 23 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3929`, `:3939`, `:3966` |
112+
| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11767` |
113+
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11950` |
114+
| 20 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped | `objectql/src/engine.ts:10415` |
115+
| 21 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10548`, `readonly-strict-errors.ts:66` |
116+
| 22 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:6253` |
117+
| 23 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3930`, `:3940`, `:3967` |
118118
| 24 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` |
119119
| 25 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:99` |
120-
| 26 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6951` |
121-
| 27 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:12548` |
122-
| 28 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:12477` |
120+
| 26 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6952` |
121+
| 27 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:12569` |
122+
| 28 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:12498` |
123123
| 29 | **Bulk data event `organizationId` OMITTED** — the batch is published "not asserted" | plugin-security | Get: nothing — the `data.records.*` event still publishes. Lose: the per-organization attribution: this exit is taken before the security middleware composes any tenant wall, so it records no Layer 0 verdict on the operation (`OperationContext.tenantLayer0Verdict`, #15813), and the engine's bulk producer — which reads that recorded verdict and nothing else — omits the key rather than filling it from the caller's `tenantId`; a tenant-scoped consumer then does not deliver the event inside an organization wall (#15225) | `security-plugin.ts:1686` |
124124

125125
### 3. Sharing (`plugin-sharing`)
@@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them.
179179

180180
| # | Site | Package | What it does |
181181
|:--|:---|:---|:---|
182-
| 62 | `objectql/src/engine.ts:3736` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183-
| 63 | `objectql/src/engine.ts:14995` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
182+
| 62 | `objectql/src/engine.ts:3737` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183+
| 63 | `objectql/src/engine.ts:15016` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
184184
| 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report |
185185
| 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across |
186186

@@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs.
195195
|:---|:---|:---|
196196
| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:2032` (rationale at `:1942`–`1944`, #3760), `flow.zod.ts:743` |
197197
| "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) |
198-
| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10377`–`10394` |
198+
| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10398`–`10415` |
199199
| "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1590` (#3493 / #6640) |
200200
| "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` |
201201
| "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` |

‎content/docs/references/api/contract.mdx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data);
2727

2828
| Property | Type | Required | Description |
2929
| :--- | :--- | :--- | :--- |
30-
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +295 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) |
30+
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +296 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) |
3131
| **declaredCode** | `string` | optional | The producer-declared code, verbatim, when it is not a member of the closed `code` vocabulary — the open, author-authored channel (app-specific spellings; ADR-0112) |
3232
| **message** | `string` | ✅ | Readable error message |
3333
| **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim. Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution for anything unmarked. Status-agnostic; never replaces `message`. |
@@ -180,6 +180,7 @@ const result = ApiErrorSchema.parse(data);
180180
* `FILE_NOT_FOUND`
181181
* `FILTER_TOKEN_UNKNOWN`
182182
* `FILTER_TOKEN_UNRESOLVED`
183+
* `FIND_HOOK_RESULT_NOT_ARRAY`
183184
* `FLOW_CONVERSION_CONFLICT`
184185
* `FLOW_DISABLED`
185186
* `FLOW_FAILED`

‎content/docs/references/api/error-code-ledger.mdx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ const result = ErrorCode.parse(data);
296296
* `FILE_NOT_FOUND`
297297
* `FILTER_TOKEN_UNKNOWN`
298298
* `FILTER_TOKEN_UNRESOLVED`
299+
* `FIND_HOOK_RESULT_NOT_ARRAY`
299300
* `FLOW_CONVERSION_CONFLICT`
300301
* `FLOW_DISABLED`
301302
* `FLOW_FAILED`

0 commit comments

Comments
 (0)