[IAmJSD]

Root cause

enable_cdn = true only changed the Lambda path: it flips the Function URL to AWS_IAM behind CloudFront OAC (compute.tf). On the Fargate path, frontend.tf already puts CloudFront in front of the ALB (the origin is the ALB, or its api-<env> alias, over http-only). But module.networking's ALB security group always admitted HTTP/HTTPS from 0.0.0.0/0 and ::/0, whatever enable_cdn was set to. So with compute_platform = "fargate" and enable_cdn = true, clients could call the internet-facing ALB directly and skip CloudFront, along with any WAF attached to it. The prod_requires_network_authenticated_ingress check from #401/#128 only looks at the Lambda auth type, and it had nothing to check for Fargate.

Fix

• modules/networking/aws
  • Adds a new variable, restrict_alb_ingress_to_cloudfront (bool, default false).
    • When it is on, the ALB group has exactly one ingress rule: tcp/80 from AWS's com.amazonaws.global.cloudfront.origin-facing managed prefix list (data.aws_ec2_managed_prefix_list).
    • It is port 80 only because the frontend module reaches the Fargate ALB with origin_protocol = "http-only".
    • It is one rule only because a rule that references this prefix list counts against the default 60 inbound-rules-per-group quota at the list's max-entries weight. A second rule (say, 443) would exceed that quota and fail the apply.
  • When it is off, the four existing internet-facing rules are built from a local list. They come out identical to before (proof below).
  • Adds a new output, alb_ingress_cloudfront_only. It is computed from the group's inline ingress rules, not just echoed from the variable.
• environments/aws/networking.tf: passes restrict_alb_ingress_to_cloudfront = var.compute_platform == "fargate" && var.enable_cdn. This follows the same pattern as local.lambda_function_url_auth_type, which is also derived from enable_cdn.
• environments/aws/checks.tf: adds a new non-blocking check "fargate_cdn_requires_cloudfront_only_alb" (following sec(iac/gcp): prod Cloud Run is unauthenticated and internet-facing, Cloud Armor sits unattached #128's convention). It asserts !(fargate && enable_cdn) || module.networking.alb_ingress_cloudfront_only.
• ci-cd-permissions/policy_networking.tf: grants ec2:DescribeManagedPrefixLists and ec2:GetManagedPrefixListEntries, which the prefix-list lookup needs. Without them, the first Fargate+CDN plan would fail with AccessDenied. Both are read-only, and the lookup only runs when fargate && enable_cdn.
• CI (ci.yml, aws matrix): adds a new step, "Test AWS ALB ingress guard". It runs init -lockfile=readonly, fmt -check and terraform test for modules/networking/aws and environments/aws. Both lock files now include hashes for linux_amd64, darwin_arm64 and darwin_amd64. Provider versions are unchanged. The env lock's constraints strings were also refreshed to match the current required_providers.
• .gitallowed: adds one entry, anchored on path, line and the whole line, for the synthetic account_id = "123456789012" in the env test's aws_caller_identity mock. The provider rejects a boundary ARN whose account is not 12 digits. On Linux, git-secrets flags this line without the entry (verified below).

Live-infrastructure impact (owner please confirm)

• No current stack should plan a change. Every committed tfvars (github-{dev,staging,prod}.tfvars, dev.tfvars.example) sets enable_cdn = false. deploy-aws-fargate.yml / destroy-fargate-dev.yml only override compute_platform=fargate, and the variable's default is false. With the restriction off, the planned ALB security group is identical to origin/main (mock-plan comparison below). I did not run a real terraform plan.
• The behavior change happens only when someone sets enable_cdn = true on a Fargate stack. That is exactly what this issue asks for, but whoever flips it should know three things, and these are owner decisions:
  1. deploy-aws-fargate.yml's post-deploy gates and module.deployment_checks call the ALB / api-<env> alias directly. With the restriction on, those calls will stop connecting. deployment_checks reports this as warnings (on_failure = continue / check blocks). The workflow's test-deployment gate would need to target the CloudFront URL. The Lambda+CDN path has the same property today, because the Function URL becomes AWS_IAM.
  2. Pre-existing issue, not changed here: when enable_https is on, the ALB's port-80 listener redirects to 443, but CloudFront talks to the Fargate origin http-only. Fargate + CDN + HTTPS therefore already redirects clients to the ALB itself, and the ALB is now unreachable from them. The real fix is an https-only origin for Fargate when the ALB has a certificate, plus a 443 prefix-list rule instead of 80. I left that out of scope.
  3. The ci-cd-permissions stack is applied by a human. The two read-only EC2 actions take effect only after that stack is re-applied, and they are only needed once a Fargate stack enables the CDN.

Verification

All of this is unit/mock/fixture evidence. Nothing ran against a real AWS account, and there was no real terraform plan/apply.

Module test (terraform/modules/networking/aws/tests/alb_ingress.tftest.hcl, mock provider, command = plan):

• Default: the four pre-sec(iac/aws): CloudFront in front of Fargate ALB, guard against exposed ALB when compute_platform=fargate #412 rules (tcp 80/443 from 0.0.0.0/0 / ::/0), no prefix-list lookup, output false.
• Restricted: the lookup targets com.amazonaws.global.cloudfront.origin-facing, there is exactly one rule (tcp/80, prefix list only, no CIDRs), the output is true, and egress is unchanged.
• Without an ALB group (Lambda): nothing is created or looked up, and the output is false.

Environment test (terraform/environments/aws/tests/fargate_cdn_alb.tftest.hcl): mocks all four providers, overrides every module except module.networking, and plans the real environment root.

• fargate + cdn: the ALB is CloudFront-only, and the new check stays quiet. A failing check block fails a terraform test run.
• fargate without cdn: the ALB stays public.
• lambda + cdn: there is no ALB group, and the check does not fire.

Go guard (TestAWSFargateCDNGuardPresent in prod_exposure_guard_test.go): pins the exact check condition and the networking.tf wiring. No mock plan can drive the environment into the failing state, so this test is what guards the condition.

Fail-before / pass-after:

• go test ./terraform/environments/aws/ -run TestAWS with origin/main's checks.tf + networking.tf: FAIL: TestAWSFargateCDNGuardPresent ... checks.tf missing check "fargate_cdn_requires_cloudfront_only_alb" (#412). Passes after the fix.
• Both tftest files fail against an origin/main copy of the tree (git archive): the module test fails with data resource "aws_ec2_managed_prefix_list" "cloudfront_origin_facing" has not been declared, and the env test with module.networking ... does not have an attribute named "alb_ingress_cloudfront_only". Both pass after the fix.
• Mutation probes on a temp copy of the fixed module, each caught by the module test:
  • keep the public rules when restricted
  • drop the prefix-list rule
  • move the prefix-list rule to 443
  • always restrict
• Env mutation restrict_alb_ingress_to_cloudfront = false: the run fails with Check block assertion failed ... check "fargate_cdn_requires_cloudfront_only_alb", plus the assertion.
• Output predicate: terraform test cannot drive the output's per-rule predicate false (configured nested-block attributes cannot be overridden, and the module has, deliberately, no input that adds a rule while restricted). TestAWSALBIngressOutputPredicatePinned (prod_exposure_guard_test.go) pins each clause instead; dropping the IPv4 CIDR clause, replacing it with true, dropping the self clause or making the security-group clause true each fail it. Rules attached from outside the resource (aws_vpc_security_group_ingress_rule) are still not visible to the output.
• Deploy-role actions: TestDeployPolicyAllowsManagedPrefixListLookup (ci-cd-permissions/policy_guard_test.go) fails if policy_networking.tf stops allowing either ec2:DescribeManagedPrefixLists or ec2:GetManagedPrefixListEntries while the module still declares the lookup (each removal probed).

No-diff proof for current stacks: I mock-planned aws_security_group.alb (ingress, egress, description, name_prefix) with the restriction off, from both the origin/main module and this branch's module. Comparing the JSON dumps from the two gave IDENTICAL.

After merging origin/main (c8bdd46): main's #718 check lambda_concurrency_bounds_db_connections fires for Lambda with the default lambda_reserved_concurrency = -1, so the env test's Lambda run now sets lambda_reserved_concurrency = 20 (20 x 3 fits 80 - 10). Without it the run fails with Check block assertion failed (2/3); with it 3/3, on macOS (1.15.8) and on Linux in hashicorp/terraform:1.10.5.

Commands run:

# macOS, Terraform 1.15.8, TF_DATA_DIR in a mktemp dir
terraform -chdir=terraform/modules/networking/aws init -backend=false -lockfile=readonly && terraform ... validate && terraform ... test   # 3 passed
terraform -chdir=terraform/environments/aws    init -backend=false -lockfile=readonly && terraform ... validate && terraform ... test   # 3 passed
terraform fmt -check -recursive terraform/

# Linux amd64, hashicorp/terraform:1.10.5 (CI's pin), on a copy of the tree
for d in modules/networking/aws environments/aws modules/compute/aws/lambda: init -lockfile=readonly, fmt -check, test   # 3/3/7 passed
terraform fmt -check -recursive terraform/ ; terraform -chdir=terraform/environments/aws validate   # ok

AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./terraform/environments/aws/... -count=1   # ok (incl. ci-cd-permissions policy guards)
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./internal/mocks/ -count=1                  # ok
go build ./... ; go vet ./terraform/environments/aws/...                                                            # ok
golangci-lint v2.10.1 run --allow-parallel-runners ./terraform/environments/aws/...                                 # 0 issues
gocyclo -over 10 -ignore "_test\.go" terraform/environments/aws/                                                    # silent
actionlint .github/workflows/ci.yml                                                                                 # ok
markdownlint-cli@0.47.0 terraform/modules/networking/aws/README.md                                                  # ok
trivy config --severity HIGH,CRITICAL --ignorefile .trivyignore terraform/modules/networking/aws                     # 0 misconfigurations
git-secrets --register-aws + scripts/setup-git-secrets.sh, --scan over the changed files in a fresh repo, isolated HOME:
  macOS: clean; Linux (ubuntu:24.04): clean with the new .gitallowed entry, flagged (exit 1) without it
bash scripts/test-git-secrets-allowlist.sh                                                                          # 42 PASS, 0 FAIL

The Go tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null, because this machine's default AWS profile is invalid. Not run locally: tflint (not installed), zizmor.

Out of scope

• The Fargate http-only origin vs. HTTPS-redirect mismatch (item 2 above).
• Pointing deploy-aws-fargate.yml's post-deploy gates at the CloudFront URL when the CDN is on (item 1 above).
• The AWS Lambda enable_cdn cutover (sec(iac): flip dev/staging/prod to AWS_IAM + enable CloudFront OAC (runtime cut-over for #424) #16).

Closes #412

Maintainers: please mirror the issue's triage labels onto this PR: priority/p2, severity/medium, triaged, urgency/this-quarter, impact/few, effort/m, type/security.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 15 billable files and costs up to $3.75.}

Or wait 9 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 1c5787a
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9292f458-cae9-40e6-b8b1-fc0d272dc1f3

Commits

Reviewing files that changed from the base of the PR and between 1c5787a and d5667a4.

Files selected for processing: 15

• .gitallowed
• .github/workflows/ci.yml
• terraform/environments/aws/.terraform.lock.hcl
• terraform/environments/aws/checks.tf
• terraform/environments/aws/ci-cd-permissions/policy_guard_test.go
• terraform/environments/aws/ci-cd-permissions/policy_networking.tf
• terraform/environments/aws/networking.tf
• terraform/environments/aws/prod_exposure_guard_test.go
• terraform/environments/aws/tests/fargate_cdn_alb.tftest.hcl
• terraform/modules/networking/aws/.terraform.lock.hcl
• terraform/modules/networking/aws/README.md
• terraform/modules/networking/aws/main.tf
• terraform/modules/networking/aws/outputs.tf
• terraform/modules/networking/aws/tests/alb_ingress.tftest.hcl
• terraform/modules/networking/aws/variables.tf

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Heads-up on merge order: #1090 (fck-nat persistent ENI, #716) also edits terraform/modules/networking/aws/main.tf, terraform/environments/aws/ci-cd-permissions/policy_networking.tf and .github/workflows/ci.yml. Whichever merges second will need main merged in to resolve those hunks. I'll handle that on our side when the first one lands.

[cristim]

VERDICT: REQUEST_CHANGES 8ba439d

Base: merge-base 8caed39, current origin/main b6a0649. One commit, 440+/35- = 475 changed lines (cap 400: non-blocking process nit).

Findings

• blocking (conflict) .gitallowed:163 - git merge-tree origin/main 8ba439d9 conflicts: main appended the sec(scripts): git-secrets AWS secret-key pattern is inverted and never matches a key #295 OIDC-thumbprint entry at the same spot as the new tftest account_id entry. Keep both lines.
• blocking (conflict) .github/workflows/ci.yml:616 - main added the "Test monitoring module" step (chore(iac): no alert reaches anyone; instantiate monitoring and wire failure notifications #714) where this PR adds "Test AWS ALB ingress guard". Keep both steps.
• blocking (conflict) terraform/environments/aws/checks.tf:58 - main added check "lambda_concurrency_bounds_db_connections" (chore(iac): size the DB connection pool per platform; GCP prod can exceed Cloud SQL max_connections #718) after the sec(iac/gcp): prod Cloud Run is unauthenticated and internet-facing, Cloud Armor sits unattached #128 block, the same place as the new sec(iac/aws): CloudFront in front of Fargate ALB, guard against exposed ALB when compute_platform=fargate #412 check. Keep both blocks.
• blocking (semantic, after a trivial both-sides resolution) terraform/environments/aws/tests/fargate_cdn_alb.tftest.hcl:124-129 - run "lambda_with_cdn_does_not_trip_the_fargate_check" fails on the merged tree: compute_platform = "lambda" with the default lambda_reserved_concurrency = -1 trips main's chore(iac): size the DB connection pool per platform; GCP prod can exceed Cloud SQL max_connections #718 check ("Check block assertion failed", checks.tf:68). Passes 3/3 at the PR head, 2/3 merged. Fix: set lambda_reserved_concurrency to a positive value in that run (or in the file-level variables). CI on the merged result would fail the new "Test AWS ALB ingress guard" step.
• nit terraform/modules/networking/aws/outputs.tf:67-77 - weakening the output predicate (dropping the IPv4 cidr_blocks term) survives every test (probe M1 below). The PR body already lists this as a known gap; noting it so it stays visible.
• nit terraform/environments/aws/ci-cd-permissions/policy_networking.tf:71-77 - no test pins the two new read-only actions; dropping one would surface only as AccessDenied on the first Fargate+CDN plan. Acceptable given the disclosure.

Checked and fine

• Mechanism: one tcp/80 ingress rule sourced from data.aws_ec2_managed_prefix_list "com.amazonaws.global.cloudfront.origin-facing" (main.tf:583-586, 608-618), matching frontend.tf:21 (http-only origin for Fargate). Single rule rationale (prefix-list weight against the 60-rule quota) holds.
• enable_cdn=false: the data source has count 0 (no new IAM call), the dynamic block emits the same four 0.0.0.0/0 and ::/0 rules with the same descriptions; every committed tfvars sets enable_cdn = false, and deploy-aws-fargate.yml only overrides compute_platform. The next AWS deploy (lambda) plans no SG change; a Fargate deploy without CDN plans none either.
• Deploy role: ec2:DescribeManagedPrefixLists + ec2:GetManagedPrefixListEntries are what the data source calls; read-only; need a manual bootstrap re-apply before anyone enables Fargate+CDN (disclosed in the PR body).
• The PR body's operational caveats (post-deploy gates hitting the ALB directly; HTTPS redirect with an http-only origin) are accurate and pre-existing.

Local results (Terraform 1.10.5, /opt/homebrew/bin/terraform, merged tree with conflicts resolved as both-sides)

• fmt -check -recursive terraform/: ok
• modules/networking/aws: init -backend=false -lockfile=readonly ok, validate ok, test 3/3 pass
• environments/aws: init ok, validate ok, test 2 passed 1 failed (above). At PR head: 3/3.
• go test ./terraform/environments/aws/... (merged): 63 passed

Attacked:

• merge-tree against current origin/main; both-sides resolution; reran tests on the merged tree.
• M1 drop IPv4 cidr term from output predicate: survives (known gap).
• M2 swap prefix list name to the ipv6 list: killed (module test).
• M3 move the prefix-list rule to 443: killed (module test).
• M4 output : true when not restricted: killed (2 module runs fail).
• M5 networking.tf wiring = var.enable_cdn only: survives the tftests, killed by TestAWSFargateCDNGuardPresent.
• Searched tfvars and workflows for enable_cdn overrides; read the IAM statement the actions were added to.

[cristim]

Independent review 2 (claude-fleet, cold): REQUEST_CHANGES 8ba439d

The fix itself is right for #412. Both blocking items come from main having moved.

Findings:

• [blocking] terraform/environments/aws/tests/fargate_cdn_alb.tftest.hcl:126 (run "lambda_with_cdn_does_not_trip_the_fargate_check") - This run fails once merged with origin/main (b6a0649).
  • Main added check "lambda_concurrency_bounds_db_connections" (checks.tf:68 on the merged tree), and lambda_reserved_concurrency defaults to -1.
  • A failed check block fails a terraform test run: "Check block assertion failed ... var.lambda_reserved_concurrency is -1".
  • After a rebase, the new "Test AWS ALB ingress guard" CI step would go red.
  • Fix: set a positive lambda_reserved_concurrency in that run (or in the file's variables), or add expect_failures = [check.lambda_concurrency_bounds_db_connections].
• [blocking, mechanical] merge-tree reports content conflicts in .gitallowed, .github/workflows/ci.yml and terraform/environments/aws/checks.tf.
  • All three are additive, with both sides appending in the same place.
  • checks.tf needs hand resolution, because the closing EOT } } lines are shared.
  • The PR comment already predicts the conflict with fix(iac/aws): route private egress through a persistent fck-nat ENI #1090.
• [non-blocking, disclosed] terraform/modules/networking/aws/outputs.tf:69-77 - The per-rule predicate in alb_ingress_cloudfront_only is untested. Probe M3 (the cidr_blocks clause replaced with true) passes every test.
• [non-blocking] terraform/environments/aws/ci-cd-permissions/policy_networking.tf:77 - ec2:GetManagedPrefixListEntries sits inside the alphabetised Describe* run. This is cosmetic, and the policy guard tests pass.

Against #412:

• The module's ALB SG was open to 0.0.0.0/0 and ::/0 whatever enable_cdn was.
• Restricting it to the CloudFront origin-facing prefix list on port 80 matches frontend.tf:21 (http-only for Fargate).
• The check and the Go guard cover the issue's items 2 and 3.
• The SG's callers (compute.tf:192, fargate/main.tf:421 and :448) are unaffected. No migrations.

Attacked:

• Mutation probes on a scratch copy. All probed files were restored and match the head byte for byte (cmp against git show).
  • M1, the networking.tf wiring forced to false: the env tftest FAILS, and the Go guard FAILS (prod_exposure_guard_test.go:57). Caught.
  • M2, public rules kept while restricted: the module test FAILS at :74, :79 and :90. Caught.
  • M3, the output ignores the IPv4 CIDR clause: all pass. Gap, as reported above.
  • M4, the IPv6 prefix list name used instead: FAILS at :69. Caught.
  • M5, the CloudFront rule moved to port 443: FAILS at :79. Caught.
  • M6, the check condition made || true: the Go guard FAILS at :48. Caught.
  • M7, the wiring made var.enable_cdn only: all pass. This is an equivalent mutant, because create_alb_security_group is false on Lambda.
• Fail-before, with the new tests on base:
  • The module test fails (undeclared data source and output).
  • The env test fails (unsupported attribute).
  • TestAWSFargateCDNGuardPresent fails.

Verified locally, with Terraform 1.10.5 (the CI pin) and no cloud credentials, under the heavy lock:

• Head, on modules/networking/aws and environments/aws:
  • init -backend=false -lockfile=readonly, validate and fmt -check -recursive: ok.
  • terraform test: 3/3 pass in each.
• Head, Go and repo-wide checks:
  • terraform fmt -check -recursive terraform/: ok.
  • go build ./..., go vet ./terraform/environments/aws/... and go test -count=1 ./terraform/environments/aws/... (including the ci-cd-permissions guards): ok.
  • gofmt -l on the changed Go file: clean.
• Merged-tree export, with conflicts resolved by keeping both sides:
  • The networking module tests: 3/3 pass.
  • The environment tests: 2 pass, 1 fails (the first finding).
  • Go tests: ok.

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.

[IAmJSD]

Addressing both reviews of 8ba439d (VERDICT REQUEST_CHANGES, and Independent review 2). New head: bac8cff. I merged origin/main (c8bdd46) in 60ddfb9 rather than rebasing. The fixes are in bac8cff.

Blocking

• Conflicts (both reviews): all three were resolved by keeping both sides.
  • .gitallowed:163 keeps the sec(iac/aws): CloudFront in front of Fargate ALB, guard against exposed ALB when compute_platform=fargate #412 tftest entry, and :169 keeps main's sec(scripts): git-secrets AWS secret-key pattern is inverted and never matches a key #295 OIDC-thumbprint entry.
  • ci.yml:618 keeps "Test AWS ALB ingress guard", and :629 keeps main's chore(iac): no alert reaches anyone; instantiate monitoring and wire failure notifications #714 "Test monitoring module".
  • checks.tf:71 keeps fargate_cdn_requires_cloudfront_only_alb, and :93 keeps main's chore(iac): size the DB connection pool per platform; GCP prod can exceed Cloud SQL max_connections #718 lambda_concurrency_bounds_db_connections. I rebuilt the shared EOT } } closers by hand.
  • These were the only files both sides touched. fix(iac/aws): route private egress through a persistent fck-nat ENI #1090 has not landed yet.
• lambda_with_cdn_does_not_trip_the_fargate_check fails on the merged tree: tests/fargate_cdn_alb.tftest.hcl:134 now sets lambda_reserved_concurrency = 20. That is the prod value, and 20 x the default db_max_connections (3) fits within 80 - 10 reserved, so the db_max_connections validation passes too.
  • Without the line, the run fails with Check block assertion failed (2/3). With it, all 3 runs pass.

Non-blocking

• outputs.tf:67-77 predicate gap (M1/M3): added TestAWSALBIngressOutputPredicatePinned (terraform/environments/aws/prod_exposure_guard_test.go:66), which pins each per-rule clause of alb_ingress_cloudfront_only.
  • Probes, each of which now fails the test: drop the IPv4 CIDR clause; replace that clause with true; drop the self clause; set the security-group clause to true.
  • It is a source pin, like TestAWSFargateCDNGuardPresent, because terraform test still cannot drive the predicate false.
  • Rules attached through aws_vpc_security_group_ingress_rule are still invisible to the output. That limit is unchanged and noted in the PR body.
• No test pins the two new actions: added TestDeployPolicyAllowsManagedPrefixListLookup (ci-cd-permissions/policy_guard_test.go:1994).
  • It requires an Allow in policy_networking.tf that covers both actions while modules/networking/aws/main.tf still declares the lookup.
  • Removing either action makes it fail (probed).
• GetManagedPrefixListEntries inside the Describe* run: moved it to its alphabetical place (policy_networking.tf:90, after DisassociateRouteTable), with a pointer to the sec(iac/aws): CloudFront in front of Fargate ALB, guard against exposed ALB when compute_platform=fargate #412 comment at :76.
• 475 lines vs the 400 cap: left as is. The diff is mostly tests and lock-file hashes, and splitting it now would only add churn.

PR body: I replaced the "Known gap" paragraph with the two new guards and added a note on the post-merge #718 interaction. Closes #412 is unchanged.

Verification on the merged tree (bac8cff)

• Terraform: checked on macOS (1.15.8) and on Linux in hashicorp/terraform:1.10.5 (the CI pin), against a copy of the tree.
  • For modules/networking/aws and environments/aws: init -backend=false -lockfile=readonly, fmt -check, validate and test all pass. Each directory passes 3/3. fmt -check -recursive terraform/ is ok.
• Go: go build ./... is ok.
  • go vet ./terraform/environments/aws/... is ok, with and without -tags integration.
  • go test -count=1 is ok for ./terraform/environments/aws/, ./terraform/environments/aws/ci-cd-permissions/ and ./internal/mocks/. All tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null.
• Linters: golangci-lint v2.10.1 --allow-parallel-runners reports 0 issues, with and without --build-tags integration.
  • gocyclo -over 10 on terraform/environments/aws/ is silent, and actionlint .github/workflows/ci.yml is ok.
• git-secrets: I scanned a fresh repo with an isolated HOME, all 15 changed files tracked, using scripts/setup-git-secrets.sh patterns.
  • macOS: clean.
  • Linux (ubuntu:24.04): clean, and it flags fargate_cdn_alb.tftest.hcl:23 when .gitallowed is removed.
  • A planted AWS key was flagged on both, so the scan was live.
  • scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

Nothing ran against real AWS. The PR stays a draft.

[cristim]

Independent review 2 (claude-fleet, cold), re-review: APPROVE bac8cff

Both blockers from my review at 8ba439d are fixed.

Findings (none blocking):

• [non-blocking] terraform/environments/aws/prod_exposure_guard_test.go:77-84 - The new predicate pin checks each per-rule clause, but not the alltrue([ wrapper around them.
  • Probe M3c changed alltrue to anytrue in outputs.tf:69, and both the Go tests and the module tftests still passed.
  • That mutated output stays true if just one rule is the prefix-list rule, even when other rules are open, so the sec(iac/aws): CloudFront in front of Fargate ALB, guard against exposed ALB when compute_platform=fargate #412 check would stay quiet.
  • The fix is one line: add alltrue([ for r in aws_security_group.alb[0].ingress : to the pinned clauses.

Changes beyond the rebase (bac8cff, plus merge 60ddfb9):

• Merge resolution (checked with git show --remerge-diff): all three conflicts keep both sides correctly.
  • checks.tf: the sec(iac/aws): CloudFront in front of Fargate ALB, guard against exposed ALB when compute_platform=fargate #412 check gets its own closing EOT } }, and the chore(iac): size the DB connection pool per platform; GCP prod can exceed Cloud SQL max_connections #718 block follows intact.
  • .gitallowed and ci.yml: both entries and both steps are kept.
• fargate_cdn_alb.tftest.hcl, the lambda run: it now sets lambda_reserved_concurrency = 20.
  • That is valid: the variable's validation allows max(rc,0) x db_max_connections (3) up to database_max_connections (80) minus the reserved connections, and 60 fits.
  • checks.tf is unchanged for lambda_concurrency_bounds_db_connections, so main's check is not weakened.
  • Setting the variable is a better fix than expect_failures.
• TestAWSALBIngressOutputPredicatePinned: a whitespace-normalised pin on each per-rule clause. This closes most of the earlier M3 gap.
• TestDeployPolicyAllowsManagedPrefixListLookup: it reuses findStatements, statementActions and actionPermitted, with no new parsing code.
• policy_networking.tf: ec2:GetManagedPrefixListEntries moved into alphabetical position. The action set is unchanged.
• Drift:
  • merge-tree --write-tree origin/main bac8cff against origin/main 6af022f is clean.
  • On main, only ci.yml changed among the touched paths.
  • In the merged ci.yml, the "Test AWS ALB ingress guard" step sits in terraform-validate, under if: matrix.cloud == 'aws'.

Attacked: probes on a scratch copy, each restored and confirmed identical with cmp against git show bac8cff:<f>:

• M1, wiring forced to false: the env tftest FAILS, and the Go test FAILS at :57.
• M2, public rules kept while restricted: the module tftest FAILS at :74, :79 and :90.
• M3, the IPv4 CIDR clause replaced with true: the Go test FAILS at :86. This was missed at 8ba439d and is now caught.
• M3b, the self clause deleted: FAILS at :86.
• M3c, alltrue changed to anytrue: passes. This is the gap above.
• M5, the CloudFront rule moved to port 443: FAILS at :79.
• M6, the check condition made || true: the Go test FAILS at :48.
• M8, ec2:GetManagedPrefixListEntries removed from the policy: policy_guard_test.go:2016 FAILS.

Verified locally on the merged tree, with Terraform v1.10.5 and no cloud credentials, under the heavy lock:

• modules/networking/aws: fmt -check -recursive, init -backend=false -lockfile=readonly and validate are ok. test passes 3/3.
• environments/aws: fmt, init and validate are ok. test passes 3/3, including the lambda run that failed in the last review.
• Repo-wide and Go checks:
  • terraform fmt -check -recursive terraform/: ok.
  • go build ./..., go vet ./terraform/environments/aws/... and go test -count=1 ./terraform/environments/aws/... (including ci-cd-permissions): ok.
  • gofmt -l: clean.
• No migrations.

Same reviewer as the earlier review, re-reviewing the new head. Review only: nothing was pushed, and the GitHub review state is unchanged.

[cristim]

VERDICT: APPROVE bac8cff

Delta 8ba439d..bac8cff:

• merge 60ddfb9 (main c8bdd46 into the branch) resolves the .gitallowed, ci.yml and checks.tf conflicts. All three PR hunks were kept next to main's content: the fargate_cdn_alb account_id allowlist line, the new "Test AWS ALB ingress guard" step after "Test AWS compute modules", and the sec(iac/aws): CloudFront in front of Fargate ALB, guard against exposed ALB when compute_platform=fargate #412 check placed before the chore(iac): size the DB connection pool per platform; GCP prod can exceed Cloud SQL max_connections #718 DB budget check.
• bac8cff sets lambda_reserved_concurrency = 20 in the lambda tftest run, adds TestAWSALBIngressOutputPredicatePinned and TestDeployPolicyAllowsManagedPrefixListLookup, and moves ec2:GetManagedPrefixListEntries into alphabetical order.

Both blockers are fixed. 8ba439d still conflicts with main 6af022f on the same 3 files. bac8cff merges cleanly (tree 11f3c606).

Findings

• nit terraform/environments/aws/prod_exposure_guard_test.go (TestAWSALBIngressOutputPredicatePinned): this is a whitespace-normalized text pin of outputs.tf. A harmless rewrite of the predicate would also fail it. Acceptable, because no tftest can drive the output false.

Results on the merged tree (terraform 1.10.5, /opt/homebrew/bin/terraform; never plan or apply):

• fmt -check -recursive, init -backend=false -lockfile=readonly and validate pass in modules/networking/aws, environments/aws and environments/aws/ci-cd-permissions.
• terraform test: networking 3/3 pass, env/aws 3/3 pass, including lambda_with_cdn_does_not_trip_the_fargate_check.
• go test on env/aws and ci-cd-permissions: 65 passed.
• actionlint 1.7.12 on ci.yml is clean.

Mutation probes (all restored):

• drop lambda_reserved_concurrency: the lambda run FAILs, which reproduces the old blocker
• restrict_alb_ingress_to_cloudfront = false: fargate_with_cdn FAILs
• drop GetManagedPrefixListEntries: the policy test FAILs
• drop the security_groups clause or the self clause: the pin test FAILs

CI on bac8cff: Build Docker Image, Security Scanning and CI Success fail on GO-2026-6809 (stdlib, fixed in 1.26.10). That comes from the old base (go 1.26.9); main has 1.26.10. Validate Terraform (aws), which includes the new step, Lint Workflows and pre-commit all pass.

@IAmJSD CI on this head is red only from the old merge base (govulncheck on go1.26.9, fixed on main by #1236). Please merge or rebase onto current main so CI runs on a fresh merge ref.

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE d5667a47bd415d17be6fa5832da347147b834a3e

Blocking:

• none

Non-blocking:

• terraform/modules/networking/aws/main.tf:583 (data cloudfront_origin_facing): the com.amazonaws.global.cloudfront.origin-facing prefix list admits every CloudFront distribution, not only ours. Anyone can create their own distribution with this ALB's DNS name as the origin and reach the ALB without going through our distribution or any WAF on it. The Lambda path is tied to our distribution (OAC plus aws:SourceArn), so the issue's "mirroring OAC" is only partly met. This is the fix the issue asked for and it closes direct-internet access, so it is fine for this PR. The usual way to close the rest is a secret custom origin header set on the distribution, with an ALB listener rule that returns 403 when the header is missing. I suggest a follow-up issue.
• With the restriction on and enable_https = true, the ALB's :80 listener redirects to :443, and :443 is now closed to clients. So Fargate + CDN + HTTPS goes from "works but bypasses CDN" to "broken". The body's item 2 describes this. Whoever flips enable_cdn on a Fargate stack should know it.
• tests/fargate_cdn_alb.tftest.hcl overrides every module except module.networking by name. A module added to the env root later will run against mock providers unless someone adds an override for it. That costs maintenance, not correctness.

Attacked:

• Checked that the default (unrestricted) dynamic rules match the four old static rules field for field (null vs unset cidr_blocks/ipv6_cidr_blocks). Checked that nothing outside the resource attaches rules to the ALB SG (no aws_security_group_rule or aws_vpc_security_group_ingress_rule in terraform/). The fargate module consumes the SG only on the ALB and in the tasks SG's source.
• Checked that frontend.tf really uses http-only for Fargate, so a port-80-only rule is correct. Checked the output predicate's clauses, and that the data source runs only when the restriction is on.
• Fail-before on terraform 1.10.5 (CI's pin), run separately. (1) With networking.tf reverted to origin/main, the env tftest fails with Check block assertion failed and TestAWSFargateCDNGuardPresent fails. (2) With the public rules kept while restricted, the module tftest fails restricted_admits_only_cloudfront_prefix_list and the env tftest fails too. Restored afterwards.
• Ran: terraform init -lockfile=readonly + fmt -check + test on both dirs (3/3 and 3/3 pass), terraform fmt -check -recursive terraform/, go build ./..., go vet (with and without -tags integration), go test -race -short -count=1 ./terraform/environments/aws/..., golangci-lint (0 issues), secrets-scan (clean; planted key flagged), test-git-secrets-allowlist.sh (137 PASS).

Body-stale:

• none found. The body says test-git-secrets-allowlist.sh gave 42 PASS. It now gives 137 because main has grown, which is immaterial.

_{Reviewer: rB2. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at d5667a47bd415d17be6fa5832da347147b834a3e merged onto main (26004a6d), together with #974, #1106, #1151, #1193, #1199, #1212, #1217, #1218, #1219, #1221, #1223, #1224, #1225, #1226, #1227, #1256, #1262, #1266, #1272, #1275, #1283, #1286, #1293, #1299, #1300, #1313 in that order (combined tree d3d95c83, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 2 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 2 approving independent review(s) on d5667a47 (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly d5667a47bd415d17be6fa5832da347147b834a3e.