Skip to content

Commit f2626c7

Browse files
fix(plugin-security)!: the Layer 0 tenant write wall refuses an update that empties a row's organization (#22317)
Fixes #22278 Clause-②: no (narrowing) ## What changes `plugin-security`'s Layer 0 tenant write wall (`security-plugin.ts`, step 3.7, the tenant post-image check) judged only an organization a write *names*. A non-system update that *emptied* the row's organization was never judged, on either half of the step: the payload as sent, and the row the engine is about to store (the #20013 seam, by id and by predicate). On UPDATE the step now holds its stated invariant whole: where the tenant wall applies, the row's organization after a non-system update is one the wall admits. An emptied organization is refused on both halves with the wall's `403 PERMISSION_DENIED` (ADR-0112 envelope), and nothing is stored. The step's comment states the invariant. - INSERT keeps today's rule: an absent or empty organization is the stamp's to fill. - System writes, the platform-administrator posture exemption on posture-permitting objects, and the `single` posture are unchanged. - An update that does not touch the column, or keeps the row's own organization, is unchanged. Scope as ruled (triage `6059271770`, the seat's ruling B): one region of `security-plugin.ts`, one test file, one changeset. No `packages/spec`, no change to how `@objectstack/core` reads an organization-less grant, no change to `grant-holder-membership-refusal.ts`, nothing that widens access. ## Tests: `tenant-wall-organization-removal.test.ts` Both SQL driver families (driver-sql on better-sqlite3, driver-sqlite-wasm), `isolated` posture unless a cell says otherwise. Refusals assert `code` and `status`, that the refusal names the object, and that the stored rows are unchanged. - Negative pins (7 per driver): an emptied organization on both grant tables, by id and by predicate; the platform administrator on the grant tables (their posture grants no exemption); an ordinary tenant object on both update paths; the stored-row half (a value the `beforeUpdate` chain wrote) on both update paths. - Controls (10 per driver): an untouched-column update on both grant tables and on an ordinary tenant object; a system write; an explicit foreign organization, still refused; the platform-administrator exemption on a posture-permitting object, stored-row half included; the `single` posture; an INSERT keeps its stamp. | run | tree | result | |---|---|---| | before the fix (merged base, corrected controls) | `d3ec5074fb` | 12 negatives red, 20 controls green | | with the fix | `edafba04e9` | 34 of 34 green | ### The two controls that were red on base: wrong expectation, diagnosed "A platform administrator on a posture-permitting object is exempt" and "the `single` posture is unchanged" asserted that a caller-*sent* empty organization is stored on an ordinary tenant object. It is not, wall or no wall. There `organization_id` is the registry's injected `readonly` column, and the engine's static-readonly strip (`stripReadonlyFields`) drops a non-system caller's value before the statement; a value the hook chain wrote survives the strip. The grant tables declare the column themselves, writable, so a sent value is stored as sent. Measured in the corrected `single`-posture control, on both drivers: the injected column reads `readonly: true` and the grant table's does not; a sent empty value is stored on the grant table and dropped on the ordinary object; a hook-written one is stored. Both controls now read each store as the engine writes it (the platform-administrator control drives the stored-row half through the hook), and are green before and after the fix. ### Ablation One-shot, from the committed fix at `edafba04e9`, through `scripts/ablation-replace.mjs` (anchor count 1 to 0 on disk each time; every restore proven: blob equal to HEAD's, `git diff HEAD` empty). The subject is imported from source (`./security-plugin.js`), so no dist leg applies. | mutation | red | reading | |---|---|---| | A1: the payload-half refusal disabled | 2 (ordinary tenant object, both drivers) | on the grant tables the stored-row half still refuses: the halves overlap there | | A2: the stored-row-half refusal disabled | 2 (the hook-written cell, both drivers) | | | A3: the emptied-organization predicate disabled (both halves) | 14 (all 7 negatives, both drivers); 20 controls green | | A first A3 attempt was refused by the tool before anything ran (its anchor sat inside the replacement text); it measured nothing. ## Blast radius (at `edafba04e9`) - `@objectstack/plugin-security`, full suite: 183 files, 3865 passed, 45 skipped. - `@objectstack/organizations`, full suite: 11 files, 151 passed. - `@objectstack/dogfood`, the 32 files that write `sys_user_position` / `sys_user_permission_set` rows, against a dist rebuilt at `edafba04e9`: 32 files, 279 passed, 1 skipped. - `pnpm --filter @objectstack/plugin-security typecheck`: exit 0, test layer included (`check:test-typecheck`). - Non-system callers that empty an organization on update: none found in `packages/**` source. Every non-test null `organization_id` literal there is a read predicate or a system-context insert. ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths) derived 65 commands at `edafba04e9`. All 65 ran on that tree with their exit codes recorded, and `--ran` answered: "65 derived famil(ies) accounted for — 65 run, 0 NOT-MEASURED". `check:dual-build-cjs-loads` first answered PREREQUISITE NOT MET (exit 3: eight packages had no dist); after building those it answered exit 0, and that is the recorded run. Selected verdict lines: - `check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition.` - `This diff introduces no major bump.` - `check-nul-bytes: OK (scanned 10272 text file(s) ... no raw ASCII control bytes).` - `check-type-check-coverage: OK`, `check-test-source-alias OK`, `check:cross-package-test-inputs OK`. Lint (the repo-wide run is CI's): eslint on the two touched `.ts` files, `--no-inline-config --format json`, counted 2 files, 0 errors, 0 warnings. The repo's one `eslint.config.mjs` enables no type-aware linting (its own comment says so: no `parserOptions.project`, no typed rules), so this diff cannot move a verdict on any untouched file. ## Acceptance notes - `main` moved three commits after this branch merged `fe72ad7828` (a spec type-only change, `cli`, `platform-objects`). None touches `plugin-security`, `objectql`, `core` or `organizations`, so the branch was not merged again; CI and the queue validate the merge. - The Setup UI (`objectui`) is not checked out in this container, so whether a form there can send an emptied organization on these tables is NOT MEASURED. If one can, it now receives the 403 above. - Changeset: one `@objectstack/plugin-security` `minor`, with the breaking banner and the ADR-0087 disposition `not-required (no-migration-prescription)`. --- _Generated by [Claude Code](https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 59d993c commit f2626c7

3 files changed

Lines changed: 472 additions & 6 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
'@objectstack/plugin-security': minor
3+
---
4+
5+
fix(plugin-security)!: the Layer 0 tenant write wall refuses a non-system update that would leave a row with no organization
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export, type or stored shape is added, removed, renamed or re-shaped, and no stored row is read, rewritten, converted or dropped, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is a runtime write door: where the Layer 0 tenant wall applies, a non-system update that would store a row with no organization is refused with the existing 403 PERMISSION_DENIED envelope. The other categories are closed on facts: the one bumped package publishes (not unpublished); no ADR-0087 id covers this path and this diff adds none (not registered / already-registered); and nothing exported is removed or narrowed, so it is neither runtime-interface-only nor type-surface-only. -->
10+
11+
**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
12+
13+
**What stops being accepted.** Under a walled tenancy posture (`isolated` or `group`), on an object the Layer 0 tenant wall covers, a non-system update that would leave the row with no organization is refused with `403 PERMISSION_DENIED`. That holds for the value the caller sends and for the value the stored row ends up with after the `beforeUpdate` hooks, on the by-id and the predicate update paths alike, and for every non-system caller the wall covers, platform administrators on public tenant objects included. The rule the wall already stated for an update, that a row's organization stays one the caller's organization scope admits, now covers "no organization" as well as "another organization". An organization-less row is not inert: on an ordinary tenant object it leaves every reader's scope, and on the organization-scoped grant tables (`sys_user_position`, `sys_user_permission_set`) it is read as a grant that applies in every organization.
14+
15+
**What stays accepted.**
16+
17+
- An update that does not touch `organization_id`, and one that keeps the row's own organization.
18+
- System-context writes (seed replay, imports, migrations, platform bootstraps).
19+
- A platform administrator on an object whose posture permits crossing the wall, as before.
20+
- The `single` posture, where no tenant wall applies.
21+
- Inserts: an absent or empty organization on an insert is still filled with the caller's active organization.
22+
23+
**What changes for you.** A script or flow that cleared a row's organization through a user or service-principal context now receives the `403` above. Keep the row in its organization; if a row genuinely must carry no organization (a global grant, for example), write it from a system context.

‎packages/plugins/plugin-security/src/security-plugin.ts‎

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3960,12 +3960,29 @@ export class SecurityPlugin implements Plugin {
39603960
// cross-tenant `organization_id` change-set is caught too (the check inspects
39613961
// the change-set value here, and [#20013] every stored row in the engine).
39623962
//
3963-
// Scope: only a SUPPLIED `organization_id` is validated — an ABSENT value on
3964-
// insert is the organizations-plugin auto-stamp's responsibility, and an
3965-
// update that doesn't touch `organization_id` carries no value here, so
3966-
// ordinary writes are unaffected. [#20013] "Supplied" includes a value the
3967-
// engine's `beforeInsert` / `beforeUpdate` chain wrote: that half is judged
3968-
// on the stored row, below. A pure plugin-security deployment has no
3963+
// [#22278] The invariant, stated whole: wherever the wall applies to a
3964+
// non-system UPDATE, the row's organization after the write is one the
3965+
// wall admits. So an update that EMPTIES the column (`null` or `''`) is
3966+
// refused as well — a re-point to NO tenant, not only to another one.
3967+
// Judging only a value that NAMES an organization left that move
3968+
// unjudged on both halves below, and an organization-less row is not
3969+
// inert: on an ordinary tenant object it drops out of every reader's
3970+
// scope, and on the organization-scoped grant tables
3971+
// (`sys_user_position`, `sys_user_permission_set`) `@objectstack/core`'s
3972+
// `grantAppliesInTenant` reads it as a grant that applies in EVERY
3973+
// organization. The stored organization is judged on every non-system
3974+
// update path — the payload as sent here, and the row the engine is
3975+
// about to store (by id and by predicate) below.
3976+
//
3977+
// Scope: on INSERT only a SUPPLIED `organization_id` is validated — an
3978+
// ABSENT or empty value is the organizations-plugin auto-stamp's
3979+
// responsibility, and INSERT keeps that rule. On UPDATE nothing stamps an
3980+
// emptied value back, so there the column is judged whenever the write
3981+
// sets it, empty included; an update that doesn't touch `organization_id`
3982+
// carries no value here, so ordinary writes are unaffected. [#20013]
3983+
// "Supplied" includes a value the engine's `beforeInsert` /
3984+
// `beforeUpdate` chain wrote: that half is judged on the stored row,
3985+
// below. A pure plugin-security deployment has no
39693986
// isolation active (Layer 0 → null), so this is ordering-independent w.r.t.
39703987
// the auto-stamp middleware. System / boot writes carry `isSystem` and
39713988
// short-circuited the whole middleware above, so legitimate cross-org moves
@@ -4079,6 +4096,15 @@ export class SecurityPlugin implements Plugin {
40794096
const namesOrganization = (r: Record<string, unknown>): boolean =>
40804097
r.organization_id != null && r.organization_id !== '';
40814098
const suppliedRows = writeRows.filter(namesOrganization);
4099+
// [#22278] …except on UPDATE, where no stamp runs: a row that SETS the
4100+
// column to `null` or `''` empties the organization of a row that has
4101+
// one, and that is refused wherever the wall applies. Own-property, so
4102+
// an update that never names the column is untouched; an `undefined`
4103+
// value is a column the statement leaves alone, not an emptied one.
4104+
const emptiesOrganization = (r: Record<string, unknown>): boolean =>
4105+
opCtx.operation === 'update' &&
4106+
Object.prototype.hasOwnProperty.call(r, 'organization_id') &&
4107+
(r.organization_id === null || r.organization_id === '');
40824108
// The wall is computed when a payload names an organization (as
40834109
// before) or when a walled posture may have a hook name one ([#20013]
40844110
// below). The `single` posture composes no Layer 0 at all, so it pays
@@ -4114,9 +4140,29 @@ export class SecurityPlugin implements Plugin {
41144140
{ operation: opCtx.operation, object: opCtx.object, positions, permissionSets: explicitPermissionSets },
41154141
);
41164142
};
4143+
// [#22278] The refusal of an emptied organization, shared by both
4144+
// halves the same way. Its own sentence: the row is not placed in
4145+
// another tenant, it is left in none.
4146+
const denyOrganizationRemoval = (source: string): never => {
4147+
this.logger.warn?.(
4148+
`[Security] Layer 0 tenant CHECK FAILED on ${opCtx.operation} '${opCtx.object}' — write denied ` +
4149+
`(fail-closed); ${source} organization_id is empty`,
4150+
);
4151+
throw new PermissionDeniedError(
4152+
`[Security] Access denied: the ${opCtx.operation} would leave '${opCtx.object}' with no organization ` +
4153+
`(an organization-scoped row keeps an organization on update)`,
4154+
{ operation: opCtx.operation, object: opCtx.object, positions, permissionSets: explicitPermissionSets },
4155+
`[ADR-0095 D1] tenancy posture '${this.tenancyPosture}' walls '${opCtx.object}', so a non-system update ` +
4156+
`must leave the row in an organization the Layer 0 wall admits. An empty organization_id is in none: ` +
4157+
`the row would drop out of every reader's scope, and an organization-scoped grant row would apply in ` +
4158+
`every organization. System contexts and platform operators on posture-permitting objects are unaffected.`,
4159+
);
4160+
};
41174161
// EVERY supplied row must clear the wall — one forged row in a bulk
41184162
// payload denies the whole write (fail closed, no partial landing).
41194163
if (tenantParts.length > 0 && !suppliedRows.every(clearsWall)) denyTenantPlacement('a supplied');
4164+
// [#22278] …and no row of an UPDATE payload may empty the column.
4165+
if (tenantParts.length > 0 && writeRows.some(emptiesOrganization)) denyOrganizationRemoval('a supplied');
41204166

41214167
// ── [#20013] The wall on the STORED row, after the hooks ─────────────
41224168
//
@@ -4158,6 +4204,13 @@ export class SecurityPlugin implements Plugin {
41584204
//
41594205
// Array UPDATE payloads are left as step 3.6 leaves them: the engine
41604206
// takes one payload per update, so the seam judges a single payload.
4207+
//
4208+
// [#22278] On an UPDATE the stored row is held to the whole invariant:
4209+
// an image whose organization a hook (or the payload) EMPTIED is
4210+
// refused, not skipped as "names no organization". An update that
4211+
// leaves the column alone hands over the row's own organization — the
4212+
// one the pre-image or the composed AST selected it under — so it
4213+
// clears this exactly as before.
41614214
if (tenantParts.length > 0 && (opCtx.operation === 'insert' || !Array.isArray(opCtx.data))) {
41624215
const businessCheck = writeImageCheckSeam;
41634216
writeImageCheckSeam = {
@@ -4166,6 +4219,7 @@ export class SecurityPlugin implements Plugin {
41664219
for (const row of rows) {
41674220
if (!row || typeof row !== 'object') continue;
41684221
if (namesOrganization(row) && !clearsWall(row)) denyTenantPlacement("the stored row's");
4222+
if (emptiesOrganization(row)) denyOrganizationRemoval("the stored row's");
41694223
}
41704224
},
41714225
};

0 commit comments

Comments
 (0)