cleveragents/cleveragents-core

Fork 3

lsp/runtime: LspRuntime._read_file has no workspace path containment check — path traversal allows reading arbitrary files #10490

New issue

Closed

opened 2026-04-18 10:10:14 +00:00 by HAL9000 · 2 comments

HAL9000 commented

2026-04-18 10:10:14 +00:00

Owner

Bug Report

Metadata

Module: src/cleveragents/lsp/runtime.py
Class: LspRuntime
Method: _read_file (static method)
Severity: High (security — path traversal vulnerability)
TDD Testing Issue: #10489

Summary

LspRuntime._read_file() resolves the file path using os.path.realpath() but does not verify that the resolved path is within the workspace directory. An actor (LLM-driven) that can control the file_path argument could read arbitrary files on the filesystem by passing path traversal sequences like ../../etc/passwd.

Code Evidence

In src/cleveragents/lsp/runtime.py (lines ~340-350):

@staticmethod
def _read_file(file_path: str) -> str:
    """Read file contents as UTF-8 text.

    Raises:
        LspError: If the resolved path is a directory or device.
    """
    resolved = os.path.realpath(file_path)
    if not os.path.isfile(resolved):
        raise LspError(
            f"Not a regular file: {file_path}",
            details={"file": file_path, "resolved": resolved},
        )
    with open(resolved, encoding="utf-8") as f:
        return f.read()

os.path.realpath() resolves symlinks and .. components but does NOT restrict the path to a specific directory. There is no check that resolved starts with the workspace path.

Attack Vector

The _read_file method is called by:

get_diagnostics(name, file_path) — file_path comes from actor tool calls
get_completions(name, file_path, line, column) — same
get_hover(name, file_path, line, column) — same
get_definitions(name, file_path, line, column) — same

If an LLM produces a malicious tool call with file_path = "../../etc/passwd", the runtime would read and return the contents of /etc/passwd.

Proof of Concept

from cleveragents.lsp.runtime import LspRuntime

# This reads /etc/passwd (or any file) without restriction
content = LspRuntime._read_file("../../etc/passwd")
print(content)  # Prints /etc/passwd contents

Root Cause

_read_file is a @staticmethod with no access to the workspace path. The workspace path is passed to start_server() but not stored in the LspRuntime instance, so it cannot be used for containment checks.

Fix

Store the workspace path per server in the LspRuntime instance
Change _read_file to an instance method (or accept workspace_path as parameter)
Add a containment check before reading:

def _read_file(self, file_path: str, workspace_path: str) -> str:
    """Read file contents as UTF-8 text, restricted to workspace."""
    resolved = os.path.realpath(file_path)
    workspace_resolved = os.path.realpath(workspace_path)
    
    # Containment check: resolved path must be within workspace
    if not resolved.startswith(workspace_resolved + os.sep) and resolved != workspace_resolved:
        raise LspError(
            f"File path is outside workspace: {file_path}",
            details={
                "file": file_path,
                "resolved": resolved,
                "workspace": workspace_resolved,
            },
        )
    
    if not os.path.isfile(resolved):
        raise LspError(
            f"Not a regular file: {file_path}",
            details={"file": file_path, "resolved": resolved},
        )
    with open(resolved, encoding="utf-8") as f:
        return f.read()

Acceptance Criteria

LspRuntime stores workspace path per server (e.g., self._workspace_paths: dict[str, str])
_read_file performs workspace containment check before reading
Path traversal attempts raise LspError with a clear message
TDD test from #10489 passes
All existing tests continue to pass
Coverage >= 97%

Subtasks

Add self._workspace_paths: dict[str, str] = {} to __init__
Store workspace path in start_server(): self._workspace_paths[name] = workspace_path
Update _read_file to accept workspace path and perform containment check
Update all callers of _read_file to pass the workspace path
Verify TDD test from #10489 now passes
Run nox and verify all tests pass

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off
A Git commit is created with a descriptive message
The commit is pushed to a branch and submitted as a pull request to master
The TDD testing issue #10489 is resolved (its @tdd_expected_fail scenario now passes)

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

## Bug Report ### Metadata - **Module**: `src/cleveragents/lsp/runtime.py` - **Class**: `LspRuntime` - **Method**: `_read_file` (static method) - **Severity**: High (security — path traversal vulnerability) - **TDD Testing Issue**: #10489 ### Summary `LspRuntime._read_file()` resolves the file path using `os.path.realpath()` but does not verify that the resolved path is within the workspace directory. An actor (LLM-driven) that can control the `file_path` argument could read arbitrary files on the filesystem by passing path traversal sequences like `../../etc/passwd`. ### Code Evidence In `src/cleveragents/lsp/runtime.py` (lines ~340-350): ```python @staticmethod def _read_file(file_path: str) -> str: """Read file contents as UTF-8 text. Raises: LspError: If the resolved path is a directory or device. """ resolved = os.path.realpath(file_path) if not os.path.isfile(resolved): raise LspError( f"Not a regular file: {file_path}", details={"file": file_path, "resolved": resolved}, ) with open(resolved, encoding="utf-8") as f: return f.read() ``` `os.path.realpath()` resolves symlinks and `..` components but does NOT restrict the path to a specific directory. There is no check that `resolved` starts with the workspace path. ### Attack Vector The `_read_file` method is called by: - `get_diagnostics(name, file_path)` — `file_path` comes from actor tool calls - `get_completions(name, file_path, line, column)` — same - `get_hover(name, file_path, line, column)` — same - `get_definitions(name, file_path, line, column)` — same If an LLM produces a malicious tool call with `file_path = "../../etc/passwd"`, the runtime would read and return the contents of `/etc/passwd`. ### Proof of Concept ```python from cleveragents.lsp.runtime import LspRuntime # This reads /etc/passwd (or any file) without restriction content = LspRuntime._read_file("../../etc/passwd") print(content) # Prints /etc/passwd contents ``` ### Root Cause `_read_file` is a `@staticmethod` with no access to the workspace path. The workspace path is passed to `start_server()` but not stored in the `LspRuntime` instance, so it cannot be used for containment checks. ### Fix 1. Store the workspace path per server in the `LspRuntime` instance 2. Change `_read_file` to an instance method (or accept workspace_path as parameter) 3. Add a containment check before reading: ```python def _read_file(self, file_path: str, workspace_path: str) -> str: """Read file contents as UTF-8 text, restricted to workspace.""" resolved = os.path.realpath(file_path) workspace_resolved = os.path.realpath(workspace_path) # Containment check: resolved path must be within workspace if not resolved.startswith(workspace_resolved + os.sep) and resolved != workspace_resolved: raise LspError( f"File path is outside workspace: {file_path}", details={ "file": file_path, "resolved": resolved, "workspace": workspace_resolved, }, ) if not os.path.isfile(resolved): raise LspError( f"Not a regular file: {file_path}", details={"file": file_path, "resolved": resolved}, ) with open(resolved, encoding="utf-8") as f: return f.read() ``` ### Acceptance Criteria - [ ] `LspRuntime` stores workspace path per server (e.g., `self._workspace_paths: dict[str, str]`) - [ ] `_read_file` performs workspace containment check before reading - [ ] Path traversal attempts raise `LspError` with a clear message - [ ] TDD test from #10489 passes - [ ] All existing tests continue to pass - [ ] Coverage >= 97% ### Subtasks - [ ] Add `self._workspace_paths: dict[str, str] = {}` to `__init__` - [ ] Store workspace path in `start_server()`: `self._workspace_paths[name] = workspace_path` - [ ] Update `_read_file` to accept workspace path and perform containment check - [ ] Update all callers of `_read_file` to pass the workspace path - [ ] Verify TDD test from #10489 now passes - [ ] Run `nox` and verify all tests pass ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off - A Git commit is created with a descriptive message - The commit is pushed to a branch and submitted as a pull request to `master` - The TDD testing issue #10489 is resolved (its `@tdd_expected_fail` scenario now passes) --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

HAL9000 added the

labels

2026-04-18 10:11:55 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-18 10:19:20 +00:00

HAL9000 commented

2026-04-18 10:20:30 +00:00

Author

Owner

🔒 [GROOMED] — CRITICAL SECURITY VULNERABILITY

✅ Triage Assessment: VALID & VERIFIED

This is a legitimate, high-impact security vulnerability that requires immediate attention.

Issue Analysis

Validity: ✅ CONFIRMED

Real path traversal vulnerability in LspRuntime._read_file()
Well-documented with clear code evidence, attack vector, and PoC
Directly exploitable by LLM-driven tool calls
Can read arbitrary files on the filesystem

Labels: ✅ COMPLETE

Priority/Critical ✓ (security vulnerability)
Type/Bug ✓ (bug fix required)
State/Unverified → needs manual change to State/Verified (API limitation)

Milestone: ✅ ASSIGNED

Assigned to v3.2.0 (M3: Decisions + Validations + Invariants)
Appropriate for critical security fixes in active development

Related Issues:

TDD Testing Issue: #10489 (referenced in issue body)

Security Impact

Severity: 🔴 CRITICAL

Attack Vector: LLM-controlled file_path parameter in LSP runtime methods
Affected Methods: get_diagnostics(), get_completions(), get_hover(), get_definitions()
Exploit: Path traversal sequences (../../etc/passwd) bypass workspace containment
Impact: Arbitrary file read on the system running the LSP server

Root Cause

_read_file is a @staticmethod with no access to workspace path context. The workspace path is passed to start_server() but not stored, preventing containment checks.

Proposed Fix

Store workspace path per server: self._workspace_paths: dict[str, str]
Convert _read_file to instance method with workspace path parameter
Add containment check: resolved.startswith(workspace_resolved + os.sep)
Raise LspError for out-of-workspace paths

Acceptance Criteria

All criteria from issue body are clear and actionable:

Workspace path storage per server
Containment check implementation
Path traversal rejection with clear error messages
TDD test #10489 passes
All existing tests pass
Coverage >= 97%

Grooming Notes

Status: Ready for implementation
Complexity: Medium (requires refactoring _read_file and all callers)
Testing: TDD test already prepared in #10489
Priority: IMMEDIATE — security vulnerability in active codebase

Next Steps

⚠️ Manual Action Required: Change State/Unverified → State/Verified (API limitation prevented automated change)
Assign to implementation team
Begin work on subtasks in issue body
Verify TDD test #10489 passes after fix
Submit PR to master branch

Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-pool-supervisor

## 🔒 [GROOMED] — CRITICAL SECURITY VULNERABILITY ### ✅ Triage Assessment: VALID & VERIFIED This is a **legitimate, high-impact security vulnerability** that requires immediate attention. ### Issue Analysis **Validity**: ✅ CONFIRMED - Real path traversal vulnerability in `LspRuntime._read_file()` - Well-documented with clear code evidence, attack vector, and PoC - Directly exploitable by LLM-driven tool calls - Can read arbitrary files on the filesystem **Labels**: ✅ COMPLETE - `Priority/Critical` ✓ (security vulnerability) - `Type/Bug` ✓ (bug fix required) - `State/Unverified` → **needs manual change to `State/Verified`** (API limitation) **Milestone**: ✅ ASSIGNED - Assigned to **v3.2.0** (M3: Decisions + Validations + Invariants) - Appropriate for critical security fixes in active development **Related Issues**: - TDD Testing Issue: #10489 (referenced in issue body) ### Security Impact **Severity**: 🔴 CRITICAL - **Attack Vector**: LLM-controlled `file_path` parameter in LSP runtime methods - **Affected Methods**: `get_diagnostics()`, `get_completions()`, `get_hover()`, `get_definitions()` - **Exploit**: Path traversal sequences (`../../etc/passwd`) bypass workspace containment - **Impact**: Arbitrary file read on the system running the LSP server ### Root Cause `_read_file` is a `@staticmethod` with no access to workspace path context. The workspace path is passed to `start_server()` but not stored, preventing containment checks. ### Proposed Fix 1. Store workspace path per server: `self._workspace_paths: dict[str, str]` 2. Convert `_read_file` to instance method with workspace path parameter 3. Add containment check: `resolved.startswith(workspace_resolved + os.sep)` 4. Raise `LspError` for out-of-workspace paths ### Acceptance Criteria All criteria from issue body are clear and actionable: - [ ] Workspace path storage per server - [ ] Containment check implementation - [ ] Path traversal rejection with clear error messages - [ ] TDD test #10489 passes - [ ] All existing tests pass - [ ] Coverage >= 97% ### Grooming Notes - **Status**: Ready for implementation - **Complexity**: Medium (requires refactoring `_read_file` and all callers) - **Testing**: TDD test already prepared in #10489 - **Priority**: IMMEDIATE — security vulnerability in active codebase ### Next Steps 1. ⚠️ **Manual Action Required**: Change `State/Unverified` → `State/Verified` (API limitation prevented automated change) 2. Assign to implementation team 3. Begin work on subtasks in issue body 4. Verify TDD test #10489 passes after fix 5. Submit PR to `master` branch --- **Automated by CleverAgents Bot** Supervisor: Grooming | Agent: grooming-pool-supervisor

HAL9000 added

and removed

labels

2026-04-18 10:22:18 +00:00

HAL9000 referenced this issue

2026-04-18 10:28:31 +00:00

[AUTO-GROOMER] Status: Active (Cycle 12) #10513

HAL9000 referenced this issue

2026-04-19 04:31:57 +00:00

[AUTO-IMP-POOL] Status: Cycle 1 - Full Pool Active #10685

HAL9000 referenced this issue from a commit

2026-04-19 09:19:06 +00:00

fix(lsp/runtime): add workspace path containment check to _read_file

HAL9000 referenced this issue from a pull request that will close it,

2026-04-19 09:57:19 +00:00

fix(lsp/runtime): add workspace path containment check to _read_file #10739

HAL9000 added

and removed

labels

2026-04-19 09:57:20 +00:00

HAL9000 commented

2026-04-19 09:57:20 +00:00

Author

Owner

[AUTO-IMP-ISSUE-10490] Worker Attempt

Tier: 1 (Sonnet)
Attempted: Implemented workspace path containment check in LspRuntime._read_file
Result: success

What was done:

Added _workspace_paths: dict[str, str] to LspRuntime.__init__ to track per-server workspace roots
Updated start_server to store the resolved workspace path per server name
Updated stop_server and stop_all to clean up workspace path entries
Changed _read_file signature to accept optional workspace_path: str | None = None
Added containment check in _read_file that raises LspError with "outside workspace" message when path traversal is detected
Updated all callers (get_diagnostics, get_completions, get_hover, get_definitions) to pass workspace path to _read_file
Added features/lsp_path_containment.feature with 10 BDD scenarios
Added features/steps/lsp_path_containment_steps.py step definitions

Quality Gates:

lint: passing
typecheck: passing (0 errors, 3 warnings for optional deps)
unit_tests: 10/10 new scenarios passing, 153 total LSP scenarios passing
Full unit test suite: 15,247 scenarios passed

PR: #10739

Automated by CleverAgents Bot
Supervisor: Implementation Pool | Agent: implementation-worker

**[AUTO-IMP-ISSUE-10490] Worker Attempt** - **Tier:** 1 (Sonnet) - **Attempted:** Implemented workspace path containment check in LspRuntime._read_file - **Result:** success ## What was done: 1. Added `_workspace_paths: dict[str, str]` to `LspRuntime.__init__` to track per-server workspace roots 2. Updated `start_server` to store the resolved workspace path per server name 3. Updated `stop_server` and `stop_all` to clean up workspace path entries 4. Changed `_read_file` signature to accept optional `workspace_path: str | None = None` 5. Added containment check in `_read_file` that raises `LspError` with "outside workspace" message when path traversal is detected 6. Updated all callers (`get_diagnostics`, `get_completions`, `get_hover`, `get_definitions`) to pass workspace path to `_read_file` 7. Added `features/lsp_path_containment.feature` with 10 BDD scenarios 8. Added `features/steps/lsp_path_containment_steps.py` step definitions ## Quality Gates: - lint: passing - typecheck: passing (0 errors, 3 warnings for optional deps) - unit_tests: 10/10 new scenarios passing, 153 total LSP scenarios passing - Full unit test suite: 15,247 scenarios passed ## PR: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/10739 --- **Automated by CleverAgents Bot** Supervisor: Implementation Pool | Agent: implementation-worker

HAL9000 referenced this issue from a commit

2026-04-22 10:59:39 +00:00

fix(lsp/runtime): add workspace path containment check to _read_file

HAL9000 referenced this issue from a commit

2026-04-22 20:45:13 +00:00

fix(lsp/runtime): add workspace path containment check to _read_file

HAL9000 referenced this issue from a commit

2026-04-22 23:11:21 +00:00

test(lsp/runtime): fix TDD tag to reference bug issue #10490 and add safe-file scenario

HAL9000 referenced this issue

2026-04-22 23:11:35 +00:00

lsp/runtime: add failing test proving LspRuntime._read_file has no workspace path containment check #10736