Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/objectql/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export {
parseSecretRef,
collectSecretFields,
collectMaskedReadFields,
collectCredentialFields,
} from './secret-fields.js';

// Utilities
Expand Down
42 changes: 42 additions & 0 deletions packages/objectql/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type { ICryptoProvider, CryptoHandle } from '@objectstack/spec/contracts'
import {
collectSecretFields,
collectMaskedReadFields,
collectCredentialFields,
makeSecretRef,
parseSecretRef,
isSecretRef,
Expand Down Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions packages/objectql/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export {
parseSecretRef,
collectSecretFields,
collectMaskedReadFields,
collectCredentialFields,
} from './secret-fields.js';

// Export Utilities
Expand Down
48 changes: 48 additions & 0 deletions packages/objectql/src/secret-fields.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
});
});
25 changes: 25 additions & 0 deletions packages/objectql/src/secret-fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}