Skip to content

Commit 2c45aec

Browse files
voidstackloopclaude
andcommitted
rust: retention/cap enforcement for the SQLite audit backend
Closes the last documented gap from the SQLite cutover slice. Two new Rust functions, trim_audit_events_to_cap and purge_audit_events_older_than - both single indexed DELETEs, cheap regardless of table size (unlike the JSON backend's full-file-rewrite equivalent, which is why that backend needed a soft cap with batching in the first place). audit-log-store.ts's SQLite write path now enforces the same MAX_EVENTS/TRIM_BATCH cap and age-based retention as the JSON backend, with equivalent semantics (purge runs on every write when retention is configured; trim still batches to avoid a DELETE per insert, though a single trim isn't expensive here either). Found and fixed a real bug in the process: get_last_audit_event_hash crashed on a legacy-shaped last event (NULL event_hash column) - none of the original tests exercised that case. A new TypeScript retention integration test seeded exactly that shape and caught it. Fixed by reading the column as Option<String> and flattening the two independently-nullable layers instead of assuming non-null. New dedicated Rust test covers it directly. 7 new tests (4 Rust: trim/purge correctness and no-op cases, 1 Rust bug regression test, 2 TypeScript bridge tests) plus 1 new integration test proving retention purge works end-to-end through the real SQLite path. Verified through the real built addon, with and without it present, plus full e2e suite (10/10) and app unit suite (584/585, 1 intentional skip). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 57bdf1d commit 2c45aec

6 files changed

Lines changed: 284 additions & 26 deletions

File tree

app/src/audit-log-store.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,26 @@ describe("audit-log-store", () => {
362362
settingsStore.saveSettings({ auditLogBackend: undefined });
363363
expect(auditLogStore.listEvents()).toHaveLength(0);
364364
});
365+
366+
it("purges expired events on write too, same as the JSON backend", async () => {
367+
const { openAuditStore, migrateAuditLogFromJson } = await import("./native-sqlite-store");
368+
const sqliteDbPath = path.join(app.getPath("userData"), "audit-log.sqlite3");
369+
370+
settingsStore.saveSettings({ auditLogBackend: "sqlite", auditLogRetentionDays: 30 });
371+
openAuditStore(sqliteDbPath);
372+
const old = new Date(Date.now() - 40 * 24 * 60 * 60 * 1000).toISOString(); // 40 days ago
373+
migrateAuditLogFromJson(sqliteDbPath, JSON.stringify([{ id: "old-1", timestamp: old, actionCategory: "case-viewed" }]));
374+
375+
// Any write triggers the purge, per the same semantics as the
376+
// JSON backend's "purges expired events on write too" test.
377+
auditLogStore.recordEvent("case-created", { targetId: "new-write" });
378+
379+
const remaining = auditLogStore.listEvents();
380+
expect(remaining.some((e) => e.id === "old-1")).toBe(false);
381+
expect(remaining.some((e) => e.targetId === "new-write")).toBe(true);
382+
383+
settingsStore.saveSettings({ auditLogRetentionDays: undefined });
384+
});
365385
});
366386

367387
it("falls back to the JSON backend when sqlite is requested but the addon isn't available", () => {

app/src/audit-log-store.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ import {
1212
migrateAuditLogFromJson,
1313
getLastAuditEventHash,
1414
listAuditEventsJson,
15+
trimAuditEventsToCap,
16+
purgeAuditEventsOlderThan,
17+
auditEventCount as sqliteAuditEventCount,
1518
} from "./native-sqlite-store";
1619
import type { z } from "zod";
1720

@@ -79,13 +82,16 @@ function writeAll(events: AuditEvent[]): void {
7982
// --- Optional SQLite backend (Settings → Audit & Privacy, experimental) ---
8083
//
8184
// Opt-in only; unset/"json" (the default) never touches any of this. See
82-
// docs/RUST_MIGRATION_ASSESSMENT.md for why this exists and what it
83-
// deliberately doesn't do yet: retention purging and the MAX_EVENTS soft
84-
// cap below are JSON-backend-only in this first slice — the SQLite backend
85-
// currently grows without a cap. That's a real, known gap (disk usage over
86-
// a long-lived install), not an oversight; SQLite inserts don't have the
87-
// JSON file's O(n²) growth problem regardless of table size, so it's a
88-
// lower-urgency follow-up than the bug that motivated the JSON-side fix.
85+
// docs/RUST_MIGRATION_ASSESSMENT.md. Retention purging and the MAX_EVENTS
86+
// cap are enforced here too (via trimAuditEventsToCap/
87+
// purgeAuditEventsOlderThan), reusing the same MAX_EVENTS/TRIM_BATCH
88+
// constants as the JSON backend — but unlike JSON, both are single indexed
89+
// SQLite DELETEs regardless of table size, so purging can run on every
90+
// write (matching the JSON backend's exact purge-on-write semantics) without
91+
// the O(n) cost that made the JSON backend's own cap soft in the first
92+
// place. The trim is still batched (TRIM_BATCH slack) purely to avoid an
93+
// extra DELETE on every single insert, not because a single trim is
94+
// expensive here.
8995

9096
function sqliteDbPath(): string {
9197
return path.join(app.getPath("userData"), "audit-log.sqlite3");
@@ -151,6 +157,19 @@ function recordEventSqlite(dbPath: string, actionCategory: AuditActionCategory,
151157
// single new event, rather than a separate INSERT code path in Rust —
152158
// this event's id is always fresh (randomUUID()), so it always inserts.
153159
migrateAuditLogFromJson(dbPath, JSON.stringify([event]));
160+
161+
const retentionDays = getSettings().auditLogRetentionDays;
162+
if (retentionDays && retentionDays > 0) {
163+
const cutoff = new Date(Date.now() - retentionDays * 24 * 60 * 60 * 1000).toISOString();
164+
purgeAuditEventsOlderThan(dbPath, cutoff);
165+
}
166+
// Soft cap, same shape as the JSON backend's (MAX_EVENTS + TRIM_BATCH
167+
// slack) — only actually issues a DELETE once every TRIM_BATCH writes
168+
// past the cap, not on every single one.
169+
if (sqliteAuditEventCount(dbPath) > MAX_EVENTS + TRIM_BATCH) {
170+
trimAuditEventsToCap(dbPath, MAX_EVENTS);
171+
}
172+
154173
return event;
155174
}
156175

app/src/native-sqlite-store.test.ts

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,16 @@ import {
99
migrateAuditLogFromJson,
1010
auditEventCount,
1111
verifyAuditStore,
12+
listAuditEventsJson,
13+
trimAuditEventsToCap,
14+
purgeAuditEventsOlderThan,
1215
} from "./native-sqlite-store";
1316

1417
// Same addon-presence-varies-by-environment situation as
15-
// native-datastore.test.ts — see that file's comment. This module is inert
16-
// (see its own header comment): nothing in the running app calls it, so
17-
// these tests exist purely to prove the scaffold itself is correct ahead of
18-
// any future cutover, not to protect a live code path.
18+
// native-datastore.test.ts — see that file's comment. audit-log-store.ts
19+
// reaches this module only when a user has explicitly opted into the
20+
// experimental SQLite backend (Settings → Audit & Privacy); these tests
21+
// exercise the bridge directly, independent of that opt-in gate.
1922
const addonPresent = fs.existsSync(path.join(__dirname, "..", "native"));
2023

2124
function tempDbPath(): string {
@@ -74,4 +77,49 @@ describe(`native-sqlite-store (addon ${addonPresent ? "present" : "unavailable"}
7477
(!addonPresent ? it : it.skip)("throws (rather than silently proceeding) when the addon isn't available", () => {
7578
expect(() => openAuditStore(tempDbPath())).toThrow();
7679
});
80+
81+
(addonPresent ? it : it.skip)("trimAuditEventsToCap keeps only the newest rows", () => {
82+
const dbPath = tempDbPath();
83+
try {
84+
const events = Array.from({ length: 5 }, (_, i) => ({
85+
id: `e${i}`,
86+
timestamp: `2026-01-01T00:00:0${i}.000Z`,
87+
actionCategory: "case-viewed",
88+
}));
89+
migrateAuditLogFromJson(dbPath, JSON.stringify(events));
90+
91+
const deleted = trimAuditEventsToCap(dbPath, 2);
92+
expect(deleted).toBe(3);
93+
expect(auditEventCount(dbPath)).toBe(2);
94+
95+
const remaining = JSON.parse(listAuditEventsJson(dbPath)) as { id: string }[];
96+
expect(remaining.map((e) => e.id)).toEqual(["e3", "e4"]);
97+
} finally {
98+
fs.rmSync(dbPath, { force: true });
99+
fs.rmSync(`${dbPath}-wal`, { force: true });
100+
fs.rmSync(`${dbPath}-shm`, { force: true });
101+
}
102+
});
103+
104+
(addonPresent ? it : it.skip)("purgeAuditEventsOlderThan removes only expired rows", () => {
105+
const dbPath = tempDbPath();
106+
try {
107+
const events = [
108+
{ id: "old", timestamp: "2020-01-01T00:00:00.000Z", actionCategory: "case-viewed" },
109+
{ id: "new", timestamp: "2030-01-01T00:00:00.000Z", actionCategory: "case-viewed" },
110+
];
111+
migrateAuditLogFromJson(dbPath, JSON.stringify(events));
112+
113+
const deleted = purgeAuditEventsOlderThan(dbPath, "2025-01-01T00:00:00.000Z");
114+
expect(deleted).toBe(1);
115+
expect(auditEventCount(dbPath)).toBe(1);
116+
117+
const remaining = JSON.parse(listAuditEventsJson(dbPath)) as { id: string }[];
118+
expect(remaining.map((e) => e.id)).toEqual(["new"]);
119+
} finally {
120+
fs.rmSync(dbPath, { force: true });
121+
fs.rmSync(`${dbPath}-wal`, { force: true });
122+
fs.rmSync(`${dbPath}-shm`, { force: true });
123+
}
124+
});
77125
});

app/src/native-sqlite-store.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ interface NativeAddon {
2121
verifyAuditStore(dbPath: string): StoreIntegrityReport;
2222
getLastAuditEventHash(dbPath: string): string | null;
2323
listAuditEventsJson(dbPath: string): string;
24+
trimAuditEventsToCap(dbPath: string, keepCount: number): number;
25+
purgeAuditEventsOlderThan(dbPath: string, cutoffIso: string): number;
2426
}
2527

2628
export interface MigrationReport {
@@ -106,3 +108,20 @@ export function getLastAuditEventHash(dbPath: string): string | null {
106108
export function listAuditEventsJson(dbPath: string): string {
107109
return getNativeAddon().listAuditEventsJson(dbPath);
108110
}
111+
112+
/** Deletes the oldest rows beyond `keepCount`, returning how many were
113+
* removed. A single indexed-adjacent DELETE regardless of table size —
114+
* unlike the JSON backend's soft-cap trim, this doesn't need to be batched
115+
* to stay cheap, though audit-log-store.ts still batches its *calls* to
116+
* avoid running a DELETE on every single insert. */
117+
export function trimAuditEventsToCap(dbPath: string, keepCount: number): number {
118+
return getNativeAddon().trimAuditEventsToCap(dbPath, keepCount);
119+
}
120+
121+
/** Deletes every event older than `cutoffIso` (ISO-8601 UTC), returning how
122+
* many were removed. Cheap enough (indexed DELETE) to call on every write
123+
* when age-based retention is configured, matching the JSON backend's
124+
* purge-on-every-write behavior without reintroducing an O(n) cost. */
125+
export function purgeAuditEventsOlderThan(dbPath: string, cutoffIso: string): number {
126+
return getNativeAddon().purgeAuditEventsOlderThan(dbPath, cutoffIso);
127+
}

docs/RUST_MIGRATION_ASSESSMENT.md

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -282,18 +282,37 @@ results on both backends, clearAll clears both) plus a fallback test
282282
JSON — this one runs in every environment, addon or not). All verified
283283
through the real built addon, both with and without it present.
284284

285-
**Known gap, explicitly not solved in this slice:** the SQLite backend has
286-
no retention purging or row cap yet — `MAX_EVENTS`/`TRIM_BATCH` are
287-
JSON-backend-only. This is a real, intentional scope cut, not an oversight:
288-
SQLite inserts don't have the JSON file's O(n²) growth problem regardless
289-
of table size, so unbounded row growth here is a disk-usage concern to
290-
address before this leaves "experimental," not the kind of performance
291-
cliff that motivated the JSON-side fix. No Settings UI toggle exists for
292-
`auditLogBackend` yet either — it's reachable today only by writing
293-
`settings.json` directly or programmatically, which is appropriate for an
294-
explicitly experimental first slice but would need a real UI (with the
295-
capability-report-driven "addon not available, staying on JSON" messaging
296-
this design already supports) before recommending it to anyone.
285+
**Both gaps flagged above have since been closed**, in two follow-up passes:
286+
287+
- **Settings UI**: Audit & Privacy now has an "Audit log storage backend"
288+
card — current-backend badge, a disabled state with the specific reason
289+
when SQLite isn't available (via a new `audit:sqliteCapability` IPC
290+
handler), and a switch button. New e2e spec drives the real app: create a
291+
case on JSON, switch to SQLite, confirm the event migrated, switch back,
292+
confirm it's still there.
293+
- **Retention/cap enforcement**: two more Rust functions,
294+
`trim_audit_events_to_cap` and `purge_audit_events_older_than`, both
295+
single indexed `DELETE`s — cheap regardless of table size, unlike the
296+
JSON backend's full-file-rewrite equivalent, which is *why* the JSON
297+
backend needed a soft cap with batching in the first place and this one
298+
doesn't strictly need to (it still batches trim calls via the same
299+
`TRIM_BATCH` constant, purely to avoid a `DELETE` on every single insert
300+
— not because a single trim is expensive here). Age-based purge now runs
301+
on every write when retention is configured, matching the JSON backend's
302+
exact semantics without reintroducing an O(n) cost.
303+
304+
**A real bug this surfaced, worth recording:** `get_last_audit_event_hash`
305+
crashed (`Invalid column type Null`) whenever the store's most recent row
306+
had a NULL `event_hash` — i.e. a legacy-shaped last event. None of the
307+
original Rust unit tests exercised that specific case (both events in the
308+
insertion-order test had hashes); a new TypeScript-level retention
309+
integration test seeded exactly that shape and caught it immediately. Fixed
310+
by reading the column as `Option<String>` and flattening the two
311+
independently-nullable layers (`.optional()` for "no rows at all" vs. the
312+
column itself being NULL) instead of assuming a non-null `String`. A
313+
dedicated Rust test (`last_event_hash_is_none_when_the_last_row_has_no_hash_column_value`)
314+
now covers this directly. Re-verified through the real built addon, in both
315+
the fallback and native states.
297316

298317
**Explicitly not done, with reasons, per "stop and document the blocker"
299318
rather than half-finish:**
@@ -303,8 +322,6 @@ rather than half-finish:**
303322
for one store; repeating it for the others is real, separately-scoped
304323
work per store, not a mechanical copy-paste (their schemas and access
305324
patterns differ).
306-
- Retention/cap enforcement on the SQLite backend (see above).
307-
- A Settings UI toggle for `auditLogBackend` (see above).
308325
- Crypto vault, RAG index, filesystem capability layer, process supervisor,
309326
system-inspection rewrite, ingestion, safety-scanning engine, recommender
310327
inference — none started. Each is a substantial, independently-scoped

0 commit comments

Comments
 (0)