azaion/satellite-provider
1
0
Fork 0
mirror of https://github.com/azaion/satellite-provider.git synced 2026-06-21 10:21:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

[AZ-794] [AZ-795] [AZ-796] Cycle 7 Steps 12-15 sync (test-spec / docs / security / perf)

Browse Source 
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>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-22 11:24:27 +03:00
parent 865dfdb3b9
commit bc04ba7f99
17 changed files with 779 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
_docs/05_security/dependency_scan_cycle7.md
+73
View File
@@ -0,0 +1,73 @@
# Dependency Scan (Cycle 7)
**Date**: 2026-05-22
**Mode**: Delta scan
**Scope**: Cycle-7 delta over the cycle-5 dependency scan (`_docs/05_security/dependency_scan_cycle5.md`); cycle 6 did not produce a dependency scan, so the last scanned baseline is cycle 5
**Trigger**: AZ-794 (wire-format rename — no manifest changes) + AZ-795 (strict-validation epic — adds FluentValidation 12.0.0 + FluentValidation.DependencyInjectionExtensions 12.0.0) + AZ-796 (per-endpoint validator — no manifest changes beyond what AZ-795 added)
**Method**: Manifest diff + WebSearch CVE lookup against GitHub Security Advisories + NVD + ReversingLabs Spectra Assure. `dotnet list package --vulnerable` is intentionally not run (the AGENTS.md operational note in this workspace says it hangs the agent shell); the manifest diff + advisory lookup is the deterministic substitute.
## Cycle-7 Package Manifest Diff
| csproj | Cycle 5 baseline (post-AZ-503) | Cycle 7 change | Net effect on supply chain |
|--------|--------------------------------|----------------|----------------------------|
| `SatelliteProvider.Api/SatelliteProvider.Api.csproj` | references `Microsoft.AspNetCore.OpenApi 10.0.7`, `Microsoft.AspNetCore.Authentication.JwtBearer 10.0.7`, `Newtonsoft.Json 13.0.4`, `Serilog.AspNetCore 8.0.3`, `Serilog.Sinks.File 6.0.0`, `SixLabors.ImageSharp 3.1.11`, `Swashbuckle.AspNetCore 10.1.7` | **+2 PackageReferences**: `FluentValidation 12.0.0` and `FluentValidation.DependencyInjectionExtensions 12.0.0` (both new at AZ-795). | New supply-chain node. Both packages are MIT/Apache-2.0; no transitive Microsoft.* version bumps. |
| `SatelliteProvider.Common/SatelliteProvider.Common.csproj` | unchanged from cycle 5 | **+0 PackageReferences** — the cycle-7 DTO changes (`[JsonRequired]` on `TileCoord.Z/X/Y`) are BCL-only. | None. |
| `SatelliteProvider.DataAccess/SatelliteProvider.DataAccess.csproj` | unchanged from cycle 5 | **+0 PackageReferences**. | None. |
| `SatelliteProvider.Services.TileDownloader/SatelliteProvider.Services.TileDownloader.csproj` | unchanged from cycle 5 | **+0 PackageReferences**. | None. |
| `SatelliteProvider.Services.RegionProcessing/SatelliteProvider.Services.RegionProcessing.csproj` | unchanged from cycle 5 | **+0 PackageReferences**. | None. |
| `SatelliteProvider.Services.RouteManagement/SatelliteProvider.Services.RouteManagement.csproj` | unchanged from cycle 5 | **+0 PackageReferences**. | None. |
| `SatelliteProvider.Tests/SatelliteProvider.Tests.csproj` | unchanged from cycle 5 | **+0 PackageReferences** — `FluentValidation.TestHelper` is the namespace inside the main `FluentValidation` package consumed transitively via `ProjectReference` to `SatelliteProvider.Api`. | None at the manifest level; one new transitive runtime node at test execution (FluentValidation main assembly). |
| `SatelliteProvider.IntegrationTests/SatelliteProvider.IntegrationTests.csproj` | unchanged from cycle 5 | **+0 PackageReferences** — the new `ProblemDetailsAssertions.cs` + `TileInventoryValidationTests.cs` use only BCL + the existing `Xunit` + `Microsoft.AspNetCore` ProjectReference. | None. |
| `SatelliteProvider.TestSupport/SatelliteProvider.TestSupport.csproj` | unchanged from cycle 5 | **+0 PackageReferences**. | None. |
**Net cycle-7 dependency change**: two new `PackageReference` lines (FluentValidation 12.0.0 + FluentValidation.DependencyInjectionExtensions 12.0.0). All other csprojs are byte-identical at the manifest level (verified by `git diff cycle5_tip..HEAD -- '*.csproj'` in the implementation phase).
## Cycle-7 Dependency CVE Lookup
### FluentValidation 12.0.0
| Source | Result |
|--------|--------|
| GitHub Security Advisories (https://github.com/FluentValidation/FluentValidation/security/advisories) | No published advisories. |
| NVD CVE database (search: `FluentValidation`) | No CVEs against this .NET library. (One historical record matched on the substring "FluentForms" — a WordPress plugin unrelated to FluentValidation; explicitly excluded.) |
| ReversingLabs Spectra Assure Community (https://secure.software/nuget/packages/fluentvalidation/12.0.0) | "No known vulnerabilities detected" for the package. One "Hardening" note (`1 outdated toolchain detected`) — not a CVE. |
| Historical Regex DoS (Issue #120 — `EmailAddressValidator`) | Pre-2017, resolved in commit `ebe3720`. v12.0.0 ships with the fixed implementation. Cycle 7 does not use `EmailAddressValidator` (no `Matches`/`EmailAddress` rules — all rules are integer ranges and collection-count predicates). |
| Latest published version | 12.1.1 (5 months ago at time of audit). v12.0.0 → v12.1.1 is a hardening release (no security advisories between the two); the bump is recommended but not security-mandatory. |
### FluentValidation.DependencyInjectionExtensions 12.0.0
| Source | Result |
|--------|--------|
| GitHub Security Advisories | No published advisories. |
| NVD CVE database | No CVEs. |
| ReversingLabs Spectra Assure Community (https://secure.software/nuget/packages/fluentvalidation.dependencyinjectionextensions/vulnerabilities) | "No known vulnerabilities detected". |
| Latest published version | 12.1.1. Same posture as the main package. |
### Cycle-5 carry-overs unchanged
- **D2-cy4** (`Microsoft.NET.Test.Sdk 17.8.0` transitive `NuGet.Frameworks` Medium — test-runtime exposure only) — unchanged. AZ-795 did not bump `Microsoft.NET.Test.Sdk`; it remains the same package at the same version with the same exposure surface. Still owned by a follow-up task at the next Test SDK refresh cycle.
## Cycle-7 New Source Code Runtime Surface
The two new NuGet packages introduce the following runtime surface in the API process:
| Surface | Risk class | Notes |
|---------|------------|-------|
| `IValidator<T>` registration via `AddValidatorsFromAssemblyContaining<Program>()` | Reflection-based DI scan | Bounded to the API assembly only (`SatelliteProvider.Api.dll`). Cannot pick up validators from upstream test assemblies or runtime-loaded DLLs. |
| `ValidatorOptions.Global.PropertyNameResolver` (set by `GlobalValidatorConfig.ApplyOnce`) | Process-wide static state | Idempotent under a `lock` guard. Only affects how error-map keys are rendered. Cannot affect parsing or business logic. |
| `IValidator<T>.ValidateAsync(arg, CancellationToken)` invocation in `ValidationEndpointFilter<T>` | User-controlled DTO entering managed code | DTOs are already deserialized by System.Text.Json (with `UnmappedMemberHandling.Disallow`); the validator receives strongly-typed properties only — no string injection surface. Rules in cycle 7 are integer-only (no regex, no string contains). |
## Cycle-7 Findings
**F-DEPS-AZ795-1 (Low / Hardening)** — `FluentValidation` 12.0.0 → 12.1.1 minor refresh available
- Severity: Low (no CVE; hardening release only)
- Impact: 12.1.1 includes minor lifecycle fixes published in the upstream changelog; none are flagged as security advisories.
- Remediation: Bump `FluentValidation` and `FluentValidation.DependencyInjectionExtensions` to 12.1.1 in a follow-up cycle alongside other minor dependency rolls. Not blocking for cycle-7 release.
No Critical / High / Medium findings.
## Verdict
**PASS** (cycle-7 delta) — zero new CVEs, zero new supply-chain blockers. One Low/hardening recommendation (minor version bump to 12.1.1).
Cumulative verdict (carrying forward earlier cycles): **PASS_WITH_WARNINGS** — D2-cy4 (cycle 4 Medium, test-runtime only) still in effect; cycle 7 adds one Low.
_docs/05_security/infrastructure_review_cycle7.md
+51
View File
@@ -0,0 +1,51 @@
# Infrastructure & Configuration Review (Cycle 7)
**Date**: 2026-05-22
**Mode**: Delta scan
**Scope**: Cycle-7 changes to deployment configs, CI/CD files, and shell scripts only.
## Cycle-7 Infrastructure-Layer Diff
Computed via `git log --since=2026-05-19 -- Dockerfile* docker-compose* .woodpecker.yml .github/** scripts/**` against the cycle-7 commit (`865dfdb`):
| File | Diff | Security relevance |
|------|------|--------------------|
| `docker-compose.yml` | Host port for Postgres moved `5432:5432` → `5433:5432`. Container-internal port unchanged. | Local-dev only; the host port move avoids a sibling-project conflict. Does not affect production (production runs containers on a private docker network without host-port mapping per the existing deployment model). No exposure change. |
| `scripts/probe_inventory_validation.sh` | NEW manual probe script. | Reviewed in `static_analysis_cycle7.md` Test Code Review § `scripts/probe_inventory_validation.sh`. No embedded credentials; fails fast under `set -o errexit -o pipefail -o nounset`. `curl --insecure` used and justified for the dev self-signed cert. ✓ |
No changes to:
- `Dockerfile`, `Dockerfile.tests`, `Dockerfile.api`, or any image-build file.
- `docker-compose.tests.yml`, `docker-compose.prod.yml`, or any orchestration file other than the one host-port edit above.
- `.woodpecker.yml`, `.github/workflows/**`, or any CI/CD pipeline definition.
- `scripts/run-tests.sh`, `scripts/run-performance-tests.sh`, or any other harness shell script.
## Container & Image Security — Carried Forward Unchanged
| Check | Status (carried from cycle 5/6) | Cycle-7 impact |
|-------|---------------------------------|----------------|
| Non-root container user (Dockerfile `USER` directive) | Already in effect | None |
| Minimal base image (alpine/distroless/etc.) | The API image uses the .NET 10 SDK base — same as cycle 5; image hardening is owned by a separate, still-unscheduled follow-up task. | None |
| No secrets in build args | Verified cycle 5; no `Dockerfile` change in cycle 7 | None |
| Health checks | Compose `healthcheck` block on Postgres unchanged | None |
## CI/CD Security — Carried Forward Unchanged
| Check | Status | Cycle-7 impact |
|-------|--------|----------------|
| Secrets management (env vars / vault, not pipeline literals) | Existing pattern preserved | None |
| No credentials in pipeline definitions | `.woodpecker.yml` untouched in cycle 7 | None |
| Artifact signing | Existing posture (none — owned by a separate operational improvement track) | None |
| Dependency-audit step in pipeline | Existing posture (manual audit per `dependency_scan_cycle*.md`; no automated `dotnet list package --vulnerable` in CI due to the build-hang issue noted in `AGENTS.md`) | None |
## Environment & Secrets
- `.env.example` — not modified in cycle 7. The cycle-7 code reads no new env vars (FluentValidation has no config knobs; `GlobalValidatorConfig` is pure code).
- `appsettings.Development.json` — minor edit during cycle 7 (the connection-string port change, mirroring the compose-file edit). No new secret material.
- `appsettings.json` — production template; unchanged in cycle 7.
## Verdict (Phase 4)
**PASS** — zero new infrastructure-layer findings.
The single docker-compose host-port edit is a local-developer-convenience change with no exposure implication. The new probe shell script is dev/test only, env-driven, and contains no embedded secrets.
_docs/05_security/owasp_review_cycle7.md
+93
View File
@@ -0,0 +1,93 @@
# OWASP Top 10 Review (Cycle 7)
**Date**: 2026-05-22
**Mode**: Delta scan against OWASP Top 10:2021 (current at time of audit per https://owasp.org/www-project-top-ten/)
**Scope**: Cycle-7 delta only — AZ-794 wire-format rename, AZ-795 strict-validation infrastructure, AZ-796 inventory-endpoint validator. Earlier cycles' OWASP reviews remain authoritative for their respective surfaces; this file does NOT re-walk the full cycle-5 surface.
## A01 — Broken Access Control
**Status**: PASS
- `.RequireAuthorization()` is preserved on every existing endpoint and is chained on the cycle-7 inventory endpoint at `Program.cs:217` ahead of `.WithValidation<TileInventoryRequest>()` on line 218.
- Endpoint-filter execution order is governed by ASP.NET Core's middleware → routing → endpoint-filter pipeline. `UseAuthorization()` (line 201) reads the endpoint metadata produced by `.RequireAuthorization()` and short-circuits anonymous callers with 401 BEFORE the endpoint dispatch reaches any endpoint filter. Cycle-7 verification: `TileInventoryValidationTests` does not include a "no token → 400" case because the framework prevents that path; the suite's `TileInventoryTests.UnauthenticatedRequestReturns401_AC6` already covers it.
- No new CORS policy in cycle 7. `TilesCors` (cycle-6 baseline) is unchanged.
- No new IDOR paths — the inventory endpoint operates on caller-supplied identifiers but does not couple them to any tenant or owner field; tiles are globally-scoped in the post-AZ-484 model.
## A02 — Cryptographic Failures
**Status**: N/A (cycle 7)
- Cycle 7 has no cryptographic operations. JWT validation is unchanged from cycle 4 (HS256 with ≥ 32-byte secret, `ValidateLifetime + ValidateIssuer + ValidateAudience = true`, ClockSkew = 30s).
- The cycle-5 UUIDv5 SHA-1 surface is unaffected.
- TLS posture (Kestrel `Http1AndHttp2` with self-signed dev cert / ingress termination in prod) — unchanged from cycle 6.
## A03 — Injection
**Status**: PASS
- No SQL / Dapper / Npgsql usage in any cycle-7 new file.
- No `Process.Start` / shell-out / `eval` in any cycle-7 new file.
- All inputs reaching the validator are strongly typed (`int`, `Guid`, `IReadOnlyList<TileCoord>`) — System.Text.Json has already parsed and rejected anything malformed before the validator runs.
- The cycle-7 deserializer hardening (`UnmappedMemberHandling.Disallow`) raises the bar for the entire HTTP JSON surface by rejecting mass-assignment / property-injection attempts at parse time.
## A04 — Insecure Design
**Status**: PASS (improvement)
- AZ-795 / AZ-796 are themselves a *design fix* for ad-hoc validation. Pre-cycle-7, endpoints used inline `try/catch` blocks and per-handler defensive logic — easy to miss a path, easy to drift the shape of 4xx bodies. Cycle 7 centralises validation behind one `ValidationEndpointFilter<T>` + one `GlobalExceptionHandler`, both honouring a single contract (`error-shape.md` v1.0.0).
- The architecture doc (`_docs/02_document/architecture.md` § 9) now carries a coverage table that names every public endpoint and its validation status — making future drift visible.
## A05 — Security Misconfiguration
**Status**: PASS
- `UnmappedMemberHandling.Disallow` is a defense-in-depth hardening (mass-assignment prevention).
- `Swagger` exposure is still gated by `app.Environment.IsDevelopment()` (unchanged).
- `appsettings.Development.json` clearly tags DEV-ONLY JWT iss/aud values; `appsettings.json` ships empty so production fail-fast triggers if env vars are missing (unchanged from cycle 4).
- The new `AddProblemDetails()` registration is benign — it only standardises ProblemDetails generation for endpoints that explicitly return them.
- Note: `AddValidatorsFromAssemblyContaining<Program>()` is scoped to `SatelliteProvider.Api.dll` only — it cannot pick up `IValidator<T>` definitions from any other assembly (deliberate; the validators MUST live in the API project where the endpoint contract lives).
## A06 — Vulnerable & Outdated Components
**Status**: PASS_WITH_WARNINGS (Low)
- See `dependency_scan_cycle7.md` for the full table. Summary:
- FluentValidation 12.0.0 — no known CVEs; latest is 12.1.1 (hardening). Low/Hardening recommendation only.
- FluentValidation.DependencyInjectionExtensions 12.0.0 — same.
- All other packages unchanged from cycle 5.
## A07 — Identification and Authentication Failures
**Status**: PASS
- JWT validation parameters unchanged from cycle 4 (`AddSatelliteJwt`).
- No new auth-bypass paths introduced by cycle 7. The new endpoint filter cannot run for anonymous callers (see A01).
- The integration test `TileInventoryValidationTests` mints a valid token via the shared `JwtTokenFactory` — proves the happy path is properly auth-gated and not relying on any test-only bypass.
## A08 — Software and Data Integrity Failures
**Status**: N/A (cycle 7)
- No CI/CD changes, no artifact-signing changes, no auto-update paths touched in cycle 7.
## A09 — Security Logging and Monitoring Failures
**Status**: PASS_WITH_WARNINGS
- `GlobalExceptionHandler` 5xx branch logs `Method`, `Path`, `correlationId`, and the exception object (via Serilog default). This is the appropriate level — enough to debug, with correlationId for cross-referencing.
- The 4xx branch (cycle-7 new logic) does NOT log the exception. This is intentional and correct: malformed request bodies would otherwise create a noisy log signal and could push PII / token content into the log file if attached unwisely. The cost of "no log of the 400" is acceptable because the response itself carries the field path and a deterministic error message, so the client can self-debug.
- The Low information-disclosure findings (F-AZ795-1, F-AZ795-2) from `static_analysis_cycle7.md` belong here as well, but the impact is limited to a Low because the leaked content is type-name metadata, not credentials or PII.
## A10 — Server-Side Request Forgery (SSRF)
**Status**: N/A (cycle 7)
- No URL-input fields, no outbound HTTP calls triggered by the cycle-7 surface. (Pre-existing: `GoogleMapsDownloaderV2` makes outbound calls to Google Maps; not modified by cycle 7.)
## Cross-Reference with `security_approach.md`
The repo does not contain `_docs/00_problem/security_approach.md` (the pre-existing security audit cycles never produced one). The OWASP review proceeds against the cycle-5 + cycle-6 architectural decisions documented in `_docs/02_document/architecture.md` § 7 (Security Architecture) — which cycle-7 input-validation work cleanly extends rather than contradicts.
## Verdict (Phase 3)
**PASS_WITH_WARNINGS** — 1 dependency-hardening Low (D-AZ795-1: 12.0.0 → 12.1.1) + 2 information-disclosure Lows (F-AZ795-1, F-AZ795-2). Zero Critical / High / Medium findings.
_docs/05_security/security_report_cycle7.md
+106
View File
@@ -0,0 +1,106 @@
# Security Audit Report (Cycle 7)
**Date**: 2026-05-22
**Scope**: Cycle-7 delta over the cycle-5 audit (`_docs/05_security/security_report_cycle5.md`); cycle 6 produced no security report, so cycle 5 is the last full baseline. Cycle-7 surface = AZ-794 (`tileZoom/tileX/tileY` → `z/x/y` rename) + AZ-795 (strict-validation epic: FluentValidation, `UnmappedMemberHandling.Disallow`, GlobalExceptionHandler, error-shape contract) + AZ-796 (inventory-endpoint 9-rule validator).
**Trigger**: `/autodev` Step 14 (Security Audit) — feature cycle 7, post-implementation, post-test-spec-sync, post-docs-update.
**Verdict (cycle-7 delta)**: **PASS_WITH_WARNINGS** (3 Low findings; no Critical/High/Medium).
**Verdict (cumulative)**: **PASS_WITH_WARNINGS** (carries forward 1 cycle-4 Medium dep finding via D2-cy4 + 2 cycle-5 Low informational notes + cycle-7's 3 Lows).
## Summary
| Severity | Cycle 5 delta | Cycle 7 delta | Cumulative |
|----------|---------------|---------------|------------|
| Critical | 0 | 0 | 0 |
| High | 0 | 0 | 0 |
| Medium | 0 | 0 | 1 (D2-cy4 carry — `Microsoft.NET.Test.Sdk 17.8.0` transitive `NuGet.Frameworks`; test-runtime exposure only) |
| Low | 2 informational notes | **3 NEW** (F-AZ795-1, F-AZ795-2, D-AZ795-1) | 5+ |
## OWASP Top 10:2021 Assessment
| Category | Status (cycle-7 delta) | Findings |
|----------|------------------------|----------|
| A01 — Broken Access Control | PASS | — |
| A02 — Cryptographic Failures | N/A | No crypto in cycle 7 |
| A03 — Injection | PASS | — |
| A04 — Insecure Design | PASS (improvement) | AZ-795 / AZ-796 centralise validation behind one filter + one error handler — direct improvement |
| A05 — Security Misconfiguration | PASS | `UnmappedMemberHandling.Disallow` is defense-in-depth (mass-assignment prevention) |
| A06 — Vulnerable Components | PASS_WITH_WARNINGS | D-AZ795-1 (Low; bump 12.0.0 → 12.1.1 hardening release) |
| A07 — Auth Failures | PASS | JWT validation unchanged; new endpoint filter cannot run for anonymous callers |
| A08 — Data Integrity Failures | N/A | No CI/CD or artifact-signing surface in cycle 7 |
| A09 — Logging Failures | PASS_WITH_WARNINGS | F-AZ795-1 + F-AZ795-2 (Lows; `JsonException.Message` / `BadHttpRequestException.Message` echoed to client) |
| A10 — SSRF | N/A | No URL-input fields in cycle 7 |
## Findings
| # | Severity | Category | Location | Title |
|---|----------|----------|----------|-------|
| F-AZ795-1 | Low | Information Disclosure (A09) | `SatelliteProvider.Api/GlobalExceptionHandler.cs:108–117` | `JsonException.Message` propagated to client in 400 response (type-name + parse-position leak) |
| F-AZ795-2 | Low | Information Disclosure (A09) | `SatelliteProvider.Api/GlobalExceptionHandler.cs:88–93` | Generic `BadHttpRequestException.Message` propagated as `Detail` for non-JSON 400 paths |
| D-AZ795-1 | Low | Vulnerable & Outdated Components (A06) | NuGet | `FluentValidation` + `FluentValidation.DependencyInjectionExtensions` 12.0.0 → 12.1.1 (hardening release; no published CVE) |
### Finding Details
**F-AZ795-1: `JsonException.Message` propagated to client in 400 response** (Low / A09 — Information Disclosure)
- Location: `SatelliteProvider.Api/GlobalExceptionHandler.cs:108–117` (`TryExtractDeserializationErrors`)
- Description: `System.Text.Json.JsonException.Message` is echoed in the 400 `ValidationProblemDetails.errors[fieldPath]` array. The default message includes the offending .NET type (`System.Int32`, `System.Guid`, …), the JSON path (already separately captured as the key), and the byte position / line number in the payload — e.g. *"The JSON value could not be converted to System.Int32. Path: $.tiles[0].z | LineNumber: 0 | BytePositionInLine: 27."*.
- Impact: Low. The `UseAuthentication` + `UseAuthorization` middleware short-circuits anonymous callers with 401 before any endpoint filter runs, so the leak is only reachable by authenticated callers. The leaked content (type names, parse positions, `System.Text.Json` fingerprint) is already inferable from the OpenAPI spec at `/swagger/v1/swagger.json`; this finding narrows the attack surface for an authenticated tenant operator but does not expose secrets, PII, or pivot vectors.
- Remediation: Sanitise the response message to a generic string (e.g. `"Could not deserialize value at this field path."`) while continuing to log the raw `jsonEx.Message` server-side under the request's `correlationId`. Update `error-shape.md` test case `validation-type-mismatch` and the integration tests to assert no `System.*` substring appears in any `errors[]` value.
- Status: filed for next cycle.
**F-AZ795-2: Generic `BadHttpRequestException.Message` propagated as `Detail`** (Low / A09 — Information Disclosure)
- Location: `SatelliteProvider.Api/GlobalExceptionHandler.cs:88–93` (fallback 400 path when there is no `JsonException` inner exception)
- 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 `ProblemDetails.Detail`. ASP.NET Core message strings for these paths can include parameter names 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 or worsen it.
- Remediation: Same as F-AZ795-1 — sanitise the `Detail` to a generic string and log the raw `Message` server-side. Best done in tandem with F-AZ795-1.
- Status: filed for next cycle.
**D-AZ795-1: FluentValidation 12.0.0 → 12.1.1 hardening refresh** (Low / A06 — Vulnerable & Outdated Components)
- Location: `SatelliteProvider.Api/SatelliteProvider.Api.csproj` (`FluentValidation` + `FluentValidation.DependencyInjectionExtensions`)
- Description: 12.0.0 has no known CVEs (verified against GitHub Security Advisories, NVD, ReversingLabs Spectra Assure). 12.1.1 is the latest version (~5 months newer at audit time) and is a hardening release — minor upstream fixes, no security advisories.
- Impact: Low. Pure forward-compatibility hardening.
- Remediation: Bump both packages to 12.1.1 in a future minor-dependency-roll cycle.
- Status: filed for next cycle. Not release-blocking.
## Dependency Vulnerabilities
| Package | CVE | Severity | Fix Version | Status |
|---------|-----|----------|-------------|--------|
| FluentValidation 12.0.0 | — (hardening only) | Low | 12.1.1 | D-AZ795-1 — filed |
| FluentValidation.DependencyInjectionExtensions 12.0.0 | — (hardening only) | Low | 12.1.1 | D-AZ795-1 — filed (same item) |
| Microsoft.NET.Test.Sdk 17.8.0 (transitive `NuGet.Frameworks`) | — (cycle-4 carry-over D2-cy4) | Medium | TBD (next Test SDK refresh cycle) | carry-over from cycle 4 — owned by a separate unscheduled task |
## Recommendations
### Immediate (Critical/High)
None.
### Short-term (Medium)
None new in cycle 7. Cycle-4 carry-over D2-cy4 (`Microsoft.NET.Test.Sdk` Medium) remains in the backlog.
### Long-term (Low / Hardening)
1. **Sanitise client-visible 400 messages** (F-AZ795-1 + F-AZ795-2). Single change in `GlobalExceptionHandler.WriteClientErrorAsync` + matching test assertion. Estimated 1 hour of effort. Should be filed as a small follow-up child of AZ-795 (or as a standalone task under the same epic).
2. **Bump FluentValidation 12.0.0 → 12.1.1** (D-AZ795-1). Single `.csproj` edit + a regression test pass; no API surface change in 12.0.0 → 12.1.1 per the upstream changelog.
### Cumulative reminders (carry-overs)
- Cycle-4 D2-cy4 — `Microsoft.NET.Test.Sdk 17.8.0` transitive `NuGet.Frameworks` Medium-severity finding, test-runtime exposure only. Owned by the next Test SDK refresh.
## Cycle-7 Architectural Wins
The audit specifically wants to record three improvements introduced this cycle:
1. **Mass-assignment prevention by default** — `UnmappedMemberHandling.Disallow` on the global JSON pipeline rejects any unknown root or nested field across every public endpoint. The cycle-7 acceptance criteria explicitly enumerate this for the inventory endpoint; the protection is in force for every other endpoint that consumes a JSON body too.
2. **Uniform 4xx contract** — `error-shape.md` v1.0.0 unifies the wire shape across two failure layers (deserializer + FluentValidation). Future child tickets reuse `ValidationEndpointFilter<T>`, `ProblemDetailsAssertions`, and the contract without adding new infrastructure. This dramatically reduces the chance of future endpoints drifting into their own bespoke error shapes.
3. **Auth-before-validation invariant verified** — endpoint filters added via `WithValidation<T>()` cannot run for unauthenticated callers (the routing pipeline runs `UseAuthorization` BEFORE the endpoint filter chain). The audit explicitly verified the cycle-7 inventory endpoint and re-asserted the invariant in this report.
## Verdict
**PASS_WITH_WARNINGS** — 3 Lows, 0 Mediums (cycle-7 delta), 0 Highs, 0 Criticals. Cycle 7 is **safe to release**. The 3 Lows are filed for follow-up cycles and do not block release.
Cumulative posture: PASS_WITH_WARNINGS (1 cycle-4 Medium carry-over via D2-cy4 + Lows above). No regression of the cycle-5 PASS posture.
_docs/05_security/static_analysis_cycle7.md
+160
View File
@@ -0,0 +1,160 @@
# 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:108–117` (`TryExtractDeserializationErrors`)
- **Code**:
```csharp
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:88–93` (the fallback non-`ValidationProblemDetails` path)
- **Code**:
```csharp
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:38–53` (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|secret|api.?key|bearer|token` in the new files returns matches only inside `*Tests.cs` files where they reference test-only env-var-driven `JWT_SECRET`. No hardcoded secret material. |
| 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:
```csharp
.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.