|
| 1 | +package commands |
| 2 | + |
| 3 | +import ( |
| 4 | + "context" |
| 5 | + "os" |
| 6 | + "path/filepath" |
| 7 | + "testing" |
| 8 | + |
| 9 | + "github.com/stretchr/testify/assert" |
| 10 | + "github.com/stretchr/testify/require" |
| 11 | + |
| 12 | + "github.com/basecamp/basecamp-cli/internal/harness" |
| 13 | +) |
| 14 | + |
| 15 | +// TestValidPluginScope guards the argv-injection whitelist: scope values come |
| 16 | +// from installed_plugins.json (not first-party), so a "-"-leading value must |
| 17 | +// not reach `claude plugin uninstall --scope <scope>`. |
| 18 | +func TestValidPluginScope(t *testing.T) { |
| 19 | + valid := []string{"user", "project", "local"} |
| 20 | + for _, s := range valid { |
| 21 | + if !validPluginScope(s) { |
| 22 | + t.Errorf("validPluginScope(%q) = false, want true", s) |
| 23 | + } |
| 24 | + } |
| 25 | + |
| 26 | + // "global" is not a scope `claude plugin uninstall --scope` accepts, so it |
| 27 | + // must be treated as invalid: keeping it valid would make a scoped uninstall |
| 28 | + // fail silently while suppressing the unscoped fallback, stranding the plugin. |
| 29 | + invalid := []string{"", "global", "-rf", "--force", "User", "system", "/etc", "user ", " user"} |
| 30 | + for _, s := range invalid { |
| 31 | + if validPluginScope(s) { |
| 32 | + t.Errorf("validPluginScope(%q) = true, want false", s) |
| 33 | + } |
| 34 | + } |
| 35 | +} |
| 36 | + |
| 37 | +// stubClaudeUninstall writes a stub `claude` binary that logs every invocation |
| 38 | +// to logFile and exits with failScoped/failUnscoped for uninstall calls. The |
| 39 | +// first unscoped uninstall succeeds (so the retry loop runs once) and repeats |
| 40 | +// fail, mirroring real "entry gone" behavior. Returns its absolute path. |
| 41 | +func stubClaudeUninstall(t *testing.T, failScoped bool) string { |
| 42 | + t.Helper() |
| 43 | + dir := t.TempDir() |
| 44 | + logFile := filepath.Join(dir, "calls.log") |
| 45 | + markerDir := filepath.Join(dir, "markers") |
| 46 | + require.NoError(t, os.MkdirAll(markerDir, 0o755)) |
| 47 | + |
| 48 | + scopedExit := "0" |
| 49 | + if failScoped { |
| 50 | + scopedExit = "1" |
| 51 | + } |
| 52 | + script := "#!/bin/sh\n" + |
| 53 | + "echo \"$*\" >> \"" + logFile + "\"\n" + |
| 54 | + "case \"$1 $2\" in\n" + |
| 55 | + " \"plugin uninstall\")\n" + |
| 56 | + " if [ \"$4\" = \"--scope\" ]; then exit " + scopedExit + "; fi\n" + |
| 57 | + // unscoped: succeed once per key, then fail so the retry loop ends. |
| 58 | + " MARKER=\"" + markerDir + "/$3.removed\"\n" + |
| 59 | + " if [ ! -f \"$MARKER\" ]; then > \"$MARKER\"; exit 0; fi\n" + |
| 60 | + " exit 1\n" + |
| 61 | + " ;;\n" + |
| 62 | + " *) exit 0 ;;\n" + |
| 63 | + "esac\n" |
| 64 | + path := filepath.Join(dir, "claude") |
| 65 | + require.NoError(t, os.WriteFile(path, []byte(script), 0o755)) //nolint:gosec // G306: test helper |
| 66 | + return path |
| 67 | +} |
| 68 | + |
| 69 | +func readClaudeCalls(t *testing.T, claudePath string) string { |
| 70 | + t.Helper() |
| 71 | + data, err := os.ReadFile(filepath.Join(filepath.Dir(claudePath), "calls.log")) |
| 72 | + if os.IsNotExist(err) { |
| 73 | + return "" |
| 74 | + } |
| 75 | + require.NoError(t, err) |
| 76 | + return string(data) |
| 77 | +} |
| 78 | + |
| 79 | +// TestRemoveStaleClaudePluginsAllScopesInvalid verifies the YL7 fix: when every |
| 80 | +// recorded scope fails validPluginScope (no scoped uninstall is attempted), we |
| 81 | +// fall back to an unscoped removal so the plugin isn't silently left installed. |
| 82 | +func TestRemoveStaleClaudePluginsAllScopesInvalid(t *testing.T) { |
| 83 | + claude := stubClaudeUninstall(t, false) |
| 84 | + plugins := []harness.StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"-rf", "--force"}}} |
| 85 | + |
| 86 | + removed, scopes := removeStaleClaudePlugins(context.Background(), claude, plugins) |
| 87 | + |
| 88 | + calls := readClaudeCalls(t, claude) |
| 89 | + assert.NotContains(t, calls, "--scope", "no scoped uninstall should be attempted for invalid scopes") |
| 90 | + assert.Contains(t, calls, "plugin uninstall basecamp@37signals", "unscoped fallback should run") |
| 91 | + assert.Equal(t, []string{"basecamp@37signals"}, removed) |
| 92 | + assert.Empty(t, scopes) |
| 93 | +} |
| 94 | + |
| 95 | +// TestRemoveStaleClaudePluginsGlobalScopeFallsBack verifies that a stale entry |
| 96 | +// whose only recorded scope is "global" (which `claude plugin uninstall --scope` |
| 97 | +// rejects) is treated as all-invalid, so the unscoped fallback removes it rather |
| 98 | +// than leaving it silently installed behind a failing scoped uninstall. |
| 99 | +func TestRemoveStaleClaudePluginsGlobalScopeFallsBack(t *testing.T) { |
| 100 | + claude := stubClaudeUninstall(t, false) |
| 101 | + plugins := []harness.StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"global"}}} |
| 102 | + |
| 103 | + removed, scopes := removeStaleClaudePlugins(context.Background(), claude, plugins) |
| 104 | + |
| 105 | + calls := readClaudeCalls(t, claude) |
| 106 | + assert.NotContains(t, calls, "--scope", "no scoped uninstall should be attempted for a global scope") |
| 107 | + assert.Contains(t, calls, "plugin uninstall basecamp@37signals", "unscoped fallback should run") |
| 108 | + assert.Equal(t, []string{"basecamp@37signals"}, removed) |
| 109 | + assert.Empty(t, scopes) |
| 110 | +} |
| 111 | + |
| 112 | +// TestRemoveStaleClaudePluginsValidScopeUninstallFails verifies the regression |
| 113 | +// fix: when scopes are VALID but the scoped uninstall fails at runtime, we must |
| 114 | +// NOT fall back to an unscoped removal (which would wrongly strip every scope). |
| 115 | +func TestRemoveStaleClaudePluginsValidScopeUninstallFails(t *testing.T) { |
| 116 | + claude := stubClaudeUninstall(t, true) |
| 117 | + plugins := []harness.StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"user", "project"}}} |
| 118 | + |
| 119 | + removed, scopes := removeStaleClaudePlugins(context.Background(), claude, plugins) |
| 120 | + |
| 121 | + calls := readClaudeCalls(t, claude) |
| 122 | + assert.Contains(t, calls, "plugin uninstall basecamp@37signals --scope user") |
| 123 | + assert.Contains(t, calls, "plugin uninstall basecamp@37signals --scope project") |
| 124 | + assert.NotContains(t, calls, "plugin uninstall basecamp@37signals\n", |
| 125 | + "no unscoped fallback when valid scopes were attempted but failed") |
| 126 | + assert.Empty(t, removed) |
| 127 | + assert.Empty(t, scopes) |
| 128 | +} |
0 commit comments