Skip to content

sec(ci): check-gcp-secret-scope guard is blind to roles supplied via locals/for_each #149

Description

@cristim

Follow-up to LeanerCloud/cloud-commitments-cli#1614 / PR LeanerCloud/cloud-commitments-cli#1682, which added scripts/check-gcp-secret-scope.sh.

What

The guard walks IAM resource blocks and matches a literal roles/secretmanager.* inside them. A role that reaches the resource through a locals list plus for_each is invisible to it, because the literal never appears inside the resource block.

There is a live instance today, terraform/environments/gcp/ci-cd-permissions/service_account.tf:

locals {
  deploy_roles = toset([
    ...
    "roles/secretmanager.admin",
    ...
  ])
}

resource "google_project_iam_member" "service_account" {
  for_each = local.deploy_roles
  project  = var.project_id
  role     = each.key
  member   = "serviceAccount:${google_service_account.cudly_deploy.email}"
}

That is a project-scope roles/secretmanager.admin, strictly broader than the secretAccessor the guard exists to stop, and scripts/check-gcp-secret-scope.sh exits 0 on that file. Verified directly.

The grant itself is probably fine

This is the Terraform deploy service account, which does need to create and manage secrets. This issue is not a request to remove that grant. It is about the guard's coverage, and about not letting a green check imply a property it never inspected.

Already handled in PR LeanerCloud/cloud-commitments-cli#1682

The self-test previously asserted "repository terraform/ tree is clean", which was false as written. That claim has been narrowed to exactly what the guard verifies (no directly expressed scope-wide Secret Manager role in an IAM resource block), and the known ci-cd-permissions grant is documented in the script and the fixture directory with a pointer to this issue. So nothing currently claims a property that is not checked; this issue tracks closing the gap for real.

Fix direction

Per-file pre-pass in the existing awk scanner: collect the roles/secretmanager.* literals in each locals block, and treat a scope-wide IAM resource with for_each = local.<name> / role = each.key as carrying every role in that list. That is a bounded addition to a 25-line scanner and it closes the one shape this issue demonstrates.

The ci-cd-permissions grant is intentional (see above), so the guard needs a way to permit it once it can see it. Use an inline waiver comment on the grant itself, e.g.

# gcp-secret-scope: allow - deploy SA creates and manages the secrets (LeanerCloud/cloud-commitments-platform#149)
"roles/secretmanager.admin",

rather than an allowlist file. It sits next to the thing it excuses, moves and dies with it, is visible in the diff that would widen the grant, and cannot silently match nothing.

Not doing now: replacing the scanner with hcl2json / terraform-config-inspect, or a conftest/OPA policy against plan JSON. Both are reasonable if the guard keeps accumulating blind spots, but neither is needed for the shape reported here, and swapping the awk scanner for a parsed-tree query plus a new build dependency is a larger change than the defect. Revisit when a third blind spot lands.

Acceptance

  • The guard exits 1 on terraform/environments/gcp/ci-cd-permissions/service_account.tf with the waiver comment removed, and 0 with it present. Both directions are asserted, so the waiver cannot go vacuous.
  • scripts/test-gcp-secret-scope.sh gains a fixture for the locals + for_each shape.
  • The locals / for_each bullet is removed from the LIMITATIONS header in scripts/check-gcp-secret-scope.sh, since it is no longer a blind spot.

Remedy simplified (2026-08-03): the hcl2json rewrite was dropped in favour of the cheapest option already listed (the locals pre-pass), which closes the demonstrated shape; the allowlist file plus its own self-test became an inline waiver comment, which needs no new mechanism and cannot match nothing. The acceptance criteria now exercise the waiver in both directions rather than only asserting one exists.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions