Worked review 3: a CI pipeline and policy checks

The feature

The prompt was: “Write a GitHub Actions pipeline that runs Terraform plan on pull requests and apply on merge to main, using OIDC instead of stored AWS credentials, with policy checks.” What follows is the kind of first draft an AI assistant hands back for exactly that prompt — plausible YAML, a real OIDC action, a Rego policy that reads like every OPA tutorial. It has 10 real bugs; check the AI bug catalog for the general pattern behind several of them (entries 4, 18, 19, 20, 22). This review complements — and links to — CI/CD for Infrastructure, which shows the corrected pipeline shape in full; this lesson focuses on catching the mistakes in a draft before it gets there. Every claim below was proven with real actionlint, tflint, checkov, and conftest (via docker run --rm against the pinned rhysd/actionlint, ghcr.io/terraform-linters/tflint, bridgecrew/checkov, and openpolicyagent/conftest images), plus real Terraform exit codes and a real terragrunt run --all plan --out-dir.

The generated code

# .github/workflows/terraform.yml, as generated -- fragment, not a collected fence
name: terraform
on:
  pull_request:
  push:
    branches: [main]

jobs:
  plan:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@master
      - uses: hashicorp/setup-terraform@v3
      - name: Assume AWS role via OIDC
        uses: aws-actions/configure-aws-credentials@v6
        with:
          role-to-assume: arn:aws:iam::111122223333:role/github-actions-terraform
          aws-region: us-east-1
      - run: terraform init
      - name: plan
        run: |
          echo "TF_VAR_db_password=$TF_VAR_db_password"
          terraform plan -detailed-exitcode -out=$PLAN_FILE
        env:
          PLAN_FILE: tfplan
          TF_VAR_db_password: ${{ secrets.DB_PASSWORD }}

  apply:
    needs: plan
    if: github.ref == 'refs/heads/main'
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@master
      - uses: hashicorp/setup-terraform@v3
      - run: terraform init
      - run: terraform apply -auto-approve

# ci/gitignore, as generated -- fragment, not a collected fence
.terraform/
.terraform.lock.hcl
*.tfstate
*.tfstate.backup

# terraform.tfvars, as generated -- fragment, committed alongside the workflow above
db_password = "Sup3rSecret!2024"
api_token   = "ghp_1234567890abcdefghijklmnopqrstuvwxyz"

# policy/deny_no_tags.rego, as generated -- fragment, not a collected fence
package main

deny[msg] {
  resource := input.resource_changes[_]
  resource.type == "aws_instance"
  msg := sprintf("%s must set tags", [resource.address])
}

# .tflint.hcl, as generated -- fragment, not a collected fence
plugin "aws" {
  enabled = true
  version = "0.40.0"
  source  = "github.com/terraform-linters/tflint-ruleset-aws"
}

The review walk

Workflow: actions/checkout@master — a mutable branch treated as pinned

Symptom — real actionlint, rhysd/actionlint, one-shot container:

$ docker run --rm -v "$PWD:/repo" -w /repo rhysd/actionlint:latest -color
(no output -- 0 exit code)

🐛 Spot the bug

- uses: actions/checkout@master

Show the answer

Same bug as AI bug catalog entry 19 — and proven fresh here with a sharper distinction. actionlint only requires a @ref token to exist syntactically; removing it entirely (uses: actions/checkout with no @ at all) is a real, caught error — confirmed separately: specifying action "actions/checkout" in invalid format because ref is missing. But @master supplies a syntactically valid ref, so actionlint accepts it identically to a pinned commit SHA — it has no opinion on whether that ref is mutable. Per GitHub’s own hardening guide, only a full-length commit SHA is immutable. Fix: uses: actions/checkout@<40-char-sha> # v5.0.0.

Workflow: a shellcheck issue actionlint genuinely does catch

Symptom — real actionlint against run: terraform plan -detailed-exitcode -out=$PLAN_FILE:

$ docker run --rm -v "$PWD:/repo" -w /repo rhysd/actionlint:latest -color
.github/workflows/terraform.yml:14:9: shellcheck reported issue in this script: SC2086:info:1:40: Double quote to prevent globbing and word splitting [shellcheck]
   |
14 |         run: |
   |         ^~~~

🐛 Spot the bug

run: |
terraform plan -detailed-exitcode -out=$PLAN_FILE

Show the answer

actionlint embeds shellcheck against every multi-line run: script — this is a real, useful catch, unlike the @master entry above. $PLAN_FILE unquoted is subject to word-splitting and glob expansion by the shell; with a plan filename containing a space or a glob character, -out=$PLAN_FILE silently does the wrong thing. Fix: -out="$PLAN_FILE".

Workflow: env: TF_VAR_db_password echoed in a debug line

Symptom — real, env reproducing exactly what a CI runner’s environment looks like once env: sets a secret-backed variable:

$ export TF_VAR_db_password="SuperSecret123"
$ env | grep TF_VAR_db_password
TF_VAR_db_password=SuperSecret123

🐛 Spot the bug

- name: plan
run: |
  echo "TF_VAR_db_password=$TF_VAR_db_password"
  terraform plan -detailed-exitcode -out=$PLAN_FILE
env:
  TF_VAR_db_password: ${{ secrets.DB_PASSWORD }}

Show the answer

GitHub Actions does mask a recognized secret value in raw log output, but that masking is a best-effort string match — proven above, the plain shell fact underneath is that TF_VAR_db_password is a completely ordinary environment variable the moment env: sets it, readable by env, printenv, a crash dump, a debug step, or any tool invoked in the same job that dumps its environment for diagnostics. The echo line here defeats the masking outright by rebuilding the string ("TF_VAR_db_password=$TF_VAR_db_password"), which GitHub’s masking does not always catch. Fix: never echo a secret-backed variable, even for debugging; if terraform itself needs it, let it read the env var directly without printing it anywhere.

apply: no saved plan — terraform apply -auto-approve re-plans from scratch

Symptom — real, side by side. First, apply with no saved plan file:

$ terraform apply -auto-approve
terraform_data.x: Refreshing state...
Plan: 1 to add, 0 to change, 0 to destroy.
Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Then, a real saved plan applied by its file path instead:

$ terraform plan -out=tfplan
$ terraform apply tfplan
Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

🐛 Spot the bug

- run: terraform init
- run: terraform apply -auto-approve

Show the answer

The apply job never downloads the plan the plan job computed and (in this draft) never even names a plan file to save in the first place. Proven above: apply -auto-approve with no plan file prints its own fresh Refreshing state... / Plan: lines — it re-derives a plan from scratch, immediately before applying it, with zero human review of that specific plan. Applying a real saved plan file instead (apply tfplan) prints neither line — it just applies, because there is nothing left to compute. Between the plan job’s review and the apply job’s real run, real infrastructure or configuration can drift, and -auto-approve on a freshly-recomputed plan means nobody ever sees what actually gets applied. Fix: terraform plan -out=tfplan as a build artifact in the plan job, downloaded and applied verbatim (terraform apply tfplan, no -auto-approve needed since a plan file is already an explicit target) in the apply job.

plan: -detailed-exitcode computed, its three-way exit code never actually read

Symptom — real, the same command’s real exit code in two different states:

$ terraform plan -detailed-exitcode   # a resource actually needs to change
$ echo $?
2

$ terraform plan -detailed-exitcode   # re-run immediately after, nothing left to change
$ echo $?
0

🐛 Spot the bug

terraform plan -detailed-exitcode -out=$PLAN_FILE
# (workflow never checks $? here at all)

Show the answer

-detailed-exitcode exists specifically so a script can tell “real changes are ready to review” (2) apart from “a real error occurred” (1) apart from “nothing to do” (0) — proven above, the exact same command returns a genuinely different code depending on whether there is a diff. This draft’s run: block never captures $? at all: because the step’s shell block has a later line after the terraform plan call, a non-zero exit here is silently swallowed inside the run: block unless the script explicitly propagates it — this specific script has no later line, so YAML’s default run: behavior (exit non-zero fails the step) accidentally happens to work, but nothing in the workflow distinguishes exit 2 (a real diff — expected, should post the plan for review) from exit 1 (a real error — should alert loudly, differently). Fix: capture $? explicitly, branch on it (0 → no changes, skip commenting; 1 → fail the job; 2 → real changes, post the plan and proceed), the way this course’s CI/CD for Infrastructure lesson’s pipeline does.

.gitignore: .terraform.lock.hcl gitignored alongside .terraform/

# .gitignore, as generated
.terraform/
.terraform.lock.hcl
*.tfstate
*.tfstate.backup

Same bug as AI bug catalog entry 4 — a generic Terraform .gitignore template ignores both .terraform/ (correct — it’s a real, large, locally-regenerable download cache) and .terraform.lock.hcl (wrong — that file exists specifically to make provider resolution reproducible across every machine and every CI run). Fix:

# .gitignore, fixed
.terraform/
*.tfstate
*.tfstate.backup

terraform.tfvars: a real secret, committed alongside the workflow

# terraform.tfvars, as generated
db_password = "Sup3rSecret!2024"
api_token   = "ghp_1234567890abcdefghijklmnopqrstuvwxyz"

Same bug as AI bug catalog entry 22 — worse in a CI context specifically, because a committed .tfvars means the secret ships to every checkout, every fork’s pull request that can read the repository, and every CI log that happens to cat a file for debugging. This draft’s workflow already demonstrates the correct pattern one step away (env: TF_VAR_db_password: ${{ secrets.DB_PASSWORD }}) — the .tfvars file is a straight-up leftover from local development that was never removed before committing. Fix: delete the file from the working tree and from git history; source db_password/api_token exclusively from TF_VAR_* environment variables backed by CI secrets, never a checked-in file.

policy/deny_no_tags.rego: pre-v1 Rego syntax fails to load

# policy/deny_no_tags.rego, as generated -- @expect-error rego_parse_error: `if` keyword is required before rule body
package main

deny[msg] {
  resource := input.resource_changes[_]
  resource.type == "aws_instance"
  msg := sprintf("%s must set tags", [resource.address])
}

Symptom — real conftest test, current openpolicyagent/conftest, against a real terraform show -json plan:

$ docker run --rm -v "$(pwd)":/project openpolicyagent/conftest test --policy policy plan.json
Error: running test: load: loading policies: load: 2 errors occurred during loading:
policy/deny_no_tags.rego:3: rego_parse_error: `if` keyword is required before rule body
policy/deny_no_tags.rego:3: rego_parse_error: `contains` keyword is required for partial set rules

🐛 Spot the bug

deny[msg] {
resource := input.resource_changes[_]
resource.type == "aws_instance"
msg := sprintf("%s must set tags", [resource.address])
}

Show the answer

Same bug as AI bug catalog entry 20 — the bracket-set rule shape years of Rego tutorials use is exactly what OPA v1 (current conftest’s default) rejects, as a load error, not a rule that silently never fires. Fix:

# policy/deny_no_tags.rego, fixed
package main

import rego.v1

deny contains msg if {
  resource := input.resource_changes[_]
  resource.type == "aws_instance"
  msg := sprintf("%s must set tags", [resource.address])
}

Real, against the identical plan:

$ docker run --rm -v "$(pwd)":/project openpolicyagent/conftest test --policy policy plan.json
FAIL - plan.json - main - aws_instance.app must set tags

1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions

.tflint.hcl: no required_version in the Terraform config the pipeline actually lints

Symptom — real tflint, ghcr.io/terraform-linters/tflint, one-shot container, against the pipeline’s own main.tf:

$ docker run --rm -v "$(pwd)":/data -w /data ghcr.io/terraform-linters/tflint
1 issue(s) found:

Warning: terraform "required_version" attribute is required (terraform_required_version)

  on main.tf line 1:
   1: terraform {

🏗️ A gap in the pipeline, not in the .tflint.hcl file shown

Unlike AI bug catalog entry 21 (an invented rule name that silently checks nothing), this is a rule that fires correctly, for real, against the actual generated main.tf — but nothing in the generated CI workflow ever runs tflint at all. A .tflint.hcl file sitting in the repo, never invoked by any CI step, provides exactly the same protection as no file at all. Fix: add a real tflint step to the plan job, failing the job on any warning (docker run --rm -v "$PWD:/data" -w /data ghcr.io/terraform-linters/tflint, checked exit code), not just checked in as inert configuration.

Policy config present, no checkov step wired into the pipeline either

Symptom — real checkov, bridgecrew/checkov, one-shot container, against the pipeline’s own main.tf (aws_instance with no encryption, no IMDSv2 enforcement, no attached IAM role):

$ docker run --rm -v "$(pwd)":/tf bridgecrew/checkov -d /tf --compact --quiet
Passed checks: 2, Failed checks: 5, Skipped checks: 0

Check: CKV_AWS_135: "Ensure that EC2 is EBS optimized"
FAILED for resource: aws_instance.app
Check: CKV_AWS_126: "Ensure that detailed monitoring is enabled for EC2 instances"
FAILED for resource: aws_instance.app
Check: CKV_AWS_79: "Ensure Instance Metadata Service Version 1 is not enabled"
FAILED for resource: aws_instance.app

Same class of gap as the .tflint.hcl entry above — real, working checks, never actually invoked by the generated workflow. Fix: a checkov step in the plan job, before terraform plan, failing the job on any FAILED check (or an explicit, reviewed skip list — never silence by omission).

run --all plan with no --out-dir at the Terragrunt layer

Symptom — real terragrunt run --all plan --out-dir, a real per-unit plan file for every unit in the tree, and the exact next command printed:

$ terragrunt run --all --non-interactive --no-color --out-dir /tmp/tg-plans -- plan
[live/prod/app] terraform apply "/tmp/tg-plans/live/prod/app/tfplan.tfplan"

❯❯ Run Summary  4 units
   Succeeded    4

$ find /tmp/tg-plans -maxdepth 3
/tmp/tg-plans/live/dev/network
/tmp/tg-plans/live/dev/app
/tmp/tg-plans/live/prod/network
/tmp/tg-plans/live/prod/app

🐛 Spot the bug

terragrunt run --all -- plan
# ... later, in a separate job/step ...
terragrunt run --all -- apply -auto-approve

Show the answer

Exactly the same bug as the plain-Terraform “no -out” entry above, one layer up — run --all plan with no --out-dir computes and discards a plan per unit; the apply job re-plans every unit from scratch. Proven above: --out-dir <dir> writes one real, real .tfplan file per unit, in a directory structure mirroring the live tree, and even tells you the exact terraform apply "<path>" command that would apply it unchanged. Fix: run --all plan --out-dir <dir> in the plan job (uploaded as a CI artifact), run --all apply --out-dir <dir> (the same directory) in the apply job, applying the exact saved plans.

OIDC trust policy scoped too broadly

🏗️ Docs-sourced, not run-proven

No AWS account exists in this course to create a real IAM OIDC provider or role in, so this entry is quoted directly rather than reproduced with a live aws iam call — same convention as CI/CD for Infrastructure’s own OIDC section. Per GitHub’s OIDC-in-AWS guide: “Edit the trust policy, adding the sub field to the validation conditions,” scoped to something like repo:my-org/my-infra:ref:refs/heads/main.

The generated workflow’s role-to-assume: arn:aws:iam::111122223333:role/github-actions-terraform is real and correctly-shaped, but nothing in this lesson’s generated code shows the IAM role’s own trust policy — the part that actually decides which GitHub identities may assume it. A trust policy whose sub condition is missing entirely, or scoped only to the repository without the ref (repo:my-org/my-infra:*), lets any branch — including a pull request from a fork, if the workflow is ever triggered from one — mint a token that can assume this role. Fix: scope the trust policy’s sub condition to the exact branch that runs apply (repo:my-org/my-infra:ref:refs/heads/main), and use a separate, more narrowly-scoped role (or no role at all) for the plan job that runs on pull requests from untrusted forks.

What changed and why

#	Bug	Fix
1	actions/checkout@master — mutable ref, actionlint accepts it identically to a SHA	Pin to a full commit SHA — real actionlint run proved it does not distinguish a branch ref from a SHA
2	-out=$PLAN_FILE unquoted in a run: script	-out="$PLAN_FILE" — real actionlint/shellcheck SC2086 finding
3	Secret-backed TF_VAR_db_password echoed in a debug line	Never echo it — real env output proved it is a plain, readable variable once set
4	apply -auto-approve with no saved plan file	plan -out=tfplan as an artifact, apply tfplan in the apply job — real Refreshing state.../Plan: lines proved a fresh re-plan happens without one
5	-detailed-exitcode computed, its 3-way exit code never branched on	Capture $?, branch on 0/1/2 explicitly — real exit codes 2 (changes) vs 0 (none) proven
6	.terraform.lock.hcl gitignored alongside .terraform/	Only .terraform/ ignored — same mechanics as AI bug catalog entry 4
7	terraform.tfvars with a real secret, committed	Deleted; secrets sourced from TF_VAR_* CI secrets only — same mechanics as AI bug catalog entry 22
8	Pre-v1 Rego (deny[msg] { ... })	import rego.v1 + deny contains msg if { ... } — real conftest load error before, real FAIL after
9	.tflint.hcl/checkov config present, no CI step ever runs either	Real tflint/checkov steps added to the plan job — both tools proven to find real issues against this exact main.tf when actually run
10	No --out-dir on terragrunt run --all plan	--out-dir <dir> in plan, same dir in apply — real per-unit .tfplan files and the exact apply command proven
—	OIDC trust policy’s sub condition missing or too broad	Scope to repo:org/repo:ref:refs/heads/main — docs-sourced, no AWS account in this course

Mental Model

Three of these bugs (8’s load error, the shellcheck finding, and a bare unpinned uses: with no ref at all) are hard, tool-level rejections — conftest/actionlint refuse to proceed. Two config-file bugs (9) are the most dangerous shape in this entire course: real, working tools that would have caught real problems, sitting in the repository completely disconnected from the pipeline that was supposed to run them — proven by running them directly and getting real findings the CI never sees. The rest — an unpinned action, a re-derived plan, an unbranched exit code, a leaked secret, a gitignored lockfile, a committed .tfvars, an under-scoped trust policy — never produce a single error anywhere in the entire toolchain; they are all schema-valid, all individually plausible, and every one of them was proven here by running the actual tool against the actual generated artifact rather than trusting that a policy file’s mere presence means it’s enforced.

Common Misconceptions

• “If a .tflint.hcl/checkov config file exists in the repo, it’s being enforced.” Proven above: both tools, run directly against the pipeline’s own main.tf, found real issues (required_version missing, five failed checkov checks) that a CI run relying only on the config files’ presence — with no actual tflint/checkov step — would never see.
• “actionlint catches unpinned third-party actions.” Proven above, precisely: it requires a @ref to exist syntactically (a bare uses: actions/checkout with no ref is a real, caught error), but does not — and cannot, from the YAML alone — distinguish a mutable branch name like @master from an immutable commit SHA.
• “GitHub’s secret masking means a TF_VAR_* secret can’t leak through CI logs.” Proven above: the moment a secret becomes a plain environment variable, it is readable by any tool that dumps its environment; masking is a best-effort string match on already-known secret values, not a guarantee.

flowchart TD
  Y["workflow YAML"] --> AL["actionlint"]
  AL -->|"1: @master accepted, ref exists"| AL
  AL -->|"2: unquoted $PLAN_FILE, real SC2086"| AL
  Y --> R["terraform plan -detailed-exitcode"]
  R -->|"3: TF_VAR_* echoed"| R
  R -->|"5: exit code 0/1/2 never branched"| R
  R --> A["terraform apply"]
  A -->|"4: no -out, re-plans from scratch"| A
  G[".gitignore / terraform.tfvars"] -->|"6: lockfile ignored"| G
  G -->|"7: secret committed"| G
  P["policy/*.rego"] --> CF["conftest"]
  CF -->|"8: pre-v1 Rego, load error"| CF
  L[".tflint.hcl / checkov config"] -->|"9: never wired into CI"| L
  TG["terragrunt run --all plan"] -->|"10: no --out-dir"| TG
  IAM["IAM role trust policy"] -->|"OIDC sub too broad -- docs"| IAM

Where each bug sits in this pipeline's real toolchain

🏗️ The one-line takeaway

A CI pipeline’s real safety comes from what actually runs, in order, on the actual artifact — not from a policy file existing, a tool being installed, or a flag being present in a command. Every silent bug in this lesson is a gap between “looks wired up” and “is actually invoked and actually checked,” found the same way every time: run the real tool, on the real generated file, and compare its real output to what the pipeline assumes.

actions/checkout@master and a bare uses: actions/checkout with no ref at all are both potential problems. What does real actionlint show for each?

Both are flagged identically as unpinned @master passes with 0 findings (a syntactically valid ref exists, actionlint does not check whether it is mutable); the bare form with no ref fails with a real "ref is missing" error -- actionlint checks that a ref token exists, not what kind it is Neither is flagged -- actionlint does not check uses: refs at all The bare form passes but @master fails

terraform apply -auto-approve runs with no saved plan file from an earlier plan step. What does the real apply output show, compared to applying a saved plan file?

No difference -- apply always behaves identically either way Real "Refreshing state..." and "Plan:" lines appear before the apply, proving it recomputed its own fresh plan; applying a real saved plan file (apply tfplan) shows neither line, since there is nothing left to compute apply refuses to run at all without a saved plan file The saved-plan version is slower because it re-validates the file

The exact same terraform plan -detailed-exitcode command is run twice: once where a resource genuinely needs to change, once immediately after with nothing left to change. What do the real exit codes prove?

Both return 0, since detailed-exitcode only affects output formatting The first returns 2 (real changes present), the second returns 0 (no changes) -- a real, meaningful distinction a script that only checks "exit code != 0" throws away, since it would treat exit 2 the same as a real error (exit 1) The first returns 1, treating any diff as an error Exit codes are randomized and cannot be relied on for branching

A .tflint.hcl and a checkov config both exist in the repository, but the generated GitHub Actions workflow never has a step that runs either tool. Running them directly against the pipeline's own main.tf -- what do they find?

Nothing -- the main.tf is already clean, which is why no CI step was needed Real findings from both: tflint reports a missing required_version, checkov reports 5 real failed checks (EBS optimization, detailed monitoring, IMDSv2, encryption, missing IAM role) -- config files present in a repo provide zero protection until an actual CI step invokes them Both tools refuse to run without being invoked from within a GitHub Actions workflow Only checkov produces findings; tflint requires cloud credentials to run

A workflow step sets env: TF_VAR_db_password: ${{ secrets.DB_PASSWORD }} and a later line does echo "TF_VAR_db_password=$TF_VAR_db_password". Why is GitHub's log masking not a reliable defense here?

Masking only applies to secrets.* references directly in YAML, and it is a best-effort match on the known secret string -- proven that the moment a secret becomes a plain shell environment variable, any command that dumps or rebuilds that string (like this echo) is a real, readable leak path masking cannot guarantee catching Masking works perfectly here, so this pattern is actually safe env: blocks are encrypted separately from secrets.*, so this never reaches the shell at all This only leaks if the workflow is triggered from a fork

Submit

Mark as complete