fix(service-settings): the settings audit trail records secret-valued settings with the keyed digest

.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 `<encrypted:hmac-sha256:…>`) 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 `<encrypted>` 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.

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<string, any>();
+  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<string> {
+    return 'kms:' + Buffer.from(plaintext, 'utf8').toString('base64');
+  }
+  async decrypt(ciphertext: string): Promise<string> {
+    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: '<encrypted:' + keyed + '>' });
+    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('<encrypted:' + keyed + '>');
+    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(['<encrypted>', '<encrypted>']);
+    expect(JSON.stringify([ledger, settingAudit])).not.toContain(sha256('pw-a'));
+    // The write itself still landed.
+    expect((await svc.get<string>('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('<encrypted>');
+    expect((await svc.get<string>('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 });
+  });
+});

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(/^<encrypted:fnv32:/);
+    // No crypto provider is wired, so there is no keyed digest; the adapter's
+    // unkeyed digest is never recorded for a secret (crypto-provider contract).
+    expect(apiKeyEvent.valueDigest).toBe('<encrypted>');
   });
 });
 
@@ -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');
   });
 

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<string>();
+  /**
+   * `<namespace>.<key>` pairs whose missing secret audit fingerprint has
+   * already been reported. See {@link secretAuditDigest}.
+   */
+  private readonly reportedUnkeyedAuditDigests = new Set<string>();
   /**
    * 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<string | null> {
+    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 ? '<encrypted:' + digest + '>' : digest,
+            valueDigest: isEncrypted
+              ? digest === null ? '<encrypted>' : '<encrypted:' + digest + '>'
+              : 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

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

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.
  *