test(security): pin server-managed field set + tenant-wall write guard; fix systemFields docs (#3058)

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<string> =>
+    new Set<string>([
+      ...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<string>([
+    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<string>([...activelyInjected(), ...reservedDefenseInDepth]);
+    const denylist = new Set<string>(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);
+  });
+});

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<string, unknown> = {}) =>
+      ({ 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] });

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()