mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-11 03:38:15 +00:00
added new GitIgnoreController to handle nested .gitignore files closes #7921
This commit is contained in:
John Evans
parent
7c4635fcc6
commit
d74a9b1cfe
9 changed files with 872 additions and 90 deletions
122
src/core/ignore/BaseIgnoreController.ts
Normal file
122
src/core/ignore/BaseIgnoreController.ts
Normal file
|
|
@ -0,0 +1,122 @@
|
|||
import path from "path"
|
||||
import fsSync from "fs"
|
||||
import ignore, { Ignore } from "ignore"
|
||||
import * as vscode from "vscode"
|
||||
|
||||
/**
|
||||
* Base class for ignore controllers that provides common functionality
|
||||
* for handling ignore patterns and file validation.
|
||||
*/
|
||||
export abstract class BaseIgnoreController {
|
||||
protected cwd: string
|
||||
protected ignoreInstance: Ignore
|
||||
protected disposables: vscode.Disposable[] = []
|
||||
|
||||
constructor(cwd: string) {
|
||||
this.cwd = cwd
|
||||
this.ignoreInstance = ignore()
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize the controller - must be implemented by subclasses
|
||||
*/
|
||||
abstract initialize(): Promise<void>
|
||||
|
||||
/**
|
||||
* Check if a file should be accessible (not ignored by patterns)
|
||||
* Automatically resolves symlinks
|
||||
* @param filePath - Path to check (relative to cwd)
|
||||
* @returns true if file is accessible, false if ignored
|
||||
*/
|
||||
validateAccess(filePath: string): boolean {
|
||||
// Allow subclasses to override the "no patterns" check
|
||||
if (!this.hasPatterns()) {
|
||||
return true
|
||||
}
|
||||
|
||||
try {
|
||||
const absolutePath = path.resolve(this.cwd, filePath)
|
||||
|
||||
// Follow symlinks to get the real path
|
||||
let realPath: string
|
||||
try {
|
||||
realPath = fsSync.realpathSync(absolutePath)
|
||||
} catch {
|
||||
// If realpath fails (file doesn't exist, broken symlink, etc.),
|
||||
// use the original path
|
||||
realPath = absolutePath
|
||||
}
|
||||
|
||||
// Convert real path to relative for ignore checking
|
||||
const relativePath = path.relative(this.cwd, realPath).toPosix()
|
||||
|
||||
// Check if the real path is ignored
|
||||
return !this.ignoreInstance.ignores(relativePath)
|
||||
} catch (error) {
|
||||
// Allow access to files outside cwd or on errors (backward compatibility)
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Filter an array of paths, removing those that should be ignored
|
||||
* @param paths - Array of paths to filter (relative to cwd)
|
||||
* @returns Array of allowed paths
|
||||
*/
|
||||
filterPaths(paths: string[]): string[] {
|
||||
try {
|
||||
return paths
|
||||
.map((p) => ({
|
||||
path: p,
|
||||
allowed: this.validateAccess(p),
|
||||
}))
|
||||
.filter((x) => x.allowed)
|
||||
.map((x) => x.path)
|
||||
} catch (error) {
|
||||
console.error("Error filtering paths:", error)
|
||||
return [] // Fail closed for security
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean up resources when the controller is no longer needed
|
||||
*/
|
||||
dispose(): void {
|
||||
this.disposables.forEach((d) => d.dispose())
|
||||
this.disposables = []
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the controller has any patterns loaded
|
||||
* Must be implemented by subclasses
|
||||
*/
|
||||
protected abstract hasPatterns(): boolean
|
||||
|
||||
/**
|
||||
* Set up file watchers with debouncing to avoid rapid reloads
|
||||
* @param pattern - VSCode RelativePattern for the files to watch
|
||||
* @param reloadCallback - Function to call when files change
|
||||
*/
|
||||
protected setupFileWatcher(pattern: vscode.RelativePattern, reloadCallback: () => void): void {
|
||||
const fileWatcher = vscode.workspace.createFileSystemWatcher(pattern)
|
||||
|
||||
// Debounce rapid changes
|
||||
let reloadTimeout: NodeJS.Timeout | undefined
|
||||
const debouncedReload = () => {
|
||||
if (reloadTimeout) {
|
||||
clearTimeout(reloadTimeout)
|
||||
}
|
||||
reloadTimeout = setTimeout(reloadCallback, 100)
|
||||
}
|
||||
|
||||
// Watch for changes, creation, and deletion
|
||||
this.disposables.push(
|
||||
fileWatcher.onDidChange(debouncedReload),
|
||||
fileWatcher.onDidCreate(debouncedReload),
|
||||
fileWatcher.onDidDelete(debouncedReload),
|
||||
)
|
||||
|
||||
// Add fileWatcher itself to disposables
|
||||
this.disposables.push(fileWatcher)
|
||||
}
|
||||
}
|
||||
203
src/core/ignore/GitIgnoreController.ts
Normal file
203
src/core/ignore/GitIgnoreController.ts
Normal file
|
|
@ -0,0 +1,203 @@
|
|||
import path from "path"
|
||||
import { fileExistsAtPath } from "../../utils/fs"
|
||||
import fs from "fs/promises"
|
||||
import ignore from "ignore"
|
||||
import * as vscode from "vscode"
|
||||
import { BaseIgnoreController } from "./BaseIgnoreController"
|
||||
|
||||
/**
|
||||
* Controls file access by enforcing nested .gitignore patterns.
|
||||
* Handles multiple .gitignore files throughout the directory tree, unlike ripgrep which only honors top-level .gitignore.
|
||||
* Designed to be instantiated once and passed to file manipulation services.
|
||||
* Uses the 'ignore' library to support standard .gitignore syntax.
|
||||
*/
|
||||
export class GitIgnoreController extends BaseIgnoreController {
|
||||
private gitignoreFiles: string[] = []
|
||||
private gitignoreContents: Map<string, string> = new Map()
|
||||
|
||||
constructor(cwd: string) {
|
||||
super(cwd)
|
||||
this.gitignoreFiles = []
|
||||
this.gitignoreContents = new Map()
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize the controller by discovering and loading all .gitignore files
|
||||
* Must be called after construction and before using the controller
|
||||
*/
|
||||
async initialize(): Promise<void> {
|
||||
await this.discoverAndLoadGitignoreFiles()
|
||||
this.setupGitIgnoreWatchers()
|
||||
}
|
||||
|
||||
/**
|
||||
* Discover and load .gitignore files (root + common subdirectories)
|
||||
*/
|
||||
private async discoverAndLoadGitignoreFiles(): Promise<void> {
|
||||
try {
|
||||
// Reset state
|
||||
this.ignoreInstance = ignore()
|
||||
this.gitignoreFiles = []
|
||||
this.gitignoreContents.clear()
|
||||
|
||||
// Check for common .gitignore file locations (manually defined for simplicity)
|
||||
const commonGitignorePaths = [
|
||||
path.join(this.cwd, ".gitignore"), // Root
|
||||
path.join(this.cwd, "src", ".gitignore"), // src/
|
||||
path.join(this.cwd, "lib", ".gitignore"), // lib/
|
||||
path.join(this.cwd, "test", ".gitignore"), // test/
|
||||
path.join(this.cwd, "tests", ".gitignore"), // tests/
|
||||
]
|
||||
|
||||
// Check each location and load if it exists
|
||||
for (const gitignorePath of commonGitignorePaths) {
|
||||
if (await fileExistsAtPath(gitignorePath)) {
|
||||
this.gitignoreFiles.push(gitignorePath)
|
||||
await this.loadGitignoreFile(gitignorePath)
|
||||
}
|
||||
}
|
||||
|
||||
// Always ignore .gitignore files themselves
|
||||
this.ignoreInstance.add(".gitignore")
|
||||
} catch (error) {
|
||||
console.error("Error discovering .gitignore files:", error)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recursively find all .gitignore files in the directory tree
|
||||
*/
|
||||
private async findGitignoreFilesRecursively(dirPath: string): Promise<void> {
|
||||
try {
|
||||
// Skip the root directory since we already checked it in discoverAndLoadGitignoreFiles
|
||||
if (dirPath === this.cwd) {
|
||||
// Get all subdirectories
|
||||
const entries = await fs.readdir(dirPath, { withFileTypes: true })
|
||||
const subdirs = entries
|
||||
.filter((entry) => entry.isDirectory() && !entry.name.startsWith("."))
|
||||
.map((entry) => path.join(dirPath, entry.name))
|
||||
|
||||
// Recursively search subdirectories
|
||||
for (const subdir of subdirs) {
|
||||
await this.findGitignoreFilesRecursively(subdir)
|
||||
}
|
||||
} else {
|
||||
// For subdirectories, check for .gitignore and continue recursively
|
||||
const gitignorePath = path.join(dirPath, ".gitignore")
|
||||
|
||||
// Check if .gitignore exists in current directory
|
||||
if (await fileExistsAtPath(gitignorePath)) {
|
||||
this.gitignoreFiles.push(gitignorePath)
|
||||
}
|
||||
|
||||
// Get all subdirectories
|
||||
const entries = await fs.readdir(dirPath, { withFileTypes: true })
|
||||
const subdirs = entries
|
||||
.filter((entry) => entry.isDirectory() && !entry.name.startsWith("."))
|
||||
.map((entry) => path.join(dirPath, entry.name))
|
||||
|
||||
// Recursively search subdirectories
|
||||
for (const subdir of subdirs) {
|
||||
await this.findGitignoreFilesRecursively(subdir)
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
// Skip directories we can't read
|
||||
console.debug(`Could not read directory ${dirPath}:`, error)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Load content from a specific .gitignore file
|
||||
*/
|
||||
private async loadGitignoreFile(gitignoreFile: string): Promise<void> {
|
||||
try {
|
||||
const content = await fs.readFile(gitignoreFile, "utf8")
|
||||
this.gitignoreContents.set(gitignoreFile, content)
|
||||
|
||||
// Add patterns to ignore instance with proper context
|
||||
// For nested .gitignore files, we need to adjust patterns relative to the workspace root
|
||||
const relativeDir = path.relative(this.cwd, path.dirname(gitignoreFile))
|
||||
|
||||
if (relativeDir) {
|
||||
// For nested .gitignore files, prefix patterns with the relative directory
|
||||
const lines = content.split("\n").filter((line) => line.trim() && !line.startsWith("#"))
|
||||
const adjustedPatterns = lines.map((pattern) => {
|
||||
const trimmed = pattern.trim()
|
||||
if (trimmed.startsWith("/")) {
|
||||
// Absolute patterns (starting with /) are relative to the .gitignore location
|
||||
return path.posix.join(relativeDir, trimmed.slice(1))
|
||||
} else if (trimmed.startsWith("!")) {
|
||||
// Negation patterns
|
||||
const negatedPattern = trimmed.slice(1)
|
||||
if (negatedPattern.startsWith("/")) {
|
||||
return "!" + path.posix.join(relativeDir, negatedPattern.slice(1))
|
||||
} else {
|
||||
return "!" + path.posix.join(relativeDir, "**", negatedPattern)
|
||||
}
|
||||
} else {
|
||||
// Relative patterns apply to the directory and all subdirectories
|
||||
return path.posix.join(relativeDir, "**", trimmed)
|
||||
}
|
||||
})
|
||||
|
||||
this.ignoreInstance.add(adjustedPatterns)
|
||||
} else {
|
||||
// Root .gitignore file - add patterns as-is (like RooIgnoreController)
|
||||
this.ignoreInstance.add(content)
|
||||
}
|
||||
} catch (error) {
|
||||
console.warn(`Could not read .gitignore at ${gitignoreFile}:`, error)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Set up file watchers for all .gitignore files in the workspace
|
||||
*/
|
||||
private setupGitIgnoreWatchers(): void {
|
||||
// Create a watcher for .gitignore files throughout the workspace
|
||||
const gitignorePattern = new vscode.RelativePattern(this.cwd, "**/.gitignore")
|
||||
this.setupFileWatcher(gitignorePattern, () => this.discoverAndLoadGitignoreFiles())
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the controller has any patterns loaded
|
||||
*/
|
||||
protected hasPatterns(): boolean {
|
||||
return this.gitignoreFiles.length > 0
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all discovered .gitignore file paths
|
||||
* @returns Array of absolute paths to .gitignore files
|
||||
*/
|
||||
getGitignoreFiles(): string[] {
|
||||
return [...this.gitignoreFiles]
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the content of a specific .gitignore file
|
||||
* @param gitignoreFile - Absolute path to the .gitignore file
|
||||
* @returns Content of the file or undefined if not found
|
||||
*/
|
||||
getGitignoreContent(gitignoreFile: string): string | undefined {
|
||||
return this.gitignoreContents.get(gitignoreFile)
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if any .gitignore files exist in the workspace
|
||||
* @returns true if at least one .gitignore file exists
|
||||
*/
|
||||
hasGitignoreFiles(): boolean {
|
||||
return this.gitignoreFiles.length > 0
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean up resources when the controller is no longer needed
|
||||
*/
|
||||
override dispose(): void {
|
||||
super.dispose()
|
||||
this.gitignoreContents.clear()
|
||||
this.gitignoreFiles = []
|
||||
}
|
||||
}
|
||||
|
|
@ -1,9 +1,9 @@
|
|||
import path from "path"
|
||||
import { fileExistsAtPath } from "../../utils/fs"
|
||||
import fs from "fs/promises"
|
||||
import fsSync from "fs"
|
||||
import ignore, { Ignore } from "ignore"
|
||||
import ignore from "ignore"
|
||||
import * as vscode from "vscode"
|
||||
import { BaseIgnoreController } from "./BaseIgnoreController"
|
||||
|
||||
export const LOCK_TEXT_SYMBOL = "\u{1F512}"
|
||||
|
||||
|
|
@ -12,18 +12,14 @@ export const LOCK_TEXT_SYMBOL = "\u{1F512}"
|
|||
* Designed to be instantiated once in Cline.ts and passed to file manipulation services.
|
||||
* Uses the 'ignore' library to support standard .gitignore syntax in .rooignore files.
|
||||
*/
|
||||
export class RooIgnoreController {
|
||||
private cwd: string
|
||||
private ignoreInstance: Ignore
|
||||
private disposables: vscode.Disposable[] = []
|
||||
export class RooIgnoreController extends BaseIgnoreController {
|
||||
rooIgnoreContent: string | undefined
|
||||
|
||||
constructor(cwd: string) {
|
||||
this.cwd = cwd
|
||||
this.ignoreInstance = ignore()
|
||||
super(cwd)
|
||||
this.rooIgnoreContent = undefined
|
||||
// Set up file watcher for .rooignore
|
||||
this.setupFileWatcher()
|
||||
this.setupRooIgnoreWatcher()
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -37,25 +33,9 @@ export class RooIgnoreController {
|
|||
/**
|
||||
* Set up the file watcher for .rooignore changes
|
||||
*/
|
||||
private setupFileWatcher(): void {
|
||||
private setupRooIgnoreWatcher(): void {
|
||||
const rooignorePattern = new vscode.RelativePattern(this.cwd, ".rooignore")
|
||||
const fileWatcher = vscode.workspace.createFileSystemWatcher(rooignorePattern)
|
||||
|
||||
// Watch for changes and updates
|
||||
this.disposables.push(
|
||||
fileWatcher.onDidChange(() => {
|
||||
this.loadRooIgnore()
|
||||
}),
|
||||
fileWatcher.onDidCreate(() => {
|
||||
this.loadRooIgnore()
|
||||
}),
|
||||
fileWatcher.onDidDelete(() => {
|
||||
this.loadRooIgnore()
|
||||
}),
|
||||
)
|
||||
|
||||
// Add fileWatcher itself to disposables
|
||||
this.disposables.push(fileWatcher)
|
||||
this.setupFileWatcher(rooignorePattern, () => this.loadRooIgnore())
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -81,38 +61,10 @@ export class RooIgnoreController {
|
|||
}
|
||||
|
||||
/**
|
||||
* Check if a file should be accessible to the LLM
|
||||
* Automatically resolves symlinks
|
||||
* @param filePath - Path to check (relative to cwd)
|
||||
* @returns true if file is accessible, false if ignored
|
||||
* Check if the controller has any patterns loaded
|
||||
*/
|
||||
validateAccess(filePath: string): boolean {
|
||||
// Always allow access if .rooignore does not exist
|
||||
if (!this.rooIgnoreContent) {
|
||||
return true
|
||||
}
|
||||
try {
|
||||
const absolutePath = path.resolve(this.cwd, filePath)
|
||||
|
||||
// Follow symlinks to get the real path
|
||||
let realPath: string
|
||||
try {
|
||||
realPath = fsSync.realpathSync(absolutePath)
|
||||
} catch {
|
||||
// If realpath fails (file doesn't exist, broken symlink, etc.),
|
||||
// use the original path
|
||||
realPath = absolutePath
|
||||
}
|
||||
|
||||
// Convert real path to relative for .rooignore checking
|
||||
const relativePath = path.relative(this.cwd, realPath).toPosix()
|
||||
|
||||
// Check if the real path is ignored
|
||||
return !this.ignoreInstance.ignores(relativePath)
|
||||
} catch (error) {
|
||||
// Allow access to files outside cwd or on errors (backward compatibility)
|
||||
return true
|
||||
}
|
||||
protected hasPatterns(): boolean {
|
||||
return this.rooIgnoreContent !== undefined
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -171,34 +123,6 @@ export class RooIgnoreController {
|
|||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Filter an array of paths, removing those that should be ignored
|
||||
* @param paths - Array of paths to filter (relative to cwd)
|
||||
* @returns Array of allowed paths
|
||||
*/
|
||||
filterPaths(paths: string[]): string[] {
|
||||
try {
|
||||
return paths
|
||||
.map((p) => ({
|
||||
path: p,
|
||||
allowed: this.validateAccess(p),
|
||||
}))
|
||||
.filter((x) => x.allowed)
|
||||
.map((x) => x.path)
|
||||
} catch (error) {
|
||||
console.error("Error filtering paths:", error)
|
||||
return [] // Fail closed for security
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean up resources when the controller is no longer needed
|
||||
*/
|
||||
dispose(): void {
|
||||
this.disposables.forEach((d) => d.dispose())
|
||||
this.disposables = []
|
||||
}
|
||||
|
||||
/**
|
||||
* Get formatted instructions about the .rooignore file for the LLM
|
||||
* @returns Formatted instructions or undefined if .rooignore doesn't exist
|
||||
|
|
|
|||
283
src/core/ignore/__tests__/GitIgnoreController.security.spec.ts
Normal file
283
src/core/ignore/__tests__/GitIgnoreController.security.spec.ts
Normal file
|
|
@ -0,0 +1,283 @@
|
|||
// npx vitest core/ignore/__tests__/GitIgnoreController.security.spec.ts
|
||||
|
||||
import type { Mock } from "vitest"
|
||||
|
||||
import { GitIgnoreController } from "../GitIgnoreController"
|
||||
import * as path from "path"
|
||||
import * as fs from "fs/promises"
|
||||
import * as fsSync from "fs"
|
||||
import { fileExistsAtPath } from "../../../utils/fs"
|
||||
|
||||
// Mock dependencies
|
||||
vi.mock("fs/promises")
|
||||
vi.mock("fs")
|
||||
vi.mock("../../../utils/fs")
|
||||
vi.mock("vscode", () => {
|
||||
const mockDisposable = { dispose: vi.fn() }
|
||||
|
||||
return {
|
||||
workspace: {
|
||||
createFileSystemWatcher: vi.fn(() => ({
|
||||
onDidCreate: vi.fn(() => mockDisposable),
|
||||
onDidChange: vi.fn(() => mockDisposable),
|
||||
onDidDelete: vi.fn(() => mockDisposable),
|
||||
dispose: vi.fn(),
|
||||
})),
|
||||
},
|
||||
RelativePattern: vi.fn().mockImplementation((base, pattern) => ({
|
||||
base,
|
||||
pattern,
|
||||
})),
|
||||
}
|
||||
})
|
||||
|
||||
describe("GitIgnoreController Security Tests", () => {
|
||||
const TEST_CWD = "/test/path"
|
||||
let controller: GitIgnoreController
|
||||
let mockFileExists: Mock<typeof fileExistsAtPath>
|
||||
let mockReadFile: Mock<typeof fs.readFile>
|
||||
|
||||
beforeEach(async () => {
|
||||
// Reset mocks
|
||||
vi.clearAllMocks()
|
||||
|
||||
// Setup mocks
|
||||
mockFileExists = fileExistsAtPath as Mock<typeof fileExistsAtPath>
|
||||
mockReadFile = fs.readFile as Mock<typeof fs.readFile>
|
||||
|
||||
// Setup fsSync mocks with default behavior (return path as-is, like regular files)
|
||||
const mockRealpathSync = vi.mocked(fsSync.realpathSync)
|
||||
mockRealpathSync.mockImplementation((filePath: any) => filePath.toString())
|
||||
|
||||
// By default, setup .gitignore to exist with some patterns
|
||||
mockFileExists.mockResolvedValue(true)
|
||||
mockReadFile.mockResolvedValue("node_modules/\n.git/\nsecrets/**\n*.log\nprivate/")
|
||||
|
||||
// Create and initialize controller
|
||||
controller = new GitIgnoreController(TEST_CWD)
|
||||
await controller.initialize()
|
||||
})
|
||||
|
||||
describe("Path traversal protection", () => {
|
||||
/**
|
||||
* Tests protection against path traversal attacks
|
||||
*/
|
||||
it("should handle path traversal attempts", () => {
|
||||
// Test simple path
|
||||
expect(controller.validateAccess("secrets/keys.json")).toBe(false)
|
||||
|
||||
// Attempt simple path traversal
|
||||
expect(controller.validateAccess("secrets/../secrets/keys.json")).toBe(false)
|
||||
|
||||
// More complex traversal
|
||||
expect(controller.validateAccess("public/../secrets/keys.json")).toBe(false)
|
||||
|
||||
// Deep traversal
|
||||
expect(controller.validateAccess("public/css/../../secrets/keys.json")).toBe(false)
|
||||
|
||||
// Traversal with normalized path
|
||||
expect(controller.validateAccess(path.normalize("public/../secrets/keys.json"))).toBe(false)
|
||||
|
||||
// Allowed files shouldn't be affected by traversal protection
|
||||
expect(controller.validateAccess("public/css/../../public/app.js")).toBe(true)
|
||||
})
|
||||
|
||||
/**
|
||||
* Tests absolute path handling
|
||||
*/
|
||||
it("should handle absolute paths correctly", () => {
|
||||
// Absolute path to ignored file within cwd
|
||||
const absolutePathToIgnored = path.join(TEST_CWD, "secrets/keys.json")
|
||||
expect(controller.validateAccess(absolutePathToIgnored)).toBe(false)
|
||||
|
||||
// Absolute path to allowed file within cwd
|
||||
const absolutePathToAllowed = path.join(TEST_CWD, "src/app.js")
|
||||
expect(controller.validateAccess(absolutePathToAllowed)).toBe(true)
|
||||
|
||||
// Absolute path outside cwd should be allowed
|
||||
expect(controller.validateAccess("/etc/hosts")).toBe(true)
|
||||
expect(controller.validateAccess("/var/log/system.log")).toBe(true)
|
||||
})
|
||||
|
||||
/**
|
||||
* Tests that paths outside cwd are allowed
|
||||
*/
|
||||
it("should allow paths outside the current working directory", () => {
|
||||
// Paths outside cwd should be allowed
|
||||
expect(controller.validateAccess("../outside-project/file.txt")).toBe(true)
|
||||
expect(controller.validateAccess("../../other-project/secrets/keys.json")).toBe(true)
|
||||
|
||||
// Edge case: path that would be ignored if inside cwd
|
||||
expect(controller.validateAccess("/other/path/secrets/keys.json")).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("Complex pattern handling", () => {
|
||||
/**
|
||||
* Tests combinations of paths and patterns
|
||||
*/
|
||||
it("should correctly apply complex patterns to various paths", async () => {
|
||||
// Setup complex patterns
|
||||
mockReadFile.mockResolvedValue(`
|
||||
# Node modules and logs
|
||||
node_modules/
|
||||
*.log
|
||||
|
||||
# Version control
|
||||
.git/
|
||||
.svn/
|
||||
|
||||
# Secrets and config
|
||||
config/secrets/**
|
||||
**/*secret*
|
||||
**/password*.*
|
||||
|
||||
# Build artifacts
|
||||
dist/
|
||||
build/
|
||||
|
||||
# Comments and empty lines should be ignored
|
||||
`)
|
||||
|
||||
// Reinitialize controller
|
||||
await controller.initialize()
|
||||
|
||||
// Test standard ignored paths
|
||||
expect(controller.validateAccess("node_modules/package.json")).toBe(false)
|
||||
expect(controller.validateAccess("app.log")).toBe(false)
|
||||
expect(controller.validateAccess(".git/config")).toBe(false)
|
||||
|
||||
// Test wildcards and double wildcards
|
||||
expect(controller.validateAccess("config/secrets/api-keys.json")).toBe(false)
|
||||
expect(controller.validateAccess("src/config/secret-keys.js")).toBe(false)
|
||||
expect(controller.validateAccess("lib/utils/password-manager.ts")).toBe(false)
|
||||
|
||||
// Test build artifacts
|
||||
expect(controller.validateAccess("dist/main.js")).toBe(false)
|
||||
expect(controller.validateAccess("build/index.html")).toBe(false)
|
||||
|
||||
// Test paths that should be allowed
|
||||
expect(controller.validateAccess("src/app.js")).toBe(true)
|
||||
expect(controller.validateAccess("README.md")).toBe(true)
|
||||
})
|
||||
|
||||
/**
|
||||
* Tests nested .gitignore files with different patterns
|
||||
*/
|
||||
it("should handle nested .gitignore files correctly", async () => {
|
||||
// Setup mocks for both root and nested .gitignore files
|
||||
mockFileExists.mockImplementation((filePath: string) => {
|
||||
return Promise.resolve(
|
||||
filePath === path.join(TEST_CWD, ".gitignore") ||
|
||||
filePath === path.join(TEST_CWD, "src", ".gitignore"),
|
||||
)
|
||||
})
|
||||
|
||||
// Mock different content for each file
|
||||
mockReadFile.mockImplementation((filePath: any) => {
|
||||
if (filePath.toString().endsWith("src/.gitignore")) {
|
||||
return Promise.resolve("*.tmp\n*.cache\n")
|
||||
}
|
||||
return Promise.resolve("node_modules/\n*.log\n")
|
||||
})
|
||||
|
||||
await controller.initialize()
|
||||
|
||||
// Verify patterns from both files are applied
|
||||
expect(controller.validateAccess("node_modules/package.json")).toBe(false) // Root .gitignore
|
||||
expect(controller.validateAccess("debug.log")).toBe(false) // Root .gitignore
|
||||
expect(controller.validateAccess("src/temp.tmp")).toBe(false) // Nested .gitignore
|
||||
expect(controller.validateAccess("src/data.cache")).toBe(false) // Nested .gitignore
|
||||
expect(controller.validateAccess("src/index.ts")).toBe(true) // Should be allowed
|
||||
})
|
||||
})
|
||||
|
||||
describe("filterPaths security", () => {
|
||||
/**
|
||||
* Tests filtering paths for security
|
||||
*/
|
||||
it("should correctly filter mixed paths", () => {
|
||||
const paths = [
|
||||
"src/app.js", // allowed
|
||||
"node_modules/package.json", // ignored
|
||||
"README.md", // allowed
|
||||
"secrets/keys.json", // ignored
|
||||
".git/config", // ignored
|
||||
"app.log", // ignored
|
||||
"test/test.js", // allowed
|
||||
]
|
||||
|
||||
const filtered = controller.filterPaths(paths)
|
||||
|
||||
// Should only contain allowed paths
|
||||
expect(filtered).toEqual(["src/app.js", "README.md", "test/test.js"])
|
||||
|
||||
// Length should match allowed files
|
||||
expect(filtered.length).toBe(3)
|
||||
})
|
||||
|
||||
/**
|
||||
* Tests error handling in filterPaths
|
||||
*/
|
||||
it("should fail closed (securely) when errors occur", () => {
|
||||
// Mock validateAccess to throw error
|
||||
vi.spyOn(controller, "validateAccess").mockImplementation(() => {
|
||||
throw new Error("Test error")
|
||||
})
|
||||
|
||||
// Spy on console.error
|
||||
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {})
|
||||
|
||||
// Even with mix of allowed/ignored paths, should return empty array on error
|
||||
const filtered = controller.filterPaths(["src/app.js", "node_modules/package.json"])
|
||||
|
||||
// Should fail closed (return empty array)
|
||||
expect(filtered).toEqual([])
|
||||
|
||||
// Should log error
|
||||
expect(consoleSpy).toHaveBeenCalledWith("Error filtering paths:", expect.any(Error))
|
||||
|
||||
// Clean up
|
||||
consoleSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
describe("Edge cases", () => {
|
||||
/**
|
||||
* Tests unusual file paths
|
||||
*/
|
||||
it("should handle unusual file paths", () => {
|
||||
expect(controller.validateAccess(".node_modules_temp/file.js")).toBe(true) // Doesn't match node_modules/
|
||||
expect(controller.validateAccess("node_modules.bak/file.js")).toBe(true) // Doesn't match node_modules/
|
||||
expect(controller.validateAccess("not_secrets/file.json")).toBe(true) // Doesn't match secrets
|
||||
|
||||
// Files with dots
|
||||
expect(controller.validateAccess("src/file.with.multiple.dots.js")).toBe(true)
|
||||
|
||||
// Files with no extension
|
||||
expect(controller.validateAccess("bin/executable")).toBe(true)
|
||||
|
||||
// Hidden files
|
||||
expect(controller.validateAccess(".env")).toBe(true) // Not ignored by default
|
||||
})
|
||||
|
||||
/**
|
||||
* Tests empty and malformed .gitignore files
|
||||
*/
|
||||
it("should handle empty and malformed .gitignore files", async () => {
|
||||
// Test empty .gitignore
|
||||
mockReadFile.mockResolvedValue("")
|
||||
await controller.initialize()
|
||||
|
||||
// Should allow all files when .gitignore is empty
|
||||
expect(controller.validateAccess("any/file.txt")).toBe(true)
|
||||
|
||||
// Test .gitignore with only comments and whitespace
|
||||
mockReadFile.mockResolvedValue("# This is a comment\n\n \n# Another comment\n")
|
||||
await controller.initialize()
|
||||
|
||||
// Should allow all files when .gitignore has no patterns
|
||||
expect(controller.validateAccess("any/file.txt")).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
238
src/core/ignore/__tests__/GitIgnoreController.spec.ts
Normal file
238
src/core/ignore/__tests__/GitIgnoreController.spec.ts
Normal file
|
|
@ -0,0 +1,238 @@
|
|||
// npx vitest core/ignore/__tests__/GitIgnoreController.spec.ts
|
||||
|
||||
import type { Mock } from "vitest"
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"
|
||||
import path from "path"
|
||||
import * as vscode from "vscode"
|
||||
import { GitIgnoreController } from "../GitIgnoreController"
|
||||
import { fileExistsAtPath } from "../../../utils/fs"
|
||||
import * as fs from "fs/promises"
|
||||
import * as fsSync from "fs"
|
||||
|
||||
// Mock dependencies
|
||||
vi.mock("fs/promises")
|
||||
vi.mock("fs")
|
||||
vi.mock("../../../utils/fs")
|
||||
|
||||
// Mock vscode
|
||||
vi.mock("vscode", () => {
|
||||
const mockDisposable = { dispose: vi.fn() }
|
||||
|
||||
return {
|
||||
workspace: {
|
||||
createFileSystemWatcher: vi.fn(() => ({
|
||||
onDidCreate: vi.fn(() => mockDisposable),
|
||||
onDidChange: vi.fn(() => mockDisposable),
|
||||
onDidDelete: vi.fn(() => mockDisposable),
|
||||
dispose: vi.fn(),
|
||||
})),
|
||||
},
|
||||
RelativePattern: vi.fn().mockImplementation((base, pattern) => ({
|
||||
base,
|
||||
pattern,
|
||||
})),
|
||||
}
|
||||
})
|
||||
|
||||
describe("GitIgnoreController", () => {
|
||||
const TEST_CWD = "/test/path"
|
||||
let controller: GitIgnoreController
|
||||
let mockFileExists: Mock<typeof fileExistsAtPath>
|
||||
let mockReadFile: Mock<typeof fs.readFile>
|
||||
let mockWatcher: any
|
||||
|
||||
beforeEach(() => {
|
||||
// Reset mocks
|
||||
vi.clearAllMocks()
|
||||
|
||||
// Setup mock file watcher
|
||||
mockWatcher = {
|
||||
onDidCreate: vi.fn().mockReturnValue({ dispose: vi.fn() }),
|
||||
onDidChange: vi.fn().mockReturnValue({ dispose: vi.fn() }),
|
||||
onDidDelete: vi.fn().mockReturnValue({ dispose: vi.fn() }),
|
||||
dispose: vi.fn(),
|
||||
}
|
||||
|
||||
// @ts-expect-error - Mocking
|
||||
vscode.workspace.createFileSystemWatcher.mockReturnValue(mockWatcher)
|
||||
|
||||
// Setup fs mocks (exactly like RooIgnoreController)
|
||||
mockFileExists = fileExistsAtPath as Mock<typeof fileExistsAtPath>
|
||||
mockReadFile = fs.readFile as Mock<typeof fs.readFile>
|
||||
|
||||
// Setup fsSync mocks with default behavior (return path as-is, like regular files)
|
||||
const mockRealpathSync = vi.mocked(fsSync.realpathSync)
|
||||
mockRealpathSync.mockImplementation((filePath: any) => filePath.toString())
|
||||
|
||||
// Create controller
|
||||
controller = new GitIgnoreController(TEST_CWD)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
if (controller) {
|
||||
controller.dispose()
|
||||
}
|
||||
})
|
||||
|
||||
describe("initialization", () => {
|
||||
it("should initialize without .gitignore files", async () => {
|
||||
// Setup mocks to simulate no .gitignore files
|
||||
mockFileExists.mockResolvedValue(false)
|
||||
|
||||
await controller.initialize()
|
||||
|
||||
// Should allow all access when no .gitignore files exist
|
||||
expect(controller.validateAccess("any/file.ts")).toBe(true)
|
||||
expect(controller.hasGitignoreFiles()).toBe(false)
|
||||
})
|
||||
|
||||
it("should discover and load root .gitignore file", async () => {
|
||||
// Setup mocks to simulate root .gitignore file
|
||||
mockFileExists.mockImplementation((filePath: string) => {
|
||||
return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore"))
|
||||
})
|
||||
mockReadFile.mockResolvedValue("node_modules/\n*.log\n")
|
||||
|
||||
await controller.initialize()
|
||||
|
||||
// Verify file was discovered
|
||||
expect(controller.hasGitignoreFiles()).toBe(true)
|
||||
expect(controller.getGitignoreFiles()).toContain(path.join(TEST_CWD, ".gitignore"))
|
||||
|
||||
// Verify patterns are applied
|
||||
expect(controller.validateAccess("node_modules/package.json")).toBe(false)
|
||||
expect(controller.validateAccess("debug.log")).toBe(false)
|
||||
expect(controller.validateAccess("src/index.ts")).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("validateAccess", () => {
|
||||
beforeEach(async () => {
|
||||
// Setup a basic .gitignore for testing
|
||||
mockFileExists.mockImplementation((filePath: string) => {
|
||||
return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore"))
|
||||
})
|
||||
mockReadFile.mockResolvedValue("node_modules/\n*.log\n/build\n")
|
||||
|
||||
await controller.initialize()
|
||||
})
|
||||
|
||||
it("should block files matching .gitignore patterns", () => {
|
||||
expect(controller.validateAccess("node_modules/package.json")).toBe(false)
|
||||
expect(controller.validateAccess("debug.log")).toBe(false)
|
||||
expect(controller.validateAccess("build")).toBe(false)
|
||||
})
|
||||
|
||||
it("should allow files not matching .gitignore patterns", () => {
|
||||
expect(controller.validateAccess("src/index.ts")).toBe(true)
|
||||
expect(controller.validateAccess("README.md")).toBe(true)
|
||||
expect(controller.validateAccess("package.json")).toBe(true)
|
||||
})
|
||||
|
||||
it("should allow all access when no .gitignore files exist", async () => {
|
||||
// Create a new controller with no .gitignore
|
||||
mockFileExists.mockResolvedValue(false)
|
||||
const emptyController = new GitIgnoreController(TEST_CWD)
|
||||
await emptyController.initialize()
|
||||
|
||||
expect(emptyController.validateAccess("node_modules/package.json")).toBe(true)
|
||||
expect(emptyController.validateAccess("debug.log")).toBe(true)
|
||||
expect(emptyController.validateAccess("any/file.ts")).toBe(true)
|
||||
|
||||
emptyController.dispose()
|
||||
})
|
||||
|
||||
it("should discover and load nested .gitignore files", async () => {
|
||||
// Setup mocks to simulate both root and nested .gitignore files
|
||||
mockFileExists.mockImplementation((filePath: string) => {
|
||||
return Promise.resolve(
|
||||
filePath === path.join(TEST_CWD, ".gitignore") ||
|
||||
filePath === path.join(TEST_CWD, "src", ".gitignore"),
|
||||
)
|
||||
})
|
||||
|
||||
// Mock different content for each file
|
||||
mockReadFile.mockImplementation((filePath: any) => {
|
||||
if (filePath.toString().endsWith("src/.gitignore")) {
|
||||
return Promise.resolve("*.tmp\n")
|
||||
}
|
||||
return Promise.resolve("node_modules/\n*.log\n")
|
||||
})
|
||||
|
||||
await controller.initialize()
|
||||
|
||||
// Verify both files were discovered
|
||||
expect(controller.hasGitignoreFiles()).toBe(true)
|
||||
const gitignoreFiles = controller.getGitignoreFiles()
|
||||
expect(gitignoreFiles).toContain(path.join(TEST_CWD, ".gitignore"))
|
||||
expect(gitignoreFiles).toContain(path.join(TEST_CWD, "src", ".gitignore"))
|
||||
|
||||
// Verify patterns from both files are applied
|
||||
expect(controller.validateAccess("node_modules/package.json")).toBe(false) // Root .gitignore
|
||||
expect(controller.validateAccess("debug.log")).toBe(false) // Root .gitignore
|
||||
expect(controller.validateAccess("src/temp.tmp")).toBe(false) // Nested .gitignore
|
||||
expect(controller.validateAccess("src/index.ts")).toBe(true) // Should be allowed
|
||||
})
|
||||
})
|
||||
|
||||
describe("filterPaths", () => {
|
||||
beforeEach(async () => {
|
||||
// Setup .gitignore patterns
|
||||
mockFileExists.mockImplementation((filePath: string) => {
|
||||
return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore"))
|
||||
})
|
||||
mockReadFile.mockResolvedValue("*.log\nnode_modules/\n")
|
||||
|
||||
await controller.initialize()
|
||||
})
|
||||
|
||||
it("should filter out ignored paths", () => {
|
||||
const paths = ["src/index.ts", "debug.log", "node_modules/package.json", "README.md", "error.log"]
|
||||
|
||||
const filtered = controller.filterPaths(paths)
|
||||
|
||||
expect(filtered).toEqual(["src/index.ts", "README.md"])
|
||||
})
|
||||
|
||||
it("should return all paths when no .gitignore exists", async () => {
|
||||
// Create controller with no .gitignore
|
||||
mockFileExists.mockResolvedValue(false)
|
||||
const emptyController = new GitIgnoreController(TEST_CWD)
|
||||
await emptyController.initialize()
|
||||
|
||||
const paths = ["src/index.ts", "debug.log", "node_modules/package.json"]
|
||||
const filtered = emptyController.filterPaths(paths)
|
||||
|
||||
expect(filtered).toEqual(paths)
|
||||
|
||||
emptyController.dispose()
|
||||
})
|
||||
})
|
||||
|
||||
describe("utility methods", () => {
|
||||
it("should return gitignore file paths", async () => {
|
||||
mockFileExists.mockImplementation((filePath: string) => {
|
||||
return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore"))
|
||||
})
|
||||
mockReadFile.mockResolvedValue("*.log\n")
|
||||
|
||||
await controller.initialize()
|
||||
|
||||
const files = controller.getGitignoreFiles()
|
||||
expect(files).toContain(path.join(TEST_CWD, ".gitignore"))
|
||||
})
|
||||
|
||||
it("should return gitignore content", async () => {
|
||||
const content = "*.log\nnode_modules/\n"
|
||||
mockFileExists.mockImplementation((filePath: string) => {
|
||||
return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore"))
|
||||
})
|
||||
mockReadFile.mockResolvedValue(content)
|
||||
|
||||
await controller.initialize()
|
||||
|
||||
const gitignoreFile = path.join(TEST_CWD, ".gitignore")
|
||||
expect(controller.getGitignoreContent(gitignoreFile)).toBe(content)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -519,6 +519,9 @@ describe("RooIgnoreController", () => {
|
|||
const onDeleteHandler = mockWatcher.onDidDelete.mock.calls[0][0]
|
||||
await onDeleteHandler()
|
||||
|
||||
// Wait for debounced reload to complete (100ms timeout + buffer)
|
||||
await new Promise((resolve) => setTimeout(resolve, 150))
|
||||
|
||||
// Verify content was reset
|
||||
expect(controller.rooIgnoreContent).toBeUndefined()
|
||||
|
||||
|
|
|
|||
|
|
@ -83,6 +83,7 @@ import { ToolRepetitionDetector } from "../tools/ToolRepetitionDetector"
|
|||
import { restoreTodoListForTask } from "../tools/updateTodoListTool"
|
||||
import { FileContextTracker } from "../context-tracking/FileContextTracker"
|
||||
import { RooIgnoreController } from "../ignore/RooIgnoreController"
|
||||
import { GitIgnoreController } from "../ignore/GitIgnoreController"
|
||||
import { RooProtectedController } from "../protect/RooProtectedController"
|
||||
import { type AssistantMessageContent, presentAssistantMessage } from "../assistant-message"
|
||||
import { AssistantMessageParser } from "../assistant-message/AssistantMessageParser"
|
||||
|
|
@ -234,6 +235,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
|
||||
toolRepetitionDetector: ToolRepetitionDetector
|
||||
rooIgnoreController?: RooIgnoreController
|
||||
gitIgnoreController?: GitIgnoreController
|
||||
rooProtectedController?: RooProtectedController
|
||||
fileContextTracker: FileContextTracker
|
||||
urlContentFetcher: UrlContentFetcher
|
||||
|
|
@ -1587,6 +1589,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
this.rooIgnoreController.dispose()
|
||||
this.rooIgnoreController = undefined
|
||||
}
|
||||
if (this.gitIgnoreController) {
|
||||
this.gitIgnoreController.dispose()
|
||||
this.gitIgnoreController = undefined
|
||||
}
|
||||
} catch (error) {
|
||||
console.error("Error disposing RooIgnoreController:", error)
|
||||
// This is the critical one for the leak fix.
|
||||
|
|
|
|||
|
|
@ -58,6 +58,7 @@ export async function searchFilesTool(
|
|||
regex,
|
||||
filePattern,
|
||||
cline.rooIgnoreController,
|
||||
cline.gitIgnoreController,
|
||||
)
|
||||
|
||||
const completeMessage = JSON.stringify({ ...sharedMessageProps, content: results } satisfies ClineSayTool)
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import * as readline from "readline"
|
|||
import * as vscode from "vscode"
|
||||
|
||||
import { RooIgnoreController } from "../../core/ignore/RooIgnoreController"
|
||||
import { GitIgnoreController } from "../../core/ignore/GitIgnoreController"
|
||||
import { fileExistsAtPath } from "../../utils/fs"
|
||||
/*
|
||||
This file provides functionality to perform regex searches on files using ripgrep.
|
||||
|
|
@ -142,6 +143,7 @@ export async function regexSearchFiles(
|
|||
regex: string,
|
||||
filePattern?: string,
|
||||
rooIgnoreController?: RooIgnoreController,
|
||||
gitIgnoreController?: GitIgnoreController,
|
||||
): Promise<string> {
|
||||
const vscodeAppRoot = vscode.env.appRoot
|
||||
const rgPath = await getBinPath(vscodeAppRoot)
|
||||
|
|
@ -212,10 +214,10 @@ export async function regexSearchFiles(
|
|||
|
||||
// console.log(results)
|
||||
|
||||
// Filter results using RooIgnoreController if provided
|
||||
const filteredResults = rooIgnoreController
|
||||
? results.filter((result) => rooIgnoreController.validateAccess(result.file))
|
||||
: results
|
||||
// Filter results using both controllers if provided
|
||||
const filteredResults = results
|
||||
.filter((result) => gitIgnoreController?.validateAccess(result.file) ?? true)
|
||||
.filter((result) => rooIgnoreController?.validateAccess(result.file) ?? true)
|
||||
|
||||
return formatResults(filteredResults, cwd)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue