Story (Required)

As a Repository Maintainer trying to securely approve a pipeline run for an external contribution on GitHub I want to be able to specify the exact commit SHA that /ok-to-test applies to, so that an attacker cannot use a race condition (TOCTOU) to force-push a malicious commit and have it run by PAC.

This story mitigates a TOCTOU (Time-of-Check to Time-of-Use) vulnerability specific to the GitHub provider. Currently, when an admin issues an /ok-to-test, PAC fetches the latest SHA from the pull request. An attacker can force-push a new, malicious SHA in the small window between the comment being posted and PAC processing it. This feature introduces a mechanism to tie the approval directly to a specific commit, preventing this attack.

Background (Required)

A potential TOCTOU (Time-of-Check to Time-of-Use) race condition was identified (and detailed by BoostSecurity) in how PAC handles `/ok-to-test` commands on GitHub.

Unlike GitLab and Bitbucket (which include the commit SHA in the comment event payload), the GitHub `issue_comment` event payload does not include the commit SHA. This forces PAC to make a separate API call to fetch the pull request details, which retrieves the latest commit SHA.

This creates a small window of opportunity for an attacker:
1. Admin reviews a pull request at a safe commit (SHA 'A').
2. Admin posts an `/ok-to-test` comment.
3. An attacker, watching for this comment, force-pushes a malicious commit (SHA 'B') to the same PR.
4. PAC receives the comment webhook, makes an API call to get the PR's SHA, and receives the malicious SHA 'B'.
5. PAC executes the pipeline on the malicious commit 'B', believing it was approved by the admin.

The proposed solution is to add a mode (globally or per-repository) that requires the admin to specify the exact SHA they are approving, e.g., `/ok-to-test <sha>`.

• Source Thread: https://redhat-internal.slack.com/archives/C0382KN768J/p1750954315966259
• Related Blog Post: https://boostsecurity.io/blog/split-second-side-doors-how-bot-delegated-toctou-breaks-the-cicd-threat-model
• Related Internal Issue: https://issues.redhat.com/browse/SRVKP-8020
• Proposed Plan: https://hackmd.io/@chmouel/Byz3Uc1xZx

  Out of scope

• This change only addresses the GitHub provider. GitLab and Bitbucket are not affected and require no changes.
• Implementing a new flow based on GitHub "Review Comments" (`pull_request_review` event) is out of scope for this story.
• Timestamp-based matching (due to its fragility) is not part of this solution.

  Approach (Required)

1. Introduce a new global setting (or a setting in the `Repository` CR) to configure the `/ok-to-test` behavior for GitHub. This could be a policy string, e.g., `github_ok_to_test_policy: "enforce-sha"`.
2. When this policy is not set (or set to a "legacy" default), PAC will maintain its current behavior (fetching the latest SHA) for backward compatibility.
3. When this policy is set to `enforce-sha`:

• PAC will parse `/ok-to-test` comments for a specific commit SHA (e.g., `/ok-to-test 1234567890abcdef...`).
• If the comment is just `/ok-to-test` (without a SHA), PAC will not run a pipeline. Instead, it will post a reply to the PR, instructing the user that a full commit SHA is required (e.g., "Please approve by specifying the full commit SHA: `/ok-to-test <sha>`").
• If the comment includes a SHA, PAC will use that specific SHA to find and run the pipeline, not the latest SHA from the PR.
• PAC should be able to handle both short and long SHAs.

  Dependencies

• None. This is a self-contained change to the PAC GitHub provider logic.

Acceptance Criteria (Mandatory)

• Scenario 1: Legacy Mode
• *Given:* The new SHA enforcement policy is not set.
• *When:* A user comments `/ok-to-test` on a GitHub PR.
• *Then:* PAC runs the pipeline on the latest commit SHA of the PR (current behavior).

• Scenario 2: Enforced Mode - No SHA
• *Given:* The SHA enforcement policy is enabled (`enforce-sha`).
• *When:* A user comments `/ok-to-test` (without a SHA) on a GitHub PR.
• *Then:* PAC does not run a pipeline.
• *And:* PAC posts a comment back to the PR stating that a commit SHA is required.

• Scenario 3: Enforced Mode - With SHA
• *Given:* The SHA enforcement policy is enabled (`enforce-sha`).
• *When:* A user comments `/ok-to-test 1234567890abcdef1234567890abcdef12345678` on a GitHub PR.
• *Then:* PAC runs the pipeline specifically against commit `1234567890abcdef1234567890abcdef12345678`.

• Scenario 4: Enforced Mode - With Short SHA
• *Given:* The SHA enforcement policy is enabled (`enforce-sha`).
• *When:* A user comments `/ok-to-test 1234567` (a short SHA) on a GitHub PR.
• *Then:* PAC successfully resolves the short SHA to the full SHA and runs the pipeline against it.

• Scenario 5: Enforced Mode - Invalid SHA
• *Given:* The SHA enforcement policy is enabled (`enforce-sha`).
• *When:* A user comments `/ok-to-test <invalid_sha>` on a GitHub PR.
• *Then:* PAC does not run a pipeline.
• *And:* PAC posts a comment back to the PR stating the provided SHA could not be found.

INVEST Checklist

( ) Dependencies identified

( ) Blockers noted and expected delivery timelines set

( ) Design is implementable

( ) Acceptance criteria agreed upon

( ) Story estimated

Legend

( ) Unknown

Verified

Unsatisfied

Done Checklist

( ) * Code is completed, reviewed, documented and checked in
( ) * Unit and integration test automation have been delivered and running cleanly in continuous integration/staging/canary environment
( ) * Continuous Delivery pipeline(s) is able to proceed with new code included
( ) * Customer facing documentation, API docs etc. are produced/updated, reviewed and published
( ) * Acceptance criteria are met