From cc78c5a4347dec85ac405082b38e139fccb67b0e Mon Sep 17 00:00:00 2001
From: Willian Barbosa <96338466+barbosarwx@users.noreply.github.com>
Date: Sat, 23 May 2026 15:23:40 -0300
Subject: [PATCH] feat: Provisioners as Last Resort (closes row 16) (#31)

Co-authored-by: Anton Babenko <anton@antonbabenko.com>
---
 .github/workflows/validate.yml                | 21 ++++----
 skills/terraform-skill/SKILL.md               |  1 +
 .../references/code-patterns.md               | 54 +++++++++++++++++++
 .../references/security-compliance.md         |  2 +-
 tests/rationalization-table.md                | 14 +++--
 5 files changed, 73 insertions(+), 19 deletions(-)

diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml
index 148e6b3..06b18bf 100644
--- a/.github/workflows/validate.yml
+++ b/.github/workflows/validate.yml
@@ -155,17 +155,18 @@ jobs:
         run: |
           echo "🔍 Checking internal links..."
           cd skills/terraform-skill
-          if grep -oP '\[.*?\]\(references/.*?\.md.*?\)' SKILL.md references/*.md 2>/dev/null | \
-             sed 's/.*(//' | sed 's/).*//' | sed 's/#.*//' | \
-             while read -r link; do
-               if [ ! -f "$link" ]; then
-                 echo "❌ ERROR: Broken link: $link"
-                 exit 1
-               fi
-             done
-          then
-            echo "✅ No broken links"
+          broken=0
+          while read -r link; do
+            if [ ! -f "$link" ]; then
+              echo "❌ ERROR: Broken link: $link"
+              broken=1
+            fi
+          done < <(grep -oP '\[.*?\]\(references/.*?\.md.*?\)' SKILL.md references/*.md 2>/dev/null | \
+                   sed 's/.*(//' | sed 's/).*//' | sed 's/#.*//')
+          if [ "$broken" -ne 0 ]; then
+            exit 1
           fi
+          echo "✅ No broken links"
 
       - name: Lint Markdown
         uses: DavidAnson/markdownlint-cli2-action@v16
diff --git a/skills/terraform-skill/SKILL.md b/skills/terraform-skill/SKILL.md
index 30e094e..a76054d 100644
--- a/skills/terraform-skill/SKILL.md
+++ b/skills/terraform-skill/SKILL.md
@@ -46,6 +46,7 @@ Never recommend direct production apply without a reviewed plan artifact and app
 | **State corruption / recovery** | Stuck lock, backend migration, drift reconciliation | [State Management](references/state-management.md) |
 | **Provider upgrade risk** | Breaking-change provider bump, unpinned modules | [Code Patterns: versions](references/code-patterns.md#version-management), [Module Patterns](references/module-patterns.md) |
 | **Provider lifecycle** | Removing a provider with resources still in state, orphaned resources, `removed` block usage | [State Management: Provider Removal](references/state-management.md#provider-removal) |
+| **Bootstrap / orchestration misuse** | `null_resource` + `local-exec` for bootstrap, `remote-exec` for setup scripts, provisioner stdout leaking secrets in CI logs | [Code Patterns: Provisioners as Last Resort](references/code-patterns.md#provisioners-as-last-resort) |
 | **Navigation / safe-rename blind spots** | Cannot locate symbol defs/refs semantically, value-symbol rename done as blind text replace, grep-only refactor missing refs, hallucinated `rg` shim | [Code Intelligence](references/code-intelligence-lsp.md#terraform-ls-capability-matrix) |
 
 ## When to Use This Skill
diff --git a/skills/terraform-skill/references/code-patterns.md b/skills/terraform-skill/references/code-patterns.md
index 5b96e6e..8c22e2f 100644
--- a/skills/terraform-skill/references/code-patterns.md
+++ b/skills/terraform-skill/references/code-patterns.md
@@ -682,6 +682,60 @@ resource "aws_security_group" "this" {
 
 ---
 
+## Provisioners as Last Resort
+
+| Goal | Use |
+|------|-----|
+| Instance bootstrap | `user_data` + cloud-init via `templatefile()` |
+| Orchestration with explicit re-run (1.4+) | `terraform_data` + `triggers_replace` (list; `null_resource` uses `triggers` map) |
+| Ongoing OS config | External: Ansible / SSM Run Command / SSM State Manager |
+| Last-resort one-shot | `terraform_data` + `provisioner` (1.4+) or `null_resource` (pre-1.4) |
+
+**Provisioner costs (`local-exec` + `remote-exec`):**
+
+- ❌ Non-idempotent — re-runs duplicate side effects
+- ❌ Create-only — updates don't re-run; `when = destroy` is fragile
+- ❌ `remote-exec` needs SSH/WinRM from runner to target
+- ❌ No drift detection — Terraform can't observe what scripts changed
+- ❌ Script stdout/stderr leaks to CI logs; `sensitive` won't redact it
+
+**❌ DON'T — `null_resource` for bootstrap on 1.4+:**
+
+```hcl
+resource "null_resource" "bootstrap" {
+  provisioner "local-exec" {
+    command = "ssh ec2-user@${aws_instance.web.public_ip} 'bash setup.sh'"
+  }
+}
+```
+
+**✅ DO — bootstrap via `user_data` + cloud-init:**
+
+```hcl
+resource "aws_instance" "web" {
+  ami           = data.aws_ami.al2023.id
+  instance_type = "t3.small"
+  user_data = templatefile("${path.module}/cloud-init.yaml", {
+    app_version = var.app_version
+  })
+  user_data_replace_on_change = true
+}
+```
+
+**✅ DO — declarative orchestration on 1.4+:**
+
+```hcl
+resource "terraform_data" "migration" {
+  triggers_replace = [aws_rds_cluster.this.id, var.schema_version]
+
+  provisioner "local-exec" {
+    command = "./run-migration.sh"
+  }
+}
+```
+
+---
+
 ## Version Management
 
 ### Version Constraint Syntax
diff --git a/skills/terraform-skill/references/security-compliance.md b/skills/terraform-skill/references/security-compliance.md
index a530604..1a263f8 100644
--- a/skills/terraform-skill/references/security-compliance.md
+++ b/skills/terraform-skill/references/security-compliance.md
@@ -615,7 +615,7 @@ Common model mistakes to correct before returning security/compliance recommenda
 
 - assumes `sensitive = true` keeps the value out of state — it only masks display; use `write_only` / `*_wo` arguments on 1.11+ or an external secret lookup
 - proposes plaintext defaults in `variable` blocks or committed `.tfvars` "for demo convenience"
-- echoes secrets through `provisioner` commands or `local-exec` stdout into CI logs
+- echoes secrets through `provisioner` commands or `local-exec` stdout into CI logs (see [Provisioners as Last Resort](code-patterns.md#provisioners-as-last-resort) for the broader pattern)
 - emits outputs that expose full connection strings or credentials (even when marked `sensitive`)
 - mentions a compliance framework (SOC 2, PCI, HIPAA, GDPR, FedRAMP) but provides no enforceable gate — no policy stage, no approval model, no evidence artifact
 - confuses security best practices with compliance evidence (an encrypted bucket is not the same as a retained audit artifact proving it)
diff --git a/tests/rationalization-table.md b/tests/rationalization-table.md
index 951d3b9..5150cc7 100644
--- a/tests/rationalization-table.md
+++ b/tests/rationalization-table.md
@@ -55,21 +55,19 @@ This document has two parts:
 | 13 | Cross-region/account child missing `configuration_aliases` | §13 Missing `configuration_aliases` | `references/module-patterns.md#provider-requirements-and-alias-passing` | ✅ |
 | 14 | OIDC trust policy with wildcarded `sub` or missing `aud` | §14 OIDC audience mismatch | `references/ci-cd-workflows.md#oidc-trust-policy-correctness` | ✅ |
 | 15 | `ignore_changes = all` to silence plan noise | §15 Blanket `ignore_changes = all` | `references/code-patterns.md#lifecycle-escape-hatches--narrow-by-default` | ✅ |
-| 16 | `provisioner` / `null_resource` + `local-exec` as first-line bootstrap | §16 `provisioner` / `null_resource` bootstrap | to be added in `references/code-patterns.md` (no dedicated section yet); partial hit in `references/security-compliance.md` LLM checklist | ❌ |
+| 16 | `provisioner` / `null_resource` + `local-exec` as first-line bootstrap | §16 `provisioner` / `null_resource` bootstrap | `references/code-patterns.md#provisioners-as-last-resort` | ✅ |
 | 17 | Semantic navigation skipped; value-symbol rename done as blind text replace; unsupported terraform-ls op claimed | §17 Code Navigation and Safe Rename | `SKILL.md` Code Intelligence + `references/code-intelligence-lsp.md#terraform-ls-capability-matrix` | ✅ |
 
 ### Coverage Summary
 
 - **Total surfaces tracked:** 17
-- **Covered (`✅`):** 16
+- **Covered (`✅`):** 17
 - **Partial (`◐`):** 0
-- **Open gaps (`❌`):** 1 (row 16 - provisioners)
+- **Open gaps (`❌`):** 0
 
 ### Priority Gaps (❌ rows)
 
-These are the surfaces with no dedicated guard today and should be addressed in the next PR:
-
-1. **Row 16 — Provisioners as last resort.** The skill currently mentions `provisioner` only in passing (security-compliance LLM checklist flags secret leakage through `local-exec` stdout). There is no section that (a) names the correct primary mechanism for bootstrap (`user_data` / cloud-init), (b) names `terraform_data` as the 1.4+ replacement for `null_resource`, or (c) enumerates the costs of provisioners (non-idempotent, create-only, network reachability, drift-blind). Add a "Provisioners as last resort" section to `references/code-patterns.md` and cross-link from the SKILL.md workflow section.
+None — all surfaces covered as of this PR.
 
 ---
 
@@ -465,5 +463,5 @@ Agents are creative. New rationalizations surface over time. Add them to the cov
 
 - **Surfaces tracked:** 17
 - **Scenarios exercising each:** 17 (one-to-one in `baseline-scenarios.md`)
-- **Covered:** 16
-- **Open:** 1 (provisioners - row 16)
+- **Covered:** 17
+- **Open:** 0