tf-best-practices

---
name: tf-best-practices
description: >-
  Best-practice authoring guidance AND a read-only policy gate for AWS Terraform generated by a
  migration skill. Load during any phase that writes a terraform/ directory — first as the "what to
  emit" posture rules + security-baseline spec, then after writing as the deterministic policy verdict.
  Read-only: it reports whether the generated Terraform passes; it never edits .tf files, never touches
  .phase-status.json, and never decides phase completion. Complements (does not replace) terraform
  fmt/init/validate.
---

# tf-best-practices — Generated-IaC posture rules + read-only policy gate

A **shared authoring guide and verdict producer**, not a workflow. It answers two
questions for a phase that generates AWS Terraform:

1. **Before writing** — "what security posture must the generated `terraform/` follow?"
   (the posture rules + the `baseline.tf` account-hardening spec)
2. **After writing** — "does the generated `terraform/` pass policy?" (a deterministic,
   **read-only** verdict + a machine-readable report)

## Routing — load the part that matches your context

This skill is entered at two touchpoints in the caller's Generate flow, with the caller's
own terraform-authoring work in between. **The caller states which touchpoint it is at when it
loads this skill**, and reads the corresponding part:

| Caller context                                              | Load                                                                                                      | Why                                                                                               |
| ----------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- |
| **About to author `terraform/`** (before writing)           | Part 1 → [`references/security-posture-rules.md`](references/security-posture-rules.md)                   | The "what to emit" AWS authoring rules (gate-enforced + authoring-only + compliance-conditional). |
| **`terraform/` written, ready to validate** (after writing) | Part 2 → [`references/terraform-validation.md`](references/terraform-validation.md) + run the gate script | The `fmt → init → validate → policy` protocol and the read-only verdict.                          |

Everything this skill states is **source-cloud-agnostic** (pure AWS Terraform). Any GCP/Heroku
detection or artifact reading is the caller's job; where a rule needs a caller-known fact (e.g.
declared compliance frameworks), the caller passes it as a **caller-context signal** — see
`references/security-posture-rules.md` § _Caller-context signals_.

## Boundary (read this first)

This unit is a **verdict producer, never a mutator**. Its entire write surface is the
JSON verdict it is asked to emit. Specifically it **MUST NOT**:

- edit, format, or rewrite any `.tf` file (the caller owns remediation),
- read or write `.phase-status.json` or any run-state file (interpreter-owned),
- decide whether a phase may complete, or prompt the user (caller policy).

The caller (a migration skill's Generate phase) owns: the fix-and-retry loop that edits
the `.tf` it generated, `terraform fmt` auto-apply, the retry/skip/abort prompt, the
Phase Completion gate, and every `.phase-status.json` write. See the consuming skill's
generate phase for how the verdict feeds those decisions.

> **Consumers:** `gcp-to-aws` (prose Generate) and `heroku-to-aws` (DSL Generate). The
> contract is source-agnostic; each caller wires the two touchpoints in its own Generate
> idiom — gcp-to-aws as prose steps, heroku-to-aws as a fragment step plus a fail-closed
> `_postconditions` assert enforced by the interpreter.

## Part 1 — Authoring posture (load before writing `terraform/`)

Emit generated Terraform that satisfies the posture in
[`references/security-posture-rules.md`](references/security-posture-rules.md).

These are the "what good AWS Terraform looks like" rules. Following them makes the Part 2 gate
pass by construction. This unit does not read the caller's artifacts — it consumes only
caller-context signals the caller passes in.

> **Scope.** `security-posture-rules.md` covers, in three tiers:
>
> - **Gate-enforced** (Part 2 verifies statically): ALB TLS, no-public-database, RDS +
>   ElastiCache encryption-at-rest, no-public-DB-port ingress, no-public admin/datastore-port
>   ingress, no-wildcard-IAM.
> - **Authoring-only** (not gate-checkable, still required): `deletion_protection`,
>   master-password-via-Secrets-Manager, S3 hardening, Fargate/EKS/ECR settings, private-subnet
>   placement, backups, baseline monitoring.
> - **Compliance-conditional** (emitted when the caller declares `soc2`/`pci`/`hipaa`/`fedramp`):
>   VPC flow logs, S3 access logging, secret rotation, customer-managed KMS.
>
> Still the **caller's** own generation concern (candidates to migrate here later): the
> account-hardening `baseline.tf` layer (CloudTrail, GuardDuty, Config, Security Hub).

## Part 2 — Policy gate (run after writing `terraform/`)

Run the read-only checker against the generated directory. Resolve the script path relative to
the plugin root (`$PLUGIN_ROOT/skills/tf-best-practices/scripts/...`), the same convention the
plugin uses for its other scripts:

```bash
python3 "$PLUGIN_ROOT/skills/tf-best-practices/scripts/validate-terraform-policy.py" "$TERRAFORM_DIR" --json "$VERDICT_PATH"
```

- `$TERRAFORM_DIR` — **required, caller-supplied**: the generated `terraform/` directory
  (e.g. `$MIGRATION_DIR/terraform`). This skill never defaults or discovers it — the caller
  always passes the path it wrote Terraform to.
- `--json $VERDICT_PATH` — optional; writes a machine-readable verdict the caller can merge
  into its own `validation-report.json`.

The policy check is one stage of a larger validation flow (`fmt → init → validate → policy`).
The full protocol — including offline-fallback behavior and how the policy verdict maps into a
`validation-report.json` — is documented in
[`references/terraform-validation.md`](references/terraform-validation.md). That protocol is
descriptive: the **caller** owns the fmt/init/validate execution, the fix-and-retry loop, and
the report write; this unit contributes only the read-only policy stage + verdict shape.

### Exit codes → caller action

| Exit | stdout        | Meaning            | Caller does                                                                     |
| ---- | ------------- | ------------------ | ------------------------------------------------------------------------------- |
| `0`  | `POLICY_OK`   | posture satisfied  | proceed                                                                         |
| `1`  | `POLICY_FAIL` | violations present | read `violations[]`, edit the named `.tf` sites, re-run (caller's retry budget) |
| `2`  | (usage error) | bad path / IO      | surface to user; do not treat as pass                                           |

### Verdict shape (`--json`)

```json
{
  "check": "policy",
  "policy_status": "POLICY_OK | POLICY_FAIL",
  "violations": [
    {
      "check": "policy",
      "rule": "alb_https_listener | alb_http_redirect | no_tf_files",
      "file": "compute.tf",
      "line": 7,
      "severity": "error",
      "summary": "human-readable violation",
      "fix_hint": "concrete remediation the caller can apply"
    }
  ]
}
```

Each `violations[]` entry is **actionable evidence** — `file` + `line` + `fix_hint` tell the
caller exactly what to edit. The caller applies the edit; this unit only reports.

## Policy rules enforced today

Every rule is **fail-open on ambiguity** — it fires only on unambiguous, in-block literal
evidence, so a valid stack is never falsely blocked (a `POLICY_FAIL` is a hard completion gate
for the caller, so a false positive would block a real migration).

**Internet-facing ALB TLS posture** (an ALB is internet-facing when `internal` is absent,
`false`, or variable-driven — fail-safe):

- **`alb_https_listener`** — must have an HTTPS listener on `443` with `certificate_arn` and a
  `forward` action.
- **`alb_http_redirect`** — an HTTP `:80` listener must `redirect` to HTTPS, never `forward`
  to targets. Internal ALBs (`internal = true`) are exempt.

> **Elastic Beanstalk ALBs are invisible to these rules.** The ALB rules inspect standalone
> `aws_lb_listener` blocks. An EB **LoadBalanced** environment provisions its ALB from
> `aws_elastic_beanstalk_environment` `setting` blocks, which the static checker does not read —
> so a pure-EB design passes the ALB rules **vacuously** (no listener to inspect). EB listener/TLS
> posture is therefore authoring-only, not gate-enforced. (Fixtures `good-heroku-eb-only`
> and `good-heroku-eb-singleinstance` document this; `good-heroku-eb-loadbalanced` carries a
> standalone ALB so the listener rules are exercised on real blocks.)

**Managed database exposure & encryption** (`aws_db_instance`, `aws_rds_cluster`):

- **`rds_not_public`** — must not set `publicly_accessible = true` (absent/variable → fail-open).
- **`rds_encryption_at_rest`** — must set `storage_encrypted = true`; missing or literal `false`
  fires (RDS defaults to unencrypted), variable-driven fails open. S3 is not checked (default
  SSE-S3 since Jan 2023).

**ElastiCache encryption** (`aws_elasticache_replication_group`, Redis `aws_elasticache_cluster`):

- **`elasticache_encryption_at_rest`** — a replication group must set
  `at_rest_encryption_enabled = true`; missing or literal `false` fires, variable-driven fails
  open.
- **`elasticache_cluster_encryption`** — a Redis-engine `aws_elasticache_cluster` (single-node:
  `engine = "redis"`, no `replication_group_id`) must set BOTH `at_rest_encryption_enabled = true`
  and `transit_encryption_enabled = true`; missing or literal `false` on either fires,
  variable-driven fails open. `engine = "memcached"` clusters (and variable-driven/absent engine)
  are exempt — Memcached does not support these attributes.

**Security group ingress:**

- **`db_sg_no_public_ingress`** — an inline `aws_security_group` ingress covering `5432`/`3306`
  must not allow `0.0.0.0/0` or `::/0`.
- **`sg_no_public_admin_ingress`** — an inline ingress must not open a curated never-public
  admin/datastore port (`22`, `3389`, `6379`, `11211`, `27017`, `9200`/`9300`, `5601`) to
  `0.0.0.0/0` or `::/0`. Web (`80`/`443`) and app/game ports are not flagged; DB ports are
  handled by the rule above. Both check `cidr_blocks` and `ipv6_cidr_blocks` independently, so a
  benign IPv4 list does not mask an open IPv6 one. Both: separate `aws_security_group_rule` /
  `aws_vpc_security_group_ingress_rule` resources fail open (not correlated).

**IAM least-privilege** (`aws_iam_policy`, `aws_iam_role_policy`, `aws_iam_group_policy`,
`aws_iam_user_policy`):

- **`no_wildcard_iam`** — an `Allow` statement must not use `Action`/`Resource` `"*"`.
  The one narrow exception is an isolated
  `elasticbeanstalk:CreateStorageLocation` statement with `Resource = "*"` because
  AWS does not support resource-level permissions for that action.
  `aws_iam_policy_document` data sources and assume-role trust policies fail open.

> The checker is a zero-dependency static HCL reader (no `terraform init`, no provider
> download) — it runs even when the registry is unreachable. It uses brace-depth matching
> for nested blocks, so a valid HTTPS listener written with a nested `forward { ... }`
> block is **not** a false failure.

## Fixtures (also the checker's regression suite)

`fixtures/terraform-policy/` holds **intentionally-shaped** Terraform used by
`scripts/test_validate_terraform_policy.py`:

- `bad-http-forward/` — internet-facing ALB that forwards plaintext HTTP → MUST `POLICY_FAIL`.
- `internal-alb-only/` — internal ALB on HTTP → MUST `POLICY_OK` (HTTP allowed internally).
- `good-https-redirect/` — the correct pattern → `POLICY_OK`.

These are deliberately non-compliant test data (never deployed). They are excluded from the
repo-wide `checkov` scan via `.checkov.yaml` `skip-path`; do **not** "harden" them — doing so
breaks the tests that assert the failure paths.

## Verification

```bash
# from skills/tf-best-practices/
uv run --python 3.12 --with pytest python -m pytest scripts/test_validate_terraform_policy.py -q
```