From 8970873156153a4c95b492e30e4411ca51a2ec33 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Mon, 29 Jun 2026 09:37:13 +0800 Subject: [PATCH] feat(auth): streamline GitHub token handling and enhance download asset logic --- internal/pluginstore/auth.go | 15 +--- internal/pluginstore/auth_test.go | 18 +++++ internal/pluginstore/github.go | 37 ++++------ internal/pluginstore/install_test.go | 105 ++++++++++++++++++++++++++- 4 files changed, 138 insertions(+), 37 deletions(-) diff --git a/internal/pluginstore/auth.go b/internal/pluginstore/auth.go index c72240287..72d16a72e 100644 --- a/internal/pluginstore/auth.go +++ b/internal/pluginstore/auth.go @@ -82,11 +82,7 @@ func AuthConfigured(auth []AuthConfig, requestURL string, kind string) bool { case AuthTypeNone: return false case AuthTypeBearer, AuthTypeGitHubToken: - envName := item.TokenEnv - if envName == "" && item.Type == AuthTypeGitHubToken { - envName = "GITSTORE_GIT_TOKEN" - } - return strings.TrimSpace(os.Getenv(envName)) != "" + return strings.TrimSpace(os.Getenv(item.TokenEnv)) != "" case AuthTypeBasic: return strings.TrimSpace(os.Getenv(item.UsernameEnv)) != "" && strings.TrimSpace(os.Getenv(item.PasswordEnv)) != "" case AuthTypeHeader: @@ -162,12 +158,9 @@ func applyPluginStoreAuth(headers http.Header, auth []AuthConfig, requestURL str } headers.Set(item.HeaderName, value) case AuthTypeGitHubToken: - token := strings.TrimSpace(os.Getenv(strings.TrimSpace(item.TokenEnv))) - if token == "" { - token = strings.TrimSpace(os.Getenv("GITSTORE_GIT_TOKEN")) - } - if token == "" { - return fmt.Errorf("plugin store auth missing token-env") + token, errToken := envValueRequired(item.TokenEnv, "token-env") + if errToken != nil { + return errToken } headers.Set("Authorization", "Bearer "+token) default: diff --git a/internal/pluginstore/auth_test.go b/internal/pluginstore/auth_test.go index d672911ed..07ea25bee 100644 --- a/internal/pluginstore/auth_test.go +++ b/internal/pluginstore/auth_test.go @@ -44,6 +44,24 @@ func TestPluginStoreAuthMatchesURLHostAndPathBoundaries(t *testing.T) { } } +func TestPluginStoreGitHubTokenUsesExplicitTokenEnv(t *testing.T) { + t.Setenv("PLUGIN_STORE_TOKEN", "secret-token") + headers := http.Header{} + auth := []AuthConfig{{ + Match: "https://api.github.com/repos/author-name/sample-provider/releases/", + ApplyTo: []string{RequestKindArtifact}, + Type: AuthTypeGitHubToken, + TokenEnv: "PLUGIN_STORE_TOKEN", + }} + + if errAuth := applyPluginStoreAuth(headers, auth, "https://api.github.com/repos/author-name/sample-provider/releases/assets/1", RequestKindArtifact); errAuth != nil { + t.Fatalf("applyPluginStoreAuth() error = %v", errAuth) + } + if gotAuth := headers.Get("Authorization"); gotAuth != "Bearer secret-token" { + t.Fatalf("Authorization = %q, want Bearer secret-token", gotAuth) + } +} + func TestPluginAuthConfiguredCoversInstallRequestKinds(t *testing.T) { t.Setenv("PLUGIN_STORE_TOKEN", "secret-token") diff --git a/internal/pluginstore/github.go b/internal/pluginstore/github.go index 836758871..2db6299ed 100644 --- a/internal/pluginstore/github.go +++ b/internal/pluginstore/github.go @@ -7,7 +7,6 @@ import ( "io" "net/http" "net/url" - "os" "strings" "github.com/router-for-me/CLIProxyAPI/v7/internal/httpfetch" @@ -115,9 +114,12 @@ func ReleaseVersion(release Release) (string, error) { } func (c Client) DownloadAsset(ctx context.Context, asset ReleaseAsset) ([]byte, error) { - downloadURL := strings.TrimSpace(asset.APIURL) - if downloadURL == "" { - downloadURL = strings.TrimSpace(asset.BrowserDownloadURL) + downloadURL := strings.TrimSpace(asset.BrowserDownloadURL) + apiURL := strings.TrimSpace(asset.APIURL) + if downloadURL == "" || c.releaseAssetAPIAuthenticated(apiURL) { + if apiURL != "" { + downloadURL = apiURL + } } if downloadURL == "" { return nil, fmt.Errorf("asset %q missing download url", asset.Name) @@ -125,6 +127,14 @@ func (c Client) DownloadAsset(ctx context.Context, asset ReleaseAsset) ([]byte, return c.get(ctx, downloadURL, "application/octet-stream", RequestKindArtifact, 0) } +func (c Client) releaseAssetAPIAuthenticated(apiURL string) bool { + apiURL = strings.TrimSpace(apiURL) + if apiURL == "" { + return false + } + return AuthConfigured(c.Auth, apiURL, RequestKindArtifact) +} + func (c Client) get(ctx context.Context, requestURL string, accept string, kind string, maxSize int64) ([]byte, error) { currentURL := strings.TrimSpace(requestURL) for redirects := 0; ; redirects++ { @@ -138,11 +148,6 @@ func (c Client) get(ctx context.Context, requestURL string, accept string, kind if errAuth := applyPluginStoreAuth(headers, c.Auth, currentURL, kind); errAuth != nil { return nil, errAuth } - if headers.Get("Authorization") == "" { - if token := gitHubAPIToken(currentURL); token != "" { - headers.Set("Authorization", "Bearer "+token) - } - } resp, errDo := pluginStoreGetNoRedirect(ctx, c.httpClient(), currentURL, headers) if errDo != nil { return nil, errDo @@ -165,20 +170,6 @@ func (c Client) get(ctx context.Context, requestURL string, accept string, kind } } -// gitHubAPIToken returns the optional GitHub token for GitHub API requests to -// raise the unauthenticated rate limit, mirroring the management asset updater. -func gitHubAPIToken(requestURL string) string { - parsed, errParse := url.Parse(requestURL) - if errParse != nil || !strings.EqualFold(parsed.Host, "api.github.com") { - return "" - } - gitURL := strings.ToLower(strings.TrimSpace(os.Getenv("GITSTORE_GIT_URL"))) - if !strings.Contains(gitURL, "github.com") { - return "" - } - return strings.TrimSpace(os.Getenv("GITSTORE_GIT_TOKEN")) -} - func (c Client) httpClient() HTTPDoer { if c.HTTPClient != nil { return c.HTTPClient diff --git a/internal/pluginstore/install_test.go b/internal/pluginstore/install_test.go index 9fcae5932..e20cc01a3 100644 --- a/internal/pluginstore/install_test.go +++ b/internal/pluginstore/install_test.go @@ -347,9 +347,7 @@ func TestInstallUsesLatestReleaseVersion(t *testing.T) { } } -func TestInstallDownloadsReleaseAssetsViaAPIURL(t *testing.T) { - t.Parallel() - +func TestInstallDownloadsReleaseAssetsViaBrowserDownloadURL(t *testing.T) { root := t.TempDir() archiveData := makeZip(t, map[string]string{"sample-provider.dylib": "library-data"}) archiveName := "sample-provider_0.2.0_darwin_arm64.zip" @@ -370,6 +368,49 @@ func TestInstallDownloadsReleaseAssetsViaAPIURL(t *testing.T) { } ] }`), + "https://downloads.example/missing.zip": archiveData, + "https://downloads.example/missing-checksums.txt": []byte(hex.EncodeToString(checksum[:]) + " " + archiveName + "\n"), + }} + + result, errInstall := client.Install(context.Background(), testPlugin(), InstallOptions{ + PluginsDir: root, + GOOS: "darwin", + GOARCH: "arm64", + }) + if errInstall != nil { + t.Fatalf("Install() error = %v", errInstall) + } + if result.Version != "0.2.0" { + t.Fatalf("Version = %q, want 0.2.0 from latest release tag", result.Version) + } + data, errRead := os.ReadFile(filepath.Join(root, "darwin", "arm64", "sample-provider-v0.2.0.dylib")) + if errRead != nil { + t.Fatalf("ReadFile() error = %v", errRead) + } + if string(data) != "library-data" { + t.Fatalf("installed data = %q, want library-data", data) + } +} + +func TestInstallFallsBackToReleaseAssetAPIURLWhenBrowserDownloadURLEmpty(t *testing.T) { + root := t.TempDir() + archiveData := makeZip(t, map[string]string{"sample-provider.dylib": "library-data"}) + archiveName := "sample-provider_0.2.0_darwin_arm64.zip" + checksum := sha256.Sum256(archiveData) + client := Client{HTTPClient: mapHTTPDoer{ + "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/latest": []byte(`{ + "tag_name": "v0.2.0", + "assets": [ + { + "name": "` + archiveName + `", + "url": "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/assets/1" + }, + { + "name": "checksums.txt", + "url": "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/assets/2" + } + ] + }`), "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/assets/1": archiveData, "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/assets/2": []byte(hex.EncodeToString(checksum[:]) + " " + archiveName + "\n"), }} @@ -394,6 +435,64 @@ func TestInstallDownloadsReleaseAssetsViaAPIURL(t *testing.T) { } } +func TestDownloadAssetUsesAPIURLWhenAuthMatchesArtifact(t *testing.T) { + t.Setenv("PLUGIN_STORE_TOKEN", "secret-token") + apiURL := "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/assets/1" + client := Client{ + HTTPClient: authCheckingHTTPDoer{ + url: apiURL, + wantAuth: "Bearer secret-token", + responseBytes: []byte("artifact-data"), + }, + Auth: []AuthConfig{{ + Match: "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/", + ApplyTo: []string{RequestKindArtifact}, + Type: AuthTypeBearer, + TokenEnv: "PLUGIN_STORE_TOKEN", + }}, + } + + data, errDownload := client.DownloadAsset(context.Background(), ReleaseAsset{ + Name: "sample-provider_0.2.0_darwin_arm64.zip", + APIURL: apiURL, + BrowserDownloadURL: "https://downloads.example/sample-provider.zip", + }) + if errDownload != nil { + t.Fatalf("DownloadAsset() error = %v", errDownload) + } + if string(data) != "artifact-data" { + t.Fatalf("DownloadAsset() = %q, want artifact-data", data) + } +} + +func TestDownloadAssetUsesBrowserDownloadURLWithUnrelatedAuth(t *testing.T) { + t.Setenv("PLUGIN_STORE_TOKEN", "secret-token") + browserURL := "https://downloads.example/sample-provider.zip" + client := Client{ + HTTPClient: mapHTTPDoer{ + browserURL: []byte("artifact-data"), + }, + Auth: []AuthConfig{{ + Match: "https://registry.example/", + ApplyTo: []string{RequestKindRegistry}, + Type: AuthTypeBearer, + TokenEnv: "PLUGIN_STORE_TOKEN", + }}, + } + + data, errDownload := client.DownloadAsset(context.Background(), ReleaseAsset{ + Name: "sample-provider_0.2.0_darwin_arm64.zip", + APIURL: "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/assets/1", + BrowserDownloadURL: browserURL, + }) + if errDownload != nil { + t.Fatalf("DownloadAsset() error = %v", errDownload) + } + if string(data) != "artifact-data" { + t.Fatalf("DownloadAsset() = %q, want artifact-data", data) + } +} + func TestInstallVersionUsesPinnedReleaseTag(t *testing.T) { t.Parallel()