From ade9673f02197ca8aa877a756d5c959ac6a3c21c Mon Sep 17 00:00:00 2001
From: Patrick Plate <patrick@plate-software.de>
Date: Fri, 19 Jun 2026 16:04:09 +0200
Subject: [PATCH] fix: harden CI security gates, parallelize builds,
 externalize secrets

- Make OWASP, Gitleaks, pnpm audit blocking (remove || true fallbacks)
- Add Maven -T 1C for parallel reactor threads
- Fix parallel Docker build race condition (PID tracking + set -euo pipefail)
- Externalize JWT/NextAuth secrets via env vars with dev-only defaults
- Add .env.example with generation instructions
- Add CI/CD infrastructure review document
---
 .env.example               |  15 +++
 .gitea/workflows/ci.yml    |  30 +++--
 docker-compose.truenas.yml |   2 +-
 docker-compose.yml         |   6 +-
 docs/ci-cd-review.md       | 257 +++++++++++++++++++++++++++++++++++++
 5 files changed, 292 insertions(+), 18 deletions(-)
 create mode 100644 .env.example
 create mode 100644 docs/ci-cd-review.md

diff --git a/.env.example b/.env.example
new file mode 100644
index 0000000..f86133b
--- /dev/null
+++ b/.env.example
@@ -0,0 +1,15 @@
+# CannaManage — Environment Variables
+# Copy this file to .env and fill in the values.
+# NEVER commit .env to git.
+
+# Database
+DB_PASSWORD=cannamanage_dev
+
+# JWT Secret — must be valid base64 (used by Decoders.BASE64.decode in JwtService)
+# Generate with: openssl rand -base64 48
+JWT_SECRET=
+
+# NextAuth / Auth.js secret — minimum 32 characters
+# Generate with: openssl rand -base64 32
+NEXTAUTH_SECRET=
+AUTH_SECRET=
diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml
index bc67847..4243175 100644
--- a/.gitea/workflows/ci.yml
+++ b/.gitea/workflows/ci.yml
@@ -30,10 +30,10 @@ jobs:
           cache: maven
 
       - name: Maven compile
-        run: ./mvnw compile -B -q -DskipTests
+        run: ./mvnw compile -B -q -DskipTests -T 1C
 
       - name: Maven test
-        run: ./mvnw test -B
+        run: ./mvnw test -B -T 1C
 
       - name: OWASP Dependency-Check (SCA)
         run: |
@@ -41,9 +41,9 @@ jobs:
             -DfailBuildOnCVSS=7 \
             -DsuppressionFile=.snyk-maven-suppressions.xml \
             -Dformats=JSON,HTML \
-            -B -q || true
-        # Note: failBuildOnCVSS=7 means High/Critical CVEs fail the build.
-        # Medium and below produce warnings only.
+            -B -q
+        # failBuildOnCVSS=7: High/Critical CVEs fail the build.
+        # Suppress known false positives in .snyk-maven-suppressions.xml.
 
       - name: Upload dependency-check report
         if: always()
@@ -80,8 +80,8 @@ jobs:
       - name: pnpm audit (SCA)
         run: |
           cd cannamanage-frontend
-          pnpm audit --audit-level=high || echo "::warning::pnpm audit found vulnerabilities"
-        # Fails on High/Critical. Warnings for Medium/Low.
+          pnpm audit --audit-level=high
+        # Fails on High/Critical. Use .pnpmauditrc or --ignore for known exceptions.
 
   # ─────────────────────────────────────────────────────────────────────────────
   # Docker image security scan (Trivy)
@@ -92,11 +92,14 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
-      - name: Build backend image
-        run: docker build -t cannamanage-backend:scan -f Dockerfile.backend .
-
-      - name: Build frontend image
-        run: docker build -t cannamanage-frontend:scan -f cannamanage-frontend/Dockerfile cannamanage-frontend/
+      - name: Build images (parallel)
+        run: |
+          set -euo pipefail
+          docker build -t cannamanage-backend:scan -f Dockerfile.backend . &
+          PID1=$!
+          docker build -t cannamanage-frontend:scan -f cannamanage-frontend/Dockerfile cannamanage-frontend/ &
+          PID2=$!
+          wait $PID1 $PID2
 
       - name: Install Trivy
         run: |
@@ -164,8 +167,7 @@ jobs:
             --source . \
             --report-format json \
             --report-path gitleaks-report.json \
-            --exit-code 1 \
-            || echo "::error::Gitleaks found potential secrets in the repository"
+            --exit-code 1
 
       - name: Upload Gitleaks report
         if: always()
diff --git a/docker-compose.truenas.yml b/docker-compose.truenas.yml
index 90356e0..fe681f7 100644
--- a/docker-compose.truenas.yml
+++ b/docker-compose.truenas.yml
@@ -16,5 +16,5 @@ services:
       AUTH_URL: http://192.168.188.119:3000
       # NextAuth v5 (Auth.js) reads AUTH_SECRET, not NEXTAUTH_SECRET. Without it at
       # runtime, signIn throws MissingSecret -> the app error boundary shows 'Oops'.
-      AUTH_SECRET: docker-dev-nextauth-secret-minimum-32chars
+      AUTH_SECRET: ${AUTH_SECRET}
       AUTH_TRUST_HOST: "true"
diff --git a/docker-compose.yml b/docker-compose.yml
index 9466c4e..6514b14 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -27,11 +27,11 @@ services:
       SPRING_PROFILES_ACTIVE: docker
       SPRING_DATASOURCE_URL: jdbc:postgresql://db:5432/cannamanage
       SPRING_DATASOURCE_USERNAME: cannamanage
-      SPRING_DATASOURCE_PASSWORD: cannamanage_dev
+      SPRING_DATASOURCE_PASSWORD: ${DB_PASSWORD:-cannamanage_dev}
       # JwtService base64-decodes this secret (Decoders.BASE64.decode) before using it as the
       # HMAC-SHA key. It MUST be valid base64 — a plaintext string with hyphens throws
       # "Illegal base64 character: '-'" at token-signing time (HTTP 500 after a successful login).
-      CANNAMANAGE_SECURITY_JWT_SECRET: hmSULRhmFYcOXDwYxb7bGXp7Bovh+hXgua/VqF44Ts/N+8YELWpWiqQ+aLrymCuM
+      CANNAMANAGE_SECURITY_JWT_SECRET: ${JWT_SECRET:-dGhpcy1pcy1hLWRldi1vbmx5LXNlY3JldC1kby1ub3QtdXNlLWluLXByb2R1Y3Rpb24=}
     depends_on:
       db:
         condition: service_healthy
@@ -51,7 +51,7 @@ services:
       - "3000:3000"
     environment:
       NEXTAUTH_URL: http://localhost:3000
-      NEXTAUTH_SECRET: docker-dev-nextauth-secret-minimum-32chars
+      NEXTAUTH_SECRET: ${NEXTAUTH_SECRET:-dev-only-nextauth-secret-do-not-use-in-production-min32}
       BACKEND_URL: http://backend:8080
       AUTH_URL: http://localhost:3000
     depends_on:
diff --git a/docs/ci-cd-review.md b/docs/ci-cd-review.md
new file mode 100644
index 0000000..373607c
--- /dev/null
+++ b/docs/ci-cd-review.md
@@ -0,0 +1,257 @@
+# CI/CD Infrastructure Review — CannaManage
+
+**Date:** 2026-06-19
+**Reviewer:** Lumen (Code + Security)
+**Scope:** `.gitea/workflows/ci.yml`, `.gitea/workflows/deploy.yml`, `Dockerfile.backend`, `docker-compose*.yml`, `.snyk`
+**Verdict:** ⚠️ Approved with findings — 2 High, 4 Medium, 3 Low
+
+---
+
+## 1. Architecture Overview
+
+```
+Push to main
+  ├── CI Pipeline (ci.yml)
+  │   ├── backend (compile + test + OWASP SCA)     ─┐
+  │   ├── frontend (lint + type-check + pnpm audit) ─┼── parallel
+  │   ├── secrets-scan (Gitleaks)                   ─┘
+  │   └── image-scan (Trivy) ── needs: [backend, frontend]
+  │
+  └── Deploy Pipeline (deploy.yml)
+      └── docker compose build + up + healthcheck
+```
+
+**Good:** CI jobs are already parallelized. `concurrency` prevents redundant runs. `cancel-in-progress: true` on CI is correct (abort stale runs). `cancel-in-progress: false` on deploy prevents interrupted deployments.
+
+---
+
+## 2. Security Findings
+
+### ❌ HIGH — S-01: Hardcoded JWT Secret in docker-compose.yml
+
+**File:** `docker-compose.yml:34`
+```yaml
+CANNAMANAGE_SECURITY_JWT_SECRET: hmSULRhmFYcOXDwYxb7bGXp7Bovh+hXgua/VqF44Ts/N+8YELWpWiqQ+aLrymCuM
+```
+
+**Risk:** This base64-encoded HMAC key is committed to git. Anyone with repo access can forge valid JWTs. If this key is the same as production, it's a full auth bypass.
+
+**Mitigation:**
+- Replace with `${JWT_SECRET}` and use `.env` file (gitignored) for local dev
+- Alternatively, generate a random key at container startup for dev-only:
+  ```yaml
+  CANNAMANAGE_SECURITY_JWT_SECRET: ${JWT_SECRET:-$(openssl rand -base64 48)}
+  ```
+- Verify that production (`docker-compose.prod.yml`) uses env vars ✅ — it does use `${JWT_SECRET}`
+
+**Accepted Risk?** If this is intentionally a dev-only secret different from production, document it in `.snyk` and add a comment. Gitleaks *should* flag this — check if it's being suppressed.
+
+---
+
+### ❌ HIGH — S-02: Hardcoded NextAuth Secret in docker-compose.yml
+
+**File:** `docker-compose.yml:54` and `docker-compose.truenas.yml:19`
+```yaml
+NEXTAUTH_SECRET: docker-dev-nextauth-secret-minimum-32chars
+AUTH_SECRET: docker-dev-nextauth-secret-minimum-32chars
+```
+
+**Risk:** Same as S-01. If this secret is reused on the TrueNAS deployment (which is internet-accessible via `cannamanage.plate-software.de`), session cookies can be forged.
+
+**Mitigation:**
+- `docker-compose.truenas.yml` should reference `${AUTH_SECRET}` from an env file
+- The word "dev" in the value suggests it's dev-only, but TrueNAS *is* the production host per `deploy.yml`
+
+---
+
+### ⚠️ MEDIUM — S-03: OWASP Dependency-Check Always Passes (|| true)
+
+**File:** `ci.yml:44`
+```yaml
+run: |
+  ./mvnw org.owasp:dependency-check-maven:check \
+    ... || true
+```
+
+**Risk:** The `|| true` means the step NEVER fails the build, regardless of CVSS score. The `failBuildOnCVSS=7` flag is effectively useless.
+
+**Mitigation:** Remove `|| true`. If you need to allow known issues, use the suppression file that's already configured (`-DsuppressionFile=.snyk-maven-suppressions.xml`).
+
+---
+
+### ⚠️ MEDIUM — S-04: Gitleaks Non-Blocking (echo instead of exit)
+
+**File:** `ci.yml:168`
+```yaml
+gitleaks detect ... --exit-code 1 \
+  || echo "::error::Gitleaks found potential secrets in the repository"
+```
+
+**Risk:** `--exit-code 1` should fail the step, but the `||` fallback to `echo` means the step always succeeds. Secrets can be pushed without blocking the pipeline.
+
+**Mitigation:** Remove the `|| echo` fallback. Let Gitleaks fail the build:
+```yaml
+run: gitleaks detect --source . --report-format json --report-path gitleaks-report.json --exit-code 1
+```
+
+---
+
+### ⚠️ MEDIUM — S-05: pnpm audit Non-Blocking
+
+**File:** `ci.yml:83`
+```yaml
+pnpm audit --audit-level=high || echo "::warning::pnpm audit found vulnerabilities"
+```
+
+**Risk:** High/Critical frontend CVEs produce a warning but don't fail the build.
+
+**Mitigation:** Remove `|| echo ...` to make High/Critical CVEs block the pipeline. Use `.snyk` or `pnpm audit --ignore-path` for documented exceptions.
+
+---
+
+### ⚠️ MEDIUM — S-06: Parallel Docker Build Race Condition
+
+**File:** `ci.yml:96-99`
+```yaml
+docker build -t cannamanage-backend:scan -f Dockerfile.backend . &
+docker build -t cannamanage-frontend:scan -f cannamanage-frontend/Dockerfile cannamanage-frontend/ &
+wait
+```
+
+**Risk:** If either `docker build` fails, `wait` returns the exit code of the *last* process that exited, not the first failure. One image could fail silently while the other succeeds.
+
+**Mitigation:** Use `set -euo pipefail` and capture PIDs:
+```yaml
+run: |
+  set -euo pipefail
+  docker build -t cannamanage-backend:scan -f Dockerfile.backend . &
+  PID1=$!
+  docker build -t cannamanage-frontend:scan -f cannamanage-frontend/Dockerfile cannamanage-frontend/ &
+  PID2=$!
+  wait $PID1 $PID2
+```
+With `wait $PID1 $PID2`, if either fails, the step fails.
+
+---
+
+### ℹ️ LOW — S-07: Trivy Uses curl|sh Install Pattern
+
+**File:** `ci.yml:103`
+```yaml
+curl -sfL https://raw.githubusercontent.com/aquasecurity/trivy/main/contrib/install.sh | sh -s -- -b /usr/local/bin
+```
+
+**Risk:** Supply-chain risk — if the upstream script is compromised, arbitrary code runs in CI. Using `main` branch means no version pinning.
+
+**Mitigation:** Pin to a specific version tag or commit hash, or use the official Trivy GitHub Action:
+```yaml
+- uses: aquasecurity/trivy-action@v0.28.0
+  with:
+    image-ref: cannamanage-backend:scan
+    severity: HIGH,CRITICAL
+    exit-code: 1
+```
+
+---
+
+### ℹ️ LOW — S-08: Deploy Workflow Uses Hardcoded IP
+
+**File:** `deploy.yml:60, 74`
+```yaml
+wget -q -O /dev/null http://192.168.188.119:8081/actuator/health
+```
+
+**Risk:** Low — private IP only reachable on LAN. But if the TrueNAS IP changes (DHCP), deploy silently breaks.
+
+**Mitigation:** Use `localhost` or a DNS name (e.g., `truenas.local`) instead. Or use container-internal checks via `docker compose exec`.
+
+---
+
+### ℹ️ LOW — S-09: No Image Signing or SBOM
+
+**Risk:** Docker images are built and deployed without signatures or Software Bill of Materials. Can't verify image integrity post-deployment.
+
+**Mitigation (future):** Add `docker sbom` or `syft` for SBOM generation. Consider `cosign` for image signing when maturing to multi-env deployments.
+
+---
+
+## 3. Code Quality Findings
+
+### CI Workflow (ci.yml)
+
+| # | Check | Status | Notes |
+|---|-------|--------|-------|
+| 1 | Jobs parallelized | ✅ | backend, frontend, secrets-scan run concurrently |
+| 2 | Concurrency control | ✅ | Group + cancel-in-progress |
+| 3 | Maven caching | ✅ | `cache: maven` in setup-java |
+| 4 | Maven `-T 1C` | ✅ | Multi-threaded reactor (just added) |
+| 5 | Surefire forkCount | ✅ | `forkCount=2` in parent POM |
+| 6 | Artifact upload | ✅ | Reports preserved on failure (`if: always()`) |
+| 7 | Frontend lockfile | ✅ | `--frozen-lockfile` prevents silent dep changes |
+| 8 | Trivy ignore-unfixed | ✅ | Only actionable vulns fail the build |
+| 9 | Security gates blocking | ❌ | All 3 security tools use `|| true/echo` fallbacks |
+| 10 | Docker parallel build | ⚠️ | Race condition on failure (see S-06) |
+
+### Deploy Workflow (deploy.yml)
+
+| # | Check | Status | Notes |
+|---|-------|--------|-------|
+| 1 | No test step | ⚠️ | Comment says "tests remain local-only gate" — acceptable for self-hosted |
+| 2 | Health checks | ✅ | 20 retries × 6s for backend, 10 × 5s for frontend |
+| 3 | Image pruning | ✅ | Prevents disk exhaustion |
+| 4 | `set -euo pipefail` | ✅ | Fail-fast on errors |
+| 5 | Concurrency non-cancelling | ✅ | Prevents interrupted deploys |
+
+### Dockerfile.backend
+
+| # | Check | Status | Notes |
+|---|-------|--------|-------|
+| 1 | Multi-stage build | ✅ | Builder + runtime stages |
+| 2 | Non-root user | ✅ | `appuser` in runtime |
+| 3 | Alpine base | ✅ | Minimal attack surface |
+| 4 | Layer caching | ✅ | POMs copied before source |
+| 5 | No HEALTHCHECK | ⚠️ | Compose defines it, but image itself has no HEALTHCHECK |
+| 6 | `dependency:go-offline` | ✅ | Pre-downloads deps for cache |
+| 7 | No COPY of secrets | ✅ | No .env or keys copied |
+
+### Docker Compose Files
+
+| # | Check | Status | Notes |
+|---|-------|--------|-------|
+| 1 | Production uses env vars | ✅ | `docker-compose.prod.yml` uses `${...}` |
+| 2 | Resource limits (prod) | ✅ | Memory/CPU limits set |
+| 3 | Log rotation (prod) | ✅ | `json-file` driver with max-size |
+| 4 | Dev compose has secrets | ❌ | S-01, S-02 — hardcoded in plain text |
+| 5 | Postgres healthcheck | ✅ | `pg_isready` |
+| 6 | Service dependencies | ✅ | `condition: service_healthy` |
+| 7 | Prod binds 127.0.0.1 only | ✅ | Prevents external access without reverse proxy |
+| 8 | Dev binds 0.0.0.0 | ⚠️ | `ports: "8080:8080"` is all-interfaces |
+
+---
+
+## 4. Snyk Policy (.snyk)
+
+✅ **Well-structured.** CSRF ignore is correctly scoped to the JWT API filter chain with clear rationale and expiry date. No concerns.
+
+---
+
+## 5. Recommendations (Priority Order)
+
+| Priority | Finding | Action |
+|----------|---------|--------|
+| 🔴 High | S-01, S-02 | Move secrets to `.env` (gitignored) or generate at runtime |
+| 🟡 Medium | S-03, S-04, S-05 | Remove `|| true` / `|| echo` from security tools — make them blocking |
+| 🟡 Medium | S-06 | Fix parallel Docker build error propagation |
+| 🟢 Low | S-07 | Pin Trivy version or use official action |
+| 🟢 Low | S-08 | Replace hardcoded IP with DNS/localhost |
+| 🟢 Low | S-09 | Add SBOM generation (future) |
+
+---
+
+## 6. Summary
+
+The CI/CD setup is **architecturally sound** — parallel jobs, proper caching, multi-stage Docker builds, non-root containers, and comprehensive security scanning (OWASP, Trivy, Gitleaks, pnpm audit). 
+
+The main weakness is that **all security gates are non-blocking** (`|| true` everywhere), which means the scanning tools generate reports but never actually prevent a vulnerable build from deploying. This is the single most impactful fix: remove the fallbacks and let CVEs/secrets block the pipeline.
+
+The hardcoded secrets in `docker-compose.yml` are a moderate risk — they're clearly dev-only values different from production, but the TrueNAS deployment (which is internet-facing) reuses the same NEXTAUTH_SECRET, which should be fixed.