mirror of
https://github.com/azaion/ui.git
synced 2026-06-21 18:51:10 +00:00
Oleksandr Bezdieniezhnykh
f7dd6c98d8
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>
185 lines
13 KiB
Markdown
185 lines
13 KiB
Markdown
# OWASP Top 10 Review — Azaion UI
|
|
|
|
**Date**: 2026-05-12
|
|
**Framework**: [OWASP Top 10 — 2021](https://owasp.org/www-project-top-ten/) (current edition; 2024 revision is in draft and not yet final at time of audit)
|
|
**Scope**: Browser SPA (`src/`) + nginx + supporting infrastructure
|
|
**Cycle**: Phase B / Cycle 2
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
| # | Category | Status | Notes |
|
|
|---|----------|--------|-------|
|
|
| A01 | Broken Access Control | PASS_WITH_KNOWN | Server-authoritative; 1 known UX-only client gap (`/admin` route) |
|
|
| A02 | Cryptographic Failures | PASS_WITH_KNOWN | Bearer in memory; refresh in HttpOnly cookie; 1 accepted trade-off (SSE bearer-in-query) |
|
|
| A03 | Injection | PASS | No eval/Function; React JSX escapes; URL params encoded |
|
|
| A04 | Insecure Design | PASS | Same-origin nginx + bearer-header + SameSite=Strict cookie pattern |
|
|
| A05 | Security Misconfiguration | FAIL | nginx missing CSP, X-Frame-Options, HSTS, Referrer-Policy, X-Content-Type-Options, log redaction |
|
|
| A06 | Vulnerable & Outdated Components | FAIL | 1 High vite (dev-server only) + 2 Moderate (vite, postcss) — see `dependency_scan.md` |
|
|
| A07 | Identification & Authentication Failures | PASS_WITH_KNOWN | 1 known cold-load refresh bug (F2 in `security_approach.md`) |
|
|
| A08 | Software & Data Integrity Failures | FAIL | No SBOM, no image signing, no `bun audit` in CI |
|
|
| A09 | Security Logging & Monitoring Failures | N/A | Server-side concern; SPA is operator-internal with no client telemetry |
|
|
| A10 | Server-Side Request Forgery | N/A | Browser SPA has no server-side request surface |
|
|
|
|
**Overall posture**: PASS_WITH_WARNINGS. No exploitable vulnerabilities in the production browser bundle. Multiple infrastructure-level hardening gaps are tracked at the suite level (nginx headers, CI scanning).
|
|
|
|
---
|
|
|
|
## A01 — Broken Access Control — PASS_WITH_KNOWN
|
|
|
|
**Server-authoritative model.** Per `_docs/00_problem/security_approach.md` §2 and `_docs/02_document/architecture.md` §7: every authenticated endpoint validates `User.role` and `permissions[]` server-side. The browser is treated as untrusted; the UI inspects `AuthUser.role` only to render or hide nav elements.
|
|
|
|
**Verified controls**:
|
|
- The 401-recovery path in `src/api/client.ts:90` attempts a server-side refresh and surfaces 401/403 to the user — there is no path that "promotes" a denied request client-side.
|
|
- IDOR / horizontal-escalation surface: every API URL embeds the suite-side resource ID; the UI does not assemble paths from user input that could be substituted to access another tenant's data. The server is the gate.
|
|
- CORS misconfiguration: same-origin via nginx (`nginx.conf:6-72`) — no `Access-Control-Allow-Origin: *` headers anywhere in nginx config.
|
|
- Directory traversal: nginx serves `dist/` only with the SPA fallback `try_files $uri $uri/ /index.html` (`nginx.conf:73-75`); proxy_pass directives are scoped to fixed upstream prefixes.
|
|
|
|
**Known gap (UX-only, not exploitable)**:
|
|
- `/admin` route lacks a client-side role-gate. Non-admin users navigating to `/admin` see the broken admin UI flicker before server-side 403s reject the API calls. Tracked as **finding F2 / AC-22** in `security_approach.md`. Server is authoritative — no privilege escalation. Step 4 hardening was paused; remains a Phase B candidate.
|
|
|
|
---
|
|
|
|
## A02 — Cryptographic Failures — PASS_WITH_KNOWN
|
|
|
|
**Verified controls** (per `security_approach.md` §3 + Cycle 2 audit):
|
|
- Bearer JWT held only in `AuthContext` React state (memory). Verified by manual grep + `STC-SEC3` static check + `NFT-SEC-01` test (`src/auth/AuthContext.test.tsx`).
|
|
- Refresh token in `Secure HttpOnly SameSite=Strict` cookie — never readable by JS. Verified by `STC-SEC4` static check.
|
|
- TLS termination at suite ingress (out-of-band of this workspace; documented in `security_approach.md` §9).
|
|
- No symmetric encryption performed client-side (no banned crypto libs per `tests/security/banned-deps.json` `signature_libs`).
|
|
- No localStorage/sessionStorage/IndexedDB persistence of secrets (`O2` anti-criterion, `persistence_libs` deny-list).
|
|
|
|
**Accepted trade-off (ADR-008)**:
|
|
- SSE bearer-in-query-string (`src/api/sse.ts:11`). `EventSource` cannot send headers; the bearer rides in `?access_token=…`. HTTPS encrypts the URL on the wire, but it appears in nginx access logs and (low risk) browser history. Mitigation: log redaction at the nginx layer is **NOT yet configured** (tracked under A05 below). Acknowledged in `security_approach.md` §4.
|
|
|
|
**Cycle 2 cryptography review**:
|
|
- `<TileLayer crossOrigin="use-credentials">` (AZ-498) — sends cookies on tile requests when same-origin. **Does NOT** send the bearer (bearer travels via `Authorization` header, which Leaflet does not set). Cookie is `SameSite=Strict`, so cross-site is impossible. Clean.
|
|
|
|
---
|
|
|
|
## A03 — Injection — PASS
|
|
|
|
**Browser-side surfaces**:
|
|
- **No SQL/NoSQL** — the SPA never builds SQL queries; all DB access is server-side.
|
|
- **No eval / Function constructor / setTimeout-of-string** — verified by manual grep this audit (zero matches in `src/` or `mission-planner/`).
|
|
- **No template injection** — React JSX escapes string children by default; no `dangerouslySetInnerHTML` anywhere (verified this audit + grep).
|
|
- **URL parameter construction** — searched `src/` for hand-built query strings; all observed cases use `encodeURIComponent` or template literals over typed values (e.g. `src/api/sse.ts:11`).
|
|
- **OS command injection** — N/A (browser has no shell).
|
|
|
|
**Output encoding** (XSS):
|
|
- React 19 default escaping handles all string content.
|
|
- The `HelpModal` ships hardcoded English strings inline (P6 violation — i18n only; XSS-safe).
|
|
- Annotation download tainted-canvas issue (`AnnotationsPage.handleDownload`) is a UX bug, not a security defect — image data may taint the canvas, the download silently fails. Already documented in `security_approach.md` §8.
|
|
|
|
**Cycle 2 review**: `getTileUrl()` reads `import.meta.env.VITE_SATELLITE_TILE_URL` (a build-time-frozen string) and passes it to Leaflet's `TileLayer.url` template. There is no user-controlled input on this path — no template-injection or URL-injection surface.
|
|
|
|
---
|
|
|
|
## A04 — Insecure Design — PASS
|
|
|
|
**Architectural pattern**:
|
|
- Same-origin via nginx → cookies scope cleanly; no cross-origin CSRF surface.
|
|
- Bearer in `Authorization` header (cannot be auto-attached by a cross-origin form).
|
|
- `SameSite=Strict` refresh cookie — no CSRF on the refresh endpoint either.
|
|
- Bearer-in-memory + short TTL + 401-retry → minimises window of compromise after XSS.
|
|
- No client-side persistence of mutable state — server is source of truth.
|
|
|
|
This pattern is the recommended approach for an internal-operator SPA per current OWASP cheatsheet guidance.
|
|
|
|
**No design-level violations identified.** The only design decision flagged in past reviews — the SSE bearer-in-query-string — is an `EventSource`-protocol limitation, not a design choice.
|
|
|
|
---
|
|
|
|
## A05 — Security Misconfiguration — FAIL
|
|
|
|
**Failures (confirmed by `nginx.conf` inspection)**:
|
|
- **NO** `Content-Security-Policy` header (recommended starting point in `security_approach.md` §9).
|
|
- **NO** `X-Frame-Options: DENY` (or CSP `frame-ancestors`). Clickjacking surface.
|
|
- **NO** `Referrer-Policy: strict-origin-when-cross-origin`.
|
|
- **NO** `Strict-Transport-Security` (TLS terminated at suite ingress; HSTS should be set there or here — needs decision).
|
|
- **NO** `X-Content-Type-Options: nosniff`.
|
|
- **NO** bearer-redaction in nginx access logs for SSE URLs (acknowledged in `security_approach.md` §4 and §9).
|
|
|
|
**Other**:
|
|
- Default credentials: not applicable — auth is server-side via the `admin/` service.
|
|
- Unnecessary features enabled: nginx config is minimal (only `client_max_body_size 500M` + per-service proxy_pass).
|
|
- Verbose error messages in production: not applicable — Vite production build strips dev banners.
|
|
|
|
**Remediation**: track all six header/redaction items as a single Phase B ticket against the SPA's `nginx.conf` (low risk, code-only change). See `infrastructure_review.md` F-INF-2.
|
|
|
|
---
|
|
|
|
## A06 — Vulnerable & Outdated Components — FAIL
|
|
|
|
**Findings** (full detail in `dependency_scan.md`):
|
|
- F-DEP-1: vite `<= 6.4.1` — Arbitrary File Read via Dev Server WebSocket — HIGH (dev-server only).
|
|
- F-DEP-2: vite `<= 6.4.1` — Path Traversal in Optimized Deps `.map` — MODERATE (dev-server only).
|
|
- F-DEP-3: postcss `< 8.5.10` — XSS via Unescaped `</style>` — MODERATE (low surface — no untrusted CSS in this build).
|
|
|
|
**Production-bundle exploitability**: NONE — all three findings are dev-time only. Production runtime is `nginx:alpine` serving pre-built static assets.
|
|
|
|
**Verdict**: FAIL on the audit category because `bun audit` reports a HIGH advisory and the remediation is trivially available (`bun update vite`). Lifting these immediately is straightforward.
|
|
|
|
---
|
|
|
|
## A07 — Identification & Authentication Failures — PASS_WITH_KNOWN
|
|
|
|
**Verified controls**:
|
|
- Bearer JWT signed and validated server-side; UI never inspects token contents.
|
|
- Refresh-token rotation on 401 (`src/api/client.ts:88-99`).
|
|
- Server is authoritative on lockout, brute-force, and MFA enforcement.
|
|
- Logout: `POST /api/admin/auth/logout` clears bearer in memory; server invalidates the refresh cookie.
|
|
|
|
**Known gap**:
|
|
- Bootstrap (cold-load) refresh missing `credentials:'include'` (`src/auth/AuthContext.tsx:24`). Effect: even with a valid refresh cookie, cold-load refresh fails → user is bounced to `/login`. Tracked as **F2 / AC-01** with a `it.fails` quarantined test that flips when the fix lands. Documented in `security_approach.md` §1. Functional/UX bug, not a security regression — server still rejects unauthenticated requests properly.
|
|
|
|
---
|
|
|
|
## A08 — Software & Data Integrity Failures — FAIL
|
|
|
|
**Verified controls**:
|
|
- `bun install --frozen-lockfile` in `Dockerfile:4` enforces lockfile fidelity (no in-build dependency drift).
|
|
- `AZAION_REVISION=$CI_COMMIT_SHA` baked into the image (`Dockerfile:9-10`); OCI labels stamped at push time (`.woodpecker/build-arm.yml:23-28`).
|
|
|
|
**Failures**:
|
|
- **NO SBOM emission** (Syft / cyclonedx-bom). Cannot audit the produced image's bill of materials post-hoc.
|
|
- **NO image signing** (cosign / docker content trust). The registry has no integrity guarantee on `ui:dev-arm` / `ui:stage-arm` / `ui:main-arm`.
|
|
- **NO vulnerability scan** (Trivy / Grype) on the produced image. Base-image CVEs (e.g. in `nginx:alpine`) are invisible to CI.
|
|
- **NO `bun audit` step** in `.woodpecker/build-arm.yml` — Cycle 2 dependency findings would not have failed CI.
|
|
|
|
**Remediation priorities**:
|
|
1. Quick: add `bun audit --high` exit-code gate to the pipeline (catches future regressions).
|
|
2. Medium: add Trivy scan on the produced image (surfaces base-image and OS-package CVEs).
|
|
3. Long: SBOM + cosign signing — coordinate at the suite level (depends on registry capabilities).
|
|
|
|
See `infrastructure_review.md` F-INF-1, F-INF-3, F-INF-4.
|
|
|
|
---
|
|
|
|
## A09 — Security Logging & Monitoring Failures — N/A
|
|
|
|
The SPA does not emit audit logs. All audit events are emitted by the server-side suite services (`admin/`, `flights/`, `annotations/`, `detect/`, `loader/`, `resource/`, `gps-denied-*`, `autopilot/`). The browser console is the only client-side log surface; no centralized client telemetry exists today.
|
|
|
|
**Justification for N/A** (per `security_approach.md` §10 + `_docs/00_problem/anti_criteria.md`): the SPA is internal/operator-only — observability is a suite-level concern intentionally NOT duplicated client-side.
|
|
|
|
---
|
|
|
|
## A10 — Server-Side Request Forgery — N/A
|
|
|
|
The codebase under audit is a browser SPA. There is no server-side request surface that accepts URLs from user input. The SPA's outbound calls are:
|
|
- Same-origin nginx proxies (`/api/<service>/*`) — fixed paths, server-authoritative routing.
|
|
- Build-time-fixed env URLs: `VITE_OWM_BASE_URL` (defaults to `https://api.openweathermap.org/data/2.5`), `VITE_SATELLITE_TILE_URL` (defaults to `http://localhost:5100/...`).
|
|
|
|
Neither URL is user-controllable at runtime. The only browser endpoint resembling SSRF — passing the user's address through to Google Geocode in `mission-planner/` — is in the port-source which is NOT shipped (see `static_analysis.md` F-SAST-1). The suite-level recommendation is to proxy any future geocoding through a server-side endpoint to remove the client-visible API key, which would naturally introduce real SSRF surface; that future ticket should explicitly validate URL inputs.
|
|
|
|
---
|
|
|
|
## Self-verification
|
|
|
|
- [x] All ten OWASP Top 10 (2021) categories assessed
|
|
- [x] Every FAIL has at least one specific finding with file path / line
|
|
- [x] N/A categories have explicit justification
|
|
- [x] `security_approach.md` cross-referenced — every existing known-gap is reflected here, not hidden
|
|
- [x] Cycle 2 changes (AZ-498, AZ-499) reviewed under each applicable category
|