fix(lsp): reject non-ASCII header bytes in transport to prevent header injection

Fix all code review blockers for PR #10608 addressing CVE security
vulnerability (issue #7112) in the LSP stdio transport layer.

Production fixes (src/cleveragents/lsp/transport.py):
- Remove redundant inline ``LspError`` imports from function bodies
  (lines 117, 124) now that top-level import exists at line 30,
  complying with Python import rules requiring all imports at the
  file top.
- Update ``_read_one_message()`` docstring to document strict ASCII
  enforcement via ``errors="strict"``, ``LspError`` exceptions raised
  on non-ASCII byte detection, and the printable-ASCII guard that
  rejects characters outside codepoint range 0x20 (space) through
  0x7E (tilde).

Test fixes (features/lsp_header_injection_security.feature):
- Move feature-level tags (@tdd_issue, @tdd_issue_7112) to their own
  lines before ``Feature:`` keyword, fixing Gherkin parse errors.
- Remove trailing whitespace from scenario title lines and step
  definitions (lines 14, 22, 30).

Test step fixes (features/steps/lsp_header_injection_security_steps.py):
- Fix ``_patched_select()`` to return proper 3-tuple
  ``([readable[0]], [], [])`` matching the ``select.select()`` API
  contract instead of a single-element list that caused ValueError
  during tuple unpacking.
- Remove unregistered custom parse type ``:r`` from ``@given``
  decorator; use plain string parameter and ``eval()`` inside step
  body for bytes literal parsing.
- Add top-level ``LspError`` import, remove inline function import.
- Remove trailing whitespace (tabs) on line after docstring closing.

Documentation fixes:
- CHANGELOG.md: Remove leading space before ``-`` bullet in the LSP
  security entry to fix ruff formatting lint failure.
- CONTRIBUTORS.md: Move HAL 9000 prose contribution entry from the
  name list section (reserved for ``* Name <email>`` format) into
  the ``# Details`` section, consistent with all other entries.

CI compliance checklist:
[ ] CHANGELOG.md -- updated (removed leading space before LSP bullet)
[ ] CONTRIBUTORS.md -- updated (moved prose entry to Details section)
[ ] Commit footer -- includes ISSUES CLOSED: #7112
[ ] CI passes -- lint, typecheck, unit_tests, coverage >= 97%
[ ] BDD/Behave tests -- fixed: _patched_select returns 3-tuple, :r parser type removed, tags on own lines, trailing whitespace removed
[ ] Epic reference -- issue #7112 parent epic is #824
[ ] Labels -- State/In Review, Priority/Critical, MoSCoW/Must Have, Type/Bug (already present)
[ ] Milestone -- v3.6.0 assigned

ISSUES CLOSED: #7112

CHANGELOG.md

@@ -160,7 +160,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
    relative globs also match absolute paths. Added BDD regression tests in
    `execute_phase_context_assembler_coverage.feature` and `project_context_phase_analysis.feature`.
 
- - **LSP transport header injection vulnerability (#7112)**: Replaced ``errors="replace"`` with
+- **LSP transport header injection vulnerability (#7112)**: Replaced ``errors="replace"`` with
   ``errors="strict"`` in the ``_read_one_message()`` method of
   ``src/cleveragents/lsp/transport.py``, so that non-ASCII bytes in message headers raise a
   clearly-typed :class:`~cleveragents.lsp.errors.LspError` instead of silently replacing the

CONTRIBUTORS.md

@@ -2,7 +2,6 @@
 
 * Aditya Chhabra <aditya.chhabra@cleverthis.com>
 * Brent E. Edwards <brent.edwards@cleverthis.com>
-* HAL 9000 has contributed the LSP transport header injection security fix (#7112): replaced non-strict ASCII decoding in the LSP stdio transport's ``_read_one_message()`` method to prevent message boundary manipulation via encoding attacks, with comprehensive BDD test coverage for the vulnerability scenario.
 * HAL 9000 <hal9000@cleverthis.com>
 * Hamza Khyari <hamza.khyari@cleverthis.com>
 * Jeffrey Phillips Freeman <jeffrey.freeman@syncleus.com>
@@ -26,6 +25,7 @@ Below are some of the specific details of various contributions.
 * HAL 9000 has contributed the decision recording hook for the Strategize phase (issue #8522): captures every decision point with question, chosen option, alternatives, confidence, rationale, and full context snapshot for replay and correction.
 * HAL 9000 has contributed automated specification maintenance, documentation updates, and bot-driven PR authorship.
 * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc.
+* HAL 9000 has contributed the LSP transport header injection security fix (#7112): replaced non-strict ASCII decoding in the LSP stdio transport's ``_read_one_message()`` method with `errors="strict"`, added a printable-ASCII guard rejecting characters outside 0x20–0x7E, and added comprehensive BDD test coverage for all three header injection vulnerability scenarios.
 * HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system.
 * HAL 9000 has contributed the file edit encoding parameter fix (PR #8258 / issue #7559).
 * HAL 9000 has contributed the architecture-pool-supervisor milestone assignment feature (PR #8188 / issue #7521): added `forgejo_update_pull_request` permission and documented the PR workflow for major spec changes, enabling automatic milestone assignment for specification PRs.

features/lsp_header_injection_security.feature

@@ -1,4 +1,6 @@
-@tdd_issue @tdd_issue_7112 Feature: LSP transport header injection vulnerability (issue #7112)
+@tdd_issue
+@tdd_issue_7112
+Feature: LSP transport header injection vulnerability (issue #7112)
 
     The _read_one_message() method in the LSP stdio transport must strictly
     enforce ASCII-only headers.  Non-ASCII bytes silently replaced by the
@@ -11,7 +13,7 @@
     Then it should raise an LspError exception
     And the error message must contain "non-ASCII"
 
-@tdd_issue_7112 Scenario: Non-ASCII byte in a valid Content-Length name raises LspError  
+@tdd_issue_7112 Scenario: Non-ASCII byte in a valid Content-Length name raises LspError
     Given a Transport mock with BytesIO stream containing b"Content-Length\xef\x08: 99\r\n\r\n{\"jsonrpc\":\"2.0\"}"
     When _read_one_message() is invoked
     Then it should raise an LspError exception
@@ -19,7 +21,7 @@
 
 @tdd_issue_7112 Scenario: Non-ASCII byte in an unrecognized header raises LspError
     Given a Transport mock with BytesIO stream containing b"Cache-Control\xef\x08: no-cache\r\nContent-Length: 5\r\n\r\n{\"id\":1}"
-    When _read_one_message() is invoked  
+    When _read_one_message() is invoked
     Then it should raise an LspError exception
     And the error message must contain "non-ASCII"
 
@@ -27,6 +29,6 @@
     Given a Transport mock with BytesIO stream containing b"Content-Length: 46\r\n\r\n{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":\"success\"}"
     When _read_one_message() is invoked
     Then it should return a parsed JSON dict
-    And the result must contain "jsonrpc" == "2.0"  
+    And the result must contain "jsonrpc" == "2.0"
     And the result must contain "id" == 1
     And the result must contain "result" == "success"

features/steps/lsp_header_injection_security_steps.py

@@ -16,20 +16,22 @@ from unittest.mock import MagicMock, patch
 
 from behave import given, then, when
 
+from cleveragents.lsp.errors import LspError
 from cleveragents.lsp.transport import StdioTransport
 
 
 # ── Given steps ──────────────────────────────────────────────────────────
 
 
-@given("a Transport mock with BytesIO stream containing {raw_headers:r}")
-def step_transport_with_streams(context: Any, raw_headers: bytes) -> None:
+@given("a Transport mock with BytesIO stream containing {raw_headers}")
+def step_transport_with_streams(context: Any, raw_headers: str) -> None:
     """Set up a transport whose subprocess.stdout is a BytesIO initialised
     with the specified raw header+body bytes."""
+    data: bytes = eval(raw_headers)
     transport = object.__new__(StdioTransport)  # Bypass __init__ validation
     transport._process = MagicMock()
     transport._process.poll.return_value = None  # Pretend process is alive
-    context.stream = BytesIO(raw_headers)
+    context.stream = BytesIO(data)
     transport._process.stdout = context.stream
     context.transport = transport
     context.mock_process = transport._process
@@ -47,11 +49,14 @@ def step_invoke_read_message(context: Any) -> None:
     unblocking ``stdout.readline()`` and driving the header-reading loop with
     the data already sitting in the ``BytesIO`` buffer.
     """
-    
-    def _patched_select(readable, *_args: Any, timeout: float | None = None) -> list[Any]:
+    def _patched_select(
+        readable: list[Any],
+        *_args: Any,
+        **_kwargs: Any,
+    ) -> tuple[list[Any], list[Any], list[Any]]:
         if readable:
-            return [readable[0]]  # Mark as ready
-        return []  # Timeout
+            return ([readable[0]], [], [])  # Proper 3-tuple per select.select contract
+        return ([], [], [])                 # Timeout path (also a proper 3-tuple)
 
     with patch.object(select, "select", side_effect=_patched_select):
         context.raised_error = None
@@ -67,7 +72,6 @@ def step_invoke_read_message(context: Any) -> None:
 @then("it should raise an LspError exception")
 def step_raises_lsp_error(context: Any) -> None:
     assert context.raised_error is not None, "Expected an error to be raised"
-    from cleveragents.lsp.errors import LspError
     assert isinstance(
         context.raised_error, (LspError, UnicodeDecodeError)
     ), f"Expected LspError or UnicodeDecodeError, got {type(context.raised_error).__name__}"
@@ -106,11 +110,6 @@ def step_result_has_result(context: Any) -> None:
     assert context.read_result.get("result") == "success"
 
 
-@then("no error should be raised")
-def step_no_error(context: Any) -> None:
-    assert context.raised_error is None
-
-
 @then('the result must contain "Content-Length"')
 def step_result_has_content_length(context: Any) -> None:
     assert isinstance(context.read_result, dict), "Result should be a dict"

src/cleveragents/lsp/transport.py

@@ -114,15 +114,11 @@ class StdioTransport:
                 cwd=self._cwd,
             )
         except FileNotFoundError as exc:
-            from cleveragents.lsp.errors import LspError
-
             raise LspError(
                 f"LSP server command not found: {self._command}",
                 details={"command": self._command, "args": self._args},
             ) from exc
         except OSError as exc:
-            from cleveragents.lsp.errors import LspError
-
             raise LspError(
                 f"Failed to start LSP server: {exc}",
                 details={"command": self._command, "error": str(exc)},
@@ -224,7 +220,23 @@ class StdioTransport:
             return self._read_one_message(effective_timeout)
 
     def _read_one_message(self, timeout: float) -> dict[str, Any] | None:
-        """Parse a single ``Content-Length`` framed JSON-RPC message."""
+        """Parse a single ``Content-Length``-framed JSON-RPC message.
+
+        Header lines are decoded with ``errors="strict"``; any non-ASCII byte
+        raises :class:`~cleveragents.lsp.errors.LspError`.  An additional
+        printable-ASCII guard rejects decoded headers containing characters
+        outside the codepoint range 0x20 (space) to 0x7E (tilde).
+
+        Returns
+        -------
+        dict[str, Any] | None
+            Parsed JSON body, or ``None`` on EOF / timeout.
+
+        Raises
+        ------
+        LspError
+            If a header contains non-ASCII bytes or non-printable characters.
+        """
         assert self._process is not None
         assert self._process.stdout is not None