Skip to content

Limit LSP request id echo size - #3298

Merged
Widthdom merged 9 commits into
mainfrom
fix-issue3113
Jun 6, 2026
Merged

Limit LSP request id echo size#3298
Widthdom merged 9 commits into
mainfrom
fix-issue3113

Conversation

@Widthdom

@Widthdom Widthdom commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds a 4 KiB raw JSON byte limit for LSP JSON-RPC request ids.
  • Preflights the top-level id token with Utf8JsonReader before parsing retained response id state, so oversized ids are rejected with id: null instead of being echoed.
  • Adds regression coverage for oversized LSP ids, including a raw UTF-8 byte-boundary case where character count alone would be insufficient.

Validation

  • dotnet test tests/CodeIndex.Tests/CodeIndex.Tests.csproj --no-restore --filter "FullyQualifiedName~LspServerTests"
  • dotnet run --no-restore --project tools/CodeIndex.Changelog -- check
  • dotnet format CodeIndex.sln --verify-no-changes
  • dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll status --check
  • GitHub Actions at 5928ca5: Changelog Fragments, CodeQL, and Build and Test all passed. Build and Test passed across ubuntu/macos/windows for net8.0/net9.0.
  • Codex adversarial review: no actionable findings. The codex exec review --base origin/main subcommand was attempted with and without --ignore-rules, but this environment rejected its internal shell calls; a tool-free Codex review of the final change summary completed and found no actionable findings.

A full dotnet test run was also attempted after a moving origin/main refresh; it reported an unrelated McpServerTests.ProcessLineAsync_CapsTelemetryArgumentKeyCount_Issue3237 failure and then stalled. The exact failing test passed when rerun directly.

A local rerun of the new LspServerRequestIdTests regression test was attempted after moving to latest main, but the tool call timed out before returning a result. The updated PR head passed the GitHub Actions matrix listed above.

The earlier CI-only failure in McpAuditLogTests.ToolsCall_TruncatesAuditRequestId_Issue3237 is tracked by #3308 and is already resolved on current main; this PR no longer carries an additional #3308 diff.

Documentation and changelog

  • Added changelog.d/unreleased/3113.security.md.
  • No README or guide updates were required for this internal LSP safety bound.

Follow-up candidates

  • None.

Fixes #3113
Refs #3308

@Widthdom
Widthdom marked this pull request as ready for review June 5, 2026 11:10
@Widthdom
Widthdom merged commit d2fe387 into main Jun 6, 2026
10 checks passed
@Widthdom
Widthdom deleted the fix-issue3113 branch June 6, 2026 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LSP JSON-RPC request ids should cap raw size before parsing and echoing

1 participant