Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable implicit execution of batch files on Windows #14557

Conversation

straight-shoota
Copy link
Member

This patch disables the implicit execution of batch files (i.e. path name extension .bat or .cmd, case-insensitive) in Process.run on Windows with shell: false (default). There are no effects on other operating systems.
The disabled behaviour is an irrational and potentially dangerous feature of CreateProcessW which is not intended for Process.run.

If you want to run a batch file on Windows, you need to launch it through a shell, either implicitly with shell: true or explicitly by running cmd.exe with the batch file as argument.
The semantics are different on POSIX platforms where you can directly execute a shell script if it has a shebang. The mechanism on Windows works differently, as CreateProcessW implicitly injects cmd.exe /c which changes the semantics of run arguments.

A similar approach has been taken by node.js: nodejs/node@64b6777

Resolves #14536

Copy link
Member

@oprypin oprypin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

spec/std/process_spec.cr Outdated Show resolved Hide resolved
spec/std/process_spec.cr Outdated Show resolved Hide resolved
@straight-shoota straight-shoota added this to the 1.13.0 milestone May 3, 2024
@straight-shoota straight-shoota merged commit 4a4952c into crystal-lang:master May 8, 2024
60 checks passed
@straight-shoota straight-shoota deleted the feat/win32-process-run-disable-bat branch May 8, 2024 17:30
@straight-shoota straight-shoota changed the title Disable implicit execution of batch files Disable implicit execution of batch files on Windows Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential for command injection in arguments of Process.run on Windows
2 participants