diff --git a/.dockerignore b/.dockerignore index 1eed8dfe6..66d357007 100644 --- a/.dockerignore +++ b/.dockerignore @@ -23,9 +23,12 @@ vendor/ # Temporary files tmp/ -# Docker compose dev files +# Docker compose dev and test files. Excluding a Dockerfile from the context +# does not stop `docker build -f` from using it. docker-compose.yml Dockerfile.dev +docker-compose.test.yml +Dockerfile.test # Secrets and credentials .env diff --git a/.env.example b/.env.example index 196f90174..46cd66add 100644 --- a/.env.example +++ b/.env.example @@ -66,9 +66,12 @@ CREDENTIAL_ENCRYPTION_ALLOW_DEV_KEY=1 # can trace the chain in one file. ADMIN_EMAIL=admin@cudly.local ADMIN_PASSWORD_SECRET=ADMIN_PASSWORD_DEV -ADMIN_PASSWORD_DEV=LocalDev!Pass123 +# Left empty on purpose, so a copy that skips these two fails at startup +# instead of booting with a password and API key published in this file. +# Set your own values in .env.local. +ADMIN_PASSWORD_DEV= API_KEY_SECRET_ARN=ADMIN_API_KEY_DEV -ADMIN_API_KEY_DEV=cudly-local-dev-api-key-not-for-prod +ADMIN_API_KEY_DEV= # Required for signed one-click notification unsubscribe links. Generate a # unique value for every environment (for example: openssl rand -hex 32). NOTIFICATION_MUTE_SECRET= diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d8a230650..976082e67 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -143,6 +143,11 @@ jobs: fi echo "✅ All functions have acceptable cyclomatic complexity (≤10)" + - name: Run Makefile gate self-tests + # The local lint, scanner and terraform-validate targets must fail + # when their tool is missing or fails (#349). Stub tools only. + run: bash scripts/test-make-gates.sh + # GitHub Actions workflow linting # # Nothing else in this repo reads .github/workflows/ for defects. govulncheck @@ -750,12 +755,12 @@ jobs: # Check local tfvars if present if [ -f "${env}.tfvars" ]; then echo "Checking ${env}.tfvars syntax..." - terraform fmt -check "${env}.tfvars" || echo "Note: ${env}.tfvars may need formatting" + terraform fmt -check "${env}.tfvars" fi # Check GitHub CI/CD tfvars if [ -f "github-${env}.tfvars" ]; then echo "Checking github-${env}.tfvars syntax..." - terraform fmt -check "github-${env}.tfvars" || echo "Note: github-${env}.tfvars may need formatting" + terraform fmt -check "github-${env}.tfvars" fi done diff --git a/.github/workflows/database-migration.yml b/.github/workflows/database-migration.yml index d3eca39bd..07e644a76 100644 --- a/.github/workflows/database-migration.yml +++ b/.github/workflows/database-migration.yml @@ -406,9 +406,13 @@ jobs: # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5' # above and golang-migrate dispatches on the URL scheme (issue #1849). DB_URL="pgx5://cudly@${DB_ENDPOINT}:5432/cudly?sslmode=require" + # Log only. migrate prints the version on stderr, so 2>&1 is needed + # to show it, but on failure the same capture holds a multi-line + # error. Nothing reads the value, so it is not written to + # $GITHUB_ENV, where such text could fail the step or set other + # variables for later steps (issue #349). VERSION=$(migrate -path "$MIGRATIONS_PATH" -database "$DB_URL" version 2>&1 || echo "unknown") echo "Current migration version: $VERSION" - echo "MIGRATION_VERSION=$VERSION" >> "$GITHUB_ENV" # Run GCP migrations migrate-gcp: diff --git a/.golangci.yml b/.golangci.yml index ac5a3bcc9..0212d5a6a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -40,7 +40,9 @@ linters: hugeParam: sizeThreshold: 1024 gocyclo: - min-complexity: 15 + # Same threshold as the standalone `gocyclo -over 10` gates in ci.yml and + # the Makefile, so `make lint` flags what CI fails (issue #349). + min-complexity: 10 gosec: excludes: - G104 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 59ddfc5dc..10ffbc69b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,8 +29,14 @@ repos: files: \.go$ # Terraform formatting + # + # Third-party hook repos are pinned to full commit SHAs with the release tag + # as a trailing comment, the same convention as the SHA-pinned Actions: a tag + # can be moved upstream, and these hooks run arbitrary code in the CI job + # (issue #349). To bump one: `git ls-remote refs/tags/`, or + # `pre-commit autoupdate --freeze`. - repo: https://github.com/antonbabenko/pre-commit-terraform - rev: v1.105.0 + rev: d0e12caebb2ab0ee8bf98181c8bfe9702bca103d # frozen: v1.105.0 hooks: - id: terraform_fmt name: Terraform format @@ -43,7 +49,7 @@ repos: # General file checks - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v6.0.0 + rev: 3e8a8703264a2f4a69428a0aa4dcb512790b2c8c # frozen: v6.0.0 hooks: - id: trailing-whitespace name: Trim trailing whitespace @@ -51,7 +57,9 @@ repos: name: Fix end of files - id: check-yaml name: Check YAML syntax - exclude: '^(docker-compose.*\.yml|\.github/workflows/.*|cloudformation/.*|iac/.*/cloudformation/.*)$' + # Workflows are checked too (issue #349): a workflow GitHub cannot + # parse is not run at all, with no failing check to show for it. + exclude: '^(docker-compose.*\.yml|cloudformation/.*|iac/.*/cloudformation/.*)$' - id: check-json name: Check JSON syntax - id: check-added-large-files @@ -137,7 +145,7 @@ repos: # Markdown linting - repo: https://github.com/igorshubovych/markdownlint-cli - rev: v0.47.0 + rev: 76b3d32d3f4b965e1d6425253c59407420ae2c43 # frozen: v0.47.0 hooks: - id: markdownlint name: Lint Markdown files diff --git a/.snyk b/.snyk index 1ee37bf2a..83f2753d9 100644 --- a/.snyk +++ b/.snyk @@ -22,22 +22,7 @@ exclude: - '**/.terraform/**' - '**/migrations/**' -# Severity thresholds -failOnSeverity: high - -# License policy -license: - # Allow these licenses - allow: - - MIT - - Apache-2.0 - - BSD-2-Clause - - BSD-3-Clause - - ISC - - MPL-2.0 - - # Deny these licenses - deny: - - GPL-2.0 - - GPL-3.0 - - AGPL-3.0 +# No severity threshold or license policy here: the Snyk CLI does not read +# such keys from this file (issue #349). The threshold is passed on the +# command line (`--severity-threshold=high` in ci.yml and the Makefile), and a +# license policy has to be set in the Snyk organization's settings. diff --git a/.trivyignore b/.trivyignore index 48f88492f..f8f1b4622 100644 --- a/.trivyignore +++ b/.trivyignore @@ -15,10 +15,12 @@ AVD-AWS-0011 # CloudFront minimum TLS protocol version. -# Justification: pre-existing default (TLS 1.0) was tightened in a -# prior PR; the trivy ID still trips when the value is inherited -# rather than set explicitly. Tracked as a follow-up to pin -# TLSv1.2_2021 explicitly in modules/frontend/aws. +# The finding is real, not a scanner artifact: modules/frontend/aws/main.tf +# sets minimum_protocol_version = "TLSv1" explicitly whenever no ACM +# certificate is given (the default-certificate branch). +# Justification: no environment deploys the distribution today; it is only +# created with enable_cdn = true, and every shipped tfvars sets +# enable_cdn = false. Revisit before the enable_cdn = true cutover. AVD-AWS-0013 # ALB drop_invalid_header_fields. @@ -93,9 +95,9 @@ AZU-0047 # GCP Cloud SQL instance does not require TLS. # ID: GCP-0015 (surfaced by trivy v0.70.0; v0.69.3 did not report this ID). # Justification: the database module (terraform/modules/database/gcp/main.tf) -# exposes require_ssl as a variable. TLS is enforced at the application layer -# via Go's pgx driver TLS config. Tracked as a hardening follow-up to set -# require_ssl = true in the module default. +# has no require_ssl variable. Both ip_configuration blocks hardcode +# ssl_mode = "ENCRYPTED_ONLY", which rejects non-TLS connections, so the +# entry looks stale. Drop it once a Trivy config scan without it is clean. GCP-0015 # GCP Cloud Storage bucket: cleanup Cloud Function source bucket. diff --git a/Makefile b/Makefile index f49e1f94a..e4c0eb1e3 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ help: ## Display available targets @echo " lint - Run golangci-lint" @echo " complexity - Check cyclomatic complexity" @echo " complexity-report - Generate detailed complexity report" - @echo " security-scan - Run security scanners (gosec, trivy, tfsec)" + @echo " security-scan - Run security scanners (gosec, trivy)" @echo " security-scan-all - Run all security scanners including Snyk" @echo " setup-git-secrets - Set up git-secrets for preventing credential leaks" @echo " terraform-validate - Validate Terraform configurations" @@ -112,14 +112,13 @@ tidy-check: (cd "$$mod" && GOTOOLCHAIN="go$$version" GOWORK=off go mod tidy -diff) || { echo "go mod tidy check failed for module $$mod" >&2; exit 1; }; \ done -# Lint code +# Lint code. The scanner targets below share this shape: a missing tool fails +# the target, and the tool runs as its own recipe line so make stops on its +# exit status instead of a trailing echo's (issue #349). lint: @echo "Running golangci-lint..." - @if command -v golangci-lint > /dev/null; then \ - golangci-lint run --timeout=5m; \ - else \ - echo "golangci-lint not installed. Install: make install-dev-tools"; \ - fi + @command -v golangci-lint > /dev/null || { echo "golangci-lint not installed. Install: make install-dev-tools" >&2; exit 1; } + golangci-lint run --timeout=5m # Go vet vet: @@ -156,39 +155,44 @@ complexity-report: echo "gocyclo not installed. Install: make install-dev-tools"; \ fi -# Security scanning -security-scan: security-scan-go security-scan-docker security-scan-terraform +# Security scanning. Terraform is covered by trivy's misconfig scanner in +# security-scan-docker, so the tfsec target is no longer part of this chain: +# CI replaced tfsec with Trivy, and tfsec is deprecated upstream (#349). +security-scan: security-scan-go security-scan-docker # scripts/run-gosec.sh owns the gosec pin, module list and rule set, shared # with the pre-commit hook and the CI gosec step (issue #340). security-scan-go: @scripts/run-gosec.sh +# Like CI's Trivy fs step, this reports findings without failing on them (no +# --exit-code); a scan error or a missing trivy does fail it. security-scan-docker: @echo "Running trivy..." - @if command -v trivy > /dev/null; then \ - trivy fs --security-checks vuln,config . --format json --output trivy-report.json; \ - echo "✓ Container security scan complete: trivy-report.json"; \ - else \ - echo "trivy not installed. Install: https://aquasecurity.github.io/trivy/"; \ - fi + @command -v trivy > /dev/null || { echo "trivy not installed. Install: https://aquasecurity.github.io/trivy/" >&2; exit 1; } + trivy fs --scanners vuln,misconfig . --format json --output trivy-report.json + @echo "✓ Container security scan complete: trivy-report.json" +# Deprecated (see security-scan above); kept only for anyone still calling it. security-scan-terraform: @echo "Running tfsec..." - @if command -v tfsec > /dev/null; then \ - tfsec terraform/ --format json --out tfsec-report.json; \ - echo "✓ Terraform security scan complete: tfsec-report.json"; \ - else \ - echo "tfsec not installed. Install: https://aquasecurity.github.io/tfsec/"; \ - fi + @command -v tfsec > /dev/null || { echo "tfsec not installed. Install: https://aquasecurity.github.io/tfsec/" >&2; exit 1; } + tfsec terraform/ --format json --out tfsec-report.json + @echo "✓ Terraform security scan complete: tfsec-report.json" -# Terraform validation +# Terraform validation. Each terraform/environments/ is one flat root +# (no per-profile subdirectories), the same roots CI's terraform-validate job +# validates. terraform-validate: @echo "Validating Terraform configurations..." - @for dir in terraform/environments/*/dev; do \ + @n=0; \ + for dir in terraform/environments/*/; do \ + [ -d "$$dir" ] || continue; \ echo "Validating $$dir..."; \ - (cd $$dir && terraform init -backend=false && terraform validate) || exit 1; \ - done + (cd "$$dir" && terraform init -backend=false -input=false && terraform validate) || exit 1; \ + n=$$((n + 1)); \ + done; \ + [ "$$n" -gt 0 ] || { echo "no Terraform roots found under terraform/environments" >&2; exit 1; } @echo "✓ Terraform validation complete" terraform-fmt: @@ -230,12 +234,9 @@ setup-git-secrets: # Snyk security scanning security-scan-snyk: @echo "Running Snyk security scan..." - @if command -v snyk > /dev/null; then \ - snyk test --severity-threshold=high; \ - echo "✓ Snyk scan complete"; \ - else \ - echo "snyk not installed. Install: npm install -g snyk"; \ - fi + @command -v snyk > /dev/null || { echo "snyk not installed. Install: npm install -g snyk" >&2; exit 1; } + snyk test --severity-threshold=high + @echo "✓ Snyk scan complete" # Run all security scanners including Snyk security-scan-all: security-scan security-scan-snyk @@ -264,7 +265,7 @@ install-dev-tools: @echo "" @echo "Additional tools to install manually:" @echo " - trivy: https://aquasecurity.github.io/trivy/" - @echo " - tfsec: https://aquasecurity.github.io/tfsec/" + @echo " - tfsec (deprecated, only for security-scan-terraform): https://aquasecurity.github.io/tfsec/" @echo " - git-secrets: https://github.com/awslabs/git-secrets" @echo " - snyk: npm install -g snyk" @echo " - pre-commit: pip install pre-commit" diff --git a/Makefile.terraform b/Makefile.terraform index b00196a17..97161c387 100644 --- a/Makefile.terraform +++ b/Makefile.terraform @@ -81,8 +81,17 @@ clean: ## Clean Terraform cache and state files (preserves .terraform.lock.hcl) @find terraform/environments -name "*.tfstate.backup" -type f -delete 2>/dev/null || true @echo "✅ Clean complete" -clean-locks: ## Delete .terraform.lock.hcl files (causes provider re-resolution on next init) +# The lock files are the committed provider checksum pins, so deleting them is +# a supply-chain change: the next init picks whatever the version constraints +# admit today. Hence the explicit opt-in (issue #349). +clean-locks: ## Delete .terraform.lock.hcl files (needs CONFIRM_CLEAN_LOCKS=yes) + @if [ "$(CONFIRM_CLEAN_LOCKS)" != "yes" ]; then \ + echo "Refusing to delete the .terraform.lock.hcl provider pins without CONFIRM_CLEAN_LOCKS=yes." >&2; \ + echo "The next terraform init re-resolves providers to the newest builds the constraints allow." >&2; \ + exit 1; \ + fi @echo "⚠️ Deleting lock files — run terraform init to re-resolve providers" + @echo "⚠️ Review the regenerated .terraform.lock.hcl diffs as a provider supply-chain change before committing them" @find terraform/environments -name ".terraform.lock.hcl" -type f -delete 2>/dev/null || true @echo "✅ Lock files deleted" diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 83454463c..a588f4fe3 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -317,8 +317,8 @@ make security-scan # Individual scans make security-scan-go # gosec -make security-scan-docker # trivy (container + filesystem) -make security-scan-terraform # tfsec +make security-scan-docker # trivy (dependencies + Terraform/Dockerfile misconfig) +make security-scan-terraform # tfsec (deprecated, not part of security-scan) ``` ### Terraform & Docker Testing diff --git a/scripts/test-make-gates.sh b/scripts/test-make-gates.sh new file mode 100755 index 000000000..24b466488 --- /dev/null +++ b/scripts/test-make-gates.sh @@ -0,0 +1,155 @@ +#!/usr/bin/env bash +# test-make-gates.sh +# +# Asserts that the Makefile's local gates fail when they should (issue #349): +# +# 1. lint, security-scan-docker, security-scan-terraform and +# security-scan-snyk exit non-zero when their tool is missing, and when +# the tool itself exits non-zero. Each used to end its `then` branch with +# a success echo, so the target passed either way. +# 2. security-scan no longer runs the deprecated tfsec target. +# 3. terraform-validate runs `terraform init` + `validate` in every flat +# terraform/environments/ root and fails when one fails. It used to +# loop over terraform/environments/*/dev, which does not exist. +# 4. Makefile.terraform's clean-locks refuses to delete the provider lock +# files without CONFIRM_CLEAN_LOCKS=yes. +# +# Tools are stubs on a PATH that holds nothing else, so the real tools are +# never run and nothing leaves the machine. Exits 0 when all cases pass; +# exits 1 on any failure. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" +MAKE_BIN="$(command -v make)" + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +pass=0 +fail=0 + +note_pass() { + echo "PASS: $1" + ((pass++)) || true +} + +note_fail() { + echo "FAIL: $1" + ((fail++)) || true +} + +# stub NAME EXIT_CODE: a PATH dir whose only command is NAME, logging its argv +# and working directory to $WORK/.log and exiting EXIT_CODE. +stub() { + local dir="$WORK/bin-$1-$2" + mkdir -p "$dir" + cat > "$dir/$1" <> "$WORK/$1.log" +exit $2 +EOF + chmod +x "$dir/$1" + rm -f "$WORK/$1.log" + echo "$dir" +} + +# run_make PATH_VALUE ARGS...: run make in the repo with PATH set to exactly +# PATH_VALUE. make and its /bin/sh are absolute paths, and the recipes under +# test use only shell builtins besides the tool, so nothing else is needed. +run_make() { + local path="$1" + shift + PATH="$path" "$MAKE_BIN" --no-print-directory -C "$REPO_ROOT" "$@" > "$WORK/out.log" 2>&1 +} + +expect_status() { + local want="$1" desc="$2" + shift 2 + local got=0 + "$@" || got=$? + if { [ "$want" = 0 ] && [ "$got" = 0 ]; } || { [ "$want" != 0 ] && [ "$got" != 0 ]; }; then + note_pass "$desc" + else + note_fail "$desc (want ${want/1/non-zero}, got $got)" + sed 's/^/ /' "$WORK/out.log" 2>/dev/null || true + fi +} + +mkdir -p "$WORK/empty" + +# 1. Scanner targets. +for pair in lint:golangci-lint security-scan-docker:trivy security-scan-terraform:tfsec security-scan-snyk:snyk; do + target="${pair%%:*}" + tool="${pair#*:}" + expect_status 1 "$target fails when $tool is missing" \ + run_make "$WORK/empty" "$target" + expect_status 1 "$target fails when $tool exits non-zero" \ + run_make "$(stub "$tool" 1)" "$target" + expect_status 0 "$target passes when $tool exits zero" \ + run_make "$(stub "$tool" 0)" "$target" + if [ -s "$WORK/$tool.log" ]; then + note_pass "$target runs $tool" + else + note_fail "$target did not run $tool" + fi +done + +# 2. No tfsec in the security-scan chain. +if PATH="$WORK/empty" "$MAKE_BIN" --no-print-directory -C "$REPO_ROOT" -n security-scan 2>&1 | grep -q tfsec; then + note_fail "security-scan still runs tfsec" +else + note_pass "security-scan does not run tfsec" +fi + +# 3. terraform-validate over the real environment roots. +roots=() +for d in "$REPO_ROOT"/terraform/environments/*/; do + [ -d "$d" ] && roots+=("${d%/}") +done +if [ "${#roots[@]}" -eq 0 ]; then + note_fail "no terraform/environments roots found to validate" +fi + +expect_status 0 "terraform-validate passes when terraform exits zero" \ + run_make "$(stub terraform 0)" terraform-validate +for root in "${roots[@]}"; do + if grep -qxF "$root init -backend=false -input=false" "$WORK/terraform.log" 2>/dev/null && + grep -qxF "$root validate" "$WORK/terraform.log" 2>/dev/null; then + note_pass "terraform-validate inits and validates ${root#"$REPO_ROOT"/}" + else + note_fail "terraform-validate skipped ${root#"$REPO_ROOT"/}" + sed 's/^/ /' "$WORK/terraform.log" 2>/dev/null || true + fi +done +expect_status 1 "terraform-validate fails when terraform exits non-zero" \ + run_make "$(stub terraform 1)" terraform-validate +expect_status 1 "terraform-validate fails when terraform is missing" \ + run_make "$WORK/empty" terraform-validate + +# 4. clean-locks, in a scratch tree so no real lock file is touched. +tree="$WORK/tree" +mkdir -p "$tree/terraform/environments/aws" +lock="$tree/terraform/environments/aws/.terraform.lock.hcl" +touch "$lock" +clean_locks() { + "$MAKE_BIN" --no-print-directory -C "$tree" -f "$REPO_ROOT/Makefile.terraform" clean-locks "$@" > "$WORK/out.log" 2>&1 +} +expect_status 1 "clean-locks refuses without CONFIRM_CLEAN_LOCKS=yes" clean_locks +if [ -f "$lock" ]; then + note_pass "clean-locks left the lock file in place when refused" +else + note_fail "clean-locks deleted the lock file without confirmation" +fi +expect_status 1 "clean-locks refuses CONFIRM_CLEAN_LOCKS=1" clean_locks CONFIRM_CLEAN_LOCKS=1 +expect_status 0 "clean-locks runs with CONFIRM_CLEAN_LOCKS=yes" clean_locks CONFIRM_CLEAN_LOCKS=yes +if [ ! -e "$lock" ]; then + note_pass "clean-locks deleted the lock file when confirmed" +else + note_fail "clean-locks kept the lock file despite confirmation" +fi + +echo "" +echo "Results: ${pass} passed, ${fail} failed" +[ "$fail" -eq 0 ]