mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix(report): redact diagnostic logs before bundling them for a bug report (#2690)
The Agent Mode "Report an Issue" flow copied the frame log and opencode log into the bundle verbatim (`assembleReportBundle`, via `runtime.copyFile`). Those logs carry absolute home paths — which expose the OS username, the same class of leak as unredacted vault paths — and can carry emails or tokens. A user drag-dropping the bundle into a public GitHub issue would publish all of it. Now the two logs are read, redacted, and written instead of copied. Redaction is a new pure util `redactLogText` (src/utils/redactLog.ts): a deterministic pattern pass (home-dir usernames in Unix/Windows paths, emails, provider key shapes, bearer tokens, key/secret/password field values → visible markers), fast and synchronous because a frame log can be tens of MB. It is best-effort by design; the flow still shows the bundle for the user to review before attaching. `ReportRuntime` gains `readFile` and drops the now-unused `copyFile`; the two copy blocks collapse into one redact-and-write loop. The screenshot is still a binary write, unchanged. Tests: new redactLog.test.ts covers each pattern, a clean-text no-op, and idempotency; issueReport.test.ts now asserts the written logs are redacted (home path and secret gone) and that a log which fails to read is skipped rather than failing the report. Related to #213 (hiding absolute paths in Agent Mode), same exposure on the report surface. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e89bd562e9
commit
4e8e46cbc7
4 changed files with 151 additions and 36 deletions
|
|
@ -8,8 +8,9 @@ import {
|
|||
|
||||
function makeRuntime(overrides: Partial<ReportRuntime> = {}) {
|
||||
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<ReportRuntime> = {}) {
|
|||
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/<user>/vault");
|
||||
expect(text).toContain("<secret>");
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -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<void>;
|
||||
writeFile: (path: string, data: string | Uint8Array) => Promise<void>;
|
||||
copyFile: (src: string, dest: string) => Promise<void>;
|
||||
readFile: (path: string) => Promise<string>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -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"),
|
||||
};
|
||||
}
|
||||
|
|
|
|||
50
src/utils/redactLog.test.ts
Normal file
50
src/utils/redactLog.test.ts
Normal file
|
|
@ -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/<user>/vault/note.md"}'
|
||||
);
|
||||
expect(redactLogText("/home/logan/.config/app")).toBe("/home/<user>/.config/app");
|
||||
});
|
||||
|
||||
it("replaces a Windows home-directory username", () => {
|
||||
expect(redactLogText("C:\\Users\\Logan\\AppData")).toBe("C:\\Users\\<user>\\AppData");
|
||||
});
|
||||
|
||||
it("replaces email addresses", () => {
|
||||
expect(redactLogText("from logan@brevilabs.com to a@b.co")).toBe("from <email> to <email>");
|
||||
});
|
||||
|
||||
it("replaces provider API keys by their recognizable prefix", () => {
|
||||
expect(redactLogText("key sk-ant-abcdef0123456789xyz")).toBe("key <secret>");
|
||||
expect(redactLogText("AIzaSyD1234567890abcdefghijk")).toBe("<secret>");
|
||||
expect(redactLogText("token ghp_0123456789abcdefghij0")).toBe("token <secret>");
|
||||
});
|
||||
|
||||
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 <token> 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("<token>");
|
||||
});
|
||||
|
||||
it("replaces the value of a key/secret/password field in JSON or key=value form", () => {
|
||||
expect(redactLogText('"api_key": "s3cr3tvalue123"')).toBe('"api_key": "<redacted>"');
|
||||
expect(redactLogText("password=hunter2secret")).toBe("password=<redacted>");
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
52
src/utils/redactLog.ts
Normal file
52
src/utils/redactLog.ts
Normal file
|
|
@ -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<user>" },
|
||||
{ pattern: /([A-Za-z]:\\Users\\)[^\\\s"']+/gi, replacement: "$1<user>" },
|
||||
|
||||
// Email addresses.
|
||||
{ pattern: /[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}/g, replacement: "<email>" },
|
||||
|
||||
// Provider API keys with a recognizable prefix.
|
||||
{ pattern: /\bsk-[A-Za-z0-9_-]{12,}/g, replacement: "<secret>" },
|
||||
{ pattern: /\bAIza[A-Za-z0-9_-]{20,}/g, replacement: "<secret>" },
|
||||
{ pattern: /\bAKIA[0-9A-Z]{16}\b/g, replacement: "<secret>" },
|
||||
{ pattern: /\bgh[pousr]_[A-Za-z0-9]{20,}/g, replacement: "<secret>" },
|
||||
{ pattern: /\bxox[baprs]-[A-Za-z0-9-]{10,}/g, replacement: "<secret>" },
|
||||
|
||||
// Bearer tokens.
|
||||
{ pattern: /(bearer\s+)[A-Za-z0-9._-]{12,}/gi, replacement: "$1<token>" },
|
||||
|
||||
// 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<redacted>",
|
||||
},
|
||||
];
|
||||
|
||||
/** 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);
|
||||
}
|
||||
Loading…
Reference in a new issue