diff options
| author | Taylor Mullen <[email protected]> | 2025-05-17 00:00:07 -0700 |
|---|---|---|
| committer | N. Taylor Mullen <[email protected]> | 2025-05-17 00:01:35 -0700 |
| commit | feb9dee4b17e53ca14506fce6824924586fc59fa (patch) | |
| tree | c523a02dc68dcf7e0ee844abb965880c9166af49 /packages/server/src/tools/write-file.ts | |
| parent | 5dcdbe64ab6ef2ca2692e75b8600b8726ac72178 (diff) | |
fix: Prevent WriteFileTool from writing to directory paths
- Enhances WriteFileTool validation to check if the target file_path is an existing directory.
- If it is, the tool now returns a validation error "Path is a directory, not a file: <filePath>", preventing the attempt to write.
- This proactive check avoids underlying file system errors that would occur if fs.writeFileSync were called on a directory path, which could lead to console errors.
- Test cases have been updated to reflect this stricter validation.
Fixes https://b.corp.google.com/issues/418348176
Diffstat (limited to 'packages/server/src/tools/write-file.ts')
| -rw-r--r-- | packages/server/src/tools/write-file.ts | 25 |
1 files changed, 21 insertions, 4 deletions
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<WriteFileToolParams, ToolResult> { ) { 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; } |
