fix: block unsafe root-like delete_file paths

rules/git-workflow.md

@@ -66,6 +66,10 @@ gh api graphql --input .claude/tmp/resolve_thread.json
 gh api repos/dyad-sh/dyad/issues/{PR_NUMBER}/labels -f "labels[]=label-name"
 ```
 
+## Removing labels with special characters
+
+The security hook blocks `gh api` DELETE calls when the URL path contains colons (e.g., `needs-human:review-issue`) or URL-encoded colons (`%3A`). Both literal colons and percent-encoded forms trigger the "shell metacharacters" block. Currently there is no workaround within the sandbox — skip these label removals if they are non-critical.
+
 ## CI file access (claude-code-action)
 
 In CI, `claude-code-action` restricts file access to the repo working directory (e.g., `/home/runner/work/dyad/dyad`). Skills that save intermediate files (like PR diffs) must use `./filename` (current working directory), **never** `/tmp/`. Using `/tmp/` causes errors like: `cat in '/tmp/pr_*_diff.patch' was blocked. For security, Claude Code may only concatenate files from the allowed working directories`.

src/pro/main/ipc/handlers/local_agent/tools/delete_file.spec.ts

@@ -0,0 +1,140 @@
+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();
+    });
+  });
+});

src/pro/main/ipc/handlers/local_agent/tools/delete_file.ts

@@ -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