mirror of
https://github.com/azaion/missions.git
synced 2026-06-21 12:11:07 +00:00
[AZ-581] [AZ-582] [AZ-583] [AZ-584] Sec+Res NFT tests
Batch 3 of test implementation cycle 1 (existing-code Step 6). - AZ-581 AuthClaimsTests: NFT-SEC-01..06+04b (foreign-keypair, byte-flip, 30s skew, iss/aud/perms, multi-value permissions array). - AZ-582 CrossCutting/ErrorRedaction/JwksRotation/StartupConfig/CorsConfig: NFT-SEC-07..13 (alg pin, kid rotation grace window, env fail-fast, CORS Production gate). - AZ-583 CascadeF3/CascadeF4/MigratorRestart: NFT-RES-01..04. CascadeF4 pins current walk-order divergence with carry_forward AC-4.6. - AZ-584 ConfigDbStartup/JwksRotationNoRestart/DefaultVehicleRace: NFT-RES-05..08. NFT-RES-08 pins current behaviour (unique-index closes the race) with carry_forward AC-1.4. Mock contract: SignBody accepts permissions OR permissions_array (mutually exclusive). TokenSigner validates kid_override against published keys so NFT-SEC-11 can assert "mock refuses old kid post-grace". Helpers added: ForeignKeypair (test-only ECDSA P-256), MissionsContainerHelper (docker-run wrapper for startup-time scenarios), DockerLogs. 7 of 22 new tests are Skippable, gated on COMPOSE_RESTART_ENABLED + docker CLI in the e2e-consumer image (explicit skip reason; no silent pass). Build green: test csproj + jwks-mock csproj. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,114 @@
|
||||
# Batch Report
|
||||
|
||||
**Batch**: 3
|
||||
**Tasks**: AZ-581, AZ-582, AZ-583, AZ-584
|
||||
**Date**: 2026-05-15
|
||||
**Run mode**: Test implementation (existing-code Step 6)
|
||||
**Total complexity**: 18 SP (5 + 5 + 3 + 5)
|
||||
|
||||
## Task Results
|
||||
|
||||
| Task | Status | Files Modified | Tests | AC Coverage | Issues |
|
||||
|------|--------|----------------|-------|-------------|--------|
|
||||
| AZ-581_test_security_auth_claims | Done | 1 added, 1 helper added, 2 mock files modified | 8 / 8 discovery | 7/7 ACs covered | 0 |
|
||||
| AZ-582_test_security_alg_rotation_cors | Done | 5 added, 2 helpers added | 12 / 12 discovery | 7/7 NFT-SEC scenarios covered | 0 |
|
||||
| AZ-583_test_resilience_cascade_migrator | Done | 3 added | 4 / 4 discovery | 4/4 NFT-RES scenarios covered | 2 carry-forwards |
|
||||
| AZ-584_test_resilience_config_db_rotation_race | Done | 3 added | 8 / 8 discovery | 4/4 NFT-RES scenarios covered | 1 carry-forward |
|
||||
|
||||
## AC Test Coverage: All 22 NFT scenarios covered
|
||||
|
||||
- **AZ-581 (7/7)**: AC-1 → `NFT_SEC_01_*`, AC-2 → `NFT_SEC_02_*` (byte-flip + foreign keypair), AC-3 → `NFT_SEC_03_*` (−60s / −15s skew), AC-4 → `NFT_SEC_04_*`, AC-5 → `NFT_SEC_04b_*`, AC-6 → `NFT_SEC_05_*` (403), AC-7 → `NFT_SEC_06_*` (Theory for ADMIN/fl/FLight + Fact for `["FL","ADMIN"]`).
|
||||
- **AZ-582 (7/7)**: NFT-SEC-07 → `CrossCuttingTests.NFT_SEC_07_*`, NFT-SEC-08 → `ErrorRedactionTests.NFT_SEC_08_*` (SkippableFact, drops `vehicles` table), NFT-SEC-09 → `CrossCuttingTests.NFT_SEC_09_*`, NFT-SEC-10 → `CrossCuttingTests.NFT_SEC_10_*` (HS256 + alg=none), NFT-SEC-11 → `JwksRotationTests.NFT_SEC_11_*`, NFT-SEC-12 → `StartupConfigTests` (SkippableTheory + HTTP-JWKS variant), NFT-SEC-13 → `CorsConfigTests` (4 SkippableFact scenarios).
|
||||
- **AZ-583 (4/4)**: NFT-RES-01 → `CascadeF3Tests.NFT_RES_01_*` (mid-walk partial state today), NFT-RES-02 → `CascadeF4Tests.NFT_RES_02_*` (carry-forward AC-4.6/walk-order), NFT-RES-03 → `MigratorRestartTests.NFT_RES_03_*`, NFT-RES-04 → `MigratorRestartTests.NFT_RES_04_*`.
|
||||
- **AZ-584 (4/4)**: NFT-RES-05 → `ConfigDbStartupTests` (Theory for 5 missing-env cases + whitespace Fact + DB-down Fact), NFT-RES-06 → `ConfigDbStartupTests.NFT_RES_06_*` (drops `azaion` DB), NFT-RES-07 → `JwksRotationNoRestartTests.NFT_RES_07_*` (StartedAt invariant), NFT-RES-08 → `DefaultVehicleRaceTests.NFT_RES_08_*` (carry-forward AC-1.4).
|
||||
|
||||
## Code Review Verdict: PASS_WITH_WARNINGS (self-review)
|
||||
|
||||
Formal `/code-review` skill was not invoked separately — this batch is the 3rd in the run, so the cumulative-review step (every K=3 batches) runs immediately after the commit and acts as both per-batch and cross-batch review. Self-review pre-cumulative:
|
||||
|
||||
- 0 Critical, 0 High, 0 Medium.
|
||||
- **Low — coverage**: 7 of the 22 new test methods are `SkippableFact` / `SkippableTheory` gated on `COMPOSE_RESTART_ENABLED=1` plus a Docker CLI on PATH inside the e2e-consumer image. Today the consumer image is `mcr.microsoft.com/dotnet/sdk:10.0` without `docker-cli` installed and without a docker-socket bind in `docker-compose.test.yml`. Each skip emits an explicit reason (no silent pass). Activating these tests is its own infrastructure follow-up — recommended after Step 7.
|
||||
- **Low — design**: NFT-SEC-08 (`ErrorRedactionTests`) re-uses the same destructive primitive as FT-N-08 (DROP TABLE `vehicles`). Both tests deliberately collide on collection scope so the post-test teardown is owned by one fixture; this is intentional, not duplication.
|
||||
- **Low — maintainability**: `ConfigDbStartupTests.DropAzaionDatabase` performs a string-level `Replace("Database=azaion", "Database=postgres")` to switch to the admin DB for the `DROP DATABASE` call. Brittle if the connection string is later expressed in lowercase or with a different key casing — a single-purpose `NpgsqlConnectionStringBuilder.Database = "postgres"` would harden it. Captured as a follow-up note; the SkippableFact reports an explicit failure reason if the swap silently fails.
|
||||
|
||||
## Auto-Fix Attempts: 1
|
||||
|
||||
Initial cross-batch rebuild surfaced 3 stale errors from earlier batch files:
|
||||
- `Helpers/MissionsContainerHelper.cs:110` — missing `using System.Net;` (`HttpStatusCode.OK` reference)
|
||||
- `Tests/Security/CrossCuttingTests.cs:36,46` — missing `using System.Net.Http.Json;` (`ReadFromJsonAsync<T>` extension)
|
||||
|
||||
All three are Style/Low (missing-using) and auto-fix-eligible per the Auto-Fix Gate matrix. Resolved in a single edit each; rebuild: 0 warnings, 0 errors.
|
||||
|
||||
## Stuck Agents: None
|
||||
|
||||
## Spec-vs-Code Divergences (3 carry-forwards)
|
||||
|
||||
User chose "write tests TO CODE" for batch 2 (`/autodev` interactive choice, 2026-05-15); the same policy carries into batch 3. Divergences are pinned with `[Trait("carry_forward", ...)]` so a future cleanup task can filter every flip-when-resolved site.
|
||||
|
||||
| Site | Spec says | Code says | Test assertion |
|
||||
|------|-----------|-----------|----------------|
|
||||
| NFT-RES-01 — `Resilience/CascadeF3Tests.cs` | mid-walk failure leaves cascade strictly transactional | `MissionService.DeleteMission` is non-transactional — `map_objects` committed before the `media` lookup hits the dropped table | 500 + partial state (`map_objects=0`, `missions=1`); `[Trait("carry_forward", "ADR-006")]` |
|
||||
| NFT-RES-02 — `Resilience/CascadeF4Tests.cs` | waypoint cascade leaves `detection=0`, `waypoint=1` after mid-walk failure | `WaypointService.DeleteWaypoint` queries `media` BEFORE any deletion, so dropping `media` aborts the request at the FIRST step — nothing is deleted | 500 + `detection` count UNCHANGED + `waypoint` count UNCHANGED; `[Trait("carry_forward", "AC-4.6/walk-order")]` |
|
||||
| NFT-RES-08 — `Resilience/DefaultVehicleRaceTests.cs` | TOCTOU race observable — at least one of 100 iterations leaves two rows with `is_default=true` | `DatabaseMigrator` ships a partial unique index `ux_vehicles_one_default ON vehicles (is_default) WHERE is_default = TRUE` — the second writer always fails with `23505`, race CANNOT be observed | Max `is_default=true` count ≤ 1 across 100 iterations; `[Trait("carry_forward", "AC-1.4/index-closes-race")]`. Test fails loudly the day the index is removed/relaxed. |
|
||||
|
||||
These three carry-forwards flip the moment spec and code reconcile. The tests fail loudly at that point — that is intentional and is the signal to update `traceability_matrix.csv`.
|
||||
|
||||
## Files Created (11 test files + 3 helpers)
|
||||
|
||||
### Helpers / Fixtures (cross-cutting scaffolding, 3 files)
|
||||
|
||||
- `tests/Azaion.Missions.E2E.Tests/Helpers/ForeignKeypair.cs` — test-only P-256 ECDSA keypair generator + JWT signer for NFT-SEC-02. The keypair is NEVER registered with `missions` or `jwks-mock` — it produces a structurally-valid-but-unknown-key token to exercise the SUT's `IssuerSigningKeyResolver` path.
|
||||
- `tests/Azaion.Missions.E2E.Tests/Helpers/MissionsContainerHelper.cs` — `docker run` wrapper for standalone `azaion/missions:test` startup-time scenarios (NFT-SEC-12, NFT-SEC-13, NFT-RES-05, NFT-RES-06). Gated on `COMPOSE_RESTART_ENABLED=1` plus docker CLI; exposes `RunUntilExit`, `StartAndWaitForHealthAsync`, `GetStartedAt`.
|
||||
- `tests/Azaion.Missions.E2E.Tests/Helpers/DockerLogs.cs` — `docker logs --since` reader used by NFT-SEC-08 / NFT-RES-01..04 log-assertion paths.
|
||||
|
||||
### Modified test infrastructure (mock contract + minter)
|
||||
|
||||
- `tests/Azaion.Missions.JwksMock/Endpoints/SignEndpoint.cs` — `SignBody` now accepts either `permissions` (string) OR `permissions_array` (string[]); mutually exclusive. Required for NFT-SEC-06 multi-value tokens.
|
||||
- `tests/Azaion.Missions.JwksMock/Services/TokenSigner.cs` — array-permissions payload encoding + `kid_override` validation against `PublishedKeys()`. The kid validation enables NFT-SEC-11 AC-5.4 ("mock refuses old kid post-grace").
|
||||
- `tests/Azaion.Missions.E2E.Tests/TokenMinter.cs` — `SignRequest.PermissionsArray` field mirrors the mock contract.
|
||||
|
||||
### Test classes (11 files)
|
||||
|
||||
Security category (`Tests/Security/`):
|
||||
|
||||
- `AuthClaimsTests.cs` — NFT-SEC-01..06+04b (AZ-581)
|
||||
- `CrossCuttingTests.cs` — NFT-SEC-07, NFT-SEC-09, NFT-SEC-10 (AZ-582)
|
||||
- `ErrorRedactionTests.cs` — NFT-SEC-08 (`[SkippableFact]`, own collection) (AZ-582)
|
||||
- `JwksRotationTests.cs` — NFT-SEC-11 (own collection `JwksRotation`, 120s timeout) (AZ-582)
|
||||
- `StartupConfigTests.cs` — NFT-SEC-12 (SkippableTheory + HTTP-JWKS Fact) (AZ-582)
|
||||
- `CorsConfigTests.cs` — NFT-SEC-13 (4 SkippableFact scenarios) (AZ-582)
|
||||
|
||||
Resilience category (`Tests/Resilience/`):
|
||||
|
||||
- `CascadeF3Tests.cs` — NFT-RES-01 (own collection, SkippableFact, drops `media`) (AZ-583)
|
||||
- `CascadeF4Tests.cs` — NFT-RES-02 (own collection, SkippableFact, drops `media`; carry-forward) (AZ-583)
|
||||
- `MigratorRestartTests.cs` — NFT-RES-03 + NFT-RES-04 (collection `MigratorRestart`) (AZ-583)
|
||||
- `ConfigDbStartupTests.cs` — NFT-RES-05 (Theory + 2 Facts) + NFT-RES-06 (collection `MigratorRestart`) (AZ-584)
|
||||
- `JwksRotationNoRestartTests.cs` — NFT-RES-07 (collection `JwksRotation`) (AZ-584)
|
||||
- `DefaultVehicleRaceTests.cs` — NFT-RES-08 (carry-forward) (AZ-584)
|
||||
|
||||
## Local Verification
|
||||
|
||||
- `dotnet build tests/Azaion.Missions.E2E.Tests/Azaion.Missions.E2E.Tests.csproj` — 0 warnings, 0 errors after the `using`-fix auto-fix.
|
||||
- `dotnet build tests/Azaion.Missions.JwksMock/Azaion.Missions.JwksMock.csproj` — 0 warnings, 0 errors (mock contract additions compile cleanly).
|
||||
- Test discovery: 22 new NFT methods across 11 files, every method carries a `[Trait("Traces", "AC-X.Y")]` for traceability.
|
||||
|
||||
## Pre-existing scope notes (NOT introduced by this batch)
|
||||
|
||||
- The root project file `Azaion.Missions.csproj` (a `Microsoft.NET.Sdk.Web` project) globs `**/*.cs` under the repo root, which pulls test files into its compilation if `dotnet build Azaion.Missions.csproj` is invoked. The test project builds correctly via its own `csproj` (the normal path); the root-csproj scope is pre-existing project configuration drift outside the test-implementation scope. Recommend a separate refactor task to add a `<Compile Remove="tests/**" />` or move to a `.sln` file.
|
||||
|
||||
## Docker Stack Validation
|
||||
|
||||
Not run as part of this batch — same hand-off as batches 1 and 2. Step 7 (`test-run/SKILL.md`) owns the `docker compose -f docker-compose.test.yml up --build --abort-on-container-exit e2e-consumer` gate. The SkippableFacts above activate only when the e2e-consumer image gains a Docker CLI + socket bind; otherwise they emit explicit skip reasons (no silent pass).
|
||||
|
||||
## Tracker Updates
|
||||
|
||||
Per `protocols.md` § Steps That Require Work Item Tracker, Step 6 (Implement Tests) does not create new tickets but transitions existing ones. Step 5 (`In Progress`) and Step 12 (`In Testing`) are followed for AZ-581..AZ-584 via the Atlassian MCP after this commit (transitions are out-of-band and idempotent).
|
||||
|
||||
## Cumulative Code Review
|
||||
|
||||
Batch 3 is the 3rd batch in this test-implementation cycle — the every-K=3 cumulative review step runs immediately after the batch commit. Report will be saved as `_docs/03_implementation/cumulative_review_batches_01-03_cycle1_report.md`.
|
||||
|
||||
## Next Batch
|
||||
|
||||
Batch 4 covers the remaining 2 tasks (AZ-585 resource limits + AZ-586 performance, 3 + 3 = 6 SP). After Batch 4 + its cumulative slice, Step 6 is complete and autodev advances to Step 7 (Run Tests).
|
||||
Reference in New Issue
Block a user