Skip to content

fix(pr-pipeline): start the main-run review only from pr-require-checks.yml by path (hardening deferred) #10912

Open

Description

@kyle-sexton

Problem

.github/workflows/pr-review-main.yml starts on workflow_run of pr-require-checks and matches that workflow by its name. A pull request's head can add a workflow with the same name:. Its runs then start main-run review runs, and each one spends review model legs.

These runs cannot bind a forged verdict. The binder requires a run of pr-review-main.yml on the default branch, and the reviewed head comes from the gate step's own reads. So the cost is spend only. The independent security review of #10743 rated it P5.

Proposed fix

In the trigger gate step, check the triggering run's path (github.event.workflow_run.path) equals .github/workflows/pr-require-checks.yml, and its head_repository is this repository where that applies. Skip the review otherwise, before any model leg runs.

Acceptance criteria

  • A workflow_run from a workflow named pr-require-checks at any other path starts no review leg.
  • The real pr-require-checks.yml completion still starts the review.

Refs: #10743

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 29, 2026
  2. github-actions commented on Oct 29, 2026

    @github-actions
    Contributor

    Proposed work class: work-class: scoped
    Basis: automated intake triage; the lanes App applied this class.

    Size: Small
    Complexity: Low
    Uncertainty: Known
    Flags: none
    Blocked by: none

    Understood: pr-review-main.yml matches the triggering workflow by name only, so a PR head can add a same-named workflow and start main-run review legs that spend model budget. The issue proposes gating on workflow_run.path (and head repository) in the trigger gate step. The report says it cannot bind a forged verdict, so this is a low-severity spend issue and gets ordinary triage. The acceptance criteria are clear. I found no likely duplicates.

    Automated triage, applied by the lanes App.

  3. added
    priority: mediumReal value, no hard deadline; normal backlog flow.
    and removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 29, 2026
  4. added
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    and removed
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    on Oct 29, 2026
  5. melodic-automation-lanes commented on Oct 29, 2026

    @melodic-automation-lanes
    Contributor

    Not admitted to the issue lane: a criterion names .github/workflows/pr-require-checks.yml, which the issue lane cannot push (patch-protected-path); a builder session takes it. Admission takes the issue once each of those is met, when intake triages it again (.github/actions/resolve-config/admission-rules.json).

  6. melodic-automation-lanes commented on Oct 29, 2026

    @melodic-automation-lanes
    Contributor

    The latest triage verdict holds nothing: admission moved the issue to status: confirmed; its comment says why.

  7. changed the title [-]fix(pr-pipeline): start the main-run review only from pr-require-checks.yml by path[/-] [+]fix(pr-pipeline): start the main-run review only from pr-require-checks.yml by path (hardening deferred)[/+] on Oct 30, 2026
  8. kyle-sexton commented on Oct 30, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded. Hardening deferred, closed as not planned under owner standing direction §10: "Phase: maximum throughput until open issues and open PRs both reach 0. Hardening (re-adding holds, pins, reviews) is deferred to after that, as configuration edits."

    This issue adds a hold, a pin or a check that guards against our own actors, and none of its gaps can leak a token or secret, compromise the owner accounts or bill. §10 counts only those as a §3 boundary. Secret-leak scanning (gitleaks) stays on, as do both review lanes and ci-status (owner Q15).

    Reopen trigger: open issues = 0 and open PRs = 0. The hardening pass finds this with is:issue is:closed reason:"not planned" in:title "hardening deferred".

  9. kyle-sexton commented on Oct 31, 2026

    @kyle-sexton
    MemberAuthor

    Reopening into the After zero milestone. Remaining value: trigger by path, so model legs are not spent needlessly. Closed as deferred; the owner rule of 2026-10-31 keeps work with value open and cordoned.

  10. added this to the After zero milestone on Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: mediumReal value, no hard deadline; normal backlog flow.status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions