Restrict interaction target href schemes

Co-Authored-By: Paperclip <noreply@paperclip.ing>

packages/shared/src/issue-thread-interactions.test.ts

@@ -70,29 +70,37 @@ describe("issue thread interaction schemas", () => {
   });
 
   it("accepts custom targets for request_confirmation interactions", () => {
-    const parsed = createIssueThreadInteractionSchema.parse({
-      kind: "request_confirmation",
-      payload: {
-        version: 1,
-        prompt: "Proceed with the external checklist?",
-        target: {
-          type: "custom",
-          key: "external-checklist",
-          revisionId: "checklist-v1",
-          revisionNumber: 1,
-          label: "Checklist v1",
-          href: "https://example.com/checklist",
+    for (const href of [
+      "https://example.com/checklist",
+      "http://example.com/checklist",
+      "/PAP/issues/PAP-123#document-plan",
+      "#document-plan",
+    ]) {
+      const parsed = createIssueThreadInteractionSchema.parse({
+        kind: "request_confirmation",
+        payload: {
+          version: 1,
+          prompt: "Proceed with the external checklist?",
+          target: {
+            type: "custom",
+            key: "external-checklist",
+            revisionId: "checklist-v1",
+            revisionNumber: 1,
+            label: "Checklist v1",
+            href,
+          },
         },
-      },
-    });
+      });
 
-    expect(parsed.kind).toBe("request_confirmation");
-    if (parsed.kind !== "request_confirmation") return;
-    expect(parsed.payload.target).toMatchObject({
-      type: "custom",
-      key: "external-checklist",
-      label: "Checklist v1",
-    });
+      expect(parsed.kind).toBe("request_confirmation");
+      if (parsed.kind !== "request_confirmation") return;
+      expect(parsed.payload.target).toMatchObject({
+        type: "custom",
+        key: "external-checklist",
+        label: "Checklist v1",
+        href,
+      });
+    }
   });
 
   it("rejects unsafe request_confirmation target hrefs", () => {
@@ -110,7 +118,16 @@ describe("issue thread interaction schemas", () => {
       },
     } as const;
 
-    for (const href of ["javascript:alert(1)", "data:text/html,hi", "//evil.example/path"]) {
+    for (const href of [
+      "javascript:alert(1)",
+      "data:text/html,hi",
+      "//evil.example/path",
+      "file:///tmp/x",
+      "mailto:user@example.com",
+      "slack://channel?id=1",
+      "vscode://file/tmp/x",
+      "ftp://example.com/file",
+    ]) {
       expect(() => createIssueThreadInteractionSchema.parse({
         ...base,
         payload: {
@@ -120,7 +137,7 @@ describe("issue thread interaction schemas", () => {
             href,
           },
         },
-      })).toThrow("href must not use javascript:, data:, or protocol-relative URLs");
+      })).toThrow("href must be a root-relative path, same-page fragment, or http(s) URL");
     }
   });
 
@@ -202,6 +219,36 @@ describe("issue thread interaction schemas", () => {
     })).toThrow("maxSelected must be greater than or equal to minSelected");
   });
 
+  it("rejects unsafe request_checkbox_confirmation target hrefs", () => {
+    const base = {
+      kind: "request_checkbox_confirmation",
+      payload: {
+        version: 1,
+        prompt: "Which items should be archived?",
+        options: [{ id: "item-1", label: "Draft report" }],
+        target: {
+          type: "custom",
+          key: "external-checklist",
+          revisionId: "checklist-v1",
+          label: "Checklist v1",
+        },
+      },
+    } as const;
+
+    for (const href of ["file:///tmp/x", "slack://channel?id=1", "vscode://file/tmp/x"]) {
+      expect(() => createIssueThreadInteractionSchema.parse({
+        ...base,
+        payload: {
+          ...base.payload,
+          target: {
+            ...base.payload.target,
+            href,
+          },
+        },
+      })).toThrow("href must be a root-relative path, same-page fragment, or http(s) URL");
+    }
+  });
+
   it("accepts empty checkbox selections and rejects duplicate selected option ids", () => {
     expect(acceptIssueThreadInteractionSchema.parse({ selectedOptionIds: [] })).toEqual({
       selectedOptionIds: [],

packages/shared/src/validators/issue.ts

@@ -682,11 +682,10 @@ export const askUserQuestionsResultSchema = z.object({
 });
 
 const requestConfirmationHrefSchema = z.string().trim().min(1).max(2000).refine((value) => {
-  const lower = value.toLowerCase();
-  return !lower.startsWith("javascript:")
-    && !lower.startsWith("data:")
-    && !value.startsWith("//");
-}, "href must not use javascript:, data:, or protocol-relative URLs");
+  if (value.startsWith("#")) return true;
+  if (value.startsWith("/")) return !value.startsWith("//");
+  return /^https?:\/\//i.test(value);
+}, "href must be a root-relative path, same-page fragment, or http(s) URL");
 
 const requestConfirmationTargetBaseSchema = z.object({
   label: z.string().trim().min(1).max(120).nullable().optional(),

ui/src/components/IssueThreadInteractionCard.tsx

@@ -8,6 +8,7 @@ import {
   collectSuggestedTaskClientKeys,
   countSuggestedTaskNodes,
   getCheckboxConfirmationSelectedLabels,
+  getRequestConfirmationTargetHref,
   getQuestionAnswerLabels,
   type AskUserQuestionsAnswer,
   type AskUserQuestionsInteraction,
@@ -983,21 +984,6 @@ function requestConfirmationTargetLabel(target: RequestConfirmationTarget) {
   return `${target.key}${revision}`;
 }
 
-function requestConfirmationTargetHref({
-  issueId,
-  target,
-}: {
-  issueId: string;
-  target: RequestConfirmationTarget;
-}) {
-  if (target.href) return target.href;
-  if (target.type === "issue_document") {
-    const targetIssueId = target.issueId ?? issueId;
-    return `/issues/${targetIssueId}#document-${encodeURIComponent(target.key)}`;
-  }
-  return null;
-}
-
 function RequestConfirmationTargetChip({
   issueId,
   target,
@@ -1009,7 +995,7 @@ function RequestConfirmationTargetChip({
 }) {
   if (!target) return null;
 
-  const href = requestConfirmationTargetHref({ issueId, target });
+  const href = getRequestConfirmationTargetHref({ issueId, target });
   const className = cn(
     "inline-flex max-w-full items-center gap-1.5 rounded-sm border px-2 py-0.5 text-[10px] font-medium uppercase tracking-[0.16em]",
     tone === "default"

ui/src/lib/issue-thread-interactions.test.ts

@@ -5,7 +5,9 @@ import {
   collectSuggestedTaskClientKeys,
   countSuggestedTaskNodes,
   getCheckboxConfirmationSelectedLabels,
+  getRequestConfirmationTargetHref,
   getQuestionAnswerLabels,
+  normalizeRequestConfirmationTargetHref,
 } from "./issue-thread-interactions";
 
 describe("buildSuggestedTaskTree", () => {
@@ -186,6 +188,51 @@ describe("issue thread interaction helpers", () => {
     expect(labels).toEqual(["Charlie", "Alpha"]);
   });
 
+  it("allows only safe confirmation target hrefs for rendering", () => {
+    for (const href of [
+      "https://example.com/checklist",
+      "http://example.com/checklist",
+      "/PAP/issues/PAP-123#document-plan",
+      "#document-plan",
+    ]) {
+      expect(normalizeRequestConfirmationTargetHref(href)).toBe(href);
+    }
+
+    for (const href of [
+      "file:///tmp/x",
+      "mailto:user@example.com",
+      "slack://channel?id=1",
+      "vscode://file/tmp/x",
+      "ftp://example.com/file",
+      "//evil.example/path",
+    ]) {
+      expect(normalizeRequestConfirmationTargetHref(href)).toBeNull();
+    }
+  });
+
+  it("does not return unsafe custom target hrefs from accepted payloads", () => {
+    expect(getRequestConfirmationTargetHref({
+      issueId: "issue-1",
+      target: {
+        type: "custom",
+        key: "unsafe-target",
+        label: "Unsafe target",
+        href: "file:///tmp/x",
+      },
+    })).toBeNull();
+
+    expect(getRequestConfirmationTargetHref({
+      issueId: "issue-1",
+      target: {
+        type: "issue_document",
+        issueId: "issue-2",
+        key: "plan",
+        revisionId: "11111111-1111-4111-8111-111111111111",
+        href: "slack://channel?id=1",
+      },
+    })).toBe("/issues/issue-2#document-plan");
+  });
+
   it("maps stored option ids back to labels for answered summaries", () => {
     const labels = getQuestionAnswerLabels({
       question: {

ui/src/lib/issue-thread-interactions.ts

@@ -33,6 +33,7 @@ import type {
   RequestCheckboxConfirmationPayload,
   RequestCheckboxConfirmationResult,
   RequestConfirmationInteraction,
+  RequestConfirmationTarget,
   SuggestedTaskDraft,
   SuggestTasksInteraction,
   SuggestTasksResultCreatedTask,
@@ -73,6 +74,31 @@ export function getCheckboxConfirmationSelectedLabels(args: {
     .filter((label): label is string => typeof label === "string");
 }
 
+export function normalizeRequestConfirmationTargetHref(href: string) {
+  const value = href.trim();
+  if (value.startsWith("#")) return value;
+  if (value.startsWith("/")) return value.startsWith("//") ? null : value;
+  return /^https?:\/\//i.test(value) ? value : null;
+}
+
+export function getRequestConfirmationTargetHref({
+  issueId,
+  target,
+}: {
+  issueId: string;
+  target: RequestConfirmationTarget;
+}) {
+  if (target.href) {
+    const safeHref = normalizeRequestConfirmationTargetHref(target.href);
+    if (safeHref) return safeHref;
+  }
+  if (target.type === "issue_document") {
+    const targetIssueId = target.issueId ?? issueId;
+    return `/issues/${targetIssueId}#document-${encodeURIComponent(target.key)}`;
+  }
+  return null;
+}
+
 export function buildIssueThreadInteractionSummary(
   interaction: IssueThreadInteraction,
 ) {