From 2e13d2cad0ecfc157493eee917e1f99dd385e9f7 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 16 Oct 2026 13:09:18 +0200 Subject: [PATCH] fix(cli): refuse an --output path that already exists An explicit --output path was opened with os.Create, silently truncating any existing file (including a previous report or the --input-csv file). validateFilePaths only checked the parent directory. Fail validation when the path exists (file, directory or symlink, detected with Lstat so a dangling symlink cannot be followed and overwritten) and on any stat error other than not-exist. The message names the path and tells the operator to pick a new path or remove the old file. Auto-generated timestamped filenames do not set CSVOutput and are unaffected. --input-csv equal to --output now fails with the same message. Closes #2050 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++++ README.md | 2 ++ cmd/validators.go | 9 ++++++++ cmd/validators_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++ docs/cli/README.md | 2 +- 5 files changed, 65 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4d9cf771..d46ff72ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed +- An explicit `--output` path that already exists (file, directory or symlink) + is now refused during validation instead of being silently truncated. Choose a + new path or remove the old file; scripts that reuse one path must do the same. + Auto-generated filenames are unaffected (#2050). - `--regions` now also narrows EC2 Instance Savings Plan recommendations to the listed regions by the plan's own region (previously only `--include-regions` did); a plan with no known region is dropped. Compute, SageMaker and Database plans stay diff --git a/README.md b/README.md index 0e6249f97..92e23a1ea 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,8 @@ Export a reviewable report when you need to share results: ./cudly --services rds --profile default --output recommendations.csv ``` +The path must not already exist; the run is refused rather than overwriting it. + All purchase operations can spend money. Check the account, region, quantity, and selected commitment before confirming - see [Safety Features](#safety-features) for what is and is not enforced today. ## Credentials and provider status diff --git a/cmd/validators.go b/cmd/validators.go index 24ba719c3..dda52a279 100644 --- a/cmd/validators.go +++ b/cmd/validators.go @@ -293,6 +293,15 @@ func validateFilePaths() error { return fmt.Errorf("output directory does not exist: %s", dir) } } + // Lstat so a symlink (even a dangling one) counts as existing; the + // later os.Create would otherwise follow it and truncate its target. + _, err := os.Lstat(toolCfg.CSVOutput) + if err == nil { + return fmt.Errorf("output path already exists: %s (it would be overwritten); choose a new --output path or remove the existing file", toolCfg.CSVOutput) + } + if !os.IsNotExist(err) { + return fmt.Errorf("cannot check output path %s: %w", toolCfg.CSVOutput, err) + } } // Validate CSV input path if provided diff --git a/cmd/validators_test.go b/cmd/validators_test.go index a008ece77..99ef4207c 100644 --- a/cmd/validators_test.go +++ b/cmd/validators_test.go @@ -347,6 +347,55 @@ func TestValidateFilePaths(t *testing.T) { wantErr: true, errMsg: "output directory does not exist", }, + { + name: "CSV output file already exists", + setupFunc: func() func() { + p := filepath.Join(tmpDir, "existing.csv") + if err := os.WriteFile(p, []byte("keep"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + toolCfg.CSVOutput = p + return func() { os.Remove(p) } + }, + wantErr: true, + errMsg: "output path already exists", + }, + { + name: "CSV output path is an existing directory", + setupFunc: func() func() { + toolCfg.CSVOutput = tmpDir + return func() {} + }, + wantErr: true, + errMsg: "output path already exists", + }, + { + name: "CSV output is a dangling symlink", + setupFunc: func() func() { + p := filepath.Join(tmpDir, "dangling.csv") + if err := os.Symlink(filepath.Join(tmpDir, "missing-target"), p); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + toolCfg.CSVOutput = p + return func() { os.Remove(p) } + }, + wantErr: true, + errMsg: "output path already exists", + }, + { + name: "CSV input equals output", + setupFunc: func() func() { + p := filepath.Join(tmpDir, "same.csv") + if err := os.WriteFile(p, []byte("keep"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + toolCfg.CSVOutput = p + toolCfg.CSVInput = p + return func() { os.Remove(p) } + }, + wantErr: true, + errMsg: "output path already exists", + }, { name: "CSV input file does not exist", setupFunc: func() func() { diff --git a/docs/cli/README.md b/docs/cli/README.md index daf3bb7de..61cf08c9b 100644 --- a/docs/cli/README.md +++ b/docs/cli/README.md @@ -87,7 +87,7 @@ All flags belong to the root command unless noted otherwise. | Flag | Short | Default | Description | |------|-------|---------|-------------| -| `--output` | `-o` | auto-generated | Output CSV file path. Auto-name format: `ri-helper-dryrun-YYYYMMDD-HHMMSS.csv` or `ri-helper-purchase-YYYYMMDD-HHMMSS.csv`. | +| `--output` | `-o` | auto-generated | Output CSV file path. Auto-name format: `ri-helper-dryrun-YYYYMMDD-HHMMSS.csv` or `ri-helper-purchase-YYYYMMDD-HHMMSS.csv`. An explicit path that already exists (file, directory or symlink) is refused instead of overwritten; choose a new path or remove the old file. | | `--input-csv` | `-i` | (none) | Read recommendations from an existing CSV instead of fetching from cloud APIs. The file must have a `.csv` extension and exist at the path given. The header must contain `Service`, `Region`, `ResourceType`, `Count`, `Term` and `PaymentOption`; a missing column is a load error that names it. `Term` must be `1yr` or `3yr` and `PaymentOption` must be `all-upfront`, `partial-upfront` or `no-upfront` on every row; a blank or unknown value fails the load with the row number. CSVs with only the older four columns are rejected. | ### Authentication