Harden /deploy-preview against TOCTOU SHA swaps#5
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e76965a21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const commits = await github.paginate(github.rest.pulls.listCommits, { | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: context.issue.number, | ||
| per_page: 100, |
There was a problem hiding this comment.
Avoid using truncated PR commit list for SHA validation
This validation can reject legitimate deploys on large PRs because pulls.listCommits is capped by GitHub (the endpoint does not provide an unbounded commit history for very large PRs), so commitShas.has(requestedSha) may fail even when the SHA is actually in the PR. In that case maintainers cannot run /deploy-preview <sha> for older commits in large PRs, which is a regression from the intended “any PR commit” pinning behavior.
Useful? React with 👍 / 👎.
Motivation
/deploy-previewslash-command previously fetched the PR head SHA at runtime and used it for deploys, creating a TOCTOU window where a malicious PR author could push new commits after a maintainer approved and cause unreviewed code to run with deploy secrets.Description
/deploy-preview <full-40-char-commit-sha>.checkjob and extract the requested SHA, failing the job when the usage is incorrect.pulls.listCommits(viagithub.paginate) and fail if the SHA is not present.pr.data.head.shaoutput with the validated, pinned SHA so downstreamcheckout,build,deploy, andsmoke-testjobs operate on the exact reviewed commit.Testing
pnpm exec prettier --check .github/workflows/deploy-preview-command.ymlwhich succeeded, confirming workflow formatting.Codex Task