diff --git a/packages/server/src/tools/write-file.test.ts b/packages/server/src/tools/write-file.test.ts index fc2ca61b..9a690521 100644 --- a/packages/server/src/tools/write-file.test.ts +++ b/packages/server/src/tools/write-file.test.ts @@ -101,14 +101,15 @@ describe('WriteFileTool', () => { ); }); - it('should return null for path that is the root itself', () => { + it('should return error for path that is the root itself', () => { const params = { - file_path: rootDir, // Attempting to write to the root directory itself (as a file) + file_path: rootDir, // Attempting to write to the root directory itself content: 'hello', }; - // This is a tricky case. The validation should allow it if it's treated as a file path. - // The actual write operation might fail if it's a directory, but validation should pass. - expect(tool.validateToolParams(params)).toBeNull(); + // With the new validation, this should now return an error as rootDir is a directory. + expect(tool.validateToolParams(params)).toMatch( + `Path is a directory, not a file: ${rootDir}`, + ); }); it('should return error for path that is just / and root is not /', () => { diff --git a/packages/server/src/tools/write-file.ts b/packages/server/src/tools/write-file.ts index 2979ffb8..c628ce75 100644 --- a/packages/server/src/tools/write-file.ts +++ b/packages/server/src/tools/write-file.ts @@ -86,12 +86,29 @@ export class WriteFileTool extends BaseTool { ) { return 'Parameters failed schema validation.'; } - if (!path.isAbsolute(params.file_path)) { - return `File path must be absolute: ${params.file_path}`; + const filePath = params.file_path; + if (!path.isAbsolute(filePath)) { + return `File path must be absolute: ${filePath}`; } - if (!this.isWithinRoot(params.file_path)) { - return `File path must be within the root directory (${this.config.getTargetDir()}): ${params.file_path}`; + if (!this.isWithinRoot(filePath)) { + return `File path must be within the root directory (${this.config.getTargetDir()}): ${filePath}`; } + + try { + // This check should be performed only if the path exists. + // If it doesn't exist, it's a new file, which is valid for writing. + if (fs.existsSync(filePath)) { + const stats = fs.lstatSync(filePath); + if (stats.isDirectory()) { + return `Path is a directory, not a file: ${filePath}`; + } + } + } catch (statError: unknown) { + // If fs.existsSync is true but lstatSync fails (e.g., permissions, race condition where file is deleted) + // this indicates an issue with accessing the path that should be reported. + return `Error accessing path properties for validation: ${filePath}. Reason: ${statError instanceof Error ? statError.message : String(statError)}`; + } + return null; }