diff --git a/.env.example b/.env.example index d5f213f..dd21ee9 100644 --- a/.env.example +++ b/.env.example @@ -22,11 +22,14 @@ JWT_SECRET= # # Production values MUST be confirmed by the admin team before deploy # (the admin API stamps the `iss` claim; satellite-provider validates -# the `aud` claim). For local dev these can use the DEV-ONLY values -# baked into appsettings.Development.json — leave blank to fall back. +# the `aud` claim). # -# Example values (NEVER use these in prod): -# JWT_ISSUER=DEV-ONLY-iss-admin-azaion-local -# JWT_AUDIENCE=DEV-ONLY-aud-satellite-provider -JWT_ISSUER= -JWT_AUDIENCE= +# For local dev / CI: use the DEV-ONLY values below. The integration +# test runner and scripts/run-tests.sh read these directly from the +# environment (no appsettings fallback on the test side), so leaving +# them blank will cause run-tests.sh to refuse to start. +# +# NEVER ship these DEV-ONLY values to prod — they exist only to make +# local-dev mints validate against appsettings.Development.json: +JWT_ISSUER=DEV-ONLY-iss-admin-azaion-local +JWT_AUDIENCE=DEV-ONLY-aud-satellite-provider diff --git a/_docs/03_implementation/cumulative_review_batches_04-05_cycle3_report.md b/_docs/03_implementation/cumulative_review_batches_04-05_cycle3_report.md new file mode 100644 index 0000000..308c595 --- /dev/null +++ b/_docs/03_implementation/cumulative_review_batches_04-05_cycle3_report.md @@ -0,0 +1,76 @@ +# Cumulative Code Review — Batches 04–05 cycle 3 + +**Batch range**: 04-05 (cycle 3) +**Cycle**: 3 +**Date**: 2026-05-12 +**Verdict**: PASS_WITH_WARNINGS +**Trigger**: Implement skill Step 14.5 (final cumulative — closes cycle 3 batch coverage; first cumulative was for 01-03) + +## Scope + +| Batch | Tasks | Surfaces touched | +|-------|-------|------------------| +| 04 | AZ-492 | `SatelliteProvider.IntegrationTests/{PerfBootstrap.cs (added), Program.cs}`, `scripts/run-performance-tests.sh`, `_docs/02_document/{tests/performance-tests.md, tests/traceability-matrix.md, modules/tests_integration.md, module-layout.md}`, `_docs/06_metrics/retro_2026-05-11_cycle2.md`, `_docs/_process_leftovers/2026-05-11_perf-pt07-harness.md` (deleted) | +| 05 | AZ-494 | `SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs`, `SatelliteProvider.TestSupport/JwtTokenFactory.cs`, `SatelliteProvider.IntegrationTests/{JwtTestHelpers,JwtIntegrationTests,UavUploadTests,PerfBootstrap,Program}.cs`, `SatelliteProvider.Tests/Authentication/AuthenticationServiceCollectionExtensionsTests.cs`, `SatelliteProvider.Api/appsettings*.json`, `.env.example`, `docker-compose{,.tests}.yml`, `scripts/run-{tests,performance-tests}.sh`, `_docs/02_document/{architecture, modules/{api_program,tests_integration,tests_unit}, tests/traceability-matrix}.md`, `_docs/03_implementation/deploy_cycle2.md`, `_docs/05_security/{security_report,owasp_review}.md`, `_docs/02_tasks/_dependencies_table.md` | + +## Phase-by-Phase Summary (cumulative) + +### Phase 1: Context Loading + +The two batches share the theme **"operationalise what cycle-2 deferred"**. Both extend AZ-487 + AZ-491 / AZ-488 surfaces from cycle 2 rather than introduce new domains. AZ-492 makes the deferred PT-07 / PT-08 scenarios runnable; AZ-494 flips F-AUTH-2 from Medium-open to Resolved. The single point of overlap is `PerfBootstrap` + the perf harness's mint path — AZ-492 added them, AZ-494 had to thread the new iss/aud parameters through. + +### Phase 2: Spec Compliance + +- AZ-492: 6 / 6 ACs addressed at code level; AC-2 + AC-3 require live runtime measurement, gated at Step 16. +- AZ-494: 6 / 7 ACs addressed; AC-7 (cross-repo suite contract write) explicitly deferred — outside this workspace's boundary. Documented in `deploy_cycle2.md` R3 follow-up. + +No new spec-vs-reality findings emerged beyond what was already recorded in batches 01-03. + +### Phase 3: Code Quality (cumulative) + +- `JwtTokenFactory.Create` remains the single `new JwtSecurityToken(...)` site. AZ-494 added optional iss/aud parameters at the END of the signature (defaults `null`), so the cycle-2 hygiene wins survive. +- `JwtTestHelpers` (runner-side, env-reading) and `JwtTokenFactory` (pure, side-effect-free) maintain the separation introduced by AZ-491. `MintAuthenticated` / `MintExpired` are 1-2-line wrappers that don't reintroduce duplicate mint logic. +- The "resolve required value from env or config, throw fail-fast" pattern was extracted into `AuthenticationServiceCollectionExtensions.ResolveRequiredOrThrow`. `ResolveSecretOrThrow` stays separate because it has the ≥ 32-byte invariant from AZ-487. Two functions, two distinct invariants — acceptable. +- `PerfBootstrap` lives in `IntegrationTests`, not `TestSupport`, intentionally — it depends on `SixLabors.ImageSharp`, which TestSupport must not pull in. Documented in `module-layout.md` per the AZ-492 review. +- No verbose logging. No silent error suppression. No new comments that narrate code. No new annotations beyond what already existed. + +### Phase 4: Security Quick-Scan (cumulative) + +- AZ-494 closes F-AUTH-2 (Medium, cycle 2). `ValidateIssuer = true` + `ValidateAudience = true` are now wired against env-sourced values with fail-fast startup. Token reuse across satellite-suite services that share `JWT_SECRET` is now blocked. +- AZ-492 raised no security findings of its own; the perf harness uses the same JWT mint surface as the integration runner. +- One new Low finding noted in `owasp_review.md` A07: no token revocation list exists. That's a known residual gap, deliberately not in this cycle's scope. +- DEV-ONLY placeholder values (`appsettings.Development.json`, `.env.example`) use the `DEV-ONLY-` prefix so a single grep surfaces every site that must be replaced in a real prod rollout. + +### Phase 5: Test Coverage (cumulative) + +- 4 new unit tests + 2 new integration scenarios from AZ-494 — all on a path that's runtime-gated at Step 16. +- AZ-492's PT-07 + PT-08 will execute against a live API + Postgres at perf-test time. No new XUnit cases (perf harness is a bash script). +- Existing integration suite migrated to `MintAuthenticated` / `MintExpired` — no test-fixture drift left for AZ-494's iss/aud validation to break. + +### Phase 6: Cross-Task Consistency (cumulative) + +- AZ-492 + AZ-494 both depend on AZ-491's `JwtTokenFactory` consolidation. Neither reintroduced a parallel mint path. The "single mint surface" invariant from batch 02 survives both batches. +- AZ-492's `PerfBootstrap.MintToken` was updated in lock-step with AZ-494's contract change. Without this update the perf harness would have started emitting 401s. Caught in-batch. +- The `.env` / `JWT_*` env-var convention is now uniform across `JWT_SECRET`, `JWT_ISSUER`, `JWT_AUDIENCE` — same fail-fast contract, same resolution precedence, same docs location, same compose plumbing. No drift between `scripts/run-tests.sh` and `scripts/run-performance-tests.sh`. + +## Cumulative findings + +### Carried over (still open, NOT regressions) + +- L (AZ-492 batch 4): per-item gate cost reported as a derived proxy (`batch_p95 / batch_size`). True per-call timing needs server-side instrumentation — Low, documented as a future PBI. +- L (AZ-492 batch 4): duplicate JPEG-factory pattern across `UavUploadTests.CreateValidJpeg` + `PerfBootstrap.GenerateUavFixture` + the upstream gen-uav-fixture utility — Low, image-fixture consolidation tracked as a future PBI. + +### New (this cumulative) + +- L (AZ-494 batch 5): cross-repo `suite/_docs/10_auth.md` write deferred (AC-7) — Low; tracked in `deploy_cycle2.md` R3. +- L (AZ-494 batch 5): no token revocation list (residual after iss/aud validation) — Low; tracked in `owasp_review.md` A07 as a separate follow-up. + +### Acknowledged operational gate (NOT a finding) + +Option B for AZ-494 deliberately leaves `appsettings.json`'s `Jwt.Issuer` / `Jwt.Audience` empty so production deploy without `JWT_ISSUER` + `JWT_AUDIENCE` env vars fails at startup. This is the user-selected forcing function — by design, not a defect. + +## Verdict + +**PASS_WITH_WARNINGS**. + +All 4 Low findings are non-blocking. The acknowledged operational gate is intentional. Cycle 3 batch implementation is complete; ready for Step 16 (full test suite gate) before promotion to Step 17 (Update Docs). diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index 7c6d7e4..abbefc6 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -4,11 +4,11 @@ flow: existing-code step: 10 name: Implement -status: not_started +status: completed sub_step: - phase: 0 - name: awaiting-invocation - detail: "" + phase: 16 + name: full-test-suite-passed + detail: "Step 16 PASS: ./scripts/run-tests.sh --full green. 5 batches landed (AZ-495+AZ-496, AZ-491, AZ-493, AZ-492, AZ-494). 2 cumulative reviews: 01-03 + 04-05 both PASS_WITH_WARNINGS. AZ-494 AC-1/2 wrong-iss/wrong-aud assertions both PASS in integration log line 650/653. 4 new fail-fast unit tests PASS. Cycle 3 implementation complete." retry_count: 0 cycle: 3 tracker: jira