Skip to content

Add support for git commit verification#3905

Open
omehegan wants to merge 6 commits into
mainfrom
owen/SUP-6535
Open

Add support for git commit verification#3905
omehegan wants to merge 6 commits into
mainfrom
owen/SUP-6535

Conversation

@omehegan
Copy link
Copy Markdown
Contributor

@omehegan omehegan commented May 7, 2026

Description

Adds commit-on-branch verification to the checkout phase.

As part of our agent checkout improvement work, we are adding support for git commit verification. This addresses a potential security issue: if an attacker adds a malicious commit to the repo, and is then able to trigger a build which specifies commit: 3v1l5ha123 branch: main we would build that commit as if it were on the main branch, without verifying that this is the case, which could potentially lead to a production deployment of malicious code.

This change introduces a BUILDKITE_GIT_COMMIT_VERIFICATION environment variable, which defaults to false. If set to true, when we are given a commit and branch, before checkout and build, we do the following:

  1. Use git merge-base --is-ancestor to check if the commit really belongs to the specified branch.
  2. If it does, proceed; if it definitely does not, fail and exit.
  3. In ambiguous situations, it's possible that we only have a shallow clone of the repo. In that case we:
  4. Deepen the clone by 50 commits, with the assumption that this should be enough to find the specified SHA.
  5. If we still get an ambiguous result, to a full unshallow and check again.
  6. Proceed if the commit matches, fail and exit if it does not.

In terms of alternatives, the only change I considered was just going straight to unshallow before verifying any commit. But I didn't want users to have to completely give up shallow commits to get verification, so I went with the "deepen first, then unshallow if necessary" approach.

Context

See SUP-6535.

Changes

Adds a BUILDKITE_GIT_COMMIT_VERIFICATION env var to enable to functionality. When set, we perform various git operations in between fetch and checkout, to make sure the provided commit is valid on the provided branch.

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go tool gofumpt -extra -w .)

Disclosures / Credits

I used Claude to help me plan the feature and teach me some Go fundamentals. I coded the core functionality myself, and let Claude write the tests.

@omehegan omehegan requested review from a team as code owners May 7, 2026 06:36
@omehegan omehegan marked this pull request as draft May 7, 2026 06:36
@omehegan omehegan changed the base branch from main to feat/git-checkout-features May 8, 2026 03:37
@omehegan omehegan marked this pull request as ready for review May 14, 2026 04:32
Copy link
Copy Markdown
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the value in this change, but I left some questions re overall directions and the cost / gain asymmetry. Also I recommend relocating some codes to another file.

Comment thread internal/job/checkout.go Outdated
Comment thread internal/job/checkout.go
Comment thread internal/job/checkout.go Outdated
Copy link
Copy Markdown
Contributor

zhming0 commented May 22, 2026

Another thing we raised in the Agent Office hour is the big feature branch approach. We recommend targeting the main branch: 1. to derisk the big bang deployment. 2. V4 is happening in parallel, we worry that you would hit non-trivial conflicts if we hold a long running feature branch. Oz and Ben might be able to share a bit more context, they were on the meeting.

What do you think?

@omehegan
Copy link
Copy Markdown
Contributor Author

@zhming0 yeah I'm catching up on our internal thread about that topic. I was just following the example we were already using for this work, but since it sounds like folks would rather have us merge to main in discrete chunks, I'm fine with that.

@omehegan omehegan changed the base branch from feat/git-checkout-features to main May 25, 2026 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants