mirror of
https://github.com/ruvnet/ruflo.git
synced 2026-09-28 06:22:58 +08:00
Dragan Spiridonov
c77d1b5062
#3322 escaped argv on the Windows shell:true path in plugins/ruflo-core/scripts/ruflo-hook.cjs. The same shim exists in three other places, which still pass hook-derived values (a Bash tool's `command`, a file path) to cmd.exe unescaped, and are broader than the copy that was fixed — a bare `process.platform === 'win32'` with no exemption for `node`: - .claude-plugin/scripts/ruflo-hook.cjs (published in the npm package) - plugin/scripts/ruflo-hook.cjs (byte-identical sibling) - generateRufloHookCjs() in helpers-generator.ts, which `ruflo init` writes to .claude/helpers/ruflo-hook.cjs All four now follow one pattern — resolve, then escape: - resolveCommandPath() walks PATH/PATHEXT with fs only. The previous probe was `execSync('where ' + cmd)`, which spawned a shell on every hook invocation. - resolveNpmShim() maps an npm .cmd shim to the .js entrypoint it would have run, read from the package's own `bin` field rather than a guessed filename, and required to resolve inside the package directory. Both npm layouts are handled (global prefix and node_modules/.bin), as is npx, whose command and package names differ. - invokeHook() spawns `node <entry>` with shell:false, so CreateProcess receives the argv array verbatim — no second cmd.exe tokenizer, and no %VAR% expansion, which quoting does not suppress and carets do not reliably escape. - escapeCmdArg() is unchanged from #3322 and remains as the fallback for the case where no entrypoint can be identified. Returning early there would silently drop the hook instead of surfacing the problem. escapeCmdArg, resolveCommandPath, resolveNpmShim and resolveInvocation are byte-identical across all four copies, and the suite asserts that for each of them — the divergence between copies is what let this persist after #3322. Verification. #3322 could only assert the string transform, with no Windows host available. Two changes address that: - resolveCommandPath/resolveInvocation take `platform` and `env` as arguments, so the Windows branch runs on any OS. The tests build a real npm layout in a temp dir, drive it with { platform: 'win32' }, and assert the payload reaches the recorded argv byte-for-byte while the sibling .cmd/.ps1 are resolved past, never executed. - the coverage is added to plugins/ruflo-core/scripts/test-hooks.mjs, whose "Plugin hooks smoke" job already runs on windows-latest. The existing cases there go through RUFLO_HOOK_CLI_OVERRIDE and so never reach the global-shim branch; these call invokeHook() directly, so the fallback is executed against a real cmd.exe. Tests: 11 vitest cases and 6 new harness cases, each mutation-checked — every one fails when the code it covers is reverted. test-hooks.mjs goes 19/24 to 25/30 against the recorder fixture; those 5 failures are pre-existing on main (the fixture does not echo argv) and pass in CI against the real built CLI. Windows result: the windows-latest leg ran green, 32/32, including the escaped fallback executed against a real cmd.exe. The %VAR% probe was included because carets are not a reliable escape for % and quoting does not suppress percent expansion, so expansion on the shim's second parse looked plausible; the runner measured otherwise and the value arrives literal, with no redirection performed. The probe stays as a regression guard and keeps reporting the observed value rather than asserting one. Out of scope: nine sites in plugins/ruflo-metaharness/scripts spawn with `shell: process.platform === 'win32'` and a dynamic argv element — _darwin.mjs:86/129 spread the caller's own ...argv, oia-audit.mjs:105 passes JSON.stringify(payload), and audit-list/audit-trend/similarity pass `--key <key>`. Same shape, different plugin, separate change. Two neighbours that look like the same problem and are not: the ten `shell:` flags across ruflo-cost-tracker are inert, because spawnNpxSync() discards the option and forces shell:false; and mcp-launch.cjs already prefers a resolved local bin with shell:false and only falls back to npx.cmd with constant args.