fix: [MSRC] [144619]

src/cli/commands/start/start.spec.ts

@@ -0,0 +1,135 @@
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import type { ConcurrentlyCommandInput, ConcurrentlyOptions, ConcurrentlyResult } from "concurrently";
+import { EventEmitter } from "node:events";
+
+const concurrentlyMock = vi.fn();
+const spawnMock = vi.fn();
+
+vi.mock("concurrently", () => ({
+  concurrently: concurrentlyMock,
+}));
+
+vi.mock("node:child_process", () => ({
+  spawn: spawnMock,
+}));
+
+vi.mock("../../../core/utils/logger.js", () => ({
+  logger: {
+    silly: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    log: vi.fn(),
+    error: vi.fn(),
+  },
+}));
+
+vi.mock("../../../core/utils/net.js", () => ({
+  isAcceptingTcpConnections: vi.fn().mockResolvedValue(4280),
+  askNewPort: vi.fn(),
+  parseUrl: vi.fn(),
+}));
+
+vi.mock("../../../core/func-core-tools.js", () => ({
+  getCoreToolsBinary: vi.fn().mockResolvedValue("func"),
+  getNodeMajorVersion: vi.fn().mockReturnValue(20),
+  detectTargetCoreToolsVersion: vi.fn().mockReturnValue(4),
+  isCoreToolsVersionCompatible: vi.fn().mockReturnValue(true),
+}));
+
+vi.mock("../../../core/dataApiBuilder/index.js", () => ({
+  getDataApiBuilderBinaryPath: vi.fn(),
+}));
+
+vi.mock("../../../core/utils/json.js", () => ({
+  loadPackageJson: vi.fn().mockReturnValue({ version: "test" }),
+}));
+
+function createFakeChildProcess() {
+  const child = new EventEmitter() as any;
+  child.pid = 12345;
+  child.stdin = null;
+  child.stdout = null;
+  child.stderr = null;
+  return child;
+}
+
+describe("swa start command execution safety", () => {
+  let tempRoot: string;
+  let originalExit: typeof process.exit;
+
+  beforeEach(() => {
+    concurrentlyMock.mockReset();
+    spawnMock.mockReset();
+    spawnMock.mockReturnValue(createFakeChildProcess());
+
+    tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "swa-start-spec-"));
+    fs.mkdirSync(path.join(tempRoot, "api"), { recursive: true });
+    fs.mkdirSync(path.join(tempRoot, "app"), { recursive: true });
+
+    originalExit = process.exit;
+    process.exit = vi.fn() as any;
+  });
+
+  afterEach(() => {
+    process.exit = originalExit;
+    fs.rmSync(tempRoot, { recursive: true, force: true });
+  });
+
+  it("should spawn Functions with argv and shell disabled for funcArgs", async () => {
+    let capturedCommands: ConcurrentlyCommandInput[] = [];
+    let capturedOptions: Partial<ConcurrentlyOptions> = {};
+
+    concurrentlyMock.mockImplementation((commands: ConcurrentlyCommandInput[], options: Partial<ConcurrentlyOptions>) => {
+      capturedCommands = commands;
+      capturedOptions = options;
+      const result: ConcurrentlyResult = {
+        commands: [] as any,
+        result: Promise.resolve([
+          {
+            command: { name: "swa", command: "node server.js" },
+            killed: true,
+            index: 0,
+            exitCode: 0,
+            timings: { startDate: new Date(), endDate: new Date(), durationSeconds: 0 },
+          },
+        ] as any),
+      };
+      return result;
+    });
+
+    const { start } = await import("./start.js");
+    await start({
+      appLocation: path.join(tempRoot, "app"),
+      outputLocation: ".",
+      apiLocation: path.join(tempRoot, "api"),
+      apiPort: 7071,
+      host: "localhost",
+      port: 4280,
+      devserverTimeout: 60,
+      ssl: false,
+      open: false,
+      swaConfigLocation: path.join(tempRoot, "app"),
+      funcArgs: "| (echo vulnerable>swa-msrc-144619-windows.marker)",
+    } as SWACLIConfig);
+
+    const apiCommand = (capturedCommands as any[]).find((command) => command.name === "api")?.command as string;
+    expect(apiCommand).toContain("__SWA_INTERNAL_FUNC_START__:");
+    expect(apiCommand).not.toContain("vulnerable");
+    expect(capturedOptions.spawn).toBeTypeOf("function");
+
+    (capturedOptions.spawn as any)(apiCommand, { env: {} });
+
+    expect(spawnMock).toHaveBeenCalledTimes(1);
+    expect(spawnMock).toHaveBeenCalledWith(
+      "func",
+      ["start", "--cors", "*", "--port", "7071", "|", "(echo", "vulnerable>swa-msrc-144619-windows.marker)"],
+      expect.objectContaining({
+        shell: false,
+        windowsVerbatimArguments: false,
+        cwd: path.join(tempRoot, "api"),
+      }),
+    );
+  });
+});

src/cli/commands/start/start.ts

@@ -1,11 +1,12 @@
 import { concurrently, CloseEvent, ConcurrentlyOptions } from "concurrently";
+import { spawn, SpawnOptions } from "node:child_process";
 import fs from "node:fs";
 import path from "node:path";
 import { fileURLToPath } from "node:url";
 import { DEFAULT_CONFIG } from "../../../config.js";
 import { askNewPort, isAcceptingTcpConnections, parseUrl } from "../../../core/utils/net.js";
 import { logger } from "../../../core/utils/logger.js";
-import { createStartupScriptCommand } from "../../../core/utils/cli.js";
+import { createStartupScriptCommand, parseCommandLineArgs } from "../../../core/utils/cli.js";
 import { readWorkflowFile } from "../../../core/utils/workflow-config.js";
 import {
   getNodeMajorVersion,
@@ -26,6 +27,13 @@ const __dirname = path.dirname(__filename);
 
 // This is the path to ./dist/msha/server.js
 const mshaPath = path.resolve(__dirname, "../../../../dist/msha/server.js");
+const INTERNAL_FUNC_START_PREFIX = "__SWA_INTERNAL_FUNC_START__:";
+
+type InternalFuncStartPayload = {
+  binary: string;
+  args: string[];
+  cwd: string;
+};
 
 // import { createRequire } from "node:module";
 // const require = createRequire(import.meta.url);
@@ -167,6 +175,7 @@ export async function start(options: SWACLIConfig) {
   }
 
   const isApiLocationExistsOnDisk = fs.existsSync(userWorkflowConfig?.apiLocation!);
+  let apiFuncPayload: InternalFuncStartPayload | undefined = undefined;
 
   // handle the API location config
   let serveApiCommand = "echo 'No API found. Skipping'";
@@ -203,7 +212,20 @@ export async function start(options: SWACLIConfig) {
 
         // serve the api if and only if the user provides a folder via the --api-location flag
         if (isApiLocationExistsOnDisk) {
-          serveApiCommand = `cd "${userWorkflowConfig.apiLocation}" && "${funcBinary}" start --cors "*" --port ${apiPort} ${funcArgs ?? ""}`;
+          let parsedFuncArgs: string[] = [];
+          try {
+            parsedFuncArgs = parseCommandLineArgs(funcArgs);
+          } catch (error: unknown) {
+            logger.error((error as Error).message, true);
+          }
+
+          apiFuncPayload = {
+            binary: funcBinary,
+            cwd: userWorkflowConfig.apiLocation,
+            args: ["start", "--cors", "*", "--port", `${apiPort}`, ...parsedFuncArgs],
+          };
+
+          serveApiCommand = `${INTERNAL_FUNC_START_PREFIX}${Buffer.from(JSON.stringify(apiFuncPayload)).toString("base64url")}`;
         }
       }
     }
@@ -344,13 +366,36 @@ export async function start(options: SWACLIConfig) {
     env: envVarsObj,
     commands: {
       swa: concurrentlyCommands.find((c) => c.name === "swa")?.command,
-      api: concurrentlyCommands.find((c) => c.name === "api")?.command,
+      api: apiFuncPayload
+        ? `"${apiFuncPayload.binary}" ${apiFuncPayload.args.join(" ")} (cwd: ${apiFuncPayload.cwd})`
+        : concurrentlyCommands.find((c) => c.name === "api")?.command,
       dataApi: concurrentlyCommands.find((c) => c.name == "dataApi")?.command,
       run: concurrentlyCommands.find((c) => c.name === "run")?.command,
     },
   });
 
-  const concurrentlyOptions: Partial<ConcurrentlyOptions> = { restartTries: 0, killOthers: ["failure", "success"], raw: true };
+  const concurrentlyOptions: Partial<ConcurrentlyOptions> = {
+    restartTries: 0,
+    killOthers: ["failure", "success"],
+    raw: true,
+    spawn: (command: string, options: SpawnOptions) => {
+      if (command.startsWith(INTERNAL_FUNC_START_PREFIX)) {
+        const encodedPayload = command.slice(INTERNAL_FUNC_START_PREFIX.length);
+        const payload = JSON.parse(Buffer.from(encodedPayload, "base64url").toString("utf8")) as InternalFuncStartPayload;
+        const env = Object.fromEntries(Object.entries(options.env || {}).map(([key, value]) => [key, value == null ? undefined : String(value)]));
+
+        return spawn(payload.binary, payload.args, {
+          ...options,
+          cwd: payload.cwd,
+          env,
+          shell: false,
+          windowsVerbatimArguments: false,
+        });
+      }
+
+      return spawnWithShell(command, options);
+    },
+  };
   const { result } = concurrently(concurrentlyCommands, concurrentlyOptions);
 
   await result
@@ -387,3 +432,17 @@ export async function start(options: SWACLIConfig) {
       logger.error(err.message, true);
     });
 }
+
+function spawnWithShell(command: string, options: SpawnOptions) {
+  const shellOptions: SpawnOptions = { ...options };
+
+  if (process.platform === "win32") {
+    const shell = process.env.comspec || "cmd.exe";
+    return spawn(shell, ["/d", "/s", "/c", `"${command}"`], {
+      ...shellOptions,
+      windowsVerbatimArguments: true,
+    });
+  }
+
+  return spawn("/bin/sh", ["-c", command], shellOptions);
+}

src/core/utils/cli.spec.ts

@@ -4,7 +4,7 @@ vi.mock("../constants", () => {
 });
 import path from "node:path";
 import { vol } from "memfs";
-import { argv, createStartupScriptCommand, parseServerTimeout } from "./cli.js";
+import { argv, createStartupScriptCommand, parseCommandLineArgs, parseServerTimeout } from "./cli.js";
 import { logger } from "./logger.js";
 
 describe("argv()", () => {
@@ -64,6 +64,60 @@ describe("createStartupScriptCommand()", () => {
       expect(cmd).toBe("npm run start#foo --if-present");
     });
   });
+
+  describe("parseCommandLineArgs()", () => {
+    it("should return empty array for empty input", () => {
+      expect(parseCommandLineArgs()).toStrictEqual([]);
+      expect(parseCommandLineArgs("   ")).toStrictEqual([]);
+    });
+
+    it("should split regular arguments", () => {
+      expect(parseCommandLineArgs("--verbose --port 7071")).toStrictEqual(["--verbose", "--port", "7071"]);
+    });
+
+    it("should preserve quoted arguments", () => {
+      expect(parseCommandLineArgs('--name "hello world"')).toStrictEqual(["--name", "hello world"]);
+      expect(parseCommandLineArgs("--name 'hello world'")).toStrictEqual(["--name", "hello world"]);
+    });
+
+    it("should preserve shell metacharacters as literal values", () => {
+      expect(parseCommandLineArgs("| (echo vulnerable>swa-msrc-144619-windows.marker)")).toStrictEqual([
+        "|",
+        "(echo",
+        "vulnerable>swa-msrc-144619-windows.marker)",
+      ]);
+      expect(parseCommandLineArgs("--flag && whoami > out.txt")).toStrictEqual(["--flag", "&&", "whoami", ">", "out.txt"]);
+    });
+
+    it("should throw for unbalanced quotes", () => {
+      expect(() => parseCommandLineArgs('--name "hello')).toThrow("Unbalanced quotes in --func-args.");
+    });
+
+    it("should preserve mixed quote content", () => {
+      expect(parseCommandLineArgs(`--msg "a 'b' c"`)).toStrictEqual(["--msg", "a 'b' c"]);
+      expect(parseCommandLineArgs(`--msg 'a "b" c'`)).toStrictEqual(["--msg", `a "b" c`]);
+    });
+
+    it("should preserve escaped whitespace and trailing backslash", () => {
+      expect(parseCommandLineArgs(String.raw`--title hello\ world`)).toStrictEqual(["--title", "hello world"]);
+      expect(parseCommandLineArgs("--path C:\\tmp\\")).toStrictEqual(["--path", "C:tmp\\"]);
+    });
+
+    it("should preserve windows paths in quoted values", () => {
+      expect(parseCommandLineArgs(`--script "C:\\Program Files\\my app\\start.js"`)).toStrictEqual([
+        "--script",
+        `C:\\Program Files\\my app\\start.js`,
+      ]);
+    });
+
+    it("should keep option terminator as a literal token", () => {
+      expect(parseCommandLineArgs(`-- --literal --not-option`)).toStrictEqual(["--", "--literal", "--not-option"]);
+    });
+
+    it("should treat all whitespace as separators when not quoted", () => {
+      expect(parseCommandLineArgs("--a   1\t--b  2")).toStrictEqual(["--a", "1", "--b", "2"]);
+    });
+  });
   describe("yarn", () => {
     it("should parse yarn scripts (simple)", () => {
       const cmd = createStartupScriptCommand("yarn:start", {});