fix(visual): filter a vision finding's model-authored path through toPublicSafe

VisualVisionFinding's JSDoc says both path and body are already public-safe, but
parseVisualVisionResponse only ran body through toPublicSafe — path got a bare .trim().
path is model-authored free text off the vision response JSON, interpolated verbatim
into a public finding title ("Possible visual regression: <path>"), and visual-followup's
renderer trusts the JSDoc and does not re-scrub. So injected markdown/@mentions in a
model path reached a public comment live.

Run path through toPublicSafe exactly as body is (drop the entry when it returns null),
and bound it with a new MAX_VISUAL_PATH_CHARS constant so one finding's title cannot
flood a comment. An ordinary route path survives toPublicSafe byte-identically, so
findVisualEvidence's route.path === path lookup still matches (pinned by a test). body's
filtering, MAX_VISUAL_FINDINGS, the category parse, and the followup re-scrub are unchanged.

Closes #10062

Author: RealDiligent

src/review/visual/visual-findings.ts

@@ -134,6 +134,10 @@ export type VisualVisionFinding = { path: string; body: string; category?: "regr
 /** Cap on findings kept from a single vision response — mirrors `composeAdvisoryNotes`'s selectivity so a
  *  verbose model can't pad the comment with a long list of minor observations. */
 const MAX_VISUAL_FINDINGS = 3;
+/** Upper bound on a model-authored finding `path` after public-safe filtering, so one finding's title cannot
+ *  flood a public comment. A real route path is far shorter; a value past this is truncated for display and
+ *  simply won't match any captured route in findVisualEvidence (#10062). */
+const MAX_VISUAL_PATH_CHARS = 256;
 
 export const VISUAL_VISION_SYSTEM_PROMPT = [
   "You are reviewing a BEFORE (production) vs AFTER (this pull request's preview deploy) screenshot pair for the same route.",
@@ -217,10 +221,14 @@ export function parseVisualVisionResponse(text: string): VisualVisionFinding[] {
     if (out.length >= MAX_VISUAL_FINDINGS) break;
     if (!entry || typeof entry !== "object") continue;
     const record = entry as Record<string, unknown>;
-    const path = typeof record.path === "string" ? record.path.trim() : "";
+    // path is model-authored free text off the vision JSON, interpolated into a public finding title — filter
+    // it through toPublicSafe exactly as body is, dropping the entry when it returns null (#10062).
+    const rawPath = typeof record.path === "string" ? record.path : "";
+    const filteredPath = toPublicSafe(rawPath);
     const rawBody = typeof record.body === "string" ? record.body : "";
     const body = toPublicSafe(rawBody);
-    if (!path || !body) continue;
+    if (!filteredPath || !body) continue;
+    const path = filteredPath.slice(0, MAX_VISUAL_PATH_CHARS);
     const category = record.category === "regression" || record.category === "unrelated" ? record.category : undefined;
     out.push(category ? { path, body, category } : { path, body });
   }

test/unit/visual-findings.test.ts

@@ -200,6 +200,28 @@ describe("parseVisualVisionResponse", () => {
     expect(parseVisualVisionResponse(text)).toEqual([]);
   });
 
+  it("REGRESSION (#10062): filters public-unsafe markup out of a model-authored path", () => {
+    const text = JSON.stringify({ findings: [{ path: "/app [pwn](http://evil.example) @maintainer", body: "Broke." }] });
+    const [finding] = parseVisualVisionResponse(text);
+    expect(finding).toBeDefined();
+    // The live link-injection markup no longer survives into the public finding path.
+    expect(finding!.path).not.toContain("](http://evil.example)");
+    expect(finding!.path).not.toContain("[pwn]");
+  });
+
+  it("REGRESSION (#10062): bounds a path to the MAX_VISUAL_PATH_CHARS constant", () => {
+    const text = JSON.stringify({ findings: [{ path: `/${"a".repeat(500)}`, body: "Broke." }] });
+    const [finding] = parseVisualVisionResponse(text);
+    expect(finding!.path.length).toBe(256);
+  });
+
+  it("REGRESSION (#10062): an ordinary route path round-trips byte-identically and still attaches visualEvidence", () => {
+    const findings = parseVisualVisionResponse(JSON.stringify({ findings: [{ path: "/app", body: "Broke." }] }));
+    expect(findings).toEqual([{ path: "/app", body: "Broke." }]); // survives toPublicSafe unchanged
+    const built = buildVisualRegressionFindings(findings, [changedRoute("/app")]);
+    expect(built[0]?.visualEvidence?.path).toBe("/app"); // route lookup still matches on the scrubbed path
+  });
+
   it("drops an entry with a blank/empty body (fails toPublicSafe's emptiness guard)", () => {
     const text = JSON.stringify({ findings: [{ path: "/pricing", body: "" }] });
     expect(parseVisualVisionResponse(text)).toEqual([]);

test/unit/visual-followup.test.ts

@@ -5,7 +5,7 @@ import {
   selectUnrelatedVisualFindings,
 } from "../../src/review/visual/visual-followup";
 import { VISUAL_FOLLOWUP_COMMENT_MARKER } from "../../src/github/comments";
-import { VISUAL_REGRESSION_FINDING_CODE, VISUAL_UNRELATED_ISSUE_FINDING_CODE } from "../../src/review/visual/visual-findings";
+import { buildVisualRegressionFindings, parseVisualVisionResponse, VISUAL_REGRESSION_FINDING_CODE, VISUAL_UNRELATED_ISSUE_FINDING_CODE } from "../../src/review/visual/visual-findings";
 import type { AdvisoryFinding } from "../../src/types";
 
 const unrelatedFinding = (overrides: Partial<AdvisoryFinding> = {}): AdvisoryFinding => ({
@@ -135,4 +135,14 @@ describe("buildVisualFollowupComment", () => {
     expect(body).not.toContain("Possible visual regression");
     expect(body).toContain("Possible unrelated visual issue");
   });
+
+  it("REGRESSION (#10062): a hostile model path is scrubbed at parse time and never reaches the follow-up body as live markup", () => {
+    const hostile = JSON.stringify({
+      findings: [{ path: "/app [pwn](http://evil.example) @maintainer", body: "Layout looks off, unrelated to this change.", category: "unrelated" }],
+    });
+    const built = buildVisualRegressionFindings(parseVisualVisionResponse(hostile));
+    const body = buildVisualFollowupComment(built, ["jsonbored"]);
+    expect(body).not.toBeNull();
+    expect(body!).not.toContain("](http://evil.example)"); // the injected link never renders live
+  });
 });