diff --git a/SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs b/SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs index 86e4bd7..8a1a071 100644 --- a/SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs +++ b/SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs @@ -27,6 +27,15 @@ public sealed class CreateRouteRequestValidator : AbstractValidator req.Geofences!.Polygons) .NotNull().WithMessage("`geofences.polygons` is required when `geofences` is present.") .NotEmpty().WithMessage("`geofences.polygons` must contain at least 1 polygon when `geofences` is present.") + .Must(polygons => polygons is null || polygons.Count <= MaxPolygons) + .WithMessage($"`geofences.polygons` must contain at most {MaxPolygons} polygons.") .OverridePropertyName("geofences.polygons"); RuleForEach(req => req.Geofences!.Polygons) diff --git a/SatelliteProvider.IntegrationTests/CreateRouteValidationTests.cs b/SatelliteProvider.IntegrationTests/CreateRouteValidationTests.cs index 0a8c159..f4dc1fc 100644 --- a/SatelliteProvider.IntegrationTests/CreateRouteValidationTests.cs +++ b/SatelliteProvider.IntegrationTests/CreateRouteValidationTests.cs @@ -49,6 +49,9 @@ public static class CreateRouteValidationTests // Rule 9: geofence corners + NW-of-SE invariant await GeofenceNwLatNotGreaterThanSeLat_Returns400(httpClient); + // Rule 9b: geofence polygon-count cap (F-AZ809-1 security-audit fix) + await GeofencePolygonsTooMany_Returns400(httpClient); + // Rule 10/11: requestMaps + createTilesZip required await MissingRequestMaps_Returns400(httpClient); @@ -360,6 +363,48 @@ public static class CreateRouteValidationTests Console.WriteLine(" ✓ NW.lat <= SE.lat rejected by cross-field invariant"); } + private static async Task GeofencePolygonsTooMany_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-809 rule 9b (security-audit F-AZ809-1): geofence polygon-count > 50 → HTTP 400"); + + // Arrange — 51 polygons, each valid bbox. Only the count rule should fire. + var routeId = Guid.NewGuid(); + var polygonsJson = string.Join( + ",\n ", + Enumerable + .Range(0, 51) + .Select(_ => "{ \"northWest\": { \"lat\": 50.15, \"lon\": 36.05 }, \"southEast\": { \"lat\": 50.05, \"lon\": 36.15 } }")); + var body = $$""" + { + "id": "{{routeId}}", + "name": "too-many-polygons", + "regionSizeMeters": 1000, + "zoomLevel": 18, + "points": [ + { "lat": 50.10, "lon": 36.10 }, + { "lat": 50.11, "lon": 36.11 } + ], + "geofences": { + "polygons": [ + {{polygonsJson}} + ] + }, + "requestMaps": false, + "createTilesZip": false + } + """; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-809 geofence polygons too many"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem(problem, expectedStatus: 400, label: "AZ-809 geofence polygons too many", expectedErrorPath: "geofences.polygons"); + + Console.WriteLine(" ✓ 51 polygons rejected with errors[\"geofences.polygons\"] (cap is 50)"); + } + private static async Task MissingRequestMaps_Returns400(HttpClient httpClient) { Console.WriteLine(); diff --git a/SatelliteProvider.Tests/Validators/CreateRouteRequestValidatorTests.cs b/SatelliteProvider.Tests/Validators/CreateRouteRequestValidatorTests.cs index 9436b7a..c7967ca 100644 --- a/SatelliteProvider.Tests/Validators/CreateRouteRequestValidatorTests.cs +++ b/SatelliteProvider.Tests/Validators/CreateRouteRequestValidatorTests.cs @@ -249,6 +249,33 @@ public class CreateRouteRequestValidatorTests result.ShouldHaveValidationErrorFor("geofences.polygons"); } + [Fact] + public void Validate_GeofencePolygonsTooMany_FailsCountRule() + { + // Arrange — 51 polygons; cap is 50 (security-audit F-AZ809-1 fix). + // Each polygon is a valid bbox so only the count rule should fire. + var request = ValidRequest(); + request.Geofences = new Geofences + { + Polygons = Enumerable + .Range(0, 51) + .Select(_ => new GeofencePolygon + { + NorthWest = new GeoPoint(50.15, 36.05), + SouthEast = new GeoPoint(50.05, 36.15), + }) + .ToList(), + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert — OverridePropertyName makes the count rule fire at the + // wire-format path `geofences.polygons` (not the leaf-only `polygons`). + result.ShouldHaveValidationErrorFor("geofences.polygons") + .WithErrorMessage("`geofences.polygons` must contain at most 50 polygons."); + } + [Fact] public void Validate_CreateTilesZipWithoutRequestMaps_FailsCrossFieldRule() { diff --git a/_docs/02_document/contracts/api/route-creation.md b/_docs/02_document/contracts/api/route-creation.md index bbf3463..80136b1 100644 --- a/_docs/02_document/contracts/api/route-creation.md +++ b/_docs/02_document/contracts/api/route-creation.md @@ -3,9 +3,9 @@ **Component**: WebApi (`SatelliteProvider.Api`) producing rows via RouteManagement (`SatelliteProvider.Services.RouteManagement`) and feeding the background Route Map Processing flow (Flow F5) **Producer task**: AZ-809 — `_docs/02_tasks/done/AZ-809_route_endpoint_validation.md` (validator + this contract) **Consumer tasks**: `gps-denied-onboard` AZ-777 Phase 2 (preferred imagery-seeding path — route-based rather than bbox-based) -**Version**: 1.0.0 +**Version**: 1.0.1 **Status**: frozen -**Last Updated**: 2026-05-22 +**Last Updated**: 2026-05-23 ## Purpose @@ -62,7 +62,7 @@ Per-field constraints: | `points[i].lat` | number | yes (`[JsonRequired]`) | WGS84 latitude. | `[-90.0, 90.0]`. | | `points[i].lon` | number | yes (`[JsonRequired]`) | WGS84 longitude. | `[-180.0, 180.0]`. | | `geofences` | object | no | When present, intermediate points outside ALL polygons get filtered before region enqueue. | See nested shape below. | -| `geofences.polygons` | array | yes (`[JsonRequired]` when `geofences` present) | One or more bbox polygons (NW corner + SE corner). | Non-empty when `geofences` present. | +| `geofences.polygons` | array | yes (`[JsonRequired]` when `geofences` present) | One or more bbox polygons (NW corner + SE corner). | Count `[1, 50]` when `geofences` present. | | `geofences.polygons[i].northWest` | object | yes (`[JsonRequired]`) | Polygon's northwest corner. | See `GeoPoint` shape. | | `geofences.polygons[i].southEast` | object | yes (`[JsonRequired]`) | Polygon's southeast corner. | See `GeoPoint` shape. | | `requestMaps` | bool | yes (`[JsonRequired]`) | When `true`, enqueue background region-requests for every route point inside the geofences (or all points if no geofences). | No default — caller must declare intent. | @@ -166,6 +166,7 @@ Example body for a polygon corner invariant failure: - **Inv-7** (per-polygon shape): `northWest` AND `southEast` corners both present. - **Inv-8** (per-polygon invariant): `northWest.lat > southEast.lat` AND `northWest.lon < southEast.lon`. - **Inv-9**: Unknown root or nested fields → 400 (deserializer's `UnmappedMemberHandling.Disallow`). +- **Inv-10**: When `geofences` is present, `geofences.polygons.Count` is in `[1, 50]`. Cap to bound validator allocation worst case — see `_docs/05_security/security_report_cycle8.md` § F-AZ809-1. Realistic use is 1-10 polygons per route; the 50 cap leaves 5x headroom. ## Non-Goals @@ -199,6 +200,7 @@ Example body for a polygon corner invariant failure: | point-lon-out-of-range | `"points":[..., {"lat":..,"lon":181}]` | HTTP 400 + `errors["points[1].lon"]` | Rule 8 | | geofence-nw-not-north | NW.lat == SE.lat | HTTP 400 + `errors["geofences.polygons[0].northWest"]` | Rule 9 / Inv-8 | | geofence-nw-not-west | NW.lon == SE.lon | HTTP 400 + `errors["geofences.polygons[0].northWest"]` | Rule 9 / Inv-8 | +| geofence-polygons-too-many | 51-polygon array | HTTP 400 + `errors["geofences.polygons"]` ("must contain at most 50 polygons.") | Rule 9b / Inv-10 (F-AZ809-1 follow-up) | | missing-requestMaps | body without `requestMaps` | HTTP 400 + `errors[requestMaps]` | Rule 10 | | createTilesZip-without-requestMaps | `"requestMaps":false,"createTilesZip":true` | HTTP 400 + `errors[createTilesZip]` | Rule 12 (cross-field) | | unknown-root-field | extra `"debug":"..."` key | HTTP 400 + `errors[debug]` | Rule 13 (`UnmappedMemberHandling.Disallow`) | @@ -211,3 +213,4 @@ Example body for a polygon corner invariant failure: | Version | Date | Change | Author | |---------|------|--------|--------| | 1.0.0 | 2026-05-22 | Initial contract for `POST /api/satellite/route`. Publishes the FluentValidation surface (CreateRouteRequestValidator + RoutePointValidator + GeofencePolygonValidator) + the 14 rules in AZ-809, including the probe-confirmed missing-id gap and the cross-field `createTilesZip ⇒ requestMaps` invariant. References `error-shape.md` v1.0.0, `region-request.md` v1.0.0 (F5 enqueue path), and Flows F4/F5 (cross-link). | autodev (Step 10, cycle 8) | +| 1.0.1 | 2026-05-23 | Tighten `geofences.polygons` constraint from "non-empty" to "Count `[1, 50]`". New Inv-10 + test case `geofence-polygons-too-many`. Patch release per Versioning Rules (tightening an existing range). The 50-polygon cap closes a defence-gap surfaced by the cycle-8 security audit: without the cap, an authenticated caller could submit millions of polygons in a single 500 MiB request and saturate the validator's allocation heap. Realistic use is 1-10 polygons per route. | autodev (Step 14 follow-up, cycle 8) | diff --git a/_docs/02_document/tests/blackbox-tests.md b/_docs/02_document/tests/blackbox-tests.md index 9c938ab..459606e 100644 --- a/_docs/02_document/tests/blackbox-tests.md +++ b/_docs/02_document/tests/blackbox-tests.md @@ -318,7 +318,7 @@ Cycle 8 extends the AZ-795 shared validation infrastructure (FluentValidation + ## BT-29: Create Route Endpoint — Nested + Cross-Field Strict Validation **Trigger**: A family of `POST /api/satellite/route` calls covering AZ-809's 14 rules. Bodies use the post-AZ-809 wire shape `{id, name, description?, regionSizeMeters, zoomLevel, points: [{lat, lon}, …], requestMaps, createTilesZip, geofences?: {polygons: [{northWest, southEast}]}}`. -**Precondition**: API up; valid JWT attached. `error-shape.md` v1.0.0 + `route-creation.md` v1.0.0 frozen. +**Precondition**: API up; valid JWT attached. `error-shape.md` v1.0.0 + `route-creation.md` v1.0.1 frozen. **Expected**: HTTP 400 with `Content-Type: application/problem+json` for every failure sub-case; HTTP 200 + `RouteResponse` for the happy path. `errors[]` keys use the nested path (e.g. `points[1].lat`, `geofences.polygons[0].northWest`). | # | Rule | Trigger excerpt | Expected `errors` key | Test method | @@ -334,6 +334,7 @@ Cycle 8 extends the AZ-795 shared validation infrastructure (FluentValidation + | 8a | Per-point `lat` in `[-90, 90]` | `points[1].lat=91` | `points[1].lat` | `PointLatOutOfRange_Returns400` | | 8b | Per-point `lon` in `[-180, 180]` | `points[1].lon=181` | `points[1].lon` | `PointLonOutOfRange_Returns400` | | 9 | Geofence cross-field invariant (`NW.lat > SE.lat` AND `NW.lon < SE.lon`) | NW.lat ≤ SE.lat | `geofences.polygons[0].northWest` | `GeofenceNwSeInvariantViolated_Returns400` | +| 9b | `geofences.polygons.Count <= 50` (security-audit F-AZ809-1 fix, `route-creation.md` v1.0.1 Inv-10) | 51-polygon array | `geofences.polygons` ("must contain at most 50 polygons.") | `GeofencePolygonsTooMany_Returns400` | | 10 | Required `requestMaps` (no defaulting) | omit `requestMaps` | `requestMaps` | `MissingRequestMaps_Returns400` | | 12 | Cross-field: `createTilesZip=true` requires `requestMaps=true` | `{createTilesZip:true, requestMaps:false}` | top-level message (no per-field key) | `CreateTilesZipRequiresRequestMaps_Returns400` | | 13 | Unknown root field | `"debug":42` | path mentioning `debug` | `UnknownRootField_Returns400` | diff --git a/_docs/02_document/tests/traceability-matrix.md b/_docs/02_document/tests/traceability-matrix.md index 8dc83e7..1fc59ca 100644 --- a/_docs/02_document/tests/traceability-matrix.md +++ b/_docs/02_document/tests/traceability-matrix.md @@ -130,6 +130,7 @@ | AZ-808 AC-7 | OpenAPI marks `RequestRegionRequest` fields `required`, declares ranges, documents 400 response | Doc-state AC — verified at Step 13 (Update Docs) review against published `/swagger/v1/swagger.json`; Swashbuckle annotations match AZ-796 pattern | ◐ doc-verified at Step 13 | | AZ-808 AC-8 | Manual probe script exercises each failure mode via `curl` + JWT | Structural: `scripts/probe_region_validation.sh` exists and is manually runnable | ✓ | | AZ-809 AC-1 | Each of the 14 route-creation validations rejects with HTTP 400 + ValidationProblemDetails (single-rule precision) | BT-29 sub-cases 1..14 (blackbox); `CreateRouteValidationTests` (integration, 14+ failure methods covering deserializer + per-DTO + per-element + cross-field layers) + `CreateRouteRequestValidatorTests` + `RoutePointValidatorTests` + `GeofencePolygonValidatorTests` (unit, ≥ 13 methods total) | ✓ | +| AZ-809 AC-1b (security-audit F-AZ809-1 follow-up; `route-creation.md` v1.0.1 Inv-10) | `geofences.polygons.Count <= 50` rejects with HTTP 400 + `errors["geofences.polygons"]` | BT-29 sub-case 9b (blackbox); `CreateRouteValidationTests.GeofencePolygonsTooMany_Returns400` (integration); `CreateRouteRequestValidatorTests.Validate_GeofencePolygonsTooMany_FailsCountRule` (unit) | ✓ | | AZ-809 AC-2 | Happy path unchanged — valid body returns HTTP 200 + `RouteResponse`; F5 background still runs when `requestMaps=true`; probe's 2-point 132 m route completes < 20 s | BT-29 sub-case `pos` (`CreateRouteValidationTests.HappyPath_Returns200`); no regression in existing `RouteCreationTests.cs` (cycle 8 Step 11 — green) | ✓ | | AZ-809 AC-3 | `CreateRouteRequestValidator`, `RoutePointValidator`, `GeofencePolygonValidator` each in their own files under `SatelliteProvider.Api/Validators/`; ≥ 13 unit-test methods total | Structural: three validator files exist as separate `.cs` files; unit-test files together contain ≥ 13 methods covering every `RuleFor` / `RuleForEach` chain | ✓ | | AZ-809 AC-4 | `CreateRouteValidationTests.cs` covers happy + 13+ failure modes; MUST include `Post_WithMissingId_ReturnsBadRequest` | `SatelliteProvider.IntegrationTests/CreateRouteValidationTests.cs` includes `MissingId_Returns400` (the AZ-777 Phase 2 reproducer for route variant) and ≥ 14 total failure methods | ✓ | diff --git a/_docs/05_security/owasp_review_cycle8.md b/_docs/05_security/owasp_review_cycle8.md index fc03dd8..e9440ea 100644 --- a/_docs/05_security/owasp_review_cycle8.md +++ b/_docs/05_security/owasp_review_cycle8.md @@ -38,14 +38,14 @@ ## A04 — Insecure Design -**Status**: PASS_WITH_WARNINGS (Medium — F-AZ809-1) +**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()` for JSON-body endpoints (RegionRequest, CreateRouteRequest). 2. `WithValidation()` + `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. -- **However, F-AZ809-1** (from `static_analysis_cycle8.md`) identifies a design-level gap: `CreateRouteRequestValidator` lacks 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). The pattern across the API is "cap every collection field"; one collection slipped past. The global `KestrelServerOptions.Limits.MaxRequestBodySize = 500 MiB` was sized for the UAV upload endpoint but applies to every route — leaving the validator as the sole gate against a million-element collection submission on the route endpoint. **Filed as Medium**. +- **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 @@ -106,6 +106,6 @@ The repo does not contain `_docs/00_problem/security_approach.md` (same as cycle ## Verdict (Phase 3) -**PASS_WITH_WARNINGS** — A04 (Insecure Design) is `PASS_WITH_WARNINGS` because of F-AZ809-1 (Medium); A06 + A09 are `PASS_WITH_WARNINGS` from the carry-over Lows + the new F-AZ810-1 Low. Every other OWASP category is PASS or N/A. +**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 — not FAIL (FAIL is reserved for Critical or High). The cycle-8 threat model (authenticated callers only on every relevant endpoint) contains the Medium within an auth-gated surface, but F-AZ809-1 must be the highest-priority cycle-9 follow-up because the missing collection cap is the only inconsistency across the API's otherwise-uniform "cap every collection field" pattern, and the global Kestrel body limit (500 MiB) means the validator is the sole gate against an unbounded submission today. If a future feature exposes the route endpoint to an untrusted-tenant audience, this Medium would promote without warning. +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. diff --git a/_docs/05_security/security_report_cycle8.md b/_docs/05_security/security_report_cycle8.md index aec6dc6..cce3640 100644 --- a/_docs/05_security/security_report_cycle8.md +++ b/_docs/05_security/security_report_cycle8.md @@ -3,17 +3,17 @@ **Date**: 2026-05-23 **Scope**: Cycle-8 delta over the cycle-7 audit (`_docs/05_security/security_report_cycle7.md`). Cycle-8 surface = AZ-808 (region POST validator) + AZ-809 (route POST validator + per-point + per-polygon) + AZ-810 (UAV upload metadata validator + custom `UavUploadValidationFilter`) + AZ-811 (lat/lon GET validator + `RejectUnknownQueryParamsEndpointFilter`) + AZ-812 (region-API `Latitude`/`Longitude` → `Lat`/`Lon` wire rename — OSM convention). **Trigger**: `/autodev` Step 14 (Security Audit) — feature cycle 8, post-implementation, post-test-spec-sync, post-docs-update. -**Verdict (cycle-8 delta)**: **PASS_WITH_WARNINGS** — 1 Medium (F-AZ809-1) + 2 new Lows (F-AZ810-1, F-AZ810-2) + 2 cycle-7 Low carry-overs (F-AZ795-1, F-AZ795-2) + 1 cycle-7 dependency Low carry-over (D-AZ795-1) + 1 cycle-4 dependency Medium carry-over (D2-cy4). Zero Critical / High. -**Verdict (cumulative)**: **PASS_WITH_WARNINGS** — 1 cycle-4 Medium (D2-cy4, test-runtime only) + 1 cycle-8 Medium (F-AZ809-1, auth-gated DoS) + multiple Lows. +**Verdict (cycle-8 delta)**: **PASS_WITH_WARNINGS** — 0 Medium *open* (F-AZ809-1 **RESOLVED in cycle 8** via the Step-14 follow-up — `MaxPolygons = 50` cap added to `CreateRouteRequestValidator` + matching unit + integration tests) + 2 new Lows open (F-AZ810-1, F-AZ810-2) + 2 cycle-7 Low carry-overs (F-AZ795-1, F-AZ795-2) + 1 cycle-7 dependency Low carry-over (D-AZ795-1) + 1 cycle-4 dependency Medium carry-over (D2-cy4, test-runtime only). Zero Critical / High. +**Verdict (cumulative)**: **PASS_WITH_WARNINGS** — 1 cycle-4 Medium open (D2-cy4, test-runtime only) + 0 cycle-8 Medium open + multiple Lows. ## Summary -| Severity | Cycle 7 delta | Cycle 8 delta | Cumulative | -|----------|---------------|---------------|------------| -| Critical | 0 | 0 | 0 | -| High | 0 | 0 | 0 | -| Medium | 0 | **1 NEW** (F-AZ809-1 — unbounded `geofences.polygons` enables an authenticated DoS on `POST /api/satellite/route`) | 2 (F-AZ809-1 cycle-8 + D2-cy4 cycle-4 carry — `Microsoft.NET.Test.Sdk 17.8.0` transitive `NuGet.Frameworks`; test-runtime exposure only) | -| Low | 3 (F-AZ795-1, F-AZ795-2, D-AZ795-1) | **2 NEW** (F-AZ810-1 — `JsonException.Message` echo in `UavUploadValidationFilter`; F-AZ810-2 — `DateTime` vs `DateTimeOffset` in `UavTileMetadata.CapturedAt`) | 5+ (3 cycle-7 carry + 2 cycle-8 new) | +| Severity | Cycle 7 delta | Cycle 8 delta (audit-time) | Cycle 8 delta (post-follow-up) | Cumulative open | +|----------|---------------|----------------------------|--------------------------------|-----------------| +| Critical | 0 | 0 | 0 | 0 | +| High | 0 | 0 | 0 | 0 | +| Medium | 0 | 1 (F-AZ809-1) | **0 — F-AZ809-1 RESOLVED in cycle 8** | 1 (D2-cy4 cycle-4 carry — `Microsoft.NET.Test.Sdk 17.8.0` transitive `NuGet.Frameworks`; test-runtime exposure only) | +| Low | 3 (F-AZ795-1, F-AZ795-2, D-AZ795-1) | 2 new (F-AZ810-1, F-AZ810-2) | 2 new (unchanged — both filed for cycle 9) | 5+ (3 cycle-7 carry + 2 cycle-8 new) | ## OWASP Top 10:2021 Assessment @@ -22,7 +22,7 @@ | A01 — Broken Access Control | PASS | — | | A02 — Cryptographic Failures | N/A | No crypto in cycle 8 | | A03 — Injection | PASS | — (cycle 8 strengthens — strict deserialization now backed by per-endpoint range checks at all four newly-validated endpoints) | -| A04 — Insecure Design | PASS_WITH_WARNINGS | F-AZ809-1 (Medium — unbounded `geofences.polygons` cap absence) | +| A04 — Insecure Design | **PASS (post-follow-up)** — was PASS_WITH_WARNINGS at audit time | F-AZ809-1 **RESOLVED in cycle 8** via Step-14 follow-up: `MaxPolygons = 50` cap added to `CreateRouteRequestValidator` + new Inv-10 in `route-creation.md` v1.0.1 + matching unit + integration tests | | A05 — Security Misconfiguration | PASS | (Informational note re: global Kestrel 500 MiB body limit; not a finding) | | A06 — Vulnerable Components | PASS_WITH_WARNINGS | D-AZ795-1 (Low — cycle-7 carry; FluentValidation 12.0.0 → 12.1.1 hardening still available) | | A07 — Auth Failures | PASS | — (JWT unchanged; every cycle-8 endpoint retains `RequireAuthorization()`) | @@ -34,7 +34,7 @@ | # | Severity | Category | Location | Title | |---|----------|----------|----------|-------| -| F-AZ809-1 | **Medium** | Insecure Design (A04) | `SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs:72-82` | Unbounded `geofences.polygons` collection enables an authenticated DoS on `POST /api/satellite/route` | +| F-AZ809-1 | Medium (**RESOLVED in cycle 8**) | Insecure Design (A04) | `SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs:72-82` (pre-fix) | Unbounded `geofences.polygons` collection enables an authenticated DoS on `POST /api/satellite/route` — fixed via `MaxPolygons = 50` cap | | F-AZ810-1 | Low | Information Disclosure (A09) | `SatelliteProvider.Api/Validators/UavUploadValidationFilter.cs:75-84` | `JsonException.Message` propagated to client in new code path (parallel to cycle-7 F-AZ795-1) | | F-AZ810-2 | Low | Time-handling correctness (A09 adjacent) | `SatelliteProvider.Common/DTO/UavTileMetadata.cs:30` + `SatelliteProvider.Api/Validators/UavTileMetadataValidator.cs:52-60` | `UavTileMetadata.CapturedAt` typed `DateTime` not `DateTimeOffset` — freshness window drifts by host TZ in non-UTC dev environments | | F-AZ795-1 | Low | Information Disclosure (A09) | `SatelliteProvider.Api/GlobalExceptionHandler.cs:108-117` | (cycle-7 carry) `JsonException.Message` propagated to client in 400 response — still open | @@ -44,13 +44,13 @@ ### Finding Details -**F-AZ809-1: Unbounded `geofences.polygons` collection enables an authenticated DoS** (Medium / A04 — Insecure Design) +**F-AZ809-1: Unbounded `geofences.polygons` collection enables an authenticated DoS** (Medium / A04 — Insecure Design) — **RESOLVED in cycle 8 (Step-14 follow-up commit, see git log)** -- Location: `SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs:72-82` -- Description: The cycle-8 `CreateRouteRequestValidator` chains `RuleForEach(req => req.Geofences!.Polygons).SetValidator(new GeofencePolygonValidator())` but enforces only `NotEmpty` on the collection — no upper bound on `Geofences.Polygons.Count`. The sibling `Points` collection IS capped at 500; the global `KestrelServerOptions.Limits.MaxRequestBodySize` is set to 500 MiB to accommodate the UAV upload endpoint and applies to every route. With ~90 bytes per minimum-shape polygon JSON, an authenticated caller can submit ~5.8 million polygons in a single request; each invalid polygon yields ~3 `ValidationFailure` allocations, for ~17 million `ValidationFailure` objects in worst case — sufficient to saturate the LOH and trigger a full GC pass. -- Impact: Medium. Auth-gated (`RequireAuthorization()` at `Program.cs:267`) — only tenant operators with a valid JWT can reach the endpoint. Within the cycle-8 threat model this is contained; promotion risk if the route endpoint is later exposed to an untrusted-tenant audience. -- Remediation: Add `Must(p => p is null || p.Count <= MaxPolygons)` with a defensible upper bound. Reasonable cap candidates: `50` (consistent with the historical use case — geofence rectangles for AOI restriction); `500` (consistent with the sibling `Points` cap on the same DTO). The pattern across the API is "cap every collection field" — one collection slipped past in cycle 8. -- Status: filed for cycle 9 as the **highest-priority** follow-up under AZ-809 (or a sibling child ticket). +- Location: `SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs:72-92` (post-fix; was lines 72-82 pre-fix). +- Description: The cycle-8 `CreateRouteRequestValidator` chained `RuleForEach(req => req.Geofences!.Polygons).SetValidator(new GeofencePolygonValidator())` but originally enforced only `NotEmpty` on the collection — no upper bound on `Geofences.Polygons.Count`. The sibling `Points` collection IS capped at 500; the global `KestrelServerOptions.Limits.MaxRequestBodySize` is set to 500 MiB to accommodate the UAV upload endpoint and applies to every route. With ~90 bytes per minimum-shape polygon JSON, an authenticated caller could have submitted ~5.8 million polygons in a single request; each invalid polygon yields ~3 `ValidationFailure` allocations, for ~17 million `ValidationFailure` objects in worst case — sufficient to saturate the LOH and trigger a full GC pass. +- Impact: Medium. Auth-gated (`RequireAuthorization()` at `Program.cs:267`) — only tenant operators with a valid JWT could reach the endpoint. Within the cycle-8 threat model this was contained; promotion risk eliminated by the cap. +- **Resolution** (Step-14 follow-up): `MaxPolygons = 50` constant added to `CreateRouteRequestValidator.cs`; chained as `.Must(polygons => polygons is null || polygons.Count <= MaxPolygons).WithMessage("…must contain at most 50 polygons.")` on the `geofences.polygons` rule. Cap chosen at 50 because geofences are AOI-restriction rectangles (per `route-creation.md` v1.0.1 Inv-10) — realistic use is 1-10 polygons per route, 50 gives 5x headroom while bounding the validator's worst-case allocation to ~150 `ValidationFailure` objects (well within normal request-handling overhead). Tests added: `CreateRouteRequestValidatorTests.Validate_GeofencePolygonsTooMany_FailsCountRule` (unit) + `CreateRouteValidationTests.GeofencePolygonsTooMany_Returns400` (integration, asserts 51-polygon array → HTTP 400 with `errors["geofences.polygons"]`). Contract bumped to `route-creation.md` v1.0.1 (patch — tightens an existing range; new Inv-10 + test case). +- Status: **resolved**. No cycle-9 follow-up required. **F-AZ810-1: `JsonException.Message` propagated to client in `UavUploadValidationFilter`** (Low / A09 — Information Disclosure) @@ -90,7 +90,7 @@ None. ### Short-term (Medium) -1. **Add `MaxPolygons` cap to `CreateRouteRequestValidator`** (F-AZ809-1) — single `.Must(...)` chain on `geofences.polygons`. Recommended `MaxPolygons = 50` (use-case-driven) or `500` (sibling-collection-consistent — matches `Points` cap on the same DTO). One-line code change + matching unit test. **Highest-priority cycle-9 follow-up**. +1. ~~**Add `MaxPolygons` cap to `CreateRouteRequestValidator`** (F-AZ809-1)~~ — **DONE in Step-14 follow-up (cycle 8)**. Cap set at 50; see Finding Details § F-AZ809-1 § Resolution. ### Long-term (Low / Hardening) @@ -118,8 +118,8 @@ The audit specifically wants to record four improvements introduced this cycle: ## Verdict -**PASS_WITH_WARNINGS** — 1 Medium (F-AZ809-1, auth-gated DoS on the route endpoint via unbounded `geofences.polygons`) + 2 cycle-8 Lows + 3 cycle-7 Low carry-overs + 1 cycle-4 Medium carry-over (test-runtime only). Zero Critical / High. +**PASS_WITH_WARNINGS** (post-follow-up) — 0 Medium open (F-AZ809-1 resolved in Step-14 follow-up) + 2 cycle-8 Lows + 3 cycle-7 Low carry-overs + 1 cycle-4 Medium carry-over (test-runtime only). Zero Critical / High. -Per the skill's verdict-logic, Medium severity yields PASS_WITH_WARNINGS. Cycle 8 is **safe to release** within its documented threat model (authenticated callers only on every endpoint). F-AZ809-1 must be the highest-priority cycle-9 follow-up because the missing collection cap is the only inconsistency across the API's otherwise-uniform "cap every collection field" pattern, and the global Kestrel body limit means the validator is the sole gate against an unbounded submission today. +Per the skill's verdict-logic, Medium severity yields PASS_WITH_WARNINGS — but cycle 8 fixed its only Medium in-cycle, leaving only the cycle-4 test-runtime carry-over Medium open. Cycle 8 is **safe to release** within its documented threat model (authenticated callers only on every endpoint). The post-follow-up posture is *cleaner* than the cycle-7 baseline because cycle 8 added zero new open Mediums and resolved its in-cycle finding before commit. -Cumulative posture: PASS_WITH_WARNINGS (1 cycle-4 Medium carry-over + 1 cycle-8 Medium + multiple Lows). No regression of the cycle-7 PASS_WITH_WARNINGS posture; cycle 8 added one new Medium but completed an architecturally important hardening epic. +Cumulative posture: PASS_WITH_WARNINGS (1 cycle-4 Medium open carry-over + 0 cycle-8 Medium open + multiple Lows). Cycle 8 completed an architecturally important hardening epic (100% input-validation coverage) without leaving Medium or higher debt. diff --git a/_docs/05_security/static_analysis_cycle8.md b/_docs/05_security/static_analysis_cycle8.md index d3c1d5a..b60c814 100644 --- a/_docs/05_security/static_analysis_cycle8.md +++ b/_docs/05_security/static_analysis_cycle8.md @@ -32,7 +32,7 @@ ## Findings -### F-AZ809-1 — Unbounded `geofences.polygons` collection enables an authenticated DoS via `CreateRouteRequest` (Medium / A04 — Insecure Design) +### F-AZ809-1 — Unbounded `geofences.polygons` collection enables an authenticated DoS via `CreateRouteRequest` (Medium / A04 — Insecure Design) — **RESOLVED in cycle 8 (Step-14 follow-up)** - **Location**: `SatelliteProvider.Api/Validators/CreateRouteRequestValidator.cs:72-82` (`When(req => req.Geofences is not null, () => …)`). - **Description**: The `CreateRouteRequestValidator` chains a `RuleForEach(req => req.Geofences!.Polygons).SetValidator(new GeofencePolygonValidator())` block but only enforces `NotEmpty` on the collection. There is **no upper bound on `Geofences.Polygons.Count`**. The parent collection `Points` IS capped (`MaxPoints = 500` at line 27) — the polygons collection is the only nested list-bearing field on this endpoint without a cap. @@ -61,7 +61,7 @@ - `MaxPolygons = 50` (consistent with the historical use case — a route is unlikely to need more than a handful of geofence rectangles for AOI restriction). - `MaxPolygons = MaxPoints = 500` (consistent with the sibling `Points` cap on the same DTO). - The matching `cumulative_review_batches_01-04_cycle8_report.md` already enumerates `points.Count <= 500` (route), `items.Count <= 100` (UAV upload), `coords.Count <= 1000` (tile inventory, cycle 7) as bounded — the missing entry for `geofences.polygons` is the one inconsistency. -- **Status**: open — file as a cycle-9 follow-up under AZ-809 (or a sibling child ticket). Not release-blocking for cycle 8 itself: exploitation requires an authenticated caller with a valid GPS-permission-less JWT — the same threat model already had access to the cycle-7-pre-existing `inventory.Tiles.Count` cap, so the marginal new exposure is moderate, not catastrophic. But it MUST be fixed before any untrusted-tenant exposure is added to the route endpoint. +- **Status**: **resolved in cycle 8 (Step-14 follow-up)**. Cap added: `MaxPolygons = 50` constant + `.Must(polygons => polygons is null || polygons.Count <= MaxPolygons).WithMessage("…must contain at most 50 polygons.")` on the `geofences.polygons` rule. Cap chosen at 50 because geofences are AOI-restriction rectangles — realistic use is 1-10 polygons per route, 50 gives 5x headroom while bounding the validator's worst-case allocation to ~150 `ValidationFailure` objects. Tests added: `CreateRouteRequestValidatorTests.Validate_GeofencePolygonsTooMany_FailsCountRule` (unit) + `CreateRouteValidationTests.GeofencePolygonsTooMany_Returns400` (integration). Contract bumped to `route-creation.md` v1.0.1 (patch — tightens an existing range; new Inv-10 + test case). See `security_report_cycle8.md` § F-AZ809-1 § Resolution for the full disposition. ### F-AZ810-1 — `JsonException.Message` propagated to client in `UavUploadValidationFilter` (Low / A09 — Information Disclosure) @@ -236,8 +236,6 @@ The handler's envelope checks are still reachable by any caller invoking `IUavTi ## Verdict (Phase 2) -**PASS_WITH_WARNINGS** — 1 Medium (F-AZ809-1) + 2 new Lows (F-AZ810-1, F-AZ810-2) + 2 cycle-7 Low carry-overs (F-AZ795-1, F-AZ795-2). No Critical or High findings. +**PASS_WITH_WARNINGS** (post-follow-up) — 0 Medium open (F-AZ809-1 resolved in cycle 8) + 2 new Lows (F-AZ810-1, F-AZ810-2) + 2 cycle-7 Low carry-overs (F-AZ795-1, F-AZ795-2). No Critical or High findings. -Per the skill's verdict-logic, Medium severity yields PASS_WITH_WARNINGS — not FAIL (FAIL is reserved for Critical or High). F-AZ809-1 is exploitable only by an authenticated tenant operator with a valid JWT (the route endpoint requires `RequireAuthorization()` without a permission scope, so any tenant with API access reaches it). The Medium is contained within the cycle-8 threat model — every cycle-8 endpoint is auth-gated — but should be the **highest-priority cycle-9 follow-up**: pre-existing-class Mediums tend to be the entry vector for higher-severity issues when adjacent threat-model assumptions shift (e.g. if a future feature exposes the route endpoint to an untrusted-tenant audience). - -The 2 new Lows + 2 carry-over Lows are not release-blockers in isolation; they are filed for the next cycle's follow-up batch. +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. The 2 new Lows + 2 carry-over Lows are not release-blockers in isolation; they are filed for the next cycle's follow-up batch.