Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/find-afterfind-array-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
"@objectstack/objectql": minor
"@objectstack/spec": minor
---

`ObjectQL.find()` now guarantees the array it declares: an `afterFind` hook that replaces the result container is refused with `FIND_HOOK_RESULT_NOT_ARRAY`.

`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.

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.

**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.

What to do if this refusal fires:

- to answer no rows, assign `[]`;
- to refuse the read, `throw` from the handler — the supported way for any hook guard to say no;
- to hand a caller a different structure, build it in the caller, not in the hook.

`@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.

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.
24 changes: 12 additions & 12 deletions content/docs/permissions/system-context.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,17 @@ that silently does not happen.

| # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
|:--|:---|:---|:---|:---|
| 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` |
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11929` |
| 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` |
| 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` |
| 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` |
| 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` |
| 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` |
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11950` |
| 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` |
| 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` |
| 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` |
| 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` |
| 24 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` |
| 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` |
| 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` |
| 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` |
| 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` |
| 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` |
| 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` |
| 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` |
| 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` |

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

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

Expand All @@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs.
|:---|:---|:---|
| "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` |
| "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) |
| "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` |
| "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` |
| "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) |
| "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` |
| "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` |
Expand Down
3 changes: 2 additions & 1 deletion content/docs/references/api/contract.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **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) |
| **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) |
| **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) |
| **message** | `string` | ✅ | Readable error message |
| **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`. |
Expand Down Expand Up @@ -180,6 +180,7 @@ const result = ApiErrorSchema.parse(data);
* `FILE_NOT_FOUND`
* `FILTER_TOKEN_UNKNOWN`
* `FILTER_TOKEN_UNRESOLVED`
* `FIND_HOOK_RESULT_NOT_ARRAY`
* `FLOW_CONVERSION_CONFLICT`
* `FLOW_DISABLED`
* `FLOW_FAILED`
Expand Down
1 change: 1 addition & 0 deletions content/docs/references/api/error-code-ledger.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ const result = ErrorCode.parse(data);
* `FILE_NOT_FOUND`
* `FILTER_TOKEN_UNKNOWN`
* `FILTER_TOKEN_UNRESOLVED`
* `FIND_HOOK_RESULT_NOT_ARRAY`
* `FLOW_CONVERSION_CONFLICT`
* `FLOW_DISABLED`
* `FLOW_FAILED`
Expand Down
Loading
Loading