ci: pin security review to the PR head commit - #3425
Open
jakemas wants to merge 1 commit into
Open
Conversation
jakemas
force-pushed
the
ci-security-review-pin-sha
branch
2 times, most recently
from
August 13, 2026 04:39
c26dbae to
83c090b
Compare
The security review passed SOURCE_VERSION="pr/<number>" to
`aws codebuild start-build`. CodeBuild resolves that when it fetches the
source, so it races updates to the pull request: if the PR is pushed to
while a run is queued, CodeBuild can fetch a different commit than the
one that triggered the workflow, while the commit status is attached to
github.event.pull_request.head.sha. The reported result can then describe
a different commit than the one being merged.
Use the reference-and-commit-ID form instead,
refs/pull/<number>/head^{<sha>}. This pins the exact commit while still
naming the ref, so CodeBuild downloads only that ref rather than the
whole repository, which a bare commit ID would require. pr/<number>
already resolves to refs/pull/<number>/head, so the code being reviewed
is unchanged.
jakemas
force-pushed
the
ci-security-review-pin-sha
branch
from
August 13, 2026 04:41
83c090b to
94f7945
Compare
jakemas
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3425 +/- ##
==========================================
- Coverage 78.24% 78.22% -0.02%
==========================================
Files 696 696
Lines 124511 124511
Branches 17279 17280 +1
==========================================
- Hits 97418 97404 -14
- Misses 26168 26183 +15
+ Partials 925 924 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
nhatnghiho
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
security-review.ymlstarts the CodeBuild review withSOURCE_VERSION="pr/<number>". CodeBuild resolves that ref when it fetches the source, so it races pushes to the PR, while the commit status is posted againstgithub.event.pull_request.head.sha. When those disagree a maintainer seessecurity-review / report: successon the commit they are about to merge, for a review of different code. It is the only unpinned reference in the workflow — the checkout (line 60) and the status (line 128) already usehead.sha.Seen on #3277: three runs completed
successfor82871d85d,4d12f7bb6anddcb7a8317, while the report stayed ona5bf56b3and kept listing findings already fixed in the newer commits. The workflow only checks that CodeBuild exited cleanly, never which commit it fetched.Fix: use the reference-and-commit-ID form,
refs/pull/<number>/head^{<sha>}. Per the source version docs a bare commit ID would make CodeBuild "download the entire repository to find the version", while naming the ref means it "downloads only the specified branch" — so this pins the commit without the clone costpr/<number>avoids. It also does not change what is reviewed: the docs give the pull request form asrefs/pull/1/head, sopr/<number>already resolved to the PR head, not a merge commit.Not runnable outside this repo's CI (needs the
SecurityReview-aws-lcproject and OIDC role). Verified offline: the workflow parses, the value renders torefs/pull/3277/head^{dcb7a8317...}and survives the quoted shell expansion, and the otherpull_request.numberuses are correctly per-PR.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.