From 8eeba57bc330e642844d7aca7d895556e16424e5 Mon Sep 17 00:00:00 2001 From: larryro <371767072@qq.com> Date: Thu, 3 Sep 2026 20:33:39 +0800 Subject: [PATCH 1/8] fix(platform): hash the audit record in its stored form The audit writer hashed the caller's in-memory strings while the verifier rebuilds the record from the stored row, so anything Postgres alters on the way in made an untouched row read as TAMPERED, or failed the write outright. A lone UTF-16 surrogate (a slice through an emoji) is stored as U+FFFD but was hashed as the escape sequence JSON.stringify emits for it; a NUL or a lone surrogate inside a jsonb field made the INSERT throw and took the user's transaction with it; an undefined array item, a sparse hole, or a Date in a jsonb payload diverged silently. Normalize every text field, text[] element, and jsonb key and string into the form the column hands back (toWellFormed, NUL to U+FFFD, jsonb through one JSON round-trip) before hashing AND inserting, so the writer signs exactly what a later read rebuilds. The normalization is the identity on storable content: rows already written keep recomputing to their own hash. Rows a lone surrogate had already broken stay broken (their hash covers content that was never stored) - no history is silently re-signed. Parity test models the text/jsonb round trip; the integration check writes a row with a lone surrogate, NUL, quotes, control characters, a Date, an array hole, and a 200 KB payload, and verifies the chain clean. --- .../domains/audit_logs/hash-input.test.ts | 275 ++++++++++++++++++ .../backend/domains/audit_logs/hash-input.ts | 108 ++++++- .../backend/domains/audit_logs/service.ts | 40 ++- .../platform/backend/integration-check.ts | 85 ++++++ 4 files changed, 488 insertions(+), 20 deletions(-) create mode 100644 services/platform/backend/domains/audit_logs/hash-input.test.ts diff --git a/services/platform/backend/domains/audit_logs/hash-input.test.ts b/services/platform/backend/domains/audit_logs/hash-input.test.ts new file mode 100644 index 0000000000..f51c83b872 --- /dev/null +++ b/services/platform/backend/domains/audit_logs/hash-input.test.ts @@ -0,0 +1,275 @@ +import { describe, expect, it } from 'vitest'; + +import { canonicalizeForTest } from '../../core/lib/helpers/audit_hash.ts'; +import { + buildAuditRecordHashInput, + normalizeStoredJson, + normalizeStoredText, + rowToHashInput, + toStoredAuditRecord, + type StoredAuditRecord, +} from './hash-input.ts'; +import type { AuditLogRow } from './types.ts'; + +/** + * What Postgres hands back for a `text` column: the UTF-8 encoder replaces a + * lone surrogate with U+FFFD; a NUL byte is refused outright (and with it the + * whole INSERT). The integration check proves this against a real database; + * here the model keeps the parity test honest. + */ +function asPostgresText(value: string): string { + if (value.includes('\u0000')) { + throw new Error('invalid byte sequence for encoding "UTF8": 0x00'); + } + return Buffer.from(value, 'utf8').toString('utf8'); +} + +/** + * What a `jsonb` column hands back: the serialized document parsed again. + * `JSON.stringify` only ever emits a `\udXXX` escape for a LONE surrogate + * (pairs are written raw), and the jsonb parser refuses both that and + * `\u0000`. + */ +function asPostgresJsonb( + value: Record, +): Record { + const text = JSON.stringify(value); + if (text.includes('\\u0000')) { + throw new Error('unsupported Unicode escape sequence'); + } + if (/\\ud[89a-f][0-9a-f]{2}/i.test(text)) { + throw new Error('Unicode low surrogate must follow a high surrogate'); + } + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- the input was a record; JSON round-trips a record to a record + return JSON.parse(text) as Record; +} + +/** The row `app.audit_logs` would hand back for a record it stored. */ +function rowFromStorage(stored: StoredAuditRecord): AuditLogRow { + const text = (value: string | undefined): string | null => + value === undefined ? null : asPostgresText(value); + const json = ( + value: Record | undefined, + ): Record | null => + value === undefined ? null : asPostgresJsonb(value); + return { + id: 'row-1', + organizationId: stored.organizationId, + actorId: asPostgresText(stored.actorId), + actorEmail: text(stored.actorEmail), + actorEmailHash: text(stored.actorEmailHash), + actorRole: text(stored.actorRole), + actorType: stored.actorType, + action: asPostgresText(stored.action), + category: stored.category, + resourceType: asPostgresText(stored.resourceType), + resourceId: text(stored.resourceId), + resourceName: text(stored.resourceName), + previousState: json(stored.previousState), + newState: json(stored.newState), + changedFields: + stored.changedFields.length > 0 + ? stored.changedFields.map(asPostgresText) + : null, + sessionId: text(stored.sessionId), + ipAddress: text(stored.ipAddress), + actorIpHash: text(stored.actorIpHash), + userAgent: text(stored.userAgent), + requestId: text(stored.requestId), + timestamp: stored.timestamp, + status: stored.status, + errorMessage: text(stored.errorMessage), + metadata: json(stored.metadata), + integrityHash: 'not-part-of-the-input', + previousHash: null, + piiScrubbed: null, + }; +} + +const canonical = (record: Record): string => + canonicalizeForTest(record); + +const LONE_HIGH = '🎉'.slice(0, 1); +const LONE_LOW = '🎉'.slice(1); + +/** A record every one of whose tricky fields a real writer can produce: + * a `.slice()` through an emoji, binary in an error, a Date in metadata, + * a `.map()` that yielded undefined, a 100 KB payload. */ +function trickyRecord(): StoredAuditRecord { + const sparse: unknown[] = [1]; + sparse[2] = 3; + return { + organizationId: 'org-1', + actorId: 'user-1', + actorEmail: 'zoë@example.com', + actorType: 'user', + action: 'itest.tricky', + category: 'data', + resourceType: 'document', + resourceName: + 'quote " backslash \\ newline \n tab \t emoji 🎉 zero-width \u200B 漢字 é', + errorMessage: `truncated at an emoji ${LONE_HIGH}`, + previousState: { + 'we,ird"{key}': 'old', + nested: { 'kéy 🎉': ['✓', '\u0001'] }, + }, + newState: { + 'we,ird"{key}': 'new', + nested: { 'kéy 🎉': ['✓', '\u0002'] }, + when: new Date(0), + }, + changedFields: ['we,ird"{key}', 'nested', 'when'], + metadata: { + nul: 'a\u0000b', + lone: `${LONE_LOW} head`, + holes: [1, undefined, 'x'], + sparse, + big: 'x'.repeat(100_000), + huge: 1e21, + negativeZero: -0, + notANumber: Number.NaN, + [`k${LONE_HIGH}`]: 'lone surrogate in a key', + }, + timestamp: 1_756_900_000_000, + status: 'failure', + }; +} + +describe('normalizeStoredText', () => { + it('replaces a lone surrogate with U+FFFD — the byte Postgres stores', () => { + expect(normalizeStoredText(`tail ${LONE_HIGH}`)).toBe('tail \uFFFD'); + expect(normalizeStoredText(`${LONE_LOW} head`)).toBe('\uFFFD head'); + expect(normalizeStoredText(`${LONE_HIGH}${LONE_HIGH}`)).toBe( + '\uFFFD\uFFFD', + ); + }); + + it('replaces NUL, which neither text nor jsonb can hold', () => { + expect(normalizeStoredText('a\u0000b\u0000')).toBe('a\uFFFDb\uFFFD'); + }); + + it('is the identity on every storable string', () => { + for (const value of [ + '', + 'plain', + 'quote " backslash \\ newline \n tab \t', + '🎉 paired emoji, 漢字, é combining, \u200B zero-width', + '\u0001\u001F control chars', + '\uFFFD already a replacement mark', + ]) { + expect(normalizeStoredText(value)).toBe(value); + } + }); +}); + +describe('normalizeStoredJson', () => { + it('yields what a jsonb column hands back', () => { + const sparse: unknown[] = [1]; + sparse[2] = 3; + const normalized = normalizeStoredJson({ + dropped: undefined, + holes: [1, undefined, 'x'], + sparse, + when: new Date(0), + notANumber: Number.NaN, + infinite: Number.POSITIVE_INFINITY, + negativeZero: -0, + nested: { keep: 'value', gone: undefined }, + }); + expect(normalized).toStrictEqual({ + holes: [1, null, 'x'], + sparse: [1, null, 3], + when: '1970-01-01T00:00:00.000Z', + notANumber: null, + infinite: null, + negativeZero: 0, + nested: { keep: 'value' }, + }); + }); + + it('text-normalizes keys and strings at every depth', () => { + const normalized = normalizeStoredJson({ + [`k${LONE_HIGH}`]: { deep: [`${LONE_LOW}x`, 'a\u0000b'] }, + }); + expect(normalized).toStrictEqual({ + 'k\uFFFD': { deep: ['\uFFFDx', 'a\uFFFDb'] }, + }); + }); + + it('keeps a large payload intact', () => { + const big = 'x'.repeat(100_000); + expect(normalizeStoredJson({ big })).toStrictEqual({ big }); + }); +}); + +describe('the writer hashes the STORED form', () => { + it('a record hashed pre-storage cannot be rebuilt from its row (the defect)', () => { + const raw = trickyRecord(); + // The jsonb fields refuse the INSERT outright… + expect(() => rowFromStorage(raw)).toThrow(); + // …and a text-only lone surrogate stores as U+FFFD while the writer's + // canonical carried the `\udXXX` escape: a false tamper verdict forever. + const textOnly: StoredAuditRecord = { + ...raw, + previousState: undefined, + newState: undefined, + metadata: undefined, + changedFields: [], + }; + expect(canonical(rowToHashInput(rowFromStorage(textOnly)))).not.toBe( + canonical(buildAuditRecordHashInput(textOnly)), + ); + }); + + it('the stored form is storable and rebuilds to the identical canonical', () => { + const stored = toStoredAuditRecord(trickyRecord()); + const row = rowFromStorage(stored); + expect(canonical(rowToHashInput(row))).toBe( + canonical(buildAuditRecordHashInput(stored)), + ); + // The normalization is visible in the row, not a no-op. + expect(row.errorMessage).toBe('truncated at an emoji \uFFFD'); + expect(row.metadata).toMatchObject({ + nul: 'a\uFFFDb', + lone: '\uFFFD head', + holes: [1, null, 'x'], + sparse: [1, null, 3], + huge: 1e21, + negativeZero: 0, + notANumber: null, + 'k\uFFFD': 'lone surrogate in a key', + }); + expect(row.newState).toMatchObject({ when: '1970-01-01T00:00:00.000Z' }); + expect(row.changedFields).toStrictEqual(['we,ird"{key}', 'nested', 'when']); + }); + + it('is the identity on a plain record — rows already written keep their hash', () => { + const plain: StoredAuditRecord = { + organizationId: 'org-1', + actorId: 'user-1', + actorEmail: 'admin@example.com', + actorRole: 'owner', + actorType: 'user', + action: 'member.role_changed', + category: 'member', + resourceType: 'member', + resourceId: 'user-2', + resourceName: 'Zoë 🎉', + previousState: { role: 'member', tags: ['a', 'b'] }, + newState: { role: 'admin', tags: ['a'] }, + changedFields: ['role', 'tags'], + ipAddress: '203.0.113.7', + userAgent: 'Mozilla/5.0 (X11; Linux x86_64)', + requestId: 'req-1', + timestamp: 1_756_900_000_000, + status: 'success', + metadata: { note: 'quotes " and \\ and\nnewlines', count: 3 }, + }; + expect( + canonical(buildAuditRecordHashInput(toStoredAuditRecord(plain))), + ).toBe(canonical(buildAuditRecordHashInput(plain))); + expect(canonical(rowToHashInput(rowFromStorage(plain)))).toBe( + canonical(buildAuditRecordHashInput(plain)), + ); + }); +}); diff --git a/services/platform/backend/domains/audit_logs/hash-input.ts b/services/platform/backend/domains/audit_logs/hash-input.ts index 0fd4b4bd6e..9343767857 100644 --- a/services/platform/backend/domains/audit_logs/hash-input.ts +++ b/services/platform/backend/domains/audit_logs/hash-input.ts @@ -2,11 +2,11 @@ import { isRecord } from '../../../lib/utils/type-utils.ts'; import type { AuditLogRow, CreateAuditLogArgs } from './types.ts'; /** - * Pure audit-record shaping — redaction, diffing, and the canonical hash - * input. Ported from `convex/audit_logs/helpers.ts` (which dies with the - * component); the hash ALGORITHM itself is reused unported from - * `convex/lib/helpers/audit_hash.ts` so 0.4 chains stay verifiable after the - * cutover data import. + * Pure audit-record shaping — redaction, diffing, the STORED-form + * normalization the writer hashes, and the canonical hash input. Ported from + * `convex/audit_logs/helpers.ts` (which dies with the component); the hash + * ALGORITHM itself is reused unported from `convex/lib/helpers/audit_hash.ts` + * so 0.4 chains stay verifiable after the cutover data import. */ const SENSITIVE_FIELDS = new Set([ @@ -161,6 +161,104 @@ export function buildAuditRecordHashInput( }; } +/** + * The two things a JS string can carry that Postgres cannot store: a lone + * UTF-16 surrogate (the UTF-8 encoder writes U+FFFD in its place — a + * `.slice()` through an emoji is the usual source) and U+0000 (text and + * jsonb both reject it, failing the INSERT and the user's transaction with + * it). Both become U+FFFD, the same visible replacement mark, so the string + * the writer hashes is the string a later read hands back. Identity on every + * storable string. + */ +export function normalizeStoredText(value: string): string { + const wellFormed = value.toWellFormed(); + return wellFormed.includes('\u0000') + ? wellFormed.replaceAll('\u0000', '\uFFFD') + : wellFormed; +} + +function normalizeJsonStrings(value: unknown): unknown { + if (typeof value === 'string') return normalizeStoredText(value); + if (Array.isArray(value)) return value.map(normalizeJsonStrings); + if (isRecord(value)) { + const out: Record = {}; + for (const [key, item] of Object.entries(value)) { + out[normalizeStoredText(key)] = normalizeJsonStrings(item); + } + return out; + } + return value; +} + +/** + * A jsonb payload in the form the column hands back: one JSON round-trip + * (drops undefined-valued keys, turns undefined array items and sparse holes + * into null, Date/`toJSON` objects into their JSON form, NaN/±Infinity into + * null), then every key and string value text-normalized. Key ORDER is not + * part of the contract — Postgres reorders keys and the canonicalizer sorts + * them. + */ +export function normalizeStoredJson( + value: Record, +): Record { + const roundTripped: unknown = JSON.parse(JSON.stringify(value)); + const normalized = normalizeJsonStrings(roundTripped); + return isRecord(normalized) ? normalized : {}; +} + +/** The writer's record once every field carries its final value. */ +export type StoredAuditRecord = CreateAuditLogArgs & { + changedFields: string[]; + timestamp: number; +}; + +const OPTIONAL_TEXT_FIELDS = [ + 'actorEmail', + 'actorEmailHash', + 'actorRole', + 'resourceId', + 'resourceName', + 'sessionId', + 'ipAddress', + 'actorIpHash', + 'userAgent', + 'requestId', + 'errorMessage', +] as const; + +const JSON_FIELDS = ['previousState', 'newState', 'metadata'] as const; + +/** + * Shape the record into its STORED form before it is hashed and inserted: + * the hash must cover what a later read rebuilds (`rowToHashInput`), not + * what the caller held in memory. Without this, a lone surrogate in an + * error message verified as TAMPERED forever (stored U+FFFD, hashed as the + * `\udXXX` escape `JSON.stringify` emits for it), and a NUL or lone + * surrogate inside a jsonb field failed the INSERT — and the user action or + * run settle with it. Identity on a record every field of which is + * storable, so rows already written keep recomputing to their own hash. + */ +export function toStoredAuditRecord( + source: StoredAuditRecord, +): StoredAuditRecord { + const stored: StoredAuditRecord = { + ...source, + actorId: normalizeStoredText(source.actorId), + action: normalizeStoredText(source.action), + resourceType: normalizeStoredText(source.resourceType), + changedFields: source.changedFields.map(normalizeStoredText), + }; + for (const field of OPTIONAL_TEXT_FIELDS) { + const value = stored[field]; + if (value !== undefined) stored[field] = normalizeStoredText(value); + } + for (const field of JSON_FIELDS) { + const value = stored[field]; + if (value !== undefined) stored[field] = normalizeStoredJson(value); + } + return stored; +} + /** * Rebuild the hash input from a PERSISTED row. Postgres surfaces absent * optionals as NULL where the writer had `undefined`; the canonicalizer diff --git a/services/platform/backend/domains/audit_logs/service.ts b/services/platform/backend/domains/audit_logs/service.ts index c3e4952eff..dc218a54a9 100644 --- a/services/platform/backend/domains/audit_logs/service.ts +++ b/services/platform/backend/domains/audit_logs/service.ts @@ -7,6 +7,7 @@ import { computeChangedFields, redactSensitiveFields, rowToHashInput, + toStoredAuditRecord, } from './hash-input.ts'; import type { AuditContext, @@ -22,7 +23,9 @@ import type { * per-org chain head (`FOR UPDATE`), so concurrent appends serialize and the * chain cannot fork; the audit row commits or rolls back atomically with the * change it describes. Hash algorithm and canonical record layout are the - * 0.4 ones, so chains imported at cutover keep verifying. + * 0.4 ones, so chains imported at cutover keep verifying. The hash covers + * the record in its STORED form (`toStoredAuditRecord`) — what a later read + * rebuilds — never the caller's in-memory strings. */ const ROW_COLUMNS = ` @@ -132,13 +135,20 @@ export async function createAuditLog( // before the head it chains off (see the 0.4 writer's tradeoff note). const timestamp = Math.max(Date.now(), head.lastTs + 1); - const recordForHash = buildAuditRecordHashInput({ + // Hash the STORED form and insert exactly that: every text field and jsonb + // payload shaped the way Postgres hands it back (lone surrogates and NUL + // → U+FFFD, jsonb through one JSON round-trip). The verifier rebuilds the + // record from the row, so anything the caller held that storage would + // alter must be altered here first — or an untouched row reads as + // tampered, or the INSERT fails and takes the user's transaction with it. + const stored = toStoredAuditRecord({ ...args, previousState: redactedPreviousState, newState: redactedNewState, changedFields, timestamp, }); + const recordForHash = buildAuditRecordHashInput(stored); const integrityHash = await computeAuditHash(head.lastHash, recordForHash); const inserted = await tx<{ id: string }[]>` @@ -149,19 +159,19 @@ export async function createAuditLog( ip_address, actor_ip_hash, user_agent, request_id, ts, status, error_message, metadata, integrity_hash, previous_hash ) VALUES ( - ${args.organizationId}, ${args.actorId}, ${args.actorEmail ?? null}, - ${args.actorEmailHash ?? null}, ${args.actorRole ?? null}, - ${args.actorType}, ${args.action}, ${args.category}, - ${args.resourceType}, ${args.resourceId ?? null}, - ${args.resourceName ?? null}, - ${redactedPreviousState === undefined ? null : tx.json(toJson(redactedPreviousState))}, - ${redactedNewState === undefined ? null : tx.json(toJson(redactedNewState))}, - ${changedFields.length > 0 ? changedFields : null}, - ${args.sessionId ?? null}, ${args.ipAddress ?? null}, - ${args.actorIpHash ?? null}, ${args.userAgent ?? null}, - ${args.requestId ?? null}, ${timestamp}, ${args.status}, - ${args.errorMessage ?? null}, - ${args.metadata === undefined ? null : tx.json(toJson(args.metadata))}, + ${stored.organizationId}, ${stored.actorId}, ${stored.actorEmail ?? null}, + ${stored.actorEmailHash ?? null}, ${stored.actorRole ?? null}, + ${stored.actorType}, ${stored.action}, ${stored.category}, + ${stored.resourceType}, ${stored.resourceId ?? null}, + ${stored.resourceName ?? null}, + ${stored.previousState === undefined ? null : tx.json(toJson(stored.previousState))}, + ${stored.newState === undefined ? null : tx.json(toJson(stored.newState))}, + ${stored.changedFields.length > 0 ? stored.changedFields : null}, + ${stored.sessionId ?? null}, ${stored.ipAddress ?? null}, + ${stored.actorIpHash ?? null}, ${stored.userAgent ?? null}, + ${stored.requestId ?? null}, ${timestamp}, ${stored.status}, + ${stored.errorMessage ?? null}, + ${stored.metadata === undefined ? null : tx.json(toJson(stored.metadata))}, ${integrityHash}, ${head.lastHash === '' ? null : head.lastHash} ) RETURNING id diff --git a/services/platform/backend/integration-check.ts b/services/platform/backend/integration-check.ts index 3c80b6494f..1273d7f1bb 100644 --- a/services/platform/backend/integration-check.ts +++ b/services/platform/backend/integration-check.ts @@ -28325,6 +28325,91 @@ async function checkAuditSurface( `fleet=${fleet.includes(orgId)}, head=${headReached}, broken=${brokenRun.broken}, alert=${alertStatus.success ? alertStatus.data.status.alertActive : 'ERR'}, bells=${bells[0]?.count} (want 1), recovered=${recovered}`, ); + // --- Stored-form hashing: tricky text stores and verifies clean ------- + // A `.slice()` through an emoji (lone surrogate), binary in an error + // (NUL), quotes, control characters, a Date, an array with a hole, a + // 200 KB payload. Before the writer hashed the STORED form, a lone + // surrogate in a text column verified as TAMPERED forever (Postgres stores + // U+FFFD, the canonical carried the escape), and NUL / a lone surrogate in + // jsonb failed the INSERT — and the user's action with it. + const loneHigh = '🎉'.slice(0, 1); + const loneLow = '🎉'.slice(1); + const nul = String.fromCharCode(0); + const replacement = String.fromCharCode(0xfffd); + const holes: unknown[] = [1]; + holes[2] = 3; + let trickyWritten = false; + let trickyError = ''; + try { + await sql.begin((tx) => + createAuditLog(tx, { + organizationId: orgId, + actorId: 'itest', + actorEmail: 'zoë@example.com', + actorType: 'system', + action: 'itest.tricky_text', + category: 'admin', + resourceType: 'itest', + resourceName: `quote " backslash \\ newline \n tab \t emoji 🎉 漢字 é cut ${loneHigh}`, + errorMessage: `binary ${nul} in an error, cut emoji ${loneLow}`, + previousState: { + 'we,ird"{key}': 'old', + nested: { 'kéy 🎉': ['✓', String.fromCharCode(1)] }, + }, + newState: { + 'we,ird"{key}': 'new', + nested: { 'kéy 🎉': ['✓', String.fromCharCode(2)] }, + when: new Date(0), + }, + metadata: { + nul: `a${nul}b`, + lone: `${loneLow} head`, + holes, + gaps: [1, undefined, 'x'], + big: 'x'.repeat(200_000), + huge: 1e21, + [`k${loneHigh}`]: 'lone surrogate in a key', + }, + status: 'failure', + }), + ); + trickyWritten = true; + } catch (error) { + trickyError = String(error); + } + const trickyRows = await sql< + { errorMessage: string | null; metadata: Record | null }[] + >` + SELECT error_message AS "errorMessage", metadata FROM app.audit_logs + WHERE org_id = ${orgId} AND action = 'itest.tricky_text' + LIMIT 1 + `; + const trickyRow = trickyRows[0]; + const trickyNormalized = + trickyRow?.errorMessage === + `binary ${replacement} in an error, cut emoji ${replacement}` && + trickyRow.metadata?.nul === `a${replacement}b` && + trickyRow.metadata?.lone === `${replacement} head` && + trickyRow.metadata?.[`k${replacement}`] === 'lone surrogate in a key'; + const trickyVerify = z + .object({ valid: z.boolean(), verifiedCount: z.number() }) + .loose() + .safeParse( + await ( + await post(`/api/app/audit-logs/integrity/verify?orgId=${orgId}`, { + maxEntries: 5000, + }) + ).json(), + ); + record( + 'audit chain: tricky text (lone surrogate, NUL, quotes, control chars, large jsonb) stores and verifies clean', + trickyWritten && + trickyNormalized && + trickyVerify.success && + trickyVerify.data.valid, + `written=${trickyWritten}${trickyError === '' ? '' : ` (${trickyError.slice(0, 90)})`}, normalized=${trickyNormalized}, verify=${trickyVerify.success ? `${trickyVerify.data.valid}/${trickyVerify.data.verifiedCount}` : 'ERR'}`, + ); + // --- Export: CSV into the org store behind a presigned GET ------------ const exported = z .object({ storageId: z.string(), fileName: z.string(), url: z.string() }) From dbc5b95bf6dfa9e044a9c587c145b0d5282710e6 Mon Sep 17 00:00:00 2001 From: larryro <371767072@qq.com> Date: Thu, 3 Sep 2026 20:35:45 +0800 Subject: [PATCH 2/8] fix(platform): re-anchor the verify walk past reaped rows, keep the bell Two ways the scheduled audit-integrity walk misreported the chain. A resume anchor the retention sweep had reaped (a job outage longer than the window, or a backlog outpacing the daily page) made the first surviving row's previous_hash mismatch the stored resume hash: a false "chain broken" verdict, a critical bell, and a walk that never advanced past it - clearable only by hand in the database. The walk now takes the org's effective audit-retention cutoff (the same clamped, cooldown- overlaid policy the sweep enforces); an anchor that is gone AND older than that cutoff re-anchors on the first surviving row exactly as a fresh walk would. An anchor missing INSIDE the window is not excused - nothing legitimately deletes an audit row there - so the linkage check still reports it. A real break stamped the alert fingerprint BEFORE writing the bell; when the bell write failed, every later run read the stamp as "already alerted" and never retried - one transient failure and the admins were never told. The bell is now re-asserted on every broken run, idempotent through its dedupe key, and the run reports whether it landed. Unit tests drive both walks over a fake connection; the integration check blocks the bell table with a CHECK constraint and sees the bell land once the block lifts, and points the progress row at a reaped anchor (older than the cutoff: re-anchors to head) and at a vanished in-window one (still a break). --- .../backend/domains/audit_logs/verify.test.ts | 265 ++++++++++++++++++ .../backend/domains/audit_logs/verify.ts | 140 ++++++--- .../backend/domains/retention/service.ts | 33 ++- .../platform/backend/integration-check.ts | 155 ++++++++++ 4 files changed, 556 insertions(+), 37 deletions(-) create mode 100644 services/platform/backend/domains/audit_logs/verify.test.ts diff --git a/services/platform/backend/domains/audit_logs/verify.test.ts b/services/platform/backend/domains/audit_logs/verify.test.ts new file mode 100644 index 0000000000..3935efa67a --- /dev/null +++ b/services/platform/backend/domains/audit_logs/verify.test.ts @@ -0,0 +1,265 @@ +import type { Sql } from 'postgres'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { computeAuditHash } from '../../core/lib/helpers/audit_hash.ts'; +import { writeNotificationForOrgs } from '../notifications/service.ts'; +import { auditLogRetentionCutoff } from '../retention/service.ts'; +import { rowToHashInput } from './hash-input.ts'; +import type { AuditLogRow } from './types.ts'; +import { runScheduledIntegrityCheck, verifyAuditChain } from './verify.ts'; + +vi.mock('../notifications/service.ts', () => ({ + writeNotificationForOrgs: vi.fn(), +})); +vi.mock('../retention/service.ts', () => ({ + auditLogRetentionCutoff: vi.fn(), +})); + +/** Whatever a statement answers with — audit rows, a progress row, nothing. */ +type Row = object; + +/** + * A postgres.js tagged-template stand-in: the test answers each statement + * from its (whitespace-collapsed) text; `begin` hands the same tag back as + * the transaction. Only the shapes the verify walk touches are modelled. + */ +function fakeDb(answer: (text: string, values: unknown[]) => Row[]): { + db: Sql; + statements: { text: string; values: unknown[] }[]; +} { + const statements: { text: string; values: unknown[] }[] = []; + const tag = ( + strings: TemplateStringsArray, + ...values: unknown[] + ): Promise => { + const text = strings.join('?').replaceAll(/\s+/g, ' ').trim(); + statements.push({ text, values }); + return Promise.resolve(answer(text, values)); + }; + const db = Object.assign(tag, { + unsafe: (text: string) => text, + begin: (callback: (tx: unknown) => Promise) => callback(db), + }); + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- a three-member stand-in for the postgres.js template function + return { db: db as unknown as Sql, statements }; +} + +const ORG = 'org-1'; + +/** A genuine chain of `count` rows, one second apart, hash-linked. */ +async function buildChain( + count: number, + startTs: number, +): Promise { + const rows: AuditLogRow[] = []; + let previous = ''; + for (let index = 0; index < count; index += 1) { + const row: AuditLogRow = { + id: `row-${index + 1}`, + organizationId: ORG, + actorId: 'user-1', + actorEmail: null, + actorEmailHash: null, + actorRole: null, + actorType: 'user', + action: `action.${index + 1}`, + category: 'data', + resourceType: 'probe', + resourceId: null, + resourceName: null, + previousState: null, + newState: null, + changedFields: null, + sessionId: null, + ipAddress: null, + actorIpHash: null, + userAgent: null, + requestId: null, + timestamp: startTs + index * 1000, + status: 'success', + errorMessage: null, + metadata: null, + integrityHash: '', + previousHash: previous === '' ? null : previous, + piiScrubbed: null, + }; + row.integrityHash = await computeAuditHash(previous, rowToHashInput(row)); + previous = row.integrityHash; + rows.push(row); + } + return rows; +} + +const isRowPage = (text: string): boolean => + text.startsWith('SELECT ? FROM app.audit_logs'); + +describe('verifyAuditChain — a resume anchor the retention sweep reaped', () => { + const START = 1_700_000_000_000; + + it('resumes after an anchor that is still there', async () => { + const chain = await buildChain(5, START); + const [, r2, r3, r4, r5] = chain; + if (!r2 || !r3 || !r4 || !r5) throw new Error('chain'); + const { db } = fakeDb((text) => (isRowPage(text) ? [r2, r3, r4, r5] : [])); + const result = await verifyAuditChain(db, ORG, { + fromTimestamp: r2.timestamp, + afterId: r2.id, + previousExpectedHash: r2.integrityHash, + reapedBefore: START - 1, + }); + expect(result.valid).toBe(true); + expect(result.verifiedCount).toBe(3); + expect(result.lastVerifiedId).toBe(r5.id); + expect(result.reanchored).toBeUndefined(); + }); + + it('re-anchors when the anchor (and its successor) were reaped past the cutoff', async () => { + const chain = await buildChain(5, START); + const [, r2, r3, r4, r5] = chain; + if (!r2 || !r3 || !r4 || !r5) throw new Error('chain'); + // The sweep took row-2 and row-3: the first survivor links to the reaped + // row-3, not to the resume hash (row-2). Retention, not tampering. + const { db } = fakeDb((text) => (isRowPage(text) ? [r4, r5] : [])); + const result = await verifyAuditChain(db, ORG, { + fromTimestamp: r2.timestamp, + afterId: r2.id, + previousExpectedHash: r2.integrityHash, + reapedBefore: r3.timestamp + 1, + }); + expect(result.valid).toBe(true); + expect(result.reanchored).toBe(true); + expect(result.verifiedCount).toBe(2); + expect(result.lastVerifiedId).toBe(r5.id); + expect(result.lastVerifiedHash).toBe(r5.integrityHash); + }); + + it('still reports a break when the anchor vanished INSIDE the retention window', async () => { + const chain = await buildChain(5, START); + const [, r2, r3, r4, r5] = chain; + if (!r2 || !r3 || !r4 || !r5) throw new Error('chain'); + const { db } = fakeDb((text) => (isRowPage(text) ? [r4, r5] : [])); + const result = await verifyAuditChain(db, ORG, { + fromTimestamp: r2.timestamp, + afterId: r2.id, + previousExpectedHash: r2.integrityHash, + // The cutoff is OLDER than the anchor: nothing legitimately deleted it. + reapedBefore: r2.timestamp - 1, + }); + expect(result.valid).toBe(false); + expect(result.reanchored).toBeUndefined(); + expect(result.firstBrokenAt).toMatchObject({ + logId: r4.id, + expected: r2.integrityHash, + actual: r3.integrityHash, + }); + }); + + it('never excuses a missing anchor when the org has no audit retention', async () => { + const chain = await buildChain(5, START); + const [, r2, , r4, r5] = chain; + if (!r2 || !r4 || !r5) throw new Error('chain'); + const { db } = fakeDb((text) => (isRowPage(text) ? [r4, r5] : [])); + const result = await verifyAuditChain(db, ORG, { + fromTimestamp: r2.timestamp, + afterId: r2.id, + previousExpectedHash: r2.integrityHash, + }); + expect(result.valid).toBe(false); + expect(result.firstBrokenAt?.logId).toBe(r4.id); + }); +}); + +describe('runScheduledIntegrityCheck — the tamper bell survives a failed write', () => { + beforeEach(() => { + vi.mocked(writeNotificationForOrgs).mockReset(); + vi.mocked(auditLogRetentionCutoff).mockReset(); + vi.mocked(auditLogRetentionCutoff).mockResolvedValue(null); + }); + + async function brokenOrg(): Promise<{ + rows: AuditLogRow[]; + anchor: AuditLogRow; + fingerprint: string; + }> { + const chain = await buildChain(3, 1_700_000_000_000); + const [r1, r2, r3] = chain; + if (!r1 || !r2 || !r3) throw new Error('chain'); + // row-2 tampered after the fact: its stored hash no longer recomputes. + const tampered: AuditLogRow = { ...r2, action: 'action.tampered' }; + return { + rows: [r1, tampered, r3], + anchor: r1, + fingerprint: `${tampered.id}:${tampered.integrityHash}`, + }; + } + + function progressDb( + rows: AuditLogRow[], + anchor: AuditLogRow, + stampedFingerprint: string | null, + ) { + return fakeDb((text) => { + if (isRowPage(text)) return rows; + if (text.startsWith('SELECT last_verified_ts')) { + return [ + { + lastVerifiedTs: anchor.timestamp, + lastVerifiedId: anchor.id, + lastVerifiedHash: anchor.integrityHash, + lastAlertedFingerprint: stampedFingerprint, + }, + ]; + } + return []; + }); + } + + it('stamps the break even when the bell write fails, and reports the miss', async () => { + const { rows, anchor, fingerprint } = await brokenOrg(); + vi.mocked(writeNotificationForOrgs).mockRejectedValueOnce( + new Error('bell down'), + ); + const { db, statements } = progressDb(rows, anchor, null); + const run = await runScheduledIntegrityCheck(db, ORG); + expect(run.broken).toBe(true); + expect(run.alerted).toBe(false); + const stamp = statements.find((statement) => + statement.text.startsWith('INSERT INTO app.audit_integrity_progress'), + ); + expect(stamp?.values).toContain(fingerprint); + expect(writeNotificationForOrgs).toHaveBeenCalledTimes(1); + }); + + it('re-asserts the bell on the next run instead of trusting the stamp', async () => { + const { rows, anchor, fingerprint } = await brokenOrg(); + // The previous run stamped the fingerprint but its bell never landed. + vi.mocked(writeNotificationForOrgs).mockResolvedValue(undefined); + const { db } = progressDb(rows, anchor, fingerprint); + const run = await runScheduledIntegrityCheck(db, ORG); + expect(run.broken).toBe(true); + expect(run.alerted).toBe(true); + expect(writeNotificationForOrgs).toHaveBeenCalledTimes(1); + expect( + vi.mocked(writeNotificationForOrgs).mock.calls[0]?.[1], + ).toMatchObject({ + organizationIds: [ORG], + severity: 'critical', + titleKey: 'auditIntegrityFailed', + dedupeKey: `audit-integrity:${fingerprint}`, + }); + }); + + it('asks retention for the cutoff only when there is an anchor to excuse', async () => { + const { rows, anchor } = await brokenOrg(); + vi.mocked(writeNotificationForOrgs).mockResolvedValue(undefined); + await runScheduledIntegrityCheck(progressDb(rows, anchor, null).db, ORG); + expect(auditLogRetentionCutoff).toHaveBeenCalledWith( + expect.anything(), + ORG, + ); + vi.mocked(auditLogRetentionCutoff).mockClear(); + const fresh = fakeDb((text) => (isRowPage(text) ? rows.slice(0, 1) : [])); + await runScheduledIntegrityCheck(fresh.db, ORG); + expect(auditLogRetentionCutoff).not.toHaveBeenCalled(); + }); +}); diff --git a/services/platform/backend/domains/audit_logs/verify.ts b/services/platform/backend/domains/audit_logs/verify.ts index 636e7022a3..87869b71fd 100644 --- a/services/platform/backend/domains/audit_logs/verify.ts +++ b/services/platform/backend/domains/audit_logs/verify.ts @@ -2,6 +2,7 @@ import type { Sql } from 'postgres'; import { computeAuditHash } from '../../core/lib/helpers/audit_hash.ts'; import { writeNotificationForOrgs } from '../notifications/service.ts'; +import { auditLogRetentionCutoff } from '../retention/service.ts'; import { rowToHashInput } from './hash-input.ts'; import type { AuditLogRow } from './types.ts'; @@ -11,7 +12,13 @@ import type { AuditLogRow } from './types.ts'; * * Anchoring: the walk trusts the FIRST REMAINING row's stored * `previous_hash` (retention deletes prefixes, so genesis is usually gone); - * a resume passes the previous page's hash instead. Scrubbed rows (GDPR + * a resume passes the previous page's hash instead — unless the resume + * anchor itself is gone AND old enough for the retention sweep to have + * reaped it (`reapedBefore`), in which case the walk re-anchors on the first + * surviving row exactly as a fresh walk would. An anchor that vanished + * INSIDE the retention window is not excused: nothing legitimately deletes + * an audit row there, so the linkage check runs against the resume hash and + * reports the break. Scrubbed rows (GDPR * Art 17) skip recompute — their content was intentionally blanked — but * still participate in linkage, and each one must be covered by an erasure * receipt; a scrubbed row with NO matching receipt counts into @@ -33,6 +40,9 @@ export interface VerifyChainResult { expected: string; actual: string; }; + /** The resume anchor had been reaped by retention; the walk re-anchored + * on the first surviving row's stored `previous_hash`. */ + reanchored?: boolean; } const VERIFY_COLUMNS = ` @@ -58,6 +68,11 @@ export async function verifyAuditChain( fromTimestamp?: number; afterId?: string; previousExpectedHash?: string; + /** Rows with `ts` below this may have been reaped by the org's retention + * sweep (its current cutoff). A resume anchor (`afterId` at + * `fromTimestamp`) that is missing AND older than this re-anchors the + * walk instead of reading as a break. */ + reapedBefore?: number; } = {}, ): Promise { const maxEntries = Math.min(Math.max(1, args.maxEntries ?? 1000), 5000); @@ -74,9 +89,24 @@ export async function verifyAuditChain( // Exact resume: rows up to and INCLUDING afterId are skipped, so // same-timestamp siblings the `>=` re-returned still get verified. let startIndex = 0; + let reanchored = false; if (afterId !== null) { const idx = rows.findIndex((row) => row.id === afterId); - if (idx !== -1) startIndex = idx + 1; + if (idx !== -1) { + startIndex = idx + 1; + } else if ( + args.reapedBefore !== undefined && + fromTs !== null && + fromTs < args.reapedBefore + ) { + // The anchor row is gone and was old enough for the sweep to have + // reaped it (a job outage longer than the window, or a backlog that + // outpaced the daily page). Its successors' linkage still proves the + // chain from the first survivor on; holding the walk to the reaped + // row's hash would report retention as tampering — a false verdict a + // broken pass never advances past. + reanchored = true; + } } const walk = rows.slice(startIndex, startIndex + maxEntries); const truncated = rows.length - startIndex > maxEntries; @@ -99,10 +129,13 @@ export async function verifyAuditChain( for (const row of covered) receiptCovered.add(row.id); } - let previousHash = args.previousExpectedHash ?? walk[0]?.previousHash ?? null; + let previousHash = reanchored + ? (walk[0]?.previousHash ?? null) + : (args.previousExpectedHash ?? walk[0]?.previousHash ?? null); let verifiedCount = 0; let unsignedScrubCount = 0; let lastVerified: AuditLogRow | undefined; + const reanchorFlag = reanchored ? { reanchored: true } : {}; for (const row of walk) { const expectedPrevious = previousHash ?? null; @@ -127,6 +160,7 @@ export async function verifyAuditChain( expected: expectedPrevious ?? '', actual: storedPrevious ?? '', }, + ...reanchorFlag, }; } if (row.piiScrubbed === true) { @@ -157,6 +191,7 @@ export async function verifyAuditChain( expected: recomputed, actual: row.integrityHash, }, + ...reanchorFlag, }; } } @@ -178,6 +213,7 @@ export async function verifyAuditChain( } : {}), unsignedScrubCount, + ...reanchorFlag, }; } @@ -237,16 +273,30 @@ export async function getIntegrityStatus( const SCHEDULED_PAGE = 2000; +export interface ScheduledIntegrityRun { + verified: number; + broken: boolean; + /** With `broken`: the admins' bell for this break is in place (written + * now, or already there from an earlier run). `false` means the write + * failed and the next run re-asserts it. */ + alerted?: boolean; + /** The resume anchor had been reaped by retention and the walk + * re-anchored on the first surviving row (see `verifyAuditChain`). */ + reanchored?: boolean; +} + /** * One org's scheduled incremental walk: resume from the progress row, - * verify up to a page, stamp progress. A break stamps the alert - * fingerprint and bells the org admins ONCE per fingerprint; a clean pass - * that re-covers the previously-broken region clears the alert. + * verify up to a page, stamp progress. A break stamps the alert fingerprint + * and bells the org admins — the bell is re-asserted on EVERY broken run + * and deduplicated per fingerprint, so a failed write is retried rather + * than lost; a clean pass that re-covers the previously-broken region + * clears the alert. */ export async function runScheduledIntegrityCheck( sql: Sql, organizationId: string, -): Promise<{ verified: number; broken: boolean }> { +): Promise { const progress = await sql< { lastVerifiedTs: number | null; @@ -262,6 +312,13 @@ export async function runScheduledIntegrityCheck( FROM app.audit_integrity_progress WHERE org_id = ${organizationId} `; const resume = progress[0]; + // The sweep may have reaped a resume anchor older than the org's audit + // retention window since the last walk; the walk needs that cutoff to + // tell a reaped anchor from a row that vanished inside the window. + const reapedBefore = + resume?.lastVerifiedId != null + ? await auditLogRetentionCutoff(sql, organizationId) + : null; const result = await verifyAuditChain(sql, organizationId, { maxEntries: SCHEDULED_PAGE, ...(resume?.lastVerifiedTs != null @@ -273,12 +330,18 @@ export async function runScheduledIntegrityCheck( ...(resume?.lastVerifiedHash != null ? { previousExpectedHash: resume.lastVerifiedHash } : {}), + ...(reapedBefore !== null ? { reapedBefore } : {}), }); + const reanchorFlag = result.reanchored === true ? { reanchored: true } : {}; + if (result.reanchored === true) { + console.warn( + `[audit-integrity] org ${organizationId}: resume anchor ${resume?.lastVerifiedId ?? '?'} was reaped by retention — re-anchored on the first surviving row`, + ); + } const now = Date.now(); if (!result.valid && result.firstBrokenAt !== undefined) { const fingerprint = `${result.firstBrokenAt.logId}:${result.firstBrokenAt.actual}`; - const alreadyAlerted = resume?.lastAlertedFingerprint === fingerprint; await sql` INSERT INTO app.audit_integrity_progress ( org_id, head_reached, updated_at_ms, last_alerted_fingerprint, @@ -294,31 +357,42 @@ export async function runScheduledIntegrityCheck( ELSE app.audit_integrity_progress.last_alerted_at_ms END `; - if (!alreadyAlerted) { - try { - const brokenLogId = result.firstBrokenAt.logId; - await sql.begin((tx) => - writeNotificationForOrgs(tx, { - organizationIds: [organizationId], - category: 'security', - severity: 'critical', - titleKey: 'auditIntegrityFailed', - bodyKey: 'auditIntegrityFailedDetails', - params: { - reason: `hash chain broken at log ${brokenLogId}`, - }, - link: { kind: 'audit-logs', logId: brokenLogId }, - dedupeKey: `audit-integrity:${fingerprint}`, - }), - ); - } catch (error) { - console.error( - `[audit-integrity] alert bell failed for org ${organizationId}:`, - error, - ); - } + // Re-assert the bell on EVERY broken run. The dedupe key makes a bell + // that is already there a no-op, so this costs one idempotent INSERT + // per run — and a bell whose write failed last time lands now. Gating + // it on the stamped fingerprint is what silently lost the alarm: the + // stamp landed, the bell did not, and every later run believed the + // admins had already been told. + let alerted = false; + try { + const brokenLogId = result.firstBrokenAt.logId; + await sql.begin((tx) => + writeNotificationForOrgs(tx, { + organizationIds: [organizationId], + category: 'security', + severity: 'critical', + titleKey: 'auditIntegrityFailed', + bodyKey: 'auditIntegrityFailedDetails', + params: { + reason: `hash chain broken at log ${brokenLogId}`, + }, + link: { kind: 'audit-logs', logId: brokenLogId }, + dedupeKey: `audit-integrity:${fingerprint}`, + }), + ); + alerted = true; + } catch (error) { + console.error( + `[audit-integrity] alert bell failed for org ${organizationId} — re-asserted on the next run:`, + error, + ); } - return { verified: result.verifiedCount, broken: true }; + return { + verified: result.verifiedCount, + broken: true, + alerted, + ...reanchorFlag, + }; } await sql` @@ -349,7 +423,7 @@ export async function runScheduledIntegrityCheck( last_alerted_fingerprint = NULL, last_alerted_at_ms = NULL `; - return { verified: result.verifiedCount, broken: false }; + return { verified: result.verifiedCount, broken: false, ...reanchorFlag }; } /** Every org with at least one audit row — the scheduled job's fleet. */ diff --git a/services/platform/backend/domains/retention/service.ts b/services/platform/backend/domains/retention/service.ts index ca98a5e7dd..742ac71591 100644 --- a/services/platform/backend/domains/retention/service.ts +++ b/services/platform/backend/domains/retention/service.ts @@ -891,6 +891,33 @@ async function sweepAutomationRuns(sql: Sql, org: OrgPolicy): Promise { return rows.length; } +/** The audit-log sweep's cutoff for one clamped policy: rows with `ts` + * below it are reap candidates; null when the category is off. */ +function auditLogCutoffFor(config: RetentionPolicyConfig): number | null { + if (config.auditLogEnabled !== true) return null; + const days = config.auditLogRetentionDays; + if (typeof days !== 'number' || days <= 0 || !Number.isFinite(days)) { + return null; + } + return Date.now() - days * DAY_MS; +} + +/** + * The oldest audit `ts` the org's sweep would still keep right now — the + * same clamped, cooldown-overlaid policy `sweepAuditLogs` enforces — or + * null when nothing legitimately deletes the org's audit rows (category + * off, no valid policy, bounds never applied). The scheduled integrity walk + * uses it to tell a resume anchor the sweep reaped (re-anchor) from a row + * that vanished inside the window (a break). + */ +export async function auditLogRetentionCutoff( + sql: Sql, + organizationId: string, +): Promise { + const policy = await clampedPolicyFor(sql, organizationId); + return policy === null ? null : auditLogCutoffFor(policy.config); +} + /** * Audit logs are PREFIX-ONLY: the hash chain anchors on the oldest * remaining row's stored `previous_hash`, so a mid-chain hole would break @@ -903,12 +930,10 @@ async function sweepAuditLogs( org: OrgPolicy, holds: ActiveHolds, ): Promise { - if (org.config.auditLogEnabled !== true) return 0; - const days = org.config.auditLogRetentionDays; - if (typeof days !== 'number' || days <= 0) return 0; + const cutoff = auditLogCutoffFor(org.config); + if (cutoff === null) return 0; // Refuse to delete the very table that records why the hold exists. if (holds.orgHeld) return 0; - const cutoff = Date.now() - days * DAY_MS; const candidates = await sql< { id: string; actorId: string | null; ts: number }[] >` diff --git a/services/platform/backend/integration-check.ts b/services/platform/backend/integration-check.ts index 1273d7f1bb..edce775f3f 100644 --- a/services/platform/backend/integration-check.ts +++ b/services/platform/backend/integration-check.ts @@ -28410,6 +28410,161 @@ async function checkAuditSurface( `written=${trickyWritten}${trickyError === '' ? '' : ` (${trickyError.slice(0, 90)})`}, normalized=${trickyNormalized}, verify=${trickyVerify.success ? `${trickyVerify.data.valid}/${trickyVerify.data.verifiedCount}` : 'ERR'}`, ); + // --- Alert durability: a failed bell write is retried, never lost ------ + // Break the chain on a fresh row while the bell table refuses the write: + // the break is stamped but no bell lands. Lift the block, run again: the + // bell lands. Before, the stamped fingerprint suppressed every retry — + // one transient failure and the admins were never told. + const bellProbesStartedAt = Date.now(); + const countIntegrityBells = async (): Promise => + Number( + ( + await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.notifications + WHERE org_id = ${orgId} AND title_key = 'auditIntegrityFailed' + ` + )[0]?.count ?? '0', + ); + const bellsBefore = await countIntegrityBells(); + await sql.begin((tx) => + createAuditLog(tx, { + organizationId: orgId, + actorId: 'itest', + actorType: 'system', + action: 'itest.bell_probe', + category: 'admin', + resourceType: 'itest', + status: 'success', + }), + ); + const bellProbeRows = await sql<{ id: string }[]>` + SELECT id FROM app.audit_logs + WHERE org_id = ${orgId} AND action = 'itest.bell_probe' + ORDER BY ts DESC LIMIT 1 + `; + const bellProbeId = bellProbeRows[0]?.id ?? ''; + // Walk the rows added above to head first, so the break is the only finding. + const preBreak = await runScheduledIntegrityCheck(sql, orgId); + await sql` + UPDATE app.audit_logs SET action = 'itest.bell_probe_tampered' + WHERE id = ${bellProbeId} + `; + await sql` + ALTER TABLE app.notifications + ADD CONSTRAINT itest_bell_blocked + CHECK (title_key <> 'auditIntegrityFailed') NOT VALID + `; + const blockedRun = await runScheduledIntegrityCheck(sql, orgId); + const bellsBlocked = await countIntegrityBells(); + await sql`ALTER TABLE app.notifications DROP CONSTRAINT itest_bell_blocked`; + const retriedRun = await runScheduledIntegrityCheck(sql, orgId); + const bellsRetried = await countIntegrityBells(); + await sql` + UPDATE app.audit_logs SET action = 'itest.bell_probe' + WHERE id = ${bellProbeId} + `; + const progressSnapshot = async (): Promise<{ + headReached: boolean; + fingerprint: string | null; + lastVerifiedId: string | null; + }> => { + const rows = await sql< + { + headReached: boolean; + fingerprint: string | null; + lastVerifiedId: string | null; + }[] + >` + SELECT head_reached AS "headReached", + last_alerted_fingerprint AS fingerprint, + last_verified_id AS "lastVerifiedId" + FROM app.audit_integrity_progress WHERE org_id = ${orgId} + `; + return ( + rows[0] ?? { headReached: false, fingerprint: null, lastVerifiedId: null } + ); + }; + const walkToCleanHead = async (): Promise => { + for (let i = 0; i < 10; i++) { + const run = await runScheduledIntegrityCheck(sql, orgId); + const status = await progressSnapshot(); + if (!run.broken && status.headReached && status.fingerprint === null) { + return true; + } + } + return false; + }; + const bellRecovered = await walkToCleanHead(); + record( + 'audit surface: integrity bell survives a failed write (re-asserted next run)', + !preBreak.broken && + blockedRun.broken && + blockedRun.alerted === false && + bellsBlocked === bellsBefore && + retriedRun.broken && + retriedRun.alerted === true && + bellsRetried === bellsBefore + 1 && + bellRecovered, + `blocked → broken=${blockedRun.broken}/alerted=${String(blockedRun.alerted)}/bells=${bellsBlocked} (want ${bellsBefore}), retried → alerted=${String(retriedRun.alerted)}/bells=${bellsRetried} (want ${bellsBefore + 1}), recovered=${bellRecovered}`, + ); + + // --- Reaped resume anchor: retention pruning is not tampering ---------- + // Point the progress row at a last-verified row that no longer exists, + // older than the org's audit-retention cutoff — what the sweep leaves + // behind when the walk falls behind the window. The walk must re-anchor + // on the first surviving row and reach head (before: a standing false + // tamper verdict + critical bell no later run could clear). The same gap + // INSIDE the window stays a break — nothing legitimately deletes there. + const { auditLogRetentionCutoff } = + await import('./domains/retention/service.ts'); + const auditCutoff = await auditLogRetentionCutoff(sql, orgId); + const firstRows = await sql<{ ts: number }[]>` + SELECT ts::float8 AS ts FROM app.audit_logs + WHERE org_id = ${orgId} ORDER BY ts ASC, id ASC LIMIT 1 + `; + const firstRowTs = firstRows[0]?.ts ?? Date.now(); + await sql` + UPDATE app.audit_integrity_progress SET + last_verified_ts = ${(auditCutoff ?? Date.now()) - 24 * 3_600_000}, + last_verified_id = 'itest-reaped-anchor', + last_verified_hash = 'itest-reaped-hash', + head_reached = false + WHERE org_id = ${orgId} + `; + const reanchorRun = await runScheduledIntegrityCheck(sql, orgId); + const reanchorHead = await walkToCleanHead(); + const reanchorProgress = await progressSnapshot(); + await sql` + UPDATE app.audit_integrity_progress SET + last_verified_ts = ${firstRowTs - 1}, + last_verified_id = 'itest-vanished-anchor', + last_verified_hash = 'itest-vanished-hash', + head_reached = false + WHERE org_id = ${orgId} + `; + const vanishedRun = await runScheduledIntegrityCheck(sql, orgId); + // Leave the org as the earlier probes did: a clean walk at head, no alert, + // only the bells that were there before this block. + await sql`DELETE FROM app.audit_integrity_progress WHERE org_id = ${orgId}`; + const cleanAgain = await walkToCleanHead(); + await sql` + DELETE FROM app.notifications + WHERE org_id = ${orgId} AND title_key = 'auditIntegrityFailed' + AND created_at_ms >= ${bellProbesStartedAt} + `; + record( + 'audit surface: reaped resume anchor re-anchors (retention is not tampering); in-window gap is a break', + auditCutoff !== null && + !reanchorRun.broken && + reanchorRun.reanchored === true && + reanchorHead && + reanchorProgress.lastVerifiedId !== 'itest-reaped-anchor' && + vanishedRun.broken && + vanishedRun.reanchored !== true && + cleanAgain, + `cutoff=${auditCutoff === null ? 'none' : 'set'}, reaped → broken=${reanchorRun.broken}/reanchored=${String(reanchorRun.reanchored)}/head=${reanchorHead}, vanished → broken=${vanishedRun.broken}, clean=${cleanAgain}`, + ); + // --- Export: CSV into the org store behind a presigned GET ------------ const exported = z .object({ storageId: z.string(), fileName: z.string(), url: z.string() }) From 63f32b5a42e877ed1506b777eb26bd82b7748b9e Mon Sep 17 00:00:00 2001 From: larryro <371767072@qq.com> Date: Thu, 3 Sep 2026 20:38:22 +0800 Subject: [PATCH 3/8] fix(platform): bound-check agentRuns retention from one field map Four hand-rolled copies paired retention categories with their policy fields, and they had drifted: the sweep's clamp and the policy save's bounds check omitted agentRuns (a value below the operator's floor saved fine and the sweep deleted by it), the bounds banner's impact preview included it (promising a clamp that never happened), and the shortening detector missed agentRuns and notifications (shortening either skipped the 7-day cooldown) while still listing a field the schema no longer has. One exported map, RETENTION_POLICY_FIELD_BY_CATEGORY, exhaustive over RETENTION_CATEGORIES by type, now drives all four. The clamp also skips a category the org's applied snapshot does not bound yet instead of crashing the sweep on it - the banner proposes the new category. Tests pin the map's completeness, the agentRuns clamp, preview/clamp parity over every category, and the detector; the integration check saves agentRuns below the floor and is refused with RETENTION_BELOW_FLOOR. --- .../retention_bounds_proposal.test.ts | 48 ++++++++++++++- .../governance/retention_bounds_proposal.ts | 25 ++------ .../core/governance/retention_floors.test.ts | 57 +++++++++++++++++- .../core/governance/retention_floors.ts | 59 ++++++++++++------- .../domains/governance/settings-tail.test.ts | 38 ++++++++++++ .../domains/governance/settings-tail.ts | 45 +++++++++----- .../backend/domains/retention/routes.ts | 24 +++----- .../platform/backend/integration-check.ts | 23 ++++++++ 8 files changed, 245 insertions(+), 74 deletions(-) create mode 100644 services/platform/backend/domains/governance/settings-tail.test.ts diff --git a/services/platform/backend/core/governance/retention_bounds_proposal.test.ts b/services/platform/backend/core/governance/retention_bounds_proposal.test.ts index 1b75dabe6e..b5084c7aee 100644 --- a/services/platform/backend/core/governance/retention_bounds_proposal.test.ts +++ b/services/platform/backend/core/governance/retention_bounds_proposal.test.ts @@ -1,6 +1,15 @@ import { describe, expect, it } from 'vitest'; -import { diffBounds } from './retention_bounds_proposal'; +import { + RETENTION_CATEGORIES, + type AppliedBoundsByCategory, +} from '../../../lib/shared/schemas/retention'; +import { buildImpactPreview, diffBounds } from './retention_bounds_proposal'; +import { + RETENTION_POLICY_FIELD_BY_CATEGORY, + clampConfigToBounds, + type EffectiveBoundDef, +} from './retention_floors'; describe('diffBounds', () => { it('returns an empty diff for identical snapshots', () => { @@ -121,3 +130,40 @@ describe('diffBounds', () => { expect(diff.every((d) => d.direction === 'tighten')).toBe(true); }); }); + +describe('buildImpactPreview ↔ clampConfigToBounds parity', () => { + it('promises exactly the clamp the sweep performs, for every category', () => { + // Every category bounded to [10, 20]; the stored policy sits below the + // floor on half of them and above the ceiling on the other half. + const proposed: AppliedBoundsByCategory = {}; + const bounds: Partial> = {}; + const stored: Record = {}; + RETENTION_CATEGORIES.forEach((category, index) => { + proposed[category] = { min: 10, max: 20 }; + bounds[category] = { + category, + min: 10, + max: 20, + default: 15, + unit: category.endsWith('Hours') ? 'hours' : 'days', + source: 'file', + minEnv: { envName: '', source: 'none', applied: false }, + maxEnv: { envName: '', source: 'none', applied: false }, + defaultEnv: { envName: '', source: 'none', applied: false }, + }; + stored[RETENTION_POLICY_FIELD_BY_CATEGORY[category]] = + index % 2 === 0 ? 1 : 99; + }); + const preview = buildImpactPreview(proposed, stored); + const clamped = clampConfigToBounds(bounds, stored); + expect(preview).toHaveLength(RETENTION_CATEGORIES.length); + for (const entry of preview) { + expect(clamped[entry.field]).toBe(entry.willClampTo); + expect(clamped[entry.field]).not.toBe(entry.current); + } + // The category whose preview used to lie: clamp and preview agree. + expect(clamped.agentRunsRetentionDays).toBe( + preview.find((entry) => entry.category === 'agentRuns')?.willClampTo, + ); + }); +}); diff --git a/services/platform/backend/core/governance/retention_bounds_proposal.ts b/services/platform/backend/core/governance/retention_bounds_proposal.ts index 986a946f38..ac5e7e96cb 100644 --- a/services/platform/backend/core/governance/retention_bounds_proposal.ts +++ b/services/platform/backend/core/governance/retention_bounds_proposal.ts @@ -1,25 +1,10 @@ import { RETENTION_CATEGORIES, type AppliedBoundsByCategory, - type RetentionCategory, } from '../../../lib/shared/schemas/retention'; import { isRecord } from '../../../lib/utils/type-utils'; -const POLICY_FIELD_BY_CATEGORY: Record = { - documents: 'documentsRetentionDays', - userTempHours: 'userTempRetentionHours', - agentTempHours: 'agentTempRetentionHours', - chatHistory: 'chatHistoryRetentionDays', - auditLog: 'auditLogRetentionDays', - workflowLog: 'workflowLogRetentionDays', - usageLedger: 'usageLedgerRetentionDays', - loginAttempt: 'loginAttemptRetentionDays', - chatFilterEvents: 'chatFilterEventsRetentionDays', - messageFeedback: 'messageFeedbackRetentionDays', - contacts: 'contactsRetentionDays', - externalConversations: 'externalConversationsRetentionDays', - notifications: 'notificationsRetentionDays', - agentRuns: 'agentRunsRetentionDays', -}; +import { RETENTION_POLICY_FIELD_BY_CATEGORY } from './retention_floors'; + export interface BoundDiffEntry { category: string; field: 'min' | 'max'; @@ -125,7 +110,9 @@ export function diffBounds( * For each diffed category, project what would happen to the org's * stored retention value if the proposal is applied. Reads * `governancePolicies.retention_policy.config` and clamps each - * `RetentionDays/Hours` field to the proposed `[min, max]`. + * `RetentionDays/Hours` field to the proposed `[min, max]` — + * the same field↔category pairing `clampConfigToBounds` enforces, so the + * preview can only promise what the sweep does. */ export function buildImpactPreview( proposed: AppliedBoundsByCategory, @@ -136,7 +123,7 @@ export function buildImpactPreview( for (const cat of RETENTION_CATEGORIES) { const bound = proposed[cat]; if (!bound) continue; - const field = POLICY_FIELD_BY_CATEGORY[cat]; + const field = RETENTION_POLICY_FIELD_BY_CATEGORY[cat]; const current = storedConfig[field]; if (typeof current !== 'number' || !Number.isFinite(current)) continue; const clamped = Math.min(Math.max(current, bound.min), bound.max); diff --git a/services/platform/backend/core/governance/retention_floors.test.ts b/services/platform/backend/core/governance/retention_floors.test.ts index 9253363d1f..8a71dbfa81 100644 --- a/services/platform/backend/core/governance/retention_floors.test.ts +++ b/services/platform/backend/core/governance/retention_floors.test.ts @@ -1,7 +1,11 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; -import type { RetentionDefaultsConfig } from '../../../lib/shared/schemas/retention'; import { + RETENTION_CATEGORIES, + type RetentionDefaultsConfig, +} from '../../../lib/shared/schemas/retention'; +import { + RETENTION_POLICY_FIELD_BY_CATEGORY, RetentionBoundsViolation, RetentionConfigMissingError, applyEnvTightening, @@ -376,3 +380,54 @@ describe('isRetentionDisabled', () => { expect(isRetentionDisabled()).toBe(false); }); }); + +describe('RETENTION_POLICY_FIELD_BY_CATEGORY — the one field↔category map', () => { + it('names a distinct policy field for every retention category', () => { + const fields = RETENTION_CATEGORIES.map( + (category) => RETENTION_POLICY_FIELD_BY_CATEGORY[category], + ); + expect(new Set(fields).size).toBe(RETENTION_CATEGORIES.length); + for (const field of fields) { + expect(field).toMatch(/Retention(Days|Hours)$/); + } + // The category the hand-rolled lists kept forgetting. + expect(RETENTION_POLICY_FIELD_BY_CATEGORY.agentRuns).toBe( + 'agentRunsRetentionDays', + ); + }); + + it('clampConfigToBounds clamps agentRuns like every other category', () => { + const bound: EffectiveBoundDefLike = { + category: 'agentRuns', + min: 30, + max: 365, + default: 90, + unit: 'days', + source: 'file', + minEnv: { envName: '', source: 'none', applied: false }, + maxEnv: { envName: '', source: 'none', applied: false }, + defaultEnv: { envName: '', source: 'none', applied: false }, + }; + const out = clampConfigToBounds( + { agentRuns: bound }, + { agentRunsRetentionDays: 7, agentRunsEnabled: true }, + ); + expect(out.agentRunsRetentionDays).toBe(30); + expect(out.agentRunsEnabled).toBe(true); + }); + + it('leaves a category the bounds snapshot does not cover untouched', () => { + // An applied snapshot that predates a category must not crash the sweep + // (nor clamp by a bound nobody applied); the banner proposes it instead. + const out = clampConfigToBounds( + {}, + { agentRunsRetentionDays: 7, documentsRetentionDays: 1 }, + ); + expect(out).toStrictEqual({ + agentRunsRetentionDays: 7, + documentsRetentionDays: 1, + }); + }); +}); + +type EffectiveBoundDefLike = Parameters[0]; diff --git a/services/platform/backend/core/governance/retention_floors.ts b/services/platform/backend/core/governance/retention_floors.ts index 137c88b6f6..398042dd6c 100644 --- a/services/platform/backend/core/governance/retention_floors.ts +++ b/services/platform/backend/core/governance/retention_floors.ts @@ -40,6 +40,7 @@ * to take effect; see docs/self-hosted/configuration/retention.md. */ +import type { RetentionPolicyConfig } from '../../../lib/shared/schemas/governance'; import { RETENTION_CATEGORIES, type RetentionCategory, @@ -340,45 +341,59 @@ export function clampToBounds( } /** - * Map of every retention-config field to its `RetentionCategory`. Drives - * `clampConfigToBounds` so an org's stored values never bypass freshly - * tightened bounds, even when the row was persisted under the old - * config. + * THE pairing of each retention category with the policy field that carries + * its value — the one place it lives. The sweep's clamp + * (`clampConfigToBounds`), the policy save's bounds check, the bounds + * banner's impact preview, and the shortening detector all derive from it, + * so a category can no longer be enforced in one and forgotten in another: + * `agentRuns` was missing from the clamp and the save check while the + * preview promised a clamp that never happened. Exhaustive over + * `RETENTION_CATEGORIES` by type; the completeness test pins it. */ -const CONFIG_FIELD_TO_CATEGORY: Record = { - documentsRetentionDays: 'documents', - userTempRetentionHours: 'userTempHours', - agentTempRetentionHours: 'agentTempHours', - chatHistoryRetentionDays: 'chatHistory', - auditLogRetentionDays: 'auditLog', - workflowLogRetentionDays: 'workflowLog', - usageLedgerRetentionDays: 'usageLedger', - loginAttemptRetentionDays: 'loginAttempt', - chatFilterEventsRetentionDays: 'chatFilterEvents', - messageFeedbackRetentionDays: 'messageFeedback', - contactsRetentionDays: 'contacts', - externalConversationsRetentionDays: 'externalConversations', - notificationsRetentionDays: 'notifications', +export const RETENTION_POLICY_FIELD_BY_CATEGORY: Record< + RetentionCategory, + keyof RetentionPolicyConfig +> = { + documents: 'documentsRetentionDays', + userTempHours: 'userTempRetentionHours', + agentTempHours: 'agentTempRetentionHours', + chatHistory: 'chatHistoryRetentionDays', + auditLog: 'auditLogRetentionDays', + workflowLog: 'workflowLogRetentionDays', + usageLedger: 'usageLedgerRetentionDays', + loginAttempt: 'loginAttemptRetentionDays', + chatFilterEvents: 'chatFilterEventsRetentionDays', + messageFeedback: 'messageFeedbackRetentionDays', + contacts: 'contactsRetentionDays', + externalConversations: 'externalConversationsRetentionDays', + notifications: 'notificationsRetentionDays', + agentRuns: 'agentRunsRetentionDays', }; /** * Clamp every retention-config field to current effective bounds. * Returns a shallow-cloned config; original is unchanged. Fields absent - * from the input or whose value is non-numeric are left untouched. + * from the input or whose value is non-numeric are left untouched, and so + * is a category the bounds map does not cover (an applied snapshot that + * predates the category — the bounds banner proposes it; the sweep must + * not crash on it). * * Pure: takes a pre-resolved `boundsByCategory` map. Build it via * `buildBoundsByCategory(orgConfig)` after loading the file at the IO * boundary. */ export function clampConfigToBounds>( - boundsByCategory: Record, + boundsByCategory: Partial>, config: C, ): C { const out = { ...config }; - for (const [field, category] of Object.entries(CONFIG_FIELD_TO_CATEGORY)) { + for (const category of RETENTION_CATEGORIES) { + const field = RETENTION_POLICY_FIELD_BY_CATEGORY[category]; + const bound = boundsByCategory[category]; const value = out[field]; + if (bound === undefined) continue; if (typeof value !== 'number' || !Number.isFinite(value)) continue; - const clamped = clampToBounds(boundsByCategory[category], value); + const clamped = clampToBounds(bound, value); if (clamped !== value) { // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- field exists on out (just read above) (out as Record)[field] = clamped; diff --git a/services/platform/backend/domains/governance/settings-tail.test.ts b/services/platform/backend/domains/governance/settings-tail.test.ts new file mode 100644 index 0000000000..bee4780b8a --- /dev/null +++ b/services/platform/backend/domains/governance/settings-tail.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest'; + +import { RETENTION_POLICY_FIELD_BY_CATEGORY } from '../../core/governance/retention_floors.ts'; +import { detectRetentionShortening } from './settings-tail.ts'; + +describe('detectRetentionShortening', () => { + it('sees a shortening in every bounded category, agentRuns and notifications included', () => { + for (const field of Object.values(RETENTION_POLICY_FIELD_BY_CATEGORY)) { + const summary = detectRetentionShortening( + { [field]: 30 }, + { [field]: 7 }, + ); + expect(summary, field).not.toBeNull(); + expect(summary).toContain('(30 → 7)'); + } + }); + + it('still counts the grace window and ignores a category the new config disabled', () => { + expect( + detectRetentionShortening( + { deletionGraceDays: 14 }, + { deletionGraceDays: 2 }, + ), + ).toBe('Reduced: deletion grace (14 → 2)'); + expect( + detectRetentionShortening( + { agentRunsRetentionDays: 30 }, + { agentRunsRetentionDays: 7, agentRunsEnabled: false }, + ), + ).toBeNull(); + expect( + detectRetentionShortening( + { agentRunsRetentionDays: 7 }, + { agentRunsRetentionDays: 30 }, + ), + ).toBeNull(); + }); +}); diff --git a/services/platform/backend/domains/governance/settings-tail.ts b/services/platform/backend/domains/governance/settings-tail.ts index f8d2b246a0..41df6bb1bb 100644 --- a/services/platform/backend/domains/governance/settings-tail.ts +++ b/services/platform/backend/domains/governance/settings-tail.ts @@ -5,7 +5,12 @@ import { DEFAULT_DSAR_GOVERNANCE, type DsarGovernanceConfig, } from '../../../lib/shared/schemas/governance.ts'; +import { + RETENTION_CATEGORIES, + type RetentionCategory, +} from '../../../lib/shared/schemas/retention.ts'; import { isLoosening } from '../../core/governance/dsar_policy.ts'; +import { RETENTION_POLICY_FIELD_BY_CATEGORY } from '../../core/governance/retention_floors.ts'; import { decryptSecret, encryptSecret } from '../../core/lib/secret_box.ts'; import { toJson } from '../../db/sql.ts'; import { writeGovernancePolicyFile } from '../../lib/governance-policy-write.ts'; @@ -292,26 +297,38 @@ export async function getPendingRetentionChange( return row; } +/** The summary's label per category (the pending-change banner's text). */ +const RETENTION_CATEGORY_LABELS: Record = { + documents: 'documents', + userTempHours: 'user temp files', + agentTempHours: 'agent temp files', + chatHistory: 'chat history', + auditLog: 'audit log', + workflowLog: 'workflow logs', + usageLedger: 'usage ledger', + loginAttempt: 'login attempts', + chatFilterEvents: 'chat filter events', + messageFeedback: 'message feedback', + contacts: 'contacts', + externalConversations: 'external conversations', + notifications: 'notifications', + agentRuns: 'agent runs', +}; + /** The 0.4 shortening detector — a category disabled in the new config - * deletes nothing, so its smaller number is not a shortening. */ + * deletes nothing, so its smaller number is not a shortening. Walks every + * bounded category through the ONE field↔category map (a hand list here + * missed `notifications` and `agentRuns`, so shortening either skipped the + * cooldown) plus the grace window. */ export function detectRetentionShortening( oldConfig: Record, newConfig: Record, ): string | null { const checks: Array<[string, string]> = [ - ['documentsRetentionDays', 'documents'], - ['userTempRetentionHours', 'user temp files'], - ['agentTempRetentionHours', 'agent temp files'], - ['chatHistoryRetentionDays', 'chat history'], - ['auditLogRetentionDays', 'audit log'], - ['workflowLogRetentionDays', 'workflow logs'], - ['usageLedgerRetentionDays', 'usage ledger'], - ['loginAttemptRetentionDays', 'login attempts'], - ['chatFilterEventsRetentionDays', 'chat filter events'], - ['promptTemplatesRetentionDays', 'prompt templates'], - ['messageFeedbackRetentionDays', 'message feedback'], - ['contactsRetentionDays', 'contacts'], - ['externalConversationsRetentionDays', 'external conversations'], + ...RETENTION_CATEGORIES.map((category): [string, string] => [ + RETENTION_POLICY_FIELD_BY_CATEGORY[category], + RETENTION_CATEGORY_LABELS[category], + ]), ['deletionGraceDays', 'deletion grace'], ]; const reduced: string[] = []; diff --git a/services/platform/backend/domains/retention/routes.ts b/services/platform/backend/domains/retention/routes.ts index 34991196b1..7bb0536b74 100644 --- a/services/platform/backend/domains/retention/routes.ts +++ b/services/platform/backend/domains/retention/routes.ts @@ -6,7 +6,7 @@ import { z } from 'zod'; import { retentionPolicyConfigSchema } from '../../../lib/shared/schemas/governance.ts'; import { hashAppliedBounds, - type RetentionCategory, + RETENTION_CATEGORIES, } from '../../../lib/shared/schemas/retention.ts'; import type { Auth } from '../../auth/auth.ts'; import { isAdminRole } from '../../auth/membership.ts'; @@ -22,6 +22,7 @@ import { RetentionBoundsViolation, applyEnvTighteningAll, isRetentionDisabled, + RETENTION_POLICY_FIELD_BY_CATEGORY, RetentionConfigMissingError, } from '../../core/governance/retention_floors.ts'; import { writeGovernancePolicyFile } from '../../lib/governance-policy-write.ts'; @@ -191,22 +192,11 @@ export function createRetentionRoutes(deps: { } const boundsByCategory = buildBoundsByCategory(orgConfig); const cfg = parsed.data; - const checks: Array<[RetentionCategory, unknown]> = [ - ['documents', cfg.documentsRetentionDays], - ['userTempHours', cfg.userTempRetentionHours], - ['agentTempHours', cfg.agentTempRetentionHours], - ['chatHistory', cfg.chatHistoryRetentionDays], - ['auditLog', cfg.auditLogRetentionDays], - ['workflowLog', cfg.workflowLogRetentionDays], - ['usageLedger', cfg.usageLedgerRetentionDays], - ['loginAttempt', cfg.loginAttemptRetentionDays], - ['chatFilterEvents', cfg.chatFilterEventsRetentionDays], - ['messageFeedback', cfg.messageFeedbackRetentionDays], - ['contacts', cfg.contactsRetentionDays], - ['externalConversations', cfg.externalConversationsRetentionDays], - ['notifications', cfg.notificationsRetentionDays], - ]; - for (const [category, value] of checks) { + // Every bounded category, from the ONE field↔category map — a hand + // list here once omitted `agentRuns`, so its value bypassed the floor + // the sweep would then delete by. + for (const category of RETENTION_CATEGORIES) { + const value = cfg[RETENTION_POLICY_FIELD_BY_CATEGORY[category]]; if (typeof value !== 'number') continue; assertWithinBounds(boundsByCategory[category], value); } diff --git a/services/platform/backend/integration-check.ts b/services/platform/backend/integration-check.ts index edce775f3f..8230b5eb2a 100644 --- a/services/platform/backend/integration-check.ts +++ b/services/platform/backend/integration-check.ts @@ -27829,6 +27829,29 @@ async function checkGovernanceSettingsTail( await get(`/api/app/retention/pending-change?orgId=${orgId}`) ).json(), ); + // F0: every bounded category is checked against its floor on save — the + // hand-rolled check list once omitted agentRuns, so a value below the + // operator's floor saved fine and the sweep deleted by it while the bounds + // banner's preview promised a clamp that never happened. + const agentRunsBelowFloor = await post( + `/api/app/retention/policy?orgId=${orgId}`, + { config: fullPolicy({ agentRunsRetentionDays: 0 }) }, + ); + const agentRunsRefusal = z + .object({ + error: z.string(), + data: z.object({ category: z.string(), bound: z.number() }).loose(), + }) + .loose() + .safeParse(await agentRunsBelowFloor.json()); + record( + 'retention policy save: agentRuns is bound-checked like every other category', + agentRunsBelowFloor.status === 400 && + agentRunsRefusal.success && + agentRunsRefusal.data.error === 'RETENTION_BELOW_FLOOR' && + agentRunsRefusal.data.data.category === 'agentRuns', + `save agentRuns=0 → ${agentRunsBelowFloor.status} ${agentRunsRefusal.success ? `${agentRunsRefusal.data.error}/${agentRunsRefusal.data.data.category}/floor=${agentRunsRefusal.data.data.bound}` : 'ERR'} (want 400 RETENTION_BELOW_FLOOR/agentRuns)`, + ); // F1: shorten messageFeedback 7 → 2. The file flips immediately; the // SWEEP must keep enforcing 7 until the cooldown elapses. const savedShort = z.object({ ok: z.boolean() }).safeParse( From d8eb65b7788815d73974e59d02c4f7cf6aa13267 Mon Sep 17 00:00:00 2001 From: larryro <371767072@qq.com> Date: Thu, 3 Sep 2026 20:44:01 +0800 Subject: [PATCH 4/8] fix(platform): let a Trash restore survive the next retention sweep Restoring an expired document or chat thread from the admin Trash flipped only its lifecycle column. The sweep ages documents by created_at_ms and chat threads by threads.updated_at_ms, neither of which a restore touches, so the very next nightly pass re-expired the row the admin had just brought back - and with a zero grace window hard-deleted it outright, silently. The restore already stamps status_changed_at_ms; the document and chat sweeps now age a live row by GREATEST(its age column, that stamp), so a restore restarts the retention window from the moment of the restore. No schema change: the stamp is the clock. Documented on the Trash page in en/de/fr. The integration check restores an expired document and an expired chat thread through the Trash API, sweeps with a 30-day window, and sees both still live while an untouched old control document expires. --- docs/de/platform/admin/governance/trash.md | 2 +- docs/en/platform/admin/governance/trash.md | 2 +- docs/fr/platform/admin/governance/trash.md | 2 +- .../backend/domains/retention/service.ts | 27 ++++- .../platform/backend/integration-check.ts | 109 ++++++++++++++++++ 5 files changed, 133 insertions(+), 9 deletions(-) diff --git a/docs/de/platform/admin/governance/trash.md b/docs/de/platform/admin/governance/trash.md index 24db395a90..e1139e5d2c 100644 --- a/docs/de/platform/admin/governance/trash.md +++ b/docs/de/platform/admin/governance/trash.md @@ -11,7 +11,7 @@ Um einen Chat-Verlauf-Thread wiederherzustellen, öffne **Einstellungen > Richtl ## Die zwei Status -**Verworfen** ist der normale Soft-Delete-Zustand. Das Aufbewahrungsfenster der Zeile ist abgelaufen, sie ist in den Papierkorb gewandert, und das Kulanzfenster tickt noch. Wiederherstellen führt die Zeile in ihre Quellliste zurück, ohne die Richtlinie zu überschreiben. +**Verworfen** ist der normale Soft-Delete-Zustand. Das Aufbewahrungsfenster der Zeile ist abgelaufen, sie ist in den Papierkorb gewandert, und das Kulanzfenster tickt noch. Wiederherstellen führt die Zeile in ihre Quellliste zurück, ohne die Richtlinie zu überschreiben. Das Aufbewahrungsfenster beginnt dabei von vorn — ein wiederhergestellter Chat-Thread oder ein wiederhergestelltes Dokument zählt ab dem Moment der Wiederherstellung, und der nächste Cleanup lässt die Zeile in Ruhe, statt sie erneut ablaufen zu lassen. **Abgelaufen** ist der zweite Zustand — das Kulanzfenster ist abgelaufen und die Zeile ist für die endgültige Löschung im nächsten Cleanup vorgemerkt. Wiederherstellen ist weiterhin möglich, aber es ist eine Überschreibung: der Dialog verlangt, dass du `restore` tippst, und das Audit-Log dokumentiert die Überschreibung mit deinem Namen. diff --git a/docs/en/platform/admin/governance/trash.md b/docs/en/platform/admin/governance/trash.md index e0df5429c6..1c02086fe5 100644 --- a/docs/en/platform/admin/governance/trash.md +++ b/docs/en/platform/admin/governance/trash.md @@ -11,7 +11,7 @@ To restore a chat history thread, open **Settings > Governance > Trash** and swi ## The two statuses -**Trashed** is the normal soft-delete state. The row's retention window elapsed, it moved to trash, and the grace window is still ticking. Restore returns the row to its source list with no policy override. +**Trashed** is the normal soft-delete state. The row's retention window elapsed, it moved to trash, and the grace window is still ticking. Restore returns the row to its source list with no policy override. The retention clock restarts at the restore — a restored chat thread or document counts from that moment, so the next cleanup pass leaves it alone instead of expiring it again. **Expired** is the second state — the grace window ran out and the row is queued for permanent deletion at the next cleanup. Restore is still possible but is an override: the dialog asks you to type `restore` and the audit log records the override with your name. diff --git a/docs/fr/platform/admin/governance/trash.md b/docs/fr/platform/admin/governance/trash.md index db0aaaeddb..08202c9a6b 100644 --- a/docs/fr/platform/admin/governance/trash.md +++ b/docs/fr/platform/admin/governance/trash.md @@ -11,7 +11,7 @@ Pour restaurer un thread d'historique de chat, ouvre **Paramètres > Gouvernance ## Les deux statuts -**Mis à la corbeille** est l'état soft-delete normal. La fenêtre de rétention de la ligne a expiré, elle s'est déplacée à la corbeille, et la fenêtre de grâce tourne encore. Restaurer ramène la ligne dans sa liste source sans dépasser la politique. +**Mis à la corbeille** est l'état soft-delete normal. La fenêtre de rétention de la ligne a expiré, elle s'est déplacée à la corbeille, et la fenêtre de grâce tourne encore. Restaurer ramène la ligne dans sa liste source sans dépasser la politique. La fenêtre de rétention repart de zéro au moment de la restauration — un thread de chat ou un document restauré compte à partir de ce moment, et le prochain nettoyage le laisse tranquille au lieu de le faire expirer à nouveau. **Expiré** est le second état — la fenêtre de grâce s'est écoulée et la ligne est en file pour suppression définitive au prochain nettoyage. Restaurer reste possible mais est un dépassement : la boîte de dialogue te demande de taper `restore` et le journal d'audit enregistre le dépassement avec ton nom. diff --git a/services/platform/backend/domains/retention/service.ts b/services/platform/backend/domains/retention/service.ts index 742ac71591..a96eb90f82 100644 --- a/services/platform/backend/domains/retention/service.ts +++ b/services/platform/backend/domains/retention/service.ts @@ -477,7 +477,11 @@ async function sweepDocuments( const protectedIds = [...holds.userMembershipIds]; let processed = 0; - // Pass A (grace > 0): flip active expired rows into the admin Trash. + // Pass A (grace > 0): flip active expired rows into the admin Trash. A + // document's age is its creation — or its last lifecycle change, when a + // restore from the Trash stamped one: a restore restarts the retention + // clock, or the very next sweep re-expires the row the admin just brought + // back (and, with no grace, hard-deletes it outright). if (graceDays > 0) { const flipped = await sql<{ id: string }[]>` UPDATE app.documents SET @@ -485,7 +489,8 @@ async function sweepDocuments( WHERE id IN ( SELECT id FROM app.documents WHERE org_id = ${org.organizationId} AND lifecycle_status IS NULL - AND created_at_ms < ${cutoff} + AND GREATEST(created_at_ms, coalesce(status_changed_at_ms, 0)) + < ${cutoff} AND (${protectedIds.length === 0} OR created_by <> ALL(${protectedIds})) LIMIT ${BATCH_LIMIT} @@ -533,7 +538,9 @@ async function sweepDocuments( history_files AS "historyFiles" FROM app.documents WHERE org_id = ${org.organizationId} - AND ((lifecycle_status IS NULL AND created_at_ms < ${cutoff}) + AND ((lifecycle_status IS NULL + AND GREATEST(created_at_ms, coalesce(status_changed_at_ms, 0)) + < ${cutoff}) OR lifecycle_status IN ('trashed', 'expired')) LIMIT ${BATCH_LIMIT} `; @@ -604,14 +611,19 @@ async function sweepChatHistory( let processed = 0; // Pass A (grace > 0): expire active chat threads past the cutoff — they - // land in the admin Trash for the grace window. + // land in the admin Trash for the grace window. A thread's age is its + // last activity — or its last lifecycle change, when a restore from the + // Trash stamped one: the restore restarts the retention clock, or the + // next sweep re-expires the thread the admin just brought back. if (graceDays > 0) { const candidates = await sql<{ threadId: string; userId: string }[]>` SELECT tm.thread_id AS "threadId", tm.user_id AS "userId" FROM app.thread_metadata tm JOIN app.threads t ON t.id = tm.thread_id WHERE tm.org_id = ${org.organizationId} AND tm.chat_type = 'chat' - AND tm.status = 'active' AND t.updated_at_ms < ${cutoff} + AND tm.status = 'active' + AND GREATEST(t.updated_at_ms, coalesce(tm.status_changed_at_ms, 0)) + < ${cutoff} LIMIT ${BATCH_LIMIT} `; for (const thread of candidates) { @@ -645,7 +657,10 @@ async function sweepChatHistory( JOIN app.threads t ON t.id = tm.thread_id WHERE tm.org_id = ${org.organizationId} AND tm.chat_type = 'chat' AND tm.branch_root_id IS NULL - AND ((tm.status = 'active' AND t.updated_at_ms < ${cutoff}) + AND ((tm.status = 'active' + AND GREATEST(t.updated_at_ms, + coalesce(tm.status_changed_at_ms, 0)) + < ${cutoff}) OR tm.status IN ('trashed', 'expired')) LIMIT ${BATCH_LIMIT} `; diff --git a/services/platform/backend/integration-check.ts b/services/platform/backend/integration-check.ts index 8230b5eb2a..35345a2fe3 100644 --- a/services/platform/backend/integration-check.ts +++ b/services/platform/backend/integration-check.ts @@ -8492,6 +8492,115 @@ async function checkGovernance( const governance = await import('./domains/governance/service.ts'); const orgConfig = await import('./lib/org-config.ts'); + const restoreViaTrash = (body: unknown): Promise => + fetch(`${base}/api/app/governance/trash/restore?orgId=${orgId}`, { + method: 'POST', + headers: { 'content-type': 'application/json', cookie, origin: base }, + body: JSON.stringify(body), + }); + // A restore must SURVIVE the next sweep. An expired document (aged by its + // creation) and an expired chat thread (aged by its last activity) that an + // admin restored from the Trash used to be re-expired by the very next + // cleanup pass — and, with no grace, hard-deleted — because the restore + // stamped neither clock. The restore's lifecycle stamp now restarts the + // retention window; an untouched old row (the control) still expires. + const longAgo = Date.now() - 400 * 24 * 3_600_000; + const recently = Date.now() - 40 * 24 * 3_600_000; + const restoreOwner = 'itest-restore-owner'; + const restoredDocRows = await sql<{ id: string }[]>` + INSERT INTO app.documents ( + org_id, title, created_by, created_at_ms, updated_at_ms, + lifecycle_status, status_changed_at_ms + ) VALUES ( + ${orgId}, 'Restore survives the sweep', ${restoreOwner}, ${longAgo}, + ${longAgo}, 'expired', ${recently} + ) RETURNING id + `; + const restoredDocId = restoredDocRows[0]?.id ?? ''; + const controlDocRows = await sql<{ id: string }[]>` + INSERT INTO app.documents ( + org_id, title, created_by, created_at_ms, updated_at_ms + ) VALUES ( + ${orgId}, 'Control: still expires', ${restoreOwner}, ${longAgo}, + ${longAgo} + ) RETURNING id + `; + const controlDocId = controlDocRows[0]?.id ?? ''; + const restoredThreadRows = await sql<{ id: string }[]>` + INSERT INTO app.threads (org_id, user_id, title, kind, created_at_ms, + updated_at_ms) + VALUES (${orgId}, ${restoreOwner}, 'Restore survives the sweep', 'chat', + ${longAgo}, ${longAgo}) + RETURNING id + `; + const restoredThreadId = restoredThreadRows[0]?.id ?? ''; + await sql` + INSERT INTO app.thread_metadata ( + thread_id, org_id, user_id, chat_type, status, status_changed_at_ms, + created_at_ms + ) VALUES ( + ${restoredThreadId}, ${orgId}, ${restoreOwner}, 'chat', 'expired', + ${recently}, ${longAgo} + ) + `; + const restoreDoc = await restoreViaTrash({ + resourceType: 'document', + id: restoredDocId, + }); + const restoreThread = await restoreViaTrash({ + resourceType: 'chatThread', + id: restoredThreadId, + }); + const { sweepOrgPhase2: sweepAfterRestore } = + await import('./domains/retention/service.ts'); + const { loadActiveHolds: holdsAfterRestore } = + await import('./domains/legal_holds/service.ts'); + const holdsNow = await holdsAfterRestore(sql, orgId); + await sweepAfterRestore( + sql, + { + organizationId: orgId, + config: { + documentsEnabled: true, + documentsRetentionDays: 30, + chatHistoryEnabled: true, + chatHistoryRetentionDays: 30, + deletionGraceDays: 7, + }, + }, + holdsNow, + ); + const docsAfterSweep = await sql< + { id: string; lifecycleStatus: string | null }[] + >` + SELECT id, lifecycle_status AS "lifecycleStatus" FROM app.documents + WHERE id IN (${restoredDocId}, ${controlDocId}) + `; + const restoredDocState = docsAfterSweep.find( + (row) => row.id === restoredDocId, + )?.lifecycleStatus; + const controlDocState = docsAfterSweep.find( + (row) => row.id === controlDocId, + )?.lifecycleStatus; + const threadAfterSweep = await sql<{ status: string }[]>` + SELECT status FROM app.thread_metadata WHERE thread_id = ${restoredThreadId} + `; + const restoredThreadState = threadAfterSweep[0]?.status; + await sql`DELETE FROM app.thread_metadata WHERE thread_id = ${restoredThreadId}`; + await sql`DELETE FROM app.threads WHERE id = ${restoredThreadId}`; + await sql` + DELETE FROM app.documents WHERE id IN (${restoredDocId}, ${controlDocId}) + `; + record( + 'governance trash: a restored expired document + chat thread survive the next sweep', + restoreDoc.ok && + restoreThread.ok && + restoredDocState === null && + controlDocState === 'expired' && + restoredThreadState === 'active', + `restore doc → ${restoreDoc.status}, thread → ${restoreThread.status}; after sweep (orgHeld=${holdsNow.orgHeld}): doc=${String(restoredDocState)} (want null), control=${String(controlDocState)} (want expired), thread=${String(restoredThreadState)} (want active)`, + ); + // The chat turns and tool dispatches already run accumulated buckets. const buckets = await governance.readUsageBuckets(sql, { organizationId: orgId, From 4fc31cc9e3f1f74471b11fd0eaae0a414cb0fd43 Mon Sep 17 00:00:00 2001 From: larryro <371767072@qq.com> Date: Thu, 3 Sep 2026 20:49:52 +0800 Subject: [PATCH 5/8] fix(platform): apply a matured DSAR loosening server-side A staged DSAR-policy loosening wrote only its pending row; the file flip happened lazily inside the policy page's read once effective_at passed. The erasure lane's enforcement read went straight to the file and never applied a matured change, so the loosening the UI announced as "effective at " stayed inert until an admin happened to reopen the editor - the owner waited out the 24h grace and filing still enforced the old cooling-off, dual-approval, or daily limit. The apply is now one seam, applyMaturedDsarPolicyChange (file write, then DELETE ... RETURNING as the claim, then a system audit row policy.dsar_governance_loosening_applied), and every reader that must see the EFFECTIVE policy runs it first: the erasure lane's readEffectiveDsarPolicy (filing + approval), the editor's read, and a new 5-minute schedule, governance.apply_dsar_policy_changes, that keeps the promise even when nobody opens the page or files a request. Idempotent and race-safe; works inside a caller's transaction or opens its own. The integration check ages a staged loosening past its grace and sees the sweep apply it (row gone, audit row written) before any page read, then stages another and sees the enforcement read apply it on its own. --- .../backend/domains/erasure/service.ts | 21 ++- .../domains/governance/settings-tail.ts | 157 ++++++++++++++---- .../platform/backend/integration-check.ts | 49 ++++++ services/platform/backend/jobs/schedules.ts | 4 + services/platform/backend/jobs/task-list.ts | 10 ++ services/platform/backend/jobs/tasks.ts | 7 + 6 files changed, 211 insertions(+), 37 deletions(-) diff --git a/services/platform/backend/domains/erasure/service.ts b/services/platform/backend/domains/erasure/service.ts index 3a9f170475..6e03887270 100644 --- a/services/platform/backend/domains/erasure/service.ts +++ b/services/platform/backend/domains/erasure/service.ts @@ -18,6 +18,7 @@ import { import { checkOrganizationRateLimit } from '../../lib/rate-limit.ts'; import { emitHintInTx } from '../../realtime/outbox.ts'; import { createAuditLog } from '../audit_logs/service.ts'; +import { applyMaturedDsarPolicyChange } from '../governance/settings-tail.ts'; import { loadActiveHolds } from '../legal_holds/service.ts'; import { writeNotificationForOrgs } from '../notifications/service.ts'; @@ -78,7 +79,21 @@ export function isValidErasureReasonCode(code: string): boolean { ); } -async function dsarPolicy(sql: Sql | TransactionSql, organizationId: string) { +/** + * The DSAR policy the erasure lane ENFORCES. A staged loosening whose grace + * has elapsed applies here first, so filing and approval see the policy the + * owner was told would be in force at that time — not whatever the last + * visit to the policy page happened to leave behind. + */ +export async function readEffectiveDsarPolicy( + sql: Sql | TransactionSql, + organizationId: string, +): Promise<{ + coolingOffHours: number; + dailyLimitPerAdmin: number; + requireDualApproval: boolean; +}> { + await applyMaturedDsarPolicyChange(sql, organizationId); const config = await readGovernancePolicyForOrg( sql, organizationId, @@ -142,7 +157,7 @@ export async function requestErasure( if (args.targetUserId === args.actorId) { return deny('self_deletion_forbidden'); } - const policy = await dsarPolicy(sql, args.organizationId); + const policy = await readEffectiveDsarPolicy(sql, args.organizationId); try { await checkOrganizationRateLimit( sql, @@ -1409,7 +1424,7 @@ export async function confirmAndScheduleErasure( ); } - const policy = await dsarPolicy(tx, row.organizationId); + const policy = await readEffectiveDsarPolicy(tx, row.organizationId); const effectiveAt = Date.now() + policy.coolingOffHours * HOUR_MS; await tx` UPDATE app.gdpr_erasure_requests SET effective_at_ms = ${effectiveAt} diff --git a/services/platform/backend/domains/governance/settings-tail.ts b/services/platform/backend/domains/governance/settings-tail.ts index 41df6bb1bb..c930a815b1 100644 --- a/services/platform/backend/domains/governance/settings-tail.ts +++ b/services/platform/backend/domains/governance/settings-tail.ts @@ -24,9 +24,11 @@ import { createAuditLog } from '../audit_logs/service.ts'; /** * The governance settings TAIL: legal matters (grouping for holds), the * retention-shortening cooldown store, the DSAR owner-only grace flow - * (loosening staged 24h; APPLIED LAZILY on the next read past its - * effective time — no cron), the guardrails secret store, and the - * chat-filter event listing the Security page reads. + * (loosening staged 24h; applied once its effective time passes — by the + * erasure lane's enforcement read, by the editor's read, and by the + * 5-minute `governance.apply_dsar_policy_changes` sweep, so the change + * takes effect whether or not anyone opens the page), the guardrails + * secret store, and the chat-filter event listing the Security page reads. */ export class GovernanceTailError extends Error { @@ -457,8 +459,120 @@ async function readDsarConfig( return parsed.success ? parsed.data : DEFAULT_DSAR_GOVERNANCE; } -/** The editor's read; a pending change past its grace APPLIES here (write - * the file, drop the row) before the answer — the lazy-apply seam. */ +interface DsarPendingRow { + id: string; + pendingConfig: Record; + effectiveAt: number; + proposedBy: string; + proposedByEmail: string | null; + proposedAt: number; +} + +const DSAR_PENDING_COLUMNS = ` + id, pending_config AS "pendingConfig", + effective_at_ms::float8 AS "effectiveAt", proposed_by AS "proposedBy", + proposed_by_email AS "proposedByEmail", proposed_at_ms::float8 AS "proposedAt" +`; + +/** Drop the pending row (the claim — whoever gets it back records the + * apply) and audit the change becoming effective. */ +async function claimMaturedDsarChange( + tx: TransactionSql, + organizationId: string, + row: DsarPendingRow, + applied: DsarGovernanceConfig | null, +): Promise { + const claimed = await tx<{ id: string }[]>` + DELETE FROM app.dsar_policy_pending_changes + WHERE id = ${row.id} RETURNING id + `; + if (!claimed[0] || applied === null) return false; + await createAuditLog(tx, { + organizationId, + actorId: 'system', + actorType: 'system', + action: 'policy.dsar_governance_loosening_applied', + category: 'security', + resourceType: 'governance_policy', + resourceId: 'dsar_governance', + newState: { + config: applied, + effectiveAt: row.effectiveAt, + proposedBy: row.proposedBy, + }, + status: 'success', + }); + await emitHintInTx(tx, { + orgId: organizationId, + entity: 'governance_policy', + entityId: 'dsar_governance', + }); + return true; +} + +/** + * Apply the org's staged DSAR loosening once its grace has elapsed: write + * the policy file, drop the pending row, audit it. Every reader that must + * see the EFFECTIVE policy calls this first — the erasure lane's + * enforcement read, the editor's read, and the scheduled sweep — so the + * change the owner was told would be in force at is in force then, + * not whenever someone next happens to open the policy page. Idempotent and + * race-safe: two racers write the same file; the DELETE decides who records + * the audit row. Works inside a caller's transaction (postgres.js marks one + * with `savepoint`) or opens its own. Returns true when a change applied. + */ +export async function applyMaturedDsarPolicyChange( + db: Sql | TransactionSql, + organizationId: string, +): Promise { + const rows = await db` + SELECT ${db.unsafe(DSAR_PENDING_COLUMNS)} + FROM app.dsar_policy_pending_changes + WHERE org_id = ${organizationId} AND effective_at_ms <= ${Date.now()} + LIMIT 1 + `; + const row = rows[0]; + if (row === undefined) return false; + const parsed = dsarGovernanceConfigSchema.safeParse(row.pendingConfig); + const orgSlug = await resolveOrgSlug(db, organizationId); + let applied: DsarGovernanceConfig | null = null; + if (parsed.success && orgSlug !== null) { + await writeGovernancePolicyFile(orgSlug, 'dsar_governance', parsed.data); + applied = parsed.data; + } else { + console.warn( + `[governance] dropping a pending DSAR policy change for org ${organizationId} that cannot apply:`, + parsed.success ? 'organization not found' : parsed.error.message, + ); + } + const claim = (tx: TransactionSql): Promise => + claimMaturedDsarChange(tx, organizationId, row, applied); + return 'savepoint' in db ? claim(db) : db.begin(claim); +} + +/** The scheduled twin of the lazy apply: every org whose staged loosening + * has matured gets it applied now. One org's failure never starves the rest. */ +export async function applyMaturedDsarPolicyChanges(sql: Sql): Promise { + const due = await sql<{ orgId: string }[]>` + SELECT DISTINCT org_id AS "orgId" FROM app.dsar_policy_pending_changes + WHERE effective_at_ms <= ${Date.now()} + `; + let applied = 0; + for (const { orgId } of due) { + try { + if (await applyMaturedDsarPolicyChange(sql, orgId)) applied += 1; + } catch (error) { + console.error( + `[governance] DSAR policy apply failed for org ${orgId}:`, + error, + ); + } + } + return applied; +} + +/** The editor's read: a matured pending change applies first (as it does + * on every other path), then the effective config and what is still staged. */ export async function getDsarPolicyForUi( sql: Sql, auth: { organizationId: string; role: string }, @@ -467,39 +581,14 @@ export async function getDsarPolicyForUi( pending: DsarPendingView | null; callerIsOwner: boolean; }> { - const rows = await sql< - { - id: string; - pendingConfig: Record; - effectiveAt: number; - proposedBy: string; - proposedByEmail: string | null; - proposedAt: number; - }[] - >` - SELECT id, pending_config AS "pendingConfig", - effective_at_ms::float8 AS "effectiveAt", - proposed_by AS "proposedBy", - proposed_by_email AS "proposedByEmail", - proposed_at_ms::float8 AS "proposedAt" + await applyMaturedDsarPolicyChange(sql, auth.organizationId); + const rows = await sql` + SELECT ${sql.unsafe(DSAR_PENDING_COLUMNS)} FROM app.dsar_policy_pending_changes WHERE org_id = ${auth.organizationId} LIMIT 1 `; - let pendingRow: (typeof rows)[number] | undefined = rows[0]; - if (pendingRow !== undefined && pendingRow.effectiveAt <= Date.now()) { - const parsed = dsarGovernanceConfigSchema.safeParse( - pendingRow.pendingConfig, - ); - const orgSlug = await resolveOrgSlug(sql, auth.organizationId); - if (parsed.success && orgSlug !== null) { - await writeGovernancePolicyFile(orgSlug, 'dsar_governance', parsed.data); - } - await sql` - DELETE FROM app.dsar_policy_pending_changes WHERE id = ${pendingRow.id} - `; - pendingRow = undefined; - } + const pendingRow = rows[0]; const config = await readDsarConfig(sql, auth.organizationId); let pending: DsarPendingView | null = null; if (pendingRow !== undefined) { diff --git a/services/platform/backend/integration-check.ts b/services/platform/backend/integration-check.ts index 35345a2fe3..169a2d0736 100644 --- a/services/platform/backend/integration-check.ts +++ b/services/platform/backend/integration-check.ts @@ -27739,6 +27739,23 @@ async function checkGovernanceSettingsTail( SET effective_at_ms = ${Date.now() - 1000} WHERE org_id = ${orgId} `; + // The matured change applies SERVER-SIDE — here through the scheduled + // sweep, further down through the erasure lane's enforcement read — not + // only when someone opens the policy page. Before, only the page read + // applied it: the owner waited out the grace and filing still enforced + // the old policy until an admin happened to revisit the editor. + const { applyMaturedDsarPolicyChanges } = + await import('./domains/governance/settings-tail.ts'); + const dsarSwept = await applyMaturedDsarPolicyChanges(sql); + const dsarPendingAfterSweep = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.dsar_policy_pending_changes + WHERE org_id = ${orgId} + `; + const dsarAppliedAudits = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.audit_logs + WHERE org_id = ${orgId} + AND action = 'policy.dsar_governance_loosening_applied' + `; const dsarApplied = z .object({ config: z.object({ coolingOffHours: z.number() }).loose(), @@ -27776,6 +27793,38 @@ async function checkGovernanceSettingsTail( `read=${dsarRead.success ? dsarRead.data.config.coolingOffHours : 'ERR'} (want 1), tighten=${tightened.success ? tightened.data.staged : 'ERR'} (want false), loosen=${loosened.success ? loosened.data.staged : 'ERR'} (want true), dupPending=${pendingBlocks.status} (want 400), pendingRead=${dsarPendingRead.success ? `${dsarPendingRead.data.config.coolingOffHours}/${dsarPendingRead.data.pending.config.coolingOffHours}` : 'ERR'}, cancel=${dsarCancelled.success}, after=${dsarAfterCancel.success}, applied=${dsarApplied.success ? dsarApplied.data.config.coolingOffHours : 'ERR'} (want 1), leftover=${dsarPendingGone[0]?.count} (want 0)`, ); + // The enforcement read applies a matured change on its own as well: stage + // one more loosening (a higher daily limit), age it past its grace, and + // read the policy the way filing does — no page view in between. + await post(`/api/app/governance/dsar/policy?orgId=${orgId}`, { + config: { + coolingOffHours: 1, + requireDualApproval: false, + dailyLimitPerAdmin: 6, + }, + }); + await sql` + UPDATE app.dsar_policy_pending_changes + SET effective_at_ms = ${Date.now() - 1000} + WHERE org_id = ${orgId} + `; + const { readEffectiveDsarPolicy } = + await import('./domains/erasure/service.ts'); + const enforcedDsar = await readEffectiveDsarPolicy(sql, orgId); + const dsarPendingAfterRead = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.dsar_policy_pending_changes + WHERE org_id = ${orgId} + `; + record( + 'governance tail: a matured DSAR loosening applies server-side (sweep + enforcement read), not on page open', + dsarSwept >= 1 && + dsarPendingAfterSweep[0]?.count === '0' && + dsarAppliedAudits[0]?.count === '1' && + enforcedDsar.dailyLimitPerAdmin === 6 && + dsarPendingAfterRead[0]?.count === '0', + `sweep applied=${dsarSwept} (want ≥1), pending after sweep=${dsarPendingAfterSweep[0]?.count} (want 0), applied-audit rows=${dsarAppliedAudits[0]?.count} (want 1), enforcement read limit=${enforcedDsar.dailyLimitPerAdmin} (want 6), pending after read=${dsarPendingAfterRead[0]?.count} (want 0)`, + ); + // --- D. Moderation secret + offline test stub --------------------------- const statusEmpty = z .object({ masked: z.null() }) diff --git a/services/platform/backend/jobs/schedules.ts b/services/platform/backend/jobs/schedules.ts index b3889dd833..44bb1a8c15 100644 --- a/services/platform/backend/jobs/schedules.ts +++ b/services/platform/backend/jobs/schedules.ts @@ -35,6 +35,10 @@ const SCHEDULES: CronSchedule[] = [ { name: 'knowledge.reconcile_corpus', cron: '45 4 * * *' }, { name: 'audit.integrity_check', cron: '30 4 * * *' }, { name: 'governance.effect_hold_releases', cron: '15 4 * * *' }, + // A staged DSAR-policy loosening promises "effective at