Files
mxaccessgw/archreview/2026-08-19/40-security-dashboard.md
Joseph Doherty f4919ba97c
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m19s
ci / java (push) Successful in 2m25s
ci / portable (push) Successful in 8m21s
docs(archreview): cycle-3 architecture review (2026-08-19 @ 8b3e3b3)
Six domain reports + overall synthesis. All 47 cycle-2 Done claims verified
at HEAD with zero false Dones and zero regressions; 34 new findings
(0 Critical, 0 High, 4 Medium, 25 Low, 5 Info): GWC-31..38, WRK-32..34,
IPC-33..39, SEC-37..41, CLI-46..50, TST-31..36.
2026-08-20 09:10:12 -04:00

111 lines
30 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Security & Dashboard — Re-review (Cycle 3)
**Date:** 2026-08-19 · **Commit:** `8b3e3b3` (`main`, clean) · **Baseline:** `4f5371f` (2026-07-12 cycle) · **Method:** static review on the macOS working tree (no builds, no runtime probes, no source modifications). Every claim cites a `path:line` read at HEAD. Prior findings are the `SEC-NN` IDs from `archreview/remediation/40-security-dashboard.md` (cycle 1) and `archreview/2026-07-12/40-security-dashboard.md` / `archreview/2026-07-12/remediation/40-security-dashboard.md` (cycle 2); new findings continue from SEC-37. This domain absorbed the most new machinery since the baseline (~55 of ~247 commits touched Security/Dashboard/Configuration): the SEC-31/32 failure-limiter re-partition, SEC-33/34/36 remediations, the SEC-25 tag-ACL epic (`DashboardTags` constraints, session tag inheritance, `GroupToTag`/`UntaggedSessionVisibility`, events-hub + in-process ACL), the `ShowTagValues` three-seam closure, the secrets-store adoption (`${secret:}` expansion, `/admin/secrets`), the bounded async audit writer with retention, and the snapshot-feed/alarm-page reliability rework.
## Prior-finding verification
### Cycle-2 findings (SEC-31…SEC-36)
| ID | Claimed | Verified? | Evidence (file:line at HEAD) | Notes |
|----|---------|-----------|------------------------------|-------|
| SEC-31 | Done | **Yes** | Composite `(transport peer, key id)` partition + cross-peer aggregate + probe admission, exactly per the remediation design: `Security/Authorization/ApiKeyFailureLimiter.cs:14-27` (two-layer remarks), `:143-204` (`Check` with probe claim/compensation), `:206-235` (`RecordFailure` into both layers), `:237-276` (`Reset` reachable-while-throttled); interceptor threads the partition and reuses the check-time resolution (`GatewayGrpcAuthorizationInterceptor.cs:83-115`, `:133-137`); new knobs validated (`SecurityOptions.ApiKeyFailureAggregateLimit`/`ApiKeyFailureProbeIntervalSeconds`, wired at `ApiKeyFailureLimiter.cs:74-83`); throttle metric tagged by stage only (`Metrics/GatewayMetrics.cs:350-356`, with an explicit SEC-14/SEC-20 cardinality comment). `ApiKeyFailureLimiterTests.cs` exists. The lockout inversion is structurally closed: a correct secret always reaches the verifier via a probe slot and resets both layers. | Probe-claim compensation is version-stamped (`:341-383`), closing the same-tick stomp the follow-up commits (`5b681ee`, `acebe18`) addressed. Two new residual corners found in the rework — see **SEC-38** (aggregate-layer bypass via the per-peer key-id cap) and **SEC-39** (null==null fallback-partition reset). Neither reopens the SEC-31 lockout: they weaken bounds, they do not deny the legitimate holder. |
| SEC-32 | Done | **Yes** | Token shape checked before a key-id partition is minted: `GatewayGrpcAuthorizationInterceptor.cs:149-193` (`TryResolveKeyId` — `mxgw` must be the entire first segment, key id ≤ 64 chars, non-empty third segment; internal for direct test pinning). Per-peer key-id cap `MaxKeyIdPartitionsPerPeer = 32` with overflow collapsing to the address's fallback partition (`ApiKeyFailureLimiter.cs:48-53`, `:393-444`). Eviction never removes load-bearing state below a 2× ceiling and prefers expired windows (`:479-548`). | The junk-token flush is closed as designed. The cap's overflow-collapse rule has a side effect the design did not anticipate — see **SEC-38**. |
| SEC-33 | Done | **Yes** | `IsRootedForAnyPlatform` is gone; rooting is host-meaningful `Path.IsPathRooted` in the shared `Configuration/GatewayConfigPathRules.cs:14-39` (used by both validators, with the SEC-01/SEC-33 mechanism named in the doc-comment). `appsettings.json` no longer carries either Windows literal (Authentication section `:20-24` has no `SqlitePath`; Galaxy section `:79-84` has no `SnapshotCachePath`). Gateway-side Galaxy default seeded pre-bind (`GatewayApplication.cs:156-168` — the init-only-property deviation is documented in place) and validated by the new `Configuration/GalaxyRepositoryOptionsValidator.cs:44-72` registered with `ValidateOnStart` (`GatewayApplication.cs:172-176`). No stray `C:*` artifact under `src/` (`find`-verified). | Went beyond the finding: `882c7ca` added `AddIfUnderContentRoot` (`GatewayConfigPathRules.cs:93-124`) after a real 2026-08-09 production key-loss incident (store inside the renamed app directory), and the secrets store got the same absolute default treatment (`GatewayApplication.cs:201-239`). |
| SEC-34 | Done | **Yes** | Invalidate race closed with the per-key generation counter: bump-before-evict (`Security/Authentication/CachingApiKeyVerifier.cs:181-199`), snapshot-before-inner + set-then-recheck with self-evict (`:128-141`, `:147-178`); `TryParseKeyId` correctness argument (full key id, `_` forbidden by both key-creation paths) documented at `:205-218`. Expiry window took the documented fallback per the recorded outcome: the library verification identity carries no `ExpiresUtc`, so the ≤ TTL expiry window is documented in the remarks (`:49-60`) with the donor-library ask. | Matches the remediation outcome exactly, including the deviation. Cache-key design unchanged (SHA-256 of the token, success-only). |
| SEC-35 | N/A (doc-only) | **Yes** | `docs/GatewayConfiguration.md:282` — the "hard-stops key on the exact environment name" contract is documented (both guards fire only on `IHostEnvironment.IsProduction()`). Validator unchanged: `GatewayOptionsValidator.cs:38`, `:211-214` (LDAP), `:387-391` (DisableLogin). | Note the consequence is now live: windev runs as `Staging` (NEXT-07 relabel), so neither guard applies there — that is the posture gap promoted as **SEC-37**. |
| SEC-36 | Done | **Yes (repo side + rotation executed)** | `appsettings.json:33` ships `"ServiceAccountPassword": "${secret:ldap/mxgateway/bind}"` — a fail-closed encrypted-store reference expanded pre-host (`GatewayApplication.cs:75-106`), not a literal. `git grep` for the retired value returns nothing in tracked files. `glauth.md` samples use placeholders; `docs/runbooks/SEC-36-ldap-credential-rotation.md` records the 2026-08-07 rotation (GLAuth `passsha256` replaced in `scadaproj` commit `aada53b`, instance recreated, dashboard `/login` verified through the real `DashboardAuthenticator` path post-NEXT-07). `<UserSecretsId>mxaccessgw-server</UserSecretsId>` present in the csproj; the blank-password validator message names both channels. | The load-bearing half (rotation) is done: the historically recoverable value no longer binds `dc=zb,dc=local`. Fully closed. |
### Cycle-1 residuals (still-open SEC-13…SEC-27 era)
| ID | Prior status | At HEAD | Evidence | Notes |
|----|--------------|---------|----------|-------|
| SEC-13 | Open | **Open, impact reduced** | `Diagnostics/GatewayLogRedactor.cs:11-17` still lists only `AuthenticateUser`/`WriteSecured`/`WriteSecured2` — the `WriteSecuredBulk`/`WriteSecured2Bulk` command kinds (`Grpc/MxAccessGatewayService.cs:364-381`) are still missing. However the redactor is no longer latent: `GatewayLogRedactorSeam` is registered as the telemetry `ILogRedactor` (`GatewayApplication.cs:139`) and masks **every** non-null `CommandValue` because the seam exposes no value-logging opt-in (`Diagnostics/GatewayLogRedactorSeam.cs:38-56`; honestly documented at `docs/Diagnostics.md:120`). The Bulk omission only bites if a future opt-in knob passes `valueLoggingEnabled: true`. | Fix the two missing names when (if) the opt-in knob lands; until then fail-closed. |
| SEC-14 | Open | **Confirmed open** | `GatewayApplication.cs:347-348` — `MapZbHealth()`/`MapZbMetrics()` with no `RequireAuthorization`. | Mitigating discipline held: the new throttle counter deliberately carries no key/peer tags because "/metrics is not authenticated (open SEC-14)" (`GatewayMetrics.cs:352-354`). Note `/health/active` has off-repo consumers (family Overview dashboard), so any auth change needs cross-repo coordination. |
| SEC-15 | Open (accepted) | **Confirmed open** | GET `/logout` still skips antiforgery, intentional (`Dashboard/DashboardEndpointRouteBuilderExtensions.cs:92-99`). | Accepted risk; unchanged. |
| SEC-16 | Open | **Confirmed open** | `Program.cs:37-39` still maps `command.Pepper` → `MxGateway:ApiKeyPepper` (pepper on the CLI command line). | |
| SEC-17 | Open | **Confirmed open** | `GatewayGrpcAuthorizationInterceptor.cs:27-54` overrides only unary + server-streaming. All current RPCs remain those shapes. | |
| SEC-18 | Open | **Confirmed open** | `Dashboard/DashboardApiKeyManagementService.cs:22,289` — `"pepper unavailable"` message-substring match. | |
| SEC-19 | Open | **Resolved** (this period) | `Security/Audit/SqliteCanonicalAuditStore.cs:23-31` — the `CREATE TABLE IF NOT EXISTS` is latched: `AuditDrainService` runs `EnsureInitializedAsync` once at startup and later operations skip the DDL round-trip (self-bootstrap kept for the CLI/DI-free paths; the latch is deliberately racy-idempotent). Landed with the `e2ac5d1`/`7b2d046` audit rework (bounded async writer, batched inserts). | |
| SEC-21 | Open | **Resolved** (this period) | `Dashboard/DashboardSnapshotService.cs:25,38,42,287-310` — API-key summaries refresh at most every 15 s behind a zero-timeout gate with a 2 s store timeout, and the snapshot tick itself is idle-gated (`77c5731`, `DashboardSnapshotFeed` idle gate). The per-tick unconditional store read is gone. | |
| SEC-23 | Open | **Confirmed open** | `GatewayOptionsValidator.ValidateDashboard` (`:380-455`) gained GroupToRole/GroupToTag/UntaggedSessionVisibility shape checks and the Production `DisableLogin` stop, but still ignores `AutoLoginUser`, `RequireHttpsCookie`, and `CookieName` — an explicit `CookieName` starting `__Host-` with `RequireHttpsCookie=false` is still applied unconditionally (`DashboardServiceCollectionExtensions.cs:154-158`) with no validator diagnosis. | The runtime guard against *deriving* `__Host-` without Secure is correct (`:159-162`); only the explicit-override consistency check is missing. |
| SEC-24 | Open | **Confirmed open (narrowed)** | `Configuration/GatewayConfigurationProvider.cs:58-74` / `EffectiveDashboardConfiguration.cs:3-12` — the projection gained `ShowTagValues`, `GroupToRole`, `GroupToTag` (with comparer propagation, `ff16c69`), and `UntaggedSessionVisibility`, and the settings page renders them (`SettingsPage.razor:53-104`); it still omits `DisableLogin`/`AutoLoginUser`/`RequireHttpsCookie`/`CookieName` and the `MxGateway:Security` limiter keys the SEC-31 design asked to absorb here. Secrets stay redacted (`:20,30`). | |
| SEC-26 | Open | **Resolved** (this period) | Retention sweep exists: `SqliteCanonicalAuditStore.cs:172` (`DELETE FROM audit_event` with a documented NULL-timestamp keep rule at `:251-253`), driven by `SecurityOptions.AuditRetentionDays` (default 90, `SecurityOptions.cs:96-99`) from the audit drain service. | |
| SEC-27 | Open | **Confirmed open** | `DashboardSnapshotService.cs:17,114` — `GatewayStatus` is still the hardcoded `"Healthy"` constant. | |
### SEC-25 epic verification (the tag-ACL machinery is real and fail-closed)
Not a table row above because cycle 2 closed SEC-25 as "mitigation, ACL deferred"; the deferred ACL has now shipped and was reviewed as new code:
- **Tags come from the owning key, never the request:** `Grpc/MxAccessGatewayService.cs:36-46` (owner identity from `identityAccessor`, `owner?.EffectiveConstraints.DashboardTags` into `OpenSessionAsync`); `Sessions/GatewaySession.cs:207-212,270` (immutable ordinal-ignore-case `FrozenSet`); `Security/Authentication/ApiKeyConstraints.cs:35-44` (defensive copy on init; rides the constraints blob to avoid a schema migration; explicitly visibility-only, never data-access).
- **ACL decision table:** `Dashboard/DashboardSessionAcl.cs:50-84` — Admin (authenticated) bypass; phantom session denied; untagged session follows `UntaggedSessionVisibility` (default `AdminOnly`, fail-closed); otherwise ordinal-ignore-case tag intersection over `DashboardTagClaimType` claims; `null` principal denied outright, even under `AllViewers` (`IDashboardSessionAcl.cs:25-32`). Anonymous localhost is an empty-grant Viewer.
- **Both subscribe seams gated identically:** `Dashboard/Hubs/EventsHub.cs:59-79` (ACL before group join, registry-before-group ordering, opaque `HubException`), and the session-details in-process seam `SessionDetailsPage.razor:321-361` (ACL before any field write or `Subscribe`). The `7ec0b35` re-entrancy fix is sound: the attach generation is claimed synchronously before the first await (`:217-233`), a stale continuation returns without attaching or detaching (`:340-343`), and the pump's `ReferenceEquals` batch guard drops cross-session batches on the dispatcher (`:383-407`).
- **Claims plumbing:** tags stamped at login (`DashboardAuthenticator.cs:114-158`) and re-resolved from group claims at hub-token mint so a token bounds grant staleness to 5 minutes (`HubTokenService.cs:80-98`); token `Validate` still rejects identity-less payloads (`:118-126`) and rehydrates tag claims for the hub-path ACL (`:139-145`).
- **Owner-scoped gRPC attach (the threat-model anchor) holds:** `Grpc/EventStreamService.cs:52-63` — reattach is `PermissionDenied` unless the caller's key id ordinal-equals the session owner; null owner matches only a null caller.
- **Redaction (second, independent layer) covers all three value seams:** events-hub mirror deep-clone (`Dashboard/Hubs/DashboardEventBroadcaster.cs:91,247-259` — top-level `value` plus alarm `CurrentValue`/`LimitValue`, source never mutated), alarms hub deep-clone on both value-bearing arms (`AlarmsHubPublisher.cs:59,103-126` — the cycle-2 closeout's named "alarms-hub redaction gap" residual, closed by `eff17d1`/TST-16 on 2026-08-17), and `/browse` live values at the service boundary with the `'-'`-placeholder-on-failed-reads fix (`DashboardLiveDataService.cs:132-140`; `1ea6f60`). `docs/GatewayConfiguration.md:195` documents all three seams and the "redaction applies to successful reads only" rule accurately.
- **Tests exist for the corners:** `Tests/Gateway/Dashboard/DashboardSessionAclTests.cs`, `EventsHubTests.cs`, `SessionDetailsPageEventAclTests.cs`, `EventsHubViewerRegistryTests.cs`, `HubTokenServiceTests.cs`, `AlarmsHubPublisherTests.cs`, `DashboardGroupTagMappingTests.cs`; live-LDAP ACL proof via `IntegrationTests/DashboardLdapLiveTests.cs:186-243` (real bind → real group claims → GroupToTag → ACL admit/deny, plus Admin bypass).
- **Docs honesty:** the short-name `GroupToRole`/`GroupToTag` key constraint (full-DN keys can never match the pre-stripped groups) is stated at the mapping site (`DashboardGroupTagMapping.cs:8-13,41-47`) and in the class summaries (`db7b1db`, `4b6f907`).
## New findings
### SEC-37 · Medium (Security / deployment posture) — The shared GLAuth offers no TLS, so the Production plaintext-LDAP hard-stop is unsatisfiable for every host that authenticates against it (promotion of NEXT-08)
**Files:** `src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs:208-214`; `docs/GatewayConfiguration.md:264-265`; `glauth.md:30,357-360`; `archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md` (NEXT-08).
The SEC-06 guard refuses to start a `Production`-environment host with `Ldap:Transport=None`, and the config doc instructs "Deployed hosts must set `Ldaps` or `StartTls`" — but the only shared directory (`10.100.0.35:3893`) has LDAPS disabled, port 3894 closed, and answers StartTLS with `protocolError` (per the NEXT-07/08 record), and `glauth.md:30` documents "LDAPS | disabled in dev". Neither permitted value can work, so the instruction is unsatisfiable: every deployed gateway that binds this directory must either run plaintext (which `glauth.md:358-360` itself warns sends passwords cleartext) or avoid the `Production` label. That is exactly what happened — windev was relabelled `Staging` on 2026-08-07 to boot past the guard. The net posture is that the hard-stop protects no real deployment: hosts that would trip it are renamed around it, and the SEC-35 documentation note now formalizes the escape hatch. The dashboard admin surface (API-key CRUD, session Kill, secrets UI) sits behind credentials that transit this plaintext bind.
**Recommendation:** resolve the posture, in one of two written directions: (a) enable LDAPS (or StartTLS) on the shared GLAuth — certificate issuance plus a trust story on each gateway host, source of truth `scadaproj/infra/glauth/` — and flip the deployed hosts' `Transport`; or (b) record an explicit decision that production gateways bind a different, TLS-capable directory and that the shared GLAuth is dev/staging-only, and say so in `docs/GatewayConfiguration.md`'s `Transport` row and `glauth.md`. Until one lands, every "must use Ldaps/StartTls" sentence is aspirational and should be labeled as such. (Cross-repo: coordinate with the family per the umbrella-index rule.)
### SEC-38 · Low (Security) — The failure limiter's cross-peer aggregate layer can be sidestepped by pre-filling the per-peer key-id cap, restoring the `limit × addresses` guessing bound the layer was built to remove
**Files:** `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs:225-231` (aggregate fed only when `effectiveKeyId` is non-null), `:393-444` (`ResolvePartitionKey` returns the `(peer, null)` fallback once the address has minted `MaxKeyIdPartitionsPerPeer = 32` key ids), `:153-166` (`Check` consults the aggregate only via `effectiveKeyId`).
The SEC-32 cap collapses the 33rd-and-later key id from one address onto that address's fallback partition, and — per the comment's cardinality rationale — such failures deliberately do not feed the per-key-id aggregate. The side effect: an attacker who first mints 32 junk `mxgw`-shaped key-id partitions from an address (32 requests) makes every subsequent guess against the *victim's real key id* from that address resolve to `(peer, null)` — so those failures neither feed the victim's aggregate (`RecordFailure`, `:227`) nor consult it (`Check`, `:158-163`). A source-rotating attacker (the IPv6-/64 scenario the aggregate was designed for, per the SEC-31 remediation design's G1 analysis) thereby recovers `ApiKeyFailureLimit` guesses per window per address with the aggregate fully bypassed, at ~32 extra requests per address — instead of the designed bound of `ApiKeyFailureAggregateLimit` total followed by ~1 probe per interval globally. Severity is Low, not Medium: gateway secrets are library-generated high-entropy values, so online guessing remains computationally hopeless and the limiter is defense-in-depth plus store-read cost shielding (which still holds per-address); the finding is that the *documented* distributed-attack bound (`ApiKeyFailureLimiter.cs:17-19`, `docs/Authentication.md` limiter section) is not the actual bound.
**Recommendation:** feed (and consult) the aggregate for any *validly shaped* key id regardless of whether it earned a composite partition — the cardinality concern the comment cites is already handled by `_aggregates`' own capped eviction (`:234`), whose policy protects over-limit entries and preferentially evicts single-failure junk. Alternatively, count cap-overflow failures into the aggregate of the presented key id only when that aggregate already exists (zero new cardinality, closes the consult-bypass for hot keys). Update the class remarks and `docs/Authentication.md` either way.
### SEC-39 · Info (Security) — `Reset` treats "no key id" as owning the shared fallback partition: a verified token that fails the interceptor's key-id parse clears the address's fallback state the guard comment promises to preserve
**Files:** `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs:252-276`; `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GatewayGrpcAuthorizationInterceptor.cs:149-193`.
`Reset`'s ownership check is `string.Equals(effectiveKeyId, partition.KeyId)` (`:264`), which is true when both are null — and a partition with `KeyId = null` resolves to the transport-peer fallback key (`:398-403`), so the reset then `RemovePartition(peer)`s the *shared* fallback bucket, the exact wipe the comment above it (`:259-263`) explains must not happen because that bucket holds junk-spray failures from the same address. A null `KeyId` on a *successful* verification requires a token the library verifier accepts but `TryResolveKeyId` rejects — the identifiable shapes are a key id longer than 64 chars (`:178`) and a secret whose first character is `_` (`:187`, matching the old `Split`-based rejection); whether the shared verifier accepts either depends on library parsing not readable in this repo. Practical impact is minimal (the attacker must hold a *valid* key of an unusual shape merely to clear their own address's junk-spray throttle), hence Info — but the null==null branch contradicts the stated invariant and costs one line to close.
**Recommendation:** make the fallback non-resettable explicitly: `if (effectiveKeyId is not null && string.Equals(effectiveKeyId, partition.KeyId, StringComparison.Ordinal))`, and pin with a test (successful reset with a null-key-id partition leaves the fallback partition tracked).
### SEC-40 · Low (Security) — Alarm visibility is not tag-scoped: the session-less alarms hub (and alarms page) broadcasts galaxy-wide alarm metadata to every Viewer, including empty-grant Viewers and anonymous localhost
**Files:** `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHub.cs:12-25` (`[Authorize(HubClientsPolicy)]`, auto-join `__alarms__` on connect); `AlarmsHubPublisher.cs:48-63` (unfiltered `StreamAsync(alarmFilterPrefix: null)` re-broadcast to all clients); `Dashboard/DashboardAuthorizationHandler.cs:39-53` (loopback/disabled bypasses satisfy the Viewer requirement the hub policy uses); `docs/GatewayDashboardDesign.md:277` ("all clients receive every message").
The SEC-25 ACL scopes *session event* visibility by tag intersection, and CLAUDE.md/docs describe dashboard event visibility as tag-scoped — but the alarm feed is a parallel, session-less surface with no per-caller scoping at all: any principal satisfying `HubClientsPolicy` (any Viewer role, the `Authentication:Mode=Disabled` bypass, or anonymous localhost under the default `AllowAnonymousLocalhost=true`) receives every `AlarmFeedMessage` the central monitor emits. Values are stripped by default (`ShowTagValues`, verified above), but the surviving metadata — alarm reference (a full tag path), state, severity, category, area, and acknowledging-operator usernames — discloses process structure and operator activity for the whole subscription expression to callers the tag model would deny every single session. A Viewer with an empty tag grant, whom the ACL was explicitly designed to show nothing, still sees the full plant alarm picture. The alarm monitor being gateway-central (not per-session) means tag scoping does not map mechanically, so this may well be a deliberate posture — but no design doc states it against the ACL: `GatewayDashboardDesign.md:277` describes the fan-out without a scoping rationale, and the ACL plan (`docs/plans/2026-07-10-dashboard-session-acl-tst15.md`) does not carve alarms out.
**Recommendation:** decide and document. If deliberate, add one paragraph to the ACL section of `docs/GatewayDashboardDesign.md` (and the `GroupToTag` row of `docs/GatewayConfiguration.md`) stating that alarm metadata is intentionally visible to every dashboard Viewer and why (central monitor, no per-session provenance). If not, the cheap tightening is a dedicated policy on `AlarmsHub` (e.g. require a real role claim, excluding the anonymous-loopback bypass) or an area/tag-prefix filter derived from the caller's grant.
### SEC-41 · Low (Security) — Tag grants revoked by a `GroupToTag` config change survive on cookie principals across the restart that applies the change: the ACL trusts login-stamped tag claims for up to the 8-hour sliding cookie life, while the hub-token path re-resolves at mint
**Files:** `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs:145-149` (tags stamped as claims at login); `Dashboard/DashboardSessionAcl.cs:75-81` (ACL reads the *stamped* tag claims); `Dashboard/HubTokenService.cs:84-90` (token mint *re-resolves* tags from group claims against current options — the asymmetry); `Dashboard/DashboardServiceCollectionExtensions.cs:114-130` (`ZbCookieDefaults.Apply(..., idleTimeout: 8h)` — sliding, so an active session renews indefinitely).
`GatewayOptions` binds once at startup (no reload — noted at `DashboardSnapshotService.cs:45-49`), so changing `GroupToTag` requires a gateway restart. A restart tears down circuits and hub connections, but **not** the auth cookie: data-protection keys persist, and a browser returning after the restart re-authenticates from the cookie, whose `DashboardTagClaimType` claims were resolved under the *old* map at login. `DashboardSessionAcl` consults exactly those claims, so a user whose group's tag grant was removed from config keeps observing the corresponding sessions' event streams — hub joins and the session-details in-process seam alike — until they log out or the sliding 8-hour window finally lapses, renewable indefinitely by activity. The hub-token seam does not have this gap for the *grant* (mint re-resolves from group claims, bounding staleness to 5 minutes, exactly as its comment at `:41-47` claims) — but that comment's promise that "revoking a GroupToTag entry takes effect ... within one lifetime" holds only for token-authenticated connections, and the cookie-authenticated in-process seam it shares an ACL with is the weaker path. LDAP group-membership revocations have the same 8-hour cookie bound on both paths (standard cookie-auth behavior, but worth stating for a *security grant*).
**Recommendation:** have the ACL (or a claims transformation) re-resolve tags from the principal's `LdapGroupClaimType` claims against current options — the same one-call `DashboardGroupTagMapping.MapGroupsToTags` the token mint already uses — instead of trusting login-stamped tag claims; the group claims are already on the cookie. That makes a config-revoked grant take effect at the next subscribe after restart, symmetrically with the token path. At minimum, document the revocation bounds (restart + re-login for cookies, 5 minutes for tokens, never for an already-open subscription) in `docs/GatewayDashboardDesign.md`'s ACL section.
## Severity roll-up (new findings)
| Severity | Count | IDs |
|----------|-------|-----|
| Medium | 1 | SEC-37 |
| Low | 3 | SEC-38, SEC-40, SEC-41 |
| Info | 1 | SEC-39 |
Prior-backlog movement this period: SEC-19, SEC-21, SEC-26 resolved; SEC-31…SEC-36 all verified Done (SEC-36 including the operator rotation); still open from cycle 1: SEC-13 (impact reduced — redactor now wired fail-closed), SEC-14, SEC-15 (accepted), SEC-16, SEC-17, SEC-18, SEC-23, SEC-24 (narrowed), SEC-27.
## Still good / preserved
The load-bearing controls all survive at HEAD, several strengthened:
- **Fail-closed authorization core:** unrecognized request → `admin` scope (`GatewayGrpcScopeResolver`); opaque auth failures that never disclose the failing stage (`GatewayGrpcAuthorizationInterceptor.cs:96-109`); constant-time compare stays inside the shared verifier on every cache miss with only SHA-256 digests in memory (`CachingApiKeyVerifier.cs:36-39,284-286`); the throttle message reveals nothing about secret validity.
- **The SEC-25 ACL is genuinely fail-closed on every branch** (unknown session, empty grant, untagged-session default `AdminOnly`, null principal), enforced identically on both subscribe seams, with the decision-table corners and the live-LDAP group-shape proof pinned by tests; session tags are immutable and inherited only from the owning key's constraints, never the client request; the gRPC event-stream attach stays owner-scoped (`EventStreamService.cs:52-63`).
- **Read-only environment bypasses stay read-only:** the `Disabled`-mode and anonymous-loopback bypasses satisfy only requirements containing Viewer, never `AdminOnly` (`DashboardAuthorizationHandler.cs:37-53`); API-key CRUD and session Close/Kill keep their service-layer Admin re-checks (`DashboardApiKeyAuthorization.cs:10-18`, `DashboardSessionAdminService`) beneath the policy layer; the secrets UI mounts behind its own `secrets:manage` policy (`GatewayApplication.cs:147-150`), which the dashboard bypass handler cannot satisfy (different requirement type).
- **Value redaction is now a complete, tested, three-seam system** (events mirror, alarms hub, `/browse`), clone-only so gRPC clients and the replay ring are untouched, with the config doc telling the truth about all three seams and the failed-read placeholder rule.
- **Secret handling improved materially:** the committed LDAP literal is gone and rotated; config secrets ride fail-fast `${secret:}` references expanded pre-host; the effective-config projection redacts pepper name and LDAP password; hub tokens remain 5-minute, data-protected, identity-required; no new call site logging credentials, tag values, or grant contents was found in the changed files (the ACL logs decisions, never tag values — `DashboardAuthenticator.cs:145-146`).
- **Credential-store paths are now defended in depth:** host-meaningful rooting, the under-content-root rule born of a real outage, and absolute defaults for all three stores (auth DB, secrets store, Galaxy snapshot) via one shared rule set (`GatewayConfigPathRules`).
- **Dashboard hygiene held:** local Bootstrap only (no UI component libraries in any new page); antiforgery on POST login/logout; `SanitizeReturnUrl` still blocks open redirects (`DashboardEndpointRouteBuilderExtensions.cs:238-249`); the login limiter stays remote-IP-partitioned; the viewer registry / snapshot feed / broadcaster concurrency reworks (`EventsHubViewerRegistry.cs`, `DashboardSnapshotFeed.cs`, `DashboardEventBroadcaster.cs`) are careful, generation-guarded code with the subtle orderings documented in place and covered by dedicated tests.