From fa67db75e0ae2abc19de970d8ee1bf7854aab0bc Mon Sep 17 00:00:00 2001 From: Evan <58194240+celestial-vault@users.noreply.github.com> Date: Tue, 4 Feb 2025 08:53:49 +0800 Subject: [PATCH] Add clineignore class into cline file (#1623) --- src/core/Cline.ts | 8 +++ .../LLMFileAccessController.test.ts | 53 ++++++++--------- .../LLMFileAccessController.ts | 59 +++++++++++++++++-- 3 files changed, 89 insertions(+), 31 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 9561d3b281..2a6aaa4aa8 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -46,6 +46,7 @@ import { HistoryItem } from "../shared/HistoryItem" import { ClineAskResponse, ClineCheckpointRestore } from "../shared/WebviewMessage" import { calculateApiCost } from "../utils/cost" import { fileExistsAtPath } from "../utils/fs" +import { LLMFileAccessController } from "../services/llm-access-control/LLMFileAccessController" import { arePathsEqual, getReadablePath } from "../utils/path" import { fixModelHtmlEscaping, removeInvalidChars } from "../utils/string" import { AssistantMessageContent, parseAssistantMessage, ToolParamName, ToolUseName } from "./assistant-message" @@ -80,6 +81,7 @@ export class Cline { private chatSettings: ChatSettings apiConversationHistory: Anthropic.MessageParam[] = [] clineMessages: ClineMessage[] = [] + private llmAccessController: LLMFileAccessController private askResponse?: ClineAskResponse private askResponseText?: string private askResponseImages?: string[] @@ -123,6 +125,10 @@ export class Cline { images?: string[], historyItem?: HistoryItem, ) { + this.llmAccessController = new LLMFileAccessController(cwd) + this.llmAccessController.initialize().catch((error) => { + console.error("Failed to initialize LLMFileAccessController:", error) + }) this.providerRef = new WeakRef(provider) this.api = buildApiHandler(apiConfiguration) this.terminalManager = new TerminalManager() @@ -748,6 +754,7 @@ 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 = [] + await this.providerRef.deref()?.postStateToWebview() await this.say("text", task, images) @@ -1050,6 +1057,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..8bf6353a48 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() }) }) @@ -228,7 +227,7 @@ describe("LLMFileAccessController", () => { 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 +239,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 +252,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 b5139c43a8..40a2e46b55 100644 --- a/src/services/llm-access-control/LLMFileAccessController.ts +++ b/src/services/llm-access-control/LLMFileAccessController.ts @@ -2,6 +2,7 @@ import path from "path" import { fileExistsAtPath } from "../../utils/fs" import fs from "fs/promises" import ignore, { Ignore } from "ignore" +import * as vscode from "vscode" /** * Controls LLM access to files by enforcing ignore patterns. @@ -11,6 +12,8 @@ import ignore, { Ignore } from "ignore" export class LLMFileAccessController { private cwd: string private ignoreInstance: Ignore + private fileWatcher: vscode.FileSystemWatcher | null + private disposables: vscode.Disposable[] = [] /** * Default patterns that are always ignored for security @@ -20,19 +23,49 @@ export class LLMFileAccessController { constructor(cwd: string) { this.cwd = cwd this.ignoreInstance = ignore() - - // Add default patterns immediately this.ignoreInstance.add(LLMFileAccessController.DEFAULT_PATTERNS) + this.fileWatcher = null + + // Set up file watcher for .clineignore + this.setupFileWatcher() } /** * Initialize the controller by loading custom patterns - * This must be called and awaited before using the controller + * Must be called after construction and before using the controller */ 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(() => { + this.loadCustomPatterns().catch((error) => { + console.error("Failed to load updated .clineignore patterns:", error) + }) + }), + this.fileWatcher.onDidCreate(() => { + this.loadCustomPatterns().catch((error) => { + console.error("Failed to load new .clineignore patterns:", error) + }) + }), + this.fileWatcher.onDidDelete(() => { + this.resetToDefaultPatterns() + }), + ) + + // Add fileWatcher itself to disposables + this.disposables.push(this.fileWatcher) + } + /** * Load custom patterns from .clineignore if it exists */ @@ -40,6 +73,8 @@ export class LLMFileAccessController { try { const ignorePath = path.join(this.cwd, ".clineignore") if (await fileExistsAtPath(ignorePath)) { + // Reset ignore instance to prevent duplicate patterns + this.resetToDefaultPatterns() const content = await fs.readFile(ignorePath, "utf8") const customPatterns = content .split("\n") @@ -49,11 +84,18 @@ export class LLMFileAccessController { this.ignoreInstance.add(customPatterns) } } catch (error) { - console.error("Failed to load .clineignore:", error) // Continue with default patterns } } + /** + * Reset ignore patterns to defaults + */ + private resetToDefaultPatterns(): void { + 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) @@ -97,4 +139,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 = null + } }