Phase 16: CI dependency audit & security checks #15

Merged
luckberg merged 40 commits from gsd/phase-16-ci-dependency-audit-and-security-checks into main 2026-06-13 10:01:01 -04:00
2 changed files with 284 additions and 0 deletions
Showing only changes of commit 75b82f472c - Show all commits
@@ -0,0 +1,124 @@
# Phase 16: CI Dependency Audit, Security Checks & Image Hygiene - Context
**Gathered:** 2026-06-12
**Status:** Ready for planning
<domain>
## Phase Boundary
Extend the **existing** Gitea CI with three families of gates — implemented as additions to the current `.gitea/workflows/ci.yml` (PR workflow: `changes → fast-checks / api / harness → gate`) and `.gitea/workflows/publish.yml` (push-to-main publish, builds `--target production`). **Not** a new pipeline, **not** a new runtime dependency, **not** a new external service.
1. **Dependency audit**`pnpm audit` against the lockfile (fail on High+Critical) + a `pnpm outdated` advisory report.
2. **Security checks baseline** — secret scanning (gitleaks) + a static security lint (eslint-plugin-security folded into the existing ESLint gate).
3. **Dev↔prod image hygiene (absorbs backlog 999.17)** — provably confine `DEV_AUTH_BYPASS` and any dev affordance/secret/seed data to local dev; the published production image must never carry them and must refuse to run with dev-bypass enabled.
**Explicitly out of scope:** automated dependency *upgrade* bots (Renovate/Dependabot), Trivy/image CVE scanning, removing dev-bypass (still needed for local + Phase 7/8 harness).
</domain>
<decisions>
## Implementation Decisions
### Security-Check Baseline
- **D-01:** Baseline = **secret scanning + static security lint**. Trivy/image CVE scanning is **dropped** — not even backlogged; revisit only if a future need arises.
- **D-02:** **Secret scanning via gitleaks** (tool choice researcher may confirm vs trufflehog). Scope = **per-PR diff (blocking) + a one-time full-history/full-tree baseline scan** to catch anything already committed. The app handles real family credentials (encryption key, OIDC secret, Fastmail app passwords), so secret scanning is core.
- **D-03:** **eslint-plugin-security folded into the existing Phase 13 ESLint gate, as blocking errors** (not warnings). Accepted consequence: the plugin is heuristic/noisy (e.g. `detect-object-injection`); the executor must triage existing code — add targeted `eslint-disable` with justification or rule-tune — to get the gate green. This is expected work, not a blocker.
### Dependency Audit & Outdated
- **D-04:** `pnpm audit` **fails the build on High + Critical**; moderate/low are advisory only.
- **D-05:** Unfixable/transitive advisories are waived via a **committed allowlist file in the repo** — advisory IDs (CVE/GHSA) each with a reason + reviewer, reviewed through PR. A wrapper filters `pnpm audit` output against it (or pnpm's native `auditConfig.ignore*` if the researcher finds it cleaner — but keep it auditable and self-documenting, not silent).
- **D-06:** Outdated reporting **runs and never gates** (respects the deliberate exact-version pins in CLAUDE.md). Exact balance is an **open research question** — see OQ-01.
### Dev/Prod Image Hygiene (999.17)
- **D-07:** **Bake `ENV NODE_ENV=production` into the production Dockerfile stage.** Today the `production` stage sets no `NODE_ENV` and `CMD` runs `node dist/index.js` with no env baked in, so `devBypass.ts`'s "hard guard" (`NODE_ENV==='production'` checked first) is **not actually engaged** in the shipped image — it's only safe because the second check (`DEV_AUTH_BYPASS !== 'true'`) passes through when unset. Baking `NODE_ENV=production` engages the hard guard.
- **D-08:** Add a **boot-time refuse-to-boot guard**: on startup, if `NODE_ENV==='production'` AND `DEV_AUTH_BYPASS==='true'`, **throw and exit non-zero** instead of silently no-op'ing. Unit-tested. (This is defense-in-depth on top of D-07 — turns a silent misconfig into a loud failure.)
- **D-09:** Create a **full `.dockerignore`** (none exists today — the whole repo root is currently sent to the Docker daemon as build context). Scope = **secrets + dev + bulk**: `.env` / `.env.*` (incl. `apps/api/.env.spike`), `apps/api/scripts/seed-credential.mjs`, `.git`, `node_modules`, `dist`, `test`/`tests`, `e2e`, `.planning`, `*.sql`/dumps, Playwright artifacts (researcher enumerates the exact list against the current tree).
- **D-10:** **CI assertion = static + boot-smoke.** Static: assert `.dockerignore` exists & covers the forbidden patterns, and `publish.yml` still pins `--target production`. Boot-smoke: start the built production image with `NODE_ENV=production` + `DEV_AUTH_BYPASS=true` and assert it **refuses to boot** (exits non-zero, proving D-08 in the actual image). Full filesystem forensics deemed unnecessary — the `production` stage already copies only `apps/api/dist` + `apps/pwa/dist`.
### Gating & Noise Posture
- **D-11:** Blocking vs advisory split — **block:** gitleaks (secret found), eslint-plugin-security, `pnpm audit` High+Critical, image-hygiene boot-smoke + static checks. **Advisory (never gates):** `pnpm outdated`.
- **D-12:** **Doc-only PR behavior:** gitleaks **runs on every PR** including doc-only (a secret can land in a doc/config). `pnpm audit` + `pnpm outdated` are **gated behind the code/lockfile `changes` filter** like `api`/`harness` (mirrors Phase 15's doc-only-skip model).
- **D-13:** **Advisory results surface in the job log only** — no PR comment / Gitea API wiring. Gitea doesn't render GitHub-style annotations (see `ci.yml` Pitfall 5 / D-06: reporter `github` is overridden). Blocking checks surface via failed status + the `gate` aggregate.
- **D-14:** Any new **blocking** job must be wired into the `gate` aggregator (`if: always()`, individual `needs.X.result` checks per the Gitea 1.26.2 wildcard bug #31007) and, if it becomes a required context, into branch protection on `main`.
### Claude's Discretion / Researcher Decides
- **Job decomposition** (D-15): how the new PR-time checks (secret scan, audit, outdated) are laid out in `ci.yml` — a dedicated parallel `security` job vs folding into `fast-checks` — is **left to the researcher/planner** against runner constraints (no `actions/cache` — times out; ~30s install per job). Recommendation surfaced in discussion: a new `security` job parallel to `fast-checks` keeps the critical path fast and isolates advisory churn; eslint-plugin-security folds into the existing lint step regardless.
- Exact secret-scan tool (gitleaks vs trufflehog) and exact `.dockerignore` line list — researcher confirms.
### Folded Todos
None folded. (See Deferred — the one matched todo was already delivered in Phase 8.)
</decisions>
<open_questions>
## Open Research Questions
- **OQ-01 (outdated-vs-pins balance):** Design a pragmatic `pnpm outdated` reporting policy that **respects the intentional exact-version pins** in CLAUDE.md but still **surfaces when a pin is a liability** — e.g. the pinned version is multiple major versions behind latest, or the pinned version itself carries a known advisory. The user's words: "Version pins are fine but if there's an issue with them or if they are too far behind there should be a balance here." Output should be a concrete, advisory-only mechanism (what's reported, how a "dangerously behind" pin is flagged distinctly from routine drift). Never gates the build (D-06).
</open_questions>
<canonical_refs>
## Canonical References
**Downstream agents MUST read these before planning or implementing.**
### Existing CI to extend
- `.gitea/workflows/ci.yml` — PR workflow being extended. Jobs: `changes` (dorny/paths-filter), `fast-checks` (lint/format/md-lint/typecheck/pwa-unit), `api` (DB-backed, MariaDB service container), `harness` (Playwright iphone+pixel+desktop), `gate` (`if: always()` aggregator). Documents runner pitfalls: no `actions/cache`, no `mysql` CLI, `localhost``::1` vs IPv4, Gitea annotation non-rendering, wildcard-needs bug #31007.
- `.gitea/workflows/publish.yml` — push-to-main publish. Builds `docker build --target production -f apps/api/Dockerfile .`; tags `:latest` + `:<MILESTONE>-<shortsha>`; `REGISTRY_PAT` secret (Gitea forbids `GITEA_` prefix). This is where the image-hygiene boot-smoke + static assertion attach.
### Image hygiene (999.17)
- `apps/api/Dockerfile` — multi-stage: `base`/`builder`/`dev`/`pwa-builder`/`production`. `production` copies only `apps/api/dist` + `apps/pwa/dist`; **no `ENV NODE_ENV`** (the gap D-07 fixes). `dev` target shares the file.
- `apps/api/src/auth/devBypass.ts` — the dev-bypass middleware + `DEV_USER` (id=1). Hard guard checks `NODE_ENV==='production'` first; boot-time refuse-to-boot (D-08) extends this.
- (no `.dockerignore` exists yet — D-09 creates it)
### Constraints / precedent
- `CLAUDE.md` — pins exact dependency versions intentionally (drives D-06 / OQ-01); Technology Stack + Version Compatibility tables.
- `.planning/phases/15-doc-only-ci-skip-and-md-lint/15-CONTEXT.md` — gate-aggregation + doc-only-skip noise-control precedent (D-12, D-14 mirror it).
- `.planning/phases/08-gitea-ci/08-CONTEXT.md` — original CI design decisions + runner-constraint probes (D-PROBE-*).
### Memory (operator-confirmed gotchas)
- Gitea CI runner gotchas: `ubuntu-latest` label, no `actions/cache`, no `mysql` CLI, `GITEA_` secret prefix forbidden (use `REGISTRY_PAT`), `act` reaps backgrounded procs at step boundary.
</canonical_refs>
<code_context>
## Existing Code Insights
### Reusable Assets
- **`gate` job pattern** (`ci.yml`) — new blocking jobs plug into its per-`needs.X.result` aggregation; copy the success/skipped tolerance logic.
- **`changes`/paths-filter** (`ci.yml`) — reuse `needs.changes.outputs.code` to gate audit/outdated on code/lockfile changes (D-12).
- **Existing ESLint gate** (Phase 13, run in `fast-checks` `pnpm lint`) — eslint-plugin-security plugs into the same config/step (D-03).
- **Phase 8 inline-Node-via-mysql2 pattern** — precedent for runner steps without extra CLIs (no `mysql`, no extra binaries assumed available).
### Established Patterns
- **Image build only at publish** (`publish.yml`, push-to-main). The boot-smoke (D-10) builds/runs the image where it's already built — at publish — rather than adding a full image build to every PR.
- **Throwaway CI creds** scoped to ephemeral service containers — never reuse for any new secret-handling step.
- **Job-log-only result surfacing** — Gitea annotation non-rendering already forced `--reporter=list,html` over `github`; advisory output follows the same constraint (D-13).
### Integration Points
- New `security` (or folded) checks → `ci.yml` jobs + `gate` aggregator + possibly branch-protection required contexts.
- Boot-time guard → `apps/api` startup path (alongside/within `devBypass.ts` usage in `index.ts`) + a unit test.
- `.dockerignore` → repo root; `ENV NODE_ENV=production``production` stage of `apps/api/Dockerfile`.
- Boot-smoke + static assertion → `publish.yml` (post-build, pre/around push).
</code_context>
<specifics>
## Specific Ideas
- gitleaks preferred for secret scanning (single binary, easy on a self-hosted runner); per-PR diff + one-time full-history baseline.
- eslint-plugin-security must be **blocking** even though it's noisy — the user explicitly chose `error` over `warn`.
- The `NODE_ENV` gap in the production image was the concrete "aha" of this discussion — fixing it (D-07) is the highest-leverage, lowest-cost hardening.
- Keep the whole phase additive to existing CI — no rewrite of `ci.yml`/`publish.yml` structure.
</specifics>
<deferred>
## Deferred Ideas
- **Renovate / Dependabot automated dependency upgrades** — out of scope; detection/enforcement only this phase. Self-hosted Renovate on Gitea is its own setup + interplay with the pin strategy. Candidate for a future phase/backlog.
- **Trivy / image CVE scanning** — dropped, not backlogged per the user; reconsider only if a concrete need arises (base-image `node:22-alpine` CVE exposure).
- **PR-comment surfacing of advisory results** (Gitea API) — deferred in favor of job-log-only (D-13); revisit if visibility proves insufficient.
### Reviewed Todos (not folded)
- `2026-06-10-gitea-ci-regression-and-docker-publish.md` ("Gitea CI — full regression on PR to main + build/publish Docker image") — matched on keywords but **already delivered in Phase 8** (CI-01/CI-02). Not in Phase 16 scope; this is a stale pending-todo that should be archived.
</deferred>
---
*Phase: 16-ci-dependency-audit-and-security-checks*
*Context gathered: 2026-06-12*
@@ -0,0 +1,160 @@
# Phase 16: CI Dependency Audit, Security Checks & Image Hygiene - Discussion Log
> **Audit trail only.** Do not use as input to planning, research, or execution agents.
> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered.
**Date:** 2026-06-12
**Phase:** 16-ci-dependency-audit-and-security-checks
**Areas discussed:** Security-check baseline, Audit + outdated policy, Image-hygiene enforcement (999.17), Gating & noise posture
---
## Security-Check Baseline
### Which checks form the baseline (multiSelect)
| Option | Description | Selected |
|--------|-------------|----------|
| Secret scan on diff | gitleaks/trufflehog scans the PR diff for committed secrets | ✓ |
| Static security lint | eslint-plugin-security or CodeQL | ✓ |
| Trivy image scan | scan built production image for OS/package CVEs | ✗ (later dropped) |
| Dependency-review action | GitHub's PR action — depends on GitHub API, not on Gitea | |
**User's choice:** Secret scan + static security lint + (initially) trivy — then dropped trivy in the follow-up.
### Secret-scan scope
| Option | Description | Selected |
|--------|-------------|----------|
| Diff + one full-history scan | per-PR diff + a one-time full-history baseline | ✓ |
| PR diff only | only changed commits | |
| Full tree every run | whole repo every PR | |
### Trivy gate
| Option | Description | Selected |
|--------|-------------|----------|
| Publish-time, advisory | scan image, never block | |
| Publish-time, block on CRITICAL | fail publish on CRITICAL | |
| Per-PR (build image in PR) | earliest feedback | |
**User's choice:** "Drop Trivy for now. I don't want it in the backlog, but it can be some future thing if we need it." → Trivy removed from scope entirely.
### Static security lint severity
| Option | Description | Selected |
|--------|-------------|----------|
| Warn (advisory) | surfaced but non-blocking | |
| Error (blocking) | fails the lint gate | ✓ |
**Notes:** User accepts that blocking eslint-plugin-security will require triaging/disabling existing heuristic findings to reach green.
---
## Audit + Outdated Policy
### pnpm audit severity threshold
| Option | Description | Selected |
|--------|-------------|----------|
| High + Critical | fail on high/critical | ✓ |
| Critical only | fail only on critical | |
| Moderate+ | fail on moderate and above | |
### Waiver mechanism
| Option | Description | Selected |
|--------|-------------|----------|
| Allowlist file in repo | committed advisory-ID list + reason + reviewer | ✓ |
| pnpm overrides / config | auditConfig.ignore* in package.json | |
| No waiver mechanism yet | deal with it if/when it blocks | |
### Outdated reporting vs intentional pins
| Option | Description | Selected |
|--------|-------------|----------|
| Advisory PR comment, never gates | pnpm outdated -r as PR comment | |
| Advisory, job-log only | print to job log | |
| Skip outdated entirely | rely on audit only | |
**User's choice:** Deferred to researcher (OQ-01). "Version pins are fine but if there's an issue with them or if they are too far behind there should be a balance here." Outcome locked: advisory, never gates; researcher designs the "dangerously behind / pinned-version-has-advisory" flagging.
---
## Image-Hygiene Enforcement (999.17)
### Enforcement mechanism (multiSelect)
| Option | Description | Selected |
|--------|-------------|----------|
| Bake NODE_ENV=production into image | engages devBypass hard guard in shipped image | ✓ |
| Boot-time refuse-to-boot | throw + non-zero exit on prod + dev-bypass | ✓ |
| Build-time abort | fail build/publish on dev target/arg | |
**Notes:** publish.yml already pins `--target production`; the static CI assertion covers "stays that way."
### CI assertion depth
| Option | Description | Selected |
|--------|-------------|----------|
| Static + boot smoke | .dockerignore + --target assertion + run image with dangerous combo, assert refuses to boot | ✓ |
| Full filesystem forensics | export image fs, grep for secrets/seed/.git | |
| Static checks only | no container built/run | |
### .dockerignore scope
| Option | Description | Selected |
|--------|-------------|----------|
| Secrets + dev + bulk | .env*, seed-credential.mjs, .git, node_modules, dist, tests, e2e, .planning, *.sql, playwright artifacts | ✓ |
| Secrets-only minimal | only secret/seed/data files | |
| Researcher proposes the list | capture intent, enumerate later | |
---
## Gating & Noise Posture
### Job layout
| Option | Description | Selected |
|--------|-------------|----------|
| New 'security' job, parallel | gitleaks+audit+outdated parallel to fast-checks | |
| Fold into fast-checks | steps in existing job | |
| Researcher decides layout | pick against runner constraints | ✓ |
**Notes:** Recommendation surfaced (dedicated parallel `security` job) but final decomposition left to researcher/planner.
### Doc-only PR behavior
| Option | Description | Selected |
|--------|-------------|----------|
| Secret scan always; audit/outdated code-only | gitleaks universal, audit/outdated behind changes filter | ✓ |
| All new checks code-only | whole security job skips doc-only | |
| All new checks always run | run on every PR | |
### Result surfacing
| Option | Description | Selected |
|--------|-------------|----------|
| Job-log summary only | advisory output to job log | ✓ |
| PR comment via Gitea API | step posts/updates a PR comment | |
### Renovate / Dependabot
| Option | Description | Selected |
|--------|-------------|----------|
| Defer | out of scope; capture as deferred | ✓ |
| In scope | add upgrade-bot config this phase | |
---
## Claude's Discretion
- Job decomposition for the new PR-time checks (D-15) — researcher/planner.
- Exact secret-scan tool (gitleaks vs trufflehog) and exact `.dockerignore` line list — researcher confirms.
## Deferred Ideas
- Renovate / Dependabot automated dependency upgrades — future phase/backlog.
- Trivy / image CVE scanning — dropped, not backlogged (revisit only if needed).
- PR-comment surfacing of advisory results — deferred in favor of job-log-only.
- Stale pending todo `2026-06-10-gitea-ci-regression-and-docker-publish.md` — already delivered in Phase 8; should be archived.