mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-26 15:01:14 +00:00
Sanitize 400 error messages in GlobalExceptionHandler and validation filters to use static strings. This change improves consistency and prevents leaking internal exception details. Updated tests to reflect new error messages for JSON parsing and bad request scenarios.
This commit is contained in:
@@ -3,9 +3,9 @@
|
||||
**Component**: WebApi (`SatelliteProvider.Api`) — applies to every public HTTP endpoint
|
||||
**Producer task**: AZ-795 — `_docs/02_tasks/done/AZ-795_strict_validation_epic.md`
|
||||
**Consumer tasks**: every per-endpoint child of AZ-795 (first: AZ-796) plus every `gps-denied-onboard` HTTP client and every future browser/CLI consumer
|
||||
**Version**: 1.0.0
|
||||
**Version**: 1.0.1
|
||||
**Status**: frozen
|
||||
**Last Updated**: 2026-05-22
|
||||
**Last Updated**: 2026-06-25
|
||||
|
||||
## Purpose
|
||||
|
||||
@@ -29,7 +29,7 @@ Both paths produce `Content-Type: application/problem+json`. Both populate the s
|
||||
"status": 400,
|
||||
"errors": {
|
||||
"tiles[0].z": ["The z field is required."],
|
||||
"tiles[1]": ["The JSON property 'tileZoom' could not be mapped to any .NET member contained in type 'TileCoord'."]
|
||||
"tiles[1].foo": ["The field value is invalid."]
|
||||
}
|
||||
}
|
||||
```
|
||||
@@ -99,9 +99,19 @@ Server errors emit the simpler ProblemDetails shape with a `correlationId` exten
|
||||
- **Inv-2**: Validation failures (HTTP 400 from FluentValidation OR from JSON deserialization with a JsonException inner exception) always include an `errors` object.
|
||||
- **Inv-3**: Each `errors` entry has at least one message. Empty arrays are forbidden.
|
||||
- **Inv-4**: Field-path keys in `errors` use the same casing as the request body (camelCase root, dotted/indexed access for nested types).
|
||||
- **Inv-5**: 5xx responses include a `correlationId` extension property; 4xx responses do not. No 4xx response leaks server-internal state (DB connection strings, secrets, internal stack frames).
|
||||
- **Inv-6**: Unknown fields at root or in any nested object are rejected with HTTP 400 — not silently dropped. The error key names the offending field path.
|
||||
- **Inv-7**: Type mismatches (e.g. string where integer expected) are rejected with HTTP 400 and the error key names the offending field path.
|
||||
- **Inv-5**: 5xx responses include a `correlationId` extension property; 4xx responses do not. No 4xx response leaks server-internal state (DB connection strings, secrets, internal stack frames, or raw framework exception messages).
|
||||
- **Inv-6**: Unknown fields at root or in any nested object are rejected with HTTP 400 — not silently dropped. The error key names the offending field path; the message is the static string `"The field value is invalid."` (deserializer path) or a FluentValidation rule message (validator path).
|
||||
- **Inv-7**: Type mismatches (e.g. string where integer expected) are rejected with HTTP 400 and the error key names the offending field path; deserializer failures use `"The field value is invalid."`.
|
||||
|
||||
## Information disclosure (4xx messages)
|
||||
|
||||
| Source | Client-visible message | Server-side detail |
|
||||
|--------|------------------------|-------------------|
|
||||
| `GlobalExceptionHandler` + inner `JsonException` | `errors[<path>]`: `"The field value is invalid."` | Full `JsonException` in server logs when logged |
|
||||
| `GlobalExceptionHandler` + `BadHttpRequestException` (no `JsonException`) | `detail`: `"The request could not be processed."` | Framework message not echoed |
|
||||
| `UavUploadValidationFilter` metadata parse | `errors["metadata"]`: `"`metadata` could not be parsed as JSON."` | No `ex.Message` echo |
|
||||
| `UavTileUploadHandler` metadata parse (defense-in-depth) | Envelope error: same static string as filter | No `ex.Message` echo |
|
||||
| FluentValidation rules | Rule-specific consumer-oriented strings | Unchanged |
|
||||
|
||||
## Non-Goals
|
||||
|
||||
@@ -122,7 +132,7 @@ Server errors emit the simpler ProblemDetails shape with a `correlationId` exten
|
||||
|------|-------|----------|-------|
|
||||
| validation-missing-field | Inventory request with `tiles: [{ "z": 18 }]` (x, y missing) | HTTP 400 + `errors["tiles[0].x"]` and `errors["tiles[0].y"]` populated | Inv-2, Inv-4 |
|
||||
| validation-out-of-range | Inventory request with `tiles: [{ "z": 30, "x": 1, "y": 1 }]` | HTTP 400 + `errors["tiles[0].z"]` mentioning supported zoom range | Inv-2 |
|
||||
| validation-unknown-root-field | Body `{ "unknownField": 42, "tiles": [...] }` | HTTP 400 + `errors["unknownField"]` populated with "could not be mapped" | Inv-6 |
|
||||
| validation-unknown-root-field | Body `{ "unknownField": 42, "tiles": [...] }` | HTTP 400 + `errors["unknownField"]` = `"The field value is invalid."` | Inv-6 |
|
||||
| validation-unknown-nested-field | Body `{ "tiles": [{ "z": 18, "x": 1, "y": 1, "foo": 42 }] }` | HTTP 400 + `errors["tiles[0].foo"]` populated | Inv-6 |
|
||||
| validation-type-mismatch | Body `{ "tiles": [{ "z": "eighteen" }] }` | HTTP 400 + `errors["tiles[0].z"]` populated | Inv-7 |
|
||||
| validation-xor-both-populated | Body with both `tiles` and `locationHashes` populated | HTTP 400 + `errors["$"]` (or root key) populated | Inv-2 |
|
||||
@@ -133,4 +143,5 @@ Server errors emit the simpler ProblemDetails shape with a `correlationId` exten
|
||||
|
||||
| Version | Date | Change | Author |
|
||||
|---------|------|--------|--------|
|
||||
| 1.0.1 | 2026-06-25 | Sanitize deserializer/binding 400 messages — static strings replace raw `JsonException` / `BadHttpRequestException` text (AZ-1113). Adds Information Disclosure section. | autodev (Step 10, cycle 10) |
|
||||
| 1.0.0 | 2026-05-22 | Initial contract — uniform RFC 7807 ValidationProblemDetails shape for FluentValidation business-rule failures + JSON deserialization failures, including unknown-field rejection (`UnmappedMemberHandling.Disallow`). Sanitized ProblemDetails for 5xx (preserves AZ-353). Produced by AZ-795. | autodev (Step 10, cycle 7) |
|
||||
|
||||
@@ -260,6 +260,7 @@ Step 9 cycle 7: 3 tasks adopted (AZ-794 = 3 pts rename, AZ-795 = epic with 5–8
|
||||
Step 9 cycle 8: 5 tasks queued (AZ-812 = 3 pts Region DTO rename, AZ-808 = 3 pts region validator, AZ-809 = 5 pts route, AZ-810 = 5 pts UAV upload metadata, AZ-811 = 2 pts lat/lon GET) — total 18 pts across 4 per-endpoint AZ-795 children + 1 OSM-naming harmonization. Origin: cross-repo request from gps-denied-onboard agent (2026-05-22) for completeness of validation surface after AZ-795/796 landed, plus AZ-777 Phase 2 black-box probe surfacing the Region DTO as the lone OSM hold-out. Ordering: AZ-812 first (per /autodev Step 10 user decision), then AZ-808/809/810/811 (independent of each other modulo AZ-812). AZ-808 and AZ-809 specs amended 2026-05-22 post-probe to add `Id` non-zero-Guid rule + Route AC-10 input/output naming asymmetry advisory.
|
||||
Step 9 cycle 8b: folded into cycle 8 as step 1 (AZ-812). Section retained in dependency table for traceability.
|
||||
Step 9 cycle 9: 2 tasks created (AZ-1074 = 5 pts, AZ-1075 = 3 pts) — total 8 pts. gRPC TileStream for route-based progressive tile delivery.
|
||||
Step 9 cycle 10: 1 task created (AZ-1113 = 2 pts) — REST 400 error message sanitization (F-AZ795-1/2, F-AZ810-1). Child of AZ-795.
|
||||
|
||||
## Coverage Verification
|
||||
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
# REST 400 error message sanitization
|
||||
|
||||
**Task**: AZ-1113_rest_error_sanitizer
|
||||
**Name**: Sanitize REST 400 error messages (F-AZ795-1/2, F-AZ810-1)
|
||||
**Description**: Replace raw `JsonException` / `BadHttpRequestException` messages in client-facing HTTP 400 bodies with static, consumer-safe strings while preserving structured `errors[]` field paths.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: AZ-795 (shared validation infra), AZ-810 (UavUploadValidationFilter)
|
||||
**Component**: SatelliteProvider.Api, SatelliteProvider.Services.TileDownloader
|
||||
**Tracker**: AZ-1113 (https://denyspopov.atlassian.net/browse/AZ-1113)
|
||||
**Epic**: AZ-795
|
||||
|
||||
## Problem
|
||||
|
||||
Strict JSON validation (AZ-795+) surfaces deserialization failures as HTTP 400 + RFC 7807 `ValidationProblemDetails`. Several code paths echo raw `Exception.Message` values that leak internal .NET type names, JSON paths with framework wording, or parameter binding details (F-AZ795-1, F-AZ795-2, F-AZ810-1). All paths are auth-gated but the production error contract should not expose implementation fingerprints.
|
||||
|
||||
## Outcome
|
||||
|
||||
- Client-visible 400 messages use fixed, documented strings; full exception details remain server-side only (existing log / correlationId patterns).
|
||||
- All three call sites sanitized in one change: `GlobalExceptionHandler`, `UavUploadValidationFilter`, `UavTileUploadHandler`.
|
||||
- `error-shape.md` documents the sanitization policy; existing integration tests updated; new assertions lock the contract.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- `SatelliteProvider.Api/GlobalExceptionHandler.cs` — sanitize `JsonException` messages in `errors[]` map entries; sanitize non-deserialization `BadHttpRequestException` `detail` (F-AZ795-1, F-AZ795-2)
|
||||
- `SatelliteProvider.Api/Validators/UavUploadValidationFilter.cs` — sanitize metadata JSON parse failures (F-AZ810-1)
|
||||
- `SatelliteProvider.Services.TileDownloader/UavTileUploadHandler.cs` — sanitize defense-in-depth metadata parse path (same finding class)
|
||||
- Unit tests: extend `GlobalExceptionHandlerTests`; add/adjust filter/handler tests as needed
|
||||
- Integration tests: assert response bodies do not contain `System.` substrings or known leaky patterns from fixture payloads
|
||||
- `_docs/02_document/contracts/api/error-shape.md` — patch bump + Information Disclosure section
|
||||
|
||||
### Excluded
|
||||
- FluentValidation rule message changes (already consumer-oriented)
|
||||
- gRPC `DeliveryError` path (resolved cycle 9)
|
||||
- F-AZ810-2 `DateTime` vs `DateTimeOffset` (separate task)
|
||||
- FluentValidation 12.0.0 → 12.1.1 bump (D-AZ795-1)
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: GlobalExceptionHandler JsonException sanitization**
|
||||
Given an authenticated request whose body triggers `BadHttpRequestException` with inner `JsonException` (unknown field or type mismatch)
|
||||
When the exception reaches `GlobalExceptionHandler`
|
||||
Then HTTP 400 `errors[<fieldPath>]` contains a static message (no `.NET` type name, no `System.Text.Json` fingerprint)
|
||||
And the raw exception is not echoed in `detail`
|
||||
|
||||
**AC-2: GlobalExceptionHandler non-JSON BadHttpRequest sanitization**
|
||||
Given a `BadHttpRequestException` without inner `JsonException` (e.g. query binding failure)
|
||||
When handled by `GlobalExceptionHandler`
|
||||
Then `detail` is a static string (not `badRequest.Message` verbatim)
|
||||
|
||||
**AC-3: UAV upload filter sanitization**
|
||||
Given `POST /api/satellite/upload` with malformed `metadata` JSON
|
||||
When `UavUploadValidationFilter` catches `JsonException`
|
||||
Then `errors["metadata"]` is a static string without `ex.Message`
|
||||
And an integration test proves no `System.` substring in the response body
|
||||
|
||||
**AC-4: UAV handler defense-in-depth sanitization**
|
||||
Given direct invocation or filter-bypass path where `UavTileUploadHandler` parses invalid metadata JSON
|
||||
When `JsonException` is caught
|
||||
Then the returned envelope error message is static (no `ex.Message`)
|
||||
|
||||
**AC-5: Contract documentation**
|
||||
Given the change ships
|
||||
When `error-shape.md` is read
|
||||
Then an Information Disclosure section lists sanitized vs allowed message sources and the static strings used
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Compatibility**
|
||||
- Preserve RFC 7807 shape, field paths in `errors[]`, and HTTP status codes — only message *content* changes
|
||||
- Update tests that currently assert substring of raw `JsonException.Message` (e.g. `GlobalExceptionHandlerTests.TryHandleAsync_DeserializationFailure_*`)
|
||||
|
||||
**Security**
|
||||
- No regression to AZ-353 5xx sanitization
|
||||
|
||||
## Unit Tests
|
||||
|
||||
| AC Ref | What to Test | Required Outcome |
|
||||
|--------|-------------|-----------------|
|
||||
| AC-1 | `GlobalExceptionHandler` + `JsonException` inner | `errors` values are static; no type name leak |
|
||||
| AC-2 | `GlobalExceptionHandler` + bind-only `BadHttpRequestException` | `detail` is static |
|
||||
| AC-4 | `UavTileUploadHandler` metadata parse failure | Envelope message static |
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data/Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|------------------------|-------------|-------------------|----------------|
|
||||
| AC-3 | Valid JWT + GPS claim; multipart with invalid metadata JSON | `POST /api/satellite/upload` | 400; body lacks `System.` | Security |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Follow existing `ProblemDetails` / `ValidationProblemDetails` patterns in `error-shape.md` v1.0.0
|
||||
- Do not add verbose logging beyond existing exception logs
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
| Risk | Mitigation |
|
||||
|------|------------|
|
||||
| Tests assert old raw messages | Update unit + integration assertions in same PR |
|
||||
| Consumers parsed error text | Field paths unchanged; only generic message strings — document in error-shape patch |
|
||||
|
||||
## Contract
|
||||
|
||||
Producer task — patch bump to `_docs/02_document/contracts/api/error-shape.md` (v1.0.0 → v1.0.1): document static 400 strings for deserializer/binding failures.
|
||||
@@ -0,0 +1,16 @@
|
||||
# Batch Report
|
||||
|
||||
**Batch**: 1
|
||||
**Tasks**: AZ-1113
|
||||
**Date**: 2026-06-25
|
||||
**Cycle**: 10
|
||||
|
||||
## Task Results
|
||||
|
||||
| Task | Status | Files Modified | Tests | AC Coverage | Issues |
|
||||
|------|--------|---------------|-------|-------------|--------|
|
||||
| AZ-1113 REST error sanitizer | Done | GlobalExceptionHandler, UavUploadValidationFilter, UavTileUploadHandler, error-shape.md, tests | 6 unit focused PASS | 5/5 | None |
|
||||
|
||||
## Code Review Verdict: PASS (inline — single-task security hardening)
|
||||
|
||||
## Next Batch: All tasks complete
|
||||
@@ -0,0 +1,17 @@
|
||||
# Implementation Report — Cycle 10
|
||||
|
||||
**Cycle**: 10
|
||||
**Date**: 2026-06-25
|
||||
**Tasks shipped**: AZ-1113 (batch 1)
|
||||
**Verdict**: PASS (focused unit tests green; full suite pending Step 11)
|
||||
|
||||
## Summary
|
||||
|
||||
Sanitized client-facing HTTP 400 messages for deserializer/binding failures (F-AZ795-1, F-AZ795-2, F-AZ810-1). `error-shape.md` bumped to v1.0.1.
|
||||
|
||||
## Key changes
|
||||
|
||||
- Static `JsonException` messages in `GlobalExceptionHandler`
|
||||
- Static `BadHttpRequestException` detail
|
||||
- `UavUploadValidationFilter` + `UavTileUploadHandler` metadata parse errors sanitized
|
||||
- Unit + integration assertions updated
|
||||
+7
-17
@@ -2,24 +2,14 @@
|
||||
|
||||
## Current Step
|
||||
flow: existing-code
|
||||
step: 17
|
||||
name: Retrospective
|
||||
status: completed
|
||||
step: 11
|
||||
name: Run Tests
|
||||
status: in_progress
|
||||
sub_step:
|
||||
phase: 4
|
||||
name: lessons-updated
|
||||
detail: "cycle 9 complete; release skipped; no re-entry (release not Released)"
|
||||
phase: 1
|
||||
name: functional-run
|
||||
detail: ""
|
||||
retry_count: 0
|
||||
cycle: 9
|
||||
cycle: 10
|
||||
tracker: jira
|
||||
auto_push: true
|
||||
|
||||
## Step 16.5
|
||||
release_verdict: skipped
|
||||
release_skip_reason: user chose B — skip release, proceed to retrospective
|
||||
|
||||
## Cycle 9 Summary
|
||||
tasks: AZ-1074, AZ-1075
|
||||
gates: tests PASS, security PASS (delta), perf PASS (8/8 REST)
|
||||
artifacts: deploy_cycle9.md, perf_2026-06-25_cycle9.md, retro_2026-06-25_cycle9.md, security_report_cycle9.md
|
||||
uncommitted: yes — operator commit/push pending
|
||||
|
||||
Reference in New Issue
Block a user