Skip to content

Commit 251a7dd

Browse files
fix(organizations)!: a create naming an organization_id meets the Layer 0 write wall, as the update does (#21666) (#21680)
Fixes #21666 Clause-②: no (narrowing) ## What changes On a walled posture, the insert stamp in `@objectstack/organizations` (Middleware A) used to **overwrite** a supplied `organization_id` with the caller's active organization in every user context. The overwrite is `packages/plugins/organizations/src/organizations-plugin.ts:334` at `72f3c74d60`: `data.organization_id = opCtx.context.tenantId;` inside `if (isUserContext)`. The stamp now **fills only an absent or empty value**, for every non-system context (ADR-0105 D5). A supplied value is left as sent and meets the Layer 0 write wall in `plugin-security` (step 3.7, ADR-0095 D1). That is the wall the PATCH already meets, so the create now gets the PATCH's answer. This follows triage ruling 5975961814: "the governed, loud side wins" and "⛔ No silent replacement on any posture." No `plugin-security` code changes. The wall was already symmetric (see Zone 2.2 below). No `packages/spec`, `packages/objectql` or `packages/metadata-protocol` edit. ## Measured on a real walled boot (before → after) Harness: `@objectstack/verify` `bootStack(app, { multiTenant: true, hostRoot })`, run from a scratch host app that declares the real `@objectstack/organizations`. The stack is the real `SecurityPlugin`, the real REST routes and `sqlite-wasm`. Requests go over HTTP to `/api/v1/data/...`. Orgs: **A** is the caller's active organization, **B** is another tenant ("Tenant North D2"), and **C** is a sister organization the caller also holds. Objects: `sys_user_permission_set` and `sys_business_unit` (platform objects that declare their own `organization_id`), `qa_ledger` (a public app object with the injected `organization_id`) and `qa_vault` (a private app object, where a platform admin is posture-exempt). ### `isolated` | caller | object | create naming B | PATCH to B | `createMany` [B] | |---|---|---|---|---| | platform admin | `sys_user_permission_set` | 201, stored A → **403 PERMISSION_DENIED** | 403 PERMISSION_DENIED (both) | 403 (both) | | platform admin | `sys_business_unit` | 201, stored A → **403 PERMISSION_DENIED** | 403 (both) | 403 (both) | | platform admin | `qa_ledger` | 201, stored A → **403 PERMISSION_DENIED** | 403 (both) | 403 (both) | | platform admin | `qa_vault` (exempt) | 201, stored A + `droppedFields` readonly (unchanged) | 200, stored A + `droppedFields` (both) | 201, stored A + `droppedFields` (both) | | member | `qa_ledger` | 201, stored A → **403 PERMISSION_DENIED** | 403 (both) | 403 (both) | | member | `qa_vault` | 201, stored A → **403 PERMISSION_DENIED** | 403 (both) | 403 (both) | A create naming no organization, or naming A, answers 201 and is stored in A, before and after, in every row above. ### `group` | caller | object | create naming B | create naming C | PATCH to C | |---|---|---|---|---| | platform admin | `sys_user_permission_set` | 201 A → **403** | 201 A → **201 C** | 200 C (both) | | platform admin | `sys_business_unit` | 201 A → **403** | 201 A → **201 C** | 200 C (both) | | platform admin | `qa_ledger` | 201 A → **403** | 201 A + dropped (unchanged) | 200 A + dropped (both) | | platform admin | `qa_vault` | 201 A + dropped (unchanged, exempt) | 201 A + dropped (unchanged) | 200 A + dropped (both) | | member | `qa_ledger` | 201 A → **403** | 201 A + dropped (unchanged) | 200 A + dropped (both) | | member | `qa_vault` | 201 A → **403** | 201 A + dropped (unchanged) | 200 A + dropped (both) | PATCH to B and `createMany` [B] were 403 in every row, before and after. ### `single` (the control) `objectstack serve` mounts the runtime only under a walled posture, so the production `single` shape has no Middleware A. Every cell there is byte-identical before and after. For example, `sys_user_permission_set` create B answers 201 stored B, PATCH B answers 200 stored B, and `createMany` [B] answers 201 stored B. As an extra non-production cell, I also mounted the runtime under `single` by hand. The create naming B moved from 201 stored A to 201 stored B, which now matches that boot's PATCH and `createMany`. ### Other single-row doors (same middleware) - **Import** (`POST /data/:object/import`, `isolated`; rows [B, none]). The batch is refused by the wall and degrades to per-row `createData`. Before, both rows reported `ok` and were stored in A. After, the B row reports `PERMISSION_DENIED` and the none row reports `ok` in A. Measured as admin on `sys_business_unit`, as admin on `qa_ledger`, and as member on `qa_ledger`. - **Clone** (`POST /data/:object/:id/clone`, `group`, source row in C). For `sys_business_unit` it was 201 A and is now 201 C. For `sys_user_permission_set` it was 201 A and is now **409**: the copy would duplicate its source's `(user, set, organization)` key in C. For `qa_ledger`, the clone strips the injected column, so it is 201 A both before and after. - The `create` operation of `POST /batch` writes row by row through the same path. I read this in code; I did not measure it. ### Zone 2.2: insert vs update on the wall There is no asymmetry. Both verbs reach `computeWriteTenantCheckFilter` → `computeLayeredRlsFilter`, and they share the platform-admin exemption (only on posture-permitting objects: `private`, platform-global, better-auth-managed). They throw the same `PermissionDeniedError`, `code: PERMISSION_DENIED` with status 403. The message names the verb: "the insert would place …" and "the update would place …". So `security-plugin.ts` is not edited. Its step 3.7 comment already said the stamp "only fills a MISSING value, never overwrites a supplied one", and that sentence is now true. ## Census: who relied on the overwrite (triage's stop condition) | writer | context | sets `organization_id` itself? | reliant? | |---|---|---|---| | per-org seed replay (`seed-loader` `SEED_OPTIONS`) | system | yes | no: skips Middleware A | | default-org bootstrap (`ensureDefaultOrganization`, `claimOrgSeedOwnership`) | system | yes | no | | orphan claim (`claimOrphanOrgRows`) | system, update | yes | no: insert-only middleware | | sharing, approvals, audit, auto-org-admin grant, invitation placement, email, settings audit, better-auth adapter | system | yes | no | | storage `metadata-store` (`sys_file`, `sys_upload_session`) | caller | `= context.tenantId` | no: always equal | | messaging, outboxes, `sys-metadata-repository`, `database-loader` | no `tenantId` on the context | yes | no: the middleware no-ops | | REST import runner (`core/import-runner`) | caller | from the user's file | user input, not a platform writer; see Import above | | REST clone (`metadata-protocol` `cloneData`) | caller | copied from the source when the object declares the column | see Acceptance notes | | flow `create_record` (runAs user) | caller | flow-authored fields | user-authored input, same as REST | No non-system writer sets an `organization_id` and relies on the overwrite to correct it, so this is not a stop. `seed-loader.ts` (claimed by #21665) was read only. ## Pins (real runtime: this package's Middleware A + real `SecurityPlugin` + `ObjectQL` + `SqliteWasmDriver`) New file `packages/plugins/organizations/src/create-explicit-organization-wall.test.ts`, 14 cases: - **Another tenant's organization.** A create naming it is refused with the PATCH's code and status. Covered for a member and for a platform admin, on a declared-column object and on an injected-column object. - **Array insert.** An array insert naming it gets the single-row answer. - **No organization.** A create naming none is stamped with the active organization **before the hooks run** (asserted at the `beforeInsert` payload) and stored there. - **Own active organization.** A create naming it is admitted. - **#2937.** A member's forged `organization_id` is refused, and no row lands in either tenant. - **System context.** An explicit cross-organization value is kept (the seed-replay path). - **`group`.** A sister organization is admitted on create, as on the PATCH. An organization outside the membership set is refused with the PATCH's code. `organizations-plugin.test.ts`: the old "OVERWRITES a forged organization_id" unit is now "leaves a supplied organization_id untouched". I added an empty-string fill unit. ## Ablations (predicted direction stated before each run; both through `scripts/ablation-replace.mjs`, restore proven by blob == HEAD and `git diff HEAD` empty) 1. **Restore the overwrite.** I predicted red on exactly the 9 wall-file cells where a create names an organization and expects a refusal or a non-active placement (6 refusals, 2 array/single parity cells, the group sister cell), plus the one unit "leaves … untouched". Observed: **10 failed, 113 passed (123)**, exactly those cells. The stamped, own-organization and system cells stayed green. 2. **Drop the fill for an absent value.** I predicted red on exactly the 2 "stamped before the hooks run" cells and the 2 fill units (absent, empty). Observed: **4 failed, 119 passed (123)**, exactly those. The failure reads `the beforeInsert chain sees the stamp: expected [ undefined ] to deeply equal [ 'org_alpha' ]`. The stored-row half alone could not catch this ablation. The SQL driver fills the same value from `DriverOptions.tenantId`, measured: `createMany` [none], which Middleware A never touches, lands in A. That is why the pin asserts the payload the hooks see. ## Verification (all at `904a8e25c4`) - ① `pnpm --workspace-concurrency=2 --filter '@objectstack/organizations^...' build`: exit 0, 29 projects. - ② `pnpm --filter @objectstack/organizations test`: 9 files, **123 passed**. `typecheck`: exit 0 (tsc and the test layer, 0 errors). `pnpm --filter @objectstack/plugin-security test`: 164 files, **3527 passed**, 45 skipped. - ③ `dispatch-gates --commands` (no paths) derived **105** families. **104 exit 0.** **NOT MEASURED: `check:dual-build-cjs-loads`**, which exits 3 (PREREQUISITE NOT MET: 32 packages have no `dist/`, and it needs a full build; CI owns it). `check:skill-examples` first exited 3 for lack of a client build. I built `@objectstack/client` and `client-react` and it then exited 0. `--ran` reconciliation: 105 derived, 104 run, 1 NOT MEASURED (derived from the recorded exit 3), 0 unrun. - ESLint, narrowed to the 4 touched lintable files (`--no-inline-config --format json`): 4 files, 0 errors, 0 warnings. All 4 are inside the population of the `files` globs in `eslint.config.mjs` (`--print-config` resolves for each). The other touched files (`.md`, `.json`, `.yaml`) match no lint glob. The config enables no type-aware linting (no `parserOptions.project`), so this diff cannot move a verdict on an untouched file. - Dogfood walled-posture files: `rls-multitenant` skips by design (`@objectstack/dogfood` does not declare the runtime), and `enterprise-organizations.test.ts` passes 13. **NOT MEASURED: `attachments-permission-matrix`**: `@objectstack/service-storage` is unbuilt, and its multi-org block is `skipIf` there anyway. The walled HTTP measurement above is this card's dogfood. - The derivation flagged the tree as behind `origin/main` by 3 commits (#21664, #21674, #21677). None touches `organizations`, `plugin-security`, `objectql` or the files here. The only gate file among them is `scripts/cross-package-test-inputs.mjs`. ## Docs and skills - `content/docs/permissions/system-context.mdx` row 61 said "a forged `organization_id` is overwritten on the non-elevated path". This PR made that false, and the row now says the wall refuses it, as it refuses the update. - `content/docs/deployment/tenancy-modes.mdx` ("Filling in an absent `organization_id` … validating a supplied one") was false before and is true now, so it is untouched. - `skills/**`: no sentence about the insert stamp or about `organization_id` on create, so nothing is false there. ## Package and lockfile - `@objectstack/organizations` gains three devDependencies: `objectql`, `plugin-security`, `driver-sqlite-wasm`. Each is aliased to source in `vitest.config.ts`, as `check:test-source-alias` requires. - `pnpm-lock.yaml` carries only the organizations importer hunk. `pnpm install` also flipped an unrelated `esbuild` peer suffix in two other importers, and that churn was dropped. `pnpm install --frozen-lockfile` passes. ## Changeset `.changeset/21666-create-explicit-organization-meets-wall.md`: `@objectstack/organizations` `minor`, `fix(organizations)!`, a `**BREAKING.**` marker, and `Clause-②: no (narrowing)`. The accept set narrows: a create, or an import row, naming another tenant's organization answered 201 and is now refused. The ADR-0087 disposition is `not-required (no-migration-prescription)`, and `check:adr-0087-registration` is green on it. The one-line fix: omit `organization_id` on create or name your active organization, and a platform operator moves a row with a system-context write. `@objectstack/plugin-security` is unchanged, so it has no entry. ## Acceptance notes - **Out-of-scope finding (class a), not fixed here.** On a walled posture, a create that sends **no** `organization_id` to an app object answers 201 with `droppedFields: [{ fields: ['organization_id'], reason: 'readonly' }]`, naming a field the caller never sent. Middleware A's fill lands in the payload before `ObjectQL.insert` snapshots "what the caller sent", so the static-readonly strip reports the platform's own stamp as a caller write. Controls: `single` without the runtime reports nothing, and `createMany` [none] on `isolated` reports nothing. The seam is in `packages/objectql`, outside this card's surface, and this PR leaves it unchanged. It goes to the seat to file. - **Clone under `group`.** The clone door copies an `organization_id` that the object declares itself. A clone of a sister-organization row therefore now lands beside its source (or answers 409 on a unique key) instead of being re-homed into the active organization. The wall admits it, and so does a PATCH. Whether the clone door should strip a declared `organization_id` is a `metadata-protocol` question for the seat. This card neither answers nor edits it. - **Observation.** During the scratch import, `driver-sql` logged `DATABASE_ERROR … no such table: _objectstack_sequences`. The import still completed, the log is unrelated to this diff, and I have not filed it. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a4fd82a commit 251a7dd

8 files changed

Lines changed: 445 additions & 30 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
"@objectstack/organizations": minor
3+
---
4+
5+
fix(organizations)!: a create that names an `organization_id` meets the Layer 0 write wall, as the update does — the insert stamp no longer rewrites it (#21666)
6+
7+
Clause-②: no (narrowing)
8+
9+
**BREAKING.** On a walled posture (`isolated` / `group`), the insert stamp (Middleware A) overwrote a supplied `organization_id` with the caller's active organization in every user context. A create naming another tenant's organization answered `201` and stored the row in the caller's own organization, while the PATCH naming the same organization and the array insert (`createMany`) were refused `403 PERMISSION_DENIED`. One operation answered two ways, and the caller of the `201` had no signal that its input had been replaced.
10+
11+
The stamp now fills only an absent or empty `organization_id`, for every non-system context (ADR-0105 D5). A supplied value is left as sent and meets the Layer 0 write wall in `@objectstack/plugin-security` (ADR-0095 D1), which answers the create exactly as it answers the update:
12+
13+
- **Another tenant's organization** → `403 PERMISSION_DENIED`, nothing stored (was `201`, stored in the active organization). This holds for a member and for a platform administrator on a tenant object. A member's forged `organization_id` stays refused; the wall refuses it now, where the stamp used to rewrite it.
14+
- **No organization** → stamped with the active organization, as before.
15+
- **The caller's own active organization** → admitted, as before.
16+
- **Under `group`, a sister organization the caller holds** → admitted and stored in that organization, the same place the PATCH already moves a row to (was `201`, stored in the active organization). Where `organization_id` is the platform-injected column, the engine still strips it from a non-system payload as `readonly` and reports it in `droppedFields`, on the create as on the update.
17+
- **A platform administrator on a posture-permitting object** (`private`, platform-global, better-auth-managed) is exempt from the wall on the create as on the update.
18+
19+
Every door that writes one row at a time under the caller's context gives the same answer: `POST /data/:object`, the `create` operation of `POST /batch`, the clone route and the import runner's per-row fallback. Two of these change in ways worth knowing:
20+
21+
- An import row naming another tenant's organization is now reported as a failed row (`PERMISSION_DENIED`). Before, it was created in the active organization.
22+
- The clone route copies an `organization_id` that the object declares itself. So under `group`, a clone of a sister-organization row now lands beside its source instead of in the active organization.
23+
24+
System contexts are unchanged. The per-organization seed replay, the orphan claim, migrations and every other `isSystem` writer meet neither the stamp nor the wall. The `single` posture is unchanged too, because `objectstack serve` mounts this package only under a walled posture.
25+
26+
**What to do.** On create, either omit `organization_id` or name your active organization. If a platform operator needs a row in another organization, write it with a system-context write.
27+
28+
<!-- adr-0087: not-required (no-migration-prescription) a runtime write verdict: the insert stamp no longer rewrites a supplied organization_id, so the Layer 0 write wall judges it as it already judged the update and the array insert. No authorable key, spelling, export or stored shape moves, no stored row is read or rewritten, and which organization a caller meant to name is not something a ledger entry can rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this behaviour (not already-registered); and the change is a middleware verdict, not a TypeScript declaration (not runtime-interface-only or type-surface-only). -->

‎content/docs/permissions/system-context.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ The largest single consumer — **17 of the 114 sites**.
182182
| 58 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `packages/plugins/plugin-email/src/email-template-provenance.ts#bindEmailTemplateProvenanceStamp`, `packages/plugins/plugin-webhooks/src/webhook-provenance.ts#bindWebhookProvenanceStamp` |
183183
| 59 | **Automation flow data nodes re-add the `owner_id` stamp** (the one place row 2's gap is compensated inline) | service-automation | Get: a flow-authored INSERT under system elevation still lands owned, when the run resolved a user. Fill-only — flow-authored values win | `packages/services/service-automation/src/runtime-identity.ts#stampSystemInsertOwner`, called from `packages/services/service-automation/src/builtin/crud-nodes.ts#registerCrudNodes` |
184184
| 60 | Inbox caller refusal names `isSystem` as what was carried | service-messaging | Get: nothing — the refusal still fires. The flag only shapes the diagnostic, because privilege is not an authorization subject | `packages/services/service-messaging/src/inbox-caller.ts#resolveInboxRecipient` |
185-
| 61 | **`organization_id` is not auto-stamped on INSERT** — the organization-axis twin of the `owner_id` gap above | organizations | Get: an elevated write may name another organization deliberately, which is what the per-organization seed replay, the orphan-row claim, imports and migrations all rely on. Lose: the authoritative stamp, so an elevated insert that names no organization lands `organization_id = NULL` and the wall hides it. ⛔ This is why a forged `organization_id` is overwritten on the non-elevated path and not here: elevation is the seam the legitimate cross-organization writers use | `packages/plugins/organizations/src/organizations-plugin.ts#start` |
185+
| 61 | **`organization_id` is not auto-stamped on INSERT** — the organization-axis twin of the `owner_id` gap above | organizations | Get: an elevated write may name another organization deliberately, which is what the per-organization seed replay, the orphan-row claim, imports and migrations all rely on. Lose: the fill-only stamp, so an elevated insert that names no organization lands `organization_id = NULL` and the wall hides it. ⛔ Neither path rewrites a supplied `organization_id`: the non-elevated path fills only an absent one, and a supplied value meets the Layer 0 write wall, which refuses a forged one `403 PERMISSION_DENIED` exactly as it refuses an update re-pointing a row. Elevation skips the stamp and the wall alike, which is why it is the seam the legitimate cross-organization writers use | `packages/plugins/organizations/src/organizations-plugin.ts#start` |
186186

187187
### 6. Reads that only carry the flag onward
188188

‎packages/plugins/organizations/package.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@
2626
"@objectstack/types": "workspace:*"
2727
},
2828
"devDependencies": {
29+
"@objectstack/driver-sqlite-wasm": "workspace:*",
2930
"@objectstack/metadata-core": "workspace:*",
31+
"@objectstack/objectql": "workspace:*",
32+
"@objectstack/plugin-security": "workspace:*",
3033
"@objectstack/rest": "workspace:*",
3134
"@types/node": "^26.6.3",
3235
"tsx": "^4.23.15",

0 commit comments

Comments
 (0)