Skip to content

🔒 Warden: Update vulnerable crates and cap unbounded reader#616

Open
madmax983 wants to merge 1 commit into
trunkfrom
warden-fix-vulns-17265787013832725573
Open

🔒 Warden: Update vulnerable crates and cap unbounded reader#616
madmax983 wants to merge 1 commit into
trunkfrom
warden-fix-vulns-17265787013832725573

Conversation

@madmax983
Copy link
Copy Markdown
Owner

  • 🦠 Threat: Outdated crates serde_yml and libyml have unsound memory vulnerabilities. lru@0.12.5 violates Stacked Borrows. paste is unmaintained. Unbounded network reader opened vector for memory exhaustion DoS.
  • 🛡️ Defense: Removed serde_yml and replaced it with serde_yaml_ng. Updated litellm-rs to 0.5.0 and ratatui to 0.30 to patch their vulnerable transient dependencies. Capped socket reader with .take(1024 * 1024).
  • 💥 Severity: Critical - unsound dependencies, stacked borrow violations, and unbounded readers can crash the application or lead to unexpected behaviors.
  • 🧪 Verification: Ran cargo audit with no warnings remaining. Ran cargo test suite successfully.

PR created automatically by Jules for task 17265787013832725573 started by @madmax983

- Switched from `serde_yml` to `serde_yaml_ng` to resolve memory vulnerabilities.
- Updated `litellm-rs` and `ratatui` to remove vulnerable/unmaintained dependencies like `libyml` and `paste`, and resolve `lru` stacked borrow violation.
- Capped unbounded `tokio` socket reader with `.take()` to prevent memory exhaustion DoS.
- Fixed pedantic Clippy warnings.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates several dependencies, including upgrading ratatui to 0.30, litellm-rs to 0.5.0, and replacing serde_yml with serde_yaml_ng. It also introduces a 1MB read limit on the socket in src/run/swebench.rs and refactors test timeout matches and helper functions in src/stream/sweep_webhook.rs. The review feedback suggests optimizing the socket reading logic in src/run/swebench.rs by calling .take() directly on the socket instead of wrapping it in a redundant BufReader, which avoids unnecessary memory allocations and simplifies the subsequent .into_inner() calls.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/run/swebench.rs
Comment on lines +8027 to +8028
let reader = tokio::io::BufReader::new(socket);
let mut limited_reader = reader.take(1024 * 1024); // Cap to 1MB max reader length
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using BufReader here is redundant and inefficient because we are already reading in chunks of 8192 bytes. It also introduces unnecessary memory allocation and extra copying overhead on every connection, only to be discarded immediately after the loop. We can call .take() directly on the socket instead, which simplifies the code and avoids the double .into_inner() calls later.

                let mut limited_reader = socket.take(1024 * 1024); // Cap to 1MB max reader length

Comment thread src/run/swebench.rs
Comment on lines +8050 to +8052
let _ = limited_reader
.into_inner()
.into_inner()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since we can call .take() directly on the socket without wrapping it in a BufReader, we only need a single .into_inner() call to retrieve the underlying TcpStream for writing the response.

                let _ = limited_reader
                    .into_inner()

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 6, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.26%. Comparing base (472ab09) to head (de26e2f).

Files with missing lines Patch % Lines
src/stream/sweep_webhook.rs 33.33% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            trunk     #616   +/-   ##
=======================================
  Coverage   85.26%   85.26%           
=======================================
  Files         115      115           
  Lines       65856    65858    +2     
=======================================
+ Hits        56150    56152    +2     
  Misses       9706     9706           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant