Skip to content

Security: destructive-infrastructure safety gate is bypassable by a PR label triage-level users can apply (CICD-SEC-1 — High) #184

Description

@Kravalg

Severity

Reported: High. Adjusted: High/Medium. The override-by-label-without-attestation is a genuine, unmitigated separation-of-duties/flow-control gap. Worst-case impact is bounded: a triage-only actor cannot author the destructive diff (fork/in-repo-head gate), so exploitation requires a pre-existing write-authored in-repo destructive PR (or collusion) and still traverses environment approvals for prod, which is why the adjusted range dips to Medium; the test up destructive path remains self-service.

OWASP / standard

CICD-SEC-1 (Insufficient Flow Control Mechanisms); CWE-862/CWE-863 (missing/incorrect authorization on a safety override).

Attack scenario

The destructive-diff gate that blocks Pulumi previews which delete/replace critical resources is overridden merely by the presence of the allow-destructive-infra-change label on the PR. Both the PR guardrails workflow and the /pulumi runner stage live PR labels into .artifacts/github-event.json at runtime and then run make test-destructive-diff, so label state is read the moment the job runs.

GitHub's Triage role grants add/remove-label ability with no code, push, merge, or approval rights. A triage-level attacker adds allow-destructive-infra-change to an open in-repo PR, then (chaining the author_association weakness, #181) comments /pulumi prod up. load_destructive_override() returns True and the gate that would have blocked deletion/replacement of KMS keys, S3 state buckets, IAM roles, etc. is silently disabled. Nothing ties the override label to a write/admin reviewer; the code comment even says "Apply the label only after manual review" but no control enforces it.

Evidence

  • scripts/pulumi_ci_guardrails.py:31DESTRUCTIVE_OVERRIDE_LABEL = "allow-destructive-infra-change"
  • scripts/pulumi_ci_guardrails.py:292-295return any(isinstance(label, dict) and label.get("name") == DESTRUCTIVE_OVERRIDE_LABEL for label in labels) (keys solely on label name; no check on who applied it)
  • scripts/pulumi_ci_guardrails.py:533-547_run_destructive_gate returns pass (0) whenever override is True, even with critical delete/replace findings
  • scripts/pulumi_ci_guardrails.py:543 — advisory comment "Apply the label only after manual review" (enforced by nothing)
  • .github/workflows/pulumi-pr-command-runner.yml:258-265 and :579-586 — stage live labels via gh api .../issues/N/labels into github-event.json before make test-destructive-diff
  • .github/workflows/pulumi-pr-guardrails.yml:185-191 — copies the raw event payload (with labels) for the same gate
  • Makefile:273-282test-destructive-diff feeds --event-path .artifacts/github-event.json into pulumi_ci_guardrails.py destructive-gate

Impact

The safety control that forces maintainer sign-off before destroying/replacing stateful, security-critical AWS resources (KMS keys, state buckets, IAM roles) is authorizable by the lowest label-capable role, with no attestation tying the override to a write/admin actor. Combined with #181 this lets a triage-level insider both set the override and fire the apply.

Remediation

  • Do not let a plain PR label authorize destructive changes. Require the override to be attested by a write/admin actor: verify (via the timeline/label-events API) that the label was applied by, or the run approved by, a maintainer.
  • Alternatively move the override to a protected GitHub Environment approval or a maintainer-only slash sub-command instead of a triage-settable label.

Acceptance criteria (5/5)

  • load_destructive_override (or the gate) verifies the override label was applied by an actor with write/maintain/admin permission (via the label-events/timeline API); a unit test asserts a triage-applied label does NOT override.
  • A test asserts the destructive gate still fails on critical delete/replace findings when the label was applied by a non-write actor.
  • The override mechanism is documented as requiring maintainer attestation, and the advisory-only comment is replaced by an enforced control.
  • The test up destructive path is subject to the same attestation requirement as prod (no self-service destructive test applies).
  • CI/structural test verifies the override cannot be satisfied by label presence alone.

Verification notes

Mechanics verified across passes: load_destructive_override keys solely on label name with no applier check; _run_destructive_gate passes on override even with destructive findings; labels are read live at job time. The gate is load-bearing (test_apply depends on the destructive-diff job before make pulumi-up). The trigger linchpin holds via #181 (triage collaborator authorizes as COLLABORATOR / triage org member as MEMBER). The honest constraint: the fork/in-repo-head gate means the destructive change must sit in a write-authored in-repo PR — the triage attacker piggybacks on it rather than authoring it — and prod additionally traverses the prod environment approval, which is why several passes calibrate to Medium. Genuine CICD-SEC-1 override-without-attestation defect regardless.

References

Related: #181 (author_association trigger chain).

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions