security(iac/gcp): set allow_unauthenticated=false; document ingress override - #586
Conversation
…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.
|
Warning Review limit reached
More reviews will be available in 27 minutes and 49 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 (8)
📝 WalkthroughWalkthroughThree GCP Terraform environment configurations disable unauthenticated Cloud Run access by setting ChangesCloud Run Unauthenticated Access Hardening
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review 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
|
Reworked per design discussion: cloud_run_allow_unauthenticated is now derived from enable_cdn (mirrors PR #574's lambda_function_url_auth_type pattern). When CDN is enabled, the LB fronts with Google identity and Cloud Run locks down via IAM; when CDN is disabled (dev), allow_unauthenticated stays true so *.run.app browsers can reach the service. Tfvars overrides dropped across all 3 envs + dev.tfvars.example, env-layer variable deleted, README updated. @coderabbitai review |
|
✏️ Learnings added
✅ Actions performedFull review triggered. |
Summary
Two related Cloud Run security/ops fixes to all three environment tfvars (dev, staging, prod):
#384 - allow_unauthenticated=false:
cloud_run_allow_unauthenticatedwas set totruein all three tfvars, overriding the variable default offalse. Cloud Run's built-in IAM auth is a defence-in-depth layer independent of our application-level JWT auth; it was unintentionally disabled. Setting it tofalsere-enables it.#78 - Document ingress override:
cloud_run_ingress = "INGRESS_TRAFFIC_ALL"is a temporary override because the external HTTPS LB + Cloud Armor stack (enable_cdn=true) has not yet been provisioned. The variable default (INGRESS_TRAFFIC_INTERNAL_LOAD_BALANCER) would block*.run.apptraffic until DNS + cert + LB are in place. Updated inline comments to cross-reference both issues and state the removal condition.Closes #384
Closes #78
Test plan
terraform fmt -checkpasses on all three tfvars filesSummary by CodeRabbit