Merge branch 'fix/tst-25-ci-key-log-leak' into main (TST-25 acceptance-check hardening)
Fixes found by running the TST-25 CI acceptance checks:
- CI SSH private key no longer leaks in windows-x86 logs (base64 secret + decode;
key rotated on windev). Masking confirmed (WINDEV_SSH_KEY: *** on run #38).
- Bootstrap git fetch/checkout now runs under the worktree lock, so concurrent
runs serialize instead of colliding on .git/index.lock.
- Deflaked SessionManagerTests fail-fast timing assertions (portable flake).
Merge target df7e20d verified GREEN via the local windev path (x86 Worker build
+ Worker.Tests 356 passed); main's own CI run provides the canonical evidence.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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). |
|
||||
|
||||
@@ -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.
|
||||
|
||||
+48
-11
@@ -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,25 +64,58 @@ 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
|
||||
|
||||
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 <<PS || true
|
||||
\$ErrorActionPreference = 'Continue'
|
||||
Set-Location '$REMOTE_CLONE'
|
||||
git fetch --prune origin
|
||||
git checkout --force --detach $CI_SHA
|
||||
if (\$LASTEXITCODE -ne 0) { Write-Error "bootstrap checkout failed"; exit \$LASTEXITCODE }
|
||||
powershell -NoProfile -NonInteractive -ExecutionPolicy Bypass -File '$REMOTE_SCRIPT' -Sha '$CI_SHA' -Mode '$MODE'
|
||||
exit \$LASTEXITCODE
|
||||
\$lockDir = '$REMOTE_CLONE.lock'
|
||||
\$deadline = (Get-Date).AddMinutes(20)
|
||||
\$haveLock = \$false
|
||||
while (-not \$haveLock) {
|
||||
try { New-Item -Path \$lockDir -ItemType Directory -ErrorAction Stop | Out-Null; \$haveLock = \$true }
|
||||
catch {
|
||||
if ((Get-Date) -ge \$deadline) { Write-Error "timed out waiting for worktree lock \$lockDir"; exit 75 }
|
||||
Start-Sleep -Seconds 10
|
||||
}
|
||||
}
|
||||
\$env:MXGW_CI_LOCK_HELD = '1'
|
||||
\$rc = 1
|
||||
try {
|
||||
Set-Location '$REMOTE_CLONE'
|
||||
git fetch --prune origin
|
||||
git checkout --force --detach $CI_SHA
|
||||
if (\$LASTEXITCODE -ne 0) { Write-Error "bootstrap checkout failed"; \$rc = \$LASTEXITCODE }
|
||||
else {
|
||||
powershell -NoProfile -NonInteractive -ExecutionPolicy Bypass -File '$REMOTE_SCRIPT' -Sha '$CI_SHA' -Mode '$MODE'
|
||||
\$rc = \$LASTEXITCODE
|
||||
}
|
||||
}
|
||||
finally {
|
||||
Remove-Item -Path \$lockDir -Recurse -Force -ErrorAction SilentlyContinue
|
||||
}
|
||||
exit \$rc
|
||||
PS
|
||||
|
||||
# PowerShell -EncodedCommand wants UTF-16LE base64 on one line (avoids all ssh/quoting hazards).
|
||||
|
||||
@@ -68,19 +68,28 @@ function Invoke-Native {
|
||||
}
|
||||
|
||||
# --- acquire the worktree lock (mkdir is atomic; retry until timeout) ---
|
||||
# run-windev-ci.sh's bootstrap already holds this lock around its pre-hand-off fetch/checkout
|
||||
# (MXGW_CI_LOCK_HELD=1) so that stage can't race a concurrent run; re-use it rather than
|
||||
# re-acquire (and leave releasing to the bootstrap). A standalone/manual run (degraded mode)
|
||||
# has no such env and self-locks here as before.
|
||||
$deadline = (Get-Date).Add($lockTimeout)
|
||||
$haveLock = $false
|
||||
Write-Log "Acquiring worktree lock $lockDir (mode=$Mode sha=$Sha)"
|
||||
while (-not $haveLock) {
|
||||
try {
|
||||
New-Item -Path $lockDir -ItemType Directory -ErrorAction Stop | Out-Null
|
||||
$haveLock = $true
|
||||
}
|
||||
catch {
|
||||
if ((Get-Date) -ge $deadline) {
|
||||
throw "Timed out after $($lockTimeout.TotalMinutes) min waiting for worktree lock $lockDir. A stuck run may have left it; if no CI job is active, remove it by hand."
|
||||
if ($env:MXGW_CI_LOCK_HELD -eq '1') {
|
||||
Write-Log "Worktree lock already held by bootstrap (mode=$Mode sha=$Sha)"
|
||||
}
|
||||
else {
|
||||
Write-Log "Acquiring worktree lock $lockDir (mode=$Mode sha=$Sha)"
|
||||
while (-not $haveLock) {
|
||||
try {
|
||||
New-Item -Path $lockDir -ItemType Directory -ErrorAction Stop | Out-Null
|
||||
$haveLock = $true
|
||||
}
|
||||
catch {
|
||||
if ((Get-Date) -ge $deadline) {
|
||||
throw "Timed out after $($lockTimeout.TotalMinutes) min waiting for worktree lock $lockDir. A stuck run may have left it; if no CI job is active, remove it by hand."
|
||||
}
|
||||
Start-Sleep -Seconds 10
|
||||
}
|
||||
Start-Sleep -Seconds 10
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<SessionManagerException>(
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user