fix(ui): gate direct file opens by artifact policy

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>

ui/goose2/src/features/chat/ui/ToolCallAdapter.tsx

@@ -125,18 +125,25 @@ function InputSummary({
 
 function ToolLocations({ locations }: { locations: ToolCallLocation[] }) {
   const { t } = useTranslation("chat");
-  const { pathExists, openResolvedPath } = useArtifactPolicyContext();
+  const { pathExists, openResolvedPath, resolveMarkdownHref } =
+    useArtifactPolicyContext();
   const [openError, setOpenError] = useState<string | null>(null);
 
   const openLocation = async (path: string) => {
     try {
       setOpenError(null);
-      const exists = await pathExists(path);
-      if (!exists) {
-        setOpenError(t("tools.fileNotFound", { path }));
+      const candidate = resolveMarkdownHref(path);
+      if (!candidate?.allowed) {
+        setOpenError(candidate?.blockedReason || t("tools.pathOutsideRoots"));
         return;
       }
-      await openResolvedPath(path);
+
+      const exists = await pathExists(candidate.resolvedPath);
+      if (!exists) {
+        setOpenError(t("tools.fileNotFound", { path: candidate.resolvedPath }));
+        return;
+      }
+      await openResolvedPath(candidate.resolvedPath);
     } catch (error) {
       setOpenError(error instanceof Error ? error.message : String(error));
     }
@@ -145,19 +152,24 @@ function ToolLocations({ locations }: { locations: ToolCallLocation[] }) {
   return (
     <ToolSection label="Files">
       <div className="flex flex-wrap gap-1.5">
-        {locations.map((location) => (
-          <Button
-            key={`${location.path}:${location.line ?? ""}`}
-            type="button"
-            variant="outline-flat"
-            onClick={() => void openLocation(location.path)}
-            className="inline-flex h-auto max-w-full items-center justify-start rounded-full px-2.5 py-1 text-xs"
-            title={getToolLocationSubtitle(location)}
-          >
-            <FileText className="h-3.5 w-3.5 shrink-0" />
-            <span className="truncate">{getToolLocationTitle(location)}</span>
-          </Button>
-        ))}
+        {locations.map((location) => {
+          const candidate = resolveMarkdownHref(location.path);
+
+          return (
+            <Button
+              key={`${location.path}:${location.line ?? ""}`}
+              type="button"
+              variant="outline-flat"
+              onClick={() => void openLocation(location.path)}
+              className="inline-flex h-auto max-w-full items-center justify-start rounded-full px-2.5 py-1 text-xs"
+              title={getToolLocationSubtitle(location)}
+              disabled={!candidate?.allowed}
+            >
+              <FileText className="h-3.5 w-3.5 shrink-0" />
+              <span className="truncate">{getToolLocationTitle(location)}</span>
+            </Button>
+          );
+        })}
       </div>
       {openError ? (
         <p className="text-[11px] text-destructive">{openError}</p>
@@ -371,7 +383,7 @@ export function ToolCallAdapter({
     locations: visibleLocations,
     arguments: args,
   });
-  const { openResolvedPath } = useArtifactPolicyContext();
+  const { openResolvedPath, resolveMarkdownHref } = useArtifactPolicyContext();
   const rawResult =
     result ??
     (typeof rawOutput === "string"
@@ -389,7 +401,13 @@ export function ToolCallAdapter({
     headerFileLabel && headerFilePath
       ? splitHeaderTitle(name, headerFileLabel)
       : null;
-  const canOpenHeaderFile = Boolean(headerFilePath && headerTitleParts);
+  const headerFileCandidate = useMemo(
+    () => (headerFilePath ? resolveMarkdownHref(headerFilePath) : null),
+    [headerFilePath, resolveMarkdownHref],
+  );
+  const canOpenHeaderFile = Boolean(
+    headerTitleParts && headerFileCandidate?.allowed,
+  );
 
   return (
     <div className={cn(fitWidth && "inline-flex max-w-full flex-col")}>
@@ -405,25 +423,31 @@ export function ToolCallAdapter({
             headerTitleParts ? (
               <>
                 <span data-tool-title-prefix>{headerTitleParts.prefix}</span>
-                <button
-                  type="button"
-                  data-clickable-file
-                  onClick={(event) => {
-                    event.stopPropagation();
-                    if (!headerFilePath) {
-                      return;
-                    }
-                    void openResolvedPath(headerFilePath).catch(() => {});
-                  }}
-                  onKeyDown={(event) => {
-                    event.stopPropagation();
-                  }}
-                  title={headerFilePath}
-                  aria-label={`Open ${headerTitleParts.fileLabel}`}
-                  className="inline truncate text-foreground underline-offset-2 hover:underline"
-                >
-                  {headerTitleParts.fileLabel}
-                </button>
+                {canOpenHeaderFile ? (
+                  <button
+                    type="button"
+                    data-clickable-file
+                    onClick={(event) => {
+                      event.stopPropagation();
+                      if (!headerFileCandidate?.allowed) {
+                        return;
+                      }
+                      void openResolvedPath(
+                        headerFileCandidate.resolvedPath,
+                      ).catch(() => {});
+                    }}
+                    onKeyDown={(event) => {
+                      event.stopPropagation();
+                    }}
+                    title={headerFileCandidate?.resolvedPath ?? headerFilePath}
+                    aria-label={`Open ${headerTitleParts.fileLabel}`}
+                    className="inline truncate text-foreground underline-offset-2 hover:underline"
+                  >
+                    {headerTitleParts.fileLabel}
+                  </button>
+                ) : (
+                  <span>{headerTitleParts.fileLabel}</span>
+                )}
                 <span>{headerTitleParts.suffix}</span>
               </>
             ) : (

ui/goose2/src/features/chat/ui/__tests__/ToolCallAdapter.header-file.test.tsx

@@ -0,0 +1,146 @@
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import type { ToolCardDisplay } from "@/features/chat/hooks/ArtifactPolicyContext";
+import type { ArtifactPathCandidate } from "@/features/chat/lib/artifactPathPolicy";
+import { ToolCallAdapter } from "../ToolCallAdapter";
+
+const mockResolveToolCardDisplay =
+  vi.fn<
+    (
+      args: Record<string, unknown>,
+      name: string,
+      result?: string,
+    ) => ToolCardDisplay
+  >();
+const mockResolveMarkdownHref =
+  vi.fn<(href: string) => ArtifactPathCandidate | null>();
+const mockPathExists = vi.fn<(path: string) => Promise<boolean>>();
+const mockOpenResolvedPath = vi.fn<(path: string) => Promise<void>>();
+
+vi.mock("@/features/chat/hooks/ArtifactPolicyContext", () => ({
+  useArtifactPolicyContext: () => ({
+    resolveToolCardDisplay: mockResolveToolCardDisplay,
+    resolveMarkdownHref: mockResolveMarkdownHref,
+    pathExists: mockPathExists,
+    openResolvedPath: mockOpenResolvedPath,
+  }),
+}));
+
+const EMPTY_DISPLAY: ToolCardDisplay = {
+  role: "none",
+  primaryCandidate: null,
+  secondaryCandidates: [],
+};
+
+function makeCandidate(
+  overrides: Partial<ArtifactPathCandidate> = {},
+): ArtifactPathCandidate {
+  return {
+    id: "c-1",
+    rawPath: "/project/Sources/main.swift",
+    resolvedPath: "/project/Sources/main.swift",
+    source: "arg_key",
+    confidence: "high",
+    kind: "file",
+    allowed: true,
+    blockedReason: null,
+    toolCallId: "tool-1",
+    toolName: "edit_file",
+    toolCallIndex: 0,
+    appearanceIndex: 0,
+    ...overrides,
+  };
+}
+
+function renderAdapter(
+  overrides: Partial<Parameters<typeof ToolCallAdapter>[0]> = {},
+) {
+  return render(
+    <ToolCallAdapter
+      name="Edit main.swift"
+      arguments={{ path: "/project/Sources/main.swift", line: 1 }}
+      kind="edit"
+      status="completed"
+      result="Updated /project/Sources/main.swift"
+      open={undefined}
+      {...overrides}
+    />,
+  );
+}
+
+beforeEach(() => {
+  mockResolveToolCardDisplay.mockReset();
+  mockResolveToolCardDisplay.mockReturnValue(EMPTY_DISPLAY);
+  mockResolveMarkdownHref.mockReset();
+  mockResolveMarkdownHref.mockImplementation((href) =>
+    makeCandidate({
+      rawPath: href,
+      resolvedPath: href,
+    }),
+  );
+  mockPathExists.mockReset();
+  mockPathExists.mockResolvedValue(true);
+  mockOpenResolvedPath.mockReset();
+  mockOpenResolvedPath.mockResolvedValue(undefined);
+});
+
+describe("ToolCallAdapter header file link", () => {
+  it("opens the file from the header filename without expanding the accordion", async () => {
+    const user = userEvent.setup();
+
+    renderAdapter();
+
+    await user.click(screen.getByRole("button", { name: /open main\.swift/i }));
+
+    expect(mockOpenResolvedPath).toHaveBeenCalledWith(
+      "/project/Sources/main.swift",
+    );
+    expect(screen.queryByText("Path")).not.toBeInTheDocument();
+  });
+
+  it("does not expose a clickable header filename outside allowed roots", async () => {
+    const user = userEvent.setup();
+    mockResolveMarkdownHref.mockReturnValue(
+      makeCandidate({
+        rawPath: "/etc/passwd",
+        resolvedPath: "/etc/passwd",
+        allowed: false,
+        blockedReason: "Path is outside allowed project/artifacts roots.",
+      }),
+    );
+
+    renderAdapter({
+      name: "Edit passwd",
+      arguments: { path: "/etc/passwd", line: 1 },
+      result: "Updated /etc/passwd",
+    });
+
+    expect(
+      screen.queryByRole("button", { name: /open passwd/i }),
+    ).not.toBeInTheDocument();
+
+    await user.click(screen.getByText("passwd"));
+
+    expect(mockOpenResolvedPath).not.toHaveBeenCalled();
+    expect(screen.getByText("Path")).toBeInTheDocument();
+  });
+
+  it("opens the accordion when clicking the non-file title text", async () => {
+    const user = userEvent.setup();
+
+    const { container } = renderAdapter();
+
+    const titlePrefix = container.querySelector("[data-tool-title-prefix]");
+    expect(titlePrefix).toBeTruthy();
+
+    if (!titlePrefix) {
+      throw new Error("Expected non-file title text");
+    }
+
+    await user.click(titlePrefix);
+
+    expect(mockOpenResolvedPath).not.toHaveBeenCalled();
+    expect(screen.getByText("Path")).toBeInTheDocument();
+  });
+});

ui/goose2/src/features/chat/ui/__tests__/ToolCallAdapter.test.tsx

@@ -1,6 +1,6 @@
 import { render, screen, within } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { describe, expect, it, vi } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
 import type { ToolCardDisplay } from "@/features/chat/hooks/ArtifactPolicyContext";
 import type { ArtifactPathCandidate } from "@/features/chat/lib/artifactPathPolicy";
 import { ToolCallAdapter } from "../ToolCallAdapter";
@@ -15,13 +15,15 @@ const mockResolveToolCardDisplay =
       result?: string,
     ) => ToolCardDisplay
   >();
+const mockResolveMarkdownHref =
+  vi.fn<(href: string) => ArtifactPathCandidate | null>();
 const mockPathExists = vi.fn<(path: string) => Promise<boolean>>();
 const mockOpenResolvedPath = vi.fn<(path: string) => Promise<void>>();
 
 vi.mock("@/features/chat/hooks/ArtifactPolicyContext", () => ({
   useArtifactPolicyContext: () => ({
     resolveToolCardDisplay: mockResolveToolCardDisplay,
-    resolveMarkdownHref: () => null,
+    resolveMarkdownHref: mockResolveMarkdownHref,
     pathExists: mockPathExists,
     openResolvedPath: mockOpenResolvedPath,
   }),
@@ -55,6 +57,22 @@ function makeCandidate(
   };
 }
 
+beforeEach(() => {
+  mockResolveToolCardDisplay.mockReset();
+  mockResolveToolCardDisplay.mockReturnValue(EMPTY_DISPLAY);
+  mockResolveMarkdownHref.mockReset();
+  mockResolveMarkdownHref.mockImplementation((href) =>
+    makeCandidate({
+      rawPath: href,
+      resolvedPath: href,
+    }),
+  );
+  mockPathExists.mockReset();
+  mockPathExists.mockResolvedValue(true);
+  mockOpenResolvedPath.mockReset();
+  mockOpenResolvedPath.mockResolvedValue(undefined);
+});
+
 function renderAdapter(
   overrides: Partial<Parameters<typeof ToolCallAdapter>[0]> = {},
 ) {
@@ -113,54 +131,6 @@ describe("ToolCallAdapter — ArtifactActions", () => {
     expect(rawInputPanel).toHaveTextContent('"path": "/project/src/main.ts"');
   });
 
-  it("opens the file from the header filename without expanding the accordion", async () => {
-    const user = userEvent.setup();
-    mockResolveToolCardDisplay.mockReturnValue(EMPTY_DISPLAY);
-    mockOpenResolvedPath.mockResolvedValue(undefined);
-
-    renderAdapter({
-      name: "Edit main.swift",
-      kind: "edit",
-      arguments: { path: "/project/Sources/main.swift", line: 1 },
-      result: "Updated /project/Sources/main.swift",
-      open: undefined,
-    });
-
-    await user.click(screen.getByRole("button", { name: /open main\.swift/i }));
-
-    expect(mockOpenResolvedPath).toHaveBeenCalledWith(
-      "/project/Sources/main.swift",
-    );
-    expect(screen.queryByText("Path")).not.toBeInTheDocument();
-  });
-
-  it("opens the accordion when clicking the non-file title text", async () => {
-    const user = userEvent.setup();
-    mockResolveToolCardDisplay.mockReturnValue(EMPTY_DISPLAY);
-    mockOpenResolvedPath.mockReset();
-    mockOpenResolvedPath.mockResolvedValue(undefined);
-
-    const { container } = renderAdapter({
-      name: "Edit main.swift",
-      kind: "edit",
-      arguments: { path: "/project/Sources/main.swift", line: 1 },
-      result: "Updated /project/Sources/main.swift",
-      open: undefined,
-    });
-
-    const titlePrefix = container.querySelector("[data-tool-title-prefix]");
-    expect(titlePrefix).toBeTruthy();
-
-    if (!titlePrefix) {
-      throw new Error("Expected non-file title text");
-    }
-
-    await user.click(titlePrefix);
-
-    expect(mockOpenResolvedPath).not.toHaveBeenCalled();
-    expect(screen.getByText("Path")).toBeInTheDocument();
-  });
-
   it("renders command input summaries with a clamped preview and expanded bash highlighting", async () => {
     const user = userEvent.setup();
     mockResolveToolCardDisplay.mockReturnValue(EMPTY_DISPLAY);
@@ -227,9 +197,6 @@ describe("ToolCallAdapter — ArtifactActions", () => {
 
   it("renders multi-location fallbacks as inline file pills", async () => {
     const user = userEvent.setup();
-    mockResolveToolCardDisplay.mockReturnValue(EMPTY_DISPLAY);
-    mockPathExists.mockResolvedValue(true);
-    mockOpenResolvedPath.mockResolvedValue(undefined);
 
     renderAdapter({
       name: "read_file",
@@ -258,6 +225,40 @@ describe("ToolCallAdapter — ArtifactActions", () => {
     expect(mockOpenResolvedPath).toHaveBeenCalledWith("/project/src/index.js");
   });
 
+  it("disables multi-location pills outside allowed roots", () => {
+    mockResolveMarkdownHref.mockImplementation((href) =>
+      href === "/project/src/index.js"
+        ? makeCandidate({
+            rawPath: href,
+            resolvedPath: href,
+            allowed: false,
+            blockedReason: "Path is outside allowed project/artifacts roots.",
+          })
+        : makeCandidate({
+            rawPath: href,
+            resolvedPath: href,
+          }),
+    );
+
+    renderAdapter({
+      name: "read_file",
+      kind: "read",
+      arguments: { path: "/project/src/renderer.js", line: 42 },
+      locations: [
+        { path: "/project/src/renderer.js", line: 42 },
+        { path: "/project/src/index.js", line: 12 },
+      ],
+      result: "flattened result",
+    });
+
+    expect(
+      screen.getByRole("button", { name: /renderer\.js:42/i }),
+    ).toBeEnabled();
+    expect(
+      screen.getByRole("button", { name: /index\.js:12/i }),
+    ).toBeDisabled();
+  });
+
   it('renders "Open file" button when primary candidate exists', () => {
     const primary = makeCandidate();
     mockResolveToolCardDisplay.mockReturnValue({