Iac Reviewer

---
name: iac-reviewer
description: 'Review infrastructure-as-code changes (Terraform, Bicep, CloudFormation, CDK, Pulumi) for deploy safety and security — destructive replace/delete of stateful resources hidden in innocent diffs, state/backend safety, public exposure, over-broad IAM/RBAC widening, missing encryption, secrets in code or captured in state, tenant-isolation impact, drift from documented architecture, module pinning, and tagging/cost impact. Review-only: severity-ranked findings with file:line evidence and proposed fixes; never applies or runs plan against live backends; provided plan output is input. Use to review an IaC PR or diff, check whether an infra change is safe to apply, audit IaC for misconfiguration, or investigate drift. Do NOT use for database migrations (secure-migration-reviewer), cloud design (azure-saas-architect / aws-saas-architect), account security baselines (cloud-security-baseline-reviewer), pipeline design (ci-pipeline-architect), environment parity (environment-parity-reviewer), or applying changes.'
---

# IaC Reviewer

Terms: **IaC** means infrastructure as code; **CDK** means Cloud
Development Kit; **PR** means pull request; **IAM** means identity and
access management; **RBAC** means role-based access control; **DDL** means
data definition language; **DML** means data manipulation language;
**CI/CD** means continuous integration and continuous delivery; **DB**
means database; **ADR** means architecture decision record. **State** is
the file an IaC tool keeps to match its code to real cloud resources, and
the **backend** is where that file is stored. A **plan** (Bicep calls it
**what-if**) is the tool's preview of what applying the change would do.
**Blast radius** is everything a change can destroy, replace or expose. A
**CIDR** range is a block of network addresses; `0.0.0.0/0` means the
whole internet. **Ingress** is traffic coming in; **egress** is traffic
going out.

## Purpose

Produce a deploy-safety and security review of an infrastructure-as-code
change: severity-ranked findings with file:line evidence, the blast radius
of the diff (what gets destroyed, replaced, or exposed), and proposed
corrections — without ever applying anything. The discipline is
blast-radius-before-style: the first questions are "what does this destroy
or replace" and "what does this expose", asked against the actual resource
graph, not the diff's cosmetic size. A one-line attribute change that
forces a stateful resource replacement is a data-loss event wearing a
trivial diff.

## Use When

- Use when: asked to review a Terraform/Bicep/CloudFormation/CDK/Pulumi PR
  or diff before it is applied.
- Use when: asked whether an infrastructure change is safe, what it will
  destroy/replace, or what it exposes.
- Use when: auditing existing IaC for security misconfiguration — public
  exposure, IAM width, encryption, secrets, tagging.
- Use when: investigating suspected drift between IaC, documented
  architecture, and actual runtime state.
- Do NOT use when: the change is a database schema migration —
  `secure-migration-reviewer` (DDL/DML deploy safety is its territory;
  this skill owns the infrastructure the database runs on).
- Do NOT use when: designing the architecture the IaC should express —
  `azure-saas-architect` / `aws-saas-architect` / `cloud-architecture-decider`.
- Do NOT use when: the settings were made in a console or a managed platform
  with no IaC, and the ask is whether the configured account meets a
  security baseline — that is `cloud-security-baseline-reviewer`, which
  reads exported settings. Drift between IaC and the running account stays
  here.
- Do NOT use when: designing or editing CI/CD pipelines —
  `ci-pipeline-architect` (even though pipelines often run IaC).
- Do NOT use when: the question is how environments differ from each other
  (runtime or database versions, variables, provider modes between staging
  and production) — that is `environment-parity-reviewer`; this skill owns
  IaC diffs and IaC-to-runtime drift.
- Do NOT use when: the request is to APPLY the change — applying is a
  human-gated operation outside this skill.

## Inputs to Inspect

1. The actual IaC diff (PR or working tree) — no diff, no review.
2. Plan/what-if output if the human provides it (treated as INPUT — this
   skill does not execute plan against live backends itself; plan execution
   touches state and cloud APIs and stays with the human/pipeline).
3. The surrounding module/resource definitions the diff touches — a diff
   line's meaning lives in its resource block and its references.
4. State/backend configuration: where state lives, locking, encryption,
   who can read it (state files contain secrets and resource details).
5. The documented architecture and tenancy model (`azure-saas-architect` /
   `aws-saas-architect` output, ADRs) — the reference point for drift and
   isolation impact.
6. Provider/module version pins (lockfiles, version constraints) and the
   registry sources modules come from.
7. The tagging standard and environment layout the repo claims to follow.

## Workflow

1. **Establish blast radius first**: walk the diff for operations that
   destroy or replace stateful resources — renames without state moves,
   immutable-attribute changes forcing replacement, count/for_each keying
   changes reindexing resources, deletions. For each: the resource, the
   data it holds, and whether the change is survivable. Plan output, when
   provided, is cross-checked against this reading — a plan the human did
   not provide is requested, not simulated.
2. **Hunt public exposure**: security groups/firewall rules widening to
   0.0.0.0/0 or broad CIDRs, storage buckets/containers going public,
   public IPs on data stores, disabled private-endpoint enforcement,
   ingress objects exposing internal services. Every exposure finding
   names the resource and the reachable surface.
3. **Review identity and access width**: IAM/RBAC policies widening
   (wildcards in actions/resources/principals), role-assumption trust
   changes, service accounts gaining scopes, cross-account/cross-tenant
   grants. Width findings cite the before/after delta, not just the after.
4. **Check encryption and secrets**: encryption disabled or defaulted off
   on new stores, secrets/credentials literal in IaC source or template
   parameters, values that will be captured in state (many provider
   resources store secrets in state — flag where the design should use a
   secret store reference instead), key-management changes.
5. **Assess tenant-isolation impact**: shared-infrastructure changes that
   collapse isolation the architecture promises — per-tenant resources
   becoming shared, network boundaries between tenant tiers removed,
   tenant-scoped IAM conditions loosened. Route mechanism-level questions
   to `tenant-isolation-reviewer` when the finding is systemic.
6. **Check drift and hygiene**: does the diff match the documented
   architecture (undocumented new dependencies = drift or doc rot —
   `source-of-truth-reconciler` when contested); module/provider pinning
   (unpinned = supply-chain surface — compose
   `supply-chain-security-reviewer` for registry-source risk), module
   duplication, dead resources, tagging compliance of new resources and
   the cost visibility lost when absent.
7. **Estimate cost impact of the diff**: new always-on resources, sizing
   changes, cross-region/egress-generating paths, log/telemetry volume
   changes — order-of-magnitude flags feeding `saas-cost-architect`, not
   invented prices.
8. **File findings and corrections**: severity-ranked (blocker / high /
   medium / low) with file:line, the failure scenario, and a proposed
   corrected snippet per finding where correction is unambiguous. State
   what was NOT reviewed (files, modules, or runtime state out of reach).

## Output Format

```
IAC REVIEW — <PR/diff identifier>
Blast radius: <destroy/replace/reindex operations — resource, data at risk,
  survivable? — or "none detected">
Findings (severity-ranked, file:line each):
  <BLOCKER/HIGH/MED/LOW — resource — what fails/exposes — evidence —
   proposed correction>
Tenant-isolation impact: <collapsed/preserved boundaries, or n/a + why>
Drift notes: <IaC vs documented architecture mismatches>
Cost flags: <order-of-magnitude impacts, attribution/tagging gaps>
Apply-safety verdict: <no blocker found in reviewed evidence | corrections
  required | do not apply | insufficient evidence without plan/runtime state
  — one-line reason; review is not execution approval>
Not reviewed: <what this review could not see (state, runtime, modules)>
```

## Validation Checklist

- [ ] Blast radius was established before style/hygiene findings; every
      destroy/replace of a stateful resource is called out.
- [ ] Every finding carries file:line evidence and a concrete failure or
      exposure scenario — no vibes-based severity.
- [ ] IAM/RBAC findings show the before/after width delta.
- [ ] Secrets checked in source AND as state-capture risk.
- [ ] Tenant-isolation impact explicitly assessed (or explicitly n/a).
- [ ] Nothing was applied, no plan was executed against a live backend,
      and no cloud state was mutated by this review.
- [ ] The not-reviewed list is honest — unseen modules/state are named,
      not silently assumed fine.
- [ ] DB schema migrations found in the diff were routed to
      `secure-migration-reviewer`, not reviewed here.

## Gotchas

- Resource renames without a state move (a `moved` block or
  `terraform state mv`, which tells the tool the resource was renamed, not
  replaced) read as delete+create — the most common accidental-destroy
  pattern in review.
- `for_each`/`count` (which create several copies of one resource) key
  changes reindex collections: the diff looks like an addition but the plan
  destroys and recreates siblings.
- "Temporary" 0.0.0.0/0 rules and public-access flags ship far more often
  than they are removed; treat every widening as permanent unless the diff
  contains its own expiry mechanism.
- Default values do heavy lifting: a module upgrade that changes a default
  (encryption, public access, deletion protection) alters resources with
  zero diff lines in the caller — check module version bumps for default
  changes.
- Plan output is environment-specific: a clean plan against staging says
  nothing about production state; name which environment the evidence
  covers.
- CDK/Pulumi reviews need the synthesized output when logic is dynamic —
  reviewing only the program code misses what loops and conditions
  actually emit.

## Stop Conditions

- No actual diff or IaC source is available → no review; ask for the
  change, do not review from a verbal description.
- The change's safety hinges on live state or plan output that is not
  provided → request it from the human; do not execute plan/what-if
  against live backends or guess state contents.
- The diff intends destruction of stateful production resources (even
  legitimately) → findings + `human-approval-boundary`; destruction is
  never waved through as review-approved.
- Asked to apply, fix-and-apply, or "just make it safe and ship it" →
  stop at proposed corrections; applying is outside this skill.

## Supporting Files

- `references/iac-review-checklist.md` — per-category checklists
  (blast radius, exposure, IAM width, secrets/state, hygiene) and
  tool-specific sharp edges (Terraform/Bicep/CloudFormation/CDK).
- `evals/evals.json` — trigger + behavior cases.
- `evals/trigger-evals.json` — discrimination within the cloud cluster
  (`cloud-architecture-decider`, `azure-saas-architect`,
  `aws-saas-architect`), against shipped `secure-migration-reviewer` /
  `supply-chain-security-reviewer` / `security-pr-reviewer`, and the D71
  seams with `cloud-security-baseline-reviewer` and
  `environment-parity-reviewer`.