Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,9 @@ The selected snapshot establishes one event for applicability, compilation,
group conditions, and provider-check names. An explicit event is never replaced
with live Buildkite fields.

Buildkite's coalesced first-party pull request push maps to the `pull_request`
synchronization described in [Names and triggers](compatibility.md#names-and-triggers).

Linked webhook data can provide `merge_group` and `release`. Those events need
matching Buildkite refs, commits, and activity. Release also needs a valid
payload and a tag matching `BUILDKITE_TAG` and `BUILDKITE_BRANCH`. The GitHub
Expand Down
15 changes: 14 additions & 1 deletion docs/compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,16 @@ Upload selects one effective event, in this order:
1. The GitHub event name accompanying Buildkite's reserved linked-webhook metadata.
1. A Buildkite environment fallback.

The fallback preserves `push`, `pull_request`, `workflow_dispatch`, and
Buildkite's GitHub integration coalesces a first-party pull request
synchronization into the branch's linked push build. When a GitHub `push` build
has a positive Buildkite pull request number, upload selects one `pull_request`
event with `synchronize` activity, the pull request base branch, and
`refs/pull/<number>/head`. It does not also select `push`. A push without pull
request metadata remains a `push`, so push filters retain their normal
semantics.

Except for that coalesced mapping, the fallback preserves `push`,
`pull_request`, `workflow_dispatch`, and
`schedule` from `BUILDKITE_GITHUB_EVENT` across rebuilds. Otherwise:

| Buildkite source | Effective event |
Expand Down Expand Up @@ -252,6 +261,10 @@ commits, synthetic merge, base branch, and workflow file must agree.
The check uses the checkout's existing Git access for public, private, and fork
pull requests. It does not call GitHub or use Buildkite `if_changed`.

A coalesced first-party synchronization has a linked push payload, not a pull
request payload. The push payload cannot admit pull request path filters because
it covers only that push, not the complete pull request comparison.

| Admitted | Rejected |
| --- | --- |
| A matching added, modified, deleted, or type-changed path | No local match |
Expand Down
3 changes: 3 additions & 0 deletions docs/security.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ trusted or grant credentials.
it to the Buildkite repository, commit, workflow, and bounded local Git
history. Missing, shallow, ambiguous, or mismatched evidence blocks
admission.
- A coalesced first-party pull request synchronization can be linked to a push
payload. Buildkite pull request metadata models the trigger; the push payload
cannot grant pull request path-filter admission.
- Release ingestion matches the webhook activity and tag to Buildkite's event,
branch, and tag. The GitHub Code Access App supplies server-resolved commit
provenance. A local `HEAD` fallback preserves compatibility but cannot grant
Expand Down
9 changes: 8 additions & 1 deletion internal/buildkite/triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ func LiveEventPredicate(event string) string {
fallbackEvent += " || (" + unsupportedEvent + "))"
switch event {
case "push":
return "(" + predicate + " || (" + fallbackEvent + ` && build.pull_request.id == null && build.source != "ui" && build.source != "api" && build.source != "schedule"))`
return "((" + predicate + ` && build.pull_request.id == null) || (` + fallbackEvent + ` && build.pull_request.id == null && build.source != "ui" && build.source != "api" && build.source != "schedule"))`
case "pull_request":
return "(" + predicate + " || (" + fallbackEvent + " && build.pull_request.id != null))"
case "workflow_dispatch":
Expand All @@ -360,6 +360,13 @@ func LiveEventPredicate(event string) string {
}
}

// LivePullRequestPushPredicate matches the push delivery that Buildkite's
// GitHub integration uses for a first-party pull request synchronization.
func LivePullRequestPushPredicate() string {
githubEvent := "build.env(" + yamlScalar("BUILDKITE_GITHUB_EVENT") + ")"
return "(" + githubEvent + ` == "push" && build.pull_request.id != null)`
}

func translateTrigger(t workflow.Trigger, expressions TriggerConditionExpressions, snapshot TriggerEventSnapshot, selected bool) (string, bool, error) {
if !SupportedTriggerEvent(t.Event) {
return "", false, &UnsupportedTriggerEventError{Event: t.Event}
Expand Down
17 changes: 17 additions & 0 deletions internal/buildkite/triggers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,23 @@ func TestLiveEventPredicatePreservesNonWebhookMappings(t *testing.T) {
}
}

func TestLivePushPredicatesPartitionPullRequestBuilds(t *testing.T) {
pullRequestPush := LivePullRequestPushPredicate()
for _, want := range []string{
`build.env("BUILDKITE_GITHUB_EVENT") == "push"`,
`build.pull_request.id != null`,
} {
if !strings.Contains(pullRequestPush, want) {
t.Errorf("pull request push predicate missing %q: %s", want, pullRequestPush)
}
}
push := LiveEventPredicate("push")
want := `build.env("BUILDKITE_GITHUB_EVENT") == "push" && build.pull_request.id == null`
if !strings.Contains(push, want) {
t.Fatalf("push predicate does not exclude pull request builds: %s", push)
}
}

func TestTranslateTriggerConditionRejectsUnsafeTriggers(t *testing.T) {
tests := []struct {
name string
Expand Down
57 changes: 38 additions & 19 deletions internal/cli/buildkite_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,21 @@ func buildkiteEventSource(getenv func(string) string) ([]byte, error) {
if githubEvent := strings.TrimSpace(getenv("BUILDKITE_GITHUB_EVENT")); githubEventNamePattern.MatchString(githubEvent) {
switch githubEvent {
case "push", "pull_request", "workflow_dispatch", "schedule":
event = githubEvent
// Rebuilds retain the original GitHub event even though Buildkite reports
// their source as UI. A push may also be associated with an open pull
// request, so restore its authoritative branch or tag ref.
if event == "push" {
if strings.TrimSpace(tag) != "" {
ref = "refs/tags/" + tag
} else if strings.TrimSpace(branch) != "" {
ref = "refs/heads/" + branch
// Buildkite's GitHub integration coalesces a first-party pull request
// synchronization into its linked push build. Preserve the pull request
// compatibility snapshot when the build carries that PR identity.
if githubEvent != "push" || !buildkitePushRepresentsPullRequest(getenv) {
event = githubEvent
// Rebuilds retain the original GitHub event even though Buildkite
// reports their source as UI. Restore a push's branch or tag ref.
if event == "push" {
if strings.TrimSpace(tag) != "" {
ref = "refs/tags/" + tag
} else if strings.TrimSpace(branch) != "" {
ref = "refs/heads/" + branch
}
payload = map[string]any{"ref": ref}
}
payload = map[string]any{"ref": ref}
}
}
}
Expand Down Expand Up @@ -160,17 +164,23 @@ func buildkiteWebhookEventSource(getenv func(string) string, webhook []byte) ([]
if err := decoder.Decode(&snapshot); err != nil {
return nil, fmt.Errorf("decode Buildkite compatibility snapshot: %w", err)
}
compatibilityPayload := snapshot["payload"]
snapshot["payload"] = payload
if event := strings.TrimSpace(getenv("BUILDKITE_GITHUB_EVENT")); githubEventNamePattern.MatchString(event) {
snapshot["event"] = event
// Buildkite can associate a push-created build with an open pull
// request. Keep the authoritative execution ref consistent with the
// linked webhook event rather than retaining refs/pull/<n>/head.
if event == "push" {
if tag := strings.TrimSpace(getenv("BUILDKITE_TAG")); tag != "" {
snapshot["ref"] = "refs/tags/" + tag
} else if branch := strings.TrimSpace(getenv("BUILDKITE_BRANCH")); branch != "" {
snapshot["ref"] = "refs/heads/" + branch
if event == "push" && buildkitePushRepresentsPullRequest(getenv) {
// A push payload describes only the latest branch update, not the
// complete pull request. Retain the trusted compatibility payload and
// do not promote the push webhook to pull request admission evidence.
snapshot["event"] = "pull_request"
snapshot["payload"] = compatibilityPayload
} else {
snapshot["event"] = event
if event == "push" {
if tag := strings.TrimSpace(getenv("BUILDKITE_TAG")); tag != "" {
snapshot["ref"] = "refs/tags/" + tag
} else if branch := strings.TrimSpace(getenv("BUILDKITE_BRANCH")); branch != "" {
snapshot["ref"] = "refs/heads/" + branch
}
}
}
}
Expand Down Expand Up @@ -199,6 +209,15 @@ func buildkiteWebhookEventSource(getenv func(string) string, webhook []byte) ([]
return result, nil
}

func buildkitePushRepresentsPullRequest(getenv func(string) string) bool {
provider, _, _, _, err := parseBuildkiteRepository(getenv("BUILDKITE_REPO"))
if err != nil || provider != "github" || strings.TrimSpace(getenv("BUILDKITE_GITHUB_EVENT")) != "push" {
return false
}
number, err := strconv.Atoi(getenv("BUILDKITE_PULL_REQUEST"))
return err == nil && number > 0
}

func validateBuildkiteMergeGroup(snapshot map[string]any, getenv func(string) string) error {
payload := snapshot["payload"].(map[string]any)
mergeGroup, ok := payload["merge_group"].(map[string]any)
Expand Down
44 changes: 26 additions & 18 deletions internal/cli/buildkite_event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,37 +394,43 @@ func TestBuildkiteEventSourceDoesNotInventReleaseFromEnvironment(t *testing.T) {
}
}

func TestBuildkiteWebhookPushUsesBranchRefForPullRequestAssociatedBuild(t *testing.T) {
func TestBuildkiteWebhookPushUsesPullRequestCompatibilitySnapshot(t *testing.T) {
env := map[string]string{
"BUILDKITE": "true", "BUILDKITE_STEP_KEY": "step",
"BUILDKITE_REPO": "https://github.com/buildkite/buildkite-gha",
"BUILDKITE_COMMIT": strings.Repeat("a", 40),
"BUILDKITE_BRANCH": "feature",
"BUILDKITE_PULL_REQUEST": "42",
"BUILDKITE_GITHUB_EVENT": "push",
}
source, err := buildkiteWebhookEventSource(func(key string) string { return env[key] }, []byte(`{"ref":"refs/heads/feature"}`))
"BUILDKITE_REPO": "https://github.com/buildkite/buildkite-gha",
"BUILDKITE_COMMIT": strings.Repeat("a", 40),
"BUILDKITE_BRANCH": "feature",
"BUILDKITE_PULL_REQUEST": "42",
"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "main",
"BUILDKITE_GITHUB_EVENT": "push",
}
source, err := buildkiteWebhookEventSource(func(key string) string { return env[key] }, []byte(`{"ref":"refs/heads/feature","push_marker":"discarded"}`))
if err != nil {
t.Fatal(err)
}
var snapshot map[string]any
if err := json.Unmarshal(source, &snapshot); err != nil {
t.Fatal(err)
}
if snapshot["event"] != "push" || snapshot["ref"] != "refs/heads/feature" {
t.Fatalf("snapshot event/ref = %q / %q", snapshot["event"], snapshot["ref"])
payload := snapshot["payload"].(map[string]any)
pullRequest := payload["pull_request"].(map[string]any)
if snapshot["event"] != "pull_request" || snapshot["ref"] != "refs/pull/42/head" ||
payload["action"] != "synchronize" || payload["number"] != float64(42) || payload["push_marker"] != nil ||
pullRequest["head"].(map[string]any)["ref"] != "feature" || pullRequest["base"].(map[string]any)["ref"] != "main" {
t.Fatalf("pull request synchronization snapshot = %#v", snapshot)
}
}

func TestBuildkiteEventSourceRebuiltPushResetsPullRequestPayload(t *testing.T) {
func TestBuildkiteEventSourceRebuiltPushPreservesPullRequestCompatibilitySnapshot(t *testing.T) {
env := map[string]string{
"BUILDKITE": "true", "BUILDKITE_STEP_KEY": "step",
"BUILDKITE_REPO": "https://github.com/buildkite/buildkite-gha",
"BUILDKITE_COMMIT": strings.Repeat("a", 40),
"BUILDKITE_BRANCH": "feature",
"BUILDKITE_PULL_REQUEST": "42",
"BUILDKITE_GITHUB_EVENT": "push",
"BUILDKITE_SOURCE": "ui",
"BUILDKITE_REPO": "https://github.com/buildkite/buildkite-gha",
"BUILDKITE_COMMIT": strings.Repeat("a", 40),
"BUILDKITE_BRANCH": "feature",
"BUILDKITE_PULL_REQUEST": "42",
"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "main",
"BUILDKITE_GITHUB_EVENT": "push",
"BUILDKITE_SOURCE": "ui",
}
source, err := buildkiteEventSource(func(key string) string { return env[key] })
if err != nil {
Expand All @@ -435,7 +441,9 @@ func TestBuildkiteEventSourceRebuiltPushResetsPullRequestPayload(t *testing.T) {
t.Fatal(err)
}
payload := snapshot["payload"].(map[string]any)
if snapshot["event"] != "push" || snapshot["ref"] != "refs/heads/feature" || payload["ref"] != "refs/heads/feature" || len(payload) != 1 {
pullRequest := payload["pull_request"].(map[string]any)
if snapshot["event"] != "pull_request" || snapshot["ref"] != "refs/pull/42/head" ||
payload["action"] != "synchronize" || pullRequest["base"].(map[string]any)["ref"] != "main" {
t.Fatalf("rebuilt push snapshot = %#v", snapshot)
}
}
Expand Down
25 changes: 19 additions & 6 deletions internal/cli/effective_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ const maxWebhookMetadataBytes = 25 << 20
type effectiveEventOrigin string

const (
effectiveEventFromPath effectiveEventOrigin = "event-path"
effectiveEventFromWebhook effectiveEventOrigin = "buildkite-webhook"
effectiveEventFromBuild effectiveEventOrigin = "buildkite-environment"
effectiveEventFromPath effectiveEventOrigin = "event-path"
effectiveEventFromWebhook effectiveEventOrigin = "buildkite-webhook"
effectiveEventFromBuild effectiveEventOrigin = "buildkite-environment"
effectiveEventFromPullRequestPush effectiveEventOrigin = "buildkite-pull-request-push"
)

type effectiveEventSelection struct {
Expand All @@ -47,10 +48,18 @@ func loadEffectiveEventSource(ctx context.Context, eventPath string, agent trans
return nil, "", fmt.Errorf("buildkite:webhook exceeds %d bytes", maxWebhookMetadataBytes)
}
source, err := buildkiteWebhookEventSource(os.Getenv, webhook)
return source, effectiveEventFromWebhook, err
origin := effectiveEventFromWebhook
if buildkitePushRepresentsPullRequest(os.Getenv) {
origin = effectiveEventFromPullRequestPush
}
return source, origin, err
case errors.Is(metadataErr, transport.ErrMetadataUnavailable):
source, err := buildkiteEventSource(os.Getenv)
return source, effectiveEventFromBuild, err
origin := effectiveEventFromBuild
if buildkitePushRepresentsPullRequest(os.Getenv) {
origin = effectiveEventFromPullRequestPush
}
return source, origin, err
default:
return nil, "", metadataErr
}
Expand All @@ -70,7 +79,11 @@ func newEffectiveEvent(source []byte, origin effectiveEventOrigin) (effectiveEve
if origin == effectiveEventFromPath {
return effective, nil
}
effective.TriggerExpressions.EventPredicate = buildkitepipeline.LiveEventPredicate(event.Event)
if origin == effectiveEventFromPullRequestPush {
effective.TriggerExpressions.EventPredicate = buildkitepipeline.LivePullRequestPushPredicate()
} else {
effective.TriggerExpressions.EventPredicate = buildkitepipeline.LiveEventPredicate(event.Event)
}
return effective, nil
}

Expand Down
27 changes: 27 additions & 0 deletions internal/cli/effective_event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"os"
"path/filepath"
"reflect"
"strings"
"testing"

buildkitepipeline "github.com/buildkite/buildkite-gha/internal/buildkite"
Expand Down Expand Up @@ -43,3 +44,29 @@ func TestNewEffectiveEventSeparatesExpressionsAndSnapshot(t *testing.T) {
t.Fatalf("webhook effective event = expressions %#v, snapshot %#v", webhook.TriggerExpressions, webhook.TriggerSnapshot)
}
}

func TestNewEffectiveEventUsesPullRequestPushPredicate(t *testing.T) {
env := map[string]string{
"BUILDKITE": "true", "BUILDKITE_STEP_KEY": "importer",
"BUILDKITE_REPO": "https://github.com/acme/widgets",
"BUILDKITE_COMMIT": strings.Repeat("a", 40),
"BUILDKITE_BRANCH": "feature",
"BUILDKITE_PULL_REQUEST": "42",
"BUILDKITE_PULL_REQUEST_BASE_BRANCH": "main",
"BUILDKITE_GITHUB_EVENT": "push",
}
source, err := buildkiteEventSource(func(key string) string { return env[key] })
if err != nil {
t.Fatal(err)
}
effective, err := newEffectiveEvent(source, effectiveEventFromPullRequestPush)
if err != nil {
t.Fatal(err)
}
if effective.Event.Event != "pull_request" || effective.Event.Ref != "refs/pull/42/head" ||
effective.TriggerExpressions.EventPredicate != buildkitepipeline.LivePullRequestPushPredicate() ||
effective.TriggerSnapshot.PullRequestBaseBranch == nil || *effective.TriggerSnapshot.PullRequestBaseBranch != "main" ||
effective.TriggerSnapshot.PullRequestAction == nil || *effective.TriggerSnapshot.PullRequestAction != "synchronize" {
t.Fatalf("pull request push effective event = %#v", effective)
}
}
18 changes: 13 additions & 5 deletions internal/cli/path_filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,13 +440,21 @@ func TestBoundedCommandOutput(t *testing.T) {
}

func TestPopulateChangedPathsRequiresLinkedWebhook(t *testing.T) {
for _, event := range []string{"push", "pull_request"} {
t.Run(event, func(t *testing.T) {
for _, test := range []struct {
name string
event string
origin effectiveEventOrigin
}{
{name: "explicit push", event: "push", origin: effectiveEventFromPath},
{name: "explicit pull request", event: "pull_request", origin: effectiveEventFromPath},
{name: "pull request push", event: "pull_request", origin: effectiveEventFromPullRequestPush},
} {
t.Run(test.name, func(t *testing.T) {
snapshot := buildkitepipeline.TriggerEventSnapshot{}
populateChangedPaths(&snapshot, compiler.Event{Event: event}, effectiveEventFromPath, []workflowInput{{
Triggers: []workflow.Trigger{{Event: event, Paths: []string{"src/**"}}},
populateChangedPaths(&snapshot, compiler.Event{Event: test.event}, test.origin, []workflowInput{{
Triggers: []workflow.Trigger{{Event: test.event, Paths: []string{"src/**"}}},
}})
if snapshot.ChangedPaths.Paths != nil || !strings.Contains(snapshot.ChangedPaths.UnavailableReason, event+" path filters require linked Buildkite webhook") {
if snapshot.ChangedPaths.Paths != nil || !strings.Contains(snapshot.ChangedPaths.UnavailableReason, test.event+" path filters require linked Buildkite webhook") {
t.Fatalf("changed-path snapshot = %#v", snapshot)
}
})
Expand Down
Loading