(MODULES-12062) Gate fork acceptance on the labeling event

CLAUDE.md

@@ -97,8 +97,9 @@ and [example/module/ci.yml](example/module/ci.yml)):
   secrets), gated on an `allowed-for-ci` label that only a CODEOWNER can apply.
 - `fork_ci_label_guard.yml` **strips** that label on `synchronize`/`closed`, so
   every new commit re-requires a fresh CODEOWNER review and re-label. The caller
-  intentionally omits `synchronize` from its acceptance trigger to avoid racing
-  the guard.
+  runs it as a job in `ci.yml`, and gates fork acceptance on the `allowed-for-ci`
+  labeling event itself (never on the label's presence), so a `synchronize`
+  payload that still carries the label can't start acceptance.
 
 When touching anything in this area, preserve the property that **untrusted code
 from a fork can never run with secrets without a CODEOWNER re-approving the

docs/how-to/fork-pr-ci.md

@@ -54,7 +54,7 @@ The acceptance job uses the environment only on fork PRs (see `module_acceptance
 
 ### 5. Update the caller workflows
 
-Replace `.github/workflows/ci.yml` with the two-track pattern (see [example/module/ci.yml](../../example/module/ci.yml)). Add a new file `.github/workflows/fork_ci_label_guard.yml` that strips the label on each new commit (see [example/module/fork_ci_label_guard.yml](../../example/module/fork_ci_label_guard.yml)).
+Replace `.github/workflows/ci.yml` with the two-track pattern (see [example/module/ci.yml](../../example/module/ci.yml)). Its `Fork_CI_Label_Guard` job strips the label on each new commit and on close. Modules that render `ci.yml` from pdk-templates get the same job. Only a caller whose `ci.yml` doesn't include that job needs the separate `.github/workflows/fork_ci_label_guard.yml` (see [example/module/fork_ci_label_guard.yml](../../example/module/fork_ci_label_guard.yml)); don't use both.
 
 Keep "Require approval for first-time contributors" enabled at the org Actions setting — it's an additional pre-execution gate independent of this design.
 
@@ -75,8 +75,8 @@ The Spec track has already run by the time the reviewer sees the PR; review its
 
 - A fork contributor opens a PR. `Spec` runs immediately (public track, no secrets). `Acceptance` does not run.
 - A CODEOWNER reviews the diff, focusing on the files above, and applies `allowed-for-ci`. The `labeled` event triggers the workflow on `pull_request_target`. A single `approval_gate` job (non-matrix) targets the `puppetcore-fork` environment and pauses for environment approval — the approval prompt shows the head SHA being deployed, which the reviewer should compare against the SHA they reviewed before approving. A `@puppetlabs/modules` member approves; `approval_gate` completes; the matrix `acceptance` jobs then run with `PUPPET_FORGE_TOKEN` in scope.
-- If the contributor pushes a new commit, `fork_ci_label_guard.yml` fires on `synchronize` and removes the label. The main caller workflow does **not** include `synchronize` in its `pull_request_target` trigger, so no acceptance run is created for the new commit. To run acceptance against the new code, a CODEOWNER must review again and re-apply the label.
-- If the PR is closed, `fork_ci_label_guard.yml` also fires on `closed` and removes the label. This prevents an attack where a contributor closes the PR, pushes new commits (no events fire on a closed PR), then reopens — without close-strip the label would persist across the reopen and the new commits would inherit the prior authorisation. Side effect: closing a merged PR also strips the label, which is cosmetic (the label is only meaningful for open PRs).
+- If the contributor pushes a new commit, the guard job fires on `synchronize` and removes the label. Acceptance does not run on that event: for fork PRs it requires the `labeled` event for `allowed-for-ci` itself, not just the label's presence (a `synchronize` payload still carries the label until the guard removes it). The `synchronize` run also cancels that PR's in-flight acceptance through the concurrency group. To run acceptance against the new code, a CODEOWNER must review again and re-apply the label.
+- If the PR is closed, the guard job also fires on `closed` and removes the label. This prevents an attack where a contributor closes the PR, pushes new commits (no events fire on a closed PR), then reopens — without close-strip the label would persist across the reopen and the new commits would inherit the prior authorisation. Side effect: closing a merged PR also strips the label, which is cosmetic (the label is only meaningful for open PRs).
 
 ## Verification
 
@@ -104,7 +104,8 @@ This is tracked as a follow-up to the initial pilot. The approval gate on the `p
 
 ## Security considerations
 
-- Do **not** add `synchronize` to the `pull_request_target` types in the caller's main `ci.yml`. Doing so re-races the label-strip workflow and can allow a malicious commit to inherit a prior label's authorization.
+- Gate fork-PR acceptance on the labeling event (`github.event.action == 'labeled' && github.event.label.name == 'allowed-for-ci'`), **never** on `contains(github.event.pull_request.labels.*.name, 'allowed-for-ci')`. A `synchronize` or `reopened` payload still carries a label applied to an earlier commit, so a presence check lets a new, unreviewed commit inherit the prior authorisation. With the event gate, `synchronize` and `closed` can safely trigger `ci.yml` to run the label guard.
+- Key the caller's concurrency group on the PR number, not `github.ref`: on `pull_request_target`, `github.ref` is the base branch, so a shared group lets one PR's events cancel another PR's acceptance.
 - Do **not** pass `secrets: inherit` to the `Spec` track. It runs on `pull_request` (untrusted) and must have no secrets in scope.
 - Do **not** widen the trigger from `pull_request_target` to include user-controlled refs other than the PR head SHA pinned by the reusable workflow.
 - The label name `allowed-for-ci` is a contract; changing it requires updating the reusable workflow's `if:` expressions and the label-guard's input.

example/module/ci.yml

@@ -5,12 +5,12 @@ on:
   pull_request:
     branches:
       - "main"
-  # Trusted private track: acceptance runs with secrets, gated by label for
-  # fork PRs. `synchronize` is intentionally omitted to avoid racing with
-  # fork_ci_label_guard.yml; new commits must be re-labelled by a CODEOWNER
-  # to re-trigger acceptance.
+  # Trusted private track: acceptance runs with secrets, gated for fork PRs on
+  # the `allowed-for-ci` labeling event itself. `synchronize` and `closed` only
+  # run the label guard below, which strips the label so new commits must be
+  # re-labelled by a CODEOWNER to re-trigger acceptance.
   pull_request_target:
-    types: [labeled, reopened]
+    types: [labeled, reopened, synchronize, closed]
     branches:
       - "main"
   workflow_dispatch:
@@ -19,7 +19,11 @@ permissions:
   contents: read
 
 concurrency:
-  group: ${{ github.workflow }}-${{ github.ref }}
+  # On pull_request_target, github.ref is the base branch, so key on the PR number
+  # (and the event) to keep runs of different PRs from cancelling each other. A new
+  # commit or a close cancels that PR's running acceptance; adding any other label
+  # gets its own group so it doesn't.
+  group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}-${{ github.event.action == 'labeled' && github.event.label.name != 'allowed-for-ci' && github.run_id || 'ci' }}
   cancel-in-progress: true
 
 jobs:
@@ -28,11 +32,26 @@ jobs:
     uses: "puppetlabs/cat-github-actions/.github/workflows/module_ci.yml@main"
     # No `secrets: inherit` — public track must not see private secrets.
 
+  # Fork PRs need the `allowed-for-ci` labeling event, not just the label: a
+  # `synchronize` payload still carries the label until the guard strips it.
   Acceptance:
     if: >-
       github.event_name == 'workflow_dispatch' ||
       (github.event_name == 'pull_request_target' &&
-       (github.event.pull_request.head.repo.fork == false ||
-        contains(github.event.pull_request.labels.*.name, 'allowed-for-ci')))
+       ((github.event.pull_request.head.repo.fork == false &&
+         (github.event.action == 'labeled' || github.event.action == 'reopened')) ||
+        (github.event.pull_request.head.repo.fork == true &&
+         github.event.action == 'labeled' &&
+         github.event.label.name == 'allowed-for-ci')))
     uses: "puppetlabs/cat-github-actions/.github/workflows/module_acceptance.yml@main"
     secrets: "inherit"
+
+  # Strips allowed-for-ci on new commits and closes, so each fork-PR acceptance run
+  # needs a fresh label from a CODEOWNER.
+  Fork_CI_Label_Guard:
+    if: >-
+      github.event_name == 'pull_request_target' &&
+      (github.event.action == 'synchronize' || github.event.action == 'closed')
+    uses: "puppetlabs/cat-github-actions/.github/workflows/fork_ci_label_guard.yml@main"
+    permissions:
+      pull-requests: write

example/module/fork_ci_label_guard.yml

@@ -1,5 +1,9 @@
 name: "Fork CI Label Guard"
 
+# Only needed when the caller's ci.yml doesn't run the guard itself. The
+# example ci.yml (and pdk-templates' ci.yml) include it as the
+# Fork_CI_Label_Guard job, so a module using either needs no separate file.
+
 # Strips the `allowed-for-ci` label from a fork PR whenever it is updated in
 # a way that could invalidate the prior CODEOWNER review:
 #   - `synchronize`: a new commit is pushed.