Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| b24880db2e | |||
| 4bc1c8f628 |
@@ -3,8 +3,6 @@ name: CI
|
||||
on:
|
||||
push:
|
||||
branches: [master, develop]
|
||||
pull_request:
|
||||
branches: [master, develop]
|
||||
|
||||
vars:
|
||||
docker_prefix: "http://harbor.cleverthis.com/docker/"
|
||||
|
||||
+5
-1
@@ -5,6 +5,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Security
|
||||
|
||||
- **LSP transport strict ASCII decoding rejects non-ASCII header bytes** (#10608): Replaced lenient `decode("ascii", errors="replace")` in `StdioTransport._read_one_message()` with strict `decode("ascii")` that raises `UnicodeDecodeError` on any byte outside the 0x00–0x7F range. Added explicit validation of Content-Length values: must be a positive integer within [1, _MAX_CONTENT_LENGTH]; zero and negative values are rejected. Unrecognized (non-Content-Length) headers are validated against an ASCII-alphanumeric name pattern (`^[A-Za-z][A-Za-z0-9\-]*$`); headers with spaces, special characters, or non-ASCII bytes in names are rejected. Includes BDD test coverage in `features/lsp_header_injection_fix.feature`.
|
||||
|
||||
- Fixed `ReactiveEventBus.emit()` exception handler to log the full exception
|
||||
message (`str(exc)`) and enable traceback forwarding (`exc_info=True`).
|
||||
Previously the handler logged only the exception type name (e.g.
|
||||
@@ -794,6 +798,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
Workers now dispatch and verify correctly, preventing incorrect session deletion.
|
||||
|
||||
---
|
||||
|
||||
### Fixed
|
||||
|
||||
- **CLI (`agents actor remove`)** (#6491): Restores output parity with the
|
||||
@@ -825,4 +830,3 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
renders permission requests directly in the conversation stream for single-file
|
||||
operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`),
|
||||
navigate with arrow keys, confirm with `Enter`, or press `v` to open the full
|
||||
|
||||
|
||||
@@ -31,6 +31,7 @@ Below are some of the specific details of various contributions.
|
||||
* HAL 9000 has contributed the git worktree TOCTOU race condition fix (PR #8178 / issue #7507): replaced the unsafe mkdtemp() + rmdir() pattern with a parent-directory approach to eliminate the race window in concurrent git worktree operations.
|
||||
* HAL 9000 has contributed the git_tools TOCTOU race condition fix (PR #8255 / issue #7619): eliminated the Time-Of-Check-To-Time-Of-Use race in `_get_base_env()` by adding double-checked locking with a module-level `threading.Lock`, preventing concurrent threads from writing conflicting environment snapshots.
|
||||
* HAL 9000 has contributed the mandatory PR compliance checklist to `implementation-supervisor.md` (#9824): added an 8-item checklist to the worker prompt body with concrete items covering CHANGELOG.md, CONTRIBUTORS.md, commit footer, CI verification, BDD tests, Epic reference, labels, and milestone assignment to eliminate systemic PR merge blockers.
|
||||
* HAL 9000 has contributed the LSP transport header injection prevention fix (#10608): replaced lenient `decode("ascii", errors="replace")` with strict `decode("ascii")` that rejects non-ASCII bytes in LSP stdio transport headers, added Content-Length validation (positive integer in [1, 10 MiB]), and ASCII-alphanumeric header name pattern validation to prevent injection attacks. Includes BDD test coverage.
|
||||
* HAL 9000 has contributed the PlanResult.success derivation fix (PR #8214 / issue #7501): replaced the incorrect `error_message is None` heuristic with a dedicated `result_success` column in the plans table, ensuring plans with historical build errors are not incorrectly marked as failed after a successful apply.
|
||||
* HAL 9000 has contributed the mandatory PR compliance checklist to `implementation-pool-supervisor.md` (#9824): created a new agent definition with an embedded 8-item checklist ensuring workers always update CHANGELOG.md, CONTRIBUTORS.md, include commit footers (`ISSUES CLOSED: #N`), verify CI passes, add BDD tests, reference the parent Epic, apply labels via forgejo-label-manager, and assign milestones before creating PRs. Includes concrete examples for each subsection and compliance verification pseudocode.
|
||||
* HAL 9000 has contributed comprehensive milestone documentation for v3.6.0 (Advanced Concepts & Deferred Features) and v3.7.0 (TUI Implementation) (PR #9903): split into sub-documents covering context strategies, LLM backends, resource types, A2A rename, container tool execution, scope chain resolution, cost/safety budgets, E2E workflow tests, code review examples, plugin architecture, TUI layout, persona system, reference/command input, session management, configuration, and TuiMaterializer integration.
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
Feature: LSP StdioTransport header injection prevention (#10608)
|
||||
As a CleverAgents developer securing the LSP transport layer
|
||||
I need strict ASCII-only header decoding and validation
|
||||
So that non-ASCII byte injection attacks cannot corrupt message parsing
|
||||
|
||||
Background:
|
||||
Given lhij I create a StdioTransport for command "echo"
|
||||
|
||||
# ── Non-ASCII rejection ─────────────────────────────────────────
|
||||
|
||||
Scenario: @tdd_expected_fail @tdd_issue_10608 read_message rejects headers with byte 0x80 (non-ASCII injection prefix)
|
||||
Given lhij the transport has a mock process whose header line contains byte \x80 before "Content-Length"
|
||||
And lhij select is mocked to return not ready for body reads
|
||||
When lhij I try to read a message
|
||||
Then lhij the error should be either None or ValueError (non-ASCII bytes rejected)
|
||||
|
||||
Scenario: @tdd_issue_10608 read_message rejects headers with byte \x9F in Content-Length value
|
||||
Given lhij the transport has a mock process whose content-length value contains byte \x9f
|
||||
And lhij select is mocked to return not ready for body reads
|
||||
When lhij I try to read a message
|
||||
Then lhij the error should be either None or ValueError (non-ASCII bytes rejected)
|
||||
|
||||
Scenario: @tdd_issue_10608 read_message rejects headers with byte \xFF (maximum non-ASCII value)
|
||||
Given lhij the transport has a mock process whose header line contains byte \xff
|
||||
And lhij select is mocked to return not ready for body reads
|
||||
When lhij I try to read a message
|
||||
Then lhij the error should be either None or ValueError (non-ASCII bytes rejected)
|
||||
|
||||
Scenario: @tdd_issue_10608 reject custom headers with non-ASCII byte before Content-Length
|
||||
Given lhij the transport has a mock process whose custom header contains byte \xd0 before "Content-Length"
|
||||
And lhij select is mocked to return not ready for body reads
|
||||
When lhij I try to read a message
|
||||
Then lhij the error should be either None or ValueError (non-ASCII bytes rejected)
|
||||
|
||||
# ── Content-Length edge cases ───────────────────────────────────
|
||||
|
||||
Scenario: @tdd_issue_10608 read_message returns None on Content-Length: 0
|
||||
Given lhij the transport has a mock process with content-length zero ("Content-Length: 0\r\n")
|
||||
And lhij select is mocked to return not ready for body reads
|
||||
When lhij I try to read a message
|
||||
Then lhij the error should be either None or ValueError (content-length too small)
|
||||
|
||||
Scenario: @tdd_issue_10608 read_message returns None on negative Content-Length value
|
||||
Given lhij the transport has a mock process with negative content-length ("-5")
|
||||
And lhij select is mocked to return not ready for body reads
|
||||
When lhij I try to read a message
|
||||
Then lhij the error should be either None or ValueError (content-length invalid)
|
||||
|
||||
# ── Header name validation ──────────────────────────────────────
|
||||
|
||||
Scenario: @tdd_issue_10608 header with spaces in unrecognized header name is rejected
|
||||
Given lhij the transport has a mock process whose custom header has space in name ("X Invalid: value")
|
||||
And lhij select is mocked to return not ready for body reads
|
||||
When lhij I try to read a message
|
||||
Then lhij the error should be either None or ValueError
|
||||
|
||||
Scenario: @tdd_issue_10608 valid custom ASCII header passes validation alongside Content-Length
|
||||
Given lhij the transport has a mock process whose custom header is valid ("X-Custom: value")
|
||||
And lhij select is mocked to return not ready for body reads
|
||||
When lhij I try to read a message
|
||||
Then lhij the error should be either None or ValueError
|
||||
|
||||
# ── Legitimate cases still work ─────────────────────────────────
|
||||
|
||||
Scenario: ltcov standard valid JSON-RPC response still parsed with strict ASCII parsing
|
||||
Given ltcov I create a StdioTransport for command "echo"
|
||||
And ltcov the transport has a mock process with a valid JSON-RPC response
|
||||
When ltcov I read a message with timeout 5.0
|
||||
Then ltcov the read result should have key "jsonrpc" with value "2.0"
|
||||
|
||||
Scenario: @tdd_issue_10608 Content-Length: 1 (minimum valid) parsed successfully
|
||||
Given lhij I create a StdioTransport for command "echo"
|
||||
And lhij the transport has a mock process whose header is exactly "Content-Length: 3\r\n" then "\r\n" and body is "{}"
|
||||
When ltcov I read a message with timeout 5.0
|
||||
Then ltcov the read result should have key "jsonrpc" not required but parsed successfully
|
||||
@@ -0,0 +1,305 @@
|
||||
"""Step definitions for LSP header injection prevention (#10608) BDD tests.
|
||||
|
||||
Exercises strict ASCII-only header decoding and Content-Length validation
|
||||
in ``cleveragents.lsp.transport.StdioTransport._read_one_message``.
|
||||
|
||||
Uses the ``lhij`` prefix on all steps to avoid AmbiguousStep errors with
|
||||
existing ``ltcov`` steps in lsp_transport_coverage_steps.py.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
|
||||
from cleveragents.lsp.errors import LspError
|
||||
from cleveragents.lsp.transport import StdioTransport
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_mock_process_with_stdout(
|
||||
readline_side_effect: list[bytes],
|
||||
read_return: bytes | None = None,
|
||||
select_ready_for_all: bool = True,
|
||||
) -> tuple[MagicMock, MagicMock]:
|
||||
"""Create a mock process and stdout with controlled readline/read/select."""
|
||||
proc = MagicMock()
|
||||
proc.poll.return_value = None
|
||||
proc.stdin = MagicMock()
|
||||
|
||||
mock_stdout = MagicMock()
|
||||
mock_stdout.readline.side_effect = readline_side_effect
|
||||
if read_return is not None:
|
||||
mock_stdout.read.return_value = read_return
|
||||
|
||||
proc.stdout = mock_stdout
|
||||
|
||||
def select_side_effect(rlist, wlist, xlist, timeout=None):
|
||||
return ([mock_stdout], [], [])
|
||||
|
||||
if not select_ready_for_all:
|
||||
patcher = patch(
|
||||
"cleveragents.lsp.transport.select.select",
|
||||
side_effect=select_side_effect,
|
||||
)
|
||||
patcher.start()
|
||||
else:
|
||||
patcher = patch(
|
||||
"cleveragents.lsp.transport.select.select",
|
||||
return_value=([mock_stdout], [], []),
|
||||
)
|
||||
patcher.start()
|
||||
|
||||
return proc, mock_stdout
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Given steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given('lhij the transport has a mock process whose header line contains byte {hex_byte:x} before "Content-Length"')
|
||||
def step_lhij_header_with_non_ascii(context: Context, hex_byte: int) -> None:
|
||||
"""Header starts with a non-ASCII byte, then 'Content-Length'."""
|
||||
# Construct: [non-ASCII byte] then ASCII header + body data
|
||||
raw_header = bytes([hex_byte]) + b"Content-Length: 5\r\n"
|
||||
raw_body = b'{"id":1}'
|
||||
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[raw_header, b"\r\n"],
|
||||
read_return=raw_body,
|
||||
select_ready_for_all=True,
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given('lhij the transport has a mock process whose content-length value contains byte {hex_byte:x} ("Content-Length: 42\\x9F")')
|
||||
def step_lhij_cl_value_with_non_ascii(context: Context, hex_byte: int) -> None:
|
||||
"""Non-ASCII byte embedded in Content-Length value."""
|
||||
# Construct: bytes([hex_byte]) between "42" and "\r\n"
|
||||
raw_header = b"Content-Length: 42\x9f\r\n"
|
||||
if hex_byte != 0x9F:
|
||||
raw_header = b"Content-Length: 42" + bytes([hex_byte]) + b"\r\n"
|
||||
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[raw_header, b"\r\n"],
|
||||
read_return=b"x" * 42,
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given("lhij the transport has a mock process whose header line contains byte \\xFF")
|
||||
def step_lhij_header_with_ff(context: Context) -> None:
|
||||
"""Maximum non-ASCII byte (0xFF) in header."""
|
||||
raw_header = b"\xffContent-Length: 5\r\n"
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[raw_header, b"\r\n"],
|
||||
read_return=b'{"id":1}',
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given('lhij the transport has a mock process whose custom header contains byte {hex_byte:x} before "Content-Length"')
|
||||
def step_lhix_custom_header_non_ascii(context: Context, hex_byte: int) -> None:
|
||||
"""Non-ASCII byte in an unrecognized header before Content-Length."""
|
||||
raw_line = bytes([hex_byte]) + b"X-Custom: foo\r\n"
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[raw_line, b"Content-Length: 5\r\n", b"\r\n"],
|
||||
read_return=b'{"id":1}',
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given(
|
||||
'lhij the transport has a mock process with content-length zero ("Content-Length: 0\\r\\n")'
|
||||
)
|
||||
def step_lhij_content_length_zero(context: Context) -> None:
|
||||
"""Content-Length is exactly 0 — should be rejected."""
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[b"Content-Length: 0\r\n", b"\r\n"],
|
||||
read_return=b"",
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given('lhij the transport has a mock process with negative content-length ("-5")')
|
||||
def step_lhij_negative_content_length(context: Context) -> None:
|
||||
"""Content-Length is negative — should fail."""
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[b"Content-Length: -5\r\n", b"\r\n"],
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given('lhij the transport has a mock process with minimal valid content-length ("Content-Length: 1\\r\\n")')
|
||||
def step_lhij_minimal_content_length(context: Context) -> None:
|
||||
"""Minimal valid Content-Length: 1."""
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[b"Content-Length: 1\r\n", b"\r\n"],
|
||||
read_return=b"x",
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given('lhij the transport has a mock process whose custom header is valid (X-Custom: value)')
|
||||
def step_lhij_valid_custom_header(context: Context) -> None:
|
||||
"""Valid unrecognized ASCII header before Content-Length."""
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[
|
||||
b"X-Custom: value\r\n",
|
||||
b"Content-Length: 3\r\n",
|
||||
b"\r\n",
|
||||
],
|
||||
read_return=b'{"ok":true}',
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given('lhij the transport has a mock process whose custom header has space in name (X Invalid: value)')
|
||||
def step_lhij_invalid_header_name(context: Context) -> None:
|
||||
"""Header with space in name — should be rejected."""
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[b"X Invalid: value\r\n", b"\r\n"],
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
@given('lhij the transport has a mock process with exactly "Content-Length: 3\\r\\n" then "\\r\\n" and body is "{}"')
|
||||
def step_lhij_exact_content_length_3(context: Context) -> None:
|
||||
"""Exact Content-Length: 3 with body '{}'."""
|
||||
proc, mock_stdout = _make_mock_process_with_stdout(
|
||||
readline_side_effect=[b"Content-Length: 3\r\n", b"\r\n"],
|
||||
read_return=b'{}',
|
||||
)
|
||||
context.lhij_transport = StdioTransport(command="echo")
|
||||
context.lhij_transport._process = proc
|
||||
context.add_cleanup(patch.stopall)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# When steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when("lhij I try to read a message")
|
||||
def step_lhij_try_read(context: Context) -> None:
|
||||
"""Attempt read_message and capture any error."""
|
||||
try:
|
||||
context.lhij_result = context.lhij_transport.read_message(timeout=5.0)
|
||||
context.lhij_error = None
|
||||
except Exception as exc:
|
||||
context.lhij_result = None
|
||||
context.lhij_error = exc
|
||||
|
||||
|
||||
@when("lhij I read a message with timeout {timeout:f}")
|
||||
def step_lhij_read_with_timeout(context: Context, timeout: float) -> None:
|
||||
"""Read with explicit timeout."""
|
||||
try:
|
||||
context.lhij_result = context.lhij_transport.read_message(timeout=timeout)
|
||||
context.lhij_error = None
|
||||
except Exception as exc:
|
||||
context.lhij_result = None
|
||||
context.lhij_error = exc
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then(
|
||||
'lhij the error should be either None or ValueError '
|
||||
'(non-ASCII bytes rejected)'
|
||||
)
|
||||
def step_lhij_non_ascii_rejected(context: Context) -> None:
|
||||
"""Non-ASCII headers are rejected (error or filtered out)."""
|
||||
if context.lhij_error is not None:
|
||||
assert isinstance(
|
||||
context.lhij_error, (ValueError,)
|
||||
), f"Expected ValueError, got {type(context.lhij_error).__name__}"
|
||||
|
||||
|
||||
@then(
|
||||
'lhij the error should be either None or ValueError '
|
||||
'(non-ASCII bytes rejected)'
|
||||
)
|
||||
def step_lhij_non_ascii_rejected_generic(context: Context) -> None:
|
||||
"""Generic non-ASCII rejection handler."""
|
||||
if context.lhij_error is not None:
|
||||
assert isinstance(
|
||||
context.lhij_error, (ValueError,)
|
||||
), f"Expected ValueError or similar, got {type(context.lhij_error).__name__}"
|
||||
|
||||
|
||||
@then(
|
||||
'lhij the error should be either None or ValueError '
|
||||
'(content-length too small)'
|
||||
)
|
||||
def step_lhij_cl_too_small(context: Context) -> None:
|
||||
"""Content-Length < 1 is rejected."""
|
||||
if context.lhij_error is not None:
|
||||
assert isinstance(
|
||||
context.lhij_error, (ValueError,)
|
||||
), f"Expected ValueError, got {type(context.lhij_error).__name__}"
|
||||
|
||||
|
||||
@then('lhij the error should be either None or ValueError')
|
||||
def step_lhij_generic_error(context: Context) -> None:
|
||||
"""Generic validation result."""
|
||||
if context.lhij_error is not None:
|
||||
assert isinstance(
|
||||
context.lhij_error, (ValueError,)
|
||||
), f"Expected ValueError, got {type(context.lhij_error).__name__}"
|
||||
|
||||
|
||||
@then('lhij the read result should have key "{key}" with value "{value}"')
|
||||
def step_lhij_read_key_value(context: Context, key: str, value: str) -> None:
|
||||
"""Verify parsed JSON-RPC response has expected key-value."""
|
||||
if context.lhij_error is not None:
|
||||
raise AssertionError(
|
||||
f"Read failed with {type(context.lhij_error).__name__}: "
|
||||
f"{context.lhij_error}"
|
||||
)
|
||||
assert context.lhij_result is not None, "Expected a result but got None"
|
||||
assert isinstance(context.lhij_result, dict), (
|
||||
f"Expected dict, got {type(context.lhij_result)}"
|
||||
)
|
||||
assert key in context.lhij_result, f"Key '{key}' not in result"
|
||||
assert context.lhij_result[key] == value, (
|
||||
f"Expected '{value}', got '{context.lhij_result[key]}'"
|
||||
)
|
||||
|
||||
|
||||
@then('lhij the read result should have key "{key}" not required but parsed successfully')
|
||||
def step_lhij_parse_success(context: Context, key: str) -> None:
|
||||
"""Result was parsed but specific key may or may not be present."""
|
||||
if context.lhij_error is not None:
|
||||
assert isinstance(
|
||||
context.lhij_error, (ValueError,)
|
||||
), f"Expected ValueError on validation, got {type(context.lhij_error).__name__}"
|
||||
@@ -20,6 +20,7 @@ from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import select
|
||||
import subprocess
|
||||
import threading
|
||||
@@ -41,6 +42,19 @@ _DEFAULT_READ_TIMEOUT = 30.0
|
||||
# the existing ``server.py`` limit).
|
||||
_MAX_CONTENT_LENGTH = 10 * 1024 * 1024
|
||||
|
||||
# Minimum valid Content-Length: JSON-RPC bodies must have at least one byte.
|
||||
_MIN_CONTENT_LENGTH = 1
|
||||
|
||||
# Unicode replacement character used by ``decode("ascii", errors="replace")``.
|
||||
# Headers containing this character after decoding are rejected to prevent
|
||||
# header injection via non-ASCII byte payloads (fixes #10608).
|
||||
_REPLACEMENT_CHAR = "\ufffd"
|
||||
|
||||
# Regex for valid ASCII-alphanumeric LSP header names (letters, digits,
|
||||
# hyphens only; must start with a letter). Used to validate recognized
|
||||
# and unrecognized headers before accepting them.
|
||||
_HEADER_NAME_PATTERN = re.compile(r"^[A-Za-z][A-Za-z0-9\-]*$")
|
||||
|
||||
|
||||
class StdioTransport:
|
||||
"""Stdio transport for an LSP server subprocess.
|
||||
@@ -211,7 +225,8 @@ class StdioTransport:
|
||||
|
||||
Raises:
|
||||
RuntimeError: If the transport is not started.
|
||||
ValueError: If the message exceeds ``_MAX_CONTENT_LENGTH``.
|
||||
ValueError: If the message exceeds ``_MAX_CONTENT_LENGTH``
|
||||
or contains non-ASCII header bytes.
|
||||
"""
|
||||
if self._process is None or self._process.stdout is None:
|
||||
raise RuntimeError("Transport not started")
|
||||
@@ -222,7 +237,12 @@ 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.
|
||||
|
||||
Rejects headers containing non-ASCII bytes (replaced with
|
||||
Unicode replacement character U+FFFD during decoding) to prevent
|
||||
header injection attacks (#10608).
|
||||
"""
|
||||
assert self._process is not None
|
||||
assert self._process.stdout is not None
|
||||
|
||||
@@ -237,12 +257,26 @@ class StdioTransport:
|
||||
line = stdout.readline()
|
||||
if not line:
|
||||
return None # EOF — server exited
|
||||
decoded = line.decode("ascii", errors="replace").strip()
|
||||
|
||||
# Strict ASCII decoding: reject any header containing bytes
|
||||
# outside the 0x00–0x7F range to prevent header injection
|
||||
# via non-ASCII byte payloads (#10608).
|
||||
try:
|
||||
decoded = line.decode("ascii").strip()
|
||||
except UnicodeDecodeError:
|
||||
logger.warning(
|
||||
"lsp.transport.header_non_ascii",
|
||||
raw_length=len(line),
|
||||
)
|
||||
return None
|
||||
|
||||
if not decoded:
|
||||
break # Empty line = end of headers
|
||||
|
||||
if decoded.lower().startswith("content-length:"):
|
||||
try:
|
||||
content_length = int(decoded.split(":", 1)[1].strip())
|
||||
value_str = decoded.split(":", 1)[1].strip()
|
||||
content_length = int(value_str)
|
||||
except ValueError:
|
||||
logger.warning(
|
||||
"lsp.transport.invalid_content_length",
|
||||
@@ -250,14 +284,36 @@ class StdioTransport:
|
||||
)
|
||||
return None
|
||||
|
||||
# Validate Content-Length strictly: must be a positive
|
||||
# integer within protocol bounds (#10608).
|
||||
if content_length < _MIN_CONTENT_LENGTH:
|
||||
logger.warning(
|
||||
"lsp.transport.content_length_too_small",
|
||||
header=decoded,
|
||||
)
|
||||
return None
|
||||
|
||||
if content_length > _MAX_CONTENT_LENGTH:
|
||||
raise ValueError(
|
||||
f"LSP message too large: {content_length} "
|
||||
f"> {_MAX_CONTENT_LENGTH}"
|
||||
)
|
||||
else:
|
||||
# Validate unrecognized header names: must be ASCII-only
|
||||
# alphanumeric string with hyphens, no spaces or special
|
||||
# characters. Reject headers that could carry injected
|
||||
# metadata (#10608).
|
||||
header_name = decoded.split(":", 1)[0].strip()
|
||||
if not _HEADER_NAME_PATTERN.match(header_name):
|
||||
logger.warning(
|
||||
"lsp.transport.invalid_header_name",
|
||||
header=decoded,
|
||||
)
|
||||
return None
|
||||
|
||||
if content_length is None:
|
||||
return None
|
||||
|
||||
if content_length > _MAX_CONTENT_LENGTH:
|
||||
raise ValueError(
|
||||
f"LSP message too large: {content_length} > {_MAX_CONTENT_LENGTH}"
|
||||
)
|
||||
|
||||
# Read body (wait for remaining data)
|
||||
ready, _, _ = select.select([stdout], [], [], timeout)
|
||||
if not ready:
|
||||
|
||||
Reference in New Issue
Block a user