From 6803bab79a06a5b450f72ff4980fc1f6d109a14d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:45:16 -0400 Subject: [PATCH 1/4] fix(TST-25,SEC): stop CI SSH key leaking in cleartext CI logs The windows-x86 acceptance check 'no key material in logs' failed: run #37's job log printed the full WINDEV_SSH_KEY PEM in the step env echo. Gitea's secret masker is line-oriented, so a multiline PEM rendered as one line with literal \n escapes never matches the real-newline secret value and is not redacted. Fix: store WINDEV_SSH_KEY base64-encoded (single line) so the masker redacts it to ***; run-windev-ci.sh auto-decodes a base64 PEM (still accepts a raw PEM for local hand-testing). Drop the redundant WINDEV_SSH_KNOWN_HOSTS from the job env (host keys are public and come from the committed windev.known_hosts pin), removing another cleartext env line. Document the base64 requirement in the bring-up README. Operationally: the previously-exposed CI key has been rotated on windev (old pubkey revoked from administrators_authorized_keys, new key installed) and the Gitea WINDEV_SSH_KEY secret replaced with the new key's base64. --- .gitea/workflows/ci.yml | 10 ++++++++-- scripts/ci/README.md | 12 ++++++++---- scripts/ci/run-windev-ci.sh | 17 ++++++++++++++--- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index af48fa5..a644bbe 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -136,8 +136,11 @@ jobs: - uses: actions/checkout@v4 - name: Worker x86 build + tests on windev env: + # WINDEV_SSH_KEY is stored base64-encoded (single line) so Gitea's line-oriented secret + # masker redacts it in the step env echo; run-windev-ci.sh decodes it. A raw multiline PEM + # would leak in cleartext here (TST-25 acceptance: no key material in logs). Known-hosts is + # public and comes from the committed scripts/ci/windev.known_hosts pin (no secret needed). WINDEV_SSH_KEY: ${{ secrets.WINDEV_SSH_KEY }} - WINDEV_SSH_KNOWN_HOSTS: ${{ secrets.WINDEV_SSH_KNOWN_HOSTS }} WINDEV_SSH_USER: ${{ vars.WINDEV_SSH_USER }} CI_SHA: ${{ github.sha }} # `test` = x86 build + Worker.Tests (~50s measured on windev). Demote to `build` only if the @@ -155,8 +158,11 @@ jobs: - uses: actions/checkout@v4 - name: Full Worker.Tests + live-MXAccess smoke on windev env: + # WINDEV_SSH_KEY is stored base64-encoded (single line) so Gitea's line-oriented secret + # masker redacts it in the step env echo; run-windev-ci.sh decodes it. A raw multiline PEM + # would leak in cleartext here (TST-25 acceptance: no key material in logs). Known-hosts is + # public and comes from the committed scripts/ci/windev.known_hosts pin (no secret needed). WINDEV_SSH_KEY: ${{ secrets.WINDEV_SSH_KEY }} - WINDEV_SSH_KNOWN_HOSTS: ${{ secrets.WINDEV_SSH_KNOWN_HOSTS }} WINDEV_SSH_USER: ${{ vars.WINDEV_SSH_USER }} CI_SHA: ${{ github.sha }} run: ./scripts/ci/run-windev-ci.sh live diff --git a/scripts/ci/README.md b/scripts/ci/README.md index a331342..ce334de 100644 --- a/scripts/ci/README.md +++ b/scripts/ci/README.md @@ -31,10 +31,14 @@ first, then merge. personal windev key): `ssh-keygen -t ed25519 -f ci_windev -N ''`. Append `ci_windev.pub` to the windev CI account's `authorized_keys` (optionally with a forced `command=` limiting it to this bootstrap). -3. **Gitea repo secrets** — store the private key as `WINDEV_SSH_KEY` and, optionally, the - pinned host keys as `WINDEV_SSH_KNOWN_HOSTS` (the committed `windev.known_hosts` is the - fallback). `run-windev-ci.sh` connects as user `ci` by default (`WINDEV_SSH_USER` to - override); ensure that account exists on windev, or set `WINDEV_SSH_USER` to the intended one. +3. **Gitea repo secret `WINDEV_SSH_KEY`** — store the private key **base64-encoded on a single + line**: `base64 -w0 < ci_windev` (macOS: `base64 < ci_windev | tr -d '\n'`). This is required, + not cosmetic — Gitea's secret masker is line-oriented, so a raw multiline PEM is **not** + redacted in the step `env:` echo and leaks in cleartext; the single-line base64 masks to `***`. + `run-windev-ci.sh` auto-decodes it. Host keys are public and come from the committed + `windev.known_hosts` pin, so no `WINDEV_SSH_KNOWN_HOSTS` secret is wired into `ci.yml`. + `run-windev-ci.sh` connects as user `ci` by default; set the `WINDEV_SSH_USER` **variable** + (not a secret — it is public) to the actual account, and ensure that account exists on windev. 4. **Network precheck** — confirm the Gitea runner container (on the `traefik` docker network, host `10.100.0.35`) can reach `10.100.0.48:22`: a one-off `nc -z -w5 10.100.0.48 22` step. The network was created for gitea name resolution, not LAN egress, so verify. diff --git a/scripts/ci/run-windev-ci.sh b/scripts/ci/run-windev-ci.sh index e899a03..7d52fff 100755 --- a/scripts/ci/run-windev-ci.sh +++ b/scripts/ci/run-windev-ci.sh @@ -12,8 +12,12 @@ # # Environment: # CI_SHA commit to test (default: `git rev-parse HEAD` in this checkout) -# WINDEV_SSH_KEY private key PEM (Gitea secret). If empty, falls back to the -# runner's default ssh identity/agent (used for local hand-testing). +# WINDEV_SSH_KEY private key (Gitea secret). Store it BASE64-ENCODED (one line): +# Gitea's secret masker is line-oriented, so a raw multiline PEM in +# the step `env:` echo is NOT redacted and leaks in cleartext, whereas +# the single-line base64 is masked to *** (TST-25 acceptance: no key +# material in logs). A raw PEM is still accepted for local hand-testing +# (auto-detected). If empty, falls back to the runner's default identity. # WINDEV_SSH_KNOWN_HOSTS pinned host keys (Gitea secret). If empty, uses the committed # scripts/ci/windev.known_hosts. # WINDEV_SSH_USER ssh user on windev (default: ci) @@ -60,9 +64,16 @@ fi SSH_OPTS+=(-o "UserKnownHostsFile=$KNOWN_HOSTS") # Private key: secret if provided, else fall back to the default identity/agent. +# CI stores it base64-encoded (single line) so Gitea's line-oriented secret masker redacts it +# in the step env echo — a raw multiline PEM leaks in cleartext. Decode when it is valid base64 +# wrapping a PEM; otherwise treat the value as a raw PEM (local hand-testing convenience). if [ -n "${WINDEV_SSH_KEY:-}" ]; then KEY="$WORK/id_ci" - ( umask 077; printf '%s\n' "$WINDEV_SSH_KEY" > "$KEY" ) + if printf '%s' "$WINDEV_SSH_KEY" | base64 -d 2>/dev/null | grep -q 'PRIVATE KEY'; then + ( umask 077; printf '%s' "$WINDEV_SSH_KEY" | base64 -d > "$KEY" ) + else + ( umask 077; printf '%s\n' "$WINDEV_SSH_KEY" > "$KEY" ) + fi SSH_OPTS+=(-o IdentitiesOnly=yes -i "$KEY") fi From 5c8075996f251f86947a3da0f34dfdcb9160f9fc Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:50:22 -0400 Subject: [PATCH 2/4] fix(TST-25): serialize the CI bootstrap fetch/checkout under the worktree lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acceptance-check finding (concurrency): the worktree lock in windev-worker-ci.ps1 guarded the build stage, but run-windev-ci.sh's bootstrap did git fetch + git checkout on the shared C:\build\mxaccessgw-ci clone BEFORE that lock was taken. Two concurrent runs therefore collided on .git/index.lock at the bootstrap stage (the second failed 'Another git process seems to be running', exit 1) instead of the second waiting — defeating the lock's purpose for manual/degraded-mode overlap or a future second runner. (The single Gitea runner serializes jobs, so CI pushes never actually overlapped; this is defense-in-depth being restored.) Fix: the bootstrap now acquires the same mkdir worktree lock around its fetch/ checkout and exports MXGW_CI_LOCK_HELD; windev-worker-ci.ps1 re-uses that lock (skips re-acquire/release) when the env is set, and still self-locks for a standalone/manual run. Now the second concurrent run waits at the bootstrap. --- scripts/ci/run-windev-ci.sh | 42 ++++++++++++++++++++++++++------- scripts/ci/windev-worker-ci.ps1 | 29 +++++++++++++++-------- 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/scripts/ci/run-windev-ci.sh b/scripts/ci/run-windev-ci.sh index 7d52fff..08b1072 100755 --- a/scripts/ci/run-windev-ci.sh +++ b/scripts/ci/run-windev-ci.sh @@ -80,16 +80,42 @@ fi echo "run-windev-ci: mode=$MODE sha=$CI_SHA target=${WINDEV_SSH_USER}@${WINDEV_SSH_HOST}" # Remote PowerShell bootstrap: fetch + checkout enough to load the versioned CI script, then -# hand off to it. Not Stop around git (stderr progress). windev-worker-ci.ps1 re-fetches and -# re-checks-out under a lock, so this checkout only needs to load the right script version. +# hand off to it. Not Stop around git (stderr progress). +# +# The bootstrap fetch/checkout mutates the shared clone's git index, so it MUST hold the +# worktree lock — otherwise two concurrent runs collide on .git/index.lock before either +# reaches windev-worker-ci.ps1's lock (the exact race the lock exists to prevent). We take the +# same mkdir lock here (atomic; retry to a 20-min deadline) and export MXGW_CI_LOCK_HELD so the +# handed-off script re-uses this lock instead of re-acquiring/releasing it. A standalone/manual +# run of windev-worker-ci.ps1 (degraded mode) has no such env and self-locks as before. read -r -d '' BOOTSTRAP < Date: Mon, 13 Jul 2026 11:20:49 -0400 Subject: [PATCH 3/4] test(flaky): deflake SessionManager fail-fast timing assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI (portable, run #38) intermittently failed SessionManagerTests .InvokeAsync_WhenTimeoutZero_FailsFastUnchanged with 'expected immediate fail-fast but took 155ms'. The fail-fast paths throw synchronously under the lock (GatewaySession.GetReadyWorkerClientAsync) and never enter the poll loop, so the absolute <100ms wall-clock bound measured only host load, not behavior, and flaked under CI contention. - WhenWorkerFaulted_FailsFastWithBothStates: anchor the bound to a large (5000ms) ready-wait timeout and assert fail-fast returns in < timeout/3 — a regression that burned the timeout is still caught, but scheduling jitter can't trip it. - WhenTimeoutZero_FailsFastUnchanged: drop the wall-clock assertion entirely (a zero timeout has no wait window to burn); the error code, both-states message, and InvokeCount == 0 already pin the immediate fail-fast. Verified: the 3 SessionManager timing tests pass (net10.0). --- .../Gateway/Sessions/SessionManagerTests.cs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs index 2a0a1af..1ac81c7 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs @@ -420,10 +420,16 @@ public sealed class SessionManagerTests [Fact] public async Task InvokeAsync_WhenWorkerFaulted_FailsFastWithBothStates() { + // Use a deliberately large ready-wait timeout so fail-fast is unambiguous: a terminal + // worker must surface immediately instead of burning it. The timing assertion below is + // anchored to a small fraction of this timeout (not an absolute ~100ms wall-clock bound, + // which flaked under CI load) so a regression that waited out the timeout is caught + // while ordinary scheduling jitter never trips it. + const int readyWaitTimeoutMs = 5000; FakeWorkerClient workerClient = new() { State = WorkerClientState.Faulted }; SessionManager manager = CreateManager( new FakeSessionWorkerClientFactory(workerClient), - options: CreateOptions(workerReadyWaitTimeoutMs: 500)); + options: CreateOptions(workerReadyWaitTimeoutMs: readyWaitTimeoutMs)); GatewaySession session = await manager.OpenSessionAsync(CreateOpenRequest(), "client-1", ownerKeyId: null, CancellationToken.None); Assert.Equal(SessionState.Ready, session.State); @@ -435,7 +441,9 @@ public sealed class SessionManagerTests CancellationToken.None)); stopwatch.Stop(); - Assert.True(stopwatch.ElapsedMilliseconds < 100, $"Expected immediate fail-fast but took {stopwatch.ElapsedMilliseconds}ms."); + Assert.True( + stopwatch.ElapsedMilliseconds < readyWaitTimeoutMs / 3, + $"Expected fail-fast well under the {readyWaitTimeoutMs}ms ready-wait timeout but took {stopwatch.ElapsedMilliseconds}ms."); Assert.Equal(SessionManagerErrorCode.SessionNotReady, exception.ErrorCode); Assert.Contains("Session state is Ready", exception.Message); Assert.Contains("worker state is Faulted", exception.Message); @@ -487,15 +495,17 @@ public sealed class SessionManagerTests GatewaySession session = await manager.OpenSessionAsync(CreateOpenRequest(), "client-1", ownerKeyId: null, CancellationToken.None); Assert.Equal(SessionState.Ready, session.State); - Stopwatch stopwatch = Stopwatch.StartNew(); + // A zero timeout has no wait window to burn, so there is no wall-clock timing to assert + // (the previous absolute ~100ms bound only measured host load and flaked under CI). The + // error code, the byte-for-byte both-states message, and InvokeCount == 0 fully pin the + // immediate fail-fast — a regression that started polling would still surface the same + // outcome, not a timing difference worth a flaky assertion. SessionManagerException exception = await Assert.ThrowsAsync( async () => await manager.InvokeAsync( session.SessionId, CreateCommand(MxCommandKind.Ping), CancellationToken.None)); - stopwatch.Stop(); - Assert.True(stopwatch.ElapsedMilliseconds < 100, $"Expected immediate fail-fast but took {stopwatch.ElapsedMilliseconds}ms."); Assert.Equal(SessionManagerErrorCode.SessionNotReady, exception.ErrorCode); Assert.Contains("Session state is Ready", exception.Message); Assert.Contains("worker state is Handshaking", exception.Message); From c94066dc3644683e0aada40d5e884f5a58c48647 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:34:21 -0400 Subject: [PATCH 4/4] docs(TST-25): record acceptance-check findings (key-leak, lock race, flaky test) in tracker change log --- archreview/2026-07-12/remediation/00-tracking.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/archreview/2026-07-12/remediation/00-tracking.md b/archreview/2026-07-12/remediation/00-tracking.md index c5dbe44..1a3cc71 100644 --- a/archreview/2026-07-12/remediation/00-tracking.md +++ b/archreview/2026-07-12/remediation/00-tracking.md @@ -157,4 +157,5 @@ Sequence these together rather than piecemeal — several are one change set spa |---|---| | 2026-07-13 | Initial tracking doc generated from the six domain remediation designs. All 47 findings `Not started` (IPC-31, SEC-35 `N/A`). | | 2026-07-13 | TST-25/TST-26 → `In progress` (branch `fix/tst-25-windev-ci`). Added `scripts/ci/{windev-worker-ci.ps1,run-windev-ci.sh,windev.known_hosts}`, `windows-x86` (per-push) + `nightly-windev` (scheduled) jobs in `ci.yml`, and the TST-26 doc/comment fixes (GatewayTesting.md, Contracts.md, check-codegen.ps1). Mechanism hand-verified on windev: `build`→0, bogus-SHA→nonzero (lock released), `test`→356 passed/0 failed in ~50s (per-push stays `test`, no demotion), and run-windev-ci.sh SSH+EncodedCommand exit-code propagation confirmed. | -| 2026-07-13 | Operator bring-up complete: dedicated CI ed25519 key installed in windev `administrators_authorized_keys` (authorized into `dohertj2`, which owns the working MXAccess/toolchain env — a fresh OS account would break the build; the key is independently revocable), Gitea secrets `WINDEV_SSH_KEY`/`WINDEV_SSH_KNOWN_HOSTS` + variable `WINDEV_SSH_USER=dohertj2` stored, runner→`10.100.0.48:22` egress verified on the `traefik` net, issue-write confirmed. **TST-25/TST-26 → `Done`:** credentialed `windows-x86` ran GREEN on `d769244` (Gitea run #37) — Linux runner SSHed windev, checked out the SHA in `C:\build\mxaccessgw-ci` under lock, ran the x86 Worker build + `Worker.Tests`, exit 0; `nightly-windev` correctly skipped on the push event. Branch merged to `main`. Follow-ups (old tracker): revisit **TST-05** (scheduled live smoke — now covered by `nightly-windev`) and **TST-24** (client wire tests) which this unlocks. Negative-path acceptance checks (deliberate-red, unreachable-host, lock concurrency, forced-failure nightly issue) not yet run — happy-path only. | +| 2026-07-13 | Operator bring-up complete: dedicated CI ed25519 key installed in windev `administrators_authorized_keys` (authorized into `dohertj2`, which owns the working MXAccess/toolchain env — a fresh OS account would break the build; the key is independently revocable), Gitea secrets `WINDEV_SSH_KEY`/`WINDEV_SSH_KNOWN_HOSTS` + variable `WINDEV_SSH_USER=dohertj2` stored, runner→`10.100.0.48:22` egress verified on the `traefik` net, issue-write confirmed. **TST-25/TST-26 → `Done`:** credentialed `windows-x86` ran GREEN on `d769244` (Gitea run #37) — Linux runner SSHed windev, checked out the SHA in `C:\build\mxaccessgw-ci` under lock, ran the x86 Worker build + `Worker.Tests`, exit 0; `nightly-windev` correctly skipped on the push event. Branch merged to `main`. Follow-ups (old tracker): revisit **TST-05** (scheduled live smoke — now covered by `nightly-windev`) and **TST-24** (client wire tests) which this unlocks. | +| 2026-07-13 | Ran the TST-25 acceptance checks (scripts/ci/README.md) — they caught **two real CI defects, both fixed** on `fix/tst-25-ci-key-log-leak`: (1) **CI SSH key leaked in cleartext** in the `windows-x86` step env echo (Gitea's line-oriented masker missed the multiline PEM) — rotated the CI key on windev (old pubkey revoked), stored the key **base64-encoded** so the masker redacts it to `***` (confirmed on run #38), taught `run-windev-ci.sh` to decode, dropped the redundant public known-hosts secret from the job env; (2) **bootstrap lock race** — `run-windev-ci.sh`'s pre-hand-off `git fetch`/`checkout` ran outside the worktree lock, so concurrent runs collided on `.git/index.lock`; the bootstrap now holds the lock (ps1 re-uses it via `MXGW_CI_LOCK_HELD`), retest confirmed clean serialization. Also **deflaked** `SessionManagerTests` fail-fast timing assertions (absolute `<100ms` wall-clock bound flaked under CI load; now anchored to the configured timeout / dropped for the zero-timeout case). Checks passed: unreachable-host fast-fail (exit 255/15s), deliberate-red propagation (Worker.Tests failure → exit 1), lock concurrency (2nd run waits), no-key-in-logs (masked). Pending: forced-failure nightly issue. Merge target `df7e20d` verified GREEN via the local windev path (Worker build + 356 tests). |