Architecture remediation: P1 tier (process & hardening) #121

Merged

dohertj2 merged 19 commits from fix/archreview-p1 into main 2026-07-09 09:59:49 -04:00

Conversation 0 Commits 19 Files Changed 97

+5608 -265

dohertj2 commented 2026-07-09 09:35:47 -04:00

Owner

Copy Link

Copy Source

Architecture-remediation P1 tier (process & hardening), stacked on fix/archreview-p0 (PR #120). Executes the P1 findings in archreview/remediation/00-tracking.md; each fix updates the tracker Status and affected docs in the same change.

What's in this PR

Wave 1 — CI + codegen freshness + Rust buildability

• IPC-01/09/19/20, CLI-02 → Done; TST-03 CI pipeline authored (In review — not yet run on a Gitea runner).

Wave 2 — security & authz hardening

• SEC-01/04/06 (config path-rooting + production validator guards), SEC-05/07/08/11 (hub-token lifetime, QueryActiveAlarms scope, verification cache + last-used coalescing, login rate limit + per-peer gRPC failure limiter).
• SEC-10 core via the shared ZB.MOM.WW.Auth 0.1.4 bump (commit 197731a, authored by the concurrent HistorianGateway-remediation session) — API-key expiry enforcement.
• SEC-02/12/20 (dashboard bypass read-only scoping, session-admin audit events, dropped unbounded metric tag).

Wave 3 — size/backpressure topology + write ordering + event decoupling

• IPC-02 GatewayHello.max_frame_bytes negotiation (gateway sends, worker adopts; regen Generated/ + clients/proto descriptor with pinned protoc 34.1).
• IPC-03 gRPC-vs-pipe envelope-overhead headroom (default Worker.MaxMessageBytes → 16 MiB + 64 KiB, cross-validated at startup) + per-command MessageTooLarge → ResourceExhausted instead of faulting the whole session.
• IPC-04 DrainEvents bounded (gateway validator ≤ 10 000, worker per-reply cap 10 000, max_events = 0 → cap not "drain all").
• WRK-04 envelope Sequence stamped at write time under the write lock.
• WRK-07 worker WorkerFrameWriter is now a cooperative priority scheduler — control frames drain ahead of events.
• GWC-04 worker event enqueue decoupled from the read loop via a staging channel + dedicated event-write loop, so a full event channel can't stall a command reply.

Testing / reliability

• TST-08 resolved by evidence (the claimed orphaned-testhost leak does not reproduce on macOS or windev).
• Windows full-suite temp-file-lock flakiness fixed (unique cert .tmp name + per-process cert path + DisableParallelization on the env-var-mutating test + Sqlite ClearAllPools).

Verification

• macOS: NonWindows.slnx builds clean; targeted validator/event/security suites green (the ~42 named-pipe-harness failures are the pre-existing macOS Unix-socket-path limitation, not regressions).
• windev (Windows, x86 worker): worker builds clean; Worker.Tests 352 passed / 0 failed / 11 skipped; gateway Tests 802 passed / 1 failed — the single failure is the pre-existing environmental SelfSignedCertificateProviderTests…HasExpectedSansEkuAndValidity (machine-name/FQDN SAN); the 2 EventStreamServiceTests queue-depth timing tests are parallel-load flakes that pass in isolation.

Not in this PR (remaining P1)

• TST-05 — live-MXAccess coverage; needs windev + the live rig.
• SEC-10 polish — apikey create --expires CLI flag + dashboard staleness badge (core already delivered).

Note: contains the foreign commit 197731a (shared-monorepo auth-lib bump) that delivered SEC-10's core.

https://claude.ai/code/session_01DMXXvNuPekkkrTEyPNxEkW

Architecture-remediation **P1 tier** (process & hardening), stacked on `fix/archreview-p0` (PR #120). Executes the P1 findings in `archreview/remediation/00-tracking.md`; each fix updates the tracker Status and affected docs in the same change. ## What's in this PR **Wave 1 — CI + codegen freshness + Rust buildability** - IPC-01/09/19/20, CLI-02 → Done; TST-03 CI pipeline authored (In review — not yet run on a Gitea runner). **Wave 2 — security & authz hardening** - SEC-01/04/06 (config path-rooting + production validator guards), SEC-05/07/08/11 (hub-token lifetime, QueryActiveAlarms scope, verification cache + last-used coalescing, login rate limit + per-peer gRPC failure limiter). - SEC-10 core via the shared `ZB.MOM.WW.Auth 0.1.4` bump (commit `197731a`, authored by the concurrent HistorianGateway-remediation session) — API-key expiry enforcement. - SEC-02/12/20 (dashboard bypass read-only scoping, session-admin audit events, dropped unbounded metric tag). **Wave 3 — size/backpressure topology + write ordering + event decoupling** - **IPC-02** `GatewayHello.max_frame_bytes` negotiation (gateway sends, worker adopts; regen `Generated/` + `clients/proto` descriptor with pinned protoc 34.1). - **IPC-03** gRPC-vs-pipe envelope-overhead headroom (default `Worker.MaxMessageBytes` → 16 MiB + 64 KiB, cross-validated at startup) + per-command `MessageTooLarge` → `ResourceExhausted` instead of faulting the whole session. - **IPC-04** `DrainEvents` bounded (gateway validator ≤ 10 000, worker per-reply cap 10 000, `max_events = 0` → cap not "drain all"). - **WRK-04** envelope `Sequence` stamped at write time under the write lock. - **WRK-07** worker `WorkerFrameWriter` is now a cooperative priority scheduler — control frames drain ahead of events. - **GWC-04** worker event enqueue decoupled from the read loop via a staging channel + dedicated event-write loop, so a full event channel can't stall a command reply. **Testing / reliability** - TST-08 resolved by evidence (the claimed orphaned-testhost leak does not reproduce on macOS or windev). - Windows full-suite temp-file-lock flakiness fixed (unique cert `.tmp` name + per-process cert path + `DisableParallelization` on the env-var-mutating test + Sqlite `ClearAllPools`). ## Verification - **macOS**: `NonWindows.slnx` builds clean; targeted validator/event/security suites green (the ~42 named-pipe-harness failures are the pre-existing macOS Unix-socket-path limitation, not regressions). - **windev (Windows, x86 worker)**: worker builds clean; `Worker.Tests` **352 passed / 0 failed / 11 skipped**; gateway `Tests` **802 passed / 1 failed** — the single failure is the pre-existing environmental `SelfSignedCertificateProviderTests…HasExpectedSansEkuAndValidity` (machine-name/FQDN SAN); the 2 `EventStreamServiceTests` queue-depth timing tests are parallel-load flakes that pass in isolation. ## Not in this PR (remaining P1) - **TST-05** — live-MXAccess coverage; needs windev + the live rig. - **SEC-10 polish** — `apikey create --expires` CLI flag + dashboard staleness badge (core already delivered). > Note: contains the foreign commit `197731a` (shared-monorepo auth-lib bump) that delivered SEC-10's core. https://claude.ai/code/session_01DMXXvNuPekkkrTEyPNxEkW

dohertj2 changed target branch from fix/archreview-p0 to main 2026-07-09 09:59:11 -04:00

dohertj2 added 19 commits 2026-07-09 09:59:11 -04:00

fix(CLI-02): make Rust crate buildable outside the repository ... 219fa6ddb4

build.rs resolved the .proto files via a monorepo-relative path and packaging
relied on `cargo publish --no-verify` to hide the resulting failure. Vendor the
three canonical protos into clients/rust/protos/, resolve them via
CARGO_MANIFEST_DIR (in-repo path preferred, vendored fallback for the packaged
crate), ship them via Cargo.toml `include`, and drop `--no-verify` from
pack-clients.ps1.

archreview: CLI-02 (P1). Verified: cargo fmt/check/test (28) + clippy -D warnings,
and `cargo package` (no --no-verify) compiles standalone in the isolated staging
dir — the out-of-repo proof. Vendored-proto drift is guarded by check-codegen.ps1.

fix(archreview): CI pipeline + codegen freshness guards (IPC-01/09/19/20, TST-03) ... d5248f61a2

- IPC-01: regenerate the stale client descriptor set and add ClientProtoInputTests
  (semantic, protoc-free) so a missing contract symbol fails the build.
- IPC-20: make publish-client-proto-inputs.ps1 -Check source_code_info-normalized
  so it is protoc-version tolerant.
- IPC-19: document + guard the "Generated/ must be committed for net48" rule
  (docs/Contracts.md, csproj comment, check-codegen.ps1).
- IPC-09: harden the per-client generate-proto scripts (PATH resolution + pinned
  grpcio/protobuf/java version asserts) so regeneration is reproducible.
- TST-03: add .gitea/workflows/ci.yml (portable/java/windows/live jobs) running the
  build, tests, client checks, and the codegen guards.
- Also: check-codegen.ps1 Check 3 guards the CLI-02 vendored Rust protos against drift.

archreview: IPC-01/09/19/20 Done, TST-03 In review (pipeline authored + validated,
not yet run on a Gitea runner). Verified on macOS: NonWindows build clean,
ClientProtoInputTests 5/5, -Check exit 0.

docs(archreview): P1 Wave 1 status (IPC-01/09/19/20, CLI-02 Done; TST-03 In review) 8fb1a814d4

chore(auth): consume ZB.MOM.WW.Auth 0.1.4 — gain API-key expiry enforcement (archreview G-2) ... 197731ae2d

Bump all four ZB.MOM.WW.Auth.* package refs 0.1.2 -> 0.1.4. The shared
ApiKeyVerifier now rejects any key whose ExpiresUtc is in the past;
existing keys have NULL expiry (never expire), so nothing breaks, and the
auth SQLite DB auto-migrates to schema v3 (nullable expires_utc column) on
first boot. Implement the two IApiKeyAdminStore members added in 0.1.3
(SetScopesAsync/SetEnabledAsync) on the test FakeApiKeyAdminStore.

Build green; no new test failures (the macOS worker-pipe IPC failures are
pre-existing/environmental, identical to the pre-bump baseline).

fix(archreview): security config guards (SEC-01/04/06) ... c185f621a4

- SEC-01: derive AuthenticationOptions.SqlitePath / TlsOptions.SelfSignedCertPath
  defaults from SpecialFolder.CommonApplicationData (was Windows-literal strings
  that became CWD-relative files off-Windows); validator rejects non-rooted
  overrides; delete the stray C:\ProgramData\...gateway-auth.db files that
  materialized under src/; add a tree-hygiene test failing on any *.db under src/.
- SEC-04: GatewayOptionsValidator fails startup when IsProduction() && DisableLogin.
- SEC-06: validator rejects LDAP Transport=None in Production; document the
  MxGateway__Ldap__ServiceAccountPassword env override + dev-credential rotation.

Galaxy SnapshotCachePath rooting could not be validator-enforced (bound by the
external GalaxyRepository package, not GatewayOptions) — documented instead.

archreview: SEC-01/04/06 (P1). Verified on the Auth-0.1.4 baseline: Server build
clean, GatewayOptionsValidator + GatewayTreeHygiene tests 53/53.

docs(archreview): P1 Wave 2a status (SEC-01/04/06 Done; SEC-10 Done via G-2 Auth 0.1.4) 970613eebd

fix(archreview): security authz+hub cluster (SEC-07/05/08/11) + validation hardening ... 17f16ea181

SEC-07: add QueryActiveAlarmsRequest -> events:read scope arm; fix two tests that
  constructed StreamAlarmsRequest instead of QueryActiveAlarmsRequest.
SEC-05: shorten hub-token lifetime 30m -> 5m; document that the ?access_token= query
  carriage must never be request-logged.
SEC-08: gateway-side CachingApiKeyVerifier (short TTL, keyed on a hash of the presented
  secret) skips the per-call store read+last_used write; CoalescingMarkApiKeyStore bounds
  last_used writes to <=1/key/min; identity constraints are cached. Invalidation is wired
  at the gateway admin sites (revoke/rotate/delete); short TTL backstops out-of-process CLI.
SEC-11: fixed-window rate limit on POST /auth/login + a per-peer (key-id) failure limiter
  checked before VerifyAsync; new MxGateway:Security options bound + validated.

Also fixes regressions from the SEC-01/04/06 commit (c185f62) that a narrow test filter
missed (all now covered by a full-suite checkpoint):
- Rooting check is cross-platform: accepts Windows C:\/UNC forms on Unix so the shipped
  appsettings path validates on the macOS dev box, still rejecting bare filenames.
- AddGatewayConfiguration TryAdds a non-production IHostEnvironment fallback so the validator
  resolves in minimal test/tooling containers; the real host + apikey CLI register the actual
  environment first (TryAdd no-op there).
- Test assembly defaults ASPNETCORE_ENVIRONMENT=Development (ModuleInitializer) so full-host
  tests exercise wiring instead of tripping the SEC-04/06 production guards.
- GatewayOptionsTests asserts the SEC-01 CommonApplicationData-derived default (platform-correct).

archreview: SEC-07/05/08/11 (P1). Verified: NonWindows build clean; full gateway suite
747 passed / 42 failed, where all 42 are the pre-existing macOS named-pipe-harness failures
(Unix-socket path limit) and 0 are validation/regression failures.

docs(archreview): P1 Wave 2b status (SEC-05/07/08/11 Done) c2df4a0aae

fix(archreview-p1): SEC-02/12/20 dashboard + observability hardening ... 74e815c4d7

- SEC-02: DashboardAuthorizationHandler restricts the loopback and
  Authentication:Mode=Disabled bypasses to read-only. They now satisfy only a
  Viewer-bearing requirement (AnyDashboardRole), never AdminOnly, so anonymous
  localhost can view the dashboard but cannot reach API-key CRUD or session
  Close/Kill at the policy layer (previously guarded only by service re-checks).
- SEC-12: DashboardSessionAdminService emits canonical AuditEvents through
  IAuditWriter (actions dashboard-close-session / dashboard-kill-worker, category
  SessionAdmin) on Success/Failure/Denied, mirroring the API-key audit path so
  destructive session actions leave durable, queryable rows.
- SEC-20: drop the unbounded session_id tag from the exported
  mxgateway.heartbeats.failed counter (per-session detail stays in the snapshot/log).

Docs updated same-change: CLAUDE.md (read-only loopback + 5-min bearer),
GatewayDashboardDesign.md (bypass scoping + session-admin audit), Metrics.md.
Server build clean; 30/30 targeted + 295/295 Dashboard/Security/App/Metrics sweep.

docs(archreview-p1): TST-08 orphaned-testhost leak does not reproduce ... 8c06635ee5

Empirically verified the full ZB.MOM.WW.MxGateway.Tests suite exits cleanly:
- macOS: 0 surviving testhost after a full run.
- windev (origin/main worktree): 0 new testhost and 0 new worker processes
  after a full run, isolated by process StartTime.

Static audit confirms the prime-suspect fixtures already dispose deterministically
(WorkerClient cancels _stopCts + awaits its read/write/heartbeat tasks with a 5s
timeout; GatewaySession disposes the client; harness/E2E fixtures use await using).

Drop the stale 'leaves orphaned testhost processes' rationale from CLAUDE.md's
Source Update Workflow; keep filtered-run guidance as a speed guideline only.

Separately discovered (tracked in 00-tracking.md change log, NOT part of TST-08):
Windows-only temp-file-lock flakiness in ~4 full-host test classes (Sqlite
connection-pool retaining a temp .db handle; SelfSignedCertificateProvider's
fixed-name gw.pfx.tmp racing teardown) that macOS never surfaces.

test+fix: eliminate Windows full-suite temp-file-lock flakiness ... 11a716a07f

Full-host tests passed in isolation but failed 2-4/run under parallel execution on
Windows with 'the process cannot access the file ... because it is being used by
another process' (macOS never sees it — Unix deletes open files). Two shared-state
parallel collisions, discovered while verifying TST-08:

1. Self-signed cert generation. GatewayTlsBootstrapTests sets process-global
   Kestrel/TLS env vars that GatewayApplication.Build reads; a parallel host-building
   test inherits them mid-run and both generate a cert at the same path, racing on the
   fixed-name '<path>.tmp'. Fixes:
   - SelfSignedCertificateProvider: stage the PFX in a unique '<path>.<guid>.tmp' instead
     of a fixed name, so concurrent/interrupted writers never collide on the temp file
     (real robustness: two instances or a restart-during-write no longer clash). The
     final atomic Move (last-writer-wins) still yields an equivalent cert.
   - TestHostEnvironmentInitializer: default MxGateway__Tls__SelfSignedCertPath to a
     per-process temp path so the suite never writes the shared ProgramData default or
     fights the deployed service; first host-building test generates, the rest load it.
   - GatewayTlsBootstrapTests: [Collection] with DisableParallelization so its global
     env-var mutation cannot bleed into parallel tests (new GlobalEnvironmentCollection).

2. AuthStoreHealthCheckTests opened a real SQLite file; Microsoft.Data.Sqlite's
   connection pool keeps the .db handle open after 'await using', so the finally
   File.Delete threw. Reuse the existing TempDatabaseDirectory helper, which calls
   SqliteConnection.ClearAllPools() before deleting.

Server build clean; affected classes 17/17 on macOS. Windows full-suite verified separately.

docs(archreview-p1): record Windows temp-file-lock flakiness fix + windev verification ... e1a505d662

3 consecutive windev full-suite runs: 793 passed / 2 failed, 0 orphans (was flaky
2-4 fails). The 2 stable failures are pre-existing windev-environmental (cert SAN
machine-name assertion; event-stream concurrency timing) and pass on macOS.

feat(ipc): negotiate worker frame max, add gRPC headroom, bound DrainEvents (IPC-02/03/04 gateway half) ... c8b3a2281a

Proto foundation + gateway-side of the size/backpressure pass:

- IPC-02: add GatewayHello.max_frame_bytes (regen Generated/); gateway sends
  its negotiated worker-frame max in the handshake so the worker can adopt it
  instead of a hard-coded default. Worker read-half lands separately.
- IPC-03: give the pipe frame max envelope-overhead headroom above the public
  gRPC cap (WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes = 64 KiB;
  default Worker.MaxMessageBytes bumped to 16 MiB + reserve), cross-validate the
  headroom at startup, and pre-check command envelope size in WorkerClient so an
  oversized command fails only that correlation (ResourceExhausted) instead of
  faulting the whole session.
- IPC-04: reject DrainEvents max_events above a public ceiling in the request
  validator (worker per-reply cap lands with the worker half).

Docs: GatewayConfiguration, WorkerFrameProtocol, gateway.md.
Tests: headroom validation, DrainEvents bound, oversized-command per-command
failure (pipe-harness, verified on windev).

feat(worker): adopt negotiated frame max, bound drain, priority write scheduler (IPC-02/04 + WRK-04/07 worker half) ... ebe6aeac98

Worker half of the Wave 3 size/backpressure + write-ordering pass:

- IPC-02: the worker adopts GatewayHello.max_frame_bytes during the handshake
  (WorkerFrameProtocolOptions.AdoptNegotiatedMaxMessageBytes) instead of a
  hard-coded default; 0 keeps the default, a value above a 256 MiB ceiling is
  rejected. Reader and writer share the options instance, applied before the
  message loop.
- IPC-04: DrainEvents caps each reply at MaxDrainEventsPerReply (10_000) and
  treats max_events = 0 as that cap rather than 'drain the entire queue', so one
  diagnostic drain cannot pack a session-killing reply frame.
- WRK-04: WorkerFrameWriter stamps the envelope Sequence at the actual point of
  writing (under the write lock) instead of at envelope creation, so the on-wire
  order and the stamped sequence always agree under concurrent producers.
- WRK-07: the writer is now a cooperative priority scheduler — callers enqueue at
  Control or Event priority and the draining lock-holder writes all control
  frames before any event frame, so replies/faults/heartbeats jump ahead of an
  event backlog. Per-frame validation/size rejections fail only that frame; a
  stream write failure fails all queued frames.

Tests: monotonic gap-free sequence under concurrency, control-before-event
priority (gated stream), negotiated-max adoption, DrainEvents zero-bound.
Worker builds x86 only — verified on windev.

test(ipc): refresh proto descriptor set + MaxMessageBytes default expectation ... 309296f467

Follow-up to the IPC-02 proto change and IPC-03 headroom default:
- regenerate clients/proto/descriptors/mxaccessgw-client-v1.protoset with the
  pinned protoc 34.1 so it carries GatewayHello.max_frame_bytes (IPC-01 descriptor
  refresh; ClientProtoInputTests.Descriptor_ContainsEveryContractMessageAndField).
- update GatewayOptionsTests design-default assertion to 16 MiB + 64 KiB reserve.

docs(archreview-p1): mark IPC-02/03/04 + WRK-04/07 Done (Wave 3, windev-verified) bfbfcdd112

feat(gateway): decouple worker event enqueue from the read loop (GWC-04) ... 32d6b48910

The read loop awaited EnqueueWorkerEventAsync inline, which blocks in the bounded
event channel's timed WriteAsync (up to EventChannelFullModeTimeout, default 5s)
when the channel is full with no/slow consumer. A WorkerCommandReply or heartbeat
queued behind an event frame was then stalled, so an in-flight InvokeAsync could
hit CommandTimeout even though the worker replied in time.

Mirror the existing outbound WriteLoopAsync: add an unbounded event staging
channel and a dedicated EventWriteLoopAsync. DispatchEnvelope is now fully
synchronous — the WorkerEvent branch hands the event off with a non-blocking
TryWrite and the read loop never awaits. The event write loop owns the timed
WriteAsync into the bounded channel and the sustained-overflow ProtocolViolation
fault (unchanged contract). Registered in WaitForBackgroundTasks + completed on
close/fault/dispose.

Test: reply arriving after events with a full, consumer-less event channel is
dispatched promptly (no CommandTimeout) — pipe-harness, verified on windev.
Docs: GatewayProcessDesign read/write/event-loop section.

docs(archreview-p1): mark GWC-04 Done — Wave 3 complete de57d834b1

feat(security): apikey --expires CLI flag + dashboard expiry/staleness badge (SEC-10 polish) ... a038363e74

Gateway-side follow-up to the shared auth-lib expiry core (delivered via G-2):

- apikey create-key gains optional --expires — absolute ISO-8601 UTC or relative
  <N>d/<N>h from now; omitted means non-expiring (opt-in, unchanged default).
  Threaded ApiKeyAdminCommand -> parser -> runner into the library's
  CreateKeyAsync(..., expiresUtc, ...). list-keys shows an expiry column and an
  active/expired/revoked status.
- Dashboard API Keys page surfaces expiry: an Expires column (Never when unset)
  and a status badge reading Expired (red) / Expiring (<=7d, amber) / Revoked /
  Active. DashboardApiKeySummary.ExpiresUtc projected in DashboardSnapshotService;
  StatusBadge maps the new states.

Tests: parser (absolute/relative/invalid/none) + end-to-end past-expiry rejection
through the live verifier; dashboard summary suite green. Docs: Authentication.md
(verification-flow expiry step, CLI table + examples, dashboard badge).

Closes the tracked SEC-10 polish (SEC-10 core was already Done).

dohertj2 merged commit 69ea7937ca into main 2026-07-09 09:59:49 -04:00

dohertj2 referenced this pull request 2026-07-09 17:07:06 -04:00

Architecture remediation: P2 tier (completeness & polish) #122

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

area:auth

Authentication and authorization

area:client-dotnet

.NET client library

area:client-go

Go client library

area:client-java

Java client library

area:client-python

Python client library

area:client-rust

Rust client library

area:contracts

Protocol and generated contract work

area:dashboard

Blazor dashboard work

area:docs

Documentation

area:gateway

Gateway server work

area:tests

Automated and live tests

area:worker

MXAccess worker process work

blocked

Blocked by dependency

priority:p0

Must-have / blocking

priority:p1

High priority

priority:p2

Normal priority

type:docs

Documentation work

type:feature

Feature implementation

type:infra

Infrastructure and scaffolding

type:test

Test work

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

dohertj2 (dohertj2)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/mxaccessgw#121