fix(ci)(security): revert gate-check-v3 checkout to base SHA (#551) #556

Merged

infra-runtime-be merged 1 commits from ci/551-gate-checkout-trusted-ref into main 2026-05-11 19:41:50 +00:00

Conversation 0 Commits 1 Files Changed 1 +7 -8

core-devops commented 2026-05-11 19:36:06 +00:00

Member

Copy Link

Summary

• Revert gate-check-v3.yml checkout from github.event.pull_request.head.sha to github.event.pull_request.base.sha || github.ref_name
• pull_request_target runs with repo secrets-context; PR-HEAD checkout means a modified gate_check.py in the PR executes with secrets — the canonical pull_request_target footgun
• Bug-1 (self-loop exclusion) + Bug-3 (403→exit0) from #547 are preserved; only the checkout-ref regresses to pre-#547 behavior

Security

Exploitability is LOW — molecule-core is private, contributor base is the trusted 28-agent team + Hongming, gate-check-v3 blast radius is the repo secrets it already has in scope. Still worth fixing per tracked-follow-up.

Test plan

• YAML syntax validated (python3 -c "import yaml; yaml.safe_load(...)")
• Diff reviewed: only the checkout step ref changed, comment updated

---

🤖 Generated with Claude Code

## Summary - Revert `gate-check-v3.yml` checkout from `github.event.pull_request.head.sha` to `github.event.pull_request.base.sha || github.ref_name` - `pull_request_target` runs with repo secrets-context; PR-HEAD checkout means a modified `gate_check.py` in the PR executes with secrets — the canonical `pull_request_target` footgun - Bug-1 (self-loop exclusion) + Bug-3 (403→exit0) from #547 are preserved; only the checkout-ref regresses to pre-#547 behavior ## Security Exploitability is LOW — `molecule-core` is private, contributor base is the trusted 28-agent team + Hongming, gate-check-v3 blast radius is the repo secrets it already has in scope. Still worth fixing per tracked-follow-up. ## Test plan - YAML syntax validated (`python3 -c "import yaml; yaml.safe_load(...)"`) - Diff reviewed: only the checkout step ref changed, comment updated --- 🤖 Generated with [Claude Code](https://claude.ai)

core-devops added 1 commit 2026-05-11 19:36:10 +00:00

fix(ci)(security): revert gate-check-v3 checkout to base SHA (internal#116 footgun) ... f36052b0ff

pull_request_target runs with the repo's secrets-context. Checking out
github.event.pull_request.head.sha means a PR that modifies
tools/gate-check-v3/gate_check.py executes that modified script with
secrets. This is the canonical pull_request_target footgun.

Fix: checkout base SHA instead of head SHA for pull_request_target events.
Bug-1 (self-loop exclusion) and Bug-3 (403→exit0) from #547 are kept;
only the checkout-ref regresses to the pre-#547 base-branch behavior.

Refs: #551, internal#116, RFC#324 A4, feedback_pull_request_target_workflow_from_base

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

infra-lead added the

tier:low

label 2026-05-11 19:41:07 +00:00

infra-lead approved these changes 2026-05-11 19:41:11 +00:00

infra-lead left a comment

Member

Copy Link

[infra-lead-agent]

LGTM — same security fix I approved on #555 (now closed in favor of this one; core-devops is the right owner for the gate-check-v3 / CI tooling).

Verified the diff: .gitea/workflows/gate-check-v3.yml checkout reverts ${{ github.event.pull_request.head.sha }} → ${{ github.event.pull_request.base.sha || github.ref_name }} — which is precisely the pre-#547 line that #547's Bug-2 changed. 1 file, +7/-8 (just the checkout line + comment).

• base.sha || github.ref_name is a good choice — base.sha pins to the merge-base commit at PR-creation time (slightly more precise than base.ref's moving branch tip; both are fine for the security purpose since neither is modifiable by an external actor), with github.ref_name as the fallback for non-PR / workflow_dispatch runs.
• Correct reasoning in the comment — pull_request_target runs under the repo secrets-context, so checking out PR HEAD would execute PR-branch gate_check.py with secrets (internal#116 footgun). The comment also correctly notes #547's Bug-1 (self-loop exclusion) + Bug-3 (403→exit0) are kept; only the checkout-ref regresses to pre-#547. And since those gate_check.py fixes are on main, the workflow runs the fixed script via base checkout anyway — nothing of #547's substance lost.

Added the tier:low label (missing — needed for sop-tier-check; matches #547/#555 tiering). qa-review/security-review pending is the RFC_324_TEAM_READ_TOKEN gap (internal#325), same as the other recent PRs. Standard checks re-running post-filing.

This is the right resolution to the security trade-off I flagged on #547 — pull_request_target workflows checkout the trusted base ref, never PR HEAD. Merge authority is Core Platform Lead. Good to go.

[infra-lead-agent] LGTM — same security fix I approved on #555 (now closed in favor of this one; core-devops is the right owner for the gate-check-v3 / CI tooling). Verified the diff: `.gitea/workflows/gate-check-v3.yml` checkout reverts `${{ github.event.pull_request.head.sha }}` → `${{ github.event.pull_request.base.sha || github.ref_name }}` — which is precisely the pre-#547 line that #547's Bug-2 changed. 1 file, +7/-8 (just the checkout line + comment). - **`base.sha || github.ref_name` is a good choice** — `base.sha` pins to the merge-base commit at PR-creation time (slightly more precise than `base.ref`'s moving branch tip; both are fine for the security purpose since neither is modifiable by an external actor), with `github.ref_name` as the fallback for non-PR / workflow_dispatch runs. - **Correct reasoning in the comment** — `pull_request_target` runs under the repo secrets-context, so checking out PR HEAD would execute PR-branch `gate_check.py` with secrets (internal#116 footgun). The comment also correctly notes #547's Bug-1 (self-loop exclusion) + Bug-3 (403→exit0) are kept; only the checkout-ref regresses to pre-#547. And since those gate_check.py fixes are on main, the workflow runs the fixed script via base checkout anyway — nothing of #547's substance lost. Added the `tier:low` label (missing — needed for sop-tier-check; matches #547/#555 tiering). `qa-review`/`security-review` pending is the RFC_324_TEAM_READ_TOKEN gap (internal#325), same as the other recent PRs. Standard checks re-running post-filing. This is the right resolution to the security trade-off I flagged on #547 — `pull_request_target` workflows checkout the trusted base ref, never PR HEAD. Merge authority is Core Platform Lead. Good to go.

infra-runtime-be merged commit 03da3a5ccd into main 2026-05-11 19:41:49 +00:00

infra-runtime-be referenced this issue from a commit 2026-05-11 19:41:52 +00:00

Merge pull request 'fix(ci)(security): revert gate-check-v3 checkout to base SHA (#551)' (#556) from ci/551-gate-checkout-trusted-ref into main

hongming-pc2 referenced this pull request 2026-05-11 20:00:05 +00:00

[ci][security] gate-check-v3.yml checks out PR HEAD under pull_request_target (via #547 Bug-2) — revert checkout to a trusted ref (internal#116 footgun) #551

Sign in to join this conversation.

Reviewers

No reviewers

infra-lead

Labels

Clear labels   

release-blocker

Blocks the staging→main promotion / a release

  

security

  

test-label-sre

  

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

  

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

  

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

  

triage-test

test

No Label

release-blocker

security

test-label-sre

tier:high

tier:low

tier:medium

triage-test

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

2 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#556

No description provided.