Gateway: open config files without shell interpolation (#57921)

* Gateway: open config files without shell interpolation

Co-authored-by: peteryuqin <peter.yuqin@gmail.com>

* Gateway: align config opener review fixes

* Gateway: tidy config opener logging

* Gateway: simplify config opener error path

* Gateway: cover Windows config opener test path

* Gateway: use literal Windows config open path

---------

Co-authored-by: peteryuqin <peter.yuqin@gmail.com>

src/gateway/server-methods/config.test.ts

@@ -0,0 +1,119 @@
+import { execFile } from "node:child_process";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { configHandlers, resolveConfigOpenCommand } from "./config.js";
+import type { GatewayRequestHandlerOptions } from "./types.js";
+
+vi.mock("node:child_process", () => ({
+  execFile: vi.fn(),
+}));
+
+function invokeExecFileCallback(args: unknown[], error: Error | null) {
+  const callback = args.at(-1);
+  expect(callback).toEqual(expect.any(Function));
+  (callback as (error: Error | null) => void)(error);
+}
+
+function createOptions(
+  overrides?: Partial<GatewayRequestHandlerOptions>,
+): GatewayRequestHandlerOptions {
+  return {
+    req: { type: "req", id: "1", method: "config.openFile" },
+    params: {},
+    client: null,
+    isWebchatConnect: () => false,
+    respond: vi.fn(),
+    context: {
+      logGateway: {
+        error: vi.fn(),
+        warn: vi.fn(),
+        info: vi.fn(),
+        debug: vi.fn(),
+      },
+    },
+    ...overrides,
+  } as unknown as GatewayRequestHandlerOptions;
+}
+
+describe("resolveConfigOpenCommand", () => {
+  it("uses open on macOS", () => {
+    expect(resolveConfigOpenCommand("/tmp/openclaw.json", "darwin")).toEqual({
+      command: "open",
+      args: ["/tmp/openclaw.json"],
+    });
+  });
+
+  it("uses xdg-open on Linux", () => {
+    expect(resolveConfigOpenCommand("/tmp/openclaw.json", "linux")).toEqual({
+      command: "xdg-open",
+      args: ["/tmp/openclaw.json"],
+    });
+  });
+
+  it("uses a quoted PowerShell literal on Windows", () => {
+    expect(resolveConfigOpenCommand(String.raw`C:\tmp\o'hai & calc.json`, "win32")).toEqual({
+      command: "powershell.exe",
+      args: [
+        "-NoProfile",
+        "-NonInteractive",
+        "-Command",
+        String.raw`Start-Process -LiteralPath 'C:\tmp\o''hai & calc.json'`,
+      ],
+    });
+  });
+});
+
+describe("config.openFile", () => {
+  afterEach(() => {
+    delete process.env.OPENCLAW_CONFIG_PATH;
+    vi.clearAllMocks();
+  });
+
+  it("opens the configured file without shell interpolation", async () => {
+    process.env.OPENCLAW_CONFIG_PATH = "/tmp/config $(touch pwned).json";
+    vi.mocked(execFile).mockImplementation(((...args: unknown[]) => {
+      expect(["open", "xdg-open", "powershell.exe"]).toContain(args[0]);
+      expect(args[1]).toEqual(["/tmp/config $(touch pwned).json"]);
+      invokeExecFileCallback(args, null);
+      return {} as never;
+    }) as unknown as typeof execFile);
+
+    const opts = createOptions();
+    await configHandlers["config.openFile"](opts);
+
+    expect(opts.respond).toHaveBeenCalledWith(
+      true,
+      {
+        ok: true,
+        path: "/tmp/config $(touch pwned).json",
+      },
+      undefined,
+    );
+  });
+
+  it("returns a generic error and logs details when the opener fails", async () => {
+    process.env.OPENCLAW_CONFIG_PATH = "/tmp/config.json";
+    vi.mocked(execFile).mockImplementation(((...args: unknown[]) => {
+      invokeExecFileCallback(
+        args,
+        Object.assign(new Error("spawn xdg-open ENOENT"), { code: "ENOENT" }),
+      );
+      return {} as never;
+    }) as unknown as typeof execFile);
+
+    const opts = createOptions();
+    await configHandlers["config.openFile"](opts);
+
+    expect(opts.respond).toHaveBeenCalledWith(
+      true,
+      {
+        ok: false,
+        path: "/tmp/config.json",
+        error: "failed to open config file",
+      },
+      undefined,
+    );
+    expect(opts.context.logGateway.warn).toHaveBeenCalledWith(
+      expect.stringContaining("spawn xdg-open ENOENT"),
+    );
+  });
+});

src/gateway/server-methods/config.ts

@@ -1,4 +1,4 @@
-import { exec } from "node:child_process";
+import { execFile } from "node:child_process";
 import {
   createConfigIO,
   parseConfigJson5,
@@ -51,6 +51,11 @@ import { assertValidParams } from "./validation.js";
 
 const MAX_CONFIG_ISSUES_IN_ERROR_MESSAGE = 3;
 
+type ConfigOpenCommand = {
+  command: string;
+  args: string[];
+};
+
 function requireConfigBaseHash(
   params: unknown,
   snapshot: Awaited<ReturnType<typeof readConfigFileSnapshot>>,
@@ -125,6 +130,56 @@ function sanitizeLookupPathForLog(path: string): string {
   return sanitized.length > 120 ? `${sanitized.slice(0, 117)}...` : sanitized;
 }
 
+function escapePowerShellSingleQuotedString(value: string): string {
+  return value.replaceAll("'", "''");
+}
+
+export function resolveConfigOpenCommand(
+  configPath: string,
+  platform: NodeJS.Platform = process.platform,
+): ConfigOpenCommand {
+  if (platform === "win32") {
+    // Use a PowerShell string literal so the path stays data, not code.
+    return {
+      command: "powershell.exe",
+      args: [
+        "-NoProfile",
+        "-NonInteractive",
+        "-Command",
+        `Start-Process -LiteralPath '${escapePowerShellSingleQuotedString(configPath)}'`,
+      ],
+    };
+  }
+  return {
+    command: platform === "darwin" ? "open" : "xdg-open",
+    args: [configPath],
+  };
+}
+
+function execConfigOpenCommand(command: ConfigOpenCommand): Promise<void> {
+  return new Promise((resolve, reject) => {
+    execFile(command.command, command.args, (error) => {
+      if (error) {
+        reject(error);
+        return;
+      }
+      resolve();
+    });
+  });
+}
+
+function formatConfigOpenError(error: unknown): string {
+  if (
+    typeof error === "object" &&
+    error &&
+    "message" in error &&
+    typeof error.message === "string"
+  ) {
+    return error.message;
+  }
+  return String(error);
+}
+
 function parseValidateConfigFromRawOrRespond(
   params: unknown,
   requestName: string,
@@ -496,19 +551,23 @@ export const configHandlers: GatewayRequestHandlers = {
       undefined,
     );
   },
-  "config.openFile": ({ params, respond }) => {
+  "config.openFile": async ({ params, respond, context }) => {
     if (!assertValidParams(params, validateConfigGetParams, "config.openFile", respond)) {
       return;
     }
     const configPath = createConfigIO().configPath;
-    const platform = process.platform;
-    const cmd = platform === "darwin" ? "open" : platform === "win32" ? "start" : "xdg-open";
-    exec(`${cmd} ${JSON.stringify(configPath)}`, (err) => {
-      if (err) {
-        respond(true, { ok: false, path: configPath, error: err.message }, undefined);
-        return;
-      }
+    try {
+      await execConfigOpenCommand(resolveConfigOpenCommand(configPath));
       respond(true, { ok: true, path: configPath }, undefined);
-    });
+    } catch (error) {
+      context?.logGateway?.warn(
+        `config.openFile failed path=${sanitizeLookupPathForLog(configPath)}: ${formatConfigOpenError(error)}`,
+      );
+      respond(
+        true,
+        { ok: false, path: configPath, error: "failed to open config file" },
+        undefined,
+      );
+    }
   },
 };