chore: revert out of scope changes and fix GH_TOKEN allowlist

This commit is contained in:
Christian Gunderman
2026-05-15 11:10:48 -07:00
parent 12233fd34b
commit 62246d2fad
8 changed files with 25 additions and 69 deletions
@@ -17,7 +17,6 @@ import {
handleAtCommand,
escapeAtSymbols,
unescapeLiteralAt,
checkPermissions,
} from './atCommandProcessor.js';
import {
FileDiscoveryService,
@@ -1540,23 +1539,4 @@ describe('unescapeLiteralAt', () => {
const input = 'user@example.com and @scope/pkg';
expect(unescapeLiteralAt(escapeAtSymbols(input))).toBe(input);
});
describe('checkPermissions', () => {
it('should handle ENAMETOOLONG gracefully in checkPermissions', async () => {
const longPath = 'a'.repeat(5000);
const query = `@${longPath}`;
const localMockConfig = {
getTargetDir: () => '.',
validatePathAccess: () => true,
getResourceRegistry: () => ({
findResourceByUri: () => undefined,
}),
} as unknown as Config;
// checkPermissions should not throw ENAMETOOLONG
const permissions = await checkPermissions(query, localMockConfig);
expect(permissions).toEqual([]);
});
});
});
@@ -188,15 +188,9 @@ export async function checkPermissions(
const pathName = part.content.substring(1);
if (!pathName) continue;
let resolvedPathName: string;
try {
resolvedPathName = resolveToRealPath(
path.resolve(config.getTargetDir(), pathName),
);
} catch {
// If path resolution fails (e.g. ENAMETOOLONG), skip this path
continue;
}
const resolvedPathName = resolveToRealPath(
path.resolve(config.getTargetDir(), pathName),
);
if (config.validatePathAccess(resolvedPathName, 'read')) {
if (await fileExists(resolvedPathName)) {
@@ -372,14 +372,13 @@ describe('getSecureSanitizationConfig', () => {
it('should not filter out variables from allowed list that match NEVER_ALLOWED_NAME_PATTERNS', () => {
const requestedConfig = {
allowedEnvironmentVariables: ['SAFE_VAR', 'MY_SECRET_TOKEN', 'GH_TOKEN'],
allowedEnvironmentVariables: ['SAFE_VAR', 'MY_SECRET_TOKEN'],
};
const config = getSecureSanitizationConfig(requestedConfig);
expect(config.allowedEnvironmentVariables).toContain('SAFE_VAR');
expect(config.allowedEnvironmentVariables).toContain('MY_SECRET_TOKEN');
expect(config.allowedEnvironmentVariables).toContain('GH_TOKEN');
});
it('should deduplicate variables in allowed and blocked lists', () => {
@@ -14,11 +14,9 @@ export function sanitizeEnvironment(
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 };
}
@@ -152,28 +150,38 @@ function shouldRedactEnvironmentVariable(
key = key.toUpperCase();
value = value?.toUpperCase();
// User overrides take precedence.
if (allowedSet?.has(key)) {
return false;
}
if (key.startsWith('GEMINI_CLI_')) {
return false;
}
if (value) {
for (const pattern of NEVER_ALLOWED_VALUE_PATTERNS) {
if (pattern.test(value)) {
return true;
}
}
}
if (key.startsWith('GIT_CONFIG_')) {
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;
}
@@ -184,15 +192,6 @@ function shouldRedactEnvironmentVariable(
}
}
// 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;
}
-13
View File
@@ -602,19 +602,6 @@ describe('resolveToRealPath', () => {
/Infinite recursion detected/,
);
});
it('should handle ENAMETOOLONG gracefully', () => {
const longPath = path.resolve('/' + 'a'.repeat(5000));
vi.spyOn(fs, 'realpathSync').mockImplementation(() => {
const err = new Error('ENAMETOOLONG') as NodeJS.ErrnoException;
err.code = 'ENAMETOOLONG';
throw err;
});
// Should return the path itself if realpathSync fails with ENAMETOOLONG
expect(resolveToRealPath(longPath)).toBe(longPath);
});
});
describe('makeRelative', () => {
+1 -4
View File
@@ -460,10 +460,7 @@ function robustRealpath(p: string, visited = new Set<string>()): string {
lstatError &&
typeof lstatError === 'object' &&
'code' in lstatError &&
(lstatError.code === 'ENOENT' ||
lstatError.code === 'EISDIR' ||
lstatError.code === 'ENAMETOOLONG' ||
lstatError.code === 'EINVAL')
(lstatError.code === 'ENOENT' || lstatError.code === 'EISDIR')
)
) {
throw lstatError;
+1 -1
View File
@@ -2,7 +2,7 @@
"extends": "../../tsconfig.json",
"compilerOptions": {
"outDir": "dist",
"lib": ["DOM", "DOM.Iterable", "ES2023"],
"lib": ["DOM", "DOM.Iterable", "ES2021"],
"composite": true,
"types": ["node"]
},
+1 -1
View File
@@ -3,7 +3,7 @@
"module": "NodeNext",
"moduleResolution": "NodeNext",
"target": "ES2022",
"lib": ["ES2023", "dom"],
"lib": ["ES2022", "dom"],
"sourceMap": true,
/*
* skipLibCheck is necessary because the a2a-server package depends on