Files
Oleksandr Bezdieniezhnykh 314d1dec39
ci/woodpecker/push/01-test Pipeline was successful
ci/woodpecker/push/02-build-push Pipeline was successful
[AZ-491] [AZ-492] [AZ-493] [AZ-494] [AZ-496] Cycle 3 Step 14: security audit refresh
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>
2026-05-12 03:13:04 +03:00

220 lines
21 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Security Audit Report
**Date**: 2026-05-11 (cycle 1 baseline) · 2026-05-11 cycle 2 refresh appended below
**Scope**: Satellite Provider — full repository (Api, Common, DataAccess, Services.*, Tests, infra)
**Trigger**: `/autodev` Step 14 (Security Audit) — feature cycle 1, post-AZ-484; **cycle 2 delta scan added 2026-05-11 covering AZ-487 JWT validation baseline + AZ-488 UAV tile upload endpoint**
**Verdict (current, post-cycle-2)**: **PASS_WITH_WARNINGS**
## Summary
| Severity | Cycle 1 | Cycle 2 delta | Current total |
|----------|---------|---------------|---------------|
| Critical | 0 | 0 | 0 |
| High | 0 | 0 | 0 |
| Medium | 5 | 2 (F-AUTH-2, F-DEPS-UAV) | 7 |
| Low | 5 | 4 (F-AUTH-1, F-AUTH-3, F-UAV-2, D3) | 9 |
| Info | — | 1 (F-UAV-3) | 1 |
No Critical or High findings in either cycle. The verdict remains `PASS_WITH_WARNINGS`. Cycle 2's two new Medium findings (`iss`/`aud` not validated yet; ImageSharp decode exposure widened) are both bounded by mitigations already in place and tracked as follow-ups rather than gating items. **AZ-484 (cycle 1's only feature change) introduced zero new findings** — it remained a pure data-layer change.
## 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) — **RESOLVED cycle 3 (AZ-496): bumped to 8.0.25** |
| 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 14 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)
---
## Cycle 2 Delta Summary (AZ-487 + AZ-488)
### What changed in cycle 2
AZ-487 introduced a JWT validation baseline (HS256, `JWT_SECRET` env var, `.RequireAuthorization()` on every endpoint, Swagger Bearer hook). AZ-488 replaced the 501 `/api/satellite/upload` stub with a multipart batch endpoint that validates JPEGs via a 5-rule quality gate and persists accepted tiles. Two new packages were added: `Microsoft.AspNetCore.Authentication.JwtBearer 8.0.21` (Api) and `SixLabors.ImageSharp 3.1.11` (TileDownloader + Tests; consistent with the existing Api-level reference).
### Findings table (cycle-2 delta)
| # | Severity | Category | Location | Title |
|------------|---------------|------------------------------------------|-------------------------------------------------------------------------|-----------------------------------------------------------------------------|
| F-AUTH-1 | Low (accepted)| A02 — Misconfiguration | `SatelliteProvider.Api/appsettings.Development.json:14` | DEV-ONLY JWT secret committed; env-var overrides; operator must verify in prod |
| F-AUTH-2 | Medium | A07 — AuthN / Identification | `Authentication/AuthenticationServiceCollectionExtensions.cs:31-32` | `iss`/`aud` not validated — **RESOLVED cycle 3 (AZ-494): `ValidateIssuer` + `ValidateAudience` flipped to `true`; values sourced from `JWT_ISSUER` / `JWT_AUDIENCE` env vars with fail-fast contract; production admin-team values still to be confirmed before deploy** |
| F-AUTH-3 | Low (rec. I3) | A06 — Insecure Design | every `/api/satellite/*` endpoint | No rate limiting on 401-producing paths (extends cycle-1 I3) |
| F-UAV-1 | Medium | A03 — Supply Chain (exposure) | `Services.TileDownloader/UavTileQualityGate.cs:60-95` | ImageSharp decode now runs on attacker-controlled JPEGs (mitigations OK) |
| F-UAV-2 | Low | A07 — AuthN claim parsing | `Authentication/PermissionsRequirement.cs:84-111` | `JsonDocument.Parse` on signature-validated claim values (bounded by header cap) |
| F-UAV-3 | Informational | A06 — Insecure Design (info-disclosure) | `Services.TileDownloader/UavTileQualityGate.cs` | Reject reasons disclose gate structure (accepted UX trade-off; documented in contract) |
| D3 | Low | A03 — Supply Chain | `SatelliteProvider.Api.csproj` (new JwtBearer 8.0.21) | Shares D1 patch line; same remediation — **RESOLVED cycle 3 (AZ-496): bumped to 8.0.25 alongside OpenApi** |
| F-DEPS-UAV | Medium | A03 — Supply Chain (exposure) | new ImageSharp call site in TileDownloader | Documented in dependency_scan.md cycle-2 delta |
### Verdict reconciliation
- No new Critical or High findings → cycle 2 does NOT escalate the verdict.
- Two new Medium findings — both are *follow-ups under existing remediations*, not blockers:
- F-AUTH-2 — **RESOLVED cycle 3 (AZ-494)**: validation flipped on, config plumbed through env + appsettings; production iss/aud values gated behind admin-team confirmation at deploy time.
- F-UAV-1 + F-DEPS-UAV jointly say "subscribe to ImageSharp GHSA and bump aggressively" — no immediate change needed.
- F-AUTH-1 and F-UAV-3 are explicitly accepted.
- F-AUTH-3 + D3 fold into existing cycle-1 remediations (I3 rate limiting, D1 8.0.x patch bump).
**Current verdict: PASS_WITH_WARNINGS** (cycle 2 satisfies the autodev Step-14 gate; proceed to Step 15).
### New / refreshed cycle-2 recommendations
- **Pre-deploy gate (operational, NOT code)**: `deploy/SKILL.md` must verify `JWT_SECRET` is set to a ≥ 32-byte value distinct from the DEV-ONLY placeholder. Cycle-2 deploys without this verification step are gated.
- **Coordinate with admin team**: confirm expected `iss`/`aud` values for the prod `JWT_ISSUER` / `JWT_AUDIENCE` env vars. **Code change DONE cycle 3 (AZ-494)**`ValidateIssuer` / `ValidateAudience` are now `true`; `appsettings.Development.json` ships clearly-tagged DEV-ONLY values so local dev works out-of-the-box; production `appsettings.json` ships empty values so the app fails fast at startup until the operator supplies the real values via env. The remaining work is purely operational.
- **Bump 8.0.x ASP.NET Core packages together** — **DONE cycle 3 (AZ-496)**: both `Microsoft.AspNetCore.OpenApi` and `Microsoft.AspNetCore.Authentication.JwtBearer` bumped to `8.0.25` in `SatelliteProvider.Api.csproj`. Runtime base image uses floating `mcr.microsoft.com/dotnet/aspnet:8.0` so the deployed runtime auto-picks up the matching patch on next build.
- **ImageSharp subscribe-and-bump policy**: add to the runbook — patch within 7 days of any `SixLabors.ImageSharp` GHSA. Reconsider sandboxing if the upload endpoint is exposed beyond the trust boundary documented in architecture.md § 7.
- **Cycle-2 hardening backlog (Low priority)**:
- Pass `JsonDocumentOptions { MaxDepth = 8 }` and a max-claim-length check to `PermissionsAuthorizationHandler.TryReadJsonArray`.
- Document in `architecture.md` that reject-reason codes are NOT a security boundary.
---
## Cycle 3 Delta Summary (AZ-491 / AZ-492 / AZ-493 / AZ-494 / AZ-495 / AZ-496)
### What changed in cycle 3
Cycle 3 was test-infrastructure + dependency + iss/aud-validation hardening. The most security-relevant change is AZ-494, which flipped `iss` / `aud` validation from `false` (cycle-2 F-AUTH-2 finding) to `true`, sourcing both values from `JWT_ISSUER` / `JWT_AUDIENCE` env vars with the same fail-fast contract as `JWT_SECRET`. AZ-496 bumped the ASP.NET Core 8.0.21 family to 8.0.25 (closes D1 + D3). The other tasks (AZ-491 consolidate JWT test helpers, AZ-492 perf harness PT-07/PT-08, AZ-493 integration test DB reset hook, AZ-495 doc folder convention) added test-side surface but no new production attack surface.
### Findings table (cycle-3 delta)
| # | Severity | Category | Location | Title |
|------------|-----------------------|------------------------------------------|---------------------------------------------------------------------------|---------------------------------------------------------------------------------------------|
| D4 | Low | A03 — Supply Chain (test-only) | `SatelliteProvider.TestSupport.csproj` (`System.IdentityModel.Tokens.Jwt 7.0.3` + `Microsoft.IdentityModel.Tokens 7.0.3`) | CVE-2024-21319 (JWE DoS) in pinned 7.0.3 < 7.1.2 fix line; **test-only, never deployed**; bump to ≥ 7.1.2 in future PBI |
| F-AUTH-3 | Informational | A07 — AuthN observability (test-only) | `SatelliteProvider.IntegrationTests/Program.cs:67` | Test runner logs resolved `iss` / `aud` at startup — operator-visible only, no prod path leaks |
| F-AUTH-4 | Informational | A02 — Misconfiguration (by design) | `appsettings.Development.json` + `.env.example` | DEV-ONLY iss/aud placeholders deliberately committed (Option B forcing function for prod fail-fast) |
| F-DBR-1 | **False positive** | A05 — Injection (TRUNCATE interpolation) | `SatelliteProvider.IntegrationTests/IntegrationTestDatabaseReset.cs:32` | Hard-coded table list; no user input flows in; recorded so scanners don't re-flag |
| F-DBR-2 | Low | A08 — Data Integrity (test-only) | `SatelliteProvider.TestSupport/IntegrationTestResetGuard.cs:11-36` | Destructive TRUNCATE gated by two soft guards (env + Host allowlist); deliberate operator-bypass surface |
| F-PERF-1 | Low | A06 / A07 — Token handling (CLI-only) | `SatelliteProvider.IntegrationTests/PerfBootstrap.cs:21-48` | 4-hour `GPS`-permission token written to stdout for the perf harness; operator-trusted machine assumed |
### Verdict reconciliation
- No new Critical or High findings → cycle 3 does NOT escalate the verdict.
- One new Low finding in production-adjacent surfaces (D4) — **test-only, never deployed**; documented remediation path in dependency_scan.md.
- Two new Informational items (F-AUTH-3, F-AUTH-4) — both by-design or test-runner-only.
- One Low finding in test-side destructive op (F-DBR-2) — unit-tested guard, conservative-by-default.
- One Low finding on operator-controlled perf-CLI surface (F-PERF-1) — accepted operational trade-off.
- One false positive (F-DBR-1) — recorded for future scanner runs.
- **Cycle-2 carry-overs resolved**: F-AUTH-2 (Medium, A07) and D3 (Low, A03) both flipped to RESOLVED. Cycle-1 D1 also flipped to RESOLVED.
**Current verdict: PASS_WITH_WARNINGS** (cycle 3 satisfies the autodev Step-14 gate; proceed to Step 15).
### New / refreshed cycle-3 recommendations
- **Pre-deploy gate (operational, NOT code)** — same forcing function as cycle 2's `JWT_SECRET` gate, now extended:
- `deploy/SKILL.md` must verify `JWT_SECRET` is ≥ 32 bytes AND NOT equal to the DEV-ONLY placeholder.
- `deploy/SKILL.md` must verify `JWT_ISSUER` and `JWT_AUDIENCE` are set to admin-team-confirmed prod values, NOT the `DEV-ONLY-` placeholders. A grep for `DEV-ONLY-` in the rendered deploy environment must return zero hits.
- **TestSupport 7.0.3 bump** (D4) — future PBI: bump `Microsoft.IdentityModel.Tokens` + `System.IdentityModel.Tokens.Jwt` to ≥ 7.1.2 (or align to the 8.0.x family). Eliminates the `NU1902` warning noise on every restore. Test-only, low priority.
- **Cross-repo doc** (AZ-494 AC-7) — `suite/_docs/10_auth.md` write deferred. Outside this workspace's boundary; tracked in `deploy_cycle2.md` R3 follow-up.
- **Token revocation list** (A07 residual) — accepted as out-of-scope until requirement emerges. Re-evaluate if cycle N introduces user-revocable tokens or session management.
### Cycle-3 hardening backlog (Low priority, NOT cycle-blocking)
- Add a third guard to `IntegrationTestResetGuard` requiring explicit `INTEGRATION_TEST_DB_RESET_CONFIRM=I-UNDERSTAND-THIS-TRUNCATES` when the guard runs against a non-`postgres` host (i.e. operator's `localhost` outside Docker).
- Drop `iss` / `aud` from the integration-tests startup banner (F-AUTH-3) once the AZ-494 contract is well-understood by everyone on the team; the byte-count line is enough.
- Pipe `--mint-only` token through a process substitution / `xargs` so it never lands in shell history.
### Phase artifact status
| Artifact | Cycle 1 | Cycle 2 delta | Cycle 3 delta |
|----------|---------|---------------|---------------|
| `dependency_scan.md` | ✓ | ✓ | ✓ (D1 + D3 RESOLVED; D4 NEW) |
| `static_analysis.md` | ✓ | ✓ | ✓ (F-AUTH-3, F-AUTH-4, F-DBR-1, F-DBR-2, F-PERF-1) |
| `owasp_review.md` | ✓ | ✓ | ✓ (A03 + A07 status refreshed) |
| `infrastructure_review.md` | ✓ | ✓ | ✓ (compose env pass-through, AZ-493 guard plumbing, AZ-494 operational gates) |
| `security_report.md` | ✓ | ✓ | ✓ (this section) |