Repository navigation
fix(file): stop trusting a configured extension to be OS-launchable - #12023
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughWindows direct task execution now accepts only OS-launchable extensions. Tests cover configured extensions, interpreter mappings, Bash shebang execution, and direct CMD execution. ChangesWindows executable handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change restores shebang-based execution for configured non-native extensions while preserving supported Windows launchable extensions, with no actionable merge-blocking risk remaining after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant WindowsTask
participant task_executor
participant can_execute_directly
participant Shell
WindowsTask->>task_executor: execute task
task_executor->>can_execute_directly: check extension
can_execute_directly-->>task_executor: direct-launchable or interpreter-required
task_executor->>Shell: run through mapped interpreter when required
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR separates user-configured executable-extension policy from the fixed set of extensions Windows can launch directly.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (5): Last reviewed commit: "fix(file): stop trusting a configured ex..." | Re-trigger Greptile |
3c8ae04 to
e0f15a9
Compare
`can_execute_directly` subtracted a fixed interpreter-only list (ps1, vbs) from `windows_executable_extensions`, which a user can edit, and treated whatever was left as something `CreateProcess` could start. Against the shipped default that leaves exe/bat/cmd/com -- exactly what the OS can start -- so the shape was right for the default list and wrong for any other. Adding `sh` to the setting made mise hand a shebang script straight to `Command::new`, and a task that ran fine a moment earlier died with "%1 is not a valid Win32 application" (os error 193) without the shebang ever being read. Replaced with `OS_LAUNCHABLE_EXTENSIONS`, ANDed with the setting: a user can still narrow what mise treats as executable, but cannot widen what the OS is able to start. Answers for the default list are unchanged. The `shell_from_extension` invariant test now derives its extensions from the setting minus that list rather than from a hand-kept constant, so a new default extension without an interpreter mapping is caught too.
e0f15a9 to
b2d8130
Compare
Using
windows_executable_extensionsthe way it is documented breaks a task that worked a momentearlier.
mise-tasks/hello.sh, with a shebang:The shebang is never read in B. mise decided the OS could start the file and handed it to
Command::new.Why
can_execute_directlysubtracted a fixed interpreter-only list —["ps1", "vbs"]— fromwindows_executable_extensions, and treated whatever remained as somethingCreateProcesscanstart. Against the shipped default that leaves
exe/bat/cmd/com, which is exactly the setthe OS can start, so the shape was right for the default list and wrong for every other one. The
setting is user-editable; the list of what Windows can launch is not.
Two callers depend on that answer, and both are affected:
task_executor's file-task path — the reproduction above.backend::is_spawnable, which decides whether a tool's binary can be spawned from a path.The change
The two questions are separate, so they are asked separately and ANDed:
windows_executable_extensions, which a usermay narrow.
OS_LAUNCHABLE_EXTENSIONS—exeandcomnatively,batand
cmdbecause std routes those through cmd.exe with escaped arguments — which a user may notwiden.
For the default list the answers are identical to before (
exe/bat/cmd/comyes,ps1/vbsno), which is precisely why the old shape looked correct.INTERPRETER_ONLY_EXTENSIONSis gone:
can_execute_directlywas its only non-test use.A file that is now correctly rejected falls through to
shell_from_shebang, thenshell_from_extension, thendefault_file_shell— the existing chain, which is how the.shabove finds bash again.
Deliberately unchanged
shell_from_extensiongains no extensions. Which interpreters mise should know by default isa separate call; a user who adds
shis served by the shebang.windows_executable_extensions..pywith no shebang, say — stillfails. It reaches
cmd /crather thanCreateProcess, so os error 193 becomes a failure fromthe configured default shell. Better located, still a failure.
Tests
src/file.rs:os_can_launch_extensionacceptsexe/com/bat/cmdcase-insensitively andrejects
ps1/vbsalong with extensions a user might add (sh,py,js) and the emptystring;
can_execute_directlygives the same answers as before for the default list.src/task/task_executor.rs: theshell_from_extensioninvariant now derives its extensions fromthe setting minus the OS-launchable list instead of a hand-kept constant, so a new default
extension without an interpreter mapping is caught too. It asserts the derived set is non-empty
so the loop cannot pass vacuously.
e2e-win/task_executable_extensions.Tests.ps1: the reproduction above, both halves, plus a.cmdtask to show the whitelist still launches what it should. The script body isprintf, sothe assertion can only pass through bash.
No unit test overrides
windows_executable_extensions:Settings::override_withmutatesprocess-wide state that other tests in the same binary read through
is_executable. The widenedsetting is covered in
e2e-win, in its own process — which is also how the bug was measured.Verification
Measured on 2026.8.2 windows-x64: the A/B pair above. Existing tests were checked against the new
predicate rather than assumed —
is_spawnable_rejects_interpreter_only_extensions_case_insensitivelyasserts.PS1is rejected,which the whitelist still does, and
which_in_dirs_ignores_missing_candidatesusestool.exe.cargo fmtwas run locally. The build, clippy and the test suite were not — those are left toCI, so behaviour after the fix rests on the measurement above and on the tests rather than on a
running binary.
Found while auditing Windows behaviour, the same pass that produced #12007, #12008, #12012, #12013,
#12014 and #12016. No existing issue or discussion covers it.
Summary by CodeRabbit
New Features
.exe,.com,.bat, and.cmdfiles.Bug Fixes