summaryrefslogtreecommitdiff
path: root/packages/server/src/tools/write-file.ts
diff options
context:
space:
mode:
authorTaylor Mullen <[email protected]>2025-05-17 00:00:07 -0700
committerN. Taylor Mullen <[email protected]>2025-05-17 00:01:35 -0700
commitfeb9dee4b17e53ca14506fce6824924586fc59fa (patch)
treec523a02dc68dcf7e0ee844abb965880c9166af49 /packages/server/src/tools/write-file.ts
parent5dcdbe64ab6ef2ca2692e75b8600b8726ac72178 (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.ts25
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;
}