fix(plugin-security): close cross-tenant RLS leak (fail-closed)

Multi-bug regression where authenticated users — especially those without
an active organization — could see data from every tenant on tables like
account, sys_member, and sys_organization.

Root causes (all fail-OPEN paths, now fail-CLOSED):

1. RLSCompiler.compileFilter returned null when policies were applicable
   but none compiled, treated by callers as "no RLS configured" → no
   filter → all rows. Replaced with new exported RLS_DENY_FILTER sentinel
   ({id: '__rls_deny__:...'}) that cleanly returns zero rows on every
   driver without throwing.

2. SecurityPlugin.extractTargetField regex used `\b` after `=`, but `=`
   and the following whitespace are both non-word chars so the word
   boundary could never match. The function silently returned null for
   *every* `field = current_user.x` policy, defeating the field-existence
   safety net. Replaced trailing `\b` with `(?=\s|\()` lookahead.

3. SecurityPlugin.getObjectFieldNames only handled array-shaped fields
   and only consulted the kernel `metadata` service. Object schemas in
   this runtime are dict-shaped and live on the ObjectQL registry, so
   the lookup always returned null → "keep all policies" → wildcard
   `organization_id =` policy applied to tables that don't have that
   column. Fixed dict-shape handling and added a fallback to
   `objectql.getSchema(name)`.

4. The field-existence filter dropped policies whose target field was
   missing on the object. Tables like CRM `account` (no organization_id)
   ended up with no policies at all — fail-OPEN. Dropped policies now
   contribute the deny sentinel, so tables that haven't adopted multi-
   tenancy are denied by default while per-object overrides such as
   `sys_user_self` still grant access via OR-combine.

Verified end-to-end on `pnpm dev:crm`:
- No-org user: total=0 on sys_member/sys_organization/account; total=1
  on sys_user (own self via sys_user_self)
- With-org user: only own org's rows on sys_member/sys_organization
- Anonymous: still bypasses (documented Phase-1 limit)

Tests: 33 plugin-security tests pass (3 new fail-closed cases), full
runtime/rest/objectql/plugin-security regression green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: hotlong

CHANGELOG.md

@@ -9,7 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - **`@objectstack/driver-sql` tests failing in CI** — Added `vitest.config.ts` with resolve aliases for `@objectstack/spec/*` subpath exports (`/data`, `/contracts`, `/system`). Without these aliases, vitest could not resolve the source paths at test time, causing all 81 tests to fail with `ERR_MODULE_NOT_FOUND`.
-- **RLS over-filter when user has no active organization** (`@objectstack/plugin-security`) — `RLSCompiler.compileExpression` now treats a `null` user-context value the same as `undefined` and skips the policy. Before this fix, a logged-in user without an active organization (e.g. immediately after sign-up, before joining or creating one) caused the wildcard `tenant_isolation` policy to compile down to `organization_id = null`, which (a) returned zero rows from system tables that lack an `organization_id` column at all (e.g. `sys_user`) and (b) silently exposed any un-tenanted rows on application tables. The compiler now emits no filter for the `null` case, deferring to the more specific per-object rules (`sys_user_self`, `sys_organization_self`, …) and the field-existence safety net in `SecurityPlugin`. Two regression tests added in `packages/plugins/plugin-security/src/security-plugin.test.ts`. **Symptom this resolves:** the `_dashboard/apps/setup/sys_user` page now displays the signed-in user's own row instead of an empty list. Same fix applied to the `IN (current_user.array_property)` form for empty/missing arrays.
+- **RLS fail-open across tenants** (`@objectstack/plugin-security`) — A logged-in user with no active organization (e.g. immediately after sign-up, before joining or creating one) was previously seeing every tenant's data on `account`, `sys_member`, `sys_organization`, etc. Multiple compounding bugs were responsible:
+  1. `RLSCompiler.compileFilter` returned `null` when policies were applicable but none compiled — interpreted by callers as "no RLS configured" → no filter → all rows. **Fix:** introduced exported `RLS_DENY_FILTER` sentinel (`{ id: '__rls_deny__:00000000-0000-0000-0000-000000000000' }`); `compileFilter` now returns it when `policies.length > 0` but every policy expression failed to compile (missing `current_user.*` variable, unsupported expression, etc.). This naturally yields zero rows on every driver without throwing.
+  2. `SecurityPlugin.extractTargetField` used the regex `/^\s*([a-z_][a-z0-9_]*)\s*(=|IN|in)\b/` — the `\b` after `=` could never match (both `=` and the following whitespace are non-word characters, so no word boundary exists). The function silently returned `null` for *every* `field = current_user.x` policy, defeating the field-existence safety net entirely. **Fix:** replaced the trailing `\b` with a `(?=\s|\()` lookahead.
+  3. `SecurityPlugin.getObjectFieldNames` only handled `Array<{ name }>`-shaped fields and only consulted the kernel's `metadata` service — but object schemas in this runtime are dict-shaped (`fields: { name: Field.text(...), … }`) and live on the ObjectQL registry, not the metadata-manager. The lookup always returned `null`, which short-circuited to "keep all policies" → tables that lacked `organization_id` (e.g. CRM `account`, `sys_organization`) had a bogus `organization_id = …` filter applied, which different drivers interpreted differently (some erroring, some silently returning 0/all). **Fix:** the helper now handles both array and dict shapes and falls back to `objectql.getSchema(name)` when the metadata service has no entry.
+  4. The field-existence filter previously *dropped* policies whose target column didn't exist on the object, leaving the policy set empty for tables like `account` (no `organization_id`) — fail-OPEN. **Fix:** policies dropped for missing fields now contribute the deny sentinel, so tables that haven't adopted multi-tenancy are denied by default; per-object overrides such as `sys_user_self` (`id = current_user.id`) still grant access via OR-combine.
+  Three regression tests added in `packages/plugins/plugin-security/src/security-plugin.test.ts` (unsupported expression → deny, missing user-context variable → deny, valid tenant policy still compiles normally). End-to-end verified on `pnpm dev:crm`: a user without an active org gets `total: 0` on `sys_member`/`sys_organization`/`account`; a user with an active org sees only their org's rows.
 - **`PermissionDeniedError` returned as HTTP 404 instead of 403** (`@objectstack/rest`) — `mapDataError` previously fell through to the unknown-object heuristic for any error message containing the object name and the substring `"not"`, which matched the security message `"… on object 'sys_user' is not permitted …"`. The mapper now short-circuits on `error.code === 'PERMISSION_DENIED'` / `error.name === 'PermissionDeniedError'` / message-prefix match before the heuristic and returns HTTP 403 with `code: 'PERMISSION_DENIED'`. The CRUD route catches in `RestServer` also stop logging 403s as `[REST] Unhandled error:` (4 sites updated).
 
 ### Added

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

@@ -9,7 +9,7 @@
 
 export { SecurityPlugin } from './security-plugin.js';
 export { PermissionEvaluator } from './permission-evaluator.js';
-export { RLSCompiler } from './rls-compiler.js';
+export { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js';
 export { FieldMasker } from './field-masker.js';
 export { PermissionDeniedError, isPermissionDeniedError } from './errors.js';
 export {

packages/plugins/plugin-security/src/rls-compiler.ts

@@ -21,6 +21,25 @@ interface RLSUserContext {
   [key: string]: unknown;
 }
 
+/**
+ * Sentinel filter used when applicable RLS policies exist but none can
+ * be compiled against the current execution context (typically because a
+ * required `current_user.*` variable is missing — e.g. the user has no
+ * active organization). The filter compares `id` against a non-printable
+ * UUID-shaped string that no real record will ever carry, so the upstream
+ * SQL layer naturally returns zero rows without raising an error. This
+ * gives us **fail-closed** semantics for select/update/delete on tables
+ * that the user is not entitled to see, without forcing every caller to
+ * handle a thrown `PermissionDeniedError` for what is conceptually an
+ * empty result set.
+ *
+ * Exposed for the SecurityPlugin's optional short-circuit path and for
+ * tests; see {@link RLSCompiler.compileFilter}.
+ */
+export const RLS_DENY_FILTER: Record<string, unknown> = Object.freeze({
+  id: '__rls_deny__:00000000-0000-0000-0000-000000000000',
+});
+
 /**
  * RLSCompiler
  * 
@@ -31,6 +50,15 @@ export class RLSCompiler {
   /**
    * Compile RLS policies into a query filter for the given user context.
    * Multiple policies for the same object/operation are OR-combined (any match allows access).
+   *
+   * Return-value semantics:
+   * - `null`   → no policies applicable → caller applies no RLS filter.
+   * - non-null → caller AND's it onto the existing where clause.
+   * - {@link RLS_DENY_FILTER} → policies were defined but none could be
+   *   compiled (e.g. wildcard `tenant_isolation` against a user with no
+   *   active organization). The caller must treat this as "deny by
+   *   default" — its `id` comparison naturally yields zero rows on
+   *   select/update/delete, which is the safe fail-closed answer.
    */
   compileFilter(
     policies: RowLevelSecurityPolicy[],
@@ -54,7 +82,14 @@ export class RLSCompiler {
       }
     }
 
-    if (filters.length === 0) return null;
+    if (filters.length === 0) {
+      // Policies *were* applicable but every one of them depended on a
+      // `current_user.*` variable that wasn't populated (or used an
+      // expression we couldn't compile). Fail closed — return a sentinel
+      // filter that matches no rows. This prevents the "user without an
+      // active org sees every tenant's data" class of bug.
+      return RLS_DENY_FILTER;
+    }
     if (filters.length === 1) return filters[0];
 
     // Multiple policies: OR-combine (any policy allows access)

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

@@ -4,7 +4,7 @@ import { describe, it, expect, vi } from 'vitest';
 import { SecurityPlugin } from './security-plugin.js';
 import { PermissionEvaluator } from './permission-evaluator.js';
 import { FieldMasker } from './field-masker.js';
-import { RLSCompiler } from './rls-compiler.js';
+import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js';
 import type { PermissionSet } from '@objectstack/spec/security';
 
 // ---------------------------------------------------------------------------
@@ -310,30 +310,35 @@ describe('RLSCompiler', () => {
     expect(filter).toEqual({ $or: [{ owner_id: 'u99' }, { status: 'public' }] });
   });
 
-  it('should return null for unsupported expression', () => {
+  it('should fail-closed (deny filter) when expression is unsupported', () => {
+    // Previously returned null, which is fail-OPEN (no RLS applied →
+    // every row visible). Now returns the deny sentinel so a misconfigured
+    // policy doesn't silently disable tenant isolation.
     const compiler = new RLSCompiler();
     const policy: any = { object: 'x', operation: 'select', using: 'complex expression WITH unsupported syntax' };
     const filter = compiler.compileFilter([policy]);
-    expect(filter).toBeNull();
+    expect(filter).toEqual(RLS_DENY_FILTER);
   });
 
-  it('should skip equality policy when user-context value is null', () => {
+  it('should fail-closed when the only policy depends on a missing user-context value', () => {
     // Repro: a logged-in user without an active organization. The
-    // tenant_isolation rule would otherwise emit `organization_id = null`,
-    // which silently exposes un-tenanted rows or breaks queries on system
-    // tables that lack the column. Treating null as "skip" is safe because
-    // the field-existence check + most-permissive permission-set merge
-    // remain in force.
+    // tenant_isolation rule would compile to `organization_id = null`,
+    // which previously either (a) returned every row from tables that
+    // lack `organization_id` (e.g. sys_user) or (b) returned every row
+    // because compileFilter dropped it as null. We now fail-closed so
+    // the user sees zero rows on tenant-aware tables until they pick an
+    // active organization. Per-object rules that *do* compile (e.g.
+    // `sys_user_self`) still grant access — see the next test.
     const compiler = new RLSCompiler();
     const policy: any = { object: '*', operation: 'all', using: 'organization_id = current_user.organization_id' };
     const ctx: any = { userId: 'u1', tenantId: null, roles: [] };
     const filter = compiler.compileFilter([policy], ctx);
-    expect(filter).toBeNull();
+    expect(filter).toEqual(RLS_DENY_FILTER);
   });
 
   it('should still apply policy when only one of multiple has a usable value', () => {
-    // Same scenario as above but combined with a self-row rule. The
-    // organization_id rule should drop out, leaving only `id = current_user.id`.
+    // tenant policy can't compile (null tenantId) but sys_user_self can.
+    // Result: just the self-row filter — the broken tenant policy drops out.
     const compiler = new RLSCompiler();
     const tenantPolicy: any = { object: '*', operation: 'all', using: 'organization_id = current_user.organization_id' };
     const selfPolicy: any = { object: 'sys_user', operation: 'select', using: 'id = current_user.id' };
@@ -342,6 +347,15 @@ describe('RLSCompiler', () => {
     expect(filter).toEqual({ id: 'u1' });
   });
 
+  it('should compile tenant policy normally when an active organization is set', () => {
+    // Sanity check — the deny path is only triggered by *missing* values.
+    const compiler = new RLSCompiler();
+    const policy: any = { object: '*', operation: 'all', using: 'organization_id = current_user.organization_id' };
+    const ctx: any = { userId: 'u1', tenantId: 'org-1', roles: [] };
+    const filter = compiler.compileFilter([policy], ctx);
+    expect(filter).toEqual({ organization_id: 'org-1' });
+  });
+
   it('should get applicable policies for object and operation', () => {
     const compiler = new RLSCompiler();
     const policies: any[] = [

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

@@ -3,7 +3,7 @@
 import { Plugin, PluginContext } from '@objectstack/core';
 import type { PermissionSet, RowLevelSecurityPolicy } from '@objectstack/spec/security';
 import { PermissionEvaluator } from './permission-evaluator.js';
-import { RLSCompiler } from './rls-compiler.js';
+import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js';
 import { FieldMasker } from './field-masker.js';
 import { PermissionDeniedError } from './errors.js';
 import {
@@ -175,19 +175,33 @@ export class SecurityPlugin implements Plugin {
       // 3. RLS filter injection
       const allRlsPolicies = this.collectRLSPolicies(permissionSets, opCtx.object, opCtx.operation);
       if (allRlsPolicies.length > 0 && opCtx.ast) {
-        // Drop policies whose target field doesn't exist on the object —
-        // wildcard policies like `organization_id = ...` must not corrupt
-        // queries against system tables that lack the column (sys_jwks,
-        // sys_audit_log, etc.). When schema lookup fails we keep the
-        // policy (fail-closed for unknown objects).
-        const objectFields = await this.getObjectFieldNames(metadata, opCtx.object);
-        const safe = objectFields
+        // Field-existence safety: wildcard policies (`object: '*'`) target
+        // fields like `organization_id` that may not exist on every object
+        // (e.g. system tables, CRM apps that haven't yet adopted multi-tenancy).
+        //
+        // We treat such policies as a *deny* contribution rather than dropping
+        // them, so they fail-closed when no per-object policy provides an
+        // alternate match. Any per-object policy that DOES compile against
+        // the object will OR-combine and grant access (e.g. `sys_user_self`).
+        // When the schema lookup itself fails we keep all policies (drivers
+        // will surface column errors clearly during compilation).
+        const objectFields = await this.getObjectFieldNames(metadata, opCtx.object, ql);
+        let dropped = 0;
+        const compilable = objectFields
           ? allRlsPolicies.filter((p) => {
               const targetField = this.extractTargetField(p.using);
-              return targetField ? objectFields.has(targetField) : true;
+              const ok = targetField ? objectFields.has(targetField) : true;
+              if (!ok) dropped++;
+              return ok;
             })
           : allRlsPolicies;
-        const rlsFilter = this.rlsCompiler.compileFilter(safe, opCtx.context);
+        let rlsFilter = this.rlsCompiler.compileFilter(compilable, opCtx.context);
+        // If every applicable policy was dropped because of missing fields,
+        // contribute the deny sentinel (zero rows) — matches the rls-compiler
+        // semantics for "policies were applicable but none compiled".
+        if (rlsFilter == null && dropped > 0) {
+          rlsFilter = { ...RLS_DENY_FILTER };
+        }
         if (rlsFilter) {
           if (opCtx.ast.where) {
             opCtx.ast.where = { $and: [opCtx.ast.where, rlsFilter] };
@@ -241,13 +255,31 @@ export class SecurityPlugin implements Plugin {
   private async getObjectFieldNames(
     metadata: any,
     objectName: string,
+    ql?: any,
   ): Promise<Set<string> | null> {
     try {
-      const obj = await metadata?.get?.('object', objectName);
-      if (!obj || !Array.isArray(obj.fields)) return null;
+      let obj = await metadata?.get?.('object', objectName);
+      // Fallback: in some runtimes the kernel's `metadata` service does not
+      // hold object schemas (those live on the ObjectQL registry instead).
+      // Ask ObjectQL directly so wildcard RLS filtering still works against
+      // identity tables (sys_organization, sys_user, etc.).
+      if ((!obj || !obj.fields) && typeof ql?.getSchema === 'function') {
+        obj = ql.getSchema(objectName);
+      }
+      if (!obj || !obj.fields) return null;
       const set = new Set<string>(['id']);
-      for (const f of obj.fields) {
-        if (f?.name) set.add(String(f.name));
+      if (Array.isArray(obj.fields)) {
+        for (const f of obj.fields) {
+          if (f?.name) set.add(String(f.name));
+        }
+      } else if (typeof obj.fields === 'object') {
+        for (const key of Object.keys(obj.fields)) {
+          set.add(key);
+          const v = (obj.fields as Record<string, any>)[key];
+          if (v && typeof v === 'object' && v.name) set.add(String(v.name));
+        }
+      } else {
+        return null;
       }
       return set;
     } catch {
@@ -262,7 +294,12 @@ export class SecurityPlugin implements Plugin {
    */
   private extractTargetField(using?: string): string | null {
     if (!using) return null;
-    const m = using.match(/^\s*([a-z_][a-z0-9_]*)\s*(=|IN|in)\b/);
+    // Match `field =` or `field IN`/`in`. Note: `\b` is omitted after `=`
+    // because `=` is non-word and the next char (space) is non-word too —
+    // a word boundary cannot exist between two non-word chars, so `=\b`
+    // would never match. We instead require the alternation token to be
+    // followed by whitespace or `(`.
+    const m = using.match(/^\s*([a-z_][a-z0-9_]*)\s*(?:=|IN|in)(?=\s|\()/);
     return m ? m[1] : null;
   }
 }