From 5ffb98c76ec34edb5e8669eeeddb63adc58adc1e Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:45:40 -0700 Subject: [PATCH] fix(review): escape fix handoff paths --- src/review/fix-handoff-render.ts | 14 +++++++++++++- test/unit/fix-handoff-collapsible.test.ts | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/review/fix-handoff-render.ts b/src/review/fix-handoff-render.ts index a01f01d8ac..5d8c933454 100644 --- a/src/review/fix-handoff-render.ts +++ b/src/review/fix-handoff-render.ts @@ -33,13 +33,25 @@ export type FixHandoffBlock = { * parse fix-handoff blocks in a comment body without depending on markdown structure alone. */ const FIX_HANDOFF_MARKER = ""; +/** Public-safe inline-code escaping for a finding path/location. GitHub comments still render markdown inside + * collapsibles, so neutralize delimiters that can break out of the `...` span or table-like contexts before + * composing the location label. */ +function markdownPathCodeText(value: string): string { + return value + .replace(/\\/g, "\\\\") + .replace(/`/g, "\\`") + .replace(/\|/g, "\\|") + .replace(/[<>]/g, (char) => (char === "<" ? "<" : ">")); +} + /** PURE: build a single finding's fix-handoff block. Never throws; a finding whose `line` is not a positive * integer (0, negative, non-finite — i.e. "no commentable line") still yields a valid PATH-ONLY block rather * than being dropped, since the finding itself is still actionable context even without a line anchor. */ export function buildFixHandoffBlock(finding: InlineFinding): FixHandoffBlock { const hasLine = Number.isInteger(finding.line) && finding.line > 0; const line = hasLine ? finding.line : 0; - const location = hasLine ? `${finding.path}:${line}` : `${finding.path} (no specific line)`; + const safePath = markdownPathCodeText(finding.path); + const location = hasLine ? `${safePath}:${line}` : `${safePath} (no specific line)`; const label = finding.severity === "blocker" ? "Blocker" : "Nit"; const suggestedChange = finding.suggestion?.trim() || undefined; const suggestionBlock = suggestedChange ? `\n\nSuggested change:\n\`\`\`\n${suggestedChange}\n\`\`\`` : ""; diff --git a/test/unit/fix-handoff-collapsible.test.ts b/test/unit/fix-handoff-collapsible.test.ts index 25b41dcc11..39b786ae4b 100644 --- a/test/unit/fix-handoff-collapsible.test.ts +++ b/test/unit/fix-handoff-collapsible.test.ts @@ -44,6 +44,21 @@ describe("buildFixHandoffCollapsible (#1962)", () => { expect(c?.body).toContain("`src/b.ts (no specific line)`"); }); + it("escapes adversarial paths before rendering inline-code locations", () => { + const [block] = buildFixHandoffBlocks([ + { + path: "src/x` [Review required](https://evil.example/phish) | ", + line: 7, + severity: "blocker", + body: "Check the suspicious path rendering.", + }, + ]); + + expect(block?.path).toBe("src/x` [Review required](https://evil.example/phish) | "); + expect(block?.body).toContain("`src/x\\` [Review required](https://evil.example/phish) \\| <tag>:7`"); + expect(block?.body).not.toContain("`src/x` [Review required](https://evil.example/phish) | :7`"); + }); + it("carries the no-server-side-write local-execution boundary on every block", () => { expect(buildFixHandoffCollapsible(blocks)?.body).toContain(LOCAL_WRITE_BOUNDARY); });