Skip to content

Commit c393b56

Browse files
claude[bot]claude
andauthored
fix(objectql): publish the record's organization on every DataEvent (#15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970) `DataEventSchema.organizationId` was declared and published by the spec half but populated by nothing, so every `data.record.*` event went out with the key absent — which the contract requires a consumer to read as "this record is behind no organization wall". `publishDataEvent` now resolves it from the row itself: the written record on `created`, the post-state on `updated`, and the by-id branch's already-read pre-image on `deleted`, so no per-event read is bought. The record's organization, never `ExecutionContext.tenantId` — that is the caller's active org, and the two diverge on exactly the system/unscoped write this key most needs to label correctly. Absence keeps one spelling: the key is omitted, never `''` (which the schema refuses outright, dropping the whole event) and never an explicit `undefined` (which survives `parse` as a present key). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * docs(permissions): re-anchor the system-context census after the engine line shift Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the only correct writer for this table. Pure line rot: the `eventOrganizationId` helper and its threading shifted every later line in `packages/objectql/src/engine.ts`, so 14 anchors (15 citation sites — one source line is cited twice) pointed at the wrong lines. No population and no classification change: still 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read — the same figures as before the shift. `--fix` did not refuse, and the diff is digits and nothing else (12 lines added, 12 removed, identical once digits are stripped). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent dd999ce commit c393b56

4 files changed

Lines changed: 385 additions & 12 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): a published `DataEvent` now names the organization the RECORD belongs to
6+
7+
`DataEventSchema.organizationId` has been declared and published since the spec
8+
half landed, and its TSDoc states the obligation on the producer's side: *"a
9+
producer that omits the key on an organization-stamped row publishes a
10+
cross-tenant event, which is fixed at the publish site — never by a
11+
consumer-side lookup."* The engine populated it on no event at all. Every
12+
`data.record.created` / `updated` / `deleted` went out with the key absent,
13+
which a consumer is required to read as *"this record is behind no organization
14+
wall"* — so an organization-stamped row was published as an unwalled one, and a
15+
tenant-scoped fan-out had nothing to discriminate on.
16+
17+
`publishDataEvent` now resolves the organization from the row itself and spreads
18+
the key in when there is one. The row is already in hand at all three call
19+
sites — the written record on `created`, the post-state on `updated`, and the
20+
pre-image on `deleted` (the by-id branch reads it unconditionally for its
21+
existence gate) — so this buys **no** per-event read: the key exists precisely
22+
to keep a per-event lookup off the fan-out path.
23+
24+
Three properties are deliberate:
25+
26+
- **The RECORD's organization, never the caller's.** The row's own tenant column
27+
is the only source consulted. `ExecutionContext.tenantId` is the caller's
28+
*active* organization; the two coincide on an ordinary tenant write and
29+
diverge on a system or unscoped one, where substituting it would mislabel an
30+
administrator's write into another organization as belonging to the
31+
administrator's.
32+
- **Absence has exactly one spelling: the key is omitted.** An object that is
33+
not tenant-scoped, a row whose column is empty, and a value no id can be read
34+
off all publish the key absent rather than `null`, `''` or an explicit
35+
`undefined`. The schema refuses the empty string outright, so producing one
36+
would have thrown at the publish site and dropped the event entirely.
37+
- **The column is resolved the way the write path resolves it** — the
38+
`tenancy.enabled: false` opt-out, then a declared `tenancy.tenantField`, then
39+
the injected `organization_id` — so the event cannot name an organization for
40+
a column the engine does not actually scope by. Note the two spellings differ:
41+
the column is `organization_id`, the published key is `organizationId`.
42+
43+
No schema, no accepted shape and no public export moves: the key was already
44+
declared, already validated and already part of what consumers parse. Only the
45+
implementation changed, from omitting a declared key to populating it.

content/docs/permissions/system-context.mdx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,18 +109,18 @@ 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:11290` |
113-
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11473` |
114-
| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10025` |
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:11373` |
113+
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11556` |
114+
| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10106` |
115115
| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1795` |
116-
| 22 | 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:10073`, `readonly-strict-errors.ts:66` |
117-
| 23 | **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:5892` |
118-
| 24 | 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:3736`, `:3746`, `:3773` |
116+
| 22 | 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:10154`, `readonly-strict-errors.ts:66` |
117+
| 23 | **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:5973` |
118+
| 24 | 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:3799`, `:3809`, `:3836` |
119119
| 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` |
120120
| 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:99` |
121-
| 27 | 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:6590` |
122-
| 28 | 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:12085` |
123-
| 29 | 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:12014` |
121+
| 27 | 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:6671` |
122+
| 28 | 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:12172` |
123+
| 29 | 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:12101` |
124124

125125
### 3. Sharing (`plugin-sharing`)
126126

@@ -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:3543` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183-
| 63 | `objectql/src/engine.ts:14523` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
182+
| 62 | `objectql/src/engine.ts:3606` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183+
| 63 | `objectql/src/engine.ts:14616` | 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:1971` (rationale at `:1881``1883`, #3760), `flow.zod.ts:702` |
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:10008``10025` |
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:10089``10106` |
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:1580` (#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` |

0 commit comments

Comments
 (0)