From 33ee0cbbfa3fcd7885f9308cd2c7a10ef617c8d4 Mon Sep 17 00:00:00 2001 From: Sara Hamza Date: Mon, 17 Mar 2025 00:01:45 +0200 Subject: [PATCH 1/2] implement file ignoring mechanism --- .clinerules | 25 +- package-lock.json | 70 ++++- package.json | 2 + src/core/Cline.ts | 96 ++++++- src/core/ignore/ClineIgnoreController.ts | 189 +++++++++++++ .../__tests__/ClineIgnoreController.test.ts | 253 ++++++++++++++++++ src/core/prompts/responses.ts | 27 +- src/core/webview/ClineProvider.ts | 1 + src/services/glob/list-files.ts | 6 +- src/services/ripgrep/index.ts | 14 +- src/services/tree-sitter/index.ts | 22 +- src/shared/ExtensionMessage.ts | 1 + src/utils/__tests__/git.test.ts | 2 +- 13 files changed, 672 insertions(+), 36 deletions(-) create mode 100644 src/core/ignore/ClineIgnoreController.ts create mode 100644 src/core/ignore/__tests__/ClineIgnoreController.test.ts diff --git a/.clinerules b/.clinerules index 4f726299d61..f4f59dcf12e 100644 --- a/.clinerules +++ b/.clinerules @@ -1,22 +1,25 @@ # Code Quality Rules 1. Test Coverage: - - Before attempting completion, always make sure that any code changes have test coverage - - Ensure all tests pass before submitting changes + + - Before attempting completion, always make sure that any code changes have test coverage + - Ensure all tests pass before submitting changes 2. Lint Rules: - - Never disable any lint rules without explicit user approval - - If a lint rule needs to be disabled, ask the user first and explain why - - Prefer fixing the underlying issue over disabling the lint rule - - Document any approved lint rule disabling with a comment explaining the reason -3. Logging Guidelines: - - Always instrument code changes using the logger exported from `src\utils\logging\index.ts`. - - This will facilitate efficient debugging without impacting production (as the logger no-ops outside of a test environment.) - - Logs can be found in `logs\app.log` - - Logfile is overwritten on each run to keep it to a manageable volume. + - Never disable any lint rules without explicit user approval + - If a lint rule needs to be disabled, ask the user first and explain why + - Prefer fixing the underlying issue over disabling the lint rule + - Document any approved lint rule disabling with a comment explaining the reason +3. Logging Guidelines: + - Always instrument code changes using the logger exported from `src\utils\logging\index.ts`. + - This will facilitate efficient debugging without impacting production (as the logger no-ops outside of a test environment.) + - Logs can be found in `logs\app.log` + - Logfile is overwritten on each run to keep it to a manageable volume. # Adding a New Setting To add a new setting that persists its state, follow the steps in cline_docs/settings.md + +The `.clineignore` file allows users to specify files and directories that Cline should not access. When implementing new features, respect the `.clineignore` rules and ensure that your code does not attempt to read or modify ignored files. diff --git a/package-lock.json b/package-lock.json index 63a4c4c90d2..19081528a21 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "seen18", - "version": "1.0.3", + "version": "1.0.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "seen18", - "version": "1.0.3", + "version": "1.0.4", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", @@ -62,6 +62,7 @@ "@types/jest": "^29.5.14", "@types/mocha": "^10.0.7", "@types/node": "20.x", + "@types/should": "^13.0.0", "@types/string-similarity": "^4.0.2", "@typescript-eslint/eslint-plugin": "^7.14.1", "@typescript-eslint/parser": "^7.11.0", @@ -77,6 +78,7 @@ "npm-run-all": "^4.1.5", "prettier": "^3.4.2", "rimraf": "^6.0.1", + "should": "^13.2.3", "svgtofont": "^6.2.0", "ts-jest": "^29.2.5", "typescript": "^5.4.5" @@ -6057,6 +6059,16 @@ "@types/node": "*" } }, + "node_modules/@types/should": { + "version": "13.0.0", + "resolved": "https://registry.npmjs.org/@types/should/-/should-13.0.0.tgz", + "integrity": "sha512-Mi6YZ2ABnnGGFMuiBDP0a8s1ZDCDNHqP97UH8TyDmCWuGGavpsFMfJnAMYaaqmDlSCOCNbVLHBrSDEOpx/oLhw==", + "deprecated": "This is a stub types definition for should.js (https://github.com/shouldjs/should.js). should.js provides its own type definitions, so you don't need @types/should installed!", + "dev": true, + "dependencies": { + "should": "*" + } + }, "node_modules/@types/stack-utils": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-2.0.3.tgz", @@ -14801,6 +14813,60 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/should": { + "version": "13.2.3", + "resolved": "https://registry.npmjs.org/should/-/should-13.2.3.tgz", + "integrity": "sha512-ggLesLtu2xp+ZxI+ysJTmNjh2U0TsC+rQ/pfED9bUZZ4DKefP27D+7YJVVTvKsmjLpIi9jAa7itwDGkDDmt1GQ==", + "dev": true, + "dependencies": { + "should-equal": "^2.0.0", + "should-format": "^3.0.3", + "should-type": "^1.4.0", + "should-type-adaptors": "^1.0.1", + "should-util": "^1.0.0" + } + }, + "node_modules/should-equal": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/should-equal/-/should-equal-2.0.0.tgz", + "integrity": "sha512-ZP36TMrK9euEuWQYBig9W55WPC7uo37qzAEmbjHz4gfyuXrEUgF8cUvQVO+w+d3OMfPvSRQJ22lSm8MQJ43LTA==", + "dev": true, + "dependencies": { + "should-type": "^1.4.0" + } + }, + "node_modules/should-format": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/should-format/-/should-format-3.0.3.tgz", + "integrity": "sha512-hZ58adtulAk0gKtua7QxevgUaXTTXxIi8t41L3zo9AHvjXO1/7sdLECuHeIN2SRtYXpNkmhoUP2pdeWgricQ+Q==", + "dev": true, + "dependencies": { + "should-type": "^1.3.0", + "should-type-adaptors": "^1.0.1" + } + }, + "node_modules/should-type": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/should-type/-/should-type-1.4.0.tgz", + "integrity": "sha512-MdAsTu3n25yDbIe1NeN69G4n6mUnJGtSJHygX3+oN0ZbO3DTiATnf7XnYJdGT42JCXurTb1JI0qOBR65shvhPQ==", + "dev": true + }, + "node_modules/should-type-adaptors": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/should-type-adaptors/-/should-type-adaptors-1.1.0.tgz", + "integrity": "sha512-JA4hdoLnN+kebEp2Vs8eBe9g7uy0zbRo+RMcU0EsNy+R+k049Ki+N5tT5Jagst2g7EAja+euFuoXFCa8vIklfA==", + "dev": true, + "dependencies": { + "should-type": "^1.3.0", + "should-util": "^1.0.0" + } + }, + "node_modules/should-util": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/should-util/-/should-util-1.0.1.tgz", + "integrity": "sha512-oXF8tfxx5cDk8r2kYqlkUJzZpDBqVY/II2WhvU0n9Y3XYvAYRmeaf1PvvIvTgPnv4KJ+ES5M0PyDq5Jp+Ygy2g==", + "dev": true + }, "node_modules/side-channel": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/side-channel/-/side-channel-1.0.6.tgz", diff --git a/package.json b/package.json index 691fdb8f6d2..3efaad98419 100644 --- a/package.json +++ b/package.json @@ -367,6 +367,7 @@ "@types/jest": "^29.5.14", "@types/mocha": "^10.0.7", "@types/node": "20.x", + "@types/should": "^13.0.0", "@types/string-similarity": "^4.0.2", "@typescript-eslint/eslint-plugin": "^7.14.1", "@typescript-eslint/parser": "^7.11.0", @@ -382,6 +383,7 @@ "npm-run-all": "^4.1.5", "prettier": "^3.4.2", "rimraf": "^6.0.1", + "should": "^13.2.3", "svgtofont": "^6.2.0", "ts-jest": "^29.2.5", "typescript": "^5.4.5" diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 82479053671..e15b8f151cc 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -51,6 +51,7 @@ import { fileExistsAtPath } from "../utils/fs" import { arePathsEqual, getReadablePath } from "../utils/path" import { parseMentions } from "./mentions" import { AssistantMessageContent, parseAssistantMessage, ToolParamName, ToolUseName } from "./assistant-message" +import { ClineIgnoreController, LOCK_TEXT_SYMBOL } from "./ignore/ClineIgnoreController" import { formatResponse } from "./prompts/responses" import { SYSTEM_PROMPT } from "./prompts/system" import { modes, defaultModeSlug, getModeBySlug } from "../shared/modes" @@ -86,6 +87,7 @@ export class Cline { apiConversationHistory: (Anthropic.MessageParam & { ts?: number })[] = [] clineMessages: ClineMessage[] = [] + private clineIgnoreController: ClineIgnoreController private askResponse?: ClineAskResponse private askResponseText?: string private askResponseImages?: string[] @@ -129,6 +131,11 @@ export class Cline { historyItem?: HistoryItem | undefined, experiments?: Record, ) { + this.clineIgnoreController = new ClineIgnoreController(cwd) + this.clineIgnoreController.initialize().catch((error) => { + console.error("Failed to initialize ClineIgnoreController:", error) + }) + if (!task && !images && !historyItem) { throw new Error("Either historyItem or task/images must be provided") } @@ -732,6 +739,7 @@ export class Cline { this.browserSession.closeBrowser() // Need to await for when we want to make sure directories/files are // reverted before re-starting the task from a checkpoint. + this.clineIgnoreController.dispose() await this.diffViewProvider.revertChanges() } @@ -864,6 +872,14 @@ export class Cline { }) } + const clineIgnoreContent = this.clineIgnoreController.clineIgnoreContent + let clineIgnoreInstructions: string | undefined + if (clineIgnoreContent) { + clineIgnoreInstructions = `# .clineignore\n\n(The following is provided by a root-level .clineignore file where the user has specified files and directories that should not be accessed. When using list_files, you'll notice a ${LOCK_TEXT_SYMBOL} next to files that are blocked. Attempting to access the file's contents e.g. through read_file will result in an error.)\n\n${clineIgnoreContent}\n.clineignore` + // altering the system prompt mid-task will break the prompt cache, but in the grand scheme this will not change often so it's better to not pollute user messages with it the way we have to with + this.customInstructions += clineIgnoreInstructions + } + const { browserViewportSize, mode, @@ -944,6 +960,7 @@ export class Cline { } return { role, content } }) + const stream = this.api.createMessage(systemPrompt, cleanConversationHistory) const iterator = stream[Symbol.asyncIterator]() @@ -1257,6 +1274,16 @@ export class Cline { // wait so we can determine if it's a new file or editing an existing file break } + + // check if file is allowed to be accessed + const accessAllowed = this.clineIgnoreController.validateAccess(relPath) + if (!accessAllowed) { + await this.say("clineignore_error", relPath) + pushToolResult(formatResponse.toolError(formatResponse.clineIgnoreError(relPath))) + + break + } + // Check if file exists using cached map or fs.access let fileExists: boolean if (this.diffViewProvider.editType !== undefined) { @@ -1331,6 +1358,15 @@ export class Cline { await this.diffViewProvider.reset() break } + + const accessAllowed = this.clineIgnoreController.validateAccess(relPath) + if (!accessAllowed) { + await this.say("clineignore_error", relPath) + pushToolResult(formatResponse.toolError(formatResponse.clineIgnoreError(relPath))) + + break + } + this.consecutiveMistakeCount = 0 // if isEditingFile false, that means we have the full contents of the file already. @@ -1944,7 +1980,12 @@ export class Cline { 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.clineIgnoreController, + ) const completeMessage = JSON.stringify({ ...sharedMessageProps, content: result, @@ -1985,7 +2026,10 @@ export class Cline { } this.consecutiveMistakeCount = 0 const absolutePath = path.resolve(cwd, relDirPath) - const result = await parseSourceCodeForDefinitionsTopLevel(absolutePath) + const result = await parseSourceCodeForDefinitionsTopLevel( + absolutePath, + this.clineIgnoreController, + ) const completeMessage = JSON.stringify({ ...sharedMessageProps, content: result, @@ -2033,7 +2077,13 @@ export class Cline { } this.consecutiveMistakeCount = 0 const absolutePath = path.resolve(cwd, relDirPath) - const results = await regexSearchFiles(cwd, absolutePath, regex, filePattern) + const results = await regexSearchFiles( + cwd, + absolutePath, + regex, + filePattern, + this.clineIgnoreController, + ) const completeMessage = JSON.stringify({ ...sharedMessageProps, content: results, @@ -2214,6 +2264,18 @@ export class Cline { } this.consecutiveMistakeCount = 0 + const ignoredFileAttemptedToAccess = this.clineIgnoreController.validateCommand(command) + if (ignoredFileAttemptedToAccess) { + await this.say("clineignore_error", ignoredFileAttemptedToAccess) + pushToolResult( + formatResponse.toolError( + formatResponse.clineIgnoreError(ignoredFileAttemptedToAccess), + ), + ) + + break + } + const didApprove = await askApproval("command", command) if (!didApprove) { break @@ -3067,26 +3129,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()) + + // Filter paths through clineIgnoreController + const allowedVisibleFiles = this.clineIgnoreController + .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()) + + // Filter paths through clineIgnoreController + const allowedOpenTabs = this.clineIgnoreController + .filterPaths(openTabPaths) + .map((p) => p.toPosix()) .join("\n") - if (openTabs) { - details += `\n${openTabs}` + + if (allowedOpenTabs) { + details += `\n${allowedOpenTabs}` } else { details += "\n(No open tabs)" } @@ -3225,11 +3299,11 @@ 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.clineIgnoreController) details += result } } - + console.log("Environment details:", details) return `\n${details.trim()}\n` } diff --git a/src/core/ignore/ClineIgnoreController.ts b/src/core/ignore/ClineIgnoreController.ts new file mode 100644 index 00000000000..ebd08788e7b --- /dev/null +++ b/src/core/ignore/ClineIgnoreController.ts @@ -0,0 +1,189 @@ +import path from "path" +import { fileExistsAtPath } from "../../utils/fs" +import fs from "fs/promises" +import ignore, { Ignore } from "ignore" +import * as vscode from "vscode" + +export const LOCK_TEXT_SYMBOL = "\u{1F512}" + +/** + * Controls LLM access to files by enforcing ignore patterns. + * Designed to be instantiated once in Cline.ts and passed to file manipulation services. + * Uses the 'ignore' library to support standard .gitignore syntax in .clineignore files. + */ +export class ClineIgnoreController { + private cwd: string + private ignoreInstance: Ignore + private disposables: vscode.Disposable[] = [] + clineIgnoreContent: string | undefined + + constructor(cwd: string) { + this.cwd = cwd + this.ignoreInstance = ignore() + this.clineIgnoreContent = undefined + // Set up file watcher for .clineignore + this.setupFileWatcher() + } + + /** + * Initialize the controller by loading custom patterns + * Must be called after construction and before using the controller + */ + async initialize(): Promise { + await this.loadClineIgnore() + } + + /** + * Set up the file watcher for .clineignore changes + */ + private setupFileWatcher(): void { + const clineignorePattern = new vscode.RelativePattern(this.cwd, ".clineignore") + const fileWatcher = vscode.workspace.createFileSystemWatcher(clineignorePattern) + + // Watch for changes and updates + this.disposables.push( + fileWatcher.onDidChange(() => { + this.loadClineIgnore() + }), + fileWatcher.onDidCreate(() => { + this.loadClineIgnore() + }), + fileWatcher.onDidDelete(() => { + this.loadClineIgnore() + }), + ) + + // Add fileWatcher itself to disposables + this.disposables.push(fileWatcher) + } + + /** + * Load custom patterns from .clineignore if it exists + */ + private async loadClineIgnore(): Promise { + try { + // Reset ignore instance to prevent duplicate patterns + this.ignoreInstance = ignore() + const ignorePath = path.join(this.cwd, ".clineignore") + if (await fileExistsAtPath(ignorePath)) { + const content = await fs.readFile(ignorePath, "utf8") + this.clineIgnoreContent = content + this.ignoreInstance.add(content) + this.ignoreInstance.add(".clineignore") + } else { + this.clineIgnoreContent = undefined + } + } catch (error) { + // Should never happen: reading file failed even though it exists + console.error("Unexpected error loading .clineignore:", error) + } + } + + /** + * Check if a file should be accessible to the LLM + * @param filePath - Path to check (relative to cwd) + * @returns true if file is accessible, false if ignored + */ + validateAccess(filePath: string): boolean { + // Always allow access if .clineignore does not exist + if (!this.clineIgnoreContent) { + return true + } + try { + // Normalize path to be relative to cwd and use forward slashes + const absolutePath = path.resolve(this.cwd, filePath) + const relativePath = path.relative(this.cwd, absolutePath).toPosix() + + // Ignore expects paths to be path.relative()'d + return !this.ignoreInstance.ignores(relativePath) + } catch (error) { + // console.error(`Error validating access for ${filePath}:`, error) + // Ignore is designed to work with relative file paths, so will throw error for paths outside cwd. We are allowing access to all files outside cwd. + return true + } + } + + /** + * Check if a terminal command should be allowed to execute based on file access patterns + * @param command - Terminal command to validate + * @returns path of file that is being accessed if it is being accessed, undefined if command is allowed + */ + validateCommand(command: string): string | undefined { + // Always allow if no .clineignore exists + if (!this.clineIgnoreContent) { + return undefined + } + + // Split command into parts and get the base command + const parts = command.trim().split(/\s+/) + const baseCommand = parts[0].toLowerCase() + + // Commands that read file contents + const fileReadingCommands = [ + // Unix commands + "cat", + "less", + "more", + "head", + "tail", + "grep", + "awk", + "sed", + // PowerShell commands and aliases + "get-content", + "gc", + "type", + "select-string", + "sls", + ] + + if (fileReadingCommands.includes(baseCommand)) { + // Check each argument that could be a file path + for (let i = 1; i < parts.length; i++) { + const arg = parts[i] + // Skip command flags/options (both Unix and PowerShell style) + if (arg.startsWith("-") || arg.startsWith("/")) { + continue + } + // Ignore PowerShell parameter names + if (arg.includes(":")) { + continue + } + // Validate file access + if (!this.validateAccess(arg)) { + return arg + } + } + } + + 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 = [] + } +} diff --git a/src/core/ignore/__tests__/ClineIgnoreController.test.ts b/src/core/ignore/__tests__/ClineIgnoreController.test.ts new file mode 100644 index 00000000000..f0d440c4e00 --- /dev/null +++ b/src/core/ignore/__tests__/ClineIgnoreController.test.ts @@ -0,0 +1,253 @@ +import { ClineIgnoreController } from "../ClineIgnoreController" +import fs from "fs/promises" +import path from "path" +import os from "os" +import { after, beforeEach, describe, it } from "mocha" + +import "should" + +describe("ClineIgnoreController", () => { + let tempDir: string + let controller: ClineIgnoreController + + beforeEach(async () => { + // Create a temp directory for testing + tempDir = path.join(os.tmpdir(), `llm-test-${Date.now()}-${Math.random().toString(36).slice(2)}`) + await fs.mkdir(tempDir) + + // Create default .clineignore file + await fs.writeFile( + path.join(tempDir, ".clineignore"), + [ + ".env", + "*.secret", + "private/", + "# This is a comment", + "", + "temp.*", + "file-with-space-at-end.* ", + "**/.git/**", + ].join("\n"), + ) + + controller = new ClineIgnoreController(tempDir) + await controller.initialize() + }) + + after(async () => { + // Clean up temp directory + await fs.rm(tempDir, { recursive: true, force: true }) + }) + + describe("Default Patterns", () => { + // it("should block access to common ignored files", async () => { + // 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 = [ + controller.validateAccess("src/index.ts"), + controller.validateAccess("README.md"), + controller.validateAccess("package.json"), + ] + results.forEach((result) => result.should.be.true()) + }) + + it("should block access to .clineignore file", async () => { + const result = controller.validateAccess(".clineignore") + result.should.be.false() + }) + }) + + describe("Custom Patterns", () => { + it("should block access to custom ignored patterns", async () => { + 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 = [ + 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()) + }) + + it("should handle pattern edge cases", async () => { + await fs.writeFile( + path.join(tempDir, ".clineignore"), + ["*.secret", "private/", "*.tmp", "data-*.json", "temp/*"].join("\n"), + ) + + controller = new ClineIgnoreController(tempDir) + await controller.initialize() + + 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 + results[2].should.be.false() // script.tmp + }) + + // ToDo: handle negation patterns successfully + + // it("should handle negation patterns", async () => { + // await fs.writeFile( + // path.join(tempDir, ".clineignore"), + // [ + // "temp/*", // Ignore everything in temp + // "!temp/allowed/*", // But allow files in temp/allowed + // "docs/**/*.md", // Ignore all markdown files in docs + // "!docs/README.md", // Except README.md + // "!docs/CONTRIBUTING.md", // And CONTRIBUTING.md + // "assets/", // Ignore all assets + // "!assets/public/", // Except public assets + // "!assets/public/*.png", // Specifically allow PNGs in public assets + // ].join("\n"), + // ) + + // controller = new ClineIgnoreController(tempDir) + + // const results = [ + // // Basic negation + // controller.validateAccess("temp/file.txt"), // Should be false (in temp/) + // controller.validateAccess("temp/allowed/file.txt"), // Should be true (negated) + // controller.validateAccess("temp/allowed/nested/file.txt"), // Should be true (negated with nested) + + // // Multiple negations in same path + // controller.validateAccess("docs/guide.md"), // Should be false (matches docs/**/*.md) + // controller.validateAccess("docs/README.md"), // Should be true (negated) + // controller.validateAccess("docs/CONTRIBUTING.md"), // Should be true (negated) + // controller.validateAccess("docs/api/guide.md"), // Should be false (nested markdown) + + // // Nested negations + // 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 + // results[2].should.be.true() // temp/allowed/nested/file.txt + // results[3].should.be.false() // docs/guide.md + // results[4].should.be.true() // docs/README.md + // results[5].should.be.true() // docs/CONTRIBUTING.md + // results[6].should.be.false() // docs/api/guide.md + // results[7].should.be.false() // assets/logo.png + // results[8].should.be.true() // assets/public/logo.png + // results[9].should.be.true() // assets/public/data.json + // }) + + it("should handle comments in .clineignore", async () => { + // Create a new .clineignore with comments + await fs.writeFile( + path.join(tempDir, ".clineignore"), + ["# Comment line", "*.secret", "private/", "temp.*"].join("\n"), + ) + + controller = new ClineIgnoreController(tempDir) + await controller.initialize() + + const result = controller.validateAccess("test.secret") + result.should.be.false() + }) + }) + + describe("Path Handling", () => { + 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 = 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 = controller.validateAccess(ignoredPath) + ignoredResult.should.be.false() + + // Test absolute path in ignored directory (private/) + const ignoredDirPath = path.join(tempDir, "private/data.txt") + 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 = controller.validateAccess("./src/file.ts") + allowedResult.should.be.true() + + // Test relative path that matches an ignore pattern (*.secret) + const ignoredResult = controller.validateAccess("./config.secret") + ignoredResult.should.be.false() + + // Test relative path in ignored directory (private/) + const ignoredDirResult = controller.validateAccess("./private/data.txt") + ignoredDirResult.should.be.false() + }) + + it("should normalize paths with backslashes", async () => { + const result = controller.validateAccess("src\\file.ts") + result.should.be.true() + }) + }) + + describe("Batch Filtering", () => { + it("should filter an array of paths", async () => { + const paths = ["src/index.ts", ".env", "lib/utils.ts", ".git/config", "dist/bundle.js"] + + const filtered = controller.filterPaths(paths) + filtered.should.deepEqual(["src/index.ts", "lib/utils.ts", "dist/bundle.js"]) + }) + }) + + describe("Error Handling", () => { + it("should handle invalid paths", async () => { + // Test with an invalid path containing null byte + const result = controller.validateAccess("\0invalid") + result.should.be.true() + }) + + it("should handle missing .clineignore gracefully", async () => { + // Create a new controller in a directory without .clineignore + const emptyDir = path.join(os.tmpdir(), `llm-test-empty-${Date.now()}`) + await fs.mkdir(emptyDir) + + try { + const controller = new ClineIgnoreController(emptyDir) + await controller.initialize() + const result = controller.validateAccess("file.txt") + result.should.be.true() + } finally { + await fs.rm(emptyDir, { recursive: true, force: true }) + } + }) + + it("should handle empty .clineignore", async () => { + await fs.writeFile(path.join(tempDir, ".clineignore"), "") + + controller = new ClineIgnoreController(tempDir) + await controller.initialize() + + const result = controller.validateAccess("regular-file.txt") + result.should.be.true() + }) + }) +}) diff --git a/src/core/prompts/responses.ts b/src/core/prompts/responses.ts index f06dff3d88b..6dff08dca63 100644 --- a/src/core/prompts/responses.ts +++ b/src/core/prompts/responses.ts @@ -1,6 +1,7 @@ import { Anthropic } from "@anthropic-ai/sdk" import * as path from "path" import * as diff from "diff" +import { ClineIgnoreController, LOCK_TEXT_SYMBOL } from "../ignore/ClineIgnoreController" export const formatResponse = { toolDenied: () => `The user denied this operation.`, @@ -13,6 +14,9 @@ export const formatResponse = { toolError: (error?: string) => `The tool execution failed with the following error:\n\n${error}\n`, + clineIgnoreError: (path: string) => + `Access to ${path} is blocked by the .clineignore file settings. You must try to continue in the task without using this file, or ask the user to update the .clineignore file.`, + noToolsUsed: () => `[ERROR] You did not use a tool in your previous response! Please retry with a tool use. @@ -52,7 +56,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, + clineIgnoreController?: ClineIgnoreController, + ): string => { const sorted = files .map((file) => { // convert absolute path to relative path @@ -80,6 +89,22 @@ 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 clineIgnoreParsed = clineIgnoreController + ? 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 = !clineIgnoreController.validateAccess(absoluteFilePath) + if (isIgnored) { + return LOCK_TEXT_SYMBOL + " " + filePath + } + + return filePath + }) + : sorted + if (didHitLimit) { return `${sorted.join( "\n", diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index e5d89fa0d05..6e5579cf4e0 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -131,6 +131,7 @@ export const GlobalFileNames = { openRouterModels: "openrouter_models.json", mcpSettings: "cline_mcp_settings.json", unboundModels: "unbound_models.json", + clineRules: ".clinerules", } export class ClineProvider implements vscode.WebviewViewProvider { diff --git a/src/services/glob/list-files.ts b/src/services/glob/list-files.ts index 8578b914d72..231ea4d555c 100644 --- a/src/services/glob/list-files.ts +++ b/src/services/glob/list-files.ts @@ -46,8 +46,10 @@ export async function listFiles(dirPath: string, recursive: boolean, limit: numb 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/ripgrep/index.ts b/src/services/ripgrep/index.ts index b48c60b5b2e..99b9db5b21b 100644 --- a/src/services/ripgrep/index.ts +++ b/src/services/ripgrep/index.ts @@ -3,6 +3,7 @@ import * as childProcess from "child_process" import * as path from "path" import * as fs from "fs" import * as readline from "readline" +import { ClineIgnoreController } from "../../core/ignore/ClineIgnoreController" /* This file provides functionality to perform regex searches on files using ripgrep. @@ -50,7 +51,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 @@ -127,6 +128,7 @@ export async function regexSearchFiles( directoryPath: string, regex: string, filePattern?: string, + clineIgnoreController?: ClineIgnoreController, ): Promise { const vscodeAppRoot = vscode.env.appRoot const rgPath = await getBinPath(vscodeAppRoot) @@ -155,7 +157,7 @@ export async function regexSearchFiles( 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, @@ -178,8 +180,12 @@ export async function regexSearchFiles( if (currentResult) { results.push(currentResult as SearchResult) } + // Filter results using ClineIgnoreController if provided + const filteredResults = clineIgnoreController + ? results.filter((result) => clineIgnoreController.validateAccess(result.filepath)) + : results - return formatResults(results, cwd) + return formatResults(filteredResults, cwd) } function formatResults(results: SearchResult[], cwd: string): string { @@ -194,7 +200,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 83e02ac6158..72132a47292 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 { ClineIgnoreController } from "../../core/ignore/ClineIgnoreController" // 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, + clineIgnoreController?: ClineIgnoreController, +): Promise { // check if the path exists const dirExists = await fileExistsAtPath(path.resolve(dirPath)) if (!dirExists) { @@ -22,10 +26,13 @@ export async function parseSourceCodeForDefinitionsTopLevel(dirPath: string): Pr const languageParsers = await loadRequiredLanguageParsers(filesToParse) + // Filter filepaths for access if controller is provided + const allowedFilesToParse = clineIgnoreController ? clineIgnoreController.filterPaths(filesToParse) : filesToParse + // Parse specific files we have language parsers for // const filesWithoutDefinitions: string[] = [] for (const file of filesToParse) { - const definitions = await parseFile(file, languageParsers) + const definitions = await parseFile(file, languageParsers, clineIgnoreController) if (definitions) { result += `${path.relative(dirPath, file).toPosix()}\n${definitions}\n` } @@ -95,7 +102,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, + clineIgnoreController?: ClineIgnoreController, +): Promise { + if (clineIgnoreController && !clineIgnoreController.validateAccess(filePath)) { + return null + } const fileContent = await fs.readFile(filePath, "utf8") const ext = path.extname(filePath).toLowerCase().slice(1) @@ -156,5 +170,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/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index f22ad958a04..7c8aa41ec88 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -175,6 +175,7 @@ export type ClineSay = | "mcp_server_response" | "new_task_started" | "new_task" + | "clineignore_error" | "checkpoint_saved" export interface ClineSayTool { diff --git a/src/utils/__tests__/git.test.ts b/src/utils/__tests__/git.test.ts index 7fe59a10edf..629849c2360 100644 --- a/src/utils/__tests__/git.test.ts +++ b/src/utils/__tests__/git.test.ts @@ -107,7 +107,7 @@ describe("git utils", () => { { cwd }, expect.any(Function), ) - }, 20000) + }) it("should return empty array when git is not installed", async () => { exec.mockImplementation((command: string, options: { cwd?: string }, callback: Function) => { From 927cfb15363850c3d2bb7ea0cfea867287001ace Mon Sep 17 00:00:00 2001 From: Sara Hamza Date: Mon, 17 Mar 2025 01:40:55 +0200 Subject: [PATCH 2/2] fix read_file command in Cline.ts and fix responses.ts --- src/core/Cline.ts | 6 ++++++ src/core/prompts/responses.ts | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index e15b8f151cc..c8a4b15adfc 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -1935,6 +1935,12 @@ export class Cline { pushToolResult(await this.sayAndCreateMissingParamError("read_file", "path")) break } + const accessAllowed = this.clineIgnoreController.validateAccess(relPath) + if (!accessAllowed) { + await this.say("clineignore_error", relPath) + pushToolResult(formatResponse.toolError(formatResponse.clineIgnoreError(relPath))) + break + } this.consecutiveMistakeCount = 0 const absolutePath = path.resolve(cwd, relPath) const completeMessage = JSON.stringify({ diff --git a/src/core/prompts/responses.ts b/src/core/prompts/responses.ts index 6dff08dca63..d9c945d5a44 100644 --- a/src/core/prompts/responses.ts +++ b/src/core/prompts/responses.ts @@ -106,13 +106,13 @@ Otherwise, if you have not completed the task and do not need additional informa : sorted if (didHitLimit) { - return `${sorted.join( + return `${clineIgnoreParsed.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 (clineIgnoreParsed.length === 0 || (clineIgnoreParsed.length === 1 && clineIgnoreParsed[0] === "")) { return "No files found." } else { - return sorted.join("\n") + return clineIgnoreParsed.join("\n") } },