From 1ac5cf3e6a699c789d4d990752e375ce6c60f85f Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 08:38:05 +0000 Subject: [PATCH 1/2] chore(ci): make local gates fail honestly and pin pre-commit hook repos First slice of the 2026-07-28 review findings for this repo: - Makefile: lint, security-scan-docker and security-scan-snyk fail when the tool is missing and stop on the tool's own exit status instead of a trailing echo; drop the retired tfsec target; terraform-validate validates the flat terraform/environments/ roots instead of a nonexistent */dev glob. scripts/test-make-gates.sh covers all of this with stub tools and runs in the lint job. - Makefile.terraform: clean-locks needs CONFIRM_CLEAN_LOCKS=yes. - database-migration.yml: stop writing migrate's stderr into GITHUB_ENV. - .pre-commit-config.yaml: pin the three third-party hook repos to commit SHAs and let check-yaml see workflow files. - .golangci.yml: gocyclo min-complexity 10, matching the CI gate. - ci.yml: tfvars fmt checks no longer swallow a non-zero exit. - .env.example: no working admin password or API key. - .dockerignore: exclude the test compose/Dockerfile pair. - .trivyignore/.snyk: correct the AVD-AWS-0013 and GCP-0015 reasons and drop .snyk keys the Snyk CLI never reads. Refs #349 Co-Authored-By: Claude Opus 5.5 --- .dockerignore | 5 +- .env.example | 7 +- .github/workflows/ci.yml | 9 +- .github/workflows/database-migration.yml | 6 +- .golangci.yml | 4 +- .pre-commit-config.yaml | 16 ++- .snyk | 23 +--- .trivyignore | 16 ++- Makefile | 65 +++++----- Makefile.terraform | 11 +- docs/DEVELOPMENT.md | 3 +- scripts/test-make-gates.sh | 155 +++++++++++++++++++++++ 12 files changed, 244 insertions(+), 76 deletions(-) create mode 100755 scripts/test-make-gates.sh 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..c3d0a3adf 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: build clean test deploy help all build-server build-lambda test-unit test-integration test-compile-integration \ test-coverage full-test security-scan terraform-validate docker-build \ fmt vet lint complexity complexity-report security-scan-go security-scan-docker \ - security-scan-terraform terraform-fmt terraform-fmt-check iac-arm docker-test pre-commit \ + terraform-fmt terraform-fmt-check iac-arm docker-test pre-commit \ setup-git-secrets security-scan-snyk security-scan-all ci docker-compose-test \ install-dev-tools tidy-check @@ -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,37 @@ 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. There is no tfsec target: 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 - -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 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" -# 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 +227,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 +258,6 @@ 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 " - 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..5a034e5f0 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -317,8 +317,7 @@ 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) ``` ### Terraform & Docker Testing diff --git a/scripts/test-make-gates.sh b/scripts/test-make-gates.sh new file mode 100755 index 000000000..903d05dbb --- /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 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 depends on the retired 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-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 ] From cfbb5511a31bf994b42222619d9d28165a8fb7ee Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 08:38:42 +0000 Subject: [PATCH 2/2] chore(make): keep the deprecated tfsec target, out of security-scan Deleting it breaks #1222's gitignore test, which expects the Makefile to still write tfsec-report.json. The target now fails honestly like the others; removing it can follow once #1222 merges. Refs #349 Co-Authored-By: Claude Opus 5.5 --- Makefile | 14 +++++++++++--- docs/DEVELOPMENT.md | 1 + scripts/test-make-gates.sh | 12 ++++++------ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index c3d0a3adf..e4c0eb1e3 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: build clean test deploy help all build-server build-lambda test-unit test-integration test-compile-integration \ test-coverage full-test security-scan terraform-validate docker-build \ fmt vet lint complexity complexity-report security-scan-go security-scan-docker \ - terraform-fmt terraform-fmt-check iac-arm docker-test pre-commit \ + security-scan-terraform terraform-fmt terraform-fmt-check iac-arm docker-test pre-commit \ setup-git-secrets security-scan-snyk security-scan-all ci docker-compose-test \ install-dev-tools tidy-check @@ -156,8 +156,8 @@ complexity-report: fi # Security scanning. Terraform is covered by trivy's misconfig scanner in -# security-scan-docker. There is no tfsec target: CI replaced tfsec with Trivy, -# and tfsec is deprecated upstream (#349). +# 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 @@ -173,6 +173,13 @@ security-scan-docker: 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..." + @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. Each terraform/environments/ is one flat root # (no per-profile subdirectories), the same roots CI's terraform-validate job # validates. @@ -258,6 +265,7 @@ install-dev-tools: @echo "" @echo "Additional tools to install manually:" @echo " - trivy: https://aquasecurity.github.io/trivy/" + @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/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 5a034e5f0..a588f4fe3 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -318,6 +318,7 @@ make security-scan # Individual scans make security-scan-go # gosec 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 index 903d05dbb..24b466488 100755 --- a/scripts/test-make-gates.sh +++ b/scripts/test-make-gates.sh @@ -3,11 +3,11 @@ # # Asserts that the Makefile's local gates fail when they should (issue #349): # -# 1. lint, security-scan-docker 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 depends on the retired tfsec target. +# 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. @@ -80,7 +80,7 @@ expect_status() { mkdir -p "$WORK/empty" # 1. Scanner targets. -for pair in lint:golangci-lint security-scan-docker:trivy security-scan-snyk:snyk; do +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" \