Skip to content

Commit 11871d9

Browse files
committed
fix: reject duplicate recovery dispatches
1 parent 553d320 commit 11871d9

5 files changed

Lines changed: 507 additions & 29 deletions

File tree

.github/workflows/publish.yml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,31 @@ jobs:
441441
});
442442
}
443443
EOF
444+
- name: Require the only recovery dispatch for this control commit
445+
if: inputs.publish_operation == 'recover-v0.1.1'
446+
env:
447+
CONTROL_COMMIT: ${{ inputs.control_commit }}
448+
CURRENT_RUN_ID: ${{ github.run_id }}
449+
GH_TOKEN: ${{ github.token }}
450+
RECOVERY_DISPATCH_RUNS: ${{ runner.temp }}/publish-recovery-dispatch-runs.json
451+
shell: bash
452+
run: |
453+
set -euo pipefail
454+
gh api --paginate --slurp \
455+
"repos/${GITHUB_REPOSITORY}/actions/workflows/publish.yml/runs?branch=main&event=workflow_dispatch&head_sha=${CONTROL_COMMIT}&per_page=100" \
456+
> "$RECOVERY_DISPATCH_RUNS"
457+
node --input-type=module <<'EOF'
458+
import { readFileSync } from "node:fs";
459+
import { validateUniquePublishRecoveryRun } from "./scripts/release-workflow-validation.mjs";
460+
461+
validateUniquePublishRecoveryRun({
462+
controlCommit: process.env.CONTROL_COMMIT,
463+
currentRunId: Number(process.env.CURRENT_RUN_ID),
464+
responses: JSON.parse(
465+
readFileSync(process.env.RECOVERY_DISPATCH_RUNS, "utf8"),
466+
),
467+
});
468+
EOF
444469
- name: Freeze the Release Please run set
445470
id: release-please-snapshot
446471
env:
@@ -875,6 +900,7 @@ jobs:
875900
env:
876901
CONTROL_COMMIT: ${{ needs.verify.outputs.control-commit }}
877902
CONTROL_VALIDATOR: ${{ runner.temp }}/release-workflow-validation.mjs
903+
CURRENT_RUN_ID: ${{ github.run_id }}
878904
ENVIRONMENT_FILE: ${{ runner.temp }}/npm-environment.json
879905
EVENT_REF: ${{ github.ref }}
880906
EVENT_SHA: ${{ github.sha }}
@@ -883,6 +909,7 @@ jobs:
883909
POLICIES_FILE: ${{ runner.temp }}/npm-deployment-policies.json
884910
RECOVERY_ANNOTATIONS: ${{ runner.temp }}/publish-recovery-annotations-before-publish.json
885911
RECOVERY_ARTIFACTS: ${{ runner.temp }}/publish-recovery-artifacts-before-publish.json
912+
RECOVERY_DISPATCH_RUNS: ${{ runner.temp }}/publish-recovery-dispatch-runs-before-publish.json
886913
RECOVERY_JOBS: ${{ runner.temp }}/publish-recovery-jobs-before-publish.json
887914
RECOVERY_LIVE_LOG: ${{ runner.temp }}/publish-recovery-live-before-publish.log
888915
RECOVERY_POLICY_ID: ${{ inputs.recovery_policy_id }}
@@ -1127,6 +1154,27 @@ jobs:
11271154
version,
11281155
});
11291156
EOF
1157+
1158+
if [[ "$OPERATION" == "recover-v0.1.1" ]]; then
1159+
gh api --paginate --slurp \
1160+
"repos/${GITHUB_REPOSITORY}/actions/workflows/publish.yml/runs?branch=main&event=workflow_dispatch&head_sha=${CONTROL_COMMIT}&per_page=100" \
1161+
> "$RECOVERY_DISPATCH_RUNS"
1162+
node --input-type=module <<'EOF'
1163+
import { readFileSync } from "node:fs";
1164+
import { pathToFileURL } from "node:url";
1165+
1166+
const { validateUniquePublishRecoveryRun } = await import(
1167+
pathToFileURL(process.env.CONTROL_VALIDATOR)
1168+
);
1169+
validateUniquePublishRecoveryRun({
1170+
controlCommit: process.env.CONTROL_COMMIT,
1171+
currentRunId: Number(process.env.CURRENT_RUN_ID),
1172+
responses: JSON.parse(
1173+
readFileSync(process.env.RECOVERY_DISPATCH_RUNS, "utf8"),
1174+
),
1175+
});
1176+
EOF
1177+
fi
11301178
- name: Publish the exact artifact with provenance
11311179
env:
11321180
DIST_TAG: ${{ needs.verify.outputs.dist-tag }}

RELEASING.md

Lines changed: 68 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,26 @@ npm publication, and environment approvals require authorization from the
9090
current maintainer request. This document defines allowable mechanics but
9191
grants no standing remote-write permission.
9292

93+
## Pull-request review identity
94+
95+
GitHub does not allow a pull-request author to approve that same pull request,
96+
including when the author is an organization or repository administrator. The
97+
repository setting `can_approve_pull_request_reviews=true` authorizes eligible
98+
GitHub Actions workflows to submit approving pull-request reviews; it does not
99+
override this author-self-approval restriction. Before requesting a review,
100+
compare the PR author login with the intended reviewer login.
101+
102+
If they are the same, never ask that reviewer to select the disabled `Approve`
103+
action and never describe a `COMMENTED` review as `APPROVED`. A ruleset that
104+
requires an approving review needs a different human reviewer. When the active
105+
ruleset requires zero approvals and the release procedure asks only for an
106+
owner's exact-head audit record, the author may submit a `Comment` review whose
107+
body names the reviewed commit; verify its `user.login`, `state=COMMENTED`, and
108+
`commit_id` through the reviews API before merge. Action-authored Release Please
109+
PRs remain different: the release workflow requires a formal, exact-head
110+
`APPROVED` review from a human repository administrator whose login differs
111+
from the bot author.
112+
93113
## Candidate verification gate
94114

95115
Run from the repository root on a clean checkout:
@@ -361,8 +381,7 @@ The repository maintains four independently auditable workflows:
361381
'{before_policies: $before_policies[0],
362382
main_policy_id: null,
363383
control_commit: null,
364-
recovery_run_id: null,
365-
before_publish_run_ids: null}' > "$recovery_state"
384+
recovery_run_id: null}' > "$recovery_state"
366385
chmod 600 "$recovery_state"
367386
echo "Recovery state: $recovery_state"
368387
policy_file="$(mktemp)"
@@ -430,7 +449,7 @@ The repository maintains four independently auditable workflows:
430449
```
431450

432451
Set `RELEASE_PLEASE_ENABLED=true`, resolve the current reviewed `main` SHA as
433-
`control_commit`, save the existing Publish run IDs, and dispatch:
452+
`control_commit`, record it, and dispatch:
434453

435454
```bash
436455
set -euo pipefail
@@ -439,14 +458,9 @@ The repository maintains four independently auditable workflows:
439458
"$recovery_state")"
440459
control_commit="$(gh api repos/cometapi-dev/cometapi-node/commits/main --jq '.sha')"
441460
[[ "$control_commit" =~ ^[0-9a-f]{40}$ ]]
442-
before_runs="$(mktemp)"
443-
gh api --paginate --slurp \
444-
'repos/cometapi-dev/cometapi-node/actions/workflows/publish.yml/runs?event=workflow_dispatch&per_page=100' \
445-
| jq '[.[].workflow_runs[].id]' > "$before_runs"
446461
state_next="${recovery_state}.next"
447-
jq --arg control_commit "$control_commit" --slurpfile before_runs "$before_runs" \
448-
'.control_commit = $control_commit |
449-
.before_publish_run_ids = $before_runs[0]' \
462+
jq --arg control_commit "$control_commit" \
463+
'.control_commit = $control_commit' \
450464
"$recovery_state" > "$state_next"
451465
chmod 600 "$state_next"
452466
mv "$state_next" "$recovery_state"
@@ -476,35 +490,50 @@ The repository maintains four independently auditable workflows:
476490
JSON
477491
```
478492
479-
The dispatch endpoint returns `204` without a run ID. Poll and subtract the
480-
pre-dispatch set; never select a run merely because it is the latest:
493+
The dispatch endpoint returns `204` without a run ID. Poll the exact
494+
workflow/ref/SHA identity and require the API's unflattened `total_count` and
495+
returned run set to both equal one; never select a run merely because it is
496+
the latest. This exact filter and count check fail closed if GitHub truncates
497+
a workflow-run search:
481498
482499
```bash
483500
set -euo pipefail
484501
recovery_state="$(git rev-parse --git-path cometapi-v0.1.1-recovery-state.json)"
485502
control_commit="$(jq -er \
486503
'.control_commit | select(type == "string" and test("^[0-9a-f]{40}$"))' \
487504
"$recovery_state")"
488-
before_runs="$(mktemp)"
489-
jq -e '.before_publish_run_ids | type == "array"' "$recovery_state" >/dev/null
490-
jq '.before_publish_run_ids' "$recovery_state" > "$before_runs"
491505
actor="$(gh api user --jq '.login')"
492506
[[ "$actor" == "tensornull" ]]
493507
recovery_run_id=""
494508
for poll in {1..12}; do
495-
after_runs="$(mktemp)"
509+
run_pages="$(mktemp)"
496510
gh api --paginate --slurp \
497-
'repos/cometapi-dev/cometapi-node/actions/workflows/publish.yml/runs?event=workflow_dispatch&per_page=100' \
498-
| jq '[.[].workflow_runs[]]' > "$after_runs"
499-
recovery_run_id="$(jq -r \
500-
--arg actor "$actor" --arg control "$control_commit" \
501-
--slurpfile before "$before_runs" \
502-
'[.[] | select(.id as $id | ($before[0] | index($id) | not)) |
503-
select(.actor.login == $actor and .triggering_actor.login == $actor and
504-
.event == "workflow_dispatch" and .head_branch == "main" and
505-
.head_sha == $control and .run_attempt == 1)] |
506-
if length == 1 then .[0].id else empty end' "$after_runs")"
507-
[[ -n "$recovery_run_id" ]] && break
511+
"repos/cometapi-dev/cometapi-node/actions/workflows/publish.yml/runs?branch=main&event=workflow_dispatch&head_sha=${control_commit}&per_page=100" \
512+
> "$run_pages"
513+
candidate_count="$(jq -er \
514+
'([.[].total_count] | unique) as $counts |
515+
if ($counts | length) == 1 then $counts[0] else error("inconsistent total_count") end' \
516+
"$run_pages")"
517+
if [[ "$candidate_count" -gt 1 ]]; then
518+
echo "Multiple new Publish runs matched the recovery control commit." >&2
519+
exit 1
520+
fi
521+
if [[ "$candidate_count" == "1" ]]; then
522+
jq -e --arg actor "$actor" --arg control "$control_commit" \
523+
'([.[].workflow_runs[]]) as $runs |
524+
([.[].total_count] | unique) == [1] and ($runs | length) == 1 and
525+
$runs[0].head_branch == "main" and $runs[0].head_sha == $control and
526+
$runs[0].name == "Publish" and
527+
$runs[0].path == ".github/workflows/publish.yml" and
528+
$runs[0].repository.full_name == "cometapi-dev/cometapi-node" and
529+
$runs[0].head_repository.full_name == "cometapi-dev/cometapi-node" and
530+
$runs[0].event == "workflow_dispatch" and
531+
$runs[0].run_attempt == 1 and $runs[0].actor.login == $actor and
532+
$runs[0].triggering_actor.login == $actor' \
533+
"$run_pages" >/dev/null
534+
recovery_run_id="$(jq -r '[.[].workflow_runs[]][0].id' "$run_pages")"
535+
break
536+
fi
508537
sleep 5
509538
done
510539
[[ "$recovery_run_id" =~ ^[1-9][0-9]*$ ]]
@@ -575,8 +604,18 @@ The repository maintains four independently auditable workflows:
575604
immediately before registry mutation and fails if a run was created, rerun, or
576605
remains active while the recovery was waiting. This makes the operator freeze
577606
observable rather than relying only on timing.
578-
The one-time recovery accepts only the first workflow attempt; a rerun or a
579-
second dispatch is forbidden even when all other inputs match.
607+
The operator must create only one first-attempt recovery dispatch and must not
608+
rerun or replace it. Both the unprivileged verify job and the protected
609+
publish job read the paginated exact-branch/SHA Publish search, require its
610+
reported total to equal the returned single run, and require that run to be
611+
current. The Actions run API does not expose dispatch inputs, so an unknown
612+
same-commit dispatch is conservatively a collision. GitHub does not offer an atomic
613+
list-runs-and-publish operation: an administrator could create a dispatch
614+
after the final list call. Repository-wide non-cancelling concurrency keeps
615+
that later run behind the current run, and its own unique-run check rejects it
616+
before publication. This guarantees at most one recovery publication attempt,
617+
while the authorized operator protocol—not an impossible server-side
618+
primitive—requires that only one dispatch be created.
580619
581620
If the npm publish request may have reached the registry but its response or
582621
the remaining workflow result was lost, do not infer success or a safe retry

0 commit comments

Comments
 (0)