Commit Graph

11 Commits

Author SHA1 Message Date
Joseph Doherty f40409a688 docs: strip task-tracking IDs from code comments and fill missing XML docs
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m16s
ci / java (push) Successful in 2m5s
ci / portable (push) Failing after 5m9s
Tracking IDs are review bookkeeping that rots in shipped source; the rationale they tagged is kept. Comment-only change (CommentChecker 458 -> 16 false positives).
2026-10-07 15:34:32 -04:00
Joseph Doherty a212e145ac feat(security): DashboardTags on API-key constraints; sessions inherit owner tags (SEC-25 groundwork)
Adds a dashboard event-visibility tag to ApiKeyConstraints, riding in the existing
constraints JSON blob so no auth-store schema migration is needed (design
docs/plans/2026-07-10-dashboard-session-acl-tst15.md sections 3/3.1, open call
settled per its own recommendation). The tag is visibility-only: no read, write,
browse, or subscribe path consults it, and HasRead/HasWriteConstraints ignore it.

GatewaySession gains an immutable, ordinal-ignore-case Tags set stamped at
construction from the owning API key, forwarded by MxAccessGatewayService.OpenSession
from the resolved ApiKeyIdentity — never from the wire request, so a client cannot
label its own session with another tenant's tag. ISessionManager gains a tag-carrying
OpenSessionAsync overload whose default implementation forwards to the tagless one, so
an implementation that does not model tags opens an untagged (least visible) session.

apikey create-key gains --dashboard-tags team-a,team-b (repeatable, trimmed,
de-duplicated; an empty segment is rejected rather than dropped) and list-keys prints
the tags column. No enforcement yet — the EventsHub ACL that consumes the tag is a
later change.
2026-08-17 03:58:14 -04:00
Joseph Doherty fa9eb0c0b4 refactor(sessions): remove the dead ISessionManager.ReadEventsAsync chain
ISessionManager.ReadEventsAsync had zero production call sites: the worker
event channel is drained once by GatewaySession.MapWorkerEventsAsync (the
distributor pump), and every consumer — gRPC subscribers, the dashboard
mirror, the alarm monitor — attaches to the distributor. The interface
member, SessionManager's forwarder, and GatewaySession.ReadEventsAsync are
gone; IWorkerClient/WorkerClient.ReadEventsAsync is untouched, it is the
live worker-channel claim.

No test was removed or rewired: nothing invoked the member through the
interface. Nine ISessionManager test fakes carried a required-member stub
(seven threw NotSupportedException or yielded nothing; EventStreamServiceTests
and GatewaySessionDashboardMirrorTests forwarded to the session; the two
MxAccessGatewayService fakes yielded their Events list) — all nine stubs were
deleted. The MxAccessGatewayService suites' streaming tests already run
through FakeEventStreamService, which reads the same Events list, so their
coverage is unchanged; only the now-inaccurate doc comments on Events /
LastReadEventsSessionId were reworded.

The MapWorkerEventsAsync comment no longer describes a twin to keep in step;
it now states the single-reader claim directly. docs/Sessions.md drops
ReadEventsAsync from the SessionManager member list and from the Run-state
prose. The 2026-08-15 deferred-remediation as-built note records the removal.
2026-08-17 03:46:41 -04:00
Joseph Doherty dc2df628e3 chore(followups): reviewer-recommended tests, comments, and hardening from the remediation reviews
The remediation reviews approved every task but left a tail of small notes.
This lands the gateway-side half of them.

Hardening (behavior changes, all narrow):

- BuildFilteredWriteBulkCommand's unreachable default case failed OPEN: a
  fifth bulk-write kind added upstream without a filter case here would have
  shipped the DENIED entries to the worker while reporting them denied to the
  caller. It now throws UnreachableException.
- SqliteCanonicalAuditStore.ListRecentAsync no longer throws on a row it
  cannot date. The retention sweep deliberately preserves such rows (SQLite's
  datetime() yields NULL, so the DELETE never matches), which guaranteed the
  dashboard's recent-audit view would meet one eventually and lose the whole
  page to it. The row is now reported at DateTimeOffset.MinValue with every
  other column intact, behind an optional logger.
- The audit drain loop's finally now completes the channel writer alongside
  detaching the drain, so a producer that raced past the attached check takes
  the write-through branch instead of stranding its event in a buffer nobody
  reads until shutdown. TryComplete is idempotent, so StopAsync is unaffected.

Tests:

- MapCommandReply ownership (Assert.Same on the inner reply), mirroring the
  existing MapEvent ownership test.
- Redactor key-id length boundary at exactly 64 and 65 characters, pinning
  which way it fails. Nothing validates key-id length at creation, so
  docs/Diagnostics.md's "which no issued key id does" is now stated as the
  heuristic it is.
- ApiKeyFailureLimiter.Reset with a PartitionResolution whose partition was
  evicted between the Check and the Reset: inert, and clears nobody else's
  block.
- Constraint-cache concurrency stress: the cap is enforced by the inserting
  thread, so overshoot must be transient and proportional to the in-flight
  inserters, and the cache must settle at or under the cap.
- ListRecentAsync against a raw-SQL undateable row.

Comment/doc accuracy:

- EventsHubViewerRegistry.ReleaseConnection records that it relies on
  SignalR's default sequential per-connection dispatch
  (MaximumParallelInvocationsPerClient = 1).
- A PERF(followup) note on Invoke's double session resolve and why removing it
  needs a SessionManager overload.
- SessionEventDistributor: the volatile-field comment named the pump as the
  lock-free reader, but the pump's single capture point is inside _replayLock;
  the genuinely lock-free reader is SubscriberCount. OnSubscriberOverflow's
  "cannot be observed here" now excepts the DisposeAsync abandon path. The
  churn test names its ConcurrentDictionary bucket-order assumption and that a
  violation surfaces as a read timeout, not a silent pass.
- The two "restores the sequential drain's behavior" claims (SessionManager,
  docs/Sessions.md) were wrong: the sequential drain leaked too, because
  KillWorkerAsync's entry ThrowIfCancellationRequested aborted the whole loop
  on the first session for zero kills. Reworded to "fixes a leak the
  sequential drain also had", with the sweep-bound/shutdown-unbound
  ParallelOptions asymmetry explained.
- ISessionManager.ShutdownAsync's token doc: it degrades the drain to a kill
  sweep rather than cancelling it, with the bounded overrun stated.
  SessionShutdownHostedService.StopAsync records that its cancellation-logging
  branch is now unreachable.
2026-08-15 17:54:31 -04:00
Joseph Doherty 95f8ba918d fix(sessions): exception-total shutdown body with non-cancellable kill fallback 2026-08-15 13:19:08 -04:00
Joseph Doherty 3b6a239ed6 fix(GWC-26): attach the alarm monitor's lease before SubscribeAlarms
RunMonitorAsync issued SubscribeAlarms and the first reconcile before the
internal distributor subscriber was attached (via ISessionManager
.ReadAlarmEventsAsync). The pump has been running since MarkReady started the
dashboard mirror and only fans to subscribers registered at fan-out time, so
every transition raised in that two-round-trip window bypassed the alarm feed —
and a missed Acknowledge was never repaired, because ApplyReconcile broadcast
presence deltas only.

- The monitor now takes the internal lease directly from its session BEFORE
  SubscribeAlarms and drains it after the first reconcile; window transitions
  buffer in the lease's bounded channel. Processing them after ApplyReconcile is
  order-safe (ApplyTransition handles alarms the snapshot already placed).
- ISessionManager.ReadAlarmEventsAsync removed — zero remaining callers.
- ApplyReconcile broadcasts an Acknowledge feed transition when a both-present
  alarm's state advanced to ActiveAcked. This is a feed-level repair on the
  AlarmFeedMessage/StreamAlarms surface rebuilt from the worker's own snapshot,
  not MxEvent emission, so the "never synthesize events" rule is untouched;
  the reasoning is recorded on ApplyReconcile.

The alarm-monitor test fakes now hand the monitor a real Ready GatewaySession
with a dashboard mirror, which is what makes the window reproducible.

Docs: docs/Sessions.md and gateway.md alarm-monitor ordering notes.

Refs: archreview/2026-07-12/remediation/10-gateway-core.md GWC-26
2026-08-07 05:40:33 -04:00
Joseph Doherty 20392cf246 fix(archreview): gateway core P0 remediation (GWC-01/02/03, TST-02, TST-12)
Interlocking changes across the gateway server (shared GatewaySession.cs /
SessionManager.cs), committed together:

- GWC-01 (Critical): alarm monitor now attaches as an internal
  (non-counted) distributor subscriber instead of a second raw drain of the
  single worker event channel; WorkerClient._events -> SingleReader with a
  claimed-once guard so a future dual-consumer regression throws loudly.
- GWC-02 (High): faulted sessions are swept in CloseExpiredLeasesAsync
  (IsFaultedReapable + FaultedReason); new FaultedGraceSeconds (default 0).
- GWC-03 (High): configurable MaxSparseArrayLength (default 1_000_000)
  enforced before allocation.
- TST-02 (High, security): StreamEvents attach now enforces the opening key
  id -> PermissionDenied on owner mismatch.
- TST-12 (Medium): CLAUDE.md retention-defaults sentence corrected.

Verified: NonWindows build clean; targeted tests 135/135 on macOS, plus
WorkerClientTests 18/18 on the Windows host.
2026-07-09 05:51:57 -04:00
Joseph Doherty fca978de07 docs(src): add missing XML docs and strip tracking-ID comments
Sweep of 203 source files resolving CommentChecker findings: add
<summary>/<param>/<returns>/<inheritdoc> where missing, and remove
resolved task/issue tracking markers (Tests-NNN, Worker-NNN, Server-NNN,
Task N) from code comments. Comment/doc-only — no logic changes.
Server+Tests build clean under TreatWarningsAsErrors.
2026-07-07 14:09:49 -04:00
Joseph Doherty f5479f3ca3 feat(sessions): record OwnerKeyId on session creation
Add a nullable string? OwnerKeyId property to GatewaySession that captures
the API key identifier (KeyId) of the authenticated caller that opened the
session. Wire it through ISessionManager.OpenSessionAsync → SessionManager
→ GatewaySession constructor. The gRPC service passes identityAccessor
.Current?.KeyId; internal callers (GatewayAlarmMonitor, DashboardLiveDataService)
pass null. Covers the positive and null cases with two new TDD-first tests.
2026-06-15 12:24:29 -04:00
Joseph Doherty c5e7479ee4 Dashboard: admin-only Close session / Kill worker
Add IDashboardSessionAdminService (Admin-role gate, friendly errors,
audit logging) wrapping a new ISessionManager.KillWorkerAsync that
skips graceful shutdown and cleans up registry/metrics. Sessions,
Workers, and SessionDetails pages render Close / Kill buttons only
when CanManage; the service re-checks the role on every call so
forged clicks return Unauthenticated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-24 07:10:32 -04:00
Joseph Doherty dc9c0c950c rename: prefix gateway projects/namespaces with ZB.MOM.WW + sln→slnx
Apply the ZB.MOM.WW. prefix to all gateway-side projects, folders,
.csproj/.sln contents, C# namespaces, using directives, generated proto
C# (csharp_namespace + checked-in generated files), InternalsVisibleTo
attributes, project-name string literals (LoadProject, .sln lookups,
worker exe paths, staticwebassets manifest), and the install/script/doc
references that point at any of the above. Migrate the solution from
.sln to .slnx via `dotnet sln migrate` and delete the old file.

External-runtime identifiers are intentionally NOT prefixed so external
configuration keeps working:
- GatewayMetrics.cs MeterName ("MxGateway.Server")
- DashboardAuthenticationDefaults Scheme/Policy ("MxGateway.Dashboard")
- GatewayRequestLoggingMiddleware logger category ("MxGateway.Request")
- StaRuntime thread name ("MxGateway.Worker.STA")
- appsettings.json root section "MxGateway" + env-var prefix
  MxGateway__... and secret-name MxGateway:ApiKeyPepper
- C:\ProgramData\MxGateway\ data dir paths

Also fixes two tests that were not rename-related but became visible
while validating the rename:

- WorkerLiveMxAccessSmokeTests.ShutDownAsync: cancellation that the
  gateway service correctly maps to RpcException(Cancelled) per gRPC
  convention was being misclassified as a stream fault. Added a sibling
  catch on RpcException with StatusCode.Cancelled.

- IntegrationTestEnvironment.ResolveRepositoryRoot: extracted IsRepositoryRoot
  and made it accept either a .git marker OR a .sln/.slnx next to src/
  so the worker-exe walker works in non-git working copies.

clients/proto/proto-inputs.json's protoRoot updated to point at
src/ZB.MOM.WW.MxGateway.Contracts/Protos.

Verified by `dotnet build` and a full `dotnet test` of the .slnx with
MXGATEWAY_RUN_LIVE_{MXACCESS,LDAP,GALAXY}_TESTS=1:
  Tests: 472/472 pass
  Worker.Tests: 280/280 pass (4 dev-rig [Fact(Skip=...)] skipped)
  IntegrationTests: 18/18 pass

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 16:22:23 -04:00