[AZ-809] F-AZ809-1: cap geofences.polygons at 50 (security audit)

Closes the cycle-8 Medium DoS finding. Without the cap, an
authenticated caller could submit millions of bbox polygons in a
single 500 MiB request (Kestrel global limit) and saturate the
FluentValidation allocator on the validator hot path; each polygon
is ~90 bytes of JSON, so the body limit is not a useful gate.

Realistic use is 1-10 polygons per route — 50 leaves 5x headroom
while bounding the worst-case allocation.

Layers:
- CreateRouteRequestValidator: MaxPolygons = 50 + Must(...) chained
  before RuleForEach so the count error fires at "geofences.polygons"
  (not the leaf path).
- Unit: Validate_GeofencePolygonsTooMany_FailsCountRule.
- Integration: GeofencePolygonsTooMany_Returns400 (51 valid bbox
  polygons -> HTTP 400 + errors["geofences.polygons"]).
- Contract: route-creation.md -> v1.0.1 patch (tightening an
  existing range). New Inv-10, new geofence-polygons-too-many
  test case, changelog row.
- Test spec: BT-29 sub-case 9b + AZ-809 AC-1b row in the
  traceability matrix.
- Security report: F-AZ809-1 marked RESOLVED in cycle 8; verdict
  remains PASS_WITH_WARNINGS (Lows + carry-overs unchanged).

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-23 15:29:10 +03:00
parent ac40a8b352
commit 8fca6e0209
9 changed files with 120 additions and 34 deletions
@@ -27,6 +27,15 @@ public sealed class CreateRouteRequestValidator : AbstractValidator<CreateRouteR
private const int MaxPoints = 500;
private const int MaxNameLength = 200;
private const int MaxDescriptionLength = 1000;
// Geofences are axis-aligned bbox rectangles used for AOI restriction
// during route planning (see route-creation.md). Realistic use is 1-10
// polygons per route; cap at 50 to give 5x headroom while bounding the
// validator's worst-case allocation. The global Kestrel body limit
// (500 MiB, sized for the UAV upload endpoint) is not a useful gate
// here because polygon JSON is small (~90 bytes per minimum-shape
// polygon); without this cap a single authenticated request could
// submit millions of polygons and saturate the LOH.
private const int MaxPolygons = 50;
public CreateRouteRequestValidator()
{
@@ -74,6 +83,8 @@ public sealed class CreateRouteRequestValidator : AbstractValidator<CreateRouteR
RuleFor(req => 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)
@@ -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();
@@ -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()
{
@@ -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) |
+2 -1
View File
@@ -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` |
@@ -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 | ✓ |
+4 -4
View File
@@ -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<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.
- **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.
+20 -20
View File
@@ -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.
+4 -6
View File
@@ -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.