diff --git a/cmd/wfctl/main.go b/cmd/wfctl/main.go index 3a7d5fca..a01989f4 100644 --- a/cmd/wfctl/main.go +++ b/cmd/wfctl/main.go @@ -3,6 +3,7 @@ package main import ( "context" _ "embed" + "errors" "fmt" "io" "log/slog" @@ -174,7 +175,13 @@ func main() { // The handler already printed routing errors (unknown/missing command). // Only emit the "error:" prefix for actual command execution failures. if _, isKnown := commands[cmd]; isKnown { - fmt.Fprintf(os.Stderr, "error: %v\n", dispatchErr) //nolint:gosec // G705 + // Unwrap to the root cause so users see a concise, actionable message + // rather than the full pipeline execution chain. + rootErr := dispatchErr + for errors.Unwrap(rootErr) != nil { + rootErr = errors.Unwrap(rootErr) + } + fmt.Fprintf(os.Stderr, "error: %v\n", rootErr) //nolint:gosec // G705 } os.Exit(1) } diff --git a/cmd/wfctl/plugin_install.go b/cmd/wfctl/plugin_install.go index c47e7696..80d0db66 100644 --- a/cmd/wfctl/plugin_install.go +++ b/cmd/wfctl/plugin_install.go @@ -78,11 +78,10 @@ func runPluginInstall(args []string) error { if err := fs.Parse(args); err != nil { return err } - dataDir := &pluginDirVal // No args: install all plugins from .wfctl.yaml lockfile. if fs.NArg() < 1 { - return installFromLockfile(*dataDir, *cfgPath) + return installFromLockfile(pluginDirVal, *cfgPath) } nameArg := fs.Arg(0) @@ -121,7 +120,7 @@ func runPluginInstall(args []string) error { return registryErr } pluginName = normalizePluginName(ghRepo) - destDir := filepath.Join(*dataDir, pluginName) + destDir := filepath.Join(pluginDirVal, pluginName) if err := installFromGitHub(ghOwner, ghRepo, ghVersion, destDir); err != nil { return fmt.Errorf("registry: %w; github: %w", registryErr, err) } @@ -132,15 +131,30 @@ func runPluginInstall(args []string) error { return nil } - destDir := filepath.Join(*dataDir, pluginName) - fmt.Fprintf(os.Stderr, "Found in registry %q.\n", sourceName) + if err := installPluginFromManifest(pluginDirVal, pluginName, manifest); err != nil { + return err + } + + // Update .wfctl.yaml lockfile if name@version was provided. + if _, ver := parseNameVersion(nameArg); ver != "" { + updateLockfile(manifest.Name, manifest.Version, manifest.Repository) + } + + return nil +} + +// installPluginFromManifest downloads, extracts, and installs a plugin using the +// provided registry manifest. It is shared by runPluginInstall and runPluginUpdate. +// The plugin.json is always written/updated from the manifest to keep version tracking correct. +func installPluginFromManifest(dataDir, pluginName string, manifest *RegistryManifest) error { dl, err := manifest.FindDownload(runtime.GOOS, runtime.GOARCH) if err != nil { return err } + destDir := filepath.Join(dataDir, pluginName) if err := os.MkdirAll(destDir, 0750); err != nil { return fmt.Errorf("create plugin dir %s: %w", destDir, err) } @@ -169,12 +183,12 @@ func runPluginInstall(args []string) error { fmt.Fprintf(os.Stderr, "warning: could not normalize binary name: %v\n", err) } - // Write a minimal plugin.json if not already present (records version). + // Write plugin.json from the registry manifest. This keeps the installed + // version metadata in sync with the manifest. If the tarball already + // extracted a plugin.json, this overwrites it with the registry version. pluginJSONPath := filepath.Join(destDir, "plugin.json") - if _, err := os.Stat(pluginJSONPath); os.IsNotExist(err) { - if writeErr := writeInstalledManifest(pluginJSONPath, manifest); writeErr != nil { - fmt.Fprintf(os.Stderr, "warning: could not write plugin.json: %v\n", writeErr) - } + if writeErr := writeInstalledManifest(pluginJSONPath, manifest); writeErr != nil { + fmt.Fprintf(os.Stderr, "warning: could not write plugin.json: %v\n", writeErr) } // Verify the installed plugin.json is valid for ExternalPluginManager. @@ -184,12 +198,6 @@ func runPluginInstall(args []string) error { } fmt.Printf("Installed %s v%s to %s\n", manifest.Name, manifest.Version, destDir) - - // Update .wfctl.yaml lockfile if name@version was provided. - if _, ver := parseNameVersion(nameArg); ver != "" { - updateLockfile(manifest.Name, manifest.Version, manifest.Repository) - } - return nil } @@ -271,26 +279,59 @@ func runPluginUpdate(args []string) error { return fmt.Errorf("plugin %q is not installed", pluginName) } + // Read the local plugin.json for fallback: if the central registry doesn't + // list this plugin, we can try fetching the manifest directly from the + // plugin's own repository (the "repository" field in plugin.json). + var localRepoURL string + if data, err := os.ReadFile(filepath.Join(pluginDir, "plugin.json")); err == nil { + var pj installedPluginJSON + if json.Unmarshal(data, &pj) == nil { + localRepoURL = pj.Repository + } + } + // Check the registry for the latest version before downloading. cfg, err := LoadRegistryConfig(*cfgPath) if err != nil { return fmt.Errorf("load registry config: %w", err) } mr := NewMultiRegistry(cfg) - manifest, _, err := mr.FetchManifest(pluginName) - if err != nil { - return fmt.Errorf("fetch manifest: %w", err) + + fmt.Fprintf(os.Stderr, "Fetching manifest for %q...\n", pluginName) + manifest, sourceName, registryErr := mr.FetchManifest(pluginName) + if registryErr == nil { + fmt.Fprintf(os.Stderr, "Found in registry %q.\n", sourceName) + installedVer := readInstalledVersion(pluginDir) + if installedVer == manifest.Version { + fmt.Printf("already at latest version (%s)\n", manifest.Version) + return nil + } + fmt.Fprintf(os.Stderr, "Updating from %s to %s...\n", installedVer, manifest.Version) + return installPluginFromManifest(pluginDirVal, pluginName, manifest) } - installedVer := readInstalledVersion(pluginDir) - if installedVer == manifest.Version { - fmt.Printf("already at latest version (%s)\n", manifest.Version) - return nil + // Registry lookup failed. If the plugin's manifest declares a repository + // URL, try fetching the manifest directly from there as a fallback. + if localRepoURL != "" { + fmt.Fprintf(os.Stderr, "Not found in registry. Trying repository URL %q...\n", localRepoURL) + manifest, err = fetchManifestFromRepoURL(localRepoURL) + if err != nil { + return fmt.Errorf("registry lookup failed (%v); repository fallback also failed: %w", registryErr, err) + } + // Validate that the fetched manifest is for the plugin we're updating. + if manifest.Name != pluginName { + return fmt.Errorf("manifest name %q does not match plugin %q; refusing to update to prevent installing the wrong plugin", manifest.Name, pluginName) + } + installedVer := readInstalledVersion(pluginDir) + if installedVer == manifest.Version { + fmt.Printf("already at latest version (%s)\n", manifest.Version) + return nil + } + fmt.Fprintf(os.Stderr, "Updating from %s to %s...\n", installedVer, manifest.Version) + return installPluginFromManifest(pluginDirVal, pluginName, manifest) } - fmt.Fprintf(os.Stderr, "Updating from %s to %s...\n", installedVer, manifest.Version) - // Re-run install which will overwrite the existing installation. - return runPluginInstall(append([]string{"--plugin-dir", pluginDirVal, "--config", *cfgPath}, pluginName)) + return registryErr } func runPluginRemove(args []string) error { @@ -433,6 +474,66 @@ func verifyChecksum(data []byte, expected string) error { return nil } +// rawGitHubContentBaseURL is the base URL for raw GitHub content. It is a +// package-level variable so tests can override it to point at a local server. +var rawGitHubContentBaseURL = "https://raw.githubusercontent.com" + +// fetchManifestFromRepoURL fetches a plugin's manifest.json directly from its +// GitHub repository. It expects the repository URL in the form +// https://github.com/{owner}/{repo} and looks for a manifest.json at the root +// of the default branch. +func fetchManifestFromRepoURL(repoURL string) (*RegistryManifest, error) { + owner, repo, err := parseGitHubRepoURL(repoURL) + if err != nil { + return nil, fmt.Errorf("parse repository URL %q: %w", repoURL, err) + } + url := fmt.Sprintf("%s/%s/%s/main/manifest.json", rawGitHubContentBaseURL, owner, repo) + resp, err := http.Get(url) //nolint:gosec // G107: URL constructed from plugin's own repository field + if err != nil { + return nil, fmt.Errorf("fetch manifest from %q: %w", repoURL, err) + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusNotFound { + return nil, fmt.Errorf("no manifest.json found in repository %q (tried %s)", repoURL, url) + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("repository %q returned HTTP %d", repoURL, resp.StatusCode) + } + data, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("read manifest from %q: %w", repoURL, err) + } + var m RegistryManifest + if err := json.Unmarshal(data, &m); err != nil { + return nil, fmt.Errorf("parse manifest from %q: %w", repoURL, err) + } + return &m, nil +} + +// parseGitHubRepoURL parses a GitHub repository URL and returns the owner and +// repository name. It accepts URLs in the form https://github.com/{owner}/{repo} +// (with or without trailing slash or .git suffix) and rejects URLs with extra +// path segments (e.g. https://github.com/owner/repo/tree/main). +func parseGitHubRepoURL(repoURL string) (owner, repo string, err error) { + u := strings.TrimPrefix(repoURL, "https://") + u = strings.TrimPrefix(u, "http://") + u = strings.TrimSuffix(u, "/") + // Split into at most 4 parts to detect extra path segments. + parts := strings.SplitN(u, "/", 4) + if len(parts) < 3 || parts[0] != "github.com" || parts[1] == "" || parts[2] == "" { + return "", "", fmt.Errorf("not a GitHub repository URL: %q (expected https://github.com/owner/repo)", repoURL) + } + if len(parts) == 4 { + // Extra path segments present (e.g. /tree/main, /blob/main/file.go). + return "", "", fmt.Errorf("not a GitHub repository URL: %q (unexpected extra path; expected https://github.com/owner/repo)", repoURL) + } + repoName := strings.TrimSuffix(parts[2], ".git") + if repoName == "" { + return "", "", fmt.Errorf("not a GitHub repository URL: %q (expected https://github.com/owner/repo)", repoURL) + } + return parts[1], repoName, nil +} + // extractTarGz decompresses and extracts a .tar.gz archive into destDir. // It guards against path traversal (zip-slip) attacks. func extractTarGz(data []byte, destDir string) error { diff --git a/cmd/wfctl/plugin_install_e2e_test.go b/cmd/wfctl/plugin_install_e2e_test.go index 84b69305..56609562 100644 --- a/cmd/wfctl/plugin_install_e2e_test.go +++ b/cmd/wfctl/plugin_install_e2e_test.go @@ -574,3 +574,285 @@ func TestDownloadURL(t *testing.T) { } }) } + +// TestParseGitHubRepoURL tests the parseGitHubRepoURL helper. +func TestParseGitHubRepoURL(t *testing.T) { + tests := []struct { + input string + wantOwner string + wantRepo string + wantErr bool + }{ + { + input: "https://github.com/GoCodeAlone/workflow-plugin-authz", + wantOwner: "GoCodeAlone", + wantRepo: "workflow-plugin-authz", + }, + { + input: "https://github.com/GoCodeAlone/workflow-plugin-authz/", + wantOwner: "GoCodeAlone", + wantRepo: "workflow-plugin-authz", + }, + { + input: "https://github.com/GoCodeAlone/workflow-plugin-authz.git", + wantOwner: "GoCodeAlone", + wantRepo: "workflow-plugin-authz", + }, + { + input: "http://github.com/owner/repo", + wantOwner: "owner", + wantRepo: "repo", + }, + { + input: "https://gitlab.com/owner/repo", + wantErr: true, + }, + { + input: "https://github.com/owner", + wantErr: true, + }, + { + input: "not-a-url", + wantErr: true, + }, + { + input: "https://github.com//repo", + wantErr: true, + }, + { + // Extra path segments must be rejected. + input: "https://github.com/owner/repo/tree/main", + wantErr: true, + }, + { + // Extra path segments must be rejected. + input: "https://github.com/owner/repo/blob/main/README.md", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + owner, repo, err := parseGitHubRepoURL(tt.input) + if tt.wantErr { + if err == nil { + t.Errorf("expected error for %q, got owner=%q repo=%q", tt.input, owner, repo) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if owner != tt.wantOwner { + t.Errorf("owner: got %q, want %q", owner, tt.wantOwner) + } + if repo != tt.wantRepo { + t.Errorf("repo: got %q, want %q", repo, tt.wantRepo) + } + }) + } +} + +// TestFetchManifestFromRepoURL tests the fetchManifestFromRepoURL helper using +// an httptest server that serves a manifest.json. It overrides rawGitHubContentBaseURL +// to point at the test server so no real network calls are made. +func TestFetchManifestFromRepoURL(t *testing.T) { + const owner = "GoCodeAlone" + const repo = "workflow-plugin-authz" + + wantManifest := &RegistryManifest{ + Name: "workflow-plugin-authz", + Version: "1.2.0", + Author: "GoCodeAlone", + Description: "RBAC authorization plugin", + Type: "external", + Tier: "core", + License: "MIT", + } + manifestJSON, _ := json.Marshal(wantManifest) + + t.Run("success", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + wantPath := fmt.Sprintf("/%s/%s/main/manifest.json", owner, repo) + if r.URL.Path == wantPath { + w.Header().Set("Content-Type", "application/json") + w.Write(manifestJSON) //nolint:errcheck + return + } + http.NotFound(w, r) + })) + defer srv.Close() + + orig := rawGitHubContentBaseURL + rawGitHubContentBaseURL = srv.URL + defer func() { rawGitHubContentBaseURL = orig }() + + got, err := fetchManifestFromRepoURL(fmt.Sprintf("https://github.com/%s/%s", owner, repo)) + if err != nil { + t.Fatalf("fetchManifestFromRepoURL: %v", err) + } + if got.Name != wantManifest.Name { + t.Errorf("name: got %q, want %q", got.Name, wantManifest.Name) + } + if got.Version != wantManifest.Version { + t.Errorf("version: got %q, want %q", got.Version, wantManifest.Version) + } + }) + + t.Run("404 returns error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer srv.Close() + + orig := rawGitHubContentBaseURL + rawGitHubContentBaseURL = srv.URL + defer func() { rawGitHubContentBaseURL = orig }() + + _, err := fetchManifestFromRepoURL(fmt.Sprintf("https://github.com/%s/%s", owner, repo)) + if err == nil { + t.Fatal("expected error for 404 manifest, got nil") + } + }) + + t.Run("invalid repo URL returns error", func(t *testing.T) { + _, err := fetchManifestFromRepoURL("not-a-github-url") + if err == nil { + t.Fatal("expected error for invalid repo URL") + } + }) + + t.Run("extra path segments rejected", func(t *testing.T) { + _, err := fetchManifestFromRepoURL(fmt.Sprintf("https://github.com/%s/%s/tree/main", owner, repo)) + if err == nil { + t.Fatal("expected error for URL with extra path segments") + } + }) +} + +// TestPluginUpdateFallbackToRepo tests that runPluginUpdate falls back to the +// repository URL in plugin.json when the registry does not have the plugin. +// It uses an empty registry config (so all lookups fail) and overrides +// rawGitHubContentBaseURL to serve the manifest from a local httptest server. +func TestPluginUpdateFallbackToRepo(t *testing.T) { + const pluginName = "workflow-plugin-authz" + const owner = "GoCodeAlone" + binaryContent := []byte("#!/bin/sh\necho authz\n") + + // Build tarball for the "updated" plugin. + topDir := fmt.Sprintf("%s-%s-%s", pluginName, runtime.GOOS, runtime.GOARCH) + tarEntries := map[string][]byte{ + topDir + "/" + pluginName: binaryContent, + } + tarball := buildTarGz(t, tarEntries, 0755) + checksum := sha256Hex(tarball) + + // manifest served by the plugin's repo when registry lookup fails. + updatedManifest := &RegistryManifest{ + Name: pluginName, + Version: "2.0.0", + Author: "GoCodeAlone", + Description: "RBAC authorization plugin using Casbin", + Type: "external", + Tier: "core", + License: "MIT", + Capabilities: &RegistryCapabilities{ + ModuleTypes: []string{"authz.casbin"}, + StepTypes: []string{"step.authz_check_casbin"}, + }, + } + + // Single test server that: + // - /{owner}/{repo}/main/manifest.json → 200 with updatedManifest (simulates raw.githubusercontent.com) + // - /tarball → 200 with the plugin tarball (referenced in the manifest's downloads) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case fmt.Sprintf("/%s/%s/main/manifest.json", owner, pluginName): + // Fill in the tarball URL dynamically (srv.URL is known here). + m := *updatedManifest + m.Downloads = []PluginDownload{{ + OS: runtime.GOOS, + Arch: runtime.GOARCH, + URL: "/tarball", // relative, will be prefixed by srv.URL in the handler + SHA256: checksum, + }} + // Use absolute URL for the tarball. + m.Downloads[0].URL = "http://" + r.Host + "/tarball" + manifestJSON, _ := json.Marshal(&m) + w.Header().Set("Content-Type", "application/json") + w.Write(manifestJSON) //nolint:errcheck + case "/tarball": + w.Header().Set("Content-Type", "application/octet-stream") + w.Write(tarball) //nolint:errcheck + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + // Write an empty registry config file so the registry lookup always fails. + cfgDir := t.TempDir() + cfgFile := filepath.Join(cfgDir, "config.yaml") + if err := os.WriteFile(cfgFile, []byte("registries: []\n"), 0640); err != nil { + t.Fatalf("write config: %v", err) + } + + // Set up a plugins directory with a pre-installed plugin.json that has a + // repository field pointing to the real GitHub URL. rawGitHubContentBaseURL + // is overridden so fetchManifestFromRepoURL uses the test server instead. + pluginsDir := t.TempDir() + pluginDir := filepath.Join(pluginsDir, pluginName) + if err := os.MkdirAll(pluginDir, 0750); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + installedJSON := installedPluginJSON{ + Name: pluginName, + Version: "1.0.0", + Author: "GoCodeAlone", + Description: "RBAC authorization plugin using Casbin", + Type: "external", + Tier: "core", + License: "MIT", + Repository: fmt.Sprintf("https://github.com/%s/%s", owner, pluginName), + ModuleTypes: []string{"authz.casbin"}, + } + installedJSONBytes, _ := json.Marshal(installedJSON) + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.json"), installedJSONBytes, 0640); err != nil { + t.Fatalf("write plugin.json: %v", err) + } + + // Override rawGitHubContentBaseURL to route manifest fetches to the test server. + orig := rawGitHubContentBaseURL + rawGitHubContentBaseURL = srv.URL + defer func() { rawGitHubContentBaseURL = orig }() + + // Run the update command. It should fail the registry lookup (empty registry) + // and fall back to fetching the manifest from the repository URL. + err := runPluginUpdate([]string{ + "-plugin-dir", pluginsDir, + "-config", cfgFile, + pluginName, + }) + if err != nil { + t.Fatalf("runPluginUpdate: %v", err) + } + + // Verify the binary was installed. + binaryPath := filepath.Join(pluginDir, pluginName) + if _, statErr := os.Stat(binaryPath); statErr != nil { + t.Fatalf("expected binary at %s: %v", binaryPath, statErr) + } + + // Verify the plugin.json was updated to the new version. + data, err := os.ReadFile(filepath.Join(pluginDir, "plugin.json")) + if err != nil { + t.Fatalf("read updated plugin.json: %v", err) + } + var pj installedPluginJSON + if err := json.Unmarshal(data, &pj); err != nil { + t.Fatalf("unmarshal plugin.json: %v", err) + } + if pj.Version != "2.0.0" { + t.Errorf("plugin.json version: got %q, want %q", pj.Version, "2.0.0") + } +} diff --git a/cmd/wfctl/plugin_install_test.go b/cmd/wfctl/plugin_install_test.go index 4ea0d1fa..82952ecc 100644 --- a/cmd/wfctl/plugin_install_test.go +++ b/cmd/wfctl/plugin_install_test.go @@ -64,4 +64,3 @@ func TestPluginListAcceptsLegacyDataDirFlag(t *testing.T) { t.Errorf("-data-dir: runPluginList returned unexpected error: %v", err) } } -