cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [MCP-SECURITY] Path traversal vulnerability in SandboxPathRewriter #7084

New issue

Open

opened 2026-04-10 07:33:07 +00:00 by HAL9000 · 2 comments

HAL9000 commented

2026-04-10 07:33:07 +00:00

Owner

Metadata

Branch: bugfix/m3.2.0-mcp-sandbox-path-traversal-rewriter
Commit Message: fix(mcp): add path traversal protection and boundary validation to SandboxPathRewriter
Milestone: v3.2.0
Parent Epic: Orphan — see note below; requires manual Epic linking

Bug Report: [SECURITY] — Path traversal vulnerability in SandboxPathRewriter

Severity Assessment

Impact: Sandbox escape, unauthorized access to host filesystem, potential data exposure
Likelihood: Medium when processing untrusted MCP tool responses or arguments
Priority: Critical

Location

File: src/cleveragents/mcp/sandbox.py
Function/Class: SandboxPathRewriter._maybe_rewrite_path()
Lines: 167-179

Description

The SandboxPathRewriter only checks if a string starts with "/" to determine if it's a path, then blindly passes it to the rewrite function without validating that the path is within expected boundaries. This allows path traversal attacks that could break out of the sandboxed environment.

Evidence

# In src/cleveragents/mcp/sandbox.py lines 167-179:
@staticmethod
def _maybe_rewrite_path(
    value: str,
    rewrite_fn: Any,
) -> str:
    """Rewrite a string if it looks like an absolute path.

    Only strings starting with ``/`` are considered candidates for
    path rewriting to avoid mutating non-path strings.
    """
    if not value.startswith("/"):
        return value
    return rewrite_fn(value)

A malicious MCP response could contain paths like:

{
  "output_file": "/../../../../etc/passwd",
  "config_path": "/../../../home/user/.ssh/id_rsa"
}

These would pass the startswith("/") check and potentially be rewritten to sensitive host locations.

Expected Behavior

Path rewriting should validate that paths are within expected sandbox/host boundaries
Path traversal sequences (../) should be detected and blocked
Failed path mappings should be handled gracefully
Path rewrite operations should be logged for security auditing

Actual Behavior

Any absolute path is accepted for rewriting without boundary validation
Path traversal sequences can escape sandbox boundaries
No error handling if PathMapper fails or produces unsafe mappings
No security logging of path rewrite operations

Suggested Fix

Add boundary validation to ensure paths are within expected sandbox/host roots
Sanitize path traversal sequences before rewriting
Add error handling for PathMapper exceptions
Implement security logging for path rewrite operations
Add recursion depth and size limits for _rewrite_value() to prevent DoS

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it.

Subtasks

Write TDD Behave scenario tagged @tdd_issue, @tdd_issue_<N>, @tdd_expected_fail that demonstrates path traversal bypass
Implement path traversal sequence detection and rejection in _maybe_rewrite_path()
Add boundary validation to ensure rewritten paths remain within sandbox/host roots
Add error handling for PathMapper exceptions in _maybe_rewrite_path()
Implement security logging for all path rewrite operations
Add recursion depth and size limits to _rewrite_value() to prevent DoS
Remove @tdd_expected_fail tag once fix is implemented and test passes
Update documentation to reflect new security constraints

Definition of Done

TDD scenario exists and was previously failing (proving the bug)
_maybe_rewrite_path() rejects paths containing .. traversal sequences
_maybe_rewrite_path() validates rewritten paths remain within expected roots
PathMapper exceptions are caught and handled without silent failures
Security-relevant path rewrite events are logged via structlog
_rewrite_value() has recursion depth and size guards
All nox stages pass
Coverage >= 97%

Orphan note: No parent Epic for MCP sandbox security was found at issue creation time. This issue requires manual linking to an appropriate Epic by a maintainer.

Automated by CleverAgents Bot
Supervisor: Acting on behalf of: UAT Testing | Agent: new-issue-creator

## Metadata - **Branch**: `bugfix/m3.2.0-mcp-sandbox-path-traversal-rewriter` - **Commit Message**: `fix(mcp): add path traversal protection and boundary validation to SandboxPathRewriter` - **Milestone**: v3.2.0 - **Parent Epic**: _Orphan — see note below; requires manual Epic linking_ ## Bug Report: [SECURITY] — Path traversal vulnerability in SandboxPathRewriter ### Severity Assessment - **Impact**: Sandbox escape, unauthorized access to host filesystem, potential data exposure - **Likelihood**: Medium when processing untrusted MCP tool responses or arguments - **Priority**: Critical ### Location - **File**: `src/cleveragents/mcp/sandbox.py` - **Function/Class**: `SandboxPathRewriter._maybe_rewrite_path()` - **Lines**: 167-179 ### Description The SandboxPathRewriter only checks if a string starts with "/" to determine if it's a path, then blindly passes it to the rewrite function without validating that the path is within expected boundaries. This allows path traversal attacks that could break out of the sandboxed environment. ### Evidence ```python # In src/cleveragents/mcp/sandbox.py lines 167-179: @staticmethod def _maybe_rewrite_path( value: str, rewrite_fn: Any, ) -> str: """Rewrite a string if it looks like an absolute path. Only strings starting with ``/`` are considered candidates for path rewriting to avoid mutating non-path strings. """ if not value.startswith("/"): return value return rewrite_fn(value) ``` A malicious MCP response could contain paths like: ```json { "output_file": "/../../../../etc/passwd", "config_path": "/../../../home/user/.ssh/id_rsa" } ``` These would pass the `startswith("/")` check and potentially be rewritten to sensitive host locations. ### Expected Behavior - Path rewriting should validate that paths are within expected sandbox/host boundaries - Path traversal sequences (../) should be detected and blocked - Failed path mappings should be handled gracefully - Path rewrite operations should be logged for security auditing ### Actual Behavior - Any absolute path is accepted for rewriting without boundary validation - Path traversal sequences can escape sandbox boundaries - No error handling if PathMapper fails or produces unsafe mappings - No security logging of path rewrite operations ### Suggested Fix 1. Add boundary validation to ensure paths are within expected sandbox/host roots 2. Sanitize path traversal sequences before rewriting 3. Add error handling for PathMapper exceptions 4. Implement security logging for path rewrite operations 5. Add recursion depth and size limits for `_rewrite_value()` to prevent DoS ### Category security ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: `@tdd_issue`, `@tdd_issue_<this-issue-number>`, and `@tdd_expected_fail` to prove the bug exists before fixing it. ## Subtasks - [ ] Write TDD Behave scenario tagged `@tdd_issue`, `@tdd_issue_<N>`, `@tdd_expected_fail` that demonstrates path traversal bypass - [ ] Implement path traversal sequence detection and rejection in `_maybe_rewrite_path()` - [ ] Add boundary validation to ensure rewritten paths remain within sandbox/host roots - [ ] Add error handling for `PathMapper` exceptions in `_maybe_rewrite_path()` - [ ] Implement security logging for all path rewrite operations - [ ] Add recursion depth and size limits to `_rewrite_value()` to prevent DoS - [ ] Remove `@tdd_expected_fail` tag once fix is implemented and test passes - [ ] Update documentation to reflect new security constraints ## Definition of Done - [ ] TDD scenario exists and was previously failing (proving the bug) - [ ] `_maybe_rewrite_path()` rejects paths containing `..` traversal sequences - [ ] `_maybe_rewrite_path()` validates rewritten paths remain within expected roots - [ ] `PathMapper` exceptions are caught and handled without silent failures - [ ] Security-relevant path rewrite events are logged via structlog - [ ] `_rewrite_value()` has recursion depth and size guards - [ ] All nox stages pass - [ ] Coverage >= 97% > **Orphan note:** No parent Epic for MCP sandbox security was found at issue creation time. This issue requires manual linking to an appropriate Epic by a maintainer. --- **Automated by CleverAgents Bot** Supervisor: Acting on behalf of: UAT Testing | Agent: new-issue-creator

HAL9000 added this to the v3.2.0 milestone

2026-04-10 07:33:11 +00:00

HAL9000 added the

labels

2026-04-10 07:36:11 +00:00

HAL9000 commented

2026-04-10 07:37:13 +00:00

Author

Owner

⚠️ Orphan Issue — Requires Manual Epic Linking

This issue was created by an automated agent. A search was performed for an existing parent Epic covering MCP sandbox security or path traversal, but none was found. The closest related issue is #7038 (MCP-SECURITY: Command injection in MCPServerConfig), which is also an orphan.

Action required by a maintainer:

Link this issue to an appropriate parent Epic using Forgejo's dependency system
The correct direction is: this issue (#7084) blocks the parent Epic (the parent Epic depends on this issue)
If no suitable Epic exists, one should be created to group MCP security hardening work

Related issues for context:

#7038 — BUG-HUNT: [MCP-SECURITY] Command injection vulnerability in MCPServerConfig validation (same module, also orphan)
#7051 — BUG-HUNT: [Security] Path traversal vulnerability in Agent Skills discovery (similar pattern)
#6677 — BUG-HUNT: [security] InlineToolExecutor subprocess inherits all process environment variables

Automated by CleverAgents Bot
Supervisor: Acting on behalf of: UAT Testing | Agent: new-issue-creator

⚠️ **Orphan Issue — Requires Manual Epic Linking** This issue was created by an automated agent. A search was performed for an existing parent Epic covering MCP sandbox security or path traversal, but none was found. The closest related issue is #7038 (MCP-SECURITY: Command injection in MCPServerConfig), which is also an orphan. **Action required by a maintainer:** - Link this issue to an appropriate parent Epic using Forgejo's dependency system - The correct direction is: **this issue (#7084) blocks the parent Epic** (the parent Epic depends on this issue) - If no suitable Epic exists, one should be created to group MCP security hardening work **Related issues for context:** - #7038 — BUG-HUNT: [MCP-SECURITY] Command injection vulnerability in MCPServerConfig validation (same module, also orphan) - #7051 — BUG-HUNT: [Security] Path traversal vulnerability in Agent Skills discovery (similar pattern) - #6677 — BUG-HUNT: [security] InlineToolExecutor subprocess inherits all process environment variables --- **Automated by CleverAgents Bot** Supervisor: Acting on behalf of: UAT Testing | Agent: new-issue-creator

HAL9000 referenced this issue

2026-04-10 08:09:28 +00:00

[AUTO-BUG-HUNT] Worker 8 Complete: MCP Integration Analysis #7139

HAL9000 referenced this issue

2026-04-10 15:14:11 +00:00

BUG-HUNT: [security] Command injection risk in scripts/check-quality-gates.py via unvalidated subprocess arguments #7286

freemo added the

Priority

Critical

label

2026-04-12 18:37:26 +00:00

HAL9000 added

and removed

labels

2026-04-14 06:48:53 +00:00

HAL9000 commented

2026-04-14 06:48:53 +00:00

Author

Owner

✅ Verified — Critical security bug: path traversal in SandboxPathRewriter. MoSCoW: Must-have. Priority: Critical.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

✅ **Verified** — Critical security bug: path traversal in SandboxPathRewriter. MoSCoW: Must-have. Priority: Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor