From 2a0558de803db07e02edcbd04081941273925e55 Mon Sep 17 00:00:00 2001 From: Evan Date: Thu, 6 Feb 2025 14:51:43 -0800 Subject: [PATCH] files added --- src/core/Cline.ts | 87 +++++++++++++++---- src/core/prompts/responses.ts | 34 +++++++- src/core/prompts/system.ts | 1 + src/integrations/misc/extract-text.ts | 3 + src/services/glob/list-files.ts | 9 +- .../LLMFileAccessController.ts | 2 +- src/services/ripgrep/index.ts | 33 ++++--- src/services/tree-sitter/index.ts | 27 ++++-- src/utils/path.ts | 9 ++ 9 files changed, 161 insertions(+), 44 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 9af638085f..3722ca63b3 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 @@ -81,7 +82,7 @@ export class Cline { private chatSettings: ChatSettings apiConversationHistory: Anthropic.MessageParam[] = [] clineMessages: ClineMessage[] = [] - private llmAccessController: LLMFileAccessController + private llmFileAccessController: LLMFileAccessController private askResponse?: ClineAskResponse private askResponseText?: string private askResponseImages?: string[] @@ -125,8 +126,8 @@ export class Cline { images?: string[], historyItem?: HistoryItem, ) { - this.llmAccessController = new LLMFileAccessController(cwd) - this.llmAccessController.initialize().catch((error) => { + this.llmFileAccessController = new LLMFileAccessController(cwd) + this.llmFileAccessController.initialize().catch((error) => { console.error("Failed to initialize LLMFileAccessController:", error) }) this.providerRef = new WeakRef(provider) @@ -1057,7 +1058,7 @@ export class Cline { this.terminalManager.disposeAll() this.urlContentFetcher.closeBrowser() this.browserSession.closeBrowser() - this.llmAccessController.dispose() + this.llmFileAccessController.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 } @@ -1584,6 +1585,13 @@ export class Cline { // wait so we can determine if it's a new file or editing an existing file break } + + const accessAllowed = this.llmFileAccessController.validateAccess(relPath) + if (!accessAllowed) { + await handleError("writing file", new Error(`Access denied: ${relPath} (blocked by .clineignore)`)) + break + } + // Check if file exists using cached map or fs.access let fileExists: boolean if (this.diffViewProvider.editType !== undefined) { @@ -1701,6 +1709,7 @@ export class Cline { await this.saveCheckpoint() break } + this.consecutiveMistakeCount = 0 // if isEditingFile false, that means we have the full contents of the file already. @@ -1852,6 +1861,16 @@ export class Cline { await this.saveCheckpoint() break } + + const accessAllowed = this.llmFileAccessController.validateAccess(relPath) + if (!accessAllowed) { + await handleError( + "reading file", + new Error(`Access denied: ${relPath} (blocked by .clineignore)`), + ) + break + } + this.consecutiveMistakeCount = 0 const absolutePath = path.resolve(cwd, relPath) const completeMessage = JSON.stringify({ @@ -1915,9 +1934,17 @@ export class Cline { break } this.consecutiveMistakeCount = 0 + const absolutePath = path.resolve(cwd, relDirPath) + const [files, didHitLimit] = await listFiles(absolutePath, recursive, 200) - const result = formatResponse.formatFilesList(absolutePath, files, didHitLimit) + + const result = formatResponse.formatFilesList( + absolutePath, + files, + didHitLimit, + this.llmFileAccessController, + ) const completeMessage = JSON.stringify({ ...sharedMessageProps, content: result, @@ -1974,9 +2001,15 @@ export class Cline { await this.saveCheckpoint() break } + this.consecutiveMistakeCount = 0 + const absolutePath = path.resolve(cwd, relDirPath) - const result = await parseSourceCodeForDefinitionsTopLevel(absolutePath) + const result = await parseSourceCodeForDefinitionsTopLevel( + absolutePath, + this.llmFileAccessController, + ) + const completeMessage = JSON.stringify({ ...sharedMessageProps, content: result, @@ -2044,8 +2077,18 @@ export class Cline { break } this.consecutiveMistakeCount = 0 + const absolutePath = path.resolve(cwd, relDirPath) - const results = await regexSearchFiles(cwd, absolutePath, regex, filePattern) + const clineignorePath = path.join(cwd, ".clineignore") + const results = await regexSearchFiles( + cwd, + absolutePath, + regex, + filePattern, + this.llmFileAccessController, + ) + // Logger.log(results) + const completeMessage = JSON.stringify({ ...sharedMessageProps, content: results, @@ -3183,26 +3226,38 @@ export class Cline { // It could be useful for cline to know if the user went from one or no file to another between messages, so we always include this context details += "\n\n# VSCode Visible Files" - const visibleFiles = vscode.window.visibleTextEditors + const visibleFilePaths = vscode.window.visibleTextEditors ?.map((editor) => editor.document?.uri?.fsPath) .filter(Boolean) - .map((absolutePath) => path.relative(cwd, absolutePath).toPosix()) + .map((absolutePath) => path.relative(cwd, absolutePath)) + + // Filter paths through LLMFileAccessController + const allowedVisibleFiles = this.llmFileAccessController + .filterPaths(visibleFilePaths) + .map((p) => p.toPosix()) .join("\n") - if (visibleFiles) { - details += `\n${visibleFiles}` + + if (allowedVisibleFiles) { + details += `\n${allowedVisibleFiles}` } else { details += "\n(No visible files)" } details += "\n\n# VSCode Open Tabs" - const openTabs = vscode.window.tabGroups.all + const openTabPaths = vscode.window.tabGroups.all .flatMap((group) => group.tabs) .map((tab) => (tab.input as vscode.TabInputText)?.uri?.fsPath) .filter(Boolean) - .map((absolutePath) => path.relative(cwd, absolutePath).toPosix()) + .map((absolutePath) => path.relative(cwd, absolutePath)) + + // Filter paths through LLMFileAccessController + const allowedOpenTabs = this.llmFileAccessController + .filterPaths(openTabPaths) + .map((p) => p.toPosix()) .join("\n") - if (openTabs) { - details += `\n${openTabs}` + + if (allowedOpenTabs) { + details += `\n${allowedOpenTabs}` } else { details += "\n(No open tabs)" } @@ -3316,7 +3371,7 @@ export class Cline { details += "(Desktop files not shown automatically. Use list_files to explore if needed.)" } else { const [files, didHitLimit] = await listFiles(cwd, true, 200) - const result = formatResponse.formatFilesList(cwd, files, didHitLimit) + const result = formatResponse.formatFilesList(cwd, files, didHitLimit, this.llmFileAccessController) details += result } } diff --git a/src/core/prompts/responses.ts b/src/core/prompts/responses.ts index e932f4042c..d96fc9c18a 100644 --- a/src/core/prompts/responses.ts +++ b/src/core/prompts/responses.ts @@ -1,6 +1,8 @@ import { Anthropic } from "@anthropic-ai/sdk" import * as path from "path" import * as diff from "diff" +import { LLMFileAccessController } from "../../services/llm-access-control/LLMFileAccessController" +import { Logger } from "../../services/logging/Logger" export const formatResponse = { toolDenied: () => `The user denied this operation.`, @@ -46,7 +48,12 @@ Otherwise, if you have not completed the task and do not need additional informa return formatImagesIntoBlocks(images) }, - formatFilesList: (absolutePath: string, files: string[], didHitLimit: boolean): string => { + formatFilesList: ( + absolutePath: string, + files: string[], + didHitLimit: boolean, + llmFileAccessController: LLMFileAccessController, + ): string => { const sorted = files .map((file) => { // convert absolute path to relative path @@ -77,14 +84,33 @@ Otherwise, if you have not completed the task and do not need additional informa // the shorter one comes first return aParts.length - bParts.length }) + + const accessControlledSortedFiles = llmFileAccessController + ? sorted.map((filePath) => { + // path is relative to absolute path, not cwd + // validateAccess expects either path relative to cwd or absolute path + // otherwise, for validating against ignore patterns like "assets/icons", we would end up with just "icons", which would result in the path not being ignored. + const absoluteFilePath = path.resolve(absolutePath, filePath) + const isIgnored = !llmFileAccessController.validateAccess(absoluteFilePath) + if (isIgnored) { + return "\u{1F512} " + filePath + } + + return filePath + }) + : sorted + if (didHitLimit) { - return `${sorted.join( + return `${accessControlledSortedFiles.join( "\n", )}\n\n(File list truncated. Use list_files on specific subdirectories if you need to explore further.)` - } else if (sorted.length === 0 || (sorted.length === 1 && sorted[0] === "")) { + } else if ( + accessControlledSortedFiles.length === 0 || + (accessControlledSortedFiles.length === 1 && accessControlledSortedFiles[0] === "") + ) { return "No files found." } else { - return sorted.join("\n") + return accessControlledSortedFiles.join("\n") } }, diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 3c26f70d75..250f7804e5 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -970,6 +970,7 @@ export function addUserInstructions(settingsCustomInstructions?: string, clineRu ==== USER'S CUSTOM INSTRUCTIONS +The user may have a ".clineignore" file that blocks you from accessing certain directories and files. The paths blocked by this file will be indicated to you with a \u{1F512}, but you need not mention the \u{1F512}. For example, do NOT say things like 'I notice that {some_file}.ts is marked with \u{1F512} in the file list.' Instead, if you ever encounter an error about being blocked by the .clineignore, don't make a big deal about it, just move on without remarking on it. If you absolutely have to say something, just make a casual remark that you are 'blocked by the .clineignore'. Note that the .clineignore is also blocked. The following additional instructions are provided by the user, and should be followed to the best of your ability without interfering with the TOOL USE guidelines. diff --git a/src/integrations/misc/extract-text.ts b/src/integrations/misc/extract-text.ts index 67a580af9b..168eb6f5a3 100644 --- a/src/integrations/misc/extract-text.ts +++ b/src/integrations/misc/extract-text.ts @@ -4,8 +4,11 @@ import pdf from "pdf-parse/lib/pdf-parse" import mammoth from "mammoth" import fs from "fs/promises" import { isBinaryFile } from "isbinaryfile" +import { LLMFileAccessController } from "../../services/llm-access-control/LLMFileAccessController" export async function extractTextFromFile(filePath: string): Promise { + // First check if we have permission to access this file + try { await fs.access(filePath) } catch (error) { diff --git a/src/services/glob/list-files.ts b/src/services/glob/list-files.ts index 8578b914d7..83c13fc31f 100644 --- a/src/services/glob/list-files.ts +++ b/src/services/glob/list-files.ts @@ -1,7 +1,8 @@ import { globby, Options } from "globby" import os from "os" import * as path from "path" -import { arePathsEqual } from "../../utils/path" +import { arePathsEqual, pathExists } from "../../utils/path" +import { Logger } from "../logging/Logger" export async function listFiles(dirPath: string, recursive: boolean, limit: number): Promise<[string[], boolean]> { const absolutePath = path.resolve(dirPath) @@ -45,9 +46,11 @@ export async function listFiles(dirPath: string, recursive: boolean, limit: numb ignore: recursive ? dirsToIgnore : undefined, // just in case there is no gitignore, we ignore sensible defaults onlyFiles: false, // true by default, false means it will list directories on their own too } + // * globs all files in one dir, ** globs files in nested directories - const files = recursive ? await globbyLevelByLevel(limit, options) : (await globby("*", options)).slice(0, limit) - return [files, files.length >= limit] + const filePaths = recursive ? await globbyLevelByLevel(limit, options) : (await globby("*", options)).slice(0, limit) + + return [filePaths, filePaths.length >= limit] } /* diff --git a/src/services/llm-access-control/LLMFileAccessController.ts b/src/services/llm-access-control/LLMFileAccessController.ts index 40a2e46b55..69228751bc 100644 --- a/src/services/llm-access-control/LLMFileAccessController.ts +++ b/src/services/llm-access-control/LLMFileAccessController.ts @@ -18,7 +18,7 @@ export class LLMFileAccessController { /** * Default patterns that are always ignored for security */ - private static readonly DEFAULT_PATTERNS = [] // empty for now + private static readonly DEFAULT_PATTERNS = [".clineignore"] // empty for now constructor(cwd: string) { this.cwd = cwd diff --git a/src/services/ripgrep/index.ts b/src/services/ripgrep/index.ts index b9ebe68c77..da3600c2e8 100644 --- a/src/services/ripgrep/index.ts +++ b/src/services/ripgrep/index.ts @@ -1,8 +1,10 @@ import * as vscode from "vscode" import * as childProcess from "child_process" import * as path from "path" -import * as fs from "fs" import * as readline from "readline" +import { pathExists } from "../../utils/path" +import { LLMFileAccessController } from "../llm-access-control/LLMFileAccessController" +import { Logger } from "../logging/Logger" /* This file provides functionality to perform regex searches on files using ripgrep. @@ -50,7 +52,7 @@ const isWindows = /^win/.test(process.platform) const binName = isWindows ? "rg.exe" : "rg" interface SearchResult { - file: string + filePath: string line: number column: number match: string @@ -74,14 +76,6 @@ async function getBinPath(vscodeAppRoot: string): Promise { ) } -async function pathExists(path: string): Promise { - return new Promise((resolve) => { - fs.access(path, (err) => { - resolve(err === null) - }) - }) -} - async function execRipgrep(bin: string, args: string[]): Promise { return new Promise((resolve, reject) => { const rgProcess = childProcess.spawn(bin, args) @@ -122,7 +116,13 @@ async function execRipgrep(bin: string, args: string[]): Promise { }) } -export async function regexSearchFiles(cwd: string, directoryPath: string, regex: string, filePattern?: string): Promise { +export async function regexSearchFiles( + cwd: string, + directoryPath: string, + regex: string, + filePattern?: string, + llmFileAccessController?: LLMFileAccessController, +): Promise { const vscodeAppRoot = vscode.env.appRoot const rgPath = await getBinPath(vscodeAppRoot) @@ -150,7 +150,7 @@ export async function regexSearchFiles(cwd: string, directoryPath: string, regex results.push(currentResult as SearchResult) } currentResult = { - file: parsed.data.path.text, + filePath: parsed.data.path.text, line: parsed.data.line_number, column: parsed.data.submatches[0].start, match: parsed.data.lines.text, @@ -174,7 +174,12 @@ export async function regexSearchFiles(cwd: string, directoryPath: string, regex results.push(currentResult as SearchResult) } - return formatResults(results, cwd) + // Filter results using LLMFileAccessController if provided + const filteredResults = llmFileAccessController + ? results.filter((result) => llmFileAccessController.validateAccess(result.filePath)) + : results + + return formatResults(filteredResults, cwd) } function formatResults(results: SearchResult[], cwd: string): string { @@ -189,7 +194,7 @@ function formatResults(results: SearchResult[], cwd: string): string { // Group results by file name results.slice(0, MAX_RESULTS).forEach((result) => { - const relativeFilePath = path.relative(cwd, result.file) + const relativeFilePath = path.relative(cwd, result.filePath) if (!groupedResults[relativeFilePath]) { groupedResults[relativeFilePath] = [] } diff --git a/src/services/tree-sitter/index.ts b/src/services/tree-sitter/index.ts index 19d0234f01..291127a6bf 100644 --- a/src/services/tree-sitter/index.ts +++ b/src/services/tree-sitter/index.ts @@ -3,9 +3,13 @@ import * as path from "path" import { listFiles } from "../glob/list-files" import { LanguageParser, loadRequiredLanguageParsers } from "./languageParser" import { fileExistsAtPath } from "../../utils/fs" +import { LLMFileAccessController } from "../llm-access-control/LLMFileAccessController" // TODO: implement caching behavior to avoid having to keep analyzing project for new tasks. -export async function parseSourceCodeForDefinitionsTopLevel(dirPath: string): Promise { +export async function parseSourceCodeForDefinitionsTopLevel( + dirPath: string, + llmFileAccessController?: LLMFileAccessController, +): Promise { // check if the path exists const dirExists = await fileExistsAtPath(path.resolve(dirPath)) if (!dirExists) { @@ -24,10 +28,14 @@ export async function parseSourceCodeForDefinitionsTopLevel(dirPath: string): Pr // Parse specific files we have language parsers for // const filesWithoutDefinitions: string[] = [] - for (const file of filesToParse) { - const definitions = await parseFile(file, languageParsers) + + // Filter filepaths for access if controller is provided + const allowedFilesToParse = llmFileAccessController ? llmFileAccessController.filterPaths(filesToParse) : filesToParse + + for (const filePath of allowedFilesToParse) { + const definitions = await parseFile(filePath, languageParsers, llmFileAccessController) if (definitions) { - result += `${path.relative(dirPath, file).toPosix()}\n${definitions}\n` + result += `${path.relative(dirPath, filePath).toPosix()}\n${definitions}\n` } // else { // filesWithoutDefinitions.push(file) @@ -98,7 +106,14 @@ This approach allows us to focus on the most relevant parts of the code (defined - https://github.com/tree-sitter/tree-sitter/blob/master/lib/binding_web/test/helper.js - https://tree-sitter.github.io/tree-sitter/code-navigation-systems */ -async function parseFile(filePath: string, languageParsers: LanguageParser): Promise { +async function parseFile( + filePath: string, + languageParsers: LanguageParser, + llmFileAccessController?: LLMFileAccessController, +): Promise { + if (llmFileAccessController && !llmFileAccessController.validateAccess(filePath)) { + return null + } const fileContent = await fs.readFile(filePath, "utf8") const ext = path.extname(filePath).toLowerCase().slice(1) @@ -159,5 +174,5 @@ async function parseFile(filePath: string, languageParsers: LanguageParser): Pro if (formattedOutput.length > 0) { return `|----\n${formattedOutput}|----\n` } - return undefined + return null } diff --git a/src/utils/path.ts b/src/utils/path.ts index b61eb38bed..b9507be740 100644 --- a/src/utils/path.ts +++ b/src/utils/path.ts @@ -1,5 +1,6 @@ import * as path from "path" import os from "os" +import * as fs from "fs" /* The Node.js 'path' module resolves and normalizes paths differently depending on the platform: @@ -99,3 +100,11 @@ export function getReadablePath(cwd: string, relPath?: string): string { } } } + +export async function pathExists(path: string): Promise { + return new Promise((resolve) => { + fs.access(path, (err) => { + resolve(err === null) + }) + }) +}