From 765f3d9a1293bebf9bc7740ca149212e0106015f Mon Sep 17 00:00:00 2001 From: Michael Costello Date: Fri, 6 Nov 2026 13:53:51 -0500 Subject: [PATCH] fix: Install plugins that publish only pre-releases GitHub never marks a pre-release as the latest release, so for a repository with only pre-releases the releases/latest URL redirects to the release list. The installer took the last path segment of that URL as the tag and failed with an unexpected tag format error naming "releases". The installer now reads the tag only from a releases/tag/ redirect. When there is no latest release it lists the repository's releases through the GitHub API and picks the highest semantic version tag, skipping drafts. Plugin upgrade uses the same lookup, so a pre-release install upgrades to newer pre-releases. Tags must now be valid semantic versions, and errors name the rejected tag, the accepted format, and how to pin a release with owner/repo@. Fixes #356 Co-Authored-By: Claude Opus 5.5 --- internal/cmd/plugin/helpers.go | 132 ++++++++++++++++++++-- internal/cmd/plugin/latest_tag_test.go | 149 +++++++++++++++++++++++++ 2 files changed, 272 insertions(+), 9 deletions(-) create mode 100644 internal/cmd/plugin/latest_tag_test.go diff --git a/internal/cmd/plugin/helpers.go b/internal/cmd/plugin/helpers.go index 6449ae9..7c85eb4 100644 --- a/internal/cmd/plugin/helpers.go +++ b/internal/cmd/plugin/helpers.go @@ -91,15 +91,30 @@ func pluginAssetName(pluginName, _ string) (string, error) { return fmt.Sprintf("%s_%s_%s.%s", pluginName, osName, arch, ext), nil } -// fetchLatestTag resolves the latest GitHub Release tag via redirect. -// It issues a HEAD request to releases/latest and parses the tag from the final URL. // FetchLatestTag resolves the latest release tag for a GitHub owner/repo. // Exported so callers outside this package can pass it to pluginstore.RefreshIndex. func FetchLatestTag(ctx context.Context, owner, repo string) (string, error) { return fetchLatestTag(ctx, owner, repo) } +var githubAPIBaseURL = "https://api.github.com" + +const releaseTagFormat = "a semantic version such as v1.2.3 or v1.2.3-rc.1" + +// fetchLatestTag returns the tag GitHub marks as the latest release. GitHub +// never marks a pre-release as latest, so a repository that has published only +// pre-releases falls back to its highest pre-release tag. func fetchLatestTag(ctx context.Context, owner, repo string) (string, error) { + tag, err := fetchLatestStableTag(ctx, owner, repo) + if err != nil || tag != "" { + return tag, err + } + return fetchNewestReleaseTag(ctx, owner, repo) +} + +// fetchLatestStableTag follows the releases/latest redirect and returns the tag +// it lands on, or "" when the repository has no release marked as latest. +func fetchLatestStableTag(ctx context.Context, owner, repo string) (string, error) { url := fmt.Sprintf("https://github.com/%s/%s/releases/latest", owner, repo) client := &http.Client{ @@ -122,18 +137,117 @@ func fetchLatestTag(ctx context.Context, owner, repo string) (string, error) { } defer resp.Body.Close() - // After following redirect, the final URL has the tag at the end. - finalURL := resp.Request.URL.String() - tag := filepath.Base(finalURL) - if tag == "" || tag == "." || tag == "/" { - return "", fmt.Errorf("could not parse release tag from URL %q", finalURL) + if resp.StatusCode == http.StatusNotFound { + return "", fmt.Errorf("no GitHub repository %s/%s with published releases was found", owner, repo) + } + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("fetch latest release tag: HTTP %s", resp.Status) + } + return tagFromLatestReleaseURL(resp.Request.URL.String()) +} + +// tagFromLatestReleaseURL extracts the tag from the URL that releases/latest +// redirects to. GitHub redirects to .../releases/tag/ when a latest +// release exists and to .../releases when none does; the latter returns "". +func tagFromLatestReleaseURL(rawURL string) (string, error) { + u, err := url.Parse(rawURL) + if err != nil { + return "", fmt.Errorf("could not parse release tag from URL %q: %w", rawURL, err) + } + if strings.HasSuffix(strings.TrimSuffix(u.EscapedPath(), "/"), "/releases") { + return "", nil + } + _, escapedTag, ok := strings.Cut(u.EscapedPath(), "/releases/tag/") + if !ok || escapedTag == "" { + return "", fmt.Errorf("could not parse release tag from URL %q", rawURL) + } + tag, err := url.PathUnescape(escapedTag) + if err != nil { + return "", fmt.Errorf("could not parse release tag from URL %q: %w", rawURL, err) } - if !strings.HasPrefix(tag, "v") { - return "", fmt.Errorf("unexpected release tag format %q (expected vX.Y.Z)", tag) + if !semver.IsValid(tag) { + return "", fmt.Errorf("release tag %q is not a semantic version; expected %s", tag, releaseTagFormat) } return tag, nil } +// githubRelease is the subset of the GitHub REST release object datumctl reads. +type githubRelease struct { + TagName string `json:"tag_name"` + Draft bool `json:"draft"` + Prerelease bool `json:"prerelease"` +} + +// fetchNewestReleaseTag lists the repository's releases through the GitHub API +// and returns the highest semantic-version tag, pre-releases included. +func fetchNewestReleaseTag(ctx context.Context, owner, repo string) (string, error) { + listURL := fmt.Sprintf("%s/repos/%s/%s/releases?per_page=100", githubAPIBaseURL, owner, repo) + pinHint := fmt.Sprintf("install a specific release with %s/%s@", owner, repo) + + client := &http.Client{Timeout: 15 * time.Second, CheckRedirect: pluginstore.SafeCheckRedirect} + req, err := http.NewRequestWithContext(ctx, http.MethodGet, listURL, nil) + if err != nil { + return "", err + } + req.Header.Set("User-Agent", "datumctl-plugin-install") + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := client.Do(req) + if err != nil { + return "", fmt.Errorf("list releases for %s/%s: %w; %s", owner, repo, err, pinHint) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("list releases for %s/%s: HTTP %s; %s", owner, repo, resp.Status, pinHint) + } + + body, err := pluginstore.ReadCapped(resp.Body, pluginstore.MaxManifestBytes) + if err != nil { + return "", fmt.Errorf("read release list for %s/%s: %w", owner, repo, err) + } + var releases []githubRelease + if err := json.Unmarshal(body, &releases); err != nil { + return "", fmt.Errorf("parse release list for %s/%s: %w", owner, repo, err) + } + return newestReleaseTag(owner, repo, releases) +} + +// newestReleaseTag returns the highest semantic-version tag among published +// releases. Drafts and tags that are not semantic versions are ignored. +func newestReleaseTag(owner, repo string, releases []githubRelease) (string, error) { + var newest string + var rejected []string + for _, r := range releases { + if r.Draft { + continue + } + if !semver.IsValid(r.TagName) { + rejected = append(rejected, r.TagName) + continue + } + if newest == "" || semver.Compare(r.TagName, newest) > 0 { + newest = r.TagName + } + } + if newest != "" { + return newest, nil + } + if len(rejected) == 0 { + return "", fmt.Errorf("%s/%s has no published releases", owner, repo) + } + const maxListed = 5 + if len(rejected) > maxListed { + rejected = rejected[:maxListed] + } + quoted := make([]string, len(rejected)) + for i, tag := range rejected { + quoted[i] = fmt.Sprintf("%q", tag) + } + return "", fmt.Errorf("no release of %s/%s has an installable tag (found %s); expected %s", + owner, repo, strings.Join(quoted, ", "), releaseTagFormat) +} + // fetchChecksums downloads and parses checksums.txt from a GitHub Release. // Returns a map of filename → sha256hex. func fetchChecksums(ctx context.Context, owner, repo, tag string) (map[string]string, error) { diff --git a/internal/cmd/plugin/latest_tag_test.go b/internal/cmd/plugin/latest_tag_test.go new file mode 100644 index 0000000..99ec7fb --- /dev/null +++ b/internal/cmd/plugin/latest_tag_test.go @@ -0,0 +1,149 @@ +package plugin + +import ( + "context" + "crypto/tls" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestTagFromLatestReleaseURL(t *testing.T) { + tests := []struct { + name string + url string + want string + wantErr string + }{ + {name: "stable tag", url: "https://github.com/o/r/releases/tag/v1.2.3", want: "v1.2.3"}, + {name: "pre-release tag", url: "https://github.com/o/r/releases/tag/v1.0.0-preview.28", want: "v1.0.0-preview.28"}, + {name: "build metadata", url: "https://github.com/o/r/releases/tag/v1.0.0%2Bbuild.1", want: "v1.0.0+build.1"}, + {name: "no stable release redirects to list", url: "https://github.com/o/r/releases", want: ""}, + {name: "no stable release with trailing slash", url: "https://github.com/o/r/releases/", want: ""}, + {name: "tag without v prefix", url: "https://github.com/o/r/releases/tag/1.2.3", wantErr: `"1.2.3"`}, + {name: "non-semver tag", url: "https://github.com/o/r/releases/tag/vNext", wantErr: `"vNext"`}, + {name: "unexpected page", url: "https://github.com/o/r/releases/latest", wantErr: "could not parse release tag"}, + {name: "traversal in tag", url: "https://github.com/o/r/releases/tag/..%2F..%2Fx", wantErr: "not a semantic version"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tagFromLatestReleaseURL(tt.url) + if tt.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("tagFromLatestReleaseURL(%q) error = %v, want containing %q", tt.url, err, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("tagFromLatestReleaseURL(%q) unexpected error: %v", tt.url, err) + } + if got != tt.want { + t.Fatalf("tagFromLatestReleaseURL(%q) = %q, want %q", tt.url, got, tt.want) + } + }) + } +} + +func TestNewestReleaseTag(t *testing.T) { + tests := []struct { + name string + releases []githubRelease + want string + wantErr string + }{ + { + name: "only pre-releases picks highest", + releases: []githubRelease{ + {TagName: "v1.0.0-preview.9", Prerelease: true}, + {TagName: "v1.0.0-preview.28", Prerelease: true}, + {TagName: "v1.0.0-preview.27", Prerelease: true}, + }, + want: "v1.0.0-preview.28", + }, + { + name: "drafts are skipped", + releases: []githubRelease{ + {TagName: "v2.0.0-rc.1", Draft: true}, + {TagName: "v1.0.0-rc.1", Prerelease: true}, + }, + want: "v1.0.0-rc.1", + }, + { + name: "invalid tags are skipped", + releases: []githubRelease{ + {TagName: "nightly", Prerelease: true}, + {TagName: "v0.3.0-beta", Prerelease: true}, + }, + want: "v0.3.0-beta", + }, + { + name: "no releases", + releases: nil, + wantErr: "has no published releases", + }, + { + name: "no semver tags", + releases: []githubRelease{ + {TagName: "nightly", Prerelease: true}, + {TagName: "latest"}, + }, + wantErr: `"nightly"`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := newestReleaseTag("o", "r", tt.releases) + if tt.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("newestReleaseTag error = %v, want containing %q", err, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("newestReleaseTag unexpected error: %v", err) + } + if got != tt.want { + t.Fatalf("newestReleaseTag = %q, want %q", got, tt.want) + } + }) + } +} + +func TestFetchNewestReleaseTag_onlyPrereleases(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/repos/o/r/releases" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`[ + {"tag_name":"v1.0.0-preview.28","draft":false,"prerelease":true}, + {"tag_name":"v1.0.0-preview.27","draft":false,"prerelease":true} + ]`)) + })) + t.Cleanup(srv.Close) + + origBase := githubAPIBaseURL + githubAPIBaseURL = srv.URL + t.Cleanup(func() { githubAPIBaseURL = origBase }) + + origTransport := http.DefaultTransport + http.DefaultTransport = &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // test only + } + t.Cleanup(func() { http.DefaultTransport = origTransport }) + + got, err := fetchNewestReleaseTag(context.Background(), "o", "r") + if err != nil { + t.Fatalf("fetchNewestReleaseTag: %v", err) + } + if got != "v1.0.0-preview.28" { + t.Fatalf("fetchNewestReleaseTag = %q, want v1.0.0-preview.28", got) + } + + if _, err := fetchNewestReleaseTag(context.Background(), "o", "missing"); err == nil || + !strings.Contains(err.Error(), "o/missing@") { + t.Fatalf("fetchNewestReleaseTag on 404 error = %v, want hint naming o/missing@", err) + } +}