From 751dc5e0383f08d1d6a211c97d0479aedb39726b Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 24 Jun 2026 17:38:00 -0600 Subject: [PATCH 1/3] fix(release): don't let a failed draft lookup mask a found release FetchRelease races a published-release REST lookup against a draft-release GraphQL lookup, and returned the first result unless it was ErrReleaseNotFound. A failing draft lookup, such as a 403 when unauthenticated, could mask a release the published lookup found. Prefer a found release and only error when both fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/release/download/download_test.go | 31 +++++++++++++++++++++++ pkg/cmd/release/shared/fetch.go | 24 +++++++++++++----- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/release/download/download_test.go b/pkg/cmd/release/download/download_test.go index 37c0e3c02fa..549ae62d59b 100644 --- a/pkg/cmd/release/download/download_test.go +++ b/pkg/cmd/release/download/download_test.go @@ -218,6 +218,37 @@ func Test_downloadRun(t *testing.T) { "windows-64bit.zip", }, }, + { + name: "downloads published release when the draft lookup is unauthorized", + isTTY: true, + opts: DownloadOptions{ + TagName: "v1.2.3", + Destination: ".", + Concurrency: 2, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/releases/tags/v1.2.3"), + httpmock.StringResponse(`{ + "assets": [ + { "name": "linux.tgz", "size": 56, "url": "https://api.github.com/assets/5678" } + ] + }`), + ) + // An unauthenticated draft lookup fails over GraphQL and must not + // mask the published release found over REST. + reg.Register( + httpmock.GraphQL(`query RepositoryReleaseByTag\b`), + httpmock.StatusStringResponse(403, `{"message":"This endpoint requires you to be authenticated."}`), + ) + reg.Register(httpmock.REST("GET", "assets/5678"), httpmock.StringResponse(`5678`)) + }, + wantStdout: ``, + wantStderr: ``, + wantFiles: []string{ + "linux.tgz", + }, + }, { name: "download assets matching pattern into destination directory", isTTY: true, diff --git a/pkg/cmd/release/shared/fetch.go b/pkg/cmd/release/shared/fetch.go index f2e370ecf96..420b83b366b 100644 --- a/pkg/cmd/release/shared/fetch.go +++ b/pkg/cmd/release/shared/fetch.go @@ -201,15 +201,27 @@ func FetchRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Inte results <- fetchResult{release: release, error: err} }() - res := <-results - if errors.Is(res.error, ErrReleaseNotFound) { - res = <-results - cancel() // satisfy the linter even though no goroutines are running anymore - } else { + // Prefer a release found by either lookup. A single failed lookup, such as + // the draft lookup when unauthenticated, must not mask a release found by + // the other; only report an error when both lookups fail. + first := <-results + if first.error == nil { cancel() <-results // drain the channel + return first.release, nil } - return res.release, res.error + + second := <-results + cancel() // satisfy the linter even though no goroutines are running anymore + if second.error == nil { + return second.release, nil + } + + // Both lookups failed; prefer reporting the release as not found. + if errors.Is(second.error, ErrReleaseNotFound) { + return nil, second.error + } + return nil, first.error } // FetchLatestRelease finds the latest published release for a repository. From f0a128ad1c33a41b7d1bdc93b5d507c842713d8a Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 24 Jun 2026 17:38:05 -0600 Subject: [PATCH 2/3] feat(release): allow download without authentication Downloading assets from a public repository's release works unauthenticated over REST, so drop the login gate. A token is still used when present. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/release/download/download.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/release/download/download.go b/pkg/cmd/release/download/download.go index b907214125b..e86b36a41d7 100644 --- a/pkg/cmd/release/download/download.go +++ b/pkg/cmd/release/download/download.go @@ -111,6 +111,8 @@ func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobr cmd.Flags().BoolVar(&opts.OverwriteExisting, "clobber", false, "Overwrite existing files of the same name") cmd.Flags().BoolVar(&opts.SkipExisting, "skip-existing", false, "Skip downloading when files of the same name exist") + cmdutil.DisableAuthCheck(cmd) + return cmd } From 7b681a4e8b67d203ccdaab5ff54570bdf6ca3669 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 25 Jun 2026 10:36:40 -0600 Subject: [PATCH 3/3] fix(cmdutil): honor DisableAuthCheck under repo-override parents EnableRepoOverride's hook shadows the root auth gate, then re-runs the nearest ancestor hook to restore it. That re-run passed the ancestor as the command, so the gate judged the wrong node and ignored a leaf's DisableAuthCheck. Pass the invoked leaf instead, as cobra does for every persistent hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmdutil/repo_override.go | 27 +++++++++--- pkg/cmdutil/repo_override_test.go | 70 +++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 pkg/cmdutil/repo_override_test.go diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go index b037859e737..d657e2aecb9 100644 --- a/pkg/cmdutil/repo_override.go +++ b/pkg/cmdutil/repo_override.go @@ -9,11 +9,25 @@ import ( "github.com/spf13/cobra" ) -func executeParentHooks(cmd *cobra.Command, args []string) error { - for cmd.HasParent() { - cmd = cmd.Parent() - if cmd.PersistentPreRunE != nil { - return cmd.PersistentPreRunE(cmd, args) +// executeParentHook re-runs the nearest ancestor's persistent pre-run hook, +// which the hook installed by EnableRepoOverride would otherwise shadow. By +// default cobra runs only the nearest PersistentPreRunE found walking up from +// the invoked command, so without this the nearest ancestor hook, such as the +// root auth gate, would never run for a repo-override command. +// +// That ancestor hook receives the invoked leaf cmd, not the ancestor, matching +// how cobra passes the leaf to every persistent hook: +// https://github.com/spf13/cobra/blob/v1.10.2/command.go#L984-L986 +// +// cobra's EnableTraverseRunHooks global is the native equivalent and runs the +// whole root-to-leaf chain for us, but it is global. Enabling it would change +// pre-run behavior for every command: double-running the parents that issue +// develop and EnableRepoOverride re-run by hand, and un-suppressing the root +// gate that agent-task and skills intentionally shadow. +func executeParentHook(overrideCmd, cmd *cobra.Command, args []string) error { + for p := overrideCmd.Parent(); p != nil; p = p.Parent() { + if p.PersistentPreRunE != nil { + return p.PersistentPreRunE(cmd, args) } } return nil @@ -47,8 +61,9 @@ func EnableRepoOverride(cmd *cobra.Command, f *Factory) { return results, cobra.ShellCompDirectiveNoFileComp }) + overrideCmd := cmd cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { - if err := executeParentHooks(cmd, args); err != nil { + if err := executeParentHook(overrideCmd, cmd, args); err != nil { return err } repoOverride, _ := cmd.Flags().GetString("repo") diff --git a/pkg/cmdutil/repo_override_test.go b/pkg/cmdutil/repo_override_test.go new file mode 100644 index 00000000000..cc08b083372 --- /dev/null +++ b/pkg/cmdutil/repo_override_test.go @@ -0,0 +1,70 @@ +package cmdutil + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +// Test_EnableRepoOverride_authCheckIntegration is an integration test for the +// coupling between repo override and the root auth gate, wired through cobra's +// persistent pre-run hooks. EnableRepoOverride replaces a command's +// PersistentPreRunE, so cobra no longer reaches the root auth gate on its own; +// the override hook re-runs the ancestor hooks itself. That re-run must evaluate +// the command the user actually invoked, the same way cobra hands the target +// command to parent hooks: +// https://github.com/spf13/cobra/blob/v1.10.2/command.go#L984-L986 +// This pins that a leaf's DisableAuthCheck is honored under a repo-override +// parent. +func Test_EnableRepoOverride_authCheckIntegration(t *testing.T) { + tests := []struct { + name string + disableAuthLeaf bool + wantAuthChecked bool + }{ + { + name: "leaf opts out, honored under repo-override parent", + disableAuthLeaf: true, + wantAuthChecked: false, + }, + { + name: "leaf does not opt out, auth still checked", + disableAuthLeaf: false, + wantAuthChecked: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotAuthChecked, ranLeaf bool + + // Stand in for the real root auth gate: it judges whatever command + // it is handed, so it must receive the invoked leaf command. + root := &cobra.Command{Use: "root"} + root.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + gotAuthChecked = IsAuthCheckEnabled(cmd) + return nil + } + + parent := &cobra.Command{Use: "parent"} + EnableRepoOverride(parent, &Factory{}) + root.AddCommand(parent) + + leaf := &cobra.Command{ + Use: "leaf", + RunE: func(cmd *cobra.Command, args []string) error { ranLeaf = true; return nil }, + } + if tt.disableAuthLeaf { + DisableAuthCheck(leaf) + } + parent.AddCommand(leaf) + + root.SetArgs([]string{"parent", "leaf"}) + require.NoError(t, root.Execute()) + + require.True(t, ranLeaf, "leaf command should have executed") + require.Equal(t, tt.wantAuthChecked, gotAuthChecked) + }) + } +}