diff --git a/packages/objectql/src/core.ts b/packages/objectql/src/core.ts index 2a0381bd37..85bc050f60 100644 --- a/packages/objectql/src/core.ts +++ b/packages/objectql/src/core.ts @@ -76,6 +76,7 @@ export { parseSecretRef, collectSecretFields, collectMaskedReadFields, + collectCredentialFields, } from './secret-fields.js'; // Utilities diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 6a281fe44a..9f3df91615 100644 --- a/packages/objectql/src/engine.ts +++ b/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:` 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(); + 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 { object = this.resolveObjectName(object); + this.rejectCredentialAggregation(object, query); const driver = this.getDriver(object); this.logger.debug(`Aggregate on ${object} using ${driver.name}`, query); diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 5df6fbd5d3..abb9520d8a 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -103,6 +103,7 @@ export { parseSecretRef, collectSecretFields, collectMaskedReadFields, + collectCredentialFields, } from './secret-fields.js'; // Export Utilities diff --git a/packages/objectql/src/secret-fields.test.ts b/packages/objectql/src/secret-fields.test.ts index 0609f13030..b41782b620 100644 --- a/packages/objectql/src/secret-fields.test.ts +++ b/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/); + }); +}); diff --git a/packages/objectql/src/secret-fields.ts b/packages/objectql/src/secret-fields.ts index 4a97461dee..15f743e254 100644 --- a/packages/objectql/src/secret-fields.ts +++ b/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 | 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; +}