MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-06-28 12:17:02 -07:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): enforce case-insensitive sensitive path blocklist and vscode hitl (#27966)

Browse Source 
Co-authored-by: David Pierce <davidapierce@google.com>
This commit is contained in:
luisfelipe-alt
2026-06-26 19:36:00 +00:00
committed by GitHub
parent b14416447e
commit ae0a3aa7b9
5 changed files with 210 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/core/src/safety/built-in.test.ts
+68
View File
@@ -240,4 +240,72 @@ describe('AllowedPathChecker', () => {
const result = await checker.check(input);
expect(result.decision).toBe(SafetyCheckDecision.ALLOW);
});
describe('Security Regression: Case-Insensitive Blocklist & .vscode HITL', () => {
it('should deny sensitive paths like .git, .env, and node_modules case-insensitively, including Windows trailing character and NTFS ADS bypasses', async () => {
const sensitivePaths = [
path.join(mockCwd, '.git', 'config'),
path.join(mockCwd, '.GIT', 'config'),
path.join(mockCwd, '.Git', 'config'),
path.join(mockCwd, '.env'),
path.join(mockCwd, '.Env'),
path.join(mockCwd, '.ENV'),
path.join(mockCwd, 'node_modules', 'package', 'index.js'),
path.join(mockCwd, 'NODE_MODULES', 'package', 'index.js'),
// Windows trailing character bypasses
path.join(mockCwd, '.git ', 'config'),
path.join(mockCwd, '.git.', 'config'),
path.join(mockCwd, '.env ', 'config'),
path.join(mockCwd, '.env.', 'config'),
path.join(mockCwd, 'node_modules ', 'package', 'index.js'),
// NTFS Alternate Data Stream bypasses
path.join(mockCwd, '.git::$DATA', 'config'),
path.join(mockCwd, '.env::$DATA'),
path.join(mockCwd, 'node_modules::$DATA', 'package', 'index.js'),
];
for (const p of sensitivePaths) {
const input = createInput({ path: p });
const result = await checker.check(input);
expect(result.decision).toBe(SafetyCheckDecision.DENY);
expect(result.reason).toContain('Access to sensitive path');
}
});
it('should require ASK_USER for .vscode configuration files inside workspace, but deny them if outside, including NTFS ADS bypasses', async () => {
const vscodePaths = [
path.join(mockCwd, '.vscode', 'settings.json'),
path.join(mockCwd, '.vscode', 'settings.JSON'),
path.join(mockCwd, '.VSCODE', 'settings.json'),
path.join(mockCwd, '.vscode', 'launch.json'),
// Windows trailing character bypasses
path.join(mockCwd, '.vscode ', 'settings.json'),
path.join(mockCwd, '.vscode.', 'settings.json'),
// NTFS Alternate Data Stream bypasses
path.join(mockCwd, '.vscode::$DATA', 'settings.json'),
];
for (const p of vscodePaths) {
const input = createInput({ path: p });
const result = await checker.check(input);
expect(result.decision).toBe(SafetyCheckDecision.ASK_USER);
expect(result.reason).toContain(
'Modifying .vscode configuration files requires explicit user confirmation',
);
}
// Verify that paths outside the workspace containing .vscode are strictly denied
const outsideVscodePaths = [
path.join(testRootDir, 'outside', '.vscode', 'settings.json'),
path.join(testRootDir, 'outside', '.VSCODE', 'settings.json'),
];
for (const p of outsideVscodePaths) {
const input = createInput({ path: p });
const result = await checker.check(input);
expect(result.decision).toBe(SafetyCheckDecision.DENY);
expect(result.reason).toContain('outside of the allowed workspace');
}
});
});
});
packages/core/src/safety/built-in.ts
+68 -13
View File
@@ -5,13 +5,13 @@
*/
import * as path from 'node:path';
import * as fs from 'node:fs';
import {
SafetyCheckDecision,
type SafetyCheckInput,
type SafetyCheckResult,
} from './protocol.js';
import type { AllowedPathConfig } from '../policy/types.js';
import { resolveToRealPath } from '../utils/paths.js';
/**
* Interface for all in-process safety checkers.
@@ -45,6 +45,11 @@ export class AllowedPathChecker implements InProcessChecker {
excludedArgs,
);
// Resolve allowed directories once outside the loop to avoid redundant filesystem calls
const resolvedAllowedDirs = allowedDirs
.map((dir) => this.safelyResolvePath(dir, context.environment.cwd))
.filter((resolvedDir): resolvedDir is string => resolvedDir !== null);
// Check each path
for (const { path: p, argName } of pathsToCheck) {
const resolvedPath = this.safelyResolvePath(p, context.environment.cwd);
@@ -57,15 +62,52 @@ export class AllowedPathChecker implements InProcessChecker {
};
}
const isAllowed = allowedDirs.some((dir) => {
// Also resolve allowed directories to handle symlinks
const resolvedDir = this.safelyResolvePath(
dir,
context.environment.cwd,
);
if (!resolvedDir) return false;
return this.isPathAllowed(resolvedPath, resolvedDir);
});
// Check for blocked segments case-insensitively
let hasBlockedSegment = false;
let isVscodePath = false;
for (const resolvedDir of resolvedAllowedDirs) {
if (!this.isPathAllowed(resolvedPath, resolvedDir)) continue;
const relative = path.relative(resolvedDir, resolvedPath);
const segments = relative.split(path.sep);
for (const segment of segments) {
const clean = trimTrailingSpacesAndDots(
segment.split(':')[0],
).toLowerCase();
if (
clean === '.git' ||
clean === '.env' ||
clean === 'node_modules'
) {
hasBlockedSegment = true;
}
if (clean === '.vscode') {
isVscodePath = true;
}
}
}
if (hasBlockedSegment) {
return {
decision: SafetyCheckDecision.DENY,
reason: `Access to sensitive path "${p}" in argument "${argName}" is blocked.`,
};
}
if (isVscodePath) {
return {
decision: SafetyCheckDecision.ASK_USER,
reason: `Modifying .vscode configuration files requires explicit user confirmation.`,
};
}
let isAllowed = false;
for (const resolvedDir of resolvedAllowedDirs) {
if (this.isPathAllowed(resolvedPath, resolvedDir)) {
isAllowed = true;
break;
}
}
if (!isAllowed) {
return {
@@ -84,14 +126,15 @@ export class AllowedPathChecker implements InProcessChecker {
// Walk up the directory tree until we find a path that exists
let current = resolved;
// Stop at root (dirname(root) === root on many systems, or it becomes empty/'.' depending on implementation)
while (current && current !== path.dirname(current)) {
if (fs.existsSync(current)) {
const canonical = fs.realpathSync(current);
try {
const canonical = resolveToRealPath(current);
// Re-construct the full path from this canonical base
const relative = path.relative(current, resolved);
// path.join handles empty relative paths correctly (returns canonical)
return path.join(canonical, relative);
} catch {
// Path does not exist, continue walking up
}
current = path.dirname(current);
}
@@ -156,3 +199,15 @@ export class AllowedPathChecker implements InProcessChecker {
return paths;
}
}
/**
* Trims trailing spaces and dots from a string without using regular expressions
* to completely eliminate any potential ReDoS (Regular Expression Denial of Service) risk.
*/
function trimTrailingSpacesAndDots(str: string): string {
let end = str.length - 1;
while (end >= 0 && (str[end] === ' ' || str[end] === '.')) {
end--;
}
return str.slice(0, end + 1);
}
packages/core/src/tools/read-many-files.test.ts
+2 -5
View File
@@ -398,7 +398,7 @@ describe('ReadManyFilesTool', () => {
});
it('should NOT use default excludes if useDefaultExcludes is false', async () => {
createFile('node_modules/some-lib/index.js', 'lib code');
createFile('dist/some-lib/index.js', 'lib code');
createFile('src/app.js', 'app code');
const params = { include: ['**/*.js'], useDefaultExcludes: false };
const invocation = tool.build(params);
@@ -406,10 +406,7 @@ describe('ReadManyFilesTool', () => {
abortSignal: new AbortController().signal,
});
const content = result.llmContent as string[];
const expectedPath1 = path.join(
tempRootDir,
'node_modules/some-lib/index.js',
);
const expectedPath1 = path.join(tempRootDir, 'dist/some-lib/index.js');
const expectedPath2 = path.join(tempRootDir, 'src/app.js');
expect(
content.some((c) =>
packages/core/src/utils/workspaceContext.test.ts
+46
View File
@@ -492,4 +492,50 @@ describe('WorkspaceContext with optional directories', () => {
expect(directories).toEqual([cwd, existingDir1]);
expect(debugLogger.warn).not.toHaveBeenCalled();
});
describe('Security Regression: Case-Insensitive Sensitive Path Blocklist', () => {
it('should reject sensitive paths like .git, .env, and node_modules case-insensitively, including Windows trailing character and NTFS ADS bypasses', () => {
const workspaceContext = new WorkspaceContext(cwd);
const sensitivePaths = [
path.join(cwd, '.git', 'config'),
path.join(cwd, '.GIT', 'config'),
path.join(cwd, '.Git', 'config'),
path.join(cwd, '.env'),
path.join(cwd, '.Env'),
path.join(cwd, '.ENV'),
path.join(cwd, 'node_modules', 'package', 'index.js'),
path.join(cwd, 'NODE_MODULES', 'package', 'index.js'),
// Windows trailing character bypasses
path.join(cwd, '.git ', 'config'),
path.join(cwd, '.git.', 'config'),
path.join(cwd, '.env ', 'config'),
path.join(cwd, '.env.', 'config'),
path.join(cwd, 'node_modules ', 'package', 'index.js'),
// NTFS Alternate Data Stream bypasses
path.join(cwd, '.git::$DATA', 'config'),
path.join(cwd, '.env::$DATA'),
path.join(cwd, 'node_modules::$DATA', 'package', 'index.js'),
];
for (const p of sensitivePaths) {
expect(workspaceContext.isPathWithinWorkspace(p)).toBe(false);
}
});
it('should allow standard non-sensitive paths', () => {
const workspaceContext = new WorkspaceContext(cwd);
const safePaths = [
path.join(cwd, 'src', 'index.ts'),
path.join(cwd, '.gitignore'),
path.join(cwd, '.env.example'),
path.join(cwd, 'package.json'),
];
for (const p of safePaths) {
expect(workspaceContext.isPathWithinWorkspace(p)).toBe(true);
}
});
});
});
packages/core/src/utils/workspaceContext.ts
+26
View File
@@ -184,6 +184,20 @@ export class WorkspaceContext {
for (const dir of this.directories) {
if (this.isPathWithinRoot(fullyResolvedPath, dir)) {
// Check for blocked segments case-insensitively
const relative = path.relative(dir, fullyResolvedPath);
const segments = relative.split(path.sep);
const hasBlockedSegment = segments.some((segment) => {
const clean = trimTrailingSpacesAndDots(
segment.split(':')[0],
).toLowerCase();
return (
clean === '.git' || clean === '.env' || clean === 'node_modules'
);
});
if (hasBlockedSegment) {
return false;
}
return true;
}
}
@@ -248,3 +262,15 @@ export class WorkspaceContext {
);
}
}
/**
* Trims trailing spaces and dots from a string without using regular expressions
* to completely eliminate any potential ReDoS (Regular Expression Denial of Service) risk.
*/
function trimTrailingSpacesAndDots(str: string): string {
let end = str.length - 1;
while (end >= 0 && (str[end] === ' ' || str[end] === '.')) {
end--;
}
return str.slice(0, end + 1);
}