From c851f12e96cd053e7d70ff420f7c847b8d320851 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Mon, 17 Aug 2026 10:12:28 -0500 Subject: [PATCH 1/6] test(cli): drift guard that destructive commands require confirmation (DX-948) Enumerate commands whose verb mutates or removes real state (delete/revoke/refund/cancel/transfer/grant/push/publish/unpublish/apply/ discard/simulate-purchase) and assert each routes through confirmOrAbort so a new unguarded destructive command fails CI. A static AST guard follows helper delegation (e.g. products store apply -> applyStoreStatePlan); a behavioral test drives real commands and proves each refuses under --no-input without --yes before any state-changing call, and that --yes lets it through. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/destructive_guard_test.go | 251 +++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 internal/cli/destructive_guard_test.go diff --git a/internal/cli/destructive_guard_test.go b/internal/cli/destructive_guard_test.go new file mode 100644 index 00000000..df378e94 --- /dev/null +++ b/internal/cli/destructive_guard_test.go @@ -0,0 +1,251 @@ +package cli_test + +import ( + "go/ast" + "go/parser" + "go/token" + "io/fs" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/revenuecat/cli/internal/config" +) + +// Reversible, soft state changes (archive/restore/set-current/enable/disable) +// are deliberately excluded — the codebase documents those as "no prompt". +var destructiveVerbs = map[string]bool{ + "delete": true, + "revoke": true, + "refund": true, + "cancel": true, + "transfer": true, + "grant": true, + "push": true, + "publish": true, + "unpublish": true, + "apply": true, + "discard": true, + "simulate-purchase": true, +} + +func TestDestructiveCommands_RouteThroughConfirmOrAbort(t *testing.T) { + fset := token.NewFileSet() + pkgs, err := parser.ParseDir(fset, ".", func(fi fs.FileInfo) bool { + return !strings.HasSuffix(fi.Name(), "_test.go") + }, 0) + if err != nil { + t.Fatalf("parse package: %v", err) + } + + // Which named functions ask for consent — directly or transitively — so a + // command whose RunE delegates the prompt to a helper still counts as + // guarded (e.g. `products store apply` → applyStoreStatePlan). + guarding := guardingFuncs(pkgs) + + found := 0 + for _, pkg := range pkgs { + for _, file := range pkg.Files { + ast.Inspect(file, func(n ast.Node) bool { + lit, ok := n.(*ast.CompositeLit) + if !ok || !isCobraCommand(lit.Type) { + return true + } + verb, runE := commandVerbAndRunE(lit) + if !destructiveVerbs[verb] { + return true + } + found++ + if !runEGuarded(runE, guarding) { + pos := fset.Position(lit.Pos()) + t.Errorf("%s: destructive command %q reaches its action without confirmOrAbort — gate it via confirmOrAbort(rt, ...) so --yes/--no-input behave uniformly", pos, verb) + } + return true + }) + } + } + if found == 0 { + t.Fatal("no destructive commands found — the guard is not scanning the command definitions") + } +} + +// guardingFuncs returns the names of functions whose bodies reach confirmOrAbort, +// following one function calling another to a fixpoint. +func guardingFuncs(pkgs map[string]*ast.Package) map[string]bool { + calls := map[string]map[string]bool{} + for _, pkg := range pkgs { + for _, file := range pkg.Files { + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Body == nil { + continue + } + calls[fn.Name.Name] = calledIdents(fn.Body) + } + } + } + guarding := map[string]bool{} + for name, callees := range calls { + if callees["confirmOrAbort"] { + guarding[name] = true + } + } + for changed := true; changed; { + changed = false + for name, callees := range calls { + if guarding[name] { + continue + } + for callee := range callees { + if guarding[callee] { + guarding[name] = true + changed = true + break + } + } + } + } + return guarding +} + +func calledIdents(n ast.Node) map[string]bool { + set := map[string]bool{} + ast.Inspect(n, func(nn ast.Node) bool { + if call, ok := nn.(*ast.CallExpr); ok { + if id, ok := call.Fun.(*ast.Ident); ok { + set[id.Name] = true + } + } + return true + }) + return set +} + +func isCobraCommand(t ast.Expr) bool { + sel, ok := t.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Command" { + return false + } + pkg, ok := sel.X.(*ast.Ident) + return ok && pkg.Name == "cobra" +} + +func commandVerbAndRunE(lit *ast.CompositeLit) (verb string, runE ast.Expr) { + for _, elt := range lit.Elts { + kv, ok := elt.(*ast.KeyValueExpr) + if !ok { + continue + } + key, ok := kv.Key.(*ast.Ident) + if !ok { + continue + } + switch key.Name { + case "Use": + if bl, ok := kv.Value.(*ast.BasicLit); ok && bl.Kind == token.STRING { + if use, err := strconv.Unquote(bl.Value); err == nil { + if fields := strings.Fields(use); len(fields) > 0 { + verb = fields[0] + } + } + } + case "RunE": + runE = kv.Value + } + } + return verb, runE +} + +func runEGuarded(runE ast.Expr, guarding map[string]bool) bool { + switch v := runE.(type) { + case *ast.FuncLit: + for callee := range calledIdents(v.Body) { + if callee == "confirmOrAbort" || guarding[callee] { + return true + } + } + case *ast.Ident: + return guarding[v.Name] + } + return false +} + +// The fake server allows read-only GET preflight (some commands fetch state to +// decide whether extra confirmation is needed) but fails the test on any write, +// proving nothing is destroyed before consent. +func TestDestructiveCommands_RefuseUnderNoInputWithoutYes(t *testing.T) { + commands := [][]string{ + {"apps", "delete", "app_x"}, + {"products", "delete", "prod_x"}, + {"products", "push", "prod_x"}, + {"paywalls", "delete", "pw_x"}, + {"paywalls", "publish", "pw_x"}, + {"paywalls", "unpublish", "pw_x"}, + {"offerings", "delete", "ofrng_x"}, + {"packages", "delete", "pkg_x"}, + {"entitlements", "delete", "ent_x"}, + {"webhooks", "delete", "wh_x"}, + {"purchases", "refund", "txn_x"}, + {"subscriptions", "cancel", "sub_x"}, + {"subscriptions", "refund", "sub_x"}, + {"products", "store", "discard", "plan_x"}, + {"customer", "revoke", "cust_x", "ent_x"}, + } + for _, args := range commands { + t.Run(strings.Join(args, " "), func(t *testing.T) { + configDir := t.TempDir() + t.Setenv("RC_CONFIG_DIR", configDir) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Errorf("%s %s: state-changing call reached the network before confirmation", r.Method, r.URL.Path) + http.Error(w, "unexpected write before confirmation", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{}`)) + })) + t.Cleanup(server.Close) + if err := config.Save("", &config.Config{APIKey: "sk_test", ProjectID: "proj_test", BaseURL: server.URL}); err != nil { + t.Fatal(err) + } + + runArgs := append(append([]string{}, args...), "--no-input") + _, _, err := runCmdInConfigDir(t, configDir, runArgs...) + if err == nil { + t.Fatal("want refusal under --no-input without --yes") + } + if !strings.Contains(err.Error(), "pass --yes") { + t.Fatalf("want the confirmation gate error, got: %v", err) + } + }) + } +} + +func TestDestructiveCommand_YesBypassesConfirmation(t *testing.T) { + configDir := t.TempDir() + t.Setenv("RC_CONFIG_DIR", configDir) + var deleted bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete && strings.HasSuffix(r.URL.Path, "/apps/app_x") { + deleted = true + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{}`)) + return + } + http.Error(w, "unexpected request", http.StatusNotFound) + })) + t.Cleanup(server.Close) + if err := config.Save("", &config.Config{APIKey: "sk_test", ProjectID: "proj_test", BaseURL: server.URL}); err != nil { + t.Fatal(err) + } + + _, errb, err := runCmdInConfigDir(t, configDir, "apps", "delete", "app_x", "--yes", "--no-input") + if err != nil { + t.Fatalf("--yes should let the delete proceed: %v\nstderr: %s", err, errb) + } + if !deleted { + t.Fatal("--yes did not let the command reach the store delete call") + } +} From a8452793662a2c2be70a499e2b343a687a82e518 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Mon, 17 Aug 2026 12:38:22 -0500 Subject: [PATCH 2/6] test(cli): drive customer grant and transfer through the behavioral guard The verb list named grant and transfer but the behavioral test never drove them, so a regression that confirmed after mutating would pass CI. Both now run under --no-input and must hit the confirmation gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/destructive_guard_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/cli/destructive_guard_test.go b/internal/cli/destructive_guard_test.go index df378e94..45ec493e 100644 --- a/internal/cli/destructive_guard_test.go +++ b/internal/cli/destructive_guard_test.go @@ -192,6 +192,8 @@ func TestDestructiveCommands_RefuseUnderNoInputWithoutYes(t *testing.T) { {"subscriptions", "refund", "sub_x"}, {"products", "store", "discard", "plan_x"}, {"customer", "revoke", "cust_x", "ent_x"}, + {"customer", "grant", "cust_x", "ent_x", "--duration", "monthly"}, + {"customer", "transfer", "cust_x", "--to", "cust_y"}, } for _, args := range commands { t.Run(strings.Join(args, " "), func(t *testing.T) { From 6de8e2dbfd814ac13f127cffe83a4acc21b0b68d Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Mon, 17 Aug 2026 16:24:41 -0500 Subject: [PATCH 3/6] test(cli): drop the AST guard, keep the behavioral destructive-command check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The static AST enumeration looked thorough but had silent blind spots (variable Use strings, verb-list gaps, defeatable by refactoring) — false confidence. The behavioral test that drives each destructive command and asserts it refuses under --no-input without --yes (and --yes bypasses) is the honest guard; new destructive commands are covered by adding them to the driven list. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/destructive_guard_test.go | 167 +------------------------ 1 file changed, 4 insertions(+), 163 deletions(-) diff --git a/internal/cli/destructive_guard_test.go b/internal/cli/destructive_guard_test.go index 45ec493e..b2d4c134 100644 --- a/internal/cli/destructive_guard_test.go +++ b/internal/cli/destructive_guard_test.go @@ -1,177 +1,18 @@ package cli_test import ( - "go/ast" - "go/parser" - "go/token" - "io/fs" "net/http" "net/http/httptest" - "strconv" "strings" "testing" "github.com/revenuecat/cli/internal/config" ) -// Reversible, soft state changes (archive/restore/set-current/enable/disable) -// are deliberately excluded — the codebase documents those as "no prompt". -var destructiveVerbs = map[string]bool{ - "delete": true, - "revoke": true, - "refund": true, - "cancel": true, - "transfer": true, - "grant": true, - "push": true, - "publish": true, - "unpublish": true, - "apply": true, - "discard": true, - "simulate-purchase": true, -} - -func TestDestructiveCommands_RouteThroughConfirmOrAbort(t *testing.T) { - fset := token.NewFileSet() - pkgs, err := parser.ParseDir(fset, ".", func(fi fs.FileInfo) bool { - return !strings.HasSuffix(fi.Name(), "_test.go") - }, 0) - if err != nil { - t.Fatalf("parse package: %v", err) - } - - // Which named functions ask for consent — directly or transitively — so a - // command whose RunE delegates the prompt to a helper still counts as - // guarded (e.g. `products store apply` → applyStoreStatePlan). - guarding := guardingFuncs(pkgs) - - found := 0 - for _, pkg := range pkgs { - for _, file := range pkg.Files { - ast.Inspect(file, func(n ast.Node) bool { - lit, ok := n.(*ast.CompositeLit) - if !ok || !isCobraCommand(lit.Type) { - return true - } - verb, runE := commandVerbAndRunE(lit) - if !destructiveVerbs[verb] { - return true - } - found++ - if !runEGuarded(runE, guarding) { - pos := fset.Position(lit.Pos()) - t.Errorf("%s: destructive command %q reaches its action without confirmOrAbort — gate it via confirmOrAbort(rt, ...) so --yes/--no-input behave uniformly", pos, verb) - } - return true - }) - } - } - if found == 0 { - t.Fatal("no destructive commands found — the guard is not scanning the command definitions") - } -} - -// guardingFuncs returns the names of functions whose bodies reach confirmOrAbort, -// following one function calling another to a fixpoint. -func guardingFuncs(pkgs map[string]*ast.Package) map[string]bool { - calls := map[string]map[string]bool{} - for _, pkg := range pkgs { - for _, file := range pkg.Files { - for _, decl := range file.Decls { - fn, ok := decl.(*ast.FuncDecl) - if !ok || fn.Body == nil { - continue - } - calls[fn.Name.Name] = calledIdents(fn.Body) - } - } - } - guarding := map[string]bool{} - for name, callees := range calls { - if callees["confirmOrAbort"] { - guarding[name] = true - } - } - for changed := true; changed; { - changed = false - for name, callees := range calls { - if guarding[name] { - continue - } - for callee := range callees { - if guarding[callee] { - guarding[name] = true - changed = true - break - } - } - } - } - return guarding -} - -func calledIdents(n ast.Node) map[string]bool { - set := map[string]bool{} - ast.Inspect(n, func(nn ast.Node) bool { - if call, ok := nn.(*ast.CallExpr); ok { - if id, ok := call.Fun.(*ast.Ident); ok { - set[id.Name] = true - } - } - return true - }) - return set -} - -func isCobraCommand(t ast.Expr) bool { - sel, ok := t.(*ast.SelectorExpr) - if !ok || sel.Sel.Name != "Command" { - return false - } - pkg, ok := sel.X.(*ast.Ident) - return ok && pkg.Name == "cobra" -} - -func commandVerbAndRunE(lit *ast.CompositeLit) (verb string, runE ast.Expr) { - for _, elt := range lit.Elts { - kv, ok := elt.(*ast.KeyValueExpr) - if !ok { - continue - } - key, ok := kv.Key.(*ast.Ident) - if !ok { - continue - } - switch key.Name { - case "Use": - if bl, ok := kv.Value.(*ast.BasicLit); ok && bl.Kind == token.STRING { - if use, err := strconv.Unquote(bl.Value); err == nil { - if fields := strings.Fields(use); len(fields) > 0 { - verb = fields[0] - } - } - } - case "RunE": - runE = kv.Value - } - } - return verb, runE -} - -func runEGuarded(runE ast.Expr, guarding map[string]bool) bool { - switch v := runE.(type) { - case *ast.FuncLit: - for callee := range calledIdents(v.Body) { - if callee == "confirmOrAbort" || guarding[callee] { - return true - } - } - case *ast.Ident: - return guarding[v.Name] - } - return false -} - +// Every command that removes or mutates real state must refuse to act under +// --no-input unless --yes is passed, and --yes must let it through. This drives +// the real commands; a new destructive command is covered by adding it here. +// // The fake server allows read-only GET preflight (some commands fetch state to // decide whether extra confirmation is needed) but fails the test on any write, // proving nothing is destroyed before consent. From 95f65c3052c39372a345e06f2aa3d358ce36d8f0 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Wed, 19 Aug 2026 12:02:09 -0500 Subject: [PATCH 4/6] feat(cli): make the irreversible deletes interactive-only apps/paywalls/offerings/entitlements delete now require a person at a real terminal: --json, --no-input, and non-TTY are refused, and --yes can't bypass it, so automation can't fire the deletes that are hardest to undo. Adds a shared requireInteractive gate next to confirmOrAbort and extends the destructive-guard test with a second tier: destructive commands still allow --yes, interactive-only commands refuse even with it. Extracts the paywall force-check so it stays unit-tested. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/apps.go | 9 ++- internal/cli/confirm.go | 15 +++++ internal/cli/destructive_guard_test.go | 56 +++++++++++++++--- internal/cli/entitlements.go | 9 ++- internal/cli/offerings.go | 9 ++- internal/cli/paywalls.go | 48 ++++++++++----- internal/cli/paywalls_delete_test.go | 81 +++++++++----------------- 7 files changed, 141 insertions(+), 86 deletions(-) diff --git a/internal/cli/apps.go b/internal/cli/apps.go index 999ecb72..7ccf1e0d 100644 --- a/internal/cli/apps.go +++ b/internal/cli/apps.go @@ -351,11 +351,16 @@ but no longer associated with this app. Reversibility: irreversible. -Confirmation: prompts under TTY; pass --yes to skip. Required under --no-input.`, - Example: ` rc apps delete app_old --yes`, +Interactive-only: run it yourself in a terminal. It is unavailable under +--json or --no-input so automation can't delete apps. --yes skips the +confirmation prompt once you're in a terminal.`, + Example: ` rc apps delete app_old`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { rt := RuntimeFrom(cmd.Context()) + if err := requireInteractive(rt, cmd.CommandPath()); err != nil { + return err + } projectID, err := requireProject(rt) if err != nil { return err diff --git a/internal/cli/confirm.go b/internal/cli/confirm.go index a47e1fc2..4fdf6010 100644 --- a/internal/cli/confirm.go +++ b/internal/cli/confirm.go @@ -6,6 +6,21 @@ import ( "github.com/revenuecat/cli/internal/tui" ) +// requireInteractive gates the most destructive, irreversible commands to a +// person at a real terminal. Unlike confirmOrAbort, --yes cannot bypass it and +// there is no non-interactive path: --json, --no-input, and non-TTY sessions +// are all refused. This keeps automation from firing deletes that are hard or +// impossible to undo — those stay a deliberate, human-only action. +func requireInteractive(rt *Runtime, action string) error { + if rt.CanPrompt() { + return nil + } + return WithHint( + fmt.Errorf("%s is interactive-only and can't run with --json or --no-input", action), + "Run it yourself in an interactive terminal. It is intentionally unavailable to automation because it is irreversible.", + ) +} + // confirmOrAbort is the one way a command asks consent before acting. It owns // the full contract in one place — --yes skips it, --no-input without --yes // fails it, declining aborts with a uniform error — so no command can diff --git a/internal/cli/destructive_guard_test.go b/internal/cli/destructive_guard_test.go index b2d4c134..c581b739 100644 --- a/internal/cli/destructive_guard_test.go +++ b/internal/cli/destructive_guard_test.go @@ -17,16 +17,14 @@ import ( // decide whether extra confirmation is needed) but fails the test on any write, // proving nothing is destroyed before consent. func TestDestructiveCommands_RefuseUnderNoInputWithoutYes(t *testing.T) { + // The most destructive, irreversible commands are interactive-only and are + // covered separately below; --yes bypasses confirmation for everything here. commands := [][]string{ - {"apps", "delete", "app_x"}, {"products", "delete", "prod_x"}, {"products", "push", "prod_x"}, - {"paywalls", "delete", "pw_x"}, {"paywalls", "publish", "pw_x"}, {"paywalls", "unpublish", "pw_x"}, - {"offerings", "delete", "ofrng_x"}, {"packages", "delete", "pkg_x"}, - {"entitlements", "delete", "ent_x"}, {"webhooks", "delete", "wh_x"}, {"purchases", "refund", "txn_x"}, {"subscriptions", "cancel", "sub_x"}, @@ -71,24 +69,64 @@ func TestDestructiveCommand_YesBypassesConfirmation(t *testing.T) { t.Setenv("RC_CONFIG_DIR", configDir) var deleted bool server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodDelete && strings.HasSuffix(r.URL.Path, "/apps/app_x") { + switch { + case r.Method == http.MethodDelete && strings.HasSuffix(r.URL.Path, "/products/prod_x"): deleted = true w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{}`)) - return + case r.Method == http.MethodGet: + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{}`)) + default: + http.Error(w, "unexpected request", http.StatusNotFound) } - http.Error(w, "unexpected request", http.StatusNotFound) })) t.Cleanup(server.Close) if err := config.Save("", &config.Config{APIKey: "sk_test", ProjectID: "proj_test", BaseURL: server.URL}); err != nil { t.Fatal(err) } - _, errb, err := runCmdInConfigDir(t, configDir, "apps", "delete", "app_x", "--yes", "--no-input") + _, errb, err := runCmdInConfigDir(t, configDir, "products", "delete", "prod_x", "--yes", "--no-input") if err != nil { t.Fatalf("--yes should let the delete proceed: %v\nstderr: %s", err, errb) } if !deleted { - t.Fatal("--yes did not let the command reach the store delete call") + t.Fatal("--yes did not let the command reach the delete call") + } +} + +// The irreversible, customer-facing deletes are interactive-only: --yes does NOT +// buy a way through, and --json/--no-input are refused outright so automation +// can't fire them. A new human-only command is covered by adding it here. +func TestInteractiveOnlyCommands_RefuseEvenWithYes(t *testing.T) { + commands := [][]string{ + {"apps", "delete", "app_x"}, + {"paywalls", "delete", "pw_x"}, + {"offerings", "delete", "ofrng_x"}, + {"entitlements", "delete", "ent_x"}, + } + for _, args := range commands { + t.Run(strings.Join(args, " "), func(t *testing.T) { + configDir := t.TempDir() + t.Setenv("RC_CONFIG_DIR", configDir) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("%s %s: interactive-only command reached the network", r.Method, r.URL.Path) + http.Error(w, "must not touch the network", http.StatusInternalServerError) + })) + t.Cleanup(server.Close) + if err := config.Save("", &config.Config{APIKey: "sk_test", ProjectID: "proj_test", BaseURL: server.URL}); err != nil { + t.Fatal(err) + } + + // --yes present on purpose: it must NOT bypass the interactive gate. + runArgs := append(append([]string{}, args...), "--yes", "--no-input") + _, _, err := runCmdInConfigDir(t, configDir, runArgs...) + if err == nil { + t.Fatal("want refusal: interactive-only even with --yes") + } + if !strings.Contains(err.Error(), "interactive-only") { + t.Fatalf("want the interactive-only gate error, got: %v", err) + } + }) } } diff --git a/internal/cli/entitlements.go b/internal/cli/entitlements.go index db23c02c..8268c8ae 100644 --- a/internal/cli/entitlements.go +++ b/internal/cli/entitlements.go @@ -398,11 +398,16 @@ Reversibility: irreversible. If you only need to hide it from current Offerings, prefer ` + "`rc entitlements archive`" + ` which can be undone with ` + "`rc entitlements restore`" + `. -Confirmation: prompts under TTY; pass --yes to skip. Required under --no-input.`, - Example: ` rc entitlements delete entl_pro --yes`, +Interactive-only: run it yourself in a terminal. It is unavailable under +--json or --no-input so automation can't delete entitlements. --yes skips the +confirmation prompt once you're in a terminal.`, + Example: ` rc entitlements delete entl_pro`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { rt := RuntimeFrom(cmd.Context()) + if err := requireInteractive(rt, cmd.CommandPath()); err != nil { + return err + } projectID, err := requireProject(rt) if err != nil { return err diff --git a/internal/cli/offerings.go b/internal/cli/offerings.go index f7dcda4e..31bc2061 100644 --- a/internal/cli/offerings.go +++ b/internal/cli/offerings.go @@ -549,11 +549,16 @@ terminal to pick from a list. Reversibility: irreversible. Prefer ` + "`rc offerings archive`" + ` for reversible removal. -Confirmation: prompts under TTY; pass --yes to skip. Required under --no-input.`, - Example: ` rc offerings delete ofrng_default --yes`, +Interactive-only: run it yourself in a terminal. It is unavailable under +--json or --no-input so automation can't delete offerings. --yes skips the +confirmation prompt once you're in a terminal.`, + Example: ` rc offerings delete ofrng_default`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { rt := RuntimeFrom(cmd.Context()) + if err := requireInteractive(rt, cmd.CommandPath()); err != nil { + return err + } projectID, err := requireProject(rt) if err != nil { return err diff --git a/internal/cli/paywalls.go b/internal/cli/paywalls.go index e6c1cd1b..c4f9bd8a 100644 --- a/internal/cli/paywalls.go +++ b/internal/cli/paywalls.go @@ -427,12 +427,17 @@ unless --force is passed — --yes alone is not enough. It may be serving customers or be someone else's in-progress work; get explicit consent from the user first. -Confirmation: prompts under TTY; pass --yes to skip. Required under --no-input.`, - Example: ` rc paywalls delete pw_old --yes - rc paywalls delete pw_attached --force --yes`, +Interactive-only: run it yourself in a terminal. It is unavailable under +--json or --no-input so automation can't delete paywalls. --yes skips the +confirmation prompt once you're in a terminal.`, + Example: ` rc paywalls delete pw_old + rc paywalls delete pw_attached --force`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { rt := RuntimeFrom(cmd.Context()) + if err := requireInteractive(rt, cmd.CommandPath()); err != nil { + return err + } projectID, err := requireProject(rt) if err != nil { return err @@ -451,19 +456,8 @@ Confirmation: prompts under TTY; pass --yes to skip. Required under --no-input.` if err != nil { return err } - if !force { - switch { - case paywall.PublishedAt != nil: - return WithHint( - fmt.Errorf("paywall %s is published — customers may be seeing it", pickedID), - "Deletion is irreversible. Unpublish it first (rc paywalls unpublish "+pickedID+"), then detach it (rc paywalls detach "+pickedID+") if you only need to free the offering, or re-run with --force after the user explicitly confirms this paywall should be destroyed.", - ) - case paywall.OfferingID != "": - return WithHint( - fmt.Errorf("paywall %s is attached to offering %s and may be someone's in-progress work", pickedID, paywall.OfferingID), - "Deletion is irreversible. Re-run with --force only after the user explicitly confirms this paywall should be destroyed. To free the offering instead, detach this paywall (rc paywalls detach "+pickedID+") — it stays as a standalone draft.", - ) - } + if err := checkPaywallDeletable(paywall, pickedID, force); err != nil { + return err } if paywall.PublishedAt != nil { rt.Out.Warn("This paywall is published — customers may be seeing it.") @@ -485,6 +479,28 @@ Confirmation: prompts under TTY; pass --yes to skip. Required under --no-input.` return cmd } +// checkPaywallDeletable refuses to delete a published or attached paywall unless +// --force is passed: those may be serving customers or be someone's in-progress +// work, so deletion needs an explicit override on top of confirmation. +func checkPaywallDeletable(paywall *api.Paywall, id string, force bool) error { + if force { + return nil + } + switch { + case paywall.PublishedAt != nil: + return WithHint( + fmt.Errorf("paywall %s is published — customers may be seeing it", id), + "Deletion is irreversible. Unpublish it first (rc paywalls unpublish "+id+"), then detach it (rc paywalls detach "+id+") if you only need to free the offering, or re-run with --force after the user explicitly confirms this paywall should be destroyed.", + ) + case paywall.OfferingID != "": + return WithHint( + fmt.Errorf("paywall %s is attached to offering %s and may be someone's in-progress work", id, paywall.OfferingID), + "Deletion is irreversible. Re-run with --force only after the user explicitly confirms this paywall should be destroyed. To free the offering instead, detach this paywall (rc paywalls detach "+id+") — it stays as a standalone draft.", + ) + } + return nil +} + // wrapPaywallActionGateError explains the beta gate on the paywall // publish/unpublish v2 actions: they 404 with a bare "Resource not found" // for projects without beta API access. diff --git a/internal/cli/paywalls_delete_test.go b/internal/cli/paywalls_delete_test.go index ccf995be..6c6f8c22 100644 --- a/internal/cli/paywalls_delete_test.go +++ b/internal/cli/paywalls_delete_test.go @@ -1,64 +1,35 @@ package cli import ( - "bytes" - "context" - "io" - "net/http" - "net/http/httptest" - "strings" "testing" -) - -func TestPaywallsDeleteRequiresForceForAttachedOrPublished(t *testing.T) { - paywalls := map[string]string{ - "pw_attached": `{"object":"paywall","id":"pw_attached","name":"Hero","offering_id":"ofrng_x","created_at":1700000000000,"published_at":null}`, - "pw_published": `{"object":"paywall","id":"pw_published","name":"Live","offering_id":"ofrng_live","created_at":1700000000000,"published_at":1700000000000}`, - "pw_standalone": `{"object":"paywall","id":"pw_standalone","created_at":1700000000000,"published_at":null}`, - } - var deletedPaths []string - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - id := r.URL.Path[strings.LastIndex(r.URL.Path, "/")+1:] - switch r.Method { - case http.MethodGet: - io.WriteString(w, paywalls[id]) - case http.MethodDelete: - deletedPaths = append(deletedPaths, r.URL.Path) - io.WriteString(w, `{}`) - default: - w.WriteHeader(http.StatusNotFound) - } - })) - defer server.Close() - t.Setenv("RC_CONFIG_DIR", t.TempDir()) - t.Setenv("RC_BASE_URL", server.URL) - - del := func(args ...string) error { - root := NewRootCmd("test") - root.SetOut(io.Discard) - root.SetErr(&bytes.Buffer{}) - root.SetArgs(append([]string{"paywalls", "delete"}, append(args, "--yes", "--api-key", "sk_test", "--project-id", "proj")...)) - return root.ExecuteContext(context.Background()) - } - for _, id := range []string{"pw_attached", "pw_published"} { - if err := del(id); err == nil { - t.Fatalf("%s should require --force", id) - } - } - if len(deletedPaths) != 0 { - t.Fatalf("refusals must not issue DELETE, got %v", deletedPaths) - } + "github.com/revenuecat/cli/internal/api" +) - if err := del("pw_standalone"); err != nil { - t.Fatalf("standalone draft should delete without --force: %v", err) - } - if err := del("pw_attached", "--force"); err != nil { - t.Fatalf("--force should delete attached paywall: %v", err) +// paywalls delete is interactive-only (its refusal to run under automation is +// covered by the destructive-command guard), so the force-check is exercised +// directly here: a published or attached paywall needs --force, a standalone +// draft does not. +func TestCheckPaywallDeletable(t *testing.T) { + published := api.Millis(1700000000000) + tests := []struct { + name string + paywall *api.Paywall + force bool + wantErr bool + }{ + {name: "standalone draft deletes", paywall: &api.Paywall{ID: "pw_standalone"}}, + {name: "published needs force", paywall: &api.Paywall{ID: "pw_pub", PublishedAt: &published}, wantErr: true}, + {name: "attached needs force", paywall: &api.Paywall{ID: "pw_att", OfferingID: "ofrng_x"}, wantErr: true}, + {name: "published with force deletes", paywall: &api.Paywall{ID: "pw_pub", PublishedAt: &published}, force: true}, + {name: "attached with force deletes", paywall: &api.Paywall{ID: "pw_att", OfferingID: "ofrng_x"}, force: true}, } - want := []string{"/projects/proj/paywalls/pw_standalone", "/projects/proj/paywalls/pw_attached"} - if len(deletedPaths) != 2 || deletedPaths[0] != want[0] || deletedPaths[1] != want[1] { - t.Fatalf("DELETE paths = %v, want %v", deletedPaths, want) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkPaywallDeletable(tt.paywall, tt.paywall.ID, tt.force) + if (err != nil) != tt.wantErr { + t.Fatalf("checkPaywallDeletable err = %v, wantErr %v", err, tt.wantErr) + } + }) } } From c49fa9b0b986d23cc82d34b470a7980513e6ab65 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Wed, 19 Aug 2026 13:03:38 -0500 Subject: [PATCH 5/6] feat(cli): products + packages delete are interactive-only too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the interactive-only tier to products delete and packages delete — both irreversible like the other four — so no destructive delete is agent-drivable. Moves them out of the --yes-bypass list and into the interactive-only guard; switches the yes-bypass test to webhooks delete. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/destructive_guard_test.go | 8 ++++---- internal/cli/packages.go | 9 +++++++-- internal/cli/products.go | 9 +++++++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/internal/cli/destructive_guard_test.go b/internal/cli/destructive_guard_test.go index c581b739..e1e91799 100644 --- a/internal/cli/destructive_guard_test.go +++ b/internal/cli/destructive_guard_test.go @@ -20,11 +20,9 @@ func TestDestructiveCommands_RefuseUnderNoInputWithoutYes(t *testing.T) { // The most destructive, irreversible commands are interactive-only and are // covered separately below; --yes bypasses confirmation for everything here. commands := [][]string{ - {"products", "delete", "prod_x"}, {"products", "push", "prod_x"}, {"paywalls", "publish", "pw_x"}, {"paywalls", "unpublish", "pw_x"}, - {"packages", "delete", "pkg_x"}, {"webhooks", "delete", "wh_x"}, {"purchases", "refund", "txn_x"}, {"subscriptions", "cancel", "sub_x"}, @@ -70,7 +68,7 @@ func TestDestructiveCommand_YesBypassesConfirmation(t *testing.T) { var deleted bool server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodDelete && strings.HasSuffix(r.URL.Path, "/products/prod_x"): + case r.Method == http.MethodDelete && strings.HasSuffix(r.URL.Path, "/webhooks/wh_x"): deleted = true w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{}`)) @@ -86,7 +84,7 @@ func TestDestructiveCommand_YesBypassesConfirmation(t *testing.T) { t.Fatal(err) } - _, errb, err := runCmdInConfigDir(t, configDir, "products", "delete", "prod_x", "--yes", "--no-input") + _, errb, err := runCmdInConfigDir(t, configDir, "webhooks", "delete", "wh_x", "--yes", "--no-input") if err != nil { t.Fatalf("--yes should let the delete proceed: %v\nstderr: %s", err, errb) } @@ -104,6 +102,8 @@ func TestInteractiveOnlyCommands_RefuseEvenWithYes(t *testing.T) { {"paywalls", "delete", "pw_x"}, {"offerings", "delete", "ofrng_x"}, {"entitlements", "delete", "ent_x"}, + {"products", "delete", "prod_x"}, + {"packages", "delete", "pkg_x"}, } for _, args := range commands { t.Run(strings.Join(args, " "), func(t *testing.T) { diff --git a/internal/cli/packages.go b/internal/cli/packages.go index 361ab108..2b00c665 100644 --- a/internal/cli/packages.go +++ b/internal/cli/packages.go @@ -266,11 +266,16 @@ to pick from a list. Reversibility: irreversible. -Confirmation: prompts under TTY; pass --yes to skip. Required under --no-input.`, - Example: ` rc packages delete pkg_x --yes`, +Interactive-only: run it yourself in a terminal. It is unavailable under +--json or --no-input so automation can't delete packages. --yes skips the +confirmation prompt once you're in a terminal.`, + Example: ` rc packages delete pkg_x`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { rt := RuntimeFrom(cmd.Context()) + if err := requireInteractive(rt, cmd.CommandPath()); err != nil { + return err + } projectID, err := requireProject(rt) if err != nil { return err diff --git a/internal/cli/products.go b/internal/cli/products.go index 8ba86514..286271b2 100644 --- a/internal/cli/products.go +++ b/internal/cli/products.go @@ -669,11 +669,16 @@ to pick from a list. Reversibility: irreversible. Prefer ` + "`rc products archive`" + ` for reversible removal. -Confirmation: prompts under TTY; pass --yes to skip. Required under --no-input.`, - Example: ` rc products delete prod_x --yes`, +Interactive-only: run it yourself in a terminal. It is unavailable under +--json or --no-input so automation can't delete products. --yes skips the +confirmation prompt once you're in a terminal.`, + Example: ` rc products delete prod_x`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { rt := RuntimeFrom(cmd.Context()) + if err := requireInteractive(rt, cmd.CommandPath()); err != nil { + return err + } projectID, err := requireProject(rt) if err != nil { return err From 9fc277a87f7c0bd7f6c30843d61abcbdc49cfdfe Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Thu, 20 Aug 2026 09:22:49 -0500 Subject: [PATCH 6/6] fix(cli): interactive-only error covers the non-TTY case CanPrompt is also false in a plain non-TTY session with no flags, but the gate error only named --json/--no-input. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/confirm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cli/confirm.go b/internal/cli/confirm.go index 4fdf6010..41559d0e 100644 --- a/internal/cli/confirm.go +++ b/internal/cli/confirm.go @@ -16,7 +16,7 @@ func requireInteractive(rt *Runtime, action string) error { return nil } return WithHint( - fmt.Errorf("%s is interactive-only and can't run with --json or --no-input", action), + fmt.Errorf("%s is interactive-only: it needs a real terminal and can't run with --json or --no-input", action), "Run it yourself in an interactive terminal. It is intentionally unavailable to automation because it is irreversible.", ) }