Repository navigation
Commit ed3546f
Fixes #20013
Clause-②: no (narrowing)
## What this fixes
Step 3.7 of the security middleware is the Layer 0 tenant write wall
(ADR-0095 D1, ADR-0105 D5). Its contract,
`packages/plugins/plugin-security/src/security-plugin.ts` lines
3239-3248 on `origin/main` `246314dffe`:
> Both close identically here: a SUPPLIED (non-empty) `organization_id`
in the write payload must satisfy the SAME Layer 0 filter the read side
uses (isolation active, tenant object, platform-admin posture exemption,
fail-closed on a missing active org). For UPDATE this makes
`organization_id` effectively immutable in non-platform user contexts:
the only value that passes is the caller's active org (which — since the
pre-image already scoped the target to that org — equals the row's
current org), so a re-point to any OTHER tenant is denied. A bulk update
carrying a cross-tenant `organization_id` change-set is caught too (the
check inspects the change-set value, not a per-row post-image).
The wall judged `opCtx.data`, the payload as the caller sent it, before
`next()` runs the engine's `beforeInsert` / `beforeUpdate` chain. A
value a hook wrote into `organization_id` was never judged by the wall,
so the row was stored in whatever organization the hook named. The
engine records the insert twin of the same gap at
`packages/objectql/src/engine.ts` lines 11846-11847 on `246314dffe`:
「The Layer 0 tenant wall still judges the PRE-hook image — filed
separately, and the fix's host is this seam.」
The wall now also judges the row the engine is about to store, through
the seam the row-level `check` already uses
(`OperationContext.postHookWriteImageCheck`). The refusal is the wall's
existing one: `PERMISSION_DENIED` / 403, nothing stored. The judgement
of the payload as sent stays, so this change only ever refuses more. No
engine change.
## Mechanism hypotheses (dispatch Section 2), measured
Base: `origin/main` `26550c6603`. Real `SecurityPlugin` and `ObjectQL`,
`isolated` posture, driver-sql (better-sqlite3) and driver-sqlite-wasm.
The fixture is a tenant object whose `beforeInsert` and `beforeUpdate`
hook copies another payload field into `organization_id`, and a caller
whose active organization is `org_a`.
1. **Held.** Step 3.7 judged only rows whose `organization_id` was
present in `opCtx.data` (the `suppliedRows` filter), before `next()`.
2. **Reproduced on both drivers, all three legs** (identical on both):
| leg | outcome on the base | stored `organization_id` |
|:--|:--|:--|
| control: by-id update supplies `org_b` | refused, `PERMISSION_DENIED`
/ 403 ("the update would place 'qa_account' in another tenant") |
`org_a`, unchanged |
| control: insert supplies `org_b` | refused, `PERMISSION_DENIED` / 403
| no row |
| by-id update, hook writes `org_b` | **admitted** | **`org_b`** |
| insert, hook writes `org_b` | **admitted** | **`org_b`** |
| array insert, hook writes `org_b` on the second row | **admitted** |
first row `org_a`, second **`org_b`** |
| predicate update (`multi: true`), hook writes `org_b` | **admitted** |
**`org_b`** |
| control: by-id update, hook writes `org_a` | admitted | `org_a` |
3. **The seam is installed only where a business `check` applies**,
measured: with no row-level policy on the object, an inner middleware
saw `postHookWriteImageCheck` absent on every leg above. So the wall
gets its own installation condition: a walled posture, a tenant object,
a caller Layer 0 does not exempt (that is,
`computeWriteTenantCheckFilter` returns a filter). When step 3.6 also
installed its judgement, the two are composed into the one handle the
engine runs, in the order the middleware judges in (the `check`, then
the wall).
**A second finding shaped the fail-closed guard.** The post-`next()`
guard now covers the new installation. The ADR-0094 permission-set data
door executes an insert or update of `sys_permission_set` itself,
through the metadata protocol, and never calls `next()`. Measured on the
base with a platform administrator (active organization `org_a`) under
`isolated`: Layer 0 walls the object (`organization_id = org_a`), the
insert is admitted, and the engine's write never runs. A guard-covered
wall seam alone would turn that into a 403. No engine write runs there,
so no hook chain runs, and the payload judgement has already cleared the
only `organization_id` such a write can carry. The guard therefore
stands down for a seam that carries the wall alone on a write the door
executed itself. The fact is observed by a wrapper around the door's
registration, which records a write the door never passed to `next()` in
a plugin-private `WeakSet`. No operation field is involved that another
middleware could set. A seam carrying a row-level `check` is not stood
down: the data door keeps failing closed under one, exactly as before.
**ADR-0095 D1 "Not touched"** records that D1 added no tenant post-image
check to `computeWriteCheckFilter`. Its reason is that such a check
"would risk denying legitimate inserts before the auto-stamp runs". This
change does not touch `computeWriteCheckFilter`, and it judges an image
only when the image names an organization. An absent value (the
auto-stamp's to fill) is never judged, and a control pins that. ADR-0105
D5 affirms the direction: an explicit value is validated against the
membership set or equality.
## What changed
- `packages/plugins/plugin-security/src/security-plugin.ts`
- Step 3.7 computes the wall when a payload names an organization (as
before) or the posture walls. A `single` posture pays for nothing new.
- One refusal (`denyTenantPlacement`) serves both halves. The step
installs the stored-row judgement whenever the wall applies (insert, or
a non-array update, as step 3.6 scopes it), composed after step 3.6's
judgement when one is installed.
- The post-`next()` guard covers the new installation with the data-door
stand-down above. Its developer message names what was not evaluated:
the business-check sentence is unchanged byte for byte, and a wall-only
seam gets its own sentence.
- The data door is registered through the observing wrapper.
-
`packages/plugins/plugin-security/src/tenant-wall-post-hook-image.test.ts`
(new): 34 cells, 17 per driver.
- 11 existing plugin-security test files: engine doubles (see "Surface
beyond the claim").
- `.changeset/20013-tenant-wall-post-hook.md`:
`@objectstack/plugin-security` minor, BREAKING, remedy, `not-required
(no-migration-prescription)`.
## Tests
New file, real `SecurityPlugin` and `ObjectQL` on both SQL drivers,
`isolated` posture unless a cell says otherwise. Every refusal asserts
`code` `PERMISSION_DENIED`, `status` 403 and the wall's message ("the
insert/update would place 'OBJECT' in another tenant"), then reads the
table straight off the driver.
- **Negative cells:**
- a hook-written out-of-scope organization, refused on four paths: a
by-id update, an insert, an array insert (the whole write refused) and a
predicate update;
- the same with a business `check` installed and passing (one composed
seam);
- the composed seam still runs the `check` (an in-scope insert the check
refuses is refused);
- `group` posture: outside the membership set refused, inside admitted;
- fail-closed: a host that strips the installed wall-only seam is
refused, with the guard's wall sentence.
- **Controls:**
- an in-scope hook write is admitted on all three paths;
- a supplied out-of-scope value is refused before the hook chain, as
before;
- only refuses more: a supplied out-of-scope value stays refused when a
hook would replace it with an in-scope one;
- an update that does not touch the column is admitted on both update
paths;
- an insert that leaves the column absent is admitted and lands in
`org_a`;
- a platform administrator on a `private` object is exempt;
- a system-context write is ungated;
- the `single` posture is unchanged;
- the data door under `isolated`: admitted, and the engine's write never
ran.
**Failing first.** The file was committed before the fix (`8afaec51a1`).
On the base it gave 14 red (the 7 negative cells that existed then, × 2)
and 18 green. Every red read "expected a refusal, got a completed
write".
**Ablations**, each through `scripts/ablation-replace.mjs` in WRAP mode,
with the anchor hit 1 → 0 and a blob change reported by the tool. Each
ran under an outer `trap` restoring from `HEAD`, and was then proven
restored (blob == HEAD `278b25f19e`, empty `git diff HEAD`). They ran on
`e43cda19b4`, whose blobs for `security-plugin.ts` and the pin file
equal the final head's. The subject is imported relatively by the test
file and `@objectstack/objectql` is aliased to `src/` in this package's
`vitest.config.ts`, so no `dist/` sits between a mutation and the run.
| # | mutation | red (of 34) |
|---|---|---|
| A1 | the stored-row wall judgement never refuses | 12: by-id, insert,
array insert, predicate, composed-with-check, `group`, × 2 |
| A2 | the wall's seam installed only when a business `check` is (the
old condition) | 12: by-id, insert, array insert, predicate, `group`,
fail-closed, × 2 |
| A3 | the guard stands down for every wall-only seam | 2: fail-closed,
× 2 |
| A4 | no data-door stand-down | 2: the data door, × 2 |
| A5 | the payload judgement dropped | 4: supplied-value control and
only-refuses-more, × 2 |
| A6 | an absent `organization_id` judged too | 2: the absent-value
insert control, × 2 |
| A7 | the composed seam drops the business `check` | 2: the
composed-check cell, × 2 |
A5 has an extra reading. Without the payload judgement, a *supplied*
out-of-scope value is admitted but lands nowhere. The engine's static
`readonly` strip drops a caller-sent `organization_id`, which the
registry injects as `readonly: true`, while a hook-written value is
exempt from that strip. That exemption is why the hook path reached the
store, and why the payload refusal is the loud half of the supplied
case.
**Suites** (final head `cce969cf4d`, after merging `origin/main`
`7766b62282`, closure rebuilt):
- plugin-security: 133 files, 2648 tests passed;
- plugin-auth: 114 files, 2439 tests passed;
- runtime: 278 files, 3962 passed, 1 skipped;
- plugin-security `typecheck`: exit 0; the test layer compiles all 131
test files (`--listFiles`), and its ledger holds 0 files / 0 errors.
## Gates
`node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` on `cce969cf4d` derived 62 families: the dispatch-time 49
plus 13 more. The 13 are `check-adr-0087-registration` and
`check-empty-changeset` (each with its self-test),
`release-rehearsal-clone --self-test`, `check:engine-double-contract`,
`check:objectql-double-limit`, `check:objectui-changeset`,
`check:pm-changeset-deadline-census`, `check:query-options-erasure`,
`check:type-check-coverage`, `check:type-check-debt` and
`check:where-matcher`. All 62 ran, and `--ran` reconciled 62 derived /
62 run / 0 NOT-MEASURED, every line carrying its exit code, all 0.
`check:dual-build-cjs-loads`, `check:i18n` and `check:type-check-debt`
first answered PREREQUISITE NOT MET (exit 3) and were re-run green after
a full workspace build. The board-probing `GITHUB_TOKEN=… node
scripts/check-issue-citations.mjs` passed: 9 citations resolve.
Narrowed lint: `eslint --no-inline-config --format json` over the 13
touched `.ts` files reported 13 files, 0 errors, 0 warnings. It is a
full measurement for these files for three reasons. The population is
the `**/*.{ts,…}` and `packages/**` blocks of `eslint.config.mjs`. The
count, 13, comes from the JSON output. The config never enables
type-aware linting (no `parserOptions.project`, no typed rules), so this
diff cannot move the verdict for any untouched file. The repo-wide `pnpm
lint` and the Dogfood Regression Gate are left to CI.
## Surface beyond the claim, with reasons
The new installation runs on every walled insert and update. So every
plugin-security test harness that stands in for the engine with a
terminal that skips the seam, under a walled posture, was refused by the
fail-closed guard (48 red on the first full run). Each double now runs
the seam the way the engine does: an insert's rows as sent (no hooks in
a double), and on an update the by-id row or the matched rows merged
with the payload, through the producer's dispatch predicate where the
double dispatches updates.
- The 10 files that went red on the first run: `authz-matrix-gate`,
`can-write-object-admission`, `check-only-write-scope`,
`controlled-by-parent-detail-write-authority`,
`controlled-by-parent-master-widener`, `explain-write-verdict-inputs`,
`no-active-organization-write-refusal`, `row-write-widener-composition`,
`select-only-write-visibility` and `tenant-layer0-verdict-on-operation`.
- `explain-dependency-fault`, which arrived with the merge of
`origin/main` (PR #20030) and went red on the merged tree for the same
reason.
- `check:engine-double-contract` is green, with no ledger change.
- Census outside the package: `grep -rln postHookWriteImageCheck
packages --include=*.test.ts` names only plugin-auth's
`sys-user-self-service-route.test.ts` (PR #20012's). The hand-made
SecurityPlugin hosts under a walled posture are runtime's
`share-links-enforcement-context` and
`standalone-stack-seeder-declaration-copy`. All pass unchanged
(plugin-auth and runtime suites green), so none is touched.
## Behaviour that changes (all in the refusing direction)
- Under `isolated` or `group`, an insert, by-id update or predicate
update whose hook chain leaves `organization_id` outside the caller's
organization scope (or the delegator's, ADR-0090 D10) is refused, and
nothing is stored.
- A walled write on a host that installs the wall's judgement and never
runs it is refused after the write, with an `error` log. The ADR-0094
data door is the stated exception, for a wall-only seam.
## Pending changesets
This change makes no sentence in a pending changeset false. None says
the tenant wall is unchanged. The "admitted as before" and "judged
exactly as before" sentences in `19950-rls-check-multi-row-writes.md`
and `19989-by-id-update-post-hook-check.md` are scoped to the row-level
`check`, whose judgement this change does not alter. So no deliberate
correction was made, and `check-empty-changeset` is green.
## Acceptance notes
- `packages/objectql/src/engine.ts` lines 11846-11847 still read "The
Layer 0 tenant wall still judges the PRE-hook image — filed separately".
After this change that sentence is false: the wall judges the stored row
through that same seam. The file was held by the engine lane at
dispatch, and this PR does not touch it. Suggested replacement for the
engine lane: "(The Layer 0 tenant wall judges this row too, through the
same seam; #20013.)".
- Cost:
- Under a walled posture, every non-system insert and update now
computes the Layer 0 filter.
- A walled predicate update now always pays the engine's memoized
matched-row read, because the seam receives matched rows merged with the
payload. On an object with an update hook or a row-reading rule that
read already happened. On one with neither it is new. There it is one
`driver.find` over the composed `where`, with no row ceiling on that
path. The row-level `check` seam (#19950) already imposes the same cost
where a `check` applies. NOT MEASURED: bulk-update latency or memory.
- An absent or emptied `organization_id` is not judged by either half.
That mirrors step 3.7's "supplied (non-empty)" scope and ADR-0095 D1's
reason. A hook that clears the column on an update therefore lands a row
with no organization. It is not measured here, and no declared contract
covers it.
- An array UPDATE payload gets no stored-row wall judgement, as step 3.6
gets none (the engine takes one payload per update). The payload
judgement still covers it.
- A by-id update is now judged by the wall twice: the payload as sent,
before `next()`, and the stored row, in the engine. The first is what
keeps this change refusing only more (A5).
- The merge commit `fe1579223a` carries no `Claude-Session` trailer; the
other commits carry the model-free pair.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 655e8c0 commit ed3546f
14 files changed
Lines changed: 870 additions & 71 deletions
File tree
- .changeset
- packages/plugins/plugin-security/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 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
161 | 161 | | |
162 | 162 | | |
163 | 163 | | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
164 | 169 | | |
165 | 170 | | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
166 | 176 | | |
167 | 177 | | |
168 | 178 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
388 | 388 | | |
389 | 389 | | |
390 | 390 | | |
391 | | - | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
392 | 404 | | |
393 | 405 | | |
394 | 406 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
283 | 283 | | |
284 | 284 | | |
285 | 285 | | |
286 | | - | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
287 | 296 | | |
288 | 297 | | |
289 | 298 | | |
| |||
390 | 399 | | |
391 | 400 | | |
392 | 401 | | |
393 | | - | |
| 402 | + | |
394 | 403 | | |
395 | 404 | | |
396 | 405 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
387 | 387 | | |
388 | 388 | | |
389 | 389 | | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
390 | 394 | | |
391 | 395 | | |
392 | 396 | | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
393 | 402 | | |
394 | 403 | | |
395 | 404 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
283 | 283 | | |
284 | 284 | | |
285 | 285 | | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
286 | 290 | | |
287 | 291 | | |
288 | 292 | | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
289 | 298 | | |
290 | 299 | | |
291 | 300 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| |||
244 | 244 | | |
245 | 245 | | |
246 | 246 | | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
247 | 260 | | |
248 | 261 | | |
249 | 262 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
172 | 172 | | |
173 | 173 | | |
174 | 174 | | |
175 | | - | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
176 | 183 | | |
177 | 184 | | |
178 | 185 | | |
179 | 186 | | |
180 | 187 | | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
181 | 193 | | |
182 | 194 | | |
183 | 195 | | |
| |||
265 | 277 | | |
266 | 278 | | |
267 | 279 | | |
268 | | - | |
| 280 | + | |
269 | 281 | | |
270 | 282 | | |
271 | 283 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
124 | 124 | | |
125 | 125 | | |
126 | 126 | | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
127 | 133 | | |
128 | | - | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
129 | 143 | | |
130 | 144 | | |
131 | 145 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
256 | 256 | | |
257 | 257 | | |
258 | 258 | | |
259 | | - | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
260 | 267 | | |
261 | 268 | | |
262 | 269 | | |
263 | 270 | | |
264 | 271 | | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
265 | 277 | | |
266 | 278 | | |
267 | 279 | | |
| |||
354 | 366 | | |
355 | 367 | | |
356 | 368 | | |
357 | | - | |
| 369 | + | |
358 | 370 | | |
359 | 371 | | |
360 | 372 | | |
| |||
0 commit comments