diff --git a/src/utils/issueReport.test.ts b/src/utils/issueReport.test.ts index c9793853..fd64dcb3 100644 --- a/src/utils/issueReport.test.ts +++ b/src/utils/issueReport.test.ts @@ -8,8 +8,9 @@ import { function makeRuntime(overrides: Partial = {}) { const writes: Array<{ path: string; data: string | Uint8Array }> = []; - const copies: Array<{ src: string; dest: string }> = []; const mkdirs: string[] = []; + // Every log read carries a home path and a secret so tests can assert the + // written bundle copy is redacted rather than verbatim. const runtime: ReportRuntime = { join: (...parts) => parts.join("/"), mkdir: async (p) => { @@ -18,12 +19,14 @@ function makeRuntime(overrides: Partial = {}) { writeFile: async (p, data) => { writes.push({ path: p, data }); }, - copyFile: async (src, dest) => { - copies.push({ src, dest }); - }, + readFile: async (p) => `log from ${p} for /Users/alice/vault key sk-abcdef0123456789`, ...overrides, }; - return { runtime, writes, copies, mkdirs }; + return { runtime, writes, mkdirs }; +} + +function writtenText(writes: Array<{ path: string; data: string | Uint8Array }>, suffix: string) { + return String(writes.find((w) => w.path.endsWith(suffix))?.data ?? ""); } const baseInput: ReportInput = { @@ -43,7 +46,7 @@ const baseInput: ReportInput = { describe("assembleReportBundle", () => { it("creates a timestamped folder and writes all files when everything is present", async () => { - const { runtime, writes, copies, mkdirs } = makeRuntime(); + const { runtime, writes, mkdirs } = makeRuntime(); const result = await assembleReportBundle(baseInput, runtime); expect(result.folderPath).toBe("/tmp/reports/report-20260615-101500"); @@ -60,16 +63,19 @@ describe("assembleReportBundle", () => { "/tmp/reports/report-20260615-101500/screenshot.png" ); expect(writes.map((w) => w.path)).toContain("/tmp/reports/report-20260615-101500/report.md"); - expect(copies).toEqual([ - { - src: "/tmp/acp-frames.ndjson", - dest: "/tmp/reports/report-20260615-101500/acp-frames.ndjson.txt", - }, - { - src: "/tmp/opencode/log/session.log", - dest: "/tmp/reports/report-20260615-101500/opencode.log", - }, - ]); + }); + + it("redacts the frame and opencode logs it writes rather than copying them verbatim", async () => { + const { runtime, writes } = makeRuntime(); + await assembleReportBundle(baseInput, runtime); + + for (const name of ["acp-frames.ndjson.txt", "opencode.log"]) { + const text = writtenText(writes, name); + expect(text).toContain("/Users//vault"); + expect(text).toContain(""); + expect(text).not.toContain("/Users/alice"); + expect(text).not.toContain("sk-abcdef0123456789"); + } }); it("skips the screenshot when none was captured", async () => { @@ -83,19 +89,20 @@ describe("assembleReportBundle", () => { }); it("omits the opencode log when not provided", async () => { - const { runtime, copies } = makeRuntime(); + const { runtime, writes } = makeRuntime(); const result = await assembleReportBundle({ ...baseInput, opencodeLogPath: null }, runtime); expect(result.files).not.toContain("opencode.log"); - expect(copies.map((c) => c.dest)).not.toContain( + expect(writes.map((w) => w.path)).not.toContain( "/tmp/reports/report-20260615-101500/opencode.log" ); }); - it("skips a frame log that fails to copy instead of failing the whole report", async () => { + it("skips a frame log that fails to read instead of failing the whole report", async () => { const { runtime } = makeRuntime({ - copyFile: async (src) => { - if (src.includes("acp-frames")) throw new Error("ENOENT"); + readFile: async (p) => { + if (p.includes("acp-frames")) throw new Error("ENOENT"); + return "log for /Users/alice"; }, }); const result = await assembleReportBundle(baseInput, runtime); diff --git a/src/utils/issueReport.ts b/src/utils/issueReport.ts index 11f24ad5..653cc0a2 100644 --- a/src/utils/issueReport.ts +++ b/src/utils/issueReport.ts @@ -7,8 +7,14 @@ * * Pure of singletons: the Node runtime is injectable so the assembler is * unit-testable without touching the real filesystem. + * + * The frame and opencode logs are redacted (see `redactLog`) before they are + * written into the bundle — they are never copied verbatim, because they carry + * absolute home paths and may carry tokens or emails. */ +import { redactLogText } from "./redactLog"; + /** * End-user reports go to the PUBLIC repo. The private `obsidian-copilot-preview` * repo is for internal triage/BRAT only and must never receive user issues @@ -59,7 +65,7 @@ export interface ReportRuntime { join: (...parts: string[]) => string; mkdir: (path: string, opts: { recursive: boolean }) => Promise; writeFile: (path: string, data: string | Uint8Array) => Promise; - copyFile: (src: string, dest: string) => Promise; + readFile: (path: string) => Promise; } /** @@ -86,21 +92,21 @@ export async function assembleReportBundle( } } - if (input.frameLogPath) { + // Logs are read, redacted, and written — never copied verbatim. They contain + // absolute home paths, and can contain tokens or emails, none of which may + // leave the machine in a bug report. + for (const log of [ + { path: input.frameLogPath, name: FRAME_LOG_NAME }, + { path: input.opencodeLogPath, name: OPENCODE_LOG_NAME }, + ]) { + if (!log.path) continue; try { - await runtime.copyFile(input.frameLogPath, runtime.join(folderPath, FRAME_LOG_NAME)); - files.push(FRAME_LOG_NAME); + const redacted = redactLogText(await runtime.readFile(log.path)); + await runtime.writeFile(runtime.join(folderPath, log.name), redacted); + files.push(log.name); } catch { - // Frame log may not exist yet (logging just enabled); skip it. - } - } - - if (input.opencodeLogPath) { - try { - await runtime.copyFile(input.opencodeLogPath, runtime.join(folderPath, OPENCODE_LOG_NAME)); - files.push(OPENCODE_LOG_NAME); - } catch { - // opencode log may be absent; skip it. + // A log may not exist yet (frame logging just enabled) or be unreadable; + // skip it rather than failing the whole report. } } @@ -191,6 +197,6 @@ function getNodeReportRuntime(): ReportRuntime { await fs.mkdir(p, opts); }, writeFile: (p, data) => fs.writeFile(p, data), - copyFile: (src, dest) => fs.copyFile(src, dest), + readFile: (p) => fs.readFile(p, "utf8"), }; } diff --git a/src/utils/redactLog.test.ts b/src/utils/redactLog.test.ts new file mode 100644 index 00000000..4190028a --- /dev/null +++ b/src/utils/redactLog.test.ts @@ -0,0 +1,50 @@ +import { redactLogText } from "@/utils/redactLog"; + +describe("redactLog", () => { + describe("redactLogText()", () => { + it("replaces a home-directory username in a Unix path but keeps the path shape", () => { + expect(redactLogText('{"file":"/Users/chaoyang/vault/note.md"}')).toBe( + '{"file":"/Users//vault/note.md"}' + ); + expect(redactLogText("/home/logan/.config/app")).toBe("/home//.config/app"); + }); + + it("replaces a Windows home-directory username", () => { + expect(redactLogText("C:\\Users\\Logan\\AppData")).toBe("C:\\Users\\\\AppData"); + }); + + it("replaces email addresses", () => { + expect(redactLogText("from logan@brevilabs.com to a@b.co")).toBe("from to "); + }); + + it("replaces provider API keys by their recognizable prefix", () => { + expect(redactLogText("key sk-ant-abcdef0123456789xyz")).toBe("key "); + expect(redactLogText("AIzaSyD1234567890abcdefghijk")).toBe(""); + expect(redactLogText("token ghp_0123456789abcdefghij0")).toBe("token "); + }); + + it("removes a bearer token, whether or not it follows an Authorization field", () => { + // Bare bearer: the scheme word stays, the token becomes a marker. + expect(redactLogText("using bearer abcdef0123456789 now")).toBe("using bearer now"); + // In an Authorization header both rules fire; either way the token is gone. + const header = redactLogText("Authorization: Bearer abcdef0123456789"); + expect(header).not.toContain("abcdef0123456789"); + expect(header).toContain(""); + }); + + it("replaces the value of a key/secret/password field in JSON or key=value form", () => { + expect(redactLogText('"api_key": "s3cr3tvalue123"')).toBe('"api_key": ""'); + expect(redactLogText("password=hunter2secret")).toBe("password="); + }); + + it("leaves text without private data unchanged", () => { + const clean = "agent started, 3 tools available, elapsed 42ms"; + expect(redactLogText(clean)).toBe(clean); + }); + + it("is idempotent — re-redacting already-redacted text is a no-op", () => { + const once = redactLogText("/Users/chaoyang/x and sk-abcdef0123456789"); + expect(redactLogText(once)).toBe(once); + }); + }); +}); diff --git a/src/utils/redactLog.ts b/src/utils/redactLog.ts new file mode 100644 index 00000000..6404f27e --- /dev/null +++ b/src/utils/redactLog.ts @@ -0,0 +1,52 @@ +/** + * Redacts private data from diagnostic log text before it leaves the machine in + * a bug-report bundle. A deterministic pattern pass, not a model: a frame log + * can be tens of megabytes, so this must be fast and synchronous. + * + * It targets what actually leaks into these logs — home-directory usernames in + * absolute paths (the same exposure as hiding vault-relative paths in the UI), + * email addresses, and common secret shapes — and leaves everything else intact + * so the log stays useful. Best-effort by design: a novel secret format can slip + * through, so the report flow still has the user review the bundle before + * attaching. Replacements are visible markers so a reader knows data was removed. + */ + +interface RedactionRule { + pattern: RegExp; + replacement: string; +} + +// Order matters only in that a path/email match should not be re-touched by a +// later rule; the markers below contain none of the trigger characters, so the +// rules are effectively independent. +const RULES: RedactionRule[] = [ + // Home-directory usernames in absolute paths (Unix + Windows). The path shape + // stays so the log still reads; only the identifying segment is removed. + { pattern: /(\/(?:Users|home)\/)[^/\s"'\\:]+/g, replacement: "$1" }, + { pattern: /([A-Za-z]:\\Users\\)[^\\\s"']+/gi, replacement: "$1" }, + + // Email addresses. + { pattern: /[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}/g, replacement: "" }, + + // Provider API keys with a recognizable prefix. + { pattern: /\bsk-[A-Za-z0-9_-]{12,}/g, replacement: "" }, + { pattern: /\bAIza[A-Za-z0-9_-]{20,}/g, replacement: "" }, + { pattern: /\bAKIA[0-9A-Z]{16}\b/g, replacement: "" }, + { pattern: /\bgh[pousr]_[A-Za-z0-9]{20,}/g, replacement: "" }, + { pattern: /\bxox[baprs]-[A-Za-z0-9-]{10,}/g, replacement: "" }, + + // Bearer tokens. + { pattern: /(bearer\s+)[A-Za-z0-9._-]{12,}/gi, replacement: "$1" }, + + // Values of key/token/secret/password-ish fields, JSON or key=value form. + { + pattern: + /("?(?:api[_-]?key|access[_-]?token|refresh[_-]?token|client[_-]?secret|secret|password|passwd|authorization|license[_-]?key)"?\s*[:=]\s*"?)[^"'\s,}]{6,}/gi, + replacement: "$1", + }, +]; + +/** Return `text` with private data replaced by visible markers. */ +export function redactLogText(text: string): string { + return RULES.reduce((acc, rule) => acc.replace(rule.pattern, rule.replacement), text); +}