dev
84ae9b817e
Adds the unified-review integration that fuses CRG graph context with the ai-code-review scoring methodology and gstack-review fix-first workflow: - scoring.py: objective Layer-2 metrics (sql_risk, exception_coverage, redundancy_rate, high_risk_density, vulnerability_risk) with good/warn/fail grades, plus dedupe_findings (fingerprint merge, multi-source confidence boost, PR quality score) and report data builder - tools/scoring_tools.py + main.py: three new MCP tools (score_review_tool, dedupe_findings_tool, generate_report_tool) - assets/report-template.html: self-contained HTML report template - skills.py + skills/unified-review/: new read-only unified-review skill with language/manual-review/specialist checklists - docs and CHANGELOG updated; tests added (test_scoring, test_report, test_unified_review) and test_skills updated for 5 skills
83 lines
3.4 KiB
Markdown
83 lines
3.4 KiB
Markdown
# Unified Review — Generic Checklist
|
|
|
|
Reference for the Layer-1 chain decomposition and gstack CRITICAL sub-pass.
|
|
Load the language-specific checklist when available (`java-spring.md`,
|
|
`python-django.md`, `python-fastapi.md`, `node-express.md`, `go-gin.md`,
|
|
`csharp-dotnet.md`, `rust.md`, `php-laravel.md`, `ruby-rails.md`); otherwise
|
|
use this generic list.
|
|
|
|
## Layer 1 — eight categories
|
|
|
|
For each changed area mark ✅ Clean / ⚠️ Issues Found / — N/A.
|
|
|
|
1. **Interface** — parameter validation, response conventions, HTTP status
|
|
codes, rate limiting, API versioning, protocol correctness
|
|
2. **Business** — logic aligns with requirements, state machine correctness,
|
|
idempotency design, distributed locks
|
|
3. **Data** — SQL injection, query performance, index usage, transaction
|
|
boundaries, cache invalidation
|
|
4. **Utility** — input validity, no side effects, error return values,
|
|
date/time timezone handling
|
|
5. **Error handling** — exception classification, fallback logic, error
|
|
message sanitization, retry with backoff
|
|
6. **Security** — AuthN/AuthZ, sensitive data masking, permission control,
|
|
CSRF/XSS prevention
|
|
7. **Performance** — N+1 queries, caching strategy, connection pooling,
|
|
batch operations, blocking in async paths
|
|
8. **Observability** — structured logging with correlation IDs, metrics,
|
|
health checks
|
|
|
|
## gstack CRITICAL sub-pass (highest severity)
|
|
|
|
### SQL & Data Safety
|
|
- String interpolation in SQL — use parameterized queries
|
|
- TOCTOU check-then-set — use atomic `WHERE` + update
|
|
- Bypassing model validations for direct DB writes
|
|
- N+1 queries — missing eager loading
|
|
|
|
### Race Conditions & Concurrency
|
|
- Read-check-write without uniqueness constraint / duplicate-key retry
|
|
- find-or-create without a unique DB index
|
|
- Status transitions not atomic (`WHERE old_status = ? UPDATE ...`)
|
|
- Unsafe HTML rendering on user-controlled data
|
|
|
|
### LLM Output Trust Boundary
|
|
- LLM-generated values (emails, URLs, names) written to DB without format
|
|
validation
|
|
- Structured tool output accepted without type/shape checks
|
|
- LLM-generated URLs fetched without an allowlist (SSRF)
|
|
- LLM output stored in knowledge bases without sanitization (stored prompt
|
|
injection)
|
|
|
|
### Shell Injection
|
|
- `subprocess` with `shell=True` AND interpolated command strings
|
|
- `os.system()` with variable interpolation
|
|
- `eval()`/`exec()` on LLM-generated code without sandboxing
|
|
|
|
### Enum & Value Completeness
|
|
- New enum/status/tier values: read (not just grep) every consumer that
|
|
switches/filters/displays the value
|
|
- Check allowlists and `case`/`if-elsif` chains for fall-through
|
|
|
|
## Suppressions — do NOT flag
|
|
- Harmless redundancy that aids readability
|
|
- "Add a comment explaining a threshold" — thresholds drift
|
|
- Consistency-only changes
|
|
- Anything already addressed in the diff
|
|
|
|
## Severity
|
|
- 🔴 **blocker** — must fix before merge (injection, secrets, missing
|
|
transaction, auth bypass) → verdict FAIL
|
|
- 🟡 **major** — should fix before merge (missing validation, missing
|
|
fallback, N+1, unmasked data)
|
|
- 🔵 **minor** — can optimize later (naming, duplicate code, comments)
|
|
|
|
## Confidence calibration
|
|
- 9-10 verified by reading specific code
|
|
- 7-8 high-confidence pattern match
|
|
- 5-6 medium — show with caveat
|
|
- 3-4 low — move to appendix
|
|
- 1-2 speculation — suppress unless severity would be P0
|
|
|
|
Every finding: `[SEVERITY] (confidence: N/10) file:line — problem → fix`.
|