docs(16): create phase plan — 6 plans, 2 waves (dep audit, security checks, image hygiene)

SEC-01/02, DEP-01/02, IMG-01/02/03, CI-03. Wave 1: image-hygiene runtime guard (TDD), audit+outdated wrappers (TDD), eslint-plugin-security fold, gitleaks config+baseline+.dockerignore. Wave 2: ci.yml security job + gate wiring, publish.yml hygiene assertions + boot-smoke. esbuild GHSA-gv7w-rqvm-qjhr waivered in 16-02 before the gate goes live.
2026-06-12 23:23:18 -04:00
parent bfc93584d7
commit e039c85a22
9 changed files with 1545 additions and 29 deletions
@@ -0,0 +1,569 @@
+# Phase 16: CI Dependency Audit, Security Checks & Image Hygiene — Pattern Map
+
+**Mapped:** 2026-06-13
+**Files analyzed:** 11
+**Analogs found:** 10 / 11
+
+---
+
+## File Classification
+
+| New/Modified File | Role | Data Flow | Closest Analog | Match Quality |
+|---|---|---|---|---|
+| `.gitea/workflows/ci.yml` | CI workflow | event-driven | self (existing jobs in same file) | exact |
+| `.gitea/workflows/publish.yml` | CI workflow | event-driven | self (existing build/push steps) | exact |
+| `apps/api/Dockerfile` | config | build-time | self (existing `base`/`dev` stage ENV/WORKDIR lines) | exact |
+| `apps/api/src/index.ts` | startup / guard | request-response | `apps/api/src/auth/devBypass.ts` (existing hard guard) | exact |
+| `apps/api/src/lib/bootGuards.ts` | utility | — | `apps/api/src/auth/devBypass.ts` | role-match |
+| `apps/api/tests/lib/bootGuards.test.ts` | test | — | `apps/api/tests/auth/devBypass.test.ts` | exact |
+| `.dockerignore` | config | build-time | `.gitignore` (root) | role-match |
+| `.gitleaks.toml` | config | — | root config files (`.prettierrc`, `.markdownlint-cli2.jsonc`) | partial |
+| `scripts/check-audit.mjs` | utility script | batch | none in repo | no analog |
+| `scripts/check-outdated.mjs` | utility script | batch | none in repo | no analog |
+| `scripts/audit-allowlist.json` | config | — | none in repo | no analog |
+| `scripts/outdated-pins.json` | config | — | none in repo | no analog |
+| `eslint.config.js` | config | — | self (existing flat config) | exact |
+
+---
+
+## Pattern Assignments
+
+### `.gitea/workflows/ci.yml` — add `security` job + update `gate`
+
+**Analog:** The existing jobs in the same file.
+
+**Job skeleton pattern** — how every job starts (lines 33–54, `fast-checks`):
+```yaml
+fast-checks:
+  runs-on: ubuntu-latest
+  if: github.event_name == 'pull_request'
+  steps:
+    - uses: actions/checkout@v4
+
+    - uses: actions/setup-node@v4
+      with:
+        node-version: '22'
+
+    - name: Enable pnpm
+      run: corepack enable pnpm
+
+    # actions/cache@v4 is intentionally omitted — probe (D-PROBE-04) showed it
+    # times out on this runner.
+
+    - name: Install dependencies
+      run: pnpm install --frozen-lockfile
+```
+
+**Conditional job pattern** — `needs: [changes]` + `if:` code-gated (lines 68–72, `api`):
+```yaml
+api:
+  runs-on: ubuntu-latest
+  needs: [changes]
+  if: github.event_name == 'pull_request' && needs.changes.outputs.code == 'true'
+```
+
+**`gate` aggregator pattern — individual `needs.X.result` checks** (lines 345–367):
+```yaml
+gate:
+  runs-on: ubuntu-latest
+  needs: [fast-checks, changes, api, harness]
+  if: always()
+  steps:
+    - name: Check all required jobs passed or were skipped
+      run: |
+        # fast-checks always runs — must be success
+        if [ "${{ needs.fast-checks.result }}" != "success" ]; then
+          echo "fast-checks: ${{ needs.fast-checks.result }}"
+          exit 1
+        fi
+        # api and harness are conditionally skipped — success OR skipped are both acceptable
+        # NOTE: uses individual needs.X.result checks (not the wildcard aggregate) due to
+        # Gitea 1.26.2 bug #31007 where the wildcard expression returns false even when jobs succeed.
+        for result in "${{ needs.api.result }}" "${{ needs.harness.result }}"; do
+          if [ "$result" != "success" ] && [ "$result" != "skipped" ]; then
+            echo "Heavy job failed or was cancelled: $result"
+            exit 1
+          fi
+        done
+        echo "Gate passed."
+```
+
+**New `security` job pattern** — parallel to `fast-checks`, always runs gitleaks, conditionally runs pnpm steps:
+```yaml
+security:
+  runs-on: ubuntu-latest
+  needs: [changes]
+  if: github.event_name == 'pull_request'
+  steps:
+    - uses: actions/checkout@v4
+      with:
+        fetch-depth: 0  # Required for gitleaks git log-opts range — base.sha must be local
+
+    # ── Gitleaks (always runs per D-12) ─────────────────────────
+    - name: Install gitleaks
+      run: |
+        set -euo pipefail
+        VERSION=8.30.1
+        curl -sL \
+          "https://github.com/gitleaks/gitleaks/releases/download/v${VERSION}/gitleaks_${VERSION}_linux_x64.tar.gz" \
+          | tar -xz gitleaks
+        chmod +x gitleaks
+        mv gitleaks /usr/local/bin/gitleaks
+
+    - name: Secret scan (PR diff, blocking)
+      run: |
+        set -euo pipefail
+        BASE_SHA="${{ github.event.pull_request.base.sha }}"
+        HEAD_SHA="${{ github.event.pull_request.head.sha }}"
+        gitleaks git \
+          --log-opts="--no-merges ${BASE_SHA}..${HEAD_SHA}" \
+          --config .gitleaks.toml \
+          --baseline-path scripts/gitleaks-baseline.json \
+          --report-path /tmp/gitleaks-pr-report.json \
+          --exit-code 1
+
+    # ── pnpm audit + outdated (code-change PRs only per D-12) ───
+    - uses: actions/setup-node@v4
+      if: needs.changes.outputs.code == 'true'
+      with:
+        node-version: '22'
+
+    - name: Enable pnpm
+      if: needs.changes.outputs.code == 'true'
+      run: corepack enable pnpm
+
+    - name: Install dependencies
+      if: needs.changes.outputs.code == 'true'
+      run: pnpm install --frozen-lockfile
+
+    - name: Dependency audit (blocking on High+Critical)
+      if: needs.changes.outputs.code == 'true'
+      run: node scripts/check-audit.mjs
+
+    - name: Dependency outdated report (advisory only)
+      if: needs.changes.outputs.code == 'true'
+      run: node scripts/check-outdated.mjs
+      # Always exits 0 — log output only, never gates (D-06)
+```
+
+**Updated `gate` needs list and security check to add:**
+```yaml
+gate:
+  needs: [fast-checks, changes, api, harness, security]   # security added
+  ...
+        # security always runs — must be success (gitleaks always fires)
+        if [ "${{ needs.security.result }}" != "success" ]; then
+          echo "security: ${{ needs.security.result }}"
+          exit 1
+        fi
+```
+
+**Step-level `set -euo pipefail` pattern** — all multi-line `run:` blocks in the file use this as the first line. Follow the same convention for all new steps.
+
+---
+
+### `.gitea/workflows/publish.yml` — add static assertions + boot-smoke
+
+**Analog:** Existing steps in the same file.
+
+**Inline shell step with `set -euo pipefail`** (lines 80–93):
+```yaml
+    - name: Build and push
+      run: |
+        set -euo pipefail
+        docker build --target production \
+          -f apps/api/Dockerfile \
+          -t ${{ steps.tags.outputs.latest }} \
+          -t ${{ steps.tags.outputs.sha_tag }} \
+          .
+        docker push ${{ steps.tags.outputs.sha_tag }}
+        docker push ${{ steps.tags.outputs.latest }}
+```
+
+**`if: always()` pattern for cleanup** (lines 95–97):
+```yaml
+    - name: Docker logout
+      if: always()
+      run: docker logout git.bergerhouse.net || true
+```
+
+**New assertions placed BEFORE the `docker push` lines** (placement rule from RESEARCH D-10):
+```yaml
+    - name: Image hygiene — static assertions
+      run: |
+        set -euo pipefail
+        if [ ! -f ".dockerignore" ]; then
+          echo "FAIL: .dockerignore does not exist"
+          exit 1
+        fi
+        for pattern in ".env" "node_modules" "apps/api/scripts" ".git" ".planning" "apps/api/tests" "apps/pwa/e2e"; do
+          if ! grep -q "$pattern" .dockerignore; then
+            echo "FAIL: .dockerignore missing pattern: $pattern"
+            exit 1
+          fi
+        done
+        if ! grep -q "\-\-target production" .gitea/workflows/publish.yml; then
+          echo "FAIL: publish.yml does not build --target production"
+          exit 1
+        fi
+        echo "Static image hygiene assertions PASSED."
+
+    - name: Image hygiene — boot-smoke (must refuse dev-bypass in production)
+      run: |
+        set -euo pipefail
+        IMAGE="${{ steps.tags.outputs.sha_tag }}"
+        set +e
+        timeout 15 docker run --rm \
+          --env NODE_ENV=production \
+          --env DEV_AUTH_BYPASS=true \
+          "$IMAGE" \
+          2>&1 | head -20
+        EXIT=$?
+        set -e
+        if [ "$EXIT" -eq 0 ]; then
+          echo "FAIL: Production image started successfully with DEV_AUTH_BYPASS=true — guard not working"
+          exit 1
+        fi
+        if [ "$EXIT" -eq 124 ]; then
+          echo "FAIL: Production image did not exit within 15s — guard not firing"
+          exit 1
+        fi
+        echo "PASS: Production image refused to start with DEV_AUTH_BYPASS=true (exit $EXIT)"
+```
+
+---
+
+### `apps/api/Dockerfile` — add `ENV NODE_ENV=production` in production stage
+
+**Analog:** Existing ENV/CMD/WORKDIR conventions within the same file.
+
+**Existing `dev` stage pattern** (lines 19–22) — shows WORKDIR + CMD:
+```dockerfile
+FROM base AS dev
+WORKDIR /app/apps/api
+COPY --from=builder /app /app
+CMD ["node", "--watch", "dist/index.js"]
+```
+
+**Existing `production` stage** (lines 35–46) — the gap to fix:
+```dockerfile
+FROM base AS production
+COPY pnpm-workspace.yaml pnpm-lock.yaml package.json ./
+COPY apps/api/package.json ./apps/api/
+COPY apps/pwa/package.json ./apps/pwa/
+RUN pnpm install --frozen-lockfile --prod --filter @familysync/api...
+COPY --from=builder /app/apps/api/dist ./apps/api/dist
+WORKDIR /app/apps/api
+COPY --from=pwa-builder /app/apps/pwa/dist ./public
+CMD ["node", "dist/index.js"]
+```
+
+**Add after `WORKDIR /app/apps/api`, before `COPY --from=pwa-builder`:**
+```dockerfile
+# Enforce production identity — engages the NODE_ENV=production hard guard
+# in devBypass.ts, preventing dev-bypass activation even if DEV_AUTH_BYPASS
+# is accidentally set in the container environment. (D-07)
+ENV NODE_ENV=production
+```
+
+---
+
+### `apps/api/src/lib/bootGuards.ts` — exported guard function
+
+**Analog:** `apps/api/src/auth/devBypass.ts` — same pattern of evaluating env vars at call time, exporting a pure function with a JSDoc comment block.
+
+**Function export pattern** (devBypass.ts lines 58–76):
+```typescript
+/**
+ * Returns a Hono MiddlewareHandler ...
+ *
+ * The function evaluates env vars at call time (when the app starts), not at request time.
+ */
+export function devAuthBypass(): MiddlewareHandler {
+  // Hard production guard — FIRST check, before reading any other env var.
+  if (process.env.NODE_ENV === 'production') {
+    return async (_c, next) => next();
+  }
+  ...
+}
+```
+
+**New `bootGuards.ts` pattern to follow:**
+```typescript
+/**
+ * Boot-time production safety guards (D-08).
+ *
+ * Exported as a standalone function so it can be unit-tested without
+ * forking a process or importing the full app module graph.
+ *
+ * Call assertNotDevBypassInProduction() as the FIRST statement inside
+ * the isMainModule() block in index.ts, before VAPID config, workers,
+ * or serve().
+ */
+
+export function assertNotDevBypassInProduction(): void {
+  if (process.env.NODE_ENV === 'production' && process.env.DEV_AUTH_BYPASS === 'true') {
+    console.error(
+      '[FATAL] DEV_AUTH_BYPASS=true is set in a production environment. ' +
+        'This configuration is forbidden. Refusing to start.',
+    );
+    process.exit(1);
+  }
+}
+```
+
+---
+
+### `apps/api/src/index.ts` — add boot guard call
+
+**Analog:** Existing `isMainModule()` guard block (lines 112–147) and devBypassActive comment pattern (lines 23–29).
+
+**Placement rule** — first statement inside `if (isMainModule())` before any other startup code:
+```typescript
+if (isMainModule()) {
+  // D-08: Production safety guard — must be FIRST, before VAPID config, workers, or serve().
+  assertNotDevBypassInProduction();
+
+  // Configure VAPID credentials for web-push before starting background workers.
+  const vapidSubject = process.env.VAPID_SUBJECT ?? '';
+  // ...existing startup code unchanged...
+}
+```
+
+**Import to add** (follows existing import block pattern, lines 1–18):
+```typescript
+import { assertNotDevBypassInProduction } from './lib/bootGuards.js';
+```
+
+---
+
+### `apps/api/tests/lib/bootGuards.test.ts` — unit test
+
+**Analog:** `apps/api/tests/auth/devBypass.test.ts` — exact same role, same test framework, same env manipulation pattern.
+
+**Test file structure pattern** (devBypass.test.ts lines 1–30):
+```typescript
+/**
+ * [description of what is tested] — unit tests.
+ *
+ * Tests the [N] behavioral cases:
+ *  1. ...
+ */
+
+import { describe, it, expect, afterEach } from 'vitest';
+// Import the module under test (not Hono app — pure function test)
+
+describe('[function name]', () => {
+  const originalNodeEnv = process.env.NODE_ENV;
+  const originalBypassFlag = process.env.DEV_AUTH_BYPASS;
+
+  afterEach(() => {
+    process.env.NODE_ENV = originalNodeEnv;
+    if (originalBypassFlag === undefined) {
+      delete process.env.DEV_AUTH_BYPASS;
+    } else {
+      process.env.DEV_AUTH_BYPASS = originalBypassFlag;
+    }
+  });
+
+  it('...description...', async () => {
+    process.env.NODE_ENV = 'production';
+    process.env.DEV_AUTH_BYPASS = 'true';
+    // ...
+  });
+});
+```
+
+**Key difference for bootGuards test:** Use `vi.spyOn(process, 'exit').mockImplementation(...)` and `vi.stubEnv` from vitest instead of manual env manipulation, since `assertNotDevBypassInProduction()` calls `process.exit(1)` directly. Import `vi` from vitest.
+
+**Test cases required:**
+1. `NODE_ENV=production` + `DEV_AUTH_BYPASS=true` → calls `process.exit(1)`
+2. `NODE_ENV=development` + `DEV_AUTH_BYPASS=true` → does NOT call `process.exit`
+3. `NODE_ENV=production` + `DEV_AUTH_BYPASS` unset → does NOT call `process.exit`
+
+---
+
+### `.dockerignore` — new root-level file
+
+**Analog:** `.gitignore` at repo root for pattern style and comment conventions.
+
+**`.gitignore` comment/section style** (lines 1–30):
+```gitignore
+# Dependencies
+node_modules/
+
+# Build output
+dist/
+.dist/
+
+# Environment — NEVER commit secrets at rest ...
+.env
+.env.*
+!.env.example
+```
+
+**Follow the same section-header comment style.** Refer to the full recommended content in RESEARCH.md (the `.dockerignore` section) — it is already fully specified there. Key sections: Secrets, VCS, Build artifacts, Dependencies, Tests, Playwright artifacts, Planning/docs, Editor/OS, CI config files, SQL dumps.
+
+---
+
+### `.gitleaks.toml` — new root-level config file
+
+**Analog:** No close analog in the repo. Root-level TOML config files follow a "title + sections" structure. The repo has `.markdownlint-cli2.jsonc` as a comparable root config (different format).
+
+**Pattern:** Follow the content exactly as specified in RESEARCH.md — the full `.gitleaks.toml` content is pre-authored there. Key structural rules:
+- `title = "..."` at the top
+- `[extend] useDefault = true` to inherit built-in ruleset
+- `[[allowlists]]` blocks with `description` + `paths` fields for known-safe false-positive files
+
+---
+
+### `eslint.config.js` — add `eslint-plugin-security`
+
+**Analog:** Itself — the existing flat config is the pattern to extend.
+
+**Existing plugin registration pattern** (lines 7–12, imports + `tseslint.config()` wrapper):
+```javascript
+import js from '@eslint/js';
+import tseslint from 'typescript-eslint';
+import reactPlugin from 'eslint-plugin-react';
+import reactHooks from 'eslint-plugin-react-hooks';
+import prettierConfig from 'eslint-config-prettier/flat';
+
+export default tseslint.config(
+```
+
+**Existing config block with `files:` + `extends:` + `rules:` pattern** (lines 27–43):
+```javascript
+  {
+    files: ['apps/**/*.{ts,tsx}'],
+    extends: [js.configs.recommended, tseslint.configs.recommendedTypeChecked],
+    languageOptions: { ... },
+    rules: {
+      '@typescript-eslint/no-unused-vars': ['error', { ... }],
+    },
+  },
+```
+
+**New block to insert BEFORE `prettierConfig` (section 5, which MUST remain last):**
+```javascript
+import pluginSecurity from 'eslint-plugin-security';
+
+// ... inside tseslint.config(...):
+
+  // ── N. eslint-plugin-security: blocking errors per D-03 ──────────────────
+  // Applied to all TS/TSX in both apps. detect-object-injection disabled globally
+  // due to very high false-positive rate on Drizzle ORM bracket access patterns;
+  // real risk sites carry inline eslint-disable with justification comment.
+  {
+    files: ['apps/**/*.{ts,tsx}'],
+    ...pluginSecurity.configs.recommended,
+    rules: {
+      ...pluginSecurity.configs.recommended.rules,
+      'security/detect-object-injection': 'off', // High FP rate; Drizzle + TS generics — see triage notes
+    },
+  },
+
+  prettierConfig, // MUST remain last
+```
+
+---
+
+### `scripts/check-audit.mjs` — new Node.js wrapper script
+
+**Analog:** No existing script analog. Pattern is a standalone ESM Node.js script using `node:child_process` and `node:fs` built-ins.
+
+**Conventions to follow from RESEARCH.md:**
+- Use `import { execSync } from 'node:child_process'` and `import { readFileSync } from 'node:fs'` (node: prefix protocol)
+- Run `pnpm audit --json` without `--audit-level` (captures all severities in JSON)
+- Filter `audit.advisories` by `severity` in code
+- Cross-check against `scripts/audit-allowlist.json` by `github_advisory_id`
+- Exit 1 on unwaived High+Critical; exit 0 on all waived or no findings
+- Print advisory-only findings (moderate/low) to stdout before exiting 0
+
+---
+
+### `scripts/check-outdated.mjs` — new Node.js wrapper script
+
+**Analog:** No existing script analog.
+
+**Conventions to follow from RESEARCH.md:**
+- Run `pnpm outdated --format json -r` and parse JSON
+- Read `scripts/outdated-pins.json` for known-intentional pin explanations
+- Classify each entry: AUDIT-ADVISORY / MAJOR-BEHIND / INTENTIONAL-PIN / ROUTINE-DRIFT
+- Always exits 0 — advisory-only (D-06)
+- Cross-check `pnpm audit --json` output to flag pinned versions with active advisories
+
+---
+
+### `scripts/audit-allowlist.json` and `scripts/outdated-pins.json` — new JSON config files
+
+**Analog:** No existing analog.
+
+**`audit-allowlist.json` format:**
+```json
+{
+  "GHSA-xxxx-xxxx-xxxx": {
+    "reason": "...",
+    "reviewer": "luc",
+    "expires": "YYYY-MM-DD"
+  }
+}
+```
+Must include the pre-existing `GHSA-gv7w-rqvm-qjhr` (esbuild High, transitive through drizzle-kit/vitest/vite — dev-only) as the initial entry.
+
+**`outdated-pins.json` format:**
+```json
+{
+  "package-name": "Human-readable reason for the intentional pin."
+}
+```
+Initial entries: `eslint`, `@eslint/js`, `zod`, `@types/node` (all with reasons matching RESEARCH.md).
+
+---
+
+## Shared Patterns
+
+### `set -euo pipefail` in all shell steps
+**Source:** `.gitea/workflows/ci.yml` — every multi-line `run:` block starts with this.
+**Apply to:** Every new `run: |` block in both `ci.yml` and `publish.yml`.
+
+### No `actions/cache`
+**Source:** `.gitea/workflows/ci.yml` line 46–48 comment.
+**Apply to:** The new `security` job — do NOT add `actions/cache@v4`. The ~30s pnpm install + ~5s gitleaks download are acceptable per D-PROBE-04.
+
+### Individual `needs.X.result` checks in `gate` (not wildcards)
+**Source:** `.gitea/workflows/ci.yml` lines 358–365, comment referencing Gitea bug #31007.
+**Apply to:** The updated `gate` aggregator — add `needs.security.result` as a separate individual check, not folded into the `for result in ...` loop (security must always succeed, not "success OR skipped").
+
+### `node:` prefix for built-in imports in scripts
+**Source:** `apps/api/src/index.ts` lines 1–2: `import { fileURLToPath } from 'node:url'`, `import { realpathSync } from 'node:fs'`.
+**Apply to:** `scripts/check-audit.mjs` and `scripts/check-outdated.mjs`.
+
+### JSDoc comment block on exported functions
+**Source:** `apps/api/src/auth/devBypass.ts` lines 1–25 (file-level) and 49–57 (function-level).
+**Apply to:** `apps/api/src/lib/bootGuards.ts` — the exported `assertNotDevBypassInProduction()` function must have a JSDoc block explaining its purpose, placement requirement (first in `isMainModule()`), and the D-08 reference.
+
+### afterEach env restoration in unit tests
+**Source:** `apps/api/tests/auth/devBypass.test.ts` lines 19–29.
+**Apply to:** `apps/api/tests/lib/bootGuards.test.ts` — restore `process.env.NODE_ENV` and `process.env.DEV_AUTH_BYPASS` in `afterEach`.
+
+---
+
+## No Analog Found
+
+| File | Role | Data Flow | Reason |
+|---|---|---|---|
+| `scripts/check-audit.mjs` | utility script | batch | No audit/wrapper scripts exist in the repo |
+| `scripts/check-outdated.mjs` | utility script | batch | No outdated/wrapper scripts exist in the repo |
+| `scripts/audit-allowlist.json` | config data | — | No allowlist/waiver JSON pattern exists in the repo |
+| `scripts/outdated-pins.json` | config data | — | No pin-reason config pattern exists in the repo |
+| `.gitleaks.toml` | tool config | — | No TOML configs exist in the repo; RESEARCH.md content is the full spec |
+| `scripts/gitleaks-baseline.json` | generated artifact | — | Generated by running gitleaks locally; not handwritten |
+
+---
+
+## Metadata
+
+**Analog search scope:** `.gitea/workflows/`, `apps/api/src/`, `apps/api/tests/`, `eslint.config.js`, `apps/api/Dockerfile`, `.gitignore`
+**Files scanned:** 9 source files read directly
+**Pattern extraction date:** 2026-06-13