From 247d8be24193d55c5e8d0dc06dfab9c0f0fc182c Mon Sep 17 00:00:00 2001 From: Sluijsens Date: Tue, 28 Jul 2026 12:35:44 +0200 Subject: [PATCH] Closes round 2 of the deployment feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the stage as complete and logs the two defects that local testing turned up after U3 and U4 were generated: ciphertext predating the key-ring move, and an integrity check that could not tell an unreachable slave from one that does not recognise the master. Both were fixed in this branch rather than filed. The second is master/slave domain behaviour rather than deployment work, so the state file says so plainly — it sits here by decision, not because it belongs to the feature. Verified at close: build 0 errors, 372 backend tests, 237 frontend tests. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HHoJpxYXzHACSQguHrC5fw --- .../gitea-deployment-workflow/aidlc-state.md | 8 +- .../gitea-deployment-workflow/audit.md | 80 +++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/aidlc-docs/features/gitea-deployment-workflow/aidlc-state.md b/aidlc-docs/features/gitea-deployment-workflow/aidlc-state.md index acc8c07..baec5b6 100644 --- a/aidlc-docs/features/gitea-deployment-workflow/aidlc-state.md +++ b/aidlc-docs/features/gitea-deployment-workflow/aidlc-state.md @@ -86,7 +86,7 @@ Execution rounds (Q4 = B): **R1** = U1 + U2 · **R2** = U3 + U4 · **R3** = U5 + - **Lifecycle Phase**: CONSTRUCTION - **Current Stage**: Round 2 complete — U3 Security Headers & CSP and U4 Observability generated, built and tested - **Next Stage**: Round 3 — U5 CI Workflow & Gates + U6 Deploy Workflow (U6 needs Infrastructure Design first) -- **Status**: Awaiting Round 2 code approval. Rounds 1 and 2 committed, nothing pushed +- **Status**: Round 2 **closed** 2026-07-28. Rounds 1 and 2 committed, nothing pushed. Awaiting approval to start Round 3 ## Round 2 Design Record (2026-07-28) - Functional Design U3 + U4 complete and committed (`357d395`) @@ -108,11 +108,15 @@ Execution rounds (Q4 = B): **R1** = U1 + U2 · **R2** = U3 + U4 · **R3** = U5 + ## Round 2 Verification Record (2026-07-28) - `dotnet build SlpModularCms.sln -c Release` — 0 errors (70 warnings, all pre-existing package advisories) -- Backend tests — **366 passed, 0 failed** (Core 196, Availability 82, Master 51, Identity 37); was 253 after Round 1 +- Backend tests — **372 passed, 0 failed** at stage close (Core 196, Availability 82, Master 57, Identity 37); was 253 after Round 1, 366 after U4, +6 from the integrity-check fix - Frontend tests — **237 passed, 0 failed**; baseline 213 - `npx tsc -b` clean; eslint on every changed frontend file reports 0 problems; full `pnpm run lint` unchanged at the pre-existing 5 errors / 1 warning (FR-21, U5) - `Sentry.AspNetCore` 6.8.0 ships a native **`net10.0`** asset — the carried-forward compatibility question is closed - `@sentry/react` 10.68.0; `pnpm-lock.yaml` diff is additions only - **Two findings**: `z.string().url()` accepts `htp://` in Zod 4 (URL constructor accepts any scheme), so the pre-existing frontend validation never caught the typo BR-U4-24 names — now `z.url({ protocol: /^https?$/ })`. And `appsettings.json` comments are verified by `DeployedConfigurationTests` against the real provider rather than assumed, because the failure mode is both hosts refusing to start - **One deviation**: `IAdminTokenValidator` collapsed to a single `Validate` → `AdminTokenResult` instead of adding an overload. Two methods with an invisible difference at the call site let a substitute silently invert the access decision while both compiled — see U4's `generation-summary.md` +- **Two defects found in local testing after Round 2, both fixed in this branch** rather than filed, per the standing rule that tech debt is for large or high-impact changes only: + 1. Ciphertext written before U2 cannot be decrypted by the database key ring (`980dc80`). Recorded as ASM-08; local rows cleared and re-registered + 2. `VerifyIntegrityAsync` could not tell an unreachable slave from one that does not recognise the master, so the only recoverable state was never repaired (`6957ec7`). Now four distinct outcomes; automatic registration on a rejected key is safe because the slave refuses any key that does not match an existing registration +- **Note**: fix 2 is master/slave **domain** behaviour, not deployment work. It sits in this branch by explicit decision, not because it belongs to the feature - **Carried to phase-level Build and Test**: trace-ID propagation master → slave, `TraceId` present in rendered console output, tunnel status codes, the `security_event` tag on a real Sentry event, threshold behaviour end to end, and CSP/HSTS header presence on real static assets and error responses diff --git a/aidlc-docs/features/gitea-deployment-workflow/audit.md b/aidlc-docs/features/gitea-deployment-workflow/audit.md index d9b68aa..6e68725 100644 --- a/aidlc-docs/features/gitea-deployment-workflow/audit.md +++ b/aidlc-docs/features/gitea-deployment-workflow/audit.md @@ -947,3 +947,83 @@ Also noted: `MigrationFailure` uses synchronous `SentrySdk.Flush`, because `Migr **No blocking security findings. No new deviation.** --- + +## 2026-07-28 — CONSTRUCTION: Round 2 closed, plus two fixes found in local testing + +Round 2 (U3 + U4) is complete. Running both hosts afterwards surfaced two defects that were not +part of the unit scope. Both were small and contained, so both were fixed in this branch rather +than filed — per the user's standing rule that tech debt is reserved for large or high-impact +changes and code should be left better than it was found. + +### Fix 1 — the key ring move leaves older ciphertext unreadable + +Covered in full in the previous entry and in `u2-data-durability/code/generation-summary.md`. +Recorded as **ASM-08**; no migration ships because the user confirmed nothing is deployed yet. +Local state was repaired by clearing the stale registration rows on both sides. + +### Fix 2 — the integrity check could not distinguish "down" from "does not know us" + +**Symptom.** A slave registered with the frontend URL (`http://localhost:5174/`, the vite dev +server) instead of the API URL. The only log line was `Status push failed for slave `, and the +instance could not recover. + +**Cause.** `SlaveApiClient.GetRegisteredMasterUrlAsync` mapped four distinct outcomes onto a single +`null` — no HTTP response, a 404, a rejected key, and a genuine answer. `VerifyIntegrityAsync` +treated any `null` as "unreachable" and re-registered only when a *non-null* URL differed. A slave +with no registration returns exactly `null`, so the self-healing path could never repair the one +state that was actually repairable. Recovery required editing the database by hand. + +**Fix.** The call now returns `RegisteredMasterUrlResult` carrying a `SlaveContactOutcome`: + +| Outcome | Integrity check behaviour | +|---|---| +| `Unreachable` | Mark failed, retry next tick — unchanged | +| `NotAProtocolEndpoint` (404) | Mark failed, and log that the host does not serve the protocol | +| `Unauthorized` (401) | **Register** — repairs a slave that has no registration | +| `Ok` + differing master URL | Re-register — unchanged | +| `Ok` + matching | Healthy | + +The 404 branch is the one that would have saved the diagnosis time: it names the actual mistake — +a URL pointing at something other than an instance's API — instead of hiding it behind a generic +contact failure. + +**Why automatic registration on a rejected key is safe.** The slave accepts a registration only when +it has none, and refuses any key that does not match an existing one +(`MasterAvailabilityService.RegisterAsync`). Registration therefore succeeds exactly for a slave +that was never registered and fails harmlessly for one that belongs to another master — no takeover +is possible. **The guarantee lives on the slave, not the master**, which makes it easy to weaken by +accident, so `RegisterAsync_ReturnsFalse_WhenKeyMismatch` now states in its documentation that the +master's automatic registration depends on it. + +**Scope note.** This is master/slave domain behaviour, not deployment work — the same separation the +user drew when correcting the earlier framing of availability as a health check. It sits in this +branch by the user's explicit decision (no separate branch), and should be read as an in-passing +fix rather than part of the feature. + +### Verification at stage close + +- `dotnet build SlpModularCms.sln -c Release` — 0 errors +- Backend tests — **372 passed, 0 failed** (Core 196, Availability 82, Master 57, Identity 37) +- Frontend tests — **237 passed, 0 failed** +- `npx tsc -b` clean; eslint clean on all changed frontend files; full `pnpm run lint` unchanged at + the pre-existing 5 errors / 1 warning (FR-21, U5) + +### Commits on `feature/gitea-deployment-workflow` + +| Commit | Contents | +|---|---| +| `8568ca4` | Inception: reverse engineering, requirements, design, unit decomposition | +| `29a93ef` | U1 — website/admin split, `/health`, availability gate hardening | +| `5f3eda2` | U2 — key ring in the database, automatic migration, host wiring | +| `589146a` | README: sample password removed (pre-existing change, kept separate) | +| `357d395` | Functional design U3 + U4 | +| `5102f86` | NFR design U3 + U4; OPEN-01 closed, REF-U3-01 raised | +| `a122548` | U3 — security headers and CSP | +| `8e79a72` | U4 — logging, Sentry, tunnel, Umami, six security events | +| `980dc80` | ASM-08 — key ring migration gap recorded | +| `6957ec7` | Integrity check: distinguishes unreachable from unregistered | + +Nothing pushed. Next: Round 3 — U5 CI Workflow & Gates and U6 Deploy Workflow, the latter preceded +by Infrastructure Design. U5 carries REF-U3-01's Umami origin gate plus FR-21 and FR-22. + +---