Skip to content

sec(iac/aws): restrict the Fargate ALB to CloudFront when enable_cdn is on - #1106

Draft
IAmJSD wants to merge 3 commits into
LeanerCloud:mainfrom
IAmJSD:fix/issue-412
Draft

IAmJSD wants to merge 3 commits into
LeanerCloud:mainfrom
IAmJSD:fix/issue-412

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

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):

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

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

…is on

With compute_platform = "fargate" and enable_cdn = true, CloudFront fronts
the ALB (frontend.tf), but the ALB security group still admitted HTTP/HTTPS
from 0.0.0.0/0 and ::/0, so clients could call the ALB directly and bypass
the distribution and any WAF on it. enable_cdn only gated the Lambda
Function URL's auth type.

- modules/networking/aws: new restrict_alb_ingress_to_cloudfront (default
  false). When set, the ALB group's only ingress is tcp/80 from the
  com.amazonaws.global.cloudfront.origin-facing managed prefix list (the
  frontend module reaches the Fargate ALB over http-only; one rule because
  a prefix-list rule's weight leaves no room for a second under the default
  quota). With it off, the four internet-facing rules are unchanged.
  New output alb_ingress_cloudfront_only, derived from the group's inline
  ingress rules.
- environments/aws: pass compute_platform == "fargate" && enable_cdn, and
  add the non-blocking check "fargate_cdn_requires_cloudfront_only_alb".
  Every committed tfvars sets enable_cdn = false, so no current stack plans
  a change.
- ci-cd-permissions: grant ec2:DescribeManagedPrefixLists and
  ec2:GetManagedPrefixListEntries for the prefix-list lookup.
- Tests: terraform test for the module and the environment (mock
  providers), Go guard for the wiring and check condition, CI step running
  both with -lockfile=readonly, lock files for linux_amd64/darwin_arm64/
  darwin_amd64, and an anchored .gitallowed entry for the mock account id.

Closes LeanerCloud#412

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@IAmJSD IAmJSD added priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged urgency/this-quarter Within the quarter impact/few Limited audience effort/m Days type/security Security finding labels Oct 19, 2026
@IAmJSD

IAmJSD commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

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

cristim commented Oct 20, 2026

Copy link
Copy Markdown
Member

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

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

cristim commented Oct 20, 2026

Copy link
Copy Markdown
Member

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.
  • [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 and others added 2 commits October 21, 2026 09:44
Resolve three additive conflicts by keeping both sides: the LeanerCloud#295
OIDC-thumbprint entry in .gitallowed, the LeanerCloud#714 "Test monitoring module"
CI step, and the LeanerCloud#718 lambda_concurrency_bounds_db_connections check
in terraform/environments/aws/checks.tf.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oud#718 and pin the ALB guard's inputs

- fargate_cdn_alb.tftest.hcl: set lambda_reserved_concurrency = 20 in the
  lambda run, so main's lambda_concurrency_bounds_db_connections check
  (LeanerCloud#718) stays quiet; its default of -1 failed the run on the merged tree.
- TestAWSALBIngressOutputPredicatePinned: pin each per-rule clause of the
  networking module's alb_ingress_cloudfront_only output, which no
  tftest can drive false.
- TestDeployPolicyAllowsManagedPrefixListLookup: pin the two read-only
  EC2 actions the prefix-list lookup needs in policy_networking.tf.
- policy_networking.tf: move ec2:GetManagedPrefixListEntries out of the
  alphabetized Describe* run.

Refs LeanerCloud#412

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD

IAmJSD commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

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

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/few Limited audience priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/security Security finding urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sec(iac/aws): CloudFront in front of Fargate ALB, guard against exposed ALB when compute_platform=fargate

2 participants