Unverified 提交 7b3f0852 authored 作者: Will Chen's avatar Will Chen 提交者: GitHub

fix: block unsafe root-like delete_file paths (#2859)

## Summary - Reject blank and whitespace-only `delete_file` paths at schema level. - Add runtime guard to refuse project-root-equivalent delete targets (like `.`, `./`, `.\\`, or normalized `foo/..`). - Add unit tests covering root-path rejection and normal file/directory delete behavior. ## Test plan - [x] `npm test -- src/pro/main/ipc/handlers/local_agent/tools/delete_file.spec.ts` - [x] `npm run fmt && npm run lint:fix && npm run ts` Made with [Cursor](https://cursor.com) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2859" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end -->
上级 1606ae9c
import { describe, it, expect, vi, beforeEach } from "vitest";
import fs from "node:fs";
import { deleteFileTool } from "./delete_file";
import type { AgentContext } from "./types";
import { gitRemove } from "@/ipc/utils/git_utils";
vi.mock("node:fs", async () => {
const actual = await vi.importActual<typeof import("node:fs")>("node:fs");
return {
...actual,
default: {
existsSync: vi.fn(),
lstatSync: vi.fn(),
rmdirSync: vi.fn(),
unlinkSync: vi.fn(),
},
};
});
vi.mock("electron-log", () => ({
default: {
scope: () => ({
log: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
}),
},
}));
vi.mock("@/ipc/utils/git_utils", () => ({
gitRemove: vi.fn().mockResolvedValue(undefined),
}));
vi.mock("../../../../../../supabase_admin/supabase_management_client", () => ({
deleteSupabaseFunction: vi.fn().mockResolvedValue(undefined),
}));
describe("deleteFileTool", () => {
const mockContext: AgentContext = {
event: {} as any,
appId: 1,
appPath: "/test/app",
chatId: 1,
supabaseProjectId: null,
supabaseOrganizationSlug: null,
messageId: 1,
isSharedModulesChanged: false,
isDyadPro: false,
todos: [],
dyadRequestId: "test-request",
fileEditTracker: {},
onXmlStream: vi.fn(),
onXmlComplete: vi.fn(),
requireConsent: vi.fn().mockResolvedValue(true),
appendUserMessage: vi.fn(),
onUpdateTodos: vi.fn(),
};
beforeEach(() => {
vi.clearAllMocks();
});
describe("schema validation", () => {
it("rejects empty path", () => {
const schema = deleteFileTool.inputSchema;
expect(() => schema.parse({ path: "" })).toThrow("Path cannot be empty");
});
it("rejects whitespace-only path", () => {
const schema = deleteFileTool.inputSchema;
expect(() => schema.parse({ path: " " })).toThrow(
"Path cannot be empty",
);
});
});
describe("execute safety checks", () => {
it.each([".", "./", ".\\", "foo/..", "foo\\.."])(
"rejects project-root-equivalent path: %s",
async (path) => {
await expect(
deleteFileTool.execute({ path }, mockContext),
).rejects.toThrow(/Refusing to delete project root/);
expect(fs.existsSync).not.toHaveBeenCalled();
expect(fs.unlinkSync).not.toHaveBeenCalled();
expect(fs.rmdirSync).not.toHaveBeenCalled();
expect(gitRemove).not.toHaveBeenCalled();
},
);
});
describe("execute delete behavior", () => {
it("deletes files with unlink and removes from git", async () => {
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.lstatSync).mockReturnValue({
isDirectory: () => false,
} as any);
const result = await deleteFileTool.execute(
{ path: "src/file.ts" },
mockContext,
);
expect(fs.unlinkSync).toHaveBeenCalledWith("/test/app/src/file.ts");
expect(fs.rmdirSync).not.toHaveBeenCalled();
expect(gitRemove).toHaveBeenCalledWith({
path: "/test/app",
filepath: "src/file.ts",
});
expect(result).toBe("Successfully deleted src/file.ts");
});
it("deletes directories with rmdir recursive", async () => {
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.lstatSync).mockReturnValue({
isDirectory: () => true,
} as any);
const result = await deleteFileTool.execute(
{ path: "src/dir" },
mockContext,
);
expect(fs.rmdirSync).toHaveBeenCalledWith("/test/app/src/dir", {
recursive: true,
});
expect(fs.unlinkSync).not.toHaveBeenCalled();
expect(result).toBe("Successfully deleted src/dir");
});
});
describe("buildXml", () => {
it("returns undefined for blank path", () => {
const result = deleteFileTool.buildXml?.({ path: " " }, false);
expect(result).toBeUndefined();
});
});
});
......@@ -18,7 +18,12 @@ function getFunctionNameFromPath(input: string): string {
}
const deleteFileSchema = z.object({
path: z.string().describe("The file path to delete"),
path: z
.string()
.refine((value) => value.trim().length > 0, {
message: "Path cannot be empty",
})
.describe("The file path to delete"),
});
export const deleteFileTool: ToolDefinition<z.infer<typeof deleteFileSchema>> =
......@@ -32,11 +37,24 @@ export const deleteFileTool: ToolDefinition<z.infer<typeof deleteFileSchema>> =
getConsentPreview: (args) => `Delete ${args.path}`,
buildXml: (args, _isComplete) => {
if (!args.path) return undefined;
if (!args.path?.trim()) return undefined;
return `<dyad-delete path="${escapeXmlAttr(args.path)}"></dyad-delete>`;
},
execute: async (args, ctx: AgentContext) => {
const normalizedPath = path.posix.normalize(
args.path.replace(/\\/g, "/"),
);
if (
normalizedPath === "." ||
normalizedPath === "./" ||
normalizedPath === ""
) {
throw new Error(
`Refusing to delete project root for path: "${args.path}"`,
);
}
const fullFilePath = safeJoin(ctx.appPath, args.path);
// Track if this is a shared module
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论