t1032.1: Implement code-audit-helper.sh unified audit orchestrator#1376
t1032.1: Implement code-audit-helper.sh unified audit orchestrator#1376marcusquinn merged 2 commits intomainfrom
Conversation
WalkthroughThe script undergoes a comprehensive rewrite transforming it from a simple snapshot-oriented tool into a unified audit orchestrator. It introduces a normalized SQLite schema with Changes
Sequence DiagramsequenceDiagram
participant Client
participant Orchestrator as Orchestrator<br/>(cmd_audit)
participant CR as CodeRabbit<br/>Collector
participant SC as SonarCloud<br/>Collector
participant CD as Codacy<br/>Collector
participant CF as CodeFactor<br/>Collector
participant DB as SQLite DB<br/>(audit_runs/<br/>audit_findings)
participant Dedup as Deduplication<br/>Logic
Client->>Orchestrator: audit command
Orchestrator->>DB: Create audit_run row
Orchestrator->>CR: collect_coderabbit(run_id)
CR->>DB: Insert findings
Orchestrator->>SC: collect_sonarcloud(run_id)
SC->>DB: Insert findings
Orchestrator->>CD: collect_codacy(run_id)
CD->>DB: Insert findings
Orchestrator->>CF: collect_codefactor(run_id)
CF->>DB: Insert findings
Orchestrator->>Dedup: deduplicate_findings(run_id)
Dedup->>DB: Mark duplicates<br/>(dedup_key match)
Orchestrator->>DB: Mark run complete
Orchestrator->>Client: Audit complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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 @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 code auditing capabilities by introducing a unified orchestrator. It centralizes the collection and reporting of code quality findings from multiple services, providing a consolidated and deduplicated view of potential issues. This change streamlines the code review process and offers a clearer picture of code health across different analysis tools. Highlights
Changelog
Activity
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
|
🔍 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: Fri Feb 13 15:02:35 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive script for orchestrating code audits. The implementation is well-structured and follows many of the established patterns. My review focuses on a few key areas to enhance security, robustness, and adherence to the repository style guide. I've identified a critical SQL injection vulnerability that needs immediate attention, along with a high-severity bug in the SQL escaping logic. Other comments point out style guide violations and opportunities to improve maintainability and robustness. Addressing these points will make this excellent script even better.
🔍 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: Fri Feb 13 15:37:57 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comprehensive test script (tests/test-audit-e2e.sh) that exercises all 7 checkpoints of the unified audit cycle: 1. Service polling — verifies collectors exist for all 4 services 2. Unified DB — seeds test findings, validates schema and dedup 3. Task creation — verifies finding-to-task pipeline with correct IDs 4. Phase 10b — validates TODO.md append, cooldown, commit/push 5. Phase 0 — confirms auto-dispatch pickup and dispatch wiring 6. Worker PRs — validates dispatch/evaluate infrastructure 7. Trend tracking — tests audit_snapshots table and WoW deltas Results: 42 pass, 0 fail, 10 skip (all skips due to unmerged deps). Documents 10 gaps, all traceable to 3 open PRs (#1376, #1377, #1378). Decision: Treat stub code-audit-helper.sh as skip not fail since the implementation exists in PR #1376 — matches existing codebase patterns.
Comprehensive test script (tests/test-audit-e2e.sh) that exercises all 7 checkpoints of the unified audit cycle: 1. Service polling — verifies collectors exist for all 4 services 2. Unified DB — seeds test findings, validates schema and dedup 3. Task creation — verifies finding-to-task pipeline with correct IDs 4. Phase 10b — validates TODO.md append, cooldown, commit/push 5. Phase 0 — confirms auto-dispatch pickup and dispatch wiring 6. Worker PRs — validates dispatch/evaluate infrastructure 7. Trend tracking — tests audit_snapshots table and WoW deltas Results: 42 pass, 0 fail, 10 skip (all skips due to unmerged deps). Documents 10 gaps, all traceable to 3 open PRs (#1376, #1377, #1378). Decision: Treat stub code-audit-helper.sh as skip not fail since the implementation exists in PR #1376 — matches existing codebase patterns.
…2.1) Replace 6-line placeholder with full orchestrator that: - Iterates configured services from code-audit-config.json - Calls each collector (CodeRabbit, Codacy, SonarCloud, CodeFactor) - Merges results into shared audit_findings SQLite table - Deduplicates cross-service findings on same file+line - Outputs unified summary with severity/category/file breakdowns ShellCheck: zero violations. Syntax: verified. Integration: tested.
- [critical] Add numeric validation for pr_number, run_id, limit before SQL interpolation to prevent injection in cmd_audit, cmd_report, cmd_summary - [high] Remove incorrect backslash un-escaping in sql_escape() that could corrupt legitimate input containing backslashes - [medium] Add explicit return to db() per project style guide - [medium] Separate local declaration from assignment in sql_escape() - [medium] Use cut -c1-500 instead of head -c 500 for UTF-8 safe truncation - [medium] Use read -ra array pattern for services iteration to prevent glob expansion
marcusquinn
force-pushed
the
feature/t1032.1
branch
from
f0ae9b9 to
4f29c4d
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: Fri Feb 13 19:09:35 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.agents/scripts/code-audit-helper.sh:
- Around line 636-639: Auto-detected pr_number from the gh command is not
validated and could contain non-numeric output; after the assignment to
pr_number inside the auto-detect block (the gh pr view call), validate that
pr_number is purely numeric (e.g., match against ^[0-9]+$ or use a numeric test)
and if it fails, set it to 0 or exit with an error before any SQL interpolation
occurs (the later use of pr_number in the SQL on line 653 must only receive a
validated numeric value); ensure this validation happens immediately after the
gh pr view assignment so the sanitized pr_number is used everywhere.
🧹 Nitpick comments (2)
.agents/scripts/code-audit-helper.sh (2)
247-259: CodeRabbit importer builds SQL via shell string interpolation — less robust than the jq approach used by other collectors.The SonarCloud, Codacy, and CodeFactor collectors all use jq with a
sql_strfilter to generate SQL, which is more controlled. Here,sql_escapeis called on individual fields but the overall SQL is assembled via shell string concatenation inside a pipeline subshell. A description containing characters that break shell quoting (or a truly adversarial code comment) could corrupt the generated SQL.Additionally,
$pr_numberon line 250 is interpolated directly into the inner DB query withoutsql_escape— it's safe because it's validated as numeric upstream, but it's inconsistent with the escaping applied to other fields.Consider refactoring to use a jq-based approach consistent with the other collectors, or at minimum, wrapping the inserts in a transaction for atomicity.
187-193:sql_escapeis correct for SQLite but consider handling newlines.Single-quote doubling is the right escaping for SQLite string literals. However, multi-line descriptions (from API responses or code comments) could contain literal newlines that break the generated SQL file when each INSERT is expected on one logical line — particularly in
import_coderabbit_findingswhere SQL is appended line-by-line to a file.The jq-based collectors avoid this because jq's output is single-line by default. For the shell-based
import_coderabbit_findings, a newline in$bodycould split the INSERT across multiple lines in$sql_file, causing parse errors.🛡️ Proposed hardening
sql_escape() { local val val="$1" + # Replace newlines and carriage returns with spaces + val="${val//$'\n'/ }" + val="${val//$'\r'/}" val="${val//\'/\'\'}" echo "$val" return 0 }
| # Auto-detect PR if not specified | ||
| if [[ "$pr_number" -eq 0 ]]; then | ||
| pr_number=$(gh pr view --json number -q .number 2>/dev/null || echo "0") | ||
| fi |
There was a problem hiding this comment.
Auto-detected PR number bypasses numeric validation.
Line 626 validates pr_number when provided via --pr, but the auto-detected value from gh pr view on line 638 is never validated. If gh returns unexpected output (e.g., an error string leaking to stdout), it flows directly into SQL interpolation on line 653.
🛡️ Proposed fix
if [[ "$pr_number" -eq 0 ]]; then
pr_number=$(gh pr view --json number -q .number 2>/dev/null || echo "0")
+ if ! [[ "$pr_number" =~ ^[0-9]+$ ]]; then
+ log_warn "Could not auto-detect PR number, defaulting to 0"
+ pr_number=0
+ fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Auto-detect PR if not specified | |
| if [[ "$pr_number" -eq 0 ]]; then | |
| pr_number=$(gh pr view --json number -q .number 2>/dev/null || echo "0") | |
| fi | |
| # Auto-detect PR if not specified | |
| if [[ "$pr_number" -eq 0 ]]; then | |
| pr_number=$(gh pr view --json number -q .number 2>/dev/null || echo "0") | |
| if ! [[ "$pr_number" =~ ^[0-9]+$ ]]; then | |
| log_warn "Could not auto-detect PR number, defaulting to 0" | |
| pr_number=0 | |
| fi | |
| fi |
🤖 Prompt for AI Agents
In @.agents/scripts/code-audit-helper.sh around lines 636 - 639, Auto-detected
pr_number from the gh command is not validated and could contain non-numeric
output; after the assignment to pr_number inside the auto-detect block (the gh
pr view call), validate that pr_number is purely numeric (e.g., match against
^[0-9]+$ or use a numeric test) and if it fails, set it to 0 or exit with an
error before any SQL interpolation occurs (the later use of pr_number in the SQL
on line 653 must only receive a validated numeric value); ensure this validation
happens immediately after the gh pr view assignment so the sanitized pr_number
is used everywhere.
1 participant
Summary
code-audit-config.json(CodeRabbit, Codacy, SonarCloud, CodeFactor)audit_findingsSQLite table with columns: source, severity, path, line, description, category, rule_idImplementation Details
Commands:
audit,report,summary,status,reset,helpService collectors:
coderabbit-collector-helper.shSQLite DBcurl+jqparsingcurl+jqparsingcurl+jqparsingQuality: ShellCheck zero violations, bash syntax verified, integration tested with deduplication verification.
Patterns followed:
local var="$1", explicit returns,db()wrapper with busy_timeout (t135.3),_save_cleanup_scope/push_cleanupfor temp files, stderr logging from collector subshells.Testing
bash -nsyntax check: PASSshellcheck -x: zero violationshelpcommand: worksstatuscommand: shows dependencies, service availability, DB statsauditcommand: gracefully skips services without tokens, full pipeline runssummaryandreportcommands: work with latest runCloses #1363
Summary by CodeRabbit
New Features
audit,report,summary,status,reset, andhelpfor improved workflow.Refactor