diff --git a/CHANGELOG.md b/CHANGELOG.md index b5c2577f6..0515c111f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -216,6 +216,7 @@ Docs: https://docs.openclaw.ai - Agents/embedded runner: recover canonical allowlisted tool names from malformed `toolCallId` and malformed non-blank tool-name variants before dispatch, while failing closed on ambiguous matches. (#34485) thanks @yuweuii. - Agents/failover: classify ZenMux quota-refresh `402` responses as `rate_limit` so model fallback retries continue instead of stopping on a temporary subscription window. (#43917) thanks @bwjoke. - Agents/failover: classify HTTP 422 malformed-request responses as `format` and recognize OpenRouter "requires more credits" billing errors so provider fallback triggers instead of surfacing raw errors. (#43823) thanks @jnMetaCode. +- Memory/QMD Windows: fail closed when `qmd.cmd` or `mcporter.cmd` wrappers cannot be resolved to a direct entrypoint, so memory search no longer falls back to shell execution on Windows. ## 2026.3.8 diff --git a/src/memory/qmd-manager.test.ts b/src/memory/qmd-manager.test.ts index 48c8a4ec5..5a5b577dc 100644 --- a/src/memory/qmd-manager.test.ts +++ b/src/memory/qmd-manager.test.ts @@ -1078,7 +1078,23 @@ describe("QmdMemoryManager", () => { it("resolves bare qmd command to a Windows-compatible spawn invocation", async () => { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const previousPath = process.env.PATH; try { + const nodeModulesDir = path.join(tmpRoot, "node_modules"); + const shimDir = path.join(nodeModulesDir, ".bin"); + const packageDir = path.join(nodeModulesDir, "qmd"); + const scriptPath = path.join(packageDir, "dist", "cli.js"); + await fs.mkdir(path.dirname(scriptPath), { recursive: true }); + await fs.mkdir(shimDir, { recursive: true }); + await fs.writeFile(path.join(shimDir, "qmd.cmd"), "@echo off\r\n", "utf8"); + await fs.writeFile( + path.join(packageDir, "package.json"), + JSON.stringify({ name: "qmd", version: "0.0.0", bin: { qmd: "dist/cli.js" } }), + "utf8", + ); + await fs.writeFile(scriptPath, "module.exports = {};\n", "utf8"); + process.env.PATH = `${shimDir};${previousPath ?? ""}`; + const { manager } = await createManager({ mode: "status" }); await manager.sync({ reason: "manual" }); @@ -1093,19 +1109,14 @@ describe("QmdMemoryManager", () => { for (const call of qmdCalls) { const command = String(call[0]); const options = call[2] as { shell?: boolean } | undefined; - if (/(^|[\\/])qmd(?:\.cmd)?$/i.test(command)) { - // Wrapper unresolved: keep `.cmd` and use shell for PATHEXT lookup. - expect(command.toLowerCase().endsWith("qmd.cmd")).toBe(true); - expect(options?.shell).toBe(true); - } else { - // Wrapper resolved to node/exe entrypoint: shell fallback should not be used. - expect(options?.shell).not.toBe(true); - } + expect(command).not.toMatch(/(^|[\\/])qmd\.cmd$/i); + expect(options?.shell).not.toBe(true); } await manager.close(); } finally { platformSpy.mockRestore(); + process.env.PATH = previousPath; } }); @@ -1576,9 +1587,25 @@ describe("QmdMemoryManager", () => { await manager.close(); }); - it("uses mcporter.cmd on Windows when mcporter bridge is enabled", async () => { + it("resolves mcporter to a direct Windows entrypoint without enabling shell mode", async () => { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const previousPath = process.env.PATH; try { + const nodeModulesDir = path.join(tmpRoot, "node_modules"); + const shimDir = path.join(nodeModulesDir, ".bin"); + const packageDir = path.join(nodeModulesDir, "mcporter"); + const scriptPath = path.join(packageDir, "dist", "cli.js"); + await fs.mkdir(path.dirname(scriptPath), { recursive: true }); + await fs.mkdir(shimDir, { recursive: true }); + await fs.writeFile(path.join(shimDir, "mcporter.cmd"), "@echo off\r\n", "utf8"); + await fs.writeFile( + path.join(packageDir, "package.json"), + JSON.stringify({ name: "mcporter", version: "0.0.0", bin: { mcporter: "dist/cli.js" } }), + "utf8", + ); + await fs.writeFile(scriptPath, "module.exports = {};\n", "utf8"); + process.env.PATH = `${shimDir};${previousPath ?? ""}`; + cfg = { ...cfg, memory: { @@ -1612,21 +1639,17 @@ describe("QmdMemoryManager", () => { const callCommand = mcporterCall?.[0]; expect(typeof callCommand).toBe("string"); const options = mcporterCall?.[2] as { shell?: boolean } | undefined; - if (isMcporterCommand(callCommand)) { - expect(callCommand).toBe("mcporter.cmd"); - expect(options?.shell).toBe(true); - } else { - // If wrapper entrypoint resolution succeeded, spawn may invoke node/exe directly. - expect(options?.shell).not.toBe(true); - } + expect(callCommand).not.toBe("mcporter.cmd"); + expect(options?.shell).not.toBe(true); await manager.close(); } finally { platformSpy.mockRestore(); + process.env.PATH = previousPath; } }); - it("retries mcporter search with bare command on Windows EINVAL cmd-shim failures", async () => { + it("fails closed on Windows EINVAL cmd-shim failures instead of retrying through the shell", async () => { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); const previousPath = process.env.PATH; try { @@ -1647,7 +1670,6 @@ describe("QmdMemoryManager", () => { }, } as OpenClawConfig; - let sawRetry = false; let firstCallCommand: string | null = null; spawnMock.mockImplementation((cmd: string, args: string[]) => { if (args[0] === "call" && firstCallCommand === null) { @@ -1661,12 +1683,6 @@ describe("QmdMemoryManager", () => { }); return child; } - if (args[0] === "call" && cmd === "mcporter") { - sawRetry = true; - const child = createMockChild({ autoClose: false }); - emitAndClose(child, "stdout", JSON.stringify({ results: [] })); - return child; - } const child = createMockChild({ autoClose: false }); emitAndClose(child, "stdout", "[]"); return child; @@ -1675,16 +1691,16 @@ describe("QmdMemoryManager", () => { const { manager } = await createManager(); await expect( manager.search("hello", { sessionKey: "agent:main:slack:dm:u123" }), - ).resolves.toEqual([]); + ).rejects.toThrow(/without shell execution|EINVAL/); const attemptedCmdShim = (firstCallCommand ?? "").toLowerCase().endsWith(".cmd"); if (attemptedCmdShim) { - expect(sawRetry).toBe(true); - expect(logWarnMock).toHaveBeenCalledWith( - expect.stringContaining("retrying with bare mcporter"), - ); - } else { - // When wrapper resolution upgrades to a direct node/exe entrypoint, cmd-shim retry is unnecessary. - expect(sawRetry).toBe(false); + expect( + spawnMock.mock.calls.some( + (call: unknown[]) => + call[0] === "mcporter" && + (call[2] as { shell?: boolean } | undefined)?.shell === true, + ), + ).toBe(false); } await manager.close(); } finally { diff --git a/src/memory/qmd-manager.ts b/src/memory/qmd-manager.ts index 986d526e0..46a801566 100644 --- a/src/memory/qmd-manager.ts +++ b/src/memory/qmd-manager.ts @@ -8,11 +8,7 @@ import { resolveStateDir } from "../config/paths.js"; import { writeFileWithinRoot } from "../infra/fs-safe.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { isFileMissingError, statRegularFile } from "./fs-utils.js"; -import { - isWindowsCommandShimEinval, - resolveCliSpawnInvocation, - runCliCommand, -} from "./qmd-process.js"; +import { resolveCliSpawnInvocation, runCliCommand } from "./qmd-process.js"; import { deriveQmdScopeChannel, deriveQmdScopeChatType, isQmdScopeAllowed } from "./qmd-scope.js"; import { listSessionFilesForAgent, @@ -1248,50 +1244,21 @@ export class QmdMemoryManager implements MemorySearchManager { args: string[], opts?: { timeoutMs?: number }, ): Promise<{ stdout: string; stderr: string }> { - const runWithInvocation = async (spawnInvocation: { - command: string; - argv: string[]; - shell?: boolean; - windowsHide?: boolean; - }): Promise<{ stdout: string; stderr: string }> => - await runCliCommand({ - commandSummary: `${spawnInvocation.command} ${spawnInvocation.argv.join(" ")}`, - spawnInvocation, - // Keep mcporter and direct qmd commands on the same agent-scoped XDG state. - env: this.env, - cwd: this.workspaceDir, - timeoutMs: opts?.timeoutMs, - maxOutputChars: this.maxQmdOutputChars, - }); - - const primaryInvocation = resolveCliSpawnInvocation({ + const spawnInvocation = resolveCliSpawnInvocation({ command: "mcporter", args, env: this.env, packageName: "mcporter", }); - try { - return await runWithInvocation(primaryInvocation); - } catch (err) { - if ( - !isWindowsCommandShimEinval({ - err, - command: primaryInvocation.command, - commandBase: "mcporter", - }) - ) { - throw err; - } - // Some Windows npm cmd shims can still throw EINVAL on spawn; retry through - // shell command resolution so PATH/PATHEXT can select a runnable entrypoint. - log.warn("mcporter.cmd spawn returned EINVAL on Windows; retrying with bare mcporter"); - return await runWithInvocation({ - command: "mcporter", - argv: args, - shell: true, - windowsHide: true, - }); - } + return await runCliCommand({ + commandSummary: `${spawnInvocation.command} ${spawnInvocation.argv.join(" ")}`, + spawnInvocation, + // Keep mcporter and direct qmd commands on the same agent-scoped XDG state. + env: this.env, + cwd: this.workspaceDir, + timeoutMs: opts?.timeoutMs, + maxOutputChars: this.maxQmdOutputChars, + }); } private async runQmdSearchViaMcporter(params: { diff --git a/src/memory/qmd-process.test.ts b/src/memory/qmd-process.test.ts new file mode 100644 index 000000000..84237c43c --- /dev/null +++ b/src/memory/qmd-process.test.ts @@ -0,0 +1,75 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { resolveCliSpawnInvocation } from "./qmd-process.js"; + +describe("resolveCliSpawnInvocation", () => { + let tempDir = ""; + let platformSpy: { mockRestore(): void } | null = null; + const originalPath = process.env.PATH; + const originalPathExt = process.env.PATHEXT; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-qmd-win-spawn-")); + platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + }); + + afterEach(async () => { + platformSpy?.mockRestore(); + process.env.PATH = originalPath; + process.env.PATHEXT = originalPathExt; + if (tempDir) { + await fs.rm(tempDir, { recursive: true, force: true }); + tempDir = ""; + } + }); + + it("unwraps npm cmd shims to a direct node entrypoint", async () => { + const binDir = path.join(tempDir, "node_modules", ".bin"); + const packageDir = path.join(tempDir, "node_modules", "qmd"); + const scriptPath = path.join(packageDir, "dist", "cli.js"); + await fs.mkdir(path.dirname(scriptPath), { recursive: true }); + await fs.mkdir(binDir, { recursive: true }); + await fs.writeFile(path.join(binDir, "qmd.cmd"), "@echo off\r\n", "utf8"); + await fs.writeFile( + path.join(packageDir, "package.json"), + JSON.stringify({ name: "qmd", version: "0.0.0", bin: { qmd: "dist/cli.js" } }), + "utf8", + ); + await fs.writeFile(scriptPath, "module.exports = {};\n", "utf8"); + + process.env.PATH = `${binDir};${originalPath ?? ""}`; + process.env.PATHEXT = ".CMD;.EXE"; + + const invocation = resolveCliSpawnInvocation({ + command: "qmd", + args: ["query", "hello"], + env: process.env, + packageName: "qmd", + }); + + expect(invocation.command).toBe(process.execPath); + expect(invocation.argv).toEqual([scriptPath, "query", "hello"]); + expect(invocation.shell).not.toBe(true); + expect(invocation.windowsHide).toBe(true); + }); + + it("fails closed when a Windows cmd shim cannot be resolved without shell execution", async () => { + const binDir = path.join(tempDir, "bad-bin"); + await fs.mkdir(binDir, { recursive: true }); + await fs.writeFile(path.join(binDir, "qmd.cmd"), "@echo off\r\nREM no entrypoint\r\n", "utf8"); + + process.env.PATH = `${binDir};${originalPath ?? ""}`; + process.env.PATHEXT = ".CMD;.EXE"; + + expect(() => + resolveCliSpawnInvocation({ + command: "qmd", + args: ["query", "hello"], + env: process.env, + packageName: "qmd", + }), + ).toThrow(/without shell execution/); + }); +}); diff --git a/src/memory/qmd-process.ts b/src/memory/qmd-process.ts index 7c0b1a6c3..bb0eea803 100644 --- a/src/memory/qmd-process.ts +++ b/src/memory/qmd-process.ts @@ -43,27 +43,11 @@ export function resolveCliSpawnInvocation(params: { env: params.env, execPath: process.execPath, packageName: params.packageName, - allowShellFallback: true, + allowShellFallback: false, }); return materializeWindowsSpawnProgram(program, params.args); } -export function isWindowsCommandShimEinval(params: { - err: unknown; - command: string; - commandBase: string; -}): boolean { - if (process.platform !== "win32") { - return false; - } - const errno = params.err as NodeJS.ErrnoException | undefined; - if (errno?.code !== "EINVAL") { - return false; - } - const escapedBase = params.commandBase.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - return new RegExp(`(^|[\\\\/])${escapedBase}\\.cmd$`, "i").test(params.command); -} - export async function runCliCommand(params: { commandSummary: string; spawnInvocation: CliSpawnInvocation;