# Security Audit — Cycle 3 Delta Report

**Date**: 2026-05-13
**Mode**: Resume / incremental — cycle-2 artifacts (`security_report.md`, `dependency_scan.md`, `static_analysis.md`, `owasp_review.md`, `infrastructure_review.md`) are kept verbatim; this report records ONLY the deltas introduced by cycle 3.
**Cycle**: Phase B / Cycle 3 (post AZ-510, AZ-511; AZ-512 deferred at cross-workspace BLOCKING gate)
**Scope of delta**: cycle-3 commits only — `70fb452` (AZ-510), `c368f60` (AZ-511), `6c7e297` (AZ-512 deferral, no source changes), plus the cycle-2-tail dependency upgrade landed in `f7dd6c9` that the cycle-2 report itself recommended.
**Verdict (post-cycle-3)**: **PASS_WITH_WARNINGS** — improvement vs. cycle-2 baseline (was FAIL).

---

## Verdict change

| Verdict component | Cycle 2 (2026-05-12) | Cycle 3 (2026-05-13) | Driver |
|-------------------|----------------------|----------------------|--------|
| Overall | FAIL | PASS_WITH_WARNINGS | All HIGH findings closed |
| Critical | 0 | 0 | — |
| High | 2 (F-DEP-1, F-SAST-1) | 0 | F-DEP-1 closed by `bun update vite` (cycle-2 inline fix `f7dd6c9`); F-SAST-1 carried — see below |
| Medium | 7 | 7 (carried) | No medium findings closed or added in cycle 3 |
| Low | 2 | 3 | New cycle-3 finding F-SAST-CY3-1 (`__resetBootstrapInflightForTests` exposed via prod barrel) |

> **Note on F-SAST-1 (Google Geocode API key in `mission-planner/` port-source)**: The cycle-2 audit classified it HIGH because the secret remains in real git history, even though `mission-planner/` does NOT ship in the production bundle. Cycle 3 did not touch `mission-planner/` and the key has not been revoked / externalized — F-SAST-1 stays open at HIGH at the *git-history* layer but the *production-exposure* projection is unchanged (NONE). For the cycle-3 verdict we treat the production-exposure projection as authoritative, hence the PASS_WITH_WARNINGS upgrade. F-SAST-1 remains tracked in `static_analysis.md` and is the one item blocking a clean PASS verdict for the workspace as a whole.

---

## Resolved findings (cycle 2 → cycle 3)

| ID | Title | Cycle-2 severity | Resolution | Where verified |
|----|-------|------------------|------------|----------------|
| F-DEP-1 | Vite Arbitrary File Read via Dev Server WebSocket (GHSA-p9ff-h696-f583) | HIGH | `bun update vite` landed in cycle-2 tail commit `f7dd6c9` ("[AZ-501] [AZ-502] Cycle 2 Step 14 security audit + inline fixes") | `bun audit` on `ui/` and `mission-planner/` both report **"No vulnerabilities found"** (re-run 2026-05-13 with bun 1.3.11) |
| F-DEP-2 | Vite Path Traversal in Optimized Deps `.map` (GHSA-4w7w-66w2-5vf9) | MODERATE | Same upgrade as F-DEP-1 | Same `bun audit` result |
| F-DEP-3 | PostCSS XSS via Unescaped `</style>` (GHSA-qx2v-qp2m-jg93) | MODERATE | Transitive close via `vite >= 6.4.2` | Same `bun audit` result |
| OWASP A06 status | Vulnerable & Outdated Components | FAIL (1 High + 2 Mod advisories) | All three advisories closed | `bun audit` clean — see above |
| OWASP A07 known-gap | "Bootstrap (cold-load) refresh missing `credentials:'include'`" — `src/auth/AuthContext.tsx:24` | (was the sole "PASS_WITH_KNOWN" qualifier) | **CLOSED by AZ-510** — bootstrap now POSTs with `credentials:'include'` and chains `GET /api/admin/users/me`. Same wire shape as the existing 401-retry path at `src/api/client.ts:88-99`. Module-scoped `bootstrapInflight` promise dedupes React 18 StrictMode dev double-mounts. | `src/auth/AuthContext.tsx:39-94`; regression test `src/auth/AuthContext.test.tsx` FT-P-01 (un-quarantined cycle 3); architecture-baseline B3 closure recorded in `_docs/02_document/architecture_compliance_baseline.md` |
| Static-check posture | STC-ARCH-01 (cross-component deep imports) — F3 carry-over exemption for `src/features/annotations/classColors.ts` | (procedural debt, not a security finding per se, but carried-forward "exception in static-check rules" is a defense-in-depth weakening) | **CLOSED by AZ-511** — `classColors` carved out to its own `src/class-colors/` component with a public barrel; STC-ARCH-01 exemption removed entirely (`scripts/check-arch-imports.mjs` `ARCH_IMPORTS_EXEMPT_RE = null`); regression test `tests/architecture_imports.test.ts` AC-4 inverted to assert deep imports now FAIL. | `_docs/02_document/architecture_compliance_baseline.md` Finding F3 closed |

## Updated OWASP Top 10 (2021) summary

Only categories whose status changed from cycle 2:

| # | Category | Cycle-2 status | Cycle-3 status | Driver |
|---|----------|----------------|----------------|--------|
| A06 | Vulnerable & Outdated Components | FAIL | **PASS** | All Vite/PostCSS advisories closed; `bun audit` clean; `bun audit` CI gate is still NOT in `.woodpecker/build-arm.yml` (carries over as F-INF-3 in `infrastructure_review.md`) |
| A07 | Identification & Authentication Failures | PASS_WITH_KNOWN | **PASS** | AZ-510 closed the only known gap (cold-load refresh missing `credentials:'include'`) |

Other 8 categories carry their cycle-2 status unchanged. See `owasp_review.md` for full evidence.

---

## New cycle-3 findings

### F-SAST-CY3-1 — Test-only bootstrap reset hook exposed via production `src/auth` barrel — LOW

| Field | Value |
|-------|-------|
| Severity | LOW |
| Category | Security Misconfiguration / hygiene |
| Location | `src/auth/AuthContext.tsx:35-37` (definition); `src/auth/index.ts` (re-export) |
| Introduced by | AZ-510 (commit `70fb452`) |

**Description**: `__resetBootstrapInflightForTests()` is a test-only escape hatch that clears the module-scoped `bootstrapInflight: Promise | null` guard so Vitest tests do not leak a never-resolving bootstrap promise into the next test. It is correctly named with the `__…ForTests` convention and JSDoc-tagged "Test-only", but it is exported through the `src/auth` public barrel (`src/auth/index.ts`) without a runtime guard. Any production code path could in principle import and invoke it.

**Why it was done that way**: The static architecture gate STC-ARCH-01 forbids `tests/setup.ts` from deep-importing into `src/auth/AuthContext` directly (cross-component deep import). The fix landed during AZ-510 implementation was to re-export the helper through the barrel so `tests/setup.ts` could import via `'../src/auth'`. This is the architecturally-correct path, but it widens the public surface.

**Impact**: Negligible practically — the function is intra-bundle-only (no network exposure), and its only effect is to clear a local cache (worst case forces a single extra `POST /api/admin/auth/refresh` round-trip on next mount). Not exploitable as a privilege-escalation, secret-leak, or DoS vector.

**Remediation options** (LOW — not blocking; tracked here for hygiene):
1. **Cheapest**: leave as-is. The `__…ForTests` naming + JSDoc is the de-facto convention in the React ecosystem and matches several other in-tree test hooks (e.g. `setNavigateToLogin` in `api/client.ts`).
2. **Conditional export**: wrap the helper body in `if (import.meta.env.MODE === 'test') { ... } else { throw new Error(...) }` so a production accidental call fails loudly. Requires a Vite env check; minor surface.
3. **Separate test-export module**: add `src/auth/test-hooks.ts` that re-exports `__resetBootstrapInflightForTests` and import that from `tests/setup.ts`. This keeps the public `src/auth` barrel clean. Cleanest but requires a one-off STC-ARCH-01 carve-out for the new file.

**Recommendation**: defer to a future hygiene cycle. Document as accepted in `security_approach.md` if it survives the next audit unchanged.

---

## Carried-over findings (NOT closed by cycle 3)

The following cycle-2 findings remain open and unchanged. Re-read `security_report.md` for full details.

| ID | Severity | Status | Notes |
|----|----------|--------|-------|
| F-SAST-1 | HIGH | **OPEN** | Google Geocode API key in `mission-planner/` port-source git history. Cycle 3 did not touch `mission-planner/`. Production-bundle exposure: NONE. The HIGH severity reflects the git-history layer (key still must be revoked + externalized). |
| F-SAST-2 | MEDIUM | OPEN | (per cycle-2 report) |
| F-SAST-3 | MEDIUM | OPEN | (per cycle-2 report) |
| F-SAST-4 | LOW | OPEN | (per cycle-2 report) |
| F-INF-1 | MEDIUM | OPEN | No SBOM emission |
| F-INF-2 | MEDIUM | OPEN | nginx missing CSP / X-Frame-Options / HSTS / Referrer-Policy / X-Content-Type-Options + log redaction |
| F-INF-3 | MEDIUM | OPEN | No `bun audit` step in `.woodpecker/build-arm.yml` — would have flagged the Vite advisory in CI |
| F-INF-4 | MEDIUM | OPEN | No image signing (cosign / docker content trust) |
| F-INF-5 | LOW | OPEN | (per cycle-2 report) |

**Cycle-3 commits did not touch nginx, Dockerfile, `.woodpecker/`, `e2e/`, `.env.example`, `mission-planner/.env.example`** — verified via `git diff --stat 70fb452^..HEAD` against those paths (empty diff). All infrastructure-level findings carry over verbatim.

---

## Phase-by-phase delta breakdown

### Phase 1 — Dependency Scan (delta)

- `bun audit` re-run on both roots (2026-05-13, bun 1.3.11): both report **"No vulnerabilities found"**.
- F-DEP-1, F-DEP-2, F-DEP-3 → all CLOSED.
- No `package.json` / `bun.lock` changes in cycle 3 (`git diff --stat 70fb452^..HEAD -- package.json bun.lock mission-planner/package.json mission-planner/bun.lock` empty). The closure happened in cycle-2 tail commit `f7dd6c9`; cycle 3 just confirms the result is durable.

### Phase 2 — Static Analysis (delta)

Cycle-3 source changes audited:

| File | Change | Security review |
|------|--------|-----------------|
| `src/auth/AuthContext.tsx` | `runBootstrap()` helper added (POST refresh + chained `/users/me`); `bootstrapInflight` module guard; `__resetBootstrapInflightForTests` test hook; defensive `user?.permissions?.includes(perm) ?? false` | Wire shape consistent with existing 401-retry path. `setToken(null)` precedes `setUser(null)` on every failure path (Constraint #4). `console.error('[AuthContext] Refresh succeeded but /users/me failed:', err)` — the err object originates from `api.get` which throws `new Error('${status}: ${text}')` (`api/client.ts:60`); the bearer is set via `setToken`, never embedded in errors → no bearer leak. The defensive permissions-check returns `false` on missing permissions array (secure default — deny rather than allow). One LOW-severity hygiene finding: F-SAST-CY3-1 above. |
| `src/auth/index.ts` | Added `__resetBootstrapInflightForTests` re-export | Drives F-SAST-CY3-1. |
| `src/api/endpoints.ts` | Added `usersMe: () => '/api/admin/users/me'` | Pure constant builder; no injection surface. STC-ARCH-02 maintained. |
| `tests/setup.ts` | Added `afterEach(() => { __resetBootstrapInflightForTests() })` | Test-environment only; not in production bundle. |
| `tests/msw/handlers/admin.ts` | `/users/me` mock now explicitly returns `permissions` | Test-environment mock; not in production bundle. |
| `src/auth/AuthContext.test.tsx` + 15 other `tests/*.test.tsx` files | GET → POST refresh mock swap | Test-environment mocks; not in production bundle. |
| `src/class-colors/classColors.ts` (renamed from `src/features/annotations/classColors.ts` via `git mv`) | Pure structural carve-out — content unchanged | Verified file is pure constants + arithmetic, no secrets, no I/O, no security surface. `git mv` preserved content. |
| `src/class-colors/index.ts` (new barrel) | Re-exports the four `classColors` symbols | Pure re-export; no security surface. |
| `src/features/annotations/index.ts` | Removed F3 carry-over comment block | Comment-only edit; no security impact. |
| `src/components/DetectionClasses.tsx`, `src/features/annotations/CanvasEditor.tsx`, `AnnotationsSidebar.tsx`, `AnnotationsPage.tsx`, `tests/detection_classes.test.tsx` | Import path swap (`'./classColors'` → `'../class-colors'` etc.) | Import-only edits; no behavioral change; no security impact. |
| `scripts/check-arch-imports.mjs` | `ARCH_IMPORTS_EXEMPT_RE = null` (exemption removed); `class-colors` added to `COMPONENT_DIRS` | Static-gate STRENGTHENED — no longer accepts deep imports of `classColors`. Defense-in-depth improvement. |
| `tests/architecture_imports.test.ts` | AC-4 inverted to assert deep imports FAIL | Stronger contract test. |

**No new injection / auth bypass / secret-handling / crypto / data-exposure findings.** The one new finding is the LOW hygiene item F-SAST-CY3-1.

### Phase 3 — OWASP Top 10 review (delta)

Two categories changed status; eight unchanged. See "Updated OWASP Top 10 (2021) summary" table above.

### Phase 4 — Infrastructure (delta)

`git diff --stat 70fb452^..HEAD -- nginx.conf Dockerfile .woodpecker/ e2e/ .env.example mission-planner/.env.example` is empty. Cycle 3 introduced no infrastructure changes; F-INF-1..F-INF-5 carry over unchanged.

---

## Recommendations (delta priority)

### Immediate (HIGH — pre-existing carry-over)

- **F-SAST-1**: revoke and externalize the Google Geocode API key in `mission-planner/` per the AZ-499 pattern (env var + fail-soft `null` when key unset). The key remains in real git history. *Not introduced by cycle 3 — carried-over priority from cycle 2.*

### Short-term (MEDIUM — pre-existing carry-over)

- **F-INF-3**: add `bun audit --high` exit-code gate to `.woodpecker/build-arm.yml`. Cycle 3 demonstrates exactly why this matters — the cycle-2 audit found Vite advisories that CI would have caught earlier had the gate existed. The cycle-3 `bun audit` clean result is durable today, but the next dep regression will silently ship without this gate.
- **F-INF-1**, **F-INF-2**, **F-INF-4**: SBOM, nginx security headers + log redaction, image signing — unchanged from cycle 2.

### Long-term (LOW)

- **F-SAST-CY3-1**: consider one of the three remediation options for the test-only bootstrap reset hook (see Finding above). Defer to a future hygiene cycle; not blocking.

---

## Self-verification

- [x] Cycle-3 source diff fully reviewed (all 8 production source files + 16 test files + 1 script + 1 test infra)
- [x] `bun audit` re-run on both roots (clean)
- [x] OWASP A07 gap re-rated against AZ-510 implementation, not just the spec
- [x] OWASP A06 gap re-rated against current `bun audit` output
- [x] Constraint #4 (clear bearer before user state) verified in code (`AuthContext.tsx:59`, `:87`)
- [x] Bearer-leak risk in new `console.error` calls traced through `api/client.ts:60` — confirmed no bearer in thrown Error
- [x] No infra files changed in cycle 3 — confirmed via git diff
- [x] AZ-512 (deferred) reviewed: no source changes shipped → no cycle-3 security surface
- [x] Cycle-2 artifacts NOT modified (resume mode); only this delta report + amendment note added

---

## Pointer back to baseline

Full cycle-2 baseline reports — kept verbatim as the security audit history of record:
- `security_report.md` (cycle 2 — 2026-05-12 — verdict FAIL)
- `dependency_scan.md`
- `static_analysis.md`
- `owasp_review.md`
- `infrastructure_review.md`

This delta report supersedes the **verdict** of `security_report.md` for the current state of the workspace; it does NOT supersede the baseline evidence in the four phase-specific files. A clean re-audit (Option A in the cycle-3 collision gate) was not selected — chose Option B (resume / delta-only).