diff --git a/.changeset/21792-settings-audit-secret-keyed-digest.md b/.changeset/21792-settings-audit-secret-keyed-digest.md new file mode 100644 index 00000000000..cbe252e5b6c --- /dev/null +++ b/.changeset/21792-settings-audit-secret-keyed-digest.md @@ -0,0 +1,14 @@ +--- +'@objectstack/service-settings': patch +'@objectstack/spec': patch +--- + +The settings audit trail records a secret-valued setting (an encrypted key) with the crypto provider's keyed digest, never an unkeyed one (#21792). + +Clause-②: no + +- **Both ledgers.** The `sys_audit_log` `config_change` row (`valueDigest`, spelled ``) and the `sys_setting_audit` row (`new_hash`) now carry `ICryptoProvider.keyedDigest(value)` for a secret-valued setting. Before, they carried the unkeyed `digest` (`sha256:…`). This covers the `sys_secret` path and the legacy inline-adapter path. +- **Change detection still works.** The keyed digest is stable for equal values under one key, so the trail still shows whether a secret changed and whether it went back to an earlier value. Rotating the data key changes every later fingerprint. Rows written before this release keep their old `sha256:` value. +- **No keyed digest, no fingerprint.** When no crypto provider is wired (a host that builds `SettingsService` with only a `CryptoAdapter`), or the provider refuses a keyed digest, the audit rows record the write with no value fingerprint: `valueDigest` is `` and `new_hash` is null. The service logs this once per key at `warn`. The settings write itself is never refused for it. The adapter's own `digest` is no longer used for secrets. +- **Non-secret settings are unchanged.** They keep the adapter's `digest` of the canonical JSON. +- **Contract text (`@objectstack/spec`).** The `ICryptoProvider` docs for `digest` and `keyedDigest` now state the rule: a secret's audit fingerprint comes from `keyedDigest`, never from `digest`, and with no keyed digest the trail records none. No type, export or schema changes. diff --git a/packages/services/service-settings/src/settings-audit-secret-digest.test.ts b/packages/services/service-settings/src/settings-audit-secret-digest.test.ts new file mode 100644 index 00000000000..29fd4554140 --- /dev/null +++ b/packages/services/service-settings/src/settings-audit-secret-digest.test.ts @@ -0,0 +1,192 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The fingerprint the settings audit trail records for a write. + * + * Contract (`ICryptoProvider` in `@objectstack/spec/contracts`): a + * secret-valued setting (an `encryptedKeys` member) is recorded on BOTH + * ledgers — the generic `sys_audit_log` row (`SettingsAuditSink.valueDigest`) + * and the per-key `sys_setting_audit` row (`SettingsAuditWriter.newHash`) — + * with the provider's KEYED digest, never an unkeyed one; with no keyed digest + * available it records no fingerprint at all. Non-secret settings keep the + * adapter's unkeyed `digest` of the canonical JSON, unchanged. + */ + +import { createHash, randomBytes } from 'node:crypto'; +import { describe, expect, it, vi } from 'vitest'; +import type { ICryptoProvider } from '@objectstack/spec/contracts'; +import { SettingsService } from './settings-service.js'; +import type { CryptoAdapter } from './crypto-adapter.js'; +import { NoopCryptoAdapter } from './crypto-adapter.js'; +import { LocalCryptoProvider } from './local-crypto-provider.js'; +import { mailSettingsManifest } from './manifests/mail.manifest.js'; +import type { SettingsSecretStore } from './settings-service.types.js'; + +const KEYED_SHAPE = /^hmac-sha256:[0-9a-f]{64}$/; +const sha256 = (s: string) => 'sha256:' + createHash('sha256').update(s, 'utf8').digest('hex'); + +function memorySecretStore(): SettingsSecretStore { + const rows = new Map(); + return { + async insert(row) { rows.set(row.id, row); return { id: row.id }; }, + async get(id) { return rows.get(id) ?? null; }, + async update(id, patch) { rows.set(id, { ...rows.get(id), ...patch }); }, + }; +} + +/** A confidential stand-in for an injected KMS adapter (legacy inline path). */ +class ConfidentialTestAdapter implements CryptoAdapter { + readonly confidential = true; + async encrypt(plaintext: string): Promise { + return 'kms:' + Buffer.from(plaintext, 'utf8').toString('base64'); + } + async decrypt(ciphertext: string): Promise { + return Buffer.from(ciphertext.replace(/^kms:/, ''), 'base64').toString('utf8'); + } + digest(plaintext: string): string { + return sha256(plaintext); + } +} + +function boot(opts: { + cryptoProvider?: ICryptoProvider; + secretStore?: SettingsSecretStore; + crypto?: CryptoAdapter; + logger?: { error: (m: string) => void; warn?: (m: string) => void }; +}) { + const ledger: any[] = []; + const settingAudit: any[] = []; + const svc = new SettingsService({ + env: {}, + ...opts, + audit: { record: (e) => { ledger.push(e); } }, + auditWriter: { write: (e) => { settingAudit.push(e); } }, + }); + svc.registerManifest(mailSettingsManifest); + const forKey = (k: string) => ({ + ledger: ledger.filter((e) => e.key === k), + settingAudit: settingAudit.filter((e) => e.key === k), + }); + return { svc, forKey }; +} + +const writeSecret = (svc: SettingsService, value: string) => + svc.setMany('mail', { provider: 'resend', api_key: value, from_email: 'ops@example.com' }); + +describe('settings audit trail — secret-valued settings', () => { + it('records the provider keyed digest on both ledgers, not the unkeyed hash of the value', async () => { + const provider = new LocalCryptoProvider({ key: randomBytes(32) }); + const { svc, forKey } = boot({ cryptoProvider: provider, secretStore: memorySecretStore() }); + + await writeSecret(svc, 'pw-123456'); + + const { ledger, settingAudit } = forKey('api_key'); + const keyed = await provider.keyedDigest('pw-123456'); + expect(keyed).toMatch(KEYED_SHAPE); + + expect(settingAudit).toHaveLength(1); + expect(settingAudit[0]).toMatchObject({ encrypted: true, newHash: keyed }); + expect(settingAudit[0].newHash).not.toBe(provider.digest('pw-123456')); + expect(settingAudit[0].newHash).not.toBe(sha256('pw-123456')); + + expect(ledger).toHaveLength(1); + expect(ledger[0]).toMatchObject({ encrypted: true, valueDigest: '' }); + expect(JSON.stringify([ledger, settingAudit])).not.toContain(sha256('pw-123456')); + expect(JSON.stringify([ledger, settingAudit])).not.toContain('pw-123456'); + }); + + it('is stable for equal values and distinct for different ones', async () => { + const provider = new LocalCryptoProvider({ key: randomBytes(32) }); + const { svc, forKey } = boot({ cryptoProvider: provider, secretStore: memorySecretStore() }); + + await writeSecret(svc, 'same-value'); + await writeSecret(svc, 'other-value'); + await writeSecret(svc, 'same-value'); + + const hashes = forKey('api_key').settingAudit.map((e) => e.newHash); + expect(hashes).toHaveLength(3); + expect(hashes[0]).toBe(hashes[2]); + expect(hashes[1]).not.toBe(hashes[0]); + const digests = forKey('api_key').ledger.map((e) => e.valueDigest); + expect(digests[0]).toBe(digests[2]); + expect(digests[1]).not.toBe(digests[0]); + }); + + it('records no fingerprint on a reset', async () => { + const provider = new LocalCryptoProvider({ key: randomBytes(32) }); + const { svc, forKey } = boot({ cryptoProvider: provider, secretStore: memorySecretStore() }); + + await writeSecret(svc, 'pw-1'); + await svc.set('mail', 'api_key', null); + + const reset = forKey('api_key').settingAudit[1]; + expect(reset).toMatchObject({ action: 'reset', newHash: null }); + }); + + it('legacy inline-adapter path with a provider wired records the keyed digest, not the adapter digest', async () => { + const provider = new LocalCryptoProvider({ key: randomBytes(32) }); + // No secret store: the write takes the inline `crypto.encrypt` branch. + const { svc, forKey } = boot({ cryptoProvider: provider, crypto: new ConfidentialTestAdapter() }); + + await writeSecret(svc, 'pw-legacy'); + + const { ledger, settingAudit } = forKey('api_key'); + const keyed = await provider.keyedDigest('pw-legacy'); + expect(settingAudit[0].newHash).toBe(keyed); + expect(ledger[0].valueDigest).toBe(''); + expect(JSON.stringify([ledger, settingAudit])).not.toContain(sha256('pw-legacy')); + }); + + it('legacy inline-adapter path with no provider records no fingerprint and reports it once', async () => { + const warn = vi.fn(); + const { svc, forKey } = boot({ crypto: new ConfidentialTestAdapter(), logger: { error: vi.fn(), warn } }); + + await writeSecret(svc, 'pw-a'); + await writeSecret(svc, 'pw-b'); + + const { ledger, settingAudit } = forKey('api_key'); + expect(settingAudit.map((e) => e.newHash)).toEqual([null, null]); + expect(settingAudit.map((e) => e.action)).toEqual(['set', 'set']); + expect(ledger.map((e) => e.valueDigest)).toEqual(['', '']); + expect(JSON.stringify([ledger, settingAudit])).not.toContain(sha256('pw-a')); + // The write itself still landed. + expect((await svc.get('mail', 'api_key')).value).toBe('pw-b'); + expect(warn.mock.calls.filter(([m]) => String(m).includes('mail.api_key'))).toHaveLength(1); + }); + + it('a provider that refuses a keyed digest leaves the write intact and records no fingerprint', async () => { + const real = new LocalCryptoProvider({ key: randomBytes(32) }); + const refusing: ICryptoProvider = { + encrypt: real.encrypt.bind(real), + decrypt: real.decrypt.bind(real), + rotateKey: real.rotateKey.bind(real), + digest: real.digest.bind(real), + keyedDigest: async () => { throw new Error('no key material'); }, + }; + const { svc, forKey } = boot({ + cryptoProvider: refusing, + secretStore: memorySecretStore(), + logger: { error: vi.fn(), warn: vi.fn() }, + }); + + await writeSecret(svc, 'pw-x'); + + expect(forKey('api_key').settingAudit[0].newHash).toBeNull(); + expect(forKey('api_key').ledger[0].valueDigest).toBe(''); + expect((await svc.get('mail', 'api_key')).value).toBe('pw-x'); + }); +}); + +describe('settings audit trail — non-secret settings', () => { + it('keep the unkeyed adapter digest of the canonical JSON, unchanged', async () => { + const provider = new LocalCryptoProvider({ key: randomBytes(32) }); + const { svc, forKey } = boot({ cryptoProvider: provider, secretStore: memorySecretStore() }); + + await writeSecret(svc, 'pw-1'); + + const expected = new NoopCryptoAdapter().digest(JSON.stringify('ops@example.com')); + const { ledger, settingAudit } = forKey('from_email'); + expect(settingAudit[0]).toMatchObject({ encrypted: false, newHash: expected }); + expect(ledger[0]).toMatchObject({ encrypted: false, valueDigest: expected }); + }); +}); diff --git a/packages/services/service-settings/src/settings-service.test.ts b/packages/services/service-settings/src/settings-service.test.ts index bfe9863c9db..05478ebbfc4 100644 --- a/packages/services/service-settings/src/settings-service.test.ts +++ b/packages/services/service-settings/src/settings-service.test.ts @@ -184,7 +184,7 @@ describe('SettingsService — global scope', () => { }); describe('SettingsService — audit sink', () => { - it('records masked digest for encrypted values', async () => { + it('records an encrypted value without the adapter digest when no keyed digest is available', async () => { const events: any[] = []; const svc = new SettingsService({ env: {}, @@ -203,7 +203,9 @@ describe('SettingsService — audit sink', () => { const apiKeyEvent = events.find((e) => e.key === 'api_key'); expect(apiKeyEvent).toBeTruthy(); expect(apiKeyEvent.encrypted).toBe(true); - expect(apiKeyEvent.valueDigest).toMatch(/^'); }); }); @@ -2005,7 +2007,7 @@ describe('SettingsService — Phase 3 sys_secret + crypto provider + audit', () action: 'set', encrypted: true, }); - expect(auditRows[0].newHash).toMatch(/^sha256:/); + expect(auditRows[0].newHash).toMatch(/^hmac-sha256:[0-9a-f]{64}$/); expect(auditRows[0].newHash).not.toContain('super-secret-key'); }); diff --git a/packages/services/service-settings/src/settings-service.ts b/packages/services/service-settings/src/settings-service.ts index 97a48672d9a..a148f503e35 100644 --- a/packages/services/service-settings/src/settings-service.ts +++ b/packages/services/service-settings/src/settings-service.ts @@ -554,6 +554,11 @@ export class SettingsService { * otherwise repeat one operator-actionable line per attempt. */ private readonly reportedCryptoRefusals = new Set(); + /** + * `.` pairs whose missing secret audit fingerprint has + * already been reported. See {@link secretAuditDigest}. + */ + private readonly reportedUnkeyedAuditDigests = new Set(); /** * Namespaces whose pre-bind READ has already been reported (#10250). Deduped * for the same reason the two sets above are, and keyed by NAMESPACE rather @@ -1575,6 +1580,55 @@ export class SettingsService { throw err; } + /** + * The audit fingerprint of a secret-valued setting: the crypto provider's + * KEYED digest (`ICryptoProvider.keyedDigest`), never an unkeyed one. + * + * Both ledgers (`sys_audit_log` via {@link SettingsAuditSink} and + * `sys_setting_audit` via {@link SettingsAuditWriter}) are readable by + * people who must not be able to learn a secret. An unkeyed content hash + * lets any such reader confirm a guessed value offline — and for the short, + * low-entropy secrets settings carry (passwords, tokens of a known format), + * guessing is the attack. Under the provider's server-held key the + * fingerprint still answers "did this value change, and back to what it was + * before?" (stable per key for equal input) without answering "is it X?". + * + * When no keyed digest can be had — no provider is wired (the legacy + * inline-adapter path on a host that supplies none), or the provider + * rejects — the ledgers record NO fingerprint (`null`), reported once per + * key. Falling back to an unkeyed digest would be the exposure this method + * exists to close; failing the write would let a ledger veto a settings + * save, which neither audit seam is allowed to do. + */ + private async secretAuditDigest( + namespace: string, + key: string, + plain: string, + ): Promise { + const provider = this.cryptoProvider; + let reason: string; + if (provider && typeof provider.keyedDigest === 'function') { + try { + return await provider.keyedDigest(plain); + } catch (err: any) { + reason = `the crypto provider refused a keyed digest (${err?.message ?? err})`; + } + } else { + reason = 'no crypto provider with a keyed digest is wired'; + } + const dedupeAt = `${namespace}.${key}`; + if (!this.reportedUnkeyedAuditDigests.has(dedupeAt)) { + this.reportedUnkeyedAuditDigests.add(dedupeAt); + const message = + `[SettingsService] ${namespace}.${key}: the audit trail records this secret-valued ` + + `setting's write without a value fingerprint because ${reason}. Wire an ICryptoProvider ` + + `(SettingsServiceOptions.cryptoProvider) to record its keyed digest.`; + if (this.logger?.warn) this.logger.warn(message); + else console.warn(message); + } + return null; + } + /** Persist a single key. Throws SettingsLockedError when env-locked. */ async set( namespace: string, @@ -1669,7 +1723,9 @@ export class SettingsService { let storedValue: unknown | null = null; let storedEnc: string | null = null; - let digest = ''; + // The fingerprint both ledgers record. `null` only for a secret no + // keyed digest could be computed for — see `secretAuditDigest`. + let digest: string | null = null; if (!isNull) { if (isEncrypted) { @@ -1696,7 +1752,7 @@ export class SettingsService { ciphertext: handle.ciphertext, }); storedEnc = handle.id; - digest = this.cryptoProvider.digest(plain); + digest = await this.secretAuditDigest(namespace, key, plain); } else { // #8026 — the legacy inline-adapter path persists only through an // adapter that declares real confidentiality. The base64 default @@ -1706,7 +1762,9 @@ export class SettingsService { // branch and fall open. this.assertEncryptionAvailable(namespace, key); storedEnc = await this.crypto.encrypt(plain, { namespace, key }); - digest = this.crypto.digest(plain); + // Not `this.crypto.digest` — an adapter's digest is not keyed by + // contract, and a secret is never fingerprinted with an unkeyed one. + digest = await this.secretAuditDigest(namespace, key, plain); } } else { storedValue = rawValue; @@ -1750,7 +1808,9 @@ export class SettingsService { // an audit row invisible to RLS readers — see `SettingsAuditSink`. tenantId: ctx.tenantId, action: isNull ? 'reset' : 'set', - valueDigest: isEncrypted ? '' : digest, + valueDigest: isEncrypted + ? digest === null ? '' : '' + : digest ?? '', encrypted: isEncrypted, requestId: ctx.requestId, }); @@ -2431,7 +2491,7 @@ export class SettingsService { * * Nothing else can reference the handle: ids are minted per `encrypt()` call, * `sys_setting.value_enc` is the only column that holds one, and the audit - * trail records digests (`sha256:…`) rather than handles — so it stays + * trail records digests (`hmac-sha256:…`) rather than handles — so it stays * readable after the ciphertext is gone. * * **Best-effort, and deliberately after the repoint.** The write has already diff --git a/packages/services/service-settings/src/sys-secret-orphan-report.test.ts b/packages/services/service-settings/src/sys-secret-orphan-report.test.ts index b9acdf691a7..3866ff0155f 100644 --- a/packages/services/service-settings/src/sys-secret-orphan-report.test.ts +++ b/packages/services/service-settings/src/sys-secret-orphan-report.test.ts @@ -445,9 +445,10 @@ describe('#8103 reachability fact 3 — the audit trail records digests, not han expect(serialised).not.toContain('tok-1'); expect(serialised).not.toContain('tok-2'); - // What IS recorded is a content digest. + // What IS recorded is a digest of the content — the provider's keyed one, + // since the value is a secret. const encryptedEntry = auditEntries.find((e) => e.encrypted === true)!; - expect(String(encryptedEntry.newHash)).toMatch(/^sha256:[0-9a-f]{64}$/); + expect(String(encryptedEntry.newHash)).toMatch(/^hmac-sha256:[0-9a-f]{64}$/); // The other edge of the same fact: because no handle is ever recorded, the // audit trail cannot tell an operator which handles once existed — a row diff --git a/packages/services/service-settings/src/sys-secret-orphan-report.ts b/packages/services/service-settings/src/sys-secret-orphan-report.ts index 1418ec9ecf4..a6e0810639b 100644 --- a/packages/services/service-settings/src/sys-secret-orphan-report.ts +++ b/packages/services/service-settings/src/sys-secret-orphan-report.ts @@ -36,7 +36,8 @@ * enumerable — it is every `secret`-typed field on every registered object, * including tenant-authored ones. * - ✅ The audit trail records digests, not handles (`old_hash` / `new_hash` - * are content digests; `SettingsService` passes the provider's `digest()`), + * are content digests; `SettingsService` passes the provider's + * `keyedDigest()` for secret-valued keys, `digest()` otherwise), * so audit stays readable after a ciphertext is destroyed — and, the other * way round, audit can never be used to reconstruct which handles existed. * diff --git a/packages/spec/src/contracts/crypto-provider.ts b/packages/spec/src/contracts/crypto-provider.ts index 26c772619b3..12f1287a196 100644 --- a/packages/spec/src/contracts/crypto-provider.ts +++ b/packages/spec/src/contracts/crypto-provider.ts @@ -220,14 +220,18 @@ export interface ICryptoProvider { rotateKey(handle: CryptoHandle, ctx: CryptoContext): Promise; /** - * Stable hex digest of `plain` used for audit logging. SHOULD NOT - * reveal the plaintext (use HMAC or SHA-256 of canonical JSON). - * Same hash for same input enables operators to detect duplicate - * writes without exposing secrets. + * Stable hex digest of `plain` used for audit logging of NON-secret + * values. SHOULD NOT reveal the plaintext (use HMAC or SHA-256 of + * canonical JSON). Same hash for same input enables operators to detect + * duplicate writes. * * Not keyed by contract: plain SHA-256 satisfies it, so anyone holding a - * candidate input can recompute it. A value that must not be computable - * without the provider's key comes from {@link ICryptoProvider.keyedDigest}. + * candidate input can recompute it — which is why it is never the audit + * fingerprint of a secret-valued input (a password, an encrypted settings + * key): anyone who can read the audit trail could confirm a guess about + * the secret offline. A secret's audit fingerprint, and any other value + * that must not be computable without the provider's key, comes from + * {@link ICryptoProvider.keyedDigest}. */ digest(plain: string): string; @@ -246,10 +250,16 @@ export interface ICryptoProvider { * output in every process and on every node holding that key, so a * value one node hands out compares equal when a caller echoes it to * another. Replacing the key changes every output. - * 3. **Not a substitute for {@link ICryptoProvider.digest}.** `digest` - * keeps its own contract and the stability the audit trail relies on; - * nothing that records or compares audit digests moves to this method, - * and this method is not an audit fingerprint. + * 3. **The audit fingerprint of a secret.** An audit trail records a + * secret-valued input (a password, an encrypted settings key) with + * this method, ⛔ never with {@link ICryptoProvider.digest}: by + * requirement 2 equal secrets still record equal fingerprints under + * one key, so "this value changed" and "it went back to an earlier + * value" stay readable, while a reader of the trail cannot confirm a + * guess about the secret offline. If no keyed digest can be had, the + * audit trail records no fingerprint for the secret — it ⛔ never + * falls back to `digest`. Non-secret inputs keep `digest` and its + * stability, unchanged. * * Output: `hmac-sha256:` followed by the 64 lowercase hex characters of an * HMAC-SHA-256 — 76 characters drawn from `[0-9a-z:-]`. That one token