From fa037a7f0f42593c1d975ead862171e36e369c0d Mon Sep 17 00:00:00 2001 From: callumalpass Date: Thu, 17 Sep 2026 07:54:13 +1000 Subject: [PATCH 1/5] test(windows): exercise actual CLI JSON lifecycle contract --- .../workflows/windows-daemon-lifecycle.yml | 41 +++++++++++- scripts/diagnostics/windows-cli-lifecycle.mjs | 64 +++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 scripts/diagnostics/windows-cli-lifecycle.mjs diff --git a/.github/workflows/windows-daemon-lifecycle.yml b/.github/workflows/windows-daemon-lifecycle.yml index d75757925..14278a4f8 100644 --- a/.github/workflows/windows-daemon-lifecycle.yml +++ b/.github/workflows/windows-daemon-lifecycle.yml @@ -3,10 +3,11 @@ name: Windows daemon lifecycle on: workflow_call: push: - branches: [investigate/428-windows-startup] + branches: [investigate/428-windows-startup, fix/428-windows-lifecycle-contract] paths: - .github/workflows/windows-daemon-lifecycle.yml - scripts/diagnostics/windows-428.ps1 + - scripts/diagnostics/windows-cli-lifecycle.mjs - scripts/diagnostics/windows-service-probe/** - crates/connect-cli/src/service.rs - crates/connect-cli/src/service/** @@ -16,6 +17,44 @@ permissions: contents: read jobs: + cli-contract: + name: Windows actual CLI JSON lifecycle + runs-on: windows-2025 + timeout-minutes: 30 + defaults: + run: + working-directory: mdbase-connect + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + path: mdbase-connect + persist-credentials: false + - id: engine + shell: bash + run: echo "revision=$(tr -d '\r\n' < deploy/docker/mdbase-rs-revision)" >> "$GITHUB_OUTPUT" + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: callumalpass/mdbase-rs + ref: ${{ steps.engine.outputs.revision }} + path: mdbase-rs + persist-credentials: false + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24 + - name: Build the current CLI (not the service probe) + shell: pwsh + run: | + cargo build --locked -p mdbase-cli + if ($LASTEXITCODE -ne 0) { throw 'CLI build failed.' } + - name: Exercise the desktop JSON subprocess contract + run: node scripts/diagnostics/windows-cli-lifecycle.mjs target/debug/mdbase.exe + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() + with: + name: issue428-cli-contract + path: mdbase-connect/.artifacts/issue428/cli-contract.json + if-no-files-found: warn + qualify: name: Windows ${{ matrix.scenario }} runs-on: windows-2025 diff --git a/scripts/diagnostics/windows-cli-lifecycle.mjs b/scripts/diagnostics/windows-cli-lifecycle.mjs new file mode 100644 index 000000000..c6bce8462 --- /dev/null +++ b/scripts/diagnostics/windows-cli-lifecycle.mjs @@ -0,0 +1,64 @@ +// Real CLI + Task Scheduler on a disposable Windows runner. Never run on a user's machine. +import assert from 'node:assert/strict'; +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import { mkdir, writeFile } from 'node:fs/promises'; +import { resolve } from 'node:path'; +const exec = promisify(execFile); +assert.equal(process.platform, 'win32'); +assert.equal(process.env.GITHUB_ACTIONS, 'true'); +const binary = resolve(process.argv[2]); +const options = { timeout: 30_000, windowsHide: true, env: { ...process.env } }; +delete options.env.MDBASE_CONNECT_HOME; +delete options.env.MDBASE_CONNECT_SOCKET; +const report = { results: [], passed: false }; +let ownsTask = false; +async function cli(command) { + const result = await exec(binary, ['--json', 'connect', 'daemon', command], options); + report.results.push({ command, ...result }); + // Match Electron: parse ALL stdout, not the last line or first JSON-looking substring. + const value = JSON.parse(result.stdout); + assert.ok(value && typeof value === 'object' && !Array.isArray(value)); + return value; +} +async function running(expected) { + for (let i = 0; i < 30; i++) { + if ((await cli('status')).running === expected) return; + await new Promise(resolve => setTimeout(resolve, 500)); + } + assert.fail(`Daemon did not reach running=${expected}`); +} +try { + const existing = await exec('schtasks', ['/Query', '/TN', 'mdbase connect'], options).then(() => true, error => { + if (error.code !== 1) throw error; + return false; + }); + assert.equal(existing, false, 'Refusing to replace an existing task'); + assert.equal((await cli('status')).installed, false); + ownsTask = true; // Installation can succeed even if its stdout cannot be parsed. + assert.equal((await cli('install')).installed, true); + await running(true); + // Reconciliation replaces an installed runtime, exercising stop/create/run output together. + assert.equal((await cli('install')).installed, true); + await running(true); + assert.equal((await cli('stop')).stopped, true); + await running(false); + assert.equal((await cli('start')).started, true); + await running(true); + assert.equal((await cli('restart')).restarted, true); + await running(true); + await cli('stop'); + assert.equal((await cli('uninstall')).installed, false); + assert.equal((await cli('status')).installed, false); + report.passed = true; +} catch (error) { + report.error = { message: error.message, stdout: error.stdout, stderr: error.stderr }; + throw error; +} finally { + if (ownsTask) { + await exec('schtasks', ['/End', '/TN', 'mdbase connect'], options).catch(() => {}); + await exec('schtasks', ['/Delete', '/F', '/TN', 'mdbase connect'], options).catch(() => {}); + } + await mkdir('.artifacts/issue428', { recursive: true }); + await writeFile('.artifacts/issue428/cli-contract.json', JSON.stringify(report, null, 2)); +} From e9f1e093785108edb89ad1be0d128b68786c6471 Mon Sep 17 00:00:00 2001 From: callumalpass Date: Thu, 17 Sep 2026 07:56:15 +1000 Subject: [PATCH 2/5] test(windows): investigate elevated task replacement under standard user --- .../workflows/windows-daemon-lifecycle.yml | 2 +- scripts/diagnostics/windows-428.ps1 | 40 +++++++++++++++---- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/.github/workflows/windows-daemon-lifecycle.yml b/.github/workflows/windows-daemon-lifecycle.yml index 14278a4f8..f75d41906 100644 --- a/.github/workflows/windows-daemon-lifecycle.yml +++ b/.github/workflows/windows-daemon-lifecycle.yml @@ -62,7 +62,7 @@ jobs: strategy: fail-fast: false matrix: - scenario: [registration, fresh, upgrade] + scenario: [registration, elevated-registration, fresh, upgrade] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/scripts/diagnostics/windows-428.ps1 b/scripts/diagnostics/windows-428.ps1 index 9ece354a9..04c0c34f3 100644 --- a/scripts/diagnostics/windows-428.ps1 +++ b/scripts/diagnostics/windows-428.ps1 @@ -1,5 +1,5 @@ param( - [ValidateSet('registration', 'fresh', 'upgrade')][string]$Scenario = 'registration', + [ValidateSet('registration', 'elevated-registration', 'fresh', 'upgrade')][string]$Scenario = 'registration', [switch]$Child, [string]$Root ) @@ -17,7 +17,7 @@ if (-not $Child) { $user = 'mdbase428test' $password = ConvertTo-SecureString ('Aa1!' + [guid]::NewGuid().ToString('N')) -AsPlainText -Force $localUser = $null - if ($Scenario -eq 'registration') { $localUser = New-LocalUser -Name $user -Password $password -Description 'Disposable issue 428 qualification' } + if ($Scenario -in @('registration', 'elevated-registration')) { $localUser = New-LocalUser -Name $user -Password $password -Description 'Disposable issue 428 qualification' } Add-Type @' using System; using System.Text; @@ -37,6 +37,22 @@ public static class TestUserProfile { & icacls.exe $Root /grant "${env:COMPUTERNAME}\${user}:(OI)(CI)M" | Out-Null if ($LASTEXITCODE -ne 0) { throw 'Could not grant fixture access.' } } + if ($Scenario -eq 'elevated-registration') { + # Administrator-created task for the SAME standard user. This differs + # from a task created and replaced entirely by the unprivileged user. + $sid = $localUser.SID.Value + $xml = @" + +true$sid +$sidInteractiveTokenLeastPrivilege +C:\Windows\System32\cmd.exe/c exit 0 + +"@ + $taskXml = Join-Path $Root 'elevated-task.xml' + $xml | Set-Content -Encoding Unicode $taskXml + & schtasks.exe /Create /F /TN 'mdbase connect' /XML $taskXml + if ($LASTEXITCODE -ne 0) { throw 'Could not seed elevated task fixture.' } + } # Use Process directly: no PowerShell Start-Process job-tree wait when a # tested CLI launches its long-running background daemon. $info = [Diagnostics.ProcessStartInfo]::new((Get-Process -Id $PID).Path) @@ -125,8 +141,8 @@ function Wait-Running([string]$Binary, [bool]$Expected) { throw "Task did not bring daemon running state to $Expected in this logon session." } try { - if ($Scenario -eq 'registration' -and -not $report.standardUser) { throw 'Registration must use a non-administrator test token.' } - $report['lifecycleAccount'] = if ($Scenario -eq 'registration') { 'noninteractive-standard-user-registration-only' } else { 'existing-interactive-runner-account-with-limited-task' } + if ($Scenario -in @('registration', 'elevated-registration') -and -not $report.standardUser) { throw 'Registration must use a non-administrator test token.' } + $report['lifecycleAccount'] = if ($Scenario -in @('registration', 'elevated-registration')) { 'noninteractive-standard-user-registration-only' } else { 'existing-interactive-runner-account-with-limited-task' } # Replace inherited runner-administrator environment with the profile that # Windows registered for this authenticated SID. No product state overrides. $profileKey = "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\$($identity.User.Value)" @@ -146,17 +162,27 @@ try { } $initial = if ($Scenario -eq 'upgrade') { Binary '96' } else { $binary } - Require-Success (Invoke-Probe 'production-scoped-install' $probe @('install', $initial, $state)) + $installation = Invoke-Probe 'production-scoped-install' $probe @('install', $initial, $state) + if ($Scenario -eq 'elevated-registration') { + # Diagnostic hypothesis test, NOT an assertion of successful recovery. + $report['elevatedReplacementSucceeded'] = $installation.exitCode -eq 0 + if ($installation.exitCode -ne 0) { + if ($installation.stderr -notmatch 'Access is denied') { throw 'Unexpected elevated-task replacement failure.' } + $report['passed'] = $true + return + } + } + Require-Success $installation $task = Get-ScheduledTask -TaskName 'mdbase connect' $report['task'] = @{ triggerMatchesUser = (Is-CurrentUser $task.Triggers[0].UserId); principalMatchesUser = (Is-CurrentUser $task.Principal.UserId); runLevel = [string]$task.Principal.RunLevel; logonType = [string]$task.Principal.LogonType } if ((Is-CurrentUser 'S-1-5-18') -or -not $report.task.triggerMatchesUser -or -not $report.task.principalMatchesUser -or $report.task.runLevel -ne 'Limited' -or $report.task.logonType -ne 'Interactive') { throw 'Task identity/least-privilege invariant failed.' } - if ($Scenario -ne 'registration') { + if ($Scenario -in @('fresh', 'upgrade')) { Wait-Running $initial $true Require-Success (Invoke-Probe 'stop-before-replacement' $probe @('stop')) Wait-Running $initial $false } Require-Success (Invoke-Probe 'production-scoped-replacement' $probe @('install', $binary, $state)) - if ($Scenario -ne 'registration') { + if ($Scenario -in @('fresh', 'upgrade')) { Wait-Running $binary $true Require-Success (Invoke-Probe 'stop-before-cold-start' $probe @('stop')) Wait-Running $binary $false From 3627e0ee99a231b2547ebde25d56eb73c93c5687 Mon Sep 17 00:00:00 2001 From: callumalpass Date: Thu, 17 Sep 2026 07:57:57 +1000 Subject: [PATCH 3/5] fix(windows): keep scheduler output out of CLI JSON stdout --- config/architecture-budgets.json | 2 +- crates/connect-cli/src/service.rs | 8 ++-- .../connect-cli/src/service/windows_task.rs | 42 +++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/config/architecture-budgets.json b/config/architecture-budgets.json index 2e07b54b2..595d1aa2f 100644 --- a/config/architecture-budgets.json +++ b/config/architecture-budgets.json @@ -46,7 +46,7 @@ "productionFiles": 695, "relativeImports": 1512, "workspacePackages": 24, - "rustPublicDeclarations": 3208, + "rustPublicDeclarations": 3209, "typeScriptExportDeclarations": 2466, "mdbaseCollectionReferences": 17, "typedCollectionReferences": 1 diff --git a/crates/connect-cli/src/service.rs b/crates/connect-cli/src/service.rs index 11141fb40..55c69417a 100644 --- a/crates/connect-cli/src/service.rs +++ b/crates/connect-cli/src/service.rs @@ -648,7 +648,7 @@ mod platform { .map_err(|error| format!("Could not write the Connect task definition: {error}"))?; // Close before schtasks reads it, and remove it on both success and failure. let task_path = task_file.into_temp_path(); - run_checked( + windows_task::run( Command::new("schtasks") .args(["/Create", "/F", "/TN", "mdbase connect", "/XML"]) .arg(&task_path), @@ -666,7 +666,7 @@ mod platform { } pub fn uninstall() -> Result<(), String> { - let _ = run_checked( + let _ = windows_task::run( Command::new("schtasks").args(["/Delete", "/F", "/TN", "mdbase connect"]), "remove the Connect background task", ); @@ -677,14 +677,14 @@ mod platform { } pub fn start() -> Result<(), String> { - run_checked( + windows_task::run( Command::new("schtasks").args(["/Run", "/TN", "mdbase connect"]), "start the Connect daemon", ) } pub fn stop() -> Result<(), String> { - run_checked( + windows_task::run( Command::new("schtasks").args(["/End", "/TN", "mdbase connect"]), "stop the Connect daemon", ) diff --git a/crates/connect-cli/src/service/windows_task.rs b/crates/connect-cli/src/service/windows_task.rs index ceb8d9b66..c569b16d2 100644 --- a/crates/connect-cli/src/service/windows_task.rs +++ b/crates/connect-cli/src/service/windows_task.rs @@ -1,6 +1,30 @@ //! Windows task definitions must scope the logon trigger as well as the //! execution principal. An unscoped ONLOGON trigger requires administrator rights. +/// Scheduler chatter is not CLI output: in particular, `--json` callers parse +/// the entire stdout stream. Capture both pipes and retain diagnostics on error. +#[cfg(windows)] +pub(super) fn run(command: &mut std::process::Command, action: &str) -> Result<(), String> { + use std::os::windows::process::CommandExt; + command.creation_flags(0x0800_0000); // CREATE_NO_WINDOW + let output = command + .output() + .map_err(|error| format!("Could not {action}: {error}"))?; + if output.status.success() { + return Ok(()); + } + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + Err(format!( + "Could not {action}: command exited with {}.\n{}\n{}", + output.status.code().unwrap_or(-1), + stderr.trim(), + stdout.trim(), + ) + .trim() + .to_string()) +} + pub(super) fn definition(executable: &str, state_dir: &str, sid: &str) -> Vec { // The Windows command-line parser consumes backslashes before a closing // quote. Preserve a root/trailing separator in the quoted state directory. @@ -118,6 +142,24 @@ pub(super) fn current_user_sid() -> Result { mod tests { use super::*; + #[cfg(windows)] + #[test] + fn scheduler_errors_retain_exit_code_and_diagnostics() { + let error = run( + std::process::Command::new("cmd.exe").args([ + "/D", + "/C", + "echo task detail & echo Access is denied 1>&2 & exit /b 5", + ]), + "install the Connect background task", + ) + .unwrap_err(); + assert!(error.contains("Could not install the Connect background task")); + assert!(error.contains("command exited with 5")); + assert!(error.contains("Access is denied")); + assert!(error.contains("task detail")); + } + fn xml(executable: &str, state_dir: &str, sid: &str) -> String { let bytes = definition(executable, state_dir, sid); assert_eq!(&bytes[..2], &[0xff, 0xfe]); From eedab4e595ffd05faa41557daa958b2451aeb699 Mon Sep 17 00:00:00 2001 From: callumalpass Date: Thu, 17 Sep 2026 08:00:51 +1000 Subject: [PATCH 4/5] test(windows): qualify owner-assisted repair without bypassing task ACLs --- docs/ci-qualification.md | 15 +++++++++++++++ docs/cli-daemon.md | 14 +++++++++++++- scripts/diagnostics/windows-428.ps1 | 27 +++++++++++++++++++++------ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/docs/ci-qualification.md b/docs/ci-qualification.md index 17b221766..a270be8c0 100644 --- a/docs/ci-qualification.md +++ b/docs/ci-qualification.md @@ -104,6 +104,21 @@ new non-interactive test accounts cannot execute `InteractiveToken` tasks, and successful registration alone is not counted as daemon execution. This does not replace packaged Electron/Squirrel or actual sign-out/logon qualification. +A separate job builds the **current actual CLI**, using the pinned engine, and +executes `--json connect daemon` commands through Node's `execFile` with the +same whole-stdout `JSON.parse` contract as Electron. It verifies install, +replacement, stop, start, restart, uninstall, and real daemon running state. +Exit-code-only service tests cannot detect scheduler output corrupting CLI JSON. +Raw stdout/stderr and the failing command are retained as a bounded artifact. +This job uses the interactive runner account, not a standard-user desktop session. + +The elevated-registration fixture creates a task as the runner administrator +for a separate standard-user principal. It proves that principal cannot replace +the administrator-owned task, then has the fixture owner remove it and verifies +normal standard-user registration again. This qualifies an ACL failure and +owner-assisted repair, **not** automatic permission recovery or same-account +UAC token transitions. No production task permissions are weakened. + ## Publication Server and client images are built after a successful `main` push qualification, diff --git a/docs/cli-daemon.md b/docs/cli-daemon.md index 9fe66136d..522fc442a 100644 --- a/docs/cli-daemon.md +++ b/docs/cli-daemon.md @@ -105,7 +105,19 @@ with `/RL LIMITED`. Registration uses a temporary UTF-16 task definition and keeps the existing task name and installed marker, so an accessible existing task can be replaced normally. Installation does not elevate or take ownership of another account's task. Account credentials and application grants are not -changed by task registration. +changed by task registration. Task Scheduler subprocess output is captured: +its `SUCCESS:` messages never enter CLI JSON stdout, while failed commands +retain their diagnostic text and exit code. Log commands still stream normally. + +An existing administrator-owned task can still deny replacement by a standard +user. A successful fresh-user registration test does not qualify this upgrade +case. Do not automatically elevate, loosen the task ACL, or launch a competing +daemon. First inspect the task's account and executable to establish ownership. +If it is confirmed to be this installation's stale task, its owner/administrator +can stop and remove **only that task**, then launch Connect normally to recreate +it for the current user. Preserve the Connect state directory, credentials, +collections, and grants. This task repair does not resolve rejected account +credentials or missing mirror files; those require separate diagnosis. The CLI models daemon targeting explicitly. With no state or endpoint override, daemon lifecycle commands target the default installed per-user service. Any diff --git a/scripts/diagnostics/windows-428.ps1 b/scripts/diagnostics/windows-428.ps1 index 04c0c34f3..2ff8a707e 100644 --- a/scripts/diagnostics/windows-428.ps1 +++ b/scripts/diagnostics/windows-428.ps1 @@ -68,6 +68,22 @@ public static class TestUserProfile { $process = [Diagnostics.Process]::Start($info) if (-not $process.WaitForExit(240000)) { $process.Kill($true); throw 'Native lifecycle child timed out; partial results retained.' } if ($process.ExitCode -ne 0) { throw "Native lifecycle child failed with exit $($process.ExitCode)." } + if ($Scenario -eq 'elevated-registration') { + $reportPath = Join-Path $Root 'result.json' + $beforeRepair = Get-Content $reportPath -Raw | ConvertFrom-Json + if ($beforeRepair.elevatedReplacementSucceeded -ne $false) { throw 'Admin-owned task denial was not reproduced.' } + # Only the elevated fixture owner removes its task. Product code must + # not bypass its ACL or start a competing task/process on denial. + & schtasks.exe /Delete /F /TN 'mdbase connect' | Out-Null + if ($LASTEXITCODE -ne 0) { throw 'Fixture owner could not remove its task.' } + $info.ArgumentList[$info.ArgumentList.IndexOf('-Scenario') + 1] = 'registration' + $process = [Diagnostics.Process]::Start($info) + if (-not $process.WaitForExit(240000)) { $process.Kill($true); throw 'Post-repair registration timed out.' } + $afterRepair = Get-Content $reportPath -Raw | ConvertFrom-Json + @{ scenario = $Scenario; beforeRepair = $beforeRepair; afterRepair = $afterRepair; passed = ($process.ExitCode -eq 0 -and $afterRepair.passed) } | + ConvertTo-Json -Depth 12 | Set-Content -Encoding UTF8 $reportPath + if ($process.ExitCode -ne 0 -or -not $afterRepair.passed) { throw 'Standard-user registration after task removal failed.' } + } } finally { $old = $ErrorActionPreference $ErrorActionPreference = 'Continue' @@ -164,13 +180,12 @@ try { $initial = if ($Scenario -eq 'upgrade') { Binary '96' } else { $binary } $installation = Invoke-Probe 'production-scoped-install' $probe @('install', $initial, $state) if ($Scenario -eq 'elevated-registration') { - # Diagnostic hypothesis test, NOT an assertion of successful recovery. + # Assert the reproduced ACL boundary, NOT successful automatic recovery. $report['elevatedReplacementSucceeded'] = $installation.exitCode -eq 0 - if ($installation.exitCode -ne 0) { - if ($installation.stderr -notmatch 'Access is denied') { throw 'Unexpected elevated-task replacement failure.' } - $report['passed'] = $true - return - } + if ($installation.exitCode -eq 0) { throw 'Admin-owned task denial was not reproduced.' } + if ($installation.stderr -notmatch 'Access is denied') { throw 'Unexpected elevated-task replacement failure.' } + $report['passed'] = $true + return } Require-Success $installation $task = Get-ScheduledTask -TaskName 'mdbase connect' From e8d995c6c5b77c572bc6ccb4fc6cb3a0a7f449c8 Mon Sep 17 00:00:00 2001 From: callumalpass Date: Thu, 17 Sep 2026 08:02:06 +1000 Subject: [PATCH 5/5] docs: record scheduler output boundary and reviewed surface --- docs/code-quality.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/code-quality.md b/docs/code-quality.md index 0c4f2b1ae..34d57ca8a 100644 --- a/docs/code-quality.md +++ b/docs/code-quality.md @@ -172,6 +172,13 @@ actual installer, alongside pure encoding tests. This adds one production file (total: 3,200), not a second service owner, recovery state, or public protocol. File-size and cycle budgets are unchanged. +The follow-up Windows CLI JSON repair adds one parent-visible scheduler-command +helper to that same module (integrated Rust visibility budget: 3,209). It replaces +inherited scheduler stdout at all four lifecycle call sites with captured output +and retained failure diagnostics. It deliberately does not change the streaming +log runner. Native actual-CLI tests reproduce the corrupt JSON before the fix; +no new production module, protocol, state, or permission bypass is introduced. + Composition roots and package facades should approach these end-state shapes: - server `app.ts`: registration and lifecycle wiring only;