From bbecb5dcd2f6deefd88d4751c8d57912e0137abc Mon Sep 17 00:00:00 2001 From: John Evans Date: Wed, 8 Oct 2025 09:47:41 -0400 Subject: [PATCH 1/2] fix test bug and path resolution bug --- src/services/ripgrep/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/services/ripgrep/index.ts b/src/services/ripgrep/index.ts index d384b27c9..8a4cfedb3 100644 --- a/src/services/ripgrep/index.ts +++ b/src/services/ripgrep/index.ts @@ -233,7 +233,9 @@ function formatResults(fileResults: SearchFileResult[], cwd: string): string { // Group results by file name fileResults.slice(0, MAX_RESULTS).forEach((file) => { - const relativeFilePath = path.relative(cwd, file.file) + // If file.file is already a relative path, use it as-is + // Otherwise, make it relative to cwd + const relativeFilePath = path.isAbsolute(file.file) ? path.relative(cwd, file.file) : file.file if (!groupedResults[relativeFilePath]) { groupedResults[relativeFilePath] = [] From ee47c23272e345bfcf774eeb6ea6454e65987889 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Fri, 10 Oct 2025 17:00:27 -0500 Subject: [PATCH 2/2] feat(ignore): port upstream GitIgnoreController/BaseIgnoreController and integrate with RooIgnoreController, Task, searchFilesTool; add tests; ripgrep formatting fix --- src/core/ignore/BaseIgnoreController.ts | 122 +++++ src/core/ignore/GitIgnoreController.ts | 230 +++++++++ src/core/ignore/RooIgnoreController.ts | 96 +--- .../GitIgnoreController.security.spec.ts | 285 +++++++++++ .../__tests__/GitIgnoreController.spec.ts | 471 ++++++++++++++++++ .../__tests__/RooIgnoreController.spec.ts | 3 + src/core/task/Task.ts | 11 + src/core/tools/searchFilesTool.ts | 1 + ...search-files.gitignore.integration.spec.ts | 222 +++++++++ src/services/ripgrep/index.ts | 10 +- 10 files changed, 1361 insertions(+), 90 deletions(-) create mode 100644 src/core/ignore/BaseIgnoreController.ts create mode 100644 src/core/ignore/GitIgnoreController.ts create mode 100644 src/core/ignore/__tests__/GitIgnoreController.security.spec.ts create mode 100644 src/core/ignore/__tests__/GitIgnoreController.spec.ts create mode 100644 src/services/ripgrep/__tests__/regex-search-files.gitignore.integration.spec.ts diff --git a/src/core/ignore/BaseIgnoreController.ts b/src/core/ignore/BaseIgnoreController.ts new file mode 100644 index 000000000..d00106663 --- /dev/null +++ b/src/core/ignore/BaseIgnoreController.ts @@ -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 + + /** + * 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) + } +} diff --git a/src/core/ignore/GitIgnoreController.ts b/src/core/ignore/GitIgnoreController.ts new file mode 100644 index 000000000..ca1a89b91 --- /dev/null +++ b/src/core/ignore/GitIgnoreController.ts @@ -0,0 +1,230 @@ +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 = 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 { + await this.discoverAndLoadGitignoreFiles() + this.setupGitIgnoreWatchers() + } + + /** + * Discover and load .gitignore files (root + common subdirectories) + */ + private async discoverAndLoadGitignoreFiles(): Promise { + 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) { + const exists = await fileExistsAtPath(gitignorePath) + + if (exists) { + this.gitignoreFiles.push(gitignorePath) + await this.loadGitignoreFile(gitignorePath) + } + } + + // Also discover arbitrary nested .gitignore files across the workspace + await this.findGitignoreFilesRecursively(this.cwd) + + // Load any files discovered by recursion that weren't loaded yet + for (const p of this.gitignoreFiles) { + if (!this.gitignoreContents.has(p)) { + await this.loadGitignoreFile(p) + } + } + + // 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 { + 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 { + 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, we need to create patterns that match files within that directory + const lines = content.split(/\r?\n/).filter((line) => line.trim() && !line.startsWith("#")) + // Convert Windows paths to POSIX for consistent pattern matching + const normalizedRelativeDir = relativeDir.split(path.sep).join("/") + + const adjustedPatterns = lines.flatMap((pattern) => { + const trimmed = pattern.trim() + + if (trimmed.startsWith("/")) { + // Absolute patterns (starting with /) are relative to the .gitignore location + return [normalizedRelativeDir + trimmed] + } else if (trimmed.startsWith("!")) { + // Negation patterns + const negatedPattern = trimmed.slice(1) + if (negatedPattern.startsWith("/")) { + return ["!" + normalizedRelativeDir + negatedPattern] + } else { + // For relative negation patterns, match in the directory and subdirectories + return [ + "!" + normalizedRelativeDir + "/" + negatedPattern, + "!" + normalizedRelativeDir + "/**/" + negatedPattern, + ] + } + } else { + // Relative patterns - match files in the directory and all subdirectories + // For "*.tmp" in src/.gitignore, we need TWO patterns: + // - src/*.tmp (matches direct children like src/temp.tmp) + // - src/**/*.tmp (matches descendants like src/subdir/temp.tmp) + const patterns = [ + normalizedRelativeDir + "/" + trimmed, + normalizedRelativeDir + "/**/" + trimmed, + ] + return patterns + } + }) + + 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 = [] + } +} diff --git a/src/core/ignore/RooIgnoreController.ts b/src/core/ignore/RooIgnoreController.ts index 45054cce9..78e74cefb 100644 --- a/src/core/ignore/RooIgnoreController.ts +++ b/src/core/ignore/RooIgnoreController.ts @@ -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 diff --git a/src/core/ignore/__tests__/GitIgnoreController.security.spec.ts b/src/core/ignore/__tests__/GitIgnoreController.security.spec.ts new file mode 100644 index 000000000..f1ba1248c --- /dev/null +++ b/src/core/ignore/__tests__/GitIgnoreController.security.spec.ts @@ -0,0 +1,285 @@ +// 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 + let mockReadFile: Mock + + beforeEach(async () => { + // Reset mocks + vi.clearAllMocks() + + // Setup mocks + mockFileExists = fileExistsAtPath as Mock + mockReadFile = fs.readFile as Mock + + // 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) => { + // Normalize path separators for cross-platform compatibility + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.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) + }) + }) +}) diff --git a/src/core/ignore/__tests__/GitIgnoreController.spec.ts b/src/core/ignore/__tests__/GitIgnoreController.spec.ts new file mode 100644 index 000000000..c67a86a18 --- /dev/null +++ b/src/core/ignore/__tests__/GitIgnoreController.spec.ts @@ -0,0 +1,471 @@ +// 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 + let mockReadFile: Mock + 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 + mockReadFile = fs.readFile as Mock + + // 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) => { + // Normalize path separators for cross-platform compatibility + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.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 + }) + + it("should filter deeply nested files from nested .gitignore patterns", async () => { + // Setup mocks to simulate nested .gitignore file + 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) => { + // Normalize path separators for cross-platform compatibility + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.endsWith("src/.gitignore")) { + // Pattern that should match files at any depth within src/ + return Promise.resolve("*.tmp\n*.cache\n") + } + return Promise.resolve("node_modules/\n") + }) + + await controller.initialize() + + // Test direct children of src/ + expect(controller.validateAccess("src/temp.tmp")).toBe(false) + expect(controller.validateAccess("src/data.cache")).toBe(false) + + // Test deeply nested files (2 levels deep) + expect(controller.validateAccess("src/utils/temp.tmp")).toBe(false) + expect(controller.validateAccess("src/components/data.cache")).toBe(false) + + // Test very deeply nested files (3+ levels deep) + expect(controller.validateAccess("src/utils/helpers/temp.tmp")).toBe(false) + expect(controller.validateAccess("src/components/ui/buttons/data.cache")).toBe(false) + + // Test that non-matching files are allowed at all depths + expect(controller.validateAccess("src/index.ts")).toBe(true) + expect(controller.validateAccess("src/utils/helper.ts")).toBe(true) + expect(controller.validateAccess("src/components/ui/Button.tsx")).toBe(true) + + // Test that patterns don't leak outside src/ + expect(controller.validateAccess("temp.tmp")).toBe(true) + expect(controller.validateAccess("lib/temp.tmp")).toBe(true) + expect(controller.validateAccess("test/data.cache")).toBe(true) + }) + }) + + 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) + }) + }) + + describe("escaped literals", () => { + it("should treat \\# as a literal # pattern, not a comment", async () => { + // Setup .gitignore with escaped # pattern + mockFileExists.mockImplementation((filePath: string) => { + return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore")) + }) + // \#foo should match a file literally named "#foo" + // Also include a real comment to verify it's ignored + mockReadFile.mockResolvedValue("\\#foo\n# This is a comment\n*.log\n") + + await controller.initialize() + + // File named "#foo" should be blocked (pattern matches literal #) + expect(controller.validateAccess("#foo")).toBe(false) + + // File named "# This is a comment" should NOT be blocked (it was a comment line) + expect(controller.validateAccess("# This is a comment")).toBe(true) + + // Other files should follow normal patterns + expect(controller.validateAccess("test.log")).toBe(false) + expect(controller.validateAccess("src/index.ts")).toBe(true) + }) + it("should treat \\# as a literal # pattern in nested .gitignore (exposes bug in line 136)", async () => { + // This test exposes the bug in line 136 of GitIgnoreController.ts + // where !line.startsWith("#") incorrectly filters out \# patterns + mockFileExists.mockImplementation((filePath: string) => { + return Promise.resolve( + filePath === path.join(TEST_CWD, ".gitignore") || + filePath === path.join(TEST_CWD, "src", ".gitignore"), + ) + }) + + mockReadFile.mockImplementation((filePath: any) => { + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.endsWith("src/.gitignore")) { + // Escaped # should match literal # file + return Promise.resolve("\\#special\n# Real comment\n") + } + return Promise.resolve("") + }) + + await controller.initialize() + + // File named "#special" in src/ should be blocked + // BUG: This currently passes (file is allowed) because line 136 filters out \#special + expect(controller.validateAccess("src/#special")).toBe(false) + + // Real comment should not create a pattern + expect(controller.validateAccess("src/# Real comment")).toBe(true) + }) + + it("should treat \\! as a literal ! pattern, not a negation", async () => { + // Setup .gitignore with escaped ! pattern + mockFileExists.mockImplementation((filePath: string) => { + return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore")) + }) + // First ignore all .txt files, then \!keep.txt should match literal "!keep.txt" + mockReadFile.mockResolvedValue("*.txt\n\\!keep.txt\n") + + await controller.initialize() + + // All .txt files should be blocked + expect(controller.validateAccess("file.txt")).toBe(false) + expect(controller.validateAccess("keep.txt")).toBe(false) + + // File literally named "!keep.txt" should also be blocked (not negated) + expect(controller.validateAccess("!keep.txt")).toBe(false) + + // Non-.txt files should be allowed + expect(controller.validateAccess("src/index.ts")).toBe(true) + }) + + it("should handle multiple escaped patterns in the same file", async () => { + mockFileExists.mockImplementation((filePath: string) => { + return Promise.resolve(filePath === path.join(TEST_CWD, ".gitignore")) + }) + // Mix of escaped and normal patterns + mockReadFile.mockResolvedValue("\\#comment-like\n\\!negation-like\n*.log\n") + + await controller.initialize() + + // Escaped patterns should match literal files + expect(controller.validateAccess("#comment-like")).toBe(false) + expect(controller.validateAccess("!negation-like")).toBe(false) + + // Normal patterns should work + expect(controller.validateAccess("debug.log")).toBe(false) + expect(controller.validateAccess("src/index.ts")).toBe(true) + }) + + it("should handle escaped patterns in nested .gitignore files", async () => { + mockFileExists.mockImplementation((filePath: string) => { + return Promise.resolve( + filePath === path.join(TEST_CWD, ".gitignore") || + filePath === path.join(TEST_CWD, "src", ".gitignore"), + ) + }) + + mockReadFile.mockImplementation((filePath: any) => { + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.endsWith("src/.gitignore")) { + // Nested .gitignore with escaped patterns + // Include a real comment to verify it's properly ignored + return Promise.resolve("\\#special\n# This is a comment\n\\!important\n") + } + return Promise.resolve("*.log\n") + }) + + await controller.initialize() + + // Escaped patterns in nested .gitignore should match literal files in that directory + expect(controller.validateAccess("src/#special")).toBe(false) + expect(controller.validateAccess("src/!important")).toBe(false) + + // Real comment should not create a pattern + expect(controller.validateAccess("src/# This is a comment")).toBe(true) + + // Should not affect files outside src/ + expect(controller.validateAccess("#special")).toBe(true) + expect(controller.validateAccess("!important")).toBe(true) + + // Root patterns should still work + expect(controller.validateAccess("debug.log")).toBe(false) + }) + + it("should not treat escaped \\! as negation in nested .gitignore", async () => { + mockFileExists.mockImplementation((filePath: string) => { + return Promise.resolve( + filePath === path.join(TEST_CWD, ".gitignore") || + filePath === path.join(TEST_CWD, "src", ".gitignore"), + ) + }) + + mockReadFile.mockImplementation((filePath: any) => { + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.endsWith("src/.gitignore")) { + // First ignore all .txt, then try to use escaped ! (should NOT negate) + return Promise.resolve("*.txt\n\\!keep.txt\n") + } + return Promise.resolve("") + }) + + await controller.initialize() + + // All .txt files in src/ should be blocked + expect(controller.validateAccess("src/file.txt")).toBe(false) + expect(controller.validateAccess("src/keep.txt")).toBe(false) + + // File literally named "!keep.txt" should also be blocked (not negated) + expect(controller.validateAccess("src/!keep.txt")).toBe(false) + + // Non-.txt files should be allowed + expect(controller.validateAccess("src/index.ts")).toBe(true) + }) + + it("should correctly distinguish between comments and escaped # patterns", async () => { + mockFileExists.mockImplementation((filePath: string) => { + return Promise.resolve( + filePath === path.join(TEST_CWD, ".gitignore") || + filePath === path.join(TEST_CWD, "src", ".gitignore"), + ) + }) + + mockReadFile.mockImplementation((filePath: any) => { + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.endsWith("src/.gitignore")) { + // Mix of real comments and escaped # patterns + return Promise.resolve("# This is a comment\n" + "\\#not-a-comment\n" + "*.tmp\n") + } + return Promise.resolve("# Root comment\n*.log\n") + }) + + await controller.initialize() + + // Escaped # pattern should match literal file + expect(controller.validateAccess("src/#not-a-comment")).toBe(false) + + // Comments should not create patterns + expect(controller.validateAccess("src/# This is a comment")).toBe(true) + + // Normal patterns should work + expect(controller.validateAccess("src/file.tmp")).toBe(false) + expect(controller.validateAccess("test.log")).toBe(false) + }) + }) +}) diff --git a/src/core/ignore/__tests__/RooIgnoreController.spec.ts b/src/core/ignore/__tests__/RooIgnoreController.spec.ts index 41d79476c..7ad3ec0b0 100644 --- a/src/core/ignore/__tests__/RooIgnoreController.spec.ts +++ b/src/core/ignore/__tests__/RooIgnoreController.spec.ts @@ -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() diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 851df91e6..92de7135b 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -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 implements TaskLike { toolRepetitionDetector: ToolRepetitionDetector rooIgnoreController?: RooIgnoreController + gitIgnoreController?: GitIgnoreController rooProtectedController?: RooProtectedController fileContextTracker: FileContextTracker urlContentFetcher: UrlContentFetcher @@ -342,6 +344,7 @@ export class Task extends EventEmitter implements TaskLike { this.taskNumber = -1 this.rooIgnoreController = new RooIgnoreController(this.cwd) + this.gitIgnoreController = new GitIgnoreController(this.cwd) this.rooProtectedController = new RooProtectedController(this.cwd) this.fileContextTracker = new FileContextTracker(provider, this.taskId) @@ -349,6 +352,10 @@ export class Task extends EventEmitter implements TaskLike { console.error("Failed to initialize RooIgnoreController:", error) }) + this.gitIgnoreController.initialize().catch((error) => { + console.error("Failed to initialize GitIgnoreController:", error) + }) + this.apiConfiguration = apiConfiguration this.api = buildApiHandler(apiConfiguration) this.autoApprovalHandler = new AutoApprovalHandler() @@ -1587,6 +1594,10 @@ export class Task extends EventEmitter 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. diff --git a/src/core/tools/searchFilesTool.ts b/src/core/tools/searchFilesTool.ts index b6ee97f87..85db061d5 100644 --- a/src/core/tools/searchFilesTool.ts +++ b/src/core/tools/searchFilesTool.ts @@ -58,6 +58,7 @@ export async function searchFilesTool( regex, filePattern, cline.rooIgnoreController, + cline.gitIgnoreController, ) const completeMessage = JSON.stringify({ ...sharedMessageProps, content: results } satisfies ClineSayTool) diff --git a/src/services/ripgrep/__tests__/regex-search-files.gitignore.integration.spec.ts b/src/services/ripgrep/__tests__/regex-search-files.gitignore.integration.spec.ts new file mode 100644 index 000000000..052192cc0 --- /dev/null +++ b/src/services/ripgrep/__tests__/regex-search-files.gitignore.integration.spec.ts @@ -0,0 +1,222 @@ +import { describe, it, expect, beforeEach, vi, type Mock } from "vitest" +import path from "path" + +// Under test +import * as ripgrepMod from "../index" +import { regexSearchFiles } from "../index" +import { GitIgnoreController } from "../../../core/ignore/GitIgnoreController" + +// Mocks +import * as fsPromises from "fs/promises" +import type { Dirent } from "fs" +import * as fileUtils from "../../../utils/fs" + +// Mock vscode (env + watchers used by controllers) +vi.mock("vscode", () => { + const mockDisposable = { dispose: vi.fn() } + return { + env: { appRoot: "/fake/vscode" }, + workspace: { + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(() => mockDisposable), + onDidChange: vi.fn(() => mockDisposable), + onDidDelete: vi.fn(() => mockDisposable), + dispose: vi.fn(), + })), + }, + RelativePattern: vi.fn().mockImplementation((base: string, pattern: string) => ({ base, pattern })), + } +}) + +// Mock child_process.spawn to simulate ripgrep JSON line output +vi.mock("child_process", () => { + const { PassThrough } = require("stream") + const { EventEmitter } = require("events") + + return { + spawn: (_bin: string, _args: string[]) => { + const proc = new EventEmitter() + const stdout = new PassThrough() + const stderr = new PassThrough() + // Expose stdout/stderr streams + ;(proc as any).stdout = stdout + ;(proc as any).stderr = stderr + ;(proc as any).kill = vi.fn(() => { + stdout.end() + stderr.end() + }) + + // Defer writing until next tick to simulate async process output + setImmediate(() => { + const lines: string[] = (globalThis as any).__RG_LINES__ ?? [] + for (const ln of lines) { + stdout.write(ln + "\n") + } + stdout.end() + }) + + return proc + }, + } +}) + +// Ensure fs/promises and file utils are mockable from tests +// Provide explicit mock factory so readdir/readFile are defined vi.fn() +vi.mock("fs/promises", () => { + const mockReaddir = vi.fn() + const mockReadFile = vi.fn() + return { + readdir: mockReaddir, + readFile: mockReadFile, + default: { + readdir: mockReaddir, + readFile: mockReadFile, + }, + } +}) +vi.mock("../../../utils/fs") +// Mock fs so BaseIgnoreController's realpathSync won't touch the real filesystem +vi.mock("fs", () => ({ + realpathSync: vi.fn((filePath: any) => filePath.toString()), +})) + +describe("regexSearchFiles + GitIgnoreController integration (nested .gitignore filtering)", () => { + const REPO = "/tmp/repo" // test workspace root + let mockReaddir: Mock + let mockReadFile: Mock + let mockFileExists: Mock + + beforeEach(() => { + vi.clearAllMocks() + + // Obtain mocked fs/promises fns from mock factory + const anyFs = fsPromises as any + mockReaddir = anyFs.readdir as unknown as Mock + mockReadFile = anyFs.readFile as unknown as Mock + + mockFileExists = fileUtils.fileExistsAtPath as unknown as Mock + + // Provide a fake ripgrep path so getBinPath succeeds regardless of VSCode layout + vi.spyOn(ripgrepMod, "getBinPath").mockResolvedValue("/fake/rg") + + // realpathSync handled by vi.mock("fs") factory above + + // Default: no files exist + mockFileExists.mockResolvedValue(false) + + // Default dirents helper + const dirent = (name: string, isDir: boolean): Dirent => + ({ + name, + isDirectory: () => isDir, + isFile: () => !isDir, + isSymbolicLink: () => false, + }) as unknown as Dirent + // Default readdir: empty + mockReaddir.mockImplementation(async (_p: any, _opts?: any) => { + return [] as any + }) + + // Default readFile: empty + mockReadFile.mockResolvedValue("") + }) + + it("excludes matches from files ignored by nested src/.gitignore patterns while keeping allowed files", async () => { + // Arrange a nested .gitignore structure: + // REPO/ + // src/.gitignore => '*.tmp' (ignore), '!keep.tmp' (negation) + // src/ignored.tmp (should be filtered) + // src/keep.tmp (should be kept due to negation) + // README.md (not under src, unaffected) + // + // GitIgnoreController recursively discovers src/.gitignore and adjusts patterns relative to REPO. + + // File existence for .gitignore files AND ripgrep binary resolution + mockFileExists.mockImplementation(async (p: string) => { + // Make getBinPath succeed by faking rg binary under VSCode appRoot + const binName = process.platform.startsWith("win") ? "rg.exe" : "rg" + const rgCandidate = path.join("/fake/vscode", "node_modules/@vscode/ripgrep/bin/", binName) + if (p === rgCandidate) return true + + if (p === path.join(REPO, "src", ".gitignore")) return true + // root .gitignore does not exist for this test + if (p === path.join(REPO, ".gitignore")) return false + return false + }) + + // Directory tree: REPO has 'src' subdir + const dirent = (name: string, isDir: boolean): Dirent => + ({ + name, + isDirectory: () => isDir, + isFile: () => !isDir, + isSymbolicLink: () => false, + }) as unknown as Dirent + + mockReaddir.mockImplementation(async (p: any, _opts?: any) => { + if (p === REPO) { + return [dirent("src", true)] as any + } + if (p === path.join(REPO, "src")) { + // No further subdirectories required for this test + return [] as any + } + return [] as any + }) + + // src/.gitignore content + mockReadFile.mockImplementation(async (p: any, _enc?: any) => { + if (p === path.join(REPO, "src", ".gitignore")) { + return "*.tmp\n!keep.tmp\n" + } + return "" + }) + + // Prepare ripgrep JSON lines for three files: ignored.tmp, keep.tmp, README.md + const rgLines = [ + // src/ignored.tmp + JSON.stringify({ type: "begin", data: { path: { text: "src/ignored.tmp" } } }), + JSON.stringify({ + type: "match", + data: { line_number: 1, lines: { text: "foo" }, absolute_offset: 1 }, + }), + JSON.stringify({ type: "end", data: {} }), + + // src/keep.tmp + JSON.stringify({ type: "begin", data: { path: { text: "src/keep.tmp" } } }), + JSON.stringify({ + type: "match", + data: { line_number: 2, lines: { text: "foo" }, absolute_offset: 10 }, + }), + JSON.stringify({ type: "end", data: {} }), + + // README.md (outside src, unaffected) + JSON.stringify({ type: "begin", data: { path: { text: "README.md" } } }), + JSON.stringify({ + type: "match", + data: { line_number: 3, lines: { text: "foo" }, absolute_offset: 20 }, + }), + JSON.stringify({ type: "end", data: {} }), + ] + ;(globalThis as any).__RG_LINES__ = rgLines + + // Initialize controller with nested .gitignore + const git = new GitIgnoreController(REPO) + await git.initialize() + // Sanity-check controller behavior before invoking ripgrep filter + expect(git.hasGitignoreFiles()).toBe(true) + expect(git.validateAccess("src/ignored.tmp")).toBe(false) + expect(git.validateAccess("src/keep.tmp")).toBe(true) + + // Act + const out = await regexSearchFiles(REPO, REPO, "foo", "*", undefined, git) + + // Assert: filtered summary and per-file sections + // - src/ignored.tmp must be filtered out + // - src/keep.tmp must be present (negation) + // - README.md must be present + expect(out).not.toContain("# src/ignored.tmp") + expect(out).toContain("# src/keep.tmp") + expect(out).toContain("# README.md") + }) +}) diff --git a/src/services/ripgrep/index.ts b/src/services/ripgrep/index.ts index 8a4cfedb3..ff0e213c4 100644 --- a/src/services/ripgrep/index.ts +++ b/src/services/ripgrep/index.ts @@ -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 { 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) }