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..850e2554d --- /dev/null +++ b/features/steps/tdd_lsp_path_containment_steps.py @@ -0,0 +1,154 @@ +"""Step definitions for TDD Issue #10490 — LspRuntime._read_file path containment. + +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 +``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 #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. +""" + +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 + + +@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 +# --------------------------------------------------------------------------- + + +@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 #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. + + 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 #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" + 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}" + ) + + +@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', but got: {context.lsp_pc_result!r}" + ) diff --git a/features/tdd_lsp_path_containment.feature b/features/tdd_lsp_path_containment.feature new file mode 100644 index 000000000..f19080dcd --- /dev/null +++ b/features/tdd_lsp_path_containment.feature @@ -0,0 +1,37 @@ +@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 #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 + 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 #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. + + @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" + + 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