-
Notifications
You must be signed in to change notification settings - Fork 353
Add support for git commit verification #3905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
omehegan
wants to merge
6
commits into
feat/git-checkout-features
Choose a base branch
from
owen/SUP-6535
base: feat/git-checkout-features
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0ed99f0
Add support for git commit verification
omehegan 49736dc
Add a flag to agent start
omehegan 179d0fc
Refactor to support the strict vs warn config, and cover a couple mor…
omehegan 3931a2c
Linting
omehegan 4ccd274
Skip if a custom refspec is used
omehegan 7904808
Move commit verification into its own files for cleanliness
omehegan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| package job | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| "github.com/buildkite/agent/v3/internal/shell" | ||
| ) | ||
|
|
||
| // ErrCommitVerificationFailed indicates that git has definitively determined | ||
| // the commit is not on the specified branch. This is the security-relevant case. | ||
| var ErrCommitVerificationFailed = errors.New("commit verification failed") | ||
|
|
||
| // ErrCommitVerificationUnavailable indicates that git was unable to perform | ||
| // the ancestry check (e.g. due to repo corruption, misconfigured remotes, etc). | ||
| // This is NOT evidence of an attack — it's an infrastructure problem. | ||
| var ErrCommitVerificationUnavailable = errors.New("commit verification unavailable") | ||
|
|
||
| // checkCommitOnBranch performs the actual git ancestry check, handling shallow | ||
| // clones by deepening or unshallowing as needed. It returns: | ||
| // - nil if the commit is verified on the branch | ||
| // - ErrCommitVerificationFailed if the commit is definitively not on the branch | ||
| // - ErrCommitVerificationUnavailable if the check cannot be performed | ||
| func (e *Executor) checkCommitOnBranch(ctx context.Context) error { | ||
| e.shell.Commentf("Verifying commit %q is on branch %q", e.Commit, e.Branch) | ||
|
|
||
| // Try the ancestry check | ||
| err := e.shell.Command("git", "merge-base", "--is-ancestor", e.Commit, e.Branch).Run(ctx) | ||
| exitCode := shell.ExitCode(err) | ||
|
|
||
| switch exitCode { | ||
| case 0: | ||
| return nil // verified! | ||
| case 1: | ||
| return fmt.Errorf("%w: commit %q is not on branch %q", ErrCommitVerificationFailed, e.Commit, e.Branch) | ||
| case 128: | ||
| // We might have a shallow clone, try to deepen or unshallow to find the commit | ||
| output, _ := e.shell.Command("git", "rev-parse", "--is-shallow-repository").RunAndCaptureStdout(ctx) | ||
|
|
||
| if strings.TrimSpace(output) != "true" { | ||
| // Not shallow — this is a genuine error | ||
| return fmt.Errorf("%w: unable to verify commit %q on branch %q: %w", ErrCommitVerificationUnavailable, e.Commit, e.Branch, err) | ||
| } | ||
|
|
||
| // Try deepening by 50 commits first | ||
| e.shell.Commentf("Shallow clone detected, deepening by 50 commits...") | ||
| _ = e.shell.Command("git", "fetch", "--deepen=50").Run(ctx) | ||
|
|
||
| retryErr := e.shell.Command("git", "merge-base", "--is-ancestor", e.Commit, e.Branch).Run(ctx) | ||
| retryCode := shell.ExitCode(retryErr) | ||
|
|
||
| if retryCode == 0 { | ||
| return nil // Found a valid commit after deepening | ||
| } | ||
| if retryCode == 1 { | ||
| return fmt.Errorf("%w: commit %q is not on branch %q", ErrCommitVerificationFailed, e.Commit, e.Branch) | ||
| } | ||
|
|
||
| // Still 128 - full unshallow as last resort | ||
| e.shell.Commentf("Deepening insufficient, performing a full unshallow...") | ||
| _ = e.shell.Command("git", "fetch", "--unshallow").Run(ctx) | ||
|
|
||
| retryErr = e.shell.Command("git", "merge-base", "--is-ancestor", e.Commit, e.Branch).Run(ctx) | ||
| retryCode = shell.ExitCode(retryErr) | ||
|
|
||
| if retryCode == 0 { | ||
| return nil // Found a valid commit after unshallowing | ||
| } | ||
| if retryCode == 1 { | ||
| return fmt.Errorf("%w: commit %q is not on branch %q", ErrCommitVerificationFailed, e.Commit, e.Branch) | ||
| } | ||
|
|
||
| return fmt.Errorf("%w: unable to verify commit %q on branch %q after unshallowing: %w", ErrCommitVerificationUnavailable, e.Commit, e.Branch, retryErr) | ||
| default: | ||
| return fmt.Errorf("%w: unable to verify commit %q on branch %q: %w", ErrCommitVerificationUnavailable, e.Commit, e.Branch, err) | ||
| } | ||
| } | ||
|
|
||
| // verifyCommit is called if the user has commit verification enabled. It ensures that the commit we are | ||
| // asked to build exists and is reachable on the branch we are given. | ||
| func (e *Executor) verifyCommit(ctx context.Context) error { | ||
| // Skip if not enabled | ||
| if e.GitCommitVerification == "" { | ||
| return nil | ||
| } | ||
|
|
||
| // Skip if commit is HEAD (nothing to verify) | ||
| if e.Commit == "HEAD" { | ||
| return nil | ||
| } | ||
|
|
||
| // Skip if we haven't been given a branch - e.g. it's a tag push event | ||
| if e.Branch == "" { | ||
| return nil | ||
| } | ||
|
|
||
| // Skip if this is a tag build — tags are not branch-specific | ||
| if e.Tag != "" { | ||
| return nil | ||
| } | ||
|
|
||
| // Skip if this is a PR build — the commit may be on a merge ref, not the target branch | ||
| if e.PullRequest != "" { | ||
| return nil | ||
| } | ||
|
|
||
| // Skip if a custom refspec is set — the fetch may not populate standard branch refs, | ||
| // making ancestry verification unreliable | ||
| if e.RefSpec != "" { | ||
| return nil | ||
| } | ||
|
|
||
| // Perform the verification | ||
| err := e.checkCommitOnBranch(ctx) | ||
|
|
||
| // Verification passed | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| // Handle verification results based on error type and mode. | ||
| // | ||
| // We distinguish between two failure types: | ||
| // - ErrCommitVerificationFailed: git definitively determined the commit is NOT | ||
| // on the branch. This is the security-relevant case (potential branch spoofing). | ||
| // - ErrCommitVerificationUnavailable: git was unable to perform the check at all | ||
| // (repo issues, corrupted refs, etc). This is NOT evidence of an attack. | ||
| // | ||
| // In strict mode, only a definitive failure (ErrCommitVerificationFailed) blocks | ||
| // the build. If verification is unavailable, we warn but allow the build to proceed. | ||
| // This avoids users disabling verification entirely due to infrastructure false | ||
| // positives, which would leave them with no protection at all. | ||
| switch e.GitCommitVerification { | ||
| case "strict": | ||
| if errors.Is(err, ErrCommitVerificationFailed) { | ||
| return err | ||
| } | ||
| // Verification was unavailable — warn but don't block | ||
| e.shell.Warningf("Commit verification unavailable: %v", err) | ||
| return nil | ||
| case "warn": | ||
| e.shell.Warningf("Commit verification failed: %v", err) | ||
| return nil | ||
| default: | ||
| e.shell.Warningf("Unknown git-commit-verification value %q, skipping verification", e.GitCommitVerification) | ||
| return nil | ||
| } | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.