Skip to content
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
42 changes: 42 additions & 0 deletions packages/core/src/services/environmentSanitization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,48 @@ describe('sanitizeEnvironment', () => {
});
});

describe('value-first security: secret values must be caught even for allowed variable names', () => {
it('should redact ALWAYS_ALLOWED variables whose values contain a GitHub token', () => {
const env = {
HOME: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
PATH: '/usr/bin',
};
const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS);
expect(sanitized).toEqual({ PATH: '/usr/bin' });
});

it('should redact ALWAYS_ALLOWED variables whose values contain a certificate', () => {
const env = {
SHELL:
'-----BEGIN RSA PRIVATE KEY-----\nMIIE...\n-----END RSA PRIVATE KEY-----',
USER: 'alice',
};
const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS);
expect(sanitized).toEqual({ USER: 'alice' });
});

it('should redact user-allowlisted variables whose values contain a secret', () => {
const env = {
MY_SAFE_VAR: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
OTHER: 'fine',
};
const sanitized = sanitizeEnvironment(env, {
allowedEnvironmentVariables: ['MY_SAFE_VAR'],
blockedEnvironmentVariables: [],
enableEnvironmentVariableRedaction: true,
});
expect(sanitized).toEqual({ OTHER: 'fine' });
});

it('should NOT redact GEMINI_CLI_ variables even if their value looks like a secret (fully trusted)', () => {
const env = {
GEMINI_CLI_INTERNAL: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
};
const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS);
expect(sanitized).toEqual(env);
});
});

it('should ensure all names in the sets are capitalized', () => {
for (const name of ALWAYS_ALLOWED_ENVIRONMENT_VARIABLES) {
expect(name).toBe(name.toUpperCase());
Expand Down
32 changes: 13 additions & 19 deletions packages/core/src/services/environmentSanitization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@
processEnv: NodeJS.ProcessEnv,
config: EnvironmentSanitizationConfig,
): NodeJS.ProcessEnv {
// 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 };
}
Expand Down Expand Up @@ -148,28 +146,33 @@
key = key.toUpperCase();
value = value?.toUpperCase();

// User overrides take precedence.
if (key.startsWith('GEMINI_CLI_')) {
return false;
}

if (value) {
for (const pattern of NEVER_ALLOWED_VALUE_PATTERNS) {
if (pattern.test(value)) {

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
that depends on
library input
may run slow on strings starting with 'ftp://9:' and with many repetitions of 'ftp://9:'.
This
regular expression
that depends on
library input
may run slow on strings starting with 'eyj' and with many repetitions of 'eyj'.
This
regular expression
that depends on
library input
may run slow on strings starting with 'ftp://9:' and with many repetitions of 'ftp://9:'.
This
regular expression
that depends on
library input
may run slow on strings starting with 'eyj' and with many repetitions of 'eyj'.
This
regular expression
that depends on
library input
may run slow on strings starting with 'ftp://9:' and with many repetitions of 'ftp://9:'.
This
regular expression
that depends on
library input
may run slow on strings starting with 'eyj' and with many repetitions of 'eyj'.
return true;
}
}
}

if (allowedSet?.has(key)) {
return false;
}
if (blockedSet?.has(key)) {
return true;
}

// These are never redacted.
if (
ALWAYS_ALLOWED_ENVIRONMENT_VARIABLES.has(key) ||
key.startsWith('GEMINI_CLI_')
) {
if (ALWAYS_ALLOWED_ENVIRONMENT_VARIABLES.has(key)) {
return false;
}

// These are always redacted.
if (NEVER_ALLOWED_ENVIRONMENT_VARIABLES.has(key)) {
return true;
}

// If in strict mode (e.g. GitHub Action), and not explicitly allowed, redact it.
if (isStrictSanitization) {
return true;
}
Expand All @@ -180,14 +183,5 @@
}
}

// Redact if the value looks like a key/cert.
if (value) {
for (const pattern of NEVER_ALLOWED_VALUE_PATTERNS) {
if (pattern.test(value)) {
return true;
}
}
}

return false;
}
Loading