fix(security): close four P0 launch-readiness findings

.changeset/p0-security-hardening.md

@@ -0,0 +1,27 @@
+---
+'@objectstack/plugin-auth': patch
+'@objectstack/plugin-security': patch
+'@objectstack/driver-sql': patch
+'@objectstack/driver-mongodb': patch
+---
+
+fix(security): close four P0 launch-readiness findings
+
+- **plugin-auth (P0-1):** `generateSecret()` now throws (fails boot) when no
+  `OS_AUTH_SECRET` is set and `NODE_ENV==='production'`, instead of silently
+  falling back to a predictable `dev-secret-<timestamp>` (session forgery). The
+  dev/test fallback is unchanged.
+- **plugin-security (P0-2):** the permission-resolution `catch` now **fails
+  closed** — it logs at ERROR and throws `PermissionDeniedError` rather than
+  `return next()`. A degraded metadata service can no longer let every
+  authenticated request bypass RBAC/RLS. System operations still bypass as before.
+- **driver-sql (P0-3):** the `contains` / `$contains` operator now escapes LIKE
+  metacharacters (`%` / `_` / `\`) in the user value and binds an explicit
+  `ESCAPE '\'`, so a value of `%` matches literally instead of every row
+  (filter bypass). Correct across SQLite/MySQL/Postgres.
+- **driver-mongodb (P0-4):** the field-operator translator now rejects unknown
+  `$`-operators instead of passing them through, blocking `$where` / `$function`
+  / `$expr` (server-side JS execution / query-intent bypass). All legitimate
+  ObjectQL operators remain allowlisted.
+
++12 regression tests across the four packages.

docs/launch-readiness.md

@@ -55,7 +55,7 @@ fix or acceptance.**
 - **Action:** Throw (fail boot) when no secret is configured and
   `NODE_ENV === 'production'`; add a pre-boot config validation. Document
   `OS_AUTH_SECRET` as a required go-live env var.
-- **Owner:** _______  ·  Verify ☐  ·  Sign-off ☐  ·  Notes: _______
+- **Owner:** _______  ·  Verify ✅ (confirmed real @ `main`)  ·  Sign-off ☐  ·  Notes: Fixed — `generateSecret()` throws in production; +3 tests. Awaiting human sign-off.
 
 ### P0-2 — Metadata-service failure bypasses all RBAC/RLS (fail-open)
 - **Area:** `plugin-security` — `src/security-plugin.ts:309–312`
@@ -65,7 +65,7 @@ fix or acceptance.**
 - **Action:** Add a circuit-breaker + ERROR-level alerting; add an integration
   test asserting "metadata service down ⇒ request denied", not allowed. Decide
   fail-closed vs. fail-open explicitly and record the decision.
-- **Owner:** _______  ·  Verify ☐  ·  Sign-off ☐  ·  Notes: _______
+- **Owner:** _______  ·  Verify ✅ (confirmed real @ `main`)  ·  Sign-off ☐  ·  Notes: Decision = **fail-closed**. `catch` now logs ERROR + throws `PermissionDeniedError`; system ops still bypass. +2 tests. Awaiting human sign-off.
 
 ### P0-3 — Unescaped LIKE metacharacters in `contains`
 - **Area:** `driver-sql` — `src/sql-driver.ts:1565, 1656`
@@ -75,7 +75,7 @@ fix or acceptance.**
   (HIGH, data)
 - **Action:** Escape `%` and `_` (and the escape char) before building the LIKE
   pattern; add a test with a `%`/`_` payload.
-- **Owner:** _______  ·  Verify ☐  ·  Sign-off ☐  ·  Notes: _______
+- **Owner:** _______  ·  Verify ✅ (confirmed real @ `main`)  ·  Sign-off ☐  ·  Notes: Fixed — escape `%`/`_`/`\` + explicit `ESCAPE '\'` (SQLite needs it); +3 tests. Awaiting human sign-off.
 
 ### P0-4 — MongoDB filter passes arbitrary `$` operators through
 - **Area:** `driver-mongodb` — `src/mongodb-filter.ts:82–84`
@@ -85,7 +85,7 @@ fix or acceptance.**
 - **Action:** Allowlist safe operators (`$eq/$ne/$gt/$gte/$lt/$lte/$in/$nin/$and/$or/...`);
   reject unknown ones at filter-build time. If MongoDB is not a v1 driver, mark
   Roadmap instead.
-- **Owner:** _______  ·  Verify ☐  ·  Sign-off ☐  ·  Notes: _______
+- **Owner:** _______  ·  Verify ✅ (confirmed real @ `main`)  ·  Sign-off ☐  ·  Notes: Fixed — translator now rejects unknown `$`-operators (blocks `$where`/`$function`); +4 tests. Awaiting human sign-off. (Still confirm whether MongoDB is a v1 driver.)
 
 ### P0-5 — Realtime & feed are in-memory only (no cluster coordination)
 - **Area:** `service-realtime` (`in-memory-realtime-adapter.ts`), `service-feed`

packages/plugins/driver-mongodb/src/mongodb-filter.test.ts

@@ -180,4 +180,22 @@ describe('MongoDB Filter Translator', () => {
       });
     });
   });
+
+  describe('operator allowlist (P0-4)', () => {
+    it('rejects $where (server-side JS execution)', () => {
+      expect(() => translateFilter({ name: { $where: 'this.x == 1' } })).toThrow(/unsupported filter operator '\$where'/);
+    });
+
+    it('rejects $function', () => {
+      expect(() => translateFilter({ name: { $function: { body: 'fn', args: [], lang: 'js' } } })).toThrow(/unsupported filter operator/);
+    });
+
+    it('rejects unknown $-operators rather than passing them through', () => {
+      expect(() => translateFilter({ name: { $expr: 1 } })).toThrow(/unsupported filter operator '\$expr'/);
+    });
+
+    it('still accepts every allowlisted field operator', () => {
+      expect(() => translateFilter({ a: { $eq: 1 }, b: { $in: [1, 2] }, c: { $contains: 'x' }, d: { $exists: true } })).not.toThrow();
+    });
+  });
 });

packages/plugins/driver-mongodb/src/mongodb-filter.ts

@@ -165,8 +165,11 @@ function translateFieldOperators(ops: Record<string, unknown>): Record<string, u
         break;
 
       default:
-        // Pass through unknown operators as-is
-        result[op] = value;
+        // Reject unknown operators instead of passing them through (P0). Keys
+        // like `$where` / `$function` / `$expr` / `$accumulator` would reach
+        // MongoDB and execute server-side JavaScript or bypass query intent.
+        // Every legitimate ObjectQL field operator is allowlisted above.
+        throw new Error(`[mongodb] unsupported filter operator '${op}'`);
     }
   }
 

packages/plugins/driver-sql/src/sql-driver-like-escape.test.ts

@@ -0,0 +1,51 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { SqlDriver } from '../src/index.js';
+
+/**
+ * P0-3 regression: the `contains` / `$contains` operator must escape LIKE
+ * metacharacters (`%` / `_`) in the user value so they match literally. An
+ * unescaped `%` would expand to `%%%` and match every row — a filter bypass.
+ */
+describe('SqlDriver — contains escapes LIKE metacharacters (P0-3)', () => {
+  let driver: SqlDriver;
+  let knex: any;
+
+  beforeEach(async () => {
+    driver = new SqlDriver({
+      client: 'better-sqlite3',
+      connection: { filename: ':memory:' },
+      useNullAsDefault: true,
+    });
+    knex = (driver as any).knex;
+    await knex.schema.createTable('docs', (t: any) => {
+      t.string('id').primary();
+      t.string('title');
+    });
+    await knex('docs').insert([
+      { id: '1', title: '50% off sale' }, // literal %
+      { id: '2', title: 'plain title' }, // no metacharacter
+      { id: '3', title: 'a_b underscore' }, // literal _
+    ]);
+  });
+
+  afterEach(async () => {
+    await knex.destroy();
+  });
+
+  it('a "%" value matches only rows containing a literal %, not every row', async () => {
+    const r = await driver.find('docs', { where: { title: { $contains: '%' } } });
+    expect(r.map((x: any) => x.id)).toEqual(['1']);
+  });
+
+  it('a "_" value matches only rows containing a literal _, not any single char', async () => {
+    const r = await driver.find('docs', { where: { title: { $contains: '_' } } });
+    expect(r.map((x: any) => x.id)).toEqual(['3']);
+  });
+
+  it('an ordinary substring still matches normally', async () => {
+    const r = await driver.find('docs', { where: { title: { $contains: 'sale' } } });
+    expect(r.map((x: any) => x.id)).toEqual(['1']);
+  });
+});

packages/plugins/driver-sql/src/sql-driver.ts

@@ -1562,7 +1562,7 @@ export class SqlDriver implements IDataDriver {
             const methodNotIn = nextJoin === 'or' ? 'orWhereNotIn' : 'whereNotIn';
 
             if (op === 'contains') {
-              b[method](field, 'like', `%${value}%`);
+              this.applyContainsLike(b, method, field, value);
               return;
             }
 
@@ -1596,6 +1596,20 @@ export class SqlDriver implements IDataDriver {
     }
   }
 
+  /**
+   * Apply a `contains` substring match as a parameterized `LIKE '%…%'`, escaping
+   * the LIKE metacharacters `%` / `_` (and the escape char `\`) in the user value
+   * so they match literally instead of acting as wildcards — otherwise a value of
+   * `%` matches every row (a filter-bypass, P0). Binds an explicit `ESCAPE '\'`
+   * because SQLite does not honour a default escape character (MySQL/Postgres do,
+   * but the explicit clause is correct for all three).
+   */
+  private applyContainsLike(builder: any, method: string, field: string, value: unknown): void {
+    const escaped = String(value).replace(/[\\%_]/g, '\\$&');
+    const rawMethod = method.startsWith('or') ? 'orWhereRaw' : 'whereRaw';
+    builder[rawMethod]('?? LIKE ? ESCAPE ?', [field, `%${escaped}%`, '\\']);
+  }
+
   protected applyFilterCondition(builder: Knex.QueryBuilder, condition: any, logicalOp: 'and' | 'or' = 'and', tableHint?: string | null) {
     if (!condition || typeof condition !== 'object') return;
     const table = tableHint ?? this.tableNameForBuilder(builder);
@@ -1653,7 +1667,7 @@ export class SqlDriver implements IDataDriver {
               break;
             }
             case '$contains':
-              (builder as any)[method](field, 'like', `%${opValue}%`);
+              this.applyContainsLike(builder, method, field, opValue);
               break;
             default:
               (builder as any)[method](field, coerced);

packages/plugins/plugin-auth/src/auth-manager.test.ts

@@ -1143,4 +1143,39 @@ describe('AuthManager', () => {
       expect(als.getStore()).toBeUndefined();
     });
   });
+
+  describe('generateSecret – production secret guard (P0-1)', () => {
+    const ENV_KEYS = ['NODE_ENV', 'OS_AUTH_SECRET', 'AUTH_SECRET', 'BETTER_AUTH_SECRET'] as const;
+    let saved: Record<string, string | undefined>;
+
+    beforeEach(() => {
+      saved = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]]));
+      for (const k of ['OS_AUTH_SECRET', 'AUTH_SECRET', 'BETTER_AUTH_SECRET']) delete process.env[k];
+    });
+    afterEach(() => {
+      for (const k of ENV_KEYS) {
+        if (saved[k] === undefined) delete process.env[k];
+        else process.env[k] = saved[k];
+      }
+    });
+
+    it('throws (fails boot) in production when no secret is configured', () => {
+      process.env.NODE_ENV = 'production';
+      const m = new AuthManager({ baseUrl: 'http://localhost:3000' } as any);
+      expect(() => (m as any).generateSecret()).toThrow(/OS_AUTH_SECRET is required in production/);
+    });
+
+    it('falls back to an ephemeral dev secret outside production', () => {
+      process.env.NODE_ENV = 'development';
+      const m = new AuthManager({ baseUrl: 'http://localhost:3000' } as any);
+      expect((m as any).generateSecret()).toMatch(/^dev-secret-/);
+    });
+
+    it('uses OS_AUTH_SECRET when set, even in production', () => {
+      process.env.NODE_ENV = 'production';
+      process.env.OS_AUTH_SECRET = 'a-strong-production-secret-value';
+      const m = new AuthManager({ baseUrl: 'http://localhost:3000' } as any);
+      expect((m as any).generateSecret()).toBe('a-strong-production-secret-value');
+    });
+  });
 });

packages/plugins/plugin-auth/src/auth-manager.ts

@@ -1051,23 +1051,28 @@ export class AuthManager {
    */
   private generateSecret(): string {
     const envSecret = readEnvWithDeprecation('OS_AUTH_SECRET', ['AUTH_SECRET', 'BETTER_AUTH_SECRET']);
-
-    if (!envSecret) {
-      // In production, a secret MUST be provided
-      // For development/testing, we'll use a fallback but warn about it
-      const fallbackSecret = 'dev-secret-' + Date.now();
-
-      console.warn(
-        '⚠️  WARNING: No OS_AUTH_SECRET environment variable set! ' +
-        'Using a temporary development secret. ' +
-        'This is NOT secure for production use. ' +
-        'Please set OS_AUTH_SECRET in your environment variables.'
+    if (envSecret) return envSecret;
+
+    // No secret configured. In production this is FATAL: a predictable
+    // `dev-secret-<timestamp>` makes session tokens forgeable (session
+    // forgery). Refuse to boot rather than run insecurely.
+    if (process.env.NODE_ENV === 'production') {
+      throw new Error(
+        '[auth] OS_AUTH_SECRET is required in production but is not set. ' +
+        'Refusing to boot with a temporary development secret — session tokens ' +
+        'would be forgeable. Set OS_AUTH_SECRET to a strong random value.'
       );
-
-      return fallbackSecret;
     }
 
-    return envSecret;
+    // Development / test only: fall back to an ephemeral secret, loudly.
+    const fallbackSecret = 'dev-secret-' + Date.now();
+    console.warn(
+      '⚠️  WARNING: No OS_AUTH_SECRET environment variable set! ' +
+      'Using a temporary development secret. ' +
+      'This is NOT secure for production use. ' +
+      'Please set OS_AUTH_SECRET in your environment variables.'
+    );
+    return fallbackSecret;
   }
 
   /**

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

@@ -23,7 +23,7 @@ describe('SecurityPlugin', () => {
     const plugin = new SecurityPlugin();
     const manifestService = { register: vi.fn() };
     const ctx: any = {
-      logger: { info: vi.fn(), warn: vi.fn() },
+      logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
       registerService: vi.fn(),
       getService: vi.fn().mockImplementation((name: string) => {
         if (name === 'manifest') return manifestService;
@@ -41,7 +41,7 @@ describe('SecurityPlugin', () => {
     const plugin = new SecurityPlugin();
     const manifestService = { register: vi.fn() };
     const ctx: any = {
-      logger: { info: vi.fn(), warn: vi.fn() },
+      logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
       registerService: vi.fn(),
       getService: vi.fn().mockImplementation((name: string) => {
         if (name === 'manifest') return manifestService;
@@ -57,7 +57,7 @@ describe('SecurityPlugin', () => {
     const plugin = new SecurityPlugin();
     const manifestService = { register: vi.fn() };
     const ctx: any = {
-      logger: { info: vi.fn(), warn: vi.fn() },
+      logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
       registerService: vi.fn(),
       getService: vi.fn().mockImplementation((name: string) => {
         if (name === 'manifest') return manifestService;
@@ -74,7 +74,7 @@ describe('SecurityPlugin', () => {
     const registerMiddleware = vi.fn();
     const manifestService = { register: vi.fn() };
     const ctx: any = {
-      logger: { info: vi.fn(), warn: vi.fn() },
+      logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
       registerService: vi.fn(),
       getService: vi.fn().mockImplementation((name: string) => {
         if (name === 'manifest') return manifestService;
@@ -127,7 +127,7 @@ describe('SecurityPlugin', () => {
       services['org-scoping'] = { name: 'com.objectstack.org-scoping' };
     }
     const ctx: any = {
-      logger: { info: vi.fn(), warn: vi.fn() },
+      logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
       registerService: vi.fn(),
       getService: (name: string) => {
         if (!(name in services)) throw new Error(`service not registered: ${name}`);
@@ -344,6 +344,38 @@ describe('SecurityPlugin', () => {
     });
   });
 
+  it('fails CLOSED when permission resolution throws — denies, never bypasses (P0-2)', async () => {
+    const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
+    const harness = makeMiddlewareCtx({ permissionSets: [tenantPolicySet] });
+    await plugin.init(harness.ctx);
+    await plugin.start(harness.ctx);
+    // Simulate the permission/metadata subsystem failing mid-resolution.
+    (plugin as any).permissionEvaluator = {
+      resolvePermissionSets: async () => { throw new Error('metadata service unavailable'); },
+    };
+    const opCtx: any = {
+      object: 'task',
+      operation: 'find',
+      data: {},
+      context: { userId: 'u1', tenantId: 'org-1', roles: ['member'], permissions: [] },
+    };
+    // Resolution failed → the request must be DENIED, not waved through.
+    await expect(harness.run(opCtx)).rejects.toThrow(/permission subsystem unavailable/);
+    expect(harness.ctx.logger.error).toHaveBeenCalled();
+  });
+
+  it('a system operation still bypasses security regardless (P0-2)', async () => {
+    const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
+    const harness = makeMiddlewareCtx({ permissionSets: [tenantPolicySet] });
+    await plugin.init(harness.ctx);
+    await plugin.start(harness.ctx);
+    (plugin as any).permissionEvaluator = {
+      resolvePermissionSets: async () => { throw new Error('should not be called'); },
+    };
+    const opCtx: any = { object: 'task', operation: 'find', context: { isSystem: true } };
+    await expect(harness.run(opCtx)).resolves.toBeDefined(); // bypass short-circuits before resolution
+  });
+
   it('FLS write — multiple forbidden fields are all listed in the error', async () => {
     const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
     const harness = makeMiddlewareCtx({

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

@@ -307,9 +307,22 @@ export class SecurityPlugin implements Plugin {
           permissionSets = fallback;
         }
       } catch (e) {
-        // If metadata service is misconfigured, log and continue without permission checks
-        // rather than blocking all operations
-        return next();
+        // Fail CLOSED. A permission-resolution failure must DENY the request,
+        // never bypass the checks (that would let a degraded metadata service
+        // expose every tenant's data). System/bootstrap operations already
+        // short-circuited above (`opCtx.context?.isSystem`), so reaching here
+        // means an authenticated user request whose RBAC/RLS could not be
+        // resolved — deny it and alert.
+        ctx.logger.error(
+          `[security] permission resolution failed for operation '${opCtx.operation}' on ` +
+          `object '${opCtx.object}' (user ${opCtx.context?.userId ?? 'unknown'}) — ` +
+          `denying request (fail-closed)`,
+          e instanceof Error ? e : new Error(String(e)),
+        );
+        throw new PermissionDeniedError(
+          `[Security] Access denied: permission subsystem unavailable for ` +
+          `operation '${opCtx.operation}' on object '${opCtx.object}'`,
+        );
       }
 
       // 2. CRUD permission check