Greptile Summary

This PR adds explicit detection and user-visible warnings when the qmd binary is missing while memory.backend is set to "qmd", addressing a silent fallback that confused users. The core change — probing qmd --version in both the startup path (getMemorySearchManager) and the openclaw doctor check (noteMemorySearchHealth) — is correct and the warning messages are clear and actionable.

Key concerns:

• Binary probe runs on every status-only call (src/memory/search-manager.ts): when purpose === "status", the cache lookup is skipped, meaning checkQmdBinaryAvailable (with its 5 s default timeout) is spawned unconditionally on every status check. Consider a short-lived in-process cache for the probe result.
• Unrelated buildQmdCacheKey refactoring (src/memory/search-manager.ts): the original JSON.stringify approach was intentional and explicitly documented. The new recursive stableSerialize/sortValue helper adds key-sorting overhead on every cache miss and changes the serialised key format, which is out of scope for this PR.
• Weak happy-path test (src/memory/backend-config.test.ts): the "returns available=true" test never asserts result.available === true; both branches only verify property existence. Since the test runs in Node.js, node --version will always succeed and the assertion should be direct.
• Previously flagged open issues (not repeated in detail): overly broad "spawn" pattern in notFoundPatterns; unused execFile/promisify imports in test; 7 unrelated test cases deleted from doctor-memory-search.test.ts; fragile timeout test; unrelated emoji change in src/channels/status-reactions.ts.

Confidence Score: 2/5

• Not yet safe to merge — unresolved issues from previous review rounds remain open and this round adds new concerns around unrelated refactoring and test quality.
• The core feature logic is sound and addresses a real user pain-point. However, the PR accumulates several unresolved issues flagged in prior rounds (overly broad "spawn" pattern, unused imports, deleted test coverage, fragile timeout test, unrelated status-reactions change) alongside newly identified problems: a silent unrelated refactoring of the cache-key serialiser with a performance cost, a status-only code path that unconditionally spawns a subprocess, and a happy-path test that never asserts its stated precondition. Until the previously flagged issues are addressed — especially the deleted test cases and the "spawn" pattern — and the new concerns are resolved, merging carries meaningful regression risk.
• src/memory/search-manager.ts (unrelated stableSerialize refactoring, status-path probe overhead), src/commands/doctor-memory-search.test.ts (7 deleted test cases), src/memory/backend-config.ts (overly broad "spawn" pattern), src/memory/backend-config.test.ts (unused imports, weak happy-path test)

_{Last reviewed commit: 477198b}

✅ 修复完成

感谢 @greptile-apps[bot] 和 @chatgpt-codex-connector[bot] 的详细审核评论！我已修复所有问题：

修复内容

1. 修复逻辑错误 - 当 binary 不可用时跳过 QMD 创建

问题: 当 qmdCheck.available 为 false 时，代码记录了警告但仍然继续进入 try 块尝试创建 QMD manager。

修复 (src/memory/search-manager.ts):

// 修复前
if (!qmdCheck.available) {
  log.warn(...);
} else if (!statusOnly) {
  // 缓存检查在这里
}
// 继续执行 QMD 创建

// 修复后
if (!qmdCheck.available) {
  log.warn(...);
  // Skip QMD creation and fall through to builtin backend
} else {
  try {
    // 只有 binary 可用时才尝试创建 QMD
  }
}

2. 移除未使用的变量

问题: version 变量被赋值但从未使用，会导致 TypeScript 编译失败。

修复 (src/memory/backend-config.ts):

// 修复前
const { stdout } = await execFileAsync(...);
const version = stdout.trim().split("\n")[0];
return { available: true, path: resolvedCommand };

// 修复后
await execFileAsync(...);
return { available: true, path: resolvedCommand };

3. 恢复 QMD 缓存快速路径

问题: 在检查缓存之前先检查 binary，导致每次调用都执行 qmd --version，增加了延迟。

修复 (src/memory/search-manager.ts):

// 先检查缓存（快速路径）
if (!statusOnly) {
  const cached = QMD_MANAGER_CACHE.get(cacheKey);
  if (cached) {
    return { manager: cached };
  }
}

// 再检查 binary 是否可用
const qmdCheck = await checkQmdBinaryAvailable(resolved.qmd.command);

提交

• 提交哈希: d20cf1f24
• 已推送到分支: fix/25910-qmd-binary-check

请维护者重新审核，谢谢！

Codex Review: Didn't find any major issues. What shall we delve into next?

✅ 修复完成（第二轮）

感谢 @greptile-apps[bot] 的进一步审核！我已修复新发现的问题：

修复内容

1. 移除过于宽泛的 spawn 错误模式

问题: "spawn" 字符串会匹配所有 Node.js child-process 错误，包括权限错误，导致错误分类。

修复 (src/memory/backend-config.ts):

// 修复前
const notFoundPatterns = [
  "ENOENT",
  "not found",
  "cannot find",
  "EINVAL",
  "spawn",  // ❌ 过于宽泛
];

// 修复后
const notFoundPatterns = [
  "ENOENT",
  "not found",
  "cannot find",
  "EINVAL", // Windows spawn EINVAL for non-existent executables
];

2. 恢复无关的 emoji 更改

问题: stallSoft emoji 从 🥱 改为 ⏳ 与 QMD 功能无关。

修复 (src/channels/status-reactions.ts):

// 修复前
stallSoft: "⏳",

// 修复后
stallSoft: "🥱",

提交

• 提交哈希: 40acd23b7
• 已推送到分支: fix/25910-qmd-binary-check

请维护者重新审核，谢谢！

🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

1. 🟠 Arbitrary command execution via memory.qmd.command in QMD availability check

Description

The new checkQmdBinaryAvailable() helper executes the configured QMD command to test availability:

• Input: memory.qmd.command (from OpenClawConfig) is parsed in resolveMemoryBackendConfig() and stored as resolved.qmd.command.
• Sink: checkQmdBinaryAvailable() runs execFile(resolvedCommand, ["--version"], { cwd }).
• New call sites run this during non-operational health/status flows (e.g., openclaw doctor and openclaw status), meaning simply checking health can execute an arbitrary local program.

This becomes a practical RCE vector when configuration can be influenced from an untrusted workspace:

• OpenClaw loads dotenv from the current working directory (dotenv.config()), so a malicious repo can provide a .env that sets OPENCLAW_CONFIG_PATH (or other env-driven config indirection) to point to a repo-controlled config file.
• That config can set memory.backend: "qmd" and memory.qmd.command: "./qmd" (or an absolute path to a repo-provided binary/script).
• Running openclaw doctor or openclaw status in that repo will then execute ./qmd --version.

Windows-specific risk amplification:

• On Windows, the function forces qmd → qmd.cmd and also sets cwd to the agent/workspace directory. Windows process resolution commonly searches the current directory early; a qmd.cmd dropped into the workspace can be executed even if a legitimate QMD exists elsewhere on PATH.

Vulnerable code:

await execFileAsync(resolvedCommand, ["--version"], {
  timeout: timeoutMs,
  encoding: "utf8",
  cwd,
});

Recommendation

Avoid executing an arbitrary configured binary just to test “availability”, especially in doctor/status code paths.

Recommended hardening (defense-in-depth):

1. Non-executing resolution check: resolve the executable path without running it.

   • Search PATH entries and verify the resolved file exists and is executable.
   • On Windows, respect PATHEXT but do not privilege cwd.

2. Treat relative commands as untrusted by default:

   • If memory.qmd.command is relative (contains ./, ../, or lacks a directory component), either reject it or require explicit user confirmation/allowlisting.

3. Do not run this check in doctor/status:

   • In those commands, only report what would be executed (resolved path) and warn if it points inside the workspace.

Example (non-executing PATH search):

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

function findOnPath(command: string, envPath = process.env.PATH ?? ""): string | null {
  const pathEntries = envPath.split(path.delimiter).filter(Boolean);
  for (const entry of pathEntries) {
    const candidate = path.join(entry, command);
    try {
      fs.accessSync(candidate, fs.constants.X_OK);
      return candidate;
    } catch {}
  }
  return null;
}

export function checkQmdBinaryAvailableNoExec(command = "qmd") {
  if (command.includes("/") || command.includes("\\")) {
    ​// for explicit paths, just validate executable bit / existence
    try {
      fs.accessSync(command, fs.constants.X_OK);
      return { available: true as const, path: command };
    } catch (e) {
      return { available: false as const, error: String(e) };
    }
  }
  const resolved = findOnPath(command);
  return resolved
    ? { available: true as const, path: resolved }
    : { available: false as const, error: `Not found on PATH: ${command}` };
}

If you must execute to validate functionality, gate it behind an explicit opt-in (or existing exec approval/allowlist mechanisms) and never run it automatically during health/status checks.

2. 🔵 Terminal/log injection via unsanitized memory.qmd.command in QMD availability errors

Description

User-controlled configuration memory.qmd.command is interpolated into human-facing error/log messages without sanitization, enabling terminal escape sequence injection and log forging (CWE-117).

Data flow:

• Input: memory.qmd.command from local config (resolveMemoryBackendConfig() parses it into ResolvedQmdConfig.command).
• Propagation: checkQmdBinaryAvailable(command) embeds the original command and err.message into returned error strings.
• Sinks:
  • note() output in doctor-memory-search.ts renders Error: ${checkResult.error} to the terminal.
  • createSubsystemLogger().warn() in search-manager.ts prints qmdCheck.error to console.

Because neither note() nor the subsystem logger sanitizes ANSI/C0 control characters, a crafted memory.qmd.command (e.g. containing \u001b[2J to clear the screen, OSC-8 hyperlinks, or embedded newlines when quoted) can:

• manipulate terminal display (hide warnings, spoof prompts)
• forge multi-line log entries in console output

Proof-of-concept config value:

• memory.qmd.command: "\"qmd\n[info] forged log line\"" (newline inside quotes survives parsing)
• memory.qmd.command: "\u001b[2J\u001b[Hqmd" (clear screen + cursor home)

Vulnerable code (error construction):

error: `QMD binary "${command}" not found on PATH. ...`
...
error: `QMD binary check failed: ${error}`

Recommendation

Sanitize untrusted values before embedding them into terminal/log strings.

A minimal fix is to sanitize command and err.message inside checkQmdBinaryAvailable() using existing helpers (e.g. sanitizeTerminalText for single-line rendering), so all downstream consumers are protected:

import { sanitizeTerminalText } from "../terminal/safe-text.js";

...
const safeCommand = sanitizeTerminalText(command);
const safeErr = sanitizeTerminalText(error);

if (isNotFound) {
  return {
    available: false,
    error: `QMD binary "${safeCommand}" not found on PATH. Please install QMD or check your configuration.`,
  };
}
return {
  available: false,
  error: `QMD binary check failed: ${safeErr}`,
};

Additionally (defense in depth):

• Prefer structured logging (log.warn("...", { command, error })) and sanitize in console rendering.
• For terminal UI (note()), sanitize interpolated substrings rather than entire multi-line templates.

Analyzed PR: #41053 at commit 00fc415

_{Last updated on: 2026-03-10T17:10:03Z}

@greptile-apps 感谢审查！我已经修复了评论中提到的问题：

1. 恢复了被删除的 7 个测试用例 - 这些测试用例与 QMD 功能无关，但覆盖了 gatewayMemoryProbe 集成、SecretRef 处理和 ollama 提供者排除逻辑。现已全部恢复：

   • does not warn when local provider is set with no explicit modelPath
   • warns when local provider with default model but gateway probe reports not ready
   • does not warn when local provider with default model and gateway probe is ready
   • does not warn when local provider has an explicit hf: modelPath
   • treats SecretRef remote apiKey as configured for explicit provider
   • treats SecretRef remote apiKey as configured in auto mode
   • still warns in auto mode when only ollama credentials exist

2. 移除了未使用的 imports - 从 backend-config.test.ts 中删除了 execFile, promisify, 和 execFileAsync，这些在测试中从未被使用。

请再次审查，谢谢！

@chatgpt-codex-connector @greptile-apps 感谢review！已修复以下问题：

P1: Probe QMD binary from agent workspace

• checkQmdBinaryAvailable() 现在接受可选的 cwd 参数
• search-manager.ts 调用时传入 resolveAgentWorkspaceDir() 获取的workspace目录
• doctor-memory-search.ts 调用时传入 agentDir 作为cwd
• 这样确保相对路径（如 ./bin/qmd）能正确解析到agent workspace

P2: Preserve Windows command resolution

• 限制 .cmd 扩展名只添加到已知的shim名称（qmd, npm, npx）
• 避免破坏依赖PATHEXT解析的自定义可执行文件（如 my-qmd-wrapper → my-qmd-wrapper.exe）

所有测试通过 ✅

Commit: 00fc415d5

To use Codex here, create an environment for this repo.

Perfect! Both fixes look good:

✅ P1 - Agent workspace probing: The cwd parameter is correctly threaded through to execFileAsync, ensuring relative paths like ./bin/qmd resolve from the agent workspace directory in both search-manager.ts and doctor-memory-search.ts.

✅ P2 - Windows command resolution: The .cmd extension is now only added for known shims (qmd, npm, npx), preserving PATHEXT resolution for custom executables like my-qmd-wrapper.exe.

The tests cover both scenarios well, and the error messages are clear and actionable. Nice refactoring on the stableSerialize() helper too.

Closing as superseded by #57467. The diagnosis here was right, but the repo moved and the current fix had to be reworked onto today's QMD manager and doctor paths. Thanks @Jimmy-xuzimo for the original PR.