From 57af6f7d7d23e74035860130c01322a8da173434 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 05:48:50 +0000 Subject: [PATCH 1/3] fix(service-settings): settings audit records the previous value's digest and the declared SHA-256 old_hash was hard-coded null and a non-secret new_hash was the adapter's FNV-1a 32 while both audit columns declare SHA-256 of the canonical value. The previous value comes off the identity probe upsertRow already makes. Claude-Session: https://claude.ai/code/session_01FEkQ5ojTc5Fbj8xxXSfo1K Co-authored-by: Claude --- .../service-settings/src/crypto-adapter.ts | 33 +++++++++++-- .../service-settings/src/settings-service.ts | 48 +++++++++++++++---- 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/packages/services/service-settings/src/crypto-adapter.ts b/packages/services/service-settings/src/crypto-adapter.ts index d36a7b2582a..c182c7c2b70 100644 --- a/packages/services/service-settings/src/crypto-adapter.ts +++ b/packages/services/service-settings/src/crypto-adapter.ts @@ -1,5 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +import { createHash } from 'node:crypto'; + /** * Pluggable adapter for at-rest encryption of `Specifier.encrypted: true` * values. The default {@link NoopCryptoAdapter} provides a transparent @@ -14,8 +16,14 @@ export interface CryptoAdapter { /** Returns the plaintext used by the resolver. */ decrypt(ciphertext: string, ctx: { namespace: string; key: string }): Promise; /** - * Stable, short, non-reversible digest used for audit-log entries so - * operators can correlate value changes without leaking secrets. + * Stable, short, non-reversible digest of a plaintext. + * + * Not read by the settings audit trail since #26465: `sys_setting_audit` + * and `sys_platform_setting_audit` declare SHA-256 for `old_hash` / + * `new_hash`, so the non-secret fingerprint is {@link settingAuditDigest} + * whatever adapter is injected, and a secret is fingerprinted only by the + * crypto provider's keyed digest. Kept on the interface so existing + * adapters still satisfy it. */ digest(plaintext: string): string; /** @@ -79,7 +87,8 @@ export class NoopCryptoAdapter implements CryptoAdapter { return Buffer.from(ciphertext.slice(4), 'base64').toString('utf8'); } digest(plaintext: string): string { - // FNV-1a 32-bit — short, stable, non-cryptographic. Audit-only. + // FNV-1a 32-bit — short, stable, non-cryptographic. The settings audit + // no longer records it (#26465, see `settingAuditDigest`). let h = 0x811c9dc5; for (let i = 0; i < plaintext.length; i++) { h ^= plaintext.charCodeAt(i); @@ -89,6 +98,24 @@ export class NoopCryptoAdapter implements CryptoAdapter { } } +/** + * The fingerprint the settings audit trail records for a NON-secret value + * (#26465): `sha256:` followed by the hex SHA-256 of the value's canonical + * JSON, which is the algorithm `old_hash` / `new_hash` declare on + * `sys_setting_audit` and `sys_platform_setting_audit`. + * + * Computed here rather than through {@link CryptoAdapter.digest}, so an + * injected adapter cannot change the column's format. The prefix keeps the + * value self-describing beside the rows written before it (`fnv32:` from the + * old adapter digest) and the keyed secret fingerprints (`hmac-sha256:`), so a + * reader comparing two rows can tell a different algorithm from a different + * value. Never used for a secret: an unkeyed hash of a short secret lets any + * reader of the audit rows confirm a guess offline. + */ +export function settingAuditDigest(canonicalJson: string): string { + return 'sha256:' + createHash('sha256').update(canonicalJson, 'utf8').digest('hex'); +} + /** * Is this adapter allowed to hold a declared-secret value at rest (#8026)? * diff --git a/packages/services/service-settings/src/settings-service.ts b/packages/services/service-settings/src/settings-service.ts index 82173a167cf..a50d89bdec2 100644 --- a/packages/services/service-settings/src/settings-service.ts +++ b/packages/services/service-settings/src/settings-service.ts @@ -16,6 +16,7 @@ import { type CryptoAdapter, NoopCryptoAdapter, providesConfidentiality, + settingAuditDigest, } from './crypto-adapter.js'; import { type SettingsActionHandler, @@ -2425,7 +2426,8 @@ export class SettingsService { } } else { storedValue = rawValue; - digest = this.crypto.digest(stableStringify(rawValue)); + // The declared SHA-256 (#26465), not the adapter's digest. + digest = settingAuditDigest(stableStringify(rawValue)); } } @@ -2443,7 +2445,7 @@ export class SettingsService { updated_at: new Date().toISOString(), updated_by: ctx.userId ?? null, }; - const previousEnc = await this.upsertRow(rowForKey); + const { previousEnc, previous } = await this.upsertRow(rowForKey); // Destroy the ciphertext the row USED to name (#8030) — but only once a // re-read confirms the row really stopped naming it (#8262); "a new @@ -2495,7 +2497,10 @@ export class SettingsService { // [ADR-0131 D3/D9] The organization whose value changed — the same // owner the settings row carries; none for a global value. ...(organizationId ? { organizationId } : {}), - oldHash: null, + // #26465: the previous value's digest, off the row the upsert + // already read. A secret write keeps no old fingerprint (its keyed + // digest path is unchanged), see `previousAuditDigest`. + oldHash: isEncrypted ? null : SettingsService.previousAuditDigest(previous), newHash: isNull ? null : digest, encrypted: isEncrypted, requestId: ctx.requestId, @@ -3255,7 +3260,30 @@ export class SettingsService { return typeof value === 'string' && value !== '' ? value : null; } - private async upsertRow(row: SettingsRow): Promise { + /** + * The `old_hash` of a non-secret write (#26465): the declared SHA-256 of + * the value the row held before it, or `null` when it held none (no row, or + * a reset row whose value is `null`). + * + * A row that held a SECRET also answers `null`: its value is ciphertext or a + * handle, and an unkeyed hash of the decrypted secret is exactly the + * fingerprint the audit trail must never carry (see `secretAuditDigest`). + */ + private static previousAuditDigest(previous: SettingsRow | null): string | null { + if (!previous || previous.encrypted) return null; + if (previous.value === null || typeof previous.value === 'undefined') return null; + return settingAuditDigest(stableStringify(previous.value)); + } + + /** + * Insert or update the row at its identity, and report what that identity + * held BEFORE: `previousEnc` for the secret reaper (#8030), `previous` for + * the audit's `old_hash` (#26465). Both come off the existence probe the + * upsert makes anyway, so a write with no previous row costs no extra read. + */ + private async upsertRow( + row: SettingsRow, + ): Promise<{ previousEnc: string | null; previous: SettingsRow | null }> { if (this.engine) { const { object, where, data } = this.rowIdentity(row); // All three engine calls carry the explicit system opt-in @@ -3268,13 +3296,13 @@ export class SettingsService { context: SETTINGS_SYSTEM_CONTEXT, }); if (existing[0]) { - const previousEnc = (existing[0] as { value_enc?: unknown }).value_enc; + const previous = existing[0] as SettingsRow; await this.engine.update(object, { where, data, context: SETTINGS_SYSTEM_CONTEXT, }); - return SettingsService.handleOf(previousEnc); + return { previousEnc: SettingsService.handleOf(previous.value_enc), previous }; } // Under the opt-in the engine no longer checks that a user-scope row's // `user_id` names a user, so the service keeps that refusal before the @@ -3294,16 +3322,16 @@ export class SettingsService { ); } await this.engine.insert(object, data, { context: SETTINGS_SYSTEM_CONTEXT }); - return null; + return { previousEnc: null, previous: null }; } const idx = this.memoryIndexOf(row); if (idx >= 0) { - const previousEnc = this.memory[idx].value_enc; + const previous = this.memory[idx]; this.memory[idx] = row; - return SettingsService.handleOf(previousEnc); + return { previousEnc: SettingsService.handleOf(previous.value_enc), previous }; } this.memory.push(row); - return null; + return { previousEnc: null, previous: null }; } /** From 25c0629400031e62d8cd099438ccbaf48464dcdf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 05:55:10 +0000 Subject: [PATCH 2/3] test(service-settings): pin old_hash/new_hash through the settings REST door Claude-Session: https://claude.ai/code/session_01FEkQ5ojTc5Fbj8xxXSfo1K Co-authored-by: Claude --- .../src/settings-audit-old-hash.test.ts | 351 ++++++++++++++++++ .../src/settings-audit-secret-digest.test.ts | 10 +- .../service-settings/src/settings-service.ts | 7 +- 3 files changed, 362 insertions(+), 6 deletions(-) create mode 100644 packages/services/service-settings/src/settings-audit-old-hash.test.ts diff --git a/packages/services/service-settings/src/settings-audit-old-hash.test.ts b/packages/services/service-settings/src/settings-audit-old-hash.test.ts new file mode 100644 index 00000000000..857e84036c3 --- /dev/null +++ b/packages/services/service-settings/src/settings-audit-old-hash.test.ts @@ -0,0 +1,351 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#26465] The settings audit trail's `old_hash` / `new_hash`, as the columns + * declare them (ADR-0049 enforce arm). + * + * `sys_setting_audit` and its global-rung twin `sys_platform_setting_audit` + * declare `old_hash` as the SHA-256 of the previous value's canonical JSON, + * null only when it was unset, and `new_hash` as the SHA-256 of the new value, + * null on a reset. The write path used to pass `oldHash: null` on every write + * and record the adapter's FNV-1a 32 (`fnv32:…`) as the new hash. + * + * Every case drives `PUT /api/settings/:namespace` through the real route set, + * over a real `ObjectQL` with the real platform objects and the plugin's own + * audit writer, and reads the audit rows back out of the driver's store. The + * expected digests are computed here with `node:crypto`, independently of the + * implementation. + * + * A secret-valued key is the CONTROL: its keyed fingerprint path + * (`hmac-sha256:…`, `secretAuditDigest`) is untouched, so its rows keep a null + * `old_hash` and a keyed `new_hash`. + */ + +import { createHash } from 'node:crypto'; +import { describe, expect, it } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { + SysPlatformSetting, + SysPlatformSettingAudit, + SysSecret, + SysSetting, + SysSettingAudit, +} from '@objectstack/platform-objects/system'; +import type { SettingsManifest } from '@objectstack/spec/system'; +import type { IHttpRequest, IHttpResponse, IHttpServer, RouteHandler } from '@objectstack/spec/contracts'; +import { SettingsService } from './settings-service.js'; +import { + SettingsServicePlugin, + buildSettingAuditWriter, + wrapEngineAsSettingsEngine, +} from './settings-service-plugin.js'; +import { registerSettingsRoutes } from './settings-routes.js'; +import { LocalCryptoProvider } from './local-crypto-provider.js'; +import { withEngineRegistry } from './__tests__/settings-organization.testkit.js'; + +const OWNER_PACKAGE = 'com.objectstack.test.settings-audit-old-hash'; + +const SHA256_SHAPE = /^sha256:[0-9a-f]{64}$/; +const KEYED_SHAPE = /^hmac-sha256:[0-9a-f]{64}$/; +/** The declared digest, computed independently: SHA-256 of the canonical JSON. */ +const sha256Of = (value: unknown) => + 'sha256:' + createHash('sha256').update(JSON.stringify(value), 'utf8').digest('hex'); + +/** A tenant-scope namespace: its audit rows land in `sys_setting_audit`. */ +const TENANT_MANIFEST: SettingsManifest = { + namespace: 'hash_tenant', + version: 1, + label: 'Hash probe (tenant)', + scope: 'tenant', + readPermission: 'setup.access', + writePermission: 'setup.write', + specifiers: [{ type: 'text', key: 'timezone', label: 'Timezone', required: false }], +}; + +/** A global-scope namespace: its audit rows land in `sys_platform_setting_audit`. */ +const GLOBAL_MANIFEST: SettingsManifest = { + namespace: 'hash_global', + version: 1, + label: 'Hash probe (global)', + scope: 'global', + readPermission: 'manage_platform_settings', + writePermission: 'manage_platform_settings', + specifiers: [ + { type: 'text', key: 'host', label: 'Host', required: false }, + { type: 'password', key: 'api_key', label: 'API key', required: false, encrypted: true }, + ], +}; + +/** A user-scope namespace: one row per user, so the previous value is per-user. */ +const USER_MANIFEST: SettingsManifest = { + namespace: 'hash_user', + version: 1, + label: 'Hash probe (user)', + scope: 'user', + readPermission: 'setup.access', + writePermission: 'setup.write', + specifiers: [{ type: 'text', key: 'theme', label: 'Theme', required: false }], +}; + +/** A driver over plain Maps — the verbs the settings path reaches through `ObjectQL`. */ +function makeMemoryDriver() { + const store = new Map>>(); + let nextId = 0; + const copy = (r: Record) => ({ ...r }); + const rowsOf = (object: string) => { + let s = store.get(object); + if (!s) { s = new Map(); store.set(object, s); } + return s; + }; + // `$or` is the one combinator the settings reads emit; anything else refuses, + // so a predicate this double cannot express never reads as "no such row". + const matches = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]) => { + if (k === '$or') return (v as any[]).some((b) => matches(row, b)); + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + return (row[k] ?? null) === (v ?? null); + }); + }; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {} as any, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, ast: any) { + const hits = [...rowsOf(object).values()].filter((r) => matches(r, ast?.where)); + const page = typeof ast?.limit === 'number' ? hits.slice(0, ast.limit) : hits; + return page.map(copy); + }, + async findOne(object: string, ast: any) { + for (const r of rowsOf(object).values()) if (matches(r, ast?.where)) return copy(r); + return null; + }, + async create(object: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `row_${nextId}`; + const row = { ...data, id }; + rowsOf(object).set(id, row); + return copy(row); + }, + async update(object: string, id: string, data: Record) { + const s = rowsOf(object); + const cur = s.get(id); + if (!cur) return null; + const next = { ...cur, ...data, id }; + s.set(id, next); + return copy(next); + }, + async upsert(object: string, data: Record) { + const id = data.id as string | undefined; + return id && rowsOf(object).has(id) ? this.update(object, id, data) : this.create(object, data); + }, + async delete(object: string, id: string) { return rowsOf(object).delete(id); }, + async count(object: string, ast: any) { return (await this.find(object, ast)).length; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async updateMany(object: string, ast: any, data: Record) { + const rows = await this.find(object, ast); + const s = rowsOf(object); + for (const r of rows) s.set(r.id as string, { ...s.get(r.id as string), ...data, id: r.id }); + return rows.length; + }, + async deleteMany(object: string, ast: any) { + const rows = await this.find(object, ast); + for (const r of rows) rowsOf(object).delete(r.id as string); + return rows.length; + }, + async syncSchema() {}, async dropTable() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, rowsOf }; +} + +/** Minimal `IHttpServer` that keeps the handlers so a route can be invoked. */ +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(p: string, h: RouteHandler) { this.add('GET', p, h); return this as any; } + post(p: string, h: RouteHandler) { this.add('POST', p, h); return this as any; } + put(p: string, h: RouteHandler) { this.add('PUT', p, h); return this as any; } + delete(p: string, h: RouteHandler) { this.add('DELETE', p, h); return this as any; } + patch(p: string, h: RouteHandler) { this.add('PATCH', p, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +/** The pieces the running server binds: real engine, real objects, the plugin's audit writer, real routes. */ +async function boot() { + const engine = new ObjectQL(); + const { driver, rowsOf } = makeMemoryDriver(); + engine.registerDriver(driver, true); + await engine.init(); + for (const o of [SysSetting, SysPlatformSetting, SysSecret, SysSettingAudit, SysPlatformSettingAudit]) { + engine.registry.registerObject(o as any, OWNER_PACKAGE); + } + const svc = new SettingsService({ + env: {}, + engine: withEngineRegistry(wrapEngineAsSettingsEngine(engine as any), engine), + cryptoProvider: new LocalCryptoProvider(), + secretStore: (new SettingsServicePlugin() as any).buildSecretStore(engine), + auditWriter: buildSettingAuditWriter(engine as any), + }); + for (const m of [TENANT_MANIFEST, GLOBAL_MANIFEST, USER_MANIFEST]) svc.registerManifest(m); + + // The caller each request resolves to; a case switches it to write as another user. + let userId = 'usr_admin'; + const http = new MockHttp(); + registerSettingsRoutes(http, svc, { + contextFromRequest: () => ({ + enforced: true, + permissions: ['setup.access', 'setup.write', 'manage_platform_settings'], + userId, + tenantId: 'org_1', + }), + }); + + /** `PUT /api/settings/:namespace` through the mounted route. Status starts at 0 so 200 is asserted. */ + const put = async (namespace: string, body: Record) => { + const handler = http.routes.get('PUT /api/settings/:namespace')!; + const req = { + params: { namespace }, + query: {}, + body, + headers: {}, + method: 'PUT', + path: `/api/settings/${namespace}`, + } as unknown as IHttpRequest; + const state: { status: number; body?: any } = { status: 0 }; + const res = { + json: (data: any) => { state.body = data; if (state.status === 0) state.status = 200; }, + send: () => {}, + status: (code: number) => { state.status = code; return res; }, + header: () => res, + } as unknown as IHttpResponse; + await handler(req, res); + expect(state.status, JSON.stringify(state.body)).toBe(200); + return state; + }; + + /** The audit rows of one key, oldest first, read out of the driver's store. */ + const auditOf = (object: 'sys_setting_audit' | 'sys_platform_setting_audit', key: string) => + [...rowsOf(object).values()].filter((r) => r.key === key); + + return { put, auditOf, asUser: (id: string) => { userId = id; } }; +} + +describe('#26465 — old_hash / new_hash on the tenant rung (sys_setting_audit)', () => { + it('first set: old_hash is null; new_hash is the declared SHA-256 of the value', async () => { + const { put, auditOf } = await boot(); + + await put('hash_tenant', { timezone: 'UTC' }); + + const rows = auditOf('sys_setting_audit', 'timezone'); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ action: 'set', old_hash: null, new_hash: sha256Of('UTC'), encrypted: false }); + expect(rows[0].new_hash).toMatch(SHA256_SHAPE); + }); + + it('overwrite: old_hash is the digest of the previous value', async () => { + const { put, auditOf } = await boot(); + + await put('hash_tenant', { timezone: 'UTC' }); + await put('hash_tenant', { timezone: 'Asia/Tokyo' }); + + const rows = auditOf('sys_setting_audit', 'timezone'); + expect(rows).toHaveLength(2); + expect(rows[1]).toMatchObject({ action: 'set', old_hash: sha256Of('UTC'), new_hash: sha256Of('Asia/Tokyo') }); + // The chain reads: each row's old_hash is the row before's new_hash. + expect(rows[1].old_hash).toBe(rows[0].new_hash); + }); + + it('same-value write: old_hash equals new_hash, so a duplicate write is detectable', async () => { + const { put, auditOf } = await boot(); + + await put('hash_tenant', { timezone: 'Asia/Tokyo' }); + await put('hash_tenant', { timezone: 'Asia/Tokyo' }); + + const rows = auditOf('sys_setting_audit', 'timezone'); + expect(rows).toHaveLength(2); + expect(rows[1].old_hash).toBe(sha256Of('Asia/Tokyo')); + expect(rows[1].old_hash).toBe(rows[1].new_hash); + }); + + it('set → reset → set: the reset carries the previous digest and no new one; the next set starts from unset', async () => { + const { put, auditOf } = await boot(); + + await put('hash_tenant', { timezone: 'UTC' }); + // A null value is the family's removal: there is no DELETE verb on the + // settings routes, so this IS the delete. + await put('hash_tenant', { timezone: null }); + await put('hash_tenant', { timezone: 'Europe/Paris' }); + + const rows = auditOf('sys_setting_audit', 'timezone'); + expect(rows.map((r) => r.action)).toEqual(['set', 'reset', 'set']); + expect(rows[1]).toMatchObject({ old_hash: sha256Of('UTC'), new_hash: null }); + expect(rows[2]).toMatchObject({ old_hash: null, new_hash: sha256Of('Europe/Paris') }); + }); +}); + +describe('#26465 — the previous value is read under the scope the write targets', () => { + it('a user-scope write reads its OWN previous row, not another user\'s', async () => { + const { put, auditOf, asUser } = await boot(); + + asUser('usr_a'); + await put('hash_user', { theme: 'dark' }); + asUser('usr_b'); + await put('hash_user', { theme: 'light' }); + asUser('usr_a'); + await put('hash_user', { theme: 'light' }); + + const rows = auditOf('sys_setting_audit', 'theme'); + expect(rows.map((r) => [r.actor_id, r.old_hash, r.new_hash])).toEqual([ + ['usr_a', null, sha256Of('dark')], + // usr_b had no value of its own: usr_a's row is not its previous value. + ['usr_b', null, sha256Of('light')], + ['usr_a', sha256Of('dark'), sha256Of('light')], + ]); + }); +}); + +describe('#26465 — the global rung (sys_platform_setting_audit) records the same hashes', () => { + it('first set, overwrite and same-value write', async () => { + const { put, auditOf } = await boot(); + + await put('hash_global', { host: 'smtp.example.com' }); + await put('hash_global', { host: 'smtp2.example.com' }); + await put('hash_global', { host: 'smtp2.example.com' }); + + const rows = auditOf('sys_platform_setting_audit', 'host'); + expect(rows.map((r) => [r.old_hash, r.new_hash])).toEqual([ + [null, sha256Of('smtp.example.com')], + [sha256Of('smtp.example.com'), sha256Of('smtp2.example.com')], + [sha256Of('smtp2.example.com'), sha256Of('smtp2.example.com')], + ]); + }); + + it('CONTROL — a secret value keeps its keyed path: keyed new_hash, null old_hash, no unkeyed digest anywhere', async () => { + const { put, auditOf } = await boot(); + + await put('hash_global', { api_key: 'pw-alpha' }); + await put('hash_global', { api_key: 'pw-beta' }); + + const rows = auditOf('sys_platform_setting_audit', 'api_key'); + expect(rows).toHaveLength(2); + for (const row of rows) { + expect(row.encrypted).toBe(true); + expect(row.old_hash).toBeNull(); + expect(row.new_hash).toMatch(KEYED_SHAPE); + } + expect(rows[1].new_hash).not.toBe(rows[0].new_hash); + const unkeyed = [sha256Of('pw-alpha'), sha256Of('pw-beta'), 'sha256:' + createHash('sha256').update('pw-alpha').digest('hex')]; + for (const row of rows) for (const d of unkeyed) expect(JSON.stringify(row)).not.toContain(d.slice('sha256:'.length)); + }); +}); 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 index 29fd4554140..5c51e32593b 100644 --- a/packages/services/service-settings/src/settings-audit-secret-digest.test.ts +++ b/packages/services/service-settings/src/settings-audit-secret-digest.test.ts @@ -8,8 +8,9 @@ * 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. + * available it records no fingerprint at all. Non-secret settings record the + * declared `sha256:` digest of the canonical JSON (#26465), whatever adapter + * is injected; the adapter's own `digest` is no longer read. */ import { createHash, randomBytes } from 'node:crypto'; @@ -178,15 +179,16 @@ describe('settings audit trail — secret-valued settings', () => { }); describe('settings audit trail — non-secret settings', () => { - it('keep the unkeyed adapter digest of the canonical JSON, unchanged', async () => { + it('record the declared SHA-256 of the canonical JSON on both ledgers, not the adapter digest (#26465)', 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 expected = sha256(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 }); + expect(settingAudit[0].newHash).not.toBe(new NoopCryptoAdapter().digest(JSON.stringify('ops@example.com'))); }); }); diff --git a/packages/services/service-settings/src/settings-service.ts b/packages/services/service-settings/src/settings-service.ts index a50d89bdec2..7425ed93544 100644 --- a/packages/services/service-settings/src/settings-service.ts +++ b/packages/services/service-settings/src/settings-service.ts @@ -3296,13 +3296,16 @@ export class SettingsService { context: SETTINGS_SYSTEM_CONTEXT, }); if (existing[0]) { - const previous = existing[0] as SettingsRow; + // Snapshot BEFORE the update: an engine may hand back the stored + // object itself, which the update then rewrites in place. + const previous = { ...(existing[0] as SettingsRow) }; + const previousEnc = SettingsService.handleOf(previous.value_enc); await this.engine.update(object, { where, data, context: SETTINGS_SYSTEM_CONTEXT, }); - return { previousEnc: SettingsService.handleOf(previous.value_enc), previous }; + return { previousEnc, previous }; } // Under the opt-in the engine no longer checks that a user-scope row's // `user_id` names a user, so the service keeps that refusal before the From ea57f45a7eb0e262c6073254930f7cef15b53a5d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 05:56:49 +0000 Subject: [PATCH 3/3] docs(qa): settings audit item computes the SHA-256 the service now writes; changeset Claude-Session: https://claude.ai/code/session_01FEkQ5ojTc5Fbj8xxXSfo1K Co-authored-by: Claude --- .changeset/26465-settings-audit-old-hash.md | 14 ++++++++++++ .../areas/platform-core.json | 22 ++++++++++++------- .../src/sys-secret-orphan-report.ts | 3 ++- 3 files changed, 30 insertions(+), 9 deletions(-) create mode 100644 .changeset/26465-settings-audit-old-hash.md diff --git a/.changeset/26465-settings-audit-old-hash.md b/.changeset/26465-settings-audit-old-hash.md new file mode 100644 index 00000000000..bbc136718b2 --- /dev/null +++ b/.changeset/26465-settings-audit-old-hash.md @@ -0,0 +1,14 @@ +--- +'@objectstack/service-settings': patch +--- + +fix(service-settings): the settings audit trail records the previous value's digest in `old_hash`, and a non-secret value's digest is the declared SHA-256 + +Clause-②: no +Metadata-protocol: none + +`sys_setting_audit` and `sys_platform_setting_audit` declare `old_hash` as the SHA-256 of the previous value (null when it was unset) and `new_hash` as the SHA-256 of the new value (null on a reset). The write path passed `old_hash: null` on every write and recorded the adapter's FNV-1a 32 (`fnv32:…`) as the new hash, so an operator could not tell from the trail whether a write changed anything. + +- **`old_hash`** now holds the digest of the value the row held before the write, read off the row the upsert already looks up: no extra read, and none at all for a first write. It is null for a first write, after a reset, and for a row that held a secret. +- **A non-secret value's digest** is now `sha256:` followed by the hex SHA-256 of its canonical JSON, on both `new_hash` and the `config_change` row's digest in `sys_audit_log`. It no longer goes through `CryptoAdapter.digest`, so an injected adapter cannot change the format. The prefix keeps it distinct from the `fnv32:` rows already stored, which are left as they are: compare hashes only between rows that carry the same prefix. +- **Secret values are unchanged.** They keep the crypto provider's keyed digest (`hmac-sha256:…`) as `new_hash`, or none, and `old_hash` stays null for them. diff --git a/docs/qa/platform-checklist/areas/platform-core.json b/docs/qa/platform-checklist/areas/platform-core.json index 1082db442a5..36da7341078 100644 --- a/docs/qa/platform-checklist/areas/platform-core.json +++ b/docs/qa/platform-checklist/areas/platform-core.json @@ -3136,7 +3136,7 @@ "title": "The settings audit trail keeps no fingerprint of a secret-valued setting that a reader can match offline: both ledgers record the platform's keyed digest or none, never the value or an unkeyed hash, so a caller who can only read the audit rows learns that the secret changed and nothing about its value", "since": "v17", "status": "active", - "revision": 4, + "revision": 5, "priority": "P1", "surface": "api", "personas": [ @@ -3159,10 +3159,10 @@ "generate a fresh random value V1 for this run (never a value used anywhere else). As admin, PUT /api/settings/mail { provider: 'resend', api_key: V1, from_email: 'ops@example.com' }", "read the per-key ledger: GET /api/v1/data/sys_platform_setting_audit filtered to namespace=mail and key=api_key; capture the newest row (action set, encrypted true, new_hash). mail is a GLOBAL-scope namespace, and the per-key ledger of the global rung is the deployment-level sys_platform_setting_audit (ADR-0131 D7, packages/services/service-settings/src/settings-service-plugin.ts#settingAuditObjectFor); a tenant- or user-scope key's row lands in sys_setting_audit", "read the generic ledger: GET /api/v1/data/sys_audit_log filtered to action=config_change; capture the newest row for mail.api_key (object_name 'sys_platform_setting' — mail is a GLOBAL-scope namespace, and a global-scope value's row names the tenant-less table it lives in, packages/services/service-settings/src/config-change-audit.ts#CONFIG_CHANGE_GLOBAL_OBJECT_NAME; a tenant-scope key's row names 'sys_setting'; new_value is a JSON string carrying namespace, key, scope and digest; metadata carries encrypted: true)", - "OFFLINE, with no server key: compute the unkeyed digests a leak would carry — the FNV-1a 32 of V1's canonical JSON (the quoted string), spelled 'fnv32:' + 8 lowercase hex, which is the unkeyed digest this settings service writes for a non-secret key (packages/services/service-settings/src/crypto-adapter.ts#digest), and, belt and braces, the SHA-256 hex of V1 (bare, and with a 'sha256:' prefix) — and search both captured rows, every field, for each of them and for V1 itself", + "OFFLINE, with no server key: compute the unkeyed digests a leak would carry — the SHA-256 of V1's canonical JSON (the quoted string), spelled 'sha256:' + 64 lowercase hex, which is the unkeyed digest this settings service writes for a non-secret key (packages/services/service-settings/src/crypto-adapter.ts#settingAuditDigest), and, belt and braces, the bare SHA-256 hex of V1 and of its canonical JSON, and the FNV-1a 32 of its canonical JSON ('fnv32:' + 8 lowercase hex, the non-secret digest rows written before #26465 carry) — and search both captured rows, every field, for each of them and for V1 itself", "write V1 again, then a different value V2, then V1 a third time; re-read both ledgers and compare the fingerprints of the three writes", "reset the key: PUT /api/settings/mail { provider: 'log', api_key: null } (a null value is the reset; 'log' needs no api_key), then read the reset rows", - "control: write the NON-secret key mail.from_email and compute OFFLINE the FNV-1a 32 of its canonical JSON (the quoted string), spelled 'fnv32:' + 8 lowercase hex. Its rows must carry exactly that unkeyed digest. This proves step 4's offline search — which computes the same algorithm — WOULD have found an unkeyed digest, and that the keyed rule is scoped to secret-valued keys" + "control: write the NON-secret key mail.from_email and compute OFFLINE the SHA-256 of its canonical JSON (the quoted string), spelled 'sha256:' + 64 lowercase hex. Its rows must carry exactly that unkeyed digest. This proves step 4's offline search — which computes the same algorithm — WOULD have found an unkeyed digest, and that the keyed rule is scoped to secret-valued keys" ], "acceptance": [ { @@ -3172,7 +3172,7 @@ "evidence": "both captured rows, quoted in full" }, { - "clause": "a reader of the audit rows who holds no server key cannot confirm a guess: no fingerprint in either row can be reproduced from the value alone. No offline unkeyed digest of V1 — the FNV-1a 32 of its canonical JSON ('fnv32:…', the algorithm the service uses for non-secret keys), nor the SHA-256 of V1 with or without the sha256 prefix — matches anything in either row", + "clause": "a reader of the audit rows who holds no server key cannot confirm a guess: no fingerprint in either row can be reproduced from the value alone. No offline unkeyed digest of V1 — the SHA-256 of its canonical JSON ('sha256:…', the algorithm the service uses for non-secret keys), the bare SHA-256 of V1, nor the FNV-1a 32 of its canonical JSON ('fnv32:…') — matches anything in either row", "oracle": "api", "verify": "the step 4 offline search over both rows finds no hit, AND the step 7 control's offline digest DOES match the non-secret row. Without the control, a no-hit proves nothing about the search", "evidence": "the offline digest values, the two secret rows, and the matching control row" @@ -3190,9 +3190,9 @@ "evidence": "the two reset rows" }, { - "clause": "non-secret settings are unchanged: a non-encrypted key keeps the unkeyed digest of its canonical JSON on both ledgers. The keyed rule applies to secret-valued keys only", + "clause": "non-secret settings record the unkeyed digest of their canonical JSON on both ledgers, the SHA-256 the per-key ledger's new_hash column declares. The keyed rule applies to secret-valued keys only", "oracle": "api", - "verify": "mail.from_email's rows carry the FNV-1a 32 digest of the quoted JSON string ('fnv32:' + 8 lowercase hex), computed offline in step 7", + "verify": "mail.from_email's rows carry the SHA-256 digest of the quoted JSON string ('sha256:' + 64 lowercase hex), computed offline in step 7", "evidence": "the control rows and the offline digest" }, { @@ -3203,7 +3203,7 @@ } ], "negative": [ - "an unkeyed digest of a secret-valued setting (the 'fnv32:' FNV-1a 32 of its canonical JSON, or a plain SHA-256 of the value, with or without a prefix) on EITHER ledger is a FAIL. Anyone who can read the audit rows could confirm a guessed value offline, and settings secrets are short, guessable values", + "an unkeyed digest of a secret-valued setting (the 'sha256:' SHA-256 of its canonical JSON, a plain SHA-256 of the value, or the 'fnv32:' FNV-1a 32 of its canonical JSON) on EITHER ledger is a FAIL. Anyone who can read the audit rows could confirm a guessed value offline, and settings secrets are short, guessable values", "the plaintext of a secret-valued setting in any field of either ledger is a FAIL", "a host with no keyed digest falling back to an unkeyed one is a FAIL; the rule is to record no fingerprint", "a secret write REFUSED because its audit fingerprint could not be computed is a FAIL: the audit trail never vetoes a settings save", @@ -3214,7 +3214,7 @@ ], "automated": { "kind": "unit", - "ref": "packages/services/service-settings/src/settings-audit-secret-digest.test.ts — acceptance[5] and negative[2]–[3] ONLY: the no-keyed-digest arm a stock boot cannot reach (knownGaps[0]), through 'legacy inline-adapter path with no provider records no fingerprint and reports it once' and 'a provider that refuses a keyed digest leaves the write intact and records no fingerprint'. The pin drives SettingsService over stub audit sinks. So its other cases — 'records the provider keyed digest on both ledgers, not the unkeyed hash of the value', 'is stable for equal values and distinct for different ones', 'records no fingerprint on a reset', the provider-wired inline-adapter case, and the non-secret 'keep the unkeyed adapter digest of the canonical JSON, unchanged' — are service-layer evidence for acceptance[0]–[4] and negative[0]–[1], never their pass: those score the ledger rows read back over the API (steps[0]–[6])" + "ref": "packages/services/service-settings/src/settings-audit-secret-digest.test.ts — acceptance[5] and negative[2]–[3] ONLY: the no-keyed-digest arm a stock boot cannot reach (knownGaps[0]), through 'legacy inline-adapter path with no provider records no fingerprint and reports it once' and 'a provider that refuses a keyed digest leaves the write intact and records no fingerprint'. The pin drives SettingsService over stub audit sinks. So its other cases — 'records the provider keyed digest on both ledgers, not the unkeyed hash of the value', 'is stable for equal values and distinct for different ones', 'records no fingerprint on a reset', the provider-wired inline-adapter case, and the non-secret 'record the declared SHA-256 of the canonical JSON on both ledgers, not the adapter digest (#26465)' — are service-layer evidence for acceptance[0]–[4] and negative[0]–[1], never their pass: those score the ledger rows read back over the API (steps[0]–[6])" }, "source": [ "packages/services/service-settings/src/settings-service.ts#secretAuditDigest (one helper supplies a secret write's fingerprint on both write branches, the sys_secret path and the legacy inline-adapter path: the provider's keyedDigest, or null with a once-per-key warn; never the adapter's unkeyed digest)", @@ -3248,6 +3248,12 @@ "date": "2026-10-20", "change": "automated.ref narrowed per clause — the sweep-wide rule-6 contradiction QA run #24840 (18.0 sweep round 4, Tier 1 at 307ce4c4) names at settings-audit acceptance[0]–[4], round 15 of wave anchor #24579. The ref listed seven cases whose names read as acceptance[0], [2], [3] and [4]. The pin drives SettingsService over stub audit sinks and never reads sys_platform_setting_audit or sys_audit_log over the API, which is what those clauses score. By RUNNER rule 6 (a pin scores only the layer it drives) it scores acceptance[5] and its negatives [2] and [3] only (QA run #24838 scored acceptance[0]–[4] live). Read at base b1676441ba. No clause changed", "ref": "#24891" + }, + { + "revision": 5, + "date": "2026-10-27", + "change": "steps[3] and [6], acceptance[1].clause, [4] (clause, verify), negative[0] and automated.ref re-pointed: a non-secret setting's audit digest is now the SHA-256 the sys_setting_audit / sys_platform_setting_audit columns declare, spelled 'sha256:' + 64 hex (crypto-adapter.ts settingAuditDigest), no longer the adapter's FNV-1a 32 ('fnv32:'). The step-7 control computes the algorithm the service now writes, so it can still prove the offline search would find an unkeyed digest. Not loosened: the secret search keeps the FNV-1a 32 as a second net, so a secret row carrying either unkeyed form still fails", + "ref": "#26465" } ] }, 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 a6e0810639b..97d96f4a3c7 100644 --- a/packages/services/service-settings/src/sys-secret-orphan-report.ts +++ b/packages/services/service-settings/src/sys-secret-orphan-report.ts @@ -37,7 +37,8 @@ * including tenant-authored ones. * - ✅ The audit trail records digests, not handles (`old_hash` / `new_hash` * are content digests; `SettingsService` passes the provider's - * `keyedDigest()` for secret-valued keys, `digest()` otherwise), + * `keyedDigest()` for secret-valued keys, the `sha256:` digest of + * `settingAuditDigest()` 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. *