diff --git a/cloudformation/stacks/CUDly/template.yaml b/cloudformation/stacks/CUDly/template.yaml index 8da66879f..d2d5fd717 100644 --- a/cloudformation/stacks/CUDly/template.yaml +++ b/cloudformation/stacks/CUDly/template.yaml @@ -420,6 +420,7 @@ Resources: - Sid: CostExplorer Effect: Allow Action: + - ce:GetCostAndUsage - ce:GetReservationPurchaseRecommendation - ce:GetReservationUtilization - ce:GetReservationCoverage @@ -458,8 +459,20 @@ Resources: - ec2:AcceptReservedInstancesExchangeQuote - ec2:DescribeRegions - ec2:DescribeInstanceTypeOfferings + - ec2:DescribeInstanceTypes + - ec2:CreateReservedInstancesListing + - ec2:DescribeReservedInstancesListings + - ec2:CancelReservedInstancesListing Resource: "*" + # Post-purchase tagging of EC2 RIs; the only action here that + # supports resource-level scoping. + - Sid: EC2ReservedInstanceTagging + Effect: Allow + Action: + - ec2:CreateTags + Resource: "arn:aws:ec2:*:*:reserved-instances/*" + # OpenSearch Reserved Instances - Sid: OpenSearchReservedInstances Effect: Allow diff --git a/terraform/modules/compute/aws/fargate/main.tf b/terraform/modules/compute/aws/fargate/main.tf index 31b6362ae..546dc44b0 100644 --- a/terraform/modules/compute/aws/fargate/main.tf +++ b/terraform/modules/compute/aws/fargate/main.tf @@ -347,7 +347,12 @@ resource "aws_iam_role_policy" "ri_exchange" { "ec2:AcceptReservedInstancesExchangeQuote", "ec2:PurchaseReservedInstancesOffering", "ec2:DescribeInstanceTypeOfferings", + "ec2:DescribeInstanceTypes", "ec2:DescribeRegions", + # EC2 RI Marketplace listings (sell path) + "ec2:CreateReservedInstancesListing", + "ec2:DescribeReservedInstancesListings", + "ec2:CancelReservedInstancesListing", # RDS reserved instances "rds:DescribeReservedDBInstances", "rds:DescribeReservedDBInstancesOfferings", @@ -370,6 +375,7 @@ resource "aws_iam_role_policy" "ri_exchange" { "memorydb:DescribeReservedNodesOfferings", "memorydb:PurchaseReservedNodesOffering", # Cost Explorer + "ce:GetCostAndUsage", "ce:GetReservationUtilization", "ce:GetReservationPurchaseRecommendation", "ce:GetReservationCoverage", @@ -383,6 +389,14 @@ resource "aws_iam_role_policy" "ri_exchange" { "savingsplans:CreateSavingsPlan", ] Resource = "*" + }, + { + # Post-purchase tagging of EC2 RIs (tagReservedInstance in + # providers/aws/services/ec2/client.go). Unlike the purchase and + # describe actions above, this one supports resource-level scoping. + Effect = "Allow" + Action = ["ec2:CreateTags"] + Resource = "arn:aws:ec2:*:*:reserved-instances/*" } ] }) diff --git a/terraform/modules/compute/aws/lambda/main.tf b/terraform/modules/compute/aws/lambda/main.tf index 133b6e60e..acbdbd4ae 100644 --- a/terraform/modules/compute/aws/lambda/main.tf +++ b/terraform/modules/compute/aws/lambda/main.tf @@ -355,7 +355,12 @@ resource "aws_iam_role_policy" "ri_exchange" { "ec2:AcceptReservedInstancesExchangeQuote", "ec2:PurchaseReservedInstancesOffering", "ec2:DescribeInstanceTypeOfferings", + "ec2:DescribeInstanceTypes", "ec2:DescribeRegions", + # EC2 RI Marketplace listings (sell path) + "ec2:CreateReservedInstancesListing", + "ec2:DescribeReservedInstancesListings", + "ec2:CancelReservedInstancesListing", # RDS reserved instances "rds:DescribeReservedDBInstances", "rds:DescribeReservedDBInstancesOfferings", @@ -378,6 +383,7 @@ resource "aws_iam_role_policy" "ri_exchange" { "memorydb:DescribeReservedNodesOfferings", "memorydb:PurchaseReservedNodesOffering", # Cost Explorer + "ce:GetCostAndUsage", "ce:GetReservationUtilization", "ce:GetReservationPurchaseRecommendation", "ce:GetReservationCoverage", @@ -391,6 +397,14 @@ resource "aws_iam_role_policy" "ri_exchange" { "savingsplans:CreateSavingsPlan", ] Resource = "*" + }, + { + # Post-purchase tagging of EC2 RIs (tagReservedInstance in + # providers/aws/services/ec2/client.go). Unlike the purchase and + # describe actions above, this one supports resource-level scoping. + Effect = "Allow" + Action = ["ec2:CreateTags"] + Resource = "arn:aws:ec2:*:*:reserved-instances/*" } ] }) diff --git a/terraform/modules/compute/aws/runtime_iam_coverage_test.go b/terraform/modules/compute/aws/runtime_iam_coverage_test.go new file mode 100644 index 000000000..06b13146e --- /dev/null +++ b/terraform/modules/compute/aws/runtime_iam_coverage_test.go @@ -0,0 +1,258 @@ +// Detects CE/EC2 action names missing from runtime IaC, including gaps shared +// by all three flavors that check-aws-iam-parity.sh cannot detect (#1967/#1968). +// This is source/action-presence coverage, not IAM evaluation: Effect, resource +// scope, role attachment, and inline comments require separate review. Unused +// grants (#1322) and other service namespaces are outside this guard's scope. +package aws_test + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "testing" +) + +// sdkServiceToIAMPrefix maps the CE/EC2 aws-sdk-go-v2 service package names to +// the IAM action prefixes they authorize against. Cost Explorer's package is +// costexplorer but its actions are ce:*. sts is deliberately absent: +// GetCallerIdentity requires no IAM permission and must never enter the set. +var sdkServiceToIAMPrefix = map[string]string{ + "costexplorer": "ce", + "ec2": "ec2", +} + +// requiredDerivedActions is the floor from #1967/#1968: actions the code is +// known to call that no runtime flavor grants today. Asserting the scanner +// finds these (independent of whether any file grants them) catches a broken +// walker directly, rather than letting it pass by inspecting nothing. +var requiredDerivedActions = []string{ + "ce:GetCostAndUsage", + "ec2:CreateReservedInstancesListing", + "ec2:DescribeReservedInstancesListings", + "ec2:CancelReservedInstancesListing", + "ec2:DescribeInstanceTypes", + "ec2:CreateTags", +} + +// scanRoots are walked, relative to the repo root, for SDK call sites. +// +// pkg/ is included because internal/ imports it at runtime: pkg/exchange +// builds the RI exchange quote and accept inputs, reached from +// internal/api/handler_ri_exchange.go and internal/server/ladder_write.go. +// Being a separate Go module does not matter here; the walker reads files. +// +// cmd/ is excluded because the CLI runs under operator credentials, a +// different identity than the runtime role this test guards +// (organizations:DescribeAccount is CLI-only for exactly this reason, per +// #1322). Note cmd/server IS a runtime entry point: the exclusion is safe +// only while cmd/server and cmd/cudly-mcp import no SDK service package +// directly, which holds today. If either starts issuing its own SDK calls, +// add it here. +var scanRoots = []string{ + filepath.Join("providers", "aws"), + "internal", + "pkg", +} + +// runtimeIAMFiles are the IaC files that grant the runtime role's IAM +// actions, relative to this package's directory (the three flavors compared +// by check-aws-iam-parity.sh comparison 1). +var runtimeIAMFiles = []string{ + filepath.Join("lambda", "main.tf"), + filepath.Join("fargate", "main.tf"), + filepath.Join("..", "..", "..", "..", "cloudformation", "stacks", "CUDly", "template.yaml"), +} + +// calledAction records where a derived action was first seen, so a failure +// message names a place the reader can open. +type calledAction struct { + file string + line int +} + +func TestRuntimeGrantsEveryCalledAction(t *testing.T) { + called := deriveCalledActions(t) + + for _, action := range requiredDerivedActions { + if _, ok := called[action]; !ok { + t.Errorf("scanner did not derive %s from %v; it is a known SDK call the runtime role must be granted (#1967/#1968) and its absence here means the walker is broken, not that the call went away", action, scanRoots) + } + } + + for _, rel := range runtimeIAMFiles { + t.Run(rel, func(t *testing.T) { + granted := grantedActions(t, rel) + for action, site := range called { + if !granted[action] { + t.Errorf("%s does not grant %s, called at %s:%d", rel, action, site.file, site.line) + } + } + }) + } +} + +// deriveCalledActions walks scanRoots and returns every IAM action implied by +// an SDK *Input request literal, keyed by action. +func deriveCalledActions(t *testing.T) map[string]calledAction { + t.Helper() + + root := repoRoot(t) + fset := token.NewFileSet() + actions := map[string]calledAction{} + filesScanned := 0 + + for _, rel := range scanRoots { + dir := filepath.Join(root, rel) + err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() || filepath.Ext(path) != ".go" || strings.HasSuffix(path, "_test.go") { + return nil + } + filesScanned++ + return scanFileForActions(t, fset, root, path, actions) + }) + if err != nil { + t.Fatalf("walking %s: %v", dir, err) + } + } + + if filesScanned == 0 { + t.Fatalf("walked %v and parsed zero Go files; every assertion below would pass by inspecting nothing", scanRoots) + } + if len(actions) == 0 { + t.Fatalf("derived zero SDK actions from %v; a broken import or literal match would pass every assertion below by inspecting nothing", scanRoots) + } + return actions +} + +// scanFileForActions parses one Go file and records every action its SDK +// *Input literals imply into actions, keyed by action so the first call site +// wins. +func scanFileForActions(t *testing.T, fset *token.FileSet, root, path string, actions map[string]calledAction) error { + t.Helper() + + file, parseErr := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution) + if parseErr != nil { + t.Fatalf("parsing %s: %v", path, parseErr) + } + + aliasToPrefix := sdkImportAliases(file) + if len(aliasToPrefix) == 0 { + return nil + } + + relPath, relErr := filepath.Rel(root, path) + if relErr != nil { + return relErr + } + + ast.Inspect(file, func(n ast.Node) bool { + action, ok := actionFromCompositeLit(n, aliasToPrefix) + if !ok { + return true + } + if _, exists := actions[action]; exists { + return true + } + actions[action] = calledAction{file: relPath, line: fset.Position(n.Pos()).Line} + return true + }) + return nil +} + +// actionFromCompositeLit reports the IAM action implied by n, if n is a +// composite literal of an SDK *Input request type whose package alias +// resolves through aliasToPrefix (e.g. &ec2.CreateTagsInput{...} -> "ec2:CreateTags"). +func actionFromCompositeLit(n ast.Node, aliasToPrefix map[string]string) (string, bool) { + cl, ok := n.(*ast.CompositeLit) + if !ok { + return "", false + } + sel, ok := cl.Type.(*ast.SelectorExpr) + if !ok { + return "", false + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return "", false + } + prefix, ok := aliasToPrefix[ident.Name] + if !ok { + return "", false + } + typeName := sel.Sel.Name + op := strings.TrimSuffix(typeName, "Input") + if op == "" || op == typeName { + return "", false + } + return prefix + ":" + op, true +} + +// sdkImportAliases returns, for one file, the map from the local identifier +// an aws-sdk-go-v2 service package is used under to the IAM prefix it +// authorizes against. A subpackage import such as .../service/ec2/types is +// deliberately excluded: it defines the enum and shape types the *Input +// structs embed, not the *Input structs themselves, so its alias must never +// stand in for the service package's. +func sdkImportAliases(file *ast.File) map[string]string { + const svcPrefix = "github.com/aws/aws-sdk-go-v2/service/" + + aliases := map[string]string{} + for _, imp := range file.Imports { + path, err := strconv.Unquote(imp.Path.Value) + if err != nil { + continue + } + rest := strings.TrimPrefix(path, svcPrefix) + if rest == path || strings.Contains(rest, "/") { + continue + } + prefix, ok := sdkServiceToIAMPrefix[rest] + if !ok { + continue + } + alias := rest + if imp.Name != nil { + if imp.Name.Name == "_" || imp.Name.Name == "." { + continue + } + alias = imp.Name.Name + } + aliases[alias] = prefix + } + return aliases +} + +// grantedActionPattern is the CE/EC2 subset of check-aws-iam-parity.sh's +// extraction regex, rewritten with a capture group so the match includes only +// the action itself, not the boundary byte before it. +var grantedActionPattern = regexp.MustCompile(`(?:^|[^A-Za-z])((?:ce|ec2):[A-Z][A-Za-z]+)`) + +// hashCommentLinePattern matches a whole line whose first non-blank byte is +// #, the comment style both Terraform and CloudFormation YAML use here. +var hashCommentLinePattern = regexp.MustCompile(`(?m)^[ \t]*#.*$`) + +// grantedActions extracts action names from path after stripping whole-line +// # comments. It does not parse policy semantics or other comment forms. +func grantedActions(t *testing.T, path string) map[string]bool { + t.Helper() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading %s: %v", path, err) + } + content := hashCommentLinePattern.ReplaceAllString(string(data), "") + + granted := map[string]bool{} + for _, m := range grantedActionPattern.FindAllStringSubmatch(content, -1) { + granted[m[1]] = true + } + return granted +}