Uploaded image for project: 'OpenShift Pipelines'
  1. OpenShift Pipelines
  2. SRVKP-9315

Mitigate the Time-of-Check to Time-of-Use (TOCTOU) race condition vulnerability in the GitHub `/ok-to-test` workflow.

XMLWordPrintable

    • mitigate-toctou
    • False
    • Hide

      None

      Show
      None
    • False
    • Done
    • 0% To Do, 0% In Progress, 100% Done
    • Implemented require-ok-to-test-sha to enforce commit SHA validation for /ok-to-test commands, mitigating potential CI race conditions.
    • Enhancement
    • Proposed

      Why is this important?
      On GitHub, the `issue_comment` webhook payload does not include the commit SHA. This forces PAC to make a separate API call to fetch the pull request details. A small window exists between an admin's `/ok-to-test` comment and PAC fetching the SHA. An attacker can force-push a malicious commit in this window, potentially tricking PAC into executing untrusted code on the new commit. This vulnerability is being publicly disclosed by security researchers (BoostSecurity). This does not affect GitLab or Bitbucket, as their comment payloads include the commit SHA.

      Scenarios
      1. A repository admin comments `/ok-to-test` on a PR from an external contributor.
      2. In the milliseconds after the comment is posted but before PAC processes it, the contributor force-pushes a new, malicious commit to the same PR.
      3. PAC fetches the PR details, sees the new malicious SHA, and (incorrectly) executes the pipeline against it, believing it was the one approved by the admin.

      Acceptance Criteria (Mandatory)

      • A new mechanism must be implemented to mitigate this TOCTOU vulnerability on GitHub.
      • The primary mitigation will be to support and enforce the use of `/ok-to-test <SHA>`.
      • A new global or `Repository` CR setting must be added to require the commit SHA to be present in the `/ok-to-test` command.
      • When this "strict SHA" mode is enabled:
      • A simple `/ok-to-test` (without a SHA) MUST NOT trigger a pipeline.
      • An `/ok-to-test <SHA>` command MUST trigger a pipeline only for the specified commit SHA.
      • Documentation must be updated to explain the vulnerability and the new configuration setting.

      Dependencies (internal and external)

      • *Internal:* Requires changes to PAC's GitHub provider logic to parse the SHA and enforce the new setting.
      • *External:* The vulnerability itself is a result of limitations in the GitHub `issue_comment` webhook payload, which does not provide the commit SHA.

      Previous Work (Optional):

      Open questions::

      • Should the "strict SHA" setting be global, per `Repository` CR, or both? (The discussion leaned towards a global setting first).
      • Should we investigate alternative mitigations long-term?
      • Using GitHub PR review comments (`pull_request_review` event) instead? This payload does include the `commit_id`, but it's a new event to handle and a different UX.
      • Attempting to match the comment timestamp against the commit's `update_date` from the GitHub API? (Considered potentially fragile).
      • Can we find a GitHub contact to lobby for adding the commit SHA to the `issue_comment` webhook payload for pull requests?

              cboudjna@redhat.com Chmouel Boudjnah
              cboudjna@redhat.com Chmouel Boudjnah
              Akshay Pant Akshay Pant
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Created:
                Updated:
                Resolved: