fix: bump MCP SDK/sample dependencies and harden log sanitization for control-character injection - #1382
fix: bump MCP SDK/sample dependencies and harden log sanitization for control-character injection#1382YasiruDEX wants to merge 3 commits into
Conversation
… bandit security exception for gateway placeholder
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesDependency updates
Groq gateway configuration
Log sanitization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
samples/hotel-booking-agent/agent/app.py (1)
54-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for the sanitizer.
Cover CR/LF escaping, removal of C0 controls/DEL/ESC, and preservation of printable content. Based on the PR objective, no automated tests currently cover this updated sample-agent behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@samples/hotel-booking-agent/agent/app.py` around lines 54 - 67, Add regression tests for _sanitize_for_log covering CR and LF escaping, removal of C0 control characters, DEL, and ESC, while asserting printable content remains unchanged. Place the tests with the existing sample-agent test suite and preserve the sanitizer’s current behavior.samples/crewai-agent/requirements.txt (1)
1-2: 🔒 Security & Privacy | 🔵 TrivialAudit the resolved dependency tree before merging.
crewai==1.15.5pulls inchromadbdirectly; check whether the resolvedchromadbrelease is in the affected range.Pillowandpython-multipartare not direct dependencies of these pins.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@samples/crewai-agent/requirements.txt` around lines 1 - 2, Audit the resolved dependency tree for the pinned crewai and crewai-tools requirements, specifically determining which chromadb version is installed and whether it falls within the affected range. Update the dependency pins or related configuration only if needed to avoid the affected chromadb release, and do not treat Pillow or python-multipart as direct dependencies.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@samples/crewai-agent/requirements.txt`:
- Around line 1-2: Audit the resolved dependency tree for the pinned crewai and
crewai-tools requirements, specifically determining which chromadb version is
installed and whether it falls within the affected range. Update the dependency
pins or related configuration only if needed to avoid the affected chromadb
release, and do not treat Pillow or python-multipart as direct dependencies.
In `@samples/hotel-booking-agent/agent/app.py`:
- Around line 54-67: Add regression tests for _sanitize_for_log covering CR and
LF escaping, removal of C0 control characters, DEL, and ESC, while asserting
printable content remains unchanged. Place the tests with the existing
sample-agent test suite and preserve the sanitizer’s current behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ddedfefc-8822-4844-b455-533dc95d9202
⛔ Files ignored due to path filters (2)
agent-manager-observer/go.sumis excluded by!**/*.sumagent-manager-service/go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
agent-manager-observer/go.modagent-manager-service/go.modlibs/amp-evaluation/src/amp_evaluation/evaluators/base.pysamples/crewai-agent/requirements.txtsamples/hotel-booking-agent/agent/app.pysamples/hotel-booking-agent/agent/requirements.txt
Dependency Validation ResultsDependency name: github.com/modelcontextprotocol/go-sdk Dependency name: github.com/modelcontextprotocol/go-sdk |
Dependency Validation ResultsDependency name: github.com/modelcontextprotocol/go-sdk Dependency name: github.com/modelcontextprotocol/go-sdk |
1 participant
Purpose
Continues the security-remediation effort on this branch. Two issues addressed:
agent-manager-observerandagent-manager-servicewere pinned to outdated versions ofmodelcontextprotocol/go-sdkandgoogle/jsonschema-go, and thecrewai/langgraphsample agents were pinned to older releases — all had newer patched versions available._sanitize_for_login the hotel-booking-agent sample only escaped\r/\n, leaving other ASCII control characters (including ESC, enabling terminal escape-sequence injection) unsanitized when untrusted user input is logged.Resolves: N/A (no linked issue — add one if this maps to a tracked ticket)
Goals
jsonschema-godependency up to the latest patched release on both Go services.crewai/crewai-toolsandlanggraphsample-agent pins to current releases (note:langgraphmoves 0.5.0 → 1.2.9, a major version jump)._sanitize_for_logto strip all non-printable control characters, not just CR/LF, closing a log/terminal injection gap.nosecsuppression for the"api_key": "gateway"placeholder inamp-evaluation's Groq client construction, which bandit's B105 (hardcoded password) rule flags as a false positive — the real credential is injected separately viadefault_headers, never throughapi_key.Approach
go.mod/go.sumupdated viago get -uforgithub.com/modelcontextprotocol/go-sdk(1.5.0 → 1.6.1) andgithub.com/google/jsonschema-go(0.4.2 → 0.4.3) in bothagent-manager-observerandagent-manager-service.requirements.txtpins bumped forcrewai/crewai-tools(1.15.2 → 1.15.5) andlanggraph(0.5.0 → 1.2.9) in the sample agents._OTHER_CONTROL_CHARS_RE) matching the C0 control-character range plus DEL, applied after the existing CR/LF escaping in_sanitize_for_log(samples/hotel-booking-agent/agent/app.py).# nosec B105comment with rationale directly on thereturninlibs/amp-evaluation/src/amp_evaluation/evaluators/base.pywhere the gateway placeholder api_key is constructed.No UI changes — screenshot/GIF not applicable.
User stories
N/A — internal security hardening, not user-facing functionality.
Release note
Updated MCP SDK and related dependencies to their latest patched versions; hardened log sanitization to strip control characters from user-supplied input before logging.
Documentation
N/A — no user-facing behavior changed; dependency bumps and internal log-sanitization hardening only.
Training
N/A — no training content impact.
Certification
N/A — no certification-exam impact; internal dependency/security hardening only.
Marketing
N/A — no marketing impact.
Automation tests
Security checks
Samples
samples/crewai-agentandsamples/hotel-booking-agent/agenthad their dependency pins bumped;hotel-booking-agent/agent/app.py's log sanitization was hardened.Related PRs
N/A
Migrations (if applicable)
N/A — dependency version bumps only, no schema or data migration.
Test environment
Local K3d Setup
Learning
The B105 bandit rule (hardcoded password) pattern-matches on string literals passed to
api_key-shaped fields regardless of whether the value is a real secret — same class of false positive as SAST tools flaggingreq.SetBasicAuth(...)calls by call-site pattern rather than true taint analysis. Suppressing with a justified inline comment (rather than restructuring working code) is the appropriate fix when the underlying design is intentional and documented.Summary by CodeRabbit