fix(core): preserve system PATH in Git environment to fix ENOENT (#25034) (#26587)

This commit is contained in:
Coco Sheng
2026-05-07 14:24:49 -04:00
committed by GitHub
parent a809bc7c51
commit 49456e4e15
2 changed files with 154 additions and 1 deletions
@@ -304,6 +304,146 @@ describe('GitService', () => {
);
expect(systemConfigContent).toBe('');
});
describe('environment variable preservation', () => {
const customPath = '/custom/bin';
const safeHome = '/home/user';
const sensitiveKey = 'sk-123456789';
beforeEach(() => {
vi.stubEnv('PATH', customPath);
vi.stubEnv('HOME', safeHome);
vi.stubEnv('API_KEY', sensitiveKey);
vi.stubEnv('UNRELATED_VAR', 'some-value');
// Explicitly unset strict mode triggers to ensure predictable test behavior
// across local and CI environments.
vi.stubEnv('GITHUB_SHA', '');
vi.stubEnv('SURFACE', '');
hoistedMockCheckIsRepo.mockResolvedValue(false);
});
afterEach(() => {
vi.unstubAllEnvs();
});
it('should preserve system PATH in the Git environment', async () => {
const service = new GitService(projectRoot, storage);
await service.setupShadowGitRepository();
expect(hoistedMockEnv).toHaveBeenCalledWith(
expect.objectContaining({
PATH: customPath,
GIT_CONFIG_GLOBAL: expect.any(String),
GIT_AUTHOR_NAME: SHADOW_REPO_AUTHOR_NAME,
}),
);
});
it('should preserve safe environment variables like HOME', async () => {
const service = new GitService(projectRoot, storage);
await service.setupShadowGitRepository();
expect(hoistedMockEnv).toHaveBeenCalledWith(
expect.objectContaining({
HOME: safeHome,
}),
);
});
it('should NOT include sensitive environment variables like API_KEY', async () => {
const service = new GitService(projectRoot, storage);
await service.setupShadowGitRepository();
const callArgs = hoistedMockEnv.mock.calls[0][0];
expect(callArgs.API_KEY).toBeUndefined();
});
it('should preserve unrelated environment variables in non-strict mode', async () => {
const service = new GitService(projectRoot, storage);
await service.setupShadowGitRepository();
const callArgs = hoistedMockEnv.mock.calls[0][0];
expect(callArgs.UNRELATED_VAR).toBe('some-value');
});
it('should explicitly unset GIT_DIR and GIT_WORK_TREE to maintain isolation', async () => {
const service = new GitService(projectRoot, storage);
await service.setupShadowGitRepository();
const callArgs = hoistedMockEnv.mock.calls[0][0];
expect(callArgs.GIT_DIR).toBeUndefined();
expect(callArgs.GIT_WORK_TREE).toBeUndefined();
});
});
describe('GIT_CONFIG isolation', () => {
beforeEach(() => {
vi.stubEnv('GIT_CONFIG_GLOBAL', '/user/global/config');
vi.stubEnv('GIT_CONFIG_SYSTEM', '/user/system/config');
hoistedMockCheckIsRepo.mockResolvedValue(false);
});
afterEach(() => {
vi.unstubAllEnvs();
});
it('should override GIT_CONFIG environment variables from process.env', async () => {
const service = new GitService(projectRoot, storage);
await service.setupShadowGitRepository();
const expectedConfigPath = path.join(repoDir, '.gitconfig');
const expectedSystemPath = path.join(
repoDir,
'.gitconfig_system_empty',
);
expect(hoistedMockEnv).toHaveBeenCalledWith(
expect.objectContaining({
GIT_CONFIG_GLOBAL: expectedConfigPath,
GIT_CONFIG_SYSTEM: expectedSystemPath,
}),
);
// Ensure it's not using the values from stubbed process.env
const callArgs = hoistedMockEnv.mock.calls[0][0];
expect(callArgs.GIT_CONFIG_GLOBAL).not.toBe('/user/global/config');
expect(callArgs.GIT_CONFIG_SYSTEM).not.toBe('/user/system/config');
});
});
describe('shadowGitRepository prioritization', () => {
beforeEach(() => {
vi.stubEnv('GIT_DIR', '/user/fake/.git');
vi.stubEnv('GIT_WORK_TREE', '/user/fake/worktree');
hoistedMockCheckIsRepo.mockResolvedValue(true);
});
afterEach(() => {
vi.unstubAllEnvs();
});
it('should prioritize internal GIT_DIR and GIT_WORK_TREE over process.env', async () => {
const service = new GitService(projectRoot, storage);
// Trigger a call to shadowGitRepository (e.g., via getCurrentCommitHash)
hoistedMockRaw.mockResolvedValue('hash');
await service.getCurrentCommitHash();
const expectedRepoDir = storage.getHistoryDir();
const expectedGitDir = path.join(expectedRepoDir, '.git');
expect(hoistedMockEnv).toHaveBeenCalledWith(
expect.objectContaining({
GIT_DIR: expectedGitDir,
GIT_WORK_TREE: projectRoot,
}),
);
// Ensure user env was overridden
const callArgs = hoistedMockEnv.mock.calls[0][0];
expect(callArgs.GIT_DIR).not.toBe('/user/fake/.git');
expect(callArgs.GIT_WORK_TREE).not.toBe('/user/fake/worktree');
});
});
});
describe('createFileSnapshot', () => {
+14 -1
View File
@@ -11,6 +11,10 @@ import { spawnAsync } from '../utils/shell-utils.js';
import { simpleGit, CheckRepoActions, type SimpleGit } from 'simple-git';
import type { Storage } from '../config/storage.js';
import { debugLogger } from '../utils/debugLogger.js';
import {
sanitizeEnvironment,
getSecureSanitizationConfig,
} from './environmentSanitization.js';
export const SHADOW_REPO_AUTHOR_NAME = 'Gemini CLI';
export const SHADOW_REPO_AUTHOR_EMAIL = 'gemini-cli@google.com';
@@ -58,9 +62,18 @@ export class GitService {
const gitConfigPath = path.join(repoDir, '.gitconfig');
const systemConfigPath = path.join(repoDir, '.gitconfig_system_empty');
return {
...sanitizeEnvironment(
process.env,
getSecureSanitizationConfig({
enableEnvironmentVariableRedaction: true,
}),
),
// Prevent git from using the user's global git config.
GIT_CONFIG_GLOBAL: gitConfigPath,
GIT_CONFIG_SYSTEM: systemConfigPath,
// Ensure we don't inherit isolation-breaking variables from the user environment.
GIT_DIR: undefined,
GIT_WORK_TREE: undefined,
// Explicitly provide identity to prevent "Author identity unknown" errors
// inside sandboxed environments like Docker where the gitconfig might not
// be picked up properly.
@@ -126,9 +139,9 @@ export class GitService {
private get shadowGitRepository(): SimpleGit {
const repoDir = this.getHistoryDir();
return simpleGit(this.projectRoot).env({
...this.getShadowRepoEnv(repoDir),
GIT_DIR: path.join(repoDir, '.git'),
GIT_WORK_TREE: this.projectRoot,
...this.getShadowRepoEnv(repoDir),
});
}