Repository navigation
fix: keep user argv out of cmd.exe on the Windows launcher path (#2196) - #2205
Conversation
The default npm Codex install is a codex.cmd shim whose %* forwarding re-parses every user argument through cmd.exe's line parser — embedded double quotes split tokens, %VAR% expands, &|<>^() execute, empty args vanish. Plain spaced prompts survived, which is why the #679 space-truncation fix did not expose it. - codex resolution (win32): per PATH dir the native codex.exe wins over the shim; a .cmd/.bat hit beside a trusted npm layout (node_modules/@openai/codex named @openai/codex with a resolvable codex bin entry, shim text referencing that package) upgrades to node <official bin/codex.js> — same vendor-lookup/env-init/signal-forwarding behavior, lossless CreateProcess argv encoding. BILI_CLIENT_BIN override precedence unchanged. - every remaining comspec wrap site (launcher planClientSpawn + dsh-channel planDshSpawn) now refuses argv the line parser cannot carry verbatim with an actionable error before any spawn, instead of mangling or interpreting it. Direct-spawn .exe paths are untouched (Node encodes argv losslessly). Refusal messages name position + class, never token content (log-masking discipline). - tests/win-cmd-argv.test.ts: predicate table, guard behavior incl. byte-identical safe-plan pins for #679, layout-detection cases, resolution-order cases, and win32-gated real round-trips asserting the child's RECEIVED argv equals the caller array item-by-item over the full corpus (empty arg, spaces, embedded quotes, TOML -c, JSON, Unicode, trailing backslashes, %COMSPEC%, !VAR!, metacharacters) — plus detection against the real global @openai/codex install and the wrapper's --help booting through plain node. - CI: hard gate in ci-windows-codex.yml running the new suite on windows-latest. - docs: CLIENTS.md (+zh) codex section on the new resolution order, pass-through contract, and workaround.
📦 Built Package ArtifactBranch: Option A — Install this exact build (recommended)npm install -g billion-context@0.0.2265-pr.2205.08a09d0This exact version is immutable and reproducible. The rolling Option B — Download artifact
tar xzf billion-context-pr2205.tgz
npm install -g packageInstall instructions are refreshed on each push. |
… test (#2196) The assertion message interpolated res.stderr while stderr was set to 'ignore' (null) — the interpolation threw unconditionally before the status check could report. Pipe stderr and assert.fail with a guarded message.
[bot] 🏷 PR #2205 review — Windows launcher argv 保真(修 #2196) 已拉取分支 ① 是什么问题:Windows 默认 npm 装的 Codex 是 ② 什么原因导致的:根因不是「引号没加对」,而是把用户 argv 重新喂回 shell 语法解释(producer 在 shim 启动路径)。 ③ 怎么修复的(两层,均指向移除 producer):
④ 是否有回归问题——逐项过既有行为:
⑤ 是否破坏缓存:不涉及会话/压缩/重试/去重键。唯一缓存是进程内 ⑥ 是否改了设计方向:不涉及架构转向。新增聚焦模块 ⑦ 是否引入新配置:无新增 env/config/字段/flag。 测试有效性(④质量关):有牙,能兜底——safe argv 字节级回归钉死;不安全输入断言「spawn 未被调用」;专门一条断言拒绝消息不含 token 内容(脱敏);issue 原始复现( CI / 基线:head 判定:代码逻辑、边界、错误处理、测试、文档、披露均到位,无阻塞缺陷。建议人工合并——按 §7.4 自动合入门禁,本 PR 触及「跨所有 Windows cmd 路径客户端的行为变更 + 安全相关(消除 |
📦 Merged Build PublishedThis merge to master was published as npm install -g billion-context@0.0.57-master.84b47ea # this exact build
npm install -g billion-context@master # always the newest merged build
|
Closes #2196
What was broken
The default npm Codex install is a
codex.cmdshim, and its%*forwarding re-parses every user argument through cmd.exe's line parser. That parser has no escape mechanism: embedded double quotes split tokens (Please say "hello world" exactlyarrived as four arguments), TOML-cvalues lost their quotes,%VAR%expanded,&|<>^()acted as command operators, and empty arguments vanished. Plain spaced prompts happened to survive, so the #679 space-truncation fix never exposed this. The issue's A/B againstBILI_CLIENT_BIN=<codex.exe>confirmed the corruption lived in bili's shim launch path, not in Codex.Fix strategy (remove the producer, not the symptom)
The producer of the corrupted bytes was re-feeding user argv into shell-syntax interpretation. Two complementary changes remove that mechanism:
codex.exewins overcodex.cmd/.bat; a.cmd/.bathit beside a trusted npm layout —<dir>/node_modules/@openai/codexnamed@openai/codexwith a resolvable"codex"bin entry AND shim text referencing that package (a hand-written shim beside an unrelated tree cannot hijack) — is upgraded tonode <official bin/codex.js>. This reproduces exactly what the shim does (vendor-binary lookup, env init, signal forwarding) while Node's own CreateProcess argv encoding carries every argument verbatim. Unrecognized layouts (yarn-classic.bintrees, pnpm store shims without the local link) keep the legacy cmd path under contract 2. TheBILI_CLIENT_BINoverride keeps its top precedence.planClientSpawn(launcher) andplanDshSpawn(dsh channel — same mechanism family) now validate every token against a safe-set predicate (src/win-cmd.ts) before building the line; an unsafe token throws with position + class + actionable hint before any process starts. Direct-spawn.exelaunches are untouched — Node encodes their argv losslessly itself. Refusal messages never echo token content (prompts routinely carry secrets; log-masking discipline).Behavior changes (mandatory disclosure)
%VAR%,&|<>^(), empty arg, line breaks, whitespace-bearing odd trailing-backslash runs) was silently corrupted or interpreted on EVERY Windows.cmd/.bat/extensionless launch — all clients + dsh channelBILI_CLIENT_BIN/BILI_DSH_BIN; the client is never spawned.cmd/.bat/.exeper directory (first match won).exe/.cmd/.bat; a.cmdhit upgrades to node + official JS entry when the trusted layout is detectedNo wire-format, persistence, cache, retry, or session-binding changes. No
kernel/changes.Config surface
None added.
BILI_CLIENT_BIN(pre-existing override, highest precedence, unchanged semantics) is documented in CLIENTS.md as the power-user knob / temporary workaround. No new env vars, config fields, or CLI flags — nothing for CONFIGURATION.md to mirror.Verification
tests/win-cmd-argv.test.ts+fix-679+dsh-refresh+dsh-native+fix-1292: 108 pass / 0 fail (7 win32-gated skipped locally);launcher.test.ts269 pass / 0 fail. Full suite left to CI per local-test discipline.ci-windows-codex.yml): full-corpus round-trip asserting the child's RECEIVED argv equals the caller array item-by-item through (a) detection + spawn of an official-wrapper-shaped entry and (b) the direct-spawn leg alone; detection against the real global@openai/codexinstall; the official wrapper's--helpbooting through plain node. Corpus covers the issue's acceptance list: empty arg, plain spaces, embedded double quotes, TOML-cvalues, JSON, Unicode, trailing backslashes,%COMSPEC%,!VAR!,&|<>^().Review notes
!(delayed expansion is opt-in; standard npm shims never enable it) and even-run trailing backslashes; the refused backslash case is specifically the MSVCRT closing-quote escape interaction for whitespace-bearing tokens.quoteWinTokenstays a pure formatter; its docstring now states callers must pre-validate. The old test titled "embedded double quotes stay bare (today's behavior preserved)" is reframed — that framing pinned the bug as intended behavior.resolveClientCommandgained an optionalplatformparameter (defaultprocess.platform); bothrunLaunchcall sites passdeps.platform ?? process.platformso tests can inject. Existing call sites/tests are untouched.