From 2c6f21a00cab38b21baa748326f15b686a140b1d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 30 Sep 2026 01:44:36 +0200 Subject: [PATCH] fix(cli): require interactive purchase confirmation Remove the --yes bypass from both purchase coordinators while preserving unattended dry runs. Cover flag rejection, nonterminal refusal and dry-run reports, and update the purchase-safety reference. Closes #1943 --- README.md | 4 +- cmd/effective_dry_run_test.go | 16 ++- cmd/helpers.go | 13 +-- cmd/helpers_test.go | 76 +------------ cmd/main.go | 2 - cmd/multi_service.go | 8 +- cmd/multi_service_test.go | 5 - cmd/purchase_confirmation_test.go | 182 ++++++++++++++++++++++++++++++ docs/cli/README.md | 5 +- docs/cli/purchase-safety.md | 29 ++--- 10 files changed, 224 insertions(+), 116 deletions(-) create mode 100644 cmd/purchase_confirmation_test.go diff --git a/README.md b/README.md index fe1c544e3..aeecf78a7 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ CUDly is an open source CLI for discovering and purchasing AWS Reserved Instances and Savings Plans in a single command. It is dry-run by default: nothing is purchased until you pass `--purchase`. `configure-azure` and `configure-gcp` bootstrap credentials for the separate [self-hosted platform](https://github.com/LeanerCloud/cloud-commitments-platform); this CLI's own recommend-and-purchase workflow is AWS-only today. See [cloud setup](docs/cli/cloud-setup.md). -It is also built to be driven by an AI agent for the discovery and analysis side: searching recommendations, sizing a plan, filtering by account or region. The purchase step still needs a human to review the numbers before committing money. **`--yes` currently skips the confirmation prompt outright, including for a non-interactive caller** (a script, a CI job, an agent driving the CLI as a subprocess): see [Safety Features](#safety-features) and [#1943](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1943) before wiring `--purchase --yes` into anything unattended. +It is also built to be driven by an AI agent for the discovery and analysis side: searching recommendations, sizing a plan, filtering by account or region. Real purchases require `--purchase` and confirmation at an interactive terminal. The `--yes` bypass has been removed; scripts and agents should hand their dry-run recommendations to a human for review and purchase. The CLI depends on the published shared Go modules in [cloud-commitments-go](https://github.com/LeanerCloud/cloud-commitments-go), pinned to fixed versions in `go.mod`. No sibling checkout or parent workspace is needed for local development. @@ -17,7 +17,7 @@ The CLI depends on the published shared Go modules in [cloud-commitments-go](htt ## Safety Features 1. **Dry-run by default** - no purchase without the explicit `--purchase` flag. -2. **Confirmation prompt** - `--purchase` prints a summary of instance count and estimated savings, then prompts for confirmation. `--yes` skips this prompt, including for a non-interactive caller - it is not currently an automation boundary. [#1943](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1943) tracks closing that gap. +2. **Confirmation prompt** - At an interactive terminal, `--purchase` prints the total instance count and estimated savings, then asks for confirmation once for the whole run. Nonterminal input is refused, including piped `yes`. Dry runs need no confirmation. 3. **Coverage and instance limits** - `--coverage`, `--target-coverage`, and `--max-instances` shape what a dry run recommends before there is anything to confirm. 4. **RDS extended-support filtering** - by default, recommendations for instances running an engine version in AWS Extended Support are excluded, since the surcharge can erase RI savings; pass `--include-extended-support` to include them. 5. **Audit log written per recommendation** - the audit log path is checked for writability before any cloud API call. Each recommendation then gets its own audit record: for a dry run, written as soon as its (local, no-API-call) result is generated; for a real purchase, written after that purchase call returns. diff --git a/cmd/effective_dry_run_test.go b/cmd/effective_dry_run_test.go index 5e2195c77..28f38e980 100644 --- a/cmd/effective_dry_run_test.go +++ b/cmd/effective_dry_run_test.go @@ -1,6 +1,20 @@ package main -import "testing" +import ( + "strings" + "testing" +) + +func TestYesFlagRemoved(t *testing.T) { + if flag := rootCmd.Flags().Lookup("yes"); flag != nil { + t.Fatal("--yes must not bypass interactive purchase confirmation") + } + for _, arg := range []string{"--yes", "--yes=false"} { + if err := rootCmd.ParseFlags([]string{arg}); err == nil || !strings.Contains(err.Error(), "unknown flag: --yes") { + t.Errorf("%s must be rejected as an unknown flag, got %v", arg, err) + } + } +} // TestEffectiveDryRun documents the single-flag purchase contract: a run is a // dry run unless the user opts into real purchases with --purchase. This guards diff --git a/cmd/helpers.go b/cmd/helpers.go index e8a4cf450..3102869f3 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -200,17 +200,10 @@ func ApplyInstanceLimit(recs []common.Recommendation, maxInstances int32) []comm return result } -// ConfirmPurchase asks the user for confirmation before proceeding. -// totalSavings is the estimated monthly savings from the purchase (not the purchase cost), -// matching the EstimatedSavings column and the "Estimated monthly savings" summary. -// Returns false without prompting if stdin is not a TTY and skipConfirmation is false. -func ConfirmPurchase(totalInstances int, totalSavings float64, skipConfirmation bool) bool { - if skipConfirmation { - return true - } - +// ConfirmPurchase requires terminal input; totalSavings is monthly savings, not purchase cost. +func ConfirmPurchase(totalInstances int, totalSavings float64) bool { if !term.IsTerminal(int(os.Stdin.Fd())) { //nolint:gosec // G115: uintptr->int for file descriptor; FD values are always small positive integers - log.Printf("stdin is not a terminal and --yes was not set; skipping purchase") + log.Printf("stdin is not a terminal; interactive confirmation is required, skipping purchase") return false } diff --git a/cmd/helpers_test.go b/cmd/helpers_test.go index 980488c62..6a55648d5 100644 --- a/cmd/helpers_test.go +++ b/cmd/helpers_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "math" - "strings" "sync" "testing" "time" @@ -655,76 +654,11 @@ func TestGetEngineFromRecommendation(t *testing.T) { } } -// confirmPurchaseWithInput is a testable variant of ConfirmPurchase that reads -// from the provided reader rather than os.Stdin, allowing stdin to be mocked in tests. -func confirmPurchaseWithInput(skipConfirmation bool, input string) bool { - if skipConfirmation { - return true - } - response := strings.TrimSpace(strings.ToLower(strings.SplitN(input, "\n", 2)[0])) - return response == "yes" || response == "y" -} - -func TestConfirmPurchase(t *testing.T) { - tests := []struct { - name string - totalInstances int - totalCost float64 - skipConfirmation bool - expected bool - }{ - { - name: "Skip confirmation returns true", - totalInstances: 10, - totalCost: 100.50, - skipConfirmation: true, - expected: true, - }, - { - name: "Skip confirmation with zero cost", - totalInstances: 0, - totalCost: 0.0, - skipConfirmation: true, - expected: true, - }, - { - name: "Skip confirmation with high cost", - totalInstances: 1000, - totalCost: 999999.99, - skipConfirmation: true, - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := ConfirmPurchase(tt.totalInstances, tt.totalCost, tt.skipConfirmation) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestConfirmPurchaseInput(t *testing.T) { - // Tests for the interactive stdin branch of ConfirmPurchase logic - tests := []struct { - name string - input string - expected bool - }{ - {name: "yes accepts", input: "yes\n", expected: true}, - {name: "y accepts", input: "y\n", expected: true}, - {name: "YES accepts (case insensitive)", input: "YES\n", expected: true}, - {name: "Y accepts (case insensitive)", input: "Y\n", expected: true}, - {name: "no rejects", input: "no\n", expected: false}, - {name: "n rejects", input: "n\n", expected: false}, - {name: "empty string rejects", input: "\n", expected: false}, - {name: "arbitrary text rejects", input: "maybe\n", expected: false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := confirmPurchaseWithInput(false, tt.input) - assert.Equal(t, tt.expected, result) +func TestConfirmPurchaseRejectsNonterminalInput(t *testing.T) { + for _, input := range []string{"yes\n", "y\n", "YES\n", "no\n", ""} { + t.Run(input, func(t *testing.T) { + setConfirmationStdin(t, input) + assert.False(t, ConfirmPurchase(5, 125)) }) } } diff --git a/cmd/main.go b/cmd/main.go index 44b3f07b9..b3690cc56 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -69,7 +69,6 @@ type Config struct { IncludeExtendedSupport bool AllServices bool ActualPurchase bool - SkipConfirmation bool // RecLookbackPeriod controls the LookbackPeriodInDays passed to // GetReservationPurchaseRecommendation. Valid values: "7d", "30d", "60d" // (recommendations.DefaultRecLookbackPeriod is the shared default). @@ -122,7 +121,6 @@ func init() { rootCmd.Flags().StringSliceVar(&toolCfg.ExcludeEngines, "exclude-engines", []string{}, "Exclude these engines (comma-separated)") rootCmd.Flags().StringSliceVar(&toolCfg.IncludeAccounts, "include-accounts", []string{}, "Only include recommendations for these account names (comma-separated)") rootCmd.Flags().StringSliceVar(&toolCfg.ExcludeAccounts, "exclude-accounts", []string{}, "Exclude recommendations for these account names (comma-separated)") - rootCmd.Flags().BoolVar(&toolCfg.SkipConfirmation, "yes", false, "Skip confirmation prompt for purchases (use with caution)") rootCmd.Flags().Int32Var(&toolCfg.MaxInstances, "max-instances", 0, "Maximum total number of instances to purchase (0 = no limit)") rootCmd.Flags().Int32Var(&toolCfg.OverrideCount, "override-count", 0, "Override recommendation count with fixed number for all selected RIs (0 = use recommendation or coverage)") rootCmd.Flags().StringVar(&toolCfg.ValidationProfile, "validation-profile", "", "AWS profile to use for validating running instances (if different from main profile)") diff --git a/cmd/multi_service.go b/cmd/multi_service.go index 9729092a0..ed7887e2a 100644 --- a/cmd/multi_service.go +++ b/cmd/multi_service.go @@ -185,7 +185,7 @@ func runToolMultiService(ctx context.Context, cfg Config) { // runToolMultiService within the cyclomatic-complexity limit. func runPurchaseAndReport(ctx context.Context, awsCfg aws.Config, scoredResult scorer.ScoredResult, isDryRun bool, cfg Config, drops *common.DropSummary) { runID := uuid.New().String() - if !confirmPurchaseRun(scoredResult.Passed, isDryRun, cfg) { + if !confirmPurchaseRun(scoredResult.Passed, isDryRun) { printDropSummary(drops) AppLogger.Printf("\n❌ Purchase canceled.\n") return @@ -204,12 +204,12 @@ func runPurchaseAndReport(ctx context.Context, awsCfg aws.Config, scoredResult s // and the --input-csv path (runToolFromCSV) so both entry points show the // operator the total they are actually authorizing and require exactly one // confirmation per invocation. -func confirmPurchaseRun(recs []common.Recommendation, isDryRun bool, cfg Config) bool { +func confirmPurchaseRun(recs []common.Recommendation, isDryRun bool) bool { if isDryRun { return true } totalInstances, totalSavings := sumPassedRecs(recs) - return ConfirmPurchase(totalInstances, totalSavings, cfg.SkipConfirmation) + return ConfirmPurchase(totalInstances, totalSavings) } // writeReportAndSummary writes the CSV report and prints the final summary. @@ -571,7 +571,7 @@ func prepareCSVPurchaseRun(ctx context.Context, cfg Config, csvModeCoverage floa AppLogger.Println("⚠️ No recommendations to process after filtering") return nil, aws.Config{}, "", nil } - if !confirmPurchaseRun(recs, isDryRun, cfg) { + if !confirmPurchaseRun(recs, isDryRun) { AppLogger.Printf("\n❌ Purchase canceled.\n") return nil, aws.Config{}, "", nil } diff --git a/cmd/multi_service_test.go b/cmd/multi_service_test.go index 4da38b760..0c165221e 100644 --- a/cmd/multi_service_test.go +++ b/cmd/multi_service_test.go @@ -1264,7 +1264,6 @@ func TestProcessPurchaseLoopPurchaseFailure(t *testing.T) { toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 80.0 - toolCfg.SkipConfirmation = true recs := []common.Recommendation{ {Service: common.ServiceRDS, ResourceType: "db.t3.large", Count: 1, EstimatedSavings: 500}, @@ -1315,7 +1314,6 @@ func TestProcessServicePurchasesUserCancellation(t *testing.T) { toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 85.0 - toolCfg.SkipConfirmation = true // Skip for testing recs := []common.Recommendation{ {Service: common.ServiceElastiCache, ResourceType: "cache.r6g.large", Count: 2, EstimatedSavings: 200}, @@ -1459,7 +1457,6 @@ func TestProcessPurchaseLoopActualPurchase(t *testing.T) { toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 80.0 - toolCfg.SkipConfirmation = true // Skip confirmation for testing recs := []common.Recommendation{ {Service: common.ServiceEC2, ResourceType: "t3.small", Count: 1, SourceRecommendation: "EC2 Test 1", EstimatedSavings: 100}, @@ -1978,7 +1975,6 @@ func TestProcessPurchaseLoop_WritesAuditRecordForRealPurchase(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") - toolCfg.SkipConfirmation = true t.Setenv("DISABLE_PURCHASE_DELAY", "true") recs := []common.Recommendation{ @@ -2066,7 +2062,6 @@ rds,us-west-2,db.t3.medium,postgres,3,1yr,All Upfront,123456789012 toolCfg.CSVOutput = reportPath toolCfg.AuditLog = auditPath toolCfg.ActualPurchase = true - toolCfg.SkipConfirmation = false toolCfg.Coverage = 100.0 toolCfg.TargetCoverage = 0 toolCfg.MaxInstances = 0 diff --git a/cmd/purchase_confirmation_test.go b/cmd/purchase_confirmation_test.go new file mode 100644 index 000000000..d0c5062a6 --- /dev/null +++ b/cmd/purchase_confirmation_test.go @@ -0,0 +1,182 @@ +package main + +import ( + "context" + "fmt" + "net" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/scorer" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func setConfirmationStdin(t *testing.T, input string) { + t.Helper() + reader, writer, err := os.Pipe() + require.NoError(t, err) + _, err = writer.WriteString(input) + require.NoError(t, err) + require.NoError(t, writer.Close()) + previous := os.Stdin + os.Stdin = reader + t.Cleanup(func() { + os.Stdin = previous + assert.NoError(t, reader.Close()) + }) +} + +func assertNoPurchaseOutput(t *testing.T, cfg Config) { + t.Helper() + _, err := os.Stat(cfg.CSVOutput) + require.True(t, os.IsNotExist(err), "refused purchase must not produce a report: %v", err) + data, err := os.ReadFile(cfg.AuditLog) + if err != nil { + require.True(t, os.IsNotExist(err), "unexpected audit read error: %v", err) + } + assert.Empty(t, data, "refused purchase must not produce purchase audit records") +} + +func TestRunPurchaseAndReportRequiresTerminal(t *testing.T) { + setConfirmationStdin(t, "yes\n") + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + http.Error(w, "unexpected purchase request", http.StatusBadRequest) + })) + t.Cleanup(server.Close) + transport := server.Client().Transport.(*http.Transport).Clone() + transport.DialContext = func(ctx context.Context, network, address string) (net.Conn, error) { + if address != server.Listener.Addr().String() { + return nil, fmt.Errorf("nonlocal provider address rejected: %s", address) + } + return (&net.Dialer{}).DialContext(ctx, network, address) + } + t.Cleanup(transport.CloseIdleConnections) + awsCfg := aws.Config{ + Region: "us-east-1", BaseEndpoint: aws.String(server.URL), HTTPClient: &http.Client{Transport: transport}, + Credentials: aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) { + return aws.Credentials{AccessKeyID: "test", SecretAccessKey: "test"}, nil + }), + } + for _, service := range getAllServices() { + for _, dryRun := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/dryRun=%t", service, dryRun), func(t *testing.T) { + dir := t.TempDir() + cfg := Config{AuditLog: filepath.Join(dir, "audit.jsonl"), CSVOutput: filepath.Join(dir, "report.csv")} + recs := []common.Recommendation{ + {Service: service, Region: "us-east-1", ResourceType: "test-instance", Count: 2, EstimatedSavings: 50}, + {Service: service, Region: "us-west-2", ResourceType: "test-instance", Count: 3, EstimatedSavings: 75}, + } + output := captureAppOutput(t, func() { + runPurchaseAndReport(context.Background(), awsCfg, scorer.ScoredResult{Passed: recs}, dryRun, cfg, &common.DropSummary{}) + }) + assert.Zero(t, requests.Load(), "confirmation refusal and dry runs must never reach a purchase provider") + if dryRun { + assert.NotContains(t, output, "Purchase canceled") + _, rows := readPurchaseReport(t, cfg.CSVOutput) + assert.Len(t, rows, 2) + data, err := os.ReadFile(cfg.AuditLog) + require.NoError(t, err) + assert.Contains(t, string(data), `"dry_run":true`) + } else { + assert.Contains(t, output, "Purchase canceled") + assertNoPurchaseOutput(t, cfg) + } + }) + } + } +} + +func TestRunToolFromCSVRequiresTerminal(t *testing.T) { + mode := os.Getenv("CUDLY_CONFIRMATION_TEST_MODE") + if mode == "" { + for _, childMode := range []string{"purchase", "dry-run"} { + t.Run(childMode, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + child := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestRunToolFromCSVRequiresTerminal$", "-test.v") + child.Env = []string{"CUDLY_CONFIRMATION_TEST_MODE=" + childMode} + output, err := child.CombinedOutput() + require.NoError(t, err, "%s", output) + }) + } + return + } + actualPurchase := mode == "purchase" + setConfirmationStdin(t, "yes\n") + isolateAWSEnv(t) + var purchases atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodConnect || r.URL.IsAbs() { + t.Errorf("nonlocal provider request rejected: %s %s", r.Method, r.URL) + http.Error(w, "nonlocal request denied", http.StatusForbidden) + return + } + if !assert.NoError(t, r.ParseForm()) { + return + } + action := r.Form.Get("Action") + body := "" + switch action { + case "DescribeRegions": + _, _ = fmt.Fprint(w, `us-east-1`) + return + case "DescribeDBInstances": + body = `` + case "DescribeDBMajorEngineVersions": + body = `` + case "DescribeReservedDBInstances": + body = `` + default: + purchases.Add(1) + t.Errorf("unexpected provider action %q", action) + http.Error(w, "unexpected request", http.StatusBadRequest) + return + } + _, _ = fmt.Fprintf(w, `<%sResponse><%sResult>%s`, action, action, body, action, action) + })) + t.Cleanup(server.Close) + // A fresh process prevents Go's cached proxy settings from preceding this fixture. + t.Setenv("HTTP_PROXY", server.URL) + t.Setenv("HTTPS_PROXY", server.URL) + t.Setenv("NO_PROXY", "") + for _, key := range []string{"AWS_ENDPOINT_URL", "AWS_ENDPOINT_URL_RDS", "AWS_ENDPOINT_URL_EC2"} { + t.Setenv(key, server.URL) + } + t.Setenv("AWS_IGNORE_CONFIGURED_ENDPOINT_URLS", "false") + cfg := toolCfg + cfg.ActualPurchase = actualPurchase + cfg.Coverage = 100 + cfg.IncludeExtendedSupport = true + cfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") + cfg.CSVOutput = filepath.Join(t.TempDir(), "report.csv") + cfg.CSVInput = writeTestRecommendationsCSV(t, `Service,Region,ResourceType,Engine,Deployment,Count,Term,PaymentOption,EstimatedSavings +rds,us-east-1,db.t3.small,postgres,single-az,2,1yr,all-upfront,50 +rds,us-west-2,db.t3.small,postgres,single-az,3,1yr,all-upfront,75 +`) + output := captureAppOutput(t, func() { + require.NoError(t, runToolFromCSV(context.Background(), cfg)) + }) + assert.Zero(t, purchases.Load()) + if actualPurchase { + assert.Contains(t, output, "Purchase canceled") + assertNoPurchaseOutput(t, cfg) + } else { + assert.NotContains(t, output, "Purchase canceled") + _, rows := readPurchaseReport(t, cfg.CSVOutput) + assert.Len(t, rows, 2) + data, err := os.ReadFile(cfg.AuditLog) + require.NoError(t, err) + assert.Contains(t, string(data), `"dry_run":true`) + } +} diff --git a/docs/cli/README.md b/docs/cli/README.md index a0dee3980..fb92213d1 100644 --- a/docs/cli/README.md +++ b/docs/cli/README.md @@ -11,7 +11,7 @@ This section documents the full CLI surface of the `cudly` binary. The Makefile | Page | Covers | |------|--------| | [filtering.md](filtering.md) | Account, region, engine, instance-type, and Savings Plan type include/exclude filters; numeric threshold filters (min-count, min-savings-pct, max-break-even-months, min-pool-size, max-instances) | -| [purchase-safety.md](purchase-safety.md) | Purchase pipeline guardrails: dry-run, --purchase, --yes, --audit-log, --idempotency-window, --rebuy-window-days, DISABLE_PURCHASE_DELAY env | +| [purchase-safety.md](purchase-safety.md) | Purchase pipeline guardrails: dry-run, --purchase, interactive confirmation, --audit-log, --idempotency-window, --rebuy-window-days, DISABLE_PURCHASE_DELAY env | | [cloud-setup.md](cloud-setup.md) | `configure-azure` and `configure-gcp` subcommands for self-hosted credential bootstrap | ## Complete flag reference @@ -47,8 +47,7 @@ All flags belong to the root command unless noted otherwise. | Flag | Short | Default | Description | |------|-------|---------|-------------| -| `--purchase` | | `false` | Execute real purchases. This is the only purchase control: a bare run is always a dry run, and `--purchase` alone executes real purchases (identically in cloud-fetch and `--input-csv` modes). Still gated by the `--yes` / interactive confirmation prompt. See [purchase-safety.md](purchase-safety.md). | -| `--yes` | | `false` | Skip the interactive confirmation prompt. Use with caution in automation. | +| `--purchase` | | `false` | Opt into real purchases, requiring confirmation at an interactive terminal. A bare run is always a dry run. Applies to both cloud-fetch and `--input-csv` modes. See [purchase-safety.md](purchase-safety.md). | | `--audit-log` | | `./cudly-audit.jsonl` | Path to the JSONL audit log file. Written for every recommendation (dry-run and real). See [purchase-safety.md](purchase-safety.md). | | `--idempotency-window` | | `24h` | Lookback window for duplicate purchase detection. A Go duration string that must be a positive whole number of hours (e.g. `24h`, `72h`); anything else is rejected at startup. See [purchase-safety.md](purchase-safety.md). | diff --git a/docs/cli/purchase-safety.md b/docs/cli/purchase-safety.md index 474ed0d20..2be678926 100644 --- a/docs/cli/purchase-safety.md +++ b/docs/cli/purchase-safety.md @@ -4,7 +4,9 @@ CUDly is designed to be safe by default. Real purchases require an explicit `--p ## Automation and AI agents -An AI agent (or any other non-interactive caller) can safely drive discovery, sizing, and filtering: that reads recommendations and existing commitments (for example, `--target-coverage`'s coverage lookup and the duplicate check that runs before every purchase), but never purchases anything on its own. Purchasing is different. `--purchase --yes` executes a real purchase from any invocation - a script, a CI job, or an agent running `cudly` as a subprocess included - because `--yes` skips the confirmation prompt before the interactive-terminal check ever runs. There is currently no automation boundary on the purchase path; [#1943](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1943) tracks closing that gap. Until it lands, treat `--purchase --yes` as unattended purchase automation, and keep it out of anything an agent can trigger on its own. +Scripts and AI agents can run discovery, sizing, filtering and dry-run reports without confirmation. Real purchases require `--purchase` and an affirmative response at an interactive terminal, once for the whole run. Nonterminal stdin is refused, even if it contains `yes`. + +The `--yes` bypass has been removed ([#1943](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1943)). Existing commands that pass it are rejected during command parsing. Hand dry-run recommendations to a human, who reviews the totals and runs `--purchase` at a terminal. This prompt is an operator safeguard, not a security boundary against software that can control a terminal. ## The purchase decision: --purchase @@ -18,40 +20,31 @@ Whether a run executes real purchases is controlled by a single flag: isDryRun = !ActualPurchase ``` -A bare invocation is always a dry run; passing `--purchase` is the one and only opt-in that moves money. This rule is identical in both cloud-fetch mode (the default) and CSV input mode (`--input-csv`). +A bare invocation is always a dry run; passing `--purchase` opts into real purchases, subject to interactive confirmation. This rule is identical in both cloud-fetch mode (the default) and CSV input mode (`--input-csv`). | `--purchase` | Result | |---|---| | (not set / false) | Dry run - nothing purchased | -| `true` | Real purchases | +| `true` | Real purchases after interactive confirmation | -> **History:** earlier versions had a separate `--dry-run` flag. As a default-true flag it silently suppressed purchases even when `--purchase` was set (you had to pass `--purchase --dry-run=false` to actually buy - a footgun surfaced on #1364), and once its default was flipped to false it became a redundant "force dry-run even with `--purchase`" override that only muddied the contract. It has been removed in favour of the single `--purchase` control. Real purchases still require the `--yes` confirmation (or the interactive prompt) below, so moving money remains a deliberate act. +> **History:** earlier versions had a separate `--dry-run` flag. As a default-true flag it silently suppressed purchases even when `--purchase` was set (you had to pass `--purchase --dry-run=false` to actually buy - a footgun surfaced on #1364), and once its default was flipped to false it became a redundant "force dry-run even with `--purchase`" override that only muddied the contract. It has been removed in favour of the single `--purchase` control. Real purchases still require the interactive confirmation below. ```bash # Dry run (the default - nothing is purchased): cudly --services rds -# Execute real purchases (prompts for confirmation unless --yes is given): +# Execute real purchases (requires interactive confirmation): cudly --services rds --purchase # CSV mode behaves identically: cudly --input-csv recs.csv --purchase ``` -## Confirmation prompt: --yes - -```text ---yes bool default: false -``` +## Confirmation prompt -When running in purchase mode (`isDryRun=false`), cudly prints a summary of the total instance count and estimated savings and prompts for confirmation before executing any purchase. Pass `--yes` to skip this prompt in automation. +When running in purchase mode (`isDryRun=false`) at a terminal, cudly prints the total instance count and estimated monthly savings and prompts once before executing purchases. Enter `yes` or `y` to proceed; any other answer or an input error cancels the entire run. Input is case-insensitive. There is no flag to skip confirmation. -`--yes` skips the prompt unconditionally - it is not gated on whether the process has a real, interactive terminal. A script, a CI job, or an agent driving `cudly` as a subprocess can pass `--yes` and execute a purchase exactly as a human at a terminal would. Treat `--purchase --yes` as fully unattended purchase automation, not as a convenience for a human who already confirmed elsewhere. See [#1943](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1943) for the tracked work to close this gap. - -```bash -# Unattended purchase (use with care - see the note above): -cudly --services rds --purchase --yes -``` +Dry runs never prompt. A purchase run with nonterminal stdin is canceled, so piping `yes` into the command does not authorize a purchase. ## Audit log: --audit-log @@ -140,4 +133,4 @@ Before any real purchase run: 4. Narrow the scope with `--include-regions`, `--include-accounts`, or `--min-savings-pct` before buying across all services. 5. Consider `--max-instances` as a final safety cap for a first run. 6. Set `--idempotency-window` to cover the time since the earlier run (e.g. `72h` for a re-run two days later), and note that a failed existing-commitments lookup makes a dry run proceed un-deduplicated with a warning, while a `--purchase` run refuses to purchase for that (service, region) instead ([#1941](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1941)); watch the log for either signal and check the audit log afterward. -7. If an AI agent or other automation drives `cudly`, never pass `--yes` to it directly - have the agent hand off the dry-run recommendation to a human, who runs `--purchase` themselves. See [Automation and AI agents](#automation-and-ai-agents). +7. If an AI agent or other automation drives `cudly`, have it hand off the dry-run recommendation to a human, who reviews the totals and confirms `--purchase` at a terminal. See [Automation and AI agents](#automation-and-ai-agents).