fix(rebuy): refuse gated rebuy unless the account owns all in-scope usage

CHANGELOG.md

@@ -54,6 +54,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 - `--target-coverage` with `--rebuy-window-days` no longer buys replacement EC2 and
   other non-RDS RIs again when they were already bought inside the coverage
   lookback: the expiring RIs are netted against those replacements (#2235).
+- `--rebuy-window-days` with `--target-coverage` now refuses to run (exit 1, no
+  CSV, dry runs included) when the credentials' account does not own all the
+  in-scope RI usage, instead of netting one account's expiring RIs against
+  broader demand. Member accounts of an AWS Organization are refused. A failed
+  STS lookup, role check, scope probe (new Cost Explorer
+  `GetReservationCoverage` query grouped by linked account) or RI inventory
+  read also exits 1 on these runs. These runs now need the read-only
+  `organizations:DescribeOrganization` permission (#2131).
 - `--min-savings-pct` and `--max-break-even-months` now apply on `--input-csv`
   runs instead of being refused. The CSV report gains `SavingsPercentage` and
   `BreakEvenMonths` columns; replaying a report that lacks a column a set flag

cmd/main.go

@@ -157,11 +157,16 @@ func init() {
 		"Number of calendar days of historical demand fed to GetReservationCoverage "+
 			"when computing the existing-RI coverage map for --target-coverage sizing. "+
 			"Match this to your AWS console coverage report window to reconcile "+
-			"CUDly's ExistingCoverage column against the console export. Default 30.")
+			"CUDly's ExistingCoverage column against the console export. Range 1-365, default 30.")
 	rootCmd.Flags().IntVar(&toolCfg.RebuyWindowDays, "rebuy-window-days", 0,
 		"When >0, treat existing RIs expiring within this many days as already "+
 			"uncovered, so --target-coverage sizes recommendations to replace them "+
-			"before they expire. Default 0 = trust existing coverage fully.")
+			"before they expire. Default 0 = trust existing coverage fully. "+
+			"With --target-coverage, refuses to run from a member account of an AWS "+
+			"Organization, when the credentials' account does not own all in-scope "+
+			"usage, or when a scope check or the RI inventory read fails. Needs "+
+			"read-only sts:GetCallerIdentity, organizations:DescribeOrganization "+
+			"and ce:GetReservationCoverage; see docs/cli/purchase-safety.md.")
 	rootCmd.Flags().Float64Var(&toolCfg.MinPoolSize, "min-pool-size", 0,
 		"When >0, drop RI recommendations for pools with AverageInstancesUsedPerHour "+
 			"below this threshold. Useful with --target-coverage to skip tiny pools "+

cmd/multi_service.go

@@ -172,6 +172,7 @@ func runToolMultiService(ctx context.Context, cfg Config) {
 	if adapter, ok := recClient.(*awsprovider.RecommendationsClientAdapter); ok && cfg.RecLookbackPeriod != "" {
 		adapter.SetRecLookbackPeriod(cfg.RecLookbackPeriod)
 	}
+	cfg = mustEnforceRebuyScope(ctx, awsCfg, cfg, servicesToProcess)
 	engineData := mustFetchEngineVersionData(ctx, cfg, extendedSupportCheckNeeded(cfg, servicesToProcess))
 
 	// Fetch existing-RI coverage so --target-coverage can subtract what
@@ -385,6 +386,9 @@ func exitOnFetchError(err error) {
 	if errors.Is(err, errRecommendationFetch) {
 		log.Fatalf("Cannot complete the recommendation fetch: %v", err)
 	}
+	if errors.Is(err, errRebuyScope) {
+		log.Fatalf("Cannot verify the rebuy scope: %v", err)
+	}
 	exitOnExclusionError(err)
 }
 

cmd/multi_service_fetch_all.go

@@ -104,7 +104,7 @@ func fetchAndFilterRegionRecs(
 	if err := populateRecommendationAccountNames(ctx, recs, accountCache, cfg); err != nil {
 		return nil, fmt.Errorf("%w: %w", errRecommendationFetch, err)
 	}
-	if err := requireRegionInventory(recs, cfg, engineData); err != nil {
+	if err := requireRecPreconditions(recs, cfg, engineData); err != nil {
 		return nil, err
 	}
 	recs = applyRegionFilters(recs, engineData, region, cfg, drops)
@@ -120,17 +120,13 @@ func fetchAndFilterRegionRecs(
 	serviceClient := createServiceClient(service, regionalCfg)
 
 	// Fetch existing commitments up front when --rebuy-window-days is set so
-	// the sizing step can deduct expiring RIs from ExistingCoveragePct.
-	// Best-effort: a failure here logs and continues; expiry adjustment
-	// becomes a no-op for this region.
-	var expiringCommitments []common.Commitment
-	if cfg.RebuyWindowDays > 0 && serviceClient != nil {
-		commits, err := serviceClient.GetExistingCommitments(ctx)
-		if err != nil {
-			AppLogger.Printf("  ⚠️  Could not fetch existing commitments for expiry check: %v\n", err)
-		} else {
-			expiringCommitments = commits
-		}
+	// the sizing step can deduct expiring RIs from ExistingCoveragePct. Under
+	// the rebuy scope gate (RI service, coverage target) a failed read refuses
+	// the run; otherwise it warns and the expiry adjustment is a no-op for this
+	// region.
+	expiringCommitments, inventoryErr := fetchRebuyInventory(ctx, serviceClient, service, cfg)
+	if inventoryErr != nil {
+		return nil, inventoryErr
 	}
 
 	recs = applyCoverageAndOverrides(recs, cfg, coverageMap, expiringCommitments, drops)

cmd/rds_family_expiry_command_test.go

@@ -72,6 +72,9 @@ type rdsExpiryScenario struct {
 	absentLogLine string
 	// parent documents the count the commit before the fix produced.
 	parent []int
+	// foreignUsage is an account the scope probe reports besides the caller.
+	foreignUsage string
+	orgMaster    string
 }
 
 const rdsExpiryTarget = 80
@@ -120,6 +123,8 @@ func rdsExpiryCase(name string) rdsExpiryScenario {
 			recs:  one("db.r6g.xlarge", 4), want: []int{2}, existing: 60, parent: []int{0},
 		}
 	case "multi-size-linked-accounts":
+		// Refused since cli#2131 (rebuy_scope_command_test.go): the recs span
+		// three accounts. Sizing math kept for the day the probe allows it.
 		// Two sizes and three linked-account rows in one family: the 16 expiring
 		// NU are deducted once (family 40% -> 20%), so the 60% gap (48 NU) splits
 		// 2 + 4 + 2. A double deduction would buy 2 + 5 + 2; no deduction, 1 + 2 + 1.
@@ -132,14 +137,17 @@ func rdsExpiryCase(name string) rdsExpiryScenario {
 			want: []int{2, 4, 2}, existing: 20, parent: []int{1, 2, 1},
 		}
 	case "partial-inventory":
-		// Cost Explorer covers 64 NU org-wide but the single-account inventory
-		// lists only the 4 expiring large RIs (16 NU); the other accounts' RIs
-		// are missing. The deduction is capped at the 16 expiring NU (80% ->
-		// 60%, buy 4 large), never the whole supplied-vs-covered difference.
+		// Refused since cli#2131: Cost Explorer covers 64 NU org-wide but the
+		// single-account inventory lists only the 4 expiring large RIs (16 NU);
+		// the other accounts' RIs are missing, which the scope probe detects as
+		// another account's usage in the pool. The sizing math below is kept for
+		// the day the probe allows it (deduction capped at the 16 expiring NU:
+		// 80% -> 60%, buy 4 large).
 		return rdsExpiryScenario{
-			usage: []rdsExpiryUsage{mysql("db.r6g.large", 20, 80)},
-			ris:   []rdsExpiryRI{ri("db.r6g.large", 4, 15)},
-			recs:  one("db.r6g.large", 8), want: []int{4}, existing: 60, parent: []int{0},
+			foreignUsage: "999999999999",
+			usage:        []rdsExpiryUsage{mysql("db.r6g.large", 20, 80)},
+			ris:          []rdsExpiryRI{ri("db.r6g.large", 4, 15)},
+			recs:         one("db.r6g.large", 8), want: []int{4}, existing: 60, parent: []int{0},
 		}
 	case "unusable-expiring-warned":
 		// Expiring RIs the family pass cannot use (multi-AZ with no coverage
@@ -207,8 +215,8 @@ func rdsExpiryCase(name string) rdsExpiryScenario {
 
 func TestRDSFamilyExpiryCommand(t *testing.T) {
 	for _, name := range []string{
-		"already-at-target", "below-target", "large-ri-xlarge-usage", "multi-size-linked-accounts",
-		"partial-inventory", "unusable-expiring-warned", "control-outside-window", "control-engine-isolation",
+		"already-at-target", "below-target", "large-ri-xlarge-usage",
+		"unusable-expiring-warned", "control-outside-window", "control-engine-isolation",
 		"rerun-replacement-bought", "control-replacement-outside-lookback",
 	} {
 		t.Run(name, func(t *testing.T) { runCompletenessScenario(t, rdsExpiryService, "explicit", name) })
@@ -324,6 +332,9 @@ func (p *completenessProxy) respondRDSExpiry(req *http.Request, body []byte) *ht
 
 func (p *completenessProxy) rdsExpiryResponse(host, op string, body []byte, values url.Values) (int, string, string) {
 	s := rdsExpiryCase(p.details)
+	if payload, ok := p.describeOrganization(host, op, s.orgMaster); ok {
+		return 200, "application/x-amz-json-1.1", payload
+	}
 	switch {
 	case host == "ce.us-east-1.amazonaws.com" && op == "AWSInsightsIndexService.GetReservationPurchaseRecommendation":
 		var request map[string]any
@@ -345,6 +356,9 @@ func (p *completenessProxy) rdsExpiryResponse(host, op string, body []byte, valu
 		require.NoError(p.t, err)
 		return 200, "application/x-amz-json-1.1", string(encoded)
 	case host == "ce.us-east-1.amazonaws.com" && op == "AWSInsightsIndexService.GetReservationCoverage":
+		if payload, ok := p.linkedAccountCoverage(body, "Amazon Relational Database Service", s.foreignUsage); ok {
+			return 200, "application/x-amz-json-1.1", payload
+		}
 		return 200, "application/x-amz-json-1.1", p.rdsExpiryCoverage(s, body)
 	case host == "organizations.us-east-1.amazonaws.com" && op == "AWSOrganizationsV20161128.DescribeAccount":
 		var request map[string]string
@@ -433,6 +447,7 @@ func (p *completenessProxy) assertRDSExpiryRequests(t *testing.T) {
 		t.Logf("rds expiry op %s x%d", op, n)
 	}
 	require.Equal(t, 1, p.requests["ce.us-east-1.amazonaws.com/AWSInsightsIndexService.GetReservationPurchaseRecommendation"])
-	require.Equal(t, 12, p.requests["ce.us-east-1.amazonaws.com/AWSInsightsIndexService.GetReservationCoverage"])
+	require.Equal(t, 13, p.requests["ce.us-east-1.amazonaws.com/AWSInsightsIndexService.GetReservationCoverage"], "12 coverage-map calls plus the scope probe")
+	require.Equal(t, 1, p.requests[orgDescribeKey], "one organization role check")
 	require.Positive(t, p.requests["rds.us-east-1.amazonaws.com/DescribeReservedDBInstances"])
 }

cmd/rebuy_scope.go

@@ -0,0 +1,151 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/provider"
+	"github.com/aws/aws-sdk-go-v2/aws"
+)
+
+// errRebuyScope marks a gated --rebuy-window-days run that cannot prove the
+// credentials' account owns all the in-scope usage: either another account's
+// usage is in scope, or the check itself could not run. Both refuse the run.
+var errRebuyScope = errors.New("rebuy scope")
+
+// rebuyScopeError carries the operator-facing text unchanged (the exit message
+// supplies the "Cannot verify the rebuy scope" prefix) while matching
+// errRebuyScope under errors.Is.
+type rebuyScopeError struct{ err error }
+
+func (e rebuyScopeError) Error() string        { return e.err.Error() }
+func (e rebuyScopeError) Unwrap() error        { return e.err }
+func (e rebuyScopeError) Is(target error) bool { return target == errRebuyScope }
+
+func refuseRebuyScope(format string, args ...any) error {
+	return rebuyScopeError{fmt.Errorf(format, args...)}
+}
+
+// rebuyScopeRemedy ends every refusal that names another account's usage.
+const rebuyScopeRemedy = "gated rebuy needs a standalone account, or a management account whose member accounts have no in-scope usage; otherwise omit --rebuy-window-days"
+
+// scopeCheckFailure is the refusal for a check that could not run, as opposed
+// to one that ran and found another account's usage.
+func scopeCheckFailure(cause error) error {
+	return refuseRebuyScope("the scope check could not run (%w); fix access or retry, or omit --rebuy-window-days", cause)
+}
+
+// rebuyGated reports whether --rebuy-window-days is enforced for the service:
+// it adjusts RI sizing only, and only with a coverage target.
+func rebuyGated(cfg Config, service common.ServiceType) bool {
+	return cfg.RebuyWindowDays > 0 && cfg.TargetCoverage > 0 && !common.IsSavingsPlan(service)
+}
+
+// rebuyScopeActive reports whether any selected service is gated.
+func rebuyScopeActive(cfg Config, services []common.ServiceType) bool {
+	for _, service := range services {
+		if rebuyGated(cfg, service) {
+			return true
+		}
+	}
+	return false
+}
+
+// enforceRebuyScope resolves the credentials' account when the rebuy gate
+// holds, refuses a member account, refuses if the scope probe finds usage in
+// another account, and returns cfg carrying the account for the
+// per-recommendation scope check. An unresolvable or empty account refuses.
+func enforceRebuyScope(ctx context.Context, awsCfg aws.Config, cfg Config, services []common.ServiceType) (Config, error) {
+	if !rebuyScopeActive(cfg, services) {
+		return cfg, nil
+	}
+	account, err := callerAccountFetcher(ctx, awsCfg)
+	if err != nil {
+		return cfg, scopeCheckFailure(fmt.Errorf("cannot determine the AWS account of the credentials: %w", err))
+	}
+	out := cfg
+	out.CallerAccountID = account
+	if out.CallerAccountID == "" {
+		return cfg, scopeCheckFailure(errors.New("the AWS account of the credentials is unknown"))
+	}
+	if orgErr := requireNotMember(ctx, rebuyOrgClient(awsCfg), out.CallerAccountID); orgErr != nil {
+		return cfg, orgErr
+	}
+	if probeErr := probeRebuyScope(ctx, rebuyCoverageClient(awsCfg), out, services); probeErr != nil {
+		return cfg, probeErr
+	}
+	return out, nil
+}
+
+// mustEnforceRebuyScope is enforceRebuyScope for the main pipeline; it exits
+// through the same route as fetch errors.
+func mustEnforceRebuyScope(ctx context.Context, awsCfg aws.Config, cfg Config, services []common.ServiceType) Config {
+	out, err := enforceRebuyScope(ctx, awsCfg, cfg, services)
+	if err != nil {
+		exitIfInterrupted(ctx, "checking the rebuy scope")
+		exitOnFetchError(err)
+	}
+	return out
+}
+
+// requireRecPreconditions runs the checks every fetched batch must pass
+// before filtering: the scope check first, then the RDS inventory check.
+func requireRecPreconditions(recs []common.Recommendation, cfg Config, data engineVersionData) error {
+	if err := requireRebuyRecScope(recs, cfg); err != nil {
+		return err
+	}
+	return requireRegionInventory(recs, cfg, data)
+}
+
+// requireRebuyRecScope refuses a gated run when an RI recommendation belongs to
+// an account other than the credentials'. The existing-commitment inventory
+// reads only the credentials' own account, so expiry netting against another
+// account's demand would be wrong. A recommendation with no account passes
+// here; the scope probe covers it, over the longer of --coverage-lookback-days
+// and --rec-lookback-period, so usage the recommendations were sized on is seen.
+func requireRebuyRecScope(recs []common.Recommendation, cfg Config) error {
+	for i := range recs {
+		if !rebuyGated(cfg, recs[i].Service) {
+			continue
+		}
+		err := checkPurchaseAccount(recs[i], cfg.CallerAccountID)
+		if err == nil {
+			continue
+		}
+		if cfg.CallerAccountID == "" {
+			return scopeCheckFailure(err)
+		}
+		return refuseRebuyScope("%s recommendation for account %s in %s, but the credentials are for account %s; %s",
+			recs[i].Service, recs[i].Account, recs[i].Region, cfg.CallerAccountID, rebuyScopeRemedy)
+	}
+	return nil
+}
+
+// fetchRebuyInventory reads the owned commitments the sizing step deducts
+// expiring RIs from. Under the gate a missing client or a failed read refuses:
+// sizing without the inventory would silently skip the expiry netting.
+// Savings Plans and a gate that is off keep the warning and continue with no
+// inventory.
+func fetchRebuyInventory(ctx context.Context, client provider.ServiceClient, service common.ServiceType, cfg Config) ([]common.Commitment, error) {
+	if cfg.RebuyWindowDays <= 0 {
+		return nil, nil
+	}
+	gated := rebuyGated(cfg, service)
+	if client == nil {
+		if gated {
+			return nil, scopeCheckFailure(fmt.Errorf("no %s client to read existing commitments", service))
+		}
+		return nil, nil
+	}
+	commits, err := client.GetExistingCommitments(ctx)
+	if err == nil {
+		return commits, nil
+	}
+	if gated {
+		return nil, scopeCheckFailure(fmt.Errorf("reading existing %s commitments: %w", service, err))
+	}
+	AppLogger.Printf("  ⚠️  Could not fetch existing commitments for expiry check: %v\n", err)
+	return nil, nil
+}