Fix socket firewall for windows (#3172)

src/ipc/processors/executeAddDependency.test.ts

@@ -365,6 +365,25 @@ describe("executeAddDependency", () => {
     });
   });
 
+  it("rejects invalid npm package specs before invoking the shell", async () => {
+    await expect(
+      executeAddDependency({
+        packages: ["react@^18.0.0"],
+        message: {
+          id: 1,
+          content:
+            '<dyad-add-dependency packages="react@^18.0.0"></dyad-add-dependency>',
+        } as any,
+        appPath: "/tmp/app",
+      }),
+    ).rejects.toMatchObject({
+      displaySummary: "Invalid npm package name: react@^18.0.0",
+      warningMessages: [],
+    });
+
+    expect(runCommandMock).not.toHaveBeenCalled();
+  });
+
   it("escapes package attributes and install output before storing the tag", async () => {
     ensureSocketFirewallInstalledMock.mockResolvedValue({
       available: false,
@@ -376,18 +395,18 @@ describe("executeAddDependency", () => {
     });
 
     await executeAddDependency({
-      packages: ['react"&<safe>'],
+      packages: ["react-safe"],
       message: {
         id: 1,
         content:
-          '<dyad-add-dependency packages="react&quot;&amp;&lt;safe&gt;"></dyad-add-dependency>',
+          '<dyad-add-dependency packages="react-safe"></dyad-add-dependency>',
       } as any,
       appPath: "/tmp/app",
     });
 
     expect(dbUpdateSetMock).toHaveBeenCalledWith({
       content:
-        '<dyad-add-dependency packages="react&quot;&amp;&lt;safe&gt;">installed &lt;react&gt;</dyad-add-dependency>',
+        '<dyad-add-dependency packages="react-safe">installed &lt;react&gt;</dyad-add-dependency>',
     });
   });
 });

src/ipc/processors/executeAddDependency.ts

@@ -3,6 +3,7 @@ import { messages } from "../../db/schema";
 import { eq } from "drizzle-orm";
 import { Message } from "@/ipc/types";
 import { readEffectiveSettings } from "@/main/settings";
+import { DyadError, DyadErrorKind } from "@/errors/dyad_error";
 import {
   ADD_DEPENDENCY_INSTALL_TIMEOUT_MS,
   buildAddDependencyCommand,
@@ -30,6 +31,8 @@ export interface ExecuteAddDependencyResult {
   warningMessages: string[];
 }
 
+const NPM_PACKAGE_NAME_PATTERN = /^(@[a-z0-9-_.]+\/)?[a-z0-9-_.]+$/;
+
 const DISPLAY_SUMMARY_PATTERNS = [
   /\bblocked\b/i,
   /\bfailed\b/i,
@@ -161,6 +164,19 @@ export async function executeAddDependency({
   message: Message;
   appPath: string;
 }): Promise<ExecuteAddDependencyResult> {
+  const invalidPackage = packages.find(
+    (pkg) => !NPM_PACKAGE_NAME_PATTERN.test(pkg),
+  );
+  if (invalidPackage) {
+    throw new ExecuteAddDependencyError({
+      error: new DyadError(
+        `Invalid npm package name: ${invalidPackage}`,
+        DyadErrorKind.Validation,
+      ),
+      warningMessages: [],
+    });
+  }
+
   const settings = await readEffectiveSettings();
   const warningMessages: string[] = [];
 

src/ipc/utils/socket_firewall.test.ts

@@ -185,9 +185,53 @@ describe("resolveExecutableName", () => {
 
 describe("buildPtyInvocation", () => {
   it("wraps Windows .cmd shims through cmd.exe for PTY execution", () => {
-    expect(buildPtyInvocation("npx", ["--yes", "sfw@2.0.4"], "win32")).toEqual({
+    expect(
+      buildPtyInvocation("npx", ["--yes", "sfw@2.0.4"], "win32", "cmd.exe"),
+    ).toEqual({
       command: "cmd.exe",
-      args: ["/d", "/s", "/c", '"npx.cmd" "--yes" "sfw@2.0.4"'],
+      args: ["/d", "/s", "/c", "npx.cmd --yes sfw@2.0.4"],
+    });
+  });
+
+  it("quotes Windows arguments containing spaces and embedded quotes", () => {
+    expect(
+      buildPtyInvocation(
+        "npx",
+        ["--message", 'value with spaces and "quotes"'],
+        "win32",
+        "cmd.exe",
+      ),
+    ).toEqual({
+      command: "cmd.exe",
+      args: [
+        "/d",
+        "/s",
+        "/c",
+        'npx.cmd --message "value with spaces and ""quotes"""',
+      ],
+    });
+  });
+
+  it("quotes Windows arguments containing cmd metacharacters without mutating them", () => {
+    expect(
+      buildPtyInvocation(
+        "npx",
+        ["--filter", "name&echo^(injected)"],
+        "win32",
+        "cmd.exe",
+      ),
+    ).toEqual({
+      command: "cmd.exe",
+      args: ["/d", "/s", "/c", 'npx.cmd --filter "name&echo^(injected)"'],
+    });
+  });
+
+  it("quotes empty Windows arguments so their position is preserved", () => {
+    expect(
+      buildPtyInvocation("npx", ["--flag", ""], "win32", "cmd.exe"),
+    ).toEqual({
+      command: "cmd.exe",
+      args: ["/d", "/s", "/c", 'npx.cmd --flag ""'],
     });
   });
 
@@ -220,7 +264,7 @@ describe("runCommand", () => {
 
       expect(runPtyCommandMock).toHaveBeenCalledWith(
         "cmd.exe",
-        ["/d", "/s", "/c", '"npx.cmd" "--yes" "sfw@2.0.4"'],
+        ["/d", "/s", "/c", "npx.cmd --yes sfw@2.0.4"],
         expect.objectContaining({
           displayCommand: "npx --yes sfw@2.0.4",
         }),

src/ipc/utils/socket_firewall.ts

@@ -13,6 +13,7 @@ const SOCKET_FIREWALL_NPX_ARGS = [
   SOCKET_FIREWALL_PACKAGE,
 ];
 const WINDOWS_BATCH_COMMAND_PATTERN = /\.(cmd|bat)$/i;
+const WINDOWS_CMD_NEEDS_QUOTING_PATTERN = /[\s"&|<>^%!()]/u;
 export const SOCKET_FIREWALL_PROBE_TIMEOUT_MS = 30 * 1000;
 export const PACKAGE_MANAGER_PROBE_TIMEOUT_MS = 30 * 1000;
 export const ADD_DEPENDENCY_INSTALL_TIMEOUT_MS = DEFAULT_PTY_COMMAND_TIMEOUT_MS;
@@ -75,6 +76,12 @@ export function resolveExecutableName(
 }
 
 function quoteWindowsCmdArg(value: string): string {
+  // `cmd.exe /d /s /c` strips an outer quoted command string, so simple args
+  // stay unquoted while empty or shell-significant values are quoted/escaped.
+  if (value !== "" && !WINDOWS_CMD_NEEDS_QUOTING_PATTERN.test(value)) {
+    return value;
+  }
+
   return `"${value.replace(/"/g, '""')}"`;
 }