From b5110761c2061792774668303b7d5ca510e23769 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Sun, 19 Apr 2026 08:36:29 +0000 Subject: [PATCH 1/3] lsp/runtime: add failing test proving LspRuntime._read_file has no workspace path containment check Added features/tdd_lsp_path_containment.feature: Introduces a TDD Behave feature with a @tdd_expected_fail scenario to demonstrate that LspRuntime._read_file() does not enforce workspace path containment, guiding a fix. Added features/steps/tdd_lsp_path_containment_steps.py: Provides step definitions for executing the new test. ISSUES CLOSED: #10489 --- .../steps/tdd_lsp_path_containment_steps.py | 132 ++++++++++++++++++ features/tdd_lsp_path_containment.feature | 32 +++++ 2 files changed, 164 insertions(+) create mode 100644 features/steps/tdd_lsp_path_containment_steps.py create mode 100644 features/tdd_lsp_path_containment.feature diff --git a/features/steps/tdd_lsp_path_containment_steps.py b/features/steps/tdd_lsp_path_containment_steps.py new file mode 100644 index 000000000..bf68d9bc7 --- /dev/null +++ b/features/steps/tdd_lsp_path_containment_steps.py @@ -0,0 +1,132 @@ +"""Step definitions for TDD Issue #10489 — LspRuntime._read_file path containment. + +This test captures bug #10489: ``LspRuntime._read_file()`` in +``src/cleveragents/lsp/runtime.py`` resolves symlinks and ``..`` components +via ``os.path.realpath()`` but does NOT verify that the resolved path is +contained within the workspace directory. An attacker who can control +``file_path`` (e.g. via a malicious LLM tool call) could read any file on +the filesystem by supplying a traversal path such as +``/../../../etc/passwd``. + +The test uses the ``@tdd_expected_fail`` tag until the fix in #10489 is +merged. The tag inversion mechanism causes CI to report the scenario as +passed while the bug is still unfixed. +See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags. +""" + +from __future__ import annotations + +import os +import tempfile + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.lsp.errors import LspError +from cleveragents.lsp.runtime import LspRuntime + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given("lsp_pc a workspace directory containing a safe file") +def step_lsp_pc_workspace_with_safe_file(context: Context) -> None: + """Create a temporary workspace directory with a safe file inside it.""" + context.lsp_pc_workspace = tempfile.mkdtemp(prefix="lsp_pc_workspace_") + safe_file_path = os.path.join(context.lsp_pc_workspace, "safe.py") + with open(safe_file_path, "w", encoding="utf-8") as f: + f.write("# safe content\n") + context.lsp_pc_safe_file = safe_file_path + + def _cleanup() -> None: + import shutil + + shutil.rmtree(context.lsp_pc_workspace, ignore_errors=True) + + context.add_cleanup(_cleanup) + + +@given("lsp_pc a sensitive file exists outside the workspace") +def step_lsp_pc_sensitive_file_outside_workspace(context: Context) -> None: + """Create a temporary file OUTSIDE the workspace directory.""" + fd, outside_path = tempfile.mkstemp( + prefix="lsp_pc_sensitive_", + suffix=".txt", + ) + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write("sensitive content — should not be readable via workspace traversal\n") + context.lsp_pc_outside_file = outside_path + + def _cleanup() -> None: + if os.path.exists(outside_path): + os.unlink(outside_path) + + context.add_cleanup(_cleanup) + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when("lsp_pc I call _read_file with a path that traverses outside the workspace") +def step_lsp_pc_call_read_file_traversal(context: Context) -> None: + """Attempt to read the outside file via a path traversal from the workspace. + + Constructs a path like ``/../`` + which resolves to the sensitive file outside the workspace. The current + implementation of ``_read_file`` does NOT check workspace containment, + so it will successfully read the file instead of raising ``LspError``. + """ + outside_basename = os.path.basename(context.lsp_pc_outside_file) + # Build a traversal path: workspace + "/../" + # os.path.realpath will resolve this to the actual outside path. + traversal_path = os.path.join( + context.lsp_pc_workspace, + "..", + outside_basename, + ) + context.lsp_pc_traversal_path = traversal_path + context.lsp_pc_error = None + try: + context.lsp_pc_result = LspRuntime._read_file(traversal_path) + except LspError as exc: + context.lsp_pc_error = exc + except Exception as exc: + context.lsp_pc_error = exc + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then('lsp_pc an LspError should be raised with message containing "{expected_msg}"') +def step_lsp_pc_lsp_error_raised(context: Context, expected_msg: str) -> None: + """Assert that _read_file raised LspError with the expected message. + + Bug #10489: This assertion FAILS while the bug exists because + ``_read_file`` does NOT raise ``LspError`` for paths outside the + workspace — it successfully reads the sensitive file instead. + + The ``@tdd_expected_fail`` tag on the scenario inverts this failure + so CI reports the scenario as passed until the fix is merged. + """ + assert context.lsp_pc_error is not None, ( + "Bug #10489: LspRuntime._read_file() did NOT raise any error when " + f"given a path traversal to a file outside the workspace.\n" + f" Traversal path : {context.lsp_pc_traversal_path!r}\n" + f" Resolved path : {os.path.realpath(context.lsp_pc_traversal_path)!r}\n" + f" Workspace : {context.lsp_pc_workspace!r}\n" + f" File was read successfully — this proves the path containment " + f"check is missing." + ) + assert isinstance(context.lsp_pc_error, LspError), ( + f"Expected LspError but got {type(context.lsp_pc_error).__name__}: " + f"{context.lsp_pc_error}" + ) + assert expected_msg in str(context.lsp_pc_error), ( + f"Expected error message to contain {expected_msg!r}, " + f"but got: {context.lsp_pc_error}" + ) diff --git a/features/tdd_lsp_path_containment.feature b/features/tdd_lsp_path_containment.feature new file mode 100644 index 000000000..43c2c04fc --- /dev/null +++ b/features/tdd_lsp_path_containment.feature @@ -0,0 +1,32 @@ +@tdd_issue @tdd_issue_10489 +Feature: TDD Issue #10489 — LspRuntime._read_file has no workspace path containment check + As a security-conscious developer + I want LspRuntime._read_file() to restrict file access to the workspace directory + So that a malicious or misconfigured LLM tool call cannot read arbitrary files + + This test captures bug #10489. The ``_read_file`` static method in + ``src/cleveragents/lsp/runtime.py`` calls ``os.path.realpath()`` to + resolve symlinks and ``..`` components, but does NOT verify that the + resolved path is contained within the workspace directory. An attacker + who can control ``file_path`` (e.g. via a malicious LLM tool call) could + read any file on the filesystem by supplying a path such as + ``/../../../etc/passwd``. + + The method is a ``@staticmethod`` and has no access to the workspace path, + so it cannot perform containment checks without a design change. + + The scenario below uses ``@tdd_expected_fail`` because the assertion + FAILS while the bug exists — ``_read_file`` does NOT raise ``LspError`` + when given a path that resolves outside the workspace. The tag inversion + mechanism causes CI to report the scenario as passed until the fix lands. + Once the fix for #10489 is merged, the ``@tdd_expected_fail`` tag must be + removed so the scenario runs normally. + + See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags. + + @tdd_expected_fail + Scenario: _read_file should reject paths outside workspace but currently does not + Given lsp_pc a workspace directory containing a safe file + And lsp_pc a sensitive file exists outside the workspace + When lsp_pc I call _read_file with a path that traverses outside the workspace + Then lsp_pc an LspError should be raised with message containing "outside workspace" -- 2.52.0 From e619c1be583957607544ad4cdc3ada57fd796172 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Wed, 22 Apr 2026 23:11:20 +0000 Subject: [PATCH 2/3] test(lsp/runtime): fix TDD tag to reference bug issue #10490 and add safe-file scenario MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corrected @tdd_issue_10489 → @tdd_issue_10490 so the tag references the actual bug issue (path traversal vulnerability) rather than the TDD issue itself. Added a positive scenario verifying _read_file can still read files inside the workspace, as suggested in review. ISSUES CLOSED: #10489 --- .../steps/tdd_lsp_path_containment_steps.py | 33 ++++++++++++++++--- features/tdd_lsp_path_containment.feature | 13 +++++--- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/features/steps/tdd_lsp_path_containment_steps.py b/features/steps/tdd_lsp_path_containment_steps.py index bf68d9bc7..c6bc0c5f7 100644 --- a/features/steps/tdd_lsp_path_containment_steps.py +++ b/features/steps/tdd_lsp_path_containment_steps.py @@ -1,6 +1,6 @@ -"""Step definitions for TDD Issue #10489 — LspRuntime._read_file path containment. +"""Step definitions for TDD Issue #10490 — LspRuntime._read_file path containment. -This test captures bug #10489: ``LspRuntime._read_file()`` in +This test captures bug #10490: ``LspRuntime._read_file()`` in ``src/cleveragents/lsp/runtime.py`` resolves symlinks and ``..`` components via ``os.path.realpath()`` but does NOT verify that the resolved path is contained within the workspace directory. An attacker who can control @@ -8,7 +8,7 @@ contained within the workspace directory. An attacker who can control the filesystem by supplying a traversal path such as ``/../../../etc/passwd``. -The test uses the ``@tdd_expected_fail`` tag until the fix in #10489 is +The test uses the ``@tdd_expected_fail`` tag until the fix in #10490 is merged. The tag inversion mechanism causes CI to report the scenario as passed while the bug is still unfixed. See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags. @@ -97,6 +97,16 @@ def step_lsp_pc_call_read_file_traversal(context: Context) -> None: context.lsp_pc_error = exc +@when("lsp_pc I call _read_file with the safe file path") +def step_lsp_pc_call_read_file_safe(context: Context) -> None: + """Call _read_file with a path to the safe file inside the workspace.""" + context.lsp_pc_error = None + try: + context.lsp_pc_result = LspRuntime._read_file(context.lsp_pc_safe_file) + except Exception as exc: + context.lsp_pc_error = exc + + # --------------------------------------------------------------------------- # Then steps # --------------------------------------------------------------------------- @@ -106,7 +116,7 @@ def step_lsp_pc_call_read_file_traversal(context: Context) -> None: def step_lsp_pc_lsp_error_raised(context: Context, expected_msg: str) -> None: """Assert that _read_file raised LspError with the expected message. - Bug #10489: This assertion FAILS while the bug exists because + Bug #10490: This assertion FAILS while the bug exists because ``_read_file`` does NOT raise ``LspError`` for paths outside the workspace — it successfully reads the sensitive file instead. @@ -114,7 +124,7 @@ def step_lsp_pc_lsp_error_raised(context: Context, expected_msg: str) -> None: so CI reports the scenario as passed until the fix is merged. """ assert context.lsp_pc_error is not None, ( - "Bug #10489: LspRuntime._read_file() did NOT raise any error when " + "Bug #10490: LspRuntime._read_file() did NOT raise any error when " f"given a path traversal to a file outside the workspace.\n" f" Traversal path : {context.lsp_pc_traversal_path!r}\n" f" Resolved path : {os.path.realpath(context.lsp_pc_traversal_path)!r}\n" @@ -130,3 +140,16 @@ def step_lsp_pc_lsp_error_raised(context: Context, expected_msg: str) -> None: f"Expected error message to contain {expected_msg!r}, " f"but got: {context.lsp_pc_error}" ) + + +@then("lsp_pc the file content should be returned successfully") +def step_lsp_pc_file_content_returned(context: Context) -> None: + """Assert that _read_file successfully returned the safe file content.""" + assert context.lsp_pc_error is None, ( + f"Expected _read_file to succeed for a file inside the workspace, " + f"but got error: {context.lsp_pc_error}" + ) + assert context.lsp_pc_result == "# safe content\n", ( + f"Expected file content '# safe content\\n', " + f"but got: {context.lsp_pc_result!r}" + ) diff --git a/features/tdd_lsp_path_containment.feature b/features/tdd_lsp_path_containment.feature index 43c2c04fc..f19080dcd 100644 --- a/features/tdd_lsp_path_containment.feature +++ b/features/tdd_lsp_path_containment.feature @@ -1,10 +1,10 @@ -@tdd_issue @tdd_issue_10489 -Feature: TDD Issue #10489 — LspRuntime._read_file has no workspace path containment check +@tdd_issue @tdd_issue_10490 +Feature: TDD Issue #10490 — LspRuntime._read_file has no workspace path containment check As a security-conscious developer I want LspRuntime._read_file() to restrict file access to the workspace directory So that a malicious or misconfigured LLM tool call cannot read arbitrary files - This test captures bug #10489. The ``_read_file`` static method in + This test captures bug #10490. The ``_read_file`` static method in ``src/cleveragents/lsp/runtime.py`` calls ``os.path.realpath()`` to resolve symlinks and ``..`` components, but does NOT verify that the resolved path is contained within the workspace directory. An attacker @@ -19,7 +19,7 @@ Feature: TDD Issue #10489 — LspRuntime._read_file has no workspace path contai FAILS while the bug exists — ``_read_file`` does NOT raise ``LspError`` when given a path that resolves outside the workspace. The tag inversion mechanism causes CI to report the scenario as passed until the fix lands. - Once the fix for #10489 is merged, the ``@tdd_expected_fail`` tag must be + Once the fix for #10490 is merged, the ``@tdd_expected_fail`` tag must be removed so the scenario runs normally. See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags. @@ -30,3 +30,8 @@ Feature: TDD Issue #10489 — LspRuntime._read_file has no workspace path contai And lsp_pc a sensitive file exists outside the workspace When lsp_pc I call _read_file with a path that traverses outside the workspace Then lsp_pc an LspError should be raised with message containing "outside workspace" + + Scenario: _read_file can read a file inside the workspace + Given lsp_pc a workspace directory containing a safe file + When lsp_pc I call _read_file with the safe file path + Then lsp_pc the file content should be returned successfully -- 2.52.0 From 446f447c036294975e5c1b1395637021abd9d1d1 Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Fri, 24 Apr 2026 13:39:35 +0000 Subject: [PATCH 3/3] style(test): apply ruff format to tdd_lsp_path_containment_steps.py --- features/steps/tdd_lsp_path_containment_steps.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/features/steps/tdd_lsp_path_containment_steps.py b/features/steps/tdd_lsp_path_containment_steps.py index c6bc0c5f7..850e2554d 100644 --- a/features/steps/tdd_lsp_path_containment_steps.py +++ b/features/steps/tdd_lsp_path_containment_steps.py @@ -150,6 +150,5 @@ def step_lsp_pc_file_content_returned(context: Context) -> None: f"but got error: {context.lsp_pc_error}" ) assert context.lsp_pc_result == "# safe content\n", ( - f"Expected file content '# safe content\\n', " - f"but got: {context.lsp_pc_result!r}" + f"Expected file content '# safe content\\n', but got: {context.lsp_pc_result!r}" ) -- 2.52.0