diff --git a/src/services/checkpoints/ShadowCheckpointService.ts b/src/services/checkpoints/ShadowCheckpointService.ts index be2c86852ab..0b512b7e903 100644 --- a/src/services/checkpoints/ShadowCheckpointService.ts +++ b/src/services/checkpoints/ShadowCheckpointService.ts @@ -12,6 +12,7 @@ import { executeRipgrep } from "../../services/search/file-search" import { CheckpointDiff, CheckpointResult, CheckpointEventMap } from "./types" import { getExcludePatterns } from "./excludes" +import { MAX_CHECKPOINT_FILE_SIZE_BYTES } from "../constants/file-limits" export abstract class ShadowCheckpointService extends EventEmitter { public readonly taskId: string @@ -292,9 +293,25 @@ export abstract class ShadowCheckpointService extends EventEmitter { const absPath = path.join(cwdPath, relPath) const before = await this.git.show([`${from}:${relPath}`]).catch(() => "") - const after = to - ? await this.git.show([`${to}:${relPath}`]).catch(() => "") - : await fs.readFile(absPath, "utf8").catch(() => "") + let after = "" + if (to) { + after = await this.git.show([`${to}:${relPath}`]).catch(() => "") + } else { + // Check file size before reading from filesystem + try { + const stats = await fs.stat(absPath) + if (stats.size > MAX_CHECKPOINT_FILE_SIZE_BYTES) { + console.warn( + `Checkpoint file ${absPath} exceeds size limit (${stats.size} bytes > ${MAX_CHECKPOINT_FILE_SIZE_BYTES} bytes)`, + ) + after = `[File too large to display: ${stats.size} bytes]` + } else { + after = await fs.readFile(absPath, "utf8") + } + } catch (err) { + after = "" + } + } result.push({ paths: { relative: relPath, absolute: absPath }, content: { before, after } }) } diff --git a/src/services/constants/file-limits.ts b/src/services/constants/file-limits.ts new file mode 100644 index 00000000000..0e347060247 --- /dev/null +++ b/src/services/constants/file-limits.ts @@ -0,0 +1,28 @@ +/** + * File size limits for various file types across Roo-Code services + * These constants help prevent memory exhaustion and performance issues + */ + +// General file size limit (1MB) - used for code files and general file operations +export const MAX_FILE_SIZE_BYTES = 1 * 1024 * 1024 // 1MB + +// Configuration files should be smaller (100KB) +export const MAX_CONFIG_FILE_SIZE_BYTES = 100 * 1024 // 100KB + +// .gitignore files should be even smaller (50KB) +export const MAX_GITIGNORE_FILE_SIZE_BYTES = 50 * 1024 // 50KB + +// Checkpoint files can be larger but still need a reasonable limit (5MB) +export const MAX_CHECKPOINT_FILE_SIZE_BYTES = 5 * 1024 * 1024 // 5MB + +/** + * Error class for file size limit violations + */ +export class FileSizeLimitError extends Error { + constructor(filePath: string, size: number, limit: number) { + const sizeInKB = Math.round(size / 1024) + const limitInKB = Math.round(limit / 1024) + super(`File ${filePath} exceeds size limit (${sizeInKB}KB > ${limitInKB}KB)`) + this.name = "FileSizeLimitError" + } +} diff --git a/src/services/glob/__tests__/list-files-gitignore-size.spec.ts b/src/services/glob/__tests__/list-files-gitignore-size.spec.ts new file mode 100644 index 00000000000..47511c442b6 --- /dev/null +++ b/src/services/glob/__tests__/list-files-gitignore-size.spec.ts @@ -0,0 +1,175 @@ +import { vi, describe, it, expect, beforeEach, afterEach } from "vitest" +import * as path from "path" +import { MAX_GITIGNORE_FILE_SIZE_BYTES } from "../../constants/file-limits" + +// Use vi.hoisted to ensure mocks are available during hoisting +const { mockConsoleWarn, mockStat, mockReadFile, mockAccess } = vi.hoisted(() => ({ + mockConsoleWarn: vi.fn(), + mockStat: vi.fn(), + mockReadFile: vi.fn(), + mockAccess: vi.fn(), +})) + +vi.mock("fs", () => ({ + promises: { + stat: mockStat, + readFile: mockReadFile, + access: mockAccess, + readdir: vi.fn().mockResolvedValue([]), + }, +})) + +// Mock other dependencies +vi.mock("../../ripgrep", () => ({ + getBinPath: vi.fn(() => Promise.resolve("/mock/path/to/rg")), +})) + +vi.mock("vscode", () => ({ + env: { + appRoot: "/mock/app/root", + }, +})) + +vi.mock("child_process", () => ({ + spawn: vi.fn(() => { + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + setTimeout(() => callback("test-file.txt\n"), 10) + } + }), + }, + stderr: { on: vi.fn() }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 20) + } + }), + kill: vi.fn(), + } + return mockProcess + }), +})) + +vi.mock("../../path", () => ({ + arePathsEqual: vi.fn().mockReturnValue(false), +})) + +// Import the function to test after mocks are set up +import { listFiles } from "../list-files" + +describe("list-files .gitignore size validation", () => { + beforeEach(() => { + vi.clearAllMocks() + console.warn = mockConsoleWarn + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it("should skip .gitignore files that exceed size limit", async () => { + // Mock finding a .gitignore file + mockAccess.mockResolvedValue(undefined) // File exists + + // Mock .gitignore file that exceeds size limit + const largeSize = MAX_GITIGNORE_FILE_SIZE_BYTES + 1000 + mockStat.mockResolvedValue({ size: largeSize }) + + // Call listFiles + await listFiles("/test/dir", false, 100) + + // Verify that console.warn was called with the appropriate message + expect(mockConsoleWarn).toHaveBeenCalledWith(expect.stringContaining("Skipping large .gitignore file")) + expect(mockConsoleWarn).toHaveBeenCalledWith( + expect.stringContaining(`${largeSize} bytes > ${MAX_GITIGNORE_FILE_SIZE_BYTES} bytes`), + ) + + // Verify that readFile was NOT called for the large .gitignore + expect(mockReadFile).not.toHaveBeenCalledWith(expect.stringContaining(".gitignore"), "utf8") + }) + + it("should read .gitignore files within size limit", async () => { + // Mock finding a .gitignore file + mockAccess.mockResolvedValue(undefined) // File exists + + // Mock .gitignore file within size limit + const normalSize = 1024 // 1KB, well under limit + mockStat.mockResolvedValue({ size: normalSize }) + mockReadFile.mockResolvedValue("*.log\nnode_modules/\n") + + // Call listFiles + await listFiles("/test/dir", false, 100) + + // Verify that readFile was called for the .gitignore + expect(mockReadFile).toHaveBeenCalledWith(expect.stringContaining(".gitignore"), "utf8") + + // Verify no warning was logged + expect(mockConsoleWarn).not.toHaveBeenCalledWith(expect.stringContaining("Skipping large .gitignore file")) + }) + + it("should handle multiple .gitignore files with mixed sizes", async () => { + // Mock finding .gitignore files at different levels + mockAccess.mockResolvedValue(undefined) // All files exist + + // Mock different .gitignore files with different sizes + // First call is for the .gitignore in the current directory + mockStat + .mockResolvedValueOnce({ size: 1024 }) // First .gitignore - OK + .mockResolvedValueOnce({ size: MAX_GITIGNORE_FILE_SIZE_BYTES + 1000 }) // Second - too large + + mockReadFile.mockResolvedValueOnce("*.log\n") + + // Call listFiles with recursive to trigger .gitignore checks + await listFiles("/test/dir", true, 100) + + // Verify that readFile was called only for files within limit + expect(mockReadFile).toHaveBeenCalled() + + // Verify warning was logged for the large file if multiple .gitignore files were found + // The test might only find one .gitignore depending on the mock setup + const warnCalls = mockConsoleWarn.mock.calls + const hasLargeFileWarning = warnCalls.some((call) => call[0].includes("Skipping large .gitignore file")) + + // If we had multiple stat calls, we should have seen the warning + if (mockStat.mock.calls.length > 1) { + expect(hasLargeFileWarning).toBe(true) + } + }) + + it("should continue processing when .gitignore stat fails", async () => { + // Mock finding a .gitignore file + mockAccess.mockResolvedValue(undefined) // File exists + + // Mock stat failure + mockStat.mockRejectedValue(new Error("Permission denied")) + + // Call listFiles + const [results] = await listFiles("/test/dir", false, 100) + + // Should still return results despite .gitignore stat failure + expect(results.length).toBeGreaterThan(0) + + // Verify warning was logged + expect(mockConsoleWarn).toHaveBeenCalledWith(expect.stringContaining("Error reading .gitignore")) + }) + + it("should handle .gitignore at exactly the size limit", async () => { + // Mock finding a .gitignore file + mockAccess.mockResolvedValue(undefined) // File exists + + // Mock .gitignore file at exactly the size limit + mockStat.mockResolvedValue({ size: MAX_GITIGNORE_FILE_SIZE_BYTES }) + mockReadFile.mockResolvedValue("*.tmp\n") + + // Call listFiles + await listFiles("/test/dir", false, 100) + + // Verify that readFile was called (file at limit should be read) + expect(mockReadFile).toHaveBeenCalledWith(expect.stringContaining(".gitignore"), "utf8") + + // Verify no warning was logged + expect(mockConsoleWarn).not.toHaveBeenCalledWith(expect.stringContaining("Skipping large .gitignore file")) + }) +}) diff --git a/src/services/glob/list-files.ts b/src/services/glob/list-files.ts index 05fa8a1d7b4..5e25410f3c1 100644 --- a/src/services/glob/list-files.ts +++ b/src/services/glob/list-files.ts @@ -7,6 +7,7 @@ import ignore from "ignore" import { arePathsEqual } from "../../utils/path" import { getBinPath } from "../../services/ripgrep" import { DIRS_TO_IGNORE } from "./constants" +import { MAX_GITIGNORE_FILE_SIZE_BYTES } from "../constants/file-limits" /** * List files in a directory, with optional recursive traversal @@ -257,6 +258,15 @@ async function createIgnoreInstance(dirPath: string): Promise MAX_GITIGNORE_FILE_SIZE_BYTES) { + console.warn( + `Skipping large .gitignore file: ${gitignoreFile} (${stats.size} bytes > ${MAX_GITIGNORE_FILE_SIZE_BYTES} bytes)`, + ) + continue + } + const content = await fs.promises.readFile(gitignoreFile, "utf8") ignoreInstance.add(content) } catch (err) { diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 10a74712ef0..4ab919cde13 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -32,6 +32,7 @@ import { import { fileExistsAtPath } from "../../utils/fs" import { arePathsEqual } from "../../utils/path" import { injectVariables } from "../../utils/config" +import { MAX_CONFIG_FILE_SIZE_BYTES } from "../constants/file-limits" export type McpConnection = { server: McpServer @@ -276,9 +277,29 @@ export class McpHub { this.configChangeDebounceTimers.set(key, timer) } + /** + * Safely reads a config file with size validation + * @param filePath Path to the config file + * @returns The file content or null if the file is too large + * @throws Error if file cannot be read (other than size limit) + */ + private async readConfigFile(filePath: string): Promise { + const stats = await fs.stat(filePath) + if (stats.size > MAX_CONFIG_FILE_SIZE_BYTES) { + const errorMessage = `Config file ${filePath} exceeds size limit (${stats.size} bytes > ${MAX_CONFIG_FILE_SIZE_BYTES} bytes)` + console.error(errorMessage) + vscode.window.showErrorMessage(errorMessage) + return null + } + return await fs.readFile(filePath, "utf-8") + } + private async handleConfigFileChange(filePath: string, source: "global" | "project"): Promise { try { - const content = await fs.readFile(filePath, "utf-8") + const content = await this.readConfigFile(filePath) + if (!content) { + return // File too large, error already shown + } let config: any try { @@ -364,7 +385,10 @@ export class McpHub { const projectMcpPath = await this.getProjectMcpPath() if (!projectMcpPath) return - const content = await fs.readFile(projectMcpPath, "utf-8") + const content = await this.readConfigFile(projectMcpPath) + if (!content) { + return // File too large, error already shown + } let config: any try { @@ -492,7 +516,10 @@ export class McpHub { return } - const content = await fs.readFile(configPath, "utf-8") + const content = await this.readConfigFile(configPath) + if (!content) { + return // File too large, error already shown + } const config = JSON.parse(content) const result = McpSettingsSchema.safeParse(config) @@ -846,14 +873,18 @@ export class McpHub { const projectMcpPath = await this.getProjectMcpPath() if (projectMcpPath) { configPath = projectMcpPath - const content = await fs.readFile(configPath, "utf-8") - serverConfigData = JSON.parse(content) + const content = await this.readConfigFile(configPath) + if (content) { + serverConfigData = JSON.parse(content) + } } } else { // Get global MCP settings path configPath = await this.getMcpSettingsFilePath() - const content = await fs.readFile(configPath, "utf-8") - serverConfigData = JSON.parse(content) + const content = await this.readConfigFile(configPath) + if (content) { + serverConfigData = JSON.parse(content) + } } if (serverConfigData) { alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || [] @@ -1118,15 +1149,17 @@ export class McpHub { const globalPath = await this.getMcpSettingsFilePath() let globalServers: Record = {} try { - const globalContent = await fs.readFile(globalPath, "utf-8") - const globalConfig = JSON.parse(globalContent) - globalServers = globalConfig.mcpServers || {} - const globalServerNames = Object.keys(globalServers) - vscode.window.showInformationMessage( - t("mcp:info.global_servers_active", { - mcpServers: `${globalServerNames.join(", ") || "none"}`, - }), - ) + const globalContent = await this.readConfigFile(globalPath) + if (globalContent) { + const globalConfig = JSON.parse(globalContent) + globalServers = globalConfig.mcpServers || {} + const globalServerNames = Object.keys(globalServers) + vscode.window.showInformationMessage( + t("mcp:info.global_servers_active", { + mcpServers: `${globalServerNames.join(", ") || "none"}`, + }), + ) + } } catch (error) { console.log("Error reading global MCP config:", error) } @@ -1135,15 +1168,17 @@ export class McpHub { let projectServers: Record = {} if (projectPath) { try { - const projectContent = await fs.readFile(projectPath, "utf-8") - const projectConfig = JSON.parse(projectContent) - projectServers = projectConfig.mcpServers || {} - const projectServerNames = Object.keys(projectServers) - vscode.window.showInformationMessage( - t("mcp:info.project_servers_active", { - mcpServers: `${projectServerNames.join(", ") || "none"}`, - }), - ) + const projectContent = await this.readConfigFile(projectPath) + if (projectContent) { + const projectConfig = JSON.parse(projectContent) + projectServers = projectConfig.mcpServers || {} + const projectServerNames = Object.keys(projectServers) + vscode.window.showInformationMessage( + t("mcp:info.project_servers_active", { + mcpServers: `${projectServerNames.join(", ") || "none"}`, + }), + ) + } } catch (error) { console.log("Error reading project MCP config:", error) } @@ -1175,8 +1210,8 @@ export class McpHub { private async notifyWebviewOfServerChanges(): Promise { // Get global server order from settings file const settingsPath = await this.getMcpSettingsFilePath() - const content = await fs.readFile(settingsPath, "utf-8") - const config = JSON.parse(content) + const content = await this.readConfigFile(settingsPath) + const config = content ? JSON.parse(content) : { mcpServers: {} } const globalServerOrder = Object.keys(config.mcpServers || {}) // Get project server order if available @@ -1184,9 +1219,11 @@ export class McpHub { let projectServerOrder: string[] = [] if (projectMcpPath) { try { - const projectContent = await fs.readFile(projectMcpPath, "utf-8") - const projectConfig = JSON.parse(projectContent) - projectServerOrder = Object.keys(projectConfig.mcpServers || {}) + const projectContent = await this.readConfigFile(projectMcpPath) + if (projectContent) { + const projectConfig = JSON.parse(projectContent) + projectServerOrder = Object.keys(projectConfig.mcpServers || {}) + } } catch (error) { // Silently continue with empty project server order } @@ -1310,7 +1347,10 @@ export class McpHub { } // Read and parse the config file - const content = await fs.readFile(configPath, "utf-8") + const content = await this.readConfigFile(configPath) + if (!content) { + throw new Error("Config file too large or unreadable") + } const config = JSON.parse(content) // Validate the config structure @@ -1401,7 +1441,10 @@ export class McpHub { throw new Error("Settings file not accessible") } - const content = await fs.readFile(configPath, "utf-8") + const content = await this.readConfigFile(configPath) + if (!content) { + throw new Error("Config file too large or unreadable") + } const config = JSON.parse(content) // Validate the config structure @@ -1539,7 +1582,10 @@ export class McpHub { const normalizedPath = process.platform === "win32" ? configPath.replace(/\\/g, "/") : configPath // Read the appropriate config file - const content = await fs.readFile(normalizedPath, "utf-8") + const content = await this.readConfigFile(normalizedPath) + if (!content) { + throw new Error("Config file too large or unreadable") + } const config = JSON.parse(content) if (!config.mcpServers) { diff --git a/src/services/roo-config/__tests__/index.spec.ts b/src/services/roo-config/__tests__/index.spec.ts index 8e9bf929cc3..4c36a1f46f1 100644 --- a/src/services/roo-config/__tests__/index.spec.ts +++ b/src/services/roo-config/__tests__/index.spec.ts @@ -2,10 +2,11 @@ import * as path from "path" import { describe, it, expect, beforeEach, afterEach, vi } from "vitest" // Use vi.hoisted to ensure mocks are available during hoisting -const { mockStat, mockReadFile, mockHomedir } = vi.hoisted(() => ({ +const { mockStat, mockReadFile, mockHomedir, mockConsoleWarn } = vi.hoisted(() => ({ mockStat: vi.fn(), mockReadFile: vi.fn(), mockHomedir: vi.fn(), + mockConsoleWarn: vi.fn(), })) // Mock fs/promises module @@ -30,11 +31,14 @@ import { getRooDirectoriesForCwd, loadConfiguration, } from "../index" +import { MAX_CONFIG_FILE_SIZE_BYTES } from "../../constants/file-limits" describe("RooConfigService", () => { beforeEach(() => { vi.clearAllMocks() mockHomedir.mockReturnValue("/mock/home") + // Mock console.warn + console.warn = mockConsoleWarn }) afterEach(() => { @@ -157,51 +161,91 @@ describe("RooConfigService", () => { }) describe("readFileIfExists", () => { - it("should return file content for existing file", async () => { + it("should return file content for existing file within size limit", async () => { + mockStat.mockResolvedValue({ size: 1024 }) // 1KB, well under limit mockReadFile.mockResolvedValue("file content") const result = await readFileIfExists("/some/file.txt") expect(result).toBe("file content") + expect(mockStat).toHaveBeenCalledWith("/some/file.txt") expect(mockReadFile).toHaveBeenCalledWith("/some/file.txt", "utf-8") }) + it("should return null for file exceeding size limit", async () => { + const largeSize = MAX_CONFIG_FILE_SIZE_BYTES + 1 + mockStat.mockResolvedValue({ size: largeSize }) + + const result = await readFileIfExists("/large/file.txt") + + expect(result).toBe(null) + expect(mockStat).toHaveBeenCalledWith("/large/file.txt") + expect(mockReadFile).not.toHaveBeenCalled() + expect(mockConsoleWarn).toHaveBeenCalledWith( + `File /large/file.txt exceeds size limit (${largeSize} bytes > ${MAX_CONFIG_FILE_SIZE_BYTES} bytes)`, + ) + }) + + it("should return file content for file exactly at size limit", async () => { + mockStat.mockResolvedValue({ size: MAX_CONFIG_FILE_SIZE_BYTES }) + mockReadFile.mockResolvedValue("file content at limit") + + const result = await readFileIfExists("/exact/limit/file.txt") + + expect(result).toBe("file content at limit") + expect(mockStat).toHaveBeenCalledWith("/exact/limit/file.txt") + expect(mockReadFile).toHaveBeenCalledWith("/exact/limit/file.txt", "utf-8") + }) + it("should return null for non-existing file", async () => { const error = new Error("ENOENT") as any error.code = "ENOENT" - mockReadFile.mockRejectedValue(error) + mockStat.mockRejectedValue(error) const result = await readFileIfExists("/non/existing/file.txt") expect(result).toBe(null) + expect(mockReadFile).not.toHaveBeenCalled() }) it("should return null for ENOTDIR error", async () => { const error = new Error("ENOTDIR") as any error.code = "ENOTDIR" - mockReadFile.mockRejectedValue(error) + mockStat.mockRejectedValue(error) const result = await readFileIfExists("/not/a/directory/file.txt") expect(result).toBe(null) + expect(mockReadFile).not.toHaveBeenCalled() }) it("should return null for EISDIR error", async () => { const error = new Error("EISDIR") as any error.code = "EISDIR" - mockReadFile.mockRejectedValue(error) + mockStat.mockRejectedValue(error) const result = await readFileIfExists("/is/a/directory") expect(result).toBe(null) + expect(mockReadFile).not.toHaveBeenCalled() }) - it("should throw unexpected errors", async () => { + it("should throw unexpected errors from stat", async () => { const error = new Error("Permission denied") as any error.code = "EACCES" - mockReadFile.mockRejectedValue(error) + mockStat.mockRejectedValue(error) await expect(readFileIfExists("/permission/denied/file.txt")).rejects.toThrow("Permission denied") + expect(mockReadFile).not.toHaveBeenCalled() + }) + + it("should throw unexpected errors from readFile", async () => { + mockStat.mockResolvedValue({ size: 1024 }) // Within limit + const error = new Error("Read error") as any + error.code = "EIO" + mockReadFile.mockRejectedValue(error) + + await expect(readFileIfExists("/io/error/file.txt")).rejects.toThrow("Read error") }) }) @@ -219,7 +263,9 @@ describe("RooConfigService", () => { it("should load global configuration only when project does not exist", async () => { const error = new Error("ENOENT") as any error.code = "ENOENT" - mockReadFile.mockResolvedValueOnce("global content").mockRejectedValueOnce(error) + // Mock stat calls for readFileIfExists + mockStat.mockResolvedValueOnce({ size: 100 }).mockRejectedValueOnce(error) + mockReadFile.mockResolvedValueOnce("global content") const result = await loadConfiguration("rules/rules.md", "/project/path") @@ -233,7 +279,9 @@ describe("RooConfigService", () => { it("should load project configuration only when global does not exist", async () => { const error = new Error("ENOENT") as any error.code = "ENOENT" - mockReadFile.mockRejectedValueOnce(error).mockResolvedValueOnce("project content") + // Mock stat calls for readFileIfExists + mockStat.mockRejectedValueOnce(error).mockResolvedValueOnce({ size: 100 }) + mockReadFile.mockResolvedValueOnce("project content") const result = await loadConfiguration("rules/rules.md", "/project/path") @@ -245,6 +293,8 @@ describe("RooConfigService", () => { }) it("should merge global and project configurations with project overriding global", async () => { + // Mock stat calls for readFileIfExists + mockStat.mockResolvedValueOnce({ size: 100 }).mockResolvedValueOnce({ size: 100 }) mockReadFile.mockResolvedValueOnce("global content").mockResolvedValueOnce("project content") const result = await loadConfiguration("rules/rules.md", "/project/path") @@ -259,7 +309,8 @@ describe("RooConfigService", () => { it("should return empty merged content when neither exists", async () => { const error = new Error("ENOENT") as any error.code = "ENOENT" - mockReadFile.mockRejectedValueOnce(error).mockRejectedValueOnce(error) + // Mock stat calls for readFileIfExists + mockStat.mockRejectedValueOnce(error).mockRejectedValueOnce(error) const result = await loadConfiguration("rules/rules.md", "/project/path") @@ -273,7 +324,7 @@ describe("RooConfigService", () => { it("should propagate unexpected errors from global file read", async () => { const error = new Error("Permission denied") as any error.code = "EACCES" - mockReadFile.mockRejectedValueOnce(error) + mockStat.mockRejectedValueOnce(error) await expect(loadConfiguration("rules/rules.md", "/project/path")).rejects.toThrow("Permission denied") }) @@ -284,18 +335,37 @@ describe("RooConfigService", () => { const projectError = new Error("Permission denied") as any projectError.code = "EACCES" - mockReadFile.mockRejectedValueOnce(globalError).mockRejectedValueOnce(projectError) + mockStat.mockRejectedValueOnce(globalError).mockRejectedValueOnce(projectError) await expect(loadConfiguration("rules/rules.md", "/project/path")).rejects.toThrow("Permission denied") }) it("should use correct file paths", async () => { + mockStat.mockResolvedValue({ size: 100 }) mockReadFile.mockResolvedValue("content") await loadConfiguration("rules/rules.md", "/project/path") + expect(mockStat).toHaveBeenCalledWith(path.join("/mock/home", ".roo", "rules/rules.md")) + expect(mockStat).toHaveBeenCalledWith(path.join("/project/path", ".roo", "rules/rules.md")) expect(mockReadFile).toHaveBeenCalledWith(path.join("/mock/home", ".roo", "rules/rules.md"), "utf-8") expect(mockReadFile).toHaveBeenCalledWith(path.join("/project/path", ".roo", "rules/rules.md"), "utf-8") }) + + it("should handle large config files gracefully", async () => { + const largeSize = MAX_CONFIG_FILE_SIZE_BYTES + 1 + // Global file is too large, project file is normal + mockStat.mockResolvedValueOnce({ size: largeSize }).mockResolvedValueOnce({ size: 100 }) + mockReadFile.mockResolvedValueOnce("project content") + + const result = await loadConfiguration("rules/rules.md", "/project/path") + + expect(result).toEqual({ + global: null, + project: "project content", + merged: "project content", + }) + expect(mockConsoleWarn).toHaveBeenCalledWith(expect.stringContaining("exceeds size limit")) + }) }) }) diff --git a/src/services/roo-config/index.ts b/src/services/roo-config/index.ts index b46c39e354b..ba0042034f4 100644 --- a/src/services/roo-config/index.ts +++ b/src/services/roo-config/index.ts @@ -1,6 +1,7 @@ import * as path from "path" import * as os from "os" import fs from "fs/promises" +import { MAX_CONFIG_FILE_SIZE_BYTES } from "../constants/file-limits" /** * Gets the global .roo directory path based on the current platform @@ -100,6 +101,15 @@ export async function fileExists(filePath: string): Promise { */ export async function readFileIfExists(filePath: string): Promise { try { + // Check file size before reading + const stats = await fs.stat(filePath) + if (stats.size > MAX_CONFIG_FILE_SIZE_BYTES) { + console.warn( + `File ${filePath} exceeds size limit (${stats.size} bytes > ${MAX_CONFIG_FILE_SIZE_BYTES} bytes)`, + ) + return null + } + return await fs.readFile(filePath, "utf-8") } catch (error: any) { // Only catch expected "not found" errors