Repository navigation
Commit afc3b64
fix(objectql): scope the lookup existence probe to the caller's organization (#19836)
Fixes #19808
Clause-②: no
## What this changes
`ObjectQL.referenceExists`, the probe behind `assertReferencesResolve`
(the write-path lookup existence check), read under a bare `{ isSystem:
true }`. That context has no `tenantId`, so `buildDriverOptions` sent
the driver no tenant and the check looked in every organization.
The probe now runs under `ObjectQL.referenceCheckContext(context)`. This
is the `sudo()`-shaped `{ ...callerContext, isSystem: true }` that the
pre-delete reference check already uses, so both reference checks now
build their elevated context the same way:
- `isSystem` still skips RBAC/RLS/FLS. That is why the original check
was elevated: a user may link to a record they are not allowed to read.
- The caller's `tenantId` now reaches the driver. Under the `group`
posture, `accessible_org_ids` reaches it too and is widened into
`tenantIds`.
- A record in another organization now gets the same
`reference_not_found` refusal as a record that exists nowhere.
- `buildDriverOptions` still sends no tenant for `tenancy.enabled:
false` objects and for federated objects. References to those objects
keep working from any organization.
`assertReferencesResolve` now passes its `context` (already its 4th
parameter) to the probe. All three call sites (insert, update by id,
bulk update) already passed `opCtx.context`, so no call site changed.
The dangling-reference audit calls the probe without a context. It gets
`referenceCheckContext(undefined)`, which is `{ isSystem: true }`, the
same as before. The audit's behaviour does not change.
Code changes are confined to `assertReferencesResolve` and
`referenceExists` (one argument, one context expression, rewritten
docblocks). The docblock section that explained why the probe ignored
tenancy now says why it skips RLS but keeps the tenant filter.
## The card's first step: measured before the fix
Question from triage: after the cross-organization reference is stored,
can the org-X caller read any field of the org-Y row through any read
path?
Measured on `origin/main` c118524. The setup was a real
`SecurityPlugin` (posture `isolated`, `org-scoping` on) over a real
`ObjectQL` over `SqlDriver` (better-sqlite3 `:memory:`). The test was a
scratch file under `plugin-security`, which aliases
`@objectstack/objectql` and `@objectstack/driver-sql` to source. The
file was deleted after the run and never committed.
| read path, org-X member, stored contact.account = org-Y account |
result |
|:--|:--|
| `find` with `expand: { account: {} }` | `account` stays the bare id
`acc_y`, no fields |
| `find` with `expand: { account: { fields: [name, secret, status] } }`
| bare id `acc_y`, no fields |
| `findOne` with `expand` | bare id `acc_y`, no fields |
| direct `find` of the org-Y account | `[]` |
| roll-up `summary` on the org-Y parent (count of contacts) | org-Y
`contact_count` stays 0. The org-X insert threw `SummaryRecomputeError`
after the row was written, because the recompute's update is
tenant-scoped and cannot find the parent |
| `count` of the org-Y account | 0 |
| master-detail header with a `requiredWhen: parent.status == 'locked'`
detail field, note omitted | header = org-Y locked row: `note:
required`. Header = org-Y open row: **write committed** |
So no read path returned an org-Y field value. The last row is
different: a parent-scoped predicate over an org-Y header can be
observed. `resolveMasterDetailParents` / `resolveMasterDetailParent`
read the header under a bare `{ isSystem: true }` too, and this oracle
**survives this fix**. After the fix: locked header → `note: required`,
open header → `reference_not_found`. The two answers still differ,
because `evaluateValidationRules` runs before `assertReferencesResolve`.
This is reported to the PM as a separate finding. It is not changed
here: it is outside this card's two methods, and open PR #19728 also
edits `engine.ts`.
## After the fix: same harness
| write, org-X member | before | after |
|:--|:--|:--|
| lookup to a row only in org Y | committed (plus
`SummaryRecomputeError` on the roll-up) | `VALIDATION_FAILED` /
`reference_not_found` |
| lookup to an id that exists nowhere | `VALIDATION_FAILED` /
`reference_not_found` | same |
| lookup to an org-X row | commits | commits |
| lookup to a `tenancy: { enabled: false }` row | commits | commits |
| lookup to an org-X row of an object the member may NOT read
(per-object `allowRead: false`, direct read = 403 `PERMISSION_DENIED`) |
commits | commits (RLS still skipped under the real SecurityPlugin) |
| lookup to an org-Y row of that unreadable object | commits |
`reference_not_found` |
| lookup to a NULL-organization row of an object exempted only by the
deployment's `platformGlobalObjects` | commits | commits |
| lookup to an org-Y-stamped row of that deployment-exempted object |
commits | `reference_not_found` |
The last row is a behaviour change, and it is the hazard the dispatch
asked me to measure. The driver still filters a
`platformGlobalObjects`-exempted object by organization (#15831, open,
`pm:blocked`). So the probe now agrees with what the caller's own `find`
of that object already returns (measured: only the NULL-organization
row). This PR does not work around it. The changeset tells deployments
about it.
## Tests
New file `packages/objectql/src/engine-reference-tenant-scope.test.ts`,
15 cases. objectql cannot import `driver-sql`, so the test driver copies
`SqlDriver.applyTenantScope`: it filters on `DriverOptions.tenantId`,
keeps `OR organization_id IS NULL`, and honours the `tenantIds` union.
It also records every call's options.
- Refusal, on all three doors (insert, update by id, bulk update):
`ValidationError`. `resolveThrownHttpError` reads `{ status: 400, code:
'VALIDATION_FAILED' }` and the fields are `[{ field: 'account', code:
'reference_not_found' }]`. Nothing is written or repointed.
- Oracle shut, on all three doors: "only in another organization" and
"exists nowhere" give the same envelope. The messages are also identical
once the caller's own id is removed.
- Controls: a same-org reference commits on all three doors. A
`tenancy.enabled: false` target commits, and its probe's driver options
carry no `tenantId`; the row is stamped with another org on purpose, so
it passes only because the engine withholds the tenant. An `isSystem`
write stays unchecked and runs no probe.
- Wiring checks: the probe's operation context is `{ isSystem: true,
tenantId, userId }` and the driver sees `tenantId`. Under the `group`
posture the membership union reaches the probe. The audit's unscoped
probe is pinned, so any change to its behaviour has to be a deliberate
decision.
Ablation, run on committed HEAD 92662fe through
`scripts/ablation-replace.mjs`, with an EXIT/INT/TERM trap that restores
and checks the blob hash. The test imports `./engine.js` from source, so
no dist build was involved.
- Leg A: the probe context was changed back to the pre-fix bare `{
isSystem: true }`, with an injected marker. On disk: anchor 1 → 0,
marker 0 → 1. Result: **7 failed / 8 passed**. Failed: refusal ×3,
oracle ×3, probe wiring.
- Leg B: a hand-picked `{ isSystem: true, tenantId }` with no spread.
Result: **2 failed**: probe wiring (`userId` missing) and `group`
posture (a legitimate reference refused).
- Restore after each leg: blob equals HEAD (`4ac24d149603`) and `git
diff HEAD` is empty. Control run afterwards: 15/15.
## Local verification (final head 19624a5)
- `pnpm --filter @objectstack/objectql test`: 304 files / 5072 tests
passed. The trailing `-- --maxWorkers=2` in my command was dropped by
vitest; the whole package ran, which is what I intended.
- `pnpm --filter @objectstack/objectql typecheck`: exit 0.
`check:test-typecheck` OK with the ledger unchanged (40 files / 234
errors / 65 signatures). `tsc --listFilesOnly -p tsconfig.test.json`
includes the new test file.
- Downstream suites that combine a real engine, tenants and lookups
(objectql `dist` rebuilt, or aliased to source): plugin-security 6 files
/ 88 tests, plugin-sharing 4 / 284, plugin-audit 4 / 83,
service-automation 1 / 6, service-settings 1 / 18. All passed.
- `node scripts/pm/dispatch-gates.mjs --commands` listed 64 commands.
All were run on 19624a5, with each exit code captured before any
pipe. 62 exited 0. `--ran` verdict: `64 derived famil(ies) accounted for
— 62 run, 2 NOT-MEASURED (2 DERIVED from a recorded exit 3)`.
- NOT MEASURED: `check:dual-build-cjs-loads` (exit 3, PREREQUISITE NOT
MET). It needs every package's `dist`, and a full workspace build does
not fit the 10-minute foreground limit. Narrower check instead:
`require()` of objectql's `dist/index.js` and `dist/core.js` both load
(exit 0).
- NOT MEASURED: `check:type-check-debt` (exit 3, PREREQUISITE NOT MET).
It needs 14 unbuilt workspace packages for the same reason. objectql's
own test layer is covered by `check:test-typecheck` above.
- `check:query-options-erasure` failed on the first pass: test surface
236 → 238, from two `as any` option bags in the new test. Fixed in
19624a5 by typing them. It is now at the ceiling (236).
- `check-engine-split-ratio --days 90` first refused on the shallow
clone. It passed after `git fetch --shallow-since=2026-06-18 origin
main`.
## Acceptance notes
- The `inspectDanglingReferences` docblock still says the audit "can
never be more or less strict than the rule it reports on". That is no
longer true for cross-organization references: the write check refuses
them, and the audit's unscoped probe does not report them. The
`referenceExists` docblock states this gap. The audit's docblock and its
behaviour are left alone, because both are outside this card's region
and the audit question is open with the PM.
- The docblock of `referenceCheckContext` still describes only the
pre-delete check. The write-path probe now uses it too. Left as is
(outside the region).
- Before the fix, a cross-organization child write under a roll-up
parent was written and then threw `SummaryRecomputeError`: HTTP 500
after commit. Non-system writes are now refused before the write.
`isSystem` writes that name another organization's parent were not
measured. Issue not filed; no current owner.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01TEhopqrWQYBycZzyJHpAZr)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent a6a4361 commit afc3b64
3 files changed
Lines changed: 432 additions & 12 deletions
File tree
- .changeset
- packages/objectql/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
Lines changed: 369 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
0 commit comments