-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add security analysis with OSV, Ferret, and git history scanning #274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add security-analysis.md subagent with AI-powered vulnerability detection - Add security-helper.sh script for CLI operations - Support multiple scan modes: diff, staged, branch, full codebase - Add git history scanning for historical vulnerabilities - Integrate OSV-Scanner for dependency vulnerability detection - Integrate Ferret for AI CLI configuration security (prompt injection, jailbreaks) - Support SARIF output for CI/CD integration - Update subagent-index.toon with new tools
Summary of ChangesHello @marcusquinn, 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 enhances the project's security posture by introducing a comprehensive, AI-powered security analysis framework. It provides developers with a powerful set of tools to identify vulnerabilities across various aspects of the codebase, from uncommitted changes and full repositories to historical commits and third-party dependencies, as well as specialized scanning for AI CLI configurations. This integration aims to proactively detect and mitigate security risks throughout the development lifecycle. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughAdds a new Bash AI-powered security helper CLI script, updates the TOON tooling index to expose it and other helpers, and introduces comprehensive documentation for the security-analysis workflow, scan modes, tooling, reporting, and CI/pre-commit integration. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Dev (CLI)
participant Helper as "security-helper.sh"
participant Git as "Git"
participant Secretlint as "secretlint"
participant OSV as "OSV-Scanner"
participant Ferret as "Ferret / npx ferret"
participant FS as "Filesystem (OUTPUT_DIR)"
Dev->>Helper: run subcommand (analyze / scan-deps / ferret / status / report)
Helper->>Git: resolve target files/commits (diff/staged/branch/full/history)
alt secretlint available
Helper->>Secretlint: lint target files
Secretlint-->>Helper: findings
else
Helper-->>Dev: advise missing secretlint
end
alt scan-deps requested
Helper->>OSV: run dependency scan on path
OSV-->>Helper: vuln/warn/ok + output
end
alt ferret requested
Helper->>Ferret: scan AI config (local or via npx)
Ferret-->>Helper: config issues / findings
end
Helper->>FS: write reports (text / JSON / SARIF) into OUTPUT_DIR
Helper-->>Dev: exit code + CI-friendly summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🔍 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: Mon Feb 2 04:03:35 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this 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 introduces a comprehensive security analysis helper script and its associated documentation, adding capabilities for dependency scanning, git history analysis, and AI configuration checks. The script is well-structured and robust. My review focuses on improving correctness in file handling, reducing false positives in the history scan, and aligning the documentation with the script's implemented features. Overall, this is a significant and valuable addition to the repository's tooling.
.agent/scripts/security-helper.sh
Outdated
| fi | ||
|
|
||
| # shellcheck disable=SC2086 | ||
| echo "$files_to_scan" | xargs $secretlint_cmd 2>/dev/null || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current use of xargs is not safe for filenames that contain spaces. By default, xargs splits arguments by whitespace, which will cause a file named "my file.js" to be passed as two separate arguments (my and file.js), leading to errors.
To correctly handle filenames with spaces, you can use xargs -I {}. This processes each line of input as a single, quoted argument. While this approach invokes secretlint for each file individually, which might be less performant for a large number of files, it ensures correctness.
| echo "$files_to_scan" | xargs $secretlint_cmd 2>/dev/null || true | |
| echo "$files_to_scan" | grep . | xargs -I {} $secretlint_cmd "{}" 2>/dev/null || true |
.agent/scripts/security-helper.sh
Outdated
| diff_content=$(git show "$commit" --format="" 2>/dev/null || echo "") | ||
|
|
||
| # Quick pattern matching for common security issues | ||
| if echo "$diff_content" | grep -qiE "(password|secret|api.?key|token|credential)" 2>/dev/null; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current regex pattern for detecting secrets is quite broad and may lead to a high number of false positives by matching substrings within other words (e.g., 'secret' in 'secretly').
Using word boundaries (\b) will make the pattern more specific, matching only whole words. I've also slightly improved the pattern to catch common variations like api_key or api-key and plural forms.
| if echo "$diff_content" | grep -qiE "(password|secret|api.?key|token|credential)" 2>/dev/null; then | |
| if echo "$diff_content" | grep -qiE '\b(password|secret|api[_-]?key|token|credential)s?\b' 2>/dev/null; then |
| **"Analysis timeout"** | ||
|
|
||
| ```bash | ||
| # Use file filters for large codebases | ||
| ./.agent/scripts/security-helper.sh analyze full --include="src/**/*" --timeout=600 | ||
| ``` | ||
|
|
||
| **"Too many false positives"** | ||
|
|
||
| ```bash | ||
| # Increase confidence threshold | ||
| ./.agent/scripts/security-helper.sh analyze --confidence=high |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The examples in the "Troubleshooting" section mention --timeout and --confidence options for the analyze command. However, these options are not implemented in the security-helper.sh script and are not listed in its usage information. To avoid user confusion, these examples should be removed or updated to reflect the script's actual capabilities.
- Fix xargs to handle filenames with spaces using -I {}
- Improve secret pattern regex with word boundaries to reduce false positives
- Remove unimplemented --timeout and --confidence options from docs
🔍 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: Mon Feb 2 04:10:45 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @.agent/scripts/security-helper.sh:
- Around line 311-313: The help text incorrectly suggests piping git output into
the analyze command; update the echo lines to show a supported history-range
invocation for the analyze subcommand of .agent/scripts/security-helper.sh
(reference the analyze handler/function) — e.g., instruct users to run the
script with a commit or range argument (like "security-helper.sh analyze
<commit>" or "security-helper.sh analyze <from>..<to>") and remove the
misleading pipe example so the help matches the actual analyze CLI behavior and
argument parsing.
- Around line 33-55: The print_usage() help text advertises flags that the
script does not parse; update print_usage() to reflect only supported
commands/options or implement parsing for the advertised flags. Locate the
print_usage() function and either (a) remove the unsupported options
(--severity, --include, --exclude, --ci, --fail-on, --format, --output if
unused) from the Usage/Options block so the help only shows actual, parsed
flags, or (b) add argument parsing (e.g., in the main dispatch that handles
commands like analyze, history, scan-deps, ferret, report, install) to accept
and handle those flags and wire them into existing functions (analyze,
scan-deps, report, etc.) so they are functional and produce clear logs/exit
codes; ensure the text in print_usage() and any docs remain consistent with the
implemented behavior.
- Around line 283-299: The script aborts under set -euo pipefail because the
post-increment expressions ((current++)) and ((issues_found++)) can return
non-zero when variables are uninitialized; change these to safe arithmetic
updates (e.g., current=$((current+1)) and issues_found=$((issues_found+1))) or
explicitly initialize them as integers (declare -i current=0; declare -i
issues_found=0) before the while loop where the short_hash/commit_msg logic runs
to avoid non-zero exit codes; update the occurrences of ((current++)) and
((issues_found++)) in the while loop to use the chosen safe form.
- Around line 160-213: The cmd_analyze function lacks a guard for running
outside a git repository; add an early check (e.g., using git rev-parse
--is-inside-work-tree) at the start of cmd_analyze and if not inside a repo
print a clear error (or fall back to the "full" scope / return non-zero) so
subsequent git calls (used in the diff/staged/branch branches and in git
ls-files) won't abort the script under set -euo pipefail; update cmd_analyze to
detect repo presence, emit a user-friendly message, and gracefully return or
switch scope before any git diff/git ls-files invocations.
- Around line 405-477: The cmd_report function currently only reads a positional
format argument (local format="${1:-text}") so flags like --format=sarif are
treated as the format string; update cmd_report to parse flags before using the
positional: implement a simple args loop that recognizes --format=VALUE,
--format VALUE and -f VALUE (setting the local variable format) while shifting
consumed args, and if no flag was provided fall back to the positional first
argument as before; keep existing case handling for text/json/sarif so behavior
and backward compatibility (positional usage) remain unchanged.
- Around line 342-354: The current osv-scanner error handling block masks non-1
failures by returning 0; update the handler for the command run in the block
that calls "osv-scanner --recursive \"$path\" \"$@\"" so that when the captured
local variable "exit_code" is not 1 you propagate the real error instead of
treating it as a warning—e.g., exit or return with the original exit_code for
any value other than 1, while keeping the existing special-case behavior for
exit_code == 1 (print "Vulnerabilities found!" and return 1).
In @.agent/tools/code-review/security-analysis.md:
- Around line 377-404: The markdown contains nested triple-backtick fences
causing MD031/MD040 failures; replace the outer triple-backtick code block
surrounding the whole report with a longer fence (e.g., four backticks) and add
blank lines before and after each inner fenced block so the inner ```typescript
blocks are not directly nested; update the matching closing fence for the outer
block accordingly and ensure the Vulnerable Code and Remediation examples keep
their ```typescript fences intact and separated by blank lines, addressing the
nested fenced blocks and markdownlint errors.
🧹 Nitpick comments (2)
.agent/tools/code-review/security-analysis.md (1)
22-255: Condense inline procedures; prefer pointers and file:line references.
This doc embeds a lot of step-by-step commands/config snippets. For .agent/**/*.md, keep a concise overview and point to the helper implementation or subagent entry points using file:line references, reserving inline code for authoritative snippets only.As per coding guidelines: Apply progressive disclosure pattern by using pointers to subagents rather than including inline content in agent documentation.
As per coding guidelines: Include code examples only when authoritative; use file:line references to point to actual implementation instead of inline code snippets..agent/scripts/security-helper.sh (1)
99-105: Makenpxcalls non-interactive for CI reliability.
npxcan prompt to install packages; add-yto avoid hangs in automation.As per coding guidelines: Automation scripts - focus on reliability and robustness; clear logging and feedback; proper exit codes; error recovery mechanisms.🔧 Proposed update
- elif check_command npx && npx ferret-scan --version &>/dev/null 2>&1; then + elif check_command npx && npx -y ferret-scan --version &>/dev/null 2>&1; then @@ - elif check_command npx && npx secretlint --version &>/dev/null 2>&1; then + elif check_command npx && npx -y secretlint --version &>/dev/null 2>&1; then @@ - if check_command secretlint || (check_command npx && npx secretlint --version &>/dev/null 2>&1); then + if check_command secretlint || (check_command npx && npx -y secretlint --version &>/dev/null 2>&1); then @@ - secretlint_cmd="npx secretlint" + secretlint_cmd="npx -y secretlint" @@ - ferret_cmd="npx ferret-scan" + ferret_cmd="npx -y ferret-scan"Also applies to: 114-115, 217-221, 372-378
- Add git repository guard in cmd_analyze to prevent errors outside repos - Fix ((current++)) arithmetic to use safe $((var + 1)) form with set -e - Remove unimplemented options from help text - Propagate OSV-Scanner error codes instead of masking them - Add --format= flag parsing in cmd_report - Fix misleading pipe example in history help text - Fix markdown nested fences with 4-backtick outer fence
🤖 Augment PR SummarySummary: Adds a new security-analysis capability to aidevops, combining dependency scanning, secret detection, AI-config scanning, and lightweight git-history heuristics. Changes:
Technical Notes: The helper script runs in 🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| while IFS= read -r commit; do | ||
| ((current++)) | ||
| local short_hash="${commit:0:8}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| echo "" | ||
| echo -e "For deep analysis of specific commits, use:" | ||
| echo -e " git show <commit> | security-helper.sh analyze" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.agent/scripts/security-helper.sh
Outdated
|
|
||
| cmd_report() { | ||
| local format="${1:-text}" | ||
| shift || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd_report only accepts the format as a positional argument (e.g., report sarif), but the help/docs use report --format=sarif, which will currently be treated as an unknown format string.
Other Locations
.agent/scripts/security-helper.sh:63.agent/tools/code-review/security-analysis.md:411
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| ```bash | ||
| # Scan only specific directories | ||
| ./.agent/scripts/security-helper.sh analyze full --include="src/**/*.ts,lib/**/*.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doc recommends flags like --include/--exclude (and later --severity-threshold), but security-helper.sh doesn’t currently parse/handle these options, so users will likely assume filtering works when it won’t.
Other Locations
.agent/tools/code-review/security-analysis.md:455
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| # Use file filters for large codebases | ||
| ./.agent/scripts/security-helper.sh analyze full --include="src/**/*" --timeout=600 | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 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: Mon Feb 2 04:16:49 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.agent/tools/code-review/security-analysis.md:
- Around line 22-27: Update the Quick Reference commands line to remove the
standalone "full" entry and clarify that "full" is a scope passed to the analyze
command (i.e., use "analyze [scope]" and mention "analyze full" instead of
"full"), referencing the helper script name ".agent/scripts/security-helper.sh"
and the existing "analyze [scope]" usage so readers know to run
"security-helper.sh analyze full".
- Around line 62-255: The Quick Start and Scan Modes sections include long
inline command/config examples; replace them with a terse "how to run" summary
and remove large code blocks, then add progressive-disclosure pointers to the
authoritative implementation (the security-helper.sh script) and Ferret
integration logic (ferret scan integration) instead of embedding full examples;
update the Quick Start entries for "analyze", "history", "scan-deps", "report",
and "ferret" to one-line usage hints and add explicit references to the
implementation anchors (security-helper.sh implementation and Ferret subagent)
and OSV/dependency scan logic so readers can jump to authoritative code rather
than reading inline samples.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
I've addressed all the CodeRabbit review feedback:
All CI checks are now passing. Ready for merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.agent/tools/code-review/security-analysis.md:
- Around line 628-636: Update the documentation block to reflect the actual CLI
implementation by replacing the incorrect slash-command examples with a concise
progressive-disclosure reference to the script and its subcommands: point
readers to the security helper script (.agent/scripts/security-helper.sh) and
list the real subcommands (analyze, history, scan-deps, ferret, report) as the
supported actions, and follow the progressive disclosure pattern (show minimal
example then expand to detailed usage) matching the implementation defined
around the script's command handling.
🧹 Nitpick comments (2)
.agent/tools/code-review/security-analysis.md (2)
191-309: Consider reducing Ferret-specific content and referencing upstream documentation.The Ferret integration section (119 lines) embeds extensive configuration examples, CLI usage, and installation instructions that duplicate Ferret's own documentation. This creates a maintenance burden when Ferret updates its API or options.
Consider condensing this to a brief integration summary with a pointer to Ferret's official docs (line 642), keeping only the aidevops-specific integration points.
♻️ Suggested approach
## AI CLI Configuration Scanning (Ferret) Ferret detects security threats in AI assistant configurations (65+ rules across 9 categories: credentials, injection, exfiltration, backdoors, supply chain, permissions, persistence, obfuscation, AI-specific). **Integration**: Run via `security-helper.sh ferret` or `npx ferret-scan scan .` **Supported AI CLIs**: Claude Code, Cursor, Windsurf, Continue, Aider, Cline, and generic configs (.ai/, AGENTS.md). For installation, configuration, usage, and rule details, see: - Official docs: https://github.com/fubak/ferret-scan - Implementation: `.agent/scripts/security-helper.sh` (ferret command) **Output formats**: text, JSON, SARIF, HTML (see Ferret docs)
420-463: Consider condensing CI/CD integration examples.The GitHub Actions workflow (29 lines) and pre-commit hook (12 lines) are generic templates rather than authoritative implementation. These could be condensed to brief descriptions with pointers to the security-helper.sh implementation, maintaining progressive disclosure.
♻️ Optional condensing approach
### GitHub Actions Add security analysis to your workflow: ```yaml - name: Security Analysis run: | ./.agent/scripts/security-helper.sh analyze branch ./.agent/scripts/security-helper.sh scan-depsUpload SARIF results using
github/codeql-action/upload-sarif@v3with.security-analysis/security-report.sarif.Pre-commit Hook
Block commits with high-severity issues:
./.agent/scripts/security-helper.sh analyze staged --severity-threshold=highFor complete CI/CD integration patterns, see
.agent/scripts/security-helper.shusage and exit codes.</details> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
- Remove 'full' as standalone command (it's a scope for analyze) - Replace slash commands with CLI usage examples - Condense Ferret section with link to full docs - Reduce doc from 651 to 567 lines
|
🔍 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: Mon Feb 2 10:21:44 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |



Summary
Adds comprehensive AI-powered security vulnerability analysis to aidevops, inspired by Gemini CLI Security Extension and Ferret Scan.
Features
Scan Modes
Vulnerability Categories
Integrations
Files Changed
security-analysis.md- Comprehensive subagent documentationsecurity-helper.sh- CLI script for all security operationssubagent-index.toon- Updated with new toolsUsage
Testing
Summary by CodeRabbit
New Features
Documentation
Chores