This commit is contained in:
Evan 2025-02-02 14:19:12 -08:00
parent 8e4ab703c8
commit f6e64ccb93
3 changed files with 133 additions and 41 deletions

View file

@ -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
}

View file

@ -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()
})
})

View file

@ -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<void> {
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<void> {
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
}
}