1. src/commands/doctor-memory-search.ts, line 24-27 (link)

   Stale JSDoc comment — function now spawns a process

   The JSDoc says "config-only, no network calls", but after this PR the function also calls checkQmdBinaryAvailability, which forks a child process. The comment should be updated to reflect the new behaviour so callers aren't surprised.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/commands/doctor-memory-search.ts
   Line: 24-27
   
   Comment:
   **Stale JSDoc comment — function now spawns a process**
   
   The JSDoc says _"config-only, no network calls"_, but after this PR the function also calls `checkQmdBinaryAvailability`, which forks a child process. The comment should be updated to reflect the new behaviour so callers aren't surprised.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

1. 🟠 openclaw doctor executes configured memory.qmd.command when backend is qmd

Description

noteMemorySearchHealth (run as part of openclaw doctor) spawns a subprocess to probe the QMD backend by executing the configured command:

• Input: backendConfig.qmd?.command ultimately comes from user/project configuration (memory.qmd.command).
• Sink: checkQmdBinaryAvailability calls child_process.spawn() with that command.
• Impact: if an attacker can influence the config in a repo/workspace (or via config precedence), convincing a user/CI/editor integration to run openclaw doctor becomes an arbitrary local code execution primitive (run attacker-chosen executable).

Vulnerable code:

const qmdCheck = await checkQmdBinaryAvailability({
  command: backendConfig.qmd?.command ?? "qmd",
  env: process.env,
});

Notes:

• memory.qmd.command is not restricted to an allowlist or absolute path, and the probe executes the command (even though it is short-lived).
• The implementation does not appear to use shell: true (good), and args are not taken from config; however, selecting the executable alone is sufficient for code execution if config is untrusted.

Recommendation

Avoid executing configurable commands during diagnostics, or gate it behind explicit user intent.

Recommended mitigations (pick one or combine):

1. Do not spawn at all in doctor; instead perform a non-executing check (best):

   • Require memory.qmd.command to be an absolute path and validate it with fs.existsSync() / access checks, or
   • Resolve via PATH lookup without executing (platform-specific), then show guidance.

2. Require explicit opt-in to run probes:

   • Add a flag like openclaw doctor --allow-spawn (default false) and skip probe otherwise.

3. Constrain the command:

   • Only allow the literal qmd (or an absolute path), reject anything else.

Example (absolute-path only, no spawn):

import fs from "node:fs";
import path from "node:path";

const cmd = backendConfig.qmd?.command ?? "qmd";
if (!path.isAbsolute(cmd) || !fs.existsSync(cmd)) {
  note("QMD is configured but memory.qmd.command must be an existing absolute path; skipping execution in doctor.");
  return;
}

If you still need to execute, run only a fixed binary name and fixed args, and clearly warn users that doctor may execute local binaries.

2. 🟠 Windows untrusted search path execution when probing/spawning qmd from workspace/current directory

Description

The QMD availability probe can execute an attacker-controlled binary on Windows due to untrusted search path behavior.

• checkQmdBinaryAvailability() spawns params.command with a caller-controlled cwd (agent workspace) or defaults to process.cwd().
• When memory.qmd.command is configured as a bare name like qmd (the documented default), resolveWindowsExecutablePath() only resolves to an absolute path if it finds a match in PATH. If not found, it returns the bare name.
• On Windows, CreateProcess search order can include the current working directory. That means if a user opens an untrusted repository/workspace and the process cwd (or agent workspace dir) contains a malicious qmd.exe/qmd.cmd, it may be executed during the probe.

This turns a configuration/health check (and later QMD backend spawning which uses the same resolution path and cwd) into potential arbitrary code execution when operating in attacker-controlled directories.

Vulnerable code:

const child = spawn(spawnInvocation.command, spawnInvocation.argv, {
  env: params.env,
  cwd: params.cwd ?? process.cwd(),
  shell: spawnInvocation.shell,
  windowsHide: spawnInvocation.windowsHide,
  stdio: "ignore",
});

Recommendation

Prevent Windows current-directory binary hijacking by ensuring the spawned program is resolved to an absolute path (or fails) before calling spawn, and avoid using attacker-controlled cwd for binary resolution.

Suggested fix (fail closed if the command cannot be resolved via PATH when it is not an explicit path):

import path from "node:path";

function isExplicitPath(cmd: string) {
  return cmd.includes("/") || cmd.includes("\\") || path.isAbsolute(cmd);
}

export async function checkQmdBinaryAvailability(params: { command: string; env: NodeJS.ProcessEnv; cwd?: string; timeoutMs?: number; }) {
  const spawnInvocation = resolveCliSpawnInvocation({
    command: params.command,
    args: [],
    env: params.env,
    packageName: "qmd",
  });

  ​// On Windows, ensure resolution didn't fall back to a bare name.
  if (process.platform === "win32" && !isExplicitPath(params.command) && !path.isAbsolute(spawnInvocation.command)) {
    return { available: false, error: `qmd not found on PATH: ${params.command}` };
  }

  ​// Optionally, use a neutral cwd for the probe.
  const safeCwd = process.platform === "win32" ? process.env.SystemRoot ?? process.cwd() : (params.cwd ?? process.cwd());

  const child = spawn(spawnInvocation.command, spawnInvocation.argv, {
    env: params.env,
    cwd: safeCwd,
    shell: spawnInvocation.shell,
    windowsHide: spawnInvocation.windowsHide,
    stdio: "ignore",
  });
}

Additionally:

• Consider requiring memory.qmd.command to be an absolute path on Windows, or explicitly document/validate it.
• Apply the same absolute-resolution requirement in the actual QMD execution path (e.g., where runCliCommand is used) to avoid executing from an untrusted workspace.

3. 🟡 Terminal escape/log injection via unsanitized qmd spawn error messages in user-facing output

Description

The CLI prints and logs raw error strings originating from process spawn failures without neutralizing control characters.

• checkQmdBinaryAvailability() returns err.message verbatim (from Node spawn/validation errors).
• These messages are interpolated directly into:
  • note(...) output shown to users during openclaw doctor.
  • log.warn(...) messages in the memory search manager.
• If an attacker can influence the configured memory.qmd.command (e.g., via a shared/malicious config file) or otherwise trigger crafted error text, they may inject terminal control sequences (ANSI escapes) or forge multi-line log entries.
• Even without an active attacker, the raw error often includes sensitive local details such as absolute paths, usernames, or platform-specific spawn diagnostics.

Vulnerable code (user-facing note):

qmdCheck.error ? `Probe error: ${qmdCheck.error}` : null,

Vulnerable code (logging):

`... falling back to builtin: ${qmdBinary.error ?? "unknown error"}`

Recommendation

Neutralize control characters before writing untrusted strings to the terminal/logs, and consider hiding detailed spawn diagnostics unless --verbose is enabled.

Example mitigation:

function sanitizeForTerminal(input: string): string {
  ​// Remove C0/C1 control chars and ESC to prevent ANSI/terminal injection
  return input.replace(/[\u0000-\u001F\u007F-\u009F\u001B]/g, "");
}

const safeErr = qmdCheck.error ? sanitizeForTerminal(qmdCheck.error) : undefined;
note(
  [
    `QMD memory backend is configured, but the qmd binary could not be started (${cmd}).`,
    safeErr ? `Probe error: ${safeErr}` : null,
  ].filter(Boolean).join("\n"),
  "Memory search",
);

Also apply similar sanitization to log.warn(...) messages that include qmdBinary.error / thrown err.message.

4. 🔵 Availability probe may delay for timeout when child emits close before spawn

Description

checkQmdBinaryAvailability can take the full timeout (default 2000ms) to resolve in certain event orderings because the close handler returns early when didSpawn is false.

• The promise is only settled on spawn, error, or the timeout.
• If the ChildProcess emits close without a preceding spawn (and without an error), the handler returns early and does not resolve.
• In that case the function will only resolve when the timeout fires, causing an avoidable delay.
• If this probe is invoked frequently (e.g., per agent or per command) and an attacker/user can influence conditions that trigger this event ordering (bad/slow binary path, platform quirks), it can be used to create a controllable slowdown (application-layer DoS).

Vulnerable code:

child.once("close", () => {
  if (!didSpawn) {
    return;
  }
  finish({ available: true });
});

Recommendation

Ensure the promise always settles on close, and treat close before spawn as an immediate failure (or at least as unavailable) rather than waiting for the timeout.

Example:

child.once("close", (code, signal) => {
  if (!didSpawn) {
    finish({
      available: false,
      error: `qmd exited before spawn (code=${code}, signal=${signal ?? ""})`,
    });
    return;
  }
  finish({ available: true });
});

Additionally, defensively wrap child.kill(...) in try/catch in the timeout handler to avoid unexpected exceptions preventing settlement.