Allow opening explicit outputs outside artifact roots (#82)

* Allow opening explicit outputs outside artifact roots

* fix: tighten artifact path opening rules
This commit is contained in:
tulsi
2026-04-02 10:45:30 -07:00
committed by GitHub
parent 7b09bdcc18
commit e6e7c8b7cd
10 changed files with 529 additions and 53 deletions
@@ -3,3 +3,9 @@ pub fn get_home_dir() -> Result<String, String> {
let home_dir = dirs::home_dir().ok_or("Could not determine home directory")?;
Ok(home_dir.to_string_lossy().into_owned())
}
#[tauri::command]
#[allow(dead_code)]
pub fn path_exists(path: String) -> bool {
std::path::Path::new(&path).exists()
}
+1
View File
@@ -70,6 +70,7 @@ pub fn run() {
commands::doctor::run_doctor,
commands::doctor::run_doctor_fix,
commands::system::get_home_dir,
commands::system::path_exists,
])
.build(tauri::generate_context!())
.expect("error while building tauri application")
@@ -8,6 +8,7 @@ import {
type ReactNode,
} from "react";
import type { Message } from "@/shared/types/messages";
import { pathExists } from "@/shared/api/system";
import {
buildArtifactsIndexForMessages,
resolveMarkdownLocalHref,
@@ -27,6 +28,7 @@ interface ArtifactPolicyContextValue {
result?: string,
) => ToolCardDisplay;
resolveMarkdownHref: (href: string) => ArtifactPathCandidate | null;
pathExists: (path: string) => Promise<boolean>;
openResolvedPath: (path: string) => Promise<void>;
}
@@ -39,6 +41,7 @@ const EMPTY_DISPLAY: ToolCardDisplay = {
const DEFAULT_CONTEXT_VALUE: ArtifactPolicyContextValue = {
resolveToolCardDisplay: () => EMPTY_DISPLAY,
resolveMarkdownHref: () => null,
pathExists: async () => false,
openResolvedPath: async () => {},
};
@@ -94,24 +97,40 @@ export function ArtifactPolicyProvider({
[normalizedRoots],
);
const openResolvedPath = useCallback(async (path: string) => {
const key = path.trim().toLowerCase();
const now = Date.now();
const lastOpenAt = lastOpenAtByPathRef.current.get(key) ?? 0;
if (now - lastOpenAt < 1200) {
return;
}
lastOpenAtByPathRef.current.set(key, now);
await openPath(path);
}, []);
const checkPathExists = useCallback((path: string) => pathExists(path), []);
const openResolvedPath = useCallback(
async (path: string) => {
const exists = await checkPathExists(path);
if (!exists) {
throw new Error(`File not found: ${path}`);
}
const key = path.trim().toLowerCase();
const now = Date.now();
const lastOpenAt = lastOpenAtByPathRef.current.get(key) ?? 0;
if (now - lastOpenAt < 1200) {
return;
}
lastOpenAtByPathRef.current.set(key, now);
await openPath(path);
},
[checkPathExists],
);
const contextValue = useMemo<ArtifactPolicyContextValue>(
() => ({
resolveToolCardDisplay,
resolveMarkdownHref,
pathExists: checkPathExists,
openResolvedPath,
}),
[openResolvedPath, resolveMarkdownHref, resolveToolCardDisplay],
[
checkPathExists,
openResolvedPath,
resolveMarkdownHref,
resolveToolCardDisplay,
],
);
return (
@@ -116,6 +116,46 @@ describe("artifactPathPolicy", () => {
expect(blocked.blockedReason).toContain("outside allowed");
});
it("allows explicit write outputs outside default roots", () => {
const ranking = rankMessageToolArtifacts(
[
{
toolCallId: "write-1",
toolName: "Write coffee_shop_inventory.csv",
args: {},
result: "/Users/test/Desktop/coffee_shop_inventory.csv (new)",
toolCallIndex: 0,
},
],
["/Users/test/.goose/artifacts"],
);
expect(ranking.primaryCandidate?.resolvedPath).toBe(
"/Users/test/Desktop/coffee_shop_inventory.csv",
);
expect(ranking.primaryCandidate?.allowed).toBe(true);
expect(ranking.primaryCandidate?.blockedReason).toBeNull();
});
it("keeps write arg-key paths outside default roots blocked until a result confirms them", () => {
const candidates = extractToolCallCandidates(
{
toolCallId: "write-1",
toolName: "write_file",
args: { path: "/Users/test/Desktop/coffee_shop_inventory.csv" },
toolCallIndex: 0,
},
["/Users/test/.goose/artifacts"],
);
expect(candidates).toHaveLength(1);
expect(candidates[0].resolvedPath).toBe(
"/Users/test/Desktop/coffee_shop_inventory.csv",
);
expect(candidates[0].allowed).toBe(false);
expect(candidates[0].blockedReason).toContain("outside allowed");
});
it("keeps write-origin candidates and drops noisy non-write regex candidates", () => {
const ranking = rankMessageToolArtifacts(
[
@@ -148,6 +188,47 @@ describe("artifactPathPolicy", () => {
).toBe(false);
});
it("extracts command-style output paths from tool titles", () => {
const ranking = rankMessageToolArtifacts(
[
{
toolCallId: "cmd-1",
toolName:
"python3 scripts/build_dashboard.py --output output/table.csv",
args: {},
toolCallIndex: 0,
},
],
roots,
);
expect(ranking.primaryCandidate?.resolvedPath).toBe(
"/Users/test/project-a/output/table.csv",
);
expect(ranking.primaryCandidate?.allowed).toBe(true);
});
it("extracts command-style output paths from command args", () => {
const ranking = rankMessageToolArtifacts(
[
{
toolCallId: "cmd-1",
toolName: "functions.exec_command",
args: {
cmd: "python3 scripts/export.py > output/weather-dashboard.html",
},
toolCallIndex: 0,
},
],
roots,
);
expect(ranking.primaryCandidate?.resolvedPath).toBe(
"/Users/test/project-a/output/weather-dashboard.html",
);
expect(ranking.primaryCandidate?.allowed).toBe(true);
});
it("does not treat html tags as local paths", () => {
const candidates = extractToolCallCandidates(
{
@@ -211,15 +292,8 @@ describe("artifactPathPolicy", () => {
expect(ranking.primaryCandidate?.allowed).toBe(true);
expect(ranking.primaryCandidate?.resolvedPath).toBe(
"/Users/test/.goose/artifacts/button-interactions.html",
"/Users/test/button-interactions.html",
);
expect(
ranking.secondaryCandidates.some(
(candidate) =>
candidate.resolvedPath === "/Users/test/button-interactions.html" &&
candidate.allowed === false,
),
).toBe(true);
});
it("treats unix absolute paths outside the previous whitelist as absolute", () => {
@@ -0,0 +1,139 @@
type ToolNamePathCandidate = {
rawPath: string;
source: "arg_key" | "result_regex";
confidence: "high" | "low";
fromResultText: boolean;
};
const COMMAND_ARG_KEYS = ["cmd", "command"] as const;
function stripEnclosingQuotes(token: string): string {
const trimmed = token.trim();
if (!trimmed) return "";
if (
(trimmed.startsWith('"') && trimmed.endsWith('"')) ||
(trimmed.startsWith("'") && trimmed.endsWith("'")) ||
(trimmed.startsWith("`") && trimmed.endsWith("`"))
) {
return trimmed.slice(1, -1);
}
return trimmed;
}
function splitCommandTokens(command: string): string[] {
// Preserve quoted segments so paths with spaces stay intact.
const tokens = command.match(/"[^"]*"|'[^']*'|`[^`]*`|\S+/g);
return tokens ?? [];
}
function extractCommandPathCandidates(
command: string,
isLikelyLocalPath: (candidate: string) => boolean,
stripTokenPunctuation: (token: string) => string,
): string[] {
const tokens = splitCommandTokens(command);
if (tokens.length === 0) return [];
const candidates: string[] = [];
const pushIfLocalPath = (rawToken?: string) => {
if (!rawToken) return;
const normalized = stripTokenPunctuation(stripEnclosingQuotes(rawToken));
if (!isLikelyLocalPath(normalized)) return;
candidates.push(normalized);
};
for (let index = 0; index < tokens.length; index += 1) {
const token = tokens[index];
const normalizedToken = token.toLowerCase();
if (/^(?:\d?>|>>|\d?>>)$/.test(token)) {
pushIfLocalPath(tokens[index + 1]);
continue;
}
if (
normalizedToken === "-o" ||
normalizedToken === "--output" ||
normalizedToken === "--out" ||
normalizedToken === "--save" ||
normalizedToken === "--file" ||
normalizedToken === "--path" ||
normalizedToken === "--output-file"
) {
pushIfLocalPath(tokens[index + 1]);
continue;
}
const equalsMatch = token.match(
/^--(?:output|out|save|file|path|output-file)=(.+)$/i,
);
if (equalsMatch) {
pushIfLocalPath(equalsMatch[1]);
}
}
return candidates;
}
export function extractToolNamePathCandidates(
toolName: string,
isLikelyLocalPath: (candidate: string) => boolean,
stripTokenPunctuation: (token: string) => string,
): ToolNamePathCandidate[] {
const matches: ToolNamePathCandidate[] = [];
const trimmed = toolName.trim();
if (!trimmed) return matches;
const match = trimmed.match(/^(write|create|save)\s+(.+)$/i);
if (match) {
const remainder = match[2].trim();
if (remainder) {
const quoted = remainder.match(/^["'`](.+?)["'`]$/);
const token = quoted ? quoted[1] : remainder.split(/\s+/)[0];
const cleaned = stripTokenPunctuation(token);
if (isLikelyLocalPath(cleaned)) {
matches.push({
rawPath: cleaned,
source: "arg_key",
confidence: "high",
fromResultText: false,
});
}
}
}
for (const rawPath of extractCommandPathCandidates(
trimmed,
isLikelyLocalPath,
stripTokenPunctuation,
)) {
matches.push({
rawPath,
source: "result_regex",
confidence: "low",
fromResultText: false,
});
}
return matches;
}
export function collectCommandArgPathCandidates(
args: Record<string, unknown>,
isLikelyLocalPath: (candidate: string) => boolean,
stripTokenPunctuation: (token: string) => string,
): string[] {
const values: string[] = [];
for (const key of COMMAND_ARG_KEYS) {
const value = args[key];
if (typeof value !== "string" || !value.trim()) continue;
values.push(
...extractCommandPathCandidates(
value,
isLikelyLocalPath,
stripTokenPunctuation,
),
);
}
return values;
}
@@ -1,3 +1,8 @@
import {
collectCommandArgPathCandidates,
extractToolNamePathCandidates,
} from "@/features/chat/lib/artifactPathCommandExtraction";
export type ArtifactCandidateSource =
| "arg_key"
| "result_regex"
@@ -205,23 +210,6 @@ function extractResultPathCandidates(result: string): string[] {
return matches;
}
function extractToolNamePathCandidates(toolName: string): string[] {
const trimmed = toolName.trim();
if (!trimmed) return [];
const match = trimmed.match(/^(write|create|save)\s+(.+)$/i);
if (!match) return [];
const remainder = match[2].trim();
if (!remainder) return [];
const quoted = remainder.match(/^["'`](.+?)["'`]$/);
const token = quoted ? quoted[1] : remainder.split(/\s+/)[0];
const cleaned = stripTokenPunctuation(token);
if (!isLikelyLocalPath(cleaned)) return [];
return [cleaned];
}
function inferHomeDirFromRoots(allowedRoots: string[]): string | null {
for (const root of allowedRoots) {
const normalized = normalizePath(root);
@@ -290,7 +278,12 @@ export function resolvePathCandidate(
export function evaluatePathScope(
resolvedPath: string,
allowedRoots: string[],
options?: { allowOutsideRoots?: boolean },
): { allowed: boolean; blockedReason: string | null } {
if (options?.allowOutsideRoots) {
return { allowed: true, blockedReason: null };
}
const normalizedPath = normalizeComparablePath(resolvedPath);
const roots = allowedRoots
.map((root) => normalizeComparablePath(root))
@@ -306,6 +299,15 @@ export function evaluatePathScope(
};
}
function shouldAllowOutsideRootsForToolCandidate(
input: ToolCallArtifactInput,
candidate: {
fromResultText: boolean;
},
): boolean {
return isWriteOrientedTool(input.toolName) && candidate.fromResultText;
}
function collectArgPathCandidates(args: Record<string, unknown>): string[] {
const values: string[] = [];
for (const key of PATH_ARG_KEYS) {
@@ -361,6 +363,7 @@ export function extractToolCallCandidates(
rawPath: string;
source: ArtifactCandidateSource;
confidence: ArtifactCandidateConfidence;
fromResultText: boolean;
}> = [];
for (const rawPath of collectArgPathCandidates(input.args)) {
@@ -368,14 +371,28 @@ export function extractToolCallCandidates(
rawPath,
source: "arg_key",
confidence: "high",
fromResultText: false,
});
}
for (const rawPath of extractToolNamePathCandidates(input.toolName)) {
for (const candidate of extractToolNamePathCandidates(
input.toolName,
isLikelyLocalPath,
stripTokenPunctuation,
)) {
rawCandidates.push(candidate);
}
for (const rawPath of collectCommandArgPathCandidates(
input.args,
isLikelyLocalPath,
stripTokenPunctuation,
)) {
rawCandidates.push({
rawPath,
source: "arg_key",
confidence: "high",
source: "result_regex",
confidence: "low",
fromResultText: false,
});
}
@@ -389,15 +406,21 @@ export function extractToolCallCandidates(
rawPath,
source: "result_regex",
confidence: "low",
fromResultText: true,
});
}
}
return rawCandidates.map((raw, idx) => {
const resolvedPath = resolvePathCandidate(raw.rawPath, allowedRoots);
const allowOutsideRoots = shouldAllowOutsideRootsForToolCandidate(
input,
raw,
);
const { allowed, blockedReason } = evaluatePathScope(
resolvedPath,
allowedRoots,
{ allowOutsideRoots },
);
const appearanceIndex = startingAppearanceIndex + idx;
return {
+52 -10
View File
@@ -94,7 +94,7 @@ export function ToolCallCard({
const [openError, setOpenError] = useState<string | null>(null);
const elapsed = useElapsedTime(status);
const displayName = formatToolName(name);
const { resolveToolCardDisplay, openResolvedPath } =
const { resolveToolCardDisplay, pathExists, openResolvedPath } =
useArtifactPolicyContext();
const hasContent = Object.keys(args).length > 0 || result != null;
@@ -118,17 +118,53 @@ export function ToolCallCard({
const displayPath = (candidate: ArtifactPathCandidate): string =>
candidate.rawPath || candidate.resolvedPath;
const openCandidate = async (candidate: ArtifactPathCandidate) => {
if (!candidate.allowed) {
const openCandidate = async (
candidate: ArtifactPathCandidate,
options?: { allowFallback?: boolean },
) => {
const orderedCandidates = options?.allowFallback
? [
candidate,
...display.secondaryCandidates.filter(
(secondaryCandidate) => secondaryCandidate.id !== candidate.id,
),
]
: [candidate];
try {
setOpenError(null);
for (const orderedCandidate of orderedCandidates) {
const exists = await pathExists(orderedCandidate.resolvedPath);
if (orderedCandidate.allowed && exists) {
await openResolvedPath(orderedCandidate.resolvedPath);
return;
}
}
for (const orderedCandidate of orderedCandidates) {
const exists = await pathExists(orderedCandidate.resolvedPath);
if (exists && !orderedCandidate.allowed) {
setOpenError(
orderedCandidate.blockedReason ||
"Path is outside allowed project/artifacts roots.",
);
return;
}
}
const firstAllowedCandidate = orderedCandidates.find(
(orderedCandidate) => orderedCandidate.allowed,
);
if (firstAllowedCandidate) {
setOpenError(`File not found: ${firstAllowedCandidate.resolvedPath}`);
return;
}
setOpenError(
candidate.blockedReason ||
"Path is outside allowed project/artifacts roots.",
);
return;
}
try {
setOpenError(null);
await openResolvedPath(candidate.resolvedPath);
} catch (error) {
setOpenError(error instanceof Error ? error.message : String(error));
}
@@ -174,7 +210,9 @@ export function ToolCallCard({
type="button"
onClick={() => {
if (!display.primaryCandidate) return;
void openCandidate(display.primaryCandidate);
void openCandidate(display.primaryCandidate, {
allowFallback: true,
});
}}
className={cn(
"inline-flex max-w-full items-center gap-1.5 rounded-md border px-2.5 py-1 text-xs transition-colors",
@@ -227,7 +265,11 @@ export function ToolCallCard({
<div key={candidate.id} className="space-y-0.5">
<button
type="button"
onClick={() => void openCandidate(candidate)}
onClick={() =>
void openCandidate(candidate, {
allowFallback: false,
})
}
className={cn(
"inline-flex max-w-full items-center gap-1.5 rounded-md border px-2 py-1 text-[11px] transition-colors",
candidate.allowed
@@ -4,14 +4,19 @@ import userEvent from "@testing-library/user-event";
import { ArtifactPolicyProvider } from "../../hooks/ArtifactPolicyContext";
import { MarkdownContent } from "../MarkdownContent";
const { openPathMock } = vi.hoisted(() => ({
const { openPathMock, pathExistsMock } = vi.hoisted(() => ({
openPathMock: vi.fn(),
pathExistsMock: vi.fn().mockResolvedValue(true),
}));
vi.mock("@tauri-apps/plugin-opener", () => ({
openPath: openPathMock,
}));
vi.mock("@/shared/api/system", () => ({
pathExists: pathExistsMock,
}));
function renderWithPolicy(content: string) {
return render(
<ArtifactPolicyProvider
@@ -26,6 +31,8 @@ function renderWithPolicy(content: string) {
describe("MarkdownContent", () => {
afterEach(() => {
openPathMock.mockReset();
pathExistsMock.mockReset();
pathExistsMock.mockResolvedValue(true);
});
it("opens allowed local links through artifact policy", async () => {
@@ -5,18 +5,25 @@ import type { Message } from "@/shared/types/messages";
import { ArtifactPolicyProvider } from "../../hooks/ArtifactPolicyContext";
import { ToolCallCard } from "../ToolCallCard";
const { openPathMock } = vi.hoisted(() => ({
const { openPathMock, pathExistsMock } = vi.hoisted(() => ({
openPathMock: vi.fn(),
pathExistsMock: vi.fn().mockResolvedValue(true),
}));
vi.mock("@tauri-apps/plugin-opener", () => ({
openPath: openPathMock,
}));
vi.mock("@/shared/api/system", () => ({
pathExists: pathExistsMock,
}));
describe("ToolCallCard", () => {
afterEach(() => {
vi.useRealTimers();
openPathMock.mockReset();
pathExistsMock.mockReset();
pathExistsMock.mockResolvedValue(true);
});
it("renders tool name", () => {
@@ -159,6 +166,7 @@ describe("ToolCallCard", () => {
it("shows artifact open action only on the primary host tool card", async () => {
const user = userEvent.setup();
pathExistsMock.mockResolvedValue(true);
const readArgs = { path: "/Users/test/project-a/notes.md" };
const writeArgs = {
paths: [
@@ -219,6 +227,104 @@ describe("ToolCallCard", () => {
);
});
it("falls back to a later existing allowed candidate when the primary path is missing", async () => {
const user = userEvent.setup();
pathExistsMock.mockImplementation(async (path: string) => {
return path === "/Users/test/.goose/artifacts/index.html";
});
const writeArgs = {};
const messages: Message[] = [
{
id: "assistant-1",
role: "assistant",
created: Date.now(),
content: [
{
type: "toolRequest",
id: "tool-write",
name: "Write index.html",
arguments: writeArgs,
status: "completed",
},
{
type: "toolResponse",
id: "tool-write",
name: "Write index.html",
result: "/Users/test/.goose/artifacts/index.html (new)",
isError: false,
},
],
},
];
render(
<ArtifactPolicyProvider
messages={messages}
allowedRoots={["/Users/test/project-a", "/Users/test/.goose/artifacts"]}
>
<ToolCallCard
name="Write index.html"
arguments={writeArgs}
status="completed"
/>
</ArtifactPolicyProvider>,
);
await user.click(screen.getByText("Open file"));
expect(openPathMock).toHaveBeenCalledWith(
"/Users/test/.goose/artifacts/index.html",
);
});
it("opens explicit write outputs outside default roots", async () => {
const user = userEvent.setup();
pathExistsMock.mockImplementation(async (path: string) => {
return path === "/Users/test/coffee_shop_inventory.csv";
});
const writeArgs = {};
const messages: Message[] = [
{
id: "assistant-1",
role: "assistant",
created: Date.now(),
content: [
{
type: "toolRequest",
id: "tool-write",
name: "Write coffee_shop_inventory.csv",
arguments: writeArgs,
status: "completed",
},
{
type: "toolResponse",
id: "tool-write",
name: "Write coffee_shop_inventory.csv",
result: "/Users/test/coffee_shop_inventory.csv (new)",
isError: false,
},
],
},
];
render(
<ArtifactPolicyProvider
messages={messages}
allowedRoots={["/Users/test/.goose/artifacts"]}
>
<ToolCallCard
name="Write coffee_shop_inventory.csv"
arguments={writeArgs}
status="completed"
/>
</ArtifactPolicyProvider>,
);
await user.click(screen.getByText("Open file"));
expect(openPathMock).toHaveBeenCalledWith(
"/Users/test/coffee_shop_inventory.csv",
);
});
it("keeps secondary outputs collapsed until expanded", async () => {
const user = userEvent.setup();
const writeArgs = {
@@ -262,8 +368,18 @@ describe("ToolCallCard", () => {
expect(screen.getAllByText("Open file")).toHaveLength(2);
});
it("renders blocked candidate as disabled with reason", () => {
const writeArgs = { path: "/Users/test/outside/final_report.md" };
it("keeps a secondary open action scoped to the clicked candidate", async () => {
const user = userEvent.setup();
pathExistsMock.mockImplementation(async (path: string) => {
return path === "/Users/test/project-a/output/appendix.md";
});
const writeArgs = {
paths: [
"/Users/test/project-a/output/final_report.md",
"/Users/test/project-a/output/missing_notes.md",
"/Users/test/project-a/output/appendix.md",
],
};
const messages: Message[] = [
{
id: "assistant-1",
@@ -294,6 +410,51 @@ describe("ToolCallCard", () => {
</ArtifactPolicyProvider>,
);
await user.click(screen.getByText("More outputs (2)"));
await user.click(
screen.getByTitle("/Users/test/project-a/output/missing_notes.md"),
);
expect(openPathMock).not.toHaveBeenCalled();
expect(
screen.getByText(
"File not found: /Users/test/project-a/output/missing_notes.md",
),
).toBeInTheDocument();
});
it("keeps non-write candidates blocked outside allowed roots", () => {
const readArgs = { path: "/Users/test/outside/final_report.md" };
const messages: Message[] = [
{
id: "assistant-1",
role: "assistant",
created: Date.now(),
content: [
{
type: "toolRequest",
id: "tool-read",
name: "read_file",
arguments: readArgs,
status: "completed",
},
],
},
];
render(
<ArtifactPolicyProvider
messages={messages}
allowedRoots={["/Users/test/project-a", "/Users/test/.goose/artifacts"]}
>
<ToolCallCard
name="read_file"
arguments={readArgs}
status="completed"
/>
</ArtifactPolicyProvider>,
);
const openButton = screen.getByText("Open file").closest("button");
expect(openButton).toBeDisabled();
expect(
+4
View File
@@ -3,3 +3,7 @@ import { invoke } from "@tauri-apps/api/core";
export async function getHomeDir(): Promise<string> {
return invoke("get_home_dir");
}
export async function pathExists(path: string): Promise<boolean> {
return invoke("path_exists", { path });
}