collection-review

name	collection-review
description	Review pull requests and code changes for the network.compliance collection. Use when reviewing PRs, checking code quality, validating STIG rule implementations, or ensuring validated content standards are met.
license	GPL-3.0-or-later
compatibility	Requires gh CLI, ansible-lint, and access to the repository.
metadata	{"author":"network-compliance-team","version":"1.0"}

Collection Review

When to use this skill

Use this skill when:

• Reviewing a pull request for new STIG rules
• Reviewing platform onboarding changes
• Validating that code meets Ansible Validated Content standards
• Checking compliance with AGENTS.md conventions

Review process

Step 1: Understand the change

gh pr view <number>
gh pr diff <number>

Identify what type of change this is:

• New STIG rule → Check stig-rule-development checklist
• New platform → Check platform-onboarding checklist
• Bug fix → Verify the fix and check for regressions
• Refactor → Ensure no functional changes

Step 2: Verify STIG metadata against DISA source

For any new STIG rule, verify evaluate/vars/stig/<platform>/catN.yaml against the official DISA STIG guide. Do not approve metadata that has not been cross-referenced with the authoritative source. AI agents may generate plausible but inaccurate STIG identifiers.

[ ] V-key is correct and matches DISA source
[ ] STIG ID matches (e.g., CISC-L2-000020)
[ ] Rule ID with revision is accurate (check revision number)
[ ] Severity category is correct (cat1/cat2/cat3)
[ ] SRG ID is present and verified
[ ] CCI is present and verified
[ ] Check content matches DISA STIG guide verbatim
[ ] Fix text matches DISA STIG guide verbatim

Cross-reference with: https://public.cyber.mil/stigs/ or STIG Viewer

Step 3: Verify task conventions

[ ] Task names follow: STIG | {stig_id} | {v_key} | {severity} | {description}
[ ] Tags include: STIG ID, V-key, Rule ID, severity (lowercase), CCI
[ ] FQCNs used for ALL modules (no short names)
[ ] Variables accessed via stig_controls['V-XXXXX'] pattern
[ ] Internal variables prefixed with _ (e.g., _eval_result)
[ ] Sensitive values use no_log: true

Step 4: Verify evaluate role requirements

[ ] Works in check_mode (read-only, no device changes)
[ ] Uses <os>_command for evaluation (consistent pattern)
[ ] Uses evaluate_results or check_results + stig_result filters
[ ] Results stored in stig_results dict keyed by V-key
[ ] Status is 'not_a_finding' or 'open' (not custom values)
[ ] Finding details include specific non-compliant items
[ ] Block structure groups related tasks

Step 5: Verify remediate role requirements

[ ] Conditional on evaluation result: when stig_results[V-key].status == 'open'
[ ] Idempotent: running twice produces no changes
[ ] Uses save_when: changed (not save_when: always)
[ ] Safe-fail logic for lockout-risk operations
[ ] Remediation templates use Jinja2 loops (not hardcoded interfaces)

Step 6: Verify argument_specs.yml

[ ] New variables documented with type, description, default
[ ] Required vs optional correctly marked
[ ] Choices listed where applicable
[ ] No undocumented variables used in tasks

Step 7: Verify Molecule scenarios

[ ] Molecule scenario exists under extensions/molecule/ for the change
[ ] converge.yml applies the role with appropriate variables
[ ] verify.yml asserts both pass (compliant) and fail (non-compliant) scenarios
[ ] idempotence step included in test_sequence for remediate roles
[ ] Scenario runs cleanly: molecule test -s <scenario_name>

Step 8: Verify report integration

[ ] New rules added to evaluate/vars/stig/<platform>/catN.yaml (to_cklb and to_xccdf iterate dynamically)
[ ] Report output includes correct V-key, Rule ID, severity
[ ] Status and findings flow from stig_results via filter plugins

Step 9: Run quality checks

# Checkout the PR
gh pr checkout <number>

# Auto-fix lint and format issues
tox -e fix

# Run all CI checks
tox -e ci

# Run sanity (requires Docker)
tox -e sanity

# Run affected Molecule scenarios
molecule test -s <scenario_name>

Step 10: Check changelog

[ ] Changelog fragment exists in changelogs/fragments/
[ ] Fragment uses correct section (minor_changes, bugfixes, etc.)
[ ] Description is user-facing and mentions STIG ID

Common review issues

Issue	Fix
Short module names (ios_config)	Use FQCN (cisco.ios.ios_config)
Missing tags on tasks	Add STIG ID, V-key, severity, CCI
ignore_errors: true	Use block/rescue or failed_when
Hardcoded interface names	Use loop over compliance_access_ports
save_when: always in remediate	Change to save_when: changed
Custom status values	Use only not_a_finding, open, not_reviewed
Missing no_log on passwords	Add no_log: true
.yml extension	Use .yaml (collection standard)

Validated content quality bar

This collection follows Ansible Validated Content standards:

1. Production-ready: Passes ansible-lint production profile
2. Fully documented: argument_specs, README, task names
3. Tested: Integration tests for all rules
4. Traceable: Every task maps to a DISA STIG control
5. Secure: no_log on secrets, safe-fail on lockout risks
6. Idempotent: All remediation is safe to re-run