diff --git a/cmd/main.go b/cmd/main.go index 8a25aa9a8..eb77a29be 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -37,6 +37,10 @@ const ( // Config holds all configuration for the RI helper tool. type Config struct { + // CallerAccountID is the account the credentials belong to (STS + // GetCallerIdentity), set at run start. Purchases are refused for + // recommendations owned by any other account. + CallerAccountID string AuditLog string CSVInput string IdempotencyWindow string diff --git a/cmd/main_test.go b/cmd/main_test.go index 87dc3ada0..41e5036db 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -31,6 +31,12 @@ func TestMain(m *testing.M) { engineVersionFetcher = func(context.Context, Config, bool) (engineVersionData, error) { return engineVersionData{}, nil } + // An empty account turns the cross-account purchase check off for tests + // that do not set CallerAccountID; the check is tested explicitly in + // purchase_account_guard_test.go. + callerAccountFetcher = func(context.Context, aws.Config) (string, error) { + return "", nil + } code := m.Run() _ = os.Remove(toolCfg.AuditLog) os.Exit(code) diff --git a/cmd/multi_service.go b/cmd/multi_service.go index 2085869b6..f80cc05fd 100644 --- a/cmd/multi_service.go +++ b/cmd/multi_service.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "fmt" "log" "sync/atomic" @@ -15,6 +16,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" "github.com/aws/aws-sdk-go-v2/aws" awsconfig "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/google/uuid" ) @@ -238,6 +240,10 @@ func exitIfInterrupted(ctx context.Context, phase string) { // runToolMultiService within the cyclomatic-complexity limit. func runPurchaseAndReport(ctx context.Context, awsCfg aws.Config, scoredResult scorer.ScoredResult, isDryRun bool, cfg Config, drops *common.DropSummary) error { runID := uuid.New().String() + cfg, err := withCallerAccount(ctx, awsCfg, cfg) + if err != nil { + return err + } if !confirmPurchaseRun(scoredResult.Passed, isDryRun) { printDropSummary(drops) AppLogger.Printf("\nāŒ Purchase canceled.\n") @@ -302,6 +308,32 @@ func printDropSummary(drops *common.DropSummary) { } } +// callerAccountFetcher is the seam tests replace so they never reach STS. +var callerAccountFetcher = fetchCallerAccount + +func fetchCallerAccount(ctx context.Context, awsCfg aws.Config) (string, error) { + out, err := sts.NewFromConfig(awsCfg).GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{}) + if err != nil { + return "", err + } + if aws.ToString(out.Account) == "" { + return "", errors.New("GetCallerIdentity returned no account") + } + return aws.ToString(out.Account), nil +} + +// withCallerAccount returns cfg with CallerAccountID resolved, so the purchase +// preconditions can refuse recommendations that belong to another account. It +// runs on dry runs too, so a dry run predicts what a real run will refuse. +func withCallerAccount(ctx context.Context, awsCfg aws.Config, cfg Config) (Config, error) { + account, err := callerAccountFetcher(ctx, awsCfg) + if err != nil { + return cfg, fmt.Errorf("cannot determine the AWS account of the credentials (needed to check purchases): %w", err) + } + cfg.CallerAccountID = account + return cfg, nil +} + // loadAWSConfig builds an aws.Config from the tool config. func loadAWSConfig(ctx context.Context, cfg Config) (aws.Config, error) { var opts []func(*awsconfig.LoadOptions) error @@ -578,7 +610,7 @@ func purchaseAuditStatus(result common.PurchaseResult) string { func purchaseSingleRec(ctx context.Context, awsCfg aws.Config, rec common.Recommendation, index int, isDryRun bool, cfg Config) (purchaseResult common.PurchaseResult, auditStatus string) { label := purchaseRegionLabel(rec, rec.Region) AppLogger.Printf(" [%d] %s %s %s (count=%d)\n", index, rec.Service, label, rec.ResourceType, rec.Count) - if err := validatePurchasePreconditions(rec, rec.Region); err != nil { + if err := validatePurchasePreconditions(rec, rec.Region, cfg.CallerAccountID); err != nil { AppLogger.Printf(" āŒ cannot purchase: %v\n", err) return preconditionFailure(rec, err, isDryRun), "error" } @@ -721,6 +753,10 @@ func runToolFromCSV(ctx context.Context, cfg Config) error { return nil } + cfg, err = withCallerAccount(ctx, awsCfg, cfg) + if err != nil { + return err + } return runCSVPurchases(ctx, awsCfg, recs, isDryRun, cfg, runID) } @@ -990,7 +1026,7 @@ func processPurchaseLoop(ctx context.Context, recs []common.Recommendation, regi // runOnePurchase validates and then dry-runs or executes one CSV-path // recommendation. attempted reports whether a real purchase call was made. func runOnePurchase(ctx context.Context, rec common.Recommendation, label, region string, index int, isDryRun bool, serviceClient provider.ServiceClient, cfg Config) (result common.PurchaseResult, status string, attempted bool) { - if err := validatePurchasePreconditions(rec, region); err != nil { + if err := validatePurchasePreconditions(rec, region, cfg.CallerAccountID); err != nil { return preconditionFailure(rec, err, isDryRun), "error", false } if isDryRun { diff --git a/cmd/purchase_account_guard_test.go b/cmd/purchase_account_guard_test.go new file mode 100644 index 000000000..b3c1e1073 --- /dev/null +++ b/cmd/purchase_account_guard_test.go @@ -0,0 +1,90 @@ +package main + +import ( + "context" + "errors" + "path/filepath" + "testing" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +const ( + testCallerAccount = "640848491082" + otherAccount = "726468838177" +) + +func TestCheckPurchaseAccount(t *testing.T) { + rec := common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", Account: testCallerAccount} + + require.NoError(t, checkPurchaseAccount(rec, testCallerAccount)) + + rec.Account = "" + require.NoError(t, checkPurchaseAccount(rec, testCallerAccount), "a row with no account has nothing to compare") + + rec.Account = otherAccount + err := checkPurchaseAccount(rec, testCallerAccount) + require.Error(t, err) + assert.Contains(t, err.Error(), otherAccount) + assert.Contains(t, err.Error(), testCallerAccount) + +} + +func TestPurchaseSingleRec_RefusesOtherAccountsRecommendation(t *testing.T) { + captureAppLog(t) + cfg := Config{AuditLog: filepath.Join(t.TempDir(), "audit.jsonl"), CallerAccountID: testCallerAccount} + rec := validEC2Rec() + rec.Account = otherAccount + + for _, dry := range []bool{true, false} { + // A real run must refuse before it builds a service client: aws.Config{} + // has no credentials, so reaching the purchase call would fail + // differently, not with the account message. + result, status := purchaseSingleRec(context.Background(), aws.Config{}, rec, 1, dry, cfg) + assert.False(t, result.Success, "dryRun=%v", dry) + require.Error(t, result.Error, "dryRun=%v", dry) + assert.Contains(t, result.Error.Error(), "cross-account purchases are not supported", "dryRun=%v", dry) + assert.Equal(t, "error", status) + } +} + +func TestProcessPurchaseLoop_RefusesOtherAccountsRecommendationWithoutPurchasing(t *testing.T) { + captureAppLog(t) + cfg := Config{AuditLog: filepath.Join(t.TempDir(), "audit.jsonl"), CallerAccountID: testCallerAccount} + other := validEC2Rec() + other.Account = otherAccount + own := validEC2Rec() + own.Account = testCallerAccount + client := &MockServiceClient{} + client.On("PurchaseCommitment", mock.Anything, mock.MatchedBy(func(r common.Recommendation) bool { return r.Account == testCallerAccount }), mock.Anything). + Return(common.PurchaseResult{Success: true, CommitmentID: "ri-own"}, nil).Once() + + results, err := processPurchaseLoop(context.Background(), []common.Recommendation{other, own}, "us-east-1", false, client, cfg, "run-1") + + require.NoError(t, err) + require.Len(t, results, 2) + assert.False(t, results[0].Success) + require.Error(t, results[0].Error) + assert.Contains(t, results[0].Error.Error(), otherAccount) + assert.True(t, results[1].Success) + client.AssertNumberOfCalls(t, "PurchaseCommitment", 1) +} + +func TestWithCallerAccount(t *testing.T) { + orig := callerAccountFetcher + t.Cleanup(func() { callerAccountFetcher = orig }) + + callerAccountFetcher = func(context.Context, aws.Config) (string, error) { return "640848491082", nil } + cfg, err := withCallerAccount(context.Background(), aws.Config{}, Config{}) + require.NoError(t, err) + assert.Equal(t, "640848491082", cfg.CallerAccountID) + + callerAccountFetcher = func(context.Context, aws.Config) (string, error) { return "", errors.New("expired token") } + _, err = withCallerAccount(context.Background(), aws.Config{}, Config{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "expired token") +} diff --git a/cmd/savings_plan_purchase.go b/cmd/savings_plan_purchase.go index 80eab2452..6fe98e168 100644 --- a/cmd/savings_plan_purchase.go +++ b/cmd/savings_plan_purchase.go @@ -92,7 +92,10 @@ func purchaseRegionLabel(rec common.Recommendation, groupRegion string) string { // what the library would reject for a missing or malformed field; it does not // call any API. region is the one the client will be built from: the // recommendation's own on the main path, the CSV group's on the CSV path. -func validatePurchasePreconditions(rec common.Recommendation, region string) error { +func validatePurchasePreconditions(rec common.Recommendation, region, callerAccount string) error { + if err := checkPurchaseAccount(rec, callerAccount); err != nil { + return err + } if _, err := clientRegionFor(rec.Service, region); err != nil { return err } @@ -111,6 +114,22 @@ func validatePurchasePreconditions(rec common.Recommendation, region string) err return nil } +// checkPurchaseAccount refuses a recommendation owned by an account other than +// the one the credentials belong to. Cost Explorer returns linked-account +// recommendations to a payer, but a purchase call buys in the caller's own +// account, so buying one would commit the caller's account to another +// account's usage. callerAccount is set by withCallerAccount, which both +// purchase entry points run first and which fails the run when the account +// cannot be resolved; empty here means a caller below those entry points (unit +// tests) did not resolve it. A recommendation with no account (CSV rows may +// omit it) carries nothing to compare. +func checkPurchaseAccount(rec common.Recommendation, callerAccount string) error { + if callerAccount != "" && rec.Account != "" && rec.Account != callerAccount { + return fmt.Errorf("recommendation is for account %s but the credentials are for account %s; cross-account purchases are not supported, so run with credentials for account %s", rec.Account, callerAccount, rec.Account) + } + return nil +} + func validateSavingsPlanPreconditions(rec common.Recommendation) error { d, ok := rec.Details.(*common.SavingsPlanDetails) if !ok || d == nil { diff --git a/cmd/savings_plan_purchase_test.go b/cmd/savings_plan_purchase_test.go index d2b804c8e..5dd01b624 100644 --- a/cmd/savings_plan_purchase_test.go +++ b/cmd/savings_plan_purchase_test.go @@ -262,7 +262,7 @@ func TestValidatePurchasePreconditions(t *testing.T) { "rds": {Service: common.ServiceRDS, Region: "us-east-1"}, } for name, rec := range ok { - assert.NoError(t, validatePurchasePreconditions(rec, rec.Region), name) + assert.NoError(t, validatePurchasePreconditions(rec, rec.Region, ""), name) } bad := map[string]common.Recommendation{ "sp nil details": mut(parserShapedSP(), func(r *common.Recommendation) { r.Details = nil }), @@ -282,7 +282,7 @@ func TestValidatePurchasePreconditions(t *testing.T) { "rds no region": {Service: common.ServiceRDS}, } for name, rec := range bad { - assert.Error(t, validatePurchasePreconditions(rec, rec.Region), name) + assert.Error(t, validatePurchasePreconditions(rec, rec.Region, ""), name) } } @@ -351,7 +351,7 @@ func TestCSVRoundTrip_SavingsPlanAndEC2DetailsSurviveWriteThenRead(t *testing.T) assert.Equal(t, ec2.Details, byService[ec2.Service].Details) assert.Equal(t, rds.Details, byService[rds.Service].Details) for _, r := range out { - assert.NoError(t, validatePurchasePreconditions(r, r.Region), "%s", r.Service) + assert.NoError(t, validatePurchasePreconditions(r, r.Region, ""), "%s", r.Service) } } diff --git a/go.mod b/go.mod index 9f915c246..41f8ee0c6 100644 --- a/go.mod +++ b/go.mod @@ -88,6 +88,7 @@ require ( github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261006205158-7ff8c1aee1bb github.com/aws/aws-sdk-go-v2/service/organizations v1.45.3 github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.40.3 + github.com/aws/aws-sdk-go-v2/service/sts v1.33.17 github.com/google/uuid v1.6.0 github.com/microsoftgraph/msgraph-sdk-go v1.99.0 golang.org/x/term v0.46.0 @@ -107,7 +108,6 @@ require ( github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.33.0 // indirect github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.54.0 // indirect github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.33.17 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2 // indirect github.com/envoyproxy/go-control-plane/envoy v1.37.0 // indirect