Skip to content

fix: resolve relative paths correctly in exec tool safety guard#2826

Open
yuxuan-7814 wants to merge 1 commit intosipeed:mainfrom
yuxuan-7814:fix/2749-bash-relative-path
Open

fix: resolve relative paths correctly in exec tool safety guard#2826
yuxuan-7814 wants to merge 1 commit intosipeed:mainfrom
yuxuan-7814:fix/2749-bash-relative-path

Conversation

@yuxuan-7814
Copy link
Copy Markdown

📝 Description

Fix issue #2749: Bash evaluates relative path as absolute path

When the exec tool (bash) restricts access to the workspace, relative paths in commands were incorrectly resolved using the process's current working directory instead of the command's intended working directory. This caused legitimate relative paths (e.g., skills/file.md) to be evaluated as absolute paths from the process root, triggering false-positive safety blocks.

The fix modifies the guardCommand method in pkg/tools/shell.go to correctly resolve paths:

  • For absolute paths: use as-is (cleaned)
  • For relative paths: join with the command's working directory (cwd) before cleaning and validation

This ensures that relative paths are resolved relative to the intended working directory, not the process's global cwd.

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully tool-generated
  • 🛠️ Mixed (tool-assisted)
  • 👨‍💻 Human-written

🔗 Related Issue

Fixes #2749

📚 Technical Context (Skip for Docs)

  • Reference URL:
  • Reasoning:
    The absolutePathPattern regex matches paths in commands. The guard validates each matched path to ensure it stays within the workspace boundary. Previously, filepath.Abs(raw) was used for all matched paths, which is incorrect for relative paths because it uses the process's current working directory. The fix distinguishes absolute and relative paths, resolving them correctly against the command's cwd.

🧪 Test Environment

  • Hardware: x86_64
  • OS: Linux (Ubuntu 24.04)
  • Model/Provider: Not applicable (tooling fix)
  • Channels: All (exec tool is channel-agnostic)

📸 Evidence (Optional)

Click to view Logs/Screenshots

Manual verification: With workspace restriction enabled, running exec with a relative path like cat skills/test.md now correctly resolves to the workspace subdirectory and is allowed, while attempts to traverse outside (cat ../../etc/passwd) remain blocked.

☑️ 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.

Previously, relative paths in shell commands were resolved using the process's current working directory instead of the command's intended working directory. This caused legitimate relative paths (e.g., skills/file.md) to be evaluated as absolute paths from the process root, triggering false-positive safety blocks.

The guardCommand method now correctly distinguishes absolute and relative paths:
- Absolute paths are used as-is (cleaned)
- Relative paths are joined with the command's cwd before validation

This ensures relative paths are resolved relative to the intended working directory.

Closes sipeed#2749
Copy link
Copy Markdown
Collaborator

@afjcjsbx afjcjsbx left a comment

Choose a reason for hiding this comment

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

Hi, the patch doesn't really fix relative paths. In [pkg/tools/shell.go] absolutePathPattern only matches absolute paths, so the new else branch in [pkg/tools/shell.go] is actually dead code.
Forward you are duplicating validation logic instead of reusing the shared validator in [pkg/tools/fs/filesystem.go]. No new tests on a security-sensitive change.

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.

[BUG] Bash evaluates relative path as absolute path

2 participants