fix: clarify advisory output string concatenation (PR #5689 follow-up)#5691
fix: clarify advisory output string concatenation (PR #5689 follow-up)#5691marcusquinn merged 2 commits intomainfrom
Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughThis pull request updates the DSPy dependency from version 2.6.27 to 3.1.3 and removes the legacy Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly strengthens the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
🤖 Augment PR SummarySummary: This PR expands aidevops security UX by adding a secret-hygiene scanner, a lightweight supply-chain IoC check, and a persistent advisory system that surfaces active advisories in the session greeting until dismissed. Changes:
Technical Notes: Advisories are stored under 🤖 Was this summary useful? React with 👍 or 👎 |
| first_line=$(head -1 "$advisory_file" | sed 's/^[[:space:]]*//') | ||
| if [[ -n "$first_line" ]]; then | ||
| local entry | ||
| entry=$(printf '%s Run in your terminal: secret-hygiene-helper.sh scan | Dismiss: secret-hygiene-helper.sh dismiss %s' "$first_line" "$adv_id") |
There was a problem hiding this comment.
The greeting text suggests running secret-hygiene-helper.sh ..., but helpers are typically invoked via aidevops/_dispatch_helper and may not be on PATH, so this can lead users to a “command not found” dead-end. Consider emitting the stable entrypoints (e.g., aidevops security scan / aidevops security dismiss <id>) consistently in advisories and startup warnings.
Severity: medium
Other Locations
.agents/advisories/litellm-2026-03.advisory:15.agents/advisories/litellm-2026-03.advisory:21.agents/scripts/secret-hygiene-helper.sh:481.agents/scripts/secret-hygiene-helper.sh:505.agents/scripts/secret-hygiene-helper.sh:509
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| local found_suspicious=0 | ||
|
|
||
| # Find all Python site-packages directories | ||
| for py in /opt/homebrew/opt/python@*/bin/python3.* /usr/local/bin/python3.*; do |
There was a problem hiding this comment.
The .pth audit only iterates Homebrew and /usr/local Python installs; common system installs like /usr/bin/python3* (and other distributions) won’t be scanned, which could miss IoCs on many Linux hosts. Since this is a security detector, gaps in interpreter discovery can materially reduce coverage.
Severity: high
Other Locations
.agents/scripts/secret-hygiene-helper.sh:475
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| for py in /opt/homebrew/opt/python@*/bin/python3.* /usr/local/bin/python3.*; do | ||
| [[ -x "$py" ]] || continue | ||
| local sp | ||
| sp=$("$py" -c "import site; print(site.getsitepackages()[0])" 2>/dev/null) || continue |
There was a problem hiding this comment.
Using site.getsitepackages()[0] scans only the first site-packages directory and doesn’t cover user site-packages (site.getusersitepackages()), so .pth IoCs from pip install --user (or multi-path installs) can be missed. This is especially relevant given the advisory notes that .pth executes on every Python startup.
Severity: high
Other Locations
.agents/scripts/secret-hygiene-helper.sh:478
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| local perms | ||
| perms=$(get_perms "$aws_creds") | ||
| local key_count | ||
| key_count=$(grep -c "aws_access_key_id\|aws_secret_access_key\|aws_session_token" "$aws_creds" 2>/dev/null) || key_count=0 |
There was a problem hiding this comment.
These grep counts rely on the \| alternation escape in basic regex mode; on some grep variants this isn’t supported (or behaves differently), which can silently produce incorrect counts in the reported finding text. If the counts are meant to be reliable cross-platform, this pattern is a portability risk.
Severity: low
Other Locations
.agents/scripts/secret-hygiene-helper.sh:181.agents/scripts/secret-hygiene-helper.sh:207.agents/scripts/secret-hygiene-helper.sh:229
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.agents/scripts/secret-hygiene-helper.sh (2)
303-342: Consider broader Python installation discovery.The current Python path iteration covers Homebrew and
/usr/localinstallations, which handles most macOS setups. However, this may miss:
- pyenv installations (
~/.pyenv/versions/*/)- System Python on Linux (
/usr/lib/python*/)- User-installed Python (
~/.local/lib/python*/)For a "Chill" review, this is acceptable as-is since the most common paths are covered and the script is primarily macOS-focused. Consider expanding coverage in a future iteration if Linux support becomes a priority.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/secret-hygiene-helper.sh around lines 303 - 342, The current for-loop that iterates candidate Python executables (the for py in /opt/homebrew/... /usr/local/bin/python3.* loop) misses pyenv, system Linux and user-local installs; update the discovery to also include pyenv-managed interpreters (expand candidates to ~/.pyenv/versions/*/bin/python3*), system-wide Linux paths (e.g., /usr/lib/python*/bin/python3*), and user-local installs (e.g., ~/.local/bin/python3* or ~/.local/lib/python*/*/bin/python3*), and/or supplement with a fallback like enumerating `which -a python3` results; keep the rest of the logic unchanged (sp variable from "$py" -c "import site...", basename_pth checks, and grep/report_finding behavior).
393-399: Version spec regex may have edge cases.The check
grep -qE '>=' && ! grep -qE '=='could produce false positives for specs likepackage>=1.0,<2.0(which is actually bounded) or miss patterns likepackage~=1.0(compatible release). For the intended purpose of flagging supply-chain risk, this conservative approach is reasonable — flagging more is safer than missing potential issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/secret-hygiene-helper.sh around lines 393 - 399, The current check inside the while loop that increments unpinned uses grep -qE '>=' && ! grep -qE '==' which misclassifies bounded specs like "package>=1.0,<2.0" and misses other operators like "~="; update the conditional in the loop that touches unpinned to more robustly detect unbounded/loose specs by: consider a line unpinned if it contains any of the loose operators ('>=', '>', '~=') AND the line does NOT contain any upper-bound indicators ('<' or '<=' or a comma followed by '<'); apply this new logic where unpinned is incremented (referencing the while IFS= read -r line; do ... done <"$req_file" loop and the unpinned variable) so bounded combos like ">=... , <..." are not flagged while "~=" and pure >=/> specs without an upper bound are flagged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@requirements.txt`:
- Line 11: Update the dspy pin from dspy==2.6.27 to at least 3.0.4 (preferably
the latest 3.1.3) in requirements.txt to remediate CVE-2025-12695; after
updating, run your dependency install and tests to ensure no breakage. For
diskcache (diskcache==5.6.3) add a note to the security backlog documenting
CVE-2025-69872 and, until a patched release exists, implement operational
mitigations: restrict write access to the cache directory, avoid loading
untrusted pickles, and add CI/monitoring to flag any future diskcache upgrades.
Ensure both changes are recorded in the PR and changelog.
---
Nitpick comments:
In @.agents/scripts/secret-hygiene-helper.sh:
- Around line 303-342: The current for-loop that iterates candidate Python
executables (the for py in /opt/homebrew/... /usr/local/bin/python3.* loop)
misses pyenv, system Linux and user-local installs; update the discovery to also
include pyenv-managed interpreters (expand candidates to
~/.pyenv/versions/*/bin/python3*), system-wide Linux paths (e.g.,
/usr/lib/python*/bin/python3*), and user-local installs (e.g.,
~/.local/bin/python3* or ~/.local/lib/python*/*/bin/python3*), and/or supplement
with a fallback like enumerating `which -a python3` results; keep the rest of
the logic unchanged (sp variable from "$py" -c "import site...", basename_pth
checks, and grep/report_finding behavior).
- Around line 393-399: The current check inside the while loop that increments
unpinned uses grep -qE '>=' && ! grep -qE '==' which misclassifies bounded specs
like "package>=1.0,<2.0" and misses other operators like "~="; update the
conditional in the loop that touches unpinned to more robustly detect
unbounded/loose specs by: consider a line unpinned if it contains any of the
loose operators ('>=', '>', '~=') AND the line does NOT contain any upper-bound
indicators ('<' or '<=' or a comma followed by '<'); apply this new logic where
unpinned is incremented (referencing the while IFS= read -r line; do ... done
<"$req_file" loop and the unpinned variable) so bounded combos like ">=... ,
<..." are not flagged while "~=" and pure >=/> specs without an upper bound are
flagged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 377cba40-3402-4851-8abd-894638b28b69
📒 Files selected for processing (10)
.agents/AGENTS.md.agents/advisories/litellm-2026-03.advisory.agents/scripts/aidevops-update-check.sh.agents/scripts/commands/security-scan.md.agents/scripts/secret-hygiene-helper.sh.gitignoreREADME.mdaidevops.shrequirements.txtsetup-modules/agent-deploy.sh
|
Dispatching worker to address CodeRabbit review feedback.
|
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the security posture of the aidevops framework by introducing a new secret-hygiene-helper.sh script for scanning plaintext secrets, Python .pth files (supply chain attack indicators), unpinned dependencies, and MCP server configurations. It unifies all security checks under a single aidevops security command, implements a system for delivering and managing security advisories (including a specific advisory for the LiteLLM supply chain incident), and enforces strict dependency pinning to exact versions (==) in requirements.txt to prevent supply chain attacks. Documentation across AGENTS.md, README.md, and security-scan.md has been updated to reflect these new capabilities and the rationale behind the security improvements. Security advisory files are now deployed during agent setup.
- requirements.txt: update dspy-ai and dspy from 2.6.27 to 3.1.3 to remediate CVE-2025-12695 (fix available in dspy>=3.0.4) - requirements.txt: document CVE-2025-69872 (diskcache==5.6.3 unsafe pickle deserialization) with operational mitigations — no patched release exists yet; monitor for upstream fix - secret-hygiene-helper.sh: expand Python path discovery in scan_pth_files to include pyenv (~/.pyenv/versions/*/bin/python3*), Linux system paths (/usr/bin/python3.*, /usr/lib/python3*/bin/python3*), user-local installs (~/.local/bin/python3*), and PATH-based discovery via 'which -a python3'; deduplicate site-packages directories to avoid scanning the same dir twice - secret-hygiene-helper.sh: fix unpinned dep detection to correctly handle bounded specs (>=1.0,<2.0 is safe) and flag compatible-release (~=) which allows minor/patch upgrades; avoids false positives on bounded ranges
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Mar 25 02:24:49 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
…ain hardening - Add secret-hygiene-helper.sh: scans for plaintext secrets (AWS, GCP, Azure, k8s, Docker, npm, PyPI, SSH keys, .env files), Python .pth supply chain IoCs, unpinned dependencies, and MCP server auto-download risks. Never exposes secret values — only reports locations and key names. All remediation commands directed to separate terminal sessions, never AI chat. - Add advisory system: .agents/advisories/*.advisory files deployed via setup.sh, shown in session greeting until user dismisses with 'secret-hygiene-helper.sh dismiss <id>'. First advisory: LiteLLM PyPI supply chain attack (March 24, 2026). - Pin all Python deps in requirements.txt to exact versions (==) to prevent supply chain attacks via malicious PyPI uploads. Previously used >= which would auto-upgrade to compromised versions. - Integrate secret-hygiene startup-check and advisory display into the session greeting (aidevops-update-check.sh) and cache for Plan+ agents. - Update setup.sh agent-deploy to copy advisory files to ~/.aidevops/advisories/ Context: LiteLLM v1.82.7/v1.82.8 contained credential stealers distributed via compromised PyPI maintainer account. DSPy depends on litellm>=1.64.0 (unpinned), amplifying the attack surface. The malware used Python .pth files which execute on every Python startup regardless of imports.
20b7755 to
4300b5e
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Mar 25 02:29:33 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Actionable item addressed: dspy updated to 3.1.3 (CVE-2025-12695) in f0c47d0. Dismissing stale CHANGES_REQUESTED to unblock merge.
|
Completed via merge at 2026-03-25T02:33:18Z.
|



Summary
aidevops-update-check.shadvisory output with explicitprintf-based concatenation}was at column 1 with no whitespace), but the pattern was misleading enough to trigger a false positive from Augment Code review on PR Security advisory system, secret hygiene scanner, and unified security CLI #5689 (item 🤖 CodeRabbit Analysis Trigger - Auto-Fix Recommendations Requested #3)Closes #5689 review item #3.
Summary by CodeRabbit