[IAmJSD]

First slice of #349, the medium/low findings from the 2026-07-28 review (platform part). I re-checked every finding against current main (a14dc8f2). This PR fixes the ones that can be verified locally and don't collide with other open branches. The table below covers all of them.

Refs #349

Root cause and fix

Several local gates reported success no matter what happened. Each Makefile scanner target ended its then branch with a success echo and printed a hint instead of failing when the tool was missing. terraform-validate looped over a terraform/environments/*/dev glob that matches nothing. A few config files also kept settings that nothing reads or that undercut the gate next to them.

• Makefile: lint, security-scan-docker and security-scan-snyk now fail when their tool is missing, and the tool runs as its own recipe line so make stops on its exit status. security-scan no longer runs the deprecated tfsec target (CI replaced tfsec with Trivy). The security-scan-terraform target is kept, with the same honest shape, because chore(gitignore): ignore root artifacts written by Make targets #1222's gitignore test expects the Makefile to still write tfsec-report.json. It can be deleted once chore(gitignore): ignore root artifacts written by Make targets #1222 merges. terraform-validate now runs init -backend=false + validate in each flat terraform/environments/<cloud> root (the same roots CI validates) and fails if it finds none. security-scan-docker now uses --scanners vuln,misconfig instead of the deprecated --security-checks vuln,config. It still reports Trivy findings without failing on them, the same as CI's Trivy fs step (see the deferred items).
• Makefile.terraform: clean-locks refuses to run without CONFIRM_CLEAN_LOCKS=yes and warns that the regenerated lock files must be reviewed as a supply-chain change.
• scripts/test-make-gates.sh (new, 28 cases) runs all of the above against stub tools on a PATH that contains nothing else. It runs as a step in ci.yml's existing lint job, so ci-success's needs: list is unchanged.
• database-migration.yml: "Get migration version" no longer writes MIGRATION_VERSION to $GITHUB_ENV. Nothing read the value. migrate prints its version on stderr, so the 2>&1 capture stays for the log line, but on failure that same capture holds a multi-line error. Written to $GITHUB_ENV, an error line shaped like NAME=value would have set that variable for every later step.
• .pre-commit-config.yaml: the three third-party hook repos are pinned to commit SHAs (# frozen: <tag>), each confirmed through the GitHub API as the commit its tag points at. check-yaml no longer excludes .github/workflows/.
• .golangci.yml: gocyclo min-complexity: 10, the same threshold as the standalone gocyclo -over 10 gates.
• ci.yml: the per-file tfvars terraform fmt -check calls no longer end in || echo "Note: ...".
• .env.example: ADMIN_PASSWORD_DEV and ADMIN_API_KEY_DEV are now empty. EnvResolver.GetSecret rejects an empty value, so a copy that skips them fails at startup instead of booting with published credentials.
• .dockerignore: excludes docker-compose.test.yml and Dockerfile.test. docker build -f still uses an excluded Dockerfile, the same way Dockerfile.dev already works.
• .trivyignore / .snyk: the AVD-AWS-0013 reason is rewritten to the real one: TLSv1 is set explicitly, and the finding is unexploitable only because enable_cdn defaults to false and every shipped tfvars sets it false. The GCP-0015 reason is corrected: there is no require_ssl variable, and both blocks hardcode ssl_mode = "ENCRYPTED_ONLY". .snyk drops failOnSeverity and license, which the Snyk CLI never reads, with a comment saying where the threshold actually lives.

Per-finding status (re-verified on main a14dc8f2)

#	Finding	Status
M1	AWS Terraform IaC ungated (pre-commit --skip-dirs terraform/environments/aws, CI Trivy IaC exit-code 0)	Deferred. Still reproduces. Trivy isn't installed on the verifying machine, so I couldn't show that dropping the skip and adding exit-code: 1 leaves main green. The pre-commit job also installs Trivy v0.69.3, which panics on that directory (fixed in v0.72.0, which ci.yml already pins), so the fix also needs a pre-commit.yml Trivy bump. #1227 edits pre-commit.yml (different lines). Next slice.
M2	Makefile security/lint targets pass silently	Fixed here. security-scan-go was already fixed by #340 (scripts/run-gosec.sh).
M3	make ci / make terraform-validate broken (*/dev glob)	Fixed here.
M4	Snyk continue-on-error and outside ci-success	Deferred, needs a maintainer decision. Still reproduces. snyk-scan is skipped on fork PRs (if: ... head.repo.full_name == github.repository), and ci-success now accepts only success, so adding it to needs: as is would turn every fork PR red. The fix also edits ci-success's needs: block, which #1222, #1119 and fork branch fix/issue-59 all append to.
M5	Trivy fs scan and the 80% coverage line are advisory	Deferred, needs a maintainer decision. Still reproduces. exit-code: 1 on Trivy fs would turn main red whenever a new upstream CVE is published, and I couldn't measure what it reports today. Whether the coverage line is a gate is the owner's call.
M6	docker-build smoke test rebuilds and uses || true	Already fixed by #1139 (039698da): the image is built once with load: true, and the architecture/execution check uses no || true.
M7	Get migration version writes command output to $GITHUB_ENV	Fixed here.
M8	No Dependabot/Renovate, no CODEOWNERS	Deferred, needs a maintainer decision (see "For the maintainer").
M9	Terraform version pinned to different values	Partly fixed already: f1aceec9 moved rollback.yml to 1.10.5, and database-migration.yml is 1.10.5 too. Still reproduces: deploy-aws-lambda, deploy-aws-fargate, deploy-azure, deploy-gcp, cleanup-staging and destroy-fargate-dev pin TF_VERSION: '1.10.0', while ci.yml and pre-commit.yml pin 1.10.5. None of the open branches touch those lines. Next slice (bump to 1.10.5 plus a pin-consistency check).
L1	.env.example ships a working admin password/API key	Fixed here. docker-compose.yml still carries the same two values inline for the local dev stack. That file isn't in the finding, so I left it alone.
L2	deploy-all.yml ignores cancelled children	Already fixed by #1127 (bb2d2312): it now allowlists on success, and cancelled fails the run.
L3	.dockerignore omits the test pair	Fixed here.
L4	clean-locks without confirmation	Fixed here.
S1	Pre-commit hook repos pinned to mutable tags	Fixed here.
S2	check-yaml excludes workflows; no actionlint	Fixed here. The actionlint half was already done by #1870 (a43a575b, local hook plus the workflow-lint job). This PR removes the exclusion.
S3	.golangci.yml gocyclo 15 vs 10	Fixed here (.golangci.yml only). The Makefile complexity half is A14-037, below.
A13-013	AVD-AWS-0013 suppression gives the wrong reason	Fixed here (comment only). The code side (TLSv1 branch) is tracked separately, as the issue says.
A14-034	scripts/tf-deploy.sh builds a nonexistent per-profile root	Deferred. Fork branch fix/issue-59 rewires Makefile.terraform through tf-deploy.sh and adds scripts/test-tf-deploy.sh for it. Changing the script's directory logic now would collide with that work.
A13-016	GCP-0015 reason stale; .snyk inert keys	Fixed here (comments and .snyk). The GCP-0015 entry itself is kept until a Trivy run without it is shown to be clean.
A13-021	nginx:alpine / dpage/pgadmin4:9.2 not digest-pinned	Deferred to the next slice. No conflicts. It means choosing an nginx version and resolving digests from Docker Hub.
A14-037	Makefile complexity lacks -ignore "_test\.go"; 2>&1 || true	Deferred. Fork branch fix/issue-177 rewrites that same Makefile line (and the ci.yml gocyclo line).
A14-041	tfvars fmt -check ... || echo can't fail	Fixed here. On current main the failure scenario doesn't actually happen: the earlier "Terraform Format Check" step (terraform fmt -check -recursive terraform/) already fails on a broken tfvars file (exit 2) or an unformatted one (exit 3), both checked locally. The soft-fail was misleading, though, and is gone.

Branch overlap check

Every .github/workflows/ci.yml, pre-commit and Makefile hunk in #1222, #1227, #1119 and #1142, in fork branches fix/issue-1233, -353, -177 and -59, and in every other open PR touching CI files (#1086, #1090, #1106, #1112, #1114, #1144, #1147, #1149, #1193, #1203, #1207, #1213, #1221, #1228, #1236) was read. git merge-tree --write-tree against each of those heads is clean. On the combined trees, the sibling self-tests that read the files touched here pass: #1222's test-gitignore-make-artifacts.sh (15/15, after keeping the tfsec target, see above), #1227's check-gosec-pin.sh and test-check-gosec-pin.sh (43/43), and fix/issue-59's test-tf-deploy.sh (14/14). test-make-gates.sh passes on each combined tree too.

Verification

All of this is local, stub- or fixture-based evidence. Nothing calls a cloud API, and no workflow was triggered.

• bash scripts/test-make-gates.sh: 28 passed, 0 failed on this branch. The same script run against origin/main's Makefile/Makefile.terraform (via git archive origin/main; the run predates the tfsec-target case, so it had 24 cases) gave 11 passed, 13 failed: lint/trivy/snyk passed with the tool missing, trivy/snyk passed when the tool failed, security-scan ran tfsec, terraform-validate failed with exit 2 on the */dev glob and never called terraform, and clean-locks deleted the lock file without confirmation.
• Real make terraform-validate (actual terraform init -backend=false + validate, providers from the public registry, no backend, no credentials; TF_DATA_DIR in a temp dir): Success! The configuration is valid. for aws, azure and gcp, exit 0. Side effect: init adds platform h1: hashes to the committed .terraform.lock.hcl files (I reverted them). The committed locks also record stale constraints (for example http ~> 3.0 while the code says ~> 3.4). That's out of scope, noted below.
• $GITHUB_ENV injection: I extracted the "Get migration version" run: block from both revisions and ran it with a stub migrate that writes a two-line error to stderr and exits 1. On main, $GITHUB_ENV ended up containing MIGRATION_VERSION=error: ..., then INJECTED_VAR=attacker-value, then unknown. On this branch it stays empty. migrate printing its version on stderr was confirmed in golang-migrate's internal/cli (versionCmd → log.Println → os.Stderr).
• check-yaml: I wrote a malformed workflow fixture to .github/workflows/ (temporary, deleted after). pre-commit run check-yaml --files <fixture> printed (no files to check)Skipped with main's config and failed with a YAML parse error with this branch's. pre-commit run check-yaml --all-files passes on the branch, so every existing workflow parses.
• gocyclo threshold: a fixture function with complexity 11 (gocyclo -over 10 reports it) gave 0 issues with main's .golangci.yml and 1 issues: gocyclo with this branch's. golangci-lint run --enable-only gocyclo --build-tags integration ./... gives 0 issues on the whole repo (tests/e2e with --build-tags e2e too), and gocyclo -over 10 -ignore "_test\.go|node_modules" . reports nothing. golangci-lint config verify against the vendored v2.10 schema passes.
• docker build -f Dockerfile.test . with the new .dockerignore: exit 0.
• Hook SHAs: git ls-remote plus gh api repos/<repo>/commits/<sha> for each (chore(release): version 1.105.0, v6.0.0, Bump version 0.47.0).
• actionlint .github/workflows/ci.yml .github/workflows/database-migration.yml (v1.7.12): clean. shellcheck scripts/test-make-gates.sh (0.11.0): clean. zizmor isn't installed here, so it didn't run. git diff --check: clean.
• go build ./..., go vet ./..., go vet -tags integration ./...: pass. No Go files changed.
• Not run locally: Trivy, tfsec and Snyk themselves (not installed). The Makefile changes are covered by the stub tests only.

Combined-tree verification. This branch was tested together with the other queued branches on a local, never-pushed tree: queue/platform @ b56afb2b = origin/main a14dc8f2 plus these heads, merged in this order:

Branch	Tested head
fix/issue-204	76f85a4
fix/issue-438	78fe011
fix/issue-838	120b07f
fix/issue-143	aef1256
fix/issue-349	cfbb551
fix/issue-80	63405f9
fix/issue-199	36b9f8f
fix/issue-41	96fc237
fix/issue-299	6af14ed
fix/issue-324	3ab6729

fix/issue-324 and fix/issue-204 both edit .github/workflows/ci.yml next to the azure-kv-access-policy job. The tree kept #324's new "Run Function URL reader self-tests" step and #204's updated job comment; whichever PR merges second takes that same resolution.

Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers. The full suite ran on ed0c7174, which differs from b56afb2b only by one .gitallowed line on fix/issue-324:

• Passed: go build ./..., go vet ./..., go vet -tags integration ./... and the full go test -race -short -count=1 ./....
• Static checks: golangci-lint v2.10.1 found 0 issues on every changed package, with and without --build-tags integration. gocyclo -over 10 is silent on the changed non-test files.
• Integration tests, every changed package that has them, one package at a time (-tags integration -count=1, 60-minute local timeout): internal/api, internal/purchase, internal/scheduler and internal/server all pass.
• Secrets: the first run caught a bare placeholder account ID in fix/issue-324's new tftest, now allowlisted with an anchored .gitallowed line. Re-run at b56afb2b: git-secrets, using the repo's scripts/setup-git-secrets.sh patterns in a fresh repo with an isolated HOME, found nothing in the changed files, and a planted fake key was flagged. scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.
• Nothing was applied to any cloud.

Out of scope / follow-ups

• The deferred items in the table: M1, M4, M5, M8 (owner decisions, or tools not available here), M9 and A13-021 (next slice), A14-034 (after fix/issue-59), A14-037 (after fix/issue-177), and deleting the deprecated tfsec target (after chore(gitignore): ignore root artifacts written by Make targets #1222).
• The committed terraform/environments/*/.terraform.lock.hcl files record provider constraints that no longer match the code (for example http ~> 3.0 vs ~> 3.4), so any terraform init rewrites them.
• docker-compose.yml still carries the dev admin password and API key inline.

For the maintainer (repo-admin actions, not done here)

• Required checks: nothing new is needed for this PR. test-make-gates.sh runs inside the existing Lint Code job, which is already in ci-success. If Snyk becomes a gate (M4), either the job has to run on fork PRs (it needs SNYK_TOKEN, which forks don't get), or ci-success has to accept skipped for it specifically. That's a policy choice, plus a ruleset update if you want "Snyk Security Scan" to be required directly.
• CODEOWNERS (M8): only matters once the branch ruleset turns on "Require review from Code Owners". The file needs the owning logins/teams for .github/workflows/** and terraform/**.
• Dependabot (M8): a .github/dependabot.yml for github-actions, docker and gomod (and npm for frontend/ if the UI owner wants it) opens PRs against this repo once merged. Your call on whether you want that PR volume and how to group updates.

Could a maintainer please mirror #349's triage labels onto this PR: impact/internal, effort/l, type/chore, triaged, priority/p3, severity/low, urgency/eventually.

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 9de209f55d372b28c1805958e5101c219c620654

Blocking:

• none

Non-blocking:

• .golangci.yml:45: gocyclo is now at 10 with run.tests: true. That is safe only because the existing exclusions.rules entry drops gocyclo for _test\.go, the same carve-out as CI's gocyclo -over 10 -ignore "_test\.go". If anyone later trims that exclusion list, make lint and the CI lint job would start failing on table-driven tests. A one-line comment next to the threshold pointing at the exclusion would make the coupling visible.
• .env.example:71 tells the reader to "Set your own values in .env.local", and the header says the file is "Loaded by: load-env.sh". No load-env.sh exists in the repo, and nothing loads .env.local. This is pre-existing, but the new comment leans on it. docker-compose.yml:54/60 still ships the same dev password and API key inline, as the body notes.

Attacked:

• Makefile gates: read each recipe. A missing tool exits 1 before the tool line runs, and the tool runs as its own recipe line so make stops on its exit status. terraform-validate now walks the three flat terraform/environments/<cloud>/ roots and fails when it finds none. security-scan no longer pulls in tfsec. clean-locks refuses anything but CONFIRM_CLEAN_LOCKS=yes.
• Fail-before: copied origin/main's Makefile, Makefile.terraform and terraform/environments (via git archive) into a scratch dir and ran this PR's scripts/test-make-gates.sh against them. Result: 13 passed, 15 failed. The failures: missing or failing tools passed, security-scan still runs tfsec, terraform-validate exited 2 and skipped aws/azure/gcp, and clean-locks deleted the lock without confirmation. On the branch: 28 passed, 0 failed.
• Workflow and config changes, each verified directly:
  • nothing reads MIGRATION_VERSION;
  • every committed terraform/environments/**/*.tfvars passes terraform fmt -check, so dropping || echo doesn't turn CI red;
  • the three frozen pre-commit SHAs match git ls-remote for v1.105.0, v6.0.0 and v0.47.0;
  • pre-commit run check-yaml --all-files passes now that .github/workflows/ is included;
  • .trivyignore reasons match the code: TLSv1 is set in the no-ACM branch, enable_cdn defaults to false and is false in all three github-*.tfvars, and the GCP module hardcodes ssl_mode = "ENCRYPTED_ONLY" in both blocks;
  • EnvResolver.GetSecret rejects empty values, so the blanked .env.example really does fail fast at boot.
• Ran: golangci-lint run --enable-only gocyclo ./... with the new threshold (0 issues); golangci-lint config verify (ok); actionlint on ci.yml and database-migration.yml (clean); shellcheck scripts/test-make-gates.sh (clean). No Go code changed.

Body-stale:

• none (the fail-before count in the body, 11/13 over 24 cases, predates the four tfsec-target cases; my rerun over all 28 gave 13/15, which is consistent)

_{Reviewer: rB6. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at 9de209f55d372b28c1805958e5101c219c620654 merged onto main (5aa3df5b), together with #1155, #1219, #1221, #1223, #1224, #1225, #1226, #1227, #1256, #1262, #1266, #1270, #1272, #1275, #1283, #1286, #1293, #1297, #1299, #1300, #1301, #1304, #1307, #1311, #1313, #1325 in that order (combined tree f3a3331f, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on 9de209f5, green CI, and the combined-tree verification above. Squash-merged at exactly 9de209f55d372b28c1805958e5101c219c620654.