Conversation
Add path validation to handleReadFile, handleWriteFile, and handleEditFile in the ACP filesystem toolset. User-supplied paths are now resolved and validated to ensure they do not escape the configured working directory. Previously, paths like '../../etc/passwd' could be used to access files outside the working directory. The new resolvePath method converts paths to absolute form and checks they remain within the working directory boundary. Fixes docker#1823 Assisted-By: cagent
The persistSessionTitle function silently discarded errors from UpdateSessionTitle (other than ErrTitleGenerating). Add slog.Warn so failures are visible in debug logs. Fixes docker#1821 Assisted-By: cagent
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR adds security improvements for path traversal protection in the filesystem toolset and improves error logging. However, there is one issue that should be addressed: the path validation logic doesn't account for case-insensitive filesystems (Windows and macOS), which could potentially allow path traversal attacks on those platforms.
| return "", fmt.Errorf("failed to resolve path: %w", err) | ||
| } | ||
| // Ensure the resolved path is within the working directory | ||
| if !strings.HasPrefix(absResolved, absWorkingDir+string(filepath.Separator)) && absResolved != absWorkingDir { |
There was a problem hiding this comment.
Path validation doesn't account for case-insensitive filesystems
The path traversal check uses strings.HasPrefix, which performs a case-sensitive comparison. On Windows and macOS (case-insensitive filesystems), this could be problematic. For example, if absWorkingDir is C:\Users\App and an attacker provides a path that resolves to C:\users\app\..\..\sensitive, the case difference could cause the validation to fail incorrectly or behave unexpectedly.
Consider using case-insensitive path comparisons on Windows and macOS, or use filepath.EvalSymlinks followed by case-normalized comparison to ensure proper validation on case-insensitive filesystems.
1 participant
No description provided.