Review: Woodpecker agent secret drift -- blocks safe apply

review-256-2026-03-22 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- present (Bug)
  • [x] Lineage -- present (plan-pal-e-platform, standalone discovered)
  • [x] Repo -- present (forgejo_admin/pal-e-platform)
  • [ ] User Story -- MISSING (substituted with "What Broke" which works for bugs, but the story framing helps agents understand the "who cares")
  • [x] What Broke / Context -- present (describes three-way drift clearly)
  • [x] Repro Steps -- present
  • [x] Expected Behavior -- present
  • [x] Environment -- present
  • [ ] File Targets -- MISSING (ticket describes locations conceptually but does not list specific file paths to modify)
  • [x] Acceptance Criteria -- present (4 criteria)
  • [ ] Test Expectations -- MISSING (no test commands or verification procedures)
  • [ ] Constraints -- MISSING (no patterns to follow, no security considerations documented)
  • [ ] Checklist -- MISSING (no PR opened / tests pass checklist)
  • [x] Related -- present

File Targets

  • [x] terraform/secrets.auto.tfvars -- verified: contains woodpecker_agent_secret at line 12, current value prefix 3e053aaa
  • [x] terraform/variables.tf -- verified: variable "woodpecker_agent_secret" at line 157, marked sensitive
  • [x] terraform/main.tf -- verified: set_sensitive blocks at lines 773-783 inject value into both server.env.WOODPECKER_AGENT_SECRET and agent.env.WOODPECKER_AGENT_SECRET
  • [x] .woodpecker.yaml -- verified: TF_VAR_woodpecker_agent_secret from from_secret: tf_var_woodpecker_agent_secret at lines 70-71 and 159-160
  • [x] Makefile -- verified: woodpecker_agent_secret in TF_SECRET_VARS at line 53
  • [x] salt/pillar/secrets_registry.sls -- verified: documents the secret at line 55 with rotation_days: 180
  • [ ] k8s Secret woodpecker-default-agent-secret -- ISSUE: this k8s Secret is NOT managed by Terraform directly. The Helm chart creates it internally. Ticket should clarify whether the drift is in the Helm-managed secret or a separate manually-created k8s Secret.

Repo Placement

Correct. The fix belongs in forgejo_admin/pal-e-platform. The Woodpecker Helm chart, Terraform variables, and secrets.auto.tfvars all live here. No multi-repo coordination needed.

Dependencies

  • Board item #264 (Issue #140: Secrets pillar validation gate) -- DONE. Related effort that added validation but did not reconcile the agent secret specifically.
  • Board item #101 (Phase 17a: Woodpecker Secrets Hardening) -- DONE. Phase 17a-3 states woodpecker_agent_secret was "already wired via set_sensitive on main" but did NOT verify the three locations are in sync. The phase declared victory without runtime validation.
  • Board item #169 (todo-woodpecker-secrets-terraform) -- DONE. Related cleanup.
  • Board item #188 (Issue #109: Platform cleanup) -- IN_PROGRESS. Parent cleanup umbrella. This ticket is part of that effort.
  • Board item #254 (Issue #135: Harbor unreachable from CI pods) -- TODO. Independent but both block safe tofu apply.
  • No undocumented blockers found.

Acceptance Criteria

Assessment of testability:
  • "Identify which value is currently active and working" -- Testable but requires k8s access (kubectl get secret, kubectl exec). No test command provided.
  • "Reconcile all three locations to a single value" -- Testable but vague. "Reconcile" could mean: update tfvars to match live, update live to match tfvars, or generate a fresh value for all three. The ticket does not specify which direction.
  • "Verify Woodpecker agent auth works after reconciliation" -- Testable but no verification command provided. Should specify: check Woodpecker UI agent connection, run a test pipeline, check logs for auth errors.
  • "Document the secret rotation procedure to prevent recurrence" -- This is a documentation deliverable, not a code change. Should be a separate ticket or at minimum clarify: is this an SOP note in pal-e-docs? An update to the secrets registry? Phase 17a-9 already has a pending SOP (sop-woodpecker-db-migration) that was deferred.

Blast Radius

  • Stale data in ticket: The ticket's table claims secrets.auto.tfvars has prefix 597ea9dc, but the actual file has 3e053aaa. The 597ea9dc value does not exist anywhere in the repo. Either the tfvars was already reconciled or the ticket data was wrong when filed. An agent executing this ticket would be confused by the mismatch.
  • Woodpecker CI secret store: The from_secret: tf_var_woodpecker_agent_secret in .woodpecker.yaml means there is a FOURTH location: the Woodpecker CI secret store itself. The ticket only lists three locations but there are actually four (tfvars, Woodpecker CI secret, k8s Secret, statefulset env). The CI secret store value is what gets injected during pipeline runs.
  • Similar drift risk: 17 other secrets flow through the same tfvars-to-Woodpecker-to-Helm pipeline. If this drift happened to woodpecker_agent_secret, it could happen to woodpecker_encryption_key, woodpecker_db_password, or any other secret. No audit of sibling secrets is scoped.
  • Downstream effect: A wrong reconciliation direction (overwriting live with a stale value) would break ALL Woodpecker CI pipelines across every repo. This is a platform-wide outage risk.

Recommendation

Three issues must be resolved before this ticket is READY:
  • Update the drift table: The 597ea9dc value in the ticket does not match reality. Re-run the comparison (secrets.auto.tfvars vs kubectl get secret vs kubectl exec ... printenv) and update the table with current values. Add the Woodpecker CI secret store as a fourth location.
  • Add File Targets section: List the specific files and k8s resources the agent should modify. Include: terraform/secrets.auto.tfvars, the Woodpecker CI secret (via MCP or API), and whether a tofu apply or helm upgrade is the reconciliation mechanism.
  • Add Test Expectations: Specify verification commands: kubectl get secret -n woodpecker ... -o jsonpath=..., a test pipeline trigger, and Woodpecker agent connection check.
  • Split AC #4 (documentation): The rotation SOP is a separate deliverable. Either remove it from this ticket's AC and link to Phase 17a-9's pending SOP, or scope it as a follow-up issue.