mirror of
https://github.com/azaion/ui.git
synced 2026-06-21 09:31:10 +00:00
[AZ-501] [AZ-502] Cycle 2 Step 14 security audit + inline fixes
ci/woodpecker/push/build-arm Pipeline failed
ci/woodpecker/push/build-arm Pipeline failed
Security audit (5 phases) → reports under _docs/05_security/. AZ-501 (F-SAST-1, HIGH): Externalize hardcoded Google Geocode key from mission-planner/src/config.ts to VITE_GOOGLE_GEOCODE_KEY via new GeocodeService.ts; fail-soft warn when unset; STC-SEC1D static deny-list gate; +5 unit tests in tests/mission_planner_geocode.test.ts. AZ-502 (F-DEP-1, HIGH): Force vite>=6.4.2 and postcss>=8.5.10 via package.json overrides in both roots; clean reinstall clears all bun audit advisories. Test-spec sync (Step 12) + Update Docs (Step 13) deltas: AC-43, AC-44, NFT-SEC-09b, FT-P-61, FT-N-17, ripple log, batch_12 report. Pending user actions: revoke Google + OWM keys (AC-6 / AZ-499 AC-7). 229 PASS / 13 SKIP / 0 FAIL on static + fast suites. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,136 @@
|
||||
# Batch 12 — AZ-501 + AZ-502 (security-audit inline fixes)
|
||||
|
||||
**Date**: 2026-05-12
|
||||
**Cycle**: Phase B / Cycle 2 — autodev Step 14 (Security Audit) inline-fix sub-step
|
||||
**Tickets**: AZ-501 (Google Geocode key externalization), AZ-502 (Vite/PostCSS upgrade)
|
||||
**Trigger**: User chose option **A** ("fix BOTH inline now") on the Step 14 BLOCKING gate after the security audit reported HIGH-severity F-SAST-1 (Google key in port-source) and F-DEP-1 (Vite WebSocket file-read CVE).
|
||||
**Verdict**: PASS — both findings resolved in code; static + fast tests green; manual key-revocation deliverables (AZ-501 AC-6, AZ-499 AC-7) remain pending USER action.
|
||||
|
||||
---
|
||||
|
||||
## AZ-501 — Externalize Google Geocode API key in mission-planner port-source
|
||||
|
||||
### Status
|
||||
|
||||
- **Code**: Done
|
||||
- **Manual deliverable AC-6 (key revocation at Google Cloud Console)**: PENDING USER
|
||||
- **Jira state**: still "To Do" — must be transitioned to "In Testing" with the commit and to "Done" only after AC-6 evidence is attached
|
||||
|
||||
### Approach
|
||||
|
||||
Mirrored the AZ-499 pattern exactly:
|
||||
1. Extracted the geocode call to a new service module so the env-resolution + fail-soft contract can be unit-tested in isolation (parallels `WeatherService.ts`).
|
||||
2. Externalized the key via `import.meta.env.VITE_GOOGLE_GEOCODE_KEY`.
|
||||
3. Fail-soft when unset: returns `null`, no fetch issued, single `console.warn` (geocode is user-triggered per "Enter" keypress, so a warn-per-call is informative not spammy — distinct from the silent fail-soft chosen for `WeatherService.ts` which is called periodically).
|
||||
4. Added literal-scan defense-in-depth gate (`STC-SEC1D`) to prevent the same key string from reappearing in `src/` or `mission-planner/`.
|
||||
5. Documented the new env var in `mission-planner/.env.example` with the established `<your-...-key>` placeholder convention.
|
||||
|
||||
### Files changed
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `mission-planner/src/services/GeocodeService.ts` | NEW — env-resolved geocode with fail-soft + console.warn |
|
||||
| `mission-planner/src/config.ts` | Removed `GOOGLE_GEOCODE_KEY` literal; only `COORDINATE_PRECISION` remains |
|
||||
| `mission-planner/src/vite-env.d.ts` | Added `readonly VITE_GOOGLE_GEOCODE_KEY?: string` |
|
||||
| `mission-planner/src/flightPlanning/LeftBoard.tsx` | Replaced inline `geocodeAddress` with import from the new service module; removed `GOOGLE_GEOCODE_KEY` import |
|
||||
| `mission-planner/.env.example` | Added `VITE_GOOGLE_GEOCODE_KEY=<your-google-geocode-api-key>` + comment block |
|
||||
| `tests/security/banned-deps.json` | Added `google_key_in_source` section with the literal key as a banned pattern |
|
||||
| `scripts/check-banned-deps.mjs` | Added `'google_key_in_source'` to the source-tree-scan dispatch (1-line list extension; reuses existing `checkSourceTree`) |
|
||||
| `scripts/run-tests.sh` | Added `STC-SEC1D` static-check function + entry in the runner table |
|
||||
| `tests/mission_planner_geocode.test.ts` | NEW — 5 tests covering env-resolution (AC-1), fail-soft on missing key + warn (AC-3), fail-soft on network error, ZERO_RESULTS handling, and a defense-in-depth assertion that no fallback key is hardcoded |
|
||||
|
||||
### AC coverage
|
||||
|
||||
| AC | Status | Evidence |
|
||||
|----|--------|----------|
|
||||
| AC-1 (env-var resolution) | PASS | `tests/mission_planner_geocode.test.ts` — `'AC-1: env-var resolved API key reaches the outgoing fetch URL'` |
|
||||
| AC-2 (.env.example documentation) | PASS | `mission-planner/.env.example` lines 12-14, 33 |
|
||||
| AC-3 (fail-soft + warn) | PASS | tests `'AC-3: returns null, issues no fetch, and warns when VITE_GOOGLE_GEOCODE_KEY is unset'` and `'AC-3: still returns null and does not throw when fetch rejects'` |
|
||||
| AC-4 (static gate) | PASS | `STC-SEC1D` runs in `scripts/run-tests.sh` static profile against the new `google_key_in_source` deny-pattern |
|
||||
| AC-5 (unit test) | PASS | `tests/mission_planner_geocode.test.ts` — 5 tests, all green |
|
||||
| AC-6 (key revocation) | PENDING USER | Google Cloud Console: revoke `AIzaSyAhvDeYukuyWVrQYbRhuv91bsi_jj5_Iys` and attach evidence to AZ-501 before transitioning to Done |
|
||||
|
||||
### Design decisions
|
||||
|
||||
- **Why a separate service module instead of inline-in-component?** Same reasoning as AZ-499: the inline form is not testable without mounting `<LeftBoard>` (heavy MUI tree); extracting to a service mirrors the established `WeatherService.ts` pattern and lets `tests/mission_planner_geocode.test.ts` exercise env-resolution and fail-soft directly. Also removes the cross-cutting `import { GOOGLE_GEOCODE_KEY } from '../config'` from `LeftBoard.tsx`.
|
||||
- **Why warn (not silent) on missing key?** Geocode is user-triggered (per "Enter" keypress), so a warn-per-call is informative without being spammy. `WeatherService.ts` chose silent fail-soft because it's called periodically.
|
||||
- **Why `STC-SEC1D` instead of folding into `STC-SEC1C`?** The two gates have different ACs (AZ-499 vs AZ-501) and different secret-vendor scopes — keeping them separate makes the report rows easier to audit.
|
||||
|
||||
### Spec drift
|
||||
|
||||
None. All 6 ACs in the AZ-501 spec are addressed; AC-6 is correctly identified as a manual deliverable.
|
||||
|
||||
---
|
||||
|
||||
## AZ-502 — Update Vite + PostCSS past published CVEs
|
||||
|
||||
### Status
|
||||
|
||||
- **Code**: Done
|
||||
- **AC-5 (CI gate)**: explicitly DEFERRED to a Phase B follow-up per the ticket's own scope note ("**may be SPLIT into a sibling ticket if it expands scope**"). The Step 14 audit's F-INF-1 finding is the tracking record.
|
||||
|
||||
### Approach
|
||||
|
||||
`bun update vite` in both roots upgraded the direct `vite` dependency to `6.4.2`, but the audit still complained because `vitest@3.2.4` nests its own `vite@6.4.1` under `node_modules/vitest/node_modules/vite/`. Bun's resolver follows the nested copy (a peer-dep + nested-dep pattern), so a direct upgrade alone is insufficient.
|
||||
|
||||
Resolution: added `"overrides": { "vite": ">=6.4.2", "postcss": ">=8.5.10" }` to both `package.json` files — Bun honors the npm-compatible `overrides` field and floors all transitive resolutions, including the nested copies inside `vitest/`. After a clean reinstall (`rm -rf node_modules bun.lock && bun install`), `bun audit` reports zero advisories in both roots.
|
||||
|
||||
### Files changed
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `package.json` (root) | Added `"overrides": { "vite": ">=6.4.2", "postcss": ">=8.5.10" }` |
|
||||
| `mission-planner/package.json` | Same overrides block; bumped direct `vite` to `^6.4.2` |
|
||||
| `bun.lock` (root) | Regenerated |
|
||||
| `mission-planner/bun.lock` | Regenerated |
|
||||
|
||||
### AC coverage
|
||||
|
||||
| AC | Status | Evidence |
|
||||
|----|--------|----------|
|
||||
| AC-1 (`bun update vite` in both roots) | PASS | both `bun.lock` files regenerated |
|
||||
| AC-2 (`bun audit` zero findings in both roots) | PASS | `bun audit` exit 0 in both roots after clean reinstall (verified) |
|
||||
| AC-3 (`bun run build` succeeds) | PASS | covered by `STC-B1` in the static profile (`scripts/run-tests.sh`) — passed in this batch's full test run |
|
||||
| AC-4 (full test suite stays green) | PASS | static + fast: 229 PASS / 13 SKIP / 0 FAIL (+5 new PASS from `tests/mission_planner_geocode.test.ts`) |
|
||||
| AC-5 (CI `bun audit` gate) | DEFERRED | Phase B; tracked at `_docs/05_security/infrastructure_review.md` F-INF-1 |
|
||||
|
||||
### Design decisions
|
||||
|
||||
- **Why `overrides` instead of pinning vitest higher?** There is no newer `vitest` release that pulls a patched `vite` — the next vitest minor lands eventually but is not yet published. Bun `overrides` solves the same problem zero-cost without introducing a vitest major-version churn.
|
||||
- **Why floor PostCSS too?** PostCSS comes in transitively via Vite; once Vite is at 6.4.2 the postcss it needs is `^8.5.3` which Bun resolved to `8.5.8` (still vulnerable). The override floors it to `8.5.10` (the patched range from GHSA-qx2v-qp2m-jg93).
|
||||
- **Why only `bun update vite` not `bun update --latest`?** Avoid unrelated major-version churn in the same change. The advisory range is `<= 6.4.1`; 6.4.2 is the minimum-impact fix.
|
||||
|
||||
### Spec drift
|
||||
|
||||
AC-5 (CI gate) is explicitly deferred per the ticket's own scope note. F-INF-1 in the audit infrastructure_review.md captures the follow-up.
|
||||
|
||||
---
|
||||
|
||||
## Test results
|
||||
|
||||
Full `scripts/run-tests.sh` run (static + fast):
|
||||
|
||||
| Profile | Result | Detail |
|
||||
|---------|--------|--------|
|
||||
| static | PASS | All checks PASS, including the new `STC-SEC1D` (no Google key literal in `src/` + `mission-planner/`). 33 STC-* checks total. |
|
||||
| fast | PASS | 229 PASS / 13 SKIP / 0 FAIL (+5 new PASS from `tests/mission_planner_geocode.test.ts` vs. the cycle-2 baseline of 224 PASS). 13 skips unchanged from cycle-2 baseline. |
|
||||
| e2e | NOT RUN | (deferred — same `env-blocked` posture as `_docs/03_implementation/test_run_report_phase_b_cycle2.md`) |
|
||||
|
||||
Test-spec sync deltas (this batch):
|
||||
- `_docs/02_document/tests/security-tests.md`: appended `NFT-SEC-09b` (Google Geocode key not in source).
|
||||
- `_docs/02_document/tests/blackbox-tests.md`: appended `FT-P-61` (env-resolution) and `FT-N-17` (fail-soft + warn).
|
||||
- `_docs/02_document/tests/traceability-matrix.md`: added rows for AC-43 (geocode env hardening) and AC-44 (Vite/PostCSS upgrade); coverage summary updated to 90 total items.
|
||||
- `_docs/00_problem/acceptance_criteria.md`: added AC-43 + AC-44; coverage status appended.
|
||||
- `_docs/00_problem/security_approach.md`: added §5 paragraph on the Google key + appended findings → fix map rows.
|
||||
|
||||
## Pending manual deliverables (across all of Cycle 2)
|
||||
|
||||
1. **AZ-499 AC-7** — revoke OWM key `335799082893fad97fa36118b131f919` at https://home.openweathermap.org/api_keys; attach evidence to AZ-499.
|
||||
2. **AZ-501 AC-6** — revoke Google Geocode key `AIzaSyAhvDeYukuyWVrQYbRhuv91bsi_jj5_Iys` at https://console.cloud.google.com/google/maps-apis/credentials; attach evidence to AZ-501.
|
||||
|
||||
These are out-of-band defense-in-depth completions; the static gates `STC-SEC1C` (OWM) and `STC-SEC1D` (Google) already prevent re-introduction of the literal strings, but the rotated keys must be revoked at the providers to actually neutralize the leaked credentials.
|
||||
|
||||
## Cross-workspace gates carried forward
|
||||
|
||||
- **AZ-498 deploy** (autodev Step 16) still gated on the satellite-provider cookie-auth ticket on the satellite-provider workspace.
|
||||
- No new cross-workspace gates introduced by this batch.
|
||||
@@ -0,0 +1,61 @@
|
||||
# Test Run Report — Phase B Cycle 2 (Step 11)
|
||||
|
||||
**Date**: 2026-05-12
|
||||
**Mode**: functional
|
||||
**Runner**: `scripts/run-tests.sh` (default profiles: static + fast; e2e env-blocked, see Step 7 / cycle-1 reports)
|
||||
**Verdict**: PASS_WITH_DOCUMENTED_GATE
|
||||
**Cycle scope**: AZ-498 (self-hosted satellite tiles) + AZ-499 (mission-planner OWM env hardening) — implemented in batch 11.
|
||||
|
||||
## Profile Outcomes
|
||||
|
||||
| Profile | Status | Counts | Wall-clock | Report file |
|
||||
|---------|--------|--------|------------|-------------|
|
||||
| static | PASS | **32 / 32** including new `STC-SEC1C` (AZ-499) | ~14 s | `test-output/static-report.csv` |
|
||||
| fast | PASS | 30 files / **224 PASS / 13 SKIP / 0 FAIL** | ~17 s | `test-output/fast-report.xml` |
|
||||
| e2e | env-blocked (deferred — same registry-access block as Step 7 / cycle 1) | n/a | n/a | n/a |
|
||||
|
||||
## Delta vs Cycle 1 (Step 11) baseline
|
||||
|
||||
- Fast: 209 / 13 → **224 / 13** (+15 new tests, 0 new skips):
|
||||
- +8 in `src/features/flights/__tests__/satellite_tile.test.tsx` (AZ-498 AC-1..AC-4 — env-resolved tile URL, cookie-credentialed `<TileLayer>`, classic/satellite toggle removal).
|
||||
- +7 in `tests/mission_planner_weather.test.ts` (AZ-499 AC-1..AC-4 + happy-path + fail-soft for `getWeatherData()`).
|
||||
- Static: 31 / 31 → **32 / 32** (+1 new gate):
|
||||
- `STC-SEC1C` (AZ-499) — no literal OWM key in `src/` + `mission-planner/`. Complements `STC-SEC1` (which scans `src/` for `appid=<chars>`) and `STC-SEC1B` (dist scan) by catching the rotated literal value across both source trees.
|
||||
- Skip count unchanged at 13 — no new skips introduced this cycle.
|
||||
|
||||
## System-Under-Test Reality Gate
|
||||
|
||||
PASS:
|
||||
- `_docs/00_problem/input_data/expected_results/results_report.md` still exists; `_docs/02_document/tests/traceability-matrix.md` still maps every AC. No internal product module was faked, monkeypatched, or replaced with a deterministic fallback by this cycle's batch — verified by review for batch 11.
|
||||
- The cycle-2 surface is pure config/wire hardening:
|
||||
- AZ-498: removes the classic/satellite toggle, swaps the OSM tile URL for an env-resolved `getTileUrl()` returning the satellite-provider endpoint, and adds `crossOrigin="use-credentials"` so the browser attaches the satellite-provider auth cookie. The product code under test (`getTileUrl`, `<TileLayer>` props, removed `mapType` state) is real; only `react-leaflet` and `leaflet` (external deps) and `globalThis.fetch` (network boundary) are stubbed — same boundary discipline as the rest of the suite.
|
||||
- AZ-499: replaces the hardcoded OWM key + base URL in `mission-planner/src/services/WeatherService.ts` with `import.meta.env.VITE_OWM_*` accessors and a fail-soft return when the key is unset. The product code under test (`getWeatherData()` body) is real; only `globalThis.fetch` is stubbed.
|
||||
- CSV report inspected — all 32 static rows PASS; fast profile rolled-up PASS row points at the JUnit XML.
|
||||
|
||||
## Skipped Tests — Same 13 As Cycle 1, Still Legitimate
|
||||
|
||||
The 13 skips are byte-for-byte the same set documented in `test_run_report.md` (Step 7) and re-affirmed in `test_run_report_phase_b_cycle1.md`. None of this cycle's two tasks (AZ-498, AZ-499) touched any of the skip conditions:
|
||||
- AZ-498 changes the tile-base URL plumbing and `<TileLayer>` `crossOrigin` attribute — orthogonal to the auth bootstrap, canvas-editor, i18n, sse-lifecycle, and wire-contract skip surfaces.
|
||||
- AZ-499 changes only the `mission-planner` weather service — orthogonal to every skip in the SPA surface.
|
||||
|
||||
The user-approved acceptance from Step 7 still applies; carried forward without re-prompting.
|
||||
|
||||
## Environment Block — e2e Profile
|
||||
|
||||
Same as Step 7 / cycle 1: registry-access block on `azaion/{admin,flights,annotations,detect,loader,resource}:test` images. AZ-498 / AZ-499 do not affect Docker images or compose configuration directly — but note: AZ-498 introduced a new `e2e/stubs/tile/` server (already wired into `e2e/docker-compose.suite-e2e.yml` per the staged changes, plus a new `tests/msw/handlers/tiles.ts` and an updated `e2e/tests/infrastructure.e2e.ts`) for the satellite-tile e2e path. These will exercise once the registry-access block clears in the merge-lane CI.
|
||||
|
||||
Defer e2e to the merge-lane CI per Step 7's user-approved option A.
|
||||
|
||||
## Outcome
|
||||
|
||||
Step 11 **passes**. Auto-chain to Step 12 (Test-Spec Sync).
|
||||
|
||||
## Open Items
|
||||
|
||||
Unchanged from cycle 1:
|
||||
- F-CUM-5 production-drift backlog — feature cycles continue.
|
||||
- F-CUM-4 long-running-soak Playwright config tag — recommended fold-in to merge-lane config.
|
||||
|
||||
Cycle-2 specific (carried forward, not gating Step 11):
|
||||
- AZ-499 AC-7 — OWM key revocation at OWM dashboard (pending USER ACTION).
|
||||
- AZ-498 — satellite-provider cookie-auth (pending CROSS-WORKSPACE; gates Step 16 Deploy).
|
||||
Reference in New Issue
Block a user