[AZ-491] [AZ-492] [AZ-493] [AZ-494] [AZ-496] Cycle 3 Step 14: security audit refresh
ci/woodpecker/push/01-test Pipeline was successful
ci/woodpecker/push/02-build-push Pipeline was successful

All 5 phases refreshed against cycle-3 delta:

Phase 1 (Dependency Scan):
  - D1 RESOLVED (AZ-496): Microsoft.AspNetCore.OpenApi 8.0.21 → 8.0.25
  - D3 RESOLVED (AZ-496): JwtBearer 8.0.21 → 8.0.25
  - D4 NEW (Low, test-only): System.IdentityModel.Tokens.Jwt 7.0.3 +
    Microsoft.IdentityModel.Tokens 7.0.3 pinned in TestSupport carry
    CVE-2024-21319 (JWE DoS). Bump to ≥ 7.1.2 tracked as future PBI.

Phase 2 (Static Analysis):
  - F-AUTH-3 (Info): test runner Program.cs logs iss/aud at startup;
    production API does NOT (verified by grep).
  - F-AUTH-4 (Info): DEV-ONLY iss/aud placeholders in
    appsettings.Development.json + .env.example — by design per
    Option B for AZ-494.
  - F-DBR-1: TRUNCATE string interpolation in
    IntegrationTestDatabaseReset.cs — false positive (hard-coded
    table list).
  - F-DBR-2 (Low): TRUNCATE guard is operator-bypassable. Two-guard
    model is conservative-by-default and unit-tested.
  - F-PERF-1 (Low): perf-bootstrap --mint-only writes a 4-hour
    GPS-permission token to stdout. Operator-trusted machine assumed.

Phase 3 (OWASP Top 10):
  - A03 carries D1/D3 RESOLVED + D4 NEW.
  - A07 flips F-AUTH-2 to RESOLVED (AZ-494); residual revocation-list
    Low recorded.
  - A05 status unchanged (F-DBR-1 false positive).
  - A08 picks up F-DBR-2.

Phase 4 (Infrastructure):
  - JWT_ISSUER / JWT_AUDIENCE flow .env → compose → Kestrel config,
    same pattern as JWT_SECRET.
  - INTEGRATION_TEST_DB_RESET + ASPNETCORE_ENVIRONMENT=Testing wired
    for AZ-493 reset gate.
  - SatelliteProvider.TestSupport is IsPackable=false — never ships
    in a production container image.
  - New operational gate added to deploy runbook: grep for DEV-ONLY-
    in the rendered deploy environment must return zero hits.

Phase 5 (Security Report):
  - Verdict: PASS_WITH_WARNINGS (cycle 3 does not escalate).
  - 0 Critical, 0 High, 0 new Medium.
  - Cycle-2 F-AUTH-2 (Medium) RESOLVED; cycle-1 D1 + cycle-2 D3
    RESOLVED.

Autodev state advanced to Step 14 completed. Next: Step 15
(Performance Test, optional gate).

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-12 03:13:04 +03:00
parent e42bf62152
commit 314d1dec39
6 changed files with 248 additions and 8 deletions
+75 -2
View File
@@ -132,9 +132,82 @@
- **Description**: A client (or attacker who can present a `GPS`-permission token) can map the gate by probing inputs (1×1 black image → `WRONG_DIMENSIONS`; 1 KB JPEG → `SIZE_OUT_OF_BAND`; etc.). The thresholds are also documented in the public contract `_docs/02_document/contracts/api/uav-tile-upload.md`.
- **Disposition**: Accept — UX (helping clients self-correct) outweighs the information-hiding benefit, especially since the contract is public anyway. Flagged to keep operators aware: rule thresholds are NOT a security boundary; do not move secrets into reject details.
---
## Cycle 3 Delta (2026-05-12 — AZ-491 / AZ-492 / AZ-493 / AZ-494 / AZ-495 / AZ-496)
### Scope of this delta scan
| File | Cycle-3 task(s) | Domain |
|------|-----------------|--------|
| `SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs` | AZ-494 | Production auth (high-sensitivity surface) |
| `SatelliteProvider.Api/appsettings.json` + `appsettings.Development.json` | AZ-494 | Configuration / secrets handling |
| `SatelliteProvider.IntegrationTests/JwtTestHelpers.cs` | AZ-491, AZ-494 | Test-side, runner-only |
| `SatelliteProvider.IntegrationTests/IntegrationTestDatabaseReset.cs` | AZ-493 | Test-side; destructive DB op (TRUNCATE) gated by two-guard model |
| `SatelliteProvider.IntegrationTests/PerfBootstrap.cs` | AZ-492, AZ-494 | Test-side CLI subcommand (mint token, write JPEG fixture) |
| `SatelliteProvider.IntegrationTests/Program.cs` | AZ-491..AZ-494 | Test-side bootstrap |
| `SatelliteProvider.TestSupport/JwtTokenFactory.cs` | AZ-491, AZ-494 | Test-side, runner-only |
| `SatelliteProvider.TestSupport/IntegrationTestResetGuard.cs` | AZ-493 | Test-side; safety-guard logic |
| `SatelliteProvider.Tests/Authentication/AuthenticationServiceCollectionExtensionsTests.cs` | AZ-487, AZ-494 | Test-side unit |
| `SatelliteProvider.Tests/TestSupport/IntegrationTestResetGuardTests.cs` | AZ-493 | Test-side unit |
| `scripts/run-tests.sh` / `scripts/run-performance-tests.sh` | AZ-492, AZ-493, AZ-494 | Operator-side shell |
| `docker-compose.yml` / `docker-compose.tests.yml` | AZ-494 (env pass-through) | Infrastructure |
| `.env.example` | AZ-494 | Configuration template |
### Cycle-3 findings
#### F-AUTH-3 — Test runner logs `iss` / `aud` values at startup (Informational — test runner only, never in prod)
- **Location**: `SatelliteProvider.IntegrationTests/Program.cs:67` — `Console.WriteLine($"Auth : JWT_SECRET resolved ({…} bytes); iss={jwtIssuer}; aud={jwtAudience}");`
- **Description**: The integration-tests bootstrap prints the resolved iss and aud at startup. Values printed in this cycle's runs were the `DEV-ONLY-iss-admin-azaion-local` / `DEV-ONLY-aud-satellite-provider` placeholders, so no prod-value leak occurred. The production API (`SatelliteProvider.Api/Program.cs`) does NOT print iss/aud — verified by repo grep returning no hits.
- **Impact**: Only meaningful if the integration test runner is somehow pointed at production env vars. The fail-fast contract makes that operator decision visible at startup (the values are visible in test logs).
- **Disposition**: Accept — Informational. Operators inspecting test logs already see the secret byte count and the iss/aud, which is appropriate for a runner whose entire job is to validate against those values. No code change needed.
#### F-DBR-1 — `TRUNCATE TABLE` via string interpolation (False Positive — hard-coded table list)
- **Location**: `SatelliteProvider.IntegrationTests/IntegrationTestDatabaseReset.cs:32` — `$"TRUNCATE TABLE {string.Join(", ", TruncateOrder)} RESTART IDENTITY CASCADE"`.
- **Description**: SAST pattern flagged string-interpolated SQL. Source analysis confirms `TruncateOrder` is a `public static readonly IReadOnlyList<string>` initialised with a hard-coded array of five literal table names; no caller-supplied input flows into the SQL string.
- **Impact**: None. SQL injection here would require an attacker to modify the source file, at which point integrity is already broken.
- **Disposition**: False positive — recorded so future scanners don't re-flag.
#### F-DBR-2 — Destructive `TRUNCATE` action protected only by two soft guards (Low — operator-controlled, deliberate trade-off)
- **Location**: `SatelliteProvider.TestSupport/IntegrationTestResetGuard.cs:11-36` + `SatelliteProvider.IntegrationTests/IntegrationTestDatabaseReset.cs:24-37`.
- **Description**: The reset runs only when (a) `ASPNETCORE_ENVIRONMENT == "Testing"` AND (b) the Npgsql Host is one of `postgres` / `localhost` / `127.0.0.1`. An operator who sets `ASPNETCORE_ENVIRONMENT=Testing` and SSH-tunnels a production Postgres to `localhost:5432` could trick the guard.
- **Impact**: Loss of all `tiles`, `regions`, `routes`, `route_points`, `route_regions` rows on the targeted database.
- **Mitigations in place**: the cycle-3 spec deliberately preferred Host allowlist over DB-name pattern (per the AZ-493 review's "Spec-vs-reality" note); both DB-name and Host checks are cheap to add together if the operator surface grows. The guard is unit-tested (`IntegrationTestResetGuardTests`) with representative production hostnames (`prod-db-cluster-1.example.com`, etc.) to confirm they're rejected.
- **Disposition**: Accept — Low. The guard is conservative-by-default; bypassing requires deliberate operator action (env var + tunnel). Future PBI: add a third guard requiring an explicit `INTEGRATION_TEST_DB_RESET_CONFIRM=I-UNDERSTAND-THIS-TRUNCATES` env var when the guard runs against `localhost` from outside Docker.
#### F-PERF-1 — Perf-bootstrap mint subcommand writes a 4-hour `GPS`-permission token to stdout (Low — operator-controlled CLI, no network exposure)
- **Location**: `SatelliteProvider.IntegrationTests/PerfBootstrap.cs:21-48`.
- **Description**: `dotnet <integration-tests.dll> --mint-only` prints a 4-hour HS256 token with `permissions: GPS` claim to stdout. The token grants the same access as a production-issued `GPS` admin token for the lifetime window. The token bytes flow through the operator's shell history, terminal scrollback, and any process accounting logs.
- **Impact**: An attacker with read access to the operator's machine within the 4-hour window could replay the token against the API.
- **Mitigations in place**: lifetime is bounded to 4 hours (vs. e.g. 24 hours that would be tempting for "convenient perf runs"). The token is minted against `JWT_SECRET` from `.env` — same trust boundary as a developer's local dev setup. Operators are expected to run the perf script on a trusted machine.
- **Disposition**: Accept — Low. Future hardening: pipe the token to `xargs` / process substitution so it never lands in the shell history; consider mounting `JWT_SECRET` via a Docker secret rather than an env var when running the perf harness inside CI.
#### F-AUTH-4 — DEV-ONLY iss/aud placeholders committed to `appsettings.Development.json` + `.env.example` (Informational — by design, AZ-494 Option B)
- **Location**: `SatelliteProvider.Api/appsettings.Development.json` (`DEV-ONLY-iss-admin-azaion-local` / `DEV-ONLY-aud-satellite-provider`); `.env.example` (same placeholders).
- **Description**: AZ-494 (Option B per user decision) deliberately ships DEV-ONLY placeholder values in development config so local dev / docker-compose flows work without operator setup. Production config (`appsettings.json`) ships with empty values, triggering the fail-fast contract.
- **Impact**: None in production (the empty values guarantee a startup failure before any token validates). In development, the placeholders are clearly tagged with `DEV-ONLY-` prefix so a grep can surface them at any time.
- **Disposition**: Accept — by design. This is the explicit Option B trade-off the user selected over Option A (postpone) and Option C (hard-code prod values).
### Resolved this cycle
- **F-AUTH-2** (cycle 2): `iss` / `aud` not validated. **RESOLVED in AZ-494** — `ValidateIssuer = true` + `ValidateAudience = true` wired against env-sourced values with fail-fast startup. Verified at the source (`AuthenticationServiceCollectionExtensions.cs:37-40`).
### Patterns NOT triggered by cycle-3 changes
- **Injection**: SQL injection ✗ (only TRUNCATE with hard-coded table names — F-DBR-1 false positive). Command injection ✗ (no `Process.Start` / `exec` / `shell=True`). XSS ✗ (no HTML rendering paths added). Template injection ✗.
- **Cryptographic Failures**: no new hashing or encryption code; HS256 unchanged from AZ-487.
- **Insecure Deserialization**: ImageSharp decode path unchanged from cycle 2; no new `JsonSerializer.Deserialize<>` against attacker input.
## Self-verification
- [x] All production source directories scanned (Api, Common, DataAccess, Services.TileDownloader, Services.RegionProcessing, Services.RouteManagement)
- [x] All cycle-3 test-side surfaces scanned (TestSupport, IntegrationTests, Tests)
- [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)
- [x] False positives from test files explicitly distinguished (`GlobalExceptionHandlerTests.cs:23` "leakySecret" is a positive control); F-DBR-1 also classified as false-positive with rationale
- [x] No real secret values printed in this report (S4 is described without echoing the key; F-AUTH-4 cites placeholder values that are public-by-design)
- [x] Cycle-3 surfaces (`AddSatelliteJwt` iss/aud extension, `IntegrationTestDatabaseReset`, `PerfBootstrap`, two-guard logic) all reviewed; findings either documented above or explicitly cleared