diff --git a/internal/api/handlers/management/plugin_store.go b/internal/api/handlers/management/plugin_store.go index 969e6ce6..a405de5a 100644 --- a/internal/api/handlers/management/plugin_store.go +++ b/internal/api/handlers/management/plugin_store.go @@ -21,6 +21,10 @@ import ( ) const ( + // defaultPluginStoreInstallTimeout bounds plugin store install downloads so + // a stalled registry, release, or asset request does not hold the management + // request forever. + defaultPluginStoreInstallTimeout = 5 * time.Minute // pluginReleaseCacheTTL bounds how long a resolved latest release version is // reused before the GitHub API is queried again. pluginReleaseCacheTTL = 10 * time.Minute @@ -139,14 +143,28 @@ func (h *Handler) InstallPluginFromStore(c *gin.Context) { } func (h *Handler) installPluginFromStore(c *gin.Context, goos, goarch string) { + h.installPluginFromStoreWithTimeout(c, goos, goarch, defaultPluginStoreInstallTimeout) +} + +func (h *Handler) installPluginFromStoreWithTimeout(c *gin.Context, goos, goarch string, timeout time.Duration) { id, okID := pluginIDFromRequest(c) if !okID { return } + installCtx := c.Request.Context() + if timeout > 0 { + var cancel context.CancelFunc + installCtx, cancel = context.WithTimeout(installCtx, timeout) + defer cancel() + } pluginsEnabled, pluginsDir, proxyURL, _, host := h.pluginStoreSnapshot() client := h.newPluginStoreClient(proxyURL) - registry, errRegistry := client.FetchRegistry(c.Request.Context()) + registry, errRegistry := client.FetchRegistry(installCtx) if errRegistry != nil { + if pluginStoreRequestTimedOut(installCtx, errRegistry) { + c.JSON(http.StatusGatewayTimeout, gin.H{"error": "plugin_store_timeout", "message": "plugin store request timed out"}) + return + } c.JSON(http.StatusBadGateway, gin.H{"error": "plugin_store_registry_failed", "message": errRegistry.Error()}) return } @@ -158,7 +176,7 @@ func (h *Handler) installPluginFromStore(c *gin.Context, goos, goarch string) { pluginIsLoaded := func() bool { return pluginLoaded(host, id) } unloadedBeforeWrite := false - result, errInstall := client.Install(c.Request.Context(), plugin, pluginstore.InstallOptions{ + result, errInstall := client.Install(installCtx, plugin, pluginstore.InstallOptions{ PluginsDir: pluginsDir, GOOS: goos, GOARCH: goarch, @@ -196,6 +214,10 @@ func (h *Handler) installPluginFromStore(c *gin.Context, goos, goarch string) { }) return } + if pluginStoreRequestTimedOut(installCtx, errInstall) { + c.JSON(http.StatusGatewayTimeout, gin.H{"error": "plugin_install_timeout", "message": "plugin install timed out"}) + return + } c.JSON(http.StatusBadGateway, gin.H{"error": "plugin_install_failed", "message": errInstall.Error()}) return } @@ -250,6 +272,13 @@ func (h *Handler) installPluginFromStore(c *gin.Context, goos, goarch string) { }) } +func pluginStoreRequestTimedOut(ctx context.Context, err error) bool { + if errors.Is(err, context.DeadlineExceeded) { + return true + } + return ctx != nil && errors.Is(ctx.Err(), context.DeadlineExceeded) +} + // enablePluginConfigLocked sets plugins.configs..enabled to true while preserving // the rest of the plugin's raw configuration. Callers must hold h.mu. func (h *Handler) enablePluginConfigLocked(id string) error { diff --git a/internal/api/handlers/management/plugin_store_test.go b/internal/api/handlers/management/plugin_store_test.go index 4cb59b4e..d3ae7358 100644 --- a/internal/api/handlers/management/plugin_store_test.go +++ b/internal/api/handlers/management/plugin_store_test.go @@ -17,6 +17,7 @@ import ( "strings" "sync" "testing" + "time" "github.com/gin-gonic/gin" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" @@ -317,6 +318,62 @@ func TestInstallPluginFromStoreWritesFileAndEnablesConfig(t *testing.T) { } } +func TestInstallPluginFromStoreTimesOutDownloadingAsset(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + pluginsDir := t.TempDir() + archiveName := "sample-provider_0.1.0_" + runtime.GOOS + "_" + runtime.GOARCH + ".zip" + archiveURL := "https://downloads.example/" + archiveName + h := &Handler{ + cfg: &config.Config{ + Plugins: config.PluginsConfig{ + Enabled: false, + Dir: pluginsDir, + }, + }, + configFilePath: writeTestConfigFile(t), + pluginStoreRegistryURL: "https://registry.example/registry.json", + pluginStoreHTTPClient: blockingPluginStoreHTTPClient{ + blockURL: archiveURL, + responses: fakePluginStoreHTTPClient{ + "https://registry.example/registry.json": registryJSON(t), + "https://api.github.com/repos/author-name/cliproxy-sample-provider-plugin/releases/latest": []byte(`{ + "tag_name": "v0.1.0", + "assets": [ + {"name": "` + archiveName + `", "browser_download_url": "` + archiveURL + `"}, + {"name": "checksums.txt", "browser_download_url": "https://downloads.example/checksums.txt"} + ] + }`), + }, + }, + } + + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Params = gin.Params{{Key: "id", Value: "sample-provider"}} + c.Request = httptest.NewRequest(http.MethodPost, "/v0/management/plugin-store/sample-provider/install", nil) + + h.installPluginFromStoreWithTimeout(c, runtime.GOOS, runtime.GOARCH, 20*time.Millisecond) + + if rec.Code != http.StatusGatewayTimeout { + t.Fatalf("status = %d, want %d; body=%s", rec.Code, http.StatusGatewayTimeout, rec.Body.String()) + } + var body struct { + Error string `json:"error"` + } + if errDecode := json.Unmarshal(rec.Body.Bytes(), &body); errDecode != nil { + t.Fatalf("Unmarshal() error = %v; body=%s", errDecode, rec.Body.String()) + } + if body.Error != "plugin_install_timeout" { + t.Fatalf("error = %q, want plugin_install_timeout", body.Error) + } + targetPath := filepath.Join(pluginsDir, runtime.GOOS, runtime.GOARCH, "sample-provider"+managementPluginExtension(runtime.GOOS)) + if _, errStat := os.Stat(targetPath); !os.IsNotExist(errStat) { + t.Fatalf("target stat error = %v, want not exist", errStat) + } +} + func TestInstallPluginFromStoreOverwritesFilePreservesConfigAndReloads(t *testing.T) { t.Parallel() gin.SetMode(gin.TestMode) @@ -461,6 +518,19 @@ func (c fakePluginStoreHTTPClient) Do(req *http.Request) (*http.Response, error) }, nil } +type blockingPluginStoreHTTPClient struct { + responses fakePluginStoreHTTPClient + blockURL string +} + +func (c blockingPluginStoreHTTPClient) Do(req *http.Request) (*http.Response, error) { + if req.URL.String() == c.blockURL { + <-req.Context().Done() + return nil, req.Context().Err() + } + return c.responses.Do(req) +} + type countingPluginStoreHTTPClient struct { responses fakePluginStoreHTTPClient mu sync.Mutex