Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/agent/posix-tool-plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ export function buildCorePosixToolPlugins(
// rebuilding the plugin stack. Secret-guard and authz still hard-deny
// regardless.
const allowOutside = (): boolean => permissionGate.getSkipPermissions();
// One shared workspace-roots provider for every bound in this stack, so
// pathEscape and delete_file admit the same registered sibling worktrees.
const rootsProvider = createWorktreeRootsProvider(cwd);
const truncationOptions =
getBlobWriter !== undefined ||
getContextDir !== undefined ||
Expand All @@ -112,9 +115,9 @@ export function buildCorePosixToolPlugins(
return [
resultTruncationPlugin(truncationOptions),
toolResultSecretScrubPlugin(),
pathEscapePlugin(cwd, createWorktreeRootsProvider(cwd), { allowOutside }),
pathEscapePlugin(cwd, rootsProvider, { allowOutside }),
evidenceArchivePathGuardPlugin(),
deleteFilePlugin(cwd, { allowOutside }),
deleteFilePlugin(cwd, { allowOutside, rootsProvider }),
toolOutputUriPlugin(),
secretGuardPlugin(),
authzPlugin(),
Expand Down
2 changes: 2 additions & 0 deletions src/agent/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { advertiseEditFileLineRange } from "../plugins/edit-file-line-range.js";
import { advertiseArchiveSurface } from "../plugins/evidence-archive-search-plugin.js";
import type { Telemetry } from "../telemetry/index.js";
import type { PermissionGate } from "../permission/gate.js";
import { createWorktreeRootsProvider } from "../permission/worktree-roots.js";
import { buildCorePosixToolPlugins } from "./posix-tool-plugins.js";
import { createLazyBlobReader } from "./lazy-blob-reader.js";
import type { BlobReader } from "@intx/types/runtime";
Expand Down Expand Up @@ -583,6 +584,7 @@ export async function createAgentToolset(
}),
createListDirTool(cwd, {
allowOutside: () => permissionGate.getSkipPermissions(),
rootsProvider: createWorktreeRootsProvider(cwd),
}),
createUseSkillTool(cwd, skillDirs, args.telemetry),
createSkillSearchTool({ skills }),
Expand Down
40 changes: 38 additions & 2 deletions src/list-dir.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { test, expect, describe } from "bun:test";
import { mkdtemp, mkdir, writeFile, symlink } from "node:fs/promises";
import { mkdtemp, mkdir, realpath, symlink, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { basename, join } from "node:path";
import { listDirectory } from "./util/list-dir.js";

async function fixture(): Promise<string> {
Expand Down Expand Up @@ -81,4 +81,40 @@ describe("listDirectory", () => {
expect(out.split("\n")).toContain("other.txt");
expect(out).not.toContain("outside the workspace");
});

test("lists a registered sibling worktree root (CL-6729)", async () => {
const dir = await fixture();
const sibling = await mkdtemp(join(tmpdir(), "list-dir-sibling-"));
await writeFile(join(sibling, "sibling-file.txt"), "");
const roots = [await realpath(sibling)];

const out = await listDirectory(dir, sibling, {
rootsProvider: () => roots,
});
expect(out.split("\n")).toContain("sibling-file.txt");
expect(out).not.toContain("outside the workspace");
});

test("lists a sibling worktree via relative traversal (CL-6729)", async () => {
const dir = await fixture();
const sibling = await mkdtemp(join(tmpdir(), "list-dir-sibling-rel-"));
await writeFile(join(sibling, "sibling-file.txt"), "");
const roots = [await realpath(sibling)];

const out = await listDirectory(dir, join("..", basename(sibling)), {
rootsProvider: () => roots,
});
expect(out.split("\n")).toContain("sibling-file.txt");
expect(out).not.toContain("outside the workspace");
});

test("lists through an aliased session root (CL-6729)", async () => {
const dir = await fixture();
const realDir = await realpath(dir);
const alias = `${realDir}-alias`;
await symlink(realDir, alias);

const out = await listDirectory(alias, realDir);
expect(out.split("\n")).toEqual(["a.ts", "b.ts", "sub/"]);
});
});
89 changes: 89 additions & 0 deletions src/plugins/delete-file-plugin.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import {
chmod,
lstat,
mkdtemp,
mkdir,
readFile,
realpath,
rm,
stat,
symlink,
Expand Down Expand Up @@ -30,6 +33,15 @@ async function exists(path: string): Promise<boolean> {
}
}

async function linkExists(path: string): Promise<boolean> {
try {
await lstat(path);
return true;
} catch {
return false;
}
}

describe("deleteFilePlugin", () => {
let cwd: string;

Expand Down Expand Up @@ -129,6 +141,43 @@ describe("deleteFilePlugin", () => {
await rm(outside, { recursive: true, force: true });
});

test("deletes a dangling symlink inside cwd (CL-6729)", async () => {
const link = join(cwd, "broken-link");
await symlink(join(cwd, "does-not-exist.txt"), link);
expect(await linkExists(link)).toBe(true);

const result = await handler()(
call("broken-link"),
new AbortController().signal,
);

expect(result.isError ?? false).toBe(false);
expect(String(result.content)).toContain("Deleted file: broken-link");
expect(await linkExists(link)).toBe(false);
});

test("deletes a link with an outside referent without touching the referent (CL-6729)", async () => {
const outside = await mkdtemp(
join(tmpdir(), "corbits-delete-link-referent-"),
);
const referent = join(outside, "keep.txt");
await writeFile(referent, "keep");
const link = join(cwd, "outside-link");
await symlink(referent, link);
expect(await linkExists(link)).toBe(true);

const result = await handler()(
call("outside-link"),
new AbortController().signal,
);

expect(result.isError ?? false).toBe(false);
expect(String(result.content)).toContain("Deleted file: outside-link");
expect(await linkExists(link)).toBe(false);
expect(await readFile(referent, "utf8")).toBe("keep");
await rm(outside, { recursive: true, force: true });
});

test("allowOutside deletes a file outside the working directory", async () => {
const outside = await mkdtemp(join(tmpdir(), "corbits-delete-yolo-"));
const path = join(outside, "gone.txt");
Expand Down Expand Up @@ -201,6 +250,46 @@ describe("deleteFilePlugin", () => {
expect(await exists(path)).toBe(true);
});

test("deletes a file in a registered sibling worktree (CL-6729)", async () => {
const sibling = await mkdtemp(join(tmpdir(), "corbits-delete-sibling-"));
const path = join(sibling, "old.txt");
await writeFile(path, "old");
const roots = [await realpath(sibling)];
const tool = deleteFilePlugin(cwd, { rootsProvider: () => roots })
.tools?.[0];
if (tool === undefined)
throw new Error("delete_file tool was not registered");

const result = await tool.handler(call(path), new AbortController().signal);

expect(result.isError ?? false).toBe(false);
expect(String(result.content)).toContain("Deleted file");
expect(await exists(path)).toBe(false);
await rm(sibling, { recursive: true, force: true });
});

test("still refuses a genuinely outside file when roots are registered (CL-6729)", async () => {
const sibling = await mkdtemp(
join(tmpdir(), "corbits-delete-sibling-keep-"),
);
const outside = await mkdtemp(join(tmpdir(), "corbits-delete-outside-"));
const path = join(outside, "keep.txt");
await writeFile(path, "keep");
const roots = [await realpath(sibling)];
const tool = deleteFilePlugin(cwd, { rootsProvider: () => roots })
.tools?.[0];
if (tool === undefined)
throw new Error("delete_file tool was not registered");

const result = await tool.handler(call(path), new AbortController().signal);

expect(result.isError).toBe(true);
expect(String(result.content)).toContain("outside the working directory");
expect(await exists(path)).toBe(true);
await rm(sibling, { recursive: true, force: true });
await rm(outside, { recursive: true, force: true });
});

test("preserves filesystem failure details", async () => {
const path = join(cwd, "locked.txt");
await writeFile(path, "keep");
Expand Down
47 changes: 27 additions & 20 deletions src/plugins/delete-file-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { lstat, readFile, realpath, unlink } from "node:fs/promises";
import { dirname, isAbsolute, relative, resolve, sep } from "node:path";
import { lstat, readFile, unlink } from "node:fs/promises";
import { dirname, resolve } from "node:path";
import { type } from "arktype";
import type { ExtraTool, ToolPlugin } from "@intx/tools-posix";
import type { ToolCall, ToolResult } from "@intx/types/runtime";
import { resolveWorkspacePath } from "../permission/path-restriction.js";
import type { RootsProvider } from "../permission/worktree-roots.js";
import { formatChangeDiff } from "./change-diff.js";

const DeleteFileArgs = type({ path: "string>0" });
Expand Down Expand Up @@ -43,19 +45,14 @@ function failureDetail(error: unknown): string {
return code === undefined ? error.message : `${code}: ${error.message}`;
}

function isWithin(root: string, path: string): boolean {
const rel = relative(root, path);
return (
rel === "" ||
(rel !== ".." && !rel.startsWith(`..${sep}`) && !isAbsolute(rel))
);
}

export interface DeleteFilePluginOptions {
// When true (yolo / --dangerously-skip-permissions), delete outside the
// working directory. A getter is resolved per call so `/yolo` mid-session
// takes effect without rebuilding the plugin stack.
allowOutside?: boolean | (() => boolean);
// Workspace roots beyond cwd (the session's registered git worktrees).
// Defaults to cwd alone.
rootsProvider?: RootsProvider;
}

function resolveAllowOutside(
Expand Down Expand Up @@ -84,18 +81,28 @@ export function deleteFilePlugin(
}

const allowOutside = resolveAllowOutside(options.allowOutside);
// Containment is keyed off the parent directory, not the full target:
// lstat/unlink never follow the final component, so unlinking a link
// itself cannot escape even when the link dangles or points outside.
// Resolving the full target would refuse both (UNRESOLVABLE / outside
// referent). The shared workspace resolver still realpaths the session
// root before comparing and admits registered sibling worktree roots —
// the same boundary pathEscapePlugin enforces.
const target = resolve(cwd, args.path);
if (
!allowOutside &&
resolveWorkspacePath(
cwd,
dirname(target),
options.rootsProvider ?? (() => []),
) === undefined
) {
return errorResult(
call.id,
`${args.path} resolves outside the working directory`,
);
}
try {
const [physicalRoot, physicalParent] = await Promise.all([
realpath(cwd),
realpath(dirname(target)),
]);
if (!allowOutside && !isWithin(physicalRoot, physicalParent)) {
return errorResult(
call.id,
`${args.path} resolves outside the working directory`,
);
}
const info = await lstat(target);
if (info.isDirectory()) {
return errorResult(
Expand Down
46 changes: 24 additions & 22 deletions src/util/list-dir.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { type } from "arktype";
import { readdir, realpath } from "node:fs/promises";
import { resolve, sep } from "node:path";
import { resolve } from "node:path";
import { stringTool } from "@intx/agent";
import type { AgentTool } from "@intx/agent";
import type { ToolDefinition } from "@intx/types/runtime";
import { resolveWorkspacePath } from "../permission/path-restriction.js";
import type { RootsProvider } from "../permission/worktree-roots.js";

const ListDirArgs = type({ "path?": "string" });

Expand Down Expand Up @@ -31,6 +33,9 @@ export interface ListDirectoryOptions {
// workspace. A getter is resolved per call so `/yolo` mid-session takes
// effect without rebuilding the tool.
allowOutside?: boolean | (() => boolean);
// Workspace roots beyond cwd (the session's registered git worktrees).
// Defaults to cwd alone.
rootsProvider?: RootsProvider;
}

function resolveAllowOutside(
Expand All @@ -47,29 +52,26 @@ export async function listDirectory(
): Promise<string> {
const allowOutside = resolveAllowOutside(options.allowOutside);
const rel = path.length > 0 ? path : ".";
const abs = resolve(cwd, rel);
if (!allowOutside && abs !== cwd && !abs.startsWith(cwd + sep)) {
return `Error: ${rel} is outside the workspace.`;
}
const rootsProvider = options.rootsProvider ?? (() => []);

// A symlink inside the workspace can resolve to a target outside it; the
// string prefix check above only sees the lexical path. Resolve the real path
// of both the target and the root before comparing so symlink escapes are
// refused (unless allowOutside, which is the yolo-mode escape hatch).
// Containment is delegated to the shared workspace resolver: it realpaths
// the session root before comparing (so an aliased cwd such as macOS
// /tmp -> /private/tmp never false-denies) and admits registered sibling
// worktree roots. The canonical path feeds readdir directly, so a symlink
// retargeted after the check cannot redirect the read.
let realAbs: string;
let realCwd: string;
try {
realAbs = await realpath(abs);
realCwd = await realpath(cwd);
} catch (err) {
return `Error: cannot list ${rel}: ${err instanceof Error ? err.message : String(err)}`;
}
if (
!allowOutside &&
realAbs !== realCwd &&
!realAbs.startsWith(realCwd + sep)
) {
return `Error: ${rel} is outside the workspace.`;
if (allowOutside) {
try {
realAbs = await realpath(resolve(cwd, rel));
} catch (err) {
return `Error: cannot list ${rel}: ${err instanceof Error ? err.message : String(err)}`;
}
} else {
const resolved = resolveWorkspacePath(cwd, rel, rootsProvider);
if (resolved === undefined) {
return `Error: ${rel} is outside the workspace.`;
}
realAbs = resolved;
}

let entries;
Expand Down
Loading