Skip to content

Commit 3bf6149

Browse files
committed
fix(advisor): harden review comment location escaping
1 parent 43a83a8 commit 3bf6149

2 files changed

Lines changed: 51 additions & 3 deletions

File tree

test/pr-review-advisor.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,6 +1205,52 @@ diff --git a/test/example.test.ts b/test/example.test.ts
12051205
expect(comment).not.toContain("check </details>");
12061206
});
12071207

1208+
it("keeps hostile file locations inside checklist and table fields", () => {
1209+
const result = normalizeReviewResult(
1210+
validResult({
1211+
findings: [
1212+
{
1213+
severity: "blocker",
1214+
category: "correctness",
1215+
file: "src/a|b.ts",
1216+
line: 7,
1217+
title: "Pipe in path",
1218+
description: "Location should not add a table cell.",
1219+
},
1220+
{
1221+
severity: "warning",
1222+
category: "correctness",
1223+
file: "src/a\nb.ts",
1224+
line: 8,
1225+
title: "Newline in path",
1226+
description: "Location should stay on one rendered line.",
1227+
},
1228+
{
1229+
severity: "suggestion",
1230+
category: "correctness",
1231+
file: "src/a`b.ts",
1232+
line: 9,
1233+
title: "Backtick in path",
1234+
description: "Location should not break a Markdown code span.",
1235+
},
1236+
],
1237+
}),
1238+
metadata(),
1239+
);
1240+
const comment = buildComment({ summary: renderSummary(result), result });
1241+
const indexRows = comment.split("\n").filter((line) => /^\| `PRA-/.test(line));
1242+
1243+
expect(indexRows).toHaveLength(3);
1244+
expect(indexRows[0]).toContain("<code>src/a&#124;b.ts:7</code>");
1245+
expect(indexRows[1]).toContain("<code>src/a b.ts:8</code>");
1246+
expect(indexRows[2]).toContain("<code>src/a`b.ts:9</code>");
1247+
for (const row of indexRows) expect(row.match(/\|/g)).toHaveLength(6);
1248+
expect(comment).toContain("- [ ] `PRA-1` Fix: Pipe in path in <code>src/a&#124;b.ts:7</code>");
1249+
expect(comment).toContain("- **Location:** <code>src/a b.ts:8</code>");
1250+
expect(comment).not.toContain("src/a\nb.ts");
1251+
expect(comment).not.toContain("`src/a`b.ts:9`");
1252+
});
1253+
12081254
it("escapes advisor finding text before rendering sticky comments", () => {
12091255
const result = normalizeReviewResult(
12101256
validResult({

tools/pr-review-advisor/comment.mts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -602,19 +602,21 @@ function formatFindingLocation(finding: Finding): string {
602602
function formatInlineLocation(finding: Finding): string {
603603
if (!finding.file) return "";
604604
const line = Number.isInteger(finding.line) && Number(finding.line) > 0 ? `:${finding.line}` : "";
605-
return `\`${escapeCodeSpan(`${finding.file}${line}`)}\``;
605+
return `<code>${escapeLocationHtml(`${finding.file}${line}`)}</code>`;
606606
}
607607

608608
function formatTableLocation(finding: Finding): string {
609609
return formatInlineLocation(finding) || "—";
610610
}
611611

612-
function escapeCodeSpan(value: string): string {
612+
function escapeLocationHtml(value: string): string {
613613
return value
614+
.replace(/\s+/g, " ")
615+
.trim()
614616
.replaceAll("&", "&amp;")
615-
.replaceAll("`", "\\`")
616617
.replaceAll("<", "&lt;")
617618
.replaceAll(">", "&gt;")
619+
.replaceAll("|", "&#124;")
618620
.replaceAll("@", "&#64;");
619621
}
620622

0 commit comments

Comments
 (0)