From 61d45c925cf362a7dfcecf0c74770aa5a91df06c Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 01:38:31 -0700 Subject: [PATCH 01/10] fix(debug-retention): collect key-log-only proxy run dirs #1385 (q91 follow-up of #1380): a run dir holding only session-meta.json + sslkeylog.log was walked into and never collected, so its plaintext TLS secrets stayed on disk indefinitely (23 dirs, 5.18 MB on the owner's machine). A run dir is now recognised by either evidence file. Red before. HOLD for the owner's q91 decision: once collectable, the 48 h TTL pass removes these months-old dirs on the first prune, so this IS the sweep. Co-Authored-By: Claude Opus 5.5 --- ...9-27-retention-collects-keylog-run-dirs.md | 13 ++++++ .../storage/debugRetention.keylog.test.ts | 43 +++++++++++++++++++ src/main/storage/debugRetention.ts | 13 +++++- 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md create mode 100644 src/main/storage/debugRetention.keylog.test.ts diff --git a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md new file mode 100644 index 000000000..85f7540c1 --- /dev/null +++ b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md @@ -0,0 +1,13 @@ +# Debug retention collects key-log-only proxy run dirs (#1385) + +## Problem +`collectProxyRunDirs` recognised a run dir only by `proxy-events.jsonl`. A run dir holding just `session-meta.json` + `sslkeylog.log` was walked into and never collected. #1380 review c recounted names and sizes only (contents never read): 23 such dirs on the owner's machine, 5.18 MB of plaintext TLS session secrets, May–September 2026. + +## Fix +A run dir is recognised by either evidence file: `proxy-events.jsonl` or `sslkeylog.log`. Match the key log itself (review c). `session-meta.json` alone stays uncollected, and `_shared-conf` is still skipped. + +## CONSEQUENCE, needs the owner's decision BEFORE merge (q91) +Once collectable, these dirs fall under the normal TTL pass (48 h, `AGENT_CODE_DEBUG_TTL_HOURS`). All 23 are months old, so **the first prune after this merges deletes them**. This fix therefore IS the one-time sweep q91 asked the owner about. It permanently removes potential forensic material. If the owner says keep them, this PR must not merge as-is; the alternative is to budget them without TTL-expiring them. That is a different design, not needed if the answer is yes. + +## Tests +`debugRetention.keylog.test.ts`, on the real directory shapes (`proxy////`): a key-log-only dir is collected as a `proxy` dir artifact beside a normal run dir; `_shared-conf` and a metadata-only dir are not. It is red before the fix. diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts new file mode 100644 index 000000000..1a6ce91de --- /dev/null +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -0,0 +1,43 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, relative } from 'node:path' +import { afterEach, expect, it } from 'vitest' + +import { collectProxyRunDirs } from './debugRetention.js' + +// #1385 (q91 follow-up of #1380): retention collected a proxy run dir only +// once it held `proxy-events.jsonl`. A run dir holding just +// `session-meta.json` + `sslkeylog.log` was walked into, never collected, never +// budgeted and never removed, and those are plaintext TLS session secrets. The +// owner's machine had 23 such dirs (5.18 MB, May-September 2026; names and +// sizes recounted by #1380 review c, contents never read). Shapes below are +// those real layouts: proxy////. +const roots: string[] = [] +afterEach(() => { for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }) }) + +function runDir(root: string, parts: string[], files: Record): string { + const dir = join(root, ...parts) + mkdirSync(dir, { recursive: true }) + for (const [name, body] of Object.entries(files)) writeFileSync(join(dir, name), body) + return dir +} + +it('collects a key-log-only run dir as a proxy artifact, alongside normal run dirs', async () => { + const root = mkdtempSync(join(tmpdir(), 'proxy-retention-')) + roots.push(root) + runDir(root, ['medlo', 'shell-89d43b9b', '2026-08-28T17-30-06-452Z'], { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) + runDir(root, ['agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'], { 'session-meta.json': '{}', 'proxy-events.jsonl': '{}\n', 'sslkeylog.log': 'x'.repeat(1024) }) + // Shared mitmproxy state is never a run dir, whatever it holds. + runDir(root, ['_shared-conf'], { 'mitmproxy-ca-cert.pem': 'ca' }) + // Metadata alone is not a run's evidence; leave it for its own pass. + runDir(root, ['agent-code', 'shell-empty', '2026-09-01T00-00-00-000Z'], { 'session-meta.json': '{}' }) + + const artifacts = await collectProxyRunDirs(root) + expect(artifacts.map(artifact => relative(root, artifact.path)).sort()).toEqual([ + join('agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'), + join('medlo', 'shell-89d43b9b', '2026-08-28T17-30-06-452Z'), + ]) + const keyLogOnly = artifacts.find(artifact => artifact.path.includes('shell-89d43b9b'))! + expect(keyLogOnly).toMatchObject({ kind: 'dir', bucket: 'proxy' }) + expect(keyLogOnly.bytes).toBeGreaterThanOrEqual(4096) +}) diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index 5532ae0f1..f044ad68d 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -585,7 +585,9 @@ async function loadManualLegacyBundlePaths(): Promise> { return manual } -async function collectProxyRunDirs(root: string): Promise { +const PROXY_RUN_EVIDENCE = new Set(['proxy-events.jsonl', 'sslkeylog.log']) + +export async function collectProxyRunDirs(root: string): Promise { const out: Artifact[] = [] async function walk(dir: string, depth: number): Promise { let entries @@ -594,7 +596,14 @@ async function collectProxyRunDirs(root: string): Promise { } catch { return } - if (entries.some(entry => entry.isFile() && entry.name === 'proxy-events.jsonl')) { + // A run dir is recognised by its EVIDENCE files, either of them (#1385). + // Keying on proxy-events.jsonl alone missed run dirs that held only + // session-meta.json + sslkeylog.log: walked into, never collected, never + // budgeted, never removed. Those are plaintext TLS session secrets; the + // owner's machine had 23 such dirs (5.18 MB, May-September 2026, #1380 + // review c). Match the key log itself rather than assume it sits beside an + // events file. session-meta.json alone is NOT evidence of a run. + if (entries.some(entry => entry.isFile() && PROXY_RUN_EVIDENCE.has(entry.name))) { const artifact = await collectDirArtifact(dir, 'proxy') if (artifact) out.push(artifact) return From f72b614346d892f66b4a3bf6e135d4268e6d1737 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 07:11:28 -0700 Subject: [PATCH 02/10] fix(debug-retention): an unreadable child or bundle ledger protects, never counts as empty (q109, q115) dirStats skipped any child it could not read, so a run whose fresh data sat in an unreadable child was dated by its oldest file and TTL-removed. A failed read of the saved-bundles ledger returned an empty manual set, so manual legacy bundles were bucketed as prunable autosave. Only ENOENT now means absent; any other failure leaves the artifact uncollected. Co-Authored-By: Claude Opus 5.5 --- .../storage/debugRetention.keylog.test.ts | 101 +++++++++++++++++- src/main/storage/debugRetention.ts | 37 +++++-- 2 files changed, 127 insertions(+), 11 deletions(-) diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts index 1a6ce91de..18faac7ac 100644 --- a/src/main/storage/debugRetention.keylog.test.ts +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -1,9 +1,11 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, existsSync, mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from 'node:fs' +import { rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join, relative } from 'node:path' import { afterEach, expect, it } from 'vitest' -import { collectProxyRunDirs } from './debugRetention.js' +import { collectLegacyDebugBundleDirs, collectProxyRunDirs, loadManualLegacyBundlePaths, runPrunePasses } from './debugRetention.js' +import type { DebugStorageBucket, DebugStoragePrunePolicy } from './debugRetention.js' // #1385 (q91 follow-up of #1380): retention collected a proxy run dir only // once it held `proxy-events.jsonl`. A run dir holding just @@ -13,7 +15,11 @@ import { collectProxyRunDirs } from './debugRetention.js' // sizes recounted by #1380 review c, contents never read). Shapes below are // those real layouts: proxy////. const roots: string[] = [] -afterEach(() => { for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }) }) +const locked: string[] = [] +afterEach(() => { + for (const dir of locked.splice(0)) chmodSync(dir, 0o700) + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }) +}) function runDir(root: string, parts: string[], files: Record): string { const dir = join(root, ...parts) @@ -41,3 +47,92 @@ it('collects a key-log-only run dir as a proxy artifact, alongside normal run di expect(keyLogOnly).toMatchObject({ kind: 'dir', bucket: 'proxy' }) expect(keyLogOnly.bytes).toBeGreaterThanOrEqual(4096) }) + +// Worker rule "unknown is never empty" (q109, q115). dirStats swallowed EVERY +// child error as "best effort", so a child it could not read simply did not +// count: the run dir's age came from what WAS readable. A run whose fresh +// data sits in a child the pass cannot read (EACCES, EIO, EMFILE) looked as +// old as its oldest file, and the TTL pass removed it. An unreadable child is +// UNKNOWN, and unknown must protect the whole dir. Only ENOENT (a concurrent +// remover won the race) means "not there". Real filesystem throughout: fail +// once, recover, maintain, and the bytes survive; then a truly old dir still +// goes, so the protection is not permanent. +it('a run dir with a child it cannot read is protected, and is collected normally once readable again', async () => { + const root = mkdtempSync(join(tmpdir(), 'proxy-retention-')) + roots.push(root) + const now = Date.now() + const old = new Date(now - 30 * 24 * 3_600_000) + const dir = runDir(root, ['agent-code', 'shell-1', '2026-09-27T00-00-00-000Z'], { 'sslkeylog.log': 'k'.repeat(512) }) + utimesSync(join(dir, 'sslkeylog.log'), old, old) + const streams = join(dir, 'streams') + mkdirSync(streams) + writeFileSync(join(streams, 'fresh.bin'), 'f'.repeat(256)) + utimesSync(dir, old, old) + + const caps = {} as Record + for (const bucket of ['proxy'] as DebugStorageBucket[]) caps[bucket] = 1_000_000_000 + const policy: DebugStoragePrunePolicy = { now, ttlMs: 48 * 3_600_000, activeGraceMs: 10 * 60_000, budgetBytes: 1_000_000_000, caps } + const prune = async () => runPrunePasses(await collectProxyRunDirs(root), policy, async artifact => { + try { await rm(artifact.path, { recursive: true, force: true }); return true } catch { return false } + }) + + chmodSync(streams, 0o000) + locked.push(streams) + await prune() + expect(existsSync(join(dir, 'sslkeylog.log'))).toBe(true) + + chmodSync(streams, 0o700) + locked.splice(0) + await prune() + await prune() + expect(existsSync(join(dir, 'sslkeylog.log'))).toBe(true) + expect(existsSync(join(streams, 'fresh.bin'))).toBe(true) + + utimesSync(join(streams, 'fresh.bin'), old, old) + utimesSync(streams, old, old) + utimesSync(dir, old, old) + await prune() + expect(existsSync(dir)).toBe(false) +}) + +// Same rule, the PROTECT side. Legacy root-level bundles are classified manual +// (protected forever) or autosave (prunable) from the saved-bundles ledger. A +// failed ledger read returned an EMPTY manual set, so every manual legacy +// bundle was bucketed as autosave and aged out. Unknown must protect: a ledger +// that exists but cannot be read leaves legacy bundles uncollected that pass. +// Only a ledger that is not there (ENOENT) means "no manual bundles". +it('an unreadable bundle ledger protects legacy bundles, and they are classified normally once readable', async () => { + const root = mkdtempSync(join(tmpdir(), 'legacy-bundles-')) + roots.push(root) + const now = Date.now() + const old = new Date(now - 30 * 24 * 3_600_000) + const bundle = runDir(root, ['2026-05-01T10-00-00-000-manual-report'], { 'manifest.json': '{}' }) + utimesSync(join(bundle, 'manifest.json'), old, old) + utimesSync(bundle, old, old) + const ledger = join(root, 'saved-debug-bundles.jsonl') + writeFileSync(ledger, JSON.stringify({ event: 'saved', reason: 'manual', bundlePath: bundle }) + '\n') + + const caps = {} as Record + for (const bucket of ['debug-bundles-legacy', 'debug-bundles-manual'] as DebugStorageBucket[]) caps[bucket] = 1_000_000_000 + const policy: DebugStoragePrunePolicy = { now, ttlMs: 48 * 3_600_000, activeGraceMs: 10 * 60_000, budgetBytes: 1_000_000_000, caps } + const prune = async () => runPrunePasses( + await collectLegacyDebugBundleDirs(root, await loadManualLegacyBundlePaths(ledger)), policy, + async artifact => { try { await rm(artifact.path, { recursive: true, force: true }); return true } catch { return false } }) + + chmodSync(ledger, 0o000) + locked.push(ledger) + await prune() + expect(existsSync(join(bundle, 'manifest.json'))).toBe(true) + + chmodSync(ledger, 0o600) + locked.splice(0) + await prune() + await prune() + expect(existsSync(join(bundle, 'manifest.json'))).toBe(true) + + // A ledger that is not there really means "no manual bundles". + rmSync(ledger) + await prune() + expect(existsSync(bundle)).toBe(false) +}) + diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index f044ad68d..854db08bc 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -500,10 +500,13 @@ async function collectIncidentRunDirs(): Promise { })) } -async function collectLegacyDebugBundleDirs( +export async function collectLegacyDebugBundleDirs( dir: string, - manualLegacyBundlePaths: Set, + manualLegacyBundlePaths: Set | null, ): Promise { + // Unknown classification: collect nothing this pass rather than guess a + // bundle is autosave (see loadManualLegacyBundlePaths). + if (manualLegacyBundlePaths === null) return [] try { const entries = await readdir(dir, { withFileTypes: true }) // WHY legacy root folders are still collected: old versions wrote both @@ -554,13 +557,24 @@ function isProtectedFromDebugPrune(artifact: Artifact): boolean { artifact.bucket === 'debug-bundles-manual' } -async function loadManualLegacyBundlePaths(): Promise> { +/** + * The manual (protected) legacy bundles, or null when that is UNKNOWN. + * + * WHY null and not an empty set (q109, q115, "unknown is never empty"): this + * set is what PROTECTS a manual legacy bundle. An empty set on a failed read + * (EACCES, EIO, EMFILE) bucketed every manual legacy bundle as prunable + * autosave, and the TTL pass deleted user-saved incidents. Only ENOENT, a + * ledger that is not there, means "no manual bundles". `file` is a parameter + * so the real-fs test can point it at a temp ledger. + */ +export async function loadManualLegacyBundlePaths(file = DEBUG_BUNDLE_LOG_FILE): Promise | null> { const manual = new Set() let raw: string try { - raw = await readFile(DEBUG_BUNDLE_LOG_FILE, 'utf8') - } catch { - return manual + raw = await readFile(file, 'utf8') + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return manual + return null } for (const line of raw.split('\n')) { @@ -648,8 +662,15 @@ async function dirStats(path: string): Promise<{ bytes: number; mtimeMs: number bytes += childStats.size mtimeMs = Math.max(mtimeMs, childStats.mtimeMs) } - } catch { - // Best-effort accounting; a concurrent writer/remover can race us. + } catch (error) { + // Only ENOENT is "not there": a concurrent remover won the race, and the + // child's bytes are gone either way. Anything else (EACCES, EIO, + // EMFILE) is UNKNOWN, and unknown is never empty (q109, q115): skipping + // the child dated the dir by what WAS readable, so a run whose newest + // data sat in an unreadable child looked old and the TTL pass removed + // it. Rethrowing makes collectDirArtifact return null, which leaves the + // whole dir uncollected (protected) until a later pass can read it. + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error } } return { bytes, mtimeMs } From 608dc546dbf132b752cdd30dfeb7901b721f4047 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 07:48:05 -0700 Subject: [PATCH 03/10] revert(debug-retention): leave the manual-ledger loader to #1417 (q118) W4's #1417 owns loadManualLegacyBundlePaths and treats an absent ledger as unknown, stricter than this branch's ENOENT-means-empty. Keep only the dirStats fix; use #1417's loader as-is after it merges. Co-Authored-By: Claude Opus 5.5 --- .../storage/debugRetention.keylog.test.ts | 44 +------------------ src/main/storage/debugRetention.ts | 26 +++-------- 2 files changed, 7 insertions(+), 63 deletions(-) diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts index 18faac7ac..232197d14 100644 --- a/src/main/storage/debugRetention.keylog.test.ts +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os' import { join, relative } from 'node:path' import { afterEach, expect, it } from 'vitest' -import { collectLegacyDebugBundleDirs, collectProxyRunDirs, loadManualLegacyBundlePaths, runPrunePasses } from './debugRetention.js' +import { collectProxyRunDirs, runPrunePasses } from './debugRetention.js' import type { DebugStorageBucket, DebugStoragePrunePolicy } from './debugRetention.js' // #1385 (q91 follow-up of #1380): retention collected a proxy run dir only @@ -94,45 +94,3 @@ it('a run dir with a child it cannot read is protected, and is collected normall await prune() expect(existsSync(dir)).toBe(false) }) - -// Same rule, the PROTECT side. Legacy root-level bundles are classified manual -// (protected forever) or autosave (prunable) from the saved-bundles ledger. A -// failed ledger read returned an EMPTY manual set, so every manual legacy -// bundle was bucketed as autosave and aged out. Unknown must protect: a ledger -// that exists but cannot be read leaves legacy bundles uncollected that pass. -// Only a ledger that is not there (ENOENT) means "no manual bundles". -it('an unreadable bundle ledger protects legacy bundles, and they are classified normally once readable', async () => { - const root = mkdtempSync(join(tmpdir(), 'legacy-bundles-')) - roots.push(root) - const now = Date.now() - const old = new Date(now - 30 * 24 * 3_600_000) - const bundle = runDir(root, ['2026-05-01T10-00-00-000-manual-report'], { 'manifest.json': '{}' }) - utimesSync(join(bundle, 'manifest.json'), old, old) - utimesSync(bundle, old, old) - const ledger = join(root, 'saved-debug-bundles.jsonl') - writeFileSync(ledger, JSON.stringify({ event: 'saved', reason: 'manual', bundlePath: bundle }) + '\n') - - const caps = {} as Record - for (const bucket of ['debug-bundles-legacy', 'debug-bundles-manual'] as DebugStorageBucket[]) caps[bucket] = 1_000_000_000 - const policy: DebugStoragePrunePolicy = { now, ttlMs: 48 * 3_600_000, activeGraceMs: 10 * 60_000, budgetBytes: 1_000_000_000, caps } - const prune = async () => runPrunePasses( - await collectLegacyDebugBundleDirs(root, await loadManualLegacyBundlePaths(ledger)), policy, - async artifact => { try { await rm(artifact.path, { recursive: true, force: true }); return true } catch { return false } }) - - chmodSync(ledger, 0o000) - locked.push(ledger) - await prune() - expect(existsSync(join(bundle, 'manifest.json'))).toBe(true) - - chmodSync(ledger, 0o600) - locked.splice(0) - await prune() - await prune() - expect(existsSync(join(bundle, 'manifest.json'))).toBe(true) - - // A ledger that is not there really means "no manual bundles". - rmSync(ledger) - await prune() - expect(existsSync(bundle)).toBe(false) -}) - diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index 854db08bc..0e4f38700 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -500,13 +500,10 @@ async function collectIncidentRunDirs(): Promise { })) } -export async function collectLegacyDebugBundleDirs( +async function collectLegacyDebugBundleDirs( dir: string, - manualLegacyBundlePaths: Set | null, + manualLegacyBundlePaths: Set, ): Promise { - // Unknown classification: collect nothing this pass rather than guess a - // bundle is autosave (see loadManualLegacyBundlePaths). - if (manualLegacyBundlePaths === null) return [] try { const entries = await readdir(dir, { withFileTypes: true }) // WHY legacy root folders are still collected: old versions wrote both @@ -557,24 +554,13 @@ function isProtectedFromDebugPrune(artifact: Artifact): boolean { artifact.bucket === 'debug-bundles-manual' } -/** - * The manual (protected) legacy bundles, or null when that is UNKNOWN. - * - * WHY null and not an empty set (q109, q115, "unknown is never empty"): this - * set is what PROTECTS a manual legacy bundle. An empty set on a failed read - * (EACCES, EIO, EMFILE) bucketed every manual legacy bundle as prunable - * autosave, and the TTL pass deleted user-saved incidents. Only ENOENT, a - * ledger that is not there, means "no manual bundles". `file` is a parameter - * so the real-fs test can point it at a temp ledger. - */ -export async function loadManualLegacyBundlePaths(file = DEBUG_BUNDLE_LOG_FILE): Promise | null> { +async function loadManualLegacyBundlePaths(): Promise> { const manual = new Set() let raw: string try { - raw = await readFile(file, 'utf8') - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return manual - return null + raw = await readFile(DEBUG_BUNDLE_LOG_FILE, 'utf8') + } catch { + return manual } for (const line of raw.split('\n')) { From 949b2d9764e45ce6d4ffc4130e0aef1f108864cd Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 10:41:02 -0700 Subject: [PATCH 04/10] fix(debug-retention): collect key-log-only run dirs from new runs only; existing key logs stay (#1385) The 23 existing key-log-only dirs are an owner decision (q91). Collection now starts at runs dated on or after 2026-09-28; earlier or undated dirs are left untouched and never walked into. Co-Authored-By: Claude Opus 5.5 --- ...9-27-retention-collects-keylog-run-dirs.md | 22 ++++++++--- .../storage/debugRetention.keylog.test.ts | 15 ++++++-- src/main/storage/debugRetention.ts | 38 ++++++++++++++----- 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md index 85f7540c1..dcf916ab3 100644 --- a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md +++ b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md @@ -3,11 +3,23 @@ ## Problem `collectProxyRunDirs` recognised a run dir only by `proxy-events.jsonl`. A run dir holding just `session-meta.json` + `sslkeylog.log` was walked into and never collected. #1380 review c recounted names and sizes only (contents never read): 23 such dirs on the owner's machine, 5.18 MB of plaintext TLS session secrets, May–September 2026. -## Fix -A run dir is recognised by either evidence file: `proxy-events.jsonl` or `sslkeylog.log`. Match the key log itself (review c). `session-meta.json` alone stays uncollected, and `_shared-conf` is still skipped. +## Fix (narrowed to FUTURE runs; B6's oldest-first list) +- A run dir is recognised by either evidence file: `proxy-events.jsonl` or `sslkeylog.log`. It matches the key log itself (review c). +- A dir with `proxy-events.jsonl` is collected as before. +- A key-log-only dir is collected only when its run started at or after `KEY_LOG_ONLY_SINCE` (`2026-09-28T00-00-00-000Z`). Run dirs are named by their ISO start time, which sorts as text. +- Every earlier key-log-only dir, including the owner's 23, is left untouched and never walked into. So is one whose name cannot be dated. +- `session-meta.json` alone stays uncollected, and `_shared-conf` is still skipped. -## CONSEQUENCE, needs the owner's decision BEFORE merge (q91) -Once collectable, these dirs fall under the normal TTL pass (48 h, `AGENT_CODE_DEBUG_TTL_HOURS`). All 23 are months old, so **the first prune after this merges deletes them**. This fix therefore IS the one-time sweep q91 asked the owner about. It permanently removes potential forensic material. If the owner says keep them, this PR must not merge as-is; the alternative is to budget them without TTL-expiring them. That is a different design, not needed if the answer is yes. +## Owner decision kept (q91) +- Deleting the EXISTING key logs is still the owner's decision. This PR no longer makes it: the first prune after merge does not touch any of the 23 dirs. +- New key-log-only runs fall under the normal TTL pass (48 h, `AGENT_CODE_DEBUG_TTL_HOURS`) and the proxy budget. + +## Also (q115, "unknown is never empty") +`dirStats` no longer skips a child it cannot read. Only ENOENT means absent; any other error leaves the whole dir uncollected that pass. The manual-bundle ledger loader is NOT changed here, because W4's #1417 owns it (q118). ## Tests -`debugRetention.keylog.test.ts`, on the real directory shapes (`proxy////`): a key-log-only dir is collected as a `proxy` dir artifact beside a normal run dir; `_shared-conf` and a metadata-only dir are not. It is red before the fix. +`debugRetention.keylog.test.ts`, on the real directory shapes (`proxy////`): +- a NEW key-log-only dir is collected beside a normal run dir; +- an existing-dated one, an undated one, `_shared-conf` and a metadata-only dir are not; +- the unreadable-child test: fail once, recover, maintain, and the bytes survive. +- Mutations killed: removing the cutoff, and removing the name check. diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts index 232197d14..ed063c193 100644 --- a/src/main/storage/debugRetention.keylog.test.ts +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -28,10 +28,17 @@ function runDir(root: string, parts: string[], files: Record): s return dir } -it('collects a key-log-only run dir as a proxy artifact, alongside normal run dirs', async () => { +// Narrowed to FUTURE runs (B6 oldest-first list, owner decision q91 kept): +// a key-log-only dir is collected only when it started on or after the +// cutoff. The existing ones (the owner's 23, May-September 2026, shaped like +// medlo/shell-89d43b9b below) are left untouched, and so is one whose name +// cannot be dated. +it('collects a NEW key-log-only run dir, never an existing one, alongside normal run dirs', async () => { const root = mkdtempSync(join(tmpdir(), 'proxy-retention-')) roots.push(root) runDir(root, ['medlo', 'shell-89d43b9b', '2026-08-28T17-30-06-452Z'], { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) + runDir(root, ['medlo', 'shell-7a1b2c3d', '2026-09-29T08-15-00-000Z'], { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) + runDir(root, ['medlo', 'shell-undated', 'run-without-a-timestamp'], { 'sslkeylog.log': 'x'.repeat(128) }) runDir(root, ['agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'], { 'session-meta.json': '{}', 'proxy-events.jsonl': '{}\n', 'sslkeylog.log': 'x'.repeat(1024) }) // Shared mitmproxy state is never a run dir, whatever it holds. runDir(root, ['_shared-conf'], { 'mitmproxy-ca-cert.pem': 'ca' }) @@ -41,9 +48,9 @@ it('collects a key-log-only run dir as a proxy artifact, alongside normal run di const artifacts = await collectProxyRunDirs(root) expect(artifacts.map(artifact => relative(root, artifact.path)).sort()).toEqual([ join('agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'), - join('medlo', 'shell-89d43b9b', '2026-08-28T17-30-06-452Z'), + join('medlo', 'shell-7a1b2c3d', '2026-09-29T08-15-00-000Z'), ]) - const keyLogOnly = artifacts.find(artifact => artifact.path.includes('shell-89d43b9b'))! + const keyLogOnly = artifacts.find(artifact => artifact.path.includes('shell-7a1b2c3d'))! expect(keyLogOnly).toMatchObject({ kind: 'dir', bucket: 'proxy' }) expect(keyLogOnly.bytes).toBeGreaterThanOrEqual(4096) }) @@ -62,7 +69,7 @@ it('a run dir with a child it cannot read is protected, and is collected normall roots.push(root) const now = Date.now() const old = new Date(now - 30 * 24 * 3_600_000) - const dir = runDir(root, ['agent-code', 'shell-1', '2026-09-27T00-00-00-000Z'], { 'sslkeylog.log': 'k'.repeat(512) }) + const dir = runDir(root, ['agent-code', 'shell-1', '2026-09-29T00-00-00-000Z'], { 'sslkeylog.log': 'k'.repeat(512) }) utimesSync(join(dir, 'sslkeylog.log'), old, old) const streams = join(dir, 'streams') mkdirSync(streams) diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index 0e4f38700..2d279a723 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -1,5 +1,5 @@ import { mkdir, readFile, readdir, rm, stat, statfs } from 'node:fs/promises' -import { dirname, join, resolve } from 'node:path' +import { basename, dirname, join, resolve } from 'node:path' import { AUTOSAVE_DEBUG_BUNDLE_DIR, @@ -585,7 +585,21 @@ async function loadManualLegacyBundlePaths(): Promise> { return manual } -const PROXY_RUN_EVIDENCE = new Set(['proxy-events.jsonl', 'sslkeylog.log']) +/** + * Key-log-only run dirs are collected FORWARD ONLY (#1385, owner-approved + * narrowing in B6's oldest-first list): only those whose run timestamp is at + * or after this cutoff. Every earlier one, including the 23 dirs found on the + * owner's machine (May-September 2026), is left exactly as it is, because + * deleting existing TLS key logs is an owner decision (q91) that this PR does + * not make. The cutoff is the day this narrowing was written, so every run + * made by a build that contains it is newer. + * + * Run dirs are named by their ISO start time with `:` and `.` replaced + * (`2026-08-28T17-30-06-452Z`), which sorts as text. A name that is not in + * that shape cannot be dated, so it is not collected (unknown is never "new"). + */ +const KEY_LOG_ONLY_SINCE = '2026-09-28T00-00-00-000Z' +const RUN_DIR_NAME = /^\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z$/ export async function collectProxyRunDirs(root: string): Promise { const out: Artifact[] = [] @@ -596,16 +610,22 @@ export async function collectProxyRunDirs(root: string): Promise { } catch { return } - // A run dir is recognised by its EVIDENCE files, either of them (#1385). - // Keying on proxy-events.jsonl alone missed run dirs that held only + // A run dir is recognised by its EVIDENCE files (#1385). Keying on + // proxy-events.jsonl alone missed run dirs that held only // session-meta.json + sslkeylog.log: walked into, never collected, never // budgeted, never removed. Those are plaintext TLS session secrets; the // owner's machine had 23 such dirs (5.18 MB, May-September 2026, #1380 - // review c). Match the key log itself rather than assume it sits beside an - // events file. session-meta.json alone is NOT evidence of a run. - if (entries.some(entry => entry.isFile() && PROXY_RUN_EVIDENCE.has(entry.name))) { - const artifact = await collectDirArtifact(dir, 'proxy') - if (artifact) out.push(artifact) + // review c). A key-log-only dir is a run dir either way (never walked + // into), but it is COLLECTED only when it is new (KEY_LOG_ONLY_SINCE). + // session-meta.json alone is NOT evidence of a run. + const files = new Set(entries.filter(entry => entry.isFile()).map(entry => entry.name)) + if (files.has('proxy-events.jsonl') || files.has('sslkeylog.log')) { + const name = basename(dir) + const collectable = files.has('proxy-events.jsonl') || (RUN_DIR_NAME.test(name) && name >= KEY_LOG_ONLY_SINCE) + if (collectable) { + const artifact = await collectDirArtifact(dir, 'proxy') + if (artifact) out.push(artifact) + } return } if (depth >= 4) return From d8c62e984e0741a2f941f2eff3e334ce03a1d615 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 11:24:42 -0700 Subject: [PATCH 05/10] fix(debug-retention): key-log-only collection starts at this machine's first pass, not a date constant (#1385, #1388 review a) Co-Authored-By: Claude Opus 5.5 --- ...9-27-retention-collects-keylog-run-dirs.md | 10 ++- .../storage/debugRetention.keylog.test.ts | 43 +++++++++++-- src/main/storage/debugRetention.ts | 64 +++++++++++++++---- 3 files changed, 95 insertions(+), 22 deletions(-) diff --git a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md index dcf916ab3..3b2b1a3df 100644 --- a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md +++ b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md @@ -6,7 +6,8 @@ ## Fix (narrowed to FUTURE runs; B6's oldest-first list) - A run dir is recognised by either evidence file: `proxy-events.jsonl` or `sslkeylog.log`. It matches the key log itself (review c). - A dir with `proxy-events.jsonl` is collected as before. -- A key-log-only dir is collected only when its run started at or after `KEY_LOG_ONLY_SINCE` (`2026-09-28T00-00-00-000Z`). Run dirs are named by their ISO start time, which sorts as text. +- A key-log-only dir is collected only when its run started AFTER this machine's first retention pass with this code: `keyLogRetentionSince()` writes that moment once (exclusive create) to `STATE_DIR/debug-retention-keylog-since`, and later passes read it. Run dirs are named by their ISO start time, which sorts as text, and the marker uses the same shape. If the marker cannot be read or written, or has an unknown shape, NO key-log-only dir is collected (fail closed). +- **WHY not a date constant (#1388 review a):** the first version used tomorrow's date, so a run this build made today was excluded forever, and any earlier date would sweep key logs from before the upgrade. - Every earlier key-log-only dir, including the owner's 23, is left untouched and never walked into. So is one whose name cannot be dated. - `session-meta.json` alone stays uncollected, and `_shared-conf` is still skipped. @@ -23,3 +24,10 @@ - an existing-dated one, an undated one, `_shared-conf` and a metadata-only dir are not; - the unreadable-child test: fail once, recover, maintain, and the bytes survive. - Mutations killed: removing the cutoff, and removing the name check. + +## Review a (round 1) +- **Fixed:** the date constant replaced by the first-run marker (above). +- **Tests:** a same-day run after the marker is collected; one before it is not; a null cutoff collects no key-log-only dir; the marker is written once and kept, and fails closed on an unknown shape or an unreadable path. +- **Mutations killed:** no cutoff; null collecting everything; any marker shape accepted. Removing the up-front marker read ALONE survives, because the exclusive create then hits EEXIST and reads the stored marker. Removing both guards fails. +- **Not changed (finding 1):** a run dir with an events file AND a key log is collected whole, key log included. That is main's existing behaviour for event-bearing runs, which this PR does not touch. The owner decision (q91) is about the key-log-only dirs, which stay untouched. + diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts index ed063c193..14efe7122 100644 --- a/src/main/storage/debugRetention.keylog.test.ts +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os' import { join, relative } from 'node:path' import { afterEach, expect, it } from 'vitest' -import { collectProxyRunDirs, runPrunePasses } from './debugRetention.js' +import { collectProxyRunDirs, keyLogRetentionSince, runPrunePasses } from './debugRetention.js' import type { DebugStorageBucket, DebugStoragePrunePolicy } from './debugRetention.js' // #1385 (q91 follow-up of #1380): retention collected a proxy run dir only @@ -29,26 +29,36 @@ function runDir(root: string, parts: string[], files: Record): s } // Narrowed to FUTURE runs (B6 oldest-first list, owner decision q91 kept): -// a key-log-only dir is collected only when it started on or after the -// cutoff. The existing ones (the owner's 23, May-September 2026, shaped like -// medlo/shell-89d43b9b below) are left untouched, and so is one whose name -// cannot be dated. +// a key-log-only dir is collected only when it started after this machine's +// first retention pass with this code (the marker, below). The existing ones +// (the owner's 23, May-September 2026, shaped like medlo/shell-89d43b9b) are +// left untouched, and so is one whose name cannot be dated. #1388 review a: +// a run made the SAME DAY, after the marker, is collected (a date constant +// excluded it forever). it('collects a NEW key-log-only run dir, never an existing one, alongside normal run dirs', async () => { const root = mkdtempSync(join(tmpdir(), 'proxy-retention-')) roots.push(root) runDir(root, ['medlo', 'shell-89d43b9b', '2026-08-28T17-30-06-452Z'], { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) runDir(root, ['medlo', 'shell-7a1b2c3d', '2026-09-29T08-15-00-000Z'], { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) runDir(root, ['medlo', 'shell-undated', 'run-without-a-timestamp'], { 'sslkeylog.log': 'x'.repeat(128) }) + runDir(root, ['medlo', 'shell-same-day-before', '2026-09-27T17-59-59-999Z'], { 'sslkeylog.log': 'x'.repeat(128) }) + runDir(root, ['medlo', 'shell-same-day-after', '2026-09-27T19-00-00-000Z'], { 'sslkeylog.log': 'x'.repeat(128) }) runDir(root, ['agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'], { 'session-meta.json': '{}', 'proxy-events.jsonl': '{}\n', 'sslkeylog.log': 'x'.repeat(1024) }) // Shared mitmproxy state is never a run dir, whatever it holds. runDir(root, ['_shared-conf'], { 'mitmproxy-ca-cert.pem': 'ca' }) // Metadata alone is not a run's evidence; leave it for its own pass. runDir(root, ['agent-code', 'shell-empty', '2026-09-01T00-00-00-000Z'], { 'session-meta.json': '{}' }) - const artifacts = await collectProxyRunDirs(root) + const since = '2026-09-27T18-00-00-000Z' + const artifacts = await collectProxyRunDirs(root, since) expect(artifacts.map(artifact => relative(root, artifact.path)).sort()).toEqual([ join('agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'), join('medlo', 'shell-7a1b2c3d', '2026-09-29T08-15-00-000Z'), + join('medlo', 'shell-same-day-after', '2026-09-27T19-00-00-000Z'), + ]) + // With no established cutoff, no key-log-only dir is collected at all. + expect((await collectProxyRunDirs(root, null)).map(artifact => relative(root, artifact.path))).toEqual([ + join('agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'), ]) const keyLogOnly = artifacts.find(artifact => artifact.path.includes('shell-7a1b2c3d'))! expect(keyLogOnly).toMatchObject({ kind: 'dir', bucket: 'proxy' }) @@ -79,7 +89,7 @@ it('a run dir with a child it cannot read is protected, and is collected normall const caps = {} as Record for (const bucket of ['proxy'] as DebugStorageBucket[]) caps[bucket] = 1_000_000_000 const policy: DebugStoragePrunePolicy = { now, ttlMs: 48 * 3_600_000, activeGraceMs: 10 * 60_000, budgetBytes: 1_000_000_000, caps } - const prune = async () => runPrunePasses(await collectProxyRunDirs(root), policy, async artifact => { + const prune = async () => runPrunePasses(await collectProxyRunDirs(root, '2026-09-28T00-00-00-000Z'), policy, async artifact => { try { await rm(artifact.path, { recursive: true, force: true }); return true } catch { return false } }) @@ -101,3 +111,22 @@ it('a run dir with a child it cannot read is protected, and is collected normall await prune() expect(existsSync(dir)).toBe(false) }) + +// The marker behind "future runs only" (#1388 review a). The first pass +// records the moment this machine started collecting and later passes keep +// it. An unreadable marker, or one in an unknown shape, yields null, which +// collects NO key-log-only run (an unknown cutoff never widens collection). +it('records the key-log cutoff once, keeps it, and fails closed when it cannot be read', async () => { + const dir = mkdtempSync(join(tmpdir(), 'keylog-since-')) + roots.push(dir) + const file = join(dir, 'state', 'debug-retention-keylog-since') + const first = await keyLogRetentionSince(file, () => new Date('2026-09-27T18:20:27.123Z')) + expect(first).toBe('2026-09-27T18-20-27-123Z') + expect(await keyLogRetentionSince(file, () => new Date('2026-10-01T00:00:00.000Z'))).toBe(first) + writeFileSync(file, 'not a timestamp') + expect(await keyLogRetentionSince(file)).toBeNull() + rmSync(file) + mkdirSync(file) + expect(await keyLogRetentionSince(file)).toBeNull() +}) + diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index 804fd9e51..dab096e21 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile, readdir, rm, stat, statfs } from 'node:fs/promises' +import { mkdir, readFile, readdir, rm, stat, statfs, writeFile } from 'node:fs/promises' import { basename, dirname, join, resolve } from 'node:path' import { @@ -456,12 +456,13 @@ function bucketCaps(totalBudget: number): Record { async function collectArtifacts(): Promise { const manualLegacyBundlePaths = await loadManualLegacyBundlePaths() + const keyLogOnlySince = await keyLogRetentionSince() const [feed, manualBundles, autosaveBundles, legacyBundles, proxy, performance, incidents, heapSnapshots, sessionRecordings] = await Promise.all([ collectFiles(FEED_DEBUG_DIR, 'feed-debug', name => name.endsWith('.jsonl')), collectImmediateDirs(MANUAL_DEBUG_BUNDLE_DIR, 'debug-bundles-manual'), collectImmediateDirs(AUTOSAVE_DEBUG_BUNDLE_DIR, 'debug-bundles-autosave'), collectLegacyDebugBundleDirs(DEBUG_BUNDLE_DIR, manualLegacyBundlePaths), - collectProxyRunDirs(PROXY_EVENTS_DIR), + collectProxyRunDirs(PROXY_EVENTS_DIR, keyLogOnlySince), collectImmediateDirs(PERFORMANCE_RUNS_DIR, 'performance'), collectIncidentRunDirs(), collectFiles(HEAP_SNAPSHOT_DIR, 'heap-snapshots', name => name.endsWith('.heapsnapshot')), @@ -668,21 +669,56 @@ const PROXY_RUN_MARKERS = new Set(['proxy-events.jsonl', 'proxy-events.1.jsonl'] /** * Key-log-only run dirs are collected FORWARD ONLY (#1385, owner-approved - * narrowing in B6's oldest-first list): only those whose run timestamp is at - * or after this cutoff. Every earlier one, including the 23 dirs found on the - * owner's machine (May-September 2026), is left exactly as it is, because - * deleting existing TLS key logs is an owner decision (q91) that this PR does - * not make. The cutoff is the day this narrowing was written, so every run - * made by a build that contains it is newer. + * narrowing in B6's oldest-first list): only runs that started after the + * first retention pass of a build containing this code. Every earlier one, + * including the 23 dirs found on the owner's machine (May-September 2026), is + * left exactly as it is: deleting existing TLS key logs is an owner decision + * (q91) this PR does not make. + * + * WHY a marker written on first run and not a date constant (#1388 review a): + * a fixed cutoff can only approximate "after the upgrade". The first version + * used tomorrow's date, so a run this build made TODAY was excluded forever + * (its name never crosses the cutoff), and an earlier date would have swept + * key logs older than the upgrade. The marker records the actual moment this + * machine started collecting. * * Run dirs are named by their ISO start time with `:` and `.` replaced - * (`2026-08-28T17-30-06-452Z`), which sorts as text. A name that is not in - * that shape cannot be dated, so it is not collected (unknown is never "new"). + * (`2026-08-28T17-30-06-452Z`), which sorts as text, and the marker uses the + * same shape. A name that is not in that shape cannot be dated, so it is not + * collected (unknown is never "new"). */ -const KEY_LOG_ONLY_SINCE = '2026-09-28T00-00-00-000Z' const RUN_DIR_NAME = /^\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z$/ +const KEY_LOG_SINCE_FILE = join(STATE_DIR, 'debug-retention-keylog-since') + +/** + * When this machine started collecting key-log-only runs, in run-dir name + * form, or null when that cannot be established. Null collects NO key-log-only + * run (fail closed: an unknown cutoff must never widen collection to old key + * logs). The first call writes the marker exclusively, so two passes racing + * agree on one value. + */ +export async function keyLogRetentionSince(file = KEY_LOG_SINCE_FILE, now: () => Date = () => new Date()): Promise { + const readMarker = async (): Promise => { + const value = (await readFile(file, 'utf8')).trim() + return RUN_DIR_NAME.test(value) ? value : null + } + try { + return await readMarker() + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') return null + } + const since = now().toISOString().replace(/[:.]/g, '-') + try { + await mkdir(dirname(file), { recursive: true }) + await writeFile(file, since, { flag: 'wx', mode: 0o600 }) + return since + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EEXIST') return readMarker().catch(() => null) + return null + } +} -export async function collectProxyRunDirs(root: string): Promise { +export async function collectProxyRunDirs(root: string, keyLogOnlySince: string | null = null): Promise { const out: Artifact[] = [] async function walk(dir: string, depth: number): Promise { let entries @@ -697,7 +733,7 @@ export async function collectProxyRunDirs(root: string): Promise { // budgeted, never removed. Those are plaintext TLS session secrets; the // owner's machine had 23 such dirs (5.18 MB, May-September 2026, #1380 // review c). A key-log-only dir is a run dir either way (never walked - // into), but it is COLLECTED only when it is new (KEY_LOG_ONLY_SINCE). + // into), but it is COLLECTED only when it started after keyLogOnlySince. // session-meta.json alone is NOT evidence of a run. const files = new Set(entries.filter(entry => entry.isFile()).map(entry => entry.name)) // Events markers (the live file or its rotated `.1` generation, #1376) @@ -705,7 +741,7 @@ export async function collectProxyRunDirs(root: string): Promise { const hasEvents = [...PROXY_RUN_MARKERS].some(marker => files.has(marker)) if (hasEvents || files.has('sslkeylog.log')) { const name = basename(dir) - const collectable = hasEvents || (RUN_DIR_NAME.test(name) && name >= KEY_LOG_ONLY_SINCE) + const collectable = hasEvents || (keyLogOnlySince !== null && RUN_DIR_NAME.test(name) && name > keyLogOnlySince) if (collectable) { const artifact = await collectDirArtifact(dir, 'proxy') if (artifact) out.push(artifact) From 8bd7e32dccfaead093c4332c4f23ea2655ad097d Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 11:31:56 -0700 Subject: [PATCH 06/10] fix(debug-retention): key-log-only collection excludes a baseline captured at run start, not a timestamp (#1385, #1388 review a r2 + b) A first-prune marker excluded runs made during the boot delay forever, and any timestamp comparison admits a pre-upgrade run after a clock step back. The baseline is the set of key-log-only dirs that existed when this build first started, captured strictly and written once; with none, no key-log-only dir is collected. Co-Authored-By: Claude Opus 5.5 --- ...9-27-retention-collects-keylog-run-dirs.md | 16 ++- .../storage/debugRetention.keylog.test.ts | 101 ++++++++++------ src/main/storage/debugRetention.ts | 110 +++++++++++------- 3 files changed, 148 insertions(+), 79 deletions(-) diff --git a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md index 3b2b1a3df..2264185f5 100644 --- a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md +++ b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md @@ -6,8 +6,12 @@ ## Fix (narrowed to FUTURE runs; B6's oldest-first list) - A run dir is recognised by either evidence file: `proxy-events.jsonl` or `sslkeylog.log`. It matches the key log itself (review c). - A dir with `proxy-events.jsonl` is collected as before. -- A key-log-only dir is collected only when its run started AFTER this machine's first retention pass with this code: `keyLogRetentionSince()` writes that moment once (exclusive create) to `STATE_DIR/debug-retention-keylog-since`, and later passes read it. Run dirs are named by their ISO start time, which sorts as text, and the marker uses the same shape. If the marker cannot be read or written, or has an unknown shape, NO key-log-only dir is collected (fail closed). -- **WHY not a date constant (#1388 review a):** the first version used tomorrow's date, so a run this build made today was excluded forever, and any earlier date would sweep key logs from before the upgrade. +- A key-log-only dir is collected only when it is NOT in the BASELINE: the set of key-log-only dirs that existed when a build containing this code first started (`keyLogBaseline()`, captured at run start in `holdDebugStoragePruneUntilRecovered`, written once with an exclusive create to `STATE_DIR/debug-retention-keylog-baseline.json`). Capture is strict: any directory it cannot list means no baseline, nothing is written, and a later start retries. With no baseline, NO key-log-only dir is collected. +- **WHY a captured set (#1388 review a, two rounds):** + - a date constant excluded runs made on the merge day forever; + - a first-prune marker was written minutes after start (the boot gate delays the first prune), so runs made in between were excluded forever; + - any timestamp comparison admits a pre-upgrade run whose name sorts later after a clock step back. + Membership in "what already existed" needs no clock. - Every earlier key-log-only dir, including the owner's 23, is left untouched and never walked into. So is one whose name cannot be dated. - `session-meta.json` alone stays uncollected, and `_shared-conf` is still skipped. @@ -31,3 +35,11 @@ - **Mutations killed:** no cutoff; null collecting everything; any marker shape accepted. Removing the up-front marker read ALONE survives, because the exclusive create then hits EEXIST and reads the stored marker. Removing both guards fails. - **Not changed (finding 1):** a run dir with an events file AND a key log is collected whole, key log included. That is main's existing behaviour for event-bearing runs, which this PR does not touch. The owner decision (q91) is about the key-log-only dirs, which stay untouched. +## Review a round 2 + b (fixed at the next head) +- The marker is replaced by the baseline set, captured at run start. +- **Tests:** a baseline dir named after a new run (a clock step back) stays excluded; a run made after capture is collected; capture over an unreadable subtree yields no baseline and writes nothing; a baseline that cannot be written is not established (review b); the file is reused and a malformed one fails closed. +- **Mutations killed:** membership ignored; null collecting everything; lenient capture; capture recording nothing; returning an unsaved baseline. +- **Residuals:** + - The early-capture wiring in `holdDebugStoragePruneUntilRecovered` is not separately pinned; the boot-gate suite exercises it against a scratch state dir. + - `dirStats`' EIO/ELOOP branches (review b) are not reproducible on a real filesystem: symlink entries are skipped, and EIO cannot be produced on demand. EACCES is pinned. + diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts index 14efe7122..1545be93c 100644 --- a/src/main/storage/debugRetention.keylog.test.ts +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os' import { join, relative } from 'node:path' import { afterEach, expect, it } from 'vitest' -import { collectProxyRunDirs, keyLogRetentionSince, runPrunePasses } from './debugRetention.js' +import { collectProxyRunDirs, keyLogBaseline, runPrunePasses } from './debugRetention.js' import type { DebugStorageBucket, DebugStoragePrunePolicy } from './debugRetention.js' // #1385 (q91 follow-up of #1380): retention collected a proxy run dir only @@ -29,40 +29,40 @@ function runDir(root: string, parts: string[], files: Record): s } // Narrowed to FUTURE runs (B6 oldest-first list, owner decision q91 kept): -// a key-log-only dir is collected only when it started after this machine's -// first retention pass with this code (the marker, below). The existing ones -// (the owner's 23, May-September 2026, shaped like medlo/shell-89d43b9b) are -// left untouched, and so is one whose name cannot be dated. #1388 review a: -// a run made the SAME DAY, after the marker, is collected (a date constant -// excluded it forever). -it('collects a NEW key-log-only run dir, never an existing one, alongside normal run dirs', async () => { +// a key-log-only dir is collected only when it is NOT in the baseline, the +// set of key-log-only dirs that existed when this build first started. The +// existing ones (the owner's 23, May-September 2026, shaped like +// medlo/shell-89d43b9b) are in it and stay untouched. #1388 review a: names +// and clocks prove nothing, so a baseline dir whose name sorts AFTER a new +// run (a clock rolled back) is still excluded, and a run made minutes after +// start, before the first prune, is still new. +it('collects a key-log-only run dir only when it is not in the baseline, alongside normal run dirs', async () => { const root = mkdtempSync(join(tmpdir(), 'proxy-retention-')) roots.push(root) - runDir(root, ['medlo', 'shell-89d43b9b', '2026-08-28T17-30-06-452Z'], { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) - runDir(root, ['medlo', 'shell-7a1b2c3d', '2026-09-29T08-15-00-000Z'], { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) - runDir(root, ['medlo', 'shell-undated', 'run-without-a-timestamp'], { 'sslkeylog.log': 'x'.repeat(128) }) - runDir(root, ['medlo', 'shell-same-day-before', '2026-09-27T17-59-59-999Z'], { 'sslkeylog.log': 'x'.repeat(128) }) - runDir(root, ['medlo', 'shell-same-day-after', '2026-09-27T19-00-00-000Z'], { 'sslkeylog.log': 'x'.repeat(128) }) + const existing = join('medlo', 'shell-89d43b9b', '2026-08-28T17-30-06-452Z') + const rolledBack = join('medlo', 'shell-clock', '2026-09-27T18-02-00-000Z') + runDir(root, existing.split('/'), { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) + runDir(root, rolledBack.split('/'), { 'sslkeylog.log': 'x'.repeat(128) }) + runDir(root, ['medlo', 'shell-new', '2026-09-27T18-00-30-000Z'], { 'session-meta.json': '{}', 'sslkeylog.log': 'x'.repeat(4096) }) runDir(root, ['agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'], { 'session-meta.json': '{}', 'proxy-events.jsonl': '{}\n', 'sslkeylog.log': 'x'.repeat(1024) }) // Shared mitmproxy state is never a run dir, whatever it holds. runDir(root, ['_shared-conf'], { 'mitmproxy-ca-cert.pem': 'ca' }) // Metadata alone is not a run's evidence; leave it for its own pass. runDir(root, ['agent-code', 'shell-empty', '2026-09-01T00-00-00-000Z'], { 'session-meta.json': '{}' }) - const since = '2026-09-27T18-00-00-000Z' - const artifacts = await collectProxyRunDirs(root, since) + const baseline = new Set([existing, rolledBack]) + const artifacts = await collectProxyRunDirs(root, baseline) expect(artifacts.map(artifact => relative(root, artifact.path)).sort()).toEqual([ join('agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'), - join('medlo', 'shell-7a1b2c3d', '2026-09-29T08-15-00-000Z'), - join('medlo', 'shell-same-day-after', '2026-09-27T19-00-00-000Z'), + join('medlo', 'shell-new', '2026-09-27T18-00-30-000Z'), ]) - // With no established cutoff, no key-log-only dir is collected at all. + const keyLogOnly = artifacts.find(artifact => artifact.path.includes('shell-new'))! + expect(keyLogOnly).toMatchObject({ kind: 'dir', bucket: 'proxy' }) + expect(keyLogOnly.bytes).toBeGreaterThanOrEqual(4096) + // With no established baseline, no key-log-only dir is collected at all. expect((await collectProxyRunDirs(root, null)).map(artifact => relative(root, artifact.path))).toEqual([ join('agent-code', 'resume-a5fb379b', '2026-09-27T01-03-52-273Z'), ]) - const keyLogOnly = artifacts.find(artifact => artifact.path.includes('shell-7a1b2c3d'))! - expect(keyLogOnly).toMatchObject({ kind: 'dir', bucket: 'proxy' }) - expect(keyLogOnly.bytes).toBeGreaterThanOrEqual(4096) }) // Worker rule "unknown is never empty" (q109, q115). dirStats swallowed EVERY @@ -89,7 +89,7 @@ it('a run dir with a child it cannot read is protected, and is collected normall const caps = {} as Record for (const bucket of ['proxy'] as DebugStorageBucket[]) caps[bucket] = 1_000_000_000 const policy: DebugStoragePrunePolicy = { now, ttlMs: 48 * 3_600_000, activeGraceMs: 10 * 60_000, budgetBytes: 1_000_000_000, caps } - const prune = async () => runPrunePasses(await collectProxyRunDirs(root, '2026-09-28T00-00-00-000Z'), policy, async artifact => { + const prune = async () => runPrunePasses(await collectProxyRunDirs(root, new Set()), policy, async artifact => { try { await rm(artifact.path, { recursive: true, force: true }); return true } catch { return false } }) @@ -112,21 +112,46 @@ it('a run dir with a child it cannot read is protected, and is collected normall expect(existsSync(dir)).toBe(false) }) -// The marker behind "future runs only" (#1388 review a). The first pass -// records the moment this machine started collecting and later passes keep -// it. An unreadable marker, or one in an unknown shape, yields null, which -// collects NO key-log-only run (an unknown cutoff never widens collection). -it('records the key-log cutoff once, keeps it, and fails closed when it cannot be read', async () => { - const dir = mkdtempSync(join(tmpdir(), 'keylog-since-')) +// The baseline behind "future runs only" (#1388 review a, round 2). It is +// captured once, the first time this build starts, and reused. Capture is +// strict: a subtree it cannot read would leave its old key logs out of the +// baseline, so any unreadable directory means NO baseline (nothing +// key-log-only is collected) and nothing is written, so a later start retries. +it('captures the key-log baseline once, reuses it, and fails closed when capture or the file cannot be read', async () => { + const dir = mkdtempSync(join(tmpdir(), 'keylog-baseline-')) roots.push(dir) - const file = join(dir, 'state', 'debug-retention-keylog-since') - const first = await keyLogRetentionSince(file, () => new Date('2026-09-27T18:20:27.123Z')) - expect(first).toBe('2026-09-27T18-20-27-123Z') - expect(await keyLogRetentionSince(file, () => new Date('2026-10-01T00:00:00.000Z'))).toBe(first) - writeFileSync(file, 'not a timestamp') - expect(await keyLogRetentionSince(file)).toBeNull() - rmSync(file) - mkdirSync(file) - expect(await keyLogRetentionSince(file)).toBeNull() -}) + const root = join(dir, 'proxy') + const file = join(dir, 'state', 'debug-retention-keylog-baseline.json') + runDir(root, ['medlo', 'shell-old', '2026-08-28T17-30-06-452Z'], { 'sslkeylog.log': 'k' }) + runDir(root, ['agent-code', 'run-with-events', '2026-09-01T00-00-00-000Z'], { 'proxy-events.jsonl': '{}', 'sslkeylog.log': 'k' }) + const locked = join(root, 'locked-project') + mkdirSync(locked) + chmodSync(locked, 0o000) + try { + expect(await keyLogBaseline(file, root)).toBeNull() + expect(existsSync(file)).toBe(false) + } finally { + chmodSync(locked, 0o700) + } + + const first = await keyLogBaseline(file, root) + expect(first && [...first]).toEqual([join('medlo', 'shell-old', '2026-08-28T17-30-06-452Z')]) + runDir(root, ['medlo', 'shell-later', '2026-09-27T19-00-00-000Z'], { 'sslkeylog.log': 'k' }) + expect([...(await keyLogBaseline(file, root))!]).toEqual([join('medlo', 'shell-old', '2026-08-28T17-30-06-452Z')]) + + writeFileSync(file, '{not json') + expect(await keyLogBaseline(file, root)).toBeNull() + + // A baseline that cannot be WRITTEN is not established either (#1388 + // review b): returning the unsaved set would let the next start capture a + // different one, including key logs made in between. + const readOnlyState = join(dir, 'read-only-state') + mkdirSync(readOnlyState) + chmodSync(readOnlyState, 0o500) + try { + expect(await keyLogBaseline(join(readOnlyState, 'baseline.json'), root)).toBeNull() + } finally { + chmodSync(readOnlyState, 0o700) + } +}) diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index dab096e21..742da75c8 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -1,5 +1,5 @@ import { mkdir, readFile, readdir, rm, stat, statfs, writeFile } from 'node:fs/promises' -import { basename, dirname, join, resolve } from 'node:path' +import { dirname, join, relative, resolve } from 'node:path' import { AUTOSAVE_DEBUG_BUNDLE_DIR, @@ -200,6 +200,10 @@ function unrefTimer(timer: ReturnType): void { export function holdDebugStoragePruneUntilRecovered(): void { if (bootGateUsed) return bootGateUsed = true + // The key-log baseline is captured NOW, at the start of the run, before any + // session of this build can write a run dir; the first prune (minutes later, + // behind this gate) awaits it. See keyLogBaseline (#1388 review a). + keyLogBaselineTask ??= keyLogBaseline() const fallback = setTimeout(openDebugStoragePruneGate, DEBUG_PRUNE_BOOT_FALLBACK_MS) unrefTimer(fallback) bootGate = { pendingReason: null, fallback, opening: null } @@ -456,13 +460,15 @@ function bucketCaps(totalBudget: number): Record { async function collectArtifacts(): Promise { const manualLegacyBundlePaths = await loadManualLegacyBundlePaths() - const keyLogOnlySince = await keyLogRetentionSince() + // Captured at run start (holdDebugStoragePruneUntilRecovered); a run that + // never held the gate (tests, odd call orders) captures here instead. + const keyLogOnlyBaseline = await (keyLogBaselineTask ??= keyLogBaseline()) const [feed, manualBundles, autosaveBundles, legacyBundles, proxy, performance, incidents, heapSnapshots, sessionRecordings] = await Promise.all([ collectFiles(FEED_DEBUG_DIR, 'feed-debug', name => name.endsWith('.jsonl')), collectImmediateDirs(MANUAL_DEBUG_BUNDLE_DIR, 'debug-bundles-manual'), collectImmediateDirs(AUTOSAVE_DEBUG_BUNDLE_DIR, 'debug-bundles-autosave'), collectLegacyDebugBundleDirs(DEBUG_BUNDLE_DIR, manualLegacyBundlePaths), - collectProxyRunDirs(PROXY_EVENTS_DIR, keyLogOnlySince), + collectProxyRunDirs(PROXY_EVENTS_DIR, keyLogOnlyBaseline), collectImmediateDirs(PERFORMANCE_RUNS_DIR, 'performance'), collectIncidentRunDirs(), collectFiles(HEAP_SNAPSHOT_DIR, 'heap-snapshots', name => name.endsWith('.heapsnapshot')), @@ -669,56 +675,83 @@ const PROXY_RUN_MARKERS = new Set(['proxy-events.jsonl', 'proxy-events.1.jsonl'] /** * Key-log-only run dirs are collected FORWARD ONLY (#1385, owner-approved - * narrowing in B6's oldest-first list): only runs that started after the - * first retention pass of a build containing this code. Every earlier one, - * including the 23 dirs found on the owner's machine (May-September 2026), is - * left exactly as it is: deleting existing TLS key logs is an owner decision - * (q91) this PR does not make. + * narrowing in B6's oldest-first list): every key-log-only dir that existed + * when a build containing this code first started is in the BASELINE and is + * left exactly as it is (the 23 on the owner's machine among them): deleting + * existing TLS key logs is an owner decision (q91) this PR does not make. + * Any key-log-only dir not in the baseline was made afterwards, and is + * collected like any other run. * - * WHY a marker written on first run and not a date constant (#1388 review a): - * a fixed cutoff can only approximate "after the upgrade". The first version - * used tomorrow's date, so a run this build made TODAY was excluded forever - * (its name never crosses the cutoff), and an earlier date would have swept - * key logs older than the upgrade. The marker records the actual moment this - * machine started collecting. - * - * Run dirs are named by their ISO start time with `:` and `.` replaced - * (`2026-08-28T17-30-06-452Z`), which sorts as text, and the marker uses the - * same shape. A name that is not in that shape cannot be dated, so it is not - * collected (unknown is never "new"). + * WHY a captured set and not a timestamp (#1388 review a, two rounds): a date + * constant excluded runs made on the merge day forever; a first-prune marker + * was written minutes after start (the boot gate delays the first prune), so + * runs made in between were excluded forever; and any timestamp comparison + * admits a pre-upgrade run whose name sorts later after a clock step back. + * Membership in the set of what already existed is the exact definition and + * needs no clock. */ -const RUN_DIR_NAME = /^\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z$/ -const KEY_LOG_SINCE_FILE = join(STATE_DIR, 'debug-retention-keylog-since') +const KEY_LOG_BASELINE_FILE = join(STATE_DIR, 'debug-retention-keylog-baseline.json') +let keyLogBaselineTask: Promise | null> | null = null /** - * When this machine started collecting key-log-only runs, in run-dir name - * form, or null when that cannot be established. Null collects NO key-log-only - * run (fail closed: an unknown cutoff must never widen collection to old key - * logs). The first call writes the marker exclusively, so two passes racing - * agree on one value. + * The key-log-only run dirs (paths relative to `root`) that existed when + * this build first started, or null when that cannot be established. Null + * collects NO key-log-only run: an unknown baseline must never widen + * collection to old key logs. + * + * Captured once and written with an exclusive create; later calls read it. + * Capture is STRICT: a directory it cannot list (anything but ENOENT) could + * hide old key logs from the set, so capture fails, nothing is written, and a + * later start retries. */ -export async function keyLogRetentionSince(file = KEY_LOG_SINCE_FILE, now: () => Date = () => new Date()): Promise { - const readMarker = async (): Promise => { - const value = (await readFile(file, 'utf8')).trim() - return RUN_DIR_NAME.test(value) ? value : null +export async function keyLogBaseline(file = KEY_LOG_BASELINE_FILE, root = PROXY_EVENTS_DIR): Promise | null> { + const read = async (): Promise | null> => { + try { + const parsed = JSON.parse(await readFile(file, 'utf8')) as unknown + return Array.isArray(parsed) && parsed.every(entry => typeof entry === 'string') ? new Set(parsed as string[]) : null + } catch { + return null + } } try { - return await readMarker() + await stat(file) + return await read() } catch (error) { if ((error as NodeJS.ErrnoException).code !== 'ENOENT') return null } - const since = now().toISOString().replace(/[:.]/g, '-') + const existing: string[] = [] + async function walk(dir: string, depth: number): Promise { + let entries + try { + entries = await readdir(dir, { withFileTypes: true }) + } catch (error) { + if (dir === root && (error as NodeJS.ErrnoException).code === 'ENOENT') return + throw error + } + const files = new Set(entries.filter(entry => entry.isFile()).map(entry => entry.name)) + const hasEvents = [...PROXY_RUN_MARKERS].some(marker => files.has(marker)) + if (hasEvents || files.has('sslkeylog.log')) { + if (!hasEvents) existing.push(relative(root, dir)) + return + } + if (depth >= 4) return + for (const entry of entries) { + if (!entry.isDirectory() || entry.name === '_shared-conf') continue + await walk(join(dir, entry.name), depth + 1) + } + } try { + await walk(root, 0) await mkdir(dirname(file), { recursive: true }) - await writeFile(file, since, { flag: 'wx', mode: 0o600 }) - return since + await writeFile(file, JSON.stringify(existing.sort()), { flag: 'wx', mode: 0o600 }) + return new Set(existing) } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'EEXIST') return readMarker().catch(() => null) + if ((error as NodeJS.ErrnoException).code === 'EEXIST') return read() return null } } -export async function collectProxyRunDirs(root: string, keyLogOnlySince: string | null = null): Promise { +export async function collectProxyRunDirs(root: string, keyLogOnlyBaseline: ReadonlySet | null = null): Promise { const out: Artifact[] = [] async function walk(dir: string, depth: number): Promise { let entries @@ -733,15 +766,14 @@ export async function collectProxyRunDirs(root: string, keyLogOnlySince: string // budgeted, never removed. Those are plaintext TLS session secrets; the // owner's machine had 23 such dirs (5.18 MB, May-September 2026, #1380 // review c). A key-log-only dir is a run dir either way (never walked - // into), but it is COLLECTED only when it started after keyLogOnlySince. + // into), but it is COLLECTED only when it is not in the baseline. // session-meta.json alone is NOT evidence of a run. const files = new Set(entries.filter(entry => entry.isFile()).map(entry => entry.name)) // Events markers (the live file or its rotated `.1` generation, #1376) // are collected as before; a key-log-only dir only when it is new. const hasEvents = [...PROXY_RUN_MARKERS].some(marker => files.has(marker)) if (hasEvents || files.has('sslkeylog.log')) { - const name = basename(dir) - const collectable = hasEvents || (keyLogOnlySince !== null && RUN_DIR_NAME.test(name) && name > keyLogOnlySince) + const collectable = hasEvents || (keyLogOnlyBaseline !== null && !keyLogOnlyBaseline.has(relative(root, dir))) if (collectable) { const artifact = await collectDirArtifact(dir, 'proxy') if (artifact) out.push(artifact) From d3804ecabb3bdc49f3b1a6df2e509f3a9f0cafa3 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 11:38:53 -0700 Subject: [PATCH 07/10] fix(debug-retention): no baseline from a missing proxy root; runs born during capture stay out of it (#1385, #1388 review a round 3) Co-Authored-By: Claude Opus 5.5 --- ...9-27-retention-collects-keylog-run-dirs.md | 6 ++++ .../storage/debugRetention.keylog.test.ts | 33 +++++++++++++++++++ src/main/storage/debugRetention.ts | 17 ++++++++-- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md index 2264185f5..e830d8f0e 100644 --- a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md +++ b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md @@ -43,3 +43,9 @@ - The early-capture wiring in `holdDebugStoragePruneUntilRecovered` is not separately pinned; the boot-gate suite exercises it against a scratch state dir. - `dirStats`' EIO/ELOOP branches (review b) are not reproducible on a real filesystem: symlink entries are skipped, and EIO cannot be produced on demand. EACCES is pinned. + +## Review a round 3 (last pass) +- **Fixed, unsafe direction:** a proxy root missing at capture saved an empty baseline, so old key logs that reappeared became collectable. Capture now has no ENOENT exception, even for the root: no baseline, nothing written, retried at a later start. +- **Fixed, conservative direction:** a run created WHILE the asynchronous scan ran was baselined forever. Only dirs whose filesystem birthtime is at or before the moment capture started join the set. A missing birthtime (0) keeps the dir (conservative); the race test is macOS-only, where birthtimes exist. +- **Residual, conservative direction:** after a failed capture (for example an unwritable state dir), runs made before a later successful capture are baselined and never collected. That is a retention gap, never a deletion. The same holds on a fresh install that has no proxy folder yet: the first capture happens at the start after the folder appears. +- **Mutations killed:** the root ENOENT exception, and birthtime ignored. diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts index 1545be93c..93f1179bb 100644 --- a/src/main/storage/debugRetention.keylog.test.ts +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -155,3 +155,36 @@ it('captures the key-log baseline once, reuses it, and fails closed when capture chmodSync(readOnlyState, 0o700) } }) + +// #1388 review a round 3 (1): a proxy root that is missing at capture is +// UNKNOWN, not empty. Saving [] would let every old key log that reappears +// be collected, so no baseline is saved and none is returned. +it('saves no baseline when the proxy root is missing at capture', async () => { + const dir = mkdtempSync(join(tmpdir(), 'keylog-baseline-')) + roots.push(dir) + const file = join(dir, 'state', 'baseline.json') + expect(await keyLogBaseline(file, join(dir, 'proxy-renamed-away'))).toBeNull() + expect(existsSync(file)).toBe(false) +}) + +// #1388 review a round 3 (2): capture is asynchronous, so a run created +// while it scans must not become a permanent baseline member. Only dirs that +// existed when capture STARTED (by filesystem birthtime) join it. +it.skipIf(process.platform !== 'darwin')('keeps a run created during capture out of the baseline', async () => { + const dir = mkdtempSync(join(tmpdir(), 'keylog-baseline-')) + roots.push(dir) + const root = join(dir, 'proxy') + const file = join(dir, 'state', 'baseline.json') + runDir(root, ['p', 's', 'old-run'], { 'sslkeylog.log': 'k' }) + await new Promise(resolve => setTimeout(resolve, 20)) + const capturing = keyLogBaseline(file, root) + // Synchronously, before the capture's first await resumes: the run exists + // when the scan reaches it. A short spin puts its birthtime clearly after + // the moment capture started. + const spinUntil = Date.now() + 3 + while (Date.now() < spinUntil) { /* spin */ } + runDir(root, ['p', 's', 'new-run'], { 'sslkeylog.log': 'k' }) + const baseline = await capturing + expect(baseline && [...baseline]).toEqual([join('p', 's', 'old-run')]) +}) + diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index 742da75c8..43bf5afc0 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -705,6 +705,13 @@ let keyLogBaselineTask: Promise | null> | null = null * later start retries. */ export async function keyLogBaseline(file = KEY_LOG_BASELINE_FILE, root = PROXY_EVENTS_DIR): Promise | null> { + // Taken synchronously, before the first await (#1388 review a round 3): a + // run a session creates WHILE the scan runs must not become a permanent + // baseline member, so only dirs born before this moment join the set. A + // filesystem without birthtimes (0) keeps every dir: the conservative side + // (a new run kept, never an old one exposed). Same millisecond counts as + // existing, for the same reason. + const captureStartedAt = Date.now() const read = async (): Promise | null> => { try { const parsed = JSON.parse(await readFile(file, 'utf8')) as unknown @@ -725,13 +732,19 @@ export async function keyLogBaseline(file = KEY_LOG_BASELINE_FILE, root = PROXY_ try { entries = await readdir(dir, { withFileTypes: true }) } catch (error) { - if (dir === root && (error as NodeJS.ErrnoException).code === 'ENOENT') return + // No exception for a missing ROOT either (#1388 review a round 3): a + // proxy folder renamed away or not mounted at capture would save an + // empty baseline, and every old key log that reappeared would then be + // collected. Unknown, so no baseline; a later start retries. throw error } const files = new Set(entries.filter(entry => entry.isFile()).map(entry => entry.name)) const hasEvents = [...PROXY_RUN_MARKERS].some(marker => files.has(marker)) if (hasEvents || files.has('sslkeylog.log')) { - if (!hasEvents) existing.push(relative(root, dir)) + if (!hasEvents) { + const born = (await stat(dir)).birthtimeMs + if (!born || Math.floor(born) <= captureStartedAt) existing.push(relative(root, dir)) + } return } if (depth >= 4) return From 94204b022cb1eba91173ad395622463e12ecbe8b Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 11:45:05 -0700 Subject: [PATCH 08/10] docs(debug-retention): baseline membership, not names, decides; the q91 decision is tracked in #1460 (#1388 review c) Co-Authored-By: Claude Opus 5.5 --- docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md index e830d8f0e..b912e7222 100644 --- a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md +++ b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md @@ -12,7 +12,7 @@ - a first-prune marker was written minutes after start (the boot gate delays the first prune), so runs made in between were excluded forever; - any timestamp comparison admits a pre-upgrade run whose name sorts later after a clock step back. Membership in "what already existed" needs no clock. -- Every earlier key-log-only dir, including the owner's 23, is left untouched and never walked into. So is one whose name cannot be dated. +- Every key-log-only dir in the baseline, including the owner's 23, is left untouched and never walked into, whatever its name. Names play no part: a NEW key-log-only dir is collected even if its name is not a timestamp. The decision on the existing ones is tracked in #1460 (q91). - `session-meta.json` alone stays uncollected, and `_shared-conf` is still skipped. ## Owner decision kept (q91) @@ -25,7 +25,7 @@ ## Tests `debugRetention.keylog.test.ts`, on the real directory shapes (`proxy////`): - a NEW key-log-only dir is collected beside a normal run dir; -- an existing-dated one, an undated one, `_shared-conf` and a metadata-only dir are not; +- baseline members (including one whose name sorts after a new run), `_shared-conf` and a metadata-only dir are not; - the unreadable-child test: fail once, recover, maintain, and the bytes survive. - Mutations killed: removing the cutoff, and removing the name check. From 68baa3e5ac8ae79f7c272b2775e068a76e0e1891 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 11:56:38 -0700 Subject: [PATCH 09/10] fix(debug-retention): drop the birthtime filter; a clock step back could exclude an old key log from the baseline (#1388 review b round 3) Co-Authored-By: Claude Opus 5.5 --- ...9-27-retention-collects-keylog-run-dirs.md | 4 ++-- .../storage/debugRetention.keylog.test.ts | 22 ------------------- src/main/storage/debugRetention.ts | 19 +++++++--------- 3 files changed, 10 insertions(+), 35 deletions(-) diff --git a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md index b912e7222..ca5ccf319 100644 --- a/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md +++ b/docs/plans/2026-09-27-retention-collects-keylog-run-dirs.md @@ -46,6 +46,6 @@ ## Review a round 3 (last pass) - **Fixed, unsafe direction:** a proxy root missing at capture saved an empty baseline, so old key logs that reappeared became collectable. Capture now has no ENOENT exception, even for the root: no baseline, nothing written, retried at a later start. -- **Fixed, conservative direction:** a run created WHILE the asynchronous scan ran was baselined forever. Only dirs whose filesystem birthtime is at or before the moment capture started join the set. A missing birthtime (0) keeps the dir (conservative); the race test is macOS-only, where birthtimes exist. +- **Not fixed, conservative direction (decided with review b round 3):** a run created WHILE the startup scan runs is baselined and kept forever. A birthtime filter was tried and REVERTED: after a clock step back, a pre-existing dir's birthtime can look later than the capture start, which would exclude an old key log from the baseline (the unsafe direction). Capture starts at run start, before any session exists, so the window is the few milliseconds of the scan. - **Residual, conservative direction:** after a failed capture (for example an unwritable state dir), runs made before a later successful capture are baselined and never collected. That is a retention gap, never a deletion. The same holds on a fresh install that has no proxy folder yet: the first capture happens at the start after the folder appears. -- **Mutations killed:** the root ENOENT exception, and birthtime ignored. +- **Mutation killed:** the root ENOENT exception. diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts index 93f1179bb..dd85f3285 100644 --- a/src/main/storage/debugRetention.keylog.test.ts +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -166,25 +166,3 @@ it('saves no baseline when the proxy root is missing at capture', async () => { expect(await keyLogBaseline(file, join(dir, 'proxy-renamed-away'))).toBeNull() expect(existsSync(file)).toBe(false) }) - -// #1388 review a round 3 (2): capture is asynchronous, so a run created -// while it scans must not become a permanent baseline member. Only dirs that -// existed when capture STARTED (by filesystem birthtime) join it. -it.skipIf(process.platform !== 'darwin')('keeps a run created during capture out of the baseline', async () => { - const dir = mkdtempSync(join(tmpdir(), 'keylog-baseline-')) - roots.push(dir) - const root = join(dir, 'proxy') - const file = join(dir, 'state', 'baseline.json') - runDir(root, ['p', 's', 'old-run'], { 'sslkeylog.log': 'k' }) - await new Promise(resolve => setTimeout(resolve, 20)) - const capturing = keyLogBaseline(file, root) - // Synchronously, before the capture's first await resumes: the run exists - // when the scan reaches it. A short spin puts its birthtime clearly after - // the moment capture started. - const spinUntil = Date.now() + 3 - while (Date.now() < spinUntil) { /* spin */ } - runDir(root, ['p', 's', 'new-run'], { 'sslkeylog.log': 'k' }) - const baseline = await capturing - expect(baseline && [...baseline]).toEqual([join('p', 's', 'old-run')]) -}) - diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index 43bf5afc0..387904a02 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -705,13 +705,13 @@ let keyLogBaselineTask: Promise | null> | null = null * later start retries. */ export async function keyLogBaseline(file = KEY_LOG_BASELINE_FILE, root = PROXY_EVENTS_DIR): Promise | null> { - // Taken synchronously, before the first await (#1388 review a round 3): a - // run a session creates WHILE the scan runs must not become a permanent - // baseline member, so only dirs born before this moment join the set. A - // filesystem without birthtimes (0) keeps every dir: the conservative side - // (a new run kept, never an old one exposed). Same millisecond counts as - // existing, for the same reason. - const captureStartedAt = Date.now() + // NO birthtime filter (#1388 review b round 3): excluding dirs "born after + // capture started" re-introduced a clock comparison in the UNSAFE direction. + // After a clock step back, a pre-existing dir's birthtime can look later than + // the capture start, so it would be left out of the baseline and collected. + // The price is conservative: a run a session creates during the few + // milliseconds of the startup scan is baselined and kept forever (never + // deleted). Capture starts at run start, before any session exists. const read = async (): Promise | null> => { try { const parsed = JSON.parse(await readFile(file, 'utf8')) as unknown @@ -741,10 +741,7 @@ export async function keyLogBaseline(file = KEY_LOG_BASELINE_FILE, root = PROXY_ const files = new Set(entries.filter(entry => entry.isFile()).map(entry => entry.name)) const hasEvents = [...PROXY_RUN_MARKERS].some(marker => files.has(marker)) if (hasEvents || files.has('sslkeylog.log')) { - if (!hasEvents) { - const born = (await stat(dir)).birthtimeMs - if (!born || Math.floor(born) <= captureStartedAt) existing.push(relative(root, dir)) - } + if (!hasEvents) existing.push(relative(root, dir)) return } if (depth >= 4) return From 35001c8a47a53f37e142413ce4993360a19e21ba Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 12:56:22 -0700 Subject: [PATCH 10/10] test(debug-retention): pin the birthtime-filter revert and the baseline shape check (#1388, B6 check) Co-Authored-By: Claude Opus 5.5 --- .../storage/debugRetention.keylog.test.ts | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/main/storage/debugRetention.keylog.test.ts b/src/main/storage/debugRetention.keylog.test.ts index dd85f3285..e4db892c2 100644 --- a/src/main/storage/debugRetention.keylog.test.ts +++ b/src/main/storage/debugRetention.keylog.test.ts @@ -2,7 +2,7 @@ import { chmodSync, existsSync, mkdirSync, mkdtempSync, rmSync, utimesSync, writ import { rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join, relative } from 'node:path' -import { afterEach, expect, it } from 'vitest' +import { afterEach, expect, it, vi } from 'vitest' import { collectProxyRunDirs, keyLogBaseline, runPrunePasses } from './debugRetention.js' import type { DebugStorageBucket, DebugStoragePrunePolicy } from './debugRetention.js' @@ -166,3 +166,34 @@ it('saves no baseline when the proxy root is missing at capture', async () => { expect(await keyLogBaseline(file, join(dir, 'proxy-renamed-away'))).toBeNull() expect(existsSync(file)).toBe(false) }) + +// B6 check at 68baa3e5: the birthtime filter was reverted (a clock step back +// could leave an old key log out of the baseline), but nothing pinned the +// revert. Capture on a clock stepped back to 2020: every existing key log must +// still be in the baseline, so nothing key-log-only is collected. +it('baselines every existing key log even when the clock is behind their birthtimes', async () => { + const dir = mkdtempSync(join(tmpdir(), 'keylog-baseline-')) + roots.push(dir) + const root = join(dir, 'proxy') + runDir(root, ['p', 's', 'old-run'], { 'sslkeylog.log': 'k' }) + const clock = vi.spyOn(Date, 'now').mockReturnValue(Date.parse('2020-01-01T00:00:00Z')) + let baseline: ReadonlySet | null + try { + baseline = await keyLogBaseline(join(dir, 'state', 'baseline.json'), root) + } finally { + clock.mockRestore() + } + expect(await collectProxyRunDirs(root, baseline)).toEqual([]) +}) + +// B6 check: a baseline file that parses but has the wrong shape is unknown. +it('treats a baseline file of the wrong shape as no baseline', async () => { + const dir = mkdtempSync(join(tmpdir(), 'keylog-baseline-')) + roots.push(dir) + const file = join(dir, 'baseline.json') + writeFileSync(file, JSON.stringify({ not: 'an array' })) + expect(await keyLogBaseline(file, join(dir, 'proxy'))).toBeNull() + writeFileSync(file, JSON.stringify(['ok', 42])) + expect(await keyLogBaseline(file, join(dir, 'proxy'))).toBeNull() +}) +