fix(lsp): add per-message read timeout to prevent DoS in _read_message() #10650
No reviewers
Labels
No labels
auto/needs-reevaluation
controller-managed
overdue
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!10650
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "bugfix/m3.6.0-lsp-server-dos-message-read-timeout"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
This PR fixes a Denial of Service (DoS) vulnerability in the LSP server's
_read_message()method. Previously, the method would block indefinitely when reading the message body if a malicious client sent a validContent-Lengthheader but then stalled without delivering the actual message body. The fix introduces a configurable per-message read timeout usingselect()to enforce time limits on socket reads, while maintaining backward compatibility with in-memory streams used in tests.Changes
MESSAGE_READ_TIMEOUTconstant (30 seconds default) to prevent indefinite blocking on stalled clients_read_body_with_timeout()method usingselect()for timeout enforcement on streams with real file descriptorsread_timeoutconstructor parameter for configurability of the timeout duration# SEC:) that previously documented the vulnerabilityio.BytesIO) that don't expose real file descriptors, preserving full test compatibilityTesting
MESSAGE_READ_TIMEOUTis accessibleIssue Reference
Closes #7083
Automated by CleverAgents Bot
Agent: pr-description-writer
Implementation Attempt — Tier 1: haiku — Success
Fixed the CI lint failure caused by incorrect formatting in
features/steps/lsp_server_stub_steps.py.The original PR introduced extra blank lines (3 consecutive blank lines instead of the allowed 2) and other minor formatting issues in the new DoS protection step definitions. The
ruff formatauto-formatter was applied to fix:@whendecoratorstep_process_with_read_timeoutand the next@whenfromimports (PEP 8 style)step_message_read_timeout_valueAll quality gates verified locally:
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: implementation-worker
This PR cannot be approved because CI checks are failing. The following critical gates must pass before review can proceed:
Per company policy, all CI gates (lint, typecheck, security, unit_tests, coverage) must pass before a PR can be approved. Please fix these failures and push new commits.
Specifically:
# type: ignoreadditions)Once CI is green, a full code review will be conducted against the 10-category checklist.
This PR cannot be approved because CI checks are failing. The following critical gates must pass before review can proceed:
Per company policy, all CI gates (lint, typecheck, security, unit_tests, coverage) must pass before a PR can be approved. Please fix these failures and push new commits.
Specifically:
# type: ignoreadditions)Once CI is green, a full code review will be conducted against the 10-category checklist.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
🌱 Grooming: proceed — PR cleared for processing.
(check
no_duplicates, categoryno_duplicates)PR #10650 fixes a specific DoS vulnerability in the LSP server's _read_message() method by adding a per-message read timeout using select(). While multiple open LSP PRs address other security issues (subprocess cleanup on initialization failure, header injection in ASCII decoding, path traversal in file reading), none of them target the indefinite blocking vulnerability in the message body reading logic. This is a distinct, focused security fix with no topical overlap or duplicate among the open PRs.
📋 Estimate: tier 1.
3-file change (+162/-9) adding a DoS fix to the LSP server: new
select()-based timeout logic, a fallback path for in-memory streams, a new constructor parameter, and BDD test coverage. Multi-file scope with new logic branches (select fd check, fallback, configurable timeout) puts this firmly above tier 0. The typecheck and security CI failures are Docker pull-rate-limit infrastructure noise, not code defects. The unit_tests gate shows 2 genuine scenario failures out of 15241, indicating real implementation work remains. Task is well-defined and bounded — no architectural sprawl — so tier 2 is not warranted.36bcb04bdee2c658918b(attempt #3, tier 1)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
e2c6589.e2c658918b7a0baf30fd(attempt #4, tier 1)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
7a0baf3.7a0baf30fd32b728f7a3(attempt #5, tier 1)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
32b728f.(attempt #6, tier 1)
🔧 Implementer attempt —
blocked.Blockers:
737dbebb73but dispatch base was32b728f7a3. The implementer pushed from inside the worktree (forbidden by the git contract) OR a third party pushed during the attempt. Re-dispatch will re-prefetch and pick up the new head.737dbebb73c03498d71a(attempt #7, tier 2)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
c03498d.🌱 Grooming: proceed — PR cleared for processing.
(check
no_duplicates, categoryno_duplicates)PR #10650 adds a configurable per-message read timeout to LSP's _read_message() method using select() to prevent indefinite blocking on stalled clients. While six other LSP security fixes are open (#10597, #10608, #10632, #10644, #10625, etc.), each addresses a distinct vulnerability (subprocess cleanup, header injection, directory traversal, path traversal, env injection). None modifies message-reading loop timeout handling or closes #7083. No topical overlap or duplicated scope detected.
📋 Estimate: tier 1.
3-file security fix adding select()-based per-message read timeout to LSP _read_message(). New logic includes _read_body_with_timeout() with fd detection, fallback for in-memory streams (BytesIO), and a new constructor parameter. New BDD test coverage for timeout enforcement and fallback behavior. Socket/file-descriptor semantics and backward-compatibility reasoning required. CI failures are all infrastructure network errors (forgejo-http unreachable), not code failures — integration_tests logs show "OK" before artifact upload failure. Solidly tier 1: cross-file, new logic branches, new tests.
(attempt #10, tier 1)
🔧 Implementer attempt —
blocked.Blockers:
fab8c64ba5but dispatch base wasc03498d71a. The implementer pushed from inside the worktree (forbidden by the git contract) OR a third party pushed during the attempt. Re-dispatch will re-prefetch and pick up the new head.fab8c64ba5c3de4921bb(attempt #11, tier 2)
🔧 Implementer attempt —
rebased.Pushed 1 commit:
c3de492.(attempt #12, tier 2)
🔧 Implementer attempt —
resolved.Pushed 1 commit:
22c3cdd.Files touched:
features/lsp_server_stub.feature,features/steps/lsp_server_stub_steps.py.✅ Approved
Reviewed at commit
22c3cdd.Confidence: high.
Claimed by
merge_drive.py(pid 1627962) until2026-06-05T23:38:24.119281+00:00.This claim is advisory and will be released when the cycle ends, or after the TTL by a sibling driver's expired-claim sweep.
Approved by the controller reviewer stage (workflow 280).