From 5d9ebb4c7577db1efde0e70e528c6022c1afa3b1 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:22:55 +0000 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20implement=20issue=20#614=20?= =?UTF-8?q?=E2=80=94=20SonarCloud:=20miscellaneous=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/api/Dockerfile | 3 + apps/api/Dockerfile.worker | 3 + infra/terraform/modules/cloud-sql/main.tf | 31 ++++++- .../terraform/modules/cloud-sql/variables.tf | 5 ++ infra/terraform/modules/storage/main.tf | 37 ++++++++ infra/terraform/modules/storage/outputs.tf | 5 ++ tests/scripts/sonar-infra-hardening.test.sh | 89 +++++++++++++++++++ 7 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 tests/scripts/sonar-infra-hardening.test.sh diff --git a/apps/api/Dockerfile b/apps/api/Dockerfile index b4436f53..b53c8b35 100644 --- a/apps/api/Dockerfile +++ b/apps/api/Dockerfile @@ -11,6 +11,9 @@ FROM gcr.io/distroless/static-debian12@sha256:9c346e4be81b5ca7ff31a0d89eaeade58b COPY --from=builder /bin/server /server +# Run as the distroless unprivileged user (uid/gid 65532), never root. +USER nonroot:nonroot + EXPOSE 8080 ENTRYPOINT ["/server"] diff --git a/apps/api/Dockerfile.worker b/apps/api/Dockerfile.worker index 9f47b302..505145f2 100644 --- a/apps/api/Dockerfile.worker +++ b/apps/api/Dockerfile.worker @@ -11,4 +11,7 @@ FROM gcr.io/distroless/static-debian12@sha256:9c346e4be81b5ca7ff31a0d89eaeade58b COPY --from=builder /bin/worker /worker +# Run as the distroless unprivileged user (uid/gid 65532), never root. +USER nonroot:nonroot + ENTRYPOINT ["/worker"] diff --git a/infra/terraform/modules/cloud-sql/main.tf b/infra/terraform/modules/cloud-sql/main.tf index 77da7f43..1eafb904 100644 --- a/infra/terraform/modules/cloud-sql/main.tf +++ b/infra/terraform/modules/cloud-sql/main.tf @@ -14,6 +14,27 @@ resource "google_sql_database_instance" "main" { value = "on" } + # Audit logging: record session lifecycle, checkpoints, and lock waits. + database_flags { + name = "log_connections" + value = "on" + } + + database_flags { + name = "log_disconnections" + value = "on" + } + + database_flags { + name = "log_checkpoints" + value = "on" + } + + database_flags { + name = "log_lock_waits" + value = "on" + } + backup_configuration { enabled = true point_in_time_recovery_enabled = true @@ -27,7 +48,15 @@ resource "google_sql_database_instance" "main" { } ip_configuration { - ipv4_enabled = true + # Public IP is required: Cloud Run reaches the instance through the + # Cloud SQL connector (/cloudsql socket), and there is no VPC/private + # service access in this project. Connector traffic is IAM-authorized + # and TLS-encrypted; authorized_networks defaults to empty and rejects + # 0.0.0.0/0 (see variables.tf), so no direct public access is granted. + ipv4_enabled = true # NOSONAR — terraform:S6329 confirmed safe, see comment above + + # Reject unencrypted connections; the Cloud SQL connector always uses TLS. + ssl_mode = "ENCRYPTED_ONLY" dynamic "authorized_networks" { for_each = var.authorized_networks diff --git a/infra/terraform/modules/cloud-sql/variables.tf b/infra/terraform/modules/cloud-sql/variables.tf index a54d6f97..d87df7f8 100644 --- a/infra/terraform/modules/cloud-sql/variables.tf +++ b/infra/terraform/modules/cloud-sql/variables.tf @@ -43,6 +43,11 @@ variable "authorized_networks" { value = string })) default = [] + + validation { + condition = alltrue([for n in var.authorized_networks : !contains(["0.0.0.0/0", "::/0"], n.value)]) + error_message = "authorized_networks must not include 0.0.0.0/0 or ::/0 (open to the internet)." + } } variable "availability_type" { diff --git a/infra/terraform/modules/storage/main.tf b/infra/terraform/modules/storage/main.tf index 8eb787ec..af065fbc 100644 --- a/infra/terraform/modules/storage/main.tf +++ b/infra/terraform/modules/storage/main.tf @@ -10,6 +10,11 @@ resource "google_storage_bucket" "media" { enabled = true } + logging { + log_bucket = google_storage_bucket.access_logs.name + log_object_prefix = var.bucket_name + } + dynamic "cors" { for_each = length(var.cors_origins) > 0 ? [1] : [] content { @@ -40,3 +45,35 @@ resource "google_storage_bucket" "media" { } } } + +# Destination for media bucket usage/storage access logs. This bucket is the +# log sink itself, so it does not log its own access (that would recurse). +resource "google_storage_bucket" "access_logs" { # NOSONAR — terraform:S6258 confirmed safe: this is the access-log sink + name = "${var.bucket_name}-access-logs" + project = var.project_id + location = var.region + + uniform_bucket_level_access = true + public_access_prevention = "enforced" + force_destroy = var.environment == "dev" ? true : false + + versioning { + enabled = true + } + + lifecycle_rule { + condition { + age = 365 + } + action { + type = "Delete" + } + } +} + +# Cloud Storage writes usage and storage logs as this Google-managed group. +resource "google_storage_bucket_iam_member" "access_logs_writer" { + bucket = google_storage_bucket.access_logs.name + role = "roles/storage.objectCreator" + member = "group:cloud-storage-analytics@google.com" +} diff --git a/infra/terraform/modules/storage/outputs.tf b/infra/terraform/modules/storage/outputs.tf index 8e73bc47..4b6fbbc0 100644 --- a/infra/terraform/modules/storage/outputs.tf +++ b/infra/terraform/modules/storage/outputs.tf @@ -12,3 +12,8 @@ output "bucket_self_link" { description = "Self link of the media storage bucket" value = google_storage_bucket.media.self_link } + +output "access_logs_bucket_name" { + description = "Name of the bucket receiving media bucket access logs" + value = google_storage_bucket.access_logs.name +} diff --git a/tests/scripts/sonar-infra-hardening.test.sh b/tests/scripts/sonar-infra-hardening.test.sh new file mode 100644 index 00000000..35440b6d --- /dev/null +++ b/tests/scripts/sonar-infra-hardening.test.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# Test: infrastructure hardening that resolves the SonarCloud findings tracked +# in issue #614 stays in place. +# +# docker:S6471 — API/worker images must not run as root +# terraform:S6258 — Cloud SQL connection/lock/checkpoint logging and GCS +# access logging must be enabled +# terraform:S6403 — Cloud SQL must reject unencrypted connections +# terraform:S6329 — Cloud SQL public IP is only reachable via the Cloud SQL +# connector; authorized_networks must never open 0.0.0.0/0 +# +# Run: bash tests/scripts/sonar-infra-hardening.test.sh + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +SQL_MAIN="${REPO_ROOT}/infra/terraform/modules/cloud-sql/main.tf" +SQL_VARS="${REPO_ROOT}/infra/terraform/modules/cloud-sql/variables.tf" +STORAGE_MAIN="${REPO_ROOT}/infra/terraform/modules/storage/main.tf" + +fail=0 +pass_count=0 + +check() { + local desc="$1" + shift + if "$@"; then + echo "PASS: ${desc}" + pass_count=$((pass_count + 1)) + else + echo "FAIL: ${desc}" + fail=1 + fi +} + +# final_stage_user +# Prints the last USER instruction that appears after the final FROM. +final_stage_user() { + awk ' + /^[[:space:]]*FROM[[:space:]]/ { user = "" } + /^[[:space:]]*USER[[:space:]]/ { user = $2 } + END { print user } + ' "$1" +} + +runs_as_non_root() { + local user + user="$(final_stage_user "$1")" + [[ -n "$user" && "${user%%:*}" != "root" && "${user%%:*}" != "0" ]] +} + +# has_db_flag +# Succeeds when a database_flags block sets name = and value = . +has_db_flag() { + awk -v flag="$2" -v want="$3" ' + /database_flags[[:space:]]*\{/ { in_blk = 1; name = ""; value = ""; next } + in_blk && /name[[:space:]]*=/ { line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); name = line } + in_blk && /value[[:space:]]*=/ { line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); value = line } + in_blk && /\}/ { if (name == flag && value == want) found = 1; in_blk = 0 } + END { exit found ? 0 : 1 } + ' "$1" +} + +for dockerfile in "${REPO_ROOT}/apps/api/Dockerfile" "${REPO_ROOT}/apps/api/Dockerfile.worker"; do + check "${dockerfile#"${REPO_ROOT}/"} final stage runs as a non-root USER" runs_as_non_root "$dockerfile" +done + +for flag in log_connections log_disconnections log_checkpoints log_lock_waits; do + check "cloud-sql sets database flag ${flag}=on" has_db_flag "$SQL_MAIN" "$flag" "on" +done + +check "cloud-sql enforces encrypted connections (ssl_mode = ENCRYPTED_ONLY)" \ + grep -Eq '^[[:space:]]*ssl_mode[[:space:]]*=[[:space:]]*"(ENCRYPTED_ONLY|TRUSTED_CLIENT_CERTIFICATE_REQUIRED)"' "$SQL_MAIN" + +check "cloud-sql authorized_networks rejects 0.0.0.0/0" \ + grep -q '0\.0\.0\.0/0' "$SQL_VARS" + +check "storage media bucket has an access logging block" \ + grep -Eq '^[[:space:]]*log_bucket[[:space:]]*=' "$STORAGE_MAIN" + +check "storage grants Cloud Storage analytics write access to the log bucket" \ + grep -q 'group:cloud-storage-analytics@google.com' "$STORAGE_MAIN" + +echo +if [[ "$fail" -ne 0 ]]; then + echo "sonar-infra-hardening: FAILED (${pass_count} passed)" + exit 1 +fi +echo "sonar-infra-hardening: all ${pass_count} checks passed" From 4b0ce75602b69ead38e9aea83609b8a55190b532 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:31:44 +0000 Subject: [PATCH 2/4] fix(reviews): enforce Cloud SQL connector, validate bucket name, harden infra test Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 2 ++ infra/terraform/modules/cloud-sql/main.tf | 11 ++++-- infra/terraform/modules/storage/variables.tf | 8 +++++ tests/scripts/sonar-infra-hardening.test.sh | 37 +++++++++++++++----- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a36a45a8..7e3a922d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -204,6 +204,8 @@ jobs: run: bash tests/scripts/validate-workflows.test.sh - name: Dependabot auto-merge contract test run: bash tests/scripts/dependabot-automerge-contract.test.sh + - name: Sonar infra hardening test + run: bash tests/scripts/sonar-infra-hardening.test.sh secret-scan: name: Secret scan (gitleaks) diff --git a/infra/terraform/modules/cloud-sql/main.tf b/infra/terraform/modules/cloud-sql/main.tf index 1eafb904..af519fa4 100644 --- a/infra/terraform/modules/cloud-sql/main.tf +++ b/infra/terraform/modules/cloud-sql/main.tf @@ -9,6 +9,10 @@ resource "google_sql_database_instance" "main" { tier = var.tier availability_type = var.availability_type + # Public-IP connections must go through the Cloud SQL connector (IAM + # authorization + TLS); direct client connections are rejected. + connector_enforcement = "REQUIRED" + database_flags { name = "cloudsql.enable_pgvector" value = "on" @@ -50,9 +54,10 @@ resource "google_sql_database_instance" "main" { ip_configuration { # Public IP is required: Cloud Run reaches the instance through the # Cloud SQL connector (/cloudsql socket), and there is no VPC/private - # service access in this project. Connector traffic is IAM-authorized - # and TLS-encrypted; authorized_networks defaults to empty and rejects - # 0.0.0.0/0 (see variables.tf), so no direct public access is granted. + # service access in this project. connector_enforcement = "REQUIRED" + # (see settings above) rejects any connection that bypasses the connector, + # so even a caller-supplied authorized_networks range cannot open direct + # access; connector traffic is IAM-authorized and TLS-encrypted. ipv4_enabled = true # NOSONAR — terraform:S6329 confirmed safe, see comment above # Reject unencrypted connections; the Cloud SQL connector always uses TLS. diff --git a/infra/terraform/modules/storage/variables.tf b/infra/terraform/modules/storage/variables.tf index d821f658..9ec1c512 100644 --- a/infra/terraform/modules/storage/variables.tf +++ b/infra/terraform/modules/storage/variables.tf @@ -16,6 +16,14 @@ variable "environment" { variable "bucket_name" { description = "Name of the GCS bucket for media storage" type = string + + # The access-log bucket is "-access-logs" (12 chars appended); + # keep the derived name within the 63-char limit and free of dots, which + # would turn it into a domain-style name with different naming rules. + validation { + condition = can(regex("^[a-z0-9][a-z0-9_-]{1,49}[a-z0-9]$", var.bucket_name)) + error_message = "bucket_name must be 3-51 chars of lowercase letters, digits, hyphens or underscores (no dots), starting and ending with a letter or digit, so the derived '-access-logs' bucket name stays valid." + } } variable "cors_origins" { diff --git a/tests/scripts/sonar-infra-hardening.test.sh b/tests/scripts/sonar-infra-hardening.test.sh index 35440b6d..e70facdc 100644 --- a/tests/scripts/sonar-infra-hardening.test.sh +++ b/tests/scripts/sonar-infra-hardening.test.sh @@ -46,16 +46,19 @@ final_stage_user() { runs_as_non_root() { local user user="$(final_stage_user "$1")" - [[ -n "$user" && "${user%%:*}" != "root" && "${user%%:*}" != "0" ]] + # Only the distroless non-root identity is accepted: an unknown user name + # makes Cloud Run fall back to root, so a bare "not root" spelling is not enough. + [[ "$user" == "nonroot" || "$user" == "nonroot:nonroot" || "$user" == "65532" || "$user" == "65532:65532" ]] } # has_db_flag # Succeeds when a database_flags block sets name = and value = . has_db_flag() { awk -v flag="$2" -v want="$3" ' - /database_flags[[:space:]]*\{/ { in_blk = 1; name = ""; value = ""; next } - in_blk && /name[[:space:]]*=/ { line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); name = line } - in_blk && /value[[:space:]]*=/ { line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); value = line } + /^[[:space:]]*(#|\/\/)/ { next } + /^[[:space:]]*database_flags[[:space:]]*\{/ { in_blk = 1; name = ""; value = ""; next } + in_blk && /^[[:space:]]*name[[:space:]]*=/ { line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); name = line } + in_blk && /^[[:space:]]*value[[:space:]]*=/{ line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); value = line } in_blk && /\}/ { if (name == flag && value == want) found = 1; in_blk = 0 } END { exit found ? 0 : 1 } ' "$1" @@ -72,14 +75,32 @@ done check "cloud-sql enforces encrypted connections (ssl_mode = ENCRYPTED_ONLY)" \ grep -Eq '^[[:space:]]*ssl_mode[[:space:]]*=[[:space:]]*"(ENCRYPTED_ONLY|TRUSTED_CLIENT_CERTIFICATE_REQUIRED)"' "$SQL_MAIN" -check "cloud-sql authorized_networks rejects 0.0.0.0/0" \ - grep -q '0\.0\.0\.0/0' "$SQL_VARS" +check "cloud-sql enforces connector-only access (connector_enforcement = REQUIRED)" \ + grep -Eq '^[[:space:]]*connector_enforcement[[:space:]]*=[[:space:]]*"REQUIRED"' "$SQL_MAIN" + +check "cloud-sql authorized_networks validation rejects 0.0.0.0/0" \ + grep -Eq '^[[:space:]]*condition[[:space:]]*=.*!contains\(\[[^]]*"0\.0\.0\.0/0"' "$SQL_VARS" check "storage media bucket has an access logging block" \ grep -Eq '^[[:space:]]*log_bucket[[:space:]]*=' "$STORAGE_MAIN" -check "storage grants Cloud Storage analytics write access to the log bucket" \ - grep -q 'group:cloud-storage-analytics@google.com' "$STORAGE_MAIN" +# iam_member_grants +# Succeeds when a single google_storage_bucket_iam_member resource sets all three. +iam_member_grants() { + awk -v bucket="$2" -v role="$3" -v member="$4" ' + /^[[:space:]]*(#|\/\/)/ { next } + /^resource[[:space:]]+"google_storage_bucket_iam_member"/ { in_res = 1; b = r = m = 0; next } + in_res && /^[[:space:]]*bucket[[:space:]]*=/ { if (index($0, bucket)) b = 1 } + in_res && /^[[:space:]]*role[[:space:]]*=/ { if (index($0, "\"" role "\"")) r = 1 } + in_res && /^[[:space:]]*member[[:space:]]*=/ { if (index($0, "\"" member "\"")) m = 1 } + in_res && /^\}/ { if (b && r && m) found = 1; in_res = 0 } + END { exit found ? 0 : 1 } + ' "$1" +} + +check "storage grants Cloud Storage analytics objectCreator on the log bucket" \ + iam_member_grants "$STORAGE_MAIN" "google_storage_bucket.access_logs.name" \ + "roles/storage.objectCreator" "group:cloud-storage-analytics@google.com" echo if [[ "$fail" -ne 0 ]]; then From e468a4a9ad5ca18cef1a4ab961addfceba6898bd Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 28 Oct 2026 01:29:01 +0000 Subject: [PATCH 3/4] fix(reviews): improve infrastructure hardening validation checks - Update Cloud SQL NOSONAR comment to explicitly describe connector enforcement - Enhance sonar-infra-hardening.test.sh to verify non-root UID resolution, not just username - Improve database_flags test to skip commented lines within blocks - Strengthen authorized_networks validation check to verify active condition - Add comment-skipping to IAM member grant verification Co-Authored-By: Claude Haiku 4.5 --- infra/terraform/modules/cloud-sql/main.tf | 2 +- tests/scripts/sonar-infra-hardening.test.sh | 23 ++++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/infra/terraform/modules/cloud-sql/main.tf b/infra/terraform/modules/cloud-sql/main.tf index af519fa4..4c900e1c 100644 --- a/infra/terraform/modules/cloud-sql/main.tf +++ b/infra/terraform/modules/cloud-sql/main.tf @@ -58,7 +58,7 @@ resource "google_sql_database_instance" "main" { # (see settings above) rejects any connection that bypasses the connector, # so even a caller-supplied authorized_networks range cannot open direct # access; connector traffic is IAM-authorized and TLS-encrypted. - ipv4_enabled = true # NOSONAR — terraform:S6329 confirmed safe, see comment above + ipv4_enabled = true # NOSONAR — terraform:S6329 confirmed safe: connector_enforcement = "REQUIRED" enforces all public-IP connections use the Cloud SQL connector # Reject unencrypted connections; the Cloud SQL connector always uses TLS. ssl_mode = "ENCRYPTED_ONLY" diff --git a/tests/scripts/sonar-infra-hardening.test.sh b/tests/scripts/sonar-infra-hardening.test.sh index e70facdc..e1fc75fe 100644 --- a/tests/scripts/sonar-infra-hardening.test.sh +++ b/tests/scripts/sonar-infra-hardening.test.sh @@ -44,21 +44,26 @@ final_stage_user() { } runs_as_non_root() { - local user + local user uid user="$(final_stage_user "$1")" - # Only the distroless non-root identity is accepted: an unknown user name + # Verify the resolved UID is non-root. Only the distroless non-root identity + # (UID 65532) or explicit non-root numeric UIDs are accepted: an unknown user name # makes Cloud Run fall back to root, so a bare "not root" spelling is not enough. - [[ "$user" == "nonroot" || "$user" == "nonroot:nonroot" || "$user" == "65532" || "$user" == "65532:65532" ]] + # Extract numeric UID if format is UID:GID. + uid="${user%%:*}" + [[ "$uid" =~ ^[0-9]+$ && "$uid" != "0" ]] || [[ "$user" == "nonroot" || "$user" == "nonroot:nonroot" || "$user" == "65532" || "$user" == "65532:65532" ]] } # has_db_flag -# Succeeds when a database_flags block sets name = and value = . +# Succeeds when a database_flags block sets name = and value = , +# matching only active (uncommented) lines. has_db_flag() { awk -v flag="$2" -v want="$3" ' /^[[:space:]]*(#|\/\/)/ { next } /^[[:space:]]*database_flags[[:space:]]*\{/ { in_blk = 1; name = ""; value = ""; next } - in_blk && /^[[:space:]]*name[[:space:]]*=/ { line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); name = line } - in_blk && /^[[:space:]]*value[[:space:]]*=/{ line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); value = line } + in_blk && /^[[:space:]]*#/ { next } + in_blk && /^[[:space:]]*name[[:space:]]*=/ { if (!/^[[:space:]]*(#|\/\/)/) { line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); name = line } } + in_blk && /^[[:space:]]*value[[:space:]]*=/ { if (!/^[[:space:]]*(#|\/\/)/) { line = $0; sub(/.*=[[:space:]]*"/, "", line); sub(/".*/, "", line); value = line } } in_blk && /\}/ { if (name == flag && value == want) found = 1; in_blk = 0 } END { exit found ? 0 : 1 } ' "$1" @@ -79,17 +84,19 @@ check "cloud-sql enforces connector-only access (connector_enforcement = REQUIRE grep -Eq '^[[:space:]]*connector_enforcement[[:space:]]*=[[:space:]]*"REQUIRED"' "$SQL_MAIN" check "cloud-sql authorized_networks validation rejects 0.0.0.0/0" \ - grep -Eq '^[[:space:]]*condition[[:space:]]*=.*!contains\(\[[^]]*"0\.0\.0\.0/0"' "$SQL_VARS" + awk '/^[[:space:]]*(#|\/\/)/ { next } /^[[:space:]]*validation[[:space:]]*\{/ { in_val = 1; next } in_val && /^[[:space:]]*(#|\/\/)/ { next } in_val && /^[[:space:]]*condition[[:space:]]*=/ { cond = $0; if (index(cond, "!contains") && index(cond, "0.0.0.0/0")) found = 1 } in_val && /\}/ { in_val = 0 } END { exit found ? 0 : 1 }' "$SQL_VARS" check "storage media bucket has an access logging block" \ grep -Eq '^[[:space:]]*log_bucket[[:space:]]*=' "$STORAGE_MAIN" # iam_member_grants -# Succeeds when a single google_storage_bucket_iam_member resource sets all three. +# Succeeds when a single google_storage_bucket_iam_member resource sets all three +# together in the same resource block, with active (uncommented) fields. iam_member_grants() { awk -v bucket="$2" -v role="$3" -v member="$4" ' /^[[:space:]]*(#|\/\/)/ { next } /^resource[[:space:]]+"google_storage_bucket_iam_member"/ { in_res = 1; b = r = m = 0; next } + in_res && /^[[:space:]]*(#|\/\/)/ { next } in_res && /^[[:space:]]*bucket[[:space:]]*=/ { if (index($0, bucket)) b = 1 } in_res && /^[[:space:]]*role[[:space:]]*=/ { if (index($0, "\"" role "\"")) r = 1 } in_res && /^[[:space:]]*member[[:space:]]*=/ { if (index($0, "\"" member "\"")) m = 1 } From 7d7ab80497d38a26a8d3069f8175b6046be9f8e9 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 28 Oct 2026 01:37:27 +0000 Subject: [PATCH 4/4] fix(reviews): drop S6329 false-positive suppression, reject reserved GCS bucket names Co-Authored-By: Claude Sonnet 5.5 --- infra/terraform/modules/cloud-sql/main.tf | 5 ++++- infra/terraform/modules/storage/variables.tf | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/infra/terraform/modules/cloud-sql/main.tf b/infra/terraform/modules/cloud-sql/main.tf index 4c900e1c..c4beff0f 100644 --- a/infra/terraform/modules/cloud-sql/main.tf +++ b/infra/terraform/modules/cloud-sql/main.tf @@ -58,7 +58,10 @@ resource "google_sql_database_instance" "main" { # (see settings above) rejects any connection that bypasses the connector, # so even a caller-supplied authorized_networks range cannot open direct # access; connector traffic is IAM-authorized and TLS-encrypted. - ipv4_enabled = true # NOSONAR — terraform:S6329 confirmed safe: connector_enforcement = "REQUIRED" enforces all public-IP connections use the Cloud SQL connector + # The public IPv4 endpoint still exists, so terraform:S6329 stays an + # accepted-risk finding to be tracked via the exception process (not + # suppressed as a false positive) until private IP + VPC egress lands. + ipv4_enabled = true # Reject unencrypted connections; the Cloud SQL connector always uses TLS. ssl_mode = "ENCRYPTED_ONLY" diff --git a/infra/terraform/modules/storage/variables.tf b/infra/terraform/modules/storage/variables.tf index 9ec1c512..6e1a4bf1 100644 --- a/infra/terraform/modules/storage/variables.tf +++ b/infra/terraform/modules/storage/variables.tf @@ -24,6 +24,12 @@ variable "bucket_name" { condition = can(regex("^[a-z0-9][a-z0-9_-]{1,49}[a-z0-9]$", var.bucket_name)) error_message = "bucket_name must be 3-51 chars of lowercase letters, digits, hyphens or underscores (no dots), starting and ending with a letter or digit, so the derived '-access-logs' bucket name stays valid." } + + # Cloud Storage reserves names starting with "goog" or containing "google". + validation { + condition = !can(regex("^goog|google", var.bucket_name)) + error_message = "bucket_name must not start with 'goog' or contain 'google' (reserved by Cloud Storage)." + } } variable "cors_origins" {