sec(iac): Lambda Function URL auth hardening -- CORS wildcard rejection + AWS_IAM OAC scaffolding (#390, #424) - #574
Conversation
|
Warning Review limit reached
More reviews will be available in 17 minutes and 24 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
ca51ba6 to
fc37407
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…aform validation (CR nit)
…424) Switch the lambda module's function_url_auth_type default from "NONE" to "AWS_IAM" and add the full infrastructure needed to enforce IAM-gated access via CloudFront OAC: - lambda module: new aws_lambda_permission.function_url_cloudfront resource grants cloudfront.amazonaws.com lambda:InvokeFunctionUrl scoped to a specific distribution ARN (source_arn). Created only when auth_type = "AWS_IAM" and cloudfront_distribution_arn is provided; no-op otherwise. - lambda module: new cloudfront_distribution_arn variable (default "") and updated function_url_auth_type description + validation block. - frontend module: new aws_cloudfront_origin_access_control.lambda resource (type=lambda, sigv4, always-sign) created when enable_oac = true. OAC id wired into the origin block alongside the existing custom_origin_config. - frontend module: new enable_oac variable (bool, default false). - environments/aws/compute.tf: pass cloudfront_distribution_arn to lambda module (length() guard prevents plan errors when enable_cdn = false). - environments/aws/frontend.tf: set enable_oac when platform = lambda + AWS_IAM. - All three env tfvars: keep lambda_function_url_auth_type = "NONE" (unchanged) and add a TODO(#424) comment with the 3-step cut-over instructions. The deployed environments retain their current "NONE" value until CloudFront is enabled per the follow-up issue. Flipping to AWS_IAM without a CloudFront OAC in place would make the Function URL return HTTP 403 on all requests. The frontend client.ts already uses X-Authorization (preserves the Authorization header for CloudFront OAC SigV4) and addContentHashHeader (x-amz-content-sha256 required for POST/PUT/PATCH/DELETE through OAC) -- no frontend changes needed. Verified: terraform fmt -check passes on all touched modules and the environment. Manual verification of the auth path required after CloudFront is deployed.
…refs #424) Remove the standalone lambda_function_url_auth_type variable and derive its value via a local in compute.tf: local.lambda_function_url_auth_type = var.enable_cdn ? "AWS_IAM" : "NONE" The two valid combinations were the only sensible ones anyway: enable_cdn = false -> auth_type = "NONE" SPA hits the Function URL directly; browsers cannot SigV4-sign, so the URL must be unauthenticated at the AWS-IAM layer; auth is enforced at the application layer (login session, CSRF, API keys). enable_cdn = true -> auth_type = "AWS_IAM" CloudFront fronts the URL with an OAC that SigV4-signs every request (aws_cloudfront_origin_access_control.lambda in the frontend module). The Function URL rejects anything not signed by CloudFront's OAC. Tying the two flags together prevents the two failure modes the previous "two variables operator must keep in sync" design exposed: (a) enable_cdn = true with auth_type = "NONE" -> public Lambda URL behind a pointless CloudFront (security goal of #424 defeated). (b) enable_cdn = false with auth_type = "AWS_IAM" -> direct browser hits to Function URL return 403 (no way to sign), site is unreachable. Updated three usages (compute.tf Lambda module call, the standalone aws_lambda_permission.function_url_cloudfront resource count, and frontend.tf enable_oac derivation) to use the local / enable_cdn directly. Removed the lambda_function_url_auth_type lines from all three env tfvars (github-dev/staging/prod) plus the two profile example tfvars and the top-level dev.tfvars.example; replaced the TODO(#424) cut-over comments with a short explanation of the enable_cdn -> auth_type mapping. Verified: terraform validate clean, terraform fmt clean. Refs #424
6e8d69d to
dc971f7
Compare
|
Rebased onto latest Conflicts in Verified post-rebase: |
Every tfvars file (3 github envs + 3 examples) set this to true and no path ever set it to false, so the toggle was dead code. The Function URL is always required — CloudFront uses it as origin via OAC when CDN is enabled, and the browser hits it directly when CDN is disabled. Removing the conditional also simplifies the aws_lambda_permission.function_url count expression to just check function_url_auth_type == "NONE".
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Added moved block in commit 98c5f69 to preserve aws_lambda_function_url.main across the count removal. Without it, apply would destroy [0] and create a new bare resource with a different host, breaking CORS / DASHBOARD_URL / frontend bundles. @coderabbitai review |
|
✅ Actions performedReview triggered.
|
…_cdn (mirrors PR #574 AWS pattern) Replace the unconditional `cloud_run_allow_unauthenticated = false` override with a derived `local.cloud_run_allow_unauthenticated = !var.enable_cdn` in `terraform/environments/gcp/compute.tf`. The IAM gate now flips in lock-step with the LB stack landing, matching the parallel pattern shipped in #574 for AWS Lambda Function URL auth: AWS (#574): local.lambda_function_url_auth_type = var.enable_cdn ? "AWS_IAM" : "NONE" GCP (this): local.cloud_run_allow_unauthenticated = !var.enable_cdn Why the conditional rather than always-false: * enable_cdn = false (today's dev/staging/prod): the SPA / clients hit the *.run.app URL directly. Browsers cannot present a Google-signed identity token, so the IAM gate must accept allUsers (allow_unauthenticated = true); auth is enforced at the application layer (login session, CSRF, OIDC for scheduled tasks). The previous unconditional `= false` would have made the *.run.app URL return 403 to direct browser hits — exactly the GCP analog of PR #574's mis-configuration (b). * enable_cdn = true (post-LB-cutover): the external HTTPS LB (with Cloud Armor) fronts the service and attaches a Google-signed identity to upstream calls, so Cloud Run can enforce `roles/run.invoker` on the LB SA only and lock allUsers out at the IAM layer (closes #384). The pairing also blocks mis-configuration (a) — `enable_cdn = true` with allow_unauthenticated = true would put a public *.run.app URL behind a pointless LB. The AWS analog of CloudFront's OAC + SigV4 signing is the GCP LB SA's identity token. The two cloud sides now share the same "auth gate is derived from the fronting-LB toggle" shape. Changes: * terraform/environments/gcp/compute.tf: add `locals { cloud_run_allow_ unauthenticated = !var.enable_cdn }` with a comment block mirroring #574's documentation of the two valid combinations + the two mis-configurations it prevents. Wire `module.compute_cloud_run.allow_unauthenticated` to the local. Update the scheduled-tasks comment block (#159) to point at the new derivation. * terraform/environments/gcp/variables.tf: drop the operator-facing `cloud_run_allow_unauthenticated` variable declaration, replace with a comment block that explains the derivation (parallel to #574's treatment of `lambda_function_url_auth_type`). Update `cloud_run_ingress` description's cross-reference accordingly. * terraform/environments/gcp/{github-dev,github-staging,github-prod, dev.tfvars.example}.tfvars: remove the `cloud_run_allow_unauthenticated` line from all 4 tfvars files; add a short comment near the existing ingress override noting the derivation. * terraform/environments/gcp/README.md: update the "Cloud Run ingress" table — `allow_unauthenticated` row now describes it as derived from `enable_cdn`, not a directly-set variable; clarify that when an env flips `enable_cdn = true` the IAM gate auto-closes alongside dropping the `cloud_run_ingress` override. * terraform/modules/compute/gcp/cloud-run/main.tf: refresh the inline comment on the scheduled-task http_target — the IAM gate now references #384 (the derivation) rather than #78 (the network gate). The `cloud_run_ingress = "INGRESS_TRAFFIC_ALL"` override in the 3 env tfvars stays exactly as it was — that's a sibling concern (network door) tracked by #78 and gated on the same LB cutover, but kept as an explicit override per the existing comment so the network-vs-IAM doors are managed in lockstep but visibly separately. Verified: terraform fmt -check clean on both modules/compute/gcp and environments/gcp; terraform validate clean on both layers. Refs #384, #78, #574
…override (#586) * security(iac/gcp): set allow_unauthenticated=false; document ingress override (closes #384, #78) #384: cloud_run_allow_unauthenticated was incorrectly set to true in all three environment tfvars, overriding the variable default of false. Cloud Run's built-in IAM auth is a defence-in-depth layer independent of our application-level JWT auth; it was unintentionally disabled. Set it to false in dev, staging, and prod. #78: cloud_run_ingress is temporarily set to INGRESS_TRAFFIC_ALL in all environments because the external HTTPS LB + Cloud Armor stack (enabled by enable_cdn=true) has not yet been provisioned. The variable default is INGRESS_TRAFFIC_INTERNAL_LOAD_BALANCER, which would block *.run.app traffic until DNS + cert + LB are in place. Update the inline comments to cross-reference both issues and explain the removal condition. * security(iac/gcp): derive cloud_run_allow_unauthenticated from enable_cdn (mirrors PR #574 AWS pattern) Replace the unconditional `cloud_run_allow_unauthenticated = false` override with a derived `local.cloud_run_allow_unauthenticated = !var.enable_cdn` in `terraform/environments/gcp/compute.tf`. The IAM gate now flips in lock-step with the LB stack landing, matching the parallel pattern shipped in #574 for AWS Lambda Function URL auth: AWS (#574): local.lambda_function_url_auth_type = var.enable_cdn ? "AWS_IAM" : "NONE" GCP (this): local.cloud_run_allow_unauthenticated = !var.enable_cdn Why the conditional rather than always-false: * enable_cdn = false (today's dev/staging/prod): the SPA / clients hit the *.run.app URL directly. Browsers cannot present a Google-signed identity token, so the IAM gate must accept allUsers (allow_unauthenticated = true); auth is enforced at the application layer (login session, CSRF, OIDC for scheduled tasks). The previous unconditional `= false` would have made the *.run.app URL return 403 to direct browser hits — exactly the GCP analog of PR #574's mis-configuration (b). * enable_cdn = true (post-LB-cutover): the external HTTPS LB (with Cloud Armor) fronts the service and attaches a Google-signed identity to upstream calls, so Cloud Run can enforce `roles/run.invoker` on the LB SA only and lock allUsers out at the IAM layer (closes #384). The pairing also blocks mis-configuration (a) — `enable_cdn = true` with allow_unauthenticated = true would put a public *.run.app URL behind a pointless LB. The AWS analog of CloudFront's OAC + SigV4 signing is the GCP LB SA's identity token. The two cloud sides now share the same "auth gate is derived from the fronting-LB toggle" shape. Changes: * terraform/environments/gcp/compute.tf: add `locals { cloud_run_allow_ unauthenticated = !var.enable_cdn }` with a comment block mirroring #574's documentation of the two valid combinations + the two mis-configurations it prevents. Wire `module.compute_cloud_run.allow_unauthenticated` to the local. Update the scheduled-tasks comment block (#159) to point at the new derivation. * terraform/environments/gcp/variables.tf: drop the operator-facing `cloud_run_allow_unauthenticated` variable declaration, replace with a comment block that explains the derivation (parallel to #574's treatment of `lambda_function_url_auth_type`). Update `cloud_run_ingress` description's cross-reference accordingly. * terraform/environments/gcp/{github-dev,github-staging,github-prod, dev.tfvars.example}.tfvars: remove the `cloud_run_allow_unauthenticated` line from all 4 tfvars files; add a short comment near the existing ingress override noting the derivation. * terraform/environments/gcp/README.md: update the "Cloud Run ingress" table — `allow_unauthenticated` row now describes it as derived from `enable_cdn`, not a directly-set variable; clarify that when an env flips `enable_cdn = true` the IAM gate auto-closes alongside dropping the `cloud_run_ingress` override. * terraform/modules/compute/gcp/cloud-run/main.tf: refresh the inline comment on the scheduled-task http_target — the IAM gate now references #384 (the derivation) rather than #78 (the network gate). The `cloud_run_ingress = "INGRESS_TRAFFIC_ALL"` override in the 3 env tfvars stays exactly as it was — that's a sibling concern (network door) tracked by #78 and gated on the same LB cutover, but kept as an explicit override per the existing comment so the network-vs-IAM doors are managed in lockstep but visibly separately. Verified: terraform fmt -check clean on both modules/compute/gcp and environments/gcp; terraform validate clean on both layers. Refs #384, #78, #574
Summary
allowed_originsvia module validation; replace["*"]in all envs with explicit origins. The Function URL usesallow_credentials = true-- a wildcard origin reflects any inboundOriginheader with credentials allowed, enabling any-origin CSRF.function_url_auth_typemodule default from"NONE"to"AWS_IAM". Addaws_cloudfront_origin_access_control(type=lambda, sigv4, always-sign) to the frontend module. Add a standaloneaws_lambda_permissionat the environment layer (breaks the cycle: Lambda URL is CloudFront's origin; distribution ARN feeds the permission -- they can't both live inside a single module). All three env tfvars keep"NONE"with aTODO(#424)cut-over comment untilenable_cdn = trueis set and CloudFront is deployed.The frontend
client.tswas already OAC-ready: it usesX-Authorization(preserves theAuthorizationheader for CloudFront SigV4 signing) andaddContentHashHeader(x-amz-content-sha256required for POST/PUT/PATCH/DELETE through OAC).Files changed
terraform/modules/compute/aws/lambda/variables.tffunction_url_auth_typeto"AWS_IAM"; add validation; expand descriptionterraform/modules/frontend/aws/main.tfaws_cloudfront_origin_access_control.lambda; wireorigin_access_control_idterraform/modules/frontend/aws/variables.tfenable_oacvariableterraform/environments/aws/compute.tfaws_lambda_permission.function_url_cloudfront(cycle-free)terraform/environments/aws/frontend.tfenable_oacwhen platform=lambda + auth_type=AWS_IAMterraform/environments/aws/github-{dev,staging,prod}.tfvarsTODO(#424)cut-over comments; no functional change to valuesTest plan
terraform fmt -checkpasses on all changed modules and environments (verified pre-commit hook)terraform validatepasses (pre-commit hook)terraform planwithallowed_origins = ["*"]fails with the validation error (regression test from sec(auth): CORS wildcard origin + credentials=true on staging Lambda URL #390)terraform planwithenable_cdn = true+lambda_function_url_auth_type = "AWS_IAM"showsaws_cloudfront_origin_access_control.lambda+aws_lambda_permission.function_url_cloudfrontbeing createdManual verification note
The runtime cut-over (flipping deployed envs to
AWS_IAM) requires a follow-up deploy. See theTODO(#424)comments in each tfvar for the 3-step procedure. A follow-up issue will be filed to track this.Closes
Closes #390
Partially addresses #424 (infrastructure scaffolding complete; runtime cut-over tracked separately)