Skip to content

fix(security): Symlink TOCTOU race condition#482

Closed
GoCeylan wants to merge 1 commit intosipeed:mainfrom
GoCeylan:fix(security)-Symlink-TOCTOU-race-condition
Closed

fix(security): Symlink TOCTOU race condition#482
GoCeylan wants to merge 1 commit intosipeed:mainfrom
GoCeylan:fix(security)-Symlink-TOCTOU-race-condition

Conversation

@GoCeylan
Copy link
Contributor

@GoCeylan GoCeylan commented Feb 19, 2026

@Leeaandrob

validatePath() resolved symlinks for validation but returned the original unresolved path. An attacker could retarget the symlink between check and use. Now returns the resolved path directly.

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)

🤖 AI Code Generation

  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

Security audit finding, no existing issue.

📚 Technical Context (Skip for Docs)

  • Reference URL: N/A
  • Reasoning: TOCTOU race in pkg/tools/filesystem.go. EvalSymlinks result was checked but discarded . callers (ReadFileTool, WriteFileTool, ListDirTool) operated on the original symlink path.

🧪 Test Environment

  • Hardware: MacBook (Apple Silicon)
  • OS: macOS (Darwin 23.3.0)
  • Model/Provider: N/A
  • Channels: N/A

📸 Evidence (Optional)

One-line change: absPath = resolved after symlink validation passes.

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

Copy link

@nikolasdehor nikolasdehor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Textbook TOCTOU fix. Returning the resolved path ensures callers operate on the validated target, closing the race window between check and use. LGTM.

Copy link
Collaborator

@lxowalle lxowalle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here do not seem to completely prevent this type of issue. If the workspace has already been redirected to a dangerous path before the program starts, the problem will still inevitably occur.

@nikolasdehor
Copy link

This fixes the symlink TOCTOU race, which is also addressed in #331 (broader security hardening) and #464 (workspace sandbox TOCTOU). #331 is the most comprehensive — it covers shell escape bypass, SSRF, and working_dir validation in addition to the TOCTOU fix. Suggesting this be closed in favor of #331.

@GoCeylan
Copy link
Contributor Author

Good shout @nikolasdehor & @lxowalle .
Let's close this in favor of the tools system refactor & broader security hardening.

@GoCeylan GoCeylan closed this Feb 23, 2026
This was referenced Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants