diff --git a/packages/core/src/services/environmentSanitization.ts b/packages/core/src/services/environmentSanitization.ts index 5d82ac227d..dc9c92484d 100644 --- a/packages/core/src/services/environmentSanitization.ts +++ b/packages/core/src/services/environmentSanitization.ts @@ -14,7 +14,12 @@ export function sanitizeEnvironment( processEnv: NodeJS.ProcessEnv, config: EnvironmentSanitizationConfig, ): NodeJS.ProcessEnv { - if (!config.enableEnvironmentVariableRedaction) { + // Enable strict sanitization in GitHub actions. + const isStrictSanitization = + !!processEnv['GITHUB_SHA'] || processEnv['SURFACE'] === 'Github'; + + // Always sanitize when in GitHub actions. + if (!config.enableEnvironmentVariableRedaction && !isStrictSanitization) { return { ...processEnv }; } @@ -27,9 +32,6 @@ export function sanitizeEnvironment( (config.blockedEnvironmentVariables || []).map((k) => k.toUpperCase()), ); - // Enable strict sanitization in GitHub actions. - const isStrictSanitization = !!processEnv['GITHUB_SHA']; - for (const key in processEnv) { const value = processEnv[key]; diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 4061ed5b84..14ffafa3c0 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -84,7 +84,7 @@ const shellExecutionConfig: ShellExecutionConfig = { showColor: false, disableDynamicLineTrimming: true, sanitizationConfig: { - enableEnvironmentVariableRedaction: true, + enableEnvironmentVariableRedaction: false, allowedEnvironmentVariables: [], blockedEnvironmentVariables: [], }, @@ -1422,9 +1422,74 @@ describe('ShellExecutionService environment variables', () => { vi.fn(), new AbortController().signal, true, + { + sanitizationConfig: { + enableEnvironmentVariableRedaction: false, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, + }, + ); + + const cpEnv = mockCpSpawn.mock.calls[0][2].env; + expect(cpEnv).not.toHaveProperty('MY_SENSITIVE_VAR'); + expect(cpEnv).toHaveProperty('PATH', '/test/path'); + expect(cpEnv).toHaveProperty('GEMINI_CLI_TEST_VAR', 'test-value'); + + // Ensure child_process exits + mockChildProcess.emit('exit', 0, null); + mockChildProcess.emit('close', 0, null); + await new Promise(process.nextTick); + }); + + it('should use a sanitized environment when in a GitHub run (SURFACE=Github)', async () => { + // Mock the environment to simulate a GitHub Actions run via SURFACE variable + vi.stubEnv('SURFACE', 'Github'); + vi.stubEnv('MY_SENSITIVE_VAR', 'secret-value'); // This should be stripped out + vi.stubEnv('PATH', '/test/path'); // An essential var that should be kept + vi.stubEnv('GEMINI_CLI_TEST_VAR', 'test-value'); // A test var that should be kept + + vi.resetModules(); + const { ShellExecutionService } = await import( + './shellExecutionService.js' + ); + + // Test pty path + await ShellExecutionService.execute( + 'test-pty-command-surface', + '/', + vi.fn(), + new AbortController().signal, + true, shellExecutionConfig, ); + const ptyEnv = mockPtySpawn.mock.calls[0][2].env; + expect(ptyEnv).not.toHaveProperty('MY_SENSITIVE_VAR'); + expect(ptyEnv).toHaveProperty('PATH', '/test/path'); + expect(ptyEnv).toHaveProperty('GEMINI_CLI_TEST_VAR', 'test-value'); + + // Ensure pty process exits for next test + mockPtyProcess.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); + await new Promise(process.nextTick); + + // Test child_process path + mockGetPty.mockResolvedValue(null); // Force fallback + await ShellExecutionService.execute( + 'test-cp-command-surface', + '/', + vi.fn(), + new AbortController().signal, + true, + { + sanitizationConfig: { + enableEnvironmentVariableRedaction: false, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, + }, + ); + const cpEnv = mockCpSpawn.mock.calls[0][2].env; expect(cpEnv).not.toHaveProperty('MY_SENSITIVE_VAR'); expect(cpEnv).toHaveProperty('PATH', '/test/path');