diff --git a/.changeset/21666-create-explicit-organization-meets-wall.md b/.changeset/21666-create-explicit-organization-meets-wall.md new file mode 100644 index 00000000000..4c294624385 --- /dev/null +++ b/.changeset/21666-create-explicit-organization-meets-wall.md @@ -0,0 +1,28 @@ +--- +"@objectstack/organizations": minor +--- + +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) + +Clause-②: no (narrowing) + +**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. + +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: + +- **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. +- **No organization** → stamped with the active organization, as before. +- **The caller's own active organization** → admitted, as before. +- **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. +- **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. + +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: + +- 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. +- 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. + +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. + +**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. + + diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b8083cac832..9cbf07f77d2 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -182,7 +182,7 @@ The largest single consumer — **17 of the 114 sites**. | 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` | | 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` | | 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` | -| 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` | +| 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` | ### 6. Reads that only carry the flag onward diff --git a/packages/plugins/organizations/package.json b/packages/plugins/organizations/package.json index 90d2b75d498..4bfa1fde2c2 100644 --- a/packages/plugins/organizations/package.json +++ b/packages/plugins/organizations/package.json @@ -26,7 +26,10 @@ "@objectstack/types": "workspace:*" }, "devDependencies": { + "@objectstack/driver-sqlite-wasm": "workspace:*", "@objectstack/metadata-core": "workspace:*", + "@objectstack/objectql": "workspace:*", + "@objectstack/plugin-security": "workspace:*", "@objectstack/rest": "workspace:*", "@types/node": "^26.6.3", "tsx": "^4.23.15", diff --git a/packages/plugins/organizations/src/create-explicit-organization-wall.test.ts b/packages/plugins/organizations/src/create-explicit-organization-wall.test.ts new file mode 100644 index 00000000000..03888998bdd --- /dev/null +++ b/packages/plugins/organizations/src/create-explicit-organization-wall.test.ts @@ -0,0 +1,328 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21666] A create that NAMES an organization meets the Layer 0 write wall — + * the same wall, with the same answer, as the update that re-points one. + * + * ## The defect this pins shut + * + * Middleware A (this package's insert stamp) used to OVERWRITE a supplied + * `organization_id` with the caller's active organization in every user + * context. Measured over `POST /api/v1/data/:object` on a walled boot with + * this package mounted: a create naming another tenant's organization answered + * 201 and stored the row in the caller's own organization, while the PATCH + * that names the same organization and the array insert (which the stamp never + * touched) were both refused `403 PERMISSION_DENIED`. One operation, two + * answers, and the caller of the 201 had no way to tell its input had been + * replaced. + * + * The stamp now FILLS an absent value only (ADR-0105 D5). A supplied value + * goes on to `@objectstack/plugin-security`'s Layer 0 write wall (step 3.7, + * ADR-0095 D1), which is what the PATCH meets. + * + * ## What is real here + * + * The engine (`ObjectQL`), the driver (`SqliteWasmDriver`, the one `objectstack + * dev` uses), THIS package's `OrganizationsPlugin` — registered as the + * `org-scoping` service and installing its middleware first, the order + * `objectstack serve` mounts it in — and the real `SecurityPlugin`. Nothing + * stands in for either middleware. The fixture is the plugin context: a + * service map carrying the engine, a metadata reader over it, the two + * permission sets below, and the `tenancy` service's resolved posture. + * `ensureDefaultOrganization` is switched off because it bootstraps + * `sys_*` rows this engine does not register; it installs nothing the insert + * path reads. + * + * ## The two object shapes + * + * - `qa_assignment` DECLARES its own `organization_id`, the shape of + * `sys_user_permission_set`, where the defect was measured: nothing but the + * wall stands between a supplied value and the stored row. + * - `qa_ledger` carries the platform-injected `organization_id`, the shape of + * every app object. The engine strips that column from a non-system payload + * as `readonly` AFTER the wall has judged it, so here an admitted value is + * re-derived, and a refused one never reaches the strip. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { SecurityPlugin } from '@objectstack/plugin-security'; +import type { PermissionSet } from '@objectstack/spec/security'; +import { OrganizationsPlugin } from './organizations-plugin.js'; + +/** The caller's active organization. */ +const OWN_ORG = 'org_alpha'; +/** Another tenant — the caller holds no membership in it. */ +const FOREIGN_ORG = 'org_north'; +/** A sister organization the caller ALSO holds under `group` only. */ +const SISTER_ORG = 'org_south'; + +const DECLARED = 'qa_assignment'; +const INJECTED = 'qa_ledger'; + +const OBJECTS = [ + { + name: DECLARED, + label: 'Assignment', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + organization_id: { name: 'organization_id', type: 'text' }, + }, + }, + { + name: INJECTED, + label: 'Ledger', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + }, + }, +]; + +/** Plain CRUD and no row-level policy — the wall is the only thing judging the organization. */ +const MEMBER: PermissionSet = { + name: 'member_default', + label: 'Member', + objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, +} as unknown as PermissionSet; + +/** A platform operator: the superuser bit AND a platform-exclusive capability (ADR-0095 D3). */ +const PLATFORM_ADMIN: PermissionSet = { + name: 'admin_full_access', + label: 'Platform Administrator', + objects: { + '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true }, + }, + systemPermissions: ['manage_platform_settings', 'manage_metadata'], +} as unknown as PermissionSet; + +const SYS_CTX = { isSystem: true }; +const MEMBER_CTX = { userId: 'usr_member', tenantId: OWN_ORG, positions: [], permissions: [], posture: 'MEMBER' }; +const ADMIN_CTX = { + userId: 'usr_admin', + tenantId: OWN_ORG, + positions: [], + permissions: ['admin_full_access'], + posture: 'PLATFORM_ADMIN', +}; + +type Posture = 'isolated' | 'group'; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +interface Booted { + engine: ObjectQL; + /** `organization_id` as the engine handed it to the `beforeInsert` chain, per insert. */ + seenByHooks: unknown[]; + /** A table's rows read straight off the driver, past every scope. */ + table: (name: string) => Promise>>; +} + +async function boot(posture: Posture = 'isolated'): Promise { + const engine = new ObjectQL(); + engine.registerDriver(new SqliteWasmDriver({ filename: ':memory:' } as never) as never, true); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.create-explicit-organization-wall', + name: 'A create naming an organization meets the Layer 0 write wall', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: OBJECTS, + } as never); + await engine.syncSchemas(); + engines.push(engine); + + const seenByHooks: unknown[] = []; + for (const object of [DECLARED, INJECTED]) { + engine.on('beforeInsert', object, (async (ctx: { input: { data: Record } }) => { + seenByHooks.push(ctx.input.data.organization_id); + }) as never); + } + + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [MEMBER, PLATFORM_ADMIN], + }, + tenancy: { posture }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: (name: string, service: unknown) => { + services[name] = service; + }, + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + // Lifecycle hooks are recorded and never fired: the membership-policy + // gate waits on `kernel:bootstrapped`, which belongs to a kernel boot, + // not to the insert path under test. + hook: vi.fn(), + }; + const organizations = new OrganizationsPlugin({ ensureDefaultOrganization: false }); + const security = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + // Kernel order: every init, then every start — organizations first, as `serve` mounts it. + await organizations.init(ctx as never); + await security.init(ctx as never); + await organizations.start(ctx as never); + await security.start(ctx as never); + expect(services['org-scoping'], 'the real runtime is the org-scoping service').toBe(organizations); + // The expected refusals log at WARN through the engine's own logger. + vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined); + + for (const object of [DECLARED, INJECTED]) { + await engine.insert(object, { id: 'r1', name: 'seeded', organization_id: OWN_ORG }, { context: SYS_CTX } as never); + } + seenByHooks.length = 0; + + const table = async (name: string) => { + const driver = (engine as unknown as { getDriver(o: string): { knex: unknown } }).getDriver(name); + const knex = driver.knex as (t: string) => { select: (...c: string[]) => Promise>> }; + const rows = await knex(name).select('id', 'organization_id'); + return [...rows].sort((a, b) => String(a.id).localeCompare(String(b.id))); + }; + + return { engine, seenByHooks, table }; +} + +interface Outcome { ok: boolean; code?: string; status?: number; message?: string } + +const attempt = async (run: () => Promise): Promise => { + try { + await run(); + return { ok: true }; + } catch (e) { + const err = e as { code?: string; statusCode?: number; status?: number; message?: string }; + return { ok: false, code: err.code, status: err.statusCode ?? err.status, message: String(err.message ?? e) }; + } +}; + +const create = (b: Booted, object: string, data: unknown, context: object) => + b.engine.insert(object, data as never, { context } as never); +const repoint = (b: Booted, object: string, organization: string, context: object) => + b.engine.update(object, { id: 'r1', organization_id: organization } as never, { context } as never); + +/** The wall's refusal on the ADR-0112 envelope: code and status, plus the verb it names. */ +const expectWallRefusal = (outcome: Outcome, verb: 'insert' | 'update', object: string) => { + expect(outcome.ok, 'expected a refusal, got a completed write').toBe(false); + expect(outcome.code, 'ADR-0112 error code').toBe('PERMISSION_DENIED'); + expect(outcome.status, 'ADR-0112 HTTP status').toBe(403); + expect(outcome.message).toContain(`the ${verb} would place '${object}' in another tenant`); +}; + +const SEEDED = [{ id: 'r1', organization_id: OWN_ORG }]; + +const CALLERS: Array<[string, object]> = [ + ['a member', MEMBER_CTX], + ['a platform administrator', ADMIN_CTX], +]; + +describe('[#21666] a create naming an organization meets the Layer 0 write wall, as the PATCH does', () => { + for (const object of [DECLARED, INJECTED]) { + for (const [who, context] of CALLERS) { + it(`${who}: a create naming another tenant's organization is refused with the PATCH's code — ${object}`, async () => { + const b = await boot(); + + const created = await attempt(() => create(b, object, { id: 'r2', name: 'new', organization_id: FOREIGN_ORG }, context)); + const patched = await attempt(() => repoint(b, object, FOREIGN_ORG, context)); + + expectWallRefusal(created, 'insert', object); + expectWallRefusal(patched, 'update', object); + expect({ code: created.code, status: created.status }).toEqual({ code: patched.code, status: patched.status }); + expect(await b.table(object), 'nothing stored, nothing moved').toEqual(SEEDED); + }); + } + + it(`an array insert naming another tenant's organization gets the single-row answer — ${object}`, async () => { + const b = await boot(); + + const bulk = await attempt(() => create(b, object, [{ id: 'r2', name: 'new', organization_id: FOREIGN_ORG }], ADMIN_CTX)); + const single = await attempt(() => create(b, object, { id: 'r3', name: 'new', organization_id: FOREIGN_ORG }, ADMIN_CTX)); + + expectWallRefusal(bulk, 'insert', object); + expectWallRefusal(single, 'insert', object); + expect(await b.table(object)).toEqual(SEEDED); + }); + + it(`a create naming no organization is stamped with the active organization before the hooks run — ${object}`, async () => { + const b = await boot(); + + const outcome = await attempt(() => create(b, object, { id: 'r2', name: 'new' }, MEMBER_CTX)); + + expect(outcome.ok, outcome.message).toBe(true); + expect(b.seenByHooks, 'the beforeInsert chain sees the stamp').toEqual([OWN_ORG]); + expect(await b.table(object)).toEqual([...SEEDED, { id: 'r2', organization_id: OWN_ORG }]); + }); + + it(`a create naming the caller's own active organization is admitted and stored there — ${object}`, async () => { + const b = await boot(); + + const outcome = await attempt(() => create(b, object, { id: 'r2', name: 'new', organization_id: OWN_ORG }, MEMBER_CTX)); + + expect(outcome.ok, outcome.message).toBe(true); + expect(await b.table(object)).toEqual([...SEEDED, { id: 'r2', organization_id: OWN_ORG }]); + }); + } + + // [#2937] The forged-organization insert by an ordinary member stays refused. + // What refuses it moved: it was the stamp rewriting the value; it is now the + // Layer 0 wall, loudly, with the row never stored anywhere. + it('[#2937] a member forging another tenant\'s organization_id on insert is refused, and no row lands in either tenant', async () => { + const b = await boot(); + + const outcome = await attempt(() => create(b, DECLARED, { id: 'r2', name: 'forged', organization_id: FOREIGN_ORG }, MEMBER_CTX)); + + expectWallRefusal(outcome, 'insert', DECLARED); + expect(b.seenByHooks, 'refused before the engine ran its hooks').toEqual([]); + expect(await b.table(DECLARED)).toEqual(SEEDED); + }); + + it('a system context keeps an explicit cross-organization value — the seed-replay path meets neither the stamp nor the wall', async () => { + const b = await boot(); + + const outcome = await attempt(() => create(b, DECLARED, { id: 'r2', name: 'replayed', organization_id: FOREIGN_ORG }, SYS_CTX)); + + expect(outcome.ok, outcome.message).toBe(true); + expect(await b.table(DECLARED)).toEqual([...SEEDED, { id: 'r2', organization_id: FOREIGN_ORG }]); + }); + + describe('under the `group` posture the wall is the membership set, for the create as for the PATCH', () => { + const GROUP_MEMBER_CTX = { ...MEMBER_CTX, accessible_org_ids: [OWN_ORG, SISTER_ORG] }; + + it('a create naming a sister organization the caller holds is admitted and stored THERE, as the PATCH moves the row there', async () => { + const b = await boot('group'); + + const created = await attempt(() => create(b, DECLARED, { id: 'r2', name: 'new', organization_id: SISTER_ORG }, GROUP_MEMBER_CTX)); + const patched = await attempt(() => repoint(b, DECLARED, SISTER_ORG, GROUP_MEMBER_CTX)); + + expect(created.ok, created.message).toBe(true); + expect(patched.ok, patched.message).toBe(true); + expect(await b.table(DECLARED)).toEqual([ + { id: 'r1', organization_id: SISTER_ORG }, + { id: 'r2', organization_id: SISTER_ORG }, + ]); + }); + + it('a create naming an organization outside the membership set is refused with the PATCH\'s code', async () => { + const b = await boot('group'); + + const created = await attempt(() => create(b, DECLARED, { id: 'r2', name: 'new', organization_id: FOREIGN_ORG }, GROUP_MEMBER_CTX)); + const patched = await attempt(() => repoint(b, DECLARED, FOREIGN_ORG, GROUP_MEMBER_CTX)); + + expectWallRefusal(created, 'insert', DECLARED); + expectWallRefusal(patched, 'update', DECLARED); + expect(await b.table(DECLARED)).toEqual(SEEDED); + }); + }); +}); diff --git a/packages/plugins/organizations/src/organizations-plugin.test.ts b/packages/plugins/organizations/src/organizations-plugin.test.ts index 64061c96ee0..5f3bf54e9a8 100644 --- a/packages/plugins/organizations/src/organizations-plugin.test.ts +++ b/packages/plugins/organizations/src/organizations-plugin.test.ts @@ -107,11 +107,15 @@ describe('OrganizationsPlugin', () => { expect(opCtx.data.organization_id).toBe('org-1'); }); - // [#2937] AUTHORITATIVE overwrite (behavior delta). A user-context insert may - // not choose its tenant: a supplied — possibly forged — organization_id is - // OVERWRITTEN with the caller's active org, closing the cross-tenant insert - // gap. (Previously a non-empty value was preserved — the vulnerability.) - it('[#2937] OVERWRITES a forged organization_id in user context with the active tenant', async () => { + // [#21666] FILL-ONLY in a user context too (ADR-0105 D5). A supplied value — + // another tenant's included — is left exactly as sent, for plugin-security's + // Layer 0 write wall to judge the way it judges the PATCH that names the same + // organization. Rewriting it here answered the create 201 with the row stored + // somewhere the caller never named. The refusal half (#2937's forged insert + // is still refused, by the wall) is pinned against the real SecurityPlugin in + // `create-explicit-organization-wall.test.ts`; this unit pins only that the + // stamp no longer touches the value. + it('[#21666] leaves a supplied organization_id naming another tenant untouched in a user context', async () => { const plugin = new OrganizationsPlugin(); const { ctx, middlewares } = makeCtx(); await plugin.init(ctx); @@ -119,16 +123,30 @@ describe('OrganizationsPlugin', () => { const opCtx: any = { object: 'task', operation: 'insert', - // 'org-2' is another tenant — the attacker's forged value. + // 'org-2' is another tenant. data: { name: 'A', organization_id: 'org-2' }, context: { userId: 'u1', tenantId: 'org-1' }, }; await middlewares[0](opCtx, async () => {}); - // Normalized to the caller's active org — NOT the forged value. + expect(opCtx.data.organization_id).toBe('org-2'); + }); + + it('[#21666] fills an EMPTY-string organization_id in a user context, as it fills an absent one', async () => { + const plugin = new OrganizationsPlugin(); + const { ctx, middlewares } = makeCtx(); + await plugin.init(ctx); + await plugin.start(ctx); + const opCtx: any = { + object: 'task', + operation: 'insert', + data: { name: 'A', organization_id: '' }, + context: { userId: 'u1', tenantId: 'org-1' }, + }; + await middlewares[0](opCtx, async () => {}); expect(opCtx.data.organization_id).toBe('org-1'); }); - it('[#2937] a same-tenant explicit organization_id is preserved (idempotent overwrite)', async () => { + it('[#2937] a same-tenant explicit organization_id is preserved', async () => { const plugin = new OrganizationsPlugin(); const { ctx, middlewares } = makeCtx(); await plugin.init(ctx); @@ -159,8 +177,8 @@ describe('OrganizationsPlugin', () => { }); // [#2937] The legitimate "set org_id on behalf" path (per-org seed replay / - // clone / orphan-claim, imports, migrations) runs under SYSTEM_CTX — it must - // keep an explicit cross-org value verbatim, NOT be overwritten. + // orphan-claim, migrations) runs under SYSTEM_CTX — it must keep an explicit + // cross-org value verbatim, and it meets neither the stamp nor the wall. it('[#2937] system context preserves an explicit cross-org organization_id (on-behalf writes unaffected)', async () => { const plugin = new OrganizationsPlugin(); const { ctx, middlewares } = makeCtx(); @@ -177,8 +195,8 @@ describe('OrganizationsPlugin', () => { }); // A non-`isSystem` context that carries a tenant but NO principal (a service - // acting with an org scope) keeps the prior FILL-ONLY semantics — it may still - // set an explicit value; only USER-context inserts are overwritten. + // acting with an org scope) gets the same FILL-ONLY stamp a user context gets + // — one rule for every non-system context since #21666. it('[#2937] principal-less (non-system) context keeps fill-only semantics', async () => { const plugin = new OrganizationsPlugin(); const { ctx, middlewares } = makeCtx(); diff --git a/packages/plugins/organizations/src/organizations-plugin.ts b/packages/plugins/organizations/src/organizations-plugin.ts index 26cbdd31e3b..02c022f8787 100644 --- a/packages/plugins/organizations/src/organizations-plugin.ts +++ b/packages/plugins/organizations/src/organizations-plugin.ts @@ -316,23 +316,32 @@ export class OrganizationsPlugin implements Plugin { const fields = await this.getObjectFieldNames(metadata, opCtx.object, ql); if (fields && fields.has('organization_id')) { const data = opCtx.data as Record; - // [#2937] AUTHORITATIVE stamp for USER-context inserts. A user may not - // choose which tenant a row lands in: their insert ALWAYS carries the - // caller's active organization, so a supplied — possibly FORGED — - // `organization_id` pointing at another org is OVERWRITTEN, never - // trusted. (Previously this only FILLED a missing value, so a forged - // non-empty value slipped through and — absent the Layer 0 insert - // post-image check — landed in the victim tenant.) `isSystem` - // short-circuited above (line ~136), so legitimate on-behalf writes - // that deliberately set another org — the per-org seed replay - // / orphan-claim, imports, migrations — run under SYSTEM_CTX and are - // untouched. A non-`isSystem` context with a tenant but NO principal - // (a service acting with an org scope) keeps the prior fill-only - // semantics so it can still set an explicit value. - const isUserContext = !!opCtx.context.userId; - if (isUserContext) { - data.organization_id = opCtx.context.tenantId; - } else if (data.organization_id == null || data.organization_id === '') { + // FILL-ONLY, for every non-system context — a user's included + // (ADR-0105 D5: the engine "stamps `organization_id` from + // `ctx.tenantId` (active org) when absent, and validates any explicit + // value"). This middleware owns the first half and nothing more: an + // ABSENT or empty value becomes the caller's active organization. + // + // A SUPPLIED value is never touched here. It goes on to the Layer 0 + // write wall in `@objectstack/plugin-security` (step 3.7, ADR-0095 + // D1), the same wall an UPDATE that re-points `organization_id` + // meets, and gets the same answer: admitted where the caller's + // organization scope (or a platform administrator's posture + // exemption) admits it, otherwise refused `403 PERMISSION_DENIED`. + // A forged value from a member (#2937) is therefore REFUSED, loudly, + // rather than rewritten. Rewriting it — what this line did for user + // contexts until #21666 — answered one operation two ways: the + // create replied 201 with the row stored in an organization the + // caller never named, while the equivalent PATCH and the array + // insert (which this middleware never touched) were refused. ⛔ Do + // not reintroduce a rewrite on any posture: a silently replaced + // input is exactly what an AI author or a script cannot detect. + // + // `isSystem` short-circuited above, so the legitimate writers that + // deliberately name another organization — the per-org seed replay, + // the orphan claim, imports, migrations — run under SYSTEM_CTX and + // meet neither this stamp nor the wall. + if (data.organization_id == null || data.organization_id === '') { data.organization_id = opCtx.context.tenantId; } } diff --git a/packages/plugins/organizations/vitest.config.ts b/packages/plugins/organizations/vitest.config.ts index 5dbbff36204..58ae5d89953 100644 --- a/packages/plugins/organizations/vitest.config.ts +++ b/packages/plugins/organizations/vitest.config.ts @@ -68,6 +68,26 @@ export default defineConfig({ find: /^@objectstack\/metadata-core$/, replacement: path.resolve(__dirname, '../../metadata-core/src/index.ts'), }, + // Test-only, all three: `create-explicit-organization-wall.test.ts` boots + // THIS package's Middleware A beside the real `SecurityPlugin` on a real + // `ObjectQL` engine over a real SQLite driver, because its subject is + // what the two middlewares answer TOGETHER — the stamp here fills an + // absent `organization_id`, the Layer 0 write wall there judges a + // supplied one. Resolved from `dist/`, the wall's half of every verdict + // would be about the last build of `plugin-security`, and a dist merely + // BEHIND runs green against the old wall while saying nothing. + { + find: /^@objectstack\/objectql$/, + replacement: path.resolve(__dirname, '../../objectql/src/index.ts'), + }, + { + find: /^@objectstack\/plugin-security$/, + replacement: path.resolve(__dirname, '../plugin-security/src/index.ts'), + }, + { + find: /^@objectstack\/driver-sqlite-wasm$/, + replacement: path.resolve(__dirname, '../../drivers/driver-sqlite-wasm/src/index.ts'), + }, ], }, test: { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fb0fc439dbf..47552c04e08 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1534,9 +1534,18 @@ importers: specifier: workspace:* version: link:../../types devDependencies: + '@objectstack/driver-sqlite-wasm': + specifier: workspace:* + version: link:../../drivers/driver-sqlite-wasm '@objectstack/metadata-core': specifier: workspace:* version: link:../../metadata-core + '@objectstack/objectql': + specifier: workspace:* + version: link:../../objectql + '@objectstack/plugin-security': + specifier: workspace:* + version: link:../plugin-security '@objectstack/rest': specifier: workspace:* version: link:../../rest