diff --git a/packages/objectql/src/system-managed-fields-conformance.test.ts b/packages/objectql/src/system-managed-fields-conformance.test.ts new file mode 100644 index 0000000000..d95809d922 --- /dev/null +++ b/packages/objectql/src/system-managed-fields-conformance.test.ts @@ -0,0 +1,103 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security'; +import { SystemFieldName } from '@objectstack/spec/system'; +import { applySystemFields, SEARCH_COMPANION_FIELD } from './index.js'; + +// --------------------------------------------------------------------------- +// [#3058] Single-source conformance for the server-managed field set. +// +// `PUBLIC_FORM_SERVER_MANAGED_FIELDS` (@objectstack/spec/security) is the shared +// denylist that BOTH the anonymous public-form enforcement points read — the +// REST form-route allow-list (@objectstack/rest) and the data-layer grant strip +// (@objectstack/plugin-security). It has to name every column the server manages +// on its own, because on the anonymous surface nothing else guards them. +// +// The risk it carries is DRIFT: the actual server-managed columns are assembled +// from three unrelated injection sites (the registry's `applySystemFields`, the +// search-companion, the driver primary key) plus a few defense-in-depth reserved +// names — none of which imports the denylist. When #3022 first shipped this set, +// the fields were hand-copied; a new injected system field added later would +// silently leak through the public-form surface with no test to catch it. +// +// This test pins the denylist to an EXACT partition of two documented groups, so +// adding an injected system field (or a stray denylist entry) fails loudly here +// with a message pointing at what to reconcile — rather than becoming a live +// anonymous-write hole. It deliberately does NOT re-hardcode the 11 names; it +// derives the injected group from the real injection code. +// --------------------------------------------------------------------------- +describe('[#3058] PUBLIC_FORM_SERVER_MANAGED_FIELDS conformance', () => { + // Group A — columns OPEN-CORE actively injects onto a plain author-defined + // business object. Enumerated from the real injection code, not re-listed, so + // a new injected field is caught automatically. + const injectedByRegistry = (): string[] => { + const base: any = { name: 'proj', fields: { title: { type: 'text' } } }; + // multiTenant:true exercises the widest injection (organization_id included). + const withSys = applySystemFields(base, { multiTenant: true }); + return Object.keys(withSys.fields ?? {}).filter((k) => !(k in base.fields)); + }; + + const activelyInjected = (): Set => + new Set([ + ...injectedByRegistry(), // organization_id, created_at, created_by, updated_at, updated_by, owner_id + SEARCH_COMPANION_FIELD, // '__search' — hidden search-normalization companion (search-companion.ts) + SystemFieldName.ID, // 'id' — driver-provisioned primary key + ]); + + // Group B — names in the denylist that open-core does NOT inject as a schema + // field, kept as defense-in-depth so a forged value can never be admitted on + // the anonymous surface even where the column is contributed elsewhere: + // • tenant_id — legacy/enterprise tenant key (not injected by open-core). + // • is_deleted / deleted_at — soft-delete state, written by the lifecycle/ + // trash layer at runtime, never client-suppliable on a public form. + const reservedDefenseInDepth = new Set([ + SystemFieldName.TENANT_ID, // 'tenant_id' + 'is_deleted', + SystemFieldName.DELETED_AT, // 'deleted_at' + ]); + + it('registry injection actually produces the fields this test reasons about', () => { + // Guards the test itself: if applySystemFields stops injecting these, the + // conformance assertions below would pass vacuously. + const injected = new Set(injectedByRegistry()); + for (const f of ['organization_id', 'created_at', 'created_by', 'updated_at', 'updated_by', 'owner_id']) { + expect(injected.has(f)).toBe(true); + } + }); + + it('every server-injected system field is on the public-form denylist (no anonymous-write leak)', () => { + // The load-bearing direction: a new injected system field MUST be added to + // PUBLIC_FORM_SERVER_MANAGED_FIELDS, or it becomes client-suppliable on the + // anonymous public-form surface. + for (const field of activelyInjected()) { + expect( + PUBLIC_FORM_SERVER_MANAGED_FIELDS.has(field), + `server-injected field '${field}' is missing from PUBLIC_FORM_SERVER_MANAGED_FIELDS ` + + `(packages/spec/src/security/public-form.ts) — add it or it leaks through the public-form surface`, + ).toBe(true); + } + }); + + it('the denylist is exactly (actively-injected ∪ documented-reserved) — no stray or missing entries', () => { + const accountedFor = new Set([...activelyInjected(), ...reservedDefenseInDepth]); + const denylist = new Set(PUBLIC_FORM_SERVER_MANAGED_FIELDS); + + // No denylist entry is unexplained (a name nothing injects and not documented + // as reserved → either dead weight or a mis-categorized new field). + for (const field of denylist) { + expect( + accountedFor.has(field), + `'${field}' is on PUBLIC_FORM_SERVER_MANAGED_FIELDS but is neither injected by open-core ` + + `nor listed as defense-in-depth reserved in this test — classify it`, + ).toBe(true); + } + // And every accounted-for name is on the denylist (covered field-by-field for + // the injected group above; this closes the reserved group too). + for (const field of accountedFor) { + expect(denylist.has(field), `'${field}' should be on PUBLIC_FORM_SERVER_MANAGED_FIELDS`).toBe(true); + } + // Exact partition ⇒ identical cardinality (catches a duplicate/renamed entry). + expect(denylist.size).toBe(accountedFor.size); + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index 0cfa2bd282..38d5480939 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -384,6 +384,96 @@ describe('SecurityPlugin', () => { }); }); + // ------------------------------------------------------------------------- + // [ADR-0095 D1 / #2937 Finding 1] step 3.7 — the Layer 0 tenant wall on the + // WRITE post-image. A SUPPLIED (non-empty) `organization_id` in an insert / + // update payload must satisfy the same Layer 0 filter the read side uses; + // a forged (insert) or re-pointed (update) value that would land the row in + // another tenant is denied fail-closed. Previously only the read-side Layer 0 + // and the multitenant dogfood covered this — these are the package-level + // write-side unit tests for the wall. + // + // A plain-CRUD permission set with NO `tenant_isolation` RLS policy is used on + // purpose, so Layer 0 (step 3.7) — not a business RLS `check` (step 3.6) — is + // the sole enforcer under test. `orgScoping: true` flips + // `SecurityPlugin.orgScopingEnabled`, which is what makes Layer 0 active. + // ------------------------------------------------------------------------- + describe('organization_id tenant wall on writes (step 3.7)', () => { + const memberSet: PermissionSet = { + name: 'member_default', + label: 'Member', + objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, + } as any; + + const boot = async (findOneImpl?: (query: any) => any) => { + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + const harness = makeMiddlewareCtx({ permissionSets: [memberSet], orgScoping: true, findOneImpl }); + await plugin.init(harness.ctx); + await plugin.start(harness.ctx); + return harness; + }; + const memberCtx = (extra: Record = {}) => + ({ userId: 'u1', tenantId: 'org-1', positions: [], permissions: [], ...extra }); + + it('insert forging a cross-tenant organization_id is denied', async () => { + const harness = await boot(); + const opCtx: any = { + object: 'task', operation: 'insert', data: { name: 'A', organization_id: 'org-2' }, + context: memberCtx(), + }; + await expect(harness.run(opCtx)).rejects.toThrow(/would place .* in another tenant/); + }); + + it("insert supplying the caller's own organization_id passes", async () => { + const harness = await boot(); + const opCtx: any = { + object: 'task', operation: 'insert', data: { name: 'A', organization_id: 'org-1' }, + context: memberCtx(), + }; + await harness.run(opCtx); // must not throw + expect(opCtx.data.organization_id).toBe('org-1'); + }); + + it('update re-pointing organization_id to another tenant is denied', async () => { + // Pre-image is visible (in-tenant), so step 2.7 passes and 3.7 is reached. + const harness = await boot(() => ({ id: 't1', organization_id: 'org-1' })); + const opCtx: any = { + object: 'task', operation: 'update', data: { id: 't1', organization_id: 'org-2' }, + context: memberCtx(), + }; + await expect(harness.run(opCtx)).rejects.toThrow(/would place .* in another tenant/); + }); + + it("update echoing the caller's own organization_id passes", async () => { + const harness = await boot(() => ({ id: 't1', organization_id: 'org-1' })); + const opCtx: any = { + object: 'task', operation: 'update', data: { id: 't1', name: 'renamed', organization_id: 'org-1' }, + context: memberCtx(), + }; + await harness.run(opCtx); // must not throw + }); + + it("a write that doesn't supply organization_id is untouched (auto-stamp's job)", async () => { + const harness = await boot(); + const opCtx: any = { + object: 'task', operation: 'insert', data: { name: 'A' }, + context: memberCtx(), + }; + await harness.run(opCtx); // must not throw — no cross-tenant check on an absent value + // SecurityPlugin never stamps organization_id (that's plugin-org-scoping's job). + expect(opCtx.data.organization_id).toBeUndefined(); + }); + + it('a system-context write bypasses the wall (import / migration / SYSTEM_CTX)', async () => { + const harness = await boot(); + const opCtx: any = { + object: 'task', operation: 'insert', data: { name: 'A', organization_id: 'org-2' }, + context: memberCtx({ isSystem: true }), + }; + await harness.run(opCtx); // isSystem short-circuits the whole middleware — legit cross-org moves unaffected + }); + }); + it('without org-scoping plugin — strips tenant_isolation RLS so find applies no tenant where', async () => { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); const harness = makeMiddlewareCtx({ permissionSets: [tenantPolicySet] }); diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index e68320ea2e..2127e365ae 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -692,32 +692,39 @@ const ObjectSchemaBase = z.object({ * * The `SchemaRegistry` augments every user object with a small set of * implicit system fields at registration time so authors don't have to - * declare them per-object (Salesforce-style). Currently injected: + * declare them per-object (Salesforce-style). Currently injected + * (`packages/objectql/src/registry.ts` → `applySystemFields`): * - * - `organization_id` — `lookup → sys_organization`. Injected only when - * the kernel runs in multi-tenant mode (`OS_MULTI_ORG_ENABLED === 'true'`; - * default is off — single-tenant). - * Required for the default `tenant_isolation` RLS policy and the - * SecurityPlugin's auto-fill on insert to take effect. + * - `organization_id` — `lookup → sys_organization`. The COLUMN is + * provisioned unconditionally (subject to the opt-outs below); only its + * DB index is gated on multi-tenant mode. It stays NULL on single-tenant + * stacks and is auto-stamped on insert by `@objectstack/organizations` + * (the `org-scoping` service) in multi-tenant mode. + * - Audit columns — `created_at`, `created_by`, `updated_at`, `updated_by` + * (`readonly` + `system`). Gated by `audit` below. + * - `owner_id` — `lookup → sys_user`, auto-provisioned on user-authored + * business objects (auto-stamped to the creating user on insert; + * reassignable). Governed by the object-level `ownership` property + * (`'user' | 'org' | 'none'`), NOT by `owner` below. * * Author-declared fields with the same name always win over injection - * (no overwrite). Objects with `managedBy` set are skipped entirely — - * better-auth/system/platform tables already declare what they need. + * (no overwrite). Objects with `managedBy` set (and the `sys_*` namespace) + * are skipped for ownership; `managedBy: 'better-auth'` is skipped entirely — + * better-auth's own migrations own that column layout. * * Set `systemFields: false` to opt the object out completely. Pass an - * options object to selectively disable individual injections (currently - * only `tenant`, but reserved keys `owner`/`audit` are pre-defined for - * future expansion). + * options object to selectively disable individual injections (`tenant`, + * `audit`). * - * @default undefined (= injection enabled, gated by kernel mode) + * @default undefined (= injection enabled) */ systemFields: z .union([ z.literal(false), z.object({ - tenant: z.boolean().optional().describe('Inject organization_id (multi-tenant only). Default true.'), - owner: z.boolean().optional().describe('Reserved for future owner_id auto-injection.'), - audit: z.boolean().optional().describe('Reserved for future created_by/updated_by auto-injection.'), + tenant: z.boolean().optional().describe('Inject the organization_id column. Default true (the column is always provisioned; the multi-tenant flag governs only its index).'), + owner: z.boolean().optional().describe('Unwired: owner_id provisioning is governed by the object-level `ownership` property (`user`/`org`/`none`), not this key. Reserved.'), + audit: z.boolean().optional().describe('Inject the audit columns (created_at/created_by/updated_at/updated_by). Default true.'), }), ]) .optional()