fix(install): skip env flags like -S when parsing shebangs on Windows#28127
fix(install): skip env flags like -S when parsing shebangs on Windows#28127
Conversation
When a bin script uses `#!/usr/bin/env -S node`, the Windows shebang parser would incorrectly treat `-S` as the program name instead of recognizing it as a flag to `env`. This caused packages like `@google/gemini-cli` to fail with "interpreter executable '-S' not found in %PATH%". The fix skips any tokens starting with `-` after `/usr/bin/env` to find the actual interpreter program name. Closes #28126 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Updated 11:34 PM PT - Mar 14th, 2026
❌ @autofix-ci[bot], your commit 55ea5f0 has 6 failures in
🧪 To try this PR locally: bunx bun-pr 28127That installs a local version of the PR into your bun-28127 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughEnv-based shebang parsing now skips env flags (including -S and others) to locate the real program, captures trailing arguments, and reconstructs the launcher for Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/install/windows-shim/BinLinkingShim.zig`:
- Around line 198-204: The loop that picks the shebang program (involving
tokenizer, program, token, and parseFromBinPath/bin_path) must also skip flag
operands for env flags that consume a following token; update the logic so when
a token matches flags that take values (e.g. -u, -C, -a or --argv0, -S or
--split-string and any equivalent long forms) you consume the next
tokenizer.next() result (handling orelse the same way as for token) and
continue, while still skipping standalone flags that don't take values; ensure
this adjusted behavior is applied inside the program: while loop before breaking
out to return the selected program.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0d1d7abd-a50c-478c-b8b8-ce3a32c5bc27
📒 Files selected for processing (2)
src/install/windows-shim/BinLinkingShim.zigtest/regression/issue/28126.test.ts
| // Skip flags passed to env (e.g. -S, -u, -i, --split-string) | ||
| // to find the actual program name. | ||
| const program = program: while (true) { | ||
| const token = tokenizer.next() orelse return parseFromBinPath(bin_path); | ||
| if (token.len > 0 and token[0] == '-') continue; | ||
| break :program token; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GNU coreutils envoption semantics: which options consume a following operand token (for example-u NAME, -C DIR, --unset NAME, --chdir DIR)?
💡 Result:
GNU coreutils env options that consume an argument (i.e., take/require an option-argument, which may be the following token) are:
-a arg,--argv0=arg(setsargv[0]) [1]-u name,--unset=name(unsets a variable) [1][2]-C dir,--chdir=dir(changes working directory) [1][2]-S string,--split-string=string(splitsstringinto arguments) [1][2]
Options that take an optional argument (shown as [=SIG]) are:
--block-signal[=SIG][1][2]--default-signal[=SIG][1][2]--ignore-signal[=SIG][1][2]
For these --*-signal[=SIG] options, the argument is supplied as --opt=SIG; if you omit =SIG, env does not consume the next token as the argument. [2]
All other env options (e.g., -i, -0, -v, --list-signal-handling, --help, --version) do not consume a following operand token. [1][2]
Sources:
[1] GNU Coreutils manual, “env invocation” https://www.gnu.org/software/coreutils/manual/html_node/env-invocation.html
[2] Debian env(1) man page (coreutils) https://manpages.debian.org/bookworm/coreutils/env.1.en.html
Account for all env flags that consume a value token before selecting program.
Line 202 skips -u/-C themselves but not their required value tokens. Additionally, -a/--argv0 and -S/--split-string also consume following operands and are missing from the fix. Cases like #!/usr/bin/env -u NODE_OPTIONS node or #!/usr/bin/env -S "args" will resolve the wrong executable.
Proposed fix
- const program = program: while (true) {
+ var skip_next_flag_value = false;
+ const program = program: while (true) {
const token = tokenizer.next() orelse return parseFromBinPath(bin_path);
- if (token.len > 0 and token[0] == '-') continue;
+ if (skip_next_flag_value) {
+ skip_next_flag_value = false;
+ continue;
+ }
+ if (token.len > 0 and token[0] == '-') {
+ if (eqlComptime(token, "-u") or eqlComptime(token, "-C") or
+ eqlComptime(token, "-a") or eqlComptime(token, "-S") or
+ eqlComptime(token, "--unset") or eqlComptime(token, "--chdir") or
+ eqlComptime(token, "--argv0") or eqlComptime(token, "--split-string"))
+ {
+ skip_next_flag_value = true;
+ }
+ continue;
+ }
break :program token;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/install/windows-shim/BinLinkingShim.zig` around lines 198 - 204, The loop
that picks the shebang program (involving tokenizer, program, token, and
parseFromBinPath/bin_path) must also skip flag operands for env flags that
consume a following token; update the logic so when a token matches flags that
take values (e.g. -u, -C, -a or --argv0, -S or --split-string and any equivalent
long forms) you consume the next tokenizer.next() result (handling orelse the
same way as for token) and continue, while still skipping standalone flags that
don't take values; ensure this adjusted behavior is applied inside the program:
while loop before breaking out to return the selected program.
|
Found 5 issues this PR may fix:
🤖 Generated with Claude Code |
Handle env flags like `-u NAME`, `-C DIR`, `-a ARG` that consume the next token as their value. Without this, a shebang like `#!/usr/bin/env -S -u HOME node` would incorrectly identify "HOME" as the program name. Note: `-S`/`--split-string` is intentionally NOT treated as consuming the next token because in shebang context the text after `-S` IS the command to run. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addressed the review feedback — the parser now also skips value-consuming env flags ( Important note on Added a test for |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward fix for Windows shebang parsing to skip env flags before the interpreter name, with good regression tests.
Extended reasoning...
Overview
This PR fixes the Windows shebang parser in BinLinkingShim.zig so that #!/usr/bin/env -S node correctly identifies node as the program instead of -S. The change replaces a single tokenizer.next() call with a loop that skips tokens starting with -, then reconstructs the launcher string via pointer arithmetic on slices from the same underlying buffer. A new test file with 3 regression cases covers the fix.
Security risks
None. This is Windows-specific shim code for bin linking during package installation. The pointer arithmetic is safe because both program and rest_after_program are slices into the same line buffer from the tokenizer, so the contiguous span computation is well-defined.
Level of scrutiny
Low-to-medium. This is a small, focused bug fix in a platform-specific shim. The logic is straightforward (skip dash-prefixed tokens in a loop), and the pointer arithmetic for reconstructing the launcher string is correct — rest() returns from the delimiter after the last token to the end of the buffer, so the computed slice spans "program rest_args...".
Other factors
- No CODEOWNERS claim on these specific files.
- The nit about
-u HOMEstyle flags with separate value arguments is a valid but extremely rare edge case; the PR is a clear improvement over the old code which broke on the very common-Sflag. - Three regression tests cover the main scenarios (
-S node,-S node --no-warnings,-S bun). - The PR fixes a real-world issue affecting packages like
@google/gemini-cli.
BusyBox env (used on Alpine/musl) does not support the -S flag, causing these tests to fail at runtime. Wrap all tests in a describe.skipIf(isMusl) block. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
LGTM — my previous nit about value-consuming flags has been addressed.
Extended reasoning...
Overview
This PR fixes the Windows shebang parser in BinLinkingShim.zig to skip env flags (like -S, -u, -C, -a) when resolving the interpreter program name. Previously #!/usr/bin/env -S node would cause the shim to treat -S as the executable. A new test file with 4 test cases covers the common scenarios.
Security risks
No security risks. The change only affects how shebang lines are parsed to find the interpreter name during bun install bin linking on Windows. No auth, crypto, or permissions code is involved.
Level of scrutiny
This is a focused bug fix in the package installer shim code. The pointer arithmetic for launcher reconstruction (line 222) is straightforward — both program and rest_after_program are slices from the same tokenizer operating on the same underlying line buffer, so the span calculation is safe. The logic for skipping value-consuming flags (-u, -C, -a) correctly consumes the next token. CodeRabbit earlier feedback about missing value-consuming flag handling was addressed in a follow-up commit.
Other factors
My previous review raised a nit about value-consuming flags like -u HOME — this has been fully addressed in the current code (lines 207-212) with a corresponding test case. The CI failures (musl builds, vendor warnings) are unrelated to this PR. The fix resolves a real-world issue affecting packages like @google/gemini-cli. No bugs were found by the automated bug hunting system.
There was a problem hiding this comment.
LGTM — focused bugfix with correct flag-skipping logic and good test coverage.
Extended reasoning...
Overview
This PR fixes the Windows shebang parser in BinLinkingShim.zig to skip env flags (like -S, -u, -C, -a) when resolving the interpreter program name. Previously, #!/usr/bin/env -S node would cause the shim to look for -S as the executable. A new test file with 4 test cases covers the main scenarios.
Security risks
No security risks. This is a Windows bin-linking shim parser fix — it only affects how shebangs are parsed to determine which interpreter to invoke. No auth, crypto, or permissions code is touched.
Level of scrutiny
Low-to-medium scrutiny is appropriate. The change is small (24 lines of Zig logic), well-scoped to a single parsing function, and includes regression tests. The pointer arithmetic for launcher reconstruction is safe — both program and rest_after_program are slices into the same line buffer from the tokenizer, so the computed offset is always valid. No CODEOWNERS are affected.
Other factors
My previous inline nit about value-consuming flags (e.g., -u HOME) has been fully addressed in the current code — lines 205-212 properly consume the next token for flags that take a separate value argument. The CI failures shown in the timeline are pre-existing vendor code warnings (libuv, tinycc) and build system issues unrelated to this PR. The fix addresses a real-world issue affecting popular packages like google/gemini-cli.
|
Are there any further blockers for incorporating such a fix? Bun is unusable on Windows because of this. |
|
Closing as duplicate of #27925 — identical Zig code changes for the shebang |
2 participants
Summary
envflags (e.g.-S,-u,-i,--split-string) when resolving the interpreter program name#!/usr/bin/env -S nodewould cause the shim to look for-Sas an executable, failing with "interpreter executable '-S' not found in %PATH%"#!/usr/bin/env -S <interpreter>shebangs when installed on Windows (e.g.@google/gemini-cli)Test plan
test/regression/issue/28126.test.tswith 3 test cases:env -S nodeshebangenv -S node --no-warnings(with additional args)env -S bunshebangbun bd test test/regression/issue/28126.test.tsCloses #28126
🤖 Generated with Claude Code