diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts index 1077b7bf39..e759fc081d 100644 --- a/src/core/tools/applyDiffTool.ts +++ b/src/core/tools/applyDiffTool.ts @@ -5,7 +5,7 @@ import { TelemetryService } from "@roo-code/telemetry" import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" import { ClineSayTool } from "../../shared/ExtensionMessage" -import { getReadablePath } from "../../utils/path" +import { getReadablePath, validateFilePath, sanitizeFilePath } from "../../utils/path" import { Task } from "../task/Task" import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } from "../../shared/tools" import { formatResponse } from "../prompts/responses" @@ -69,6 +69,26 @@ export async function applyDiffToolLegacy( return } + // Validate that the path doesn't contain URLs + const pathValidation = validateFilePath(relPath) + if (!pathValidation.isValid) { + cline.consecutiveMistakeCount++ + cline.recordToolError("apply_diff") + const errorMessage = pathValidation.error || `Invalid file path: ${relPath}` + + // Try to suggest a sanitized version if possible + const sanitized = sanitizeFilePath(relPath) + let suggestion = "" + if (sanitized && sanitized !== relPath) { + suggestion = `\n\nDid you mean to use this path instead? ${sanitized}` + } + + const formattedError = `${errorMessage}${suggestion}\n\n\nThe file path appears to contain a URL or invalid characters. Please provide a valid relative or absolute file path.\n` + await cline.say("error", formattedError) + pushToolResult(formattedError) + return + } + const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath) if (!accessAllowed) { diff --git a/src/core/tools/multiApplyDiffTool.ts b/src/core/tools/multiApplyDiffTool.ts index 08bce08ede..501f0e0046 100644 --- a/src/core/tools/multiApplyDiffTool.ts +++ b/src/core/tools/multiApplyDiffTool.ts @@ -5,7 +5,7 @@ import { TelemetryService } from "@roo-code/telemetry" import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" import { ClineSayTool } from "../../shared/ExtensionMessage" -import { getReadablePath } from "../../utils/path" +import { getReadablePath, validateFilePath, sanitizeFilePath } from "../../utils/path" import { Task } from "../task/Task" import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } from "../../shared/tools" import { formatResponse } from "../prompts/responses" @@ -120,6 +120,19 @@ export async function applyDiffTool( const filePath = file.path + // Validate that the path doesn't contain URLs + const pathValidation = validateFilePath(filePath) + if (!pathValidation.isValid) { + // Try to suggest a sanitized version if possible + const sanitized = sanitizeFilePath(filePath) + let errorMsg = pathValidation.error || `Invalid file path: ${filePath}` + if (sanitized && sanitized !== filePath) { + errorMsg += ` Did you mean to use: ${sanitized}?` + } + filteredOperationErrors.push(errorMsg) + continue // Skip this file + } + // Initialize the operation in the map if it doesn't exist if (!operationsMap[filePath]) { operationsMap[filePath] = { @@ -179,6 +192,27 @@ Original error: ${errorMessage}` } else if (legacyPath && typeof legacyDiffContent === "string") { // Handle legacy parameters (old way) usingLegacyParams = true + + // Validate the legacy path + const pathValidation = validateFilePath(legacyPath) + if (!pathValidation.isValid) { + cline.consecutiveMistakeCount++ + cline.recordToolError("apply_diff") + + // Try to suggest a sanitized version if possible + const sanitized = sanitizeFilePath(legacyPath) + let suggestion = "" + if (sanitized && sanitized !== legacyPath) { + suggestion = `\n\nDid you mean to use this path instead? ${sanitized}` + } + + const formattedError = `${pathValidation.error || `Invalid file path: ${legacyPath}`}${suggestion}\n\n\nThe file path appears to contain a URL or invalid characters. Please provide a valid relative or absolute file path.\n` + await cline.say("error", formattedError) + pushToolResult(formattedError) + cline.processQueuedMessages() + return + } + operationsMap[legacyPath] = { path: legacyPath, diff: [ @@ -242,6 +276,16 @@ Original error: ${errorMessage}` for (const operation of operations) { const { path: relPath, diff: diffItems } = operation + // Additional validation before processing (in case path was modified after initial validation) + const pathValidation = validateFilePath(relPath) + if (!pathValidation.isValid) { + updateOperationResult(relPath, { + status: "blocked", + error: pathValidation.error || `Invalid file path: ${relPath}`, + }) + continue + } + // Verify file access is allowed const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath) if (!accessAllowed) { diff --git a/src/utils/__tests__/path.spec.ts b/src/utils/__tests__/path.spec.ts index a8cf84b68c..4fcea2ab5f 100644 --- a/src/utils/__tests__/path.spec.ts +++ b/src/utils/__tests__/path.spec.ts @@ -3,7 +3,7 @@ import os from "os" import * as path from "path" -import { arePathsEqual, getReadablePath, getWorkspacePath } from "../path" +import { arePathsEqual, getReadablePath, getWorkspacePath, isUrl, sanitizeFilePath, validateFilePath } from "../path" // Mock modules @@ -123,6 +123,152 @@ describe("Path Utilities", () => { expect(arePathsEqual("C:\\", "C:\\")).toBe(true) }) }) + + describe("isUrl", () => { + it("should detect HTTP URLs", () => { + expect(isUrl("http://example.com")).toBe(true) + expect(isUrl("https://example.com")).toBe(true) + expect(isUrl("https://www.google.com/search?q=package.json")).toBe(true) + }) + + it("should detect other protocol URLs", () => { + expect(isUrl("ftp://example.com")).toBe(true) + expect(isUrl("file://localhost/path")).toBe(true) + expect(isUrl("ssh://git@github.com")).toBe(true) + expect(isUrl("ws://localhost:8080")).toBe(true) + expect(isUrl("wss://secure.example.com")).toBe(true) + }) + + it("should not detect regular file paths as URLs", () => { + expect(isUrl("package.json")).toBe(false) + expect(isUrl("/usr/local/bin")).toBe(false) + expect(isUrl("C:\\Windows\\System32")).toBe(false) + expect(isUrl("./src/index.ts")).toBe(false) + expect(isUrl("../parent/file.txt")).toBe(false) + }) + + it("should handle edge cases", () => { + expect(isUrl("")).toBe(false) + expect(isUrl(null as any)).toBe(false) + expect(isUrl(undefined as any)).toBe(false) + expect(isUrl(123 as any)).toBe(false) + }) + + it("should be case-insensitive for protocols", () => { + expect(isUrl("HTTP://example.com")).toBe(true) + expect(isUrl("HTTPS://example.com")).toBe(true) + expect(isUrl("FTP://example.com")).toBe(true) + }) + }) + + describe("sanitizeFilePath", () => { + it("should return null for complete URLs", () => { + expect(sanitizeFilePath("https://www.google.com/search?q=package.json")).toBe(null) + expect(sanitizeFilePath("http://example.com/file.txt")).toBe(null) + expect(sanitizeFilePath("ftp://server.com/path")).toBe(null) + }) + + it("should extract file path from concatenated URL and path", () => { + expect(sanitizeFilePath("package.json\nhttps://google.com")).toBe("package.json") + expect(sanitizeFilePath("src/file.ts https://example.com")).toBe("src/file.ts") + // Note: The comma case returns with the comma because it's checking if there's a URL after it + expect(sanitizeFilePath("./config.json,https://api.example.com")).toBe("./config.json") + expect(sanitizeFilePath("test.txt;https://example.com")).toBe("test.txt") + }) + + it("should handle paths with URLs concatenated without separators", () => { + expect(sanitizeFilePath("package.jsonhttps://google.com")).toBe("package.json") + expect(sanitizeFilePath("src/file.tshttps://example.com/api")).toBe("src/file.ts") + expect(sanitizeFilePath("config.yamlftp://server.com")).toBe("config.yaml") + }) + + it("should return the original path if no URL is present", () => { + expect(sanitizeFilePath("package.json")).toBe("package.json") + expect(sanitizeFilePath("/usr/local/bin/app")).toBe("/usr/local/bin/app") + expect(sanitizeFilePath("C:\\Windows\\System32\\cmd.exe")).toBe("C:\\Windows\\System32\\cmd.exe") + }) + + it("should trim whitespace", () => { + expect(sanitizeFilePath(" package.json ")).toBe("package.json") + expect(sanitizeFilePath("\t./src/index.ts\n")).toBe("./src/index.ts") + }) + + it("should handle edge cases", () => { + expect(sanitizeFilePath("")).toBe(null) + expect(sanitizeFilePath(null as any)).toBe(null) + expect(sanitizeFilePath(undefined as any)).toBe(null) + expect(sanitizeFilePath(123 as any)).toBe(null) + }) + + it("should handle Windows paths with URLs", () => { + // The trailing backslash is removed as it's considered a separator before the URL + expect( + sanitizeFilePath( + "d:\\01_Proyectos\\SUPER-ADMIN\\package.json\\https:\\www.google.com\\search?q=package.json", + ), + ).toBe("d:\\01_Proyectos\\SUPER-ADMIN\\package.json") + expect(sanitizeFilePath("C:\\Users\\test\\file.txt https://example.com")).toBe( + "C:\\Users\\test\\file.txt", + ) + }) + }) + + describe("validateFilePath", () => { + it("should validate normal file paths", () => { + expect(validateFilePath("package.json")).toEqual({ isValid: true }) + expect(validateFilePath("./src/index.ts")).toEqual({ isValid: true }) + expect(validateFilePath("/usr/local/bin/app")).toEqual({ isValid: true }) + expect(validateFilePath("C:\\Windows\\System32\\cmd.exe")).toEqual({ isValid: true }) + }) + + it("should reject complete URLs", () => { + const result = validateFilePath("https://www.google.com/search?q=package.json") + expect(result.isValid).toBe(false) + expect(result.error).toContain("appears to be a URL") + }) + + it("should reject paths with URL components and suggest sanitized version", () => { + const result = validateFilePath("package.json https://google.com") + expect(result.isValid).toBe(false) + expect(result.error).toContain("contains URL components") + expect(result.error).toContain("package.json") + }) + + it("should reject paths with concatenated URLs", () => { + const result = validateFilePath("package.jsonhttps://google.com") + expect(result.isValid).toBe(false) + expect(result.error).toContain("contains URL components") + expect(result.error).toContain("package.json") + }) + + it("should handle empty or invalid inputs", () => { + expect(validateFilePath("").isValid).toBe(false) + expect(validateFilePath("").error).toContain("empty or invalid") + + expect(validateFilePath(null as any).isValid).toBe(false) + expect(validateFilePath(undefined as any).isValid).toBe(false) + expect(validateFilePath(123 as any).isValid).toBe(false) + }) + + it("should handle the exact issue case from bug report", () => { + const bugPath = + "d:\\01_Proyectos\\...\\SUPER-ADMIN\\package.json\\https:\\www.google.com\\search?q=package.json" + const result = validateFilePath(bugPath) + expect(result.isValid).toBe(false) + expect(result.error).toContain("contains URL components") + }) + + it("should accept paths with spaces", () => { + expect(validateFilePath("my folder/my file.txt")).toEqual({ isValid: true }) + expect(validateFilePath("C:\\Program Files\\app.exe")).toEqual({ isValid: true }) + }) + + it("should accept paths with special characters", () => { + expect(validateFilePath("file-name_123.test.ts")).toEqual({ isValid: true }) + expect(validateFilePath("@types/node/index.d.ts")).toEqual({ isValid: true }) + expect(validateFilePath("file[1].txt")).toEqual({ isValid: true }) + }) + }) }) describe("getReadablePath", () => { diff --git a/src/utils/path.ts b/src/utils/path.ts index 48e2ce6673..b05cc0518c 100644 --- a/src/utils/path.ts +++ b/src/utils/path.ts @@ -130,3 +130,118 @@ export const getWorkspacePathForContext = (contextPath?: string): string => { // Fall back to current behavior return getWorkspacePath() } + +/** + * Checks if a string appears to be a URL rather than a file path + * @param path The string to check + * @returns true if the string looks like a URL + */ +export function isUrl(path: string): boolean { + if (!path || typeof path !== "string") { + return false + } + + // Check for common URL protocols + const urlProtocols = /^(https?|ftp|file|data|mailto|tel|ssh|git|ws|wss):\/\//i + return urlProtocols.test(path) +} + +/** + * Sanitizes a file path by removing any URL components if present + * This helps prevent the AI from accidentally including URLs in file paths + * @param filePath The file path to sanitize + * @returns The sanitized file path, or null if the entire path is a URL + */ +export function sanitizeFilePath(filePath: string): string | null { + if (!filePath || typeof filePath !== "string") { + return null + } + + // If the entire path is a URL, return null + if (isUrl(filePath)) { + return null + } + + // First check for paths that have URLs concatenated without separators + // This handles cases like "package.jsonhttps://google.com" or Windows paths with backslashes + // Also handle the specific bug case: "d:\01_Proyectos\...\package.json\https:\www.google.com\..." + // Match URLs with either :// or :\ (for Windows paths like https:\www.google.com) + const urlMatch = filePath.match(/(.*?)(https?|ftp|file|data|mailto|tel|ssh|git|ws|wss):[\\\/]/i) + if (urlMatch && urlMatch[1]) { + // Return the part before the URL, trimming any trailing separators + let cleanPath = urlMatch[1] + // Remove trailing backslash, forward slash, comma, or semicolon if it's there + cleanPath = cleanPath.replace(/[\\\/,;]$/, "") + return cleanPath.trim() || null + } + + // Check if path contains a URL separated by whitespace or special characters + // But be careful not to split on spaces that are part of valid file paths + // Only split if we find actual URL protocols after the separator + const separators = ["\n", "\r", "\t", "|", ",", ";"] + for (const separator of separators) { + const separatorIndex = filePath.indexOf(separator) + if (separatorIndex !== -1) { + // Check if what comes after the separator is a URL + const afterSeparator = filePath.substring(separatorIndex + 1).trim() + if (isUrl(afterSeparator)) { + // Return everything before the separator + const result = filePath.substring(0, separatorIndex).trim() + return result || null + } + } + } + + // Special handling for space separator - only split if there's a URL after the space + if (filePath.includes(" ")) { + // Check if any word after a space starts with a URL protocol + const words = filePath.split(" ") + for (let i = 1; i < words.length; i++) { + if (isUrl(words[i])) { + // Found a URL after a space, return everything before this point + return words.slice(0, i).join(" ").trim() + } + } + } + + // If no URL components found, return the original path + return filePath.trim() +} + +/** + * Validates that a file path is safe to use and doesn't contain URLs + * @param filePath The file path to validate + * @returns An object with isValid boolean and an optional error message + */ +export function validateFilePath(filePath: string): { isValid: boolean; error?: string } { + if (!filePath || typeof filePath !== "string") { + return { isValid: false, error: "File path is empty or invalid" } + } + + // Check if the entire path is a URL + if (isUrl(filePath)) { + return { + isValid: false, + error: `Invalid file path: "${filePath}" appears to be a URL. Please provide a valid file path instead.`, + } + } + + // Try to sanitize the path + const sanitized = sanitizeFilePath(filePath) + if (!sanitized) { + return { + isValid: false, + error: `Invalid file path: "${filePath}" could not be sanitized to a valid path.`, + } + } + + // If sanitization changed the path, it contained URL components + if (sanitized !== filePath.trim()) { + return { + isValid: false, + error: `Invalid file path: "${filePath}" contains URL components. Did you mean "${sanitized}"?`, + } + } + + return { isValid: true } +}