Skip to content

fix(core,util): fallback to powershell on windows when bash is missing, and quote @ and comma in quotePowerShell - #1514

Open
Morendais wants to merge 1 commit into
google:mainfrom
Morendais:fix-windows-shell-and-quoting
Open

Morendais wants to merge 1 commit into
google:mainfrom
Morendais:fix-windows-shell-and-quoting

Conversation

@Morendais

Copy link
Copy Markdown

Why are these changes needed?

This PR addresses two platform and parameter handling issues in zx:

  1. Windows Default Shell Fallback (src/core.ts):
    On standard Windows environments where bash is not in PATH, useBash() throws during module initialization. Previously, the error was caught silently without assigning a fallback shell, leaving $.quote undefined and causing any subsequent command execution with $ to throw No quote function is defined.
    This change adds a fallback to usePwsh() / usePowerShell() when useBash() fails on win32.

  2. PowerShell Argument Splatting & Grouping Prevention (src/util.ts):
    In quotePowerShell, @ and , were previously included in the unquoted character whitelist. In PowerShell, @ is the splatting operator and , is the array construction operator. Passing unquoted @arg causes PowerShell to attempt variable expansion, resulting in silent argument omission or replacement.
    Removing @ and , from the unquoted set ensures they are safely enclosed in single quotes.

Checks

  • Added unit test assertions in test/util.test.js.
  • Verified command execution and argument preservation.

…g, and quote @ and comma in quotePowerShell

- In src/core.ts, fallback to usePwsh() / usePowerShell() when useBash() throws on Windows, preventing 'No quote function is defined' on clean Windows environments.
- In src/util.ts, remove '@' and ',' from unquoted character whitelist in quotePowerShell to prevent PowerShell splatting variable expansion and array grouping parameter corruption.
- Add regression tests for quotePowerShell in test/util.test.js.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant