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

Make sure we are copying the path variable #570

Merged
merged 8 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions lib/git-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,17 @@ export function setupEnvironment(
env: Record<string, string | undefined>
gitLocation: string
} {
// This will get Path, pATh, PATH et all on Windows
const PATH = process.env.PATH

const env: Record<string, string | undefined> = {
...process.env,
// Merge all of process.env except Path, PATH, et all, we'll add that in just a sec
...Object.fromEntries(
Object.entries(process.env).filter(([k]) => k.toUpperCase() !== 'PATH')
),
// Ensure PATH is always set in upper case not process.env.Path like can
// be on case-insensitive Windows
...(PATH ? { PATH } : {}),
...environmentVariables,
}

Expand All @@ -91,22 +100,13 @@ export function setupEnvironment(

if (process.platform === 'win32') {
const mingw = process.arch === 'x64' ? 'mingw64' : 'mingw32'
env.PATH = `${gitDir}\\${mingw}\\bin;${gitDir}\\${mingw}\\usr\\bin;${env.PATH}`
env.PATH = `${gitDir}\\${mingw}\\bin;${gitDir}\\${mingw}\\usr\\bin;${
env.PATH ?? ''
}`
}

env.GIT_EXEC_PATH = resolveGitExecPath(env)

if (process.platform === 'win32') {
// while reading the environment variable is case-insensitive
// you can create a hash with multiple values, which means the
// wrong value might be used when spawning the child process
//
// this ensures we only ever supply one value for PATH
if (env.Path) {
delete env.Path
}
}

// On Windows the contained Git environment (minGit) ships with a system level
// gitconfig that we can control but on macOS and Linux /etc/gitconfig is used
// as the system-wide configuration file and we're unable to modify it.
Expand Down
24 changes: 24 additions & 0 deletions test/fast/environment-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,28 @@ describe('environment variables', () => {
delete process.env.GIT_EXEC_PATH
}
})

if (process.platform === 'win32') {
it('resulting PATH contains the original PATH', () => {
const originalPathKey = Object.keys(process.env).find(
k => k.toUpperCase() === 'PATH'
)
expect(originalPathKey).not.toBeUndefined()

const originalPathValue = process.env.PATH

try {
delete process.env.PATH
process.env.Path = 'wow-such-case-insensitivity'
// This test will ensure that on platforms where env vars names are
// case-insensitive (like Windows) we don't end up with an invalid PATH
// and the original one lost in the process.
const { env } = setupEnvironment({})
expect(env.PATH).toContain('wow-such-case-insensitivity')
} finally {
delete process.env.Path
process.env[originalPathKey!] = originalPathValue
}
})
}
})
Loading