Skip to content

Commit 47ee286

Browse files
theakshaypantzakisk
authored andcommitted
fix(github): use provided target ref in GetFileInsideRepo
GetFileInsideRepo ignored the caller-supplied target ref and substituted runevent.BaseBranch. This caused OWNERS ACL checks to read from the wrong branch when a PR targets a non-default branch, and remote task fetches to resolve against the PR base instead of the specified ref. Signed-off-by: Akshay Pant <akpant@redhat.com>
1 parent 5c09648 commit 47ee286

2 files changed

Lines changed: 74 additions & 1 deletion

File tree

pkg/provider/github/github.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ func (v *Provider) GetCommitInfo(ctx context.Context, runevent *info.Event) erro
505505
func (v *Provider) GetFileInsideRepo(ctx context.Context, runevent *info.Event, path, target string) (string, error) {
506506
ref := runevent.SHA
507507
if target != "" {
508-
ref = runevent.BaseBranch
508+
ref = target
509509
} else if v.provenance == "default_branch" {
510510
ref = runevent.DefaultBranch
511511
}

pkg/provider/github/github_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,79 @@ func TestGetFileInsideRepo(t *testing.T) {
728728
}
729729
}
730730

731+
func TestGetFileInsideRepoRefSelection(t *testing.T) {
732+
fileContent := base64.StdEncoding.EncodeToString([]byte("valid owners file"))
733+
tests := []struct {
734+
name string
735+
event *info.Event
736+
target string
737+
provenance string
738+
expectedRef string
739+
}{
740+
{
741+
name: "uses SHA when target is empty",
742+
event: &info.Event{
743+
Organization: "org",
744+
Repository: "repo",
745+
SHA: "sha123",
746+
BaseBranch: "main",
747+
DefaultBranch: "main",
748+
},
749+
target: "",
750+
expectedRef: "sha123",
751+
},
752+
{
753+
name: "uses target ref when target is provided",
754+
event: &info.Event{
755+
Organization: "org",
756+
Repository: "repo",
757+
SHA: "sha123",
758+
BaseBranch: "main",
759+
DefaultBranch: "main",
760+
},
761+
target: "refs/heads/release-1.0",
762+
expectedRef: "refs/heads/release-1.0",
763+
},
764+
{
765+
name: "uses DefaultBranch when target is empty and provenance is default_branch",
766+
event: &info.Event{
767+
Organization: "org",
768+
Repository: "repo",
769+
SHA: "sha123",
770+
BaseBranch: "develop",
771+
DefaultBranch: "main",
772+
},
773+
target: "",
774+
provenance: "default_branch",
775+
expectedRef: "main",
776+
},
777+
}
778+
for _, tt := range tests {
779+
t.Run(tt.name, func(t *testing.T) {
780+
ctx, _ := rtesting.SetupFakeContext(t)
781+
fakeclient, mux, _, teardown := ghtesthelper.SetupGH()
782+
defer teardown()
783+
gvcs := Provider{
784+
ghClient: fakeclient,
785+
provenance: tt.provenance,
786+
}
787+
788+
mux.HandleFunc("/repos/org/repo/contents/OWNERS", func(w http.ResponseWriter, r *http.Request) {
789+
gotRef := r.URL.Query().Get("ref")
790+
assert.Equal(t, gotRef, tt.expectedRef)
791+
fmt.Fprintf(w, `{"name": "OWNERS", "path": "OWNERS", "sha": "ownersha"}`)
792+
})
793+
mux.HandleFunc("/repos/org/repo/git/blobs/ownersha", func(w http.ResponseWriter, _ *http.Request) {
794+
fmt.Fprintf(w, `{"content": %q, "sha": "ownersha"}`, fileContent)
795+
})
796+
797+
got, err := gvcs.GetFileInsideRepo(ctx, tt.event, "OWNERS", tt.target)
798+
assert.NilError(t, err)
799+
assert.Equal(t, got, "valid owners file")
800+
})
801+
}
802+
}
803+
731804
func TestCheckSenderOrgMembership(t *testing.T) {
732805
tests := []struct {
733806
name string

0 commit comments

Comments
 (0)