[Backport] PS-2073: Validate GitHub pull request merge heads

clicommand/bootstrap.go

@@ -59,6 +59,7 @@ type BootstrapConfig struct {
 	Plugins                      string   `cli:"plugins"`
 	Secrets                      string   `cli:"secrets"`
 	PullRequest                  string   `cli:"pullrequest"`
+	PullRequestHeadCommit        string   `cli:"pull-request-head-commit"`
 	PullRequestUsingMergeRefspec bool     `cli:"pull-request-using-merge-refspec"`
 	GitSubmodules                bool     `cli:"git-submodules"`
 	GitLFSEnabled                bool     `cli:"git-lfs-enabled"`
@@ -198,6 +199,12 @@ var BootstrapCommand = cli.Command{
 			Usage:  "The number/id of the pull request this commit belonged to",
 			EnvVar: "BUILDKITE_PULL_REQUEST",
 		},
+		cli.StringFlag{
+			Name:   "pull-request-head-commit",
+			Value:  "",
+			Usage:  "The expected head commit for a pull request build",
+			EnvVar: "BUILDKITE_PULL_REQUEST_HEAD_COMMIT",
+		},
 		cli.BoolFlag{
 			Name:   "pull-request-using-merge-refspec",
 			Usage:  "Whether the agent should attempt to checkout the pull request commit using the merge refspec. This feature is in private preview and requires backend enablement—contact support to enable (default: false)",
@@ -525,6 +532,7 @@ var BootstrapCommand = cli.Command{
 			PluginsAlwaysCloneFresh:      cfg.PluginsAlwaysCloneFresh,
 			PluginsPath:                  cfg.PluginsPath,
 			PullRequest:                  cfg.PullRequest,
+			PullRequestHeadCommit:        cfg.PullRequestHeadCommit,
 			PullRequestUsingMergeRefspec: cfg.PullRequestUsingMergeRefspec,
 			Queue:                        cfg.Queue,
 			RedactedVars:                 cfg.RedactedVars,

internal/job/checkout_fetch.go

@@ -147,6 +147,11 @@ func (e *Executor) fetchSource(ctx context.Context, addBloblessFilter bool, atte
 			}); err != nil {
 				return fmt.Errorf("fetching PR refspec %q: %w", refspecs, err)
 			}
+			if kind == refspecGithubPRMerge && e.PullRequestHeadCommit != "" {
+				if err := e.validateGithubPRMergeHead(ctx); err != nil {
+					return err
+				}
+			}
 		} else {
 			// The build is pinned to an immutable commit, and the canonical
 			// refs/pull/* fetch exists only to obtain its objects, so a
@@ -205,6 +210,52 @@ func (e *Executor) fetchSource(ctx context.Context, addBloblessFilter bool, atte
 	return nil
 }
 
+func (e *Executor) validateGithubPRMergeHead(ctx context.Context) error {
+	commit, err := e.shell.Command("git", "cat-file", "commit", "FETCH_HEAD").RunAndCaptureStdout(
+		ctx,
+		shell.ShowStderr(false),
+	)
+	if err != nil {
+		return &gitError{
+			error: fmt.Errorf("verifying fetched GitHub pull request merge commit has expected head %q: %w", e.PullRequestHeadCommit, err),
+			Type:  gitErrorFetch,
+		}
+	}
+
+	actualHead, ok := commitSecondParent(commit)
+	if !ok {
+		return &gitError{
+			error: fmt.Errorf("verifying fetched GitHub pull request merge commit has expected head %q: fetched commit has fewer than two parents", e.PullRequestHeadCommit),
+			Type:  gitErrorFetch,
+		}
+	}
+
+	if actualHead != e.PullRequestHeadCommit {
+		return &gitError{
+			error: fmt.Errorf("fetched GitHub pull request merge commit does not match the build's pull request head: expected %q, got %q", e.PullRequestHeadCommit, actualHead),
+			Type:  gitErrorFetch,
+		}
+	}
+
+	return nil
+}
+
+func commitSecondParent(commit string) (string, bool) {
+	parents := 0
+	for _, line := range strings.Split(commit, "\n") {
+		if line == "" {
+			break
+		}
+		if parent, ok := strings.CutPrefix(line, "parent "); ok {
+			parents++
+			if parents == 2 {
+				return parent, true
+			}
+		}
+	}
+	return "", false
+}
+
 func isExistingCheckoutRemoteMirrorAttempt(attempt *remoteMirrorAttempt) bool {
 	return attempt != nil &&
 		attempt.site == remoteMirrorSiteExistingCheckout &&

internal/job/checkout_test.go

@@ -125,11 +125,15 @@ func TestDefaultCheckoutPhase(t *testing.T) {
 			executor: &Executor{
 				shell: shell,
 				ExecutorConfig: ExecutorConfig{
-					Commit:        "HEAD",
-					Branch:        "main",
-					CleanCheckout: false,
-					GitCleanFlags: "-f -d -x",
-					RefSpec:       "refs/custom",
+					Commit:                       "HEAD",
+					Branch:                       "main",
+					CleanCheckout:                false,
+					GitCleanFlags:                "-f -d -x",
+					RefSpec:                      "refs/custom",
+					PullRequest:                  "124",
+					PipelineProvider:             "github",
+					PullRequestHeadCommit:        "not-the-head",
+					PullRequestUsingMergeRefspec: true,
 				},
 			},
 			projectName: "project-name-refspec",
@@ -140,12 +144,13 @@ func TestDefaultCheckoutPhase(t *testing.T) {
 			executor: &Executor{
 				shell: shell,
 				ExecutorConfig: ExecutorConfig{
-					PullRequest:      "124",
-					Commit:           "HEAD",
-					Branch:           "main",
-					CleanCheckout:    false,
-					GitCleanFlags:    "-f -d -x",
-					PipelineProvider: "github",
+					PullRequest:           "124",
+					PullRequestHeadCommit: "not-the-head",
+					Commit:                "HEAD",
+					Branch:                "main",
+					CleanCheckout:         false,
+					GitCleanFlags:         "-f -d -x",
+					PipelineProvider:      "github",
 				},
 			},
 			projectName: "project-name-pull-request",
@@ -194,6 +199,7 @@ func TestDefaultCheckoutPhase(t *testing.T) {
 					CleanCheckout:                false,
 					GitCleanFlags:                "-f -d -x",
 					PipelineProvider:             "github",
+					PullRequestHeadCommit:        "not-the-head",
 					PullRequestUsingMergeRefspec: true,
 				},
 			},
@@ -224,6 +230,35 @@ func TestDefaultCheckoutPhase(t *testing.T) {
 	}
 }
 
+func TestCommitSecondParent(t *testing.T) {
+	t.Parallel()
+
+	for _, test := range []struct {
+		name   string
+		commit string
+		want   string
+		ok     bool
+	}{
+		{
+			name:   "merge commit",
+			commit: "tree tree-id\nparent base-id\nparent head-id\nauthor Example\n\nMessage\n",
+			want:   "head-id",
+			ok:     true,
+		},
+		{
+			name:   "non-merge commit",
+			commit: "tree tree-id\nparent base-id\nauthor Example\n\nparent fake-head-in-message\n",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			got, ok := commitSecondParent(test.commit)
+			if got != test.want || ok != test.ok {
+				t.Errorf("commitSecondParent() = (%q, %t), want (%q, %t)", got, ok, test.want, test.ok)
+			}
+		})
+	}
+}
+
 func TestPrepareGitSSHKey(t *testing.T) {
 	t.Parallel()
 

internal/job/config.go

@@ -64,6 +64,10 @@ type ExecutorConfig struct {
 	// If the commit was part of a pull request, this will container the PR number
 	PullRequest string
 
+	// The expected head commit for a pull request build. Intentionally has no
+	// env tag so hooks cannot change which merge commit the agent accepts.
+	PullRequestHeadCommit string
+
 	// Whether the agent should attempt to checkout the pull request commit using the merge refspec
 	PullRequestUsingMergeRefspec bool
 

internal/job/config_test.go

@@ -24,6 +24,7 @@ func TestEnvVarsAreMappedToConfig(t *testing.T) {
 		GitCleanFlags:                "-v",
 		GitSSHKey:                    "original-key",
 		GitRemoteMirrorURL:           "https://mirror.example/original.git",
+		PullRequestHeadCommit:        "original-pull-request-head",
 		AgentName:                    "myAgent",
 		CleanCheckout:                false,
 		PluginsAlwaysCloneFresh:      false,
@@ -40,6 +41,7 @@ func TestEnvVarsAreMappedToConfig(t *testing.T) {
 		"BUILDKITE_CLEAN_CHECKOUT=true",
 		"BUILDKITE_GIT_SSH_KEY=new-key",
 		"BUILDKITE_GIT_REMOTE_MIRROR_URL=https://mirror.example/replaced.git",
+		"BUILDKITE_PULL_REQUEST_HEAD_COMMIT=replaced-pull-request-head",
 		"BUILDKITE_PLUGINS_ALWAYS_CLONE_FRESH=true",
 		"BUILDKITE_GIT_SUBMODULES=true",
 	})
@@ -75,6 +77,9 @@ func TestEnvVarsAreMappedToConfig(t *testing.T) {
 	if got, want := config.GitRemoteMirrorURL, "https://mirror.example/original.git"; got != want {
 		t.Errorf("config.GitRemoteMirrorURL = %q, want immutable %q", got, want)
 	}
+	if got, want := config.PullRequestHeadCommit, "original-pull-request-head"; got != want {
+		t.Errorf("config.PullRequestHeadCommit = %q, want immutable %q", got, want)
+	}
 
 	if got, want := config.CleanCheckout, true; got != want {
 		t.Errorf("config.CleanCheckout = %t, want %t", got, want)