From 8c7c605478ad0ab2da2001b7c5a57180a618706b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 05:54:40 -0400 Subject: [PATCH] =?UTF-8?q?docs(code-reviews):=20regenerate=20index=20?= =?UTF-8?q?=E2=80=94=206=20Critical=20findings=20resolved?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.7 (1M context) --- code-reviews/README.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/code-reviews/README.md b/code-reviews/README.md index aa7daf2..1d4ce3d 100644 --- a/code-reviews/README.md +++ b/code-reviews/README.md @@ -10,7 +10,7 @@ Each module's `findings.md` is the source of truth; this file is generated from | Module | Reviewer | Date | Commit | Status | Open | Total | |---|---|---|---|---|---|---| -| [Admin](Admin/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 12 | 12 | +| [Admin](Admin/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 10 | 12 | | [Analyzers](Analyzers/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 7 | 7 | | [Client.CLI](Client.CLI/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 10 | 10 | | [Client.Shared](Client.Shared/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 11 | 11 | @@ -18,9 +18,9 @@ Each module's `findings.md` is the source of truth; this file is generated from | [Configuration](Configuration/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 11 | 11 | | [Core](Core/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 12 | 12 | | [Core.Abstractions](Core.Abstractions/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 8 | 8 | -| [Core.AlarmHistorian](Core.AlarmHistorian/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 11 | 11 | +| [Core.AlarmHistorian](Core.AlarmHistorian/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 10 | 11 | | [Core.ScriptedAlarms](Core.ScriptedAlarms/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 12 | 12 | -| [Core.Scripting](Core.Scripting/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 11 | 11 | +| [Core.Scripting](Core.Scripting/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 10 | 11 | | [Core.VirtualTags](Core.VirtualTags/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 13 | 13 | | [Driver.AbCip](Driver.AbCip/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 15 | 15 | | [Driver.AbCip.Cli](Driver.AbCip.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 8 | 8 | @@ -29,7 +29,7 @@ Each module's `findings.md` is the source of truth; this file is generated from | [Driver.Cli.Common](Driver.Cli.Common/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 6 | 6 | | [Driver.FOCAS](Driver.FOCAS/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 12 | 12 | | [Driver.FOCAS.Cli](Driver.FOCAS.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 5 | 5 | -| [Driver.Galaxy](Driver.Galaxy/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 14 | 14 | +| [Driver.Galaxy](Driver.Galaxy/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 13 | 14 | | [Driver.Historian.Wonderware](Driver.Historian.Wonderware/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 12 | 12 | | [Driver.Historian.Wonderware.Client](Driver.Historian.Wonderware.Client/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 10 | 10 | | [Driver.Modbus](Driver.Modbus/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 12 | 12 | @@ -40,7 +40,7 @@ Each module's `findings.md` is the source of truth; this file is generated from | [Driver.S7.Cli](Driver.S7.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 7 | 7 | | [Driver.TwinCAT](Driver.TwinCAT/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 16 | 16 | | [Driver.TwinCAT.Cli](Driver.TwinCAT.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 7 | 7 | -| [Server](Server/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 15 | 15 | +| [Server](Server/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 14 | 15 | ## Pending findings @@ -48,12 +48,6 @@ Findings with status `Open` or `In Progress`, ordered by severity. | ID | Severity | Category | Location | Description | |---|---|---|---|---| -| Admin-001 | Critical | Security | `Components/Routes.razor:4-11`, `Program.cs:150` | The router uses a plain `RouteView` (not `AuthorizeRouteView`), and `MapRazorComponents()` is registered without `.RequireAuthorization()`. A page-level `[Authorize]` attribute on a routable Razor component is only enforced when the r… | -| Admin-002 | Critical | Security | `Components/Pages/Clusters/NewCluster.razor:1-7`, `Home.razor`, `Fleet.razor`, `Hosts.razor`, `AlarmsHistorian.razor`, `Clusters/ClustersList.razor`, `Clusters/Generations.razor`, `Drivers/FocasDetail.razor` | Several routable pages carry no authorization attribute at all. Most critically `NewCluster` (`/clusters/new`) is a mutating page — its `CreateAsync` writes a new `ServerCluster` row and a draft generation. Combined with Admin-001 (the rou… | -| Core.AlarmHistorian-001 | Critical | Correctness & logic bugs | `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:255-278` | `ReadBatch` builds two parallel lists, `rowIds` and `events`, that `DrainOnceAsync` later indexes together (`rowIds[i]` paired with `outcomes[i]`, where `outcomes` is 1:1 with `events`). But `rowIds.Add(reader.GetInt64(0))` runs unconditio… | -| Core.Scripting-001 | Critical | Security | `ForbiddenTypeAnalyzer.cs:45`, `ScriptSandbox.cs:54` | `System.Environment` lives in the allowed `System` namespace (it is in `System.Private.CoreLib`, which is allow-listed for primitives) and is not on the forbidden-namespace deny-list. Nothing prevents an operator-authored script from calli… | -| Driver.Galaxy-001 | Critical | Error handling & resilience | `Runtime/EventPump.cs:128`, `GalaxyDriver.cs:222` | The `ReconnectSupervisor` is constructed in `BuildProductionRuntimeAsync` and exposes `ReportTransportFailure(Exception)` as the only entry point that starts the reopen -> replay recovery loop. Nothing in the driver ever calls `ReportTrans… | -| Server-001 | Critical | Correctness & logic bugs | `src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs:1791` | `WriteNodeIdUnknown` calls itself unconditionally as its first statement, then sets `errors[i]`. Unbounded recursion with no base case overflows the stack. Called from all four `HistoryRead*` overrides whenever a HistoryRead targets a node… | | Admin-003 | High | Security | `Program.cs:137-139`, `Hubs/FleetStatusHub.cs:11`, `Hubs/AlertHub.cs:10`, `Hubs/ScriptLogHub.cs:30` | All three SignalR hubs (`/hubs/fleet`, `/hubs/alerts`, `/hubs/script-log`) are mapped with no `[Authorize]` attribute and no `.RequireAuthorization()` on the `MapHub` call. Any unauthenticated client can open a hub connection: `FleetStatus… | | Admin-004 | High | Security | `appsettings.json:3,13-14` | The checked-in `appsettings.json` contains live-looking secrets in plaintext: the `ConfigDb` connection string with `User Id=sa;Password=OtOpcUaDev_2026!` and the LDAP `ServiceAccountPassword: "serviceaccount123"`. It also sets `Encrypt=Fa… | | Admin-005 | High | Correctness & logic bugs | `Components/Pages/Login.razor:15,107-110` | `Login.razor` is an interactive component (the project default render mode is interactive server; the page declares no `@rendermode` but uses `EditForm`/`InputText` interactive binding and runs `SignInAsync` from an event handler). It call… | @@ -387,4 +381,11 @@ Findings with status `Open` or `In Progress`, ordered by severity. Findings with status `Resolved`, `Won't Fix`, or `Deferred`. -_No closed findings._ +| ID | Severity | Status | Category | Location | +|---|---|---|---|---| +| Admin-001 | Critical | Resolved | Security | `Components/Routes.razor:4-11`, `Program.cs:150` | +| Admin-002 | Critical | Resolved | Security | `Components/Pages/Clusters/NewCluster.razor:1-7`, `Home.razor`, `Fleet.razor`, `Hosts.razor`, `AlarmsHistorian.razor`, `Clusters/ClustersList.razor`, `Clusters/Generations.razor`, `Drivers/FocasDetail.razor` | +| Core.AlarmHistorian-001 | Critical | Resolved | Correctness & logic bugs | `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:255-278` | +| Core.Scripting-001 | Critical | Resolved | Security | `ForbiddenTypeAnalyzer.cs:45`, `ScriptSandbox.cs:54` | +| Driver.Galaxy-001 | Critical | Resolved | Error handling & resilience | `Runtime/EventPump.cs:128`, `GalaxyDriver.cs:222` | +| Server-001 | Critical | Resolved | Correctness & logic bugs | `src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs:1791` |