fix(engine): validate the repo and PR number behind buildResultsPayload's customer link

packages/loopover-engine/src/results-payload.ts

@@ -40,22 +40,45 @@ export type ResultsPayload = {
 
 /** Package a completed iteration into the customer-facing results payload (#4801). Pure: it formats
  *  already-fetched iteration metadata, it does not fetch, open, or deliver anything. */
+// #9611: the same path-safety guard restated locally (this engine package must not import from the miner
+// package, and governor-ledger.ts keeps its own copy for the same reason): a repo segment is entirely
+// [A-Za-z0-9._-] and is not a bare "." / ".." traversal segment.
+const REPO_SEGMENT_PATTERN = /^[A-Za-z0-9._-]+$/;
+function isValidRepoSegment(segment: string): boolean {
+  return REPO_SEGMENT_PATTERN.test(segment) && segment !== "." && segment !== "..";
+}
+
+// #9611: same non-negative-integer normalization the sibling Rent-a-Loop modules use (tenant-quota.ts etc.),
+// so a caller-supplied negative or fractional additions/deletions count can't flow into totals or the diff.
+function finiteNonNegativeInt(value: number): number {
+  return Number.isFinite(value) ? Math.max(0, Math.floor(value)) : 0;
+}
+
 export function buildResultsPayload(result: IterationResult): ResultsPayload {
   const normalized: DiffPreviewFile[] = (result.changedFiles ?? []).map((f) => ({
     path: f.path,
-    additions: f.additions ?? 0,
-    deletions: f.deletions ?? 0,
+    additions: finiteNonNegativeInt(f.additions ?? 0),
+    deletions: finiteNonNegativeInt(f.deletions ?? 0),
   }));
   const totals = normalized.reduce(
     (acc, f) => ({ files: acc.files + 1, additions: acc.additions + f.additions, deletions: acc.deletions + f.deletions }),
     { files: 0, additions: 0, deletions: 0 },
   );
 
-  const hasPr = result.prNumber !== null && result.prNumber !== undefined;
-  const prLink = hasPr ? `https://github.com/${result.repoFullName}/pull/${result.prNumber}` : null;
+  // #9611: `prLink` becomes a clickable customer-facing URL, so validate BOTH interpolated values. An
+  // unvalidated repoFullName like "acme/widgets/../../evil" resolves in the browser to github.com/evil; a
+  // non-positive or non-integer prNumber renders ".../pull/0" or ".../pull/-3". Treat repoFullName as real
+  // only when it is exactly two path-safe segments, and require a positive integer PR number.
+  const repoSegments = result.repoFullName.split("/");
+  const validRepo = repoSegments.length === 2 && repoSegments.every((segment) => isValidRepoSegment(segment));
+  const repoDisplay = validRepo ? result.repoFullName : "unknown repository";
+
+  const hasPr =
+    result.prNumber !== null && result.prNumber !== undefined && Number.isInteger(result.prNumber) && result.prNumber > 0;
+  const prLink = hasPr && validRepo ? `https://github.com/${result.repoFullName}/pull/${result.prNumber}` : null;
   const status: LoopResultStatus = result.status ?? "open";
 
-  const prPart = hasPr ? `Opened PR #${result.prNumber} in ${result.repoFullName}` : `No pull request was opened for ${result.repoFullName}`;
+  const prPart = hasPr ? `Opened PR #${result.prNumber} in ${repoDisplay}` : `No pull request was opened for ${repoDisplay}`;
   const changePart =
     totals.files === 0
       ? "no file changes"

packages/loopover-engine/test/results-payload.test.ts

@@ -0,0 +1,60 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { buildResultsPayload } from "../dist/index.js";
+import type { IterationResult } from "../dist/index.js";
+
+const base = (over: Partial<IterationResult> = {}): IterationResult => ({
+  repoFullName: "acme/widgets",
+  prNumber: 1,
+  title: "t",
+  changedFiles: [],
+  ...over,
+});
+
+test("#9611: a valid repo + positive integer PR produces the canonical link and names the repo", () => {
+  const p = buildResultsPayload(base({ prNumber: 42 }));
+  assert.equal(p.prLink, "https://github.com/acme/widgets/pull/42");
+  assert.ok(p.summary.includes("Opened PR #42 in acme/widgets"));
+});
+
+test("#9611: an invalid repoFullName (traversal / extra segment / bad char / missing half) yields no link + 'unknown repository'", () => {
+  for (const repoFullName of ["acme/widgets/../../evil", "a/b/c", "acme/wid gets", "acme/..", "acme/.", "./widgets", "/widgets", "acme/", "acme"]) {
+    const p = buildResultsPayload(base({ repoFullName, prNumber: 1 }));
+    assert.equal(p.prLink, null, `expected null prLink for ${repoFullName}`);
+    assert.ok(p.summary.includes("unknown repository"), `expected 'unknown repository' for ${repoFullName}`);
+    assert.ok(!p.summary.includes("../"), `summary must not leak raw traversal for ${repoFullName}`);
+  }
+});
+
+test("#9611: a non-positive / non-integer / absent prNumber takes the no-PR branch", () => {
+  for (const prNumber of [0, -3, 2.5, null, undefined]) {
+    const p = buildResultsPayload(base({ prNumber }));
+    assert.equal(p.prLink, null);
+    assert.ok(p.summary.includes("No pull request was opened for acme/widgets"));
+  }
+});
+
+test("#9611: a valid PR number against an invalid repo still yields prLink null", () => {
+  const p = buildResultsPayload(base({ repoFullName: "acme/../evil", prNumber: 7 }));
+  assert.equal(p.prLink, null);
+});
+
+test("#9611: additions/deletions normalize to non-negative integers (negative, fractional, non-finite)", () => {
+  const p = buildResultsPayload(
+    base({
+      changedFiles: [
+        { path: "a", additions: -5, deletions: 2.7 },
+        { path: "b", additions: Number.NaN, deletions: 3 },
+      ],
+    }),
+  );
+  assert.deepEqual(p.diffPreview[0], { path: "a", additions: 0, deletions: 2 });
+  assert.deepEqual(p.diffPreview[1], { path: "b", additions: 0, deletions: 3 });
+  assert.deepEqual(p.totals, { files: 2, additions: 0, deletions: 5 });
+});
+
+test("#9611: absent additions/deletions default to 0 (the ?? 0 arms)", () => {
+  const p = buildResultsPayload(base({ changedFiles: [{ path: "a" }] }));
+  assert.deepEqual(p.diffPreview[0], { path: "a", additions: 0, deletions: 0 });
+});

test/unit/results-payload.test.ts

@@ -73,4 +73,30 @@ describe("buildResultsPayload — packages a completed loop iteration (#4801)",
     const p = buildResultsPayload({ repoFullName: "acme/widgets", prNumber: 12, title, changedFiles: [] });
     expect(p.summary).toBe(`Opened PR #12 in acme/widgets: ${title}. no file changes. Status: open.`);
   });
+
+  it("#9611: a path-traversal repoFullName yields no link and an 'unknown repository' summary", () => {
+    const p = buildResultsPayload({ repoFullName: "acme/widgets/../../evil", prNumber: 1, title: "t", changedFiles: [] });
+    expect(p.prLink).toBeNull();
+    expect(p.summary).toContain("unknown repository");
+    expect(p.summary).not.toContain("../");
+  });
+
+  it("#9611: a non-positive or non-integer prNumber takes the no-PR branch (link null, valid repo still named)", () => {
+    for (const prNumber of [0, -3, 2.5]) {
+      const p = buildResultsPayload({ repoFullName: "acme/widgets", prNumber, title: "t", changedFiles: [] });
+      expect(p.prLink).toBeNull();
+      expect(p.summary).toContain("No pull request was opened for acme/widgets");
+    }
+  });
+
+  it("#9611: negative/fractional additions and deletions normalize to non-negative integers", () => {
+    const p = buildResultsPayload({ repoFullName: "acme/widgets", prNumber: 1, title: "t", changedFiles: [{ path: "a", additions: -5, deletions: 2.7 }] });
+    expect(p.diffPreview[0]).toEqual({ path: "a", additions: 0, deletions: 2 });
+    expect(p.totals).toEqual({ files: 1, additions: 0, deletions: 2 });
+  });
+
+  it("#9611: a valid repo + positive integer PR still produces the canonical link", () => {
+    const p = buildResultsPayload({ repoFullName: "acme/widgets", prNumber: 42, title: "t", changedFiles: [] });
+    expect(p.prLink).toBe("https://github.com/acme/widgets/pull/42");
+  });
 });