diff --git a/_docs/02_document/architecture.md b/_docs/02_document/architecture.md index 7303027..6e72c5f 100644 --- a/_docs/02_document/architecture.md +++ b/_docs/02_document/architecture.md @@ -269,7 +269,7 @@ contract beautifully and accessibly". | `06_annotations/AnnotationsSidebar` | `annotations/`, `detect/` | REST + SSE | Request-Response + Event | `POST /api/detect/${mediaId}` (sync detect — used for BOTH images and videos today); `createSSE('/api/annotations/annotations/events', ...)` for **annotation-status SSE** (NOT detect progress). **No `/api/detect/video/${id}` and no `/api/detect/stream/${jobId}` are wired today** — finding #10 / #21 confirmed. | | `06_annotations/CanvasEditor` | `annotations/` | static asset GET | — | `GET /api/annotations/annotations/${id}/image` (annotation thumbnail), `GET /api/annotations/media/${id}/file` (raw media). | | `07_dataset/DatasetPage` | `annotations/` | REST | Request-Response | `GET /api/annotations/dataset?...`, `GET /api/annotations/dataset/${annotationId}`, `POST /api/annotations/dataset/bulk-status`, **`GET /api/annotations/dataset/class-distribution`** (the endpoint **already exists**; the chart UI is what's missing — see `01_legacy_coverage_gaps.md`), ``. **Editor tab does not save** — finding #4. | -| `08_admin/AdminPage` | `annotations/` + `admin/` + `flights/` | REST | Request-Response | `GET /api/annotations/classes` (read), `POST /api/admin/classes` (create), `DELETE /api/admin/classes/${id}` (delete — no ConfirmDialog, finding B4), `POST /api/admin/users`, `PATCH /api/admin/users/${id}` (deactivate), `GET /api/flights/aircrafts`, `PATCH /api/flights/aircrafts/${id}`. **Cross-service reads** — admin page reads aircraft from `flights/` and classes from `annotations/`. | +| `08_admin/AdminPage` | `annotations/` + `admin/` + `flights/` | REST | Request-Response | `GET /api/annotations/classes` (read), `POST /api/admin/classes` (create), **`PATCH /api/admin/classes/${id}` (update — AZ-512 inline edit; full body always sent per Risk-2 mitigation; live deploy gates on `admin/` AZ-513)**, `DELETE /api/admin/classes/${id}` (delete — no ConfirmDialog, finding B4), `POST /api/admin/users`, `PATCH /api/admin/users/${id}` (deactivate), `GET /api/flights/aircrafts`, `PATCH /api/flights/aircrafts/${id}`. **Cross-service reads** — admin page reads aircraft from `flights/` and classes from `annotations/`. | | `09_settings/SettingsPage` | `annotations/` + `flights/` | REST | Request-Response | `GET/PUT /api/annotations/settings/system`, `GET/PUT /api/annotations/settings/directories`, `GET /api/flights/aircrafts`, `PATCH /api/flights/aircrafts/${id}`. **Settings endpoints route to `annotations/`**, NOT `admin/` as initially drafted. | | `05_flights/FlightsPage` | `flights/` | REST + SSE | Request-Response + Event | `GET /api/flights/aircrafts`, `GET /api/flights/${id}/waypoints`, **`createSSE('/api/flights/${id}/live-gps', ...)` — live-GPS SSE for aircraft telemetry**, `POST /api/flights`, `DELETE /api/flights/${id}`, `DELETE /api/flights/${id}/waypoints/${wpId}` (loop), `POST /api/flights/${id}/waypoints` (loop, lossy shape — finding #20). | | `05_flights/flightPlanUtils` | OpenWeatherMap (external) | REST | Request-Response | `GET https://api.openweathermap.org/data/2.5/onecall?...` with env-resolved key + base URL since AZ-448 / AZ-449 (closes the original security finding; see AC-20 + AC-42). | diff --git a/_docs/02_document/modules/src__features__admin__AdminPage.md b/_docs/02_document/modules/src__features__admin__AdminPage.md index 01a343d..2ea0937 100644 --- a/_docs/02_document/modules/src__features__admin__AdminPage.md +++ b/_docs/02_document/modules/src__features__admin__AdminPage.md @@ -1,7 +1,8 @@ # Module: `src/features/admin/AdminPage.tsx` -> **Source**: `src/features/admin/AdminPage.tsx` (209 lines) +> **Source**: `src/features/admin/AdminPage.tsx` > **Topo batch**: B4 (depends on B3: `api/client`, `components/ConfirmDialog`, `types/index`) +> **Cycle 4 update (2026-05-13, AZ-512)**: gained an inline "edit detection class" affordance — see the new state slots, the `handleStartEdit / handleCancelEdit / handleUpdateClass / handleEditKeyDown` handlers, the PATCH row in the External integrations table, the new i18n keys consumed, and the FT-P-62 / FT-N-18 entries under Tests. Closes Architecture Vision principle **P12** (Objective O9 in `tests/traceability-matrix.md`). Implementation shipped against MSW stubs under the user-authorized Option B path; the live deploy gate remains until AZ-513 ships on the `admin/` workspace. ## Purpose @@ -37,6 +38,16 @@ No props. Reads everything via `api/client` and local state. 'Annotator' }`). - `deactivateId: string | null` — drives the `ConfirmDialog`'s open state for user deactivation. + - `editingId: number | null` — id of the detection class currently + in inline-edit mode (AZ-512). A single value, not per-row, so + opening one row's editor closes any other (AC-2 single-row + invariant / Risk 3 mitigation). + - `editForm: { name; shortName; color; maxSizeM }` — the inline-edit + staging buffer; seeded from the row on edit-start. + - `editError: 'nameRequired' | 'maxSizeMustBePositive' | 'updateFailed' | null` — + discriminated error kind rendered as an inline `role="alert"`. + - `editSaving: boolean` — disables Save + Cancel while the PATCH is + in flight (Risk 4 mitigation). - **Bootstrap effect** (`useEffect([])` — runs once at mount): ```ts @@ -68,6 +79,30 @@ No props. Reads everything via `api/client` and local state. ConfirmDialog** despite this being destructive. Inconsistent with the user-deactivation flow which uses ConfirmDialog. Flag for Step 4 against `_docs/ui_design/README.md` confirmation-dialog spec. +- **`handleStartEdit(c)`** (AZ-512): sets `editingId = c.id`, seeds + `editForm` from `c`, clears `editError`. Triggered by the per-row + pencil (✎) affordance. +- **`handleCancelEdit()`** (AZ-512): clears `editingId`, `editError`, + `editSaving`. No network call. Also fires on **Escape** inside the + form (AC-4). +- **`handleUpdateClass()`** (AZ-512): + 1. Guard: `editingId !== null && !editSaving`. + 2. Validation: `editForm.name.trim()` non-empty (else + `setEditError('nameRequired')`); `editForm.maxSizeM > 0` (else + `setEditError('maxSizeMustBePositive')`). Both pre-empt the + network call (AC-5). + 3. `setEditSaving(true)`. + 4. `await api.patch(endpoints.admin.class(editingId), editForm)` — + **the complete `editForm` is always sent** (Risk 2 mitigation: + the backend's partial-merge vs full-replace semantics become + equivalent for the UI). + 5. On success: `await api.get(endpoints.annotations.classes())`, + `setClasses(...)`, `setEditingId(null)`. + 6. On failure: `setEditError('updateFailed')` — form stays open, + edits intact, NO `alert()` (Finding B4 anti-pattern). +- **`handleEditKeyDown(e)`** (AZ-512): Enter → `handleUpdateClass`; + Escape → `handleCancelEdit`. Wired at the container level so any + input in the form respects it. - **`handleAddUser()`** — analogous to `handleAddClass` against `POST endpoints.admin.users()` and `GET endpoints.admin.users()` (both → `/api/admin/users`). Guards on `email && password`. @@ -85,6 +120,11 @@ No props. Reads everything via `api/client` and local state. the UI does not). - **Layout** (left → center → right, all in one horizontal flex): - **Left column** (`w-[340px]`): detection-classes table + add row. + Each read-only row carries a pencil (✎) edit button and a `×` + delete button (AZ-512). When `c.id === editingId`, that row's + cells collapse into a single `colspan=3` form holding name / + shortName / color / maxSizeM inputs + Save + Cancel (with an + inline `role="alert"` directly below on validation/server error). - **Center column** (`flex-1 max-w-md`): AI settings form, GPS settings form, users table + add row. The AI and GPS forms have `defaultValue` only — there is **no** state, no `Save` handler @@ -115,10 +155,15 @@ backend assigns `id` and other server-managed fields. ## Configuration -- **i18n keys consumed**: `admin.classes`, `admin.aiSettings`, +- **i18n keys consumed**: `admin.classes.title` (was flat + `admin.classes` pre-AZ-512), `admin.classes.edit`, + `admin.classes.save`, `admin.classes.cancel`, + `admin.classes.nameRequired`, `admin.classes.maxSizeMustBePositive`, + `admin.classes.updateFailed`, `admin.aiSettings`, `admin.gpsSettings`, `admin.users`, `admin.aircrafts`, `admin.deactivate`, `common.save`. (Confirmed present in - `src/i18n/en.json` admin/common groups.) Plenty of hardcoded + `src/i18n/en.json` admin/common groups; ua mirror enforced by the + FT-P-22 parity gate.) Plenty of hardcoded English strings — placeholders ("Name", "Email", "Password"), table headers (`#`, `Name`, `Color`, `Email`, `Role`, `Status`), role options (`Annotator`, `Admin`, `Viewer`), the GPS protocol options @@ -143,6 +188,7 @@ backend assigns `id` and other server-managed fields. |---|---|---| | `GET` | `endpoints.annotations.classes()` → `/api/annotations/classes` | List detection classes (read path uses annotations service) | | `POST` | `endpoints.admin.classes()` → `/api/admin/classes` | Create detection class (write path uses admin service) | +| `PATCH` | `endpoints.admin.class(id)` → `/api/admin/classes/{id}` | Update detection class (AZ-512 — full body always sent; same URL as DELETE, no new endpoint helper introduced per task constraint) | | `DELETE` | `endpoints.admin.class(id)` → `/api/admin/classes/{id}` | Delete detection class | | `GET` | `endpoints.flights.aircrafts()` → `/api/flights/aircrafts` | List aircraft | | `PATCH` | `endpoints.flights.aircraft(id)` → `/api/flights/aircrafts/{id}` | Toggle `isDefault` | @@ -175,7 +221,19 @@ Path builders live in `src/api/endpoints.ts` (since AZ-486 / F7). Routed by `ngi ## Tests -None. +- `tests/admin_class_edit.test.tsx` (cycle 4, AZ-512) — 12 cases + covering AC-1 through AC-6 + AC-8; AC-7 covered by the static + FT-P-22 i18n parity gate. Traces to FT-P-62 + FT-N-18 in + `_docs/02_document/tests/blackbox-tests.md`. +- `tests/destructive_ux.test.tsx` (cycle 1) — AZ-466 class-delete + destructive-UX `it.fails()` + control pair. Updated cycle 4 to + target the `×` delete button by text after the AZ-512 ✎ button + was added to the same row's action cell. + +No dedicated `AdminPage` happy-path test predates AZ-512; the AC-8 +regression guard in `admin_class_edit.test.tsx` covers Add and +Delete inline. A broader AdminPage test fixture is a Phase B +candidate. ## Notes / open questions diff --git a/_docs/02_document/tests/blackbox-tests.md b/_docs/02_document/tests/blackbox-tests.md index 68db0ee..fd61a8c 100644 --- a/_docs/02_document/tests/blackbox-tests.md +++ b/_docs/02_document/tests/blackbox-tests.md @@ -1649,6 +1649,50 @@ The scenarios below were appended via `/test-spec` cycle-update mode after Phase --- +### FT-P-62: AdminPage class edit — inline form + PATCH wire contract + refresh + +**Traces to**: O9 (P12) — landed cycle 4 / 2026-05-13 by AZ-512. +**Profile**: fast + +**Input data**: an `` mount with at least one detection class loaded via `GET /api/annotations/classes`; the user activates the row's edit (✎) affordance. + +**Steps**: + +| Step | Consumer Action | Expected System Response | +|------|----------------|------------------------| +| 1 | Inspect each rendered row | One edit (✎) button per class row (AC-1) | +| 2 | Click the edit (✎) on row N | Row N replaces its read-only cells with editable `name` / `shortName` / `color` / `maxSizeM` inputs seeded with the row's current values; Save + Cancel buttons appear; no other row is in edit mode (AC-2 single-row invariant) | +| 3 | Click edit (✎) on row M while row N is editing | Row N reverts to read-only; row M enters edit mode | +| 4 | Modify `name` and click **Save** (or press **Enter** inside the form) | Exactly one `PATCH /api/admin/classes/{N}` is observed with body `{ name, shortName, color, maxSizeM }` (full body per Risk-2 mitigation); on 200/2xx `` re-fetches via `GET /api/annotations/classes` and row N re-renders read-only with the new values (AC-3) | + +**Pass criteria**: zero PATCH calls before step 4; exactly one PATCH in step 4 with the complete editable shape; URL pattern `^/api/admin/classes/\d+$`; success-path refresh observed via the existing `GET /api/annotations/classes` builder (no new endpoint introduced — `endpoints.admin.class(id)` reused per task constraint). +**Max execution time**: 5s. +**Expected result source**: `_docs/02_tasks/done/AZ-512_admin_edit_detection_class.md` AC-1..AC-3. + +--- + +### FT-N-18: AdminPage class edit — error paths (Cancel, validation, 5xx) + +**Traces to**: O9 (P12), O10 (B4 anti-pattern: no `alert()`) — landed cycle 4 / 2026-05-13 by AZ-512. +**Profile**: fast + +**Input data**: `` mounted with at least one class loaded; the row's edit form is open. + +**Steps**: + +| Step | Consumer Action | Expected System Response | +|------|----------------|------------------------| +| 1 | Modify any field; click **Cancel** (or press **Escape** in the form) | Zero PATCH observed; row reverts to original read-only values (AC-4) | +| 2 | Clear `name`; click Save | Zero PATCH observed; inline `role="alert"` element renders `admin.classes.nameRequired` (en / ua localized) (AC-5) | +| 3 | Set `maxSizeM ≤ 0` or NaN; click Save | Zero PATCH observed; inline `role="alert"` renders `admin.classes.maxSizeMustBePositive` (AC-5) | +| 4 | Stub PATCH to return 500; click Save with valid fields | Exactly one PATCH observed (counterpart to FT-P-62 step 4); form stays open with the user's edits intact; inline `role="alert"` renders `admin.classes.updateFailed`; `window.alert` is NEVER called (AC-6 — Finding B4 anti-pattern enforced) | + +**Pass criteria**: every error path produces exactly the documented network footprint and exactly the documented inline error key; `window.alert` is spied and asserted-zero across the entire scenario (the STC-SEC7 static check independently guards the no-`alert()` invariant in production source). +**Max execution time**: 10s. +**Expected result source**: `_docs/02_tasks/done/AZ-512_admin_edit_detection_class.md` AC-4 / AC-5 / AC-6. + +--- + ## Notes carried into Phase 3 - All tests tagged `quarantined` correspond to features either pending a Step 4 fix (e.g., AC-13 i18n detector, AC-21 panel persistence, AC-22 role-gate, AC-26/27 form hygiene, AC-39 split surface, AC-40 tile zoom) or pending Phase B implementation (AC-11 bundle gate, AC-24 SSE refresh, AC-25 async video, AC-40 tile zoom). The test is written so it activates the day the implementation lands; Phase 3 will surface them for downgrade or accept. diff --git a/_docs/02_document/tests/traceability-matrix.md b/_docs/02_document/tests/traceability-matrix.md index 0a9cbdf..e4a3e6b 100644 --- a/_docs/02_document/tests/traceability-matrix.md +++ b/_docs/02_document/tests/traceability-matrix.md @@ -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). **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) | +| O9 | Admin can edit existing detection classes (P12) | FT-P-62, FT-N-18 — landed cycle 4 / 2026-05-13 by AZ-512 (UI-side; user-authorized Option B path — implementation shipped against MSW stubs). **Live deploy gate remains** until AZ-513 ships on `admin/` and is deployed: `POST | PATCH | DELETE /classes` is verified-missing on the live admin service today; leftover `_docs/_process_leftovers/2026-05-13_az-512-admin-classes-prereq.md` stays open until then. | Covered (UI implementation + stub-tested); cross-workspace deploy gate pending AZ-513 on `admin/` | | 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 | diff --git a/_docs/05_security/security_report.md b/_docs/05_security/security_report.md index 3e33547..dc6a326 100644 --- a/_docs/05_security/security_report.md +++ b/_docs/05_security/security_report.md @@ -1,6 +1,8 @@ # 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. +> +> **AMENDMENT 2026-05-13 (cycle 4 — AZ-512)** — see `_docs/05_security/security_report_cycle4_delta.md`. Verdict carries: **PASS_WITH_WARNINGS** (unchanged). One new LOW finding (F-SAST-CY4-1 — lost-update / mid-air-collision admission on `PATCH /api/admin/classes/{id}`, by design per AZ-512 spec). No new dependencies; `bun audit` re-run clean. Implementation shipped against MSW stubs under user-authorized Option B; deploy gate to live admin/ stays open until AZ-513 lands. **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 diff --git a/_docs/05_security/security_report_cycle4_delta.md b/_docs/05_security/security_report_cycle4_delta.md new file mode 100644 index 0000000..a5d90b3 --- /dev/null +++ b/_docs/05_security/security_report_cycle4_delta.md @@ -0,0 +1,188 @@ +# Security Audit — Cycle 4 Delta Report + +**Date**: 2026-05-13 +**Mode**: Resume / incremental — cycle-2 base (`security_report.md` + companion artifacts) plus cycle-3 delta (`security_report_cycle3_delta.md`) are kept verbatim; this report records ONLY the deltas introduced by cycle 4. +**Cycle**: Phase B / Cycle 4 (AZ-512 only — `admin/` AZ-513 prerequisite still un-shipped; UI implemented under user-authorized Option B against MSW stubs) +**Scope of delta**: cycle-4 commits only — `ef56d9c` (AZ-512 reactivation chore), `ecacfa8` (AZ-512 implementation batch 16). No infrastructure / CI / nginx / Dockerfile changes; no new dependencies; no new external surface; no new secrets. +**Verdict (post-cycle-4)**: **PASS_WITH_WARNINGS** — unchanged from cycle 3. One new LOW finding documented (F-SAST-CY4-1 — lost-update / mid-air-collision admission on PATCH). All cycle-3 carries remain unchanged. + +--- + +## Verdict change + +| Verdict component | Cycle 3 (2026-05-13 — pre-cycle-4) | Cycle 4 (2026-05-13 — post AZ-512) | Driver | +|-------------------|------------------------------------|------------------------------------|--------| +| Overall | PASS_WITH_WARNINGS | PASS_WITH_WARNINGS | No change in severity ceiling | +| Critical | 0 | 0 | — | +| High | 1 carried (F-SAST-1 — Google Geocode key in `mission-planner/` git history; production-bundle exposure NONE) | 1 carried (unchanged) | User-action gate: key revocation still pending | +| Medium | 7 carried | 7 carried (unchanged) | No cycle-4 changes to CI / nginx / Dockerfile | +| Low | 3 carried | 4 (new: F-SAST-CY4-1) | New lost-update admission on `PATCH /api/admin/classes/{id}` | + +--- + +## Cycle 4 scope — exactly what changed and what each change can / cannot affect + +| File | Domain | Security-relevant? | Why / why not | +|------|--------|--------------------|----------------| +| `src/features/admin/AdminPage.tsx` | Production source | Yes — adds a new wire call to `PATCH /api/admin/classes/{id}` and a new client-side validation path. | See finding F-SAST-CY4-1 below + carry analysis. No new credentials, no new external surface, no string interpolation in URL (`endpoints.admin.class(id)` builder is unchanged from cycle 2). | +| `src/i18n/en.json`, `src/i18n/ua.json` | Production source | No | New translation keys are static strings rendered through React (auto-escaped). No interpolation of untrusted input. | +| `tests/admin_class_edit.test.tsx` | Test-only | No | Vitest fixture; never shipped. | +| `tests/msw/handlers/admin.ts` | Test-only | No | MSW worker; never shipped. `Dockerfile` final stage is `nginx:alpine` serving `dist/`. | +| `tests/destructive_ux.test.tsx` | Test-only | No | Selector-target fix; logic unchanged. | +| `_docs/02_document/**/*.md`, `_docs/03_implementation/**/*.md` | Documentation | No | Documentation only. | + +> **No new package added, no version bumped.** `bun audit` re-run 2026-05-13 against `ui/` reports **"No vulnerabilities found"** (bun 1.3.11). The cycle-3 OWASP A06 PASS verdict carries forward. + +--- + +## Resolved findings (cycle 3 → cycle 4) + +**None.** Cycle 4 did not close any prior finding. + +| Pending user-action items (carried for visibility) | +|---------------------------------------------------| +| F-SAST-1 — Google Geocode API key in `mission-planner/` git history → user-action: revoke at GCP credentials console + externalize via `VITE_GOOGLE_GEOCODE_KEY` (AZ-499 pattern). | +| OpenWeatherMap key revocation — recorded in cycle-2 retrospective; the **AZ-449** code-side fix shipped but the **revocation of the previously committed key** is still a pending user action. | + +These two pending revocations are visible in `_docs/06_metrics/retro_2026-05-12.md` and the `_docs/_process_leftovers/` set; they were not in scope for AZ-512 and remain open. + +--- + +## New cycle-4 findings + +### F-SAST-CY4-1 — Lost-update / mid-air-collision on PATCH `/api/admin/classes/{id}` — LOW + +| Field | Value | +|-------|-------| +| Severity | LOW | +| Category | Insecure Design (OWASP A04) / Software & Data Integrity (OWASP A08) | +| Location | `src/features/admin/AdminPage.tsx:57-75` (`handleUpdateClass`) | +| Introduced by | AZ-512 (commit `ecacfa8`) | +| Production exposure | The `/admin` route is gated by Header's `ADM` permission AND backend authZ on every `/api/admin/*` call. Surface is restricted to authenticated admins. | + +**Description** + +`handleUpdateClass` performs an inline edit with this sequence: + +1. Client-side validation (`name.trim()` non-empty, `maxSizeM > 0`). +2. `await api.patch(endpoints.admin.class(editingId), editForm)` — sends the **complete** edit-form body (the documented "Risk 2 mitigation" so partial-merge vs full-replace PATCH semantics are equivalent for the UI). +3. `await api.get(endpoints.annotations.classes())` — refetch list, replace `classes`, clear `editingId`. + +The intentional full-body PATCH guarantees the UI's view of the row replaces whatever is on the server. There is **no concurrency guard** (`If-Match` / `ETag` / `version`). If admin A and admin B open the same row simultaneously, the last `PATCH` wins silently and overwrites the other admin's edit without notification. + +This is a deliberate trade-off: the task spec (`AZ-512_admin_edit_detection_class.md`) explicitly scopes optimistic-locking out, and AZ-513's backend spec mirrors that (no ETag header). The risk class is documented in the task spec's "Risks" section. The audit records it for completeness so future hardening can re-open it. + +**Impact** + +- Two admins editing the same detection class in the same window → second save silently overwrites the first. +- Audit trail (if any — owned by `admin/` service) would show both PATCHes, so attribution survives. +- Detection-class editing is a low-frequency administrative operation with typically a single active admin, so practical exposure is low. + +**Production-bundle exposure** + +Limited to authenticated `ADM` users, in a low-multi-admin operation domain, with no user-data leak. **No exploitable path to data exfiltration or escalation.** This is a correctness / data-integrity weakness, not an authN/authZ break. + +**Remediation (future / out-of-cycle)** + +1. When AZ-513 lands the backend, decide whether `admin/` will emit an `ETag` on `GET /api/admin/classes/{id}` and accept `If-Match` on `PATCH`. If yes, the UI side becomes: + - Capture `etag` from the row on edit-start. + - Send `If-Match: ` header on `PATCH`. + - On `412 Precondition Failed`, render a "this class was changed by someone else — reload?" inline alert (analogous to today's `editError = 'updateFailed'`). +2. Cheaper short-term alternative: append a generated `version: number` to `DetectionClass` and have the UI assert it on PATCH; backend returns 409 on mismatch. + +**Track as**: open in `_docs/05_security/`; not blocking. To be promoted to a UI ticket only when AZ-513 lands and the backend's chosen concurrency model is known. + +--- + +## Cross-cutting cycle-4 verification + +### Static analysis — AZ-512 deltas + +- **URL construction**: `endpoints.admin.class(editingId)` is the same builder used by `handleDeleteClass` (cycle-2 audited path). `editingId: number | null` is constrained at the type level and is only set from a server-returned `DetectionClass.id`. No tainted-input → URL path. +- **JSON body**: `editForm` is a plain `{ name, shortName, color, maxSizeM }` object. React form-controlled inputs feed it; no `dangerouslySetInnerHTML`, no `innerHTML`, no template injection surface. Backend must still validate length / charset (UI relies on backend per AZ-513 ACs). +- **Error path**: the `catch` block sets a discriminated-union error kind, not the raw thrown message. No information leak from server error responses into the rendered UI. +- **Optimistic refetch**: same shape as cycle-2-audited `handleAddClass` refetch. No new surface. +- **Test-only MSW handler in `tests/msw/handlers/admin.ts`**: not bundled. Vite's `bundle-introspect.test.ts` (cycle-2 evidence) already enforces `tests/` is excluded from `dist/`. + +**Verdict**: PASS — no new injection, no new secret, no new auth-surface. + +### Authentication & authorization — AZ-512 deltas + +- **Route gating**: AZ-512 does not change `/admin` route gating. Header's `hasPermission('ADM')` continues to filter the visible nav entry. As cycle-2 noted (F2 / AC-22 carry), a user who deep-links to `/admin` without `ADM` still renders the page but every fetch 401/403s. AZ-512 inherits that posture exactly. +- **Per-action authZ**: each PATCH/DELETE/POST/GET is authZ'd server-side by `admin/`. The UI does not perform pre-flight permission checks for the edit affordance specifically. This matches the existing add / delete posture (cycle-2 audited). + +**Verdict**: PASS — no degradation; carries F2 / AC-22 unchanged. + +### Cryptographic failures, secrets, data exposure — AZ-512 deltas + +- **No new secrets** introduced. `bun audit` clean. No new env vars touched. +- **No PII** in the PATCH body (detection-class metadata only). +- **No new log output**: `client.ts` has no new logging path; `AdminPage.tsx` adds no `console.*`. +- **Error message localization**: errors are mapped to i18n keys (`admin.classes.updateFailed`) — no server-message echo into the UI string. + +**Verdict**: PASS. + +### OWASP Top 10 — categories whose status would change + +None. All ten categories carry forward from the cycle-3 delta verdict unchanged. The new LOW finding F-SAST-CY4-1 maps to A04 (Insecure Design) but the category's status was already PASS (cycle 2) and stays PASS because LOW findings do not flip the category. + +| # | Category | Cycle-3 status | Cycle-4 status | +|---|----------|----------------|----------------| +| A01 | Broken Access Control | PASS_WITH_KNOWN (F2/AC-22 carry) | **unchanged** | +| A02 | Cryptographic Failures | PASS_WITH_KNOWN (ADR-008 carry) | **unchanged** | +| A03 | Injection | PASS | **unchanged** | +| A04 | Insecure Design | PASS | **unchanged** (new LOW F-SAST-CY4-1 is informational only) | +| A05 | Security Misconfiguration | FAIL (F-INF-2 carry) | **unchanged** | +| A06 | Vulnerable & Outdated Components | PASS | **unchanged** (`bun audit` re-run clean 2026-05-13) | +| A07 | Identification & Authentication Failures | PASS | **unchanged** | +| A08 | Software & Data Integrity Failures | FAIL (F-INF-1, F-INF-3, F-INF-4 carry) | **unchanged** | +| A09 | Logging & Monitoring | N/A | **unchanged** | +| A10 | SSRF | N/A | **unchanged** | + +### Infrastructure / CI / Container — AZ-512 deltas + +**None.** Cycle 4 did not touch `Dockerfile`, `nginx.conf`, `.woodpecker/build-arm.yml`, `.env.example`, or any container/CI artifact. Carries F-INF-1..5 verbatim. + +--- + +## Cross-workspace dependency note + +AZ-512 ships against MSW stubs in tests. The live `PATCH /api/admin/classes/{id}` endpoint does not exist in production until **AZ-513** is implemented and deployed by the `admin/` workspace team. Until then: + +- A real admin clicking ✎ + Save in the deployed dev/stage/prod UI will hit a backend `404` (or 405 depending on how `admin/` rejects unknown methods). +- The UI surfaces a generic `editError = 'updateFailed'` ⇒ "Update failed" inline alert. No information leak. +- **Deploy gate**: Step 16 of cycle 4 must NOT promote this build past the boundary where AZ-513 has not yet landed. The `_docs/_process_leftovers/2026-05-13_az-512-admin-classes-prereq.md` leftover entry remains open until AZ-513 ships + deploys. + +This is a process control concern, not a security finding — captured here so the audit history records why a deploy-gate exists for an otherwise-clean cycle. + +--- + +## Updated counts (carries from cycle 3 + cycle-4 net) + +| Severity | Cycle 3 | Cycle 4 | Net change | +|----------|---------|---------|------------| +| Critical | 0 | 0 | — | +| High | 1 (F-SAST-1) | 1 (F-SAST-1) | — | +| Medium | 7 | 7 | — | +| Low | 3 (F-SAST-4, F-INF-5, F-SAST-CY3-1) | 4 (+F-SAST-CY4-1) | +1 | + +## Self-verification (Phase 5 of `security/SKILL.md`) + +- [x] All cycle-4 changed files reviewed (6 source/test files + doc files; surface enumerated above). +- [x] No duplicate findings (F-SAST-CY4-1 is new, not a restatement of F-INF-1..5 or F-SAST-CY3-1). +- [x] Every finding has remediation guidance (see F-SAST-CY4-1 § Remediation). +- [x] Verdict matches severity logic (PASS_WITH_WARNINGS = only Medium/Low new findings + carried High is pre-existing). +- [x] `bun audit` re-run is clean. +- [x] No new credentials / secrets in cycle-4 commits (`ef56d9c`, `ecacfa8`). +- [x] Cross-workspace dependency (AZ-513) is recorded as a process / deploy-gate concern, not a security finding. + +## Recommendations + +### Immediate (Critical/High) +- None new from cycle 4. Cycle-2 / cycle-3 carries unchanged: revoke Google Geocode key (F-SAST-1); revoke OpenWeatherMap key (carried). + +### Short-term (Medium) +- None new from cycle 4. Cycle-2 carries unchanged: nginx security headers (F-INF-2); `bun audit` in CI (F-INF-1); Trivy/Grype in CI (F-INF-3); SBOM + image signing (F-INF-4). + +### Long-term (Low / Hardening) +- **F-SAST-CY4-1 follow-up**: when AZ-513 lands, decide on the concurrency model with `admin/`. If `ETag` / `If-Match`: open a UI ticket to thread the header through `client.ts` and surface 412 as a "reload" alert. If `version` field: open a UI ticket to assert version on PATCH and surface 409 the same way. Cheap fix once the backend picks a model — until then, it stays LOW. diff --git a/_docs/06_metrics/perf_2026-05-13_cycle4.md b/_docs/06_metrics/perf_2026-05-13_cycle4.md new file mode 100644 index 0000000..3c31e0b --- /dev/null +++ b/_docs/06_metrics/perf_2026-05-13_cycle4.md @@ -0,0 +1,80 @@ +# Performance Test Report — Cycle 4 + +**Date**: 2026-05-13 +**Cycle**: Phase B / Cycle 4 (AZ-512 — admin class inline edit) +**Runner**: `scripts/run-performance-tests.sh --static-only` (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 project is still not wired (carries from cycle 3) +**Verdict**: **PASS** (one Pass + documented SKIPs + three documented Quarantines) + +--- + +## Scope + +Re-baseline the gzipped initial-JS bundle metric (NFT-PERF-01) after AZ-512 added ~80 lines of inline-edit code to `src/features/admin/AdminPage.tsx` plus 7 new i18n keys × 2 locales in `src/i18n/{en,ua}.json`. No new packages, no new external endpoints, no new lazy-load boundary (AdminPage continues to import statically from `src/App.tsx:8`, so its bytes count toward the initial-JS bundle). + +E2E-stack-bound scenarios (NFT-PERF-02..10) are out of scope for this cycle's measurement because: +1. The Playwright perf project remains unwired (same status as cycle 3 — tracked in `perf_2026-05-13_cycle3.md` "E2E profile status"). +2. AZ-512's surface is contained client-side state + one HTTP PATCH that does not yet exist server-side (the live endpoint is gated by AZ-513 in the `admin/` workspace). There is no live-stack perf path to measure until AZ-513 ships. + +--- + +## Results + +| Scenario | Verdict | Measured | Threshold | Source | +|----------|---------|----------|-----------|--------| +| NFT-PERF-01 (initial JS bundle, gzipped) | **PASS** | **291 332 B** (≈ 284.5 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 | QUARANTINE | — | Step 8 hardening (SSE refresh rotation) | Carried | +| NFT-PERF-04..07 | SKIP | n/a | various | Deferred — Playwright perf project not yet wired | +| NFT-PERF-08 | QUARANTINE | — | Step 4 fix (panel-width persistence) | Carried | +| NFT-PERF-09 | QUARANTINE | — | Step 4 fix (settings save error surfacing) | Carried | +| NFT-PERF-10 (warm-cache FCP on /flights) | SKIP | n/a | ≤ 3 000 ms (edge profile) | Deferred — Playwright perf project not yet wired | + +--- + +## Bundle delta vs prior cycles + +| Cycle | Measured (bytes, gzipped) | Δ vs prior cycle | % of 2 MB budget | Source | +|-------|---------------------------|------------------|------------------|--------| +| 2 | 290 465 | new baseline | ~13.85% | `perf_2026-05-12_cycle2.md` | +| 3 (post AZ-510/AZ-511) | 290 575 | **+110 B (+0.04%)** | ~13.85% | `perf_2026-05-13_cycle3.md` | +| 4 (post AZ-512) | **291 332** | **+757 B (+0.26%)** | **~13.89%** | this report | + +Net change vs cycle-2 baseline: +867 bytes / +0.30% / +0.04 percentage-points of budget across two feature cycles. Bundle growth remains in line with the rate of feature growth — no regression, no concern. + +--- + +## Bundle-size impact analysis — what cost the +757 bytes + +| Change | Pre-min source | Estimated minified+gzipped contribution | +|--------|----------------|------------------------------------------| +| `src/features/admin/AdminPage.tsx` new state (4 hooks), handlers (`handleStartEdit`/`handleCancelEdit`/`handleUpdateClass`/`handleEditKeyDown`), conditional row JSX, validation, PATCH wiring | ~80 LoC of TS + JSX | ~500–600 B | +| `src/i18n/en.json`, `src/i18n/ua.json` — `admin.classes` flat-string → nested object (`title` + 6 edit keys) per locale | 7 keys × 2 locales × ~25 B/key (English) + Cyrillic UA chars ~2× UTF-8 | ~150–200 B | +| Module doc / blackbox / traceability / report deltas | docs only | 0 (excluded from `dist/`) | + +The delta is dominated by the inline-edit handler and JSX; i18n is a small fraction. **Order-of-magnitude consistent with a tight ~80-line UI feature.** No accidental imports of `mission-planner/`, no new `react-i18next` plugins, no new icon set, no new third-party lib pulled in. + +--- + +## E2E profile status + +Carried verbatim from `perf_2026-05-13_cycle3.md` — the Playwright perf project remains unwired. Same unblock path: + +> NFT-PERF-02..10 require a Playwright performance-config profile that loads the suite stack, performs the scenario, and emits timing measurements consumable by the runner. The project's existing Playwright config drives functional e2e only (no perf assertions / reporters). Wiring this is a Phase B candidate (own ticket, ~5-point task; not in scope for AZ-512). + +No new blocker — the gap has the same shape it had in cycle 3. AZ-512 does not change the e2e-perf surface; the planned Playwright wiring (a future ticket) is what unblocks NFT-PERF-02..10. + +--- + +## Verdict + +**PASS** for cycle 4. The single executable scenario (NFT-PERF-01) is at 13.89% of the 2 MB threshold with a +0.26% cycle-over-cycle increase explained entirely by AZ-512's documented additions. All SKIPs and QUARANTINES carry forward from cycle 3 with the same rationale. **No bundle regression and no new perf concern introduced.** + +## Self-verification (test-run / perf-mode) + +- [x] NFT-PERF-01 runner executed against a freshly built `dist/` (no stale build). +- [x] Threshold sourced from `_docs/00_problem/input_data/expected_results/results_report.md` (AC-11 / row 40 — same as cycle 3). +- [x] Measured value recorded with the exact byte count from the runner. +- [x] Cycle-over-cycle delta computed and explained. +- [x] No threshold breach. +- [x] E2E profile status carried with same unblock path as cycle 3 — no new perf gating ticket needed for AZ-512. diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index 28b4b72..5f0294b 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -2,9 +2,9 @@ ## Current Step flow: existing-code -step: 10 -name: Implement -status: in_progress +step: 16 +name: Deploy +status: not_started sub_step: phase: 0 name: awaiting-invocation @@ -14,7 +14,7 @@ cycle: 4 tracker: jira ## Notes -- Cycle 3 closed 2026-05-13 (commit eef3bdf): 6/9 pts shipped, AZ-512 → backlog (AZ-513 prereq on admin/). -- Cycle 4 entry: user reactivated AZ-512 (Option B path, MSW-stubbed). Step 9 (New Task) skipped — task spec already exists in `_docs/02_tasks/todo/AZ-512_admin_edit_detection_class.md`. -- Cross-workspace dependency: AZ-513 on admin/ NOT shipped at cycle entry. Step 11 (Run Tests) will pass on MSW stubs; Step 16 (Deploy) gates on AZ-513 landing live. -- Leftovers: `2026-05-12_az-498-deploy-and-key-revocations.md` (manual), `2026-05-13_az-512-admin-classes-prereq.md` (re-opened until AZ-513 deploys live). +- Cycle 4 batch 16 shipped (commit ecacfa8): AZ-512 — 3/3 pts. Jira: To Do → In Testing. +- Cross-workspace: AZ-513 on admin/ NOT shipped. Step 16 (Deploy) gates on it. +- Leftovers: `2026-05-12_az-498-deploy-and-key-revocations.md` (manual), `2026-05-13_az-512-admin-classes-prereq.md` (re-opened). +- Pre-existing bug surfaced during AZ-512: `/api/admin/users` MSW shape (paginated) vs `AdminPage` consumption (flat `User[]`) mismatch. Flagged in batch + impl reports; needs separate UI ticket triage.