-
Epic
-
Resolution: Done
-
Critical
-
None
-
None
-
mitigate-toctou
-
False
-
-
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):
- *JIRA:* https://issues.redhat.com/browse/SRVKP-8020
- *Security Blog:* https://boostsecurity.io/blog/split-second-side-doors-how-bot-delegated-toctou-breaks-the-cicd-threat-model
- *Related Research:* https://github.com/AdnaneKhan/ActionsTOCTOU
- Slack: https://redhat-internal.slack.com/archives/C0382KN768J/p1750954315966259
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?
- is blocked by
-
SRVKP-9316 Testing for the epic
-
- To Do
-
- links to