From 6c1f05cba95563b7fd3f426d8c596e5b7b86343c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:07:27 -0400 Subject: [PATCH] docs(r2-05): flip 04/C-1 to FIXED, record execution deviations + task status --- archreview/04-adminui.md | 2 +- archreview/plans/R2-05-adminui-authz-plan.md | 36 +++++++++++++++++++ .../R2-05-adminui-authz-plan.md.tasks.json | 6 ++-- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/archreview/04-adminui.md b/archreview/04-adminui.md index a94e689a..9b790c74 100644 --- a/archreview/04-adminui.md +++ b/archreview/04-adminui.md @@ -24,7 +24,7 @@ | P-4 (Low) | `Deployments` runs a full config snapshot + hash per page load for the drift badge | **STILL OPEN** — `Deployments.razor:98` `SnapshotAndFlattenAsync` | | P-5 (Low) | `Alerts` rows lack `@key` while prepending; no `` (bounded by design) | **STILL OPEN** — `Alerts.razor:63–70` `` without `@key` | | P-6 (Low) | `inlay-hints` round-trips for a documented no-op | **STILL OPEN** — `monaco-init.js:99–117` provider active; `ScriptAnalysisService.cs:415` returns empty | -| C-1 (High) | Three authorization idioms; the largest mutating surface is bare `[Authorize]` | **STILL OPEN** — re-enumerated every page attribute: only `Deployments`/`Scripts`/`ScriptEdit` carry `Roles="Administrator,Designer"`, only `RoleGrants` carries `Policy="FleetAdmin"`; `GlobalUns`, `EquipmentPage`, all `Clusters/*` editors, all 8 driver pages, and `Reservations` remain bare `[Authorize]`. No remediation branch targeted this (STATUS.md action-list item 5, unscheduled) | +| C-1 (High) | Three authorization idioms; the largest mutating surface is bare `[Authorize]` | **FIXED (R2-05, branch `r2/05-adminui-authz`)** — one idiom now: all 38 routable pages carry an explicit named-policy `@attribute` via new `AdminUiPolicies` constants. New `ConfigEditor` (Administrator+Designer) gates the 20-page config-authoring surface incl. all 8 driver pages (live `ResilienceConfig`) + `/api/script-analysis/*`; new `AuthenticatedRead` on the 16 read pages; `FleetAdmin`/`DriverOperator` literals→constants (unchanged semantics). Anti-drift reflection guard `PageAuthorizationGuardTests` + `AdminUiPoliciesTests` semantics matrix (both green). **Two classification corrections vs. this row:** `Reservations` and `ClusterRedundancy` are read-only (→`AuthenticatedRead`, not `ConfigEditor`); `Home` carried no attribute at all (now `AuthenticatedRead`). Offline gate green; docker-dev live positive/regression pass deferred (auto-admin can't observe deny). | | C-2 (Med) | CLAUDE.md said ScriptAnalysis was `FleetAdmin`-gated; code uses `Roles="Administrator,Designer"` | **FIXED** — docs branch `9fadead6` (merged to master via `b67bd9e8`) corrected CLAUDE.md, which now reads "gated to the `Administrator,Designer` roles via `RequireAuthorization`" (CLAUDE.md:223); code unchanged and still correct | | C-3 (Good) | Driver-typed tag-editor pattern complete and consistent | **STILL HOLDS** — map/validator/editors unchanged | | C-4 (Good) | Numeric-enum serialization bug class closed and test-pinned | **STILL HOLDS** — pages + `*FormSerializationTests` unchanged | diff --git a/archreview/plans/R2-05-adminui-authz-plan.md b/archreview/plans/R2-05-adminui-authz-plan.md index 22cc3dce..55836601 100644 --- a/archreview/plans/R2-05-adminui-authz-plan.md +++ b/archreview/plans/R2-05-adminui-authz-plan.md @@ -573,3 +573,39 @@ record the two verification corrections (Reservations/ClusterRedundancy read-onl --- **Total: 16 tasks · overall effort Small-Medium (~2.5–4 h including the live pass).** + +--- + +## Execution deviations (R2-05) + +Executed 2026-07-13 on branch `r2/05-adminui-authz` (off master `1676c8f4`, not `f6eaa267` — the plan's +stated base; the two commits between are docs-only merges with zero AdminUI source change, so the census +re-verified identical). Commits: `183b72b7` (T1/2/4) · `b5bf4b73` (T3/5–11, the gate) · `aac32854` (T12 +literals) · `dad78351` (T13 docs) · plus this bookkeeping commit (T16). + +- **DriverOperator role string uses `DevAuthRoles.Operator` constant, not the `"Operator"` literal.** In + `ServiceCollectionExtensions.cs` the `RequireRole("Operator", …)` now reads `RequireRole(DevAuthRoles.Operator, …)`. + Reversible, zero behavior change (same string) — small consistency win beyond the plan's letter, which only + called for switching the *policy name* to a constant. `"Administrator"`/`"Designer"` role strings kept literal + (no constant exists for them; out of scope). +- **Guard test: two page types fully-qualified.** `typeof(Hosts)` and `typeof(Certificates)` collide with + sibling namespaces (`…Pages.Hosts`, a folder namespace exists), so those two entries use + `typeof(global::…Components.Pages.Hosts)` / `…Certificates`. The other 36 resolve via `@using`. No + `` was needed — `RouteAttribute`/`AuthorizeAttribute`/`AllowAnonymousAttribute` all + resolved transitively through the AdminUI RCL reference (the plan flagged this as a possible add). +- **`MapScriptAnalysis` → `MapScriptAnalysisEndpoints`.** The plan (and the old CLAUDE.md sentence) named the + method `MapScriptAnalysis`; the actual method is `MapScriptAnalysisEndpoints`. Corrected the doc references + to the real name while converging its literal to `AdminUiPolicies.ConfigEditor`. +- **T14 full-solution sweep run once, then constrained off.** A single `dotnet test ZB.MOM.WW.OtOpcUa.slnx` + ran before the controller's mid-task constraint (no whole-solution / no `*.IntegrationTests` — heavy suites + leak ~16 GB). All failures were triaged as pre-existing/environmental and **outside the R2-05 diff**: + Core.Abstractions.Tests `InterfaceIndependence` flags pre-existing `Historian.*` namespace types; a Runtime + historian-retry timing test; OpcUaClient.Browser needs a live OPC endpoint; AbCip/OpcUaServer/Host + IntegrationTests need Docker fixtures. The authoritative gate is the impacted unit suites — **AdminUI.Tests + 517/517 and Security.Tests 80/80, both green.** The sweep will not be re-run per the constraint. +- **T15 live `/run` deferred-live.** Marked `deferred-live` ("docker-dev live authz /run; serial live pass"). + Two reasons: (1) it is a heavy integration/browser-drive pass the controller runs serialized; (2) docker-dev + `DisableLogin=true` auto-authenticates as a full-access admin, so it can only regression-check over-gating, + never observe a role deny — the negative/deny proof is entirely CI-side (T2 + T3, green). +- **T16 does not merge.** Executor scope forbids merge/push/PR; the branch is left ready. Bookkeeping (04/C-1 + row flipped to FIXED, the two read-only classification corrections recorded, STATUS.md R2-05 row) is done. diff --git a/archreview/plans/R2-05-adminui-authz-plan.md.tasks.json b/archreview/plans/R2-05-adminui-authz-plan.md.tasks.json index c2d49094..58d12f0e 100644 --- a/archreview/plans/R2-05-adminui-authz-plan.md.tasks.json +++ b/archreview/plans/R2-05-adminui-authz-plan.md.tasks.json @@ -15,8 +15,8 @@ { "id": "T11", "subject": "Guard GREEN checkpoint + full AdminUI.Tests; commit the gate", "status": "completed", "blockedBy": ["T3", "T6", "T7", "T8", "T9", "T10"] }, { "id": "T12", "subject": "Converge non-page literals to constants (ScriptAnalysisEndpoints, imperative DriverOperator x4, Certificates FleetAdmin x2)", "status": "completed", "blockedBy": ["T11"] }, { "id": "T13", "subject": "Docs: CLAUDE.md ScriptAnalysis gate sentence, docs/security.md, STATUS.md R2-05 row", "status": "completed", "blockedBy": ["T11"] }, - { "id": "T14", "subject": "Full-solution test sweep (dotnet test ZB.MOM.WW.OtOpcUa.slnx)", "status": "pending", "blockedBy": ["T12", "T13"] }, - { "id": "T15", "subject": "Live /run positive pass on docker-dev :9200 (rebuild BOTH centrals; auto-admin drives all page tiers; negative proof stays in T2/T3)", "status": "pending", "blockedBy": ["T14"] }, - { "id": "T16", "subject": "Merge + bookkeeping (flip 04/C-1 row, record Reservations/ClusterRedundancy corrections)", "status": "pending", "blockedBy": ["T15"] } + { "id": "T14", "subject": "Full-solution test sweep (dotnet test ZB.MOM.WW.OtOpcUa.slnx)", "status": "completed", "blockedBy": ["T12", "T13"], "note": "Impacted unit suites authoritative + green: AdminUI.Tests 517/517, Security.Tests 80/80. One whole-solution sweep ran BEFORE the controller's no-whole-solution/no-IntegrationTests constraint arrived; all failures are pre-existing/env and outside the R2-05 diff (Core.Abstractions.Tests InterfaceIndependence flags pre-existing Historian.* namespace types; Runtime historian retry timing; OpcUaClient.Browser needs a live OPC endpoint; AbCip/OpcUaServer/Host IntegrationTests need Docker fixtures). Will NOT be re-run per constraint." }, + { "id": "T15", "subject": "Live /run positive pass on docker-dev :9200 (rebuild BOTH centrals; auto-admin drives all page tiers; negative proof stays in T2/T3)", "status": "deferred-live", "blockedBy": ["T14"], "note": "docker-dev live authz /run; serial live pass. Heavy integration/Playwright-dependent gate — controller runs serialized. docker-dev DisableLogin=true auto-authenticates as full-access admin, so role-deny differentiation is NOT observable there; the negative proof is CI-side (T2 AdminUiPoliciesTests + T3 PageAuthorizationGuardTests, both green). This pass only regression-checks over-gating (a typo'd policy name would render the deny slot even for the auto-admin)." }, + { "id": "T16", "subject": "Merge + bookkeeping (flip 04/C-1 row, record Reservations/ClusterRedundancy corrections)", "status": "completed", "blockedBy": ["T15"] } ] }