fix: close workflow issue audit gaps

cigen/render_gitlab.go

@@ -48,8 +48,8 @@ func renderGitLabWorkflow(p *CIPlan) (string, error) {
 
 	// Global variables. Project-level CI/CD variables (secrets) are already
 	// injected into every job's environment by GitLab, so we do NOT re-declare
-	// them as `NAME: $NAME` (a no-op that only obscures the pipeline). Only the
-	// wfctl version pin is a genuine pipeline variable.
+	// the plan-wide union globally. Only the wfctl version pin is a genuine
+	// pipeline variable.
 	b.WriteString("variables:\n")
 	fmt.Fprintf(&b, "  WFCTL_VERSION: %q\n", version)
 	b.WriteString("\n")
@@ -100,8 +100,21 @@ func renderGitLabWorkflow(p *CIPlan) (string, error) {
 			fmt.Fprintf(&b, "    - job: %s\n", prevJob)
 			b.WriteString("      artifacts: false\n")
 		}
+		secrets := p.Secrets
+		if phase.Scoped {
+			secrets = phase.Secrets
+		}
+		if len(secrets) > 0 {
+			b.WriteString("  variables:\n")
+			for _, name := range sortedSecretNames(secrets) {
+				fmt.Fprintf(&b, "    %s: $%s\n", name, name)
+			}
+		}
 		b.WriteString("  script:\n")
 		isLastPhase := i == len(p.Phases)-1
+		if p.PlanGuard {
+			writeGitLabPlanGuard(&b, phase.ConfigPath)
+		}
 		if isLastPhase && p.Migrations != nil {
 			fmt.Fprintf(&b, "    - %s\n", migrationsUpCommand(phase.ConfigPath, p.Migrations.Env))
 		}
@@ -137,3 +150,16 @@ func renderGitLabWorkflow(p *CIPlan) (string, error) {
 
 	return b.String(), nil
 }
+
+// writeGitLabPlanGuard refuses to apply when the plan includes a replace or
+// destroy of a protected resource. The plan output stays in the job log (tee)
+// and a destructive plan fails the job (exit 1) — no `|| true`.
+func writeGitLabPlanGuard(b *strings.Builder, configPath string) {
+	b.WriteString("    - |\n")
+	fmt.Fprintf(b, "      wfctl infra plan --config '%s' | tee plan-guard.txt\n", configPath)
+	b.WriteString("      if grep -Eq -- '^[[:space:]]*(- delete|-/\\+ replace)[[:space:]]' plan-guard.txt || \\\n")
+	b.WriteString("         grep -Eq -- 'Plan: .*([1-9][0-9]* to replace|[1-9][0-9]* to destroy)' plan-guard.txt; then\n")
+	b.WriteString("        echo 'Refusing apply: plan includes replace or destroy of a protected resource.' >&2\n")
+	b.WriteString("        exit 1\n")
+	b.WriteString("      fi\n")
+}

cigen/render_gitlab_test.go

@@ -51,13 +51,14 @@ func TestRenderGitLabCI_NoRedundantSecretVars(t *testing.T) {
 		t.Fatalf("RenderGitLabCI: %v", err)
 	}
 	content := files[".gitlab-ci.yml"]
+	globalVars := gitLabTopLevelBlock(content, "variables")
 
 	// Project-level CI/CD variables (secrets) are auto-injected by GitLab into
-	// every job, so the renderer must NOT re-declare them as `NAME: $NAME`
-	// no-ops in the global variables block.
+	// every job, so the renderer must NOT re-declare the plan-wide union as
+	// `NAME: $NAME` no-ops in the global variables block.
 	for _, s := range plan.Secrets {
 		redundant := "  " + s.Name + ": $" + s.Name
-		if strings.Contains(content, redundant) {
+		if strings.Contains(globalVars, redundant) {
 			t.Errorf("expected no redundant `%s: $%s` declaration in variables block", s.Name, s.Name)
 		}
 	}
@@ -97,6 +98,73 @@ func TestRenderGitLabCI_TwoPhaseNeeds(t *testing.T) {
 	}
 }
 
+func TestRenderGitLabCI_PlanGuardIsRealGate(t *testing.T) {
+	plan := richCIPlan()
+
+	files, err := cigen.RenderGitLabCI(plan)
+	if err != nil {
+		t.Fatalf("RenderGitLabCI: %v", err)
+	}
+	content := files[".gitlab-ci.yml"]
+
+	if !strings.Contains(content, "plan-guard.txt") {
+		t.Fatal("expected a plan guard when PlanGuard is set")
+	}
+	if strings.Contains(content, "|| true") {
+		t.Error("plan guard must not contain `|| true`")
+	}
+	if !strings.Contains(content, "exit 1") {
+		t.Error("plan guard must contain a failing-exit path")
+	}
+	if !strings.Contains(content, "to replace") || !strings.Contains(content, "to destroy") {
+		t.Error("plan guard should detect replace/destroy plans")
+	}
+	if !strings.Contains(content, "tee plan-guard.txt") {
+		t.Error("plan guard should keep plan output visible")
+	}
+	deploy := gitLabJobBlock(content, "apply-deploy")
+	guardIndex := strings.Index(deploy, "plan-guard.txt")
+	migrateIndex := strings.Index(deploy, "wfctl migrations up")
+	if guardIndex < 0 || migrateIndex < 0 {
+		t.Fatalf("expected plan guard and migrations in deploy job\n%s", deploy)
+	}
+	if guardIndex > migrateIndex {
+		t.Fatalf("plan guard must run before migrations\n%s", deploy)
+	}
+}
+
+func TestRenderGitLabCI_ScopedPhase(t *testing.T) {
+	p := &cigen.CIPlan{
+		DefaultBranch: "main",
+		Secrets:       []cigen.SecretRef{{Name: "UNION_ONLY"}},
+		Phases: []cigen.DeployPhase{
+			{Name: "prereq", ConfigPath: "prereq.yaml", Scoped: true, Secrets: []cigen.SecretRef{{Name: "PREREQ_ONLY"}}},
+			{Name: "deploy", ConfigPath: "deploy.yaml", Scoped: true, Secrets: []cigen.SecretRef{{Name: "DEPLOY_ONLY"}}},
+		},
+	}
+
+	files, err := cigen.RenderGitLabCI(p)
+	if err != nil {
+		t.Fatalf("RenderGitLabCI: %v", err)
+	}
+	content := files[".gitlab-ci.yml"]
+	prereq := gitLabJobBlock(content, "apply-prereq")
+	deploy := gitLabJobBlock(content, "apply-deploy")
+
+	if !strings.Contains(prereq, "PREREQ_ONLY") {
+		t.Errorf("prereq job must reference PREREQ_ONLY\n%s", prereq)
+	}
+	if strings.Contains(prereq, "DEPLOY_ONLY") || strings.Contains(prereq, "UNION_ONLY") {
+		t.Errorf("prereq job must not reference other phases' / union secrets\n%s", prereq)
+	}
+	if !strings.Contains(deploy, "DEPLOY_ONLY") {
+		t.Errorf("deploy job must reference DEPLOY_ONLY\n%s", deploy)
+	}
+	if strings.Contains(deploy, "PREREQ_ONLY") || strings.Contains(deploy, "UNION_ONLY") {
+		t.Errorf("deploy job must not reference other phases' / union secrets\n%s", deploy)
+	}
+}
+
 func TestRenderGitLabCI_NilPlan(t *testing.T) {
 	_, err := cigen.RenderGitLabCI(nil)
 	if err == nil {
@@ -117,3 +185,30 @@ func TestRenderGitLabCI_NoDeprecatedOnlySyntax(t *testing.T) {
 		t.Error(".gitlab-ci.yml uses deprecated 'only:' syntax")
 	}
 }
+
+func gitLabJobBlock(content, jobName string) string {
+	start := strings.Index(content, jobName+":\n")
+	if start < 0 {
+		return ""
+	}
+	rest := content[start+len(jobName)+2:]
+	if next := strings.Index(rest, "\napply-"); next >= 0 {
+		return content[start : start+len(jobName)+2+next]
+	}
+	if next := strings.Index(rest, "\nsmoke:"); next >= 0 {
+		return content[start : start+len(jobName)+2+next]
+	}
+	return content[start:]
+}
+
+func gitLabTopLevelBlock(content, name string) string {
+	start := strings.Index(content, name+":\n")
+	if start < 0 {
+		return ""
+	}
+	rest := content[start+len(name)+2:]
+	if next := strings.Index(rest, "\n\n"); next >= 0 {
+		return content[start : start+len(name)+2+next]
+	}
+	return content[start:]
+}

cmd/wfctl/main_test.go

@@ -319,6 +319,115 @@ pipelines:
 	}
 }
 
+func TestRunValidateRejectsConditionalRoutesWithNonStringKeys(t *testing.T) {
+	dir := t.TempDir()
+	cfg := `
+modules:
+  - name: router
+    type: http.router
+pipelines:
+  authz:
+    trigger:
+      type: mock
+    steps:
+      - name: route-by-authz
+        type: step.conditional
+        config:
+          field: authz.allowed
+          routes:
+            true: allow
+            false: deny
+      - name: allow
+        type: step.log
+        config:
+          message: allow
+      - name: deny
+        type: step.log
+        config:
+          message: deny
+`
+	path := writeTestConfig(t, dir, "conditional.yaml", cfg)
+
+	err := runValidate([]string{"--skip-unknown-types", "--allow-no-entry-points", path})
+	if err == nil {
+		t.Fatal("expected validate to fail on non-string conditional route keys")
+	}
+	if !strings.Contains(err.Error(), "step.conditional") ||
+		!strings.Contains(err.Error(), "routes") ||
+		!strings.Contains(err.Error(), "'true'") {
+		t.Fatalf("expected actionable conditional route key error, got: %v", err)
+	}
+}
+
+func TestRunValidateRejectsImportedConditionalRoutesWithNonStringKeys(t *testing.T) {
+	dir := t.TempDir()
+	imported := `
+pipelines:
+  imported:
+    steps:
+      - name: route-by-authz
+        type: step.conditional
+        config:
+          field: authz.allowed
+          routes:
+            true: allow
+            false: deny
+`
+	writeTestConfig(t, dir, "imported.yaml", imported)
+	cfg := `
+imports:
+  - imported.yaml
+modules:
+  - name: router
+    type: http.router
+pipelines: {}
+`
+	path := writeTestConfig(t, dir, "main.yaml", cfg)
+
+	err := runValidate([]string{"--skip-unknown-types", "--allow-no-entry-points", path})
+	if err == nil {
+		t.Fatal("expected validate to fail on imported non-string conditional route keys")
+	}
+	if !strings.Contains(err.Error(), "imported.yaml") ||
+		!strings.Contains(err.Error(), "step.conditional") ||
+		!strings.Contains(err.Error(), "'true'") {
+		t.Fatalf("expected actionable imported conditional route key error, got: %v", err)
+	}
+}
+
+func TestRunValidateRejectsAliasedConditionalRoutesWithNonStringKeys(t *testing.T) {
+	dir := t.TempDir()
+	cfg := `
+shared:
+  routes: &routes
+    true: allow
+    false: deny
+  config: &condition
+    field: authz.allowed
+    routes: *routes
+modules:
+  - name: router
+    type: http.router
+pipelines:
+  authz:
+    steps:
+      - name: route-by-authz
+        type: step.conditional
+        config: *condition
+`
+	path := writeTestConfig(t, dir, "conditional-alias.yaml", cfg)
+
+	err := runValidate([]string{"--skip-unknown-types", "--allow-no-entry-points", path})
+	if err == nil {
+		t.Fatal("expected validate to fail on aliased non-string conditional route keys")
+	}
+	if !strings.Contains(err.Error(), "step.conditional") ||
+		!strings.Contains(err.Error(), "routes") ||
+		!strings.Contains(err.Error(), "'true'") {
+		t.Fatalf("expected actionable aliased conditional route key error, got: %v", err)
+	}
+}
+
 func TestRunValidateMissingArg(t *testing.T) {
 	err := runValidate([]string{})
 	if err == nil {