mirror of
https://github.com/azaion/ui.git
synced 2026-06-21 08:11:10 +00:00
[AZ-510][AZ-511][AZ-512][AZ-513] Cycle 3 Steps 12-15 + admin prereq
ci/woodpecker/push/build-arm Pipeline failed
ci/woodpecker/push/build-arm Pipeline failed
Wrap up cycle 3 across the autodev existing-code Phase B steps that follow Implement (Steps 12-15), plus the cross-workspace prerequisite ticket filed for AZ-512. Step 12 - Test-Spec Sync: - Un-quarantine FT-P-01 in traceability-matrix (closed by AZ-510) - Add AZ-510 chained /users/me failure-path test reference under AC-23 - Note AZ-512 deferral status under O9 (P12 Phase B target) Step 13 - Update Docs (task mode): - Refresh src__auth__AuthContext module doc with AZ-510 wire shape (POST refresh + chained /users/me + bootstrapInflight guard) - Add usersMe() to src__api__endpoints module doc + consumer note - Rename src__features__annotations__classColors module doc to src__class-colors__classColors (matches AZ-511 git mv); refresh header - Refresh src__components__DetectionClasses + src__features__annotations module group doc for the new class-colors barrel import path - Update components/11_class-colors Module Inventory to point at the renamed module doc filename - Rewrite system-flows.md Flow F2 (Bearer auto-refresh) with the AZ-510 POST + chained /users/me sequence; close Finding B3 references - Generate ripple_log_cycle3 documenting all changed source files, their reverse-dependency search results, and the docs touched Step 14 - Security Audit (cycle-3 delta): - Resume mode against cycle-2 baseline; cycle-2 artifacts untouched - Re-run bun audit on both roots: clean (cycle-2 inline fix held) - Re-rate OWASP A06: FAIL -> PASS; A07: PASS_WITH_KNOWN -> PASS (B3 closed by AZ-510) - New finding F-SAST-CY3-1 (LOW): __resetBootstrapInflightForTests exposed via src/auth public barrel; defer to hygiene cycle - Verdict: FAIL -> PASS_WITH_WARNINGS; one HIGH (F-SAST-1 mission-planner git-history key, unchanged) remains - Add amendment banner to cycle-2 security_report.md Step 15 - Performance Test: - Static profile NFT-PERF-01 PASS (290 575 B gzipped vs 2 MB budget; ~14% of budget; no regression from AZ-510 surface additions) - E2E profile SKIP (Playwright perf project still pending AZ-457..AZ-482); legitimate skip per test-run skill, gap acknowledged in report - AZ-510 200ms p95 chain NFR verified at spec level only - no CI gate yet (covered by future AZ-457..AZ-482 work) Cross-workspace prerequisite (AZ-513 just filed): - Updated _docs/_process_leftovers/2026-05-13_az-512-admin-classes-prereq.md to reflect AZ-513 filing on admin/ workspace (parent epic AZ-509, Blocks link to AZ-512). Companion task spec added in admin/ repo (separate commit there, owned by admin/ workspace). State file: advanced to Step 16 (Deploy) per autodev existing-code flow. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -81,5 +81,5 @@ This *is* the helper. There are no further extensions inside this component.
|
||||
|
||||
| Path | Module Doc |
|
||||
|------|------------|
|
||||
| `src/class-colors/classColors.ts` | `_docs/02_document/modules/src__features__annotations__classColors.md` (path-derived name kept; content reflects the new home) |
|
||||
| `src/class-colors/classColors.ts` | `_docs/02_document/modules/src__class-colors__classColors.md` |
|
||||
| `src/class-colors/index.ts` | barrel — re-exports `getClassColor`, `getClassNameFallback`, `getPhotoModeSuffix`, `FALLBACK_CLASS_NAMES` |
|
||||
|
||||
@@ -20,6 +20,7 @@ export const endpoints = {
|
||||
authLogout: () => string
|
||||
users: () => string
|
||||
user: (id: string) => string
|
||||
usersMe: () => string // added 2026-05-13 by AZ-510 — chained read after POST refresh
|
||||
classes: () => string
|
||||
class: (id: string | number) => string
|
||||
},
|
||||
@@ -81,7 +82,7 @@ The whole object is `as const`, so each leaf's return type is the narrow string
|
||||
After the AZ-486 migration, `endpoints` is imported by:
|
||||
|
||||
- `src/api/client.ts` — internal `refreshToken()` helper uses `endpoints.admin.authRefresh()`.
|
||||
- `src/auth/AuthContext.tsx` — `authRefresh`, `authLogin`, `authLogout`.
|
||||
- `src/auth/AuthContext.tsx` — `authRefresh`, `authLogin`, `authLogout`, `usersMe` (added by AZ-510).
|
||||
- `src/components/FlightContext.tsx` — `flights.collection`, `flights.flight`, `annotations.settingsUser`.
|
||||
- `src/components/DetectionClasses.tsx` — `admin.classes`, `admin.class`.
|
||||
- `src/features/admin/AdminPage.tsx` — `admin.users`, `admin.user`.
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
# Module: `src/auth/AuthContext.tsx`
|
||||
|
||||
> **Source**: `src/auth/AuthContext.tsx` (54 lines)
|
||||
> **Source**: `src/auth/AuthContext.tsx` (~120 lines after AZ-510)
|
||||
> **Topo batch**: B3 (depends on B2 leaves: `api/client`, `types/index`)
|
||||
> **Last refresh**: 2026-05-13 — AZ-510 consolidated bootstrap onto POST refresh + chained `/users/me`; closes Vision P3 / Finding B3.
|
||||
|
||||
## Purpose
|
||||
|
||||
@@ -31,16 +32,30 @@ State:
|
||||
- `user: AuthUser | null` — `null` when unauthenticated.
|
||||
- `loading: boolean` — `true` until the initial refresh attempt resolves (success or failure). Renders should gate on this.
|
||||
|
||||
**Bootstrap effect (mount-only)**:
|
||||
**Bootstrap effect (mount-only)** — AZ-510 wire shape:
|
||||
|
||||
```ts
|
||||
api.get<{ user: AuthUser; token: string }>(endpoints.admin.authRefresh())
|
||||
.then(data => { setToken(data.token); setUser(data.user) })
|
||||
.catch(() => {})
|
||||
.finally(() => setLoading(false))
|
||||
async function runBootstrap(): Promise<AuthUser | null> {
|
||||
const refreshRes = await fetch(getApiBase() + endpoints.admin.authRefresh(), {
|
||||
method: 'POST',
|
||||
credentials: 'include',
|
||||
})
|
||||
if (!refreshRes.ok) return null
|
||||
const refreshData = (await refreshRes.json()) as { token: string }
|
||||
setToken(refreshData.token)
|
||||
try {
|
||||
return await api.get<AuthUser>(endpoints.admin.usersMe())
|
||||
} catch (err) {
|
||||
console.error('[AuthContext] Refresh succeeded but /users/me failed:', err)
|
||||
setToken(null)
|
||||
return null
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The refresh endpoint is invoked with `credentials: 'include'` only inside `client.ts`'s **internal** `refreshToken()` helper — but here we go through the public `api.get()` path, which does NOT include credentials. **This is a real divergence**: `client.ts`'s internal `refreshToken()` (used in the 401 retry) sends the cookie; the bootstrap call in `AuthContext` does not. The endpoint must therefore accept the refresh either via cookie (then bootstrap fails silently for non-cookie clients — which is everyone after a hard reload) **or** via some other mechanism (a refresh token in `localStorage`, etc.). **Flag for Step 4 verification** against the `admin/` service contract; this is likely a real bug masking by silent `.catch`. The path string itself is unaffected by AZ-486 — `endpoints.admin.authRefresh()` produces `'/api/admin/auth/refresh'` character-identically to the pre-refactor literal, so the divergence is structural, not URL-based.
|
||||
A module-scoped `bootstrapInflight: Promise | null` guard is consulted before invoking `runBootstrap`, so two concurrent `useEffect` mounts (React 18+ StrictMode dev double-mount, or rapid re-mount in tests) share a single network round-trip and avoid racing the backend's refresh-cookie rotation. A test-only escape hatch `__resetBootstrapInflightForTests()` is exported via the `src/auth` barrel and called in `tests/setup.ts`'s `afterEach` to keep the module-scoped promise from leaking between tests.
|
||||
|
||||
The bootstrap and the existing 401-retry path in `api/client.ts:73` now share a single wire shape — both POST `/api/admin/auth/refresh` with `credentials:'include'` and rely on the HttpOnly refresh cookie. The chained `GET /api/admin/users/me` request fetches the user payload (the POST refresh response is `{ token }` only). On any failure path (refresh 401, refresh network error, refresh 200 → `/users/me` 401, refresh 200 → `/users/me` network error) the bootstrap clears the bearer first then sets `user: null` + `loading: false`, so an in-flight re-render never sees `(user: null, accessToken: <stale>)`. Closes Vision principle P3 ("bearer in memory, refresh in HttpOnly cookie") and Finding B3.
|
||||
|
||||
**`login(email, password)`**:
|
||||
|
||||
@@ -60,7 +75,7 @@ setToken(null); setUser(null)
|
||||
|
||||
Network failure on logout is silently swallowed because we want to clear local auth state regardless.
|
||||
|
||||
**`hasPermission(perm)`**: returns `user?.permissions.includes(perm) ?? false`. The permission strings are not constrained at the type level — any string passes. Backend-defined.
|
||||
**`hasPermission(perm)`**: returns `user?.permissions?.includes(perm) ?? false`. Defensively handles legacy `/users/me` payloads that omit `permissions` (older backend builds; some test fixtures returning the bare `User` shape). Permission strings are not constrained at the type level — any string passes. Backend-defined; UI uses this only for affordance show/hide, never for security gates (the server is the authority — see `_docs/02_document/architecture.md` Vision P12 / O4).
|
||||
|
||||
## Dependencies
|
||||
|
||||
@@ -103,14 +118,11 @@ No env vars consumed directly — token storage policy is defined in `client.ts`
|
||||
|
||||
## Tests
|
||||
|
||||
None.
|
||||
`src/auth/AuthContext.test.tsx` — un-quarantined `FT-P-01` (bootstrap POST + `credentials:'include'` + chained `/users/me` regression guard); `FT-P-03` (refresh transparency, child re-render delta ≤ 1); `NFT-SEC-01` (bearer never in localStorage / sessionStorage across the full bootstrap + 401-retry lifecycle); `NFT-SEC-02` (no refresh-prefixed cookie visible via `document.cookie`); `AC-4 (AZ-510)` — POST refresh 200 → `/users/me` 401 clears the bearer + logs a diagnostic console.error.
|
||||
|
||||
## Notes / open questions
|
||||
|
||||
- **Bootstrap-vs-refresh divergence** (above) — the highest-priority flag in this module. Either:
|
||||
1. The refresh endpoint accepts an Authorization-less, cookie-bearing call → confirm the `admin/` service sets an HttpOnly cookie on `/login` and the cookie path matches `/api/admin/auth/refresh`. The `api.get()` path in `client.ts` does NOT send `credentials: 'include'`, so this currently CANNOT work. → **likely bug**.
|
||||
2. Or the bootstrap should be calling the internal `refreshToken()` helper, which is currently not exported.
|
||||
Either way, this needs a Step 4 fix (export `refreshToken()` and call it here, or change `api.get()` to allow per-call `credentials`).
|
||||
- ~~**Bootstrap-vs-refresh divergence**~~ — **RESOLVED 2026-05-13 by AZ-510**. Bootstrap now uses POST + `credentials:'include'` + chained `/users/me`, sharing the same wire shape as the 401-retry path. `api.get()` is intentionally NOT used for the refresh itself because it does not thread `credentials:'include'`; the bootstrap calls `fetch()` directly with the same explicit-credentials pattern documented in `api/client.ts:88`. Finding B3 closed.
|
||||
- **`AuthContext = createContext<AuthState>(null!)`**: the non-null assertion means `useAuth()` will throw at the destructuring site if it's used outside `AuthProvider`. Acceptable given `App.tsx` mounts `AuthProvider` at the top, but a guard `if (!ctx) throw new Error(...)` would be friendlier. Defer.
|
||||
- The `loading` flag is never re-set to `true` after the initial bootstrap. `login` and `logout` complete synchronously from the React tree's perspective (the `await` is inside the callback). If a future requirement demands a "logging in…" indicator, it would need its own state. Note for Step 8.
|
||||
- `useAuth` returns the raw context value (no memoisation wrapper). React 18+ behaviour means `<AuthProvider>` re-renders all `useAuth` consumers on every state update — fine here because there's no high-frequency state.
|
||||
|
||||
+3
-2
@@ -1,6 +1,7 @@
|
||||
# Module: `src/features/annotations/classColors.ts`
|
||||
# Module: `src/class-colors/classColors.ts`
|
||||
|
||||
> **Source**: `src/features/annotations/classColors.ts` (24 lines)
|
||||
> **Source**: `src/class-colors/classColors.ts` (24 lines; moved from `src/features/annotations/classColors.ts` by AZ-511 on 2026-05-13 — closes Finding F3)
|
||||
> **Public API barrel**: `src/class-colors/index.ts` re-exports `getClassColor`, `getClassNameFallback`, `getPhotoModeSuffix`, `FALLBACK_CLASS_NAMES`.
|
||||
> **Topo batch**: B1 (leaf — no internal imports)
|
||||
|
||||
## Purpose
|
||||
@@ -1,7 +1,8 @@
|
||||
# Module: `src/components/DetectionClasses.tsx`
|
||||
|
||||
> **Source**: `src/components/DetectionClasses.tsx` (99 lines)
|
||||
> **Topo batch**: B3 (depends on B2 leaves: `api/client`, `features/annotations/classColors`, `types/index`)
|
||||
> **Topo batch**: B3 (depends on B2 leaves: `api/client`, `class-colors` (via barrel), `types/index`)
|
||||
> **Last refresh**: 2026-05-13 — `getClassColor` + `FALLBACK_CLASS_NAMES` import migrated from `'../features/annotations/classColors'` to `'../class-colors'` barrel by AZ-511.
|
||||
|
||||
## Purpose
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Module group: `src/features/annotations/`
|
||||
|
||||
> Compact doc covering all 5 annotations modules (`classColors.ts` is a shared leaf — see existing `src__features__annotations__classColors.md`). The annotations feature is the **central legacy concern** of the codebase per `_docs/legacy/wpf-era.md §4` (`Azaion.Annotator` window) — what's documented here is the React port. For the canonical product spec see `_docs/ui_design/README.md` (Annotations Tab Layout, Annotation Quality Guidelines, Affiliation Icons, Combat Readiness, Annotation Row Gradient, Keyboard Shortcuts, Video Annotation Time-Window Display) and parent suite `../../../../_docs/01_annotations.md` for the API contract.
|
||||
> Compact doc covering the 4 annotations-feature modules. `classColors.ts` was carved out of this directory to its own component (`src/class-colors/`) by AZ-511 on 2026-05-13 — see `src__class-colors__classColors.md`; consumers in this feature now import via the `../../class-colors` barrel. The annotations feature is the **central legacy concern** of the codebase per `_docs/legacy/wpf-era.md §4` (`Azaion.Annotator` window) — what's documented here is the React port. For the canonical product spec see `_docs/ui_design/README.md` (Annotations Tab Layout, Annotation Quality Guidelines, Affiliation Icons, Combat Readiness, Annotation Row Gradient, Keyboard Shortcuts, Video Annotation Time-Window Display) and parent suite `../../../../_docs/01_annotations.md` for the API contract.
|
||||
|
||||
## Scope
|
||||
|
||||
@@ -20,7 +20,7 @@ Owns the `/annotations` route. Lets the user:
|
||||
|
||||
| Module | Layer | Responsibility |
|
||||
|---|---|---|
|
||||
| `classColors.ts` | leaf | (already documented separately) Class-number → colour + photoMode-suffix lookup. |
|
||||
| ~~`classColors.ts`~~ | (moved) | Carved out by AZ-511 to `src/class-colors/`; imported via the `class-colors` barrel by `CanvasEditor.tsx`, `AnnotationsSidebar.tsx`, `AnnotationsPage.tsx`. |
|
||||
| `MediaList.tsx` | sub-component | Left panel media browser. Owns `media[]` state, debounced filter, dropzone upload, blob: local-mode fallback when backend POST fails. Calls `endpoints.annotations.media(qs)`, `endpoints.annotations.mediaItem(id)` (DELETE), `endpoints.annotations.mediaBatch()` (POST). |
|
||||
| `VideoPlayer.tsx` | sub-component | Native `<video>` wrapper. `forwardRef` exposes `seek(seconds)` and `getVideoElement()`. Custom progress slider + frame-step toolbar. Global `keydown` handler for Space / ←/→ (Ctrl=±150) / M. Image / video bytes via `endpoints.annotations.mediaFile(id)`. |
|
||||
| `AnnotationsSidebar.tsx` | sub-component | Right panel: SSE-driven annotation list (`endpoints.annotations.annotationEvents()` filtered by `mediaId`), AI detect button (`endpoints.detect.media(mediaId)`), gradient row background built from per-detection class colour + confidence-modulated alpha, download button (delegates to page). |
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
# Documentation Ripple Log — Cycle 3
|
||||
|
||||
> Generated during Step 13 (Update Docs) of the autodev existing-code flow, cycle 3.
|
||||
> Task specs in scope:
|
||||
> - `_docs/02_tasks/done/AZ-510_auth_bootstrap_consolidation.md`
|
||||
> - `_docs/02_tasks/done/AZ-511_classcolors_carve_out.md`
|
||||
> - `_docs/02_tasks/backlog/AZ-512_admin_edit_detection_class.md` — DEFERRED at Step 10 (Implement) by the spec-defined Cross-Workspace Verification BLOCKING gate; no source code changes shipped, so no doc ripple from AZ-512.
|
||||
> Implementation reports: `_docs/03_implementation/batch_13_cycle3_report.md`, `_docs/03_implementation/batch_14_cycle3_report.md`, `_docs/03_implementation/batch_15_cycle3_report.md` (deferral record).
|
||||
|
||||
## Scope analysis (Task Step 0)
|
||||
|
||||
Direct source files changed by Cycle 3:
|
||||
|
||||
### AZ-510 — Auth bootstrap refresh consolidation
|
||||
|
||||
| Source file | Touched module / component / system doc |
|
||||
|---|---|
|
||||
| `src/auth/AuthContext.tsx` | `modules/src__auth__AuthContext.md` (this run — bootstrap rewrite, hasPermission defensive guard, AC-4 test reference); `components/02_auth/description.md` (refreshed by AZ-510 implementer at commit time) |
|
||||
| `src/auth/index.ts` | barrel-only edit (added `__resetBootstrapInflightForTests` re-export) — covered in module doc note for AuthContext; no separate barrel doc exists |
|
||||
| `src/api/endpoints.ts` | `modules/src__api__endpoints.md` (this run — added `usersMe()` row + AuthContext consumer note) |
|
||||
| `tests/setup.ts` | not part of `DOCUMENT_DIR/modules/` — covered by `tests/environment.md` (already documents global setup hooks; no signature change to declare) |
|
||||
| `tests/msw/handlers/admin.ts` | `tests/test-environment-msw-handlers.md` if present — checked: no specific module doc, MSW handlers are referenced from `tests/environment.md` at the table level only; permissions field addition does not change the MSW contract surface |
|
||||
| `src/auth/AuthContext.test.tsx` + 15 other `tests/*.test.tsx` files swapped GET→POST refresh mocks | covered by traceability matrix (Step 12) and module doc note |
|
||||
| Documentation already updated by the AZ-510 implementer at commit time (no second pass needed): `_docs/02_document/components/02_auth/description.md`, `_docs/02_document/architecture_compliance_baseline.md` (B3 closure), `_docs/02_document/04_verification_log.md` (B3 closure) |
|
||||
|
||||
### AZ-511 — `classColors` carve-out (`src/features/annotations/` → `src/class-colors/`)
|
||||
|
||||
| Source file | Touched module / component / system doc |
|
||||
|---|---|
|
||||
| `src/features/annotations/classColors.ts` → `src/class-colors/classColors.ts` (`git mv`) | `modules/src__features__annotations__classColors.md` → `modules/src__class-colors__classColors.md` (`git mv` this run) — header rewritten to point at new path + AZ-511 closure note |
|
||||
| `src/class-colors/index.ts` (NEW barrel) | listed in `components/11_class-colors/description.md` Module Inventory (refreshed this run to point at the renamed module doc) |
|
||||
| `src/features/annotations/index.ts` | barrel-only edit (removed F3 carry-over comment block) — no module doc change |
|
||||
| `src/features/annotations/CanvasEditor.tsx` | import-only change → `modules/src__features__annotations.md` Module Inventory note refreshed (this run) — no signature change |
|
||||
| `src/features/annotations/AnnotationsSidebar.tsx` | same — covered by the group doc refresh |
|
||||
| `src/features/annotations/AnnotationsPage.tsx` | same — covered by the group doc refresh |
|
||||
| `src/components/DetectionClasses.tsx` | `modules/src__components__DetectionClasses.md` (this run — topo-batch dependency line + last-refresh note) |
|
||||
| `tests/detection_classes.test.tsx` | covered by traceability matrix (Step 12); fixture-only import path swap, no behavior change |
|
||||
| `scripts/check-arch-imports.mjs` | static-gate infrastructure — `tests/static-checks.md` if present; checked: covered by `_docs/02_document/architecture_compliance_baseline.md` (refreshed by implementer) and `scripts/run-tests.sh` description block (refreshed by implementer) |
|
||||
| `tests/architecture_imports.test.ts` | `tests/static-checks.md` if present; covered by `_docs/02_document/architecture_compliance_baseline.md` Finding F3 closure (refreshed by implementer) |
|
||||
| Documentation already updated by the AZ-511 implementer at commit time (no second pass needed): `_docs/02_document/module-layout.md`, `_docs/02_document/components/11_class-colors/description.md`, `_docs/02_document/architecture_compliance_baseline.md` (F3 closure), `_docs/02_document/04_verification_log.md` (open questions #1, #8 closure), `scripts/run-tests.sh` description block |
|
||||
|
||||
## Import-graph ripple (Task Step 0.5)
|
||||
|
||||
Reverse-dependency search for the source files changed in cycle 3.
|
||||
|
||||
### AZ-510 ripple
|
||||
|
||||
- `src/auth/AuthContext.tsx` exports `useAuth`, `AuthProvider`, `__resetBootstrapInflightForTests`. All three are exposed via the `src/auth` barrel (per STC-ARCH-01 rules). Importers of `useAuth` / `AuthProvider`:
|
||||
- `src/auth/ProtectedRoute.tsx` — same-component import, no cross-component ripple.
|
||||
- `src/components/Header.tsx` — wire-shape unchanged (still calls `useAuth()`); no doc refresh required for the Header module doc.
|
||||
- `src/features/login/LoginPage.tsx` — wire-shape unchanged; no doc refresh required.
|
||||
- `src/App.tsx` — mounts `<AuthProvider>`; no doc refresh required.
|
||||
- `tests/setup.ts` — calls `__resetBootstrapInflightForTests` in `afterEach`; covered above.
|
||||
- `src/api/endpoints.ts` added `usersMe()`. Only consumer is `src/auth/AuthContext.tsx` (covered above). Searched for any other production import of `endpoints.admin.usersMe` — none.
|
||||
|
||||
### AZ-511 ripple
|
||||
|
||||
- `src/class-colors/classColors.ts` (formerly `src/features/annotations/classColors.ts`) exports 4 symbols. All importers re-routed to the new `src/class-colors` barrel by AZ-511 directly (covered in the AZ-511 table above):
|
||||
- `src/components/DetectionClasses.tsx`, `src/features/annotations/CanvasEditor.tsx`, `src/features/annotations/AnnotationsSidebar.tsx`, `src/features/annotations/AnnotationsPage.tsx`, `tests/detection_classes.test.tsx`.
|
||||
- No additional indirect importers found via `rg "from .*classColors"` and `rg "from .*class-colors"`.
|
||||
- `src/features/annotations/index.ts` barrel-only edit — no symbol surface change, no consumer ripple.
|
||||
|
||||
### Heuristic-mode fallback
|
||||
|
||||
Not needed — TypeScript import resolution succeeded for all changed files via `rg` with TS path patterns; no language-tooling failure.
|
||||
|
||||
## Module docs touched this run
|
||||
|
||||
- `_docs/02_document/modules/src__auth__AuthContext.md` (AZ-510)
|
||||
- `_docs/02_document/modules/src__api__endpoints.md` (AZ-510)
|
||||
- `_docs/02_document/modules/src__class-colors__classColors.md` (AZ-511 — renamed via `git mv` from `src__features__annotations__classColors.md`)
|
||||
- `_docs/02_document/modules/src__components__DetectionClasses.md` (AZ-511)
|
||||
- `_docs/02_document/modules/src__features__annotations.md` (AZ-511 — header note + Module Inventory row)
|
||||
- `_docs/02_document/components/11_class-colors/description.md` (AZ-511 — Module Inventory row updated to new doc filename)
|
||||
|
||||
## Component docs touched this run
|
||||
|
||||
None beyond the Module Inventory tweak in `11_class-colors/description.md` listed above. The substantive component-level updates for both tasks were made by their implementers at batch commit time (`02_auth/description.md`, `11_class-colors/description.md` Caveats §7, etc.) per scope discipline.
|
||||
|
||||
## System-level docs touched this run
|
||||
|
||||
- `_docs/02_document/system-flows.md` Flow F2 (Bearer auto-refresh) — rewrote the historical "two divergent paths" section, replaced the broken-bootstrap sequence diagram with the AZ-510 POST-refresh + chained `/users/me` flow, refreshed the Error Scenarios table to reflect the `runBootstrap()` failure modes (AC-4 (AZ-510) regression test reference). Finding B3 marked CLOSED.
|
||||
|
||||
## Problem-level docs touched this run
|
||||
|
||||
None. AZ-510 and AZ-511 are structural / wire-shape changes — no API input parameter, configuration, or acceptance-criteria change at the problem level. (AZ-512 would have touched `acceptance_criteria.md` O9 / Vision P12, but it was deferred — the deferral context is captured in the cycle-3 traceability-matrix update at Step 12.)
|
||||
|
||||
## Summary
|
||||
|
||||
```
|
||||
══════════════════════════════════════
|
||||
DOCUMENTATION UPDATE COMPLETE — Cycle 3
|
||||
══════════════════════════════════════
|
||||
Task(s): AZ-510, AZ-511 (AZ-512 deferred — no doc ripple)
|
||||
Module docs updated: 5 (1 renamed via git mv)
|
||||
Component docs updated: 1 (Module Inventory row only — substantive component refresh done by implementers at commit time)
|
||||
System-level docs updated: system-flows.md (Flow F2)
|
||||
Problem-level docs updated: none
|
||||
Ripple-refreshed docs (imports changed indirectly): 0 — all consumers covered by direct task scope
|
||||
══════════════════════════════════════
|
||||
```
|
||||
@@ -123,16 +123,18 @@ flowchart TD
|
||||
|
||||
---
|
||||
|
||||
## Flow F2: Bearer auto-refresh on 401 (TWO refresh paths exist in code)
|
||||
## Flow F2: Bearer auto-refresh (bootstrap + 401-retry)
|
||||
|
||||
> **Cycle 3 / 2026-05-13 — AZ-510 consolidated the two refresh paths.** The historical "two divergent paths" wording below has been rewritten. The previous bug (finding B3 / Vision P3 violation) is now CLOSED.
|
||||
|
||||
### Description
|
||||
|
||||
There are **two distinct refresh code paths** in the source — the verification pass (Step 4) caught both:
|
||||
There are two refresh trigger points in the source, but they now share a single wire shape:
|
||||
|
||||
1. **Bootstrap path** — `AuthContext.tsx:24` calls `api.get('/api/admin/auth/refresh')` on app mount. This **does NOT have `credentials:'include'`** because `api/client.ts` doesn't add it on GET. Result: the cookie is not sent, the bootstrap silently fails, the user starts unauthenticated even when they have a valid refresh cookie.
|
||||
2. **401-retry path** — `api/client.ts:44` calls `fetch('/api/admin/auth/refresh', { method: 'POST', credentials: 'include' })` automatically when any authenticated fetch returns 401. This path IS correct.
|
||||
1. **Bootstrap path** — `AuthContext.tsx` (`runBootstrap()` helper, guarded by a module-scoped `bootstrapInflight` promise to deduplicate React 18+ StrictMode dev double-mounts). On `<AuthProvider>` mount it calls `fetch(getApiBase() + endpoints.admin.authRefresh(), { method: 'POST', credentials: 'include' })`. On success it sets the bearer and **chains** `api.get<AuthUser>(endpoints.admin.usersMe())` (= `GET /api/admin/users/me`) to fetch the user record (the POST refresh response is `{ token }` only). On any failure path the bearer is cleared first, then `user: null` + `loading: false`.
|
||||
2. **401-retry path** — `api/client.ts:73` automatically calls `fetch('/api/admin/auth/refresh', { method: 'POST', credentials: 'include' })` and replays the original request when any authenticated fetch returns 401.
|
||||
|
||||
The bootstrap path is the bug surfaced as finding B3 PRIORITY. The 401-retry path is the silent fallback that does work but only after the user has already hit a 401.
|
||||
Both paths now POST with `credentials:'include'` and rely on the HttpOnly refresh cookie set on `/login`.
|
||||
|
||||
### Preconditions
|
||||
|
||||
@@ -157,7 +159,7 @@ sequenceDiagram
|
||||
ApiClient-->>Page: response
|
||||
```
|
||||
|
||||
### Sequence Diagram (Bootstrap path on app mount — broken)
|
||||
### Sequence Diagram (Bootstrap path on app mount — POST refresh + chained `/users/me`, AZ-510)
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
@@ -166,18 +168,25 @@ sequenceDiagram
|
||||
participant AdminApi as admin/ service
|
||||
|
||||
App->>AuthCtx: <AuthProvider> mounts
|
||||
AuthCtx->>AdminApi: GET /admin/auth/refresh (NO credentials:'include' — finding B3)
|
||||
AdminApi-->>AuthCtx: 401 (no cookie sent)
|
||||
AuthCtx->>AuthCtx: setLoading(false), user stays null
|
||||
AuthCtx-->>App: ProtectedRoute sees null user → redirects to /login
|
||||
AuthCtx->>AuthCtx: bootstrapInflight guard (StrictMode dedupe)
|
||||
AuthCtx->>AdminApi: POST /admin/auth/refresh (credentials:'include')
|
||||
AdminApi-->>AuthCtx: 200 {token} + Set-Cookie: refresh=...
|
||||
AuthCtx->>AuthCtx: setToken(token)
|
||||
AuthCtx->>AdminApi: GET /admin/users/me (Authorization: Bearer <token>)
|
||||
AdminApi-->>AuthCtx: 200 {id, email, permissions}
|
||||
AuthCtx->>AuthCtx: setUser(...), setLoading(false)
|
||||
AuthCtx-->>App: ProtectedRoute sees user → renders gated route
|
||||
```
|
||||
|
||||
### Error Scenarios
|
||||
|
||||
| Error | Where | Detection | Recovery |
|
||||
|-------|-------|-----------|----------|
|
||||
| Bootstrap GET refresh missing `credentials:'include'` | `AuthContext.tsx:24` | server returns 401 because cookie was not sent | **Bug today** — finding B3 PRIORITY. Symptom: a user with a valid refresh cookie still gets bounced to `/login` on every fresh tab. Step 4 fix: change to POST with `credentials:'include'` (matching the 401-retry path), or just delete the bootstrap GET and let the first authenticated fetch's 401 trigger the retry path. |
|
||||
| 401-retry path | `api/client.ts:44` | works | (no fix needed) |
|
||||
| ~~Bootstrap GET refresh missing `credentials:'include'`~~ | — | — | **CLOSED 2026-05-13 by AZ-510.** Bootstrap now POSTs with `credentials:'include'`. Finding B3 / Vision P3 violation resolved. |
|
||||
| Refresh 401 on bootstrap | `AuthContext.tsx` `runBootstrap()` | non-OK response from POST refresh | `setUser(null)` + `setLoading(false)` → `ProtectedRoute` redirects to `/login`. No console.error (expected on first visit / signed-out user). |
|
||||
| Refresh network error on bootstrap | `AuthContext.tsx` `runBootstrap()` | outer `.catch` on the POST refresh fetch | `setToken(null)` + `setUser(null)` + `setLoading(false)` + `console.error('[AuthContext] Bootstrap failed:', err)`. UI redirects to `/login`. |
|
||||
| Refresh 200 → `/users/me` failure (401, network, etc.) | `AuthContext.tsx` `runBootstrap()` | inner `try/catch` around `api.get(usersMe())` | `setToken(null)` first (Constraint #4 — bearer cleared before user state) + `console.error('[AuthContext] Refresh succeeded but /users/me failed:', err)` + return null → top-level then-handler sets `user: null` + `loading: false`. Covered by `AC-4 (AZ-510)` regression test. |
|
||||
| 401-retry path inside `api/client.ts` | `api/client.ts:73` | works | (no fix needed) |
|
||||
| Refresh cookie expired or revoked | refresh call | 401 | UI redirects to `/login`. |
|
||||
| SSE subscription holds a stale bearer | active EventSource | server closes the SSE stream | No reconnect logic today (Step 8 hardening). |
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ Maps every acceptance criterion and every restriction in `_docs/00_problem/` to
|
||||
|
||||
| AC ID | Acceptance Criterion (short) | Tests | results_report rows | Coverage |
|
||||
|-------|------------------------------|-------|---------------------|----------|
|
||||
| AC-01 | `credentials:'include'` on every authenticated fetch | FT-P-01 [Q for bootstrap], FT-P-02, NFT-PERF-02, NFT-SEC-04, NFT-RES-01, NFT-RES-08 | 01, 02, 03 | Covered |
|
||||
| AC-01 | `credentials:'include'` on every authenticated fetch | FT-P-01 (un-quarantined cycle 3 / 2026-05-13 by AZ-510 — bootstrap is now POST + `credentials:'include'` with chained `/users/me` per Vision P3; FT-P-01 runs as a regression guard on the wire shape), FT-P-02, NFT-PERF-02, NFT-SEC-04, NFT-RES-01, NFT-RES-08 | 01, 02, 03 | Covered |
|
||||
| AC-02 | Bearer never written to client storage | NFT-SEC-01 | 04 | Covered |
|
||||
| AC-03 | Refresh cookie `Secure HttpOnly SameSite=Strict` | NFT-SEC-02, NFT-SEC-03 | 05, 06, 07 | Covered |
|
||||
| AC-04 | Numeric enums match suite spec | FT-P-04, FT-P-05, FT-P-06 | 14, 15, 16, 17, 18, 19 | Covered (`enum_spec_snapshot.json` committed — Phase 3 gate resolved) |
|
||||
@@ -28,7 +28,7 @@ Maps every acceptance criterion and every restriction in `_docs/00_problem/` to
|
||||
| AC-20 | OpenWeatherMap key not in source | NFT-SEC-09 (all 3 steps active — source check un-quarantined on cycle 2 / 2026-05-12 by AZ-499) | 63 | Covered |
|
||||
| AC-21 | UserSettings panel-width persistence | FT-P-37 [Q], FT-P-38 [Q], NFT-PERF-08 [Q] | 64, 65 | Covered (quarantined) |
|
||||
| AC-22 | RBAC client-side route gates | FT-N-03 [Q], FT-N-04, FT-N-05 [Q], NFT-SEC-05 [Q], NFT-SEC-06 [Q], NFT-RES-08 | 08, 09, 10 | Covered (quarantined for `/admin` + `/settings` gates) |
|
||||
| AC-23 | Auth refresh transparency | FT-P-02, FT-P-03, NFT-PERF-02, NFT-RES-01 | 11, 12 | Covered |
|
||||
| AC-23 | Auth refresh transparency | FT-P-02, FT-P-03, NFT-PERF-02, NFT-RES-01; "AC-4 (AZ-510)" colocated test in `src/auth/AuthContext.test.tsx` covers the bootstrap edge where POST refresh succeeds but chained `/users/me` returns 401 → bearer cleared, console.error logged (added cycle 3 / 2026-05-13 by AZ-510) | 11, 12 | Covered |
|
||||
| AC-24 | SSE bearer-rotation reconnect | NFT-PERF-03 [Q], NFT-RES-02 [Q], NFT-RES-10 | 13, 97 | Covered (quarantined — Step 8 hardening) |
|
||||
| AC-25 | Detect endpoint correctness (sync + async) | FT-P-11, FT-P-12 [Q], FT-P-13 [Q] | 26, 27, 28 | Covered (async path quarantined — F7 target) |
|
||||
| AC-26 | Numeric input hygiene | FT-N-11 [Q], FT-N-12 [Q] | 66, 67 | Covered (quarantined — Step 4 fix) |
|
||||
@@ -96,7 +96,7 @@ Maps every acceptance criterion and every restriction in `_docs/00_problem/` to
|
||||
| O6 | No hardcoded credentials | NFT-SEC-09 | Covered |
|
||||
| O7 | Spec is source of truth for numeric enums | FT-P-04, FT-P-05, FT-P-06 | Covered |
|
||||
| O8 | Persist what you type (panel widths) | FT-P-37 [Q], FT-P-38 [Q] | Covered (quarantined) |
|
||||
| O9 | Admin can edit existing detection classes (P12) | NOT COVERED — feature missing today (`acceptance_criteria.md` notes P12 violation; PATCH endpoint to re-introduce in Phase B) | NOT COVERED — Phase B target |
|
||||
| O9 | Admin can edit existing detection classes (P12) | NOT COVERED — feature missing today (`acceptance_criteria.md` notes P12 violation; PATCH endpoint to re-introduce in Phase B). **Cycle 3 (2026-05-13)**: AZ-512 attempted this, but its spec-defined Cross-Workspace Verification BLOCKING gate failed — admin/ service exposes no /classes routes at all (not even the POST/DELETE that AdminPage already calls today). Task parked in `_docs/02_tasks/backlog/AZ-512_*.md` with leftover `_docs/_process_leftovers/2026-05-13_az-512-admin-classes-prereq.md` until the admin/ workspace ships POST + PATCH + DELETE /classes. | NOT COVERED — Phase B target (deferred; cross-workspace prerequisite outstanding) |
|
||||
| O10 | Destructive actions require ConfirmDialog | NFT-SEC-08, FT-P-26, FT-P-27, FT-N-07 | Covered |
|
||||
| O11 | No SSR / RSC | NFT-RES-LIM-03 (no Node in image) + STC-O11 (no `react-dom/server` import) | Partially Covered |
|
||||
| O12 | `mission-planner/` not compiled by production Vite build | NFT-RES-LIM-04 | Covered |
|
||||
@@ -108,7 +108,7 @@ Maps every acceptance criterion and every restriction in `_docs/00_problem/` to
|
||||
|
||||
| Category | Total Items | Covered | Partially Covered | Not Covered | N/A (meta) | Coverage % (Covered+Partial) |
|
||||
|----------|-------------|---------|-------------------|-------------|-----------|--------------------|
|
||||
| Acceptance Criteria | 44 | 44 | 0 | 0 | 0 | 100% (cycle-2 deltas: AC-41, AC-42, AC-43, AC-44 added; AC-20 source check no longer quarantined) |
|
||||
| Acceptance Criteria | 44 | 44 | 0 | 0 | 0 | 100% (cycle-2 deltas: AC-41, AC-42, AC-43, AC-44 added; AC-20 source check no longer quarantined. Cycle 3 deltas: FT-P-01 bootstrap part un-quarantined by AZ-510 — closes Vision P3 / Finding B3; AC-23 row gained the AZ-510 chained-`/users/me` failure-path test reference.) |
|
||||
| Anti-Criteria | 5 | 5 | 0 | 0 | 0 | 100% |
|
||||
| Restrictions | 41 | 17 | 8 | 13 | 3 | 61% |
|
||||
| **Total** | **90** | **66** | **8** | **13** | **3** | **82%** |
|
||||
@@ -132,11 +132,10 @@ Acceptance criterion coverage exceeds the 75 % template threshold. Restriction c
|
||||
|
||||
## Quarantine List (running)
|
||||
|
||||
The following 17 tests assert against a Phase B target or a Step 4 fix and are quarantined until the implementation lands. Phase 3 will decide their disposition. (Cycle 2 / 2026-05-12 update: NFT-SEC-09 source check REMOVED from this list — closed by AZ-499 + STC-SEC1C; new AC-41 / AC-42 tests added in this cycle are NOT quarantined.)
|
||||
The following 16 tests assert against a Phase B target or a Step 4 fix and are quarantined until the implementation lands. Phase 3 will decide their disposition. (Cycle 2 / 2026-05-12 update: NFT-SEC-09 source check REMOVED — closed by AZ-499 + STC-SEC1C; new AC-41 / AC-42 tests added in this cycle are NOT quarantined. Cycle 3 / 2026-05-13 update: FT-P-01 bootstrap part REMOVED — closed by AZ-510, runs as a regression guard now.)
|
||||
|
||||
| Test | Reason | Activates when |
|
||||
|------|--------|---------------|
|
||||
| FT-P-01 (bootstrap part) | Bootstrap refresh missing `credentials:'include'` per finding | Step 4 fix |
|
||||
| FT-P-12, FT-P-13 | Async video detect (F7) not wired | Phase B feature cycle |
|
||||
| FT-P-24, FT-P-25 | i18n detector + persistence missing | Step 4 fix |
|
||||
| FT-P-33 (timeout) | ProtectedRoute timeout missing | Step 4 fix |
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
# Security Audit Report — Azaion UI
|
||||
|
||||
> **AMENDMENT 2026-05-13 — verdict superseded by cycle-3 delta report.** See `_docs/05_security/security_report_cycle3_delta.md`. Current verdict (post AZ-510 + cycle-2-tail `bun update vite`): **PASS_WITH_WARNINGS** (was FAIL). All HIGH-severity dependency advisories closed; OWASP A06 → PASS, A07 → PASS. The HIGH-severity F-SAST-1 (`mission-planner/` Google Geocode API key in git history) remains open but does not affect the production browser bundle. The cycle-2 evidence below is preserved verbatim as the audit history of record.
|
||||
|
||||
**Date**: 2026-05-12
|
||||
**Scope**: `src/` (production SPA), `mission-planner/src/` (port-source — in git history but NOT in production bundle), `nginx.conf`, `Dockerfile`, `.woodpecker/build-arm.yml`, `e2e/` harness, `.env.example` files
|
||||
**Cycle**: Phase B / Cycle 2 (post AZ-498, AZ-499)
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
# 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).
|
||||
@@ -0,0 +1,105 @@
|
||||
# Performance Test Report — Cycle 3
|
||||
|
||||
**Date**: 2026-05-13
|
||||
**Cycle**: Phase B / Cycle 3 (post AZ-510, AZ-511; AZ-512 deferred and AZ-513 prerequisite filed on the admin/ workspace)
|
||||
**Runner**: `scripts/run-performance-tests.sh` (generated by test-spec Phase 4)
|
||||
**Mode**: static-only profile executed (NFT-PERF-01); e2e profile (NFT-PERF-02..10) records SKIP because the Playwright perf config is not yet wired (see "E2E profile status" below)
|
||||
**Verdict**: **PASS** (no Warn / Fail; one Pass + nine documented SKIPs + three documented Quarantines)
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
| Scenario | Result | Measured | Threshold | Source |
|
||||
|----------|--------|----------|-----------|--------|
|
||||
| NFT-PERF-01 (initial JS bundle, gzipped) | **PASS** | 290 575 B (≈ 284 KB) | ≤ 2 097 152 B (2 MB) — AC-11 / row 40 of `results_report.md` | `dist/assets/*.js` summed via `gzip -c \| wc -c` after `bun run build` |
|
||||
| NFT-PERF-02 (auth refresh round-trip p95) | SKIP | n/a | ≤ 200 ms — row 11 of `results_report.md` | Deferred — Playwright perf project not yet wired |
|
||||
| NFT-PERF-03 (SSE refresh rotation) | QUARANTINE | — | Step 8 hardening | Per script's static quarantine list |
|
||||
| NFT-PERF-04..07 | SKIP | n/a | per `performance-tests.md` | Deferred — Playwright perf project not yet wired |
|
||||
| NFT-PERF-08 (panel-width persistence) | QUARANTINE | — | Step 4 fix | Per script's static quarantine list |
|
||||
| NFT-PERF-09 (settings save error surfacing) | QUARANTINE | — | Step 4 fix | Per script's static quarantine list |
|
||||
| NFT-PERF-10 (FCP on /flights, warm-cache) | SKIP | n/a | ≤ 3 000 ms — row 98 of `results_report.md` | Deferred — Playwright perf project not yet wired |
|
||||
|
||||
**Per perf-mode gate logic** (`test-run` skill §Perf Mode step 5): only Warn or Fail block. No scenario reports either; the gate passes.
|
||||
|
||||
---
|
||||
|
||||
## What changed in cycle 3 vs the cycle-2 perf posture
|
||||
|
||||
### AZ-510 (auth bootstrap consolidation) — perf surface
|
||||
|
||||
The bootstrap path now does TWO sequential network calls on every cold mount:
|
||||
|
||||
1. `POST /api/admin/auth/refresh` (with `credentials:'include'`)
|
||||
2. `GET /api/admin/users/me` (chained, gated on the bearer set in step 1)
|
||||
|
||||
**Spec NFR budget** (from `_docs/02_tasks/done/AZ-510_auth_bootstrap_consolidation.md`): the chain must complete within **200 ms p95 on dev compose** — same nginx/auth/host topology as production. This is the same threshold NFT-PERF-02 measures (the cycle-2 test only measured the standalone refresh; cycle 3 implicitly extends the budget to cover the chain).
|
||||
|
||||
**Bundle-size impact**: the AZ-510 patch added one new endpoint builder (`endpoints.admin.usersMe()`), a `runBootstrap` helper, a module-scoped `bootstrapInflight` promise, the `__resetBootstrapInflightForTests` test hook, and a defensive `permissions?.includes` check. NFT-PERF-01 measured 290 575 B gzipped — well under the 2 MB threshold (~14% of budget). For comparison: the cycle-2 baseline measurement was not recorded in a comparable file, but the order of magnitude is unchanged. **No bundle regression.**
|
||||
|
||||
**Cold-mount p95 latency** (NFT-PERF-02): not measured this cycle because the e2e Playwright perf project is still pending (see below). The AZ-510 unit tests cover the wire-shape contract (FT-P-01 un-quarantined) but do not measure latency. **Coverage gap acknowledged**; closing it requires shipping the Playwright perf project (tracked under AZ-457..AZ-482).
|
||||
|
||||
### AZ-511 (classColors carve-out) — perf surface
|
||||
|
||||
Pure structural move + import-path swap. Function bodies unchanged. No bundle-size delta beyond noise (a second module file is now resolved, but tree-shaking eliminates any per-symbol overhead). **No measurable perf impact.**
|
||||
|
||||
### AZ-512 (deferred) — perf surface
|
||||
|
||||
No source code changes shipped. **No perf impact.**
|
||||
|
||||
---
|
||||
|
||||
## E2E profile status
|
||||
|
||||
The script's e2e profile (`NFT-PERF-02..10`) records SKIP for all scenarios because `e2e/playwright.perf.config.ts` does not exist yet. Quoting `scripts/run-performance-tests.sh:138`:
|
||||
|
||||
> `Awaiting NFT-PERF-* task implementations (AZ-457..AZ-482); until then the e2e perf scenarios are SKIPPED.`
|
||||
|
||||
This is a **legitimate skip** per the test-run skill's classification:
|
||||
|
||||
- ✅ Tracked: AZ-457..AZ-482 are the per-AC tasks that will produce the Playwright perf project.
|
||||
- ✅ Documented: the script itself names the skip rationale and the unblocking ticket range.
|
||||
- ✅ Not a "we didn't set something up" workaround — it is a "feature not yet implemented" pattern with a clear unblock path.
|
||||
- ❌ Coverage cost: NFT-PERF-02 (auth refresh ≤ 200ms p95) — directly relevant to AZ-510 — is therefore not measured this cycle.
|
||||
|
||||
**Recommendation for the next cycle**: prioritise one or more of AZ-457..AZ-482 specifically to deliver the Playwright perf project so NFT-PERF-02 can serve as the regression guard for AZ-510's bootstrap-chain latency.
|
||||
|
||||
Until then: AZ-510's latency is verified only at the spec-NFR level, not by an executable threshold check. The `console.error` diagnostic prefix on the chained `/users/me` failure path means a backend latency regression that pushes the chain over budget would still surface as a failure event in dev-tools console, but not as a CI gate.
|
||||
|
||||
---
|
||||
|
||||
## Quarantined scenarios (carry-over, unchanged in cycle 3)
|
||||
|
||||
These three are documentary-only in the script — they never gate today and have not been re-classified by cycle 3:
|
||||
|
||||
- **NFT-PERF-03** — SSE refresh rotation (deferred to Step 8 hardening — pre-existing).
|
||||
- **NFT-PERF-08** — panel-width persistence (deferred to Step 4 fix — pre-existing).
|
||||
- **NFT-PERF-09** — settings save error surfacing (deferred to Step 4 fix — pre-existing).
|
||||
|
||||
The NFT-PERF-09 quarantine is interesting in context: AZ-477 (cycle 2) added a Vitest-level test for the same 2 s error budget (`tests/settings_resilience.test.tsx`), which **passed** in the cycle 3 functional sanity run (231/231, 14.72 s total). So the *behaviour* the quarantined NFT-PERF-09 was meant to gate is now covered functionally; the perf-budget aspect remains deferred to the e2e Playwright project.
|
||||
|
||||
---
|
||||
|
||||
## Verdict
|
||||
|
||||
**PASS** for cycle 3. The single executable scenario (NFT-PERF-01) is well under threshold; all SKIPs are legitimate (Playwright perf project not yet wired, with a tracked unblock path); all QUARANTINES are pre-existing carry-overs.
|
||||
|
||||
**Coverage gap acknowledged**: AZ-510's bootstrap-chain latency (NFT-PERF-02 budget = 200 ms p95) is not executed by an automated gate. Closing this gap requires AZ-457..AZ-482 to ship the Playwright perf project.
|
||||
|
||||
---
|
||||
|
||||
## Self-verification
|
||||
|
||||
- [x] Static-only profile executed; exit code 0.
|
||||
- [x] All scenarios classified per `test-run` perf-mode step 4 (Pass / Warn / Fail / Unverified / SKIP / QUARANTINE).
|
||||
- [x] Each SKIP carries a documented rationale + tracked unblock path.
|
||||
- [x] AZ-510 perf surface explicitly addressed (bundle delta + acknowledged latency-gate gap).
|
||||
- [x] AZ-511 perf surface explicitly addressed (no measurable impact).
|
||||
- [x] AZ-512 perf surface explicitly addressed (deferred, no shipped code).
|
||||
- [x] Per-perf-mode gate logic applied: no Warn / Fail → return success.
|
||||
|
||||
## Pointer back
|
||||
|
||||
Raw runner summary: `test-output/performance-summary.txt`.
|
||||
Cycle 3 implementation report: `_docs/03_implementation/implementation_report_auth_classcolors_cycle3.md`.
|
||||
Cycle 3 security delta: `_docs/05_security/security_report_cycle3_delta.md`.
|
||||
@@ -2,8 +2,8 @@
|
||||
|
||||
## Current Step
|
||||
flow: existing-code
|
||||
step: 11
|
||||
name: Run Tests
|
||||
step: 16
|
||||
name: Deploy
|
||||
status: not_started
|
||||
sub_step:
|
||||
phase: 0
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
# 2026-05-13 — AZ-512 admin classes CRUD prerequisite (cross-workspace)
|
||||
|
||||
> **PARTIALLY RESOLVED 2026-05-13T03:51+02:00** — prerequisite ticket **AZ-513** was filed on the admin/ workspace (Jira Task, parent epic AZ-509, "Blocks" link to AZ-512). Matching task spec written to `admin/_docs/02_tasks/todo/AZ-513_classes_crud_routes.md`. AZ-512 carries a comment pointing at AZ-513. Replay obligation below now waits on AZ-513 shipping (admin/ side work), not on the autodev session itself.
|
||||
|
||||
## Summary
|
||||
|
||||
AZ-512 (Admin edit detection class) hit its spec-defined Cross-Workspace Verification BLOCKING gate during cycle 3 batch 15 implementation in the UI workspace. The `admin/` sibling service (Azaion.AdminApi) does not expose `/classes` routes at all. This leftover records (a) the deferred AZ-512 work in the UI, and (b) a separately-noted pre-existing bug discovered during verification.
|
||||
@@ -12,7 +14,7 @@ AZ-512 (Admin edit detection class) hit its spec-defined Cross-Workspace Verific
|
||||
|
||||
1. **AZ-512 implementation (UI workspace)** — the inline edit form + `PATCH /api/admin/classes/{id}` wiring on `src/features/admin/AdminPage.tsx`. Task parked in `_docs/02_tasks/backlog/AZ-512_admin_edit_detection_class.md`.
|
||||
|
||||
2. **Cross-workspace prerequisite ticket** — a NEW ticket needs to be filed on the `admin/` workspace's tracker (Jira project AZ, but linked under the admin/ module's epic) to add **POST + PATCH + DELETE `/classes` routes** to `Azaion.AdminApi/Program.cs`. This leftover does NOT itself create the ticket because the `user-atlassian-mcp` cannot reliably scope a ticket to the admin/ workspace's epic from the UI workspace's autodev session — it requires user judgment on epic linkage and ownership.
|
||||
2. ~~**Cross-workspace prerequisite ticket**~~ — **FILED as AZ-513 on 2026-05-13** with user-confirmed epic linkage (AZ-509). Spec at `admin/_docs/02_tasks/todo/AZ-513_classes_crud_routes.md`. "Blocks" link AZ-513 → AZ-512 created in Jira. Comment on AZ-512 references AZ-513. Pending: admin/ team picks up and ships AZ-513.
|
||||
|
||||
## Prerequisite payload (for the user to file)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user