diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 96879e30f6..5dd9e3fe98 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -60,6 +60,7 @@ import { SYSTEM_PROMPT } from "./prompts/system" import { addUserInstructions } from "./prompts/system" import { OpenAiHandler } from "../api/providers/openai" import { ApiStream } from "../api/transform/stream" +import { Logger } from "../services/logging/Logger" const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) ?? path.join(os.homedir(), "Desktop") // may or may not exist but fs checking existence would immediately ask for permission which would be bad UX, need to come up with a better solution @@ -125,7 +126,14 @@ export class Cline { images?: string[], historyItem?: HistoryItem, ) { + Logger.log("initializing LLMAC") this.llmAccessController = new LLMFileAccessController(cwd) + this.llmAccessController + .initialize() + .then(() => Logger.log("initialized")) + .catch((error) => { + console.error("Failed to initialize LLMFileAccessController:", error) + }) this.providerRef = new WeakRef(provider) this.api = buildApiHandler(apiConfiguration) this.terminalManager = new TerminalManager() @@ -751,8 +759,6 @@ export class Cline { // if the extension process were killed, then on restart the clineMessages might not be empty, so we need to set it to [] when we create a new Cline client (otherwise webview would show stale messages from previous session) this.clineMessages = [] this.apiConversationHistory = [] - // Initialize the LLM access controller - await this.llmAccessController.initialize() await this.providerRef.deref()?.postStateToWebview() @@ -780,9 +786,6 @@ export class Cline { // this.checkpointTrackerErrorMessage = "Checkpoints are only available for new tasks" // } - // Initialize the LLM access controller - await this.llmAccessController.initialize() - const modifiedClineMessages = await this.getSavedClineMessages() // Remove any resume messages that may have been added before @@ -1059,6 +1062,7 @@ export class Cline { this.terminalManager.disposeAll() this.urlContentFetcher.closeBrowser() this.browserSession.closeBrowser() + this.llmAccessController.dispose() await this.diffViewProvider.revertChanges() // need to await for when we want to make sure directories/files are reverted before re-starting the task from a checkpoint } diff --git a/src/services/llm-access-control/LLMFileAccessController.test.ts b/src/services/llm-access-control/LLMFileAccessController.test.ts index b8cee93e9a..c2620a9b5e 100644 --- a/src/services/llm-access-control/LLMFileAccessController.test.ts +++ b/src/services/llm-access-control/LLMFileAccessController.test.ts @@ -33,44 +33,44 @@ describe("LLMFileAccessController", () => { describe("Default Patterns", () => { // it("should block access to common ignored files", async () => { - // const results = await Promise.all([ + // const results = [ // controller.validateAccess(".env"), // controller.validateAccess(".git/config"), // controller.validateAccess("node_modules/package.json"), - // ]) + // ] // results.forEach((result) => result.should.be.false()) // }) it("should allow access to regular files", async () => { - const results = await Promise.all([ + const results = [ controller.validateAccess("src/index.ts"), controller.validateAccess("README.md"), controller.validateAccess("package.json"), - ]) + ] results.forEach((result) => result.should.be.true()) }) }) describe("Custom Patterns", () => { it("should block access to custom ignored patterns", async () => { - const results = await Promise.all([ + const results = [ controller.validateAccess("config.secret"), controller.validateAccess("private/data.txt"), controller.validateAccess("temp.json"), controller.validateAccess("nested/deep/file.secret"), controller.validateAccess("private/nested/deep/file.txt"), - ]) + ] results.forEach((result) => result.should.be.false()) }) it("should allow access to non-ignored files", async () => { - const results = await Promise.all([ + const results = [ controller.validateAccess("public/data.txt"), controller.validateAccess("config.json"), controller.validateAccess("src/temp/file.ts"), controller.validateAccess("nested/deep/file.txt"), controller.validateAccess("not-private/data.txt"), - ]) + ] results.forEach((result) => result.should.be.true()) }) @@ -83,11 +83,11 @@ describe("LLMFileAccessController", () => { controller = new LLMFileAccessController(tempDir) await controller.initialize() - const results = await Promise.all([ + const results = [ controller.validateAccess("data-123.json"), // Should be false (wildcard) controller.validateAccess("data.json"), // Should be true (doesn't match pattern) controller.validateAccess("script.tmp"), // Should be false (extension match) - ]) + ] results[0].should.be.false() // data-123.json results[1].should.be.true() // data.json @@ -112,9 +112,8 @@ describe("LLMFileAccessController", () => { // ) // controller = new LLMFileAccessController(tempDir) - // await controller.initialize() - // const results = await Promise.all([ + // const results = [ // // Basic negation // controller.validateAccess("temp/file.txt"), // Should be false (in temp/) // controller.validateAccess("temp/allowed/file.txt"), // Should be true (negated) @@ -130,7 +129,7 @@ describe("LLMFileAccessController", () => { // controller.validateAccess("assets/logo.png"), // Should be false (in assets/) // controller.validateAccess("assets/public/logo.png"), // Should be true (negated and matches *.png) // controller.validateAccess("assets/public/data.json"), // Should be true (in negated public/) - // ]) + // ] // results[0].should.be.false() // temp/file.txt // results[1].should.be.true() // temp/allowed/file.txt @@ -154,7 +153,7 @@ describe("LLMFileAccessController", () => { controller = new LLMFileAccessController(tempDir) await controller.initialize() - const result = await controller.validateAccess("test.secret") + const result = controller.validateAccess("test.secret") result.should.be.false() }) }) @@ -163,55 +162,55 @@ describe("LLMFileAccessController", () => { it("should handle absolute paths and match ignore patterns", async () => { // Test absolute path that should be allowed const allowedPath = path.join(tempDir, "src/file.ts") - const allowedResult = await controller.validateAccess(allowedPath) + const allowedResult = controller.validateAccess(allowedPath) allowedResult.should.be.true() // Test absolute path that matches an ignore pattern (*.secret) const ignoredPath = path.join(tempDir, "config.secret") - const ignoredResult = await controller.validateAccess(ignoredPath) + const ignoredResult = controller.validateAccess(ignoredPath) ignoredResult.should.be.false() // Test absolute path in ignored directory (private/) const ignoredDirPath = path.join(tempDir, "private/data.txt") - const ignoredDirResult = await controller.validateAccess(ignoredDirPath) + const ignoredDirResult = controller.validateAccess(ignoredDirPath) ignoredDirResult.should.be.false() }) it("should handle relative paths and match ignore patterns", async () => { // Test relative path that should be allowed - const allowedResult = await controller.validateAccess("./src/file.ts") + const allowedResult = controller.validateAccess("./src/file.ts") allowedResult.should.be.true() // Test relative path that matches an ignore pattern (*.secret) - const ignoredResult = await controller.validateAccess("./config.secret") + const ignoredResult = controller.validateAccess("./config.secret") ignoredResult.should.be.false() // Test relative path in ignored directory (private/) - const ignoredDirResult = await controller.validateAccess("./private/data.txt") + const ignoredDirResult = controller.validateAccess("./private/data.txt") ignoredDirResult.should.be.false() }) it("should normalize paths with backslashes", async () => { - const result = await controller.validateAccess("src\\file.ts") + const result = controller.validateAccess("src\\file.ts") result.should.be.true() }) it("should handle paths outside cwd", async () => { // Create a path that points to parent directory of cwd const outsidePath = path.join(path.dirname(tempDir), "outside.txt") - const result = await controller.validateAccess(outsidePath) + const result = controller.validateAccess(outsidePath) // Should return false for security since path is outside cwd result.should.be.false() // Test with a deeply nested path outside cwd const deepOutsidePath = path.join(path.dirname(tempDir), "deep", "nested", "outside.secret") - const deepResult = await controller.validateAccess(deepOutsidePath) + const deepResult = controller.validateAccess(deepOutsidePath) deepResult.should.be.false() // Test with a path that tries to escape using ../ const escapeAttemptPath = path.join(tempDir, "..", "escape-attempt.txt") - const escapeResult = await controller.validateAccess(escapeAttemptPath) + const escapeResult = controller.validateAccess(escapeAttemptPath) escapeResult.should.be.false() }) }) @@ -225,10 +224,56 @@ describe("LLMFileAccessController", () => { }) }) + describe("File Watcher", () => { + it("should update patterns when .clineignore is modified", async () => { + // Initial state + controller.validateAccess("test.log").should.be.true() + + // Modify .clineignore + await fs.writeFile(path.join(tempDir, ".clineignore"), "*.log") + + // Wait for the next event loop tick to ensure file watcher processes the change + await Promise.resolve() + + // Check if the new pattern is applied + controller.validateAccess("test.log").should.be.false() + }) + + it("should reset to default patterns when .clineignore is deleted", async () => { + // Initial state with .clineignore containing a pattern + await fs.writeFile(path.join(tempDir, ".clineignore"), "*.log") + await new Promise((resolve) => setTimeout(resolve, 100)) + controller.validateAccess("test.log").should.be.false() + + // Delete .clineignore + await fs.unlink(path.join(tempDir, ".clineignore")) + + // Wait for the next event loop tick to ensure file watcher processes the deletion + await Promise.resolve() + + // Should now allow previously ignored files + controller.validateAccess("test.log").should.be.true() + }) + + it("should handle .clineignore being recreated", async () => { + // Delete existing .clineignore + await fs.unlink(path.join(tempDir, ".clineignore")) + await new Promise((resolve) => setTimeout(resolve, 100)) + + // Create new .clineignore with different patterns + await fs.writeFile(path.join(tempDir, ".clineignore"), "*.temp") + await new Promise((resolve) => setTimeout(resolve, 100)) + + // Check if new patterns are applied + controller.validateAccess("file.temp").should.be.false() + controller.validateAccess("test.log").should.be.true() + }) + }) + describe("Error Handling", () => { it("should handle invalid paths", async () => { // Test with an invalid path containing null byte - const result = await controller.validateAccess("\0invalid") + const result = controller.validateAccess("\0invalid") result.should.be.true() }) @@ -240,7 +285,7 @@ describe("LLMFileAccessController", () => { try { const controller = new LLMFileAccessController(emptyDir) await controller.initialize() - const result = await controller.validateAccess("file.txt") + const result = controller.validateAccess("file.txt") result.should.be.true() } finally { await fs.rm(emptyDir, { recursive: true, force: true }) @@ -253,7 +298,7 @@ describe("LLMFileAccessController", () => { controller = new LLMFileAccessController(tempDir) await controller.initialize() - const result = await controller.validateAccess("regular-file.txt") + const result = controller.validateAccess("regular-file.txt") result.should.be.true() }) }) diff --git a/src/services/llm-access-control/LLMFileAccessController.ts b/src/services/llm-access-control/LLMFileAccessController.ts index 298bfc08eb..ad5ae50900 100644 --- a/src/services/llm-access-control/LLMFileAccessController.ts +++ b/src/services/llm-access-control/LLMFileAccessController.ts @@ -3,6 +3,7 @@ import { fileExistsAtPath } from "../../utils/fs" import fs from "fs/promises" import ignore, { Ignore } from "ignore" import * as vscode from "vscode" +import { Logger } from "../logging/Logger" /** * Controls LLM access to files by enforcing ignore patterns. @@ -27,44 +28,77 @@ export class LLMFileAccessController { // Set up file watcher for .clineignore this.setupFileWatcher() - - // Load initial patterns - this.loadCustomPatterns() } /** * Initialize the controller by loading custom patterns - * This must be called and awaited before using the controller + * Must be called after construction */ async initialize(): Promise { await this.loadCustomPatterns() } + /** + * Set up the file watcher for .clineignore changes + */ + private setupFileWatcher(): void { + const clineignorePattern = new vscode.RelativePattern(this.cwd, ".clineignore") + this.fileWatcher = vscode.workspace.createFileSystemWatcher(clineignorePattern) + + // Watch for changes and updates + this.disposables.push( + this.fileWatcher.onDidChange(() => { + Logger.log("[LLMFileAccessController] .clineignore changed - loading custom patterns") + this.loadCustomPatterns() + }), + this.fileWatcher.onDidCreate(() => { + Logger.log("[LLMFileAccessController] .clineignore created - loading custom patterns") + this.loadCustomPatterns() + }), + this.fileWatcher.onDidDelete(() => { + Logger.log("[LLMFileAccessController] .clineignore deleted - resetting to default patterns") + this.resetToDefaultPatterns() + }), + ) + + // Add fileWatcher itself to disposables + this.disposables.push(this.fileWatcher) + } + /** * Load custom patterns from .clineignore if it exists */ private async loadCustomPatterns(): Promise { try { + Logger.log("loading custom patterns.") const ignorePath = path.join(this.cwd, ".clineignore") if (await fileExistsAtPath(ignorePath)) { - // We need to reset ignore. Otherwise we will be adding duplicate patterns from re-reading the .clineignore - // Will be switching to globby in next PR - this.ignoreInstance = ignore() - this.ignoreInstance.add(LLMFileAccessController.DEFAULT_PATTERNS) + // Reset ignore instance to prevent duplicate patterns + this.resetToDefaultPatterns() const content = await fs.readFile(ignorePath, "utf8") const customPatterns = content .split("\n") .map((line) => line.trim()) .filter((line) => line && !line.startsWith("#")) + Logger.log(`[LLMFileAccessController] Loading custom patterns: ${JSON.stringify(customPatterns)}`) this.ignoreInstance.add(customPatterns) } } catch (error) { - console.error("Failed to load .clineignore:", error) + Logger.log(`[LLMFileAccessController] Error loading .clineignore: ${error}`) // Continue with default patterns } } + /** + * Reset ignore patterns to defaults + */ + private resetToDefaultPatterns(): void { + Logger.log("[LLMFileAccessController] Resetting to default patterns") + this.ignoreInstance = ignore() + this.ignoreInstance.add(LLMFileAccessController.DEFAULT_PATTERNS) + } + /** * Check if a file should be accessible to the LLM * @param filePath - Path to check (relative to cwd) @@ -108,4 +142,13 @@ export class LLMFileAccessController { 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 = [] + this.fileWatcher = undefined + } }