fix: add PATH-based Unix shell detection for stripped environments#3372
Open
Zireael wants to merge 1 commit intocode-yeongyu:devfrom
Open
fix: add PATH-based Unix shell detection for stripped environments#3372Zireael wants to merge 1 commit intocode-yeongyu:devfrom
Zireael wants to merge 1 commit intocode-yeongyu:devfrom
Conversation
When environment variables are completely stripped (e.g., parent process spawns bash with clean env), detect Unix shells by checking PATH for MSYS2/Cygwin/Git Bash indicators. This is a belt-and-suspenders fallback layer (layer 3) that runs after: 1. SHELL env var (primary) 2. BASH_VERSION, MSYSTEM, WSL_DISTRO_NAME (Windows indicators) Indicators checked: - \usr\bin or /usr/bin (MSYS2, Git Bash) - \msys64\bin or /msys64/bin (MSYS2) - \cygwin\bin or /cygwin/bin (Cygwin) - \git\usr\bin or /git/usr/bin (Git Bash) Fixes edge case where detectShellType() would incorrectly return 'powershell' when all shell env vars are stripped by parent process.
Author
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
No issues found across 1 file
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: High risk of regression on Windows: checking PATH for Unix directories may misdetect the current shell as 'unix' even when running in PowerShell/CMD if Git or MSYS2 is installed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a third detection layer for Unix shells on Windows that catches cases where all shell-related environment variables have been stripped by the parent process.
Problem
PR #3370 fixed the primary issue where
PSModulePath(always set on Windows) was checked before Unix shell indicators. However, there's an edge case where:SHELL,BASH_VERSION,MSYSTEM,WSL_DISTRO_NAME) are strippedPSModulePathandprocess.platformremainIn this case,
detectShellType()would still return"powershell".Solution
Added
detectUnixPathInPATH()function that checks PATH for Unix shell indicators:This is scoped to
process.platform === "win32"and runs as layer 3 (after env vars, before PSModulePath).Detection Priority (updated)
SHELLenv var → Unix shell (explicit)BASH_VERSION,MSYSTEM,WSL_DISTRO_NAMEdetectUnixPathInPATH()PSModulePath→ PowerShellVerification
/bindirs (.cargo/bin, user directories) correctly ignoredTesting
PR #3370 introduced regression tests. This edge case fix adds:
detectUnixPathInPATH()unit tests with mocked PATHCloses #3366
Summary by cubic
Adds a PATH-based fallback to correctly detect Unix shells on Windows when env vars are stripped, preventing false PowerShell detection. Improves
detectShellType()behavior for Git Bash, MSYS2, and Cygwin.detectUnixPathInPATH()to scanPATHfor/usr/bin,/msys64/bin,/cygwin/bin, and/git/usr/binonwin32.SHELL→BASH_VERSION/MSYSTEM/WSL_DISTRO_NAME→ PATH fallback →PSModulePath→ platform default.Written for commit 9a7b567. Summary will update on new commits.