From ef64fd1065a1c6d9bede99bbd528b12bb9426a85 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 07:23:21 +0200 Subject: [PATCH] sec(iac/gcp): fail closed on Cloud Run scheduled-task auth mode terraform/modules/compute/gcp/cloud-run derived SCHEDULED_TASK_AUTH_MODE from whether a Cloud Scheduler job was enabled: scheduled_task_auth_mode = (var.enable_scheduled_tasks || var.enable_ri_exchange_schedule) ? "oidc" : "disabled" That conflates "does Terraform create a scheduler" with "does the app authenticate /api/scheduled/*". The service is reachable from the open internet independently of the scheduler flags whenever var.allow_unauthenticated = true (its default) or ingress is INGRESS_TRAFFIC_ALL, so a tfvars flip that disables both schedulers silently turns off request auth on money-path handlers (ri_exchange, ladder_run, fire_scheduled_purchases) on an already-public service. All three GCP envs currently set enable_scheduled_tasks = true, so this has not fired in prod, but nothing guarded against it firing. terraform/modules/compute/gcp/gke already fixed the identical defect for its own Cloud Run analogue (kubernetes ingress) by falling back to a fail-closed override variable instead of a bare "disabled" literal. Port the same fix: when neither scheduler is enabled, fall back to the new var.scheduled_task_auth_mode_override (default "oidc"). With no scheduler SA, that means an empty SCHEDULED_TASK_OIDC_SUBJECTS, which internal/server/scheduledauth's configureOIDC rejects at startup (ErrConfigInvalid) rather than booting the app with /api/scheduled/* unauthenticated. "disabled" now requires an explicit, deliberate opt-in. Regression test: terraform/modules/compute/gcp/cloud_run_scheduled_task_auth_test.go asserts main.tf no longer derives the bare ... ? "oidc" : "disabled" form, does fall back to var.scheduled_task_auth_mode_override, and that the new variable defaults to "oidc" with the same three-value validation the Go validator accepts. Confirmed it fails against the pre-fix main.tf/variables.tf (git stash of the fix) and passes after. Closes #123 --- .../modules/compute/gcp/cloud-run/main.tf | 28 +++++++-- .../compute/gcp/cloud-run/variables.tf | 27 +++++++++ .../gcp/cloud_run_scheduled_task_auth_test.go | 58 +++++++++++++++++++ 3 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 terraform/modules/compute/gcp/cloud_run_scheduled_task_auth_test.go diff --git a/terraform/modules/compute/gcp/cloud-run/main.tf b/terraform/modules/compute/gcp/cloud-run/main.tf index 7f03c1360..87c89bc56 100644 --- a/terraform/modules/compute/gcp/cloud-run/main.tf +++ b/terraform/modules/compute/gcp/cloud-run/main.tf @@ -77,11 +77,29 @@ locals { var.enable_scheduled_tasks ? try(google_service_account.scheduler[0].unique_id, "") : "", var.enable_ri_exchange_schedule ? try(google_service_account.scheduler_ri_exchange[0].unique_id, "") : "", ])) - # Auth mode selector. When neither scheduler is enabled there's - # nothing to validate; staying on "disabled" avoids fail-fast on an - # empty allow-list in the validator. The resulting WARN log is - # harmless when /api/scheduled/* is never hit. - scheduled_task_auth_mode = (var.enable_scheduled_tasks || var.enable_ri_exchange_schedule) ? "oidc" : "disabled" + # Auth mode selector. Deriving "disabled" from the scheduler flags was + # wrong (#123): it conflates "does Terraform create a Cloud Scheduler + # job" with "does the app authenticate /api/scheduled/*", and the + # latter is reachable from the open internet whenever + # var.allow_unauthenticated = true regardless of the scheduler flags. + # A flag flip during a rollout, or a cost-saving change, then silently + # turns off request auth on money-path handlers on an already-public + # service. + # + # When a scheduler is enabled, auth mode is always "oidc" — not + # overridable. When neither is enabled, fall back to + # var.scheduled_task_auth_mode_override, which fails closed by default + # (mirrors terraform/modules/compute/gcp/gke's fix for the same + # defect): "oidc" with no scheduler SA means an empty + # SCHEDULED_TASK_OIDC_SUBJECTS, which internal/server/scheduledauth + # rejects at startup rather than booting with /api/scheduled/* + # unauthenticated. Set the override to "disabled" only as a deliberate, + # explicit opt-in (local-dev / dry-run). + scheduled_task_auth_mode = ( + var.enable_scheduled_tasks || var.enable_ri_exchange_schedule + ? "oidc" + : var.scheduled_task_auth_mode_override + ) } # ============================================== diff --git a/terraform/modules/compute/gcp/cloud-run/variables.tf b/terraform/modules/compute/gcp/cloud-run/variables.tf index 6da6d4bf6..cc054ce43 100644 --- a/terraform/modules/compute/gcp/cloud-run/variables.tf +++ b/terraform/modules/compute/gcp/cloud-run/variables.tf @@ -88,6 +88,33 @@ variable "allow_unauthenticated" { default = true } +variable "scheduled_task_auth_mode_override" { + description = <<-EOT + Override for SCHEDULED_TASK_AUTH_MODE used ONLY when neither + var.enable_scheduled_tasks nor var.enable_ri_exchange_schedule is true. + When either scheduler is enabled, auth mode is always "oidc" -- the + override is ignored. + + Why this exists (#123): the Cloud Run service can be reachable from the + open internet independently of the scheduler flags (see + var.allow_unauthenticated / var.ingress), so tying auth mode to those + flags would silently boot /api/scheduled/* unauthenticated whenever + both schedulers are disabled. The fail-closed default ("oidc") means a + deploy with no scheduler SA still requires the validator to be + configured with a non-empty subject allow-list, or startup fails + (internal/server/scheduledauth) instead of serving unauthenticated + money-path requests. Set this to "disabled" deliberately for + local-dev / dry-run only. + EOT + type = string + default = "oidc" + + validation { + condition = contains(["oidc", "bearer", "disabled"], var.scheduled_task_auth_mode_override) + error_message = "scheduled_task_auth_mode_override must be one of: \"oidc\", \"bearer\", \"disabled\"." + } +} + variable "database_host" { description = "Database host (Cloud SQL private IP)" type = string diff --git a/terraform/modules/compute/gcp/cloud_run_scheduled_task_auth_test.go b/terraform/modules/compute/gcp/cloud_run_scheduled_task_auth_test.go new file mode 100644 index 000000000..2b2a7b126 --- /dev/null +++ b/terraform/modules/compute/gcp/cloud_run_scheduled_task_auth_test.go @@ -0,0 +1,58 @@ +// Regression test for issue #123: the Cloud Run module derived +// SCHEDULED_TASK_AUTH_MODE from whether a Cloud Scheduler job was enabled, +// falling back to "disabled" whenever both var.enable_scheduled_tasks and +// var.enable_ri_exchange_schedule were false. That conflates "does +// Terraform create a scheduler" with "does the app require auth on +// /api/scheduled/*", and the latter is reachable from the open internet +// independently of the scheduler flags whenever var.allow_unauthenticated +// is true. A single tfvars flip could silently turn off request +// authentication on money-path handlers on an already-public service. +package gcp_test + +import ( + "os" + "regexp" + "testing" +) + +func TestCloudRunScheduledTaskAuthModeFailsClosed(t *testing.T) { + mainData, err := os.ReadFile("cloud-run/main.tf") + if err != nil { + t.Fatalf("reading cloud-run/main.tf: %v", err) + } + main := string(mainData) + + // The bug: falling back to the bare literal "disabled" whenever + // neither scheduler is enabled, with no way to require auth anyway. + if regexp.MustCompile(`scheduled_task_auth_mode\s*=\s*\([^)]*\)\s*\?\s*"oidc"\s*:\s*"disabled"`).MatchString(main) { + t.Error(`cloud-run/main.tf derives scheduled_task_auth_mode = ... ? "oidc" : "disabled" directly from the scheduler flags (#123); disabling both schedulers must not silently disable request auth on an internet-reachable service`) + } + + if !regexp.MustCompile(`scheduled_task_auth_mode\s*=\s*\(\s*\n?\s*var\.enable_scheduled_tasks\s*\|\|\s*var\.enable_ri_exchange_schedule\s*\n?\s*\?\s*"oidc"\s*\n?\s*:\s*var\.scheduled_task_auth_mode_override`).MatchString(main) { + t.Error(`cloud-run/main.tf must fall back to var.scheduled_task_auth_mode_override (not a bare "disabled" literal) when neither scheduler is enabled`) + } + + varsData, err := os.ReadFile("cloud-run/variables.tf") + if err != nil { + t.Fatalf("reading cloud-run/variables.tf: %v", err) + } + vars := string(varsData) + + if !regexp.MustCompile(`variable\s+"scheduled_task_auth_mode_override"\s*\{`).MatchString(vars) { + t.Fatal(`cloud-run/variables.tf must declare scheduled_task_auth_mode_override`) + } + + // Fail-closed default: the override must default to "oidc", the same + // safe posture terraform/modules/compute/gcp/gke already uses for the + // identical defect, not the "disabled" the bug produced. + overrideBlock := regexp.MustCompile(`(?s)variable\s+"scheduled_task_auth_mode_override"\s*\{.*?\n\}`).FindString(vars) + if overrideBlock == "" { + t.Fatal(`could not isolate the scheduled_task_auth_mode_override variable block`) + } + if !regexp.MustCompile(`default\s*=\s*"oidc"`).MatchString(overrideBlock) { + t.Error(`scheduled_task_auth_mode_override must default to "oidc" (fail closed); a deploy with no scheduler SA must still require the validator to be configured rather than defaulting to unauthenticated`) + } + if !regexp.MustCompile(`contains\(\["oidc",\s*"bearer",\s*"disabled"\],\s*var\.scheduled_task_auth_mode_override\)`).MatchString(overrideBlock) { + t.Error(`scheduled_task_auth_mode_override must validate against the exact set the Go validator (internal/server/scheduledauth) accepts: "oidc", "bearer", "disabled"`) + } +}