mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 08:03:22 +00:00
fix(wsl): pass guest argv verbatim through --exec (#15039)
* fix(wsl): pass guest argv verbatim through --exec
`wsl.exe <...> -- <argv>` expands `$name` in every argument against the
guest environment before the guest ever runs. It does this even when no
shell is involved, so `-- /usr/bin/printf %s '$HOME'` prints /home/you.
Every WSL invocation went through that preprocessor, so scripts arrived
already rewritten: `awk '{print $2}'` lost its field reference, and a
POSIX script asking for the literal `$HOME` got the expanded path.
`escapeWslShCommandForWindows` tried to compensate by escaping `$`, but
it skipped any `$` preceded by a backslash, so a script containing `\$`
was still corrupted -- and half the call sites never applied it at all.
Route every invocation through `--exec`, which passes argv through
untouched, and delete the escaper. The direct-git path already used
`--exec`, so this is not a new compatibility dependency.
A guard test fails if the `--` form reappears anywhere in the tree.
Net -50 lines of production code.
* test(wsl): drop remaining escaped-dollar assertions
* fix(wsl): cover the --exec migration's blind spots
An audit of every wsl.exe invocation found sites the first pass missed,
including two it actively broke:
- config/scripts/wsl-git-shell-benchmark.mjs imported
escapeWslShCommandForWindows, which no longer exists, so the script
threw on startup. Its wslShellArgs helper also still used `--`; the
file already had an --exec helper, so route both call sites there.
- classifySubprocessCommand unwrapped `wsl.exe <...> -- <binary>` by
breaking on `--` alone. With every Orca spawn now on --exec it never
found the guest binary and bucketed all WSL subprocesses as plain
"wsl", losing the git/gh/glab breakdown. Break on either separator,
since foreign wsl.exe processes still use `--`.
CliSkillRuntimeSetup builds its setup command as a template literal
rather than an argv array, so no array-shaped search could see it. Its
decoder accepts both separators so commands persisted before this
change still decode.
The guard now scans config/ and tests/ as well as src/, and checks the
command-string spelling alongside the argv one — the two shapes that
have each shipped a regression. It skips comment lines so prose about
the old form stays allowed, and asserts it scanned a plausible file
count so a bad root cannot make it vacuous.
* fix(wsl): restore the guard's multi-line sensitivity
The guard matched line by line, so `'--',\s*'bash'` could not span a
newline -- and every argv array in this repo is formatted one element
per line, which is exactly the shape it exists to catch. Measured
against the pre-migration tree it caught 17 files before and 9 fewer
after. It now strips comment lines and matches the rejoined text, with
a case that pins the multi-line shape so this cannot silently return.
The program list is wider than shells now, which surfaced a false
positive: tmux takes a `--` separator followed by a program too
(`split-window ... -- cat`). Matching is scoped to files that mention
WSL rather than narrowing the list back.
Also:
- Replaced the `sed` regression case, which was vacuous. A backreference
contains no `$`, so it returned `bac` under both separators and would
have passed without the fix. The block claimed every case proved the
bug. Swapped in a positional argument and a shell local, both measured
to differ -- the positional is the shape `wslUncDirectoryExists` uses,
where `--` blanked `$1` so every existing directory probed as missing.
- windows-shell-args.test.ts derived its expected argv from
buildWslExecArgs, the helper under test, so six assertions would still
pass if it regressed to `--`. Spelled the expectation out.
- Dropped two comments citing the removed `--` behavior as rationale.
---------
Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
@@ -503,13 +503,13 @@ describe('runner execFile timeout handling', () => {
|
||||
|
||||
expect(execFileMock).toHaveBeenCalledWith(
|
||||
'wsl.exe',
|
||||
['-d', 'Ubuntu', '--', 'sh', '-lc', expect.any(String)],
|
||||
['-d', 'Ubuntu', '--exec', 'sh', '-lc', expect.any(String)],
|
||||
expect.objectContaining({ cwd: undefined }),
|
||||
expect.any(Function)
|
||||
)
|
||||
const shellCommand = execFileMock.mock.calls[0]?.[1]?.[5] as string
|
||||
expect(shellCommand).toContain('getent passwd')
|
||||
expect(shellCommand).toContain('exec "\\$_orca_wsl_shell" -ilc')
|
||||
expect(shellCommand).toContain('exec "$_orca_wsl_shell" -ilc')
|
||||
expect(shellCommand).toContain('/mnt/c/repo')
|
||||
expect(shellCommand).toContain("'git'")
|
||||
expect(shellCommand).toContain('status')
|
||||
@@ -533,7 +533,7 @@ describe('runner execFile timeout handling', () => {
|
||||
|
||||
expect(execFileMock).toHaveBeenCalledWith(
|
||||
'wsl.exe',
|
||||
['-d', 'Ubuntu', '--', 'bash', '-c', expect.any(String)],
|
||||
['-d', 'Ubuntu', '--exec', 'bash', '-c', expect.any(String)],
|
||||
expect.objectContaining({ cwd: undefined }),
|
||||
expect.any(Function)
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user