satellite-provider/_docs/05_security/owasp_review_cycle8.md

# OWASP Top 10 Review (Cycle 8)

**Date**: 2026-05-23
**Mode**: Delta scan against OWASP Top 10:2021 (current at audit time per https://owasp.org/www-project-top-ten/ — verified 2026-05-23; the 2025 candidate revision is still in public-comment phase and not adopted).
**Scope**: Cycle-8 delta only — AZ-808 (region POST validator), AZ-809 (route POST validator + per-point + per-polygon), AZ-810 (UAV upload metadata validator + custom filter), AZ-811 (lat/lon GET validator + unknown-query-param filter), AZ-812 (region-API `Latitude`/`Longitude` → `Lat`/`Lon` rename). Earlier cycles' OWASP reviews remain authoritative for their respective surfaces; this file does NOT re-walk the cycle-5 / cycle-7 baselines.

## A01 — Broken Access Control

**Status**: PASS

- `.RequireAuthorization()` is preserved on every cycle-8 endpoint:
  - `POST /api/satellite/request` at `Program.cs:251`
  - `POST /api/satellite/route` at `Program.cs:267`
  - `GET /api/satellite/tiles/latlon` at `Program.cs:213`
  - `POST /api/satellite/upload` at `Program.cs:238` (additionally requires the `GPS` permission claim via `SatellitePermissions.UavUploadPolicy`).
- Endpoint-filter execution order is governed by ASP.NET Core's middleware → routing → endpoint-filter pipeline. `app.UseAuthorization()` (line 206) reads the endpoint metadata produced by `.RequireAuthorization()` and short-circuits anonymous callers with 401 BEFORE the endpoint dispatch reaches any endpoint filter. Cycle-8 verification: none of the four new validation paths runs unless the caller is authenticated.
- The new `RejectUnknownQueryParamsEndpointFilter` (lat/lon GET) does not establish its own auth gate — it relies on the endpoint chain's existing `.RequireAuthorization()`. Anonymous query-param probing is impossible.
- No new CORS policy in cycle 8. `TilesCors` (cycle-6 baseline) is unchanged.
- No new IDOR paths — the four endpoints operate on caller-supplied identifiers but do not couple them to any tenant or owner field; tiles remain globally-scoped in the post-AZ-484 model. Geospatial identifiers (`Lat`, `Lon`, `Z/X/Y`) are deterministic projections of physical reality, not capability tokens.

## A02 — Cryptographic Failures

**Status**: N/A (cycle 8)

- Cycle 8 has no cryptographic operations. JWT validation is unchanged from cycle 4 (`AddSatelliteJwt` — HS256 with ≥ 32-byte secret, `ValidateLifetime + ValidateIssuer + ValidateAudience = true`, ClockSkew = 30s).
- The cycle-5 UUIDv5 SHA-1 surface (`Uuidv5.Create`) is unaffected.
- TLS posture (Kestrel `Http1AndHttp2` with self-signed dev cert / ingress termination in prod) — unchanged from cycle 6.
- F-AZ810-2 (`DateTime` vs `DateTimeOffset` parsing) is NOT a crypto failure — it's a time-handling correctness concern documented under A09.

## A03 — Injection

**Status**: PASS

- No SQL / Dapper / Npgsql usage in any cycle-8 new file. (`grep -r 'Dapper\|Npgsql' SatelliteProvider.Api/Validators SatelliteProvider.Api/DTOs SatelliteProvider.Common/DTO` → zero matches across the cycle-8 surface.)
- No `Process.Start` / shell-out / `eval` in any cycle-8 new file.
- All inputs reaching the cycle-8 validators are strongly typed by the time the rules execute (`double`, `int`, `Guid`, `DateTime`, `string`, `IReadOnlyList<T>`). `System.Text.Json` has already parsed and rejected anything malformed before the validator runs, and the cycle-7 deserializer hardening (`UnmappedMemberHandling.Disallow`) is in force on every cycle-8 path including the multipart `metadata` field (`UavUploadValidationFilter.cs:73` uses the same global `JsonSerializerOptions` via `IOptions<JsonOptions>`).
- The cycle-8 wire-format rename (AZ-812) means the deserializer now strictly enforces `lat` / `lon` against the post-rename JSON schema — legacy `Latitude` / `Longitude` fields are rejected as unknown members, surfacing as `JsonException` → 400 via `GlobalExceptionHandler` (not silently bound to a fallback property). This is a small *anti-injection* improvement at the schema boundary.

## A04 — Insecure Design

**Status**: **PASS (post-follow-up)** — was PASS_WITH_WARNINGS at audit time; F-AZ809-1 was the only Medium and was resolved in the Step-14 follow-up commit (cycle 8). Post-follow-up posture is PASS.

- AZ-808 / AZ-809 / AZ-810 / AZ-811 are themselves a *design fix* for the remaining unprotected endpoints — completing the AZ-795 epic's per-endpoint rollout. Pre-cycle-8, four endpoints (region POST, route POST, lat/lon GET, UAV upload) used ad-hoc inline `try/catch` blocks or no input validation at all. Cycle 8 centralises every public endpoint behind one of three approved validation paths:
  1. `WithValidation<T>()` for JSON-body endpoints (RegionRequest, CreateRouteRequest).
  2. `WithValidation<T>()` + `RejectUnknownQueryParamsEndpointFilter` for query-string endpoints (GetTileByLatLonQuery).
  3. Custom `IEndpointFilter` for non-standard wire formats (`UavUploadValidationFilter` for multipart).
- The cycle-7 architecture-doc § 9 coverage table now reaches **100% of public-facing input endpoints** with validators (region POST, route POST, lat/lon GET, inventory POST, UAV upload). Future drift visibility is high.
- **F-AZ809-1** (from `static_analysis_cycle8.md`) identified a design-level gap: `CreateRouteRequestValidator` lacked a max-count cap on `Geofences.Polygons`, in contrast to every other list-bearing field across the API (route `Points` ≤ 500, UAV `Items` ≤ 100, inventory `Tiles`/`LocationHashes` ≤ 5000). **Resolved in the Step-14 follow-up (cycle 8)**: `MaxPolygons = 50` cap added + matching unit + integration tests + `route-creation.md` v1.0.1 Inv-10. The pattern "cap every collection field" is now fully consistent across the API.

## A05 — Security Misconfiguration

**Status**: PASS

- `UnmappedMemberHandling.Disallow` (cycle-7 global default) is now backed by per-endpoint FluentValidation rules at every cycle-8 endpoint, completing the defence-in-depth hardening for mass-assignment prevention.
- Swagger exposure is still gated by `app.Environment.IsDevelopment()` (unchanged).
- `appsettings.Development.json` clearly tags DEV-ONLY JWT iss/aud values; `appsettings.json` ships empty so production fail-fast triggers if env vars are missing (unchanged from cycle 4).
- The new `AddTransient<UavUploadValidationFilter>` registration in `Program.cs:128` is correct — transient (not singleton) ensures each request gets a fresh filter instance, preventing accidental cross-request state retention through filter-instance fields.
- The cycle-7 `AddValidatorsFromAssemblyContaining<Program>()` scope rule is unchanged; the cycle-8 validators all live in `SatelliteProvider.Api.dll`, so the reflection scan correctly picks them up.
- **Note (informational, not a finding)**: the global Kestrel body-size limit (`MaxRequestBodySize = 500 MiB`) was originally set for the UAV upload endpoint in cycle 2 (AZ-488). It applies to every endpoint by default because Kestrel exposes a per-server, not per-endpoint, default. Cycle 8's F-AZ809-1 highlights the consequence: tight per-endpoint count caps in validators are now the primary defence on JSON endpoints, not the framework body limit. A future hardening cycle could narrow the body limit per-endpoint via `IRequestSizeLimitMetadata` on the `RouteHandlerBuilder` for non-upload endpoints — but this is hardening, not a finding.

## A06 — Vulnerable & Outdated Components

**Status**: PASS_WITH_WARNINGS (carry-over Low)

- See `dependency_scan_cycle8.md` for the full table. Summary:
  - Cycle 8 added zero new packages and bumped zero existing packages.
  - **D-AZ795-1** (cycle-7 carry-over): `FluentValidation` + `FluentValidation.DependencyInjectionExtensions` 12.0.0 → 12.1.1 hardening recommendation — still open. Cycle 8 did not bump.
  - **D2-cy4** (cycle-4 carry-over): `Microsoft.NET.Test.Sdk 17.8.0` transitive `NuGet.Frameworks` Medium — test-runtime exposure only, still open.

## A07 — Identification and Authentication Failures

**Status**: PASS

- JWT validation parameters unchanged from cycle 4 (`AddSatelliteJwt`).
- No new auth-bypass paths introduced by cycle 8. The four new validators / filters cannot run for anonymous callers (see A01).
- The five new integration test files (`CreateRouteValidationTests`, `GetTileByLatLonValidationTests`, `RegionFieldRenameTests`, `RegionRequestValidationTests`, `UavUploadValidationTests`) all mint valid tokens via the shared `JwtTestHelpers.MintAuthenticated(...)` — proves the happy path is properly auth-gated and not relying on any test-only bypass.
- `RejectUnknownQueryParamsEndpointFilter` rejects malicious query keys (e.g. `?debug=1`, `?Authorization=…`) at the endpoint-filter layer — narrows the auth fingerprinting surface vs. the pre-cycle-8 behaviour where unknown keys silently bound to defaults.

## A08 — Software and Data Integrity Failures

**Status**: N/A (cycle 8)

- No CI/CD changes, no artifact-signing changes, no auto-update paths touched in cycle 8. The only CI/script-adjacent file modified is `scripts/run-performance-tests.sh` (wire-rename diff only — see `static_analysis_cycle8.md` § Test Code Review).

## A09 — Security Logging and Monitoring Failures

**Status**: PASS_WITH_WARNINGS (3 Lows — F-AZ810-1 new + F-AZ795-1 + F-AZ795-2 carry-over)

- `GlobalExceptionHandler` 5xx branch logs `Method`, `Path`, `correlationId`, and the exception object (via Serilog default). The 4xx branch does NOT log the exception (intentional, avoids noisy log signal from malformed-payload spam). Unchanged from cycle 7.
- The cycle-8 validators and the new `UavUploadValidationFilter` do NOT add any logging — they return `Results.ValidationProblem(...)` directly without any audit trail. This is consistent with cycle 7's `ValidationEndpointFilter<T>` posture (the response itself carries enough detail for the client to self-debug; the server logs would otherwise drown in 400s from typo-prone callers). Acceptable for the threat model.
- **F-AZ810-1** (NEW in cycle 8): `UavUploadValidationFilter.cs:82` echoes `JsonException.Message` to the client — same information-disclosure pattern as cycle-7 F-AZ795-1 in a NEW code path. Both surfaces leak `System.*` type names + parse positions. **Filed as Low**.
- **F-AZ795-1** (cycle-7 carry-over): `GlobalExceptionHandler.cs:108-117` — still open.
- **F-AZ795-2** (cycle-7 carry-over): `GlobalExceptionHandler.cs:88-93` — still open. Cycle 8 reduces practical reachability on the 4 newly-validated endpoints but doesn't eliminate it.
- **F-AZ810-2** (Informational, listed under A09 as a time-handling correctness concern with downstream logging/monitoring implications): `UavTileMetadata.CapturedAt` typed `DateTime` not `DateTimeOffset` — stale tiles could pass the freshness check in dev environments with non-UTC host TZ if the caller omits the `Z` suffix. Zero observable impact in UTC-deployed production. **Filed as Low / Informational**.

## A10 — Server-Side Request Forgery (SSRF)

**Status**: N/A (cycle 8)

- No URL-input fields, no outbound HTTP calls triggered by the cycle-8 surface. The pre-existing `GoogleMapsDownloaderV2` (outbound calls to Google Maps for tile fetches in the region/route processing paths) is not modified by cycle 8.
- The new `RejectUnknownQueryParamsEndpointFilter` works on the request's already-parsed query collection — no URL parsing, no outbound resolution.

## Cross-Reference with `security_approach.md`

The repo does not contain `_docs/00_problem/security_approach.md` (same as cycle 7). The OWASP review proceeds against the cycle-5 + cycle-6 + cycle-7 architectural decisions documented in `_docs/02_document/architecture.md` § 7 (Security Architecture) and § 9 (Strict wire-format validation at the API edge — added in cycle 7, extended in cycle 8). Cycle 8's input-validation completion cleanly extends those decisions; the F-AZ809-1 gap is the only deviation and is documented as such.

## Verdict (Phase 3)

**PASS_WITH_WARNINGS** (post-follow-up) — A04 (Insecure Design) is now **PASS** (F-AZ809-1 resolved in Step-14 follow-up); A06 + A09 remain `PASS_WITH_WARNINGS` from the carry-over Lows + the new F-AZ810-1 Low. Every other OWASP category is PASS or N/A.

Per the skill's verdict-logic, Medium severity yields PASS_WITH_WARNINGS — but cycle 8 resolved its only Medium in-cycle via the Step-14 follow-up commit before any production exposure. Post-follow-up posture is cleaner than the audit-time snapshot.