From eeda2115426a8a41fa9307abfd15530e1431349f Mon Sep 17 00:00:00 2001 From: Mando Escamilla Date: Mon, 17 Aug 2026 12:03:17 -0500 Subject: [PATCH 1/2] Stamp the secret scanner so old sessions get checked (#335) Inline scanning only covers sessions the incremental pipeline touched, so upgrading to a build with the scanner left a user's back catalogue silently unscanned, and a rules refresh changed nothing for sessions already indexed. Record which scanner version last looked at each session (resolved_sessions.secret_scan_version, schema v25; existing rows migrate to NULL) and drain the backlog after each index pass: sessions the current version hasn't stamped get their retained text read back, rescanned, and stamped. No model call, so no rate limiter, but the pass is bounded and yields so `argus run` stays responsive. Bumping SECRET_SCAN_VERSION is now step 5 of the gitleaks rules-refresh procedure. Sessions indexed with text retention off can't be reached from the store at all. `argus status` says how many, and that re-reading their transcripts is what fixes it, rather than leaving the gap silent. --- CLAUDE.md | 2 +- docs/internals/architecture.md | 5 +- docs/internals/database-schema.md | 10 +- docs/internals/secret-scanning.md | 55 ++++++- src/cli.ts | 29 ++++ src/index-ops.ts | 30 ++-- src/indexing/pipeline.ts | 14 +- src/indexing/secret-scan-drain.ts | 105 ++++++++++++ src/indexing/secret-scan-rules.ts | 4 + src/indexing/secret-scan.ts | 18 ++- src/store/store-contract.ts | 32 +++- src/store/store.ts | 147 ++++++++++++++++- src/watch.ts | 11 +- test/cli.test.ts | 66 ++++++++ test/secret-findings.test.ts | 12 +- test/secret-scan-drain.test.ts | 258 ++++++++++++++++++++++++++++++ test/store.test.ts | 75 ++++++++- 17 files changed, 821 insertions(+), 52 deletions(-) create mode 100644 src/indexing/secret-scan-drain.ts create mode 100644 test/secret-scan-drain.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index b9dc087c..c5cfb0da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -135,7 +135,7 @@ inventory so the surfaces don't drift apart. The `docs/contributing/` guides (an The pipeline is a one-way data flow, and `src/` is laid out by stage (full map in `docs/internals/architecture.md`): **`src/indexing/`** (the pipeline — discover, parse, reconcile, -materialize, plus the interpret drain), **`src/store/`** (the `argus.db` layer + its parse→store +materialize, plus the interpret and secret-scan drains), **`src/store/`** (the `argus.db` layer + its parse→store contract), **`src/reporting/`** (per-session and plugin aggregation), and **`src/api/`** (the serve layer). Cross-cutting modules (`types.ts`, `config.ts`, `paths.ts`, `pricing.ts`, `tool-categories.ts`, **`src/llm/`** [the shared LLM access layer], **`secrets.ts`** [BYO API-key storage]) and the diff --git a/docs/internals/architecture.md b/docs/internals/architecture.md index 9f451132..47a47647 100644 --- a/docs/internals/architecture.md +++ b/docs/internals/architecture.md @@ -120,7 +120,10 @@ the "save the answer" step that makes reads cheap and reconcile-free. Materialize is also where the **secret scan** (#327) lands: the pipeline runs a deterministic regex pass over each session's in-memory prompt/response text (`src/indexing/secret-scan.ts`) and persists redacted findings — never secret values — to `resolved_secret_findings`. Because the scan runs on -in-memory text at write time, it works even when conversation-text retention is off. Full design: +in-memory text at write time, it works even when conversation-text retention is off. Materialize also +stamps `secret_scan_version`, so a second pass after indexing (`src/indexing/secret-scan-drain.ts`, +#335) can catch up sessions the current scanner hasn't seen — the back catalogue after an upgrade, or +everything after a rules refresh — reading their retained text back from the store. Full design: [secret-scanning.md](./secret-scanning.md). ### What "interpret" means (default-on) diff --git a/docs/internals/database-schema.md b/docs/internals/database-schema.md index cf24ad11..0ddba7d7 100644 --- a/docs/internals/database-schema.md +++ b/docs/internals/database-schema.md @@ -271,7 +271,9 @@ timestamps, `message_count`, the `archived` flag (1 = retained but no longer on friction signals (`friction_interruptions` / `_rejections` / `_compactions` / `_turns`, `last_interruption_ms`; NULL where the source can't observe friction), the local-only UI state columns (`is_hidden`, `secret_scan_dismissed` — user state carried forward across re-materializes, never -synced), and `meta_json` (the authoritative `SessionMeta`). The root of the read model. +synced), `secret_scan_version` (#335 — which scanner version last scanned this session, NULL for never; +**not** carried forward, since the re-materialize cascades the findings away), and `meta_json` (the +authoritative `SessionMeta`). The root of the read model. ### `resolved_usage` One row per usage-bearing turn (the provider's metering grain: Claude `message.usage`, Codex @@ -353,7 +355,9 @@ at materialize time. **Redacted locators only**: `category` + `interaction_seq`/ `hint` (first/last few characters), never the secret value. `findings_digest` (the scanner's stable hash of the session's whole finding set) is denormalized onto every row so a dismissal — the matching digest stored on `resolved_sessions.secret_scan_dismissed` — can be compared in SQL, and lapses when a -re-scan produces different findings. **Local-only, never synced** (the push path never reads it). +re-scan produces different findings. Two writers: materialize (from the scan it just ran) and the +version-stamped rescan drain's `writeSessionSecretFindings` (#335), which catches up sessions the +current scanner hasn't stamped. **Local-only, never synced** (the push path never reads it). PK `(session_id, seq)`, FK → `resolved_sessions`. See [secret-scanning.md](./secret-scanning.md). ## Tier 3 — freshness & ownership @@ -385,7 +389,7 @@ unchanged sessions and pick up reindexed ones. PK `(hub_url, client_id, session_ ## Schema version & migrations -`PRAGMA user_version` holds the schema version (currently **24**) and `PRAGMA application_id` +`PRAGMA user_version` holds the schema version (currently **25**) and `PRAGMA application_id` (`0x41524753`, "ARGS") tags the file as an Argus store. Upgrades run forward-only `MIGRATIONS` in `src/store/store.ts`, each a `{ to, sql }` step applied in a transaction that bumps `user_version`, so a partial upgrade never leaves a half-migrated store. Fresh stores are created from `CREATE_SCHEMA_SQL` diff --git a/docs/internals/secret-scanning.md b/docs/internals/secret-scanning.md index b0579ea1..99227e03 100644 --- a/docs/internals/secret-scanning.md +++ b/docs/internals/secret-scanning.md @@ -19,12 +19,16 @@ The scanner engine (`src/indexing/secret-scan.ts`) is pure regex plus per-rule e LLM call, no network, no throttle — over rules defined in `src/indexing/secret-scan-rules.ts` (adapted from gitleaks). It runs **inline at materialize time**, inside `toMaterializeSessions` in `src/indexing/pipeline.ts`, over the reconciled interactions' in-memory prompt/response text. That -placement was chosen over a drain (like interpret) for two reasons: +is the primary path, chosen over a drain (like interpret) for two reasons: - It's cheap enough to run on every materialized session, so there's nothing to throttle. - It scans text *in memory*, so it works even when conversation-text retention (`retainText`) is off — a drain reading `resolved_interaction_text` back from the store would find nothing there. +Inline scanning only ever covers sessions the incremental pipeline **touched**, though, which leaves +everything already in the store unscanned. A second path closes that gap: the version-stamped +**backlog drain** (`src/indexing/secret-scan-drain.ts`). See "Rescanning" below. + Scope is deliberately the same text the Interpret stage reads (`prompt`/`response` chunks), per the issue's sketch. Tool *result* text (e.g. the output of `cat .env`) is not covered: it is never retained anywhere in the read model today, so there is nothing to scan at write time. Covering it @@ -70,7 +74,9 @@ are capped per session so a dumped key list can't produce unbounded rows. ## Storage and dismissal -One table, `resolved_secret_findings` (schema v24), FK-chained to `resolved_sessions` with +One table, `resolved_secret_findings` (schema v24), plus one column on `resolved_sessions` recording +which scanner version last looked at each session (`secret_scan_version`, schema v25; see +"Rescanning"). The findings table is FK-chained to `resolved_sessions` with `ON DELETE CASCADE` like every other leaf: re-materializing a session replaces its findings wholesale, and retracting a session removes them. Every row carries `findings_digest` — the scanner's stable hash of the session's whole finding set — denormalized so SQL can compare the @@ -127,7 +133,44 @@ derived from transcript text, so agents without transcript access don't get them ## Rescanning -Findings re-derive on every materialize of a session, and `argus index refresh` re-reads every -transcript — so a rule-set improvement lands everywhere on the next refresh. There is no scanner -version stamp in v1 (unlike the interpreter's): the scan is deterministic and free, and a refresh -is the explicit re-run path. +Findings re-derive on every materialize of a session, but the incremental pipeline only materializes +**touched** sessions. Inline scanning alone would therefore mean a user who upgrades gets findings only +for sessions that happen to change afterwards, leaving their back catalogue silently never scanned. The +same gap applies to a rule-set refresh: improving `secret-scan-rules.ts` would change nothing for +sessions already in the store. + +`SECRET_SCAN_VERSION` (`src/indexing/secret-scan.ts`) plus a drain closes it, mirroring how +interpretation is decoupled from the structural index (#153) without the model calls that make that +drain expensive: + +- **The stamp.** `resolved_sessions.secret_scan_version` records the scanner version that last + scanned each session; NULL means never scanned. Materialize writes it from the scan it just ran. A + materialize that *didn't* scan writes NULL, because the wholesale replace cascades the old findings + away and keeping the stamp would claim a scan with nothing to show. Existing rows migrate to NULL, + which is exactly the "upgraded, never scanned" state. +- **Eligibility** (`SECRET_SCAN_ELIGIBLE_SQL` in `store.ts`, alongside `INTERPRETATION_ELIGIBLE_SQL`): + the stamp is NULL or below the current version, **and** the session has retained text. Unlike the + interpreter's version, this one *is* part of eligibility, since a bump is precisely how a rules + refresh reaches already-indexed sessions. +- **The drain** (`src/indexing/secret-scan-drain.ts`, run from `runIndex` right after the structural + index) reads each eligible session's text back via `readSessionInteractions`, rescans it, and + replaces its findings wholesale with `writeSessionSecretFindings`. That write always stamps, even for + an empty finding set, so a clean session de-queues. There's no rate limiter and no failure cooldown + (nothing costs anything, nothing transient to retry), but the pass is bounded and yields every 25 + sessions so a large backlog can't make `argus run` stop responding. + +Two consequences worth keeping in view: + +- **Bump the version only when findings can actually change.** Dismissal is anchored to the finding-set + digest, so a rescan that finds something different clears the dismissal by design. That's correct for + a genuinely different finding set, but it means a gratuitous bump re-warns in bulk. The rule-refresh + procedure in `secret-scan-rules.ts` carries this as its last step. +- **Text retention bounds what the drain can reach.** With `retainText` off (#120) a session's text only + ever existed in memory during materialize, so there is nothing to rescan and eligibility excludes + those sessions. Interpretation has the same limitation, so this is a known shape rather than a new + one. The state is reported rather than silent: `secretScanProgress` counts them separately, and + `argus status` says how many sessions can't be checked without re-reading their transcripts, which is + what `argus index refresh` does. + +`argus index refresh` remains the explicit "rescan everything now" path, and the only one that covers +sessions whose text wasn't kept. It is no longer the *only* way a rules improvement lands, though. diff --git a/src/cli.ts b/src/cli.ts index e8d2ce42..fce1c7ea 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -4,6 +4,7 @@ import { defineCommand, runMain, showUsage } from "citty"; import type { ArgsDef, CommandContext, ParsedArgs } from "citty"; import { printBanner } from "./banner.ts"; import { scanStore } from "./indexing/pipeline.ts"; +import { SECRET_SCAN_VERSION } from "./indexing/secret-scan.ts"; import { STORE_FILE } from "./paths.ts"; import { ALL_SOURCES } from "./reporting/dashboard-builder.ts"; import { startServer } from "./api/serve.ts"; @@ -310,11 +311,15 @@ async function runStatus(): Promise { let interpretation: | { interpreted: number; pending: number; outdated: number } | undefined; + let secretScan: + | { scanned: number; pending: number; unscannable: number } + | undefined; try { const store = await openStore(); try { counts = await store.resolvedSessionCounts(); interpretation = await store.interpretationProgress(); + secretScan = await store.secretScanProgress(SECRET_SCAN_VERSION); } finally { await store.close(); } @@ -376,6 +381,30 @@ async function runStatus(): Promise { `(${interpretation.pending} waiting${outdated}).`, ); } + // Secret-scan backlog progress (#335). The waiting count only appears while there is a backlog — + // after an upgrade, or a rules refresh — since every newly indexed session is checked as it's + // written. Sessions whose conversation text wasn't kept can't be checked from the store at all, so + // they get their own line with the one thing that fixes it. + if (secretScan && secretScan.scanned > 0) { + const waiting = secretScan.pending + ? ` (${secretScan.pending} waiting)` + : ""; + printResultLine( + `Checked ${secretScan.scanned} session${secretScan.scanned === 1 ? "" : "s"} for exposed credentials${waiting}.`, + ); + } else if (secretScan?.pending) { + const n = secretScan.pending; + printResultLine( + `${n} session${n === 1 ? "" : "s"} waiting to be checked for exposed credentials.`, + ); + } + if (secretScan?.unscannable) { + const n = secretScan.unscannable; + printResultLine( + `${n} session${n === 1 ? "" : "s"} can't be checked for exposed credentials without re-reading ` + + `${n === 1 ? "its transcript" : "their transcripts"} · run \`argus index refresh\``, + ); + } if (pending) printResultLine("Run `argus index` to pick up new and changed sessions."); } diff --git a/src/index-ops.ts b/src/index-ops.ts index daad68ac..5747c44e 100644 --- a/src/index-ops.ts +++ b/src/index-ops.ts @@ -4,6 +4,7 @@ import { createInterface } from "node:readline"; import { sourcesFor } from "./reporting/dashboard-builder.ts"; import { syncStatsSummary, reindexSession } from "./indexing/pipeline.ts"; import { runInterpretationDrain, sessionInterpretationActive } from "./indexing/interpret/index.ts"; +import { runSecretScanDrain } from "./indexing/secret-scan-drain.ts"; import type { RepeatCollapser } from "./backoff.ts"; import { openSessionStore } from "./store/session-store.ts"; import { openStore, rebuildStore } from "./store/store.ts"; @@ -42,9 +43,9 @@ export async function runIndex( extractTasks?: boolean, debug = false, retainText?: boolean, - // Persisted across watch ticks by the caller (watchIndex) so the drain's throttle-pause / failure - // lines collapse instead of repeating every interval. Omitted for a one-shot `argus index`. - interpretCollapser?: RepeatCollapser, + // Persisted across watch ticks by the caller (watchIndex) so the post-index drains' throttle-pause / + // failure lines collapse instead of repeating every interval. Omitted for a one-shot `argus index`. + collapser?: RepeatCollapser, ): Promise { // Read argus.json once and thread it into both resolvers (avoid a double parse per pass / watch tick). const config = loadConfig(); @@ -64,17 +65,20 @@ export async function runIndex( } finally { await store.close(); } - // Decoupled, throttled interpretation (#153): after the structural index brings the store current, - // interpret a bounded, rate-limited batch of eligible sessions, reading retained text back from the - // store. A fresh handle (the pipeline closed its own) and strictly after indexing, so there's never a - // concurrent writer. No-op when task extraction is disabled — and we skip opening the store entirely. - if (sessionInterpretationActive(taskExtraction)) { - const store = await openStore(); - try { - await runInterpretationDrain(store, taskExtraction, log, interpretCollapser); - } finally { - await store.close(); + // Two decoupled passes run after the structural index brings the store current, both reading their + // inputs back from the store: the secret-scan backlog (#335) and throttled interpretation (#153). + // A fresh handle (the pipeline closed its own) and strictly after indexing, so there's never a + // concurrent writer. Both are silent no-ops when there's nothing eligible, which is the steady state. + const drainStore = await openStore(); + try { + // Cheap and unconditional: regex over text already in the store, no setting to honor. Runs first + // so a just-upgraded user gets their credential warnings without waiting on the model calls below. + await runSecretScanDrain(drainStore, log, collapser); + if (sessionInterpretationActive(taskExtraction)) { + await runInterpretationDrain(drainStore, taskExtraction, log, collapser); } + } finally { + await drainStore.close(); } } diff --git a/src/indexing/pipeline.ts b/src/indexing/pipeline.ts index 6a136526..25f52b91 100644 --- a/src/indexing/pipeline.ts +++ b/src/indexing/pipeline.ts @@ -34,7 +34,7 @@ import { NATIVE_PRODUCERS, nativeProducerForSource } from "./parse/producers/ind import type { AgentSource, MessageRecord, ParseResult } from "../types.ts"; import type { TaskFact } from "../store/store-contract.ts"; import { interpretSession, sessionInterpretationActive } from "./interpret/index.ts"; -import { scanSessionForSecrets, secretFindingsDigest } from "./secret-scan.ts"; +import { SECRET_SCAN_VERSION, scanSessionForSecrets, secretFindingsDigest } from "./secret-scan.ts"; import type { ResolvedSessionInterpretation } from "../config.ts"; export interface SyncStats { @@ -322,16 +322,20 @@ function toMaterializeSessions(output: ReconcileResult): MaterializeSession[] { const interactions = interactionsBySession.get(sid) ?? []; // Secret scan (#327): regex-cheap and deterministic, so it runs inline here for every // materialized session — no throttle, no LLM call. Scanning the in-memory interaction text - // (not the store) means findings exist even when conversation-text retention is off. + // (not the store) means findings exist even when conversation-text retention is off. The result + // is always attached, even with zero findings, so materialize stamps SECRET_SCAN_VERSION (#335) + // and the rescan drain knows this session has been seen by the current scanner. const findings = scanSessionForSecrets({ interactions }); sessions.push({ meta, messages: messagesBySession.get(sid) ?? [], tasks: output.tasksBySession.get(sid) ?? [], interactions, - ...(findings.length - ? { secretFindings: { digest: secretFindingsDigest(findings), findings } } - : {}), + secretFindings: { + version: SECRET_SCAN_VERSION, + digest: secretFindingsDigest(findings), + findings, + }, }); } return sessions; diff --git a/src/indexing/secret-scan-drain.ts b/src/indexing/secret-scan-drain.ts new file mode 100644 index 00000000..126e9823 --- /dev/null +++ b/src/indexing/secret-scan-drain.ts @@ -0,0 +1,105 @@ +// The secret-scan backlog drain (#335). Inline scanning at materialize (see ./secret-scan.ts) only +// ever covers sessions the incremental pipeline touched this run, so nothing about upgrading to a +// build that has the scanner — or refreshing the gitleaks rules — reaches a user's back catalogue. +// This pass closes that gap: it picks up every session the CURRENT scanner version hasn't stamped, +// reads its retained text back from the store, rescans it, and stamps the version. +// +// Shaped like the interpretation drain (#153) minus everything that made that one expensive: no model +// call, no network, no tokens, so no rate limiter and no per-session failure cooldown. What it does +// keep is a bound on how much it does per pass, and a yield between chunks of sessions, so a large +// backlog can't make `argus run` stop responding while it works — the rest drains on later passes. +// +// The one thing it cannot do is reach sessions indexed with text retention off (#120): their text +// only ever existed in memory during materialize, so there is nothing in the store to rescan. Those +// sessions are excluded by the eligibility predicate and counted separately by secretScanProgress, so +// `argus status` can say they need an `argus index refresh` rather than leaving the gap silent. +import type { RepeatCollapser } from "../backoff.ts"; +import type { Store } from "../store/store-contract.ts"; +import { logWarn, type Log } from "../logger.ts"; +import { + SECRET_SCAN_VERSION, + scanSessionForSecrets, + secretFindingsDigest, +} from "./secret-scan.ts"; + +// How many sessions one pass rescans. High compared to the interpretation drain's batch (5) because a +// scan is regex over text already on disk, not a model call — a fresh upgrade should catch up in a +// pass or two, not a week of watch ticks — but still bounded so one pass has an end. +const SECRET_SCAN_BATCH_PER_PASS = 500; + +// Hand the event loop back every this many sessions. Store reads and the regex pass are synchronous +// under the hood, so without this a 500-session pass would block `argus run`'s other legs (serve, the +// watch timers) for its whole duration. +const YIELD_EVERY = 25; + +// A session that fails to scan (an unreadable row, say) stays eligible, so without a backoff it would +// sit at the front of the newest-first queue and re-log its failure on every watch tick forever. Same +// time-based cooldown as the interpretation drain: never a permanent drop, always self-recovering. +// Module-level so it survives drain ticks within `--watch`. +const RETRY_COOLDOWN_MS = 15 * 60_000; +const retryAfterMs = new Map(); + +const yieldToLoop = () => new Promise((resolve) => setTimeout(resolve, 0)); + +/** + * One pass of the secret-scan backlog drain (#335). Rescans up to SECRET_SCAN_BATCH_PER_PASS eligible + * sessions (never scanned, or scanned by an older scanner version) and stamps each one. Silent when + * nothing is eligible — the steady state, since materialize stamps every session it writes. Never + * throws on a single bad session; only a fatal store error propagates, so the supervised index loop + * isn't restarted by one unreadable session. + */ +export async function runSecretScanDrain( + store: Store, + log?: Log, + collapser?: RepeatCollapser, +): Promise { + const now = Date.now(); + const batch = ( + await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION, SECRET_SCAN_BATCH_PER_PASS) + ).filter((id) => (retryAfterMs.get(id) ?? 0) <= now); + if (!batch.length) return; // quiet when idle — no noise every tick + + // Heartbeat only when the pass is big enough to take a noticeable moment; for a handful of sessions + // the summary below says everything and two lines for one session is just noise. + if (batch.length > YIELD_EVERY) { + log?.(`Checking ${batch.length} earlier sessions for exposed credentials…`); + } + let scanned = 0; + let flagged = 0; + let failures = 0; + for (const [index, sessionId] of batch.entries()) { + try { + const interactions = await store.readSessionInteractions(sessionId); + const findings = scanSessionForSecrets({ interactions }); + await store.writeSessionSecretFindings(sessionId, { + version: SECRET_SCAN_VERSION, + digest: secretFindingsDigest(findings), + findings, + }); + scanned++; + retryAfterMs.delete(sessionId); + if (findings.length) flagged++; + } catch (err) { + // One session we couldn't read or write. Leave it unstamped so a later pass retries, back it off + // for the cooldown, and keep going — a single odd session must not stall the whole backlog. + failures++; + retryAfterMs.set(sessionId, now + RETRY_COOLDOWN_MS); + if (log) logWarn(log, ` ${sessionId}: ${err instanceof Error ? err.message : String(err)}`); + } + if ((index + 1) % YIELD_EVERY === 0) await yieldToLoop(); + } + + if (scanned > 0) { + const progress = await store.secretScanProgress(SECRET_SCAN_VERSION); + const found = flagged ? ` Found possible credentials in ${flagged}.` : ""; + const left = progress.pending ? ` ${progress.pending} left to check.` : ""; + log?.( + `Checked ${scanned} session${scanned === 1 ? "" : "s"} for exposed credentials.${found}${left}`, + ); + } + if (failures > 0) { + const note = `Couldn't check ${failures} session${failures === 1 ? "" : "s"} for exposed credentials; will retry later.`; + if (collapser) collapser.note(note, "warn"); + else if (log) logWarn(log, note); + } +} diff --git a/src/indexing/secret-scan-rules.ts b/src/indexing/secret-scan-rules.ts index 0d6dcb49..8ad8592f 100644 --- a/src/indexing/secret-scan-rules.ts +++ b/src/indexing/secret-scan-rules.ts @@ -19,6 +19,10 @@ // 3. Port regexes carefully: RE2 supports inline flags mid-pattern (e.g. `(?-i:...)`) and JS // does not — hoist or restructure them (see generic_secret for the pattern). // 4. Bump PINNED_COMMIT and re-run `bun test test/secret-scan.test.ts`. +// 5. If the change can alter what a scan finds, bump SECRET_SCAN_VERSION in ./secret-scan.ts — +// that is what makes the new rules reach sessions already in the store (#335), via the rescan +// drain. Skip the bump for edits that can't change findings (comments, reordering): a bump +// rescans everything, and a session whose finding set genuinely changed loses its dismissal. // // Synced against gitleaks master commit: const PINNED_COMMIT = "b58d3f102cf3"; // 2026-07-22 — https://github.com/gitleaks/gitleaks/commit/b58d3f102cf3 diff --git a/src/indexing/secret-scan.ts b/src/indexing/secret-scan.ts index 6995cca6..499e1fb3 100644 --- a/src/indexing/secret-scan.ts +++ b/src/indexing/secret-scan.ts @@ -1,7 +1,9 @@ // Secret scanning (#327): a cheap, deterministic pass over a session's retained prompt/response // text that flags likely exposed credentials (pasted API keys, tokens, private key blocks). Runs at // materialize time on the in-memory interaction text — no LLM call, no throttle, and independent of -// the retainText setting (the text is in memory at write time either way). +// the retainText setting (the text is in memory at write time either way). Sessions materialized +// before the scanner existed, or under an older rule set, are caught up afterwards by the +// version-stamped drain in ./secret-scan-drain.ts (#335), which reads text back from the store. // // This module is the matching ENGINE only. The detection rules (patterns, entropy floors, // allowlists, stopwords) live in ./secret-scan-rules.ts, a data-only file adapted from gitleaks @@ -27,6 +29,20 @@ import { SECRET_RULES, type SecretRuleDefinition } from "./secret-scan-rules.ts" export type { SecretFinding, SecretFindingCategory } from "../store/store-contract.ts"; +/** + * The scanner's implementation version (#335), stamped on every session it scans + * (`resolved_sessions.secret_scan_version`; NULL = never scanned). Unlike the interpreter's version + * this one IS part of eligibility: bumping it makes every already-scanned session eligible for the + * rescan drain, which is the whole point — upgrading to a build that has the scanner, or refreshing + * the gitleaks rules, has to reach the back catalogue and not just sessions that happen to change + * afterwards. + * + * Bump it ONLY when the rules or the engine change in a way that could change a session's findings. + * A bump rescans every session, and a genuinely different finding set clears the user's dismissal + * (dismissal is anchored to the finding-set digest), so a gratuitous bump re-warns in bulk. + */ +export const SECRET_SCAN_VERSION = 1; + /** Cap on stored findings per session: a dumped key list shouldn't produce unbounded rows. Sessions * at the cap are vanishingly rare; the banner's message doesn't change past the first few. */ const MAX_FINDINGS_PER_SESSION = 100; diff --git a/src/store/store-contract.ts b/src/store/store-contract.ts index d5f476fe..6119f206 100644 --- a/src/store/store-contract.ts +++ b/src/store/store-contract.ts @@ -722,12 +722,17 @@ export interface MaterializeSession { * interaction_json is always text-free, and that text is persisted (opt-in, default-on, local-only) * in resolved_interaction_text (#120). */ interactions?: InteractionFact[]; - /** Secret-scan findings for this session (#327), computed by the pipeline from the in-memory + /** The secret scan's result for this session (#327), computed by the pipeline from the in-memory * interaction text (so scanning runs even when text retention is off) and persisted to * resolved_secret_findings. `digest` is the scanner's stable hash of the finding set, stamped on - * every row so a dismissal can be compared against the current set in SQL. Absent/empty → the - * session has no findings (any prior rows are replaced by the wholesale re-materialize). */ - secretFindings?: { digest: string; findings: SecretFinding[] }; + * every row so a dismissal can be compared against the current set in SQL. `version` is the + * scanner version that produced it (#335), recorded on resolved_sessions.secret_scan_version so + * the rescan drain knows which sessions the current scanner has already seen. + * + * Present with an EMPTY `findings` means "scanned, nothing found" — still a stamp. Absent means + * the caller never scanned (tests, programmatic materialize), which leaves the stamp NULL and so + * hands the session to the drain. */ + secretFindings?: { version: number; digest: string; findings: SecretFinding[] }; } /** Per-source freshness attestation. */ @@ -949,6 +954,25 @@ export interface ReadModelStore { dismissSessionSecretFindings(sessionId: string): Promise; /** Clear a session's findings dismissal, so the warning shows again. No-op if not dismissed. */ clearSessionSecretFindingsDismissal(sessionId: string): Promise; + /** Canonical ids of sessions the current scanner hasn't seen yet (#335), newest-first, capped at + * `limit`. Eligible = secret_scan_version is NULL or below `version` AND the session has retained + * text to scan. `version` is passed in (not read from the scanner) so the store stays unaware of + * the indexing layer. */ + readPendingSecretScanSessions(version: number, limit: number): Promise; + /** Replace a session's secret-scan findings and stamp the scanner version that produced them + * (#335) — the drain's write, without re-materializing anything else. Always stamps, even for an + * empty finding set, so a clean session de-queues instead of being rescanned every pass. Leaves + * `secret_scan_dismissed` alone: the read path compares it against the fresh digest, so an + * unchanged finding set stays dismissed and a changed one re-warns. */ + writeSessionSecretFindings( + sessionId: string, + result: { version: number; digest: string; findings: SecretFinding[] }, + ): Promise; + /** Secret-scan backlog progress for `argus status` (#335): sessions the scanner has stamped at + * least once, the eligible backlog (`pending`, the same predicate the drain uses), and how many + * need a scan but have no retained text to scan (`unscannable` — the retainText-off case, which + * only `argus index refresh` can reach). */ + secretScanProgress(version: number): Promise<{ scanned: number; pending: number; unscannable: number }>; // ---- Session/task labels (local-only; never synced) ---- /** All label definitions, ordered by name (case-insensitive). Excludes soft-deleted labels unless * `includeDeleted` is set. */ diff --git a/src/store/store.ts b/src/store/store.ts index 6ee1056a..7f27c226 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -71,7 +71,7 @@ import { } from "../health.ts"; import { STORE_FILE } from "../paths.ts"; -export const STORE_SCHEMA_VERSION = 24; +export const STORE_SCHEMA_VERSION = 25; export const STORE_APPLICATION_ID = 0x41524753; // "ARGS" export const DEFAULT_STORE_BUSY_TIMEOUT_MS = 2_000; @@ -538,6 +538,12 @@ const CREATE_SCHEMA_SQL = ` -- stays dismissed; different findings (new content) re-warn. NULL = not dismissed. Local-only UI -- state like is_hidden — carried forward by materialize, never selected by push. secret_scan_dismissed TEXT, + -- Which scanner version last scanned this session (#335), NULL for never scanned. Written by + -- materialize (from the scan it just ran) and by the rescan drain's writeSessionSecretFindings. + -- Deliberately NOT carried forward on a re-materialize that didn't scan: the wholesale session + -- DELETE cascades the findings away, so keeping the stamp would mark a session scanned with no + -- findings to show. Resetting to NULL instead hands it back to the drain, which self-heals. + secret_scan_version INTEGER, meta_json TEXT NOT NULL ); CREATE INDEX resolved_sessions_project ON resolved_sessions(project); @@ -550,6 +556,12 @@ const CREATE_SCHEMA_SQL = ` CREATE INDEX resolved_sessions_interpret_pending ON resolved_sessions(last_ts DESC) WHERE content_indexed_at_ms > COALESCE(interpreted_at_ms, 0); + -- Secret-scan backlog scan (#335). Not a partial index like the one above: its predicate compares + -- against the CURRENT scanner version, which changes, and a partial index can't follow that + -- without being rebuilt on every bump. Indexing the column plainly keeps the drain's newest-first + -- query off a full table scan in the steady state (every row at the current version). + CREATE INDEX resolved_sessions_secret_scan_version + ON resolved_sessions(secret_scan_version); CREATE TABLE resolved_usage ( session_id TEXT NOT NULL REFERENCES resolved_sessions(session_id) ON DELETE CASCADE, @@ -828,6 +840,20 @@ const INTERPRETATION_ELIGIBLE_SQL = `s.content_indexed_at_ms > COALESCE(s.interp WHERE t.session_id = s.session_id AND t.type = 'prompt' AND i.initiator = 'human' )`; +// The single definition of "needs a secret scan" (#335), shared by the drain's session query and the +// `argus status` counts so "waiting" can never desync from what the drain actually processes. Eligible +// = the current scanner hasn't stamped this session (NULL → never scanned; a lower number → scanned by +// an older rule set/engine) AND there is retained text to scan. Unlike interpretation, the version IS +// part of eligibility: a bump is precisely how a rules refresh reaches already-indexed sessions. +// +// The retained-text requirement is a real limitation, not an oversight: with retainText off (#120) the +// text only exists in memory during materialize, so a store-driven rescan has nothing to read. Those +// sessions are excluded here and counted separately by secretScanProgress, so the gap is visible in +// `argus status` rather than silent. Assumes resolved_sessions is aliased `s`; takes one bound +// parameter, the current scanner version. +const SECRET_SCAN_ELIGIBLE_SQL = `(s.secret_scan_version IS NULL OR s.secret_scan_version < ?) + AND EXISTS (SELECT 1 FROM resolved_interaction_text t WHERE t.session_id = s.session_id)`; + interface ResolvedSessionSnapshot { metaJson: string; messageJsons: string[]; @@ -1585,6 +1611,18 @@ const MIGRATIONS: Record = { ALTER TABLE resolved_sessions ADD COLUMN secret_scan_dismissed TEXT; `, }, + // 24 -> 25: the secret-scan version stamp (#335). Existing rows migrate to NULL — "upgraded to a + // build with the scanner, never scanned" — which is exactly what the rescan drain looks for, so a + // user's back catalogue gets scanned incrementally instead of only when a session happens to + // change. No backfill: a stamp we can't honestly claim would silently skip those sessions forever. + 24: { + to: 25, + sql: ` + ALTER TABLE resolved_sessions ADD COLUMN secret_scan_version INTEGER; + CREATE INDEX IF NOT EXISTS resolved_sessions_secret_scan_version + ON resolved_sessions(secret_scan_version); + `, + }, }; /** Apply the migration chain from `fromVersion` up to STORE_SCHEMA_VERSION, or throw if none exists. */ @@ -1753,7 +1791,7 @@ async function initializeDatabase(db: Database, path: string): Promise { await get(db, "SELECT file_id FROM index_sessions LIMIT 1"); await get( db, - "SELECT session_id, archived, title, summary, secret_scan_dismissed FROM resolved_sessions LIMIT 1", + "SELECT session_id, archived, title, summary, secret_scan_dismissed, secret_scan_version FROM resolved_sessions LIMIT 1", ); await get( db, @@ -3028,6 +3066,99 @@ export class SqliteStore implements Store { }); } + // Canonical ids of sessions the current scanner hasn't seen (#335), newest-first, capped at `limit`. + // The eligibility predicate is SECRET_SCAN_ELIGIBLE_SQL, shared with secretScanProgress. Newest-first + // so the sessions a user is most likely to look at get their warning first on a big backlog. + readPendingSecretScanSessions(version: number, limit: number): Promise { + return this.schedule(async () => { + const rows = await all<{ session_id: string }>( + this.db, + `SELECT s.session_id FROM resolved_sessions s + WHERE ${SECRET_SCAN_ELIGIBLE_SQL} + ORDER BY s.last_ts DESC, s.session_id + LIMIT ?`, + [version, limit], + ); + return rows.map((row) => row.session_id); + }); + } + + // The rescan drain's write (#335): replace this session's findings and stamp the scanner version, + // without re-materializing anything else — the counterpart to writeSessionTasks for interpretation. + // ALWAYS stamps, even for an empty finding set, so a clean session de-queues instead of being + // rescanned every pass. Deliberately leaves secret_scan_dismissed alone: the read path compares it + // against the fresh digest, so an unchanged finding set stays dismissed and a changed one re-warns. + writeSessionSecretFindings( + sessionId: string, + result: { version: number; digest: string; findings: SecretFinding[] }, + ): Promise { + return this.schedule(async () => { + await transaction(this.db, async () => { + await run( + this.db, + "DELETE FROM resolved_secret_findings WHERE session_id = ?", + [sessionId], + ); + if (result.findings.length) { + await insertRows( + this.db, + "resolved_secret_findings", + ["session_id", "seq", "category", "interaction_seq", "chunk_type", "hint", "findings_digest"], + result.findings.map((f, seq) => [ + sessionId, + seq, + f.category, + f.interactionSeq, + f.chunkType, + f.hint, + result.digest, + ]), + ); + } + await run( + this.db, + "UPDATE resolved_sessions SET secret_scan_version = ? WHERE session_id = ?", + [result.version, sessionId], + ); + }); + }); + } + + // Backlog progress for `argus status` (#335). scanned = stamped at least once. pending uses the SAME + // predicate the drain does (SECRET_SCAN_ELIGIBLE_SQL), so "waiting" can't desync from the work. + // unscannable is the retainText-off residue: needs a scan, HAS interactions, but no stored text for + // them, so the drain can never reach it — surfaced separately instead of sitting in a backlog that + // never shrinks. The interactions requirement matters: a session with nothing to scan at all (no + // interactions) isn't a retention casualty and would otherwise inflate the number for everyone. + secretScanProgress( + version: number, + ): Promise<{ scanned: number; pending: number; unscannable: number }> { + return this.schedule(async () => { + const pendingRow = await get<{ n: number }>( + this.db, + `SELECT COUNT(*) AS n FROM resolved_sessions s WHERE ${SECRET_SCAN_ELIGIBLE_SQL}`, + [version], + ); + const unscannableRow = await get<{ n: number }>( + this.db, + `SELECT COUNT(*) AS n FROM resolved_sessions s + WHERE (s.secret_scan_version IS NULL OR s.secret_scan_version < ?) + AND EXISTS (SELECT 1 FROM resolved_interactions i WHERE i.session_id = s.session_id) + AND NOT EXISTS (SELECT 1 FROM resolved_interaction_text t WHERE t.session_id = s.session_id)`, + [version], + ); + const scannedRow = await get<{ n: number }>( + this.db, + "SELECT COUNT(*) AS n FROM resolved_sessions WHERE secret_scan_version IS NOT NULL", + ); + return { + scanned: scannedRow?.n ?? 0, + pending: pendingRow?.n ?? 0, + unscannable: unscannableRow?.n ?? 0, + }; + }); + } + readSessionLabels(sessionId: string): Promise { return this.schedule(async () => { const rows = await all< @@ -4090,6 +4221,12 @@ export class SqliteStore implements Store { // read path compares it against the fresh findings' digest, so a dismissal survives a // re-materialize whose findings are unchanged and lapses when they differ. const secretScanDismissed = existingSnapshot?.secretScanDismissed ?? null; + // The scanner version stamp (#335) comes from the scan the caller just ran — NOT carried + // forward. A caller that didn't scan (a test, a programmatic materialize) leaves it NULL, + // which is honest: the wholesale DELETE below cascades the old findings away, so claiming + // "already scanned" would hide a session with no findings to show. NULL hands it to the + // rescan drain instead, which is self-healing. + const secretScanVersion = session.secretFindings?.version ?? null; // Replace this session wholesale (messages, tasks, and tool results cascade via FK). A freshly // materialized session is present on disk, so archived resets to 0. await run( @@ -4122,8 +4259,9 @@ export class SqliteStore implements Store { `INSERT INTO resolved_sessions( session_id, owner, source, project, cwd, first_ts, last_ts, message_count, first_prompt, archived, friction_interruptions, friction_rejections, friction_compactions, friction_turns, last_interruption_ms, - content_indexed_at_ms, interpreted_at_ms, interpretation_version, title, summary, is_hidden, secret_scan_dismissed, meta_json - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + content_indexed_at_ms, interpreted_at_ms, interpretation_version, title, summary, is_hidden, + secret_scan_dismissed, secret_scan_version, meta_json + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, [ sid, owner, @@ -4146,6 +4284,7 @@ export class SqliteStore implements Store { summary, isHidden ? 1 : 0, secretScanDismissed, + secretScanVersion, JSON.stringify(session.meta), ], ); diff --git a/src/watch.ts b/src/watch.ts index 19a87acc..17197c10 100644 --- a/src/watch.ts +++ b/src/watch.ts @@ -32,7 +32,7 @@ export interface WatchIndexDeps { extractTasks?: boolean, debug?: boolean, retainText?: boolean, - interpretCollapser?: RepeatCollapser, + collapser?: RepeatCollapser, ) => Promise; } @@ -44,14 +44,15 @@ export interface WatchIndexDeps { export async function watchIndex(opts: WatchIndexOptions, log: Log, signal: AbortSignal, deps: WatchIndexDeps = {}): Promise { const indexPass = deps.index ?? runIndex; const intervalMs = Math.max(MIN_INTERVAL_MIN, opts.intervalMin) * 60_000; - // One collapser for the whole watch lifetime so the interpretation drain's throttle-pause / failure - // lines (#153) are said once and not repeated every tick while the situation persists. - const interpretCollapser = new RepeatCollapser(log); + // One collapser for the whole watch lifetime so the post-index drains' throttle-pause / failure + // lines (interpretation #153, secret scanning #335) are said once and not repeated every tick while + // the situation persists. + const collapser = new RepeatCollapser(log); await superviseLoop( "indexing", async (sig) => { while (!sig.aborted) { - await indexPass(opts, log, opts.extractTasks, false, opts.retainText, interpretCollapser); + await indexPass(opts, log, opts.extractTasks, false, opts.retainText, collapser); await sleep(intervalMs, sig); } }, diff --git a/test/cli.test.ts b/test/cli.test.ts index abf5131d..c57f309c 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -5,6 +5,8 @@ import { createServer } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; import pkg from "../package.json" with { type: "json" }; +import { openStore } from "../src/store/store.ts"; +import type { MaterializeSession } from "../src/store/store-contract.ts"; // These exercise the citty argument layer end-to-end by running the real CLI. citty parses // non-strictly, so src/cli.ts adds an explicit guard (validateArgs) for unknown flags, value-less @@ -169,6 +171,46 @@ describe("index command group", () => { }); }); +/** A claude session with one retained human interaction — enough for `status` to count it and for the + * secret-scan backlog counts to have something to say about it. */ +function statusSession(sessionId: string): MaterializeSession { + const ts = 1_717_600_000_000; + return { + meta: { source: "claude", sessionId, project: "p", cwd: "/tmp/p", filePath: "/tmp/p/r.jsonl" }, + messages: [ + { + source: "claude", + sessionId, + project: "p", + cwd: "/tmp/p", + gitBranch: "main", + ts, + date: "2026-06-01", + model: "claude-opus-4", + usage: { input: 1, output: 1, cacheRead: 0, cacheWrite5m: 0, cacheWrite1h: 0 }, + attributionSkill: null, + toolUses: [], + }, + ], + interactions: [ + { + id: `${sessionId}-i0`, + source: "claude", + sourceSessionId: sessionId, + seq: 0, + initiator: "human", + disposition: "completed", + compactionCount: 0, + timestampMs: ts, + promptPosition: { originKey: "f", recordIndex: 0, itemIndex: 0 }, + position: { originKey: "f", recordIndex: 0, itemIndex: 0 }, + promptText: "an ordinary question", + responseText: "an ordinary answer", + }, + ], + }; +} + describe("read command output", () => { test("status prints its result even under --quiet", () => { const { status, stderr } = runCli(["status", "--quiet"]); @@ -192,6 +234,30 @@ describe("read command output", () => { expect(stderr).toContain("No sessions yet."); }); + test("status reports the secret-scan backlog and what can't be checked (#335)", async () => { + const dir = mkdtempSync(join(tmpdir(), "argus-cli-test-")); + const path = join(dir, "data", "argus.db"); + mkdirSync(join(dir, "data"), { recursive: true }); + // Two sessions the current scanner hasn't seen: one with retained text (the drain can reach it) + // and one indexed with retention off (only `index refresh` can). + const store = await openStore({ path }); + try { + await store.materializeSessions("claude", [statusSession("claude:waiting")]); + await store.materializeSessions("claude", [statusSession("claude:no-text")], { + retainText: false, + }); + } finally { + await store.close(); + } + + const { status, stderr } = runCli(["status"], dir); + expect(status).toBe(0); + expect(stderr).toContain("1 session waiting to be checked for exposed credentials."); + expect(stderr).toContain( + "1 session can't be checked for exposed credentials without re-reading its transcript", + ); + }); + test("config get prints an unset result even under --quiet", () => { const { status, stderr, stdout } = runCli([ "config", diff --git a/test/secret-findings.test.ts b/test/secret-findings.test.ts index 53c22255..8eae531f 100644 --- a/test/secret-findings.test.ts +++ b/test/secret-findings.test.ts @@ -7,7 +7,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { openStore } from "../src/store/store.ts"; import type { MaterializeSession, SecretFinding } from "../src/store/store-contract.ts"; -import { secretFindingsDigest } from "../src/indexing/secret-scan.ts"; +import { SECRET_SCAN_VERSION, secretFindingsDigest } from "../src/indexing/secret-scan.ts"; import { parseAllIncrementalDetailed } from "../src/indexing/pipeline.ts"; import type { MessageRecord } from "../src/types.ts"; @@ -57,9 +57,13 @@ function sessionWithFindings( return { meta: { source: "claude", sessionId, project: "p", cwd: "/tmp/p", filePath: "/tmp/p/s.jsonl" }, messages, - ...(findings.length - ? { secretFindings: { digest: secretFindingsDigest(findings), findings } } - : {}), + // Always attached, even with zero findings — that's what the pipeline does, so materialize stamps + // the scanner version (#335) and the rescan drain leaves the session alone. + secretFindings: { + version: SECRET_SCAN_VERSION, + digest: secretFindingsDigest(findings), + findings, + }, }; } diff --git a/test/secret-scan-drain.test.ts b/test/secret-scan-drain.test.ts new file mode 100644 index 00000000..8bab39a0 --- /dev/null +++ b/test/secret-scan-drain.test.ts @@ -0,0 +1,258 @@ +// The secret-scan version stamp and its backlog drain (#335): materialize stamps the scanner version, +// sessions the current scanner hasn't seen are eligible, the drain scans them from retained text and +// de-queues them, a version bump re-queues everything, and sessions with no retained text are reported +// rather than left in a backlog that never shrinks. +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { openStore } from "../src/store/store.ts"; +import { SECRET_SCAN_VERSION, secretFindingsDigest } from "../src/indexing/secret-scan.ts"; +import { runSecretScanDrain } from "../src/indexing/secret-scan-drain.ts"; +import type { MaterializeSession } from "../src/store/store-contract.ts"; + +// Synthesized, never a real credential: the AWS rule's shape (AKIA + 16 [A-Z2-7] chars, entropy ≥ 3). +const AWS_KEY = "AKIA" + "Q3G5X7BDFHJKLMNP"; + +const dirs: string[] = []; +afterEach(() => { + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +function storePath(): string { + const dir = mkdtempSync(join(tmpdir(), "argus-secret-drain-")); + dirs.push(dir); + return join(dir, "argus.db"); +} + +/** A session with one human interaction whose prompt text is retained — what the drain reads back. */ +function session(sid: string, promptText: string, ts = 1_717_600_000_000): MaterializeSession { + return { + meta: { source: "claude", sessionId: sid, project: "p", cwd: "/tmp/p", filePath: "/tmp/p/r.jsonl" }, + messages: [ + { + source: "claude", + sessionId: sid, + project: "p", + cwd: "/tmp/p", + gitBranch: "main", + ts, + date: "2026-06-01", + model: "claude-opus-4", + usage: { input: 1, output: 1, cacheRead: 0, cacheWrite5m: 0, cacheWrite1h: 0 }, + attributionSkill: null, + toolUses: [], + }, + ], + interactions: [ + { + id: `${sid}-i0`, + source: "claude", + sourceSessionId: sid, + seq: 0, + initiator: "human", + disposition: "completed", + compactionCount: 0, + timestampMs: ts, + promptPosition: { originKey: "f", recordIndex: 0, itemIndex: 0 }, + position: { originKey: "f", recordIndex: 0, itemIndex: 0 }, + promptText, + responseText: "rotated it", + }, + ], + }; +} + +/** The same session as the pipeline hands it over: scanned, so materialize stamps the version. */ +function scanned(s: MaterializeSession, findings: MaterializeSession["secretFindings"] = undefined) { + const resolved = findings ?? { version: SECRET_SCAN_VERSION, digest: secretFindingsDigest([]), findings: [] }; + return { ...s, secretFindings: resolved }; +} + +describe("secret-scan eligibility (#335)", () => { + test("a session materialized with a scan is stamped and not eligible", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [scanned(session("s:stamped", "nothing secret here"))]); + expect(await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION, 10)).toEqual([]); + const progress = await store.secretScanProgress(SECRET_SCAN_VERSION); + expect(progress).toEqual({ scanned: 1, pending: 0, unscannable: 0 }); + } finally { + await store.close(); + } + }); + + test("a session materialized without a scan is eligible — the upgraded-store case", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [session("s:unscanned", `key ${AWS_KEY} oops`)]); + expect(await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION, 10)).toEqual(["s:unscanned"]); + expect(await store.secretScanProgress(SECRET_SCAN_VERSION)).toEqual({ + scanned: 0, + pending: 1, + unscannable: 0, + }); + } finally { + await store.close(); + } + }); + + test("a newer scanner version re-queues an already-scanned session", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [scanned(session("s:old-rules", "nothing secret here"))]); + // Asked as the next scanner version would ask it: the stamp is now behind, so it comes back. + expect(await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION + 1, 10)).toEqual(["s:old-rules"]); + expect((await store.secretScanProgress(SECRET_SCAN_VERSION + 1)).pending).toBe(1); + } finally { + await store.close(); + } + }); + + test("eligible sessions come back newest-first", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [ + session("s:older", "a", 1_717_600_000_000), + session("s:newer", "b", 1_717_700_000_000), + ]); + expect(await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION, 10)).toEqual([ + "s:newer", + "s:older", + ]); + } finally { + await store.close(); + } + }); + + test("a session with no retained text is never eligible, and is reported as unscannable", async () => { + const store = await openStore({ path: storePath() }); + try { + // retainText off (#120): the text existed only in memory during materialize, so there is + // nothing in the store for the drain to read back. + await store.materializeSessions("me", [session("s:no-text", `key ${AWS_KEY} oops`)], { + retainText: false, + }); + expect(await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION, 10)).toEqual([]); + expect(await store.secretScanProgress(SECRET_SCAN_VERSION)).toEqual({ + scanned: 0, + pending: 0, + unscannable: 1, + }); + // And the drain leaves it alone rather than stamping a scan it couldn't do. + await runSecretScanDrain(store); + expect((await store.secretScanProgress(SECRET_SCAN_VERSION)).unscannable).toBe(1); + } finally { + await store.close(); + } + }); +}); + +describe("secret-scan backlog drain (#335)", () => { + test("scans an unstamped session from retained text, then de-queues it", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [session("s:drain", `aws key ${AWS_KEY} pasted`)]); + await runSecretScanDrain(store); + + const read = await store.readSessionSecretFindings("s:drain"); + expect(read.findings.map((f) => [f.category, f.hint, f.chunkType, f.interactionSeq])).toEqual([ + ["aws_access_key", "AKIA…LMNP", "prompt", 0], + ]); + expect(read.dismissed).toBe(false); + // Stamped, so nothing is waiting and a second pass has nothing to do. + expect(await store.secretScanProgress(SECRET_SCAN_VERSION)).toEqual({ + scanned: 1, + pending: 0, + unscannable: 0, + }); + await runSecretScanDrain(store); + expect((await store.readSessionSecretFindings("s:drain")).findings).toHaveLength(1); + } finally { + await store.close(); + } + }); + + test("a clean session is stamped too, so it doesn't come back every pass", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [session("s:clean", "just an ordinary question")]); + await runSecretScanDrain(store); + expect(await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION, 10)).toEqual([]); + expect((await store.readSessionSecretFindings("s:clean")).findings).toEqual([]); + } finally { + await store.close(); + } + }); + + test("a rescan finding the same thing keeps the dismissal; a different finding set re-warns", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [session("s:dismissed", `aws key ${AWS_KEY} pasted`)]); + await runSecretScanDrain(store); + expect(await store.dismissSessionSecretFindings("s:dismissed")).toBe(true); + expect((await store.readSessionSecretFindings("s:dismissed")).dismissed).toBe(true); + + // Same text, so the rescan reproduces the same digest: still dismissed. + await store.writeSessionSecretFindings("s:dismissed", { + version: SECRET_SCAN_VERSION, + digest: secretFindingsDigest((await store.readSessionSecretFindings("s:dismissed")).findings), + findings: (await store.readSessionSecretFindings("s:dismissed")).findings, + }); + expect((await store.readSessionSecretFindings("s:dismissed")).dismissed).toBe(true); + + // A rescan that finds something else (a rules refresh would) clears the dismissal by design. + const changed = [ + { category: "github_token" as const, interactionSeq: 0, chunkType: "prompt" as const, hint: "ghp_…4bC6" }, + ]; + await store.writeSessionSecretFindings("s:dismissed", { + version: SECRET_SCAN_VERSION, + digest: secretFindingsDigest(changed), + findings: changed, + }); + const after = await store.readSessionSecretFindings("s:dismissed"); + expect(after.dismissed).toBe(false); + expect(after.findings.map((f) => f.category)).toEqual(["github_token"]); + } finally { + await store.close(); + } + }); + + test("the drain's write replaces findings wholesale and leaves the rest of the session alone", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [session("s:replace", `aws key ${AWS_KEY} pasted`)]); + await runSecretScanDrain(store); + expect((await store.readSessionSecretFindings("s:replace")).findings).toHaveLength(1); + + // Nothing found this time: the old row goes, the stamp stays. + await store.writeSessionSecretFindings("s:replace", { + version: SECRET_SCAN_VERSION, + digest: secretFindingsDigest([]), + findings: [], + }); + expect((await store.readSessionSecretFindings("s:replace")).findings).toEqual([]); + expect(await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION, 10)).toEqual([]); + // The session itself is untouched — the drain never re-materializes. + expect((await store.readResolved()).sessions.has("s:replace")).toBe(true); + expect(await store.readSessionInteractionCount("s:replace")).toBe(1); + } finally { + await store.close(); + } + }); + + test("re-materializing without a scan hands the session back to the drain", async () => { + const store = await openStore({ path: storePath() }); + try { + await store.materializeSessions("me", [scanned(session("s:remat", `aws key ${AWS_KEY} pasted`))]); + // A materialize that didn't scan: the wholesale replace cascades the findings away, so the stamp + // must go with them rather than claiming a scan with nothing to show. + await store.materializeSessions("me", [session("s:remat", `aws key ${AWS_KEY} pasted`)]); + expect(await store.readPendingSecretScanSessions(SECRET_SCAN_VERSION, 10)).toEqual(["s:remat"]); + await runSecretScanDrain(store); + expect((await store.readSessionSecretFindings("s:remat")).findings).toHaveLength(1); + } finally { + await store.close(); + } + }); +}); diff --git a/test/store.test.ts b/test/store.test.ts index df0afafb..e0116d3e 100644 --- a/test/store.test.ts +++ b/test/store.test.ts @@ -220,6 +220,9 @@ function dropPostV18Schema(db: Database): void { // Post-v23 schema (#327): the findings table and the per-session dismissal record. rawExec(db, "DROP TABLE IF EXISTS resolved_secret_findings"); rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_dismissed"); + // Post-v24 schema (#335): the scanner version stamp and its index. + rawExec(db, "DROP INDEX IF EXISTS resolved_sessions_secret_scan_version"); + rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_version"); } function rawGet(db: Database, sql: string): T | undefined { @@ -576,6 +579,8 @@ describe("SQLite store", () => { await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN is_hidden"); await rawExec(db, "DROP TABLE IF EXISTS resolved_secret_findings"); await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_dismissed"); + await rawExec(db, "DROP INDEX IF EXISTS resolved_sessions_secret_scan_version"); + await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_version"); await rawExec(db, "PRAGMA user_version = 22"); }); @@ -1237,6 +1242,8 @@ describe("SQLite store", () => { await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN is_hidden"); await rawExec(db, "DROP TABLE IF EXISTS resolved_secret_findings"); await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_dismissed"); + await rawExec(db, "DROP INDEX IF EXISTS resolved_sessions_secret_scan_version"); + await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_version"); await rawExec(db, "PRAGMA user_version = 22"); }); @@ -1274,6 +1281,8 @@ describe("SQLite store", () => { await withRawDatabase(path, async (db) => { await rawExec(db, "DROP TABLE IF EXISTS resolved_secret_findings"); await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_dismissed"); + await rawExec(db, "DROP INDEX IF EXISTS resolved_sessions_secret_scan_version"); + await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_version"); await rawExec(db, "PRAGMA user_version = 23"); }); @@ -1300,6 +1309,60 @@ describe("SQLite store", () => { expect(version?.user_version).toBe(STORE_SCHEMA_VERSION); }); + test("v24 -> v25 migration adds the scanner stamp as NULL, so existing sessions get rescanned", async () => { + const path = storePath(); + const store = await openStore({ path }); + try { + await store.materializeSessions("claude", [ + { + meta: { source: "claude", sessionId: "claude:v24-scan", project: "p", cwd: "/tmp/p", filePath: "/tmp/p/r.jsonl" }, + messages: [], + interactions: [ + { + id: "claude:v24-scan-i0", + source: "claude", + sourceSessionId: "claude:v24-scan", + seq: 0, + initiator: "human", + disposition: "completed", + compactionCount: 0, + timestampMs: 1_717_600_000_000, + promptPosition: { originKey: "f", recordIndex: 0, itemIndex: 0 }, + position: { originKey: "f", recordIndex: 0, itemIndex: 0 }, + promptText: "an ordinary question", + responseText: "an ordinary answer", + }, + ], + secretFindings: { version: 1, digest: "d", findings: [] }, + }, + ]); + } finally { + await store.close(); + } + // Degrade to v24: drop the stamp column + index, simulating a store written by a build that had + // the scanner (#327) but no version stamp (#335). + await withRawDatabase(path, async (db) => { + await rawExec(db, "DROP INDEX IF EXISTS resolved_sessions_secret_scan_version"); + await rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_version"); + await rawExec(db, "PRAGMA user_version = 24"); + }); + + const migrated = await openStore({ path }); + try { + expect((await migrated.readResolved()).sessions.has("claude:v24-scan")).toBe(true); + // The whole point: the existing session migrates to "never scanned", so the drain picks it up + // instead of the user's back catalogue being silently skipped. + expect(await migrated.readPendingSecretScanSessions(1, 10)).toEqual(["claude:v24-scan"]); + expect(await migrated.secretScanProgress(1)).toEqual({ scanned: 0, pending: 1, unscannable: 0 }); + } finally { + await migrated.close(); + } + const version = await withRawDatabase(path, (db) => + rawGet<{ user_version: number }>(db, "PRAGMA user_version"), + ); + expect(version?.user_version).toBe(STORE_SCHEMA_VERSION); + }); + test("migrates a v8 store to v9, backfilling usage columns from record_json", async () => { const path = storePath(); const sid = "codex:backfill"; @@ -2167,11 +2230,11 @@ describe("session search (#155)", () => { await store.close(); } // Degrade to v19: drop the additions of every migration that will re-run from here — v20's search - // FTS/file_path index, v21's title/summary columns (#234), v23's is_hidden column, and v24's - // findings table + dismissal column (#327) (v22's label tables and v24's findings table are - // created with IF NOT EXISTS, so they don't strictly need dropping) — and set the version back, - // simulating an older store that already has interaction text + task data on disk but no search - // index yet. + // FTS/file_path index, v21's title/summary columns (#234), v23's is_hidden column, v24's + // findings table + dismissal column (#327), and v25's scanner version stamp (#335) (v22's label + // tables and v24's findings table are created with IF NOT EXISTS, so they don't strictly need + // dropping) — and set the version back, simulating an older store that already has interaction + // text + task data on disk but no search index yet. await withRawDatabase(path, (db) => { rawExec(db, "DROP TABLE resolved_interaction_text_fts"); rawExec(db, "DROP TABLE resolved_tasks_fts"); @@ -2183,6 +2246,8 @@ describe("session search (#155)", () => { rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN is_hidden"); rawExec(db, "DROP TABLE IF EXISTS resolved_secret_findings"); rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_dismissed"); + rawExec(db, "DROP INDEX IF EXISTS resolved_sessions_secret_scan_version"); + rawExec(db, "ALTER TABLE resolved_sessions DROP COLUMN secret_scan_version"); rawExec(db, "PRAGMA user_version = 19"); }); From e4b7f145136078c72a4c146af79a10db3e18752b Mon Sep 17 00:00:00 2001 From: Mando Escamilla Date: Mon, 17 Aug 2026 14:51:58 -0500 Subject: [PATCH 2/2] Harden SQLite files after secret scan drain writes --- src/store/store.ts | 1 + test/secret-scan-drain.test.ts | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/store/store.ts b/src/store/store.ts index 7f27c226..41b2cb2e 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -3121,6 +3121,7 @@ export class SqliteStore implements Store { [result.version, sessionId], ); }); + secureSqliteFiles(this.path); }); } diff --git a/test/secret-scan-drain.test.ts b/test/secret-scan-drain.test.ts index 8bab39a0..9c5bf8af 100644 --- a/test/secret-scan-drain.test.ts +++ b/test/secret-scan-drain.test.ts @@ -3,7 +3,7 @@ // de-queues them, a version bump re-queues everything, and sessions with no retained text are reported // rather than left in a backlog that never shrinks. import { afterEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync } from "node:fs"; +import { chmodSync, existsSync, mkdtempSync, rmSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { openStore } from "../src/store/store.ts"; @@ -173,6 +173,30 @@ describe("secret-scan backlog drain (#335)", () => { } }); + test("hardens database files after a drain write while the store remains open", async () => { + if (process.platform === "win32") return; + const path = storePath(); + const store = await openStore({ path }); + try { + await store.materializeSessions("me", [session("s:permissions", `aws key ${AWS_KEY} pasted`)]); + chmodSync(path, 0o644); + for (const suffix of ["-wal", "-shm"]) { + if (existsSync(`${path}${suffix}`)) chmodSync(`${path}${suffix}`, 0o644); + } + + await runSecretScanDrain(store); + + expect(statSync(path).mode & 0o777).toBe(0o600); + for (const suffix of ["-wal", "-shm"]) { + if (existsSync(`${path}${suffix}`)) { + expect(statSync(`${path}${suffix}`).mode & 0o777).toBe(0o600); + } + } + } finally { + await store.close(); + } + }); + test("a clean session is stamped too, so it doesn't come back every pass", async () => { const store = await openStore({ path: storePath() }); try {