Skip to content

Commit 02a9ec7

Browse files
committed
Key delete_file containment off the parent directory
1 parent 6def6e3 commit 02a9ec7

2 files changed

Lines changed: 58 additions & 6 deletions

File tree

src/plugins/delete-file-plugin.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
22
import {
33
chmod,
4+
lstat,
45
mkdtemp,
56
mkdir,
7+
readFile,
68
realpath,
79
rm,
810
stat,
@@ -31,6 +33,15 @@ async function exists(path: string): Promise<boolean> {
3133
}
3234
}
3335

36+
async function linkExists(path: string): Promise<boolean> {
37+
try {
38+
await lstat(path);
39+
return true;
40+
} catch {
41+
return false;
42+
}
43+
}
44+
3445
describe("deleteFilePlugin", () => {
3546
let cwd: string;
3647

@@ -130,6 +141,43 @@ describe("deleteFilePlugin", () => {
130141
await rm(outside, { recursive: true, force: true });
131142
});
132143

144+
test("deletes a dangling symlink inside cwd (CL-6729)", async () => {
145+
const link = join(cwd, "broken-link");
146+
await symlink(join(cwd, "does-not-exist.txt"), link);
147+
expect(await linkExists(link)).toBe(true);
148+
149+
const result = await handler()(
150+
call("broken-link"),
151+
new AbortController().signal,
152+
);
153+
154+
expect(result.isError ?? false).toBe(false);
155+
expect(String(result.content)).toContain("Deleted file: broken-link");
156+
expect(await linkExists(link)).toBe(false);
157+
});
158+
159+
test("deletes a link with an outside referent without touching the referent (CL-6729)", async () => {
160+
const outside = await mkdtemp(
161+
join(tmpdir(), "corbits-delete-link-referent-"),
162+
);
163+
const referent = join(outside, "keep.txt");
164+
await writeFile(referent, "keep");
165+
const link = join(cwd, "outside-link");
166+
await symlink(referent, link);
167+
expect(await linkExists(link)).toBe(true);
168+
169+
const result = await handler()(
170+
call("outside-link"),
171+
new AbortController().signal,
172+
);
173+
174+
expect(result.isError ?? false).toBe(false);
175+
expect(String(result.content)).toContain("Deleted file: outside-link");
176+
expect(await linkExists(link)).toBe(false);
177+
expect(await readFile(referent, "utf8")).toBe("keep");
178+
await rm(outside, { recursive: true, force: true });
179+
});
180+
133181
test("allowOutside deletes a file outside the working directory", async () => {
134182
const outside = await mkdtemp(join(tmpdir(), "corbits-delete-yolo-"));
135183
const path = join(outside, "gone.txt");

src/plugins/delete-file-plugin.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { lstat, readFile, unlink } from "node:fs/promises";
2-
import { resolve } from "node:path";
2+
import { dirname, resolve } from "node:path";
33
import { type } from "arktype";
44
import type { ExtraTool, ToolPlugin } from "@intx/tools-posix";
55
import type { ToolCall, ToolResult } from "@intx/types/runtime";
@@ -81,14 +81,19 @@ export function deleteFilePlugin(
8181
}
8282

8383
const allowOutside = resolveAllowOutside(options.allowOutside);
84-
// Containment is delegated to the shared workspace resolver, which
85-
// realpaths the session root before comparing and admits registered
86-
// sibling worktree roots — the same boundary pathEscapePlugin enforces.
84+
// Containment is keyed off the parent directory, not the full target:
85+
// lstat/unlink never follow the final component, so unlinking a link
86+
// itself cannot escape even when the link dangles or points outside.
87+
// Resolving the full target would refuse both (UNRESOLVABLE / outside
88+
// referent). The shared workspace resolver still realpaths the session
89+
// root before comparing and admits registered sibling worktree roots —
90+
// the same boundary pathEscapePlugin enforces.
91+
const target = resolve(cwd, args.path);
8792
if (
8893
!allowOutside &&
8994
resolveWorkspacePath(
9095
cwd,
91-
args.path,
96+
dirname(target),
9297
options.rootsProvider ?? (() => []),
9398
) === undefined
9499
) {
@@ -97,7 +102,6 @@ export function deleteFilePlugin(
97102
`${args.path} resolves outside the working directory`,
98103
);
99104
}
100-
const target = resolve(cwd, args.path);
101105
try {
102106
const info = await lstat(target);
103107
if (info.isDirectory()) {

0 commit comments

Comments
 (0)