fix(core): allow disabling environment variable redaction (#23927)

This commit is contained in:
Gal Zahavi
2026-03-26 13:04:44 -07:00
committed by GitHub
parent 1d230dbfbf
commit d33170931c
5 changed files with 34 additions and 10 deletions
@@ -112,7 +112,10 @@ describe('MacOsSandboxManager', () => {
SAFE_VAR: '1', SAFE_VAR: '1',
GITHUB_TOKEN: 'sensitive', GITHUB_TOKEN: 'sensitive',
}, },
policy: mockPolicy, policy: {
...mockPolicy,
sanitizationConfig: { enableEnvironmentVariableRedaction: true },
},
}); });
expect(result.env['SAFE_VAR']).toBe('1'); expect(result.env['SAFE_VAR']).toBe('1');
@@ -375,9 +375,9 @@ describe('sanitizeEnvironment', () => {
}); });
describe('getSecureSanitizationConfig', () => { describe('getSecureSanitizationConfig', () => {
it('should enable environment variable redaction by default', () => { it('should default enableEnvironmentVariableRedaction to false', () => {
const config = getSecureSanitizationConfig(); const config = getSecureSanitizationConfig();
expect(config.enableEnvironmentVariableRedaction).toBe(true); expect(config.enableEnvironmentVariableRedaction).toBe(false);
}); });
it('should merge allowed and blocked variables from base and requested configs', () => { it('should merge allowed and blocked variables from base and requested configs', () => {
@@ -440,13 +440,13 @@ describe('getSecureSanitizationConfig', () => {
expect(config.blockedEnvironmentVariables).toEqual(['BLOCKED_VAR']); expect(config.blockedEnvironmentVariables).toEqual(['BLOCKED_VAR']);
}); });
it('should force enableEnvironmentVariableRedaction to true even if requested false', () => { it('should respect requested enableEnvironmentVariableRedaction value', () => {
const requestedConfig = { const requestedConfig = {
enableEnvironmentVariableRedaction: false, enableEnvironmentVariableRedaction: false,
}; };
const config = getSecureSanitizationConfig(requestedConfig); const config = getSecureSanitizationConfig(requestedConfig);
expect(config.enableEnvironmentVariableRedaction).toBe(true); expect(config.enableEnvironmentVariableRedaction).toBe(false);
}); });
}); });
@@ -230,6 +230,9 @@ export function getSecureSanitizationConfig(
allowedEnvironmentVariables: [...new Set(allowed)], allowedEnvironmentVariables: [...new Set(allowed)],
blockedEnvironmentVariables: [...new Set(blocked)], blockedEnvironmentVariables: [...new Set(blocked)],
// Redaction must be enabled for secure configurations // Redaction must be enabled for secure configurations
enableEnvironmentVariableRedaction: true, enableEnvironmentVariableRedaction:
requestedConfig.enableEnvironmentVariableRedaction ??
baseConfig?.enableEnvironmentVariableRedaction ??
false,
}; };
} }
@@ -108,7 +108,18 @@ function ensureSandboxAvailable(): boolean {
if (platform === 'darwin') { if (platform === 'darwin') {
if (fs.existsSync('/usr/bin/sandbox-exec')) { if (fs.existsSync('/usr/bin/sandbox-exec')) {
return true; try {
execSync('sandbox-exec -p "(version 1)(allow default)" echo test', {
stdio: 'ignore',
});
return true;
} catch {
// eslint-disable-next-line no-console
console.warn(
'sandbox-exec is present but cannot be used (likely running inside a sandbox already). Skipping sandbox tests.',
);
return false;
}
} }
throw new Error( throw new Error(
'Sandboxing tests on macOS require /usr/bin/sandbox-exec to be present.', 'Sandboxing tests on macOS require /usr/bin/sandbox-exec to be present.',
@@ -148,6 +148,11 @@ describe('SandboxManager', () => {
MY_SECRET: 'super-secret', MY_SECRET: 'super-secret',
SAFE_VAR: 'is-safe', SAFE_VAR: 'is-safe',
}, },
policy: {
sanitizationConfig: {
enableEnvironmentVariableRedaction: true,
},
},
}; };
const result = await sandboxManager.prepareCommand(req); const result = await sandboxManager.prepareCommand(req);
@@ -158,7 +163,7 @@ describe('SandboxManager', () => {
expect(result.env['MY_SECRET']).toBeUndefined(); expect(result.env['MY_SECRET']).toBeUndefined();
}); });
it('should NOT allow disabling environment variable redaction if requested in config (vulnerability fix)', async () => { it('should allow disabling environment variable redaction if requested in config', async () => {
const req = { const req = {
command: 'echo', command: 'echo',
args: ['hello'], args: ['hello'],
@@ -175,8 +180,8 @@ describe('SandboxManager', () => {
const result = await sandboxManager.prepareCommand(req); const result = await sandboxManager.prepareCommand(req);
// API_KEY should be redacted because SandboxManager forces redaction and API_KEY matches NEVER_ALLOWED_NAME_PATTERNS // API_KEY should be preserved because redaction was explicitly disabled
expect(result.env['API_KEY']).toBeUndefined(); expect(result.env['API_KEY']).toBe('sensitive-key');
}); });
it('should respect allowedEnvironmentVariables in config but filter sensitive ones', async () => { it('should respect allowedEnvironmentVariables in config but filter sensitive ones', async () => {
@@ -191,6 +196,7 @@ describe('SandboxManager', () => {
policy: { policy: {
sanitizationConfig: { sanitizationConfig: {
allowedEnvironmentVariables: ['MY_SAFE_VAR', 'MY_TOKEN'], allowedEnvironmentVariables: ['MY_SAFE_VAR', 'MY_TOKEN'],
enableEnvironmentVariableRedaction: true,
}, },
}, },
}; };
@@ -214,6 +220,7 @@ describe('SandboxManager', () => {
policy: { policy: {
sanitizationConfig: { sanitizationConfig: {
blockedEnvironmentVariables: ['BLOCKED_VAR'], blockedEnvironmentVariables: ['BLOCKED_VAR'],
enableEnvironmentVariableRedaction: true,
}, },
}, },
}; };