mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-09-18 10:48:22 +08:00
fix(core): ensure consistent symlink evaluation in ignore path handling (#28915)
This commit is contained in:
@@ -0,0 +1,167 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import * as fs from 'node:fs/promises';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { FileDiscoveryService } from './fileDiscoveryService.js';
|
||||
import { GEMINI_IGNORE_FILE_NAME } from '../config/constants.js';
|
||||
|
||||
describe('FileDiscoveryService - Symlink Ignore Handling', () => {
|
||||
let testRootDir: string;
|
||||
let projectRoot: string;
|
||||
|
||||
async function createTestFile(filePath: string, content = '') {
|
||||
const fullPath = path.join(projectRoot, filePath);
|
||||
await fs.mkdir(path.dirname(fullPath), { recursive: true });
|
||||
await fs.writeFile(fullPath, content);
|
||||
return fullPath;
|
||||
}
|
||||
|
||||
async function createSymlink(
|
||||
targetRelativePath: string,
|
||||
linkRelativePath: string,
|
||||
) {
|
||||
const targetPath = path.join(projectRoot, targetRelativePath);
|
||||
const linkPath = path.join(projectRoot, linkRelativePath);
|
||||
await fs.mkdir(path.dirname(linkPath), { recursive: true });
|
||||
await fs.symlink(targetPath, linkPath);
|
||||
return linkPath;
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
testRootDir = await fs.mkdtemp(
|
||||
path.join(os.tmpdir(), 'file-discovery-symlink-test-'),
|
||||
);
|
||||
try {
|
||||
testRootDir = await fs.realpath(testRootDir);
|
||||
} catch {
|
||||
// Fallback
|
||||
}
|
||||
projectRoot = path.join(testRootDir, 'project');
|
||||
await fs.mkdir(projectRoot, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await fs.rm(testRootDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('should ignore an unignored symlink when pointing to an ignored target file (Scenario A)', async () => {
|
||||
await createTestFile(GEMINI_IGNORE_FILE_NAME, 'secret.txt\n');
|
||||
await createTestFile('secret.txt', 'sensitive content');
|
||||
await createSymlink('secret.txt', 'public_link.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
|
||||
expect(service.shouldIgnoreFile('secret.txt')).toBe(true);
|
||||
expect(service.shouldIgnoreFile('public_link.txt')).toBe(true);
|
||||
expect(
|
||||
service.shouldIgnoreFile(path.join(projectRoot, 'public_link.txt')),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should ignore a symlink whose name matches an ignore pattern even if target is not ignored (Scenario B)', async () => {
|
||||
await createTestFile(GEMINI_IGNORE_FILE_NAME, 'ignored_link.txt\n');
|
||||
await createTestFile('public.txt', 'public content');
|
||||
await createSymlink('public.txt', 'ignored_link.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
|
||||
expect(service.shouldIgnoreFile('public.txt')).toBe(false);
|
||||
expect(service.shouldIgnoreFile('ignored_link.txt')).toBe(true);
|
||||
expect(
|
||||
service.shouldIgnoreFile(path.join(projectRoot, 'ignored_link.txt')),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle broken symlinks gracefully without throwing unhandled exceptions (Scenario C)', async () => {
|
||||
await createTestFile(GEMINI_IGNORE_FILE_NAME, 'ignored_missing.txt\n');
|
||||
// Create broken symlink pointing to non-existent target
|
||||
const linkPath = path.join(projectRoot, 'broken_link.txt');
|
||||
await fs.symlink(path.join(projectRoot, 'non_existent.txt'), linkPath);
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
|
||||
expect(() => service.shouldIgnoreFile('broken_link.txt')).not.toThrow();
|
||||
expect(service.shouldIgnoreFile('broken_link.txt')).toBe(false);
|
||||
});
|
||||
|
||||
it('should correctly filter a mixed list of symlinks and files with filterFilesWithReport', async () => {
|
||||
await createTestFile(
|
||||
GEMINI_IGNORE_FILE_NAME,
|
||||
'private.txt\nignored_link.txt\n',
|
||||
);
|
||||
await createTestFile('private.txt', 'private');
|
||||
await createTestFile('public.txt', 'public');
|
||||
await createSymlink('private.txt', 'link_to_private.txt');
|
||||
await createSymlink('public.txt', 'ignored_link.txt');
|
||||
await createSymlink('public.txt', 'valid_link.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
|
||||
const report = service.filterFilesWithReport([
|
||||
'public.txt',
|
||||
'private.txt',
|
||||
'link_to_private.txt',
|
||||
'ignored_link.txt',
|
||||
'valid_link.txt',
|
||||
]);
|
||||
|
||||
expect(report.filteredPaths).toEqual(['public.txt', 'valid_link.txt']);
|
||||
expect(report.ignoredCount).toBe(3);
|
||||
});
|
||||
|
||||
it('should respect isSymbolicLink option when passed explicitly (Scenario D)', async () => {
|
||||
await createTestFile(GEMINI_IGNORE_FILE_NAME, 'target.txt\n');
|
||||
await createTestFile('target.txt', 'target content');
|
||||
await createSymlink('target.txt', 'link.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
|
||||
// When isSymbolicLink is explicitly passed as true
|
||||
expect(service.shouldIgnoreFile('link.txt', { isSymbolicLink: true })).toBe(
|
||||
true,
|
||||
);
|
||||
|
||||
// When isSymbolicLink is explicitly false on an unignored literal path, skips symlink resolution
|
||||
expect(
|
||||
service.shouldIgnoreFile('link.txt', { isSymbolicLink: false }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('should correctly discover ignored symlink paths in getIgnoredPaths recursive walk (Scenario E)', async () => {
|
||||
await createTestFile(GEMINI_IGNORE_FILE_NAME, 'confidential.txt\n');
|
||||
await createTestFile('confidential.txt', 'confidential');
|
||||
await createTestFile('regular.txt', 'regular');
|
||||
await createSymlink('confidential.txt', 'link_to_confidential.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
const ignoredPaths = await service.getIgnoredPaths();
|
||||
|
||||
expect(ignoredPaths).toContain(path.join(projectRoot, 'confidential.txt'));
|
||||
expect(ignoredPaths).toContain(
|
||||
path.join(projectRoot, 'link_to_confidential.txt'),
|
||||
);
|
||||
expect(ignoredPaths).not.toContain(path.join(projectRoot, 'regular.txt'));
|
||||
});
|
||||
|
||||
it('should dynamically detect if a symlink target is a directory to match directory-only ignore patterns (Scenario F)', async () => {
|
||||
await createTestFile(GEMINI_IGNORE_FILE_NAME, 'ignored_dir/\n');
|
||||
|
||||
// Create a directory and a symlink pointing to it
|
||||
const targetDir = path.join(projectRoot, 'ignored_dir');
|
||||
await fs.mkdir(targetDir, { recursive: true });
|
||||
await createTestFile('ignored_dir/file.txt', 'content');
|
||||
await createSymlink('ignored_dir', 'link_to_dir');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
|
||||
// Even though we call shouldIgnoreFile (which passes isDirectory = false),
|
||||
// it should dynamically detect that the target is a directory and match 'ignored_dir/'
|
||||
expect(service.shouldIgnoreFile('link_to_dir')).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -16,6 +16,7 @@ import { isGitRepository } from '../utils/gitUtils.js';
|
||||
import { GEMINI_IGNORE_FILE_NAME } from '../config/constants.js';
|
||||
import { isNodeError } from '../utils/errors.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
import { resolveToRealPath } from '../utils/paths.js';
|
||||
import fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
|
||||
@@ -23,6 +24,7 @@ export interface FilterFilesOptions {
|
||||
respectGitIgnore?: boolean;
|
||||
respectGeminiIgnore?: boolean;
|
||||
customIgnoreFilePaths?: string[];
|
||||
isSymbolicLink?: boolean;
|
||||
}
|
||||
|
||||
export interface FilterReport {
|
||||
@@ -118,16 +120,20 @@ export class FileDiscoveryService {
|
||||
await Promise.all(
|
||||
dirEntries.map(async (entry) => {
|
||||
const fullPath = path.join(currentDir, entry.name);
|
||||
const entryOptions: FilterFilesOptions = {
|
||||
...options,
|
||||
isSymbolicLink: entry.isSymbolicLink(),
|
||||
};
|
||||
|
||||
if (entry.isDirectory()) {
|
||||
// Optimization: If a directory is ignored, its contents are not traversed.
|
||||
if (this.shouldIgnoreDirectory(fullPath, options)) {
|
||||
if (this.shouldIgnoreDirectory(fullPath, entryOptions)) {
|
||||
ignoredPaths.push(fullPath);
|
||||
} else {
|
||||
await walk(fullPath);
|
||||
}
|
||||
} else {
|
||||
if (this.shouldIgnoreFile(fullPath, options)) {
|
||||
if (this.shouldIgnoreFile(fullPath, entryOptions)) {
|
||||
ignoredPaths.push(fullPath);
|
||||
}
|
||||
}
|
||||
@@ -209,10 +215,7 @@ export class FileDiscoveryService {
|
||||
return this._shouldIgnore(dirPath, true, options);
|
||||
}
|
||||
|
||||
/**
|
||||
* Internal unified check for paths.
|
||||
*/
|
||||
private _shouldIgnore(
|
||||
private _checkIgnoreFilters(
|
||||
filePath: string,
|
||||
isDirectory: boolean,
|
||||
options: FilterFilesOptions = {},
|
||||
@@ -247,6 +250,49 @@ export class FileDiscoveryService {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Internal unified check for paths.
|
||||
*/
|
||||
private _shouldIgnore(
|
||||
filePath: string,
|
||||
isDirectory: boolean,
|
||||
options: FilterFilesOptions = {},
|
||||
): boolean {
|
||||
if (this._checkIgnoreFilters(filePath, isDirectory, options)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
try {
|
||||
const absolutePath = path.isAbsolute(filePath)
|
||||
? filePath
|
||||
: path.resolve(this.projectRoot, filePath);
|
||||
|
||||
const isSymlink =
|
||||
options.isSymbolicLink ??
|
||||
fs
|
||||
.lstatSync(absolutePath, { throwIfNoEntry: false })
|
||||
?.isSymbolicLink() ??
|
||||
false;
|
||||
|
||||
if (isSymlink) {
|
||||
const realPath = resolveToRealPath(absolutePath);
|
||||
let targetIsDir = isDirectory;
|
||||
try {
|
||||
targetIsDir = fs.statSync(realPath).isDirectory();
|
||||
} catch {
|
||||
// Fallback to original isDirectory status if target is inaccessible
|
||||
}
|
||||
if (this._checkIgnoreFilters(realPath, targetIsDir, options)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Gracefully handle resolution errors or inaccessible paths
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the list of ignore files being used (e.g. .geminiignore) excluding .gitignore.
|
||||
*/
|
||||
|
||||
@@ -289,6 +289,10 @@ export class ReadFileTool extends BaseDeclarativeTool<
|
||||
|
||||
const fileFilteringOptions = this.config.getFileFilteringOptions();
|
||||
if (
|
||||
this.fileDiscoveryService.shouldIgnoreFile(
|
||||
sanitizedPath,
|
||||
fileFilteringOptions,
|
||||
) ||
|
||||
this.fileDiscoveryService.shouldIgnoreFile(
|
||||
resolvedPath,
|
||||
fileFilteringOptions,
|
||||
|
||||
@@ -15,6 +15,7 @@ import { ToolErrorType } from '../tools/tool-error.js';
|
||||
import { BINARY_EXTENSIONS } from './ignorePatterns.js';
|
||||
import { createRequire as createModuleRequire } from 'node:module';
|
||||
import { debugLogger } from './debugLogger.js';
|
||||
import { resolveToRealPath } from './paths.js';
|
||||
|
||||
import {
|
||||
DEFAULT_MAX_LINES_TEXT_FILE,
|
||||
@@ -269,6 +270,21 @@ function getSupportedAudioMimeTypeForFile(
|
||||
return extensionMimeType;
|
||||
}
|
||||
|
||||
export function canonicalizeMacosPath(p: string): string {
|
||||
if (process.platform === 'darwin') {
|
||||
if (p === '/var' || p.startsWith('/var/')) {
|
||||
return '/private' + p;
|
||||
}
|
||||
if (p === '/tmp' || p.startsWith('/tmp/')) {
|
||||
return '/private' + p;
|
||||
}
|
||||
if (p === '/etc' || p.startsWith('/etc/')) {
|
||||
return '/private' + p;
|
||||
}
|
||||
}
|
||||
return p;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a path is within a given root directory.
|
||||
* @param pathToCheck The absolute path to check.
|
||||
@@ -279,8 +295,12 @@ export function isWithinRoot(
|
||||
pathToCheck: string,
|
||||
rootDirectory: string,
|
||||
): boolean {
|
||||
const normalizedPathToCheck = path.resolve(pathToCheck);
|
||||
const normalizedRootDirectory = path.resolve(rootDirectory);
|
||||
const normalizedPathToCheck = canonicalizeMacosPath(
|
||||
path.resolve(pathToCheck),
|
||||
);
|
||||
const normalizedRootDirectory = canonicalizeMacosPath(
|
||||
path.resolve(rootDirectory),
|
||||
);
|
||||
|
||||
// Ensure the rootDirectory path ends with a separator for correct startsWith comparison,
|
||||
// unless it's the root path itself (e.g., '/' or 'C:\').
|
||||
@@ -290,10 +310,33 @@ export function isWithinRoot(
|
||||
? normalizedRootDirectory
|
||||
: normalizedRootDirectory + path.sep;
|
||||
|
||||
return (
|
||||
if (
|
||||
normalizedPathToCheck === normalizedRootDirectory ||
|
||||
normalizedPathToCheck.startsWith(rootWithSeparator)
|
||||
);
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Cross-platform check for macOS /private symlink aliases
|
||||
if (process.platform === 'darwin') {
|
||||
try {
|
||||
const realPathToCheck = resolveToRealPath(normalizedPathToCheck);
|
||||
const realRootDirectory = resolveToRealPath(normalizedRootDirectory);
|
||||
const realRootWithSeparator =
|
||||
realRootDirectory === path.sep || realRootDirectory.endsWith(path.sep)
|
||||
? realRootDirectory
|
||||
: realRootDirectory + path.sep;
|
||||
|
||||
return (
|
||||
realPathToCheck === realRootDirectory ||
|
||||
realPathToCheck.startsWith(realRootWithSeparator)
|
||||
);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -5,7 +5,8 @@
|
||||
*/
|
||||
|
||||
import * as path from 'node:path';
|
||||
import { isWithinRoot } from './fileUtils.js';
|
||||
import { isWithinRoot, canonicalizeMacosPath } from './fileUtils.js';
|
||||
import { resolveToRealPath } from './paths.js';
|
||||
|
||||
/**
|
||||
* Normalizes a file path to be relative to the project root and formatted for the 'ignore' library.
|
||||
@@ -28,7 +29,25 @@ export function getNormalizedRelativePath(
|
||||
return null;
|
||||
}
|
||||
|
||||
const relativePath = path.relative(projectRoot, absoluteFilePath);
|
||||
const canonicalRoot = canonicalizeMacosPath(projectRoot);
|
||||
const canonicalAbs = canonicalizeMacosPath(absoluteFilePath);
|
||||
|
||||
let relativePath = path.relative(canonicalRoot, canonicalAbs);
|
||||
|
||||
// Handle cross-platform root prefix discrepancies (e.g., macOS /var vs /private/var)
|
||||
if (process.platform === 'darwin' && relativePath.startsWith('..')) {
|
||||
try {
|
||||
const crossPlatformRel = path.relative(
|
||||
resolveToRealPath(projectRoot),
|
||||
resolveToRealPath(absoluteFilePath),
|
||||
);
|
||||
if (!crossPlatformRel.startsWith('..')) {
|
||||
relativePath = crossPlatformRel;
|
||||
}
|
||||
} catch {
|
||||
// Fallback to original relativePath
|
||||
}
|
||||
}
|
||||
|
||||
// Convert Windows backslashes to forward slashes for the 'ignore' library
|
||||
let normalized = relativePath.replace(/\\/g, '/');
|
||||
|
||||
@@ -18,6 +18,8 @@ function runPatchCreateComment(args, env = {}) {
|
||||
);
|
||||
const fullEnv = {
|
||||
...process.env,
|
||||
LC_ALL: 'en_US.UTF-8',
|
||||
LANG: 'en_US.UTF-8',
|
||||
...env,
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user