feat: expose the inspector rule packs to automation - #363
Conversation
Adds `unic inspect --json` and the `run_security_inspector` MCP tool, the first fill from #360's coverage list. Ten rule packs collapse into one call, which is the curated view a raw AWS API bridge cannot reproduce. Returns the shared v1 envelope; `data` carries scanned_at, scanner_count, finding_count, severity_counts, and findings. Per-scanner failures land in `warnings` instead of being dropped, so a partially denied scan is never reported as a clean one. Findings serialize as an empty array rather than null so agents need not branch on both. `--checklist` is a root persistent flag, so it is inherited here and advertised by `unic schema inspect`. Running the security packs while silently ignoring it would hand back the wrong report, so it is rejected explicitly until the checklist output contract is defined. Inspector is a workflow rather than a catalog feature, so it sits outside `unic resources` and outside the catalog parity test added in #361; the docs now say so. Refs #360 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2bi5Xga4wU5hDPgNPd4mv
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (8)
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 |
There was a problem hiding this comment.
Review Summary
I've completed a thorough review of PR #363, which exposes the security inspector rule packs to automation via the unic inspect --json CLI command and the run_security_inspector MCP tool.
Key Changes Reviewed
- New
inspectcommand with JSON-only output and proper contract versioning - Integration with existing
resourceRepositoryfor consistent AWS context resolution - MCP tool registration with
unic.inspect.v1contract (correctly outside theunic.resources.*namespace) - Comprehensive test coverage for CLI and MCP integration
- Proper handling of scanner warnings, empty findings, and checklist flag inheritance
Assessment
The implementation is well-designed and ready to merge. The code demonstrates strong engineering practices with proper error handling, comprehensive test coverage, and thoughtful contract design decisions. No blocking issues were identified during this review.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Summary
unic inspect --jsonand therun_security_inspectorMCP tool — the first fill from #360's coverage list, picked because it has the best ratio of agent value to surface area: ten rule packs collapse into one call, and the failure-first findings view is exactly what a raw AWS API bridge cannot reproduce.Reuses the existing
jsonEnvelope[T]andwriteResourceJSON, and resolves its AWS context throughresourceRepositoryso it behaves like every other automation command.Three deliberate contract decisions
Scanner warnings reach the agent.
RunSecurityScanalready collects per-scanner failures intoreport.Warnings; those map onto the envelope'swarnings. A scan where the cost pack was denied must not look identical to a clean scan.Findings serialize as
[], notnull. An empty slice is pre-allocated so agents never have to branch on null-vs-empty. Covered by a test.--checklistis rejected, not ignored. It is a root persistent flag, so it is inherited by this command and shows up inunic schema inspect:An agent reading that schema would reasonably pass
--checklistand expect a checklist run. Checklist Inspector produces a different report shape (ChecklistReport) with no agent contract yet, so honoring the flag is out of scope — but silently ignoring it would hand back security findings as if the checklist had run. It fails loudly instead, with a message saying why.Relationship to #361
Inspector is a workflow, not a
domain.Catalog()feature —grep -c Inspector internal/domain/{model,catalog}.goreturns 0 — so the parity guard merged in #361 does not and cannot cover it. This PR therefore:unic.resources.*namespace the parity test walks, usingunic.inspect.v1TestSecurityInspectorToolIsReadOnlyAndNotAResourceContractto pin that boundaryValidation
go build ./...,go vet ./...,go test ./...— all packages pass, including test: guard agent surface parity #361's parity test--json=falserejected,--checklistrejected with no partial output, read-only/v1 annotations presentunic capabilities --jsonandunic schema inspect --jsonboth pick the command upRefs #360 — the remaining checklist items (ElastiCache, SNS, CloudFormation, Step Functions fills) stay open.