Repository navigation
fix(release): recover committed release artifacts - #7605
Conversation
| if [ "${approved_count}" -lt 1 ]; then | ||
| echo "::error::Release PR #${pr_number} by ${author} has no human approval on final head ${head_sha}. Refusing to publish artifacts." | ||
| merge_authorized=false | ||
| if [ "${author_type}" = "Bot" ] && [ "${merger_type}" = "User" ]; then |
There was a problem hiding this comment.
Medium: The merge-authorization path drops the final-head binding the approval path enforces. The approval branch above requires .commit_id == $head — authorization is bound to the exact head SHA. This branch accepts any human merger of a bot-authored PR (author_type == "Bot" && merger_type == "User") with no equivalent head check. merged_by is not head-bound: with auto-merge, it records the human who enabled auto-merge at an earlier head, not whoever reviewed the commit that actually merged. A commit pushed to the bot PR after enablement then publishes committed dist/** artifacts under a human's name that no human authorized — the exact fail-open the .commit_id == $head binding was added to close. Consider binding this path too (e.g. require the merge commit / final head match what the human acted on, or reject when auto-merge advanced the head after enablement).
There was a problem hiding this comment.
Fixed in 98b6def. The merger path now reads the PR timeline and authorizes only when auto-merge was never enabled or its latest state is explicitly disabled. An active auto-merge state can no longer inherit authorization across a later head update. PR #7508 qualifies because auto-squash was disabled before the final release head was pushed and directly merged.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated path requires human/CODEOWNERS approval.
This PR modifies .github/workflows/release.yml, which matches the deterministic hard approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so the gate is not satisfied. Per decision rule row 2, the outcome must be escalate (no critical/high findings exist to force request-changes).
Escalation reasons
.github/workflows/release.yml(modified) is a gated path; human/CODEOWNERS review withreview_decision: APPROVEDis required before this can merge.
Medium findings (non-blocking, for reviewer attention)
.github/workflows/release.yml— the new bot-authored merge-authorization branch accepts a human merge with no final-head binding, unlike the adjacent.commit_id == $headapproval path, reopening an auto-merge fail-open window.
The workflow_dispatch recovery path is otherwise well-guarded (40-char SHA regex, HEAD equality, supported-branch allowlist, merge-base ancestor check, fail-closed on empty/version-mismatched artifacts) and publishes/tags only the validated RELEASE_COMMIT.
Why human review
- .github/workflows/release.yml (modified) matches gated path
.github/workflows/**and review_decision is REVIEW_REQUIRED (not APPROVED) — human/CODEOWNERS approval required. - This PR touches a path under a hard, non-overridable approval gate (.github/workflows/release.yml (modified) matches
.github/workflows/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
188be34 to
98b6def
Compare
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated path requires human/CODEOWNERS approval.
This PR modifies .github/workflows/release.yml, a file under a hard, non-overridable approval gate (gated_paths: true, matches .github/workflows/**). The current review_decision is unknown/not APPROVED, so decision-table row 2 fires: Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
No blocking findings: the reviewer found no Critical/High/Medium defects. The recovery workflow_dispatch path validates its input fail-closed (40-char SHA regex, HEAD match, supported-branch + ancestry checks), the loosened authorization gate still fails closed against app-merges/auto-merge, artifact detection errors when a recovery target has no committed artifacts, and the changeset-scope exemption cannot launder a protocol edit. Supporting tests are added.
This is a subsequent pass; the prior run was also escalate for the same gated-path reason and had no findings. The escalation persists solely because the release-authorization/security-posture change touches a gated workflow path and no human approval is recorded — not because of any new code defect. A CODEOWNERS/human approval lifts the gate.
Why human review
.github/workflows/release.yml(modified) matches gated path.github/workflows/**and review_decision is not APPROVED — human/CODEOWNERS approval required before merge.- Change alters release authorization / recovery posture on a load-bearing release workflow; ratification of this security-relevant change is a human act.
- This PR touches a path under a hard, non-overridable approval gate (.github/workflows/release.yml (modified) matches
.github/workflows/**) and the current GitHub review decision is 'unknown', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
Summary
RC.4 recovery
After this merges, dispatch the Release workflow on main with release_commit 9497665. The workflow will rerun release verification, recognize the human merge of bot-authored PR #7508, and complete the missing v3.2.0-rc.4 tag, GitHub Release, immutable assets, and downstream release event.
Verification
Open workspace in Conductor