From b93fc4d3512900601b32faa22ffc84432da6ca8e Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Thu, 21 May 2026 01:33:49 -0400 Subject: [PATCH] fix(wfctl): preserve required_secrets through install path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wfctl secrets setup --plugin ` reads required_secrets[] from the on-disk plugin.json. Two struct gaps were dropping the field along the install path: 1. cmd/wfctl/plugin_registry.go: RegistryManifest had no RequiredSecrets field, so json.Unmarshal silently discarded the upstream block at fetch time. 2. cmd/wfctl/plugin_install.go: installedPluginJSON likewise had no RequiredSecrets field, so writeInstalledManifest couldn't carry it through even if (1) was fixed. Both filled in with the existing PluginRequiredSecret type from secrets_setup_plugin.go. writeInstalledManifest now copies m.RequiredSecrets → pj.RequiredSecrets so the on-disk plugin.json preserves the block. Observed in practice via workflow-plugin-hover v0.2.0: - registry manifest at plugins/hover/manifest.json had required_secrets[3] - installed plugin.json at data/plugins/hover/plugin.json had no required_secrets field - wfctl secrets setup --plugin hover --scope repo reported "declares no required_secrets[]; nothing to do" Regression tests: - TestRegistryManifest_UnmarshalPreservesRequiredSecrets - TestWriteInstalledManifest_PreservesRequiredSecrets Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/wfctl/plugin_install.go | 28 ++++++++----- cmd/wfctl/plugin_install_test.go | 72 ++++++++++++++++++++++++++++++++ cmd/wfctl/plugin_registry.go | 7 ++++ 3 files changed, 97 insertions(+), 10 deletions(-) diff --git a/cmd/wfctl/plugin_install.go b/cmd/wfctl/plugin_install.go index e68d5fef..4d94c55f 100644 --- a/cmd/wfctl/plugin_install.go +++ b/cmd/wfctl/plugin_install.go @@ -1377,6 +1377,13 @@ type installedPluginJSON struct { Type string `json:"type,omitempty"` Capabilities *installedPluginCapabilities `json:"capabilities,omitempty"` IaCProvider *RegistryIaCProvider `json:"iacProvider,omitempty"` + // RequiredSecrets carries through from the registry manifest so + // `wfctl secrets setup --plugin ` can find the declared + // secrets in the on-disk plugin.json. Earlier writeInstalledManifest + // versions dropped this field, leaving secrets setup --plugin + // reporting "declares no required_secrets[]" even when the + // upstream manifest declared it. + RequiredSecrets []PluginRequiredSecret `json:"required_secrets,omitempty"` } type installedPluginCapabilities struct { @@ -1401,16 +1408,17 @@ type installedPluginCapabilities struct { // plugin.PluginManifest so that ExternalPluginManager.LoadPlugin() can validate it. func writeInstalledManifest(path string, m *RegistryManifest) error { pj := installedPluginJSON{ - Name: m.Name, - Version: m.Version, - Author: m.Author, - Description: m.Description, - License: m.License, - Repository: m.Repository, - Tier: m.Tier, - Tags: m.Keywords, - Type: m.Type, - IaCProvider: m.IaCProvider, + Name: m.Name, + Version: m.Version, + Author: m.Author, + Description: m.Description, + License: m.License, + Repository: m.Repository, + Tier: m.Tier, + Tags: m.Keywords, + Type: m.Type, + IaCProvider: m.IaCProvider, + RequiredSecrets: m.RequiredSecrets, } if m.Capabilities != nil { pj.Capabilities = &installedPluginCapabilities{ diff --git a/cmd/wfctl/plugin_install_test.go b/cmd/wfctl/plugin_install_test.go index 8a4cd1ff..f09fe90f 100644 --- a/cmd/wfctl/plugin_install_test.go +++ b/cmd/wfctl/plugin_install_test.go @@ -1182,3 +1182,75 @@ func writeCompatRegistryIndex(t *testing.T, w http.ResponseWriter, plugin, baseU w.Header().Set("Content-Type", "application/json") _, _ = w.Write(data) } + +func TestWriteInstalledManifest_PreservesRequiredSecrets(t *testing.T) { + // G4 regression: writeInstalledManifest used to drop the + // required_secrets[] block from the registry manifest, so the + // on-disk plugin.json had no required_secrets even when upstream + // declared them. `wfctl secrets setup --plugin ` then + // reported "declares no required_secrets[]" no-op. + m := &RegistryManifest{ + Name: "workflow-plugin-hover", Version: "v0.2.0", + Author: "GoCodeAlone", Description: "test", + RequiredSecrets: []PluginRequiredSecret{ + {Name: "HOVER_USERNAME", Sensitive: false, Description: "Hover username", Prompt: "Hover username"}, + {Name: "HOVER_PASSWORD", Sensitive: true, Description: "Hover password"}, + }, + } + dir := t.TempDir() + path := filepath.Join(dir, "plugin.json") + if err := writeInstalledManifest(path, m); err != nil { + t.Fatalf("writeInstalledManifest: %v", err) + } + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read back: %v", err) + } + var pj installedPluginJSON + if err := json.Unmarshal(raw, &pj); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(pj.RequiredSecrets) != 2 { + t.Fatalf("required_secrets len = %d, want 2", len(pj.RequiredSecrets)) + } + if pj.RequiredSecrets[0].Name != "HOVER_USERNAME" { + t.Errorf("required_secrets[0].name = %q, want HOVER_USERNAME", pj.RequiredSecrets[0].Name) + } + if !pj.RequiredSecrets[1].Sensitive { + t.Errorf("required_secrets[1].sensitive = false; want true (HOVER_PASSWORD)") + } + // Also assert the raw JSON contains the field — guards against a + // future installedPluginJSON refactor that adds Sensitive omitempty + // or otherwise hides the field at marshal time. + if !strings.Contains(string(raw), "\"required_secrets\":") { + t.Errorf("installed plugin.json missing required_secrets[] key:\n%s", string(raw)) + } +} + +func TestRegistryManifest_UnmarshalPreservesRequiredSecrets(t *testing.T) { + // G4 regression: RegistryManifest dropped required_secrets[] at + // json.Unmarshal time because the struct didn't carry the field. + src := `{ + "name": "workflow-plugin-hover", + "version": "0.2.0", + "author": "GoCodeAlone", + "description": "test", + "type": "external", + "tier": "community", + "license": "MIT", + "required_secrets": [ + {"name": "X", "sensitive": false, "description": "d", "prompt": "p"}, + {"name": "Y", "sensitive": true} + ] + }` + var m RegistryManifest + if err := json.Unmarshal([]byte(src), &m); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(m.RequiredSecrets) != 2 { + t.Fatalf("required_secrets len = %d, want 2", len(m.RequiredSecrets)) + } + if m.RequiredSecrets[1].Name != "Y" || !m.RequiredSecrets[1].Sensitive { + t.Errorf("unexpected required_secrets[1]: %+v", m.RequiredSecrets[1]) + } +} diff --git a/cmd/wfctl/plugin_registry.go b/cmd/wfctl/plugin_registry.go index 8f08b3ea..26b0afaf 100644 --- a/cmd/wfctl/plugin_registry.go +++ b/cmd/wfctl/plugin_registry.go @@ -44,6 +44,13 @@ type RegistryManifest struct { Downloads []PluginDownload `json:"downloads,omitempty"` Assets *PluginAssets `json:"assets,omitempty"` Dependencies []PluginDependency `json:"dependencies,omitempty"` + // RequiredSecrets mirrors the plugin.json `required_secrets[]` + // block. Consumed by `wfctl secrets setup --plugin ` to + // prompt for + write each secret to the chosen GitHub scope. + // Without this field on the registry struct, the secrets[] list + // would be silently dropped at unmarshal time even when the + // upstream manifest declared it. + RequiredSecrets []PluginRequiredSecret `json:"required_secrets,omitempty"` } // RegistryCapabilities describes what module/step/trigger types a plugin provides.