Repository navigation
fix: address audit correctness and resource bounds - #4231
Conversation
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…build - Re-externalized privacy remediation now retains sanitized provider metadata beneath the new payload envelope instead of discarding it. - Compute the resume_gap frontier from the validated integer so the TypeScript SDK declaration build type-checks. - Cover decorated local shadowing in the Python call-binding test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ss-and-resource-bounds
The pending bound equalled the per-connection execution cap, so a client pipelining a ninth request got an internal error instead of queueing for a permit. Stop reading at 64 pending requests instead; frames past the bound wait in the socket, so retained payloads stay bounded.
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| /// execution permit. At the bound the session stops reading, so further | ||
| /// frames wait in the peer's socket instead of being refused or retained; | ||
| /// a cancellation sent at the bound is read once one request settles. | ||
| pub const MAX_PENDING_REQUESTS: usize = 64; |
There was a problem hiding this comment.
🔴 Pending requests exhaust session memory
Pipelining 64 large MCP requests lets MAX_PENDING_REQUESTS retain roughly a gigabyte of frames per session. The production execution limit is eight, so 56 decoded requests can wait in memory without running.
Learn more
Each received request becomes a future held by the session until it finishes or its response is written. MCP framing accepts frames up to 16 MiB, while the production execution limit permits only eight active requests per client. Raising the retained-future ceiling to 64 lets a single connection keep 56 large decoded payloads waiting for those eight permits. The per-connection worst-case input alone approaches 1 GiB, before decoded JSON and handler allocations.
Example: A client pipelines 64 valid tools/call frames with nearly 16 MiB of arguments each while the first eight calls are busy. The server retains all 64 requests rather than bounding memory near eight frames, so concurrent clients can exhaust daemon memory.
Recommended fix: Decouple queueing from retaining decoded payloads. Bound the bytes and count of admitted pending requests against available memory, or leave excess frames in the socket until execution capacity exists while preserving cancellation processing. Measure the retained payload cost before selecting a higher bound.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Whole-line symbol edits could remove neighboring syntax, cancellation could delete payloads after an admitted SQL commit, and privacy rescans missed canonical inline bodies and external-row metadata. This change refuses unsafe line splices, makes the SQL writer own payload completion, and remediates metadata independently of body availability.
It also matches complete insertion anchors, respects Python bindings when resolving imported calls, fixes TypeScript multi-sequence gap continuity, bypasses proxies for local Rust SDK connections, bounds job output and MCP/HTTP admission, and confines file deliveries through no-follow directory handles. Eval grading now distinguishes forbidden attempts, completed abstention and missing agent runs; repetition denominators and per-child benchmark memory measurements are corrected. The security policy and optional mockup data loading reflect current behavior.
Validation:
bazel test //evals/hermetic:score_test //evals/hermetic:scorecard_test --test_output=errorspassed both targets after correcting an initial test import-path failure.git diff --checkand conventional commit lint passed.bazel test //sdks/typescript:unit_test --test_output=errorspassed. The new target brings the SDK suite intobazel test //....This is a broad fix set; CI and review remain necessary before merge. Existing staged mockup dependency changes are excluded. No feature removal, generated contract edits, credentials or operator profile exports are included.