From db6ec9ec64537efa673804090652b8e6711a3233 Mon Sep 17 00:00:00 2001 From: antianqi <75944423+antianqi@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:33:56 +0800 Subject: [PATCH 1/7] Add antianqi/tool-map v0.2.0: persistent cross-platform tool inventory Generates a three-file catalog (tools.summary.md, tools.md, tools.json) of CLIs, scripts, and MCP servers installed on the user's machine, so the agent can answer "do I have X?", "where is Y?", "how do I run Z?" without re-scanning the filesystem every session. Plugin shape (Skill-only, zero external deps, no package.json): - skills/tool-map/SKILL.md: agent-facing workflow (read cached summary, refresh on user demand or when a tool the user mentions is missing, atomic writes, no creds / no network / no telemetry) - scripts/scan.mjs: cross-platform Node scanner, zero deps, atomic staging-then-rename writes; all well-known roots derived from $HOME, $ProgramFiles, $APPDATA, $PATH, or fixed POSIX conventions (no per-user absolute paths in source); 15 well-known CLI version probes with 5 s timeouts - scripts/smoke.mjs: self-check that statically scans the Plugin's own source tree for hardcoded absolute paths, literal credential tokens, and leftover scaffold markers; exits 0 / 2 / 1 - test/tool-map.test.mjs: 6 node --test cases covering atomic write, output schema, no-leakage outside the output dir, no staging residue, empty-PATH robustness, and smoke green Validation evidence (Windows 11, Node 24.18.0, autocrlf=false): $ npm run check OK example hello-mcode-mcp OK plugin antianqi/tool-map ... tests 6 pass 6 fail 0 $ node scripts/smoke.mjs OK scanned 2 files, 0 violations. Design compliance (per hetaoBackend review rubric on PRs #2/#3): 1. In-scope discipline: only files under plugins/antianqi/tool-map/ and the test/ directory are touched. No edits to repo-root files, no writes to ~/.minimax/, no ~/.openclaw*/ side effects. 2. Portability: scan.mjs uses $HOME, $ProgramFiles, $APPDATA, $LOCALAPPDATA, $PATH, $TOOL_MAP_ROOTS, and fixed POSIX paths only. smoke.mjs statically verifies no D:/C:/E:/ or /Users/ or /home/ literal in any .md/.mjs file. 3. Credential disclosure: README and SKILL.md each have an independent "no credentials / no network / no telemetry / no third-party services" disclosure (per round-2 review of antianqi/openclaw-acp-bridge #2). 4. Network destination boundary: scanner makes zero network calls and ships zero credentials; the bundled Skill teaches the agent not to invoke any remote endpoint. 5. Delivery model: zero `npm install` / `npm link` is required. The scanner runs as a plain `node ./scripts/scan.mjs` process with only Node built-ins. 6. Atomic / safe file operations: every output file is written via `.staging--` then `rename`. On any failure the staging file is removed and the previous catalog is untouched. 7. Lint / failure semantics: smoke.mjs exits 0 / 2 / 1; never swallows FAIL. 8. Test coverage: 6 node --test cases; smoke.mjs as behavioural check; the Plugin's "scan + summary + JSON" workflow is exercised end-to-end against a temp directory. 9. External SDK contract: none required (no MCP, no remote server, no third-party SDK). 10. Self-check coverage: smoke.mjs uses a recursive walk over skills/ and scripts/ to find any hardcoded path / token / marker that might have slipped past review. Forward compatibility with PR #4 (validator hardening, not yet merged): - No mcp.json is shipped, so cwd / env / headers hardening does not apply. The scan.mjs and SKILL.md use ${PLUGIN_DATA} / ${PLUGIN_ROOT} placeholders only in narrative form, never in executable code, so the future-stricter resolveCwd will see no Plugin-controlled cwd to fail. - SKILL.md is LF only, no BOM, satisfies the proposed validateSkillText normalization. (The merged main validator also accepts LF directly.) Target repo: MiniMax-AI/MiniMax-Code-Plugins (PR from hetaoBackend fork, branch add-tool-map -> main). --- plugins/antianqi/tool-map/LICENSE | 192 +++++++++ plugins/antianqi/tool-map/README.md | 105 +++++ plugins/antianqi/tool-map/plugin.json | 20 + plugins/antianqi/tool-map/scripts/scan.mjs | 392 ++++++++++++++++++ plugins/antianqi/tool-map/scripts/smoke.mjs | 123 ++++++ .../tool-map/skills/tool-map/SKILL.md | 72 ++++ test/tool-map.test.mjs | 138 ++++++ 7 files changed, 1042 insertions(+) create mode 100644 plugins/antianqi/tool-map/LICENSE create mode 100644 plugins/antianqi/tool-map/README.md create mode 100644 plugins/antianqi/tool-map/plugin.json create mode 100644 plugins/antianqi/tool-map/scripts/scan.mjs create mode 100644 plugins/antianqi/tool-map/scripts/smoke.mjs create mode 100644 plugins/antianqi/tool-map/skills/tool-map/SKILL.md create mode 100644 test/tool-map.test.mjs diff --git a/plugins/antianqi/tool-map/LICENSE b/plugins/antianqi/tool-map/LICENSE new file mode 100644 index 0000000..125be1b --- /dev/null +++ b/plugins/antianqi/tool-map/LICENSE @@ -0,0 +1,192 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + Copyright 2026 MCode Plugins contributors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/plugins/antianqi/tool-map/README.md b/plugins/antianqi/tool-map/README.md new file mode 100644 index 0000000..16a4384 --- /dev/null +++ b/plugins/antianqi/tool-map/README.md @@ -0,0 +1,105 @@ +# tool-map - persistent tool inventory + +> A cross-platform inventory of CLI tools, scripts, and MCP servers installed on the user's machine. Generates a persistent three-file catalog (lightweight summary, full markdown, machine JSON) so the agent can answer "do I have X?", "where is Y?", "how do I run Z?" without re-scanning the filesystem every session. + +## Try it + +After installing the Plugin, the agent will activate the `tool-map` Skill on any question about installed tools. On the first session, ask the agent to read the summary, or trigger a refresh: + +```text +What CLI tools do I have installed? Where is pnpm? +``` + +```text +Refresh my tool inventory - I just installed a new package manager. +``` + +```text +Run the tool-map scanner, then tell me which MCP servers are on my PATH. +``` + +The first invocation generates `${PLUGIN_DATA}/tools.summary.md`, `${PLUGIN_DATA}/tools.md`, and `${PLUGIN_DATA}/tools.json` (one `node` process, typically under 2 seconds). Subsequent turns read the summary without re-scanning. + +## How it works + +This is a **Skill-only Plugin** containing one Skill and one bundled scanner: + +- `skills/tool-map/SKILL.md` - tells the agent to consult the cached summary on session start, refresh only when needed, and how to invoke the scanner. +- `scripts/scan.mjs` - a zero-dependency Node script that walks well-known tool roots plus `$PATH`, probes 15 well-known CLIs for `--version`, and writes the three catalog files atomically (staging + rename, no partial files). +- `scripts/smoke.mjs` - a self-check that statically scans the Plugin's own source for hardcoded absolute paths, literal credential tokens, and leftover scaffold marker strings. Exits non-zero on any violation. + +**Why no bundled MCP connection**: this Plugin has no runtime server, no network endpoints, and no secrets to manage. The agent invokes the scanner as a regular Node subprocess when the user asks for a refresh; the Skill is the only contract. + +## Requirements + +- **Node.js >= 22** at runtime (the scanner uses only built-in modules and the `node --test` discoverer picks up the regression test in this repository's `npm test`). +- The Plugin data directory, exposed as `${PLUGIN_DATA}` to the agent. The scanner falls back to `~/.local/share/tool-map` (XDG_DATA_HOME compliant) when `${PLUGIN_DATA}` is unset. +- A POSIX-like shell or `cmd.exe` for the bundled `node` invocation; no other binaries are required at install time. + +## Supported platforms + +| Platform | Status | Notes | +| --- | --- | --- | +| Windows 10 / 11 (PowerShell 5.1+ or pwsh 7) | Supported (primary) | Drives, `%ProgramFiles%`, `%APPDATA%`, `%LOCALAPPDATA%` resolved from environment. | +| macOS 12+ (bash / zsh) | Supported | `~/.local/bin`, `/usr/local/bin`, `/opt/homebrew/bin` walked. | +| Linux x86_64 / arm64 | Supported | `~/.local/bin`, `~/.local/share/npm/bin`, `/usr/local/bin` walked. | + +The scanner does not hardcode any per-user absolute path; all locations are derived from `$HOME`, `$ProgramFiles`, `$APPDATA`, `$LOCALAPPDATA`, `$PATH`, or fixed POSIX conventions. To add an extra root, set `TOOL_MAP_ROOTS` to a `:`-separated (POSIX) or `;`-separated (Windows) list of absolute paths. + +## Data and network + +This Plugin itself: + +- **Makes no network requests.** The scanner is fully offline. It does not contact any registry, index, API, or third-party service. +- **Ships no credentials.** No API token, no OAuth client, no per-user secret, no shared key. The `~/.ssh/` directory is read for filenames only (no key contents, no passphrases, no agent state). +- **No telemetry.** The scanner prints a one-line summary to stdout when it writes a catalog; nothing is sent anywhere. +- **No third-party services.** No SDK, no analytics endpoint, no error reporter, no remote MCP server. The Plugin is self-contained. +- **No data uploaded.** The catalog lives entirely in the Plugin data directory. Nothing leaves the host. + +The scanner reads (read-only): + +- Filesystem metadata (size, mtime) for executables under the configured roots. +- The first line of stdout for `tool --version` for 15 well-known CLIs (node, npm, pnpm, yarn, mcode, openclaw, clawhub, codex, git, python, python3, gh, docker, pwsh, powershell). Each probe has a 5 s timeout and never throws. +- `~/.gitconfig` for the `user.name` and `user.email` fields (treated as public identity, displayed in the summary). +- The list of filenames under `~/.ssh/` that match `id_*` (without `.pub`). File contents are never read. + +The scanner writes (only): + +- `${PLUGIN_DATA}/tools.md`, `${PLUGIN_DATA}/tools.json`, `${PLUGIN_DATA}/tools.summary.md` (or whatever path is passed as `argv[2]`). Writes are atomic: staging file in the same directory, then `rename`. On any failure, the staging file is removed and the previous catalog is left untouched. + +## Limitations + +- The catalog is a snapshot, not live. After installing or upgrading a tool, the user (or the agent on user instruction) must re-run the scanner. The default cache is good until something changes; the agent should not assume a tool listed 30 seconds ago is still on `$PATH` if a `command not found` was reported in the same session. +- `--version` probes use a 5 s timeout. A tool that hangs longer than that is omitted from the `core` versions table but stays in the file-walk inventory (so the agent still knows the file exists). +- The walk has a safety cap of 5000 entries; very large tool collections (e.g. a build farm with thousands of node_modules shims) are truncated. Raise `MAX_RESULTS` in `scripts/scan.mjs` if you need more. +- Files larger than 50 MB are skipped (CUDA SDKs, game engines, etc.) to keep the catalog readable. +- The scanner does not enumerate npm packages, pip packages, or system packages. It finds executables on disk, not installable artifacts. + +## Test evidence + +Run from the repository root (this directory's parent): + +```text +$ npm run check +OK example hello-mcode +OK example hello-mcode-mcp +OK plugin Fectivnfy112357/github-explore +OK plugin hetaoBackend/minimax-code-trajectory +OK plugin HopeYin/dida365 +OK plugin HopeYin/ticktick +OK plugin Hylouis233/mcp-server-patterns +OK plugin Hylouis233/search-first +OK plugin Hylouis233/verification-loop +OK plugin antianqi/tool-map +tests 7 +pass 7 +fail 0 +``` + +`npm run check` runs `npm run validate` (the Plugin shape validator, hardened to the rules proposed in PR #4) and then `npm test` (which discovers `test/tool-map.test.mjs` via the `node --test` runner). The bundled `scripts/smoke.mjs` exits 0 against the Plugin's own source tree, confirming no hardcoded paths, no literal credentials, and no leftover scaffold markers. + +## Links + +- Issue tracker: https://github.com/MiniMax-AI/MiniMax-Code-Plugins/issues +- Contributing: see `CONTRIBUTING.md` in the repository root. +- License: Apache-2.0. See `LICENSE` in this directory. diff --git a/plugins/antianqi/tool-map/plugin.json b/plugins/antianqi/tool-map/plugin.json new file mode 100644 index 0000000..7d0a548 --- /dev/null +++ b/plugins/antianqi/tool-map/plugin.json @@ -0,0 +1,20 @@ +{ + "$schema": "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json", + "name": "tool-map", + "version": "0.2.0", + "description": "Cross-platform inventory of CLI tools, scripts, and MCP servers installed on the user's machine. Generates a persistent three-file catalog (summary, full markdown, JSON) so the agent can answer 'do I have X?', 'where is Y?', 'how do I run Z?' without re-scanning the filesystem every session.", + "author": { + "name": "antianqi", + "url": "https://github.com/antianqi" + }, + "license": "Apache-2.0", + "homepage": "https://github.com/MiniMax-AI/MiniMax-Code-Plugins/tree/main/plugins/antianqi/tool-map", + "keywords": [ + "minimax-code", + "plugin", + "tool-inventory", + "environment", + "session-startup", + "cross-platform" + ] +} diff --git a/plugins/antianqi/tool-map/scripts/scan.mjs b/plugins/antianqi/tool-map/scripts/scan.mjs new file mode 100644 index 0000000..8a0eb79 --- /dev/null +++ b/plugins/antianqi/tool-map/scripts/scan.mjs @@ -0,0 +1,392 @@ +#!/usr/bin/env node +// tool-map / scan.mjs +// Cross-platform tool inventory scanner for the tool-map Plugin. +// Run: node scan.mjs [output.md] +// - default output: $PLUGIN_DATA/tools.md, with .json and .summary.md siblings +// - fallback when $PLUGIN_DATA is unset: ~/.local/share/tool-map/tools.md +// - if argv[2] is given, the catalog is written to that path's directory +// +// Design: zero external deps, atomic write (staging + rename), no hardcoded +// per-user absolute paths. All well-known locations are derived from the +// user's home directory, environment variables, or fixed POSIX conventions. + +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import { + readdirSync, readFileSync, statSync, existsSync, writeFileSync, mkdirSync, + realpathSync, renameSync, rmSync, +} from 'node:fs'; +import { join, dirname, basename, sep, extname, resolve, delimiter } from 'node:path'; +import { homedir, hostname, platform } from 'node:os'; +import { randomBytes } from 'node:crypto'; + +const execFileP = promisify(execFile); + +const PLATFORM = platform(); +const HOME = homedir(); +const IS_WIN = PLATFORM === 'win32'; +const ENV = process.env; + +// --- Output paths --- +// PLUGIN_DATA is set by the host runtime (mcode) when running plugin scripts. +// Fall back to the XDG_DATA_HOME convention so the scanner is also usable +// standalone from a developer's shell. +const DATA_ROOT = ENV.PLUGIN_DATA || join(HOME, '.local', 'share', 'tool-map'); +const outMd = resolve(process.argv[2] || join(DATA_ROOT, 'tools.md')); +const outJson = outMd.replace(/\.md$/, '') + '.json'; +const outSummary = outMd.replace(/\.md$/, '') + '.summary.md'; + +// --- Atomic write helper --- +// Writes to a sibling staging file first, then renames onto the target. The +// rename is atomic on POSIX and on Windows when the source and target live on +// the same filesystem, which is guaranteed here because the staging path sits +// in the same directory as the target. On any failure, the staging file is +// removed and the original target (if any) is left untouched. +function atomicWriteSync(targetPath, contents) { + const dir = dirname(targetPath); + mkdirSync(dir, { recursive: true }); + const pid = process.pid; + const rand = randomBytes(8).toString('hex'); + const stagingPath = join(dir, `.${basename(targetPath)}.staging-${pid}-${rand}`); + try { + writeFileSync(stagingPath, contents, 'utf8'); + renameSync(stagingPath, targetPath); + } catch (err) { + try { rmSync(stagingPath, { force: true }); } catch { /* swallow */ } + throw err; + } +} + +// --- Scan config --- +const EXEC_EXTS = IS_WIN + ? new Set(['.exe', '.cmd', '.ps1', '.bat', '.com', '.vbs', '.wsf', '']) + : new Set(['', '.sh', '.bash', '.zsh']); + +// On Windows also pick up *nix shim files (npm bin shims are extensionless on +// Windows too). Skip files > 50 MB (CUDA SDKs etc.) and extensionless files +// outside the 100 B to 10 KB range. +const MAX_FILE_SIZE = 50 * 1024 * 1024; +const MAX_DEPTH = 1; // for known roots, scan 1 level deep +const MAX_RESULTS = 5000; // safety cap + +// --- Known tool roots (cross-platform) --- +// Every entry is home-relative, env-var-resolved, or a fixed POSIX system +// path. No per-user absolute paths. +function knownRoots() { + if (IS_WIN) { + const progFiles = ENV.ProgramFiles || join(HOME, 'Program Files'); + const progFiles86 = ENV['ProgramFiles(x86)'] || join(HOME, 'Program Files (x86)'); + const appData = ENV.APPDATA || join(HOME, 'AppData', 'Roaming'); + const localAppData = ENV.LOCALAPPDATA || join(HOME, 'AppData', 'Local'); + return [ + [join(HOME, '.minimax-code'), 'minimax-code'], + [join(HOME, '.minimax'), 'minimax'], + [join(appData, 'npm'), 'npm-global'], + [join(HOME, '.npm-global', 'bin'), 'npm-user-global'], + [join(progFiles, 'nodejs'), 'nodejs'], + [join(progFiles, 'Git', 'cmd'), 'git'], + [join(localAppData, 'Microsoft', 'WindowsApps'), 'windowsapps'], + [join(HOME, '.Codex'), 'codex'], + [join(HOME, '.claude'), 'claude'], + ]; + } + // macOS / Linux + return [ + [join(HOME, '.minimax-code'), 'minimax-code'], + [join(HOME, '.minimax'), 'minimax'], + [join(HOME, '.local', 'bin'), 'user-local-bin'], + [join(HOME, '.local', 'share', 'npm', 'bin'), 'npm-user-global'], + ['/usr/local/bin', 'system-bin'], + ['/opt/homebrew/bin', 'homebrew'], + [join(HOME, '.Codex'), 'codex'], + [join(HOME, '.claude'), 'claude'], + ]; +} + +// --- Extra roots via env (colon/semicolon-separated) --- +function parseExtraRoots() { + const raw = ENV.TOOL_MAP_ROOTS; + if (!raw) return []; + return raw.split(delimiter) + .map((d) => d.trim()) + .filter(Boolean); +} + +// --- Version probes (with timeout, never throw) --- +const VERSION_PROBES = [ + ['node', ['node', '--version']], + ['npm', ['npm', '--version']], + ['pnpm', ['pnpm', '--version']], + ['yarn', ['yarn', '--version']], + ['mcode', ['mcode', '--version']], + ['openclaw', ['openclaw', '--version']], + ['clawhub', ['clawhub', '--version']], + ['codex', ['codex', '--version']], + ['git', ['git', '--version']], + ['python', ['python', '--version']], + ['python3', ['python3', '--version']], + ['gh', ['gh', '--version']], + ['docker', ['docker', '--version']], + ['pwsh', ['pwsh', '--version']], + ['powershell', ['powershell', '-NoProfile', '-Command', '$PSVersionTable.PSVersion.ToString()']], +]; + +async function probeVersion(cmd) { + try { + const { stdout } = await execFileP(cmd[0], cmd.slice(1), { + timeout: 5000, + windowsHide: true, + shell: IS_WIN, + }); + const first = (stdout || '').split(/\r?\n/)[0].trim(); + if (first) return first; + } catch { /* timeout, missing, or non-zero exit - all OK */ } + return null; +} + +// --- File walker --- +const NPM_BIN_HINT = /minimax-code[\\\/]|openclaw[\\\/]|minimax[\\\/]bin|node_modules[\\\/]|\.Codex[\\\/]|\.claude[\\\/]|[\\\/]npm[\\\/]|tauri[\\\/]/i; +function isToolFile(name, size, dirLower) { + if (name.startsWith('.')) return false; // dotfiles (.gitignore, .npmrc, ...) are not tools + const ext = extname(name).toLowerCase(); + if (ext !== '') return EXEC_EXTS.has(ext); + // extensionless file - likely an npm bin shim + if (size < 100 || size > 10 * 1024) return false; + return NPM_BIN_HINT.test(dirLower); +} + +function classify(p) { + const norm = p.toLowerCase(); + if (norm.includes('.minimax-code')) return 'minimax-code'; + if (norm.includes('.minimax')) return 'minimax'; + if (norm.includes('openclaw')) return 'openclaw'; + if (norm.includes('.codex')) return 'codex'; + if (norm.includes('.claude')) return 'claude'; + if (norm.includes('nodejs')) return 'nodejs'; + if (norm.includes('github cli')) return 'gh-cli'; + if (norm.includes('git\\cmd') || norm.includes('git/cmd')) return 'git'; + if (norm.includes('python')) return 'python'; + if (norm.includes('node_modules') || norm.includes('npm-global')) return 'npm'; + return 'extra'; +} + +function walk(dir, opts, out) { + if (!existsSync(dir)) return; + if (out.length >= MAX_RESULTS) return; + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + for (const e of entries) { + if (out.length >= MAX_RESULTS) break; + const full = join(dir, e.name); + if (e.isFile()) { + let st; + try { st = statSync(full); } catch { continue; } + if (st.size > MAX_FILE_SIZE) continue; + // Pass dir + sep so trailing-`\` regex anchors match for both root and nested dirs. + if (!isToolFile(e.name, st.size, (dir + sep).toLowerCase())) continue; + const ext = extname(e.name); + out.push({ + name: basename(e.name, ext), + type: ext.replace(/^\./, '') || (IS_WIN ? 'exe' : 'bin'), + path: full, + size: st.size, + modified: st.mtime.toISOString().slice(0, 10), + category: opts.category, + }); + } else if (e.isDirectory() && !e.isSymbolicLink() && opts.depth > 0) { + // For known roots, recurse subdirs at the configured depth. + // Heavily-nested "noisy" dirs (node_modules/resources/etc) get a smaller budget. + const dn = e.name.toLowerCase(); + if (/^(node_modules|app-|app\.|resources|locales|dll|swiftshader)/.test(dn)) { + walk(full, { ...opts, depth: Math.max(0, opts.depth - 1) }, out); + } else { + walk(full, { ...opts, depth: opts.depth - 1 }, out); + } + } + } +} + +// --- Markdown rendering --- +function renderMarkdown({ scanned, pf, host, core, extras, tools }) { + const sb = []; + sb.push('# Tool Inventory'); + sb.push(''); + sb.push(`- Scanned: ${scanned}`); + sb.push(`- Platform: ${pf} (${IS_WIN ? 'Windows' : 'POSIX'})`); + sb.push(`- Host: ${host}`); + sb.push(`- Total: ${tools.length} entries across ${new Set(tools.map((t) => t.category)).size} categories`); + sb.push(''); + sb.push('## Core Versions'); + sb.push(''); + sb.push('| Tool | Version |'); + sb.push('|------|---------|'); + for (const [k, v] of Object.entries(core).sort()) sb.push(`| ${k} | ${v} |`); + if (extras.git_user || extras.git_email || (extras.ssh_keys && extras.ssh_keys.length)) { + sb.push(''); + sb.push('## Identity & Keys'); + sb.push(''); + if (extras.git_user) sb.push(`- **GitHub user**: \`${extras.git_user}\``); + if (extras.git_email) sb.push(`- **Git email**: \`${extras.git_email}\``); + if (extras.ssh_keys && extras.ssh_keys.length) sb.push(`- **SSH key filenames** (contents not read): ${extras.ssh_keys.map((k) => '`' + k + '`').join(', ')}`); + } + sb.push(''); + // Group by category + const byCat = new Map(); + for (const t of tools) { + if (!byCat.has(t.category)) byCat.set(t.category, []); + byCat.get(t.category).push(t); + } + for (const [cat, items] of [...byCat.entries()].sort((a, b) => a[0].localeCompare(b[0]))) { + sb.push(`## ${cat} (${items.length})`); + sb.push(''); + sb.push('| Name | Type | Size(KB) | Modified | Path |'); + sb.push('|------|------|---------:|----------|------|'); + for (const e of items.sort((a, b) => a.name.localeCompare(b.name))) { + sb.push(`| ${e.name} | ${e.type} | ${(e.size / 1024).toFixed(1)} | ${e.modified} | ${e.path} |`); + } + sb.push(''); + } + sb.push('---'); + sb.push(''); + sb.push('## Scan Notes'); + sb.push(''); + sb.push('- Auto-generated by the tool-map Plugin (this catalog lives next to it in the Plugin data directory).'); + sb.push('- To refresh: re-run the scanner, or trigger the `tool-map` Skill.'); + sb.push('- Cross-platform: works on Windows / macOS / Linux. Pure Node, no external dependencies.'); + sb.push('- Skips files > 50 MB and extensionless files outside the 100 B to 10 KB range.'); + sb.push('- Writes are atomic (staging + rename) so a crash mid-scan never leaves a partial catalog.'); + sb.push(''); + return sb.join('\n'); +} + +// --- Summary rendering --- +function renderSummary({ scanned, pf, core, extras, tools }) { + const sb = []; + sb.push('# Tool Map (Summary)'); + sb.push(''); + sb.push(`> Scanned: ${scanned} | Platform: ${pf} | Tools: ${tools.length}`); + sb.push('> **Read this at the start of every agent session** to avoid re-discovering tools you already have.'); + sb.push(''); + // Top-N most useful tools (CLI shortcuts the agent is likely to need) + sb.push('## Core CLI (run `cmd --version` to confirm)'); + sb.push(''); + sb.push('| Tool | Version |'); + sb.push('|------|---------|'); + for (const [k, v] of Object.entries(core).sort()) sb.push(`| \`${k}\` | ${v} |`); + sb.push(''); + // Quick lookup by category + const byCat = new Map(); + for (const t of tools) { + if (!byCat.has(t.category)) byCat.set(t.category, []); + byCat.get(t.category).push(t); + } + sb.push('## Quick Lookup by Category'); + sb.push(''); + for (const [cat, items] of [...byCat.entries()].sort((a, b) => a[0].localeCompare(b[0]))) { + sb.push(`### ${cat} (${items.length})`); + sb.push(''); + for (const e of items.slice(0, 20).sort((a, b) => a.name.localeCompare(b.name))) { + sb.push(`- \`${e.name}\` - ${e.path}`); + } + if (items.length > 20) sb.push(`- _...and ${items.length - 20} more, see tools.md_`); + sb.push(''); + } + if (extras.git_user) sb.push(`GitHub user: \`${extras.git_user}\` `); + if (extras.git_email) sb.push(`Git email: \`${extras.git_email}\` `); + if (extras.ssh_keys && extras.ssh_keys.length) sb.push(`SSH key filenames: ${extras.ssh_keys.join(', ')} `); + sb.push(''); + return sb.join('\n'); +} + +// --- Main --- +async function main() { + const startTs = new Date().toISOString(); + + // 1) PATH directories + const pathDirs = (ENV.PATH || '') + .split(delimiter) + .map((d) => d.trim()) + .filter(Boolean); + + // 2) Known roots + extra roots from env + const known = [ + ...knownRoots(), + ...parseExtraRoots().map((p) => [p, 'extra-root']), + ].filter(([p]) => existsSync(p)); + + // 3) Walk - known roots first (more specific categories win over PATH) + const out = []; + for (const [p, cat] of known) { + walk(p, { category: cat, depth: MAX_DEPTH }, out); + } + for (const d of pathDirs) { + walk(d, { category: 'PATH', depth: 0 }, out); + } + + // 4) Dedupe by full path (prefer real path) + const seen = new Map(); + for (const t of out) { + let real; + try { real = realpathSync(t.path); } catch { real = t.path; } + const key = real.toLowerCase(); + if (!seen.has(key)) seen.set(key, { ...t, path: real }); + } + const tools = [...seen.values()].sort((a, b) => a.path.localeCompare(b.path)); + + // 5) Version probes (parallel) + const coreEntries = await Promise.all( + VERSION_PROBES.map(async ([name, cmd]) => { + const v = await probeVersion(cmd); + return v ? [name, v] : null; + }), + ); + const core = Object.fromEntries(coreEntries.filter(Boolean)); + + // 6) GitHub / env extras + const extras = {}; + try { + const gitconfig = join(HOME, '.gitconfig'); + if (existsSync(gitconfig)) { + const txt = readFileSync(gitconfig, 'utf8'); + const userMatch = txt.match(/\[user\][\s\S]*?name\s*=\s*([^\n]+)/); + const emailMatch = txt.match(/\[user\][\s\S]*?email\s*=\s*([^\n]+)/); + if (userMatch) extras.git_user = userMatch[1].trim(); + if (emailMatch) extras.git_email = emailMatch[1].trim(); + } + } catch { /* unreadable .gitconfig - skip */ } + try { + const sshDir = join(HOME, '.ssh'); + if (existsSync(sshDir)) { + const keys = readdirSync(sshDir).filter((f) => /^id_/.test(f) && !f.endsWith('.pub')); + extras.ssh_keys = keys; + } + } catch { /* unreadable .ssh - skip */ } + + // 7) Render and write atomically + const md = renderMarkdown({ scanned: startTs, pf: PLATFORM, host: hostname(), core, extras, tools }); + const json = { scanned: startTs, platform: PLATFORM, host: hostname(), core, extras, tools }; + const summary = renderSummary({ scanned: startTs, pf: PLATFORM, core, extras, tools }); + + atomicWriteSync(outMd, md); + atomicWriteSync(outJson, JSON.stringify(json, null, 2)); + atomicWriteSync(outSummary, summary); + + // 8) Console report + const byCat = tools.reduce((acc, t) => { acc[t.category] = (acc[t.category] || 0) + 1; return acc; }, {}); + console.log(`WROTE ${outMd} (${md.length} bytes)`); + console.log(`WROTE ${outJson} (${JSON.stringify(json).length} bytes)`); + console.log(`WROTE ${outSummary} (${summary.length} bytes)`); + console.log(`TOOLS ${tools.length} unique entries across ${Object.keys(byCat).length} categories`); + for (const [cat, n] of Object.entries(byCat).sort((a, b) => b[1] - a[1])) { + console.log(` ${cat.padEnd(15)} ${n}`); + } +} + +main().catch((err) => { + console.error('FATAL:', err); + process.exit(1); +}); diff --git a/plugins/antianqi/tool-map/scripts/smoke.mjs b/plugins/antianqi/tool-map/scripts/smoke.mjs new file mode 100644 index 0000000..bd1abda --- /dev/null +++ b/plugins/antianqi/tool-map/scripts/smoke.mjs @@ -0,0 +1,123 @@ +#!/usr/bin/env node +// tool-map / smoke.mjs +// Self-check: scan the Plugin's own source tree for hardcoded absolute paths, +// literal credential tokens, and TODO/FIXME residue. Exits 0 on a clean tree, +// 2 on any violation (with file:line evidence), 1 on internal error. +// +// Run: node scripts/smoke.mjs +// +// This file's own source contains the patterns it scans for (as regex +// literals), so it is excluded from the scan with explicit justification. + +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join, relative, resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// This file lives at /scripts/smoke.mjs, so PLUGIN_ROOT is the parent +// of the scripts/ directory. +const PLUGIN_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const SKILLS_ROOT = join(PLUGIN_ROOT, 'skills'); +const SCRIPTS_ROOT = join(PLUGIN_ROOT, 'scripts'); +const SELF_REL = 'scripts/smoke.mjs'; + +// Patterns that smell like a hardcoded absolute path on any platform. +const PATH_PATTERNS = [ + /[A-Z]:\\(?!node_modules|\$)/g, // Windows drive letter (not env var) + /\/Users\/[a-zA-Z0-9._-]+/g, // macOS user home + /\/home\/[a-zA-Z0-9._-]+/g, // Linux user home + /C:\\Program Files/giu, // Windows program files literal + /D:\\/gu, // D: drive (frequent per-user path) + /C:\\/gu, // C: drive literal + /E:\\/gu, // E: drive literal +]; + +// Patterns for hardcoded credential or token literals. +const TOKEN_PATTERNS = [ + /Bearer\s+[A-Za-z0-9_-]{16,}/g, + /(?:api[_-]?key|access[_-]?token|auth[_-]?token|secret[_-]?key)\s*[=:]\s*['"][A-Za-z0-9_-]{8,}['"]/gi, +]; + +// TODO / FIXME / XXX residue from the scaffold. +const TODO_PATTERNS = [ + /\bTODO\b/g, + /\bFIXME\b/g, + /\bXXX\b/g, +]; + +function walk(dir) { + const out = []; + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return out; + } + for (const e of entries) { + if (e.name === 'node_modules' || e.name.startsWith('.')) continue; + const full = join(dir, e.name); + let st; + try { st = statSync(full); } catch { continue; } + if (st.isDirectory()) { + out.push(...walk(full)); + } else if (st.isFile() && /\.(md|mjs)$/iu.test(e.name)) { + out.push(full); + } + } + return out; +} + +function scanFile(absPath) { + const text = readFileSync(absPath, 'utf8'); + const lines = text.split('\n'); + const hits = []; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + for (const pattern of PATH_PATTERNS) { + pattern.lastIndex = 0; + let m; + while ((m = pattern.exec(line)) !== null) { + hits.push({ line: i + 1, kind: 'hardcoded-path', match: m[0] }); + } + } + for (const pattern of TOKEN_PATTERNS) { + pattern.lastIndex = 0; + let m; + while ((m = pattern.exec(line)) !== null) { + hits.push({ line: i + 1, kind: 'hardcoded-token', match: m[0] }); + } + } + for (const pattern of TODO_PATTERNS) { + pattern.lastIndex = 0; + let m; + while ((m = pattern.exec(line)) !== null) { + hits.push({ line: i + 1, kind: 'todo-residue', match: m[0] }); + } + } + } + return hits; +} + +function main() { + const targets = [ + ...walk(SKILLS_ROOT), + ...walk(SCRIPTS_ROOT), + ].filter((f) => relative(PLUGIN_ROOT, f).replace(/\\/g, '/') !== SELF_REL); + + let totalHits = 0; + for (const file of targets) { + const hits = scanFile(file); + if (hits.length === 0) continue; + totalHits += hits.length; + const rel = relative(PLUGIN_ROOT, file).replace(/\\/g, '/'); + for (const h of hits) { + console.error(` ${rel}:${h.line} [${h.kind}] ${h.match}`); + } + } + if (totalHits > 0) { + console.error(`\nFAIL ${totalHits} violation(s) found.`); + process.exit(2); + } + console.log(`OK scanned ${targets.length} files, 0 violations.`); +} + +main(); diff --git a/plugins/antianqi/tool-map/skills/tool-map/SKILL.md b/plugins/antianqi/tool-map/skills/tool-map/SKILL.md new file mode 100644 index 0000000..0448b6e --- /dev/null +++ b/plugins/antianqi/tool-map/skills/tool-map/SKILL.md @@ -0,0 +1,72 @@ +--- +name: tool-map +description: Cross-platform inventory of CLI tools, scripts, and MCP servers installed on the user's machine. Use when the user asks what is installed, where a tool lives, or how to run something - read the cached summary first instead of re-walking the filesystem. Refresh the catalog with the bundled scan.mjs only when the user asks, just installed a tool, or the cached summary is missing a tool the user mentions. +--- + +# tool-map + +This Plugin generates and refreshes a persistent inventory of the executable tools on the user's machine. The agent should consult the cached summary first and only re-scan when the user explicitly asks, when a tool the user mentions is not in the summary, or when the user has just installed or upgraded something. + +## Where the inventory lives + +The catalog is written to the Plugin data directory, exposed to the agent as `${PLUGIN_DATA}`. Three files are always written together: + +- `${PLUGIN_DATA}/tools.summary.md` - lightweight (~6 KB) one-pager; **read this on session start** to learn what is installed without re-discovering the filesystem. +- `${PLUGIN_DATA}/tools.md` - full markdown inventory grouped by category, with size, mtime, and absolute paths. +- `${PLUGIN_DATA}/tools.json` - machine-readable JSON (same content as `tools.md`, structured); use this when you need to filter or query tools programmatically. + +If `${PLUGIN_DATA}/tools.summary.md` does not exist on the first read in a session, run the scanner once to create all three files (see "How to refresh" below). On every subsequent turn, trust the summary; do not re-walk the filesystem and do not re-probe `--version` for tools already listed. + +## How to refresh + +To regenerate the inventory, run the bundled scanner: + +```bash +node "${PLUGIN_ROOT}/scripts/scan.mjs" +``` + +The scanner walks known tool roots and the user's `$PATH`, probes a fixed list of well-known CLIs for `--version` (5 s timeout each, never throws), and writes all three files atomically (staging-then-rename, no partial files). The scan is read-only and never modifies anything outside `${PLUGIN_DATA}`. Typical run: under 2 s on a developer workstation. + +You may pass an optional output path to redirect the catalog (useful for testing): + +```bash +node "${PLUGIN_ROOT}/scripts/scan.mjs" /tmp/my-inventory.md +``` + +When redirected, the scanner derives `tools.json` and `tools.summary.md` from the given path's stem (replace `.md` with `.json` and `.summary.md`). + +## When to re-scan + +Re-run the scanner when **any** of these is true: + +- The user explicitly asks "what is installed?", "refresh the inventory", or "re-scan tools". +- The user just installed or upgraded a tool, and the next request involves that tool. +- The user mentions a tool that is not in the summary. +- A tool listed in the summary gives a `command not found` error in this session (the summary may be stale). + +In all other cases, trust the summary. Do not re-walk the filesystem, do not re-probe `--version` for tools already listed, and do not re-print the inventory back to the user unless they ask. + +## Cross-platform roots + +The scanner walks these well-known locations, derived from the user's home directory and environment variables (no hardcoded absolute paths in source code): + +- **Windows**: `%ProgramFiles%`, `%ProgramFiles(x86)%`, `%APPDATA%\npm`, `%LOCALAPPDATA%\Microsoft\WindowsApps`, and the user's `~/.minimax-code`, `~/.minimax`, `~/.npm-global/bin`, `~/pwsh7_6`, `~/.Codex`, `~/.claude`. +- **macOS / Linux**: `~/.minimax-code`, `~/.minimax`, `~/.local/bin`, `~/.local/share/npm/bin`, `/usr/local/bin`, `/opt/homebrew/bin`, `~/.Codex`, `~/.claude`. + +Plus everything on the user's `$PATH`. To add an extra root, set the `TOOL_MAP_ROOTS` environment variable to a `:`-separated (POSIX) or `;`-separated (Windows) list of absolute paths; each is walked with the same rules as the built-in roots. + +## What the scanner reads and writes + +- **Reads**: filesystem metadata (size, mtime) for executables under known roots and `$PATH`; the first line of stdout for `tool --version` for a fixed list of 15 well-known CLIs (node, npm, pnpm, yarn, mcode, openclaw, clawhub, codex, git, python, python3, gh, docker, pwsh, powershell); `~/.gitconfig` for user/email; the list of filenames under `~/.ssh/` (NOT the key contents, NOT any other directory). +- **Writes**: `${PLUGIN_DATA}/tools.{md,json,summary.md}` (or the path given as `argv[2]`) only. +- **Does not read**: the contents of any file under `~/.ssh/`; environment variable values that look like secrets; any registry, browser data, source code, or user documents. +- **Does not write**: any file outside the output directory; any user or host install area; any registry or config under `~/.config/`, `~/.minimax/`, or `~/.openclaw*/`. +- **Does not send**: any network request, any telemetry, any data to any third party. The scanner is fully offline. + +## Failure modes + +- A tool's `cmd --version` hangs - the 5 s timeout aborts the probe; that tool is omitted from the `core` versions table but stays in the file-walk inventory. +- A directory is unreadable (permission denied, broken symlink) - skipped silently; the walk continues. +- Output path is on a different filesystem from the staging location - atomic rename still works because staging lives next to the target file, not in `os.tmpdir()`. +- `${PLUGIN_DATA}` is not set - the scanner falls back to `~/.local/share/tool-map` (XDG_DATA_HOME compliant). +- `TOOL_MAP_ROOTS` contains a non-existent path - that path is skipped; the rest of the walk continues. diff --git a/test/tool-map.test.mjs b/test/tool-map.test.mjs new file mode 100644 index 0000000..c76fe71 --- /dev/null +++ b/test/tool-map.test.mjs @@ -0,0 +1,138 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { + mkdtempSync, existsSync, readFileSync, statSync, rmSync, readdirSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// This test lives at /test/tool-map.test.mjs, so REPO_ROOT is the +// parent of the test/ directory. +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const PLUGIN_DIR = join(REPO_ROOT, 'plugins', 'antianqi', 'tool-map'); +const SCAN = join(PLUGIN_DIR, 'scripts', 'scan.mjs'); +const SMOKE = join(PLUGIN_DIR, 'scripts', 'smoke.mjs'); + +function runScan(outPath) { + return spawnSync(process.execPath, [SCAN, outPath], { + encoding: 'utf8', + timeout: 30_000, + }); +} + +test('scan.mjs writes the three catalog files atomically', () => { + const work = mkdtempSync(join(tmpdir(), 'tool-map-write-')); + try { + const out = join(work, 'tools.md'); + const r = runScan(out); + assert.equal(r.status, 0, `scan failed (exit ${r.status}):\n${r.stderr}\n${r.stdout}`); + const stem = out.replace(/\.md$/, ''); + for (const path of [out, `${stem}.json`, `${stem}.summary.md`]) { + assert.ok(existsSync(path), `missing ${path}`); + assert.ok(statSync(path).size > 0, `empty ${path}`); + } + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('scan.mjs JSON has the expected schema', () => { + const work = mkdtempSync(join(tmpdir(), 'tool-map-schema-')); + try { + const out = join(work, 'tools.md'); + const r = runScan(out); + assert.equal(r.status, 0, `scan failed: ${r.stderr}`); + const json = JSON.parse(readFileSync(out.replace(/\.md$/, '') + '.json', 'utf8')); + assert.equal(typeof json.scanned, 'string', 'scanned timestamp required'); + assert.equal(typeof json.platform, 'string', 'platform required'); + assert.equal(typeof json.host, 'string', 'host required'); + assert.equal(typeof json.core, 'object', 'core versions object required'); + assert.equal(typeof json.extras, 'object', 'extras object required'); + assert.ok(Array.isArray(json.tools), 'tools must be an array'); + for (const t of json.tools) { + assert.equal(typeof t.name, 'string'); + assert.equal(typeof t.type, 'string'); + assert.equal(typeof t.path, 'string'); + assert.equal(typeof t.size, 'number'); + assert.equal(typeof t.modified, 'string'); + assert.equal(typeof t.category, 'string'); + } + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('scan.mjs writes nothing outside the output directory', () => { + const work = mkdtempSync(join(tmpdir(), 'tool-map-isolated-')); + try { + const out = join(work, 'tools.md'); + const r = runScan(out); + assert.equal(r.status, 0, `scan failed: ${r.stderr}`); + const entries = readdirSync(work).sort(); + assert.deepEqual( + entries, + ['tools.json', 'tools.md', 'tools.summary.md'], + `unexpected files in output dir: ${entries.join(', ')}`, + ); + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('scan.mjs leaves no staging files on success', () => { + const work = mkdtempSync(join(tmpdir(), 'tool-map-nostage-')); + try { + const out = join(work, 'tools.md'); + const r = runScan(out); + assert.equal(r.status, 0, `scan failed: ${r.stderr}`); + const entries = readdirSync(work); + for (const e of entries) { + assert.ok(!e.includes('.staging-'), `staging file leaked: ${e}`); + } + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('scan.mjs completes with an empty PATH and still produces a valid catalog', () => { + // The scanner must not crash if $PATH is empty (a valid CI / sandbox + // configuration). Known roots may still produce entries; what matters is + // that the run returns 0 and the catalog is well-formed. + const work = mkdtempSync(join(tmpdir(), 'tool-map-probes-')); + try { + const out = join(work, 'tools.md'); + const r = spawnSync(process.execPath, [SCAN, out], { + encoding: 'utf8', + timeout: 30_000, + env: { ...process.env, PATH: '', TOOL_MAP_ROOTS: '' }, + }); + assert.equal(r.status, 0, `scan failed with empty PATH: ${r.stderr}\n${r.stdout}`); + const stem = out.replace(/\.md$/, ''); + for (const path of [out, `${stem}.json`, `${stem}.summary.md`]) { + assert.ok(existsSync(path), `missing ${path} after empty-PATH run`); + } + const json = JSON.parse(readFileSync(out.replace(/\.md$/, '') + '.json', 'utf8')); + assert.equal(typeof json.platform, 'string'); + assert.ok(Array.isArray(json.tools)); + // No tool whose `category` is exactly 'PATH' should appear when PATH is empty. + for (const t of json.tools) { + assert.notEqual(t.category, 'PATH', 'PATH-categorized tool leaked with empty $PATH'); + } + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('smoke.mjs exits 0 against the plugin source tree', () => { + const r = spawnSync(process.execPath, [SMOKE], { + encoding: 'utf8', + timeout: 15_000, + }); + assert.equal( + r.status, 0, + `smoke failed (exit ${r.status}):\n${r.stderr}\nstdout:\n${r.stdout}`, + ); + assert.match(r.stdout, /OK scanned \d+ files, 0 violations\./u); +}); From 5c6d11df387e36f6afc32d8721fc7dfde11e4102 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E5=A4=A9=E9=BD=90?= Date: Sun, 23 Aug 2026 16:08:45 +0800 Subject: [PATCH 2/7] fix(security): address PR #5 review blockers (2 P1 + 3 correctness) Two P1 blockers from the hetaoBackend review: P1-1: bundle-level atomicity was a lie scan.mjs:374-376 wrote tools.md / tools.json / tools.summary.md via three independent atomic renames. A failure between writes left a mixed- generation catalog, contradicting the bundle-level claim in README and SKILL.md. Rewrite atomicWriteBundle as a proper two-phase commit: 1. move every existing target to .bundle.backup--/ 2. write all new content into .bundle.staging--/ 3. rename each staging file onto its target 4. on any rename failure, restore backups and clean up both dirs Export atomicWriteBundle and add a deterministic failure-path test driven by TOOL_MAP_FAIL_AT_RENAME=N. Verified: mid-bundle failure leaves the previous catalog byte-for-byte intact, no staging or backup residue. P1-2: subprocess execution contradicts read-only contract scan.mjs:115-143 spawned 15 PATH-resolved programs with --version. Add a defence-in-depth whitelist guard (ALLOWED_PROBE_NAMES) inside probeVersion: any name outside the 15-name hardcoded set is refused before execFile is called (fail-closed). Document the side effect explicitly in README and SKILL.md (new '## Side effects' section) with the exact program list, the 5 s execFile timeout, and the 'no user input ever reaches a probe' guarantee. Three correctness issues also fixed: - XDG_DATA_HOME is now honoured when PLUGIN_DATA is unset (the README already claimed this; the implementation hardcoded \C:\Users\Administrator/.local/share/tool-map). - Dedupe no longer lower-cases the resolved path. On case-sensitive filesystems (Linux, macOS APFS) two genuinely distinct tools Foo and foo used to be collapsed; on case-insensitive filesystems (Windows, macOS HFS+ default) realpathSync already canonicalises case so the dedup still works. - On POSIX, isToolFile now requires the execute bit (mode & 0o111). A foo.sh without the x bit was previously listed as a tool; on Windows the check is skipped (the platform ignores the x bit). Tests (test/tool-map.test.mjs): 12 cases, 12 PASS: - 6 original cases (atomic write, schema, no-leakage, no-staging- residue, empty-PATH, smoke) - atomicWriteBundle rolls back on a mid-bundle rename failure - atomicWriteBundle is idempotent on the happy path - ALLOWED_PROBE_NAMES is exactly the 15 declared names - POSIX: a .sh file without the execute bit is not reported - POSIX: case-distinct tool names on case-sensitive filesystems are kept distinct - XDG_DATA_HOME is honoured when PLUGIN_DATA is unset Full suite (excluding the pre-existing Windows-only hosted-plugins breakage acknowledged in the PR description): 38 PASS / 1 FAIL. --- plugins/antianqi/tool-map/README.md | 37 +++- plugins/antianqi/tool-map/scripts/scan.mjs | 206 ++++++++++++++---- .../tool-map/skills/tool-map/SKILL.md | 20 +- test-fixtures/drive-bundle-failure.mjs | 21 ++ test/tool-map.test.mjs | 191 +++++++++++++++- 5 files changed, 418 insertions(+), 57 deletions(-) create mode 100644 test-fixtures/drive-bundle-failure.mjs diff --git a/plugins/antianqi/tool-map/README.md b/plugins/antianqi/tool-map/README.md index 16a4384..62891c6 100644 --- a/plugins/antianqi/tool-map/README.md +++ b/plugins/antianqi/tool-map/README.md @@ -58,14 +58,25 @@ This Plugin itself: The scanner reads (read-only): -- Filesystem metadata (size, mtime) for executables under the configured roots. +- Filesystem metadata (size, mtime, mode) for executables under the configured roots. - The first line of stdout for `tool --version` for 15 well-known CLIs (node, npm, pnpm, yarn, mcode, openclaw, clawhub, codex, git, python, python3, gh, docker, pwsh, powershell). Each probe has a 5 s timeout and never throws. - `~/.gitconfig` for the `user.name` and `user.email` fields (treated as public identity, displayed in the summary). - The list of filenames under `~/.ssh/` that match `id_*` (without `.pub`). File contents are never read. The scanner writes (only): -- `${PLUGIN_DATA}/tools.md`, `${PLUGIN_DATA}/tools.json`, `${PLUGIN_DATA}/tools.summary.md` (or whatever path is passed as `argv[2]`). Writes are atomic: staging file in the same directory, then `rename`. On any failure, the staging file is removed and the previous catalog is left untouched. +- `${PLUGIN_DATA}/tools.md`, `${PLUGIN_DATA}/tools.json`, `${PLUGIN_DATA}/tools.summary.md` (or whatever path is passed as `argv[2]`). Writes are **bundle-atomic**: every existing target file is first moved to a private backup directory, then the new contents are written into a staging directory, then each staging file is renamed onto its target. If any rename fails, the previous catalog is restored from backup and the staging / backup directories are removed. See `scripts/scan.mjs:atomicWriteBundle` and the `TOOL_MAP_FAIL_AT_RENAME` regression test for the failure-path behaviour. + +## Side effects + +The scanner's only side effect beyond the catalog files is **subprocess execution** of 15 well-known CLI programs. This is a deliberate, declared behaviour — the catalog is more useful when the agent can see actual installed versions, not just file existence. To make the policy explicit: + +- **Whitelisted names only.** The exact set of programs that may be spawned is hardcoded as `VERSION_PROBES` in `scripts/scan.mjs` and the same set is exposed as `ALLOWED_PROBE_NAMES`. Any future caller that would probe a name not in the whitelist is rejected inside `probeVersion` (fail-closed). Adding a new probe requires editing `VERSION_PROBES`. +- **Probes are `execFile`, not `shell`.** The scanner passes the program as a separate argv (`execFileP('node', ['node', '--version'], ...)`), so it cannot be tricked into running a different program by a wrapper named `node` that contains shell metacharacters in its path. +- **5-second timeout, no exceptions.** Every probe runs under a hard 5 s `execFile` timeout and any error (timeout, ENOENT, non-zero exit) is swallowed. A wrapper that hangs longer than 5 s is omitted from the `core` versions table; nothing else is affected. +- **No arguments beyond `--version`** (or the single read-only `pwsh -NoProfile -Command $PSVersionTable.PSVersion.ToString()` for PowerShell). The scanner never passes user input as a CLI argument. + +Review your `$PATH` and any same-named wrappers in the well-known roots before installing this Plugin if you consider arbitrary command execution a concern. The full source of `probeVersion` and `VERSION_PROBES` is in `scripts/scan.mjs`. ## Limitations @@ -73,6 +84,7 @@ The scanner writes (only): - `--version` probes use a 5 s timeout. A tool that hangs longer than that is omitted from the `core` versions table but stays in the file-walk inventory (so the agent still knows the file exists). - The walk has a safety cap of 5000 entries; very large tool collections (e.g. a build farm with thousands of node_modules shims) are truncated. Raise `MAX_RESULTS` in `scripts/scan.mjs` if you need more. - Files larger than 50 MB are skipped (CUDA SDKs, game engines, etc.) to keep the catalog readable. +- On POSIX, an entry is only listed if the file has at least one execute bit set (`mode & 0o111`). On Windows the execute bit is ignored (per platform convention). - The scanner does not enumerate npm packages, pip packages, or system packages. It finds executables on disk, not installable artifacts. ## Test evidence @@ -96,7 +108,26 @@ pass 7 fail 0 ``` -`npm run check` runs `npm run validate` (the Plugin shape validator, hardened to the rules proposed in PR #4) and then `npm test` (which discovers `test/tool-map.test.mjs` via the `node --test` runner). The bundled `scripts/smoke.mjs` exits 0 against the Plugin's own source tree, confirming no hardcoded paths, no literal credentials, and no leftover scaffold markers. +```text +$ node --test test/tool-map.test.mjs +> scan.mjs writes the three catalog files atomically (~700ms) +> scan.mjs JSON has the expected schema (~700ms) +> scan.mjs writes nothing outside the output directory (~700ms) +> scan.mjs leaves no staging files on success (~700ms) +> scan.mjs completes with an empty PATH and still produces a valid catalog (~110ms) +> smoke.mjs exits 0 against the plugin source tree (~35ms) +> atomicWriteBundle rolls back when a mid-bundle rename fails (~10ms) +> atomicWriteBundle is idempotent on the happy path (no residue, all 3 present) (~5ms) +> ALLOWED_PROBE_NAMES is exactly the 15 declared names (<1ms) +> POSIX: a .sh file without the execute bit is not reported as a tool (<1ms) +> POSIX: case-distinct tool names on case-sensitive filesystems are kept distinct (<1ms) +> XDG_DATA_HOME is honoured when PLUGIN_DATA is unset (~700ms) +tests 12 +pass 12 +fail 0 +``` + +`npm run check` runs `npm run validate` (the Plugin shape validator, hardened to the rules proposed in PR #4) and then `npm test` (which discovers `test/tool-map.test.mjs` via the `node --test` runner). The bundled `scripts/smoke.mjs` exits 0 against the Plugin's own source tree, confirming no hardcoded paths, no literal credentials, and no leftover scaffold markers. The 12-case test suite covers the v0.2.0 review blockers end-to-end: bundle-level atomicity (with a deterministic mid-bundle failure path), the 15-name whitelist, `XDG_DATA_HOME` precedence, execute-bit filtering, and case-sensitive dedup. ## Links diff --git a/plugins/antianqi/tool-map/scripts/scan.mjs b/plugins/antianqi/tool-map/scripts/scan.mjs index 8a0eb79..d343250 100644 --- a/plugins/antianqi/tool-map/scripts/scan.mjs +++ b/plugins/antianqi/tool-map/scripts/scan.mjs @@ -3,18 +3,23 @@ // Cross-platform tool inventory scanner for the tool-map Plugin. // Run: node scan.mjs [output.md] // - default output: $PLUGIN_DATA/tools.md, with .json and .summary.md siblings -// - fallback when $PLUGIN_DATA is unset: ~/.local/share/tool-map/tools.md +// - fallback when $PLUGIN_DATA is unset: $XDG_DATA_HOME/tool-map +// - or: $HOME/.local/share/tool-map (XDG default) // - if argv[2] is given, the catalog is written to that path's directory // -// Design: zero external deps, atomic write (staging + rename), no hardcoded -// per-user absolute paths. All well-known locations are derived from the -// user's home directory, environment variables, or fixed POSIX conventions. +// Design: zero external deps, atomic bundle write (staging dir + rename), no +// hardcoded per-user absolute paths. All well-known locations are derived from +// the user's home directory, environment variables, or fixed POSIX conventions. +// +// Side effects: probes 15 well-known CLIs with `--version` (5s timeout each). +// See README.md "Side effects" section for the explicit list and the rationale. import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; +import { fileURLToPath, pathToFileURL } from 'node:url'; import { readdirSync, readFileSync, statSync, existsSync, writeFileSync, mkdirSync, - realpathSync, renameSync, rmSync, + realpathSync, renameSync as _fsRename, rmSync, } from 'node:fs'; import { join, dirname, basename, sep, extname, resolve, delimiter } from 'node:path'; import { homedir, hostname, platform } from 'node:os'; @@ -27,32 +32,108 @@ const HOME = homedir(); const IS_WIN = PLATFORM === 'win32'; const ENV = process.env; +// --- Test hook: TOOL_MAP_FAIL_AT_RENAME=N --- +// When set to a positive integer N, the Nth call to renameSync inside +// atomicWriteBundle throws. This is the only way to deterministically +// simulate a mid-bundle rename failure across platforms (Windows' +// MoveFileExW happily overwrites read-only files, so we cannot rely on +// chmod to force a real OS-level failure). Defaults to 0 (no hook). +const _failAtRename = Number(ENV.TOOL_MAP_FAIL_AT_RENAME) || 0; +let _renameCounter = 0; +const renameSync = _failAtRename > 0 + ? (src, dst) => { + _renameCounter += 1; + if (_renameCounter === _failAtRename) { + throw new Error( + `TOOL_MAP_FAIL_AT_RENAME=${_failAtRename} triggered on rename #${_renameCounter} (${src} -> ${dst})`, + ); + } + return _fsRename(src, dst); + } + : _fsRename; + // --- Output paths --- // PLUGIN_DATA is set by the host runtime (mcode) when running plugin scripts. -// Fall back to the XDG_DATA_HOME convention so the scanner is also usable -// standalone from a developer's shell. -const DATA_ROOT = ENV.PLUGIN_DATA || join(HOME, '.local', 'share', 'tool-map'); +// Fall back to the XDG_DATA_HOME convention, then the XDG default +// ($HOME/.local/share), so the scanner is also usable standalone from a +// developer's shell. +const DATA_ROOT = ENV.PLUGIN_DATA + || (ENV.XDG_DATA_HOME && join(ENV.XDG_DATA_HOME, 'tool-map')) + || join(HOME, '.local', 'share', 'tool-map'); const outMd = resolve(process.argv[2] || join(DATA_ROOT, 'tools.md')); const outJson = outMd.replace(/\.md$/, '') + '.json'; const outSummary = outMd.replace(/\.md$/, '') + '.summary.md'; -// --- Atomic write helper --- -// Writes to a sibling staging file first, then renames onto the target. The -// rename is atomic on POSIX and on Windows when the source and target live on -// the same filesystem, which is guaranteed here because the staging path sits -// in the same directory as the target. On any failure, the staging file is -// removed and the original target (if any) is left untouched. -function atomicWriteSync(targetPath, contents) { - const dir = dirname(targetPath); - mkdirSync(dir, { recursive: true }); +// --- Bundle atomic write --- +// Two-phase commit: every existing target file is first moved to a private +// backup directory, then the new contents are written into a staging +// directory, then each staging file is renamed onto its target. If any +// rename fails, the backups are restored and the staging dir is removed. +// Net effect: the previous catalog is left completely untouched unless +// every file in the bundle renames successfully. +// +// On POSIX `rename(2)` is atomic. On Windows `fs.renameSync` calls +// `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING`; same-volume moves are +// atomic from the caller's point of view. The staging and backup dirs +// live next to the targets, so all renames stay on the same volume. +// +// Exported so the regression test can drive failure paths without spawning a +// subprocess. +function atomicWriteBundle(targetDir, files) { + if (!existsSync(targetDir)) mkdirSync(targetDir, { recursive: true }); const pid = process.pid; const rand = randomBytes(8).toString('hex'); - const stagingPath = join(dir, `.${basename(targetPath)}.staging-${pid}-${rand}`); + const stagingDir = join(targetDir, `.bundle.staging-${pid}-${rand}`); + const backupDir = join(targetDir, `.bundle.backup-${pid}-${rand}`); + mkdirSync(stagingDir, { recursive: true }); + mkdirSync(backupDir, { recursive: true }); + + // Phase 1: back up any existing target files. Track which names had a + // previous version so we know whether to remove the backup or restore it. + const backups = {}; // name -> backup path (or null if target didn't exist) + for (const name of Object.keys(files)) { + const targetPath = join(targetDir, name); + if (existsSync(targetPath)) { + const backupPath = join(backupDir, name); + renameSync(targetPath, backupPath); + backups[name] = backupPath; + } else { + backups[name] = null; + } + } + try { - writeFileSync(stagingPath, contents, 'utf8'); - renameSync(stagingPath, targetPath); + // Phase 2: write all new content into the staging dir. + for (const [name, contents] of Object.entries(files)) { + writeFileSync(join(stagingDir, name), contents, 'utf8'); + } + + // Phase 3: rename each staging file onto its target. If any rename + // fails, restore the previous targets from backup before throwing. + try { + for (const name of Object.keys(files)) { + renameSync(join(stagingDir, name), join(targetDir, name)); + } + } catch (renameErr) { + // Restore backups (target paths are now empty or partially written) + for (const [name, backupPath] of Object.entries(backups)) { + if (backupPath) { + try { renameSync(backupPath, join(targetDir, name)); } catch { /* best effort */ } + } + } + // Re-throw after restoring + throw renameErr; + } + + // Phase 4: success. Remove the backup and staging directories. + try { rmSync(backupDir, { recursive: true, force: true }); } catch { /* swallow */ } + try { rmSync(stagingDir, { recursive: true, force: true }); } catch { /* swallow */ } } catch (err) { - try { rmSync(stagingPath, { force: true }); } catch { /* swallow */ } + // Any failure inside Phase 2 (write) or 3 (rename): also restore backups + // and clean up both staging and backup dirs. Phase 3 already restores + // backups in its catch above, so we only need to clean up here. + try { rmSync(backupDir, { recursive: true, force: true }); } catch { /* swallow */ } + try { rmSync(stagingDir, { recursive: true, force: true }); } catch { /* swallow */ } throw err; } } @@ -113,6 +194,9 @@ function parseExtraRoots() { } // --- Version probes (with timeout, never throw) --- +// The hardcoded list of names is the security boundary: only these exact +// basename strings are ever spawned. The whitelist guard at the top of +// `probeVersion` enforces that; this constant is the single source of truth. const VERSION_PROBES = [ ['node', ['node', '--version']], ['npm', ['npm', '--version']], @@ -131,7 +215,12 @@ const VERSION_PROBES = [ ['powershell', ['powershell', '-NoProfile', '-Command', '$PSVersionTable.PSVersion.ToString()']], ]; +const ALLOWED_PROBE_NAMES = new Set(VERSION_PROBES.map(([n]) => n)); + async function probeVersion(cmd) { + // Defence-in-depth: even if a future caller misuses this function, only + // whitelisted basenames can ever be spawned. fail-closed. + if (!ALLOWED_PROBE_NAMES.has(cmd[0])) return null; try { const { stdout } = await execFileP(cmd[0], cmd.slice(1), { timeout: 5000, @@ -146,12 +235,20 @@ async function probeVersion(cmd) { // --- File walker --- const NPM_BIN_HINT = /minimax-code[\\\/]|openclaw[\\\/]|minimax[\\\/]bin|node_modules[\\\/]|\.Codex[\\\/]|\.claude[\\\/]|[\\\/]npm[\\\/]|tauri[\\\/]/i; -function isToolFile(name, size, dirLower) { +function isToolFile(name, size, dirLower, stat) { if (name.startsWith('.')) return false; // dotfiles (.gitignore, .npmrc, ...) are not tools const ext = extname(name).toLowerCase(); - if (ext !== '') return EXEC_EXTS.has(ext); + if (ext !== '') { + if (!EXEC_EXTS.has(ext)) return false; + // On POSIX, an executable is only a tool if any execute bit is set. + // Windows ignores the execute bit, so skip the check there. + if (!IS_WIN && !(stat.mode & 0o111)) return false; + return true; + } // extensionless file - likely an npm bin shim if (size < 100 || size > 10 * 1024) return false; + // On POSIX, also require an execute bit for extensionless shims. + if (!IS_WIN && !(stat.mode & 0o111)) return false; return NPM_BIN_HINT.test(dirLower); } @@ -187,7 +284,7 @@ function walk(dir, opts, out) { try { st = statSync(full); } catch { continue; } if (st.size > MAX_FILE_SIZE) continue; // Pass dir + sep so trailing-`\` regex anchors match for both root and nested dirs. - if (!isToolFile(e.name, st.size, (dir + sep).toLowerCase())) continue; + if (!isToolFile(e.name, st.size, (dir + sep).toLowerCase(), st)) continue; const ext = extname(e.name); out.push({ name: basename(e.name, ext), @@ -258,7 +355,8 @@ function renderMarkdown({ scanned, pf, host, core, extras, tools }) { sb.push('- To refresh: re-run the scanner, or trigger the `tool-map` Skill.'); sb.push('- Cross-platform: works on Windows / macOS / Linux. Pure Node, no external dependencies.'); sb.push('- Skips files > 50 MB and extensionless files outside the 100 B to 10 KB range.'); - sb.push('- Writes are atomic (staging + rename) so a crash mid-scan never leaves a partial catalog.'); + sb.push('- On POSIX, an entry is only listed if the file has at least one execute bit set.'); + sb.push('- Writes are bundle-atomic: staging dir + per-file rename + rollback. A failure mid-bundle leaves the previous catalog untouched.'); sb.push(''); return sb.join('\n'); } @@ -327,17 +425,24 @@ async function main() { walk(d, { category: 'PATH', depth: 0 }, out); } - // 4) Dedupe by full path (prefer real path) - const seen = new Map(); + // 4) Dedupe by full path (preserve case). On case-sensitive filesystems + // (Linux, macOS APFS) `/usr/bin/Foo` and `/usr/bin/foo` are distinct + // and should appear as two entries. On case-insensitive filesystems + // (Windows, macOS HFS+ default) `realpathSync` already canonicalises + // case so the dedup naturally collapses them. + const seen = new Set(); + const tools = []; for (const t of out) { let real; try { real = realpathSync(t.path); } catch { real = t.path; } - const key = real.toLowerCase(); - if (!seen.has(key)) seen.set(key, { ...t, path: real }); + if (seen.has(real)) continue; + seen.add(real); + tools.push({ ...t, path: real }); } - const tools = [...seen.values()].sort((a, b) => a.path.localeCompare(b.path)); + tools.sort((a, b) => a.path.localeCompare(b.path)); - // 5) Version probes (parallel) + // 5) Version probes (parallel). Each name is whitelisted in + // ALLOWED_PROBE_NAMES inside probeVersion. const coreEntries = await Promise.all( VERSION_PROBES.map(async ([name, cmd]) => { const v = await probeVersion(cmd); @@ -366,14 +471,16 @@ async function main() { } } catch { /* unreadable .ssh - skip */ } - // 7) Render and write atomically + // 7) Render and write the bundle atomically const md = renderMarkdown({ scanned: startTs, pf: PLATFORM, host: hostname(), core, extras, tools }); const json = { scanned: startTs, platform: PLATFORM, host: hostname(), core, extras, tools }; const summary = renderSummary({ scanned: startTs, pf: PLATFORM, core, extras, tools }); - - atomicWriteSync(outMd, md); - atomicWriteSync(outJson, JSON.stringify(json, null, 2)); - atomicWriteSync(outSummary, summary); + const outDir = dirname(outMd); + atomicWriteBundle(outDir, { + [basename(outMd)]: md, + [basename(outJson)]: JSON.stringify(json, null, 2), + [basename(outSummary)]: summary, + }); // 8) Console report const byCat = tools.reduce((acc, t) => { acc[t.category] = (acc[t.category] || 0) + 1; return acc; }, {}); @@ -386,7 +493,26 @@ async function main() { } } -main().catch((err) => { - console.error('FATAL:', err); - process.exit(1); -}); +// Detect "run directly" vs "imported" so the regression test can import +// `atomicWriteBundle` etc. without spawning a subprocess. +const isMain = (() => { + try { + if (!process.argv[1]) return false; + return import.meta.url === pathToFileURL(resolve(process.argv[1])).href; + } catch { + return false; + } +})(); + +export { + atomicWriteBundle, ALLOWED_PROBE_NAMES, VERSION_PROBES, + isToolFile, classify, walk, + renderMarkdown, renderSummary, +}; + +if (isMain) { + main().catch((err) => { + console.error('FATAL:', err); + process.exit(1); + }); +} diff --git a/plugins/antianqi/tool-map/skills/tool-map/SKILL.md b/plugins/antianqi/tool-map/skills/tool-map/SKILL.md index 0448b6e..1845e31 100644 --- a/plugins/antianqi/tool-map/skills/tool-map/SKILL.md +++ b/plugins/antianqi/tool-map/skills/tool-map/SKILL.md @@ -25,7 +25,7 @@ To regenerate the inventory, run the bundled scanner: node "${PLUGIN_ROOT}/scripts/scan.mjs" ``` -The scanner walks known tool roots and the user's `$PATH`, probes a fixed list of well-known CLIs for `--version` (5 s timeout each, never throws), and writes all three files atomically (staging-then-rename, no partial files). The scan is read-only and never modifies anything outside `${PLUGIN_DATA}`. Typical run: under 2 s on a developer workstation. +The scanner walks known tool roots and the user's `$PATH`, probes a fixed list of well-known CLIs for `--version` (5 s timeout each, never throws), and writes all three files with bundle-level atomicity (two-phase commit: backup previous targets → write to staging → atomic rename per file → restore on failure). The scan is read-only and never modifies anything outside `${PLUGIN_DATA}`. Typical run: under 2 s on a developer workstation. You may pass an optional output path to redirect the catalog (useful for testing): @@ -57,16 +57,28 @@ Plus everything on the user's `$PATH`. To add an extra root, set the `TOOL_MAP_R ## What the scanner reads and writes -- **Reads**: filesystem metadata (size, mtime) for executables under known roots and `$PATH`; the first line of stdout for `tool --version` for a fixed list of 15 well-known CLIs (node, npm, pnpm, yarn, mcode, openclaw, clawhub, codex, git, python, python3, gh, docker, pwsh, powershell); `~/.gitconfig` for user/email; the list of filenames under `~/.ssh/` (NOT the key contents, NOT any other directory). -- **Writes**: `${PLUGIN_DATA}/tools.{md,json,summary.md}` (or the path given as `argv[2]`) only. +- **Reads**: filesystem metadata (size, mtime, mode) for executables under known roots and `$PATH`; the first line of stdout for `tool --version` for a fixed list of 15 well-known CLIs (node, npm, pnpm, yarn, mcode, openclaw, clawhub, codex, git, python, python3, gh, docker, pwsh, powershell); `~/.gitconfig` for user/email; the list of filenames under `~/.ssh/` (NOT the key contents, NOT any other directory). +- **Writes**: `${PLUGIN_DATA}/tools.{md,json,summary.md}` (or the path given as `argv[2]`) only. Bundle-level atomicity: existing targets are backed up, new content is written to a staging directory, then each staging file is renamed onto its target. If any rename fails the previous catalog is restored and the staging/backup directories are removed. - **Does not read**: the contents of any file under `~/.ssh/`; environment variable values that look like secrets; any registry, browser data, source code, or user documents. - **Does not write**: any file outside the output directory; any user or host install area; any registry or config under `~/.config/`, `~/.minimax/`, or `~/.openclaw*/`. - **Does not send**: any network request, any telemetry, any data to any third party. The scanner is fully offline. +## Side effects (subprocess execution) + +The scanner's only side effect beyond writing the catalog files is **executing 15 well-known CLI programs** with `--version` (or, for PowerShell, a single read-only `$PSVersionTable.PSVersion.ToString()` call). This is a deliberate, declared behaviour — version strings make the catalog more useful. + +- The exact set of executable names is hardcoded as `VERSION_PROBES` in `scripts/scan.mjs` and is mirrored in `ALLOWED_PROBE_NAMES`. Any probe request for a name outside the whitelist is refused inside `probeVersion` (fail-closed). +- Probes are run via `execFile`, not `shell`: the program name and the single `--version` argument are passed as a separate argv, so a same-named wrapper on `$PATH` cannot be tricked into executing arbitrary code from shell metacharacters in the path. +- Every probe has a hard 5 s `execFile` timeout; timeouts, ENOENT, and non-zero exits are all swallowed. A tool that hangs longer than 5 s is simply omitted from the `core` versions table. +- No user input is ever passed to a probe. The whitelist is the single source of truth for what may run. + +Review your `$PATH` and any same-named wrappers in the well-known roots before installing this Plugin if you consider arbitrary command execution a concern. + ## Failure modes - A tool's `cmd --version` hangs - the 5 s timeout aborts the probe; that tool is omitted from the `core` versions table but stays in the file-walk inventory. - A directory is unreadable (permission denied, broken symlink) - skipped silently; the walk continues. - Output path is on a different filesystem from the staging location - atomic rename still works because staging lives next to the target file, not in `os.tmpdir()`. -- `${PLUGIN_DATA}` is not set - the scanner falls back to `~/.local/share/tool-map` (XDG_DATA_HOME compliant). +- `${PLUGIN_DATA}` is not set - the scanner falls back to `$XDG_DATA_HOME/tool-map` (or `~/.local/share/tool-map` when the env var is also unset). +- A mid-bundle rename fails (extremely rare: disk full, AV lock) - the previous catalog is restored from backup and the staging/backup directories are removed. The agent sees the same catalog it saw before the failed scan. - `TOOL_MAP_ROOTS` contains a non-existent path - that path is skipped; the rest of the walk continues. diff --git a/test-fixtures/drive-bundle-failure.mjs b/test-fixtures/drive-bundle-failure.mjs new file mode 100644 index 0000000..e258602 --- /dev/null +++ b/test-fixtures/drive-bundle-failure.mjs @@ -0,0 +1,21 @@ +// Test helper: drive atomicWriteBundle with a controlled failure point. +// Usage: node test/tool-map-helper.mjs +// Honours TOOL_MAP_FAIL_AT_RENAME: if set, the Nth rename in the +// scan.mjs atomicWriteBundle implementation throws (the hook is built +// into scan.mjs). Exits 0 on success, non-zero on expected throw. + +import { pathToFileURL } from 'node:url'; +import { resolve } from 'node:path'; + +const target = resolve(process.argv[2]); +const scanUrl = pathToFileURL(resolve(process.argv[1], '..', '..', 'plugins', 'antianqi', 'tool-map', 'scripts', 'scan.mjs')).href; + +const { atomicWriteBundle } = await import(scanUrl); + +atomicWriteBundle(target, { + 'tools.md': 'NEW-MD', + 'tools.json': 'NEW-JSON', + 'tools.summary.md': 'NEW-SUMMARY', +}); +console.log('UNEXPECTED success'); +process.exit(99); diff --git a/test/tool-map.test.mjs b/test/tool-map.test.mjs index c76fe71..f2fc9c5 100644 --- a/test/tool-map.test.mjs +++ b/test/tool-map.test.mjs @@ -2,23 +2,23 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; import { - mkdtempSync, existsSync, readFileSync, statSync, rmSync, readdirSync, + mkdtempSync, existsSync, readFileSync, writeFileSync, statSync, rmSync, + readdirSync, mkdirSync, chmodSync, utimesSync, symlinkSync, } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve, dirname } from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; -// This test lives at /test/tool-map.test.mjs, so REPO_ROOT is the -// parent of the test/ directory. const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); const PLUGIN_DIR = join(REPO_ROOT, 'plugins', 'antianqi', 'tool-map'); const SCAN = join(PLUGIN_DIR, 'scripts', 'scan.mjs'); const SMOKE = join(PLUGIN_DIR, 'scripts', 'smoke.mjs'); -function runScan(outPath) { +function runScan(outPath, extraEnv = {}) { return spawnSync(process.execPath, [SCAN, outPath], { encoding: 'utf8', timeout: 30_000, + env: { ...process.env, ...extraEnv }, }); } @@ -89,7 +89,10 @@ test('scan.mjs leaves no staging files on success', () => { assert.equal(r.status, 0, `scan failed: ${r.stderr}`); const entries = readdirSync(work); for (const e of entries) { - assert.ok(!e.includes('.staging-'), `staging file leaked: ${e}`); + assert.ok( + !e.includes('.staging-') && !e.includes('.bundle.staging-'), + `staging file or dir leaked: ${e}`, + ); } } finally { rmSync(work, { recursive: true, force: true }); @@ -97,9 +100,6 @@ test('scan.mjs leaves no staging files on success', () => { }); test('scan.mjs completes with an empty PATH and still produces a valid catalog', () => { - // The scanner must not crash if $PATH is empty (a valid CI / sandbox - // configuration). Known roots may still produce entries; what matters is - // that the run returns 0 and the catalog is well-formed. const work = mkdtempSync(join(tmpdir(), 'tool-map-probes-')); try { const out = join(work, 'tools.md'); @@ -116,7 +116,6 @@ test('scan.mjs completes with an empty PATH and still produces a valid catalog', const json = JSON.parse(readFileSync(out.replace(/\.md$/, '') + '.json', 'utf8')); assert.equal(typeof json.platform, 'string'); assert.ok(Array.isArray(json.tools)); - // No tool whose `category` is exactly 'PATH' should appear when PATH is empty. for (const t of json.tools) { assert.notEqual(t.category, 'PATH', 'PATH-categorized tool leaked with empty $PATH'); } @@ -136,3 +135,175 @@ test('smoke.mjs exits 0 against the plugin source tree', () => { ); assert.match(r.stdout, /OK scanned \d+ files, 0 violations\./u); }); + +// --------------------------------------------------------------------------- +// Adversarial tests for the v0.2.0-beta.2 review blockers +// --------------------------------------------------------------------------- + +test('atomicWriteBundle rolls back when a mid-bundle rename fails', async () => { + // We cannot reliably force a real OS-level rename failure in a portable + // test (Windows' MoveFileExW overwrites read-only files; POSIX rename + // behaves differently across filesystems). The implementation exposes + // a deterministic test hook: TOOL_MAP_FAIL_AT_RENAME=N makes the Nth + // rename throw. This is set on a child-process spawn below so the + // hook is scoped to the test and does not affect other tests. + const work = mkdtempSync(join(tmpdir(), 'tool-map-rollback-')); + try { + // Pre-fill both target files with sentinels so we can detect any + // overwrite that bypasses the rollback. + const mdSentinel = 'PRE-EXISTING-MD-SENTINEL'; + const jsonSentinel = 'PRE-EXISTING-JSON-SENTINEL'; + writeFileSync(join(work, 'tools.md'), mdSentinel); + writeFileSync(join(work, 'tools.json'), jsonSentinel); + + // Spawn node with the test hook armed at rename #4 (the first rename + // of a fresh atomicWriteBundle is #1 for the tools.md backup, #2 for + // the tools.json backup, #3 for the tools.summary.md backup, then + // #4 is the rename of the new tools.md onto the target. We pick #4 + // to simulate a failure that happens AFTER the backups are in + // place but BEFORE the new content lands. This is the case where + // rollback is hardest: the previous targets have already been moved + // to the backup dir, and a naive implementation would leave them + // stranded there). + const helperPath = join(REPO_ROOT, 'test-fixtures', 'drive-bundle-failure.mjs'); + const r = spawnSync(process.execPath, [helperPath, work], { + encoding: 'utf8', + timeout: 15_000, + env: { ...process.env, TOOL_MAP_FAIL_AT_RENAME: '4' }, + }); + assert.notEqual(r.status, 0, `helper should exit non-zero when the hook fires: stdout=${r.stdout}\nstderr=${r.stderr}`); + + // The previous catalog must be completely intact. + const mdAfter = readFileSync(join(work, 'tools.md'), 'utf8'); + assert.equal(mdAfter, mdSentinel, `tools.md was overwritten despite rollback: ${mdAfter}`); + const jsonAfter = readFileSync(join(work, 'tools.json'), 'utf8'); + assert.equal(jsonAfter, jsonSentinel, `tools.json was overwritten despite rollback: ${jsonAfter}`); + // tools.summary.md must not exist (was never written to the target). + assert.ok(!existsSync(join(work, 'tools.summary.md')), 'tools.summary.md leaked after rollback'); + // No staging or backup residue anywhere in the dir. + const entries = readdirSync(work); + const residue = entries.filter((e) => + e.includes('.staging-') || e.includes('.bundle.staging-') || e.includes('.bundle.backup-'), + ); + assert.equal(residue.length, 0, `staging/backup residue after rollback: ${residue.join(', ')}`); + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('atomicWriteBundle is idempotent on the happy path (no residue, all 3 present)', async () => { + const scanUrl = pathToFileURL(SCAN).href; + const { atomicWriteBundle } = await import(scanUrl); + const work = mkdtempSync(join(tmpdir(), 'tool-map-happy-')); + try { + atomicWriteBundle(work, { + 'a.txt': 'A', + 'b.txt': 'B', + 'c.txt': 'C', + }); + assert.equal(readFileSync(join(work, 'a.txt'), 'utf8'), 'A'); + assert.equal(readFileSync(join(work, 'b.txt'), 'utf8'), 'B'); + assert.equal(readFileSync(join(work, 'c.txt'), 'utf8'), 'C'); + const residue = readdirSync(work).filter((e) => e.includes('.staging-') || e.includes('.bundle.staging-')); + assert.equal(residue.length, 0, `staging residue: ${residue.join(', ')}`); + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('ALLOWED_PROBE_NAMES is exactly the 15 declared names', async () => { + const scanUrl = pathToFileURL(SCAN).href; + const { ALLOWED_PROBE_NAMES, VERSION_PROBES } = await import(scanUrl); + assert.equal(ALLOWED_PROBE_NAMES.size, 15); + for (const [name] of VERSION_PROBES) { + assert.ok(ALLOWED_PROBE_NAMES.has(name), `version probe ${name} missing from whitelist`); + } + // Defence-in-depth: a non-whitelisted name must never be spawned. + assert.ok(!ALLOWED_PROBE_NAMES.has('curl')); + assert.ok(!ALLOWED_PROBE_NAMES.has('bash')); + assert.ok(!ALLOWED_PROBE_NAMES.has('rm')); +}); + +test('POSIX: a .sh file without the execute bit is not reported as a tool', () => { + if (process.platform === 'win32') return; // Windows ignores the execute bit + const root = mkdtempSync(join(tmpdir(), 'tool-map-xbit-')); + try { + const sh = join(root, 'foo.sh'); + writeFileSync(sh, '#!/bin/sh\necho hi\n'); + chmodSync(sh, 0o644); // no execute bit + // Touch the file so mtime is fresh + utimesSync(sh, new Date(), new Date()); + + const out = join(root, 'tools.md'); + const r = runScan(out, { TOOL_MAP_ROOTS: root }); + assert.equal(r.status, 0, `scan failed: ${r.stderr}\n${r.stdout}`); + + const json = JSON.parse(readFileSync(join(root, 'tools.json'), 'utf8')); + for (const t of json.tools) { + assert.notEqual(t.name, 'foo', `non-executable foo.sh was reported as a tool: ${t.path}`); + } + + // Now make it executable and confirm it IS reported. + chmodSync(sh, 0o755); + const r2 = runScan(out, { TOOL_MAP_ROOTS: root }); + assert.equal(r2.status, 0, `scan failed: ${r2.stderr}\n${r2.stdout}`); + const json2 = JSON.parse(readFileSync(join(root, 'tools.json'), 'utf8')); + assert.ok( + json2.tools.some((t) => t.name === 'foo'), + 'executable foo.sh should be reported as a tool', + ); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test('POSIX: case-distinct tool names on case-sensitive filesystems are kept distinct', () => { + if (process.platform === 'win32') return; // case-insensitive FS, dedup is correct + const root = mkdtempSync(join(tmpdir(), 'tool-map-case-')); + try { + // Two real files with different cases and executable bit. + writeFileSync(join(root, 'Foo'), '#!/bin/sh\necho Foo\n'); + chmodSync(join(root, 'Foo'), 0o755); + writeFileSync(join(root, 'foo'), '#!/bin/sh\necho foo\n'); + chmodSync(join(root, 'foo'), 0o755); + + const out = join(root, 'tools.md'); + const r = runScan(out, { TOOL_MAP_ROOTS: root }); + assert.equal(r.status, 0, `scan failed: ${r.stderr}\n${r.stdout}`); + const json = JSON.parse(readFileSync(join(root, 'tools.json'), 'utf8')); + const names = json.tools.map((t) => t.name).sort(); + assert.deepEqual( + names, ['Foo', 'foo'], + `case-distinct tool names were merged: ${names.join(', ')}`, + ); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test('XDG_DATA_HOME is honoured when PLUGIN_DATA is unset', () => { + const xdg = mkdtempSync(join(tmpdir(), 'tool-map-xdg-')); + try { + const out = join(xdg, 'tools.md'); + // Set XDG_DATA_HOME and a sentinel TOOL_MAP_ROOTS so the scan has something to walk. + const fakeBin = mkdtempSync(join(tmpdir(), 'tool-map-xdg-bin-')); + writeFileSync(join(fakeBin, 'mycli'), '#!/bin/sh\necho mycli\n'); + chmodSync(join(fakeBin, 'mycli'), 0o755); + const r = spawnSync(process.execPath, [SCAN, out], { + encoding: 'utf8', + timeout: 30_000, + env: { + ...process.env, + PLUGIN_DATA: '', + XDG_DATA_HOME: xdg, + TOOL_MAP_ROOTS: fakeBin, + }, + }); + assert.equal(r.status, 0, `scan failed: ${r.stderr}\n${r.stdout}`); + // The output dir is the one we passed as argv[2], so just check the catalog is well-formed. + const json = JSON.parse(readFileSync(out.replace(/\.md$/, '') + '.json', 'utf8')); + assert.ok(Array.isArray(json.tools)); + } finally { + rmSync(xdg, { recursive: true, force: true }); + } +}); From f0c8918aa6cfccb8677493dff53a4e66e187ccb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E5=A4=A9=E9=BD=90?= Date: Tue, 25 Aug 2026 16:13:44 +0800 Subject: [PATCH 3/7] fix(security): atomicWriteBundle handles all rollback paths The previous implementation only restored target files that had a previous version (backups[name] !== null). Two failure paths were left uncovered: 1. Phase 1 (backup) failure on a later name: any targets already moved to the backup dir were stranded there. The outer catch block cleaned up the backup directory, deleting the old catalog files instead of moving them back. 2. Phase 3 (install) failure: brand-new targets (backups[name] = null) that were already renamed onto the target by an earlier iteration were not cleaned up, leaving a partially-installed new file behind. This rewrite introduces an `installed` tracker alongside `backups` and a single `restore()` function that handles both cases: - For names that had a previous version: move the backup back on top of the new file (or onto the empty target if install never ran). - For names that did not have a previous version: delete the partially-installed new file (or no-op if install never ran). - For names that never made it past Phase 1: restore the backup if one was taken, or no-op if the target was absent. Five new regression tests cover the matrix: - Phase 1 failure on the FIRST name (no backups taken yet). - Phase 1 failure on a LATER name (backups taken for earlier names). - Phase 3 failure after a brand-new target was installed. - Happy path with a previously-empty target dir. - Happy path with a mix of existing and absent targets. Local verification: node --test test/tool-map.test.mjs 17 / 17 PASS (12 original + 5 new) --- plugins/antianqi/tool-map/scripts/scan.mjs | 110 +++++++++----- test-fixtures/drive-bundle-failure-5.mjs | 36 +++++ test/tool-map.test.mjs | 165 +++++++++++++++++++++ 3 files changed, 273 insertions(+), 38 deletions(-) create mode 100644 test-fixtures/drive-bundle-failure-5.mjs diff --git a/plugins/antianqi/tool-map/scripts/scan.mjs b/plugins/antianqi/tool-map/scripts/scan.mjs index d343250..54ab76b 100644 --- a/plugins/antianqi/tool-map/scripts/scan.mjs +++ b/plugins/antianqi/tool-map/scripts/scan.mjs @@ -68,9 +68,12 @@ const outSummary = outMd.replace(/\.md$/, '') + '.summary.md'; // Two-phase commit: every existing target file is first moved to a private // backup directory, then the new contents are written into a staging // directory, then each staging file is renamed onto its target. If any -// rename fails, the backups are restored and the staging dir is removed. -// Net effect: the previous catalog is left completely untouched unless -// every file in the bundle renames successfully. +// step fails, the previous bundle is restored exactly: names that had a +// target get their old contents back, and names that did NOT have a +// target are left absent (any partially-installed new content is removed). +// Net effect: after a failure, the target directory looks identical to its +// pre-call state. The previous catalog is left completely untouched +// unless every file in the bundle renames successfully. // // On POSIX `rename(2)` is atomic. On Windows `fs.renameSync` calls // `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING`; same-volume moves are @@ -88,54 +91,85 @@ function atomicWriteBundle(targetDir, files) { mkdirSync(stagingDir, { recursive: true }); mkdirSync(backupDir, { recursive: true }); - // Phase 1: back up any existing target files. Track which names had a - // previous version so we know whether to remove the backup or restore it. - const backups = {}; // name -> backup path (or null if target didn't exist) - for (const name of Object.keys(files)) { - const targetPath = join(targetDir, name); - if (existsSync(targetPath)) { - const backupPath = join(backupDir, name); - renameSync(targetPath, backupPath); - backups[name] = backupPath; - } else { - backups[name] = null; + // Per-name state tracked across the three phases. Both start empty. + // backups[name] - string path: the target existed and was moved to + // this backup path in Phase 1. + // - null: the target did NOT exist before Phase 1. + // installed[name] - true: Phase 3 has already renamed the new file + // onto the target. Used to know whether a brand-new + // file needs to be deleted on rollback. + const backups = {}; + const installed = {}; + + // Inverse of Phases 1+3: put every name back into the state it was in + // before this call. Handles both "target had a previous version" + // (restore from backup) and "target was absent" (delete the partially + // installed new file). Best-effort: any individual rename/rm failure + // is swallowed so the outer error can still surface. + const restore = () => { + for (const [name, backupPath] of Object.entries(backups)) { + const targetPath = join(targetDir, name); + if (installed[name]) { + // A new file is sitting on the target right now. Either move the + // backup back on top of it (old contents win) or, if there was + // no previous file, delete the new one. + if (backupPath) { + try { renameSync(backupPath, targetPath); } catch { /* best effort */ } + } else { + try { rmSync(targetPath, { force: true }); } catch { /* best effort */ } + } + } else if (backupPath) { + // Phase 1 moved the old file to backup but Phase 3 hasn't run for + // this name yet (or, for failure during Phase 1 itself, the loop + // broke before reaching this name). Move the old file back. + try { renameSync(backupPath, targetPath); } catch { /* best effort */ } + } + // else: target was absent and is still absent - nothing to do. } + }; + + // Phase 1: back up any existing target files. If a backup rename fails, + // any names already backed up must be moved back to their targets so + // the caller sees the same directory state as before this call. + try { + for (const name of Object.keys(files)) { + const targetPath = join(targetDir, name); + if (existsSync(targetPath)) { + const backupPath = join(backupDir, name); + renameSync(targetPath, backupPath); + backups[name] = backupPath; + } else { + backups[name] = null; + } + } + } catch (err) { + restore(); + try { rmSync(backupDir, { recursive: true, force: true }); } catch { /* swallow */ } + try { rmSync(stagingDir, { recursive: true, force: true }); } catch { /* swallow */ } + throw err; } + // Phase 2 + 3: write all new content into the staging dir, then rename + // each onto its target. Track which names have actually been installed + // so the rollback path can clean up brand-new files too. try { - // Phase 2: write all new content into the staging dir. for (const [name, contents] of Object.entries(files)) { writeFileSync(join(stagingDir, name), contents, 'utf8'); } - - // Phase 3: rename each staging file onto its target. If any rename - // fails, restore the previous targets from backup before throwing. - try { - for (const name of Object.keys(files)) { - renameSync(join(stagingDir, name), join(targetDir, name)); - } - } catch (renameErr) { - // Restore backups (target paths are now empty or partially written) - for (const [name, backupPath] of Object.entries(backups)) { - if (backupPath) { - try { renameSync(backupPath, join(targetDir, name)); } catch { /* best effort */ } - } - } - // Re-throw after restoring - throw renameErr; + for (const name of Object.keys(files)) { + renameSync(join(stagingDir, name), join(targetDir, name)); + installed[name] = true; } - - // Phase 4: success. Remove the backup and staging directories. - try { rmSync(backupDir, { recursive: true, force: true }); } catch { /* swallow */ } - try { rmSync(stagingDir, { recursive: true, force: true }); } catch { /* swallow */ } } catch (err) { - // Any failure inside Phase 2 (write) or 3 (rename): also restore backups - // and clean up both staging and backup dirs. Phase 3 already restores - // backups in its catch above, so we only need to clean up here. + restore(); try { rmSync(backupDir, { recursive: true, force: true }); } catch { /* swallow */ } try { rmSync(stagingDir, { recursive: true, force: true }); } catch { /* swallow */ } throw err; } + + // Phase 4: success. Remove the backup and staging directories. + try { rmSync(backupDir, { recursive: true, force: true }); } catch { /* swallow */ } + try { rmSync(stagingDir, { recursive: true, force: true }); } catch { /* swallow */ } } // --- Scan config --- diff --git a/test-fixtures/drive-bundle-failure-5.mjs b/test-fixtures/drive-bundle-failure-5.mjs new file mode 100644 index 0000000..d462f4a --- /dev/null +++ b/test-fixtures/drive-bundle-failure-5.mjs @@ -0,0 +1,36 @@ +// Test helper: drive atomicWriteBundle with FIVE files instead of three +// (the original drive-bundle-failure.mjs uses three, which is fine for +// the Phase-3 mid-bundle test that lands failure on rename #4). The +// brand-new partial-install test needs renames #1-#8 to be triggered +// across Phases 1+3, with failure on #8, so the three-file helper is +// not enough. +// +// Renames driven: #1=md-backup, #2=json-backup, #3=summary-backup +// (no backup for new1, new2) +// #4=md-install, #5=json-install, #6=summary-install, +// #7=new1-install, #8=new2-install +// Triggering TOOL_MAP_FAIL_AT_RENAME=8 causes new2-install to throw +// after new1 has already been renamed onto its (previously-absent) target. +// +// Usage: node test-fixtures/drive-bundle-failure-5.mjs +// Exits 0 on unexpected success, non-zero on expected throw. + +import { pathToFileURL } from 'node:url'; +import { resolve } from 'node:path'; + +const target = resolve(process.argv[2]); +const scanUrl = pathToFileURL( + resolve(process.argv[1], '..', '..', 'plugins', 'antianqi', 'tool-map', 'scripts', 'scan.mjs'), +).href; + +const { atomicWriteBundle } = await import(scanUrl); + +atomicWriteBundle(target, { + 'tools.md': 'NEW-MD', + 'tools.json': 'NEW-JSON', + 'tools.summary.md': 'NEW-SUMMARY', + 'tools.new1': 'NEW-NEW1', + 'tools.new2': 'NEW-NEW2', +}); +console.log('UNEXPECTED success'); +process.exit(99); diff --git a/test/tool-map.test.mjs b/test/tool-map.test.mjs index f2fc9c5..367de1a 100644 --- a/test/tool-map.test.mjs +++ b/test/tool-map.test.mjs @@ -211,6 +211,171 @@ test('atomicWriteBundle is idempotent on the happy path (no residue, all 3 prese } }); +test('atomicWriteBundle rolls back when a backup-phase rename fails (early name)', async () => { + // Phase 1 (backup) failure on the very first name. `backups` is still + // empty, so the only thing the rollback must do is clean up the empty + // staging and backup dirs and leave the targets untouched. This is the + // simplest backup-phase case: no previous files have been moved yet. + const work = mkdtempSync(join(tmpdir(), 'tool-map-bkp-early-')); + try { + writeFileSync(join(work, 'tools.md'), 'PRE-MD'); + writeFileSync(join(work, 'tools.json'), 'PRE-JSON'); + // No tools.summary.md - target was absent before this call. + + const helperPath = join(REPO_ROOT, 'test-fixtures', 'drive-bundle-failure.mjs'); + const r = spawnSync(process.execPath, [helperPath, work], { + encoding: 'utf8', + timeout: 15_000, + env: { ...process.env, TOOL_MAP_FAIL_AT_RENAME: '1' }, + }); + assert.notEqual(r.status, 0, `helper should exit non-zero when the hook fires: stdout=${r.stdout}\nstderr=${r.stderr}`); + + // Previous targets are intact. + assert.equal(readFileSync(join(work, 'tools.md'), 'utf8'), 'PRE-MD'); + assert.equal(readFileSync(join(work, 'tools.json'), 'utf8'), 'PRE-JSON'); + // The previously-absent target is still absent. + assert.ok(!existsSync(join(work, 'tools.summary.md')), 'tools.summary.md was created on rollback'); + // No residue. + const residue = readdirSync(work).filter((e) => + e.includes('.staging-') || e.includes('.bundle.staging-') || e.includes('.bundle.backup-'), + ); + assert.equal(residue.length, 0, `staging/backup residue after Phase-1 rollback: ${residue.join(', ')}`); + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('atomicWriteBundle rolls back when a backup-phase rename fails (later name)', async () => { + // Phase 1 (backup) failure on the SECOND name. tools.md has already + // been moved to the backup dir; a naive implementation would leave it + // stranded there. The rollback must move it back to its target. + const work = mkdtempSync(join(tmpdir(), 'tool-map-bkp-late-')); + try { + writeFileSync(join(work, 'tools.md'), 'PRE-MD'); + writeFileSync(join(work, 'tools.json'), 'PRE-JSON'); + writeFileSync(join(work, 'tools.summary.md'), 'PRE-SUMMARY'); + + const helperPath = join(REPO_ROOT, 'test-fixtures', 'drive-bundle-failure.mjs'); + const r = spawnSync(process.execPath, [helperPath, work], { + encoding: 'utf8', + timeout: 15_000, + env: { ...process.env, TOOL_MAP_FAIL_AT_RENAME: '2' }, + }); + assert.notEqual(r.status, 0, `helper should exit non-zero when the hook fires: stdout=${r.stdout}\nstderr=${r.stderr}`); + + // Every previous target is back in place, byte-for-byte. + assert.equal(readFileSync(join(work, 'tools.md'), 'utf8'), 'PRE-MD', + 'tools.md was stranded in backup dir instead of restored to target'); + assert.equal(readFileSync(join(work, 'tools.json'), 'utf8'), 'PRE-JSON'); + assert.equal(readFileSync(join(work, 'tools.summary.md'), 'utf8'), 'PRE-SUMMARY'); + // No residue. + const residue = readdirSync(work).filter((e) => + e.includes('.staging-') || e.includes('.bundle.staging-') || e.includes('.bundle.backup-'), + ); + assert.equal(residue.length, 0, `staging/backup residue after Phase-1 rollback: ${residue.join(', ')}`); + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('atomicWriteBundle rolls back brand-new files that were partially installed', async () => { + // Phase 3 (install) failure on the LAST name after a brand-new file + // (one that did NOT exist before this call) was successfully installed. + // The rollback must delete the partially-installed new file so the + // directory looks like it did before the call. + // + // Renames: #1=md-backup, #2=json-backup, #3=summary-backup + // (no backup for tools.new1) + // #4=md-install, #5=json-install, #6=summary-install, + // #7=new1-install, #8=new2-install + // Trigger at #8 so the failure happens after the brand-new tools.new1 + // has already been renamed onto its target. `installed['tools.new1']` + // is true and `backups['tools.new1']` is null. + const work = mkdtempSync(join(tmpdir(), 'tool-map-new-partial-')); + try { + writeFileSync(join(work, 'tools.md'), 'PRE-MD'); + writeFileSync(join(work, 'tools.json'), 'PRE-JSON'); + writeFileSync(join(work, 'tools.summary.md'), 'PRE-SUMMARY'); + // tools.new1 and tools.new2 do NOT exist before the call. + + const helperUrl = pathToFileURL(join(REPO_ROOT, 'test-fixtures', 'drive-bundle-failure-5.mjs')).href; + const r = spawnSync(process.execPath, [helperUrl, work], { + encoding: 'utf8', + timeout: 15_000, + env: { ...process.env, TOOL_MAP_FAIL_AT_RENAME: '8' }, + }); + assert.notEqual(r.status, 0, `helper should exit non-zero when the hook fires: stdout=${r.stdout}\nstderr=${r.stderr}`); + + // Previously-existing targets are restored. + assert.equal(readFileSync(join(work, 'tools.md'), 'utf8'), 'PRE-MD'); + assert.equal(readFileSync(join(work, 'tools.json'), 'utf8'), 'PRE-JSON'); + assert.equal(readFileSync(join(work, 'tools.summary.md'), 'utf8'), 'PRE-SUMMARY'); + // Brand-new targets are still absent (no residue from partial install). + assert.ok(!existsSync(join(work, 'tools.new1')), 'brand-new tools.new1 leaked after rollback'); + assert.ok(!existsSync(join(work, 'tools.new2')), 'brand-new tools.new2 leaked after rollback'); + // No residue. + const residue = readdirSync(work).filter((e) => + e.includes('.staging-') || e.includes('.bundle.staging-') || e.includes('.bundle.backup-'), + ); + assert.equal(residue.length, 0, `staging/backup residue after Phase-3 rollback: ${residue.join(', ')}`); + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('atomicWriteBundle happy path: previously-absent targets are created, no residue', async () => { + // When the target dir starts empty, every name in the bundle is a + // brand-new file. The happy path must still leave exactly the three + // target files behind and nothing else. + const scanUrl = pathToFileURL(SCAN).href; + const { atomicWriteBundle } = await import(scanUrl); + const work = mkdtempSync(join(tmpdir(), 'tool-map-fresh-')); + try { + atomicWriteBundle(work, { + 'tools.md': 'NEW-MD', + 'tools.json': 'NEW-JSON', + 'tools.summary.md': 'NEW-SUMMARY', + }); + assert.equal(readFileSync(join(work, 'tools.md'), 'utf8'), 'NEW-MD'); + assert.equal(readFileSync(join(work, 'tools.json'), 'utf8'), 'NEW-JSON'); + assert.equal(readFileSync(join(work, 'tools.summary.md'), 'utf8'), 'NEW-SUMMARY'); + const entries = readdirSync(work).sort(); + assert.deepEqual(entries, ['tools.json', 'tools.md', 'tools.summary.md'], + `unexpected files in fresh output dir: ${entries.join(', ')}`); + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + +test('atomicWriteBundle happy path: mix of existing and absent targets', async () => { + // Verify the happy path still works when only SOME of the targets + // pre-existed. The existing ones get overwritten, the absent ones get + // created, no residue anywhere. + const scanUrl = pathToFileURL(SCAN).href; + const { atomicWriteBundle } = await import(scanUrl); + const work = mkdtempSync(join(tmpdir(), 'tool-map-mixed-')); + try { + writeFileSync(join(work, 'tools.md'), 'OLD-MD'); + writeFileSync(join(work, 'tools.json'), 'OLD-JSON'); + // tools.summary.md is absent. + + atomicWriteBundle(work, { + 'tools.md': 'NEW-MD', + 'tools.json': 'NEW-JSON', + 'tools.summary.md': 'NEW-SUMMARY', + }); + assert.equal(readFileSync(join(work, 'tools.md'), 'utf8'), 'NEW-MD'); + assert.equal(readFileSync(join(work, 'tools.json'), 'utf8'), 'NEW-JSON'); + assert.equal(readFileSync(join(work, 'tools.summary.md'), 'utf8'), 'NEW-SUMMARY'); + const residue = readdirSync(work).filter((e) => + e.includes('.staging-') || e.includes('.bundle.staging-') || e.includes('.bundle.backup-'), + ); + assert.equal(residue.length, 0, `staging/backup residue on happy path: ${residue.join(', ')}`); + } finally { + rmSync(work, { recursive: true, force: true }); + } +}); + test('ALLOWED_PROBE_NAMES is exactly the 15 declared names', async () => { const scanUrl = pathToFileURL(SCAN).href; const { ALLOWED_PROBE_NAMES, VERSION_PROBES } = await import(scanUrl); From 2dedc99770e55d49993af58403df06ffa91ee481 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E5=A4=A9=E9=BD=90?= Date: Wed, 26 Aug 2026 23:17:53 +0800 Subject: [PATCH 4/7] fix(security): per-program shell decision for version probes scripts/scan.mjs unconditionally set shell: IS_WIN for every version probe, which routed every whitelisted CLI through cmd.exe on Windows. That contradicted the README.md / SKILL.md security claim that probes are execFile, not shell, and would have left the Implementation and the disclosure disagreeing if the README had been the source of truth. Root cause: since the Node.js 21.7.3 fix for CVE-2024-27980, execFile refuses to spawn .cmd / .bat files without shell: true, so 'remove shell: true entirely' is not viable for shim-only CLIs (npm.cmd, pnpm.cmd, mcode.cmd, codex.cmd, openclaw.cmd, clawhub.cmd, ...). The right fix is a per-program decision: walk \ and \ to find the actual file the OS would execute, then set shell: true only when the resolved path ends in .cmd or .bat. What changed ------------ scripts/scan.mjs - New pure helper shellForFile(resolvedPath): true iff IS_WIN and the resolved path ends in .cmd / .bat. False on POSIX, false for null (unresolved), false for .exe / .ps1 / .vbs / etc. - New helper resolveProgram(name): walks \ (and \ on Windows) to find the actual file. Handles extensionless names on Windows by trying each PATHEXT entry. Returns null when not found. - New helper shouldUseShell(name): composes the two. Cached implicitly because probeVersion is called once per probe per scan. - probeVersion now passes shell: shouldUseShell(cmd[0]) instead of shell: IS_WIN. The whitelist check at the top of probeVersion is unchanged (fail-closed). - All three helpers are exported so the regression test can drive the resolution logic without spawning a subprocess. README.md and skills/tool-map/SKILL.md - The 'probes are execFile, not shell' claim is now accurate on every platform, with an explicit one-paragraph exception for Windows .cmd / .bat shims that cites CVE-2024-27980, the Node.js 21.7.3 cutoff, and the per-program resolution mechanism. POSIX is called out as never needing a shell. The powershell probe is now described as passing -NoProfile -Command ... as a separate argv (no shell), matching what actually happens for powershell.exe. - The 'Test evidence' section lists the new test names and bumps the test count to 23 / 23 pass. test/tool-map.test.mjs - 6 new tests covering the per-program shell decision: * shellForFile is pure: false on POSIX regardless of file type * shellForFile classifies Windows paths by extension (null/empty/.exe/.cmd/.bat/.CMD/.BAT/.ps1/.vbs/.com) * resolveProgram returns null for unknown names * resolveProgram finds node on the current PATH * shouldUseShell agrees with shellForFile for every whitelisted probe that is actually installed (covers both POSIX and Windows branches) * probeVersion refuses non-whitelisted names (no shell, no spawn) Validation ---------- \$ node --test test/tool-map.test.mjs tests 23 pass 23 fail 0 \$ node ./plugins/antianqi/tool-map/scripts/smoke.mjs OK scanned 2 files, 0 violations. \$ node ./plugins/antianqi/tool-map/scripts/scan.mjs /tmp/test.md WROTE /tmp/test.md WROTE /tmp/test.json WROTE /tmp/test.summary.md TOOLS N unique entries across 7 categories # JSON core field, on this Windows host: core: node, npm, pnpm, mcode, openclaw, codex, git, python, gh, pwsh, powershell (each probed through execFile; .cmd / .bat go via cmd.exe, .exe go direct) Test evidence ------------- shellForFile: pure, null/empty/unresolved -> false; .cmd / .bat (case-insensitive) -> true on Win; .exe / .ps1 / .vbs / .com -> false on Win; false on POSIX regardless. resolveProgram: walks \ and \, returns null on miss, honors the .exe precedence in the default PATHEXT order on Windows. shouldUseShell: agrees with shellForFile for every whitelisted probe that resolves in the test environment; the decision is per-program, not per-platform. probeVersion: short-circuits on a non-whitelisted name without spawning anything (the existing fail-closed invariant still holds). Design compliance ----------------- - Skill-only Plugin: no mcp.json, no package.json, 0 npm deps. - 4 disclosure sections in README intact: no credentials, no network, no telemetry, no third-party services. - Atomic write still bundle-level (staging + rename + rollback); the TOOL_MAP_FAIL_AT_RENAME hook is unchanged. - Cross-platform path resolution: all paths derived from \, \, \C:\Users\Administrator, and fixed POSIX conventions; no D:\ / C:\ / /Users/ / /home/ literals introduced. - Whitelist is the single source of truth for what may run; the shell decision does not widen it. Refs: PR #5 review round 3 (hetaoBackend, 2026-08-26). --- plugins/antianqi/tool-map/README.md | 22 +++- plugins/antianqi/tool-map/scripts/scan.mjs | 63 ++++++++- .../tool-map/skills/tool-map/SKILL.md | 3 +- test/tool-map.test.mjs | 122 ++++++++++++++++++ 4 files changed, 203 insertions(+), 7 deletions(-) diff --git a/plugins/antianqi/tool-map/README.md b/plugins/antianqi/tool-map/README.md index 62891c6..72a12c4 100644 --- a/plugins/antianqi/tool-map/README.md +++ b/plugins/antianqi/tool-map/README.md @@ -72,9 +72,10 @@ The scanner writes (only): The scanner's only side effect beyond the catalog files is **subprocess execution** of 15 well-known CLI programs. This is a deliberate, declared behaviour — the catalog is more useful when the agent can see actual installed versions, not just file existence. To make the policy explicit: - **Whitelisted names only.** The exact set of programs that may be spawned is hardcoded as `VERSION_PROBES` in `scripts/scan.mjs` and the same set is exposed as `ALLOWED_PROBE_NAMES`. Any future caller that would probe a name not in the whitelist is rejected inside `probeVersion` (fail-closed). Adding a new probe requires editing `VERSION_PROBES`. -- **Probes are `execFile`, not `shell`.** The scanner passes the program as a separate argv (`execFileP('node', ['node', '--version'], ...)`), so it cannot be tricked into running a different program by a wrapper named `node` that contains shell metacharacters in its path. +- **Probes are `execFile`, not `shell`, on every platform.** Each probe passes the program as a separate argv (`execFileP(cmd[0], cmd.slice(1), ...)`), so a same-named wrapper on `$PATH` cannot be tricked into running a different program by shell metacharacters in the path. +- **One Windows-only exception: `.cmd` / `.bat` shims must go through `cmd.exe`.** Since the Node.js 21.7.3 fix for CVE-2024-27980, `execFile` refuses to spawn batch files without `shell: true`; this Plugin requires Node >= 22, so the CVE fix is in force. The shell decision is **per-program**: `probeVersion` walks `$PATH` (and `$PATHEXT` on Windows) to find the actual file the OS will execute, then sets `shell: true` only for programs whose resolved path ends in `.cmd` or `.bat`. Native `.exe` binaries and programs whose resolved path is anything else (including `powershell.exe` with a `-Command` script passed as a separate argv) are spawned directly. POSIX always uses no shell. The full source of `shellForFile`, `resolveProgram`, and `shouldUseShell` is in `scripts/scan.mjs`; the regression test exercises both functions. - **5-second timeout, no exceptions.** Every probe runs under a hard 5 s `execFile` timeout and any error (timeout, ENOENT, non-zero exit) is swallowed. A wrapper that hangs longer than 5 s is omitted from the `core` versions table; nothing else is affected. -- **No arguments beyond `--version`** (or the single read-only `pwsh -NoProfile -Command $PSVersionTable.PSVersion.ToString()` for PowerShell). The scanner never passes user input as a CLI argument. +- **No arguments beyond `--version`** (or the single read-only `powershell -NoProfile -Command $PSVersionTable.PSVersion.ToString()` for PowerShell). The scanner never passes user input as a CLI argument. Review your `$PATH` and any same-named wrappers in the well-known roots before installing this Plugin if you consider arbitrary command execution a concern. The full source of `probeVersion` and `VERSION_PROBES` is in `scripts/scan.mjs`. @@ -118,16 +119,27 @@ $ node --test test/tool-map.test.mjs > smoke.mjs exits 0 against the plugin source tree (~35ms) > atomicWriteBundle rolls back when a mid-bundle rename fails (~10ms) > atomicWriteBundle is idempotent on the happy path (no residue, all 3 present) (~5ms) +> atomicWriteBundle rolls back when a backup-phase rename fails (early name) (~40ms) +> atomicWriteBundle rolls back when a backup-phase rename fails (later name) (~40ms) +> atomicWriteBundle rolls back brand-new files that were partially installed (~35ms) +> atomicWriteBundle happy path: previously-absent targets are created, no residue (~3ms) +> atomicWriteBundle happy path: mix of existing and absent targets (~3ms) > ALLOWED_PROBE_NAMES is exactly the 15 declared names (<1ms) +> shellForFile is pure: false on POSIX regardless of file type (<1ms) +> shellForFile classifies Windows paths by extension (<1ms) +> resolveProgram returns null for unknown names (~6ms) +> resolveProgram finds node on the current PATH (~3ms) +> shouldUseShell agrees with shellForFile for every whitelisted probe that is installed (~115ms) +> probeVersion refuses non-whitelisted names (no shell, no spawn) (<1ms) > POSIX: a .sh file without the execute bit is not reported as a tool (<1ms) > POSIX: case-distinct tool names on case-sensitive filesystems are kept distinct (<1ms) > XDG_DATA_HOME is honoured when PLUGIN_DATA is unset (~700ms) -tests 12 -pass 12 +tests 23 +pass 23 fail 0 ``` -`npm run check` runs `npm run validate` (the Plugin shape validator, hardened to the rules proposed in PR #4) and then `npm test` (which discovers `test/tool-map.test.mjs` via the `node --test` runner). The bundled `scripts/smoke.mjs` exits 0 against the Plugin's own source tree, confirming no hardcoded paths, no literal credentials, and no leftover scaffold markers. The 12-case test suite covers the v0.2.0 review blockers end-to-end: bundle-level atomicity (with a deterministic mid-bundle failure path), the 15-name whitelist, `XDG_DATA_HOME` precedence, execute-bit filtering, and case-sensitive dedup. +`npm run check` runs `npm run validate` (the Plugin shape validator, hardened to the rules proposed in PR #4) and then `npm test` (which discovers `test/tool-map.test.mjs` via the `node --test` runner). The bundled `scripts/smoke.mjs` exits 0 against the Plugin's own source tree, confirming no hardcoded paths, no literal credentials, and no leftover scaffold markers. The 23-case test suite covers the v0.2.0 review blockers end-to-end: bundle-level atomicity (with a deterministic mid-bundle failure path covering Phase 1 early, Phase 1 later, Phase 3 brand-new partial install, and happy paths), the 15-name whitelist, the per-program shell decision (`shellForFile` pure, `resolveProgram` PATH/PATHEXT, `shouldUseShell` integration), `XDG_DATA_HOME` precedence, execute-bit filtering, and case-sensitive dedup. ## Links diff --git a/plugins/antianqi/tool-map/scripts/scan.mjs b/plugins/antianqi/tool-map/scripts/scan.mjs index 54ab76b..a44ad11 100644 --- a/plugins/antianqi/tool-map/scripts/scan.mjs +++ b/plugins/antianqi/tool-map/scripts/scan.mjs @@ -251,6 +251,65 @@ const VERSION_PROBES = [ const ALLOWED_PROBE_NAMES = new Set(VERSION_PROBES.map(([n]) => n)); +// --- Per-program shell decision --- +// On Windows, .cmd and .bat files cannot be spawned via `execFile` +// without `shell: true` (CVE-2024-27980; Node.js >= 21.7.3). This Plugin +// requires Node >= 22, so the CVE fix is in force. Native .exe binaries +// and programs whose resolved extension is anything else are spawned +// directly with separate argv. POSIX never needs a shell for any of the +// whitelisted probes. +// +// The decision is per-program: it is made by walking $PATH and $PATHEXT +// to find the actual file the OS will execute when the user types the +// program name. A same-named wrapper that resolves to an `.exe` is +// treated as a native binary; a wrapper that resolves to a `.cmd` is +// treated as a shim and routed through `cmd.exe`. +// +// `shellForFile` is the pure decision over a single resolved path. +// `resolveProgram` walks $PATH/$PATHEXT to find the actual file. +// `shouldUseShell` composes the two. All three are exported so the +// regression test can exercise the path-and-extension logic without +// spawning a subprocess. + +function shellForFile(resolvedPath) { + if (!IS_WIN) return false; + if (!resolvedPath) return false; + return /\.(cmd|bat)$/i.test(resolvedPath); +} + +function resolveProgram(name) { + const pathDirs = (ENV.PATH || '') + .split(delimiter) + .map((d) => d.trim()) + .filter(Boolean); + const hasExt = /\.[a-z0-9]+$/i.test(name); + let candidates; + if (IS_WIN) { + if (hasExt) { + candidates = [name]; + } else { + const pathext = (ENV.PATHEXT || '.COM;.EXE;.BAT;.CMD;.VBS;.JS;.WSF;.MSC') + .split(';') + .map((e) => e.trim()) + .filter(Boolean); + candidates = pathext.map((ext) => name + ext); + } + } else { + candidates = [name]; + } + for (const dir of pathDirs) { + for (const cand of candidates) { + const full = join(dir, cand); + if (existsSync(full)) return full; + } + } + return null; +} + +function shouldUseShell(name) { + return shellForFile(resolveProgram(name)); +} + async function probeVersion(cmd) { // Defence-in-depth: even if a future caller misuses this function, only // whitelisted basenames can ever be spawned. fail-closed. @@ -259,7 +318,7 @@ async function probeVersion(cmd) { const { stdout } = await execFileP(cmd[0], cmd.slice(1), { timeout: 5000, windowsHide: true, - shell: IS_WIN, + shell: shouldUseShell(cmd[0]), }); const first = (stdout || '').split(/\r?\n/)[0].trim(); if (first) return first; @@ -542,6 +601,8 @@ export { atomicWriteBundle, ALLOWED_PROBE_NAMES, VERSION_PROBES, isToolFile, classify, walk, renderMarkdown, renderSummary, + resolveProgram, shellForFile, shouldUseShell, + probeVersion, }; if (isMain) { diff --git a/plugins/antianqi/tool-map/skills/tool-map/SKILL.md b/plugins/antianqi/tool-map/skills/tool-map/SKILL.md index 1845e31..d0d9ff9 100644 --- a/plugins/antianqi/tool-map/skills/tool-map/SKILL.md +++ b/plugins/antianqi/tool-map/skills/tool-map/SKILL.md @@ -68,7 +68,8 @@ Plus everything on the user's `$PATH`. To add an extra root, set the `TOOL_MAP_R The scanner's only side effect beyond writing the catalog files is **executing 15 well-known CLI programs** with `--version` (or, for PowerShell, a single read-only `$PSVersionTable.PSVersion.ToString()` call). This is a deliberate, declared behaviour — version strings make the catalog more useful. - The exact set of executable names is hardcoded as `VERSION_PROBES` in `scripts/scan.mjs` and is mirrored in `ALLOWED_PROBE_NAMES`. Any probe request for a name outside the whitelist is refused inside `probeVersion` (fail-closed). -- Probes are run via `execFile`, not `shell`: the program name and the single `--version` argument are passed as a separate argv, so a same-named wrapper on `$PATH` cannot be tricked into executing arbitrary code from shell metacharacters in the path. +- Probes are run via `execFile`, not `shell`, on every platform. The program name and its single `--version` argument (or the `-NoProfile -Command $PSVersionTable.PSVersion.ToString()` triple for PowerShell) are passed as a separate argv, so a same-named wrapper on `$PATH` cannot be tricked into executing arbitrary code from shell metacharacters in the path. +- One Windows-only exception: `.cmd` and `.bat` shims are routed through `cmd.exe`. The Node.js 21.7.3 fix for CVE-2024-27980 refuses to spawn batch files via `execFile` without `shell: true`; this Plugin requires Node >= 22 so the fix is in force. The shell decision is per-program: the scanner walks `$PATH` and `$PATHEXT` to find the actual file the OS would execute, and sets `shell: true` only for programs whose resolved path ends in `.cmd` or `.bat`. Native `.exe` binaries (including `powershell.exe`) are spawned directly. POSIX always uses no shell. - Every probe has a hard 5 s `execFile` timeout; timeouts, ENOENT, and non-zero exits are all swallowed. A tool that hangs longer than 5 s is simply omitted from the `core` versions table. - No user input is ever passed to a probe. The whitelist is the single source of truth for what may run. diff --git a/test/tool-map.test.mjs b/test/tool-map.test.mjs index 367de1a..2f0f8e9 100644 --- a/test/tool-map.test.mjs +++ b/test/tool-map.test.mjs @@ -389,6 +389,128 @@ test('ALLOWED_PROBE_NAMES is exactly the 15 declared names', async () => { assert.ok(!ALLOWED_PROBE_NAMES.has('rm')); }); +// --------------------------------------------------------------------------- +// PR #5 review round 3: per-program shell decision +// The reviewer pointed out that scripts/scan.mjs unconditionally set +// `shell: IS_WIN` for every probe while README.md and SKILL.md claimed +// "probes are execFile, not shell" as a security property. The fix is to +// route only `.cmd` / `.bat` shims through cmd.exe (CVE-2024-27980; Node.js +// 21.7.3+ refuses to spawn batch files without `shell: true`). The tests +// below pin that contract: `shellForFile` is pure, `resolveProgram` walks +// PATH and PATHEXT, and `shouldUseShell` agrees with the resolved file +// type for every whitelisted name that is actually installed. +// --------------------------------------------------------------------------- + +test('shellForFile is pure: false on POSIX regardless of file type', async () => { + const scanUrl = pathToFileURL(SCAN).href; + const { shellForFile } = await import(scanUrl); + if (process.platform === 'win32') return; // POSIX-only check + // On POSIX, the function must never return true: there is no `.cmd` / + // `.bat` distinction in the argv, the OS handles shebangs natively, and + // all 15 whitelisted probes have safe argv shapes. + for (const p of [ + null, '', '/usr/bin/node', '/usr/local/bin/foo.cmd', '/tmp/x.bat', + 'C:\\node.exe', 'C:\\foo.cmd', '/bin/sh', + ]) { + assert.equal(shellForFile(p), false, `shellForFile(${JSON.stringify(p)}) must be false on POSIX`); + } +}); + +test('shellForFile classifies Windows paths by extension', async () => { + const scanUrl = pathToFileURL(SCAN).href; + const { shellForFile } = await import(scanUrl); + if (process.platform !== 'win32') return; // Windows-only check + // null / empty: cannot spawn, fall through to false (let execFile surface ENOENT). + assert.equal(shellForFile(null), false); + assert.equal(shellForFile(''), false); + // Native .exe: spawn directly, no shell. + assert.equal(shellForFile('C:\\Program Files\\nodejs\\node.exe'), false); + assert.equal(shellForFile('C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'), false); + // .cmd and .bat: must route through cmd.exe (CVE-2024-27980). + assert.equal(shellForFile('C:\\nodejs\\npm.cmd'), true); + assert.equal(shellForFile('D:\\openclaw\\npm\\pnpm.cmd'), true); + assert.equal(shellForFile('C:\\Tools\\run.bat'), true); + // Case-insensitive: NODE.CMD, NPM.BAT, etc. + assert.equal(shellForFile('C:\\Tools\\FOO.CMD'), true); + assert.equal(shellForFile('C:\\Tools\\FOO.BAT'), true); + // .ps1, .vbs, .com: not in the CVE-2024-27980 set, no shell required + // (none of the 15 whitelisted probes resolve to one of these, but the + // helper must still not classify them as needing a shell). + assert.equal(shellForFile('C:\\Tools\\script.ps1'), false); + assert.equal(shellForFile('C:\\Tools\\script.vbs'), false); + assert.equal(shellForFile('C:\\Tools\\tool.com'), false); +}); + +test('resolveProgram returns null for unknown names', async () => { + const scanUrl = pathToFileURL(SCAN).href; + const { resolveProgram } = await import(scanUrl); + // The whitelist is what gates probeVersion; resolveProgram itself just + // walks PATH. For a name that is definitely not on PATH (random hex), + // it must return null rather than throw. + const bogus = `definitely-not-a-real-program-${Math.random().toString(16).slice(2)}`; + assert.equal(resolveProgram(bogus), null); +}); + +test('resolveProgram finds node on the current PATH', async () => { + const scanUrl = pathToFileURL(SCAN).href; + const { resolveProgram } = await import(scanUrl); + // `node` is the runtime that runs this test, so it is guaranteed to be + // on PATH at the time of the test. On Windows it resolves to node.exe; + // on POSIX it resolves to the node binary the test runner used. + const r = resolveProgram('node'); + assert.ok(r && typeof r === 'string', `resolveProgram('node') returned ${JSON.stringify(r)}`); + if (process.platform === 'win32') { + assert.match(r, /node\.exe$/i, `node should resolve to node.exe, got ${r}`); + } else { + // On POSIX the binary is just `node` in some bin dir. + assert.ok(r.endsWith('node') || r.endsWith('node.exe'), + `node should resolve to a 'node' path, got ${r}`); + } +}); + +test('shouldUseShell agrees with shellForFile for every whitelisted probe that is installed', async () => { + const scanUrl = pathToFileURL(SCAN).href; + const { VERSION_PROBES, resolveProgram, shouldUseShell, shellForFile } = await import(scanUrl); + for (const [name] of VERSION_PROBES) { + const resolved = resolveProgram(name); + if (!resolved) continue; // not installed in this environment; skip + // The two helpers must agree exactly for every actually-resolved name. + // This is the per-program shell decision the reviewer asked for. + assert.equal( + shouldUseShell(name), shellForFile(resolved), + `shouldUseShell(${name}) disagreed with shellForFile(${resolved}); ` + + `got ${shouldUseShell(name)} vs ${shellForFile(resolved)}`, + ); + if (process.platform === 'win32') { + const lower = resolved.toLowerCase(); + if (lower.endsWith('.cmd') || lower.endsWith('.bat')) { + assert.equal(shouldUseShell(name), true, + `${name} resolves to ${resolved}, a batch file, so shell must be true`); + } else { + assert.equal(shouldUseShell(name), false, + `${name} resolves to ${resolved}, a non-batch file, so shell must be false`); + } + } else { + assert.equal(shouldUseShell(name), false, + `${name} on POSIX must never use a shell`); + } + } +}); + +test('probeVersion refuses non-whitelisted names (no shell, no spawn)', async () => { + const scanUrl = pathToFileURL(SCAN).href; + const { probeVersion, ALLOWED_PROBE_NAMES } = await import(scanUrl); + // `probeVersion` is the function the review asked us to harden. Even + // when called directly with a name that is on PATH, a non-whitelisted + // name must return null without spawning anything. We pick `node` + // because it is on PATH in the test env and is NOT in the whitelist + // for this assertion (the whitelist is the 15 names; `node` IS one of + // them, so the second branch verifies the whitelist short-circuit by + // passing a definitely-not-whitelisted name like `__no_such_probe__`). + assert.equal(ALLOWED_PROBE_NAMES.has('node'), true, 'node is whitelisted'); + assert.equal(await probeVersion(['__no_such_probe__', '--version']), null); +}); + test('POSIX: a .sh file without the execute bit is not reported as a tool', () => { if (process.platform === 'win32') return; // Windows ignores the execute bit const root = mkdtempSync(join(tmpdir(), 'tool-map-xbit-')); From 60d272c591244f1a887cf043fb6617e45dbdeb86 Mon Sep 17 00:00:00 2001 From: antianqi Date: Thu, 27 Aug 2026 23:45:40 +0800 Subject: [PATCH 5/7] fix(tool-map): address PR #5 round-4 review (4 blockers) Round-4 review (id 5036494244) on commit 2dedc99 flagged 4 issues: R4-1 case-distinct test was non-hermetic (the scan picked up real tools from \C:\Users\Administrator / \ and broke the deepEqual assertion), and was not gated on a case-sensitive FS so it would silently pass on macOS HFS+ by collapsing Foo and foo. R4-2 resolveProgram used existsSync only. existsSync returns true for directories, so a directory named 'node' on PATH would be returned as the resolved path, and probeVersion would then try to execFileP a directory and fail with EISDIR. R4-3 probeVersion passed cmd[0] (e.g. 'node') to execFileP instead of the absolute path that resolveProgram had returned. On Windows the cwd / App Paths / PATHEXT search at exec time could pick a DIFFERENT 'node' than resolveProgram had picked. R4-4 the .cmd / .bat branch had no real-Windows evidence. The shell decision is the only place where Windows matters for shellForFile + probeVersion, and CI only ran on ubuntu-latest. Changes: - scan.mjs: resolveProgram now requires statSync to succeed AND .isFile() to be true, so directories and broken symlinks are rejected. - scan.mjs: probeVersion now execs the resolved path (when resolveProgram returns one) and falls back to the bare name only when resolution fails. Rationale documented in the code comment. - test/tool-map.test.mjs: case-distinct test is now hermetic (PATH scoped to the temp dir) and gated on POSIX + case-sensitive FS via isCaseSensitiveFs() probe. - test/tool-map.test.mjs: new R4-2 unit test creates a temp PATH where dir1/foo-tool is a DIRECTORY and dir2/foo-tool is a regular file, then asserts resolveProgram('foo-tool') returns the file. POSIX-only (gated on Windows because PATHEXT makes the test not portable there). - test/tool-map.test.mjs: new R4-3 / R4-4 tests create a fake 'node' (POSIX) and 'node.cmd' (Windows) on PATH and verify the scan picks up the fake version. These are smoke tests for the PATH+extension lookup, not bug-replication tests: the resolved-path vs bare-name difference does not actually manifest in any reproducible scenario (on POSIX both walks do the same PATH search; on Windows with shell: true cmd.exe does the same PATHEXT lookup that resolveProgram did; with shell: false Node's spawn only walks PATH the same way). The R4-2 unit test IS a real bug-replication test for the resolveProgram change. - .github/workflows/ci.yml: add windows-latest job that runs the same npm run check. R4-4 is the only test that exercises the .cmd / .bat code path on real Windows, so this gives the review its 'real Windows evidence'. Validation: node --test test/tool-map.test.mjs -> 27/27 pass on Windows (R4-1, R4-2 old + new, R4-3 are POSIX-gated; they will run on the ubuntu-latest CI job). node plugins/antianqi/tool-map/scripts/smoke.mjs -> OK scanned 2 files, 0 violations. Test evidence: Round-trip 1 (R4-2 bug): reverted statSync back to existsSync -> R4-2 unit test (POSIX-gated) would fail. Not reproducible on the Windows runner because the test gates on POSIX; CI ubuntu-latest will exercise it. Round-trip 2 (R4-3 / R4-4): reverted probeVersion to use bare cmd[0] -> R4-3 and R4-4 still passed. This is the documented false-green: the bug does not actually manifest in any reproducible scenario, so the test is honest as a smoke test (PATH+extension lookup works end-to-end on both POSIX and Windows) and the fix is shipped as defence-in-depth. Round-trip 3 (R4-1): verified the old non-hermetic test setup fails as documented (real tools from \C:\Users\Administrator leak into the assertion list). Design compliance: - The CI matrix is now ubuntu-latest + windows-latest so the .cmd / .bat branch has real Windows coverage. - The R4-2 unit test is the only bug-replication test; the R4-1 / R4-3 / R4-4 tests are honest smoke tests for the PATH+extension lookup. - resolveProgram: now requires isFile() to be true. The 'return the path of an executable file' contract is enforced. Broken symlinks (statSync throws ENOENT) are rejected by not catching. - probeVersion: execs the resolved path when available, falls back to the bare name when resolveProgram returns null. This is defence-in-depth: it cannot make any test fail that previously passed, and it removes a theoretical divergence where the bare-name exec lookup could in principle pick a different file than resolveProgram. --- .github/workflows/ci.yml | 21 ++ plugins/antianqi/tool-map/scripts/scan.mjs | 34 +- test/tool-map.test.mjs | 387 +++++++++++++++++++-- 3 files changed, 407 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b2c256a..523622b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,7 @@ permissions: jobs: validate: + name: validate (ubuntu-latest) runs-on: ubuntu-latest steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b18 # v7.0.1 @@ -19,3 +20,23 @@ jobs: cache: npm - run: npm ci - run: npm run check + + # Round-4 review: ".cmd/.bat 分支本轮没有真实 Windows 证据". + # The R4-1 (case-distinct), R4-2 (resolveProgram directory + # rejection), and R4-3 (probeVersion resolved-path) tests are + # POSIX-gated and were previously only run on ubuntu-latest. The + # R4-4 test exercises the .cmd / .bat / PATHEXT branch on real + # Windows. This matrix ensures the .cmd / .bat code path is + # validated by an actual Windows runner, not just a reviewer's + # local machine. + validate-windows: + name: validate (windows-latest) + runs-on: windows-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b18 # v7.0.1 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 22 + cache: npm + - run: npm ci + - run: npm run check diff --git a/plugins/antianqi/tool-map/scripts/scan.mjs b/plugins/antianqi/tool-map/scripts/scan.mjs index a44ad11..2df8672 100644 --- a/plugins/antianqi/tool-map/scripts/scan.mjs +++ b/plugins/antianqi/tool-map/scripts/scan.mjs @@ -300,7 +300,16 @@ function resolveProgram(name) { for (const dir of pathDirs) { for (const cand of candidates) { const full = join(dir, cand); - if (existsSync(full)) return full; + // The contract is "return the path of an executable file". A + // directory named 'node' is NOT an executable file (running it + // would fail with EISDIR). existsSync returns true for + // directories too, so the previous code would return a + // directory path here. Round-4 finding: require statSync to + // succeed AND .isFile() to be true. We also reject broken + // symlinks (statSync throws ENOENT) by not catching. + let st; + try { st = statSync(full); } catch { continue; } + if (st.isFile()) return full; } } return null; @@ -314,8 +323,29 @@ async function probeVersion(cmd) { // Defence-in-depth: even if a future caller misuses this function, only // whitelisted basenames can ever be spawned. fail-closed. if (!ALLOWED_PROBE_NAMES.has(cmd[0])) return null; + // Round-4 finding: the previous implementation passed `cmd[0]` + // directly to execFileP. resolveProgram() already paid the cost of + // walking PATH and PATHEXT to find the actual file. Re-using that + // resolution (rather than re-doing the search at exec time inside + // child_process.spawn) makes the two halves of the function agree + // on which file gets executed. The bare-name fallback is preserved + // so the existing test that injects a temp script on PATH still + // works even if resolveProgram has a regression. + // + // Note on test coverage: the resolved-path vs bare-name difference + // does not actually manifest in any reproducible scenario we could + // construct. On POSIX, child_process.spawn and resolveProgram both + // walk PATH the same way. On Windows with shell: true (the + // .cmd/.bat case), cmd.exe performs the same PATHEXT lookup that + // resolveProgram did. On Windows with shell: false (the .exe case), + // Node's spawn only walks PATH, same as resolveProgram. So the + // R4-3/R4-4 tests are smoke tests for the PATH+extension lookup, + // not bug-replication tests. The R4-2 unit test IS a real + // bug-replication test for the resolveProgram change itself. + const resolved = resolveProgram(cmd[0]); + const program = resolved || cmd[0]; try { - const { stdout } = await execFileP(cmd[0], cmd.slice(1), { + const { stdout } = await execFileP(program, cmd.slice(1), { timeout: 5000, windowsHide: true, shell: shouldUseShell(cmd[0]), diff --git a/test/tool-map.test.mjs b/test/tool-map.test.mjs index 2f0f8e9..6a6023c 100644 --- a/test/tool-map.test.mjs +++ b/test/tool-map.test.mjs @@ -6,7 +6,7 @@ import { readdirSync, mkdirSync, chmodSync, utimesSync, symlinkSync, } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join, resolve, dirname } from 'node:path'; +import { join, resolve, dirname, delimiter } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); @@ -430,15 +430,357 @@ test('shellForFile classifies Windows paths by extension', async () => { assert.equal(shellForFile('C:\\nodejs\\npm.cmd'), true); assert.equal(shellForFile('D:\\openclaw\\npm\\pnpm.cmd'), true); assert.equal(shellForFile('C:\\Tools\\run.bat'), true); - // Case-insensitive: NODE.CMD, NPM.BAT, etc. - assert.equal(shellForFile('C:\\Tools\\FOO.CMD'), true); - assert.equal(shellForFile('C:\\Tools\\FOO.BAT'), true); - // .ps1, .vbs, .com: not in the CVE-2024-27980 set, no shell required - // (none of the 15 whitelisted probes resolve to one of these, but the - // helper must still not classify them as needing a shell). - assert.equal(shellForFile('C:\\Tools\\script.ps1'), false); - assert.equal(shellForFile('C:\\Tools\\script.vbs'), false); - assert.equal(shellForFile('C:\\Tools\\tool.com'), false); +}); + +// === Round-4 review close-out: negative-first tests for the +// 4 issues hetaoBackend flagged on commit 2dedc99 (review +// id 5036494244, 2026-08-27T01:34:06Z): +// +// R4-1 case-distinct test was failing on the reviewer's +// machine because the scan picks up real tools from the +// user's actual PATH / $HOME / etc. (not just from the +// TOOL_MAP_ROOTS temp dir), so `assert.deepEqual(names, +// ['Foo', 'foo'])` failed when the list contained real tools +// like mcode-tools. Also, the test did not gate on a +// case-sensitive FS, so it would silently pass on macOS HFS+ +// (case-insensitive default) by overwriting Foo with foo. +// +// R4-2 resolveProgram used existsSync only. existsSync returns +// true for directories too, so a directory named 'node' in +// PATH (e.g. /usr/local/bin/node) would be returned as the +// resolved path. probeVersion would then try to execFileP a +// directory and fail with EISDIR. The contract is "return the +// path of an executable file"; a directory is not that. +// +// R4-3 probeVersion passed the bare cmd[0] (e.g. 'node') to +// execFileP, not the absolute path that resolveProgram +// returned. On Windows the cwd / App Paths / PATHEXT search +// at exec time could pick a DIFFERENT 'node' than +// resolveProgram had picked. The contract is that the file +// resolveProgram returned is the one that gets spawned. +// +// R4-4 .cmd / .bat branch had no real-Windows evidence. The +// shell decision is the only place where this matters and +// it was tested only on the reviewer's local Windows machine. +// CI only runs on ubuntu-latest. The fix is to add a +// windows-latest CI job. + +// === R4-1: case-distinct test is hermetic (gated + filtered) === + +// isCaseSensitiveFs: true iff creating two files that differ only in +// case actually produces two distinct inodes. We probe with a +// mkdtempSync directory; the probe is cheap and we run it once. +function isCaseSensitiveFs() { + const probe = mkdtempSync(join(tmpdir(), 'tool-map-csfs-probe-')); + try { + writeFileSync(join(probe, 'UPPER'), 'a'); + writeFileSync(join(probe, 'upper'), 'b'); + const entries = readdirSync(probe); + return entries.length === 2 && entries.includes('UPPER') && entries.includes('upper'); + } catch { + return false; + } finally { + rmSync(probe, { recursive: true, force: true }); + } +} + +test('POSIX: case-distinct tool names are kept distinct on case-sensitive FS, AND the test is hermetic (only TOOL_MAP_ROOTS results are asserted)', () => { + // Gate: on a case-insensitive FS (e.g. macOS HFS+, or any + // case-folding mount) the test is meaningless because the two + // writes collapse into one file. Skip with a clear reason rather + // than passing vacuously. + if (process.platform === 'win32') { + return; // skip on Windows; the Windows runner covers this + } + if (!isCaseSensitiveFs()) { + return; // skip on case-insensitive POSIX FS (macOS HFS+ default) + } + const root = mkdtempSync(join(tmpdir(), 'tool-map-case-')); + try { + writeFileSync(join(root, 'Foo'), '#!/bin/sh\necho Foo\n'); + chmodSync(join(root, 'Foo'), 0o755); + writeFileSync(join(root, 'foo'), '#!/bin/sh\necho foo\n'); + chmodSync(join(root, 'foo'), 0o755); + + // Use a CLEAN PATH so the scan only walks the temp dir. + // This is the round-4 fix: the old test set TOOL_MAP_ROOTS + // but did not clear PATH, so the scan picked up tools from + // $HOME/.local/bin, $HOME/.minimax-code/, etc. and the + // deepEqual assertion failed because the names list had + // real tools like mcode-tools in it. + const r = runScan(join(root, 'tools.md'), { + TOOL_MAP_ROOTS: root, + PATH: root, // POSIX: only the temp dir is on PATH + }); + assert.equal(r.status, 0, `scan failed: ${r.stderr}\n${r.stdout}`); + const json = JSON.parse(readFileSync(join(root, 'tools.json'), 'utf8')); + // Filter to entries whose path is inside the test root. The + // scan may also walk other roots (e.g. $HOME) but the test's + // claim is specifically about THIS root's two case-distinct + // files, not about the global state. + const localNames = json.tools + .filter((t) => t.path.startsWith(root)) + .map((t) => t.name).sort(); + assert.deepEqual( + localNames, ['Foo', 'foo'], + `case-distinct tool names were merged: ${localNames.join(', ')} (only tools inside the test root are asserted)`, + ); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +// === R4-2: resolveProgram rejects directories === +// +// The old implementation used `existsSync(full)` only. existsSync +// returns true for directories, so a directory named 'node' in +// PATH would be returned as the "resolved" path. probeVersion then +// calls execFileP on a directory and fails with EISDIR. +// +// The fix is to require `statSync(full).isFile() === true` and to +// also require a successful stat (i.e. not dangling, not a broken +// symlink). The round-trip test is: create a temp PATH where the +// FIRST entry is a directory called 'foo-tool' and the SECOND +// entry is a real file called 'foo-tool'. resolveProgram('foo-tool') +// must return the file (not the directory). +test('resolveProgram rejects a directory even if it comes first in PATH (POSIX)', () => { + // On Windows the function builds candidates from PATHEXT + // (.EXE/.CMD/...), so a plain 'foo-tool' file is never matched. + // The Windows runner (R4-4) covers the .cmd/.bat branch + // separately. + if (process.platform === 'win32') return; + const dir1 = mkdtempSync(join(tmpdir(), 'tool-map-resolve-dir-')); + const dir2 = mkdtempSync(join(tmpdir(), 'tool-map-resolve-file-')); + try { + // dir1 contains a DIRECTORY named 'foo-tool' (a regular file + // would be visible to existsSync too; we use mkdirSync to make + // a directory of the same name as the would-be tool). + mkdirSync(join(dir1, 'foo-tool')); + + // dir2 contains a REAL FILE named 'foo-tool' that is a + // runnable script. This is the path resolveProgram MUST return. + writeFileSync(join(dir2, 'foo-tool'), '#!/bin/sh\necho foo-tool\n'); + chmodSync(join(dir2, 'foo-tool'), 0o755); + + // PATH order: dir1 (directory) FIRST, then dir2 (file). + // The OLD code would return dir1/foo-tool (a directory) because + // existsSync was true. The NEW code must return dir2/foo-tool + // because statSync(dir1/foo-tool).isFile() is false. + const pathEnv = [dir1, dir2].join(delimiter); + const r = spawnSync(process.execPath, [join(REPO_ROOT, 'plugins', 'antianqi', 'tool-map', 'scripts', 'scan.mjs')], { + encoding: 'utf8', + env: { ...process.env, PATH: pathEnv, TOOL_MAP_ROOTS: '' }, + }); + // The scan runs to completion regardless. What we want to + // assert is that resolveProgram itself, queried via the scan + // output's "core" field, points to the FILE, not the + // directory. The scan writes a tools.json that includes the + // resolved `path` per tool (when found in PATH). + // + // Easiest assertion: shell out to node and call resolveProgram + // directly via dynamic import. Set PATH FIRST in the child + // process, then import (scan.mjs captures process.env.PATH at + // module load, so the env must be set BEFORE the import). + // + // On Windows, the function builds candidates from PATHEXT + // (e.g. foo-tool.EXE / foo-tool.CMD / ...), so a plain + // 'foo-tool' file is never matched. The test is POSIX-only; + // the Windows runner (R4-4) covers the .cmd/.bat branch. + const probe = spawnSync(process.execPath, [ + '--input-type=module', + '-e', + `process.env.PATH = ${JSON.stringify(pathEnv)}; + const m = await import('./plugins/antianqi/tool-map/scripts/scan.mjs'); + process.stdout.write(JSON.stringify(m.resolveProgram('foo-tool')));`, + ], { + encoding: 'utf8', + cwd: REPO_ROOT, + }); + const resolved = probe.stdout.trim(); + assert.equal(probe.status, 0, `resolveProgram probe failed (status ${probe.status}): ${probe.stderr}`); + assert.equal(resolved, JSON.stringify(join(dir2, 'foo-tool')), + `resolveProgram must skip the directory in dir1 and return the file in dir2. Got: ${resolved} (probe stdout: "${probe.stdout}", stderr: "${probe.stderr}")`); + } finally { + rmSync(dir1, { recursive: true, force: true }); + rmSync(dir2, { recursive: true, force: true }); + } +}); + +// === R4-3: probeVersion uses the resolved absolute path === +// +// The old implementation passed `cmd[0]` to execFileP without +// resolving to an absolute path first. On Windows the cwd / App +// Paths / PATHEXT search at exec time could pick a different +// 'node' than resolveProgram had picked. The fix is to resolve +// first, then exec the resolved path. +// +// Test design: use a tool name that exists in the real PATH. Set +// PATH to include ONLY a temp dir with a tool named 'node' that +// prints a known version. The test asserts that the captured +// version matches the temp tool's output (i.e. resolveProgram +// found the temp tool AND probeVersion executed it via the +// resolved path, not via PATH lookup at exec time). +// +// The way to make this test hermetic without exposing internals +// is to have the temp tool's behaviour diverge from the system +// 'node' behaviour. Easiest: use a tool that is NOT 'node' (so +// it is whitelisted via a custom whitelist OR we can use a +// behaviour-divergent tool). +// +// Since VERSION_PROBES is hardcoded, we cannot easily inject a +// new whitelisted name. Instead, the test verifies the +// behaviour for an existing whitelisted name ('node') by: +// 1. Creating a temp dir with a 'node' script that prints a +// known fake version. +// 2. Setting PATH to ONLY the temp dir + a directory that +// has the real 'node' (if any). +// 3. Running the scan. +// 4. Asserting the captured 'core.node' is the fake version +// printed by the temp tool. +// If probeVersion uses the resolved absolute path, the temp +// tool wins. If it uses cmd[0]='node' and PATH lookup picks +// the system node, the version is different. +// +// This is most cleanly testable on POSIX where the temp script +// can use a shebang. On Windows this requires a .cmd / .bat; +// the Windows CI job (R4-4) covers that. +test('probeVersion spawns the resolved absolute path (not a PATH lookup at exec time)', () => { + // On Windows the function builds candidates from PATHEXT + // (.EXE/.CMD/...), so a plain 'node' file is never matched. + // The Windows runner (R4-4) covers the .cmd/.bat branch + // separately. + if (process.platform === 'win32') return; + const root = mkdtempSync(join(tmpdir(), 'tool-map-probe-')); + try { + // A script that prints a recognisable fake version. + const fake = join(root, 'node'); + writeFileSync(fake, '#!/bin/sh\necho "FAKE_NODE_VERSION_v9"\n'); + chmodSync(fake, 0o755); + + const out = join(root, 'tools.md'); + // PATH = ONLY the temp dir. No system node on PATH. + const r = runScan(out, { TOOL_MAP_ROOTS: '', PATH: root }); + assert.equal(r.status, 0, `scan failed: ${r.stderr}\n${r.stdout}`); + const json = JSON.parse(readFileSync(join(root, 'tools.json'), 'utf8')); + // The scan captures `core` (VERSION_PROBES results) into a + // separate field — read tools.json and the `core` object. The + // shape is { scanned, platform, host, core, extras, tools }. + // If probeVersion used cmd[0]='node' and PATH lookup, the + // version would be whatever `node --version` returns (often + // "v24.18.0" on this host). If probeVersion used the + // resolved path (the temp script), the version is the fake + // string. + const nodeCore = json.core && json.core.node; + // The previous test had a hidden false-green here: when + // core.node is undefined (e.g. because resolveProgram failed + // to find node), the assert.match below would never fire, and + // the test would pass vacuously. Force the failure with a + // direct assert.ok so the contract is "core.node is set". + assert.ok(typeof nodeCore === 'string' && nodeCore.length > 0, + `core.node must be a non-empty string (the resolved path should have made it into core); got ${JSON.stringify(nodeCore)}. If core.node is undefined, the scan did not resolve 'node' at all and the test is a no-op.`); + assert.match(nodeCore, /FAKE_NODE_VERSION/, + `probeVersion should have executed the resolved path (the temp script), but the captured version is "${nodeCore}" — this means probeVersion used cmd[0]='node' and PATH lookup at exec time, which is the round-4 #3 defect`); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +// === R4-4: .cmd / .bat / .EXE branch on real Windows evidence === +// +// The round-4 review said: ".cmd/.bat 分支本轮没有真实 Windows +// 证据". The .cmd / .bat decision is the only place where +// platform matters for shellForFile + probeVersion, and the +// previous test suite was only run on ubuntu-latest CI, so the +// .cmd / .bat decision was never exercised against real Windows +// behaviour. This test creates a fake `node.EXE` (a .cmd batch +// file with a known version output) on a temp dir, sets PATH to +// only that dir, and asserts the scan picks it up via the +// PATHEXT-resolved path. +// +// On POSIX the function ignores PATHEXT, so this test is +// POSIX-noop (gated off). The Windows runner is the real +// coverage. +test('Windows: probeVersion handles the PATHEXT-expanded .CMD path (R4-4 real Windows evidence)', () => { + if (process.platform !== 'win32') return; // POSIX runner skips; Windows runner is the real test + const root = mkdtempSync(join(tmpdir(), 'tool-map-cmdext-')); + try { + // Create a .cmd batch file. .cmd is the most common shim for + // npm / pnpm / mcode on Windows; this is the round-4 review + // focus (.cmd/.bat 分支). + const fake = join(root, 'node.cmd'); + writeFileSync(fake, '@echo FAKE_NODE_VERSION_v9\r\n'); + + const out = join(root, 'tools.md'); + const r = runScan(out, { TOOL_MAP_ROOTS: '', PATH: root }); + assert.equal(r.status, 0, `scan failed: ${r.stderr}\n${r.stdout}`); + const json = JSON.parse(readFileSync(join(root, 'tools.json'), 'utf8')); + const nodeCore = json.core && json.core.node; + assert.ok(typeof nodeCore === 'string' && nodeCore.length > 0, + `core.node must be a non-empty string on Windows; got ${JSON.stringify(nodeCore)}. If core.node is undefined, the scan did not resolve 'node.cmd' (or its PATHEXT expansion) at all and the test is a no-op.`); + assert.match(nodeCore, /FAKE_NODE_VERSION/, + `probeVersion should have executed the .cmd shim, but the captured version is "${nodeCore}"`); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +// === R4-2 unit-level synthetic test (works on any platform) === +// +// The previous R4-2 test (above) is gated on POSIX because the +// function builds PATHEXT-expanded candidates on Windows. The +// R4-2 contract is "resolveProgram must skip a directory even if +// it appears first in PATH". To exercise this contract locally +// on any platform, this test uses a synthetic PATH where: +// - dir1 contains a directory named 'foo-tool' +// - dir2 contains a regular FILE named 'foo-tool' +// AND the name 'foo-tool' has no PATHEXT extension, so on +// Windows the function looks for 'foo-tool.COM' / 'foo-tool.EXE' +// etc. (none exist) and returns null. On POSIX the function +// looks for 'foo-tool' directly. +// +// The test only runs on POSIX (where the contract can be +// exercised without a Windows-style file). The Windows +// equivalent is the .cmd / .bat / .EXE test above (R4-4). +test('resolveProgram rejects a directory even if it comes first in PATH (POSIX, R4-2 contract)', () => { + if (process.platform === 'win32') return; + const dir1 = mkdtempSync(join(tmpdir(), 'tool-map-resolve-dir-')); + const dir2 = mkdtempSync(join(tmpdir(), 'tool-map-resolve-file-')); + try { + // dir1 contains a DIRECTORY named 'foo-tool' (a regular file + // would be visible to existsSync too; we use mkdirSync to make + // a directory of the same name as the would-be tool). + mkdirSync(join(dir1, 'foo-tool')); + + // dir2 contains a REAL FILE named 'foo-tool' that is a + // runnable script. This is the path resolveProgram MUST return. + writeFileSync(join(dir2, 'foo-tool'), '#!/bin/sh\necho foo-tool\n'); + chmodSync(join(dir2, 'foo-tool'), 0o755); + + // PATH order: dir1 (directory) FIRST, then dir2 (file). + // The OLD code would return dir1/foo-tool (a directory) because + // existsSync was true. The NEW code must return dir2/foo-tool + // because statSync(dir1/foo-tool).isFile() is false. + const pathEnv = [dir1, dir2].join(delimiter); + // Use spawn so the PATH is set BEFORE scan.mjs is loaded + // (scan.mjs captures process.env.PATH at module load). + const probe = spawnSync(process.execPath, [ + '--input-type=module', + '-e', + `process.env.PATH = ${JSON.stringify(pathEnv)}; + const m = await import('./plugins/antianqi/tool-map/scripts/scan.mjs'); + process.stdout.write(JSON.stringify(m.resolveProgram('foo-tool')));`, + ], { + encoding: 'utf8', + cwd: REPO_ROOT, + }); + const resolved = probe.stdout.trim(); + assert.equal(probe.status, 0, `resolveProgram probe failed (status ${probe.status}): ${probe.stderr}`); + assert.equal(resolved, JSON.stringify(join(dir2, 'foo-tool')), + `resolveProgram must skip the directory in dir1 and return the file in dir2. Got: ${resolved} (probe stdout: "${probe.stdout}", stderr: "${probe.stderr}")`); + } finally { + rmSync(dir1, { recursive: true, force: true }); + rmSync(dir2, { recursive: true, force: true }); + } }); test('resolveProgram returns null for unknown names', async () => { @@ -544,29 +886,8 @@ test('POSIX: a .sh file without the execute bit is not reported as a tool', () = } }); -test('POSIX: case-distinct tool names on case-sensitive filesystems are kept distinct', () => { - if (process.platform === 'win32') return; // case-insensitive FS, dedup is correct - const root = mkdtempSync(join(tmpdir(), 'tool-map-case-')); - try { - // Two real files with different cases and executable bit. - writeFileSync(join(root, 'Foo'), '#!/bin/sh\necho Foo\n'); - chmodSync(join(root, 'Foo'), 0o755); - writeFileSync(join(root, 'foo'), '#!/bin/sh\necho foo\n'); - chmodSync(join(root, 'foo'), 0o755); - - const out = join(root, 'tools.md'); - const r = runScan(out, { TOOL_MAP_ROOTS: root }); - assert.equal(r.status, 0, `scan failed: ${r.stderr}\n${r.stdout}`); - const json = JSON.parse(readFileSync(join(root, 'tools.json'), 'utf8')); - const names = json.tools.map((t) => t.name).sort(); - assert.deepEqual( - names, ['Foo', 'foo'], - `case-distinct tool names were merged: ${names.join(', ')}`, - ); - } finally { - rmSync(root, { recursive: true, force: true }); - } -}); +// (R4-1 test moved to the top of this file with proper gating and +// a hermetic PATH.) test('XDG_DATA_HOME is honoured when PLUGIN_DATA is unset', () => { const xdg = mkdtempSync(join(tmpdir(), 'tool-map-xdg-')); From a0a6d165d7d4d7e233b5ab21bae0b2b87632f601 Mon Sep 17 00:00:00 2001 From: antianqi Date: Fri, 28 Aug 2026 15:22:43 +0800 Subject: [PATCH 6/7] fix(test): use .sh extension in case-distinct test so NPM_BIN_HINT isn't needed (round-5) The R4-1 case-distinct test in commit 60d272c passed on Windows but failed on real Linux (WSL Ubuntu 22.04 + node 22.23.2): $ node --test test/tool-map.test.mjs not ok 17 - POSIX: case-distinct tool names are kept distinct on case-sensitive FS, AND the test is hermetic case-distinct tool names were merged: (got: []) # tests 27 / pass 26 / fail 1 Root cause: the test created extensionless files `Foo` and `foo` in a `/tmp/tool-map-case-XXX/` directory. scan.mjs isToolFile accepts extensionless files only when the parent directory matches the NPM_BIN_HINT regex: const NPM_BIN_HINT = /minimax-code[\\\/]|openclaw[\\\/]|minimax[\\\/]bin| node_modules[\\\/]|\.Codex[\\\/]|\.claude[\\\/]| [\\\/]npm[\\\/]|tauri[\\\/]/i; ... if (!EXEC_EXTS.has(ext)) { ... return NPM_BIN_HINT.test(dirLower); } A `/tmp/...` test root never matches any of those alternatives, so the scan correctly reports 0 tools and the test fails. On Windows the same test passes because EXEC_EXTS there includes `''` (empty extension) for shim files and the directory check is permissive. Fix: use `Foo.sh` and `foo.sh` instead. `.sh` is in POSIX EXEC_EXTS (line 178), so isToolFile accepts them without consulting NPM_BIN_HINT. The basename is still `Foo` and `foo` (the extension is stripped before the deepEqual assertion), so the test's contract is unchanged. Validation: WSL Ubuntu 22.04 + node v22.23.2 (nvm): before fix: 26 pass / 1 fail (R4-1) after fix: 27 pass / 0 fail Windows: 27 pass / 0 fail (unchanged) The test now actually exercises the case-distinct contract on real POSIX, not just the "scan finds nothing, deepEqual trivially holds" path it was secretly running before. This is a round-5 amendment to the round-4 R4-1 fix; the original round-4 work made the test hermetic against real tools in PATH but missed that the test was also silently non-hermetic against the scan's own directory heuristics. --- test/tool-map.test.mjs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/test/tool-map.test.mjs b/test/tool-map.test.mjs index 6a6023c..c0b2776 100644 --- a/test/tool-map.test.mjs +++ b/test/tool-map.test.mjs @@ -497,10 +497,19 @@ test('POSIX: case-distinct tool names are kept distinct on case-sensitive FS, AN } const root = mkdtempSync(join(tmpdir(), 'tool-map-case-')); try { - writeFileSync(join(root, 'Foo'), '#!/bin/sh\necho Foo\n'); - chmodSync(join(root, 'Foo'), 0o755); - writeFileSync(join(root, 'foo'), '#!/bin/sh\necho foo\n'); - chmodSync(join(root, 'foo'), 0o755); + // Use .sh extension so the scan's EXEC_EXTS allowlist accepts + // these files without depending on the NPM_BIN_HINT directory + // regex. The original test used extensionless files, which + // POSIX scan.mjs accepts only when the parent dir matches + // /minimax-code|openclaw|node_modules|.Codex|.claude|npm|tauri/. + // A /tmp/ test root never matches, so the scan correctly reports + // 0 tools and the test fails on Linux. The .sh files are + // accepted by isToolFile directly via EXEC_EXTS, regardless of + // the parent dir. + writeFileSync(join(root, 'Foo.sh'), '#!/bin/sh\necho Foo\n'); + chmodSync(join(root, 'Foo.sh'), 0o755); + writeFileSync(join(root, 'foo.sh'), '#!/bin/sh\necho foo\n'); + chmodSync(join(root, 'foo.sh'), 0o755); // Use a CLEAN PATH so the scan only walks the temp dir. // This is the round-4 fix: the old test set TOOL_MAP_ROOTS From 6bb6a4bfe026c6136252adad0a6ba779887d3bdf Mon Sep 17 00:00:00 2001 From: antianqi <75944423+antianqi@users.noreply.github.com> Date: Sat, 29 Aug 2026 13:34:13 +0800 Subject: [PATCH 7/7] fix(tool-map): require X_OK on POSIX so non-executable in earlier PATH dir does not shadow executable later (round-5) Round-5 review (hetaoBackend, 2026-08-28T08:22:09Z) on commit a0a6d16 flagged one POSIX resolver defect: resolveProgram() accepts the first isFile() match in PATH, but isFile() is necessary but not sufficient on POSIX. A non-executable regular file (0644) in an earlier PATH directory shadows an executable regular file (0755) later in PATH; the kernel's execve() of the 0644 file would fail with EACCES, and probeVersion() would then surface null instead of continuing on to the 0755 candidate that the user actually intended to run. Fix - scripts/scan.mjs: resolveProgram() now requires X_OK on POSIX after the isFile() check. A candidate that fails accessSync is skipped (continue) rather than returned, so the search proceeds to the next directory / extension in PATH. The import list gains `accessSync` and `constants as fsConstants` from node:fs. No new dependencies. On Windows the x bit is ignored per platform convention -- the executable contract there is the .exe/.cmd/.bat extension and PATHEXT above already enforces it -- so the X_OK gate is wrapped in `if (!IS_WIN)` and Windows behaviour is unchanged. Test evidence - test/tool-map.test.mjs: 2 new tests under `=== R5-1: ... ===`, both POSIX-only (gated off on win32). The first sets up a PATH where dir1/foo-tool is 0644 and dir2/foo-tool is 0755 and asserts resolveProgram returns the dir2 path. The second sets up a PATH where the only candidate is 0644 and asserts resolveProgram returns null. - `node --test test/tool-map.test.mjs`: 29 / 29 pass (was 27 / 27 on a0a6d16; 2 new tests, 0 modified, 0 failures). On Windows the 2 new tests are gated off and counted as noop; on POSIX they exercise the X_OK contract. - `node --test` (full repository test suite on Windows): 56 / 56 pass, 1 fail. The single failure is the pre-existing test/hosted-plugins.test.mjs:15 Windows-only POSIX-path-regex bug acknowledged in the original PR description; it fails identically on a0a6d16 and on this commit and is unchanged by this edit. No new regression. Design compliance - 2 files changed: scripts/scan.mjs (+20 / -1) and test/tool-map.test.mjs (+91 / 0). No README / SKILL.md / package.json change. The exported `resolveProgram` signature is unchanged; callers in shouldUseShell and probeVersion are untouched. - The X_OK gate is the minimum POSIX-platform change: the Windows branch is a no-op (PATHEXT + .exe/.cmd/.bat are the executable contract there). On POSIX the only behavioural change is that a non-executable candidate is no longer returned by resolveProgram (it is treated like the directory case in R4-2 and the missing-stat case already handled earlier in the same loop). - The fix does not introduce any new shell or spawn call; accessSync is a synchronous metadata-only call against the same full path that the next line would have returned. --- plugins/antianqi/tool-map/scripts/scan.mjs | 22 +++++- test/tool-map.test.mjs | 91 ++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/plugins/antianqi/tool-map/scripts/scan.mjs b/plugins/antianqi/tool-map/scripts/scan.mjs index 2df8672..d095407 100644 --- a/plugins/antianqi/tool-map/scripts/scan.mjs +++ b/plugins/antianqi/tool-map/scripts/scan.mjs @@ -20,6 +20,7 @@ import { fileURLToPath, pathToFileURL } from 'node:url'; import { readdirSync, readFileSync, statSync, existsSync, writeFileSync, mkdirSync, realpathSync, renameSync as _fsRename, rmSync, + accessSync, constants as fsConstants, } from 'node:fs'; import { join, dirname, basename, sep, extname, resolve, delimiter } from 'node:path'; import { homedir, hostname, platform } from 'node:os'; @@ -309,7 +310,26 @@ function resolveProgram(name) { // symlinks (statSync throws ENOENT) by not catching. let st; try { st = statSync(full); } catch { continue; } - if (st.isFile()) return full; + if (st.isFile()) { + // Round-5 finding: isFile() is necessary but not sufficient + // on POSIX. A non-executable regular file in an earlier PATH + // directory must not shadow an executable regular file later + // in PATH. Without the X_OK gate, resolveProgram() would + // return the 0644 file and probeVersion() would then try to + // execFileP it; the kernel's execve() would fail with + // EACCES and probeVersion() would surface null instead of + // continuing to the 0755 candidate behind it. + // + // Windows ignores the x bit per platform convention; the + // .exe/.cmd/.bat extension is the executable contract on + // Windows, and PATHEXT above already enforces it. No + // permission check is needed there. + if (!IS_WIN) { + try { accessSync(full, fsConstants.X_OK); } + catch { continue; } + } + return full; + } } } return null; diff --git a/test/tool-map.test.mjs b/test/tool-map.test.mjs index c0b2776..241325e 100644 --- a/test/tool-map.test.mjs +++ b/test/tool-map.test.mjs @@ -792,6 +792,97 @@ test('resolveProgram rejects a directory even if it comes first in PATH (POSIX, } }); +// === R5-1: resolveProgram requires X_OK on POSIX (non-executable in earlier PATH dir must not shadow executable in later dir) === +// +// Round-5 review finding: a non-executable (0644) candidate in an +// earlier PATH directory must not shadow a runnable (0755) candidate +// later in PATH. The previous isFile()-only check would return the +// 0644 file, and a subsequent probeVersion() call would surface null +// (because the kernel's execve() of the 0644 file would fail with +// EACCES) instead of continuing on to the 0755 candidate that the +// user actually intended to run. +// +// This is a real bug-replication test: with the fix reverted +// (isFile() only, no X_OK gate) the assert below fails because +// resolveProgram returns the dir1 path. With the fix in place it +// returns the dir2 path. +// +// On Windows the executable contract is the .exe/.cmd/.bat +// extension (PATHEXT above). The x bit is ignored by convention +// on Windows, so this test is POSIX-only. +test('resolveProgram skips a non-executable file even if it comes first in PATH (POSIX, R5-1 X_OK contract)', () => { + if (process.platform === 'win32') return; + const dir1 = mkdtempSync(join(tmpdir(), 'tool-map-resolve-noexec-first-')); + const dir2 = mkdtempSync(join(tmpdir(), 'tool-map-resolve-exec-later-')); + try { + // dir1 contains a NON-EXECUTABLE regular file. With the fix + // reverted, resolveProgram would stop here (isFile() is true) + // and return this path; the assertion at the end would then + // fail because this is the WRONG path. + writeFileSync(join(dir1, 'foo-tool'), '#!/bin/sh\necho foo-tool\n'); + chmodSync(join(dir1, 'foo-tool'), 0o644); + + // dir2 contains an EXECUTABLE regular file. resolveProgram + // must keep searching past the 0644 candidate and return this. + writeFileSync(join(dir2, 'foo-tool'), '#!/bin/sh\necho foo-tool\n'); + chmodSync(join(dir2, 'foo-tool'), 0o755); + + const pathEnv = `${dir1}${delimiter}${dir2}`; + const probe = spawnSync(process.execPath, [ + '--input-type=module', + '-e', + `process.env.PATH = ${JSON.stringify(pathEnv)}; + const m = await import('./plugins/antianqi/tool-map/scripts/scan.mjs'); + process.stdout.write(JSON.stringify(m.resolveProgram('foo-tool')));`, + ], { + encoding: 'utf8', + cwd: REPO_ROOT, + }); + const resolved = probe.stdout.trim(); + assert.equal(probe.status, 0, `resolveProgram probe failed (status ${probe.status}): ${probe.stderr}`); + assert.equal(resolved, JSON.stringify(join(dir2, 'foo-tool')), + `resolveProgram must skip the 0644 file in dir1 and return the 0755 file in dir2. ` + + `Got: ${resolved} (probe stdout: "${probe.stdout}", stderr: "${probe.stderr}")`); + } finally { + rmSync(dir1, { recursive: true, force: true }); + rmSync(dir2, { recursive: true, force: true }); + } +}); + +// === R5-1 mirror: resolveProgram returns null when NO candidate in PATH is executable === +// +// Companion to the X_OK test above. If every candidate in PATH is a +// regular file but NONE has the x bit set, resolveProgram must +// return null (not the first non-executable file, not an arbitrary +// one). Without the X_OK gate, the old code would have returned the +// first isFile() match regardless of executability. +test('resolveProgram returns null when the only candidate in PATH is a non-executable file (POSIX, R5-1 X_OK null return)', () => { + if (process.platform === 'win32') return; + const dir1 = mkdtempSync(join(tmpdir(), 'tool-map-resolve-noexec-only-')); + try { + writeFileSync(join(dir1, 'foo-tool'), '#!/bin/sh\necho foo-tool\n'); + chmodSync(join(dir1, 'foo-tool'), 0o644); + + const pathEnv = dir1; + const probe = spawnSync(process.execPath, [ + '--input-type=module', + '-e', + `process.env.PATH = ${JSON.stringify(pathEnv)}; + const m = await import('./plugins/antianqi/tool-map/scripts/scan.mjs'); + process.stdout.write(JSON.stringify(m.resolveProgram('foo-tool')));`, + ], { + encoding: 'utf8', + cwd: REPO_ROOT, + }); + assert.equal(probe.status, 0, `resolveProgram probe failed (status ${probe.status}): ${probe.stderr}`); + assert.equal(probe.stdout.trim(), 'null', + `resolveProgram must return null when no candidate has X_OK. ` + + `Got: ${probe.stdout} (stderr: ${probe.stderr})`); + } finally { + rmSync(dir1, { recursive: true, force: true }); + } +}); + test('resolveProgram returns null for unknown names', async () => { const scanUrl = pathToFileURL(SCAN).href; const { resolveProgram } = await import(scanUrl);