Files
Oleksandr Bezdieniezhnykh bc04ba7f99 [AZ-794] [AZ-795] [AZ-796] Cycle 7 Steps 12-15 sync (test-spec / docs / security / perf)
Step 12 (Test-Spec Sync): adds BT-27 for the AZ-796 9-rule
validation surface and 12 cycle-7 AC rows + Coverage Summary
update to traceability-matrix.md.

Step 13 (Update Docs): module-layout + module docs for the new
SatelliteProvider.Api/Validators namespace + GlobalExceptionHandler
+ updated TileInventory DTO; tests_unit + tests_integration
document the new InventoryRequestValidatorTests (16 unit tests
covering all 9 rules) + TileInventoryValidationTests (16
integration tests) + ProblemDetailsAssertions support;
glossary entries for Validation Problem Details / FluentValidation
/ Unmapped Member Handling; system-flows F8 (Tile Inventory Bulk
Lookup) expanded with deserializer + validator gates and a 13-row
Validation Surface table; data_parameters § Tile Inventory
documents the v2 input schema + constraints; ripple_log_cycle7
captures the doc-side ripple decisions.

Step 14 (Security Audit): 5-phase audit ran; verdict
PASS_WITH_WARNINGS (3 Low findings — D-AZ795-1 FluentValidation
12.0.0 -> 12.1.1 recommended bump, F-AZ795-1 JsonException.Message
leak in 400 detail, F-AZ795-2 BadHttpRequestException.Message leak).
No Critical / High; auth runs before validation (confirmed in
Program.cs); two NuGet additions (FluentValidation 12.0.0 +
.DependencyInjectionExtensions 12.0.0) both CVE-clean. Per-phase
reports plus consolidated security_report_cycle7.md.

Step 15 (Performance Test): docker compose stack used for perf
run, scripts/run-performance-tests.sh exited 0 with 8/8 scenarios
PASS (second consecutive clean exit-0); added PT-09 cycle-7 smoke
probe (v2 z/x/y schema, 2500-tile all-miss batch) measuring
min=27ms median=44ms p95=73ms max=86ms (13.7x under AZ-505 AC-4
1000ms budget). PT-07/08 improvements traced to the cycle-6 TLS
handshake-overhead identification, not application-side change.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-22 11:24:27 +03:00

14 KiB
Raw Permalink Blame History

Static Analysis (Cycle 7)

Date: 2026-05-22 Mode: Delta scan Scope: Source code introduced or changed by AZ-794 + AZ-795 + AZ-796:

  • SatelliteProvider.Api/Program.cs (DI registration + middleware + endpoint wiring deltas)
  • SatelliteProvider.Api/Validators/{InventoryRequestValidator,ValidationEndpointFilter,ValidationEndpointFilterExtensions,GlobalValidatorConfig}.cs (new)
  • SatelliteProvider.Api/GlobalExceptionHandler.cs (new)
  • SatelliteProvider.Common/DTO/TileInventory.cs (renamed properties + [JsonRequired] markers)
  • SatelliteProvider.IntegrationTests/{ProblemDetailsAssertions,TileInventoryValidationTests}.cs (new — test code; reviewed for fixture-only secrets and auth bypass patterns)
  • SatelliteProvider.Tests/{TestSupport/ValidatorTestModuleInitializer,Validators/InventoryRequestValidatorTests}.cs (new — test code)
  • scripts/probe_inventory_validation.sh (new probe shell script — reviewed for embedded secrets and unsafe sequences)

Method: Read each new file end-to-end + targeted Grep for injection / hardcoded-credential / unsafe-API patterns. Cycle 5 + cycle 4 baselines for the pre-existing surface remain authoritative; this scan only audits the delta.

Findings

F-AZ795-1 — JsonException.Message propagated to client in 400 response (Low / Information Disclosure)

  • Location: SatelliteProvider.Api/GlobalExceptionHandler.cs:108117 (TryExtractDeserializationErrors)
  • Code:
    var message = string.IsNullOrEmpty(jsonEx.Message)
        ? "Invalid JSON."
        : jsonEx.Message;
    return new Dictionary<string, string[]> { [path] = new[] { message } };
    
  • Description: System.Text.Json.JsonException.Message is forwarded to the client as the value in the errors[path] array of a 400 ValidationProblemDetails. The default JsonException.Message typically includes the offending .NET type (System.Int32, System.Guid, …), the JSON path (already separately surfaced as the key), and the byte position / line number in the payload — for example:

    "The JSON value could not be converted to System.Int32. Path: $.tiles[0].z | LineNumber: 0 | BytePositionInLine: 27."

  • Impact: Low. The auth gate (UseAuthentication + UseAuthorization middleware) runs BEFORE the endpoint filter chain, so anonymous callers cannot reach the validator or the deserializer — they get 401 first. For an authenticated caller the type-name leak only reveals what the OpenAPI spec at /swagger/v1/swagger.json already advertises (the DTO names and shape). Parse positions and System.Text.Json fingerprinting are mild — not a credential leak, not an SSRF / IDOR pivot — but they do narrow the attack surface for an attacker who has already obtained a valid token (e.g. a curious tenant operator).
  • OWASP Mapping: A09 (Security Logging and Monitoring Failures — adjacent) / A05 (Security Misconfiguration — adjacent).
  • Remediation: Replace the raw jsonEx.Message with a generalised message such as "Could not deserialize value at this field path." (still keyed by the field path, so callers retain enough information to fix their request). The exact jsonEx.Message should be logged on the server side for support, indexed by correlationId, but not echoed in the response.
  • Test coverage gap: AZ-795 acceptance criteria did not assert anything about the response message string content beyond presence/non-emptiness. A future child task should add an assertion that no System.* type name appears in any errors[] value.
  • Status: open (filed for next cycle).

F-AZ795-2 — Generic BadHttpRequestException.Message propagated in non-JSON 400 path (Low / Information Disclosure)

  • Location: SatelliteProvider.Api/GlobalExceptionHandler.cs:8893 (the fallback non-ValidationProblemDetails path)
  • Code:
    var problem = new ProblemDetails
    {
        Status = badRequest.StatusCode,
        Title = "Bad Request",
        Detail = badRequest.Message,
    };
    
  • Description: When BadHttpRequestException has no JsonException inner exception (e.g. framework model-binding failures, unsupported Content-Type, oversized request bodies), the framework-provided Message is echoed back as Detail. ASP.NET Core message strings for these paths include hints like "Failed to read parameter '...' from query string." which can include the actual parameter name and (rarely) framework version hints.
  • Impact: Same severity as F-AZ795-1. Pre-existing-class issue (model-binding messages were always shaped this way under ASP.NET Core); cycle 7 didn't introduce it but didn't change it either.
  • Remediation: Treat the same as F-AZ795-1 — sanitise to a generic string + log the original Message with correlationId. Done in tandem.
  • Status: open (filed for next cycle).

F-AZ795-3 — correlationId for 5xx (Informational — no action required)

  • Location: SatelliteProvider.Api/GlobalExceptionHandler.cs:3853 (5xx branch)
  • Description: The 5xx branch sets Detail = "An unexpected error occurred. Use the correlationId to look up the server log entry." and adds correlationId = httpContext.TraceIdentifier to the extensions map. Original exception message NEVER goes into the body. Inv-5 of error-shape.md is honoured.
  • Status: informational only — no remediation needed. The implementation preserves AZ-353 sanitisation and adds the correlationId extension as the only non-secret identifier.

Pattern Sweep — Cycle-7 Delta

Injection (SQL / Command / XSS / Template)

Pattern Result
string.Format, interpolation $"...", or concatenation feeding into a Dapper / Npgsql command in the new files None. The new files do not touch the data layer.
Process.Start, subprocess, eval, Invoke-Expression, raw system() None.
User-input echoed into HTML (XSS) None. The API returns JSON only.
Template injection (Razor / Liquid / etc.) None. No templating in the new files.

Authentication & Authorization

Pattern Result
Hardcoded credentials, secrets, API keys Grep for `password
Missing .RequireAuthorization() on a public endpoint POST /api/satellite/tiles/inventory has .RequireAuthorization() chained at Program.cs:217; WithValidation<TileInventoryRequest>() is chained at line 218 (the chaining order on a single RouteHandlerBuilder does not affect runtime ordering — auth middleware runs at the routing layer BEFORE any endpoint filter executes). All other endpoints unchanged from prior cycles.
Validator running before auth check No. Endpoint filters run after the authorization middleware short-circuits. Anonymous callers cannot probe the schema or trigger the validator.
Permission/policy regression RequiresGpsPermission on /api/satellite/upload unchanged. No new policy added or removed in cycle 7.

Cryptographic Failures

Pattern Result
Weak hash (MD5 / SHA1) used for passwords or signatures None in cycle 7. (Pre-existing: Uuidv5.Create uses SHA-1 internally per RFC 9562 §5.5 for the UUIDv5 hash — same as cycle 5; covered there, not a finding.)
New crypto material introduced None. Cycle 7 has no cryptography.
Plaintext transmission API listens on https://+:8080 with ALPN (cycle-6 baseline, unchanged).

Data Exposure

Pattern Result
Sensitive data in logs GlobalExceptionHandler logs Method, Path, correlationId, and the exception object via Serilog. The exception object MAY contain DB query parameters or DTO field values; this is the same pre-cycle-7 risk surface (Serilog default), and cycle 7 doesn't widen it. The 4xx branch (which handles malformed payloads) does NOT log the exception at all — only the 5xx branch logs. So a malformed request body is not written to logs. ✓
Sensitive fields in API responses F-AZ795-1 / F-AZ795-2 above are the only echo-back paths. No password hashes, no PII (the inventory endpoint is metadata-only).
Debug endpoints in production Swagger is gated by app.Environment.IsDevelopment() (unchanged).
Secrets in version control .env* files are gitignored. The new shell probe scripts/probe_inventory_validation.sh reads $API_BASE/$JWT from the environment; no embedded secrets.

Insecure Deserialization

Pattern Result
Pickle / BinaryFormatter / unsafe XML / JsonConvert.DeserializeObject<T> with TypeNameHandling.All None in cycle 7. The deserializer is System.Text.Json with UnmappedMemberHandling.Disallow — a strict-mode deserializer that does NOT support polymorphic type names. Cycle 7 strengthens this surface (mass-assignment prevention) rather than weakening it.
Unbounded collection sizes TileInventoryRequest.Tiles / LocationHashes capped at TileInventoryLimits.MaxEntriesPerRequest = 5000 enforced by InventoryRequestValidator Rule 6/7. Pre-deserialization upper bound is governed by KestrelServerOptions.Limits.MaxRequestBodySize (set for the UAV upload to 500 MiB; default 30 MB for other endpoints — sufficient for a 5000-entry inventory body).

Integer Overflow / Bounded Math

The validator does a left-shift to compute 2^z for the X/Y range check:

.Must((coord, x) => coord.Z >= 0 && coord.Z <= MaxZoom && x < (1L << coord.Z))
  • (1L << coord.Z) uses a long literal, so the shift target is 64-bit.
  • coord.Z is guarded by >= 0 && <= MaxZoom (= 22). Maximum shift is 1L << 22 = 4_194_304. No overflow possible.
  • The guard is INSIDE the .Must lambda (not in a separate When); FluentValidation evaluates ALL rules unless explicitly chained with When / Cascade. The lambda returns false if Z is out-of-range, surfacing the X/Y rule failure alongside the Z rule failure rather than crashing. ✓

ReDoS / Algorithmic Complexity

Cycle 7 validation rules are all O(1) per entry × N entries:

Rule Per-entry cost Total cost (worst case N = 5000)
XOR (Custom rule on req) O(1) O(1)
.Tiles.Count <= 5000 O(1) O(1)
.LocationHashes.Count <= 5000 O(1) O(1)
RuleForEach(Tiles).SetValidator(TileCoordValidator) O(1) per entry O(N) — bounded by Rule 6 (Tiles.Count <= 5000)
Z, X, Y range checks per TileCoord O(1) per entry O(N) — bounded by Rule 6

No regex, no recursion, no nested loops. Maximum total work is ~25 000 operations at the validator level for a max-size payload. Cycle-6 perf test (PT-09) measured the entire endpoint at p95 = 66 ms for 2500-coord batches; adding the validator cost is negligible relative to the DB lookup.

Test Code Review

SatelliteProvider.Tests/Validators/InventoryRequestValidatorTests.cs

  • Pure CPU; no I/O, no network, no file system, no DB.
  • All inputs constructed inline. No fixture file reads.
  • No hardcoded JWT or test bearer token (the validator runs in isolation).
  • Calls GlobalValidatorConfig.ApplyOnce() via ValidatorTestModuleInitializer.cs ([ModuleInitializer]). This runs at test-assembly load — single source of truth for the camelCase property resolver; matches the runtime behaviour, no test drift risk.
  • ✓ No findings.

SatelliteProvider.IntegrationTests/TileInventoryValidationTests.cs + ProblemDetailsAssertions.cs

  • Uses the runner-side JwtTestHelpers.MintAuthenticated(...) to attach a Bearer token. No hardcoded secret material. The token's signing secret comes from the JWT_SECRET env var (32+ bytes, dev-only in docker-compose.tests.yml).
  • Test inputs use raw HttpRequestMessage with hand-built JSON strings — exercises the exact wire shape the validator and the deserializer see in production. The hand-built strings include all the cycle-7 negative cases (legacy tileZoom/tileX/tileY, unknown root field, type mismatch, etc.).
  • ProblemDetailsAssertions.AssertValidationProblem(...) asserts the shape of errors[] per error-shape.md Inv-2 / Inv-4. No assertion was added against message content — see F-AZ795-1 remediation: when the message is sanitised, add a "no System.* type name" assertion here.
  • ✓ No findings beyond the gap noted in F-AZ795-1.

scripts/probe_inventory_validation.sh

  • Reads ${API_BASE:-https://localhost:8080} and ${JWT:?…} from the environment.
  • set -o errexit -o pipefail -o nounset at the top of the script — fail-fast on undefined vars or broken pipes.
  • curl --insecure is used (justified — the dev cert is self-signed; the script targets localhost in dev/test only). Documented in the script header.
  • No embedded credentials.
  • ✓ No findings.

Cycle-7 Pre-existing-Surface Drift Check

Cycle 7 did not modify the data-access layer, file system layout, route processing, region processing, JWT auth setup, or the UAV upload pipeline. Cycle-5 + cycle-4 baseline findings (e.g., D2-cy4 Microsoft.NET.Test.Sdk carry-over) are unchanged. The cycle-6 dependency-scan-skip is the only audit-process gap; cycle 7 picks up the missing supply-chain delta inline (covered in dependency_scan_cycle7.md).

Verdict (Phase 2)

PASS_WITH_WARNINGS — 2 Low (F-AZ795-1, F-AZ795-2) information-disclosure findings on the new error-shaping path; both are auth-gated and reveal only type-name + parse-position metadata. No Critical / High / Medium findings. The Low findings are filed for the next cycle and are not release-blockers.