From ad8f406a5378a51cb8e35c2011c298036e7b0f61 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 5 Oct 2026 19:01:47 +0200 Subject: [PATCH 1/3] test(iac/aws): derive boundary guard walk from the module tree; scope lambda:InvokeFunction findAwsModuleFiles kept only .tf files under an "aws" path segment, so modules/build, modules/deployment-checks and the environment root's own *.tf were invisible to every guard that calls it. The walk now also follows the root's local module blocks transitively, failing closed on a non-local or missing source. lambda:InvokeFunction moves out of the Resource "*" WorkloadServiceCeiling into its own statement scoped to arn:aws:lambda:*:*:function:cudly-*; the API Lambda's self-invoke is the only workload grant and matches it. Closes #511 --- .../aws/ci-cd-permissions/policy_boundary.tf | 32 ++-- .../ci-cd-permissions/policy_guard_test.go | 161 +++++++++++++++++- 2 files changed, 171 insertions(+), 22 deletions(-) diff --git a/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf b/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf index aeee4534f..6832db877 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf @@ -127,20 +127,11 @@ resource "aws_iam_policy" "workload_boundary" { # lambda:InvokeFunctionUrl in aws_lambda_permission blocks are # RESOURCE policies granting a service principal, not grants to a # workload role, so this ceiling never gates them. - # lambda:InvokeFunction keeps a RESIDUAL of the same shape as - # ecs:RunTask's, stated here so it is not read as fully closed: - # this statement's Resource is "*", so it permits invoking ANY - # function in the account, which runs that function's code as its - # execution role with an attacker-chosen payload and needs no - # iam:PassRole. Functions this deploy role did not create carry no - # boundary. It is narrower than it looks, because the module grants - # are themselves scoped (modules/compute/aws/lambda/main.tf uses - # aws_lambda_function.main.arn, signing-key.tf uses - # arn:aws:lambda:*:*:function:${stack_name}-api*) and effective - # permissions are the intersection, so the residual is only - # reachable by a role whose own identity policy is wider. Scoping - # this entry to arn:aws:lambda:*:*:function:cudly-* would close it - # and is tracked separately rather than folded into #1723. + # lambda:InvokeFunction is pinned to cudly-* functions in the + # LambdaInvokeCeiling statement below rather than listed here, + # because this statement's Resource is "*" and an invoke runs the + # target's code as its execution role with a caller-chosen payload + # and no iam:PassRole. # - the ssm entries are AmazonSSMManagedInstanceCore v2 verbatim (the # fck-nat instance role in modules/networking/aws is the only role # that carries it, for Session Manager access). No module grants an @@ -169,7 +160,6 @@ resource "aws_iam_policy" "workload_boundary" { "es:*", "kms:*", "lambda:GetFunctionUrlConfig", - "lambda:InvokeFunction", "logs:*", "memorydb:*", "rds:*", @@ -197,6 +187,18 @@ resource "aws_iam_policy" "workload_boundary" { ] Resource = "*" }, + { + # The API Lambda's async self-invoke (modules/compute/aws/lambda/main.tf + # async_self_invoke, scoped there to the function's own ARN) is the only + # workload grant of lambda:InvokeFunction. The deploy role can only + # create cudly-* functions (policy_compute.tf), and the function is + # named "${stack_name}-api" with stack_name starting with project_name + # (default "cudly"), so cudly-* matches it and nothing broader is needed. + Sid = "LambdaInvokeCeiling" + Effect = "Allow" + Action = ["lambda:InvokeFunction"] + Resource = "arn:aws:lambda:*:*:function:cudly-*" + }, { # organizations and sts are narrowed for the same reason ecs, lambda and # ssm are pinned per action in WorkloadServiceCeiling above: at diff --git a/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go b/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go index 06f136b5f..d4de93280 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go +++ b/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go @@ -34,6 +34,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "sort" "strings" "testing" @@ -310,14 +311,53 @@ func unreadListElements(stmt string, assignPattern *regexp.Regexp) string { return strings.Join(refs, " ") } -// findAwsModuleFiles returns every *.tf file under modulesDir whose -// directory path contains an "aws" path segment, i.e. the AWS-specific -// Terraform modules and not their Azure/GCP siblings. +// moduleBlockPattern matches the opening of a `module "" {` block. +var moduleBlockPattern = regexp.MustCompile(`(?m)^[ \t]*module\s+"[^"]*"\s*\{`) + +// moduleSourcePattern captures the source argument of a module block body. +var moduleSourcePattern = regexp.MustCompile(`(?m)^[ \t]*source\s*=\s*"([^"]*)"`) + +// findAwsModuleFiles returns every *.tf file on the AWS deploy path: those +// under modulesDir whose directory path contains an "aws" path segment, plus +// everything deployPathFiles derives from envRootDir. func findAwsModuleFiles(t *testing.T) []string { t.Helper() + return deployPathFiles(t, modulesDir, envRootDir) +} - var files []string - err := filepath.WalkDir(modulesDir, func(path string, d fs.DirEntry, walkErr error) error { +// deployPathFiles returns the *.tf files the deploy actually applies: the +// "aws" path segment heuristic under modulesDir, unioned with envRoot's own +// *.tf files and every module they instantiate, followed transitively through +// local `source = "./..."` / `"../..."` module blocks. +// +// The path heuristic alone is how a module without an "aws" segment +// (modules/build, modules/deployment-checks) and the root *.tf files were left +// out of every guard that calls this, so an IAM policy added there was never +// compared against the boundary. Following the module blocks derives the set +// from what the root instantiates rather than from a directory naming +// convention. It fails closed: a module source that is not a local directory, +// or a local one that does not exist, aborts the test, because a module this +// walk cannot read is a module no guard can see. +func deployPathFiles(t *testing.T, modules, envRoot string) []string { + t.Helper() + + seen := map[string]bool{} + addDir := func(dir string) []string { + entries, err := filepath.Glob(filepath.Join(dir, "*.tf")) + if err != nil { + t.Fatalf("globbing %s: %v", dir, err) + } + var added []string + for _, e := range entries { + if !seen[e] { + seen[e] = true + added = append(added, e) + } + } + return added + } + + err := filepath.WalkDir(modules, func(path string, d fs.DirEntry, walkErr error) error { if walkErr != nil { return walkErr } @@ -326,15 +366,43 @@ func findAwsModuleFiles(t *testing.T) []string { } for _, part := range strings.Split(filepath.ToSlash(filepath.Dir(path)), "/") { if part == "aws" { - files = append(files, path) + seen[path] = true break } } return nil }) if err != nil { - t.Fatalf("walking %s for aws module .tf files: %v", modulesDir, err) + t.Fatalf("walking %s for aws module .tf files: %v", modules, err) } + + queue := addDir(envRoot) + for len(queue) > 0 { + f := queue[0] + queue = queue[1:] + content := readPolicySource(t, f) + for _, loc := range moduleBlockPattern.FindAllStringIndex(content, -1) { + body := content[loc[1]-1 : balancedBraceEnd(content, loc[1]-1)] + m := moduleSourcePattern.FindStringSubmatch(body) + if m == nil { + t.Fatalf("%s: module block %q has no literal source; this walk cannot tell which directory it instantiates", f, strings.TrimSpace(content[loc[0]:loc[1]])) + } + if !strings.HasPrefix(m[1], "./") && !strings.HasPrefix(m[1], "../") { + t.Fatalf("%s: module source %q is not a local directory; this walk cannot read a remote module's IAM policies, so the boundary guards would not see them", f, m[1]) + } + dir := filepath.Join(filepath.Dir(f), m[1]) + if info, err := os.Stat(dir); err != nil || !info.IsDir() { + t.Fatalf("%s: module source %q resolves to %s, which is not a readable directory", f, m[1], dir) + } + queue = append(queue, addDir(dir)...) + } + } + + files := make([]string, 0, len(seen)) + for f := range seen { + files = append(files, f) + } + sort.Strings(files) return files } @@ -1767,3 +1835,82 @@ func TestKMSDataPlaneActionsAreNotUnconditionallyGranted(t *testing.T) { t.Fatalf("scanned zero statements; this guard would pass vacuously") } } + +// TestDeployPathWalkReachesModulesWithoutAwsSegment pins the walk behind every +// module-tree guard to what the environment root instantiates. The fixture is +// the shape that was invisible to the old "aws" path-segment heuristic: a +// module reached through a root `module` block whose directory has no "aws" +// segment, granting an action the boundary does not carry. The guard's own +// coverage comparison must then report it. +func TestDeployPathWalkReachesModulesWithoutAwsSegment(t *testing.T) { + root := t.TempDir() + write := func(rel, content string) { + t.Helper() + p := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + write("env/main.tf", "module \"build\" {\n source = \"../modules/build\"\n}\n") + write("modules/build/main.tf", `resource "aws_iam_role_policy" "x" { + policy = jsonencode({ + Statement = [ + { + Effect = "Allow" + Action = ["ecs:DeleteCluster", "lambda:DeleteFunction"] + Resource = "*" + } + ] + }) +} +`) + write("modules/unreferenced/main.tf", "# no aws segment and no module block points here\n") + + files := deployPathFiles(t, filepath.Join(root, "modules"), filepath.Join(root, "env")) + + want := filepath.Join(root, "modules", "build", "main.tf") + if !slices.Contains(files, want) { + t.Fatalf("deployPathFiles = %v, missing %s, which env/main.tf instantiates", files, want) + } + if slices.Contains(files, filepath.Join(root, "modules", "unreferenced", "main.tf")) { + t.Errorf("deployPathFiles = %v includes a module nothing instantiates", files) + } + + allowed := boundaryAllowedActions(t) + var uncovered []string + for _, f := range files { + for _, action := range extractActionListActions(readPolicySource(t, f)) { + if !actionPermitted(allowed, action) { + uncovered = append(uncovered, action) + } + } + } + sort.Strings(uncovered) + if got, wantActions := strings.Join(uncovered, ","), "ecs:DeleteCluster,lambda:DeleteFunction"; got != wantActions { + t.Errorf("actions reported uncovered by the boundary = %q, want %q", got, wantActions) + } +} + +// TestBoundaryInvokeFunctionIsScopedToCudlyFunctions asserts the boundary never +// allows lambda:InvokeFunction on a Resource wider than cudly-* functions. The +// only workload role that uses it is the API Lambda's self-invoke, scoped to +// its own ARN; "*" would let a boundaried role run any function in the account +// as that function's execution role with a caller-chosen payload. +func TestBoundaryInvokeFunctionIsScopedToCudlyFunctions(t *testing.T) { + const wantResource = "arn:aws:lambda:*:*:function:cudly-*" + + stmts := findStatements(t, boundaryFile, func(stmt string) bool { + return statementEffect(stmt) != "Deny" && actionPermitted(extractActionListActions(stmt), "lambda:InvokeFunction") + }) + if len(stmts) == 0 { + t.Fatalf("%s no longer allows lambda:InvokeFunction; the API Lambda's async self-invoke (modules/compute/aws/lambda/main.tf) would 403 at runtime", boundaryFile) + } + for _, stmt := range stmts { + if got := statementResources(stmt); !equalStringSets(got, []string{wantResource}) { + t.Errorf("%s: statement %q allows lambda:InvokeFunction on Resource %v, want exactly [%q]", boundaryFile, statementSid(stmt), got, wantResource) + } + } +} From 4b360a0b4a51492c4ceacfee85d391af4a7ca747 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 5 Oct 2026 19:12:34 +0200 Subject: [PATCH 2/3] test(iac/aws): cover the guard walk's fail-closed aborts Extract the deployPathFiles walk into walkDeployPath, which returns the abort as an error, and assert each abort (no literal source, non-local source, missing local directory) is a hard failure. --- .../ci-cd-permissions/policy_guard_test.go | 80 ++++++++++++++++--- 1 file changed, 69 insertions(+), 11 deletions(-) diff --git a/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go b/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go index d4de93280..7c1f39c64 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go +++ b/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go @@ -341,11 +341,22 @@ func findAwsModuleFiles(t *testing.T) []string { func deployPathFiles(t *testing.T, modules, envRoot string) []string { t.Helper() + files, err := walkDeployPath(modules, envRoot) + if err != nil { + t.Fatal(err) + } + return files +} + +// walkDeployPath is deployPathFiles' body, returning the abort as an error so +// the fail-closed cases can be asserted: t.Fatalf cannot be observed from the +// test that triggers it. +func walkDeployPath(modules, envRoot string) ([]string, error) { seen := map[string]bool{} - addDir := func(dir string) []string { + addDir := func(dir string) ([]string, error) { entries, err := filepath.Glob(filepath.Join(dir, "*.tf")) if err != nil { - t.Fatalf("globbing %s: %v", dir, err) + return nil, fmt.Errorf("globbing %s: %w", dir, err) } var added []string for _, e := range entries { @@ -354,7 +365,7 @@ func deployPathFiles(t *testing.T, modules, envRoot string) []string { added = append(added, e) } } - return added + return added, nil } err := filepath.WalkDir(modules, func(path string, d fs.DirEntry, walkErr error) error { @@ -373,28 +384,39 @@ func deployPathFiles(t *testing.T, modules, envRoot string) []string { return nil }) if err != nil { - t.Fatalf("walking %s for aws module .tf files: %v", modules, err) + return nil, fmt.Errorf("walking %s for aws module .tf files: %w", modules, err) } - queue := addDir(envRoot) + queue, err := addDir(envRoot) + if err != nil { + return nil, err + } for len(queue) > 0 { f := queue[0] queue = queue[1:] - content := readPolicySource(t, f) + data, err := os.ReadFile(f) + if err != nil { + return nil, fmt.Errorf("reading %s: %w", f, err) + } + content := stripCommentLines(string(data)) for _, loc := range moduleBlockPattern.FindAllStringIndex(content, -1) { body := content[loc[1]-1 : balancedBraceEnd(content, loc[1]-1)] m := moduleSourcePattern.FindStringSubmatch(body) if m == nil { - t.Fatalf("%s: module block %q has no literal source; this walk cannot tell which directory it instantiates", f, strings.TrimSpace(content[loc[0]:loc[1]])) + return nil, fmt.Errorf("%s: module block %q has no literal source; this walk cannot tell which directory it instantiates", f, strings.TrimSpace(content[loc[0]:loc[1]])) } if !strings.HasPrefix(m[1], "./") && !strings.HasPrefix(m[1], "../") { - t.Fatalf("%s: module source %q is not a local directory; this walk cannot read a remote module's IAM policies, so the boundary guards would not see them", f, m[1]) + return nil, fmt.Errorf("%s: module source %q is not a local directory; this walk cannot read a remote module's IAM policies, so the boundary guards would not see them", f, m[1]) } dir := filepath.Join(filepath.Dir(f), m[1]) if info, err := os.Stat(dir); err != nil || !info.IsDir() { - t.Fatalf("%s: module source %q resolves to %s, which is not a readable directory", f, m[1], dir) + return nil, fmt.Errorf("%s: module source %q resolves to %s, which is not a readable directory", f, m[1], dir) } - queue = append(queue, addDir(dir)...) + added, err := addDir(dir) + if err != nil { + return nil, err + } + queue = append(queue, added...) } } @@ -403,7 +425,7 @@ func deployPathFiles(t *testing.T, modules, envRoot string) []string { files = append(files, f) } sort.Strings(files) - return files + return files, nil } // readPolicySource reads path and strips its whole-line comments, so every @@ -1914,3 +1936,39 @@ func TestBoundaryInvokeFunctionIsScopedToCudlyFunctions(t *testing.T) { } } } + +// TestDeployPathWalkFailsClosed pins the three aborts in walkDeployPath: each +// must be a returned error, never a silent skip, because a module the walk +// skips is a module no boundary guard sees. +func TestDeployPathWalkFailsClosed(t *testing.T) { + cases := []struct { + name string + module string + wantErr string + }{ + {"no literal source", "module \"m\" {\n source = var.src\n}\n", "has no literal source"}, + {"registry source", "module \"m\" {\n source = \"terraform-aws-modules/iam/aws\"\n version = \"5.0.0\"\n}\n", "is not a local directory"}, + {"git source with ref", "module \"m\" {\n source = \"git::https://example.com/m.git?ref=v1\"\n}\n", "is not a local directory"}, + {"missing local directory", "module \"m\" {\n source = \"../modules/absent\"\n}\n", "not a readable directory"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + for _, d := range []string{"env", "modules"} { + if err := os.MkdirAll(filepath.Join(root, d), 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(root, "env", "main.tf"), []byte(tc.module), 0o644); err != nil { + t.Fatal(err) + } + files, err := walkDeployPath(filepath.Join(root, "modules"), filepath.Join(root, "env")) + if err == nil { + t.Fatalf("walkDeployPath returned %v and no error; want a hard failure containing %q", files, tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("walkDeployPath error = %q, want it to contain %q", err, tc.wantErr) + } + }) + } +} From ab6dd6fc8281d3f5ea69a10e124e73b8a88c3241 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 8 Oct 2026 01:29:47 +0200 Subject: [PATCH 3/3] fix(terraform): traverse seeded AWS modules in boundary guard Queue reachable module files independently of the AWS seed set so nested local modules and unsupported remote sources are inspected. Add seeded-parent regression cases for child coverage and fail-closed remote-source handling. --- .../ci-cd-permissions/policy_guard_test.go | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go b/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go index 7c1f39c64..5524d94b7 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go +++ b/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go @@ -353,6 +353,7 @@ func deployPathFiles(t *testing.T, modules, envRoot string) []string { // test that triggers it. func walkDeployPath(modules, envRoot string) ([]string, error) { seen := map[string]bool{} + queued := map[string]bool{} addDir := func(dir string) ([]string, error) { entries, err := filepath.Glob(filepath.Join(dir, "*.tf")) if err != nil { @@ -360,8 +361,9 @@ func walkDeployPath(modules, envRoot string) ([]string, error) { } var added []string for _, e := range entries { - if !seen[e] { - seen[e] = true + seen[e] = true + if !queued[e] { + queued[e] = true added = append(added, e) } } @@ -1876,7 +1878,8 @@ func TestDeployPathWalkReachesModulesWithoutAwsSegment(t *testing.T) { t.Fatal(err) } } - write("env/main.tf", "module \"build\" {\n source = \"../modules/build\"\n}\n") + write("env/main.tf", "module \"parent\" {\n source = \"../modules/aws/parent\"\n}\n") + write("modules/aws/parent/main.tf", "module \"build\" {\n source = \"../../build\"\n}\n") write("modules/build/main.tf", `resource "aws_iam_role_policy" "x" { policy = jsonencode({ Statement = [ @@ -1895,7 +1898,7 @@ func TestDeployPathWalkReachesModulesWithoutAwsSegment(t *testing.T) { want := filepath.Join(root, "modules", "build", "main.tf") if !slices.Contains(files, want) { - t.Fatalf("deployPathFiles = %v, missing %s, which env/main.tf instantiates", files, want) + t.Fatalf("deployPathFiles = %v, missing %s, which env/main.tf instantiates through modules/aws/parent", files, want) } if slices.Contains(files, filepath.Join(root, "modules", "unreferenced", "main.tf")) { t.Errorf("deployPathFiles = %v includes a module nothing instantiates", files) @@ -1949,6 +1952,7 @@ func TestDeployPathWalkFailsClosed(t *testing.T) { {"no literal source", "module \"m\" {\n source = var.src\n}\n", "has no literal source"}, {"registry source", "module \"m\" {\n source = \"terraform-aws-modules/iam/aws\"\n version = \"5.0.0\"\n}\n", "is not a local directory"}, {"git source with ref", "module \"m\" {\n source = \"git::https://example.com/m.git?ref=v1\"\n}\n", "is not a local directory"}, + {"remote source in AWS parent", "module \"parent\" {\n source = \"../modules/aws/parent\"\n}\n", "is not a local directory"}, {"missing local directory", "module \"m\" {\n source = \"../modules/absent\"\n}\n", "not a readable directory"}, } for _, tc := range cases { @@ -1962,6 +1966,15 @@ func TestDeployPathWalkFailsClosed(t *testing.T) { if err := os.WriteFile(filepath.Join(root, "env", "main.tf"), []byte(tc.module), 0o644); err != nil { t.Fatal(err) } + if tc.name == "remote source in AWS parent" { + parent := filepath.Join(root, "modules", "aws", "parent") + if err := os.MkdirAll(parent, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(parent, "main.tf"), []byte("module \"remote\" {\n source = \"terraform-aws-modules/iam/aws\"\n}\n"), 0o644); err != nil { + t.Fatal(err) + } + } files, err := walkDeployPath(filepath.Join(root, "modules"), filepath.Join(root, "env")) if err == nil { t.Fatalf("walkDeployPath returned %v and no error; want a hard failure containing %q", files, tc.wantErr)