mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-21 19:21:14 +00:00
51b572108a
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 <cursoragent@cursor.com>
9.4 KiB
9.4 KiB
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:
"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 byConnectionStrings__DefaultConnectionindocker-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:DefaultConnectionkey fromappsettings.jsonentirely and require the env var;Program.csline 23–24 already throws when missing — keep that behaviour.
- Replace the default value with a deliberately-invalid placeholder such as
S2 — Weak Postgres credentials in docker-compose.yml (Medium, dev-only as written)
- Location:
docker-compose.yml:6-7, 30 - Vulnerable code:
POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres … - ConnectionStrings__DefaultConnection=Host=postgres;Port=5432;Database=satelliteprovider;Username=postgres;Password=postgres - Description: Same
postgres/postgrescredentials as S1. The compose file is labelledDevelopment(ASPNETCORE_ENVIRONMENT=Development), so this is contained — but the file is the only compose artifact in the repo, which means anyone runningdocker-compose upon a network-reachable host immediately exposes a Postgres-with-default-creds. - Impact: Postgres on
0.0.0.0:5432(port"5432:5432"mapping) withpostgres/postgresis 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
5432to127.0.0.1:5432rather than0.0.0.0:5432so the host firewall isn't the only protection. (Replace"5432:5432"with"127.0.0.1:5432:5432".) - Source
POSTGRES_USER/POSTGRES_PASSWORDfrom the same.envfile that already suppliesGOOGLE_MAPS_API_KEY(line 31 already shows the pattern). Provide an.env.examplewith 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.
- Bind
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—RequestRegionvalidatesSizeMetersonly;request.Latitude/request.Longitudeare uncheckedSatelliteProvider.Api/Program.cs:237—CreateRoutedelegates toRouteServicewhich validates names but does not range-check waypoint coordinates
- Description:
Latitude,Longitude, and (for region requests) the implicitMaxRoutePointSpacingMetersboundary are accepted without enforcing valid geographic ranges (-90 ≤ lat ≤ 90,-180 ≤ lon ≤ 180).ZoomLevelIS validated downstream byGoogleMapsDownloaderV2againstMapConfig.AllowedZoomLevels— so it is fine. - Impact:
- Garbage inputs (e.g.
lat=999) propagate throughGeoUtils.WorldToTilePosand the slippy-map math, eventually producing nonsensical tile coordinates that are persisted totilesandregions. 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.
- Garbage inputs (e.g.
- Remediation: Add explicit guard clauses at the API boundary (matches the existing
SizeMeters100-10000 pattern):Apply uniformly toif (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" });GetTileByLatLon,RequestRegion, and to each waypoint insideCreateRoute.
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 viagit ls-filesand.gitignore:10) - Description: The local
.envcontains a 39-characterAIzaSy…value matching the Google Maps API key format. The file is correctly excluded from git (line 10 of.gitignore) andgit log -- .envreturns 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.exampletemplate 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.exampleto the repo withGOOGLE_MAPS_API_KEY=replace-with-your-own-key-from-cloud-consoleand 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
- All production source directories scanned (Api, Common, DataAccess, Services.TileDownloader, Services.RegionProcessing, Services.RouteManagement)
- Each finding has file path and line number
- False positives from test files explicitly distinguished (
GlobalExceptionHandlerTests.cs:23"leakySecret" is a positive control) - No real secret values printed in this report (S4 is described without echoing the key)