Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,12 @@ jobs:
terraform_version: 1.10.5

- name: Terraform Format Check
run: terraform fmt -check -recursive terraform/
# Must cover every SCAN_ROOTS entry in scripts/check-gcp-secret-scope.sh,
# whose block tracking was written against fmt layout (#150). The
# guard's self-test fails if a scan root has no line here.
run: |
terraform fmt -check -recursive terraform/
terraform fmt -check -recursive iac/

- name: Test Azure federation identity guards
if: matrix.cloud == 'azure'
Expand Down
61 changes: 60 additions & 1 deletion scripts/check-gcp-secret-scope.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@
# which role it becomes. Refusing it is the fail-closed answer; the fix is to
# write the role name out.
#
# The one shape that breaks the premise above is refused outright (issue #150):
# an authoritative policy (`google_project_iam_policy`, `google_folder_iam_policy`,
# `google_organization_iam_policy`) and the `data "google_iam_policy"` source
# that builds policy_data for one. policy_data can be read with file() from a
# JSON file the guard does not scan, or handed in through a variable or module
# output, so the role it grants need never appear as text here. It is also the
# most dangerous shape: it REPLACES the scope's whole IAM policy. Any
# non-comment mention of one of those type names exits 2 before the scan runs,
# and no waiver applies. Correlating policy_data with its source is the work to
# do if one is ever genuinely needed.
#
# WAIVERS. A deliberate scope-wide Secret Manager grant is permitted by a comment
# on the line DIRECTLY above the mention it excuses, with a reason:
#
Expand All @@ -61,7 +72,8 @@
#
# Exit 0 = no un-waived Secret Manager mention and no plan-time role name found.
# Exit 1 = at least one found; each is printed to stderr.
# Exit 2 = usage error, or a file this guard cannot read (JSON encoding).
# Exit 2 = usage error, a file this guard cannot read (JSON encoding), or an
# authoritative IAM policy it cannot analyze (see above).
#
# LIMITATIONS. This is a textual guard, not a policy engine.
# - Values supplied from OUTSIDE the scanned tree are out of reach: `-var` on
Expand All @@ -75,6 +87,17 @@
# - The Terraform JSON encoding (`.tf.json`, `.tfvars.json`) is not parsed.
# Rather than report such a file as clean, the guard exits 2 if it finds one.
# None exist today.
# - The authoritative-policy shape (`google_{project,folder,organization}
# _iam_policy`, `data "google_iam_policy"`) is REJECTED with exit 2, not
# analyzed (issue #150). That includes a `google_iam_policy` data source
# feeding a per-secret `_iam_policy`, since the guard cannot tell where
# policy_data goes. None exist today.
# - Block tracking (which decides the per-secret exemption) counts braces and
# does not need `terraform fmt` layout to stay closed, but CI also runs
# `terraform fmt -check` over every SCAN_ROOTS entry, so unformatted code
# cannot land in the first place.
# - Resources inside modules sourced from outside the tree (registry, git)
# are not scanned.
#
# Usage:
# scripts/check-gcp-secret-scope.sh # scan the default roots
Expand Down Expand Up @@ -148,6 +171,42 @@ if [[ ${#files[@]} -eq 0 ]]; then
exit 2
fi

# Fail closed on the authoritative-policy shape (issue #150; see the header).
# Matches the type name as a token anywhere in a non-comment line, so a quoted
# or bare block label, any indentation, and references (`data.google_iam_policy.
# x`, `moved`, `import`) are all caught. Heredoc bodies and descriptions are
# not exempt here: over-reporting a name that should not be in the tree at all
# is the safe direction.
unsupported=$(
awk '
/^[[:space:]]*(#|\/\/)/ { next }
/(^|[^A-Za-z0-9_])google_(project|folder|organization)_iam_policy([^A-Za-z0-9_]|$)/ ||
/(^|[^A-Za-z0-9_])google_iam_policy([^A-Za-z0-9_]|$)/ {
t = $0
sub(/^[[:space:]]+/, "", t)
printf "%s:%d: %s\n", FILENAME, FNR, t
}
' "${files[@]}"
)
if [[ -n "$unsupported" ]]; then
{
echo "ERROR: this guard cannot analyze an authoritative IAM policy, and will not"
echo " report one as clean (issue #150). Found:"
while IFS= read -r finding; do
printf ' %s\n' "$finding"
done <<<"$unsupported"
echo ""
echo " google_{project,folder,organization}_iam_policy replaces the scope's"
echo " whole IAM policy with policy_data, which may come from file(), a"
echo " variable or a module output the guard cannot follow, so it cannot say"
echo " which roles are granted. Use google_{project,folder,organization}"
echo " _iam_member for non-secret roles and google_secret_manager_secret"
echo " _iam_member for secrets, or extend the guard to correlate policy_data"
echo " with its source before adding one."
} >&2
exit 2
fi

# Line scanner. Portable awk only (CI runs mawk, macOS runs BWK awk): no gawk
# extensions, no regex intervals, braces matched via bracket expressions.
#
Expand Down
74 changes: 71 additions & 3 deletions scripts/test-gcp-secret-scope.sh
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,15 @@ run_case "role passed as a module argument exits 1" 1 \
run_case "role supplied by a tfvars file exits 1" 1 \
"${FIXTURES}/roles.tfvars.fixture"

run_case "dynamic binding in google_iam_policy over a local exits 1" 1 \
# These three carried the authoritative-policy shape and exited 1 through the
# role text alone. Since issue #150 that shape is refused outright (exit 2).
run_case "dynamic binding in google_iam_policy over a local exits 2 (#150)" 2 \
"${FIXTURES}/dynamic-iam-policy.tf.fixture"

run_case "literal role in google_iam_policy data block exits 1" 1 \
run_case "literal role in google_iam_policy data block exits 2 (#150)" 2 \
"${FIXTURES}/iam-policy-data.tf.fixture"

run_case "JSON heredoc policy_data on google_project_iam_policy exits 1" 1 \
run_case "JSON heredoc policy_data on google_project_iam_policy exits 2 (#150)" 2 \
"${FIXTURES}/policy-heredoc.tf.fixture"

run_case "custom role carrying secretmanager.* permissions exits 1" 1 \
Expand All @@ -105,6 +107,42 @@ run_case "custom role carrying secretmanager.* permissions exits 1" 1 \
run_case "block not laid out the way terraform fmt does exits 1" 1 \
"${FIXTURES}/unformatted-block.tf.fixture"

# Issue #150, part 1: the authoritative-policy shape is refused (exit 2) rather
# than analyzed. Each of the first six exited 0 before #150, because the roles
# it grants never appear as `secretmanager` text in a scanned file.
run_case "google_project_iam_policy fed by file() exits 2 (#150)" 2 \
"${FIXTURES}/project-iam-policy-file.tf.fixture"

run_case "google_folder_iam_policy fed by a variable exits 2 (#150)" 2 \
"${FIXTURES}/folder-iam-policy.tf.fixture"

run_case "google_organization_iam_policy fed by jsonencode exits 2 (#150)" 2 \
"${FIXTURES}/org-iam-policy.tf.fixture"

run_case "google_iam_policy data source exported as an output exits 2 (#150)" 2 \
"${FIXTURES}/iam-policy-data-output.tf.fixture"

run_case "authoritative policy with bare block labels exits 2 (#150)" 2 \
"${FIXTURES}/iam-policy-unquoted-labels.tf.fixture"

run_case "authoritative policy with a column-0 body exits 2 (#150)" 2 \
"${FIXTURES}/iam-policy-unindented.tf.fixture"

run_case "rejected shape alongside a clean file still exits 2 (#150)" 2 \
"${FIXTURES}/clean.tf.fixture" "${FIXTURES}/project-iam-policy-file.tf.fixture"

# Negative direction: lookalike type names and comments are not refused.
run_case "lookalike IAM policy types and comments exit 0 (#150)" 0 \
"${FIXTURES}/iam-policy-lookalikes.tf.fixture"

# Issue #150, part 2: a body laid out in column 0 must not end a block early
# enough to drop a role, nor carry the per-secret exemption onward.
run_case "scope-wide grant with a column-0 body exits 1 (#150)" 1 \
"${FIXTURES}/unindented-body.tf.fixture"

run_case "column-0 per-secret body does not leak its exemption (#150)" 1 \
"${FIXTURES}/per-secret-unindented-body.tf.fixture"

# Fail closed on role names the scanner cannot resolve.
run_case "role assembled by interpolation exits 1" 1 \
"${FIXTURES}/interpolated-role.tf.fixture"
Expand Down Expand Up @@ -170,6 +208,36 @@ fi
run_case "ci-cd-permissions deploy SA grant fails without its waiver" 1 \
"${WORK}/service_account.tf"

# policy-heredoc now exits 2 on its resource type alone, which would hide a
# regression in how heredoc bodies are scanned. Retyped to a per-secret
# _iam_policy (not refused), the role inside the JSON heredoc must still be
# reported: only a plain `role = "roles/secretmanager.X"` line is exempt.
sed 's/google_project_iam_policy/google_secret_manager_secret_iam_policy/g' \
"${FIXTURES}/policy-heredoc.tf.fixture" > "${WORK}/per-secret-heredoc.tf"
run_case "JSON heredoc role on a per-secret _iam_policy exits 1" 1 \
"${WORK}/per-secret-heredoc.tf"

# Issue #150, part 2: the guard's block tracking was written against
# `terraform fmt` layout, so CI must run `terraform fmt -check -recursive` over
# every root the guard scans. Read the roots from the guard itself so adding one without a fmt gate
# fails here.
CI_YML="${REPO_ROOT}/.github/workflows/ci.yml"
# shellcheck disable=SC2016 # ${REPO_ROOT} is matched literally, not expanded.
roots=$(sed -n '/^SCAN_ROOTS=(/,/^)/s|^ *"\${REPO_ROOT}/\([^"]*\)"$|\1|p' "$CHECK")
if [[ -z "$roots" ]]; then
echo "FAIL: could not read SCAN_ROOTS from $CHECK"
(( fail++ )) || true
fi
for root in $roots; do
if grep -Eq "^[[:space:]]*(run:[[:space:]]*)?terraform fmt -check -recursive ${root}/?[[:space:]]*$" "$CI_YML"; then
echo "PASS: CI runs terraform fmt -check over scan root ${root}/"
(( pass++ )) || true
else
echo "FAIL: CI has no 'terraform fmt -check -recursive ${root}/' step for scan root ${root}/"
(( fail++ )) || true
fi
done

# Default-root discovery: run a copy of the guard from a throwaway tree laid out
# like the repo, so the cases below exercise which files it COLLECTS, not only
# how it scans files it is handed.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Fixture: an authoritative project policy built from a `google_iam_policy` data
# source whose bindings come from a `dynamic` block over a local list. Neither
# the data block nor the policy resource carries the literal. Must exit 1.
# the data block nor the policy resource carries the literal. Since issue #150
# the guard refuses the authoritative-policy shape outright. Must exit 2.

locals {
policy_roles = ["roles/viewer", "roles/secretmanager.secretAccessor"]
Expand Down
11 changes: 11 additions & 0 deletions scripts/testdata/gcp-secret-scope/folder-iam-policy.tf.fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Fixture (issue #150): an authoritative folder policy whose policy_data comes
# from a variable with no in-tree default. Must exit 2.

variable "folder_policy_data" {
type = string
}

resource "google_folder_iam_policy" "folder" {
folder = var.folder_id
policy_data = var.folder_policy_data
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Fixture (issue #150): a `google_iam_policy` data source on its own, exported as
# a module output. Its policy_data can be applied by a scope-wide policy
# resource in whichever configuration consumes the output, which the guard
# cannot follow. The role comes from a variable with no in-tree default.
# Must exit 2.

data "google_iam_policy" "workload" {
binding {
role = var.workload_role
members = ["serviceAccount:workload@example.iam.gserviceaccount.com"]
}
}

output "workload_policy_data" {
value = data.google_iam_policy.workload.policy_data
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Fixture: a literal role inside a `google_iam_policy` data block feeding
# google_project_iam_policy. The data block is not a scope-wide IAM resource, so
# a scanner keyed on resource type misses it. Must exit 1.
# a scanner keyed on resource type misses it. Since issue #150 the guard refuses
# the authoritative-policy shape outright rather than analyzing it. Must exit 2.

data "google_iam_policy" "project" {
binding {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Fixture (issue #150, negative direction): types whose names share words with
# the rejected authoritative-policy types, and a comment naming one, must not
# be rejected. Only google_{project,folder,organization}_iam_policy and
# google_iam_policy are. Must exit 0.

# A comment may say google_project_iam_policy or data "google_iam_policy".
// So may a // comment: google_organization_iam_policy.

resource "google_secret_manager_secret_iam_policy" "secret" {
project = var.project_id
secret_id = var.secret_id
policy_data = var.secret_policy_data
}

resource "google_storage_bucket_iam_policy" "bucket" {
bucket = var.bucket_name
policy_data = var.bucket_policy_data
}

resource "google_project_iam_member" "viewer" {
project = var.project_id
role = "roles/viewer"
member = "serviceAccount:workload@example.iam.gserviceaccount.com"
}

locals {
my_google_iam_policy_name = "not-a-data-source"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Fixture (issue #150): an authoritative project policy whose body starts in
# column 0, the layout `terraform fmt` would rewrite. Must exit 2.

resource "google_project_iam_policy" "project" {
project = var.project_id
policy_data = var.project_policy_data
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Fixture (issue #150): HCL accepts bare identifiers as block labels, so the
# resource type need not be quoted. Matching only `resource "google_..._policy"`
# would miss this. Must exit 2.

resource google_project_iam_policy project {
project = var.project_id
policy_data = var.project_policy_data
}
13 changes: 13 additions & 0 deletions scripts/testdata/gcp-secret-scope/org-iam-policy.tf.fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Fixture (issue #150): an authoritative organization policy built with
# jsonencode() from a role supplied by a variable with no in-tree default.
# Must exit 2.

resource "google_organization_iam_policy" "org" {
org_id = var.org_id
policy_data = jsonencode({
bindings = [{
role = var.org_role
members = ["serviceAccount:workload@example.iam.gserviceaccount.com"]
}]
})
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Fixture (issue #150): a per-secret binding whose body starts in column 0, with
# a column-0 `}` closing a nested block before a second resource. The
# per-secret exemption must not reach the project grant that follows, however
# the closing braces are laid out. Must exit 1.

resource "google_secret_manager_secret_iam_member" "unindented" {
project = var.project_id
secret_id = var.secret_id
condition {
title = "always"
expression = "true"
}
role = "roles/secretmanager.secretAccessor"
member = "serviceAccount:workload@example.iam.gserviceaccount.com"
}
resource "google_project_iam_member" "after" {
project = var.project_id
role = "roles/secretmanager.secretAccessor"
member = "serviceAccount:workload@example.iam.gserviceaccount.com"
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Fixture: an authoritative project policy given as a JSON heredoc. The role is
# heredoc text, and the JSON's own column-0 braces must not end the scan of it.
# Must exit 1.
# Since issue #150 the guard refuses google_project_iam_policy outright, so this
# file exits 2; test-gcp-secret-scope.sh also runs a copy retyped to a per-secret
# _iam_policy, which keeps the heredoc scan itself covered (exit 1).

resource "google_project_iam_policy" "project" {
project = var.project_id
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Fixture (issue #150): an authoritative project policy whose policy_data is read
# from a JSON file. The roles live in the .json file, which the guard does not
# scan, so no `secretmanager` text appears anywhere it looks. The guard cannot
# analyze this shape and must refuse it rather than report it clean. Must exit 2.

resource "google_project_iam_policy" "project" {
project = var.project_id
policy_data = file("${path.module}/project-policy.json")
}
14 changes: 14 additions & 0 deletions scripts/testdata/gcp-secret-scope/unindented-body.tf.fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Fixture (issue #150): a scope-wide grant whose body starts in column 0. The
# nested condition block's closing brace is in column 0 too, so a scanner that
# treats `^}` as the end of the resource stops attributing lines to it before
# the role. The role must still be reported. Must exit 1.

resource "google_project_iam_member" "unindented" {
project = var.project_id
condition {
title = "always"
expression = "true"
}
role = "roles/secretmanager.secretAccessor"
member = "serviceAccount:workload@example.iam.gserviceaccount.com"
}
Loading