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/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..c4beff0f 100644 --- a/infra/terraform/modules/cloud-sql/main.tf +++ b/infra/terraform/modules/cloud-sql/main.tf @@ -9,11 +9,36 @@ 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" } + # 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,8 +52,20 @@ 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_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. + # 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" + dynamic "authorized_networks" { for_each = var.authorized_networks content { 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/infra/terraform/modules/storage/variables.tf b/infra/terraform/modules/storage/variables.tf index d821f658..6e1a4bf1 100644 --- a/infra/terraform/modules/storage/variables.tf +++ b/infra/terraform/modules/storage/variables.tf @@ -16,6 +16,20 @@ 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." + } + + # 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" { diff --git a/tests/scripts/sonar-infra-hardening.test.sh b/tests/scripts/sonar-infra-hardening.test.sh new file mode 100644 index 00000000..e1fc75fe --- /dev/null +++ b/tests/scripts/sonar-infra-hardening.test.sh @@ -0,0 +1,117 @@ +#!/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 uid + user="$(final_stage_user "$1")" + # 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. + # 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 = , +# 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:]]*#/ { 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" +} + +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 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" \ + 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 +# 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 } + 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 + echo "sonar-infra-hardening: FAILED (${pass_count} passed)" + exit 1 +fi +echo "sonar-infra-hardening: all ${pass_count} checks passed"