feat(objectql): reject aggregations over secret/password fields (#3171) (#3187)

aggregate() now refuses to reference a secret/password field (measure or groupBy). find/findOne mask credential fields so plaintext never leaves the engine, but aggregate had no equivalent guard — a GROUP BY / MIN / MAX / array_agg would surface the stored value, and post-hoc masking would corrupt group keys. The gate keys off a new unconditional collectCredentialFields collector (secret OR password, ignoring managedBy — aggregating a credential is never legitimate, even on a better-auth object). Fail-closed. Recorded as the follow-up in ADR-0100.

Closes #3171.

Author: os-zhuang

packages/objectql/src/core.ts

@@ -76,6 +76,7 @@ export {
   parseSecretRef,
   collectSecretFields,
   collectMaskedReadFields,
+  collectCredentialFields,
 } from './secret-fields.js';
 
 // Utilities

packages/objectql/src/engine.ts

@@ -20,6 +20,7 @@ import type { ICryptoProvider, CryptoHandle } from '@objectstack/spec/contracts'
 import {
   collectSecretFields,
   collectMaskedReadFields,
+  collectCredentialFields,
   makeSecretRef,
   parseSecretRef,
   isSecretRef,
@@ -2919,8 +2920,49 @@ export class ObjectQL implements IDataEngine {
      return opCtx.result as number;
   }
 
+  /**
+   * Fail-closed guard (ADR-0100 / #3171): refuse to aggregate over a credential
+   * field. `secret`/`password` values are masked on the generic read path so
+   * plaintext never leaves the engine, but `aggregate()` has no equivalent mask
+   * — a GROUP BY / MIN / MAX / array_agg over such a column would surface the
+   * stored `secret:<id>` ref or the password value, and post-hoc masking would
+   * corrupt group keys. So we reject instead. The check is unconditional
+   * (ignores `managedBy`): aggregating a credential is never legitimate, even on
+   * a better-auth object, where it would be an inference oracle over hashes.
+   *
+   * Only the two output-bearing positions on `EngineAggregateOptions` carry
+   * field names: `aggregations[].field` (skip COUNT(*) — undefined or '*') and
+   * `groupBy[]` (a string, or a `{ field }` bucket object).
+   */
+  private rejectCredentialAggregation(object: string, query: EngineAggregateOptions): void {
+    const schema = this._registry.getObject(object);
+    const credentialFields = collectCredentialFields(schema);
+    if (credentialFields.length === 0) return;
+
+    const referenced = new Set<string>();
+    for (const agg of query?.aggregations ?? []) {
+      const field = (agg as { field?: string })?.field;
+      if (field && field !== '*') referenced.add(field);
+    }
+    for (const g of (query?.groupBy as unknown[]) ?? []) {
+      const field = typeof g === 'string' ? g : (g as { field?: string })?.field;
+      if (field) referenced.add(field);
+    }
+
+    const hit = credentialFields.filter((f) => referenced.has(f));
+    if (hit.length > 0) {
+      throw new Error(
+        `Cannot aggregate credential field(s) ${hit.map((f) => `"${object}.${f}"`).join(', ')}: `
+          + 'secret/password fields are masked on read so plaintext never leaves the engine, and '
+          + 'aggregating them (group-by, min/max, array_agg, …) would surface the stored value. '
+          + 'Refusing (fail-closed) — see ADR-0100 / #3171.',
+      );
+    }
+  }
+
   async aggregate(object: string, query: EngineAggregateOptions, options?: EngineReadOptions): Promise<any[]> {
       object = this.resolveObjectName(object);
+      this.rejectCredentialAggregation(object, query);
       const driver = this.getDriver(object);
       this.logger.debug(`Aggregate on ${object} using ${driver.name}`, query);
 

packages/objectql/src/index.ts

@@ -103,6 +103,7 @@ export {
   parseSecretRef,
   collectSecretFields,
   collectMaskedReadFields,
+  collectCredentialFields,
 } from './secret-fields.js';
 
 // Export Utilities

packages/objectql/src/secret-fields.test.ts

@@ -320,3 +320,51 @@ describe('objectql password-field masking (ADR-0100)', () => {
     expect(viaOne.password).toBe('hashed-by-auth');
   });
 });
+
+describe('objectql aggregate() rejects credential fields (ADR-0100 / #3171)', () => {
+  it('rejects a `secret` field used as an aggregation measure', async () => {
+    const { engine } = await buildEngine(true);
+    await expect(
+      engine.aggregate('ext_datasource', { aggregations: [{ function: 'max', field: 'db_password', alias: 'x' }] as any }),
+    ).rejects.toThrow(/credential field.*db_password/i);
+  });
+
+  it('rejects a `secret` field used as a string groupBy dimension', async () => {
+    const { engine } = await buildEngine(true);
+    await expect(
+      engine.aggregate('ext_datasource', { aggregations: [{ function: 'count', alias: 'n' }], groupBy: ['db_password'] } as any),
+    ).rejects.toThrow(/db_password/);
+  });
+
+  it('rejects a `secret` field used as a structured {field} groupBy bucket', async () => {
+    const { engine } = await buildEngine(true);
+    await expect(
+      engine.aggregate('ext_datasource', { aggregations: [{ function: 'count', alias: 'n' }], groupBy: [{ field: 'db_password' }] } as any),
+    ).rejects.toThrow(/db_password/);
+  });
+
+  it('does NOT reject when the credential field is not referenced (no false positive)', async () => {
+    const { engine } = await buildEngine(true);
+    await engine.insert('ext_datasource', { name: 'pg', db_password: 's3cr3t' });
+    // COUNT(*) touches no credential column — the object merely *has* one.
+    await expect(
+      engine.aggregate('ext_datasource', { aggregations: [{ function: 'count', alias: 'n' }] } as any),
+    ).resolves.toBeDefined();
+  });
+
+  it('rejects a generic `password` field used as a groupBy dimension', async () => {
+    const { engine } = await buildPasswordEngine();
+    await expect(
+      engine.aggregate('device', { aggregations: [{ function: 'count', alias: 'n' }], groupBy: ['admin_password'] } as any),
+    ).rejects.toThrow(/admin_password/);
+  });
+
+  it('rejects even on a better-auth object — read-masking is exempt there, aggregation is NOT', async () => {
+    const { engine } = await buildPasswordEngine();
+    // authy_user is managedBy:'better-auth' (password NOT masked on read), but
+    // aggregating the credential is still refused (unconditional collector).
+    await expect(
+      engine.aggregate('authy_user', { aggregations: [{ function: 'max', field: 'password', alias: 'x' }] as any }),
+    ).rejects.toThrow(/password/);
+  });
+});

packages/objectql/src/secret-fields.ts

@@ -97,3 +97,28 @@ export function collectMaskedReadFields(schema: ServiceObject | undefined | null
   }
   return out;
 }
+
+/**
+ * Collect the names of every credential-bearing field on an object — `secret`
+ * OR `password` — **unconditionally**, ignoring `managedBy`.
+ *
+ * This differs from {@link collectMaskedReadFields} on purpose. Read-masking
+ * exempts `password` on `managedBy: 'better-auth'` objects so login reads still
+ * see the stored value; but *aggregating* a credential field must never be
+ * allowed, even on a better-auth object — a GROUP BY / MIN / MAX over a password
+ * column is an inference oracle regardless of who owns the table. So the
+ * aggregate-rejection gate keys off this stricter, exemption-free collector,
+ * keeping the two concerns independent (they must not drift). See ADR-0100 / #3171.
+ *
+ * Returns an empty array when the schema has no fields or no credential fields,
+ * so callers can fast-path on `length === 0`.
+ */
+export function collectCredentialFields(schema: ServiceObject | undefined | null): string[] {
+  const fields = (schema as any)?.fields as Record<string, { type?: string }> | undefined;
+  if (!fields) return [];
+  const out: string[] = [];
+  for (const [name, def] of Object.entries(fields)) {
+    if (def && (def.type === 'secret' || def.type === 'password')) out.push(name);
+  }
+  return out;
+}