Repository navigation
sec(iac/gcp): cleanup-function SA gets project-wide Secret Manager access to the encryption key #1614
Description
Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.
Blast radius: customer credential material. This is not a CI/deploy-infrastructure finding. The grant below reaches the AES-256-GCM key that decrypts stored customer cloud credentials, which is why it carries priority/p0 / impact/all-users rather than the priority/p1 / impact/internal used for our own deploy-identity findings.
Where
terraform/modules/compute/gcp/cleanup-function/main.tf:9-13(the grant)terraform/modules/compute/gcp/cleanup-function/main.tf:47(the one secret the function actually needs)terraform/modules/compute/gcp/cloud-run/main.tf:247-276(the sibling module that already fixed this)terraform/modules/secrets/gcp/main.tf:258-288(the credential-encryption key that becomes readable)
What
google_project_iam_member.cleanup_secrets grants roles/secretmanager.secretAccessor at project scope to the cleanup function's service account, ungated (no count, no condition). Verified still present verbatim at 3e9660d06:
# Grant access to Secret Manager
resource "google_project_iam_member" "cleanup_secrets" {
project = var.project_id
role = "roles/secretmanager.secretAccessor"
member = "serviceAccount:${google_service_account.cleanup.email}"
}The function only ever needs one secret: var.db_password_secret_id, wired into DB_PASSWORD_SECRET at main.tf:47.
The sibling Cloud Run module explicitly documents removing this exact anti-pattern and replacing it with per-secret bindings (compute/gcp/cloud-run/main.tf:247-276):
Previously this granted the Cloud Run SA project-wide roles/secretmanager.secretAccessor, which let it read every secret in the project... Now we bind only the specific secrets
The cleanup function was never migrated.
Failure scenario
Anyone who compromises the cleanup function, or anyone who can impersonate its service account (for example a developer holding roles/iam.serviceAccountTokenCreator on the project), reads every secret in the project. That set includes:
<service>-credential-encryption-key(terraform/modules/secrets/gcp/main.tf:258-288), the AES-256-GCM key that decrypts stored customer cloud credentialsjwt-secretsession-secretscheduled-task-secret- the SendGrid API key
Full tenant-credential compromise reachable from a session-cleanup job that has no business touching any of it.
Fix direction
Replace google_project_iam_member.cleanup_secrets with a google_secret_manager_secret_iam_member scoped to var.db_password_secret_id only, mirroring the pattern at compute/gcp/cloud-run/main.tf:262-267. Then add a repo-level guard (a grep-based lint or a policy test) that fails on any new project-scope roles/secretmanager.secretAccessor binding, so the next module does not repeat it.
Related
- sec(iac): onboarding paths drift - shell/CFN/ARM templates lack the guards their Terraform siblings enforce cloud-commitments-platform#91 -
sec(iac): onboarding paths drift. This is the same "one path was hardened, its sibling was never migrated" shape, on the host-side module axis rather than the customer-onboarding-template axis. See the comment on sec(iac): onboarding paths drift - shell/CFN/ARM templates lack the guards their Terraform siblings enforce cloud-commitments-platform#91 enumerating the host-side instances.
Activity
sec(ci): fail CI on project-scope Secret Manager grants in Terraform
sec(ci): make the Secret Manager guard cover iac/ and stop overclaiming
sec(iac/gcp): scope cleanup-function SA to the one secret it reads
sec(ci): fail CI on project-scope Secret Manager grants in Terraform
sec(ci): make the Secret Manager guard cover iac/ and stop overclaiming
cristim commentedon Aug 3, 2026
Closed by PR #1682 — verification record
Auto-closed by the
Closeskeyword. Recording the verification, and one correction to this issue's severity.The narrowed grant is provably sufficient. Rather than following the call chain, the whole dependency closure was enumerated:
go list -deps ./cmd/cleanup-lambdareturns five packages, and no credential-encryption, JWT/session or SendGrid package is reachable from this binary at all. So no other secret can be read on that path regardless of control flow. Exactly oneGetSecretcall site exists within it. The IAM binding and the env var also read the same variable, so they cannot drift apart later.Severity correction: this was latent, not live. No
moduleblock sourcescompute/gcp/cleanup-function— the only two references in the repo are.trivyignoreand the guard script's own comment. Two further corroborations:main()islambda.Start(...), an AWS Lambda handler rather than a GCP entrypoint, and the module's source object isplaceholder.zip. This module has never been deployable as written, so no service account ever held the project-wide grant. Thepriority/p0+severity/criticallabelling described a landmine rather than an active exposure. Fixing it was still right — it would have shipped over-broad the moment anyone wired it up.The CI guard added alongside (
scripts/check-gcp-secret-scope.sh) is a genuine ratchet against copy-paste reintroduction, and it gates rather than reports: nopaths:filter, noif:, present inci-success.needs, and it exits 1 on the pre-fix file naming the right line. No false positives on the realterraform/tree, theiac/tree, or a per-secret +roles/cloudsql.clientfixture.Known limits of that guard, deliberately recorded rather than hidden
Six bypasses were proven, and one is live in the tree the guard reports clean: a project-scope
roles/secretmanager.admininterraform/environments/gcp/ci-cd-permissions/service_account.tf, invisible because the role literal sits in alocalsblock consumed viafor_each. The grant itself is legitimate (that is the Terraform deploy SA), but the guard's claim was narrowed to "no directly expressed scope-wide grant" and a LIMITATIONS block names it explicitly, rather than adding an allowlist entry the parser could never reach.Follow-ups: #1686 (locals/for_each blind spot), #1687 (
google_project_iam_policy+ separate data block, unindented-resource bypass), #1688 (ci-successdoes not fail onskipped).sec(iac/gcp): scope cleanup-function SA to the one secret it reads (#…