From 51b572108aee0052f72e274cfc485d4e3ca4b891 Mon Sep 17 00:00:00 2001 From: Oleksandr Bezdieniezhnykh Date: Mon, 11 May 2026 10:03:05 +0300 Subject: [PATCH] [AZ-484] Cycle 1 Steps 12-16: docs, security, perf, deploy report Captures the post-implementation autodev gates for AZ-484 multi-source tile storage: - Step 12 (Test-Spec Sync): added 7 AC rows (AZ-484 AC-1..AC-7) and a PT-07 NFR row to traceability-matrix.md; added PT-07 scenario to performance-tests.md. - Step 13 (Update Docs): refreshed data_model.md (tiles columns + indexes + selection rule + UPSERT contract + migrations 012/013), module-layout.md (Common/Enums section with L-001 guidance, DataAccess imports-from now lists 6 sites), 6 module / component docs to reflect the new repo signatures, source/captured_at fields, and Dapper enum bypass workaround. ripple_log_cycle1.md records zero out-of-scope ripple. - Step 14 (Security Audit): PASS_WITH_WARNINGS - 0 Critical, 0 High, 5 Medium, 5 Low. AZ-484 itself added zero new findings. Hardening items (Postgres default creds, .env in build context, GMaps key rotation, ASP.NET Core 8.0.21 -> 8.0.25, rate limiter) recorded for separate tickets. - Step 15 (Performance Test): all PT-01..PT-07 scenarios Unverified (non-blocking); PT-07 baseline-comparison harness deferred to a leftover for next cycle. - Step 16 (Deploy): cycle deploy report covering migration safety, rollback path, post-deploy verification, security caveats. Co-authored-by: Cursor --- .../components/02_data_access/description.md | 15 ++- .../03_tile_downloader/description.md | 4 +- _docs/02_document/data_model.md | 18 ++- _docs/02_document/module-layout.md | 9 +- .../modules/dataaccess_migrator.md | 4 +- .../02_document/modules/dataaccess_models.md | 4 +- .../modules/dataaccess_tile_repository.md | 23 ++-- .../modules/services_tile_service.md | 8 +- .../02_document/modules/tests_integration.md | 1 + _docs/02_document/ripple_log_cycle1.md | 56 ++++++++ _docs/02_document/tests/performance-tests.md | 9 ++ .../02_document/tests/traceability-matrix.md | 17 ++- .../03_implementation/deploy_cycle1_az484.md | 79 ++++++++++++ _docs/05_security/dependency_scan.md | 65 ++++++++++ _docs/05_security/infrastructure_review.md | 87 +++++++++++++ _docs/05_security/owasp_review.md | 40 ++++++ _docs/05_security/security_report.md | 120 ++++++++++++++++++ _docs/05_security/static_analysis.md | 90 +++++++++++++ .../perf_2026-05-11_cycle1_az484.md | 52 ++++++++ _docs/_autodev_state.md | 10 +- .../2026-05-11_perf-pt07-harness.md | 32 +++++ 21 files changed, 710 insertions(+), 33 deletions(-) create mode 100644 _docs/02_document/ripple_log_cycle1.md create mode 100644 _docs/03_implementation/deploy_cycle1_az484.md create mode 100644 _docs/05_security/dependency_scan.md create mode 100644 _docs/05_security/infrastructure_review.md create mode 100644 _docs/05_security/owasp_review.md create mode 100644 _docs/05_security/security_report.md create mode 100644 _docs/05_security/static_analysis.md create mode 100644 _docs/06_metrics/perf_2026-05-11_cycle1_az484.md create mode 100644 _docs/_process_leftovers/2026-05-11_perf-pt07-harness.md diff --git a/_docs/02_document/components/02_data_access/description.md b/_docs/02_document/components/02_data_access/description.md index 449bf26..94f24da 100644 --- a/_docs/02_document/components/02_data_access/description.md +++ b/_docs/02_document/components/02_data_access/description.md @@ -16,13 +16,14 @@ | Method | Input | Output | Async | Error Types | |--------|-------|--------|-------|-------------| | `GetByIdAsync` | Guid | `TileEntity?` | Yes | NpgsqlException | -| `GetByTileCoordinatesAsync` | zoom, x, y | `TileEntity?` | Yes | NpgsqlException | -| `FindExistingTileAsync` | lat, lon, tileSizeM, zoom, version | `TileEntity?` | Yes | NpgsqlException | -| `GetTilesByRegionAsync` | lat, lon, sizeM, zoom | `IEnumerable` | Yes | NpgsqlException | -| `InsertAsync` | `TileEntity` | Guid | Yes | NpgsqlException | +| `GetByTileCoordinatesAsync` | zoom, x, y | `TileEntity?` (most-recent across sources, AZ-484) | Yes | NpgsqlException | +| `GetTilesByRegionAsync` | lat, lon, sizeM, zoom | `IEnumerable` (one row per cell via `DISTINCT ON`, AZ-484) | Yes | NpgsqlException | +| `InsertAsync` | `TileEntity` | Guid (per-source UPSERT, AZ-484) | Yes | NpgsqlException | | `UpdateAsync` | `TileEntity` | int | Yes | NpgsqlException | | `DeleteAsync` | Guid | int | Yes | NpgsqlException | +`FindExistingTileAsync` was removed by AZ-376 (replaced by direct cell lookups through `GetByTileCoordinatesAsync` + `GetTilesByRegionAsync`). + ### Interface: IRegionRepository | Method | Input | Output | Async | Error Types | |--------|-------|--------|-------|-------------| @@ -58,7 +59,7 @@ |-------|-----------|----------|--------------| | GetByTileCoordinatesAsync (tile lookup) | Very High | Yes | `(tile_zoom, tile_x, tile_y)` | | GetTilesByRegionAsync (spatial) | High | Yes | `(latitude, longitude, tile_zoom)` | -| InsertAsync (tile upsert) | High | Yes | Composite unique on `(lat, lon, zoom, size, version)` | +| InsertAsync (tile per-source upsert) | High | Yes | Composite unique on `(lat, lon, tile_zoom, tile_size_meters, source)` (AZ-484: `idx_tiles_unique_location_source`) | | GetByStatusAsync (region polling) | Medium | No | `(status)` | | GetRoutesWithPendingMapsAsync | Low | No | `(request_maps, maps_ready)` | @@ -88,7 +89,9 @@ - Repository interfaces are defined in this project (not in Common), creating a dependency from Services to DataAccess - Column mapping uses SQL aliases (`tile_zoom as TileZoom`) rather than Dapper attribute mapping -- TileRepository.InsertAsync uses an upsert pattern; concurrent inserts of the same tile won't conflict +- TileRepository.InsertAsync uses a per-source UPSERT pattern (AZ-484); two producers (e.g., `google_maps` + `uav`) coexist as separate rows for the same cell, while same-source re-inserts overwrite and refresh `captured_at` +- `TileEntity.Source` is stored as a plain `string` (not the `TileSource` enum) due to Dapper issue #259 — see `_docs/LESSONS.md` L-001. Conversion happens via `SatelliteProvider.Common.Enums.TileSourceConverter` +- The frozen v1.0.0 `tile-storage` contract (`_docs/02_document/contracts/data-access/`) is the authoritative spec for all `tiles` invariants enforced here - No soft-delete; `DeleteAsync` is a hard delete ## 8. Dependency Graph diff --git a/_docs/02_document/components/03_tile_downloader/description.md b/_docs/02_document/components/03_tile_downloader/description.md index a4f9f0b..08c2338 100644 --- a/_docs/02_document/components/03_tile_downloader/description.md +++ b/_docs/02_document/components/03_tile_downloader/description.md @@ -8,7 +8,7 @@ **csproj**: `SatelliteProvider.Services.TileDownloader/SatelliteProvider.Services.TileDownloader.csproj` (split out of the monolithic `SatelliteProvider.Services` project in epic AZ-309) -**Upstream dependencies**: Common (DTOs, GeoUtils, configs, RateLimitException), DataAccess (TileEntity, ITileRepository) +**Upstream dependencies**: Common (DTOs, Enums — `TileSource` + `TileSourceConverter` since AZ-484, GeoUtils, configs, RateLimitException), DataAccess (TileEntity, ITileRepository) **Downstream consumers**: RegionProcessing and WebApi — both via `ITileService` from Common (no compile-time `ProjectReference` from any consumer to this project's concrete types). @@ -35,7 +35,7 @@ | Data | Cache Type | TTL | Invalidation | |------|-----------|-----|-------------| | Tile bytes | In-memory (IMemoryCache, owned by TileService since AZ-310) | 1h absolute, 30min sliding | None (manual restart) | -| Tile metadata | Database | Until year rollover | Version-based (current year) | +| Tile metadata | Database | Append-by-source per cell (AZ-484); reads return most-recent across sources | Per-source UPSERT keyed on `(latitude, longitude, tile_zoom, tile_size_meters, source)` overwrites the existing same-source row and refreshes `captured_at` | | Active downloads | ConcurrentDictionary | Duration of download | Removed on completion | ## 5. Implementation Details diff --git a/_docs/02_document/data_model.md b/_docs/02_document/data_model.md index b33dd85..7de0fe9 100644 --- a/_docs/02_document/data_model.md +++ b/_docs/02_document/data_model.md @@ -14,6 +14,8 @@ erDiagram varchar image_type varchar maps_version int version + varchar source + timestamp captured_at varchar file_path int tile_x int tile_y @@ -96,19 +98,25 @@ Stores metadata for downloaded satellite imagery tiles. Each tile is a single im | tile_size_meters | DOUBLE PRECISION | NOT NULL | Ground coverage in meters | | tile_size_pixels | INT | NOT NULL | Image dimension in pixels | | image_type | VARCHAR(10) | NOT NULL | Image format (e.g., "jpg") | -| maps_version | VARCHAR(50) | | Legacy free-form provider tag; post-AZ-373 new rows write NULL (column retained for forensics on pre-existing rows) | -| version | INT | NOT NULL, DEFAULT 2025 | Year-based versioning for cache invalidation | +| maps_version | VARCHAR(50) | | Legacy free-form provider tag; post-AZ-373 new rows write NULL. Vestigial post-AZ-484 (column retained for forensics on pre-existing rows; no longer part of any index) | +| version | INT | NOT NULL, DEFAULT 2025 | Year-based versioning for cache invalidation. Vestigial post-AZ-484 — removed from the unique key by migration 012 (preparation for AZ-484); column retained nullable for backward compatibility | +| source | VARCHAR(32) | NOT NULL, DEFAULT 'google_maps' | AZ-484: producer of the imagery (`'google_maps'`, `'uav'`). Closed value set — see `tile-storage` v1.0.0 contract Inv-5 and `Common.Enums.TileSourceConverter`. Backfilled to `'google_maps'` for all pre-AZ-484 rows by migration 013 | +| captured_at | TIMESTAMP | NOT NULL | AZ-484: imagery acquisition timestamp (UTC). Drives most-recent-across-sources selection. Backfilled to `created_at` for pre-AZ-484 rows by migration 013 | | file_path | VARCHAR(500) | NOT NULL | Relative path to stored image | | tile_x | INT | NOT NULL | Tile X coordinate (Slippy Map) | | tile_y | INT | NOT NULL | Tile Y coordinate (Slippy Map) | | created_at | TIMESTAMP | NOT NULL, DEFAULT NOW | | | updated_at | TIMESTAMP | NOT NULL, DEFAULT NOW | | -**Indexes**: -- `idx_tiles_unique_location` UNIQUE (latitude, longitude, tile_zoom, tile_size_meters, version) +**Indexes** (post-AZ-484): +- `idx_tiles_unique_location_source` UNIQUE (latitude, longitude, tile_zoom, tile_size_meters, source) — created by migration 013; replaces the pre-AZ-484 4-col `idx_tiles_unique_location` (which itself superseded the legacy 5-col `(…, version)` index dropped by migration 012) - `idx_tiles_coordinates` (tile_zoom, tile_x, tile_y, version) - `idx_tiles_zoom` (tile_zoom) +**Selection rule**: `GetByTileCoordinatesAsync` and `GetTilesByRegionAsync` return the most-recent row across sources for any `(latitude, longitude, tile_zoom, tile_size_meters)` cell. Tie-break: `captured_at DESC, updated_at DESC, id DESC`. Region read uses `DISTINCT ON` to enforce one-row-per-cell at the SQL layer. + +**UPSERT contract**: `INSERT … ON CONFLICT (latitude, longitude, tile_zoom, tile_size_meters, source) DO UPDATE` — same-source re-insert refreshes `file_path, tile_x, tile_y, captured_at, updated_at`. Two producers for the same cell coexist as separate rows. + ### regions Tracks region download requests and their processing status. @@ -215,3 +223,5 @@ Junction table linking routes to their generated region requests, with geofence | 009 | AddGeofencePolygonIndex | Polygon index tracking | | 010 | AddTilesZipToRoutes | ZIP generation fields | | 011 | AddTileCoordinates | Slippy map X/Y + rename zoom_level → tile_zoom | +| 012 | DropTileVersionConstraint | Drops legacy 5-col `(…, version)` unique index; replaces with 4-col `idx_tiles_unique_location` (preparation for AZ-484) | +| 013 | AddTileSourceAndCapturedAt | AZ-484: adds `source` (default `'google_maps'`) + `captured_at` columns; backfills both for pre-existing rows; replaces 4-col unique with 5-col `idx_tiles_unique_location_source`. Transactional; idempotent against partial replays | diff --git a/_docs/02_document/module-layout.md b/_docs/02_document/module-layout.md index 07e1ae5..39fa7d3 100644 --- a/_docs/02_document/module-layout.md +++ b/_docs/02_document/module-layout.md @@ -57,7 +57,7 @@ - **Internal**: (none — all repository types are public for DI registration) - **Owns**: `SatelliteProvider.DataAccess/**` - **ProjectReferences**: `SatelliteProvider.Common` -- **Imports from**: `SatelliteProvider.Common.Enums` (5 sites: `RegionRepository`, `IRegionRepository`, `Models/RegionEntity`, `Models/RoutePointEntity`, `TypeHandlers/EnumStringTypeHandler`); `SatelliteProvider.Common.Configs` (`MapConfig.DefaultTileSizePixels` in `TileRepository`); `SatelliteProvider.Common.Utils` (`GeoUtils.EarthEquatorialCircumferenceMeters`, `GeoUtils.MetersPerDegreeLatitude` in `TileRepository`). +- **Imports from**: `SatelliteProvider.Common.Enums` (6 sites: `RegionRepository`, `IRegionRepository`, `Models/RegionEntity`, `Models/RoutePointEntity`, `TypeHandlers/EnumStringTypeHandler`, `Models/TileEntity` — references `TileSourceConverter.GoogleMapsWireValue` const for the AZ-484 default value); `SatelliteProvider.Common.Configs` (`MapConfig.DefaultTileSizePixels` in `TileRepository`); `SatelliteProvider.Common.Utils` (`GeoUtils.EarthEquatorialCircumferenceMeters`, `GeoUtils.MetersPerDegreeLatitude` in `TileRepository`). - **Consumed by**: TileDownloader, RegionProcessing, RouteManagement, WebApi ### Component: TileDownloader @@ -139,6 +139,13 @@ - **Purpose**: Stateless geospatial utility functions (coordinate math, distance, bearing) - **Consumed by**: TileDownloader, RegionProcessing, RouteManagement +### Common/Enums + +- **Directory**: `SatelliteProvider.Common/Enums/` +- **Purpose**: Domain enums shared across layers (`RegionStatus`, `RoutePointType`, `TileSource`) plus their explicit wire-value converters when persistence requires snake_case strings (`TileSourceConverter`). Converter classes belong here — not in DataAccess — because they encode a domain-level vocabulary that must be visible to every component. +- **Consumed by**: DataAccess (entity defaults, type handler registration), TileDownloader (sets `TileEntity.Source` via `TileSourceConverter.ToWireValue`), Tests +- **Important constraint**: Dapper's `SqlMapper.TypeHandler` is bypassed for enum reads (Dapper issue #259 — see `_docs/LESSONS.md` L-001). For any new enum that must round-trip through a database column, prefer the `string`-on-entity + `Enum`-at-API-boundary pattern with a converter class in this folder. Do NOT register a `TypeHandler` and assume it will be honored on reads. + ## Allowed Dependencies (layering) | Layer | Components | May import from (compile-time ProjectReferences) | diff --git a/_docs/02_document/modules/dataaccess_migrator.md b/_docs/02_document/modules/dataaccess_migrator.md index 0305f86..8a6ff17 100644 --- a/_docs/02_document/modules/dataaccess_migrator.md +++ b/_docs/02_document/modules/dataaccess_migrator.md @@ -23,7 +23,7 @@ Runs DbUp-based SQL migrations against PostgreSQL on application startup. Ensure ## Consumers - `Program.cs` — instantiated directly (not via DI) and called during startup. If migration fails, the application throws and does not start. -## Migrations (11 scripts) +## Migrations (13 scripts) 1. `001_CreateTilesTable.sql` 2. `002_CreateRegionsTable.sql` 3. `003_CreateIndexes.sql` @@ -35,6 +35,8 @@ Runs DbUp-based SQL migrations against PostgreSQL on application startup. Ensure 9. `009_AddGeofencePolygonIndex.sql` 10. `010_AddTilesZipToRoutes.sql` 11. `011_AddTileCoordinates.sql` +12. `012_DropTileVersionConstraint.sql` — drops the legacy 5-col `(latitude, longitude, tile_zoom, tile_size_meters, version)` unique index, replaces with 4-col `idx_tiles_unique_location` (preparation for AZ-484). +13. `013_AddTileSourceAndCapturedAt.sql` — AZ-484 multi-source tile storage. Transactional. Adds `source` (VARCHAR(32) NOT NULL DEFAULT 'google_maps') and `captured_at` (TIMESTAMP NOT NULL) columns; backfills existing rows with `source='google_maps'`, `captured_at=created_at`; drops `idx_tiles_unique_location` and creates 5-col `idx_tiles_unique_location_source` on `(latitude, longitude, tile_zoom, tile_size_meters, source)`. Idempotent against partial replays. ## Configuration Receives connection string directly as constructor parameter. diff --git a/_docs/02_document/modules/dataaccess_models.md b/_docs/02_document/modules/dataaccess_models.md index 9d1d330..0e9048d 100644 --- a/_docs/02_document/modules/dataaccess_models.md +++ b/_docs/02_document/modules/dataaccess_models.md @@ -9,7 +9,9 @@ Database entity classes that map directly to PostgreSQL tables via Dapper. Prope Maps to `tiles` table. - `Id` (Guid), `TileZoom` (int), `TileX` (int), `TileY` (int) - `Latitude`, `Longitude` (double), `TileSizeMeters` (double), `TileSizePixels` (int) -- `ImageType` (string), `MapsVersion` (string?), `Version` (int) +- `ImageType` (string), `MapsVersion` (string?), `Version` (int) — `MapsVersion`/`Version` are vestigial post-AZ-484 (kept nullable for backward compatibility; no longer part of the unique key) +- `Source` (string) — AZ-484 producer wire value, defaults to `TileSourceConverter.GoogleMapsWireValue` (`"google_maps"`). Stored as plain string (not the `TileSource` enum) due to Dapper issue #259 — see `_docs/LESSONS.md` L-001. Convert via `SatelliteProvider.Common.Enums.TileSourceConverter.{ToWireValue,FromWireValue}`. +- `CapturedAt` (DateTime, UTC) — AZ-484 imagery acquisition timestamp; drives the most-recent-across-sources selection. - `FilePath` (string), `CreatedAt`, `UpdatedAt` (DateTime) ### RegionEntity diff --git a/_docs/02_document/modules/dataaccess_tile_repository.md b/_docs/02_document/modules/dataaccess_tile_repository.md index bf16d17..5c75d99 100644 --- a/_docs/02_document/modules/dataaccess_tile_repository.md +++ b/_docs/02_document/modules/dataaccess_tile_repository.md @@ -7,26 +7,31 @@ Dapper-based repository for the `tiles` table. Handles CRUD operations and spati ### ITileRepository (interface) - `GetByIdAsync(Guid id) → Task` -- `GetByTileCoordinatesAsync(int tileZoom, int tileX, int tileY) → Task`: finds tile by slippy map coordinates, returns latest version -- `FindExistingTileAsync(double lat, double lon, double tileSizeMeters, int zoomLevel, int version) → Task`: fuzzy coordinate match (tolerance: 0.0001° lat/lon, 1m tile size) -- `GetTilesByRegionAsync(double lat, double lon, double sizeMeters, int zoomLevel) → Task>`: spatial bounding box query with expanded range to cover tile edges -- `InsertAsync(TileEntity tile) → Task`: upsert — `ON CONFLICT (latitude, longitude, tile_zoom, tile_size_meters, version) DO UPDATE` file_path, tile_x, tile_y, updated_at -- `UpdateAsync(TileEntity tile) → Task` +- `GetByTileCoordinatesAsync(int tileZoom, int tileX, int tileY) → Task`: finds the most-recent tile across all sources for the given slippy coordinates. Selection rule: `ORDER BY captured_at DESC, updated_at DESC, id DESC LIMIT 1` (AZ-484 v1.0.0 contract). +- `GetTilesByRegionAsync(double lat, double lon, double sizeMeters, int zoomLevel) → Task>`: spatial bounding box query (expanded by 2 × tile size to cover edges); applies `DISTINCT ON (latitude, longitude, tile_zoom, tile_size_meters)` per AZ-484 to return at most one row per cell — the most-recent across sources — preserving the historical caller-facing order `latitude DESC, longitude ASC`. +- `InsertAsync(TileEntity tile) → Task`: per-source UPSERT — `ON CONFLICT (latitude, longitude, tile_zoom, tile_size_meters, source) DO UPDATE file_path, tile_x, tile_y, captured_at, updated_at` (AZ-484 5-column unique key). +- `UpdateAsync(TileEntity tile) → Task`: full row update by `id` including `source` and `captured_at`. - `DeleteAsync(Guid id) → Task` ### TileRepository (implementation) -Constructs a new `NpgsqlConnection` per method call (no connection pooling at the repository level; Npgsql pools connections internally). +Constructs a new `NpgsqlConnection` per method call (no connection pooling at the repository level; Npgsql pools connections internally). Logs a `Slow GetTilesByRegionAsync` warning when the region query exceeds 500 ms. ## Internal Logic -- `GetTilesByRegionAsync` calculates a bounding box by expanding the requested region by 2 × tile size to ensure edge tiles are included. Uses meters-to-degrees approximation (111,000 m/degree latitude, adjusted for longitude). -- `InsertAsync` uses an upsert pattern to handle duplicate tile downloads gracefully. -- `GetByTileCoordinatesAsync` orders by `version DESC` and takes the latest. +- `GetTilesByRegionAsync` calculates a bounding box by expanding the requested region by 2 × tile size to ensure edge tiles are included. Uses meters-to-degrees approximation via `GeoUtils` (post-AZ-377 — single source of truth for Earth constants). +- `InsertAsync` uses a per-source UPSERT pattern keyed on the 5-column unique index `idx_tiles_unique_location_source` (created by migration 013). Two producers (e.g., `google_maps` + `uav`) coexist for the same cell; same-source re-insert refreshes `captured_at` and `updated_at`. +- `GetByTileCoordinatesAsync` and `GetTilesByRegionAsync` apply the AZ-484 selection rule: most-recent across sources, deterministic tie-break on `(captured_at DESC, updated_at DESC, id DESC)`. +- `TileEntity.Source` is a plain `string` storing the snake_case wire value (`'google_maps'` | `'uav'`); enum<->wire conversion happens via `SatelliteProvider.Common.Enums.TileSourceConverter`. This avoids Dapper issue #259 (TypeHandler bypass for enum reads — see `_docs/LESSONS.md` L-001). +- `FindExistingTileAsync` was removed by AZ-376 (see `_docs/04_refactoring/03-code-quality-refactoring/`). ## Dependencies - NuGet: `Dapper`, `Npgsql` - `SatelliteProvider.DataAccess.Models.TileEntity` +- `SatelliteProvider.Common.GeoUtils` (Earth constants / meters-to-degrees) - `Microsoft.Extensions.Logging` +## Contract +Implements the frozen v1.0.0 contract `_docs/02_document/contracts/data-access/tile-storage.md`. Schema invariants Inv-1..Inv-5 are enforced here (UPSERT key, selection rule, source value space). + ## Consumers - `TileService` — all read/write operations - `Program.cs` (ServeTile, GetTileByLatLon handlers) — `GetByTileCoordinatesAsync`, `InsertAsync` diff --git a/_docs/02_document/modules/services_tile_service.md b/_docs/02_document/modules/services_tile_service.md index 388e48b..e40fc03 100644 --- a/_docs/02_document/modules/services_tile_service.md +++ b/_docs/02_document/modules/services_tile_service.md @@ -9,9 +9,9 @@ Orchestrates tile downloading and persistence. Bridges the downloader (Google Ma ### TileService (implements ITileService) - `DownloadAndStoreTilesAsync(double lat, double lon, double sizeMeters, int zoomLevel, CancellationToken) → Task>`: - 1. Queries existing tiles in the region from the repository (latest per `(latitude, longitude, zoom_level, tile_size_meters)` post-AZ-357) + 1. Queries existing tiles in the region from the repository — most-recent across sources per `(latitude, longitude, tile_zoom, tile_size_meters)` (AZ-484 selection rule applied by `TileRepository.GetTilesByRegionAsync` via `DISTINCT ON`) 2. Calls `ISatelliteDownloader.GetTilesWithMetadataAsync` with existing tiles to skip - 3. Creates `TileEntity` for each newly downloaded tile and inserts via repository (upsert) + 3. Creates `TileEntity` for each newly downloaded tile and inserts via repository (per-source UPSERT keyed on `(latitude, longitude, tile_zoom, tile_size_meters, source)`) 4. Returns combined list of existing + new tile metadata - `GetTileAsync(Guid id) → Task`: single tile lookup - `GetTilesByRegionAsync(double lat, double lon, double sizeMeters, int zoomLevel) → Task>`: query tiles in a region @@ -20,7 +20,8 @@ Orchestrates tile downloading and persistence. Bridges the downloader (Google Ma ## Internal Logic - New rows write `Version = null` and `MapsVersion = null` (post-AZ-357 / AZ-373); the `version` and `maps_version` columns are retained for backward compatibility with pre-existing rows -- `MapToMetadata(TileEntity) → TileMetadata`: entity-to-DTO mapping (static helper); `MapsVersion` is no longer projected onto `TileMetadata` / `DownloadTileResponse` +- AZ-484: `BuildTileEntity` stamps every newly downloaded row with `Source = TileSourceConverter.ToWireValue(TileSource.GoogleMaps)` (wire value `"google_maps"`) and `CapturedAt = DateTime.UtcNow`. The Google Maps download path is the only producer of `'google_maps'` rows; UAV ingestion (separate task) is the only producer of `'uav'` rows. +- `MapToMetadata(TileEntity) → TileMetadata`: entity-to-DTO mapping (static helper); `MapsVersion` is no longer projected onto `TileMetadata` / `DownloadTileResponse`. `Source` and `CapturedAt` are not currently projected to the public DTO (no API contract change observable for AZ-484). - `TileSizePixels` sourced from `MapConfig.TileSizePixels` (default 256, post-AZ-371); image type fixed at `"jpg"` - `IMemoryCache` keyed by `(z, x, y)` with 1h absolute / 30min sliding expiration; populated on first hit and on downloader fallback @@ -29,6 +30,7 @@ Orchestrates tile downloading and persistence. Bridges the downloader (Google Ma - `ITileRepository` - `IMemoryCache` (registered by `AddTileDownloader()`) - `SatelliteProvider.Common.DTO` — GeoPoint, TileMetadata, TileBytes +- `SatelliteProvider.Common.Enums` — `TileSource`, `TileSourceConverter` (AZ-484) - `SatelliteProvider.DataAccess.Models` — TileEntity ## Consumers diff --git a/_docs/02_document/modules/tests_integration.md b/_docs/02_document/modules/tests_integration.md index 3a05598..9f60ace 100644 --- a/_docs/02_document/modules/tests_integration.md +++ b/_docs/02_document/modules/tests_integration.md @@ -11,6 +11,7 @@ Console application that runs end-to-end integration tests against a live API in - `BasicRouteTests` — route creation with intermediate points - `ComplexRouteTests` — routes with geofencing - `ExtendedRouteTests` — routes with `requestMaps: true` and tile ZIP creation +- `MigrationTests` — direct PostgreSQL schema/index validation (no HTTP). AZ-484 cycle added: `NewUniqueConstraintIncludesSourceColumn_AZ484_AC1`, `BackfillUpdateAssignsGoogleMapsAndCapturedAt_AZ484_AC4`, `MultiSourceInsertCoexistsUnderNewIndex_AZ484_AC1`, `MostRecentAcrossSourcesSelection_AZ484_AC2`, `SameSourceUpsertReplacesPreviousRow_AZ484_AC3` (latter four use temp tables to keep production data untouched). ### Supporting Classes - `Models.cs` — HTTP response DTOs for deserialization diff --git a/_docs/02_document/ripple_log_cycle1.md b/_docs/02_document/ripple_log_cycle1.md new file mode 100644 index 0000000..b4e762e --- /dev/null +++ b/_docs/02_document/ripple_log_cycle1.md @@ -0,0 +1,56 @@ +# Ripple Log — Cycle 1 (AZ-484) + +Generated by `document/SKILL.md` Task mode, Step 0.5 (Import-Graph Ripple). + +## Cycle scope +- AZ-484 — Multi-source tile storage schema + +## Changed source identifiers (Step 0) +- `SatelliteProvider.DataAccess.Migrations.013_AddTileSourceAndCapturedAt.sql` (DDL — not a C# import target) +- `SatelliteProvider.Common.Enums.TileSource` (new) +- `SatelliteProvider.Common.Enums.TileSourceConverter` (new) +- `SatelliteProvider.DataAccess.Models.TileEntity` (added `Source`, `CapturedAt`; `Source` typed as `string` per L-001 workaround) +- `SatelliteProvider.DataAccess.Repositories.TileRepository` (`ColumnList`, `GetByTileCoordinatesAsync`, `GetTilesByRegionAsync`, `InsertAsync`, `UpdateAsync`) +- `SatelliteProvider.DataAccess.TypeHandlers.EnumStringTypeHandler` (registration list — net-zero after the L-001 pivot) +- `SatelliteProvider.Services.TileDownloader.TileService.BuildTileEntity` (stamps `Source` + `CapturedAt`) + +## Reverse-dependency analysis (Step 0.5) +Tooling: `Grep` over the C# tree for `using SatelliteProvider.Common.Enums`, `TileSource`, `TileEntity`, `ITileRepository` references. + +### Direct importers of the changed identifiers (already in the AZ-484 changeset) +- `SatelliteProvider.DataAccess/Models/TileEntity.cs` — imports `Common.Enums` (uses `TileSourceConverter.GoogleMapsWireValue` const for the field default) +- `SatelliteProvider.Services.TileDownloader/TileService.cs` — imports `Common.Enums` (calls `TileSourceConverter.ToWireValue(TileSource.GoogleMaps)`) +- `SatelliteProvider.Tests/TileServiceTests.cs`, `TileSourceConverterTests.cs`, `RepositoryRefactorTests.cs`, `EnumStringTypeHandlerTests.cs` +- `SatelliteProvider.IntegrationTests/MigrationTests.cs` + +All of the above were edited and their tests were re-run as part of AZ-484 itself; no documentation ripple is required for them. + +### Indirect importers requiring doc refresh +The only consumers that touch `TileEntity` *as a type* (not as a column-aware contract) are: + +- `SatelliteProvider.Services.TileDownloader/GoogleMapsDownloaderV2.cs` — accepts `IEnumerable` for the cache-skip parameter. **No doc ripple**: the downloader treats `TileEntity` as an opaque cache token; the new `Source` / `CapturedAt` columns are not accessed by the downloader. +- `Program.cs` (WebApi) — calls `ITileRepository.GetByTileCoordinatesAsync` and `InsertAsync`. **No doc ripple to `api_program.md`**: the endpoint contract did not change (response DTO `TileMetadata` is unaffected by AZ-484; `Source` and `CapturedAt` are not currently projected to the public API). +- `RegionService` / `RegionProcessingService` — call `ITileService` methods, not `ITileRepository` directly. **No doc ripple**: the `ITileService` method signatures are unchanged; the most-recent-across-sources read rule is fully enforced inside `TileRepository` and is invisible to region/route consumers. + +### Refreshed-anyway docs (within the immediate scope) +The following docs were refreshed during this Update Docs step because they describe the storage layer or its direct boundary, and AZ-484 changed the substantive content even though import graphs alone would not have triggered them: + +- `_docs/02_document/modules/dataaccess_tile_repository.md` — interface signatures (`FindExistingTileAsync` removal, AZ-484 selection rule, 5-col UPSERT key); changed by direct edit (in scope) +- `_docs/02_document/modules/dataaccess_models.md` — `TileEntity` field list (`Source` as `string` + L-001 note, `CapturedAt`); changed by direct edit (in scope) +- `_docs/02_document/modules/dataaccess_migrator.md` — migration count 11 → 13, entries for 012 and 013; changed by direct edit (in scope) +- `_docs/02_document/modules/services_tile_service.md` — `DownloadAndStoreTilesAsync` selection rule + UPSERT semantics, `BuildTileEntity` source stamping, `Common.Enums` dependency; changed by direct edit (in scope) +- `_docs/02_document/modules/tests_integration.md` — `MigrationTests` AZ-484 method list; changed by direct edit (in scope) +- `_docs/02_document/components/03_tile_downloader/description.md` — upstream dependency list (`Common.Enums`), tile-metadata caching strategy now reads "append-by-source per cell"; changed by direct edit (in scope) +- `_docs/02_document/components/02_data_access/description.md` — `ITileRepository` table (FindExistingTileAsync row removed; AZ-484 annotations on Get/Insert), Queries table (UPSERT key now 5-col + source), Caveats (UPSERT semantics, L-001 note, contract pointer); changed by direct edit (in scope) +- `_docs/02_document/data_model.md` — `tiles` table columns (`source`, `captured_at` added; `version` / `maps_version` flagged vestigial), Indexes section (replaced unique key entry, added selection-rule + UPSERT contract notes), Migration History (012, 013); changed by direct edit (in scope) +- `_docs/02_document/module-layout.md` — Common Public API list (`TileSourceConverter` already present; updated DataAccess "Imports from" 5 → 6 sites including `Models/TileEntity`; new Common/Enums subsection with the L-001 guidance); changed by direct edit (in scope) +- `_docs/02_document/architecture.md`, `_docs/02_document/glossary.md`, `_docs/02_document/contracts/data-access/tile-storage.md` — already updated during the AZ-484 implementation batch (see batch_25_cycle1_report.md) + +## Heuristic mode flag +Not used. Direct `Grep` over the C# tree was sufficient — no parser failure, no fall-back to directory-proximity scanning. + +## Summary +- Direct importers in AZ-484 changeset: 7 (already updated as part of the implementation) +- Indirect importers requiring NO doc ripple: 3 (GoogleMapsDownloaderV2, Program.cs, RegionService chain) +- In-scope refresh-anyway docs: 10 (listed above) +- Out-of-scope ripple-only docs: 0 diff --git a/_docs/02_document/tests/performance-tests.md b/_docs/02_document/tests/performance-tests.md index 5a22df1..7259f13 100644 --- a/_docs/02_document/tests/performance-tests.md +++ b/_docs/02_document/tests/performance-tests.md @@ -41,3 +41,12 @@ **Load**: 1 request **Expected**: Route created (with interpolation) within 5s **Pass criterion**: HTTP 200 response within 5000ms; totalPoints > 20 + +## PT-07: GetTilesByRegionAsync Latency Post-AZ-484 (multi-source baseline) + +**Trigger**: TileRepository.GetTilesByRegionAsync exercised via POST /api/satellite/request (200m region, zoom 18) against a tiles table seeded with the pre-AZ-484 data shape (single-source rows backfilled to source='google_maps'). +**Load**: 1 request, repeated 20 times to get a stable distribution. +**Expected**: 95th-percentile latency must not regress more than 10% vs the pre-AZ-484 baseline measured against PT-03 / PT-04. The new 5-column unique index `idx_tiles_unique_location_source` covers the same `(latitude, longitude, tile_zoom, tile_size_meters)` filter columns as the pre-AZ-484 4-column index, so no regression is expected. +**Pass criterion**: p95(GetTilesByRegionAsync) ≤ 1.10 × pre-AZ-484 p95 baseline. +**Source**: AZ-484 NFR (Performance) — `_docs/02_tasks/done/AZ-484_multi_source_tile_storage.md` § Non-Functional Requirements. +**Note**: This NFR is recorded for tracking. Active enforcement (running PT-07 against a real workload and comparing) is deferred to autodev Step 15 (Performance Test) when a baseline run is available. Until then, the integration test `MostRecentAcrossSourcesSelection_AZ484_AC2` provides correctness coverage for the new query shape. diff --git a/_docs/02_document/tests/traceability-matrix.md b/_docs/02_document/tests/traceability-matrix.md index 9df9868..bc1a2dd 100644 --- a/_docs/02_document/tests/traceability-matrix.md +++ b/_docs/02_document/tests/traceability-matrix.md @@ -26,6 +26,13 @@ | S1 | Migrations run on startup | RS-02 | ✓ | | S2 | Queue rejects when full | RS-04, RL-02 | ✓ | | S3 | Failed regions marked failed | RS-03 | ✓ | +| AZ-484 AC-1 | Schema accepts source + captured_at; multi-source rows coexist under 5-col unique index | `MultiSourceInsertCoexistsUnderNewIndex_AZ484_AC1`, `NewUniqueConstraintIncludesSourceColumn_AZ484_AC1` (integration) | ✓ | +| AZ-484 AC-2 | Read returns most-recent across sources | `MostRecentAcrossSourcesSelection_AZ484_AC2` (integration) | ✓ | +| AZ-484 AC-3 | Same-source UPSERT collapses to one row with refreshed captured_at | `SameSourceUpsertReplacesPreviousRow_AZ484_AC3` (integration) | ✓ | +| AZ-484 AC-4 | Migration 013 backfill leaves no orphans (count preserved, source='google_maps', captured_at=created_at) | `BackfillUpdateAssignsGoogleMapsAndCapturedAt_AZ484_AC4` (integration) | ✓ | +| AZ-484 AC-5 | Google Maps download path stamps Source='google_maps' (wire) + CapturedAt UTC | `BuildTileEntity_SetsGoogleMapsSourceAndUtcCapturedAt_AZ484_AC5` (unit) | ✓ | +| AZ-484 AC-6 | Existing region/route flows unchanged post-T1 (200 unit + smoke baseline preserved) | Full unit suite (213 tests) + integration smoke scenarios BT-01..BT-12 | ✓ | +| AZ-484 AC-7 | Vision + contract docs amended (architecture.md, glossary.md, module-layout.md, tile-storage.md frozen v1.0.0) | doc-state AC; verified by `monorepo-document` reviews | ✓ | ## Restrictions → Test Mapping @@ -40,6 +47,13 @@ | Max ZIP 50 MB | RL-01 | ✓ | | No authentication | SEC-01 through SEC-04 (all requests accepted without auth) | ✓ | +## NFRs → Test Mapping + +| NFR | Source | Tests | Coverage | +|-----|--------|-------|----------| +| AZ-484 Perf — `GetTilesByRegionAsync` p95 ≤ 1.10 × pre-AZ-484 baseline | AZ-484 task spec § Non-Functional Requirements | PT-07 (recorded; active perf comparison deferred to Step 15) | ◐ recorded | +| AZ-484 Compatibility — no public HTTP response field added/removed; vestigial `maps_version`/`version` columns preserved (nullable) | AZ-484 task spec § Non-Functional Requirements | Existing integration suite (no API contract change observable); BT-01 / region status responses verify response shape | ✓ | + ## Coverage Summary | Category | Total Tests | ACs Covered | Restrictions Covered | @@ -50,4 +64,5 @@ | Resilience | 6 | 4 | 3 | | Security | 4 | — | 1 | | Resource Limits | 4 | 3 | 4 | -| **Total** | **37** | **22/22 (100%)** | **8/8 (100%)** | +| Cycle 1 — AZ-484 (integration + unit) | 6 | 7/7 | — | +| **Total** | **43** | **29/29 (100%)** | **8/8 (100%)** | diff --git a/_docs/03_implementation/deploy_cycle1_az484.md b/_docs/03_implementation/deploy_cycle1_az484.md new file mode 100644 index 0000000..698f25f --- /dev/null +++ b/_docs/03_implementation/deploy_cycle1_az484.md @@ -0,0 +1,79 @@ +# Deploy Report — Cycle 1 (AZ-484) + +**Date**: 2026-05-11 +**Cycle**: 1 +**Scope**: Multi-source tile storage schema (AZ-484) and supporting documentation/security artifacts. + +## What is shipping + +### Code changes (already committed) + +| Commit | Subject | +|--------|---------| +| `5ba58b6` | `[AZ-484] [AZ-483] Add task spec + tile-storage v1.0.0 contract draft` | +| `687d6bd` | `[AZ-484] Multi-source tile storage: source + captured_at` | +| `e9d6db0` | `[AZ-484] Fix multi-source tile reads: drop Dapper enum handler` | + +### Database migration + +- **`013_AddTileSourceAndCapturedAt.sql`** — runs on first startup of the new image. Transactional (`BEGIN; … COMMIT;`), idempotent against partial replays. Adds `source` (VARCHAR(32) NOT NULL DEFAULT `'google_maps'`) + `captured_at` (TIMESTAMP NOT NULL); backfills both for every pre-existing row; replaces 4-col `idx_tiles_unique_location` with 5-col `idx_tiles_unique_location_source`. + +### Documentation, test-spec, audit artifacts (uncommitted in working tree, scoped to this deploy commit below) + +- `_docs/02_document/data_model.md`, `module-layout.md`, 6 module / component docs — Step 13 (Update Docs). +- `_docs/02_document/ripple_log_cycle1.md` — Step 13 ripple analysis. +- `_docs/02_document/tests/performance-tests.md`, `traceability-matrix.md` — Step 12 (Test-Spec Sync) — added AZ-484 ACs and PT-07. +- `_docs/05_security/` (5 files) — Step 14 (Security Audit) — verdict PASS_WITH_WARNINGS. +- `_docs/06_metrics/perf_2026-05-11_cycle1_az484.md` — Step 15 (Performance Test) — all scenarios Unverified, non-blocking. +- `_docs/_process_leftovers/2026-05-11_perf-pt07-harness.md` — deferred PT-07 harness work. +- `_docs/_autodev_state.md` — autodev pointer. + +## Pre-deploy checks (recap) + +| Gate | Outcome | +|------|---------| +| Step 11 — Run Tests | All unit + integration tests pass against the post-AZ-484 build (213 unit + 5 new AZ-484 integration). | +| Step 14 — Security Audit | **PASS_WITH_WARNINGS** — 0 Critical, 0 High, 5 Medium, 5 Low. AZ-484 itself added zero new findings. Top hardening items recorded in `_docs/05_security/security_report.md`. | +| Step 15 — Performance Test | All scenarios `Unverified` (PT-07 harness deferred to next cycle). Non-blocking per test-run perf-mode rules. | +| `dotnet format whitespace --verify-no-changes` | Passed (part of `scripts/run-tests.sh`). | + +## Migration safety + +- **Idempotent**: re-running migration 013 against an already-migrated database is a no-op (`ADD COLUMN IF NOT EXISTS`, `DROP INDEX IF EXISTS`, `CREATE UNIQUE INDEX IF NOT EXISTS` patterns). +- **Backwards compatible**: pre-AZ-484 application code that reads `tiles` still works — `source` defaults to `'google_maps'`, `captured_at` is backfilled to `created_at`. Vestigial columns `version` / `maps_version` are preserved nullable. +- **No data loss**: the only DDL change is dropping `idx_tiles_unique_location` (replaced by `idx_tiles_unique_location_source`); rows are not touched except by the backfill UPDATE. +- **Lock duration**: ALTER TABLE on a tiles table with O(100K-1M) rows takes a few seconds for ADD COLUMN (metadata-only in PG 11+) and a few seconds for the UPDATE backfill on existing data. Acceptable for a maintenance window-free deploy assuming current row counts (`_docs/02_document/data_model.md` storage estimate: 100K-1M tiles). + +## Rollback plan + +The migration is forward-only by DbUp design (no automatic down migrations). In the unlikely event AZ-484 needs to be reverted in production: + +1. Re-deploy the previous image (`registry.../azaion/satellite-provider:`). The pre-AZ-484 code reads `*` from `tiles` and ignores the new columns — it will function correctly against the migrated schema. +2. The 5-col `idx_tiles_unique_location_source` is a strict superset of the pre-AZ-484 4-col index for the only producer pre-AZ-484 sees (`source = 'google_maps'`); pre-AZ-484 UPSERT on `(lat, lon, tile_zoom, tile_size_meters, version)` would fail conflict detection against the new index — so do **not** roll the schema back, only roll the application back. The schema is forward-compatible with the old code. +3. If a true schema rollback is ever needed, write a follow-up migration `014_RevertAz484.sql` rather than editing `013` in place. + +## Post-deploy verification + +After the new image is deployed and migrations have run: + +1. Hit `GET /api/satellite/region/{any-existing-region-id}` — confirm 200 + status JSON (smoke test that the `tiles` reads still parse with the new entity shape). +2. Hit `GET /api/satellite/tiles/latlon?Latitude=...&Longitude=...&ZoomLevel=18` for a known cell — confirm 200 + tile bytes. +3. Run a single `psql` query: `SELECT count(*) FROM tiles WHERE source IS NULL OR captured_at IS NULL;` — expected `0`. +4. Tail Serilog logs for `Slow GetTilesByRegionAsync` warnings (>500ms) for the first hour. A spike here would be the earliest signal of a perf regression vs. the (unmeasured) pre-AZ-484 baseline. + +## CI/CD path + +The project's `.woodpecker/02-build-push.yml` builds and pushes the image on push to `dev`, `stage`, `main`. The `dev` branch is the standard target per `.cursor/rules/git-workflow.mdc`. Pushing the new commit to `origin/dev` triggers the pipeline; no manual deploy steps are needed beyond the `git push`. + +**Push policy**: per `git-workflow.mdc`, the autodev does NOT push without explicit user request. The next user prompt will offer the push as an explicit option. + +## Security caveats carried into this deploy + +The Security Audit (Step 14) flagged 5 Medium findings that should be tracked as separate hardening tasks before public-network exposure but do NOT block this internal deploy: + +- **S1+S2+I5** — Postgres default credentials in `appsettings.json` + `docker-compose.yml`; `.env` not in `.dockerignore`. +- **S4** — Google Maps API key needs rotation + `.env.example`. +- **D1** — Bump `Microsoft.AspNetCore.OpenApi 8.0.21 → 8.0.25` (CVE-2026-26130 SignalR DoS — not reachable in this app). +- **I3** — Wire `Microsoft.AspNetCore.RateLimiting`. + +None of these are AZ-484 regressions; they are pre-existing items that today's audit surfaced. Tracking them as separate Jira tickets is recommended but not enforced by this gate. diff --git a/_docs/05_security/dependency_scan.md b/_docs/05_security/dependency_scan.md new file mode 100644 index 0000000..e47b00f --- /dev/null +++ b/_docs/05_security/dependency_scan.md @@ -0,0 +1,65 @@ +# Phase 1 — Dependency Scan + +**Date**: 2026-05-11 +**Method**: Manual inventory from `*.csproj` + targeted advisory search (WebSearch against GHSA / NVD / NuGet ReversingLabs). +**Reason for manual mode**: `dotnet list package --vulnerable` is on the project's "do not run from agent" list (AGENTS.md — these commands hang in this environment). + +## Inventory + +| Project | Package | Version | Notes | +|---------|---------|---------|-------| +| Api | Microsoft.AspNetCore.OpenApi | 8.0.21 | ASP.NET Core 8 LTS patch (one behind 8.0.25) | +| Api | Newtonsoft.Json | 13.0.4 | Latest 13.x | +| Api | Serilog.AspNetCore | 8.0.3 | | +| Api | Serilog.Sinks.File | 6.0.0 | | +| Api | SixLabors.ImageSharp | 3.1.11 | | +| Api | Swashbuckle.AspNetCore | 6.6.2 | | +| Common | SixLabors.ImageSharp | 3.1.11 | | +| DataAccess | Dapper | 2.1.35 | | +| DataAccess | Npgsql | 9.0.2 | | +| DataAccess | dbup-postgresql | 6.0.3 | | +| DataAccess | Microsoft.Extensions.Configuration.Abstractions | 9.0.10 | | +| DataAccess | Microsoft.Extensions.Logging.Abstractions | 9.0.10 | | +| TileDownloader | Microsoft.Extensions.Caching.Memory | 9.0.10 | | +| TileDownloader | Microsoft.Extensions.Http | 9.0.10 | | +| TileDownloader | Microsoft.Extensions.Logging.Abstractions | 9.0.10 | | +| TileDownloader | Microsoft.Extensions.Options.ConfigurationExtensions | 9.0.10 | | +| TileDownloader | Newtonsoft.Json | 13.0.4 | | +| Tests | coverlet.collector | 6.0.0 | | +| Tests | FluentAssertions | 8.8.0 | | +| Tests | Microsoft.Extensions.* | 9.0.10 | (multiple) | +| Tests | Microsoft.NET.Test.Sdk | 17.8.0 | NuGet.Frameworks transitive CVE flag — see findings | +| Tests | Moq | 4.20.72 | | +| Tests | xunit | 2.5.3 | | +| Tests | xunit.runner.visualstudio | 2.5.3 | | + +## Findings + +| # | Severity | Package | Version | Advisory | Disposition | +|---|----------|---------|---------|----------|-------------| +| D1 | Medium (production-risk: **Low**, exposure: not reachable) | Microsoft.AspNetCore.OpenApi → ASP.NET Core 8 runtime | 8.0.21 | [CVE-2026-26130](https://github.com/dotnet/aspnetcore/security/advisories/GHSA-4vgm-c2wm-63mw) — SignalR DoS via unbounded buffer | **Not exploitable in this app**: codebase grep for `SignalR\|MapHub\|UseSignalR\|HubConnection` returns zero hits. Runtime patch still recommended. Upgrade `Microsoft.AspNetCore.OpenApi` to `8.0.25` (or current 8.0.x patch) and redeploy on a runtime ≥ 8.0.25 to remove the vulnerable code paths from the deployed image. | +| D2 | Low (test-only) | Microsoft.NET.Test.Sdk | 17.8.0 | [CVE-2022-30184](https://github.com/microsoft/vstest/issues/4409) via transitive `NuGet.Frameworks <6.2.1` — information disclosure (CVSS 5.5) | **Not exploitable in production**: package is `IsTestProject=true` only; never shipped. Upgrade to `Microsoft.NET.Test.Sdk` ≥ 17.9.0 (which dropped the `NuGet.Frameworks` dependency entirely) the next time the test project's deps are touched. | + +## Cross-version sanity (per `coderule.mdc`: keep dependency versions consistent) + +- `Microsoft.Extensions.*` is uniformly **9.0.10** across DataAccess, TileDownloader, Tests, RegionProcessing, RouteManagement — consistent. ✓ +- `Newtonsoft.Json` is **13.0.4** in both Api and TileDownloader — consistent. ✓ +- `SixLabors.ImageSharp` is **3.1.11** in both Api and Common — consistent. ✓ +- ASP.NET Core meta-package level is at **8.0.21** while extensions are at 9.0.10. The 9.x extensions ship a forward-compatible netstandard2.0 surface and load fine on the .NET 8 runtime — no functional issue, but worth flagging as a minor consistency drift for whoever next bumps the framework. + +## Items checked clean + +- SixLabors.ImageSharp 3.1.11 — newer than the patched 3.1.7 / 3.1.5 lines (CVE-2024-41131, CVE-2025-27598). No outstanding GHSA against 3.1.11 itself. +- Newtonsoft.Json 13.0.4 — past CVE-2024-21907 fix line (13.0.1). +- Npgsql 9.0.2 — outside the 4.x / 5.x / 6.x / 7.x / 8.x ranges affected by CVE-2024-32655 (SQL injection via protocol message size overflow). 9.0.x line was never affected. +- Dapper 2.1.35 — only "advisory" hit was a dependency-check false positive for SQLite CVE-2017-10989; not a Dapper issue. +- Swashbuckle.AspNetCore 6.6.2 — no published GHSA / CVE. +- Serilog.AspNetCore 8.0.3 — no published GHSA / CVE. +- dbup-postgresql 6.0.3 — no published GHSA / CVE. + +## Self-verification + +- [x] All package manifests scanned (8 csproj files) +- [x] Each finding has a CVE ID or advisory reference +- [x] Upgrade paths identified for every Medium/Low finding +- [x] No Critical or High finding remains open after exploitability triage diff --git a/_docs/05_security/infrastructure_review.md b/_docs/05_security/infrastructure_review.md new file mode 100644 index 0000000..8e2c2a4 --- /dev/null +++ b/_docs/05_security/infrastructure_review.md @@ -0,0 +1,87 @@ +# Phase 4 — Configuration & Infrastructure Review + +**Date**: 2026-05-11 +**Scope**: `Dockerfile` (API + integration tests), `docker-compose.yml`, `docker-compose.tests.yml`, `.dockerignore`, `.woodpecker/01-test.yml`, `.woodpecker/02-build-push.yml`, `appsettings*.json`, `.env` handling. + +## Findings + +### I1 — Dockerfile runs as root (Low) + +- **Location**: `SatelliteProvider.Api/Dockerfile` (no `USER` directive) +- **Description**: The final stage of the API image inherits root from `mcr.microsoft.com/dotnet/aspnet:8.0` (current Microsoft images default to root unless overridden). Any process compromise — even a low-impact one — has uid-0 inside the container. +- **Impact**: Container escape primitives (e.g., kernel CVE, sloppy bind-mount of `/var/run/docker.sock`) become host-root rather than host-uid-1000. The `02-build-push.yml` step itself bind-mounts `/var/run/docker.sock` into the build container — that's a separate concern (build host, not runtime), but it underscores why "least privilege at runtime" matters even on a single-tenant box. +- **Remediation**: Add to the `final` stage: + ```dockerfile + RUN adduser --disabled-password --gecos "" --uid 10001 satellite && \ + chown -R satellite:satellite /app + USER satellite + ``` + Also verify `./tiles`, `./ready`, `./logs` host volumes are writable by uid 10001 in deployment manifests. + +### I2 — No security headers middleware (Low) + +- **Location**: `SatelliteProvider.Api/Program.cs` (no `app.UseSecurityHeaders()` / `app.Use(headers …)` block) +- **Description**: API responses do not set `X-Content-Type-Options: nosniff`, `Referrer-Policy: no-referrer`, `X-Frame-Options: DENY`, or HSTS (`Strict-Transport-Security`) — only `app.UseHttpsRedirection()` is wired. For a JSON-only API this is **low** impact (no browser is the primary client), but the missing `Cache-Control` defaults can let proxies cache 5xx responses. +- **Impact**: Limited — JSON-only responses, no cookies, no browser session. The Swagger UI (Development-only) does render HTML; a permissive default there is more of a hygiene issue than a real risk. +- **Remediation**: Add a tiny middleware to set the standard hardening headers, OR install `NWebsec.AspNetCore.Middleware` and wire `app.UseHsts()` + the `nosniff` / `frame-options` defaults. Cheap, no behavioural change. + +### I3 — No rate limiting on any HTTP endpoint (Medium) + +- **Location**: `SatelliteProvider.Api/Program.cs` (no `app.UseRateLimiter()`, no `AddRateLimiter()`) +- **Description**: There is internal concurrency control on outbound Google Maps calls (`SemaphoreSlim`, `MaxConcurrentDownloads`), but no inbound rate limiting. An attacker can: + - Submit `N` `POST /api/satellite/request` calls in a tight loop, filling the bounded `IRegionRequestQueue` (capacity 1000) and DoS-ing the background processor. + - Submit `N` `GET /api/satellite/tiles/latlon` calls with novel lat/lon pairs, forcing the upstream Google Maps quota to drain. +- **Impact**: Service-degradation DoS. Combined with finding A01-caveat (no auth), the only protection is the network boundary. +- **Remediation**: Wire `Microsoft.AspNetCore.RateLimiting` (built into .NET 8 — no new package). Conservative starting point: + ```csharp + builder.Services.AddRateLimiter(options => + { + options.GlobalLimiter = PartitionedRateLimiter.Create(ctx => + RateLimitPartition.GetFixedWindowLimiter( + partitionKey: ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown", + factory: _ => new FixedWindowRateLimiterOptions { PermitLimit = 60, Window = TimeSpan.FromMinutes(1) })); + }); + app.UseRateLimiter(); + ``` + Tune per-endpoint after observing baseline production load. + +### I4 — No security-event logs / alerting (Low) + +- **Location**: Logging strategy across `Program.cs`, `GlobalExceptionHandler`, `CorsConfigurationValidator` +- **Description**: Operational logs are well-structured (Serilog → file rotation; correlationId propagation), but there are no log entries for what would be security-relevant events: validation failures (BadRequest stream), repeated 4xx from a single IP, malformed input bursts, or migration failures. The migration failure path does throw and crash startup (good signal), but this leaves no trail in the file logs. +- **Impact**: No way to detect abuse of the unauthenticated endpoints from logs alone. For an internal-only deployment this is acceptable; if the API ever moves toward a less-trusted network, post-deploy log-mining will not be able to reconstruct attack patterns. +- **Remediation**: Defer until/unless the trust boundary changes. When required: add a structured log line for each 400/404 (with `Method`, `Path`, `RemoteIp`, `correlationId`) and a counter for "validation failures per minute per IP". + +### I5 — `.env` is NOT in `.dockerignore` (Medium) + +- **Location**: `.dockerignore` (line by line review — no `.env` entry); `SatelliteProvider.Api/Dockerfile:15` (`COPY . .`) +- **Description**: The Dockerfile's `COPY . .` step copies the entire build context into `/src`. The build context starts at the repo root, where `.env` lives. `.env` IS in `.gitignore` so the dev-only Google Maps key never reaches the git repo, but it WILL be baked into the build-stage image layer (and into the final image, since `final` does `COPY --from=publish /app/publish .` — only `/app/publish` survives, but the `build` stage retains `.env` and is reachable if anyone introspects an intermediate layer). +- **Impact**: + - Anyone with read access to the registry can `docker pull ` (if exported) and recover the API key from the layer. + - Even just the `final` image: BuildKit cache mounts and any future Dockerfile change that does `COPY . /app` instead of `COPY --from=publish` would silently include the file. +- **Remediation**: Add `.env` to `.dockerignore`: + ``` + .env + .env.* + !.env.example + ``` + This is a one-line fix and complements finding S4. + +### I6 — `docker-compose.yml` exposes Postgres on `0.0.0.0:5432` (Medium — duplicate of S2; restated here for infra-domain completeness) + +- **Location**: `docker-compose.yml:9-10` +- **Description / Remediation**: See S2. + +## Items checked clean + +- **Secrets management in CI**: `.woodpecker/02-build-push.yml` uses `from_secret: registry_host / registry_user / registry_token` — no plaintext credentials. The `docker login` step pipes the token via `--password-stdin`, which avoids leaking the token via process list. ✓ +- **Image attribution**: build step labels images with `org.opencontainers.image.revision`, `…created`, `…source` — good provenance hygiene. ✓ +- **Healthcheck on Postgres**: `pg_isready -U postgres` configured. (Note: relies on the weak default user from S2.) +- **Log volume layout**: `./logs:/app/logs` mounted; not exposed via the API. ✓ +- **Test runner isolation**: `docker-compose.tests.yml` extends the API service (good — same image) but uses `restart: "no"` so a flapping integration test doesn't loop and amplify load. + +## Self-verification + +- [x] All Dockerfiles reviewed (Api + IntegrationTests) +- [x] All CI/CD configs reviewed (`.woodpecker/01-test.yml`, `02-build-push.yml`) +- [x] All env / config files reviewed (`appsettings*.json`, `.env`, `docker-compose*.yml`) diff --git a/_docs/05_security/owasp_review.md b/_docs/05_security/owasp_review.md new file mode 100644 index 0000000..0f9a151 --- /dev/null +++ b/_docs/05_security/owasp_review.md @@ -0,0 +1,40 @@ +# Phase 3 — OWASP Top 10:2025 Review + +**Date**: 2026-05-11 +**OWASP version**: [OWASP Top 10:2025](https://owasp.org/Top10/2025/en/) (verified at audit start) +**Project context**: Self-hosted .NET 8 backend service. Documented as an "internal/trusted network service — no auth layer" (`_docs/02_document/architecture.md` §7). Deployed via Docker behind another network boundary (per `_docs/02_document/deployment/`). The audit is scoped to the codebase as it stands; categories whose findings depend on a missing trust-boundary control are flagged accordingly. + +| # | Category | Status | Findings | Notes | +|---|----------|--------|----------|-------| +| A01 | Broken Access Control | **N/A** (with caveat) | — | The service intentionally exposes ALL endpoints without authentication or authorization — documented design (architecture.md §7). No IDOR analysis applies because there is no user concept. **Caveat**: this is only safe if the deployment puts the API behind a network-level gatekeeper (VPN, mTLS, internal-only LB). If the deploy ever moves to a public network, this category becomes the #1 risk and EVERY endpoint becomes an unauthenticated execution surface. | +| A02 | Security Misconfiguration | **FAIL** | S1, S2, I1, I2 | Default Postgres credentials in both `appsettings.json` and `docker-compose.yml`; Postgres port bound to `0.0.0.0`; container runs as root; no security headers middleware. | +| A03 | Software Supply Chain Failures | **PASS_WITH_WARNINGS** | D1, D2 | Two known transitive CVEs (D1 — ASP.NET Core 8.0.21 SignalR DoS, not exploitable here; D2 — `Microsoft.NET.Test.Sdk` 17.8.0 → `NuGet.Frameworks` info disclosure, test-only). No use of unsigned NuGet packages; no auto-update of dependencies in production. | +| A04 | Cryptographic Failures | **N/A** | — | No password storage (no users), no encryption at rest, no in-app crypto. The Google Maps integration uses HTTPS (default Npgsql/HttpClient stacks). At-rest tile storage is plain JPEG by design — these are public satellite images, not confidential data. | +| A05 | Injection | **PASS** | — | All Dapper queries use parameter objects (`new { Id = id }` etc.); no string-built or interpolated user input flows into SQL. No `Process.Start`, no shell exec, no `eval`. JSON deserialization uses `System.Text.Json` defaults (no type-name handling). XSS / template injection N/A — JSON-only API. | +| A06 | Insecure Design | **FAIL** | S3, I3 | No rate limiting on any endpoint despite the existence of an outbound rate-limited dependency (Google Maps). Latitude / longitude inputs are not range-validated at the API boundary (S3). No quota / throttling on region-request creation, which can multiply outbound calls and disk writes. | +| A07 | Authentication Failures | **N/A** (with caveat) | — | Same caveat as A01 — there is no authentication system to fail. | +| A08 | Software or Data Integrity Failures | **PASS** | — | DbUp migrations are idempotent and tracked in `schemaversions`; rollback is forward-only by design. No auto-update path. CI artifacts go through `.woodpecker/02-build-push.yml` with `from_secret: registry_token` (not in plaintext). No unsigned external scripts executed at build/deploy. | +| A09 | Security Logging and Alerting Failures | **PASS_WITH_WARNINGS** | I4 | Serilog writes structured logs with file rotation; `GlobalExceptionHandler` correlates server logs to client responses via `correlationId` (good). However: no security-event logging (e.g., bad-input bursts, repeated 4xx from same source) and no alerting on log patterns. Acceptable for an internal service; would need attention if exposed publicly. | +| A10 | Mishandling of Exceptional Conditions | **PASS** | — | `GlobalExceptionHandler` returns RFC 7807 ProblemDetails with a generic body and a correlationId — no exception text leaks to clients. `GlobalExceptionHandlerTests.cs` includes a positive control that confirms a "leakySecret"-shaped exception message is NOT echoed. | + +## Cross-reference to Phase 1 / Phase 2 findings + +| OWASP Cat | Tied finding | Severity | Source phase | +|-----------|--------------|----------|--------------| +| A02 | S1 — default password in appsettings.json | Medium | Phase 2 | +| A02 | S2 — weak Postgres creds + 0.0.0.0 binding in compose | Medium | Phase 2 | +| A02 | I1 — Dockerfile runs as root | Low | Phase 4 (next) | +| A02 | I2 — no security headers middleware | Low | Phase 4 (next) | +| A03 | D1 — CVE-2026-26130 in ASP.NET Core 8.0.21 (SignalR; not reachable) | Medium (paper) / Low (real) | Phase 1 | +| A03 | D2 — CVE-2022-30184 transitive via test SDK | Low (test-only) | Phase 1 | +| A06 | S3 — lat/lon not range-validated at API boundary | Low | Phase 2 | +| A06 | I3 — no rate limiting on any endpoint | Medium | Phase 4 (next) | +| A06 | S4 — Google Maps API key handling (no .env.example, no rotation hygiene) | Medium | Phase 2 | +| A09 | I4 — no security-event logs, no alerting | Low | Phase 4 (next) | + +## Self-verification + +- [x] All current OWASP Top 10:2025 categories assessed +- [x] Each FAIL has at least one specific finding with evidence +- [x] N/A categories have justification + caveat +- [x] No `security_approach.md` exists in `_docs/00_problem/` to cross-reference (project has not declared explicit security requirements; this audit treats the architecture-vision statement "internal/trusted network service" as the de-facto requirement) diff --git a/_docs/05_security/security_report.md b/_docs/05_security/security_report.md new file mode 100644 index 0000000..0685cc1 --- /dev/null +++ b/_docs/05_security/security_report.md @@ -0,0 +1,120 @@ +# Security Audit Report + +**Date**: 2026-05-11 +**Scope**: Satellite Provider — full repository (Api, Common, DataAccess, Services.*, Tests, infra) +**Trigger**: `/autodev` Step 14 (Security Audit) — feature cycle 1, post-AZ-484 +**Verdict**: **PASS_WITH_WARNINGS** + +## Summary + +| Severity | Count | +|----------|-------| +| Critical | 0 | +| High | 0 | +| Medium | 5 | +| Low | 5 | + +No Critical or High findings. The verdict is `PASS_WITH_WARNINGS` driven by 5 Medium findings, all of which are well-understood configuration / hardening gaps rather than exploitable vulnerabilities in the application logic itself. **AZ-484 (the cycle's only feature change) introduced zero new findings** — it is a pure data-layer change with no auth surface, no untrusted-input handling, and no new external dependencies. + +## OWASP Top 10:2025 Assessment + +| Category | Status | Findings | +|----------|--------|----------| +| A01 Broken Access Control | N/A (with caveat) | — | +| A02 Security Misconfiguration | FAIL | S1, S2/I6, I1, I2 | +| A03 Software Supply Chain Failures | PASS_WITH_WARNINGS | D1, D2 | +| A04 Cryptographic Failures | N/A | — | +| A05 Injection | PASS | — | +| A06 Insecure Design | FAIL | S3, S4, I3 | +| A07 Authentication Failures | N/A (with caveat) | — | +| A08 Software or Data Integrity Failures | PASS | — | +| A09 Security Logging and Alerting Failures | PASS_WITH_WARNINGS | I4 | +| A10 Mishandling of Exceptional Conditions | PASS | — | + +The two **N/A (with caveat)** entries (A01, A07) reflect the documented architectural choice (`architecture.md` §7) that this is an internal/trusted-network service. **The audit does not endorse that choice — it merely notes that the choice has been made deliberately.** If the deployment trust boundary ever changes, A01 and A07 immediately become FAIL and every endpoint becomes an unauthenticated surface; that decision must be re-examined before any internet-facing exposure. + +## Findings + +| # | Severity | Category | Location | Title | +|----|----------|------------------------|---------------------------------------------------------|--------------------------------------------------------------------------------------| +| S1 | Medium | A02 — Misconfiguration | `SatelliteProvider.Api/appsettings.json:24` | Default Postgres password (`postgres/postgres`) committed in `appsettings.json` | +| S2 | Medium | A02 — Misconfiguration | `docker-compose.yml:6-7,30` | Weak Postgres credentials in compose (mirrors S1) | +| S3 | Low | A06 — Insecure Design | `SatelliteProvider.Api/Program.cs:169,207,237` | Latitude/longitude inputs not range-validated at API boundary | +| S4 | Medium | A06 — Insecure Design | `.env` (workspace root) | Apparent real Google Maps API key on developer filesystem; no `.env.example` | +| D1 | Medium | A03 — Supply Chain | `SatelliteProvider.Api.csproj` — `Microsoft.AspNetCore.OpenApi 8.0.21` | CVE-2026-26130 SignalR DoS (not reachable in this app — codebase has zero SignalR use) | +| D2 | Low | A03 — Supply Chain | `SatelliteProvider.Tests.csproj` — `Microsoft.NET.Test.Sdk 17.8.0` | CVE-2022-30184 transitive via `NuGet.Frameworks <6.2.1` (test-only) | +| I1 | Low | A02 — Misconfiguration | `SatelliteProvider.Api/Dockerfile` | Container runs as root (no `USER` directive) | +| I2 | Low | A02 — Misconfiguration | `SatelliteProvider.Api/Program.cs` | No security headers middleware | +| I3 | Medium | A06 — Insecure Design | `SatelliteProvider.Api/Program.cs` | No inbound rate limiting on any HTTP endpoint | +| I4 | Low | A09 — Logging | `SatelliteProvider.Api/*` (logging strategy) | No security-event logs / alerting | +| I5 | Medium | A02 — Misconfiguration | `.dockerignore` + `Dockerfile:15` (`COPY . .`) | `.env` not in `.dockerignore` — risk of API key being baked into image layers | + +I6 in the infra report is a duplicate of S2 (same root cause) and is not double-counted in the summary. + +### Finding Details + +Full evidence and remediation for every finding lives in the per-phase reports. The detail tables there are the source of truth — this top-level report intentionally avoids restating multi-paragraph remediation steps. + +- **Phase 1**: `_docs/05_security/dependency_scan.md` — D1, D2, full dependency inventory + cross-version sanity check +- **Phase 2**: `_docs/05_security/static_analysis.md` — S1, S2, S3, S4, plus the categories that were checked clean (SQL injection, command injection, deserialization, path traversal, log leakage, exception leakage) +- **Phase 3**: `_docs/05_security/owasp_review.md` — OWASP Top 10:2025 per-category assessment + cross-reference table +- **Phase 4**: `_docs/05_security/infrastructure_review.md` — I1, I2, I3, I4, I5, I6, plus the items checked clean (CI secrets handling, image attribution labels) + +## Dependency Vulnerabilities + +| Package | CVE | Severity | Reachable? | Fix Version | +|---------|-----|----------|------------|-------------| +| Microsoft.AspNetCore.OpenApi (→ ASP.NET Core 8 runtime) | CVE-2026-26130 | High (paper) / Low (this app) | **No** — codebase has zero SignalR use | 8.0.25 | +| Microsoft.NET.Test.Sdk → NuGet.Frameworks | CVE-2022-30184 | Medium (paper) / Low (this app) | Test project only — never shipped | Microsoft.NET.Test.Sdk 17.9.0+ | + +All other dependencies (`Newtonsoft.Json 13.0.4`, `SixLabors.ImageSharp 3.1.11`, `Npgsql 9.0.2`, `Dapper 2.1.35`, `Swashbuckle.AspNetCore 6.6.2`, `Serilog.AspNetCore 8.0.3`, `dbup-postgresql 6.0.3`, `Microsoft.Extensions.* 9.0.10`) are at or beyond the patched line for every CVE I could find. + +## AZ-484 Cycle-Specific Verdict + +The reason this audit was triggered (the AZ-484 multi-source tile storage cycle) is independently **clean**: + +- Migration 013 is transactional and idempotent — no data loss / data integrity finding. +- `TileSourceConverter` enforces a closed value space at the language layer; `TileEntity.Source` is `string` only as a Dapper-bug workaround documented in `_docs/LESSONS.md` L-001. +- `TileRepository` queries continue to use parameterised Dapper — no new SQL injection surface. +- No new external dependencies, no new endpoints, no new untrusted-input flows. +- All findings in this report predate AZ-484 and are unchanged by it. + +## Recommendations + +### Immediate (Critical/High) + +None — there are no Critical or High findings. The audit does not block the next deploy on its own merit. + +### Short-term (Medium — pick before next public-network exposure or any post-deploy hardening pass) + +1. **S1 + S2 + I5** — De-default DB credentials and stop shipping the .env into the build context. One coordinated change: + - Remove `ConnectionStrings:DefaultConnection` from `appsettings.json` (rely on env-var via the existing throw on null). + - Add `POSTGRES_USER` / `POSTGRES_PASSWORD` to a tracked `.env.example` and source them from a dev `.env`; bind `5432` to `127.0.0.1`. + - Append `.env` and `.env.*` (with `!.env.example` exception) to `.dockerignore`. +2. **S4** — Rotate the Google Maps API key out-of-band, add `.env.example`, add Google Cloud key restrictions (HTTP referrer or IP allowlist + per-API quotas). The audit deliberately did not echo the key value into any artifact. +3. **D1** — Bump `Microsoft.AspNetCore.OpenApi` from `8.0.21` to the current 8.0.x patch (≥ 8.0.25) and rebuild the deployed image so the vulnerable SignalR code paths are physically absent. +4. **I3** — Wire `Microsoft.AspNetCore.RateLimiting` (built into .NET 8 — no new package). Conservative starting threshold in the per-phase report. + +### Long-term (Low — hardening backlog) + +1. **I1** — Add a non-root `USER` to the API Dockerfile. +2. **I2** — Add a tiny security-headers middleware (or pull `NWebsec.AspNetCore.Middleware`). +3. **S3** — Add explicit lat/lon range guards at the API boundary (matches the existing `SizeMeters` 100-10000 pattern). +4. **D2** — Bump `Microsoft.NET.Test.Sdk` to ≥ 17.9.0 next time the test project's deps are touched. +5. **I4** — Defer until the trust boundary changes; if/when the API moves toward a less-trusted network, add structured 4xx logging per IP + a basic alerting rule. + +## Verdict Logic + +- No Critical or High findings → **not FAIL** +- 5 Medium + 5 Low findings exist → **not PASS** +- Therefore: **PASS_WITH_WARNINGS** + +This satisfies the autodev gate to proceed to Step 15 (Performance Test). The recommendations above should be tracked as separate Jira tasks under a hardening epic before the first non-internal deployment. + +## Self-verification + +- [x] All findings from Phases 1–4 included +- [x] No duplicate findings (I6 explicitly noted as a duplicate of S2 and not double-counted) +- [x] Every finding has remediation guidance (in per-phase reports) +- [x] Verdict matches severity logic (no Critical/High → not FAIL; >0 findings → not PASS) +- [x] No real secret values printed in any audit artifact (S4 described without echoing the API key) diff --git a/_docs/05_security/static_analysis.md b/_docs/05_security/static_analysis.md new file mode 100644 index 0000000..0de1cbc --- /dev/null +++ b/_docs/05_security/static_analysis.md @@ -0,0 +1,90 @@ +# Phase 2 — Static Analysis (SAST) + +**Date**: 2026-05-11 +**Scope**: All `*.cs` files in production projects (Api, Common, DataAccess, Services.*) plus Tests for false-positive triage. Configuration files (`appsettings*.json`, `docker-compose*.yml`, `Dockerfile`, `.env`). +**Method**: Pattern-based grep + targeted file review. + +## Patterns checked + +| Category | Pattern(s) | Verdict | +|----------|-----------|---------| +| SQL injection | `$"SELECT…"`, `+ "WHERE"`, raw `CommandText`, manual SQL string assembly | **Clean** | +| Command/process injection | `Process.Start`, `ProcessStartInfo`, `cmd.exe`, `/bin/sh`, `UseShellExecute`, `eval`-equivalent | **Clean** | +| XSS | unsanitized user input flowed to HTML or `Response.Write` | **N/A** — JSON-only API, no HTML rendering | +| Template injection | Razor / scriban / handlebars on user input | **N/A** — none used | +| Hardcoded credentials | `password = "…"`, `secret = "…"`, `token = "…"`, `apikey = "…"` in source | See findings S1, S2 | +| Weak crypto | MD5/SHA1 for passwords, `RNGCryptoServiceProvider` (deprecated), hardcoded keys | **N/A** — no password storage, no crypto code in app | +| Insecure deserialization | `BinaryFormatter`, `pickle`, untrusted JSON with type-name handling | **Clean** — `System.Text.Json` with default settings; `Newtonsoft.Json` 13.0.4 used only for outbound serialization to Google session-creation endpoint (line `GoogleMapsDownloaderV2.cs`), no deserialization of untrusted inbound JSON | +| Path traversal | user input flowed into `File.Open`, `Path.Combine` | **Clean** — file paths are computed server-side from validated tile coordinates; no user-supplied path component reaches the filesystem | +| Sensitive data in logs | passwords, API keys, tokens, PII in log statements | **Clean** — `GlobalExceptionHandler.cs` logs only `Method`, `Path`, `correlationId`; client gets a generic 500 + correlationId. `CorsConfigurationValidator` warning (`PermissiveDefaultWarning`) does not include secrets. There is a deliberate test fixture `GlobalExceptionHandlerTests.cs:23` that uses `"Connection string Host=secret-db;Password=hunter2 failed at line 42"` to verify the handler does NOT echo exception messages back — this is a positive control, not a finding | +| Verbose error responses | stack traces or internal details returned to clients | **Clean** — `GlobalExceptionHandler` returns RFC 7807 ProblemDetails with `Detail = "An unexpected error occurred. Use the correlationId to look up the server log entry."` | +| Input validation | numeric ranges, geo coordinates, enum-like strings | See finding S3 | + +## Findings + +### S1 — Default DB password committed in `appsettings.json` (Medium) + +- **Location**: `SatelliteProvider.Api/appsettings.json:24` +- **Vulnerable code**: + ```json + "DefaultConnection": "Host=localhost;Database=satelliteprovider;Username=postgres;Password=postgres" + ``` +- **Description**: The default (non-Development) appsettings file ships with a weak, well-known password (`postgres/postgres`). In production this string is overridden by `ConnectionStrings__DefaultConnection` in `docker-compose.yml`/env, but the file itself becomes the fallback if env-var injection ever fails or is misconfigured (silent connect-as-default behaviour). +- **Impact**: If a deployment misconfiguration drops the env override, the app silently falls back to attempting `postgres:postgres@localhost`. On a developer workstation this connects to the local Postgres container with full superuser; in production it would fail loudly only if the prod DB has different creds. Combined with finding S2 below (matching weak creds in compose file), this normalises a credential pattern that real production deployments may inherit. +- **Remediation**: + - Replace the default value with a deliberately-invalid placeholder such as `Host=__set-via-env__;Database=__;Username=__;Password=__` so a misconfiguration fails fast at startup instead of silently falling through. + - OR remove the `ConnectionStrings:DefaultConnection` key from `appsettings.json` entirely and require the env var; `Program.cs` line 23–24 already throws when missing — keep that behaviour. + +### S2 — Weak Postgres credentials in `docker-compose.yml` (Medium, dev-only as written) + +- **Location**: `docker-compose.yml:6-7, 30` +- **Vulnerable code**: + ```yaml + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + … + - ConnectionStrings__DefaultConnection=Host=postgres;Port=5432;Database=satelliteprovider;Username=postgres;Password=postgres + ``` +- **Description**: Same `postgres/postgres` credentials as S1. The compose file is labelled `Development` (`ASPNETCORE_ENVIRONMENT=Development`), so this is contained — but the file is the only compose artifact in the repo, which means anyone running `docker-compose up` on a network-reachable host immediately exposes a Postgres-with-default-creds. +- **Impact**: Postgres on `0.0.0.0:5432` (port `"5432:5432"` mapping) with `postgres/postgres` is one of the most-scanned credential pairs on the public internet. If a developer runs this on a non-laptop host (cloud VM, shared lab, etc.) the DB is trivially compromised within minutes. +- **Remediation**: + - Bind `5432` to `127.0.0.1:5432` rather than `0.0.0.0:5432` so the host firewall isn't the only protection. (Replace `"5432:5432"` with `"127.0.0.1:5432:5432"`.) + - Source `POSTGRES_USER` / `POSTGRES_PASSWORD` from the same `.env` file that already supplies `GOOGLE_MAPS_API_KEY` (line 31 already shows the pattern). Provide an `.env.example` with placeholder values and document the required vars in the README. + - The deploy/observability docs at `_docs/02_document/deployment/` already describe a secret-manager strategy for staging/prod — fold the same pattern into the dev compose. + +### S3 — Latitude / longitude inputs not range-validated at the API boundary (Low) + +- **Locations**: + - `SatelliteProvider.Api/Program.cs:169` — `GetTileByLatLon([FromQuery] double Latitude, [FromQuery] double Longitude, [FromQuery] int ZoomLevel, …)` + - `SatelliteProvider.Api/Program.cs:207` — `RequestRegion` validates `SizeMeters` only; `request.Latitude` / `request.Longitude` are unchecked + - `SatelliteProvider.Api/Program.cs:237` — `CreateRoute` delegates to `RouteService` which validates names but does not range-check waypoint coordinates +- **Description**: `Latitude`, `Longitude`, and (for region requests) the implicit `MaxRoutePointSpacingMeters` boundary are accepted without enforcing valid geographic ranges (`-90 ≤ lat ≤ 90`, `-180 ≤ lon ≤ 180`). `ZoomLevel` IS validated downstream by `GoogleMapsDownloaderV2` against `MapConfig.AllowedZoomLevels` — so it is fine. +- **Impact**: + - Garbage inputs (e.g. `lat=999`) propagate through `GeoUtils.WorldToTilePos` and the slippy-map math, eventually producing nonsensical tile coordinates that are persisted to `tiles` and `regions`. This is a **data-quality** issue, not a code-execution issue. + - No DoS amplification: every tile-download endpoint already enforces zoom against `AllowedZoomLevels`, so an attacker cannot use lat/lon abuse to multiply outbound Google Maps traffic beyond what zoom already bounds. +- **Remediation**: Add explicit guard clauses at the API boundary (matches the existing `SizeMeters` 100-10000 pattern): + ```csharp + if (Latitude < -90 || Latitude > 90) return Results.BadRequest(new { error = "Latitude must be between -90 and 90" }); + if (Longitude < -180 || Longitude > 180) return Results.BadRequest(new { error = "Longitude must be between -180 and 180" }); + ``` + Apply uniformly to `GetTileByLatLon`, `RequestRegion`, and to each waypoint inside `CreateRoute`. + +### S4 — `.env` file on developer filesystem contains an apparently real Google Maps API key (Medium — exposure depends on key reach) + +- **Location**: `.env` (workspace root, **not** tracked — confirmed via `git ls-files` and `.gitignore:10`) +- **Description**: The local `.env` contains a 39-character `AIzaSy…` value matching the Google Maps API key format. The file is correctly excluded from git (line 10 of `.gitignore`) and `git log -- .env` returns no history, so the key was never committed to this repository. +- **Impact**: No repository exposure. **However**: + - If the same key is shared across developers via Slack / email / other repos, it has likely already leaked elsewhere. + - There is no `.env.example` template in the repo, which means new contributors typically request the real key via insecure channels rather than generating a fresh one. + - The key has no per-call attribution; abuse cannot be traced back to a specific developer. +- **Remediation**: + - **Rotate the key in the Google Cloud console** (out of scope for this audit — the key value is intentionally not echoed into this report). + - Add `.env.example` to the repo with `GOOGLE_MAPS_API_KEY=replace-with-your-own-key-from-cloud-console` and reference it in the README setup section. + - Configure Google Cloud key restrictions: HTTP referrer allowlist (for browser keys) or IP allowlist (for server keys), and per-API quotas. Optional: per-developer keys. + +## Self-verification + +- [x] All production source directories scanned (Api, Common, DataAccess, Services.TileDownloader, Services.RegionProcessing, Services.RouteManagement) +- [x] Each finding has file path and line number +- [x] False positives from test files explicitly distinguished (`GlobalExceptionHandlerTests.cs:23` "leakySecret" is a positive control) +- [x] No real secret values printed in this report (S4 is described without echoing the key) diff --git a/_docs/06_metrics/perf_2026-05-11_cycle1_az484.md b/_docs/06_metrics/perf_2026-05-11_cycle1_az484.md new file mode 100644 index 0000000..251ec6b --- /dev/null +++ b/_docs/06_metrics/perf_2026-05-11_cycle1_az484.md @@ -0,0 +1,52 @@ +# Perf Run — Cycle 1 (post-AZ-484) + +**Date**: 2026-05-11 +**Mode**: `test-run` perf mode +**Runner detected**: `scripts/run-performance-tests.sh` (PT-01..PT-06) +**Target**: `API_URL=http://localhost:18980` (would require local docker stack) +**Status**: **NOT EXECUTED** — see "Run decision" below + +## Run decision + +The perf gate did not execute scenarios in this cycle. The choice was made at the autodev Step 15 user-prompt with full context: + +- AZ-484 changed the read hot path (`GetTilesByRegionAsync` now uses `DISTINCT ON`; `GetByTileCoordinatesAsync` uses the new most-recent-across-sources tie-break). +- The cycle-specific NFR `PT-07` (recorded in `_docs/02_document/tests/performance-tests.md` and the traceability matrix during Step 12) requires `p95 ≤ 1.10 × pre-AZ-484 baseline`. The runner script `scripts/run-performance-tests.sh` does **not yet have a PT-07 implementation** — adding the baseline-capture / comparison harness is its own work item. +- Running PT-01..PT-06 requires bringing up `docker-compose` on the developer host. Per `meta-rule.mdc` execution-safety guidance, Docker / long-running perf operations need explicit user opt-in, which was not granted on this turn. + +## Scenario classification + +Per the test-run perf-mode rules, every scenario without measured data + threshold comparison is recorded as **Unverified**. Unverified is **not blocking**: the gate does not fail; it just surfaces the coverage gap. + +| Scenario | Threshold | Result | +|----------|-----------|--------| +| PT-01 Tile Download Latency (cold) | ≤ 30000 ms | **Unverified** | +| PT-02 Cached Tile Retrieval Latency | ≤ 500 ms | **Unverified** | +| PT-03 Region Processing 200m / z18 | ≤ 60000 ms (end-to-end) | **Unverified** | +| PT-04 Region Processing 500m / z18 + stitch | ≤ 120000 ms | **Unverified** | +| PT-05 5 Concurrent Regions | all complete in ≤ 300000 ms | **Unverified** | +| PT-06 Route Point Interpolation Speed | ≤ 5000 ms | **Unverified** | +| PT-07 `GetTilesByRegionAsync` p95 vs pre-AZ-484 baseline | ≤ 1.10 × baseline | **Unverified — harness not implemented** | + +## Why this is acceptable for this cycle + +- AZ-484's correctness was validated by the integration test `MostRecentAcrossSourcesSelection_AZ484_AC2` and the four other AZ-484 integration tests added in Step 11. Those tests use a temp `tiles_az484_*` table to avoid touching production data, and they exercise the same `DISTINCT ON` SQL path that PT-07 would measure. +- The Step 11 unit + integration suite passed clean against the post-AZ-484 code; there is no functional regression to feed a perf regression. +- The post-AZ-484 SQL is structurally cheaper or equivalent to the pre-AZ-484 path: `DISTINCT ON` over the same `(latitude, longitude, tile_zoom, tile_size_meters)` subset uses the existing `idx_tiles_unique_location_source` index. There is no new join, no new full-table scan, and no new lock surface. +- The audit-only nature of this gap is logged so the next cycle picks it up — see leftover entry. + +## Follow-up work (logged as a leftover) + +A leftover file `_docs/_process_leftovers/2026-05-11_perf-pt07-harness.md` records the deferred work to: + +1. Implement PT-07 in `scripts/run-performance-tests.sh` (capture baseline, run post-change measurement, compute ratio). +2. Re-run the full perf suite once the docker stack is available and the harness is in place. +3. Flip PT-07 from "recorded" to "active" in `_docs/02_document/tests/performance-tests.md` and the traceability matrix. + +## Self-verification + +- [x] Runner detection executed (`scripts/run-performance-tests.sh` exists) +- [x] Run decision recorded with reasons +- [x] Every scenario classified (Pass / Warn / Fail / Unverified) +- [x] Follow-up work captured as a tracker leftover (non-user-input blocker per `tracker.mdc`) +- [x] No fabricated metrics — every "Unverified" is genuine diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index fc79e12..7353dd7 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -2,12 +2,12 @@ ## Current Step flow: existing-code -step: 12 -name: Test-Spec Sync -status: not_started +step: 16 +name: Deploy +status: in_progress sub_step: - phase: 0 - name: awaiting-invocation + phase: 1 + name: cycle-deploy-report detail: "" retry_count: 0 cycle: 1 diff --git a/_docs/_process_leftovers/2026-05-11_perf-pt07-harness.md b/_docs/_process_leftovers/2026-05-11_perf-pt07-harness.md new file mode 100644 index 0000000..5e9131e --- /dev/null +++ b/_docs/_process_leftovers/2026-05-11_perf-pt07-harness.md @@ -0,0 +1,32 @@ +# Leftover — PT-07 perf harness + cycle 1 perf run + +- **Timestamp**: 2026-05-11T07:05:00Z +- **Origin**: autodev cycle 1, Step 15 (Performance Test) +- **Blocker class**: non-user-input — work was deferred at the Step 15 user gate; no missing user decision blocks completion. + +## What was deferred + +1. **PT-07 implementation** in `scripts/run-performance-tests.sh`: capture pre-change baseline for `GetTilesByRegionAsync` p95 latency, run post-change measurement, compute the ratio, and assert `ratio ≤ 1.10`. PT-07 was recorded as a documentation entry during Step 12 (`_docs/02_document/tests/performance-tests.md` + `traceability-matrix.md` "NFRs → Test Mapping" section) but the runner script does not yet have the corresponding scenario. +2. **Active perf run** of PT-01..PT-06 against the post-AZ-484 build. The runner exists; it requires `docker-compose up` on the dev host. Not executed this cycle (per the meta-rule "ask before kicking off Docker / long-running perf operations"). + +## Why it is safe to defer + +- AZ-484 functional correctness validated by 5 dedicated AZ-484 integration tests (Step 11). The DB read paths exercised by PT-07 are the same ones exercised by `MostRecentAcrossSourcesSelection_AZ484_AC2` etc. +- The post-AZ-484 SQL uses the same `idx_tiles_unique_location_source` index as the contract specifies; structurally there is no new full scan, join, or lock added vs. pre-AZ-484. +- Cycle 1 perf run is recorded as `Unverified` (not `Fail`) per the test-run perf-mode rules — gate does not block deploy. + +## Replay actions for next /autodev invocation + +When the next cycle's autodev runs, before any new tracker write or before re-entering Step 15 in cycle 2: + +1. Add PT-07 to `scripts/run-performance-tests.sh`: + - Capture a pre-change baseline by checking out the parent of the AZ-484 commit (`git rev-parse HEAD~N` where N points at the AZ-484 batch), running the existing PT-03/PT-04 region scenarios, and recording the `GetTilesByRegionAsync` timings (the repository already logs slow query warnings at >500 ms — extend that log line to include median/p95 captured per call window). + - Run the post-change measurement against the current `HEAD`. + - Compute the p95 ratio and fail when `> 1.10`. +2. Bring up the docker stack (`docker-compose up --build -d`) and run the full perf script with the user's explicit go-ahead. +3. Capture results into `_docs/06_metrics/perf__cycle.md`. +4. Once results are recorded, delete this leftover file. + +## Tracker action (none required this cycle) + +This leftover does NOT require a Jira ticket on its own — it tracks deferred process work, not user-visible scope. If the perf comparison reveals a regression next cycle, that finding will create a Jira bug; until then there is nothing to file.