From 48a7e2dbd8a6a1c4cbcc52980d41377fce529f0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 8 Sep 2026 17:24:42 -0400 Subject: [PATCH 01/14] test(agent): cover policy management end to end Exercise policy management through a protected standard-user client and a LocalSystem client without weakening executable or path checks. Cover validation and write denial, managed Create, Update, Repair, stale conflicts, confirmed overwrite, restart persistence, durable managed authority, and observable audit outcomes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Cargo.lock | 2 + crates/agent-policy-tester/Cargo.toml | 2 + crates/agent-policy-tester/run-as-system.ps1 | 2 +- crates/agent-policy-tester/run-unelevated.ps1 | 139 ++++++ crates/agent-policy-tester/src/windows.rs | 458 +++++++++++++++++- 5 files changed, 577 insertions(+), 26 deletions(-) create mode 100644 crates/agent-policy-tester/run-unelevated.ps1 diff --git a/Cargo.lock b/Cargo.lock index bdf99a73c..0cc021554 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -94,6 +94,8 @@ dependencies = [ "serde_json", "tempfile", "tokio 1.52.3", + "win-api-wrappers", + "windows 0.61.3", ] [[package]] diff --git a/crates/agent-policy-tester/Cargo.toml b/crates/agent-policy-tester/Cargo.toml index ba2f20f77..0c549b5d1 100644 --- a/crates/agent-policy-tester/Cargo.toml +++ b/crates/agent-policy-tester/Cargo.toml @@ -12,6 +12,8 @@ fastrand = "2" serde_json = "1" tempfile = "3" tokio = { version = "1", features = ["io-util", "macros", "net", "process", "rt-multi-thread", "time"] } +win-api-wrappers = { path = "../win-api-wrappers" } +windows = { version = "0.61", features = ["Win32_Security"] } [lints] workspace = true diff --git a/crates/agent-policy-tester/run-as-system.ps1 b/crates/agent-policy-tester/run-as-system.ps1 index 8520d10ec..14e9028e5 100644 --- a/crates/agent-policy-tester/run-as-system.ps1 +++ b/crates/agent-policy-tester/run-as-system.ps1 @@ -76,7 +76,7 @@ public static class AgentPolicyTesterNativeDirectory "Staged policy tester at $stagedTesterPath" | Out-File $outputPath -Append Get-Acl -LiteralPath $stagingPath | Format-List Owner, Sddl | Out-File $outputPath -Append Get-Acl -LiteralPath $stagedTesterPath | Format-List Owner, Sddl | Out-File $outputPath -Append - & $stagedTesterPath $agentPath 2>&1 | Out-File $outputPath -Append + & $stagedTesterPath $agentPath elevated 2>&1 | Out-File $outputPath -Append $exitCode = $LASTEXITCODE } catch { $_ | Out-File $outputPath -Append diff --git a/crates/agent-policy-tester/run-unelevated.ps1 b/crates/agent-policy-tester/run-unelevated.ps1 new file mode 100644 index 000000000..341edc706 --- /dev/null +++ b/crates/agent-policy-tester/run-unelevated.ps1 @@ -0,0 +1,139 @@ +param( + [ValidateSet("Orchestrate", "Stage", "Run", "Cleanup")] + [string] $Action = "Orchestrate", + [string] $TesterPath, + [string] $StagedTesterPath, + [string] $StagingPath, + [string] $AgentPath, + [string] $TempPath +) + +$ErrorActionPreference = "Stop" + +if ($Action -eq "Run") { + $env:TEMP = $TempPath + $env:TMP = $TempPath + & $StagedTesterPath $AgentPath unelevated + exit $LASTEXITCODE +} + +if ($Action -eq "Cleanup") { + for ($attempt = 0; $attempt -lt 20 -and (Test-Path -LiteralPath $StagingPath); $attempt++) { + try { + Remove-Item -LiteralPath $StagingPath -Recurse -Force + } catch { + if ($attempt -eq 19) { + throw "Failed to remove $StagingPath after 20 attempts: $_" + } + Start-Sleep -Milliseconds 250 + } + } + exit $(if (Test-Path -LiteralPath $StagingPath) { 1 } else { 0 }) +} + +if ($Action -eq "Stage") { + Add-Type -TypeDefinition @' +using System; +using System.ComponentModel; +using System.Runtime.InteropServices; + +public static class AgentPolicyUnelevatedTesterNativeDirectory +{ + [StructLayout(LayoutKind.Sequential)] + private struct SecurityAttributes + { + internal int Length; + internal IntPtr SecurityDescriptor; + internal int InheritHandle; + } + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern bool CreateDirectoryW(string path, ref SecurityAttributes securityAttributes); + + public static void Create(string path, byte[] securityDescriptor) + { + GCHandle pinnedDescriptor = GCHandle.Alloc(securityDescriptor, GCHandleType.Pinned); + try + { + SecurityAttributes attributes = new SecurityAttributes + { + Length = Marshal.SizeOf(), + SecurityDescriptor = pinnedDescriptor.AddrOfPinnedObject(), + InheritHandle = 0, + }; + if (!CreateDirectoryW(path, ref attributes)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } + finally + { + pinnedDescriptor.Free(); + } + } +} +'@ + $directorySecurity = [System.Security.AccessControl.DirectorySecurity]::new() + $directorySecurity.SetSecurityDescriptorSddlForm( + 'O:SYG:SYD:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;GRGX;;;BU)' + ) + [AgentPolicyUnelevatedTesterNativeDirectory]::Create( + $StagingPath, + $directorySecurity.GetSecurityDescriptorBinaryForm() + ) + if (Get-ChildItem -LiteralPath $StagingPath -Force) { + throw "The atomically protected staged tester directory was not empty" + } + + Copy-Item -LiteralPath $TesterPath -Destination $StagedTesterPath + & icacls.exe $StagedTesterPath /setowner '*S-1-5-18' + if ($LASTEXITCODE -ne 0) { + throw "Failed to set the staged tester owner" + } + & icacls.exe $StagedTesterPath /inheritance:r /grant:r '*S-1-5-18:(F)' '*S-1-5-32-544:(F)' '*S-1-5-32-545:(RX)' + if ($LASTEXITCODE -ne 0) { + throw "Failed to protect the staged tester executable" + } + Get-Acl -LiteralPath $StagingPath | Format-List Owner, Sddl + Get-Acl -LiteralPath $StagedTesterPath | Format-List Owner, Sddl + exit 0 +} + +$workspacePath = (Resolve-Path (Join-Path $PSScriptRoot "../..")).Path +$testerPath = Join-Path $workspacePath "target/debug/agent-policy-tester.exe" +$agentPath = Join-Path $workspacePath "target/debug/devolutions-agent.exe" +$outputPath = Join-Path $PSScriptRoot "agent-policy-tester-unelevated.out" +$stagingPath = Join-Path $env:ProgramData "dgw-agent-policy-tester-$([guid]::NewGuid().ToString('N'))" +$stagedTesterPath = Join-Path $stagingPath "agent-policy-tester.exe" +$tempPath = Join-Path $env:USERPROFILE "AppData\LocalLow\Temp" + +try { + Set-Content -LiteralPath $outputPath -Value "" + New-Item -ItemType Directory -Path $tempPath -Force | Out-Null + + $stageOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` + -Action Stage -TesterPath $testerPath -StagedTesterPath $stagedTesterPath -StagingPath $stagingPath 2>&1 + $stageExitCode = $LASTEXITCODE + $stageOutput | Out-File $outputPath -Append + if ($stageExitCode -ne 0) { + throw "LocalSystem tester staging failed with exit code $stageExitCode" + } + + $testerOutput = & psexec.exe -accepteula -l pwsh.exe -NoProfile -File $PSCommandPath ` + -Action Run -StagedTesterPath $stagedTesterPath -AgentPath $agentPath -TempPath $tempPath 2>&1 + $exitCode = $LASTEXITCODE + $testerOutput | Out-File $outputPath -Append +} catch { + $_ | Out-File $outputPath -Append + $exitCode = 1 +} finally { + $cleanupOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` + -Action Cleanup -StagingPath $stagingPath 2>&1 + $cleanupExitCode = $LASTEXITCODE + $cleanupOutput | Out-File $outputPath -Append + if ($cleanupExitCode -ne 0 -and $exitCode -eq 0) { + $exitCode = $cleanupExitCode + } +} + +exit $exitCode diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index db8d93d22..855f555fc 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -6,14 +6,21 @@ use anyhow::{Context as _, bail, ensure}; use serde_json::{Value, json}; use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; use tokio::net::windows::named_pipe::ClientOptions; +use win_api_wrappers::identity::sid::Sid; +use win_api_wrappers::process::Process; +use windows::Win32::Security::{TOKEN_DUPLICATE, TOKEN_QUERY, WinBuiltinAdministratorsSid, WinLocalSystemSid}; const FULL_POLICY: &str = include_str!("../../now-package-broker/src/assets/samples/corporate-allowlist.policy.json"); +const MANAGED_POLICY_RELATIVE_PATH: &str = r"Devolutions\PackageBroker\package-broker-policy.json"; +const MANAGED_AUTHORITY_MARKER: &str = r"Devolutions\PackageBroker\.package-broker-managed-authority.v1"; +const LEGACY_POLICY_RELATIVE_PATH: &str = r"Devolutions\Agent\package-broker-policy.json"; struct AgentHarness { child: tokio::process::Child, - _data_dir: tempfile::TempDir, + data_dir: tempfile::TempDir, pipe_name: String, policy_path: PathBuf, + program_data: Option, } impl AgentHarness { @@ -34,17 +41,43 @@ impl AgentHarness { Self::start_with_path(agent_path, data_dir, pipe_name, policy_path).await } + async fn start_unelevated(agent_path: &Path) -> anyhow::Result { + let data_dir = tempfile::tempdir().context("create unelevated Agent data directory")?; + let pipe_name = unique_pipe_name(); + let policy_path = data_dir.path().join("policy.json"); + Self::start_with_options(agent_path, data_dir, pipe_name, policy_path, false).await + } + + async fn start_managed_default(agent_path: &Path) -> anyhow::Result { + let data_dir = create_data_dir()?; + let pipe_name = unique_pipe_name(); + let policy_path = data_dir.path().join(MANAGED_POLICY_RELATIVE_PATH); + Self::start_with_options(agent_path, data_dir, pipe_name, policy_path, true).await + } + async fn start_with_path( agent_path: &Path, data_dir: tempfile::TempDir, pipe_name: String, policy_path: PathBuf, ) -> anyhow::Result { + Self::start_with_options(agent_path, data_dir, pipe_name, policy_path, false).await + } + + async fn start_with_options( + agent_path: &Path, + data_dir: tempfile::TempDir, + pipe_name: String, + policy_path: PathBuf, + use_managed_default: bool, + ) -> anyhow::Result { + let policy_path_config = (!use_managed_default).then_some(&policy_path); let config = json!({ + "LogFile": data_dir.path().join("agent-e2e"), "PackageBroker": { "Enabled": true, "PipeName": pipe_name, - "PolicyPath": policy_path, + "PolicyPath": policy_path_config, }, "__debug__": { "skip_broker_signature_validation": true, @@ -53,26 +86,35 @@ impl AgentHarness { std::fs::write(data_dir.path().join("agent.json"), serde_json::to_vec_pretty(&config)?) .context("write Agent configuration")?; - let child = tokio::process::Command::new(agent_path) - .env("DAGENT_CONFIG_PATH", data_dir.path()) - .arg("run") - .kill_on_drop(true) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .context("start Devolutions Agent")?; + let program_data = use_managed_default.then(|| data_dir.path().to_owned()); + let child = Self::spawn(agent_path, data_dir.path(), program_data.as_deref())?; let mut harness = Self { child, - _data_dir: data_dir, + data_dir, pipe_name, policy_path, + program_data, }; harness.wait_until_ready().await?; Ok(harness) } + fn spawn(agent_path: &Path, data_dir: &Path, program_data: Option<&Path>) -> anyhow::Result { + let mut command = tokio::process::Command::new(agent_path); + command + .env("DAGENT_CONFIG_PATH", data_dir) + .arg("run") + .kill_on_drop(true) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + if let Some(program_data) = program_data { + command.env("ProgramData", program_data); + } + command.spawn().context("start Devolutions Agent") + } + async fn wait_until_ready(&mut self) -> anyhow::Result<()> { let deadline = Instant::now() + Duration::from_secs(20); @@ -89,6 +131,34 @@ impl AgentHarness { } } } + + async fn restart(&mut self, agent_path: &Path) -> anyhow::Result<()> { + self.stop().await?; + self.start_again(agent_path).await + } + + async fn stop(&mut self) -> anyhow::Result<()> { + self.child.start_kill().context("stop Devolutions Agent")?; + self.child.wait().await.context("wait for Devolutions Agent to stop")?; + Ok(()) + } + + async fn start_again(&mut self, agent_path: &Path) -> anyhow::Result<()> { + self.child = Self::spawn(agent_path, self.data_dir.path(), self.program_data.as_deref())?; + self.wait_until_ready().await + } + + fn logs(&self) -> anyhow::Result { + let mut logs = String::new(); + for entry in std::fs::read_dir(self.data_dir.path()).context("read Agent log directory")? { + let entry = entry.context("read Agent log entry")?; + let name = entry.file_name(); + if name.to_string_lossy().starts_with("agent-e2e") { + logs.push_str(&std::fs::read_to_string(entry.path()).context("read Agent log")?); + } + } + Ok(logs) + } } impl Drop for AgentHarness { @@ -108,25 +178,94 @@ impl HttpResponse { } } +#[derive(Clone, Copy, PartialEq, Eq)] +enum Mode { + Unelevated, + Elevated, +} + +impl Mode { + fn parse(value: &str) -> anyhow::Result { + match value { + "unelevated" => Ok(Self::Unelevated), + "elevated" => Ok(Self::Elevated), + _ => bail!("unknown mode '{value}'; expected 'unelevated' or 'elevated'"), + } + } +} + pub(crate) async fn run() -> anyhow::Result<()> { - let agent_path = std::env::args_os() - .nth(1) + let mut args = std::env::args_os().skip(1); + let agent_path = args + .next() .map(PathBuf::from) - .context("usage: agent-policy-tester ")?; + .context("usage: agent-policy-tester ")?; + let mode = args + .next() + .and_then(|value| value.into_string().ok()) + .context("test mode must be 'unelevated' or 'elevated'") + .and_then(|value| Mode::parse(&value))?; + ensure!(args.next().is_none(), "unexpected extra command-line arguments"); + verify_process_token(mode)?; ensure!( agent_path.is_file(), "agent executable does not exist: {}", agent_path.display() ); - unavailable_policy_and_method_restrictions(&agent_path).await?; - complete_snapshots_across_reload(&agent_path).await?; - redirected_policy_paths_fail_closed(&agent_path).await?; - management_write_tokens_survive_watcher_reload(&agent_path).await?; + match mode { + Mode::Unelevated => standard_user_management(&agent_path).await?, + Mode::Elevated => { + unavailable_policy_and_method_restrictions(&agent_path).await?; + complete_snapshots_across_reload(&agent_path).await?; + redirected_policy_paths_fail_closed(&agent_path).await?; + management_write_tokens_survive_watcher_reload(&agent_path).await?; + managed_policy_lifecycle(&agent_path).await?; + } + } Ok(()) } +fn verify_process_token(mode: Mode) -> anyhow::Result<()> { + let token = Process::current_process() + .token(TOKEN_QUERY | TOKEN_DUPLICATE) + .context("open tester process token")?; + let administrators = + Sid::from_well_known(WinBuiltinAdministratorsSid, None).context("construct Administrators SID")?; + let is_administrator = token + .is_member(&administrators) + .context("query tester Administrators membership")?; + let system = Sid::from_well_known(WinLocalSystemSid, None).context("construct LocalSystem SID")?; + let user = token.sid_and_attributes().context("query tester user SID")?.sid; + match mode { + Mode::Unelevated => { + ensure!(user != system, "unelevated mode requires a standard user account"); + ensure!( + !is_administrator, + "unelevated mode requires disabled Administrators membership" + ); + } + Mode::Elevated => { + ensure!(is_administrator, "elevated mode requires Administrators membership"); + ensure!( + token.is_elevated().context("query tester token elevation")?, + "elevated mode requires an elevated token" + ); + ensure!(user == system, "elevated mode requires the LocalSystem account"); + } + } + Ok(()) +} + +fn unique_pipe_name() -> String { + format!( + r"\\.\pipe\Devolutions.Now.PackageBroker.tests.{}.{}", + std::process::id(), + fastrand::u64(..) + ) +} + async fn request(pipe_name: &str, method: &str, path: &str) -> anyhow::Result { request_with_body(pipe_name, method, path, None, &[]).await } @@ -360,12 +499,7 @@ async fn policy_management(agent: &AgentHarness) -> anyhow::Result { Ok(response.json()?["Management"].clone()) } -async fn replace_policy( - agent: &AgentHarness, - operation: &str, - expected_store_token: Value, - draft: Value, -) -> anyhow::Result { +async fn validate_policy(agent: &AgentHarness, draft: &Value) -> anyhow::Result { let validation_request = json!({ "RequestKind": "PolicyValidationRequest", "RequestVersion": "1.0", @@ -386,12 +520,24 @@ async fn replace_policy( ); let validation = validation_response.json()?["Validation"].clone(); ensure!(validation["IsValid"] == true, "policy validation failed"); + Ok(validation) +} + +async fn replace_policy_response( + agent: &AgentHarness, + operation: &str, + conflict_handling: &str, + expected_store_token: Value, + draft: Value, +) -> anyhow::Result { + let validation = validate_policy(agent, &draft).await?; let replacement_request = json!({ "RequestKind": "PolicyReplacementRequest", "RequestVersion": "1.0", "ExpectedStoreToken": expected_store_token, "Operation": operation, - "ConflictHandling": "Reject", + "ConflictHandling": conflict_handling, + "WarningsAcknowledged": true, "Draft": validation["CanonicalDraft"], "ValidationReceipt": validation["ValidationReceipt"] }); @@ -403,6 +549,16 @@ async fn replace_policy( &serde_json::to_vec(&replacement_request)?, ) .await?; + Ok(response) +} + +async fn replace_policy( + agent: &AgentHarness, + operation: &str, + expected_store_token: Value, + draft: Value, +) -> anyhow::Result { + let response = replace_policy_response(agent, operation, "Reject", expected_store_token, draft).await?; ensure!(response.status == 200, "{operation} returned HTTP {}", response.status); response.json() } @@ -459,6 +615,258 @@ async fn management_write_tokens_survive_watcher_reload(agent_path: &Path) -> an Ok(()) } +async fn standard_user_management(agent_path: &Path) -> anyhow::Result<()> { + let agent = AgentHarness::start_unelevated(agent_path).await?; + let management = policy_management(&agent).await?; + ensure!(management["State"] == "Missing", "expected a missing policy"); + + let valid_draft = policy_draft("tests.standard-user", "Test"); + let validation = validate_policy(&agent, &valid_draft).await?; + ensure!( + validation["CanonicalDraft"].is_object() && validation["ValidationReceipt"].is_string(), + "valid draft did not produce a canonical draft and receipt" + ); + + let mut invalid_draft = valid_draft.clone(); + invalid_draft["$schema"] = json!("https://example.com/not-the-policy-draft-schema.json"); + let invalid_request = json!({ + "RequestKind": "PolicyValidationRequest", + "RequestVersion": "1.0", + "Draft": invalid_draft + }); + let invalid_response = request_with_body( + &agent.pipe_name, + "POST", + "/v1/policy/validate", + Some("application/json"), + &serde_json::to_vec(&invalid_request)?, + ) + .await?; + ensure!( + invalid_response.status == 200, + "invalid draft validation returned HTTP {}", + invalid_response.status + ); + let invalid_validation = invalid_response.json()?["Validation"].clone(); + ensure!(invalid_validation["IsValid"] == false, "invalid draft was accepted"); + ensure!( + invalid_validation.get("CanonicalDraft").is_none(), + "invalid draft returned a canonical draft" + ); + + let denied = replace_policy_response( + &agent, + "Create", + "Reject", + management["StoreToken"].clone(), + valid_draft, + ) + .await?; + ensure!( + denied.status == 403, + "standard-user Create returned HTTP {}", + denied.status + ); + ensure!( + denied.json()?["Code"] == "AdministratorRequired", + "standard-user Create did not require an administrator" + ); + wait_for_log(&agent, "Policy management write denied").await +} + +async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { + let mut agent = AgentHarness::start_managed_default(agent_path).await?; + let initial = policy_management(&agent).await?; + ensure!( + initial["State"] == "Missing", + "managed policy was not initially missing" + ); + ensure!( + initial["Source"] == "DefaultPath" && initial["WriteCapability"] == "Writable", + "isolated managed default path was not writable" + ); + + let created = replace_policy( + &agent, + "Create", + initial["StoreToken"].clone(), + policy_draft("tests.managed-lifecycle", "Create"), + ) + .await?; + ensure!( + created["Policy"]["Metadata"]["Revision"] == 1, + "Create did not assign revision 1" + ); + let authority_marker = agent.data_dir.path().join(MANAGED_AUTHORITY_MARKER); + ensure!( + authority_marker.is_file() && std::fs::metadata(&authority_marker)?.len() == 0, + "Create did not establish durable managed authority" + ); + wait_for_log(&agent, "Policy creation succeeded").await?; + + let updated = replace_policy( + &agent, + "Update", + created["Management"]["StoreToken"].clone(), + policy_draft("tests.managed-lifecycle", "Update"), + ) + .await?; + ensure!( + updated["Policy"]["Metadata"]["Revision"] == 2, + "Update did not increment the revision" + ); + + let secret = "malformed-policy-secret-marker"; + std::fs::write(&agent.policy_path, format!(r#"{{"unterminated":"{secret}"#)) + .context("write malformed external policy")?; + let invalid = wait_for_management(&agent, |management| management["State"] == "Invalid").await?; + let diagnostics = &invalid["InvalidDiagnostics"]; + ensure!( + diagnostics["Findings"] + .as_array() + .is_some_and(|findings| !findings.is_empty()), + "invalid policy did not produce diagnostics" + ); + ensure!( + !diagnostics.to_string().contains(secret), + "invalid policy diagnostics exposed file contents" + ); + wait_for_log(&agent, "External policy change rejected").await?; + + let repaired = replace_policy( + &agent, + "Repair", + invalid["StoreToken"].clone(), + policy_draft("tests.managed-repaired", "Repair"), + ) + .await?; + ensure!( + repaired["Policy"]["Metadata"]["Revision"] == 1, + "Repair did not assign revision 1" + ); + + let stale_token = repaired["Management"]["StoreToken"].clone(); + let mut external = empty_policy(); + external["Metadata"]["Id"] = json!("tests.managed-external"); + std::fs::write(&agent.policy_path, serde_json::to_vec_pretty(&external)?).context("write valid external policy")?; + wait_for_management(&agent, |management| { + management["Policy"]["Metadata"]["Id"] == "tests.managed-external" + }) + .await?; + wait_for_log(&agent, "External policy change applied").await?; + + let stale = replace_policy_response( + &agent, + "Update", + "Reject", + stale_token, + policy_draft("tests.managed-external", "Stale"), + ) + .await?; + ensure!(stale.status == 409, "stale Update returned HTTP {}", stale.status); + let stale = stale.json()?; + ensure!( + stale["Code"] == "StalePolicyStoreToken" + && stale["Management"]["Policy"]["Metadata"]["Id"] == "tests.managed-external", + "stale Update did not return the current policy snapshot" + ); + wait_for_log(&agent, "stale_conflict").await?; + + let current_token = stale["Management"]["StoreToken"].clone(); + let confirmed = replace_policy_response( + &agent, + "Update", + "ConfirmOverwrite", + current_token.clone(), + policy_draft("tests.managed-external", "Confirmed overwrite"), + ) + .await?; + ensure!( + confirmed.status == 200, + "exact ConfirmOverwrite returned HTTP {}", + confirmed.status + ); + let confirmed = confirmed.json()?; + ensure!( + confirmed["Policy"]["Metadata"]["Revision"] == 2, + "confirmed Update did not increment the external policy revision" + ); + wait_for_log(&agent, "confirmed_overwrite").await?; + + let reused = replace_policy_response( + &agent, + "Update", + "ConfirmOverwrite", + current_token, + policy_draft("tests.managed-external", "Reused token"), + ) + .await?; + ensure!( + reused.status == 409 && reused.json()?["Code"] == "StalePolicyStoreToken", + "reused ConfirmOverwrite token did not conflict" + ); + + agent.restart(agent_path).await?; + let restarted = request(&agent.pipe_name, "GET", "/v1/policy").await?; + ensure!( + restarted.status == 200, + "policy read after restart returned HTTP {}", + restarted.status + ); + ensure!( + restarted.json()?["Policy"] == confirmed["Policy"], + "restart changed the active managed policy" + ); + ensure!( + authority_marker.is_file() && policy_management(&agent).await?["Source"] == "DefaultPath", + "restart lost durable managed authority" + ); + + agent.stop().await?; + let legacy_path = agent.data_dir.path().join(LEGACY_POLICY_RELATIVE_PATH); + let legacy_dir = legacy_path.parent().context("legacy policy path has no parent")?; + std::fs::create_dir_all(legacy_dir).context("create isolated legacy policy directory")?; + secure_policy_path(legacy_dir, true)?; + std::fs::write(&legacy_path, serde_json::to_vec_pretty(&empty_policy())?) + .context("write isolated legacy policy")?; + secure_policy_path(&legacy_path, false)?; + std::fs::remove_file(&agent.policy_path).context("remove managed policy before authority restart")?; + agent.start_again(agent_path).await?; + let authority = policy_management(&agent).await?; + ensure!( + authority["State"] == "Missing" && authority["Source"] == "DefaultPath", + "durable managed authority allowed legacy policy rollback" + ); + ensure!( + request(&agent.pipe_name, "GET", "/v1/policy").await?.status == 404, + "legacy policy became active after managed authority was established" + ); + Ok(()) +} + +async fn wait_for_management(agent: &AgentHarness, predicate: impl Fn(&Value) -> bool) -> anyhow::Result { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + let management = policy_management(agent).await?; + if predicate(&management) { + return Ok(management); + } + ensure!(Instant::now() < deadline, "timed out waiting for policy state"); + tokio::time::sleep(Duration::from_millis(50)).await; + } +} + +async fn wait_for_log(agent: &AgentHarness, expected: &str) -> anyhow::Result<()> { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + if agent.logs()?.contains(expected) { + return Ok(()); + } + ensure!(Instant::now() < deadline, "Agent log did not contain '{expected}'"); + tokio::time::sleep(Duration::from_millis(50)).await; + } +} + async fn unavailable_policy_and_method_restrictions(agent_path: &Path) -> anyhow::Result<()> { let agent = AgentHarness::start(agent_path, None).await?; From 87d54221d214262898da794dd3af847ba32a495b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 8 Sep 2026 17:24:52 -0400 Subject: [PATCH 02/14] ci(agent): run policy E2E as both identities Run the protected policy tester under a restricted standard-user token before the existing LocalSystem lifecycle. Keep feature-gated route authorization tests in the same Windows job so the development signature feature cannot hide them from the default workspace suite. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82f7f49f7..39a4dc6ff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1385,6 +1385,16 @@ jobs: exit $LASTEXITCODE } + - name: Run Agent policy tester as standard user + shell: pwsh + run: | + ./crates/agent-policy-tester/run-unelevated.ps1 + $exitCode = $LASTEXITCODE + Get-Content -Path ./crates/agent-policy-tester/agent-policy-tester-unelevated.out + if ($exitCode -ne 0) { + exit $exitCode + } + - name: Run Agent policy tester as LocalSystem shell: pwsh run: | @@ -1396,6 +1406,10 @@ jobs: exit $exitCode } + - name: Run policy route authorization tests + shell: pwsh + run: cargo test --locked -p now-package-broker --features dev-skip-broker-signature + - name: Show sccache stats if: ${{ needs.preflight.outputs.sccache == 'true' && !cancelled() }} shell: pwsh From a3e3113369e8bc4ba566ee1cea9925582e204ae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 8 Sep 2026 21:45:45 -0400 Subject: [PATCH 03/14] test(agent): separate policy client identity Keep the Agent and test server under LocalSystem while a distinct restricted process exercises the named-pipe management endpoints. Coordinate readiness and shutdown through a protected, read-only test directory so authorization regressions cannot pass by inspecting the server token. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/Cargo.toml | 2 +- crates/agent-policy-tester/run-unelevated.ps1 | 93 ++++++- crates/agent-policy-tester/src/windows.rs | 258 ++++++++++++++---- 3 files changed, 299 insertions(+), 54 deletions(-) diff --git a/crates/agent-policy-tester/Cargo.toml b/crates/agent-policy-tester/Cargo.toml index 0c549b5d1..69f0f8d67 100644 --- a/crates/agent-policy-tester/Cargo.toml +++ b/crates/agent-policy-tester/Cargo.toml @@ -13,7 +13,7 @@ serde_json = "1" tempfile = "3" tokio = { version = "1", features = ["io-util", "macros", "net", "process", "rt-multi-thread", "time"] } win-api-wrappers = { path = "../win-api-wrappers" } -windows = { version = "0.61", features = ["Win32_Security"] } +windows = { version = "0.61", features = ["Win32_Security", "Win32_System_Threading"] } [lints] workspace = true diff --git a/crates/agent-policy-tester/run-unelevated.ps1 b/crates/agent-policy-tester/run-unelevated.ps1 index 341edc706..6508507d3 100644 --- a/crates/agent-policy-tester/run-unelevated.ps1 +++ b/crates/agent-policy-tester/run-unelevated.ps1 @@ -1,11 +1,16 @@ param( - [ValidateSet("Orchestrate", "Stage", "Run", "Cleanup")] + [ValidateSet("Orchestrate", "Stage", "Server", "Run", "Signal", "Cleanup")] [string] $Action = "Orchestrate", [string] $TesterPath, [string] $StagedTesterPath, [string] $StagingPath, [string] $AgentPath, - [string] $TempPath + [string] $TempPath, + [string] $ReadyPath, + [string] $StopPath, + [string] $StatusPath, + [string] $ServerOutputPath, + [string] $Nonce ) $ErrorActionPreference = "Stop" @@ -13,10 +18,29 @@ $ErrorActionPreference = "Stop" if ($Action -eq "Run") { $env:TEMP = $TempPath $env:TMP = $TempPath - & $StagedTesterPath $AgentPath unelevated + & $StagedTesterPath $AgentPath standard-client $ReadyPath $Nonce exit $LASTEXITCODE } +if ($Action -eq "Server") { + try { + & $StagedTesterPath $AgentPath standard-server $ReadyPath $StopPath $Nonce 2>&1 | + Out-File -LiteralPath $ServerOutputPath + $exitCode = $LASTEXITCODE + } catch { + $_ | Out-File -LiteralPath $ServerOutputPath -Append + $exitCode = 1 + } finally { + Set-Content -LiteralPath $StatusPath -Value $exitCode + } + exit $exitCode +} + +if ($Action -eq "Signal") { + New-Item -ItemType File -Path $StopPath -ErrorAction Stop | Out-Null + exit 0 +} + if ($Action -eq "Cleanup") { for ($attempt = 0; $attempt -lt 20 -and (Test-Path -LiteralPath $StagingPath); $attempt++) { try { @@ -37,7 +61,7 @@ using System; using System.ComponentModel; using System.Runtime.InteropServices; -public static class AgentPolicyUnelevatedTesterNativeDirectory +public static class AgentPolicyStandardUserTesterDirectory { [StructLayout(LayoutKind.Sequential)] private struct SecurityAttributes @@ -77,7 +101,7 @@ public static class AgentPolicyUnelevatedTesterNativeDirectory $directorySecurity.SetSecurityDescriptorSddlForm( 'O:SYG:SYD:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;GRGX;;;BU)' ) - [AgentPolicyUnelevatedTesterNativeDirectory]::Create( + [AgentPolicyStandardUserTesterDirectory]::Create( $StagingPath, $directorySecurity.GetSecurityDescriptorBinaryForm() ) @@ -105,7 +129,14 @@ $agentPath = Join-Path $workspacePath "target/debug/devolutions-agent.exe" $outputPath = Join-Path $PSScriptRoot "agent-policy-tester-unelevated.out" $stagingPath = Join-Path $env:ProgramData "dgw-agent-policy-tester-$([guid]::NewGuid().ToString('N'))" $stagedTesterPath = Join-Path $stagingPath "agent-policy-tester.exe" +$readyPath = Join-Path $stagingPath "standard-user-ready.json" +$stopPath = Join-Path $stagingPath "standard-user-stop" +$statusPath = Join-Path $stagingPath "standard-user-server.status" +$serverOutputPath = Join-Path $stagingPath "standard-user-server.out" $tempPath = Join-Path $env:USERPROFILE "AppData\LocalLow\Temp" +$nonce = [guid]::NewGuid().ToString("N") +$exitCode = 1 +$serverStarted = $false try { Set-Content -LiteralPath $outputPath -Value "" @@ -119,14 +150,64 @@ try { throw "LocalSystem tester staging failed with exit code $stageExitCode" } + $serverOutput = & psexec.exe -accepteula -s -d pwsh.exe -NoProfile -File $PSCommandPath ` + -Action Server -StagedTesterPath $stagedTesterPath -AgentPath $agentPath -ReadyPath $readyPath ` + -StopPath $stopPath -StatusPath $statusPath -ServerOutputPath $serverOutputPath -Nonce $nonce 2>&1 + $serverStartExitCode = $LASTEXITCODE + $serverOutput | Out-File $outputPath -Append + if ($serverStartExitCode -ne 0) { + throw "LocalSystem test server failed to start with exit code $serverStartExitCode" + } + $serverStarted = $true + + $deadline = [DateTime]::UtcNow.AddSeconds(30) + while (-not (Test-Path -LiteralPath $readyPath)) { + if (Test-Path -LiteralPath $statusPath) { + throw "LocalSystem test server exited before publishing readiness" + } + if ([DateTime]::UtcNow -ge $deadline) { + throw "Timed out waiting for LocalSystem test server readiness" + } + Start-Sleep -Milliseconds 100 + } + Get-Content -LiteralPath $readyPath | Out-File $outputPath -Append + $testerOutput = & psexec.exe -accepteula -l pwsh.exe -NoProfile -File $PSCommandPath ` - -Action Run -StagedTesterPath $stagedTesterPath -AgentPath $agentPath -TempPath $tempPath 2>&1 + -Action Run -StagedTesterPath $stagedTesterPath -AgentPath $agentPath -TempPath $tempPath ` + -ReadyPath $readyPath -Nonce $nonce 2>&1 $exitCode = $LASTEXITCODE $testerOutput | Out-File $outputPath -Append } catch { $_ | Out-File $outputPath -Append $exitCode = 1 } finally { + if ($serverStarted) { + $signalOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` + -Action Signal -StopPath $stopPath 2>&1 + $signalExitCode = $LASTEXITCODE + $signalOutput | Out-File $outputPath -Append + if ($signalExitCode -ne 0 -and $exitCode -eq 0) { + $exitCode = $signalExitCode + } + + $deadline = [DateTime]::UtcNow.AddSeconds(30) + while (-not (Test-Path -LiteralPath $statusPath) -and [DateTime]::UtcNow -lt $deadline) { + Start-Sleep -Milliseconds 100 + } + if (Test-Path -LiteralPath $serverOutputPath) { + Get-Content -LiteralPath $serverOutputPath | Out-File $outputPath -Append + } + if (Test-Path -LiteralPath $statusPath) { + $serverExitCode = [int](Get-Content -LiteralPath $statusPath -Raw) + if ($serverExitCode -ne 0 -and $exitCode -eq 0) { + $exitCode = $serverExitCode + } + } elseif ($exitCode -eq 0) { + "Timed out waiting for LocalSystem test server shutdown" | Out-File $outputPath -Append + $exitCode = 1 + } + } + $cleanupOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` -Action Cleanup -StagingPath $stagingPath 2>&1 $cleanupExitCode = $LASTEXITCODE diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index 855f555fc..933904842 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -1,3 +1,5 @@ +use std::fs::OpenOptions; +use std::io::Write as _; use std::path::{Path, PathBuf}; use std::process::Stdio; use std::time::{Duration, Instant}; @@ -9,6 +11,7 @@ use tokio::net::windows::named_pipe::ClientOptions; use win_api_wrappers::identity::sid::Sid; use win_api_wrappers::process::Process; use windows::Win32::Security::{TOKEN_DUPLICATE, TOKEN_QUERY, WinBuiltinAdministratorsSid, WinLocalSystemSid}; +use windows::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; const FULL_POLICY: &str = include_str!("../../now-package-broker/src/assets/samples/corporate-allowlist.policy.json"); const MANAGED_POLICY_RELATIVE_PATH: &str = r"Devolutions\PackageBroker\package-broker-policy.json"; @@ -180,16 +183,18 @@ impl HttpResponse { #[derive(Clone, Copy, PartialEq, Eq)] enum Mode { - Unelevated, + StandardServer, + StandardClient, Elevated, } impl Mode { fn parse(value: &str) -> anyhow::Result { match value { - "unelevated" => Ok(Self::Unelevated), + "standard-server" => Ok(Self::StandardServer), + "standard-client" => Ok(Self::StandardClient), "elevated" => Ok(Self::Elevated), - _ => bail!("unknown mode '{value}'; expected 'unelevated' or 'elevated'"), + _ => bail!("unknown mode '{value}'; expected 'standard-server', 'standard-client', or 'elevated'"), } } } @@ -199,23 +204,33 @@ pub(crate) async fn run() -> anyhow::Result<()> { let agent_path = args .next() .map(PathBuf::from) - .context("usage: agent-policy-tester ")?; + .context("usage: agent-policy-tester [mode arguments]")?; let mode = args .next() .and_then(|value| value.into_string().ok()) - .context("test mode must be 'unelevated' or 'elevated'") + .context("test mode is required") .and_then(|value| Mode::parse(&value))?; - ensure!(args.next().is_none(), "unexpected extra command-line arguments"); - verify_process_token(mode)?; - ensure!( - agent_path.is_file(), - "agent executable does not exist: {}", - agent_path.display() - ); - match mode { - Mode::Unelevated => standard_user_management(&agent_path).await?, + Mode::StandardServer => { + verify_local_system()?; + ensure_agent_path(&agent_path)?; + let ready_path = next_path(&mut args, "ready path")?; + let stop_path = next_path(&mut args, "stop path")?; + let nonce = next_string(&mut args, "coordination nonce")?; + ensure!(args.next().is_none(), "unexpected standard-server arguments"); + standard_user_server(&agent_path, &ready_path, &stop_path, &nonce).await?; + } + Mode::StandardClient => { + let client = verify_standard_user()?; + let ready_path = next_path(&mut args, "ready path")?; + let nonce = next_string(&mut args, "coordination nonce")?; + ensure!(args.next().is_none(), "unexpected standard-client arguments"); + standard_user_management(&ready_path, &nonce, &client).await?; + } Mode::Elevated => { + verify_local_system()?; + ensure_agent_path(&agent_path)?; + ensure!(args.next().is_none(), "unexpected elevated arguments"); unavailable_policy_and_method_restrictions(&agent_path).await?; complete_snapshots_across_reload(&agent_path).await?; redirected_policy_paths_fail_closed(&agent_path).await?; @@ -227,7 +242,21 @@ pub(crate) async fn run() -> anyhow::Result<()> { Ok(()) } -fn verify_process_token(mode: Mode) -> anyhow::Result<()> { +fn ensure_agent_path(agent_path: &Path) -> anyhow::Result<()> { + ensure!( + agent_path.is_file(), + "agent executable does not exist: {}", + agent_path.display() + ); + Ok(()) +} + +struct ProcessIdentity { + pid: u32, + sid: Sid, +} + +fn current_process_identity() -> anyhow::Result<(ProcessIdentity, bool, bool)> { let token = Process::current_process() .token(TOKEN_QUERY | TOKEN_DUPLICATE) .context("open tester process token")?; @@ -236,26 +265,59 @@ fn verify_process_token(mode: Mode) -> anyhow::Result<()> { let is_administrator = token .is_member(&administrators) .context("query tester Administrators membership")?; + let identity = ProcessIdentity { + pid: std::process::id(), + sid: token.sid_and_attributes().context("query tester user SID")?.sid, + }; + Ok(( + identity, + is_administrator, + token.is_elevated().context("query tester token elevation")?, + )) +} + +fn verify_standard_user() -> anyhow::Result { + let (identity, is_administrator, _) = current_process_identity()?; let system = Sid::from_well_known(WinLocalSystemSid, None).context("construct LocalSystem SID")?; - let user = token.sid_and_attributes().context("query tester user SID")?.sid; - match mode { - Mode::Unelevated => { - ensure!(user != system, "unelevated mode requires a standard user account"); - ensure!( - !is_administrator, - "unelevated mode requires disabled Administrators membership" - ); - } - Mode::Elevated => { - ensure!(is_administrator, "elevated mode requires Administrators membership"); - ensure!( - token.is_elevated().context("query tester token elevation")?, - "elevated mode requires an elevated token" - ); - ensure!(user == system, "elevated mode requires the LocalSystem account"); - } - } - Ok(()) + ensure!( + identity.sid != system, + "standard-client mode requires a non-SYSTEM account" + ); + ensure!( + !is_administrator, + "standard-client mode requires disabled Administrators membership" + ); + Ok(identity) +} + +fn verify_local_system() -> anyhow::Result { + let (identity, is_administrator, is_elevated) = current_process_identity()?; + let system = Sid::from_well_known(WinLocalSystemSid, None).context("construct LocalSystem SID")?; + ensure!(identity.sid == system, "this mode requires the LocalSystem account"); + ensure!(is_administrator && is_elevated, "LocalSystem token is not elevated"); + Ok(identity) +} + +fn process_sid(pid: u32) -> anyhow::Result { + Process::get_by_pid(pid, PROCESS_QUERY_LIMITED_INFORMATION) + .with_context(|| format!("open process {pid}"))? + .token(TOKEN_QUERY) + .with_context(|| format!("open process {pid} token"))? + .sid_and_attributes() + .with_context(|| format!("query process {pid} SID")) + .map(|identity| identity.sid) +} + +fn next_path(args: &mut impl Iterator, name: &str) -> anyhow::Result { + args.next() + .map(PathBuf::from) + .with_context(|| format!("missing {name}")) +} + +fn next_string(args: &mut impl Iterator, name: &str) -> anyhow::Result { + args.next() + .and_then(|value| value.into_string().ok()) + .with_context(|| format!("missing or non-Unicode {name}")) } fn unique_pipe_name() -> String { @@ -490,7 +552,11 @@ async fn redirected_policy_paths_fail_closed(agent_path: &Path) -> anyhow::Resul } async fn policy_management(agent: &AgentHarness) -> anyhow::Result { - let response = request(&agent.pipe_name, "GET", "/v1/policy/management").await?; + policy_management_by_pipe(&agent.pipe_name).await +} + +async fn policy_management_by_pipe(pipe_name: &str) -> anyhow::Result { + let response = request(pipe_name, "GET", "/v1/policy/management").await?; ensure!( response.status == 200, "GET /v1/policy/management returned HTTP {}", @@ -499,14 +565,14 @@ async fn policy_management(agent: &AgentHarness) -> anyhow::Result { Ok(response.json()?["Management"].clone()) } -async fn validate_policy(agent: &AgentHarness, draft: &Value) -> anyhow::Result { +async fn validate_policy_by_pipe(pipe_name: &str, draft: &Value) -> anyhow::Result { let validation_request = json!({ "RequestKind": "PolicyValidationRequest", "RequestVersion": "1.0", "Draft": draft }); let validation_response = request_with_body( - &agent.pipe_name, + pipe_name, "POST", "/v1/policy/validate", Some("application/json"), @@ -530,7 +596,24 @@ async fn replace_policy_response( expected_store_token: Value, draft: Value, ) -> anyhow::Result { - let validation = validate_policy(agent, &draft).await?; + replace_policy_response_by_pipe( + &agent.pipe_name, + operation, + conflict_handling, + expected_store_token, + draft, + ) + .await +} + +async fn replace_policy_response_by_pipe( + pipe_name: &str, + operation: &str, + conflict_handling: &str, + expected_store_token: Value, + draft: Value, +) -> anyhow::Result { + let validation = validate_policy_by_pipe(pipe_name, &draft).await?; let replacement_request = json!({ "RequestKind": "PolicyReplacementRequest", "RequestVersion": "1.0", @@ -542,7 +625,7 @@ async fn replace_policy_response( "ValidationReceipt": validation["ValidationReceipt"] }); let response = request_with_body( - &agent.pipe_name, + pipe_name, "PUT", "/v1/policy", Some("application/json"), @@ -615,13 +698,94 @@ async fn management_write_tokens_survive_watcher_reload(agent_path: &Path) -> an Ok(()) } -async fn standard_user_management(agent_path: &Path) -> anyhow::Result<()> { - let agent = AgentHarness::start_unelevated(agent_path).await?; - let management = policy_management(&agent).await?; +async fn standard_user_server( + agent_path: &Path, + ready_path: &Path, + stop_path: &Path, + nonce: &str, +) -> anyhow::Result<()> { + ensure!(!ready_path.exists(), "standard-user readiness path already exists"); + ensure!(!stop_path.exists(), "standard-user stop path already exists"); + + let mut agent = AgentHarness::start_unelevated(agent_path).await?; + let server = verify_local_system()?; + let agent_pid = agent.child.id().context("Agent process has no PID")?; + let child_sid = process_sid(agent_pid)?; + ensure!( + child_sid == server.sid, + "Agent and test server must both run as LocalSystem" + ); + + let readiness = serde_json::to_vec(&json!({ + "Nonce": nonce, + "PipeName": agent.pipe_name, + "ServerPid": server.pid, + "ServerSid": server.sid.to_string(), + "AgentPid": agent_pid, + "AgentSid": child_sid.to_string(), + }))?; + let mut ready_file = OpenOptions::new() + .write(true) + .create_new(true) + .open(ready_path) + .context("create standard-user readiness file")?; + ready_file + .write_all(&readiness) + .context("write standard-user readiness file")?; + ready_file.sync_all().context("flush standard-user readiness file")?; + + let deadline = Instant::now() + Duration::from_secs(90); + while !stop_path.exists() { + if let Some(status) = agent.child.try_wait().context("query Agent status")? { + bail!("Agent exited during standard-user test with {status}"); + } + ensure!( + Instant::now() < deadline, + "timed out waiting for standard-user client completion" + ); + tokio::time::sleep(Duration::from_millis(50)).await; + } + + wait_for_log(&agent, "Policy management write denied").await +} + +async fn standard_user_management(ready_path: &Path, nonce: &str, client: &ProcessIdentity) -> anyhow::Result<()> { + let readiness: Value = + serde_json::from_slice(&std::fs::read(ready_path).context("read standard-user readiness file")?) + .context("parse standard-user readiness file")?; + ensure!(readiness["Nonce"] == nonce, "standard-user readiness nonce mismatch"); + let pipe_name = readiness["PipeName"] + .as_str() + .context("readiness file has no pipe name")?; + let server_pid = readiness["ServerPid"] + .as_u64() + .and_then(|pid| u32::try_from(pid).ok()) + .context("readiness file has no valid server PID")?; + let agent_pid = readiness["AgentPid"] + .as_u64() + .and_then(|pid| u32::try_from(pid).ok()) + .context("readiness file has no valid Agent PID")?; + let system = Sid::from_well_known(WinLocalSystemSid, None) + .context("construct LocalSystem SID")? + .to_string(); + ensure!( + readiness["ServerSid"] == system && readiness["AgentSid"] == system, + "test server and Agent identities were not recorded as LocalSystem" + ); + ensure!( + client.pid != server_pid && client.pid != agent_pid && server_pid != agent_pid, + "standard-user client, test server, and Agent must be distinct processes" + ); + ensure!( + client.sid.to_string() != system, + "standard-user client unexpectedly uses the server identity" + ); + + let management = policy_management_by_pipe(pipe_name).await?; ensure!(management["State"] == "Missing", "expected a missing policy"); let valid_draft = policy_draft("tests.standard-user", "Test"); - let validation = validate_policy(&agent, &valid_draft).await?; + let validation = validate_policy_by_pipe(pipe_name, &valid_draft).await?; ensure!( validation["CanonicalDraft"].is_object() && validation["ValidationReceipt"].is_string(), "valid draft did not produce a canonical draft and receipt" @@ -635,7 +799,7 @@ async fn standard_user_management(agent_path: &Path) -> anyhow::Result<()> { "Draft": invalid_draft }); let invalid_response = request_with_body( - &agent.pipe_name, + pipe_name, "POST", "/v1/policy/validate", Some("application/json"), @@ -654,8 +818,8 @@ async fn standard_user_management(agent_path: &Path) -> anyhow::Result<()> { "invalid draft returned a canonical draft" ); - let denied = replace_policy_response( - &agent, + let denied = replace_policy_response_by_pipe( + pipe_name, "Create", "Reject", management["StoreToken"].clone(), @@ -671,7 +835,7 @@ async fn standard_user_management(agent_path: &Path) -> anyhow::Result<()> { denied.json()?["Code"] == "AdministratorRequired", "standard-user Create did not require an administrator" ); - wait_for_log(&agent, "Policy management write denied").await + Ok(()) } async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { From 6ff4e4bb5e86771323e68472cd54acf147e23a08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 8 Sep 2026 23:25:25 -0400 Subject: [PATCH 04/14] ci(agent): handle detached policy tester launch Treat PsExec's detached-process PID as diagnostic output and use bounded, validated readiness as the authoritative launch result. Publish readiness atomically, preserve launch diagnostics, and keep shutdown and cleanup idempotent so orchestration errors remain actionable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/run-unelevated.ps1 | 171 ++++++++++++++---- crates/agent-policy-tester/src/windows.rs | 13 +- 2 files changed, 149 insertions(+), 35 deletions(-) diff --git a/crates/agent-policy-tester/run-unelevated.ps1 b/crates/agent-policy-tester/run-unelevated.ps1 index 6508507d3..2ea138f8f 100644 --- a/crates/agent-policy-tester/run-unelevated.ps1 +++ b/crates/agent-policy-tester/run-unelevated.ps1 @@ -1,5 +1,5 @@ param( - [ValidateSet("Orchestrate", "Stage", "Server", "Run", "Signal", "Cleanup")] + [ValidateSet("Orchestrate", "Stage", "Server", "Run", "Signal", "Cleanup", "SelfTest")] [string] $Action = "Orchestrate", [string] $TesterPath, [string] $StagedTesterPath, @@ -15,6 +15,131 @@ param( $ErrorActionPreference = "Stop" +function Test-ExplicitPsExecLaunchFailure { + param([string] $Diagnostics) + + return $Diagnostics -match '(?im)^(Couldn''t install PSEXESVC service:|Error establishing communication with PsExec service|Access is denied\.)' +} + +function Wait-ServerReadiness { + param( + [string] $Path, + [string] $ServerStatusPath, + [string] $ExpectedNonce, + [int] $TimeoutMilliseconds, + [int] $LaunchValue, + [string] $LaunchDiagnostics + ) + + if (Test-ExplicitPsExecLaunchFailure $LaunchDiagnostics) { + throw "LocalSystem test server launch failed (value $LaunchValue): $LaunchDiagnostics" + } + + $deadline = [DateTime]::UtcNow.AddMilliseconds($TimeoutMilliseconds) + while (-not (Test-Path -LiteralPath $Path)) { + if (Test-Path -LiteralPath $ServerStatusPath) { + $status = Get-Content -LiteralPath $ServerStatusPath -Raw + throw "LocalSystem test server exited with status $status before publishing readiness (launch value $LaunchValue): $LaunchDiagnostics" + } + if ([DateTime]::UtcNow -ge $deadline) { + throw "Timed out waiting for LocalSystem test server readiness (launch value $LaunchValue): $LaunchDiagnostics" + } + Start-Sleep -Milliseconds 100 + } + + $readiness = Get-Content -LiteralPath $Path -Raw | ConvertFrom-Json + if ($readiness.Nonce -cne $ExpectedNonce) { + throw "LocalSystem test server readiness nonce mismatch" + } + if ([string]::IsNullOrWhiteSpace($readiness.PipeName)) { + throw "LocalSystem test server readiness has no pipe name" + } + if ($readiness.ServerPid -le 0 -or $readiness.AgentPid -le 0 -or $readiness.ServerPid -eq $readiness.AgentPid) { + throw "LocalSystem test server readiness has invalid process identities" + } + if ($readiness.ServerSid -cne 'S-1-5-18' -or $readiness.AgentSid -cne 'S-1-5-18') { + throw "LocalSystem test server readiness has non-SYSTEM identities" + } + + return $readiness +} + +function Remove-StagingPath { + param([string] $Path) + + for ($attempt = 0; $attempt -lt 20 -and (Test-Path -LiteralPath $Path); $attempt++) { + try { + Remove-Item -LiteralPath $Path -Recurse -Force + } catch { + if ($attempt -eq 19) { + throw "Failed to remove $Path after 20 attempts: $_" + } + Start-Sleep -Milliseconds 250 + } + } + if (Test-Path -LiteralPath $Path) { + throw "Failed to remove $Path" + } +} + +function Invoke-RunnerSelfTests { + $root = Join-Path ([System.IO.Path]::GetTempPath()) "agent-policy-runner-$([guid]::NewGuid().ToString('N'))" + New-Item -ItemType Directory -Path $root | Out-Null + try { + $ready = Join-Path $root "ready.json" + $status = Join-Path $root "status" + Set-Content -LiteralPath $ready -Value ( + @{ + Nonce = "nonce" + PipeName = "\\.\pipe\test" + ServerPid = 100 + ServerSid = "S-1-5-18" + AgentPid = 200 + AgentSid = "S-1-5-18" + } | ConvertTo-Json -Compress + ) + Wait-ServerReadiness -Path $ready -ServerStatusPath $status -ExpectedNonce "nonce" ` + -TimeoutMilliseconds 50 -LaunchValue 6256 -LaunchDiagnostics "started detached process" | Out-Null + + Remove-Item -LiteralPath $ready + try { + Wait-ServerReadiness -Path $ready -ServerStatusPath $status -ExpectedNonce "nonce" ` + -TimeoutMilliseconds 50 -LaunchValue 6256 -LaunchDiagnostics "started detached process" | Out-Null + throw "Missing-readiness simulation unexpectedly succeeded" + } catch { + if ( + $_ -notmatch "Timed out waiting for LocalSystem test server readiness" -or + $_ -notmatch "started detached process" + ) { + throw + } + } + + try { + Wait-ServerReadiness -Path $ready -ServerStatusPath $status -ExpectedNonce "nonce" ` + -TimeoutMilliseconds 5000 -LaunchValue 6 ` + -LaunchDiagnostics "Couldn't install PSEXESVC service: Access is denied." | Out-Null + throw "Explicit-launch-failure simulation unexpectedly succeeded" + } catch { + if ( + $_ -notmatch "LocalSystem test server launch failed" -or + $_ -notmatch "Couldn't install PSEXESVC service" + ) { + throw + } + } + + Remove-StagingPath -Path (Join-Path $root "already-absent") + } finally { + Remove-StagingPath -Path $root + } +} + +if ($Action -eq "SelfTest") { + Invoke-RunnerSelfTests + exit 0 +} + if ($Action -eq "Run") { $env:TEMP = $TempPath $env:TMP = $TempPath @@ -37,22 +162,15 @@ if ($Action -eq "Server") { } if ($Action -eq "Signal") { - New-Item -ItemType File -Path $StopPath -ErrorAction Stop | Out-Null + if (-not (Test-Path -LiteralPath $StopPath)) { + New-Item -ItemType File -Path $StopPath -ErrorAction Stop | Out-Null + } exit 0 } if ($Action -eq "Cleanup") { - for ($attempt = 0; $attempt -lt 20 -and (Test-Path -LiteralPath $StagingPath); $attempt++) { - try { - Remove-Item -LiteralPath $StagingPath -Recurse -Force - } catch { - if ($attempt -eq 19) { - throw "Failed to remove $StagingPath after 20 attempts: $_" - } - Start-Sleep -Milliseconds 250 - } - } - exit $(if (Test-Path -LiteralPath $StagingPath) { 1 } else { 0 }) + Remove-StagingPath -Path $StagingPath + exit 0 } if ($Action -eq "Stage") { @@ -136,9 +254,10 @@ $serverOutputPath = Join-Path $stagingPath "standard-user-server.out" $tempPath = Join-Path $env:USERPROFILE "AppData\LocalLow\Temp" $nonce = [guid]::NewGuid().ToString("N") $exitCode = 1 -$serverStarted = $false +$coordinationReady = $false try { + Invoke-RunnerSelfTests Set-Content -LiteralPath $outputPath -Value "" New-Item -ItemType Directory -Path $tempPath -Force | Out-Null @@ -155,22 +274,12 @@ try { -StopPath $stopPath -StatusPath $statusPath -ServerOutputPath $serverOutputPath -Nonce $nonce 2>&1 $serverStartExitCode = $LASTEXITCODE $serverOutput | Out-File $outputPath -Append - if ($serverStartExitCode -ne 0) { - throw "LocalSystem test server failed to start with exit code $serverStartExitCode" - } - $serverStarted = $true - - $deadline = [DateTime]::UtcNow.AddSeconds(30) - while (-not (Test-Path -LiteralPath $readyPath)) { - if (Test-Path -LiteralPath $statusPath) { - throw "LocalSystem test server exited before publishing readiness" - } - if ([DateTime]::UtcNow -ge $deadline) { - throw "Timed out waiting for LocalSystem test server readiness" - } - Start-Sleep -Milliseconds 100 - } - Get-Content -LiteralPath $readyPath | Out-File $outputPath -Append + "Detached server launch value: $serverStartExitCode" | Out-File $outputPath -Append + $serverLaunchDiagnostics = ($serverOutput | Out-String).Trim() + $readiness = Wait-ServerReadiness -Path $readyPath -ServerStatusPath $statusPath -ExpectedNonce $nonce ` + -TimeoutMilliseconds 30000 -LaunchValue $serverStartExitCode -LaunchDiagnostics $serverLaunchDiagnostics + $coordinationReady = $true + $readiness | ConvertTo-Json -Compress | Out-File $outputPath -Append $testerOutput = & psexec.exe -accepteula -l pwsh.exe -NoProfile -File $PSCommandPath ` -Action Run -StagedTesterPath $stagedTesterPath -AgentPath $agentPath -TempPath $tempPath ` @@ -181,7 +290,7 @@ try { $_ | Out-File $outputPath -Append $exitCode = 1 } finally { - if ($serverStarted) { + if ($coordinationReady) { $signalOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` -Action Signal -StopPath $stopPath 2>&1 $signalExitCode = $LASTEXITCODE diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index 933904842..6d189cf19 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -724,15 +724,20 @@ async fn standard_user_server( "AgentPid": agent_pid, "AgentSid": child_sid.to_string(), }))?; + let ready_temp_path = ready_path.with_extension("tmp"); let mut ready_file = OpenOptions::new() .write(true) .create_new(true) - .open(ready_path) - .context("create standard-user readiness file")?; + .open(&ready_temp_path) + .context("create standard-user readiness temporary file")?; ready_file .write_all(&readiness) - .context("write standard-user readiness file")?; - ready_file.sync_all().context("flush standard-user readiness file")?; + .context("write standard-user readiness temporary file")?; + ready_file + .sync_all() + .context("flush standard-user readiness temporary file")?; + drop(ready_file); + std::fs::rename(&ready_temp_path, ready_path).context("publish standard-user readiness file")?; let deadline = Instant::now() + Duration::from_secs(90); while !stop_path.exists() { From 304f5782a331781d88f10e7626ecddac406b7ce4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 9 Sep 2026 00:46:35 -0400 Subject: [PATCH 05/14] test(agent): use medium-integrity policy client Launch policy authorization requests from a unique temporary standard-user account instead of a Low-integrity PsExec token. Require the exact account SID and Medium mandatory integrity level, keep credentials out of arguments and logs, and remove the account and profile after the bounded run. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/run-unelevated.ps1 | 193 +++++++++++++++++- crates/agent-policy-tester/src/windows.rs | 122 ++++++++++- 2 files changed, 296 insertions(+), 19 deletions(-) diff --git a/crates/agent-policy-tester/run-unelevated.ps1 b/crates/agent-policy-tester/run-unelevated.ps1 index 2ea138f8f..8e7a24c4d 100644 --- a/crates/agent-policy-tester/run-unelevated.ps1 +++ b/crates/agent-policy-tester/run-unelevated.ps1 @@ -10,7 +10,8 @@ param( [string] $StopPath, [string] $StatusPath, [string] $ServerOutputPath, - [string] $Nonce + [string] $Nonce, + [string] $ExpectedClientSid ) $ErrorActionPreference = "Stop" @@ -82,6 +83,163 @@ function Remove-StagingPath { } } +function New-RandomSecurePassword { + $alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz23456789!@#$%^&*" + $bytes = [byte[]]::new(32) + [System.Security.Cryptography.RandomNumberGenerator]::Fill($bytes) + $password = [System.Security.SecureString]::new() + foreach ($byte in $bytes) { + $password.AppendChar($alphabet[$byte % $alphabet.Length]) + } + foreach ($required in "Aa1!".ToCharArray()) { + $password.AppendChar($required) + } + $password.MakeReadOnly() + return $password +} + +function New-StandardUserAccount { + $name = "dgwpol$([guid]::NewGuid().ToString('N').Substring(0, 12))" + $password = New-RandomSecurePassword + try { + $user = New-LocalUser -Name $name -Password $password -AccountNeverExpires ` + -PasswordNeverExpires -UserMayNotChangePassword ` + -Description "Temporary Devolutions Agent policy E2E user" + $usersGroup = Get-LocalGroup -SID "S-1-5-32-545" + $isMember = Get-LocalGroupMember -Group $usersGroup -ErrorAction Stop | + Where-Object { $_.SID.Value -eq $user.SID.Value } + if (-not $isMember) { + Add-LocalGroupMember -Group $usersGroup -Member $user -ErrorAction Stop + } + return [pscustomobject]@{ + Name = $name + Sid = $user.SID.Value + Credential = [System.Management.Automation.PSCredential]::new( + $name, + $password + ) + } + } catch { + Remove-LocalUser -Name $name -ErrorAction SilentlyContinue + $password.Dispose() + throw + } +} + +function Set-StandardUserTempAcl { + param( + [string] $Path, + [string] $UserSid + ) + + New-Item -ItemType Directory -Path $Path -ErrorAction Stop | Out-Null + & icacls.exe $Path /inheritance:r /grant:r ` + '*S-1-5-18:(OI)(CI)(F)' ` + '*S-1-5-32-544:(OI)(CI)(F)' ` + "*$($UserSid):(OI)(CI)(M)" + if ($LASTEXITCODE -ne 0) { + throw "Failed to protect the standard-user temporary directory" + } +} + +function Invoke-StandardUserClient { + param( + [System.Management.Automation.PSCredential] $Credential, + [string] $UserSid, + [string] $ClientTempPath, + [string] $ScriptPath, + [string] $TesterExecutablePath, + [string] $AgentExecutablePath, + [string] $ReadinessPath, + [string] $ExpectedNonce + ) + + $startInfo = [System.Diagnostics.ProcessStartInfo]::new() + $startInfo.FileName = (Get-Command pwsh.exe -CommandType Application).Source + $startInfo.UseShellExecute = $false + $startInfo.CreateNoWindow = $true + $startInfo.RedirectStandardOutput = $true + $startInfo.RedirectStandardError = $true + $startInfo.LoadUserProfile = $false + $startInfo.UserName = $Credential.UserName + $startInfo.Domain = "." + $startInfo.Password = $Credential.Password + $startInfo.WorkingDirectory = $ClientTempPath + $startInfo.Environment["TEMP"] = $ClientTempPath + $startInfo.Environment["TMP"] = $ClientTempPath + foreach ($argument in @( + "-NoProfile", + "-File", + $ScriptPath, + "-Action", + "Run", + "-StagedTesterPath", + $TesterExecutablePath, + "-AgentPath", + $AgentExecutablePath, + "-TempPath", + $ClientTempPath, + "-ReadyPath", + $ReadinessPath, + "-Nonce", + $ExpectedNonce, + "-ExpectedClientSid", + $UserSid + )) { + $startInfo.ArgumentList.Add($argument) + } + + $process = [System.Diagnostics.Process]::new() + $process.StartInfo = $startInfo + try { + if (-not $process.Start()) { + throw "Failed to start the medium-integrity standard-user client" + } + $stdout = $process.StandardOutput.ReadToEndAsync() + $stderr = $process.StandardError.ReadToEndAsync() + if (-not $process.WaitForExit(60000)) { + $process.Kill($true) + $process.WaitForExit() + throw "Timed out waiting for the medium-integrity standard-user client" + } + return [pscustomobject]@{ + ExitCode = $process.ExitCode + StdOut = $stdout.GetAwaiter().GetResult() + StdErr = $stderr.GetAwaiter().GetResult() + } + } finally { + $process.Dispose() + } +} + +function Remove-StandardUserAccount { + param( + [string] $Name, + [string] $Sid + ) + + for ($attempt = 0; $attempt -lt 20; $attempt++) { + try { + $profile = Get-CimInstance -ClassName Win32_UserProfile -Filter "SID='$Sid'" -ErrorAction Stop + if ($profile) { + $profile | Remove-CimInstance -ErrorAction Stop + } + if (Get-LocalUser -Name $Name -ErrorAction SilentlyContinue) { + Remove-LocalUser -Name $Name -ErrorAction Stop + } + if (-not (Get-LocalUser -Name $Name -ErrorAction SilentlyContinue)) { + return + } + } catch { + if ($attempt -eq 19) { + throw + } + } + Start-Sleep -Milliseconds 250 + } + throw "Temporary standard-user account still exists after 20 removal attempts" +} + function Invoke-RunnerSelfTests { $root = Join-Path ([System.IO.Path]::GetTempPath()) "agent-policy-runner-$([guid]::NewGuid().ToString('N'))" New-Item -ItemType Directory -Path $root | Out-Null @@ -143,7 +301,7 @@ if ($Action -eq "SelfTest") { if ($Action -eq "Run") { $env:TEMP = $TempPath $env:TMP = $TempPath - & $StagedTesterPath $AgentPath standard-client $ReadyPath $Nonce + & $StagedTesterPath $AgentPath standard-client $ExpectedClientSid $ReadyPath $Nonce exit $LASTEXITCODE } @@ -251,15 +409,14 @@ $readyPath = Join-Path $stagingPath "standard-user-ready.json" $stopPath = Join-Path $stagingPath "standard-user-stop" $statusPath = Join-Path $stagingPath "standard-user-server.status" $serverOutputPath = Join-Path $stagingPath "standard-user-server.out" -$tempPath = Join-Path $env:USERPROFILE "AppData\LocalLow\Temp" $nonce = [guid]::NewGuid().ToString("N") $exitCode = 1 $coordinationReady = $false +$clientAccount = $null try { Invoke-RunnerSelfTests Set-Content -LiteralPath $outputPath -Value "" - New-Item -ItemType Directory -Path $tempPath -Force | Out-Null $stageOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` -Action Stage -TesterPath $testerPath -StagedTesterPath $stagedTesterPath -StagingPath $stagingPath 2>&1 @@ -269,6 +426,10 @@ try { throw "LocalSystem tester staging failed with exit code $stageExitCode" } + $clientAccount = New-StandardUserAccount + $clientTempPath = Join-Path $stagingPath "standard-user-temp" + Set-StandardUserTempAcl -Path $clientTempPath -UserSid $clientAccount.Sid + $serverOutput = & psexec.exe -accepteula -s -d pwsh.exe -NoProfile -File $PSCommandPath ` -Action Server -StagedTesterPath $stagedTesterPath -AgentPath $agentPath -ReadyPath $readyPath ` -StopPath $stopPath -StatusPath $statusPath -ServerOutputPath $serverOutputPath -Nonce $nonce 2>&1 @@ -281,11 +442,12 @@ try { $coordinationReady = $true $readiness | ConvertTo-Json -Compress | Out-File $outputPath -Append - $testerOutput = & psexec.exe -accepteula -l pwsh.exe -NoProfile -File $PSCommandPath ` - -Action Run -StagedTesterPath $stagedTesterPath -AgentPath $agentPath -TempPath $tempPath ` - -ReadyPath $readyPath -Nonce $nonce 2>&1 - $exitCode = $LASTEXITCODE - $testerOutput | Out-File $outputPath -Append + $client = Invoke-StandardUserClient -Credential $clientAccount.Credential -UserSid $clientAccount.Sid ` + -ClientTempPath $clientTempPath -ScriptPath $PSCommandPath -TesterExecutablePath $stagedTesterPath ` + -AgentExecutablePath $agentPath -ReadinessPath $readyPath -ExpectedNonce $nonce + $client.StdOut | Out-File $outputPath -Append + $client.StdErr | Out-File $outputPath -Append + $exitCode = $client.ExitCode } catch { $_ | Out-File $outputPath -Append $exitCode = 1 @@ -317,6 +479,19 @@ try { } } + if ($clientAccount) { + try { + Remove-StandardUserAccount -Name $clientAccount.Name -Sid $clientAccount.Sid + } catch { + $_ | Out-File $outputPath -Append + if ($exitCode -eq 0) { + $exitCode = 1 + } + } finally { + $clientAccount.Credential.Password.Dispose() + } + } + $cleanupOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` -Action Cleanup -StagingPath $stagingPath 2>&1 $cleanupExitCode = $LASTEXITCODE diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index 6d189cf19..8dcb471ad 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -1,5 +1,6 @@ use std::fs::OpenOptions; use std::io::Write as _; +use std::mem::size_of; use std::path::{Path, PathBuf}; use std::process::Stdio; use std::time::{Duration, Instant}; @@ -10,13 +11,20 @@ use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; use tokio::net::windows::named_pipe::ClientOptions; use win_api_wrappers::identity::sid::Sid; use win_api_wrappers::process::Process; -use windows::Win32::Security::{TOKEN_DUPLICATE, TOKEN_QUERY, WinBuiltinAdministratorsSid, WinLocalSystemSid}; -use windows::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; +use windows::Win32::Foundation::{CloseHandle, HANDLE}; +use windows::Win32::Security::{ + GetSidSubAuthority, GetSidSubAuthorityCount, GetTokenInformation, TOKEN_DUPLICATE, TOKEN_MANDATORY_LABEL, + TOKEN_QUERY, TokenIntegrityLevel, WinBuiltinAdministratorsSid, WinLocalSystemSid, +}; +use windows::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken, PROCESS_QUERY_LIMITED_INFORMATION}; const FULL_POLICY: &str = include_str!("../../now-package-broker/src/assets/samples/corporate-allowlist.policy.json"); const MANAGED_POLICY_RELATIVE_PATH: &str = r"Devolutions\PackageBroker\package-broker-policy.json"; const MANAGED_AUTHORITY_MARKER: &str = r"Devolutions\PackageBroker\.package-broker-managed-authority.v1"; const LEGACY_POLICY_RELATIVE_PATH: &str = r"Devolutions\Agent\package-broker-policy.json"; +#[cfg(test)] +const SECURITY_MANDATORY_LOW_RID: u32 = 0x1000; +const SECURITY_MANDATORY_MEDIUM_RID: u32 = 0x2000; struct AgentHarness { child: tokio::process::Child, @@ -221,7 +229,8 @@ pub(crate) async fn run() -> anyhow::Result<()> { standard_user_server(&agent_path, &ready_path, &stop_path, &nonce).await?; } Mode::StandardClient => { - let client = verify_standard_user()?; + let expected_sid = next_string(&mut args, "expected client SID")?; + let client = verify_standard_user(&expected_sid)?; let ready_path = next_path(&mut args, "ready path")?; let nonce = next_string(&mut args, "coordination nonce")?; ensure!(args.next().is_none(), "unexpected standard-client arguments"); @@ -276,18 +285,85 @@ fn current_process_identity() -> anyhow::Result<(ProcessIdentity, bool, bool)> { )) } -fn verify_standard_user() -> anyhow::Result { +fn verify_standard_user(expected_sid: &str) -> anyhow::Result { let (identity, is_administrator, _) = current_process_identity()?; - let system = Sid::from_well_known(WinLocalSystemSid, None).context("construct LocalSystem SID")?; - ensure!( - identity.sid != system, - "standard-client mode requires a non-SYSTEM account" - ); + validate_standard_user_token( + &identity.sid.to_string(), + expected_sid, + is_administrator, + current_integrity_level()?, + )?; + Ok(identity) +} + +fn validate_standard_user_token( + actual_sid: &str, + expected_sid: &str, + is_administrator: bool, + integrity_level: u32, +) -> anyhow::Result<()> { + ensure!(actual_sid == expected_sid, "standard-client account SID mismatch"); ensure!( !is_administrator, "standard-client mode requires disabled Administrators membership" ); - Ok(identity) + ensure!( + integrity_level == SECURITY_MANDATORY_MEDIUM_RID, + "standard-client mode requires Medium integrity, got RID {integrity_level:#x}" + ); + Ok(()) +} + +fn current_integrity_level() -> anyhow::Result { + let mut token = HANDLE::default(); + // SAFETY: `GetCurrentProcess` has no preconditions and returns a process pseudo-handle. + let process = unsafe { GetCurrentProcess() }; + // SAFETY: The process pseudo-handle is valid and `token` is a writable output parameter. + unsafe { + OpenProcessToken(process, TOKEN_QUERY, &mut token).context("open current process integrity token")?; + } + let result = integrity_level(token); + // SAFETY: `OpenProcessToken` returned this owned token handle. + unsafe { + CloseHandle(token).context("close current process integrity token")?; + } + result +} + +fn integrity_level(token: HANDLE) -> anyhow::Result { + let mut length = 0; + // SAFETY: A null output buffer with length zero is the documented size query. + let _ = unsafe { GetTokenInformation(token, TokenIntegrityLevel, None, 0, &mut length) }; + ensure!( + usize::try_from(length)? >= size_of::(), + "TokenIntegrityLevel returned an undersized buffer" + ); + + let word_count = usize::try_from(length)?.div_ceil(size_of::()); + let mut buffer = vec![0usize; word_count]; + // SAFETY: The aligned buffer is writable for `length` bytes and the token handle is valid. + unsafe { + GetTokenInformation( + token, + TokenIntegrityLevel, + Some(buffer.as_mut_ptr().cast()), + length, + &mut length, + ) + .context("query token integrity level")?; + } + // SAFETY: A successful TokenIntegrityLevel query initialized a TOKEN_MANDATORY_LABEL. + let label = unsafe { &*buffer.as_ptr().cast::() }; + // SAFETY: The returned token label contains a valid SID. + let sub_authority_count_ptr = unsafe { GetSidSubAuthorityCount(label.Label.Sid) }; + // SAFETY: `GetSidSubAuthorityCount` returns a pointer into the valid label SID. + let sub_authority_count = unsafe { *sub_authority_count_ptr }; + ensure!(sub_authority_count > 0, "integrity SID has no sub-authority"); + // SAFETY: The index is within the validated SID sub-authority count. + let rid_ptr = unsafe { GetSidSubAuthority(label.Label.Sid, u32::from(sub_authority_count - 1)) }; + // SAFETY: `GetSidSubAuthority` returns a pointer into the valid label SID. + let rid = unsafe { *rid_ptr }; + Ok(rid) } fn verify_local_system() -> anyhow::Result { @@ -1173,6 +1249,7 @@ async fn complete_snapshots_across_reload(agent_path: &Path) -> anyhow::Result<( if policy == &full { break; } + ensure!(Instant::now() < deadline, "agent did not reload the policy"); tokio::task::yield_now().await; } @@ -1188,3 +1265,28 @@ async fn complete_snapshots_across_reload(agent_path: &Path) -> anyhow::Result<( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn standard_user_token_requires_medium_integrity() { + validate_standard_user_token( + "S-1-5-21-1-2-3-1001", + "S-1-5-21-1-2-3-1001", + false, + SECURITY_MANDATORY_MEDIUM_RID, + ) + .expect("matching standard-user SID at Medium integrity is valid"); + + let error = validate_standard_user_token( + "S-1-5-21-1-2-3-1001", + "S-1-5-21-1-2-3-1001", + false, + SECURITY_MANDATORY_LOW_RID, + ) + .expect_err("Low integrity must not satisfy the standard-user scenario"); + assert!(error.to_string().contains("requires Medium integrity")); + } +} From 421829fec6ab98fe600961eae71cb04a84a4964a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 9 Sep 2026 01:16:00 -0400 Subject: [PATCH 06/14] ci(agent): stop policy server after launch failures Own LocalSystem test-server shutdown as soon as detached launch is attempted, even when readiness validation fails. Signal the unique protected stop marker with a direct fallback, preserve the original scenario error, and bound status collection before cleanup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/run-unelevated.ps1 | 103 ++++++++++++++---- 1 file changed, 84 insertions(+), 19 deletions(-) diff --git a/crates/agent-policy-tester/run-unelevated.ps1 b/crates/agent-policy-tester/run-unelevated.ps1 index 8e7a24c4d..e0afd9318 100644 --- a/crates/agent-policy-tester/run-unelevated.ps1 +++ b/crates/agent-policy-tester/run-unelevated.ps1 @@ -287,6 +287,58 @@ function Invoke-RunnerSelfTests { } } + $launchAttempted = $true + Set-Content -LiteralPath $ready -Value ( + @{ + Nonce = "wrong-nonce" + PipeName = "\\.\pipe\test" + ServerPid = 100 + ServerSid = "S-1-5-18" + AgentPid = 200 + AgentSid = "S-1-5-18" + } | ConvertTo-Json -Compress + ) + try { + Wait-ServerReadiness -Path $ready -ServerStatusPath $status -ExpectedNonce "nonce" ` + -TimeoutMilliseconds 50 -LaunchValue 6256 -LaunchDiagnostics "started detached process" | Out-Null + throw "Mismatched-readiness simulation unexpectedly succeeded" + } catch { + if ($_ -notmatch "readiness nonce mismatch" -or -not $launchAttempted) { + throw + } + } + + Set-Content -LiteralPath $ready -Value ( + @{ + Nonce = "nonce" + PipeName = "\\.\pipe\test" + ServerPid = 100 + ServerSid = "S-1-5-18" + AgentPid = 100 + AgentSid = "S-1-5-18" + } | ConvertTo-Json -Compress + ) + try { + Wait-ServerReadiness -Path $ready -ServerStatusPath $status -ExpectedNonce "nonce" ` + -TimeoutMilliseconds 50 -LaunchValue 6256 -LaunchDiagnostics "started detached process" | Out-Null + throw "Invalid-PID readiness simulation unexpectedly succeeded" + } catch { + if ($_ -notmatch "readiness has invalid process identities" -or -not $launchAttempted) { + throw + } + } + + Remove-Item -LiteralPath $ready + try { + Wait-ServerReadiness -Path $ready -ServerStatusPath $status -ExpectedNonce "nonce" ` + -TimeoutMilliseconds 50 -LaunchValue 6256 -LaunchDiagnostics "started detached process" | Out-Null + throw "Attempted-launch missing-readiness simulation unexpectedly succeeded" + } catch { + if ($_ -notmatch "Timed out waiting for LocalSystem test server readiness" -or -not $launchAttempted) { + throw + } + } + Remove-StagingPath -Path (Join-Path $root "already-absent") } finally { Remove-StagingPath -Path $root @@ -411,7 +463,8 @@ $statusPath = Join-Path $stagingPath "standard-user-server.status" $serverOutputPath = Join-Path $stagingPath "standard-user-server.out" $nonce = [guid]::NewGuid().ToString("N") $exitCode = 1 -$coordinationReady = $false +$serverLaunchAttempted = $false +$serverLaunchExplicitlyFailed = $false $clientAccount = $null try { @@ -430,6 +483,7 @@ try { $clientTempPath = Join-Path $stagingPath "standard-user-temp" Set-StandardUserTempAcl -Path $clientTempPath -UserSid $clientAccount.Sid + $serverLaunchAttempted = $true $serverOutput = & psexec.exe -accepteula -s -d pwsh.exe -NoProfile -File $PSCommandPath ` -Action Server -StagedTesterPath $stagedTesterPath -AgentPath $agentPath -ReadyPath $readyPath ` -StopPath $stopPath -StatusPath $statusPath -ServerOutputPath $serverOutputPath -Nonce $nonce 2>&1 @@ -437,9 +491,9 @@ try { $serverOutput | Out-File $outputPath -Append "Detached server launch value: $serverStartExitCode" | Out-File $outputPath -Append $serverLaunchDiagnostics = ($serverOutput | Out-String).Trim() + $serverLaunchExplicitlyFailed = Test-ExplicitPsExecLaunchFailure $serverLaunchDiagnostics $readiness = Wait-ServerReadiness -Path $readyPath -ServerStatusPath $statusPath -ExpectedNonce $nonce ` -TimeoutMilliseconds 30000 -LaunchValue $serverStartExitCode -LaunchDiagnostics $serverLaunchDiagnostics - $coordinationReady = $true $readiness | ConvertTo-Json -Compress | Out-File $outputPath -Append $client = Invoke-StandardUserClient -Credential $clientAccount.Credential -UserSid $clientAccount.Sid ` @@ -452,30 +506,41 @@ try { $_ | Out-File $outputPath -Append $exitCode = 1 } finally { - if ($coordinationReady) { + if ($serverLaunchAttempted) { $signalOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` -Action Signal -StopPath $stopPath 2>&1 $signalExitCode = $LASTEXITCODE $signalOutput | Out-File $outputPath -Append - if ($signalExitCode -ne 0 -and $exitCode -eq 0) { - $exitCode = $signalExitCode - } - - $deadline = [DateTime]::UtcNow.AddSeconds(30) - while (-not (Test-Path -LiteralPath $statusPath) -and [DateTime]::UtcNow -lt $deadline) { - Start-Sleep -Milliseconds 100 + if ($signalExitCode -ne 0 -and -not (Test-Path -LiteralPath $stopPath)) { + try { + New-Item -ItemType File -Path $stopPath -ErrorAction Stop | Out-Null + "Created the stop marker directly after SYSTEM signaling failed" | Out-File $outputPath -Append + } catch { + $_ | Out-File $outputPath -Append + } } - if (Test-Path -LiteralPath $serverOutputPath) { - Get-Content -LiteralPath $serverOutputPath | Out-File $outputPath -Append + if (-not (Test-Path -LiteralPath $stopPath) -and $exitCode -eq 0) { + "Failed to create the LocalSystem test server stop marker" | Out-File $outputPath -Append + $exitCode = 1 } - if (Test-Path -LiteralPath $statusPath) { - $serverExitCode = [int](Get-Content -LiteralPath $statusPath -Raw) - if ($serverExitCode -ne 0 -and $exitCode -eq 0) { - $exitCode = $serverExitCode + + if (-not $serverLaunchExplicitlyFailed) { + $deadline = [DateTime]::UtcNow.AddSeconds(30) + while (-not (Test-Path -LiteralPath $statusPath) -and [DateTime]::UtcNow -lt $deadline) { + Start-Sleep -Milliseconds 100 + } + if (Test-Path -LiteralPath $serverOutputPath) { + Get-Content -LiteralPath $serverOutputPath | Out-File $outputPath -Append + } + if (Test-Path -LiteralPath $statusPath) { + $serverExitCode = [int](Get-Content -LiteralPath $statusPath -Raw) + if ($serverExitCode -ne 0 -and $exitCode -eq 0) { + $exitCode = $serverExitCode + } + } elseif ($exitCode -eq 0) { + "Timed out waiting for LocalSystem test server shutdown" | Out-File $outputPath -Append + $exitCode = 1 } - } elseif ($exitCode -eq 0) { - "Timed out waiting for LocalSystem test server shutdown" | Out-File $outputPath -Append - $exitCode = 1 } } From 6e4d41fda482cb7813f5d0eaed529f2359492d99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 16 Sep 2026 02:32:22 +0900 Subject: [PATCH 07/14] test(agent): exercise revised policy lifecycle Cover the current policy contract, receipt invalidation, warnings, and interrupted Repair recovery. Exercise actual installer conversion and managed authority under the hosted LocalSystem gate without accepting skipped privileged tests or incomplete server completion status. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 27 +- crates/agent-policy-tester/run-as-system.ps1 | 41 +- .../run-installer-tests.ps1 | 74 +++ crates/agent-policy-tester/run-unelevated.ps1 | 59 ++- crates/agent-policy-tester/src/windows.rs | 288 ++++++++++-- .../InstalledAgentMigrationE2eTests.cs | 438 ++++++++++++++++++ 6 files changed, 885 insertions(+), 42 deletions(-) create mode 100644 crates/agent-policy-tester/run-installer-tests.ps1 create mode 100644 package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 39a4dc6ff..2a6b4e920 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1346,12 +1346,14 @@ jobs: name: Agent policy end-to-end test runs-on: windows-2022 needs: [preflight] + env: + AGENT_POLICY_TEST_SHA: ${{ inputs.ref || github.event.pull_request.head.sha || needs.preflight.outputs.ref }} steps: - name: Checkout ${{ github.repository }} uses: actions/checkout@v6 with: - ref: ${{ needs.preflight.outputs.ref }} + ref: ${{ env.AGENT_POLICY_TEST_SHA }} - name: Setup Rust cache uses: ./.github/actions/setup-rust-cache @@ -1374,8 +1376,13 @@ jobs: Add-Content -Path $env:GITHUB_PATH -Value $toolsDir - name: Build Agent policy test executables + id: build-policy-executables shell: pwsh run: | + $actualCommit = git rev-parse HEAD + if ($LASTEXITCODE -ne 0 -or $actualCommit -ne $env:AGENT_POLICY_TEST_SHA) { + throw "Agent policy tests must build the requested commit $env:AGENT_POLICY_TEST_SHA, got $actualCommit" + } cargo build --locked -p devolutions-agent --features dev-skip-broker-signature if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE @@ -1385,6 +1392,11 @@ jobs: exit $LASTEXITCODE } + - name: Build installer tests before entering LocalSystem + id: build-installer-tests + shell: pwsh + run: dotnet build package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj --configuration Debug --framework net48 + - name: Run Agent policy tester as standard user shell: pwsh run: | @@ -1396,16 +1408,27 @@ jobs: } - name: Run Agent policy tester as LocalSystem + if: ${{ !cancelled() && steps.build-policy-executables.outcome == 'success' && steps.build-installer-tests.outcome == 'success' }} shell: pwsh run: | $scriptPath = Resolve-Path -Path "./crates/agent-policy-tester/run-as-system.ps1" - psexec -accepteula -s pwsh.exe -NoProfile -File $scriptPath + $dotnetPath = (Get-Command dotnet.exe).Source + psexec -accepteula -s pwsh.exe -NoProfile -File $scriptPath -DotnetPath $dotnetPath $exitCode = $LASTEXITCODE Get-Content -Path ./crates/agent-policy-tester/agent-policy-tester.out if ($exitCode -ne 0) { exit $exitCode } + - name: Upload SYSTEM installer test results + if: ${{ always() }} + uses: actions/upload-artifact@v7 + with: + name: agent-policy-installer-system-results + path: | + crates/agent-policy-tester/installer-test-results/*.trx + crates/agent-policy-tester/agent-policy-tester.out + - name: Run policy route authorization tests shell: pwsh run: cargo test --locked -p now-package-broker --features dev-skip-broker-signature diff --git a/crates/agent-policy-tester/run-as-system.ps1 b/crates/agent-policy-tester/run-as-system.ps1 index 14e9028e5..2e23d612f 100644 --- a/crates/agent-policy-tester/run-as-system.ps1 +++ b/crates/agent-policy-tester/run-as-system.ps1 @@ -1,14 +1,32 @@ +param( + [string] $DotnetPath = (Join-Path $env:ProgramFiles "dotnet\dotnet.exe") +) + $ErrorActionPreference = "Stop" $workspacePath = (Resolve-Path (Join-Path $PSScriptRoot "../..")).Path $testerPath = Join-Path $workspacePath "target/debug/agent-policy-tester.exe" $agentPath = Join-Path $workspacePath "target/debug/devolutions-agent.exe" $outputPath = Join-Path $PSScriptRoot "agent-policy-tester.out" -$stagingPath = Join-Path $env:ProgramData "dgw-agent-policy-tester-$([guid]::NewGuid().ToString('N'))" +$stagingPath = Join-Path ([Environment]::GetFolderPath('CommonApplicationData')) "dgw-agent-policy-tester-$([guid]::NewGuid().ToString('N'))" $stagedTesterPath = Join-Path $stagingPath "agent-policy-tester.exe" +$stagedAgentPath = Join-Path $stagingPath "devolutions-agent.exe" +$installerProject = Join-Path $workspacePath "package\AgentWindowsManaged.Tests\DevolutionsAgent.Installer.Tests.csproj" +$installerOutput = Join-Path $workspacePath "package\AgentWindowsManaged.Tests\bin\Debug\net48" +$stagedInstallerOutput = Join-Path $stagingPath "installer-tests" +$resultsPath = Join-Path $PSScriptRoot "installer-test-results" +$exitCode = 1 +$previousTemp = $env:TEMP +$previousTmp = $env:TMP try { Set-Content -LiteralPath $outputPath -Value "" + if (-not [System.Security.Principal.WindowsIdentity]::GetCurrent().IsSystem) { + throw "This runner requires LocalSystem" + } + if ([System.IO.DriveInfo]::new([System.IO.Path]::GetPathRoot($workspacePath)).DriveType -ne 'Fixed') { + throw "Use a local fixed-volume workspace path visible to LocalSystem, not a mapped drive" + } Add-Type -TypeDefinition @' using System; using System.ComponentModel; @@ -61,11 +79,16 @@ public static class AgentPolicyTesterNativeDirectory if (Get-ChildItem -LiteralPath $stagingPath -Force) { throw "The atomically protected staged tester directory was not empty" } + $env:TEMP = Join-Path $stagingPath "scratch" + $env:TMP = $env:TEMP + New-Item -ItemType Directory -Path $env:TEMP | Out-Null Copy-Item -LiteralPath $testerPath -Destination $stagedTesterPath - & icacls.exe $stagedTesterPath /setowner '*S-1-5-18' 2>&1 | Out-File $outputPath -Append + Copy-Item -LiteralPath $agentPath -Destination $stagedAgentPath + Copy-Item -LiteralPath $installerOutput -Destination $stagedInstallerOutput -Recurse + & icacls.exe $stagingPath /setowner '*S-1-5-18' /T /Q 2>&1 | Out-File $outputPath -Append if ($LASTEXITCODE -ne 0) { - throw "Failed to set the staged tester owner" + throw "Failed to set the staged executable and installer test owners" } & icacls.exe $stagedTesterPath /inheritance:r /grant:r '*S-1-5-18:(F)' '*S-1-5-32-544:(F)' 2>&1 | Out-File $outputPath -Append @@ -76,18 +99,28 @@ public static class AgentPolicyTesterNativeDirectory "Staged policy tester at $stagedTesterPath" | Out-File $outputPath -Append Get-Acl -LiteralPath $stagingPath | Format-List Owner, Sddl | Out-File $outputPath -Append Get-Acl -LiteralPath $stagedTesterPath | Format-List Owner, Sddl | Out-File $outputPath -Append - & $stagedTesterPath $agentPath elevated 2>&1 | Out-File $outputPath -Append + & $stagedTesterPath $stagedAgentPath elevated 2>&1 | Out-File $outputPath -Append $exitCode = $LASTEXITCODE + & (Join-Path $PSScriptRoot "run-installer-tests.ps1") ` + -ProjectPath $installerProject -TestOutputPath $stagedInstallerOutput ` + -AgentPath $stagedAgentPath -ResultsPath $resultsPath -DotnetPath $DotnetPath ` + 2>&1 | Out-File $outputPath -Append + if ($LASTEXITCODE -ne 0) { + $exitCode = $LASTEXITCODE + } } catch { $_ | Out-File $outputPath -Append $exitCode = 1 } finally { + $env:TEMP = $previousTemp + $env:TMP = $previousTmp for ($attempt = 0; $attempt -lt 20 -and (Test-Path -LiteralPath $stagingPath); $attempt++) { try { Remove-Item -LiteralPath $stagingPath -Recurse -Force } catch { if ($attempt -eq 19) { "Failed to remove $stagingPath after 20 attempts: $_" | Out-File $outputPath -Append + $exitCode = 1 } else { Start-Sleep -Milliseconds 250 } diff --git a/crates/agent-policy-tester/run-installer-tests.ps1 b/crates/agent-policy-tester/run-installer-tests.ps1 new file mode 100644 index 000000000..e2c67c402 --- /dev/null +++ b/crates/agent-policy-tester/run-installer-tests.ps1 @@ -0,0 +1,74 @@ +param( + [Parameter(Mandatory)] [string] $ProjectPath, + [Parameter(Mandatory)] [string] $TestOutputPath, + [Parameter(Mandatory)] [string] $AgentPath, + [Parameter(Mandatory)] [string] $ResultsPath, + [Parameter(Mandatory)] [string] $DotnetPath +) + +$ErrorActionPreference = "Stop" +$exitCode = 1 +$previousAgent = $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE + +try { + if (-not [System.Security.Principal.WindowsIdentity]::GetCurrent().IsSystem) { + throw "Installer transaction tests must execute as LocalSystem" + } + $testClass = 'DevolutionsAgent.Installer.Tests.PackageBrokerInstallerTests' + $expectedTests = @( + "$testClass.TransactionTestsRunAsLocalSystem" + "$testClass.InstalledAgentConverterUsesAuthoritativeContractBeforePublication" + "$testClass.ConvertedMigrationCommitAndRepeatPreserveOriginalAndEvidence" + "$testClass.RollbackRestoresLegacyArbitrationAndMigrationCanRepeat" + "$testClass.InterruptedPublicationRecoversOnlyOwnedAuthority" + "$testClass.InvalidLegacyPolicyFailsUpgradeAndPreservesSource" + "$testClass.ExistingNewDestinationAndPublicationCollisionArePreserved" + "$testClass.ChangedSourcePreventsDestructiveRollback" + "$testClass.PreexistingAuthorityPreventsLegacyResurrection" + "$testClass.AuthorityCollisionCannotCommitSourceDeletion" + "$testClass.DestinationCollisionCannotCommitSourceDeletion" + "$testClass.UnchangedInputRollbackPreservesLastSurvivingPolicy" + 'DevolutionsAgent.Installer.Tests.InstalledAgentMigrationE2eTests.ConvertedTransactionActivatesAndRetainsManagedAuthority' + ) + $filter = ($expectedTests | ForEach-Object { "FullyQualifiedName=$_" }) -join '|' + New-Item -ItemType Directory -Path $ResultsPath -Force | Out-Null + $trxPath = Join-Path $ResultsPath "installer-system.trx" + if (Test-Path -LiteralPath $trxPath) { + Remove-Item -LiteralPath $trxPath -Force + } + $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE = (Resolve-Path -LiteralPath $AgentPath).Path + & $DotnetPath test $ProjectPath --no-build --no-restore --configuration Debug --framework net48 ` + "-p:OutputPath=$TestOutputPath\" -p:AppendTargetFrameworkToOutputPath=false ` + --filter $filter --logger "trx;LogFileName=installer-system.trx" --results-directory $ResultsPath + $exitCode = $LASTEXITCODE + if ($exitCode -ne 0) { + throw "SYSTEM installer tests exited with $exitCode" + } + [xml] $trx = Get-Content -LiteralPath $trxPath -Raw + $counters = $trx.TestRun.ResultSummary.Counters + $results = @($trx.TestRun.Results.UnitTestResult) + if ($trx.TestRun.ResultSummary.outcome -ne 'Completed' -or + [int] $counters.total -ne $expectedTests.Count -or + [int] $counters.executed -ne $expectedTests.Count -or + [int] $counters.passed -ne $expectedTests.Count -or + [int] $counters.notExecuted -ne 0 -or + $results.Count -ne $expectedTests.Count) { + throw "Expected exactly $($expectedTests.Count) executed, passed SYSTEM installer tests and zero skips: $($counters.OuterXml)" + } + foreach ($name in $expectedTests) { + $matching = @($results | Where-Object { $_.testName -eq $name }) + if ($matching.Count -ne 1 -or $matching[0].outcome -ne 'Passed') { + throw "Required SYSTEM installer test did not pass exactly once: $name" + } + } + Write-Output "Verified all 12 installer SystemFacts and the installed-Agent migration E2E: 13 passed, zero skipped" +} catch { + Write-Output $_ + if ($exitCode -eq 0) { + $exitCode = 1 + } +} finally { + $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE = $previousAgent +} + +exit $exitCode diff --git a/crates/agent-policy-tester/run-unelevated.ps1 b/crates/agent-policy-tester/run-unelevated.ps1 index e0afd9318..5dc9b2b2b 100644 --- a/crates/agent-policy-tester/run-unelevated.ps1 +++ b/crates/agent-policy-tester/run-unelevated.ps1 @@ -22,6 +22,31 @@ function Test-ExplicitPsExecLaunchFailure { return $Diagnostics -match '(?im)^(Couldn''t install PSEXESVC service:|Error establishing communication with PsExec service|Access is denied\.)' } +function Publish-ServerStatus { + param([string] $Path, [int] $ExitCode) + + $temporaryPath = "$Path.$([guid]::NewGuid().ToString('N')).tmp" + try { + [System.IO.File]::WriteAllText($temporaryPath, $ExitCode.ToString([System.Globalization.CultureInfo]::InvariantCulture)) + [System.IO.File]::Move($temporaryPath, $Path) + } finally { + if (Test-Path -LiteralPath $temporaryPath) { + Remove-Item -LiteralPath $temporaryPath -Force + } + } +} + +function Read-ServerStatus { + param([string] $Path) + + $text = [System.IO.File]::ReadAllText($Path) + $value = 0 + if ($text -notmatch '^-?[0-9]+$' -or -not [int]::TryParse($text, [ref] $value)) { + throw "LocalSystem test server published an invalid completion status" + } + return $value +} + function Wait-ServerReadiness { param( [string] $Path, @@ -39,7 +64,7 @@ function Wait-ServerReadiness { $deadline = [DateTime]::UtcNow.AddMilliseconds($TimeoutMilliseconds) while (-not (Test-Path -LiteralPath $Path)) { if (Test-Path -LiteralPath $ServerStatusPath) { - $status = Get-Content -LiteralPath $ServerStatusPath -Raw + $status = Read-ServerStatus -Path $ServerStatusPath throw "LocalSystem test server exited with status $status before publishing readiness (launch value $LaunchValue): $LaunchDiagnostics" } if ([DateTime]::UtcNow -ge $deadline) { @@ -246,6 +271,25 @@ function Invoke-RunnerSelfTests { try { $ready = Join-Path $root "ready.json" $status = Join-Path $root "status" + foreach ($expected in @(0, 1, -1)) { + Publish-ServerStatus -Path $status -ExitCode $expected + if ((Read-ServerStatus -Path $status) -ne $expected) { + throw "Completion-status round trip failed" + } + Remove-Item -LiteralPath $status + } + foreach ($invalid in @("", " ", "failed", "2147483648", "0`n1")) { + [System.IO.File]::WriteAllText($status, $invalid) + try { + Read-ServerStatus -Path $status | Out-Null + throw "Invalid completion status unexpectedly succeeded" + } catch { + if ($_ -notmatch "published an invalid completion status") { + throw + } + } + Remove-Item -LiteralPath $status + } Set-Content -LiteralPath $ready -Value ( @{ Nonce = "nonce" @@ -366,7 +410,7 @@ if ($Action -eq "Server") { $_ | Out-File -LiteralPath $ServerOutputPath -Append $exitCode = 1 } finally { - Set-Content -LiteralPath $StatusPath -Value $exitCode + Publish-ServerStatus -Path $StatusPath -ExitCode $exitCode } exit $exitCode } @@ -533,9 +577,14 @@ try { Get-Content -LiteralPath $serverOutputPath | Out-File $outputPath -Append } if (Test-Path -LiteralPath $statusPath) { - $serverExitCode = [int](Get-Content -LiteralPath $statusPath -Raw) - if ($serverExitCode -ne 0 -and $exitCode -eq 0) { - $exitCode = $serverExitCode + try { + $serverExitCode = Read-ServerStatus -Path $statusPath + if ($serverExitCode -ne 0 -and $exitCode -eq 0) { + $exitCode = $serverExitCode + } + } catch { + $_ | Out-File $outputPath -Append + $exitCode = 1 } } elseif ($exitCode -eq 0) { "Timed out waiting for LocalSystem test server shutdown" | Out-File $outputPath -Append diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index 8dcb471ad..1c136183e 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -245,6 +245,7 @@ pub(crate) async fn run() -> anyhow::Result<()> { redirected_policy_paths_fail_closed(&agent_path).await?; management_write_tokens_survive_watcher_reload(&agent_path).await?; managed_policy_lifecycle(&agent_path).await?; + legacy_contract_and_interrupted_repair(&agent_path).await?; } } @@ -662,9 +663,30 @@ async fn validate_policy_by_pipe(pipe_name: &str, draft: &Value) -> anyhow::Resu ); let validation = validation_response.json()?["Validation"].clone(); ensure!(validation["IsValid"] == true, "policy validation failed"); + ensure!( + validation["ValidatorVersion"] == "now-package-broker-policy-validator/9", + "unexpected validator contract" + ); + ensure!( + validation["CanonicalDraft"].get("$schema").is_none() + && validation["CanonicalDraft"].get("PolicyVersion").is_none() + && validation["CanonicalDraft"]["PolicyFormatVersion"] == draft["PolicyFormatVersion"], + "canonical draft changed the format version or emitted legacy fields" + ); Ok(validation) } +async fn send_replacement(pipe_name: &str, replacement: &Value) -> anyhow::Result { + request_with_body( + pipe_name, + "PUT", + "/v1/policy", + Some("application/json"), + &serde_json::to_vec(replacement)?, + ) + .await +} + async fn replace_policy_response( agent: &AgentHarness, operation: &str, @@ -700,15 +722,7 @@ async fn replace_policy_response_by_pipe( "Draft": validation["CanonicalDraft"], "ValidationReceipt": validation["ValidationReceipt"] }); - let response = request_with_body( - pipe_name, - "PUT", - "/v1/policy", - Some("application/json"), - &serde_json::to_vec(&replacement_request)?, - ) - .await?; - Ok(response) + send_replacement(pipe_name, &replacement_request).await } async fn replace_policy( @@ -827,7 +841,9 @@ async fn standard_user_server( tokio::time::sleep(Duration::from_millis(50)).await; } - wait_for_log(&agent, "Policy management write denied").await + let result = wait_for_log(&agent, "Policy management write denied").await; + agent.stop().await?; + result } async fn standard_user_management(ready_path: &Path, nonce: &str, client: &ProcessIdentity) -> anyhow::Result<()> { @@ -872,12 +888,64 @@ async fn standard_user_management(ready_path: &Path, nonce: &str, client: &Proce "valid draft did not produce a canonical draft and receipt" ); - let mut invalid_draft = valid_draft.clone(); - invalid_draft["$schema"] = json!("https://example.com/not-the-policy-draft-schema.json"); + strict_contract_validation(pipe_name).await?; + + for operation in ["Create", "Update", "Repair", "ReplaceIdentity"] { + let denied = replace_policy_response_by_pipe( + pipe_name, + operation, + "Reject", + management["StoreToken"].clone(), + valid_draft.clone(), + ) + .await?; + ensure!( + denied.status == 403 && denied.json()?["Code"] == "AdministratorRequired", + "standard-user {operation} did not require an administrator" + ); + } + ensure!( + policy_management_by_pipe(pipe_name).await?["StoreToken"] == management["StoreToken"], + "denied writes changed the store token" + ); + Ok(()) +} + +async fn strict_contract_validation(pipe_name: &str) -> anyhow::Result<()> { + for version in ["1.0.0", "1.7.3"] { + let mut draft = policy_draft("tests.contract", "Contract"); + draft["PolicyFormatVersion"] = json!(version); + validate_policy_by_pipe(pipe_name, &draft).await?; + } + for (field, value, expected_path) in [ + ( + "$schema", + "https://devolutions.net/schemas/now-policy.schema.1.0.json", + "/$schema", + ), + ("PolicyVersion", "1.0.0", "/PolicyVersion"), + ("PolicyFormatVersion", "2.0.0", "/PolicyFormatVersion"), + ("PolicyFormatVersion", "broken", "/PolicyFormatVersion"), + ] { + let mut draft = policy_draft("tests.contract", "Contract"); + draft[field] = json!(value); + assert_invalid_draft(pipe_name, draft, expected_path).await?; + } + let mut legacy = policy_draft("tests.contract", "Contract"); + legacy + .as_object_mut() + .context("draft is not an object")? + .remove("PolicyFormatVersion"); + legacy["PolicyVersion"] = json!("1.0.0"); + legacy["$schema"] = json!("https://devolutions.net/schemas/now-policy.schema.1.0.json"); + assert_invalid_draft(pipe_name, legacy, "/PolicyVersion").await +} + +async fn assert_invalid_draft(pipe_name: &str, draft: Value, expected_path: &str) -> anyhow::Result<()> { let invalid_request = json!({ "RequestKind": "PolicyValidationRequest", "RequestVersion": "1.0", - "Draft": invalid_draft + "Draft": draft }); let invalid_response = request_with_body( pipe_name, @@ -895,32 +963,25 @@ async fn standard_user_management(ready_path: &Path, nonce: &str, client: &Proce let invalid_validation = invalid_response.json()?["Validation"].clone(); ensure!(invalid_validation["IsValid"] == false, "invalid draft was accepted"); ensure!( - invalid_validation.get("CanonicalDraft").is_none(), - "invalid draft returned a canonical draft" - ); - - let denied = replace_policy_response_by_pipe( - pipe_name, - "Create", - "Reject", - management["StoreToken"].clone(), - valid_draft, - ) - .await?; - ensure!( - denied.status == 403, - "standard-user Create returned HTTP {}", - denied.status + invalid_validation.get("CanonicalDraft").is_none() + && invalid_validation.get("ValidationReceipt").is_none() + && invalid_validation["ValidatorVersion"] == "now-package-broker-policy-validator/9", + "invalid draft returned a canonical draft, receipt, or wrong validator version" ); ensure!( - denied.json()?["Code"] == "AdministratorRequired", - "standard-user Create did not require an administrator" + invalid_validation["Findings"] + .as_array() + .is_some_and(|findings| findings + .iter() + .any(|finding| finding["Severity"] == "Error" && finding["Path"] == expected_path)), + "invalid draft did not report {expected_path}: {invalid_validation}" ); Ok(()) } async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { let mut agent = AgentHarness::start_managed_default(agent_path).await?; + strict_contract_validation(&agent.pipe_name).await?; let initial = policy_management(&agent).await?; ensure!( initial["State"] == "Missing", @@ -1051,6 +1112,7 @@ async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { "reused ConfirmOverwrite token did not conflict" ); + let confirmed = warnings_identity_and_receipts(&mut agent, agent_path, &confirmed).await?; agent.restart(agent_path).await?; let restarted = request(&agent.pipe_name, "GET", "/v1/policy").await?; ensure!( @@ -1089,6 +1151,150 @@ async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { Ok(()) } +async fn warnings_identity_and_receipts( + agent: &mut AgentHarness, + agent_path: &Path, + current: &Value, +) -> anyhow::Result { + let mut draft = policy_draft("tests.replaced-identity", "Compatible contract"); + draft["PolicyFormatVersion"] = json!("1.7.3"); + draft["Enforcement"]["AuditMode"] = json!(true); + let validation = validate_policy_by_pipe(&agent.pipe_name, &draft).await?; + ensure!( + validation["Findings"].as_array().is_some_and(|findings| findings + .iter() + .any(|finding| finding["Code"] == "AuditModeEnabled" && finding["Severity"] == "Warning")), + "audit-mode draft did not produce its warning" + ); + let mut replacement = json!({ + "RequestKind": "PolicyReplacementRequest", + "RequestVersion": "1.0", + "ExpectedStoreToken": current["Management"]["StoreToken"], + "Operation": "ReplaceIdentity", + "ConflictHandling": "Reject", + "WarningsAcknowledged": false, + "Draft": validation["CanonicalDraft"], + "ValidationReceipt": validation["ValidationReceipt"] + }); + let warning = send_replacement(&agent.pipe_name, &replacement).await?; + ensure!( + warning.status == 409 && warning.json()?["Code"] == "WarningConfirmationRequired", + "unacknowledged warnings were not rejected" + ); + replacement["WarningsAcknowledged"] = json!(true); + replacement["Draft"]["Metadata"]["Publisher"] = json!("Tampered after validation"); + let tampered = send_replacement(&agent.pipe_name, &replacement).await?; + ensure!( + tampered.status == 422 && tampered.json()?["Code"] == "ValidationFailed", + "receipt authorized a different draft" + ); + replacement["Draft"] = validation["CanonicalDraft"].clone(); + agent.restart(agent_path).await?; + let restarted = policy_management(agent).await?; + replacement["ExpectedStoreToken"] = restarted["StoreToken"].clone(); + let expired = send_replacement(&agent.pipe_name, &replacement).await?; + ensure!( + expired.status == 422 && expired.json()?["Code"] == "ValidationFailed", + "pre-restart receipt remained valid in a new validator instance" + ); + ensure!( + restarted["Policy"] == current["Policy"] + && policy_management(agent).await?["StoreToken"] == restarted["StoreToken"], + "rejected requests changed the policy or exact store token" + ); + let replaced = replace_policy(agent, "ReplaceIdentity", restarted["StoreToken"].clone(), draft).await?; + ensure!( + replaced["Policy"]["Metadata"]["Id"] == "tests.replaced-identity" + && replaced["Policy"]["Metadata"]["Revision"] == 1 + && replaced["Policy"]["PolicyFormatVersion"] == "1.7.3" + && replaced["Policy"].get("$schema").is_none() + && replaced["Policy"].get("PolicyVersion").is_none(), + "ReplaceIdentity did not preserve the compatible contract and reset revision" + ); + wait_for_log(agent, "Policy change succeeded").await?; + wait_for_log(agent, "replace_identity").await?; + wait_for_log(agent, "invalid_receipt").await?; + wait_for_log(agent, "warnings_not_acknowledged").await?; + Ok(replaced) +} + +async fn legacy_contract_and_interrupted_repair(agent_path: &Path) -> anyhow::Result<()> { + let mut legacy = empty_policy(); + legacy + .as_object_mut() + .context("policy is not an object")? + .remove("PolicyFormatVersion"); + legacy["$schema"] = json!("https://devolutions.net/schemas/now-policy.schema.1.0.json"); + legacy["PolicyVersion"] = json!("1.0.0"); + let mut mixed = legacy.clone(); + mixed["PolicyFormatVersion"] = json!("1.0.0"); + for original in [ + serde_json::to_vec(&legacy)?, + serde_json::to_vec(&mixed)?, + b"malformed-policy-secret-marker".to_vec(), + ] { + for marker_staging in [false, true] { + let data_dir = create_data_dir()?; + let policy_path = data_dir.path().join("policy.json"); + std::fs::write(&policy_path, &original)?; + secure_policy_path(&policy_path, false)?; + let prefix = ".policy.json.txn-11111111-2222-4333-8444-555555555555"; + let new_path = data_dir.path().join(format!("{prefix}.new")); + std::fs::write(&new_path, b"partial replacement")?; + secure_policy_path(&new_path, false)?; + let marker_path = data_dir.path().join(format!("{prefix}.marker.prepare")); + if marker_staging { + std::fs::write(&marker_path, br#"{"Version":"#)?; + secure_policy_path(&marker_path, false)?; + } + let mut agent = + AgentHarness::start_with_path(agent_path, data_dir, unique_pipe_name(), policy_path).await?; + let mut invalid = policy_management(&agent).await?; + ensure!( + invalid["State"] == "Invalid" && invalid["WriteCapability"] == "Writable", + "interrupted Repair did not retain a repairable invalid original: {invalid}" + ); + ensure!( + std::fs::read(&agent.policy_path)? == original && !new_path.exists() && !marker_path.exists(), + "recovery changed the original or retained prepublication remnants" + ); + ensure!( + request(&agent.pipe_name, "GET", "/v1/policy").await?.status == 404, + "legacy, mixed, or malformed policy became active" + ); + if original.starts_with(b"{") { + ensure!( + invalid["InvalidDiagnostics"]["Findings"] + .as_array() + .is_some_and(|findings| findings + .iter() + .any(|finding| finding["Code"] == "UnsupportedPolicyFormatVersion")), + "legacy contract did not produce its strict diagnostic" + ); + std::fs::write(&agent.policy_path, serde_json::to_vec(&empty_policy())?)?; + wait_for_management(&agent, |management| management["State"] == "Active").await?; + std::fs::write(&agent.policy_path, &original)?; + invalid = wait_for_management(&agent, |management| management["State"] == "Invalid").await?; + wait_for_log(&agent, "legacy_policy_contract").await?; + } + let repaired = replace_policy( + &agent, + "Repair", + invalid["StoreToken"].clone(), + policy_draft("tests.interrupted-repair", "Recovered"), + ) + .await?; + agent.restart(agent_path).await?; + ensure!( + policy_management(&agent).await?["Policy"] == repaired["Policy"], + "repaired policy did not survive restart" + ); + agent.stop().await?; + } + } + Ok(()) +} + async fn wait_for_management(agent: &AgentHarness, predicate: impl Fn(&Value) -> bool) -> anyhow::Result { let deadline = Instant::now() + Duration::from_secs(10); loop { @@ -1270,6 +1476,26 @@ async fn complete_snapshots_across_reload(agent_path: &Path) -> anyhow::Result<( mod tests { use super::*; + #[test] + fn policy_fixtures_use_the_current_contract() { + for policy in [full_policy(), empty_policy(), policy_draft("tests.contract", "Test")] { + assert_eq!(policy["PolicyFormatVersion"], "1.0.0"); + assert!(policy.get("$schema").is_none()); + assert!(policy.get("PolicyVersion").is_none()); + } + } + + #[test] + fn standard_user_token_rejects_wrong_identity_and_administrators() { + for (actual_sid, administrator, integrity) in [ + ("S-1-5-21-1-2-3-1002", false, SECURITY_MANDATORY_MEDIUM_RID), + ("S-1-5-21-1-2-3-1001", true, SECURITY_MANDATORY_MEDIUM_RID), + ("S-1-5-21-1-2-3-1001", false, 0x3000), + ] { + assert!(validate_standard_user_token(actual_sid, "S-1-5-21-1-2-3-1001", administrator, integrity).is_err()); + } + } + #[test] fn standard_user_token_requires_medium_integrity() { validate_standard_user_token( diff --git a/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs b/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs new file mode 100644 index 000000000..e76e4153d --- /dev/null +++ b/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs @@ -0,0 +1,438 @@ +using DevolutionsAgent.Actions; +using DevolutionsAgent.Resources; +using Microsoft.Deployment.WindowsInstaller; +using Microsoft.Win32.SafeHandles; +using Newtonsoft.Json.Linq; +using System; +using System.ComponentModel; +using System.Diagnostics; +using System.IO; +using System.IO.Pipes; +using System.Runtime.InteropServices; +using System.Security.AccessControl; +using System.Security.Principal; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace DevolutionsAgent.Installer.Tests; + +public sealed class InstalledAgentMigrationE2eTests +{ + private const string CurrentPolicy = + """{"PolicyFormatVersion":"1.7.3","PolicyType":"PackageBrokerPolicy","Metadata":{"Id":"installer-e2e","Publisher":"Test","Revision":17,"PublishedAt":"2026-01-01T00:00:00Z"},"Enforcement":{"DefaultDecision":"Deny","RulePrecedence":"PriorityThenDeny"},"Rules":[]}"""; + private readonly ITestOutputHelper output; + + public InstalledAgentMigrationE2eTests(ITestOutputHelper output) => this.output = output; + + [Fact] + public void AgentJobTerminatesChildWhenScopeThrows() + { + using Process child = Process.Start(new ProcessStartInfo( + Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), + @"WindowsPowerShell\v1.0\powershell.exe"), + "-NoProfile -NonInteractive -Command Start-Sleep -Seconds 60") + { + UseShellExecute = false, + CreateNoWindow = true, + }); + try + { + InvalidOperationException failure = new("simulate a failed Agent assertion"); + void FailWithAssignedChild() + { + using AgentJob job = new(); + job.Assign(child); + throw failure; + } + Assert.Same(failure, Assert.Throws(FailWithAssignedChild)); + Assert.True(child.WaitForExit(10000), "Job disposal left the child running"); + } + finally + { + if (!child.HasExited) + { + child.Kill(); + Assert.True(child.WaitForExit(10000), "Cleanup did not stop the child"); + } + } + } + + [PackageBrokerInstallerTests.SystemFact] + public void ConvertedTransactionActivatesAndRetainsManagedAuthority() + { + Assert.True(WindowsIdentity.GetCurrent().IsSystem); + string agent = Environment.GetEnvironmentVariable("DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE"); + Assert.False(string.IsNullOrWhiteSpace(agent), "The SYSTEM runner must supply the built Agent executable"); + Assert.True(File.Exists(agent), agent); + Assert.Equal(Includes.EXECUTABLE_NAME, Path.GetFileName(agent), ignoreCase: true); + using PackageBrokerPolicyActions.PinnedPath installedAgent = + PackageBrokerPolicyActions.PinPathWithoutReparse( + agent, leafIsDirectory: false, allowMissingLeaf: false, + leafAccess: WinAPI.GENERIC_READ | WinAPI.READ_CONTROL, verifyTrustedAncestors: true); + Assert.True( + PackageBrokerPolicyActions.TryVerifyLegacyPolicySourceSecurity( + File.GetAccessControl(agent), out string agentSecurityDiagnostic), + agentSecurityDiagnostic); + string root = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + $"dgw-installer-e2e-{Guid.NewGuid():N}"); + PackageBrokerPolicyActions.CreateDirectoryWithSecurity(root, Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); + try + { + PackageBrokerPolicyActions.VerifyPackageBrokerSecurity(Directory.GetAccessControl(root)); + string vendor = Path.Combine(root, "Devolutions"); + string legacyDirectory = Path.Combine(vendor, "Agent"); + string managedDirectory = Path.Combine(vendor, "PackageBroker"); + foreach (string directory in new[] { vendor, legacyDirectory, managedDirectory }) + { + PackageBrokerPolicyActions.CreateDirectoryWithSecurity( + directory, Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); + } + string source = Path.Combine(legacyDirectory, "package-broker-policy.json"); + string destination = Path.Combine(managedDirectory, "package-broker-policy.json"); + string marker = Path.Combine(managedDirectory, ".installer-e2e.migration"); + string authority = Path.Combine(managedDirectory, ".package-broker-managed-authority.v1"); + byte[] original = Encoding.UTF8.GetBytes(CurrentPolicy.Replace( + "\"PolicyFormatVersion\":", + "\"$schema\":\"https://devolutions.net/schemas/now-policy.schema.1.0.json\",\"PolicyVersion\":")); + File.WriteAllBytes(source, original); + FileSecurity security = new(); + security.SetSecurityDescriptorSddlForm(Includes.PROGRAM_DATA_PACKAGE_BROKER_FILE_SDDL); + File.SetAccessControl(source, security); + + int conversions = 0; + void Migrate() + { + Assert.Equal(ActionResult.Success, PackageBrokerPolicyActions.MigrateLegacyPolicy( + output.WriteLine, source, destination, marker, input => + { + Assert.False(File.Exists(destination)); + Assert.False(File.Exists(authority)); + Assert.Equal(original, input); + byte[] converted = PackageBrokerPolicyActions.ConvertWithInstalledAgent( + Path.GetDirectoryName(agent), input); + Assert.Equal(CurrentPolicy, Encoding.UTF8.GetString(converted)); + conversions++; + return converted; + })); + Assert.Equal(CurrentPolicy, File.ReadAllText(destination)); + Assert.True(File.Exists(authority)); + Assert.Empty(File.ReadAllBytes(authority)); + AssertPreserved(); + foreach (string path in new[] { destination, authority, marker, marker + ".original" }) + { + PackageBrokerPolicyActions.VerifyPackageBrokerSecurity(File.GetAccessControl(path)); + } + } + void AssertPreserved() + { + Assert.Equal(original, File.ReadAllBytes(source)); + Assert.Equal(original, File.ReadAllBytes(marker + ".original")); + Assert.True(File.Exists(marker)); + } + + Migrate(); + Assert.Equal(ActionResult.Success, PackageBrokerPolicyActions.RollbackLegacyPolicy( + output.WriteLine, source, destination, marker)); + Assert.False(File.Exists(destination)); + Assert.False(File.Exists(authority)); + AssertPreserved(); + Migrate(); + Assert.Equal(2, conversions); + PackageBrokerPolicyActions.CommitLegacyPolicy(output.WriteLine, source, destination, marker); + AssertPreserved(); + + using AgentProcess running = new(agent, root, output); + running.Start(); + JToken active = AssertActive(running, destination); + running.Stop(); + running.Start(); + Assert.True(JToken.DeepEquals(active, AssertActive(running, destination))); + AssertPreserved(); + running.Stop(); + File.Delete(destination); + running.Start(); + JObject management = running.Get("/v1/policy/management", 200)["Management"] as JObject; + Assert.NotNull(management); + Assert.Equal("DefaultPath", (string)management["Source"]); + Assert.Equal("Missing", (string)management["State"]); + Assert.Equal("active policy is unavailable", (string)running.Get("/v1/policy", 404)["Message"]); + Assert.True(File.Exists(authority)); + AssertPreserved(); + } + finally + { + for (int attempt = 0; ; attempt++) + { + try + { + Directory.Delete(root, recursive: true); + break; + } + catch (IOException) when (attempt < 19) + { + Thread.Sleep(250); + } + } + } + } + + private static JToken AssertActive(AgentProcess agent, string destination) + { + JObject response = agent.Get("/v1/policy", 200); + JToken policy = response["Policy"]; + Assert.NotNull(policy); + Assert.Equal("1.7.3", (string)policy["PolicyFormatVersion"]); + Assert.Equal("PackageBrokerPolicy", (string)policy["PolicyType"]); + Assert.Null(policy["PolicyVersion"]); + Assert.Null(policy["$schema"]); + Assert.Equal("installer-e2e", (string)policy["Metadata"]["Id"]); + Assert.Equal("Test", (string)policy["Metadata"]["Publisher"]); + Assert.Equal(17, (int)policy["Metadata"]["Revision"]); + Assert.Equal( + JObject.Parse(CurrentPolicy)["Metadata"]["PublishedAt"], + policy["Metadata"]["PublishedAt"]); + Assert.True(JToken.DeepEquals(JObject.Parse(CurrentPolicy)["Enforcement"], policy["Enforcement"])); + Assert.True(JToken.DeepEquals(new JArray(), policy["Rules"])); + Assert.Equal(CurrentPolicy, File.ReadAllText(destination)); + JObject management = agent.Get("/v1/policy/management", 200)["Management"] as JObject; + Assert.NotNull(management); + Assert.Equal("DefaultPath", (string)management["Source"]); + Assert.Equal("Active", (string)management["State"]); + return policy; + } + + private sealed class AgentProcess : IDisposable + { + private readonly string executable; + private readonly string root; + private readonly ITestOutputHelper output; + private readonly string pipeName = $"Devolutions.Now.PackageBroker.installer-e2e.{Guid.NewGuid():N}"; + private readonly AgentJob job; + private Process process; + private Task stdout; + private Task stderr; + + internal AgentProcess(string executable, string root, ITestOutputHelper output) + { + this.executable = executable; + this.root = root; + this.output = output; + JObject config = new() + { + ["LogFile"] = Path.Combine(root, "agent-installer-e2e"), + ["PackageBroker"] = new JObject + { + ["Enabled"] = true, + ["PipeName"] = @"\\.\pipe\" + pipeName, + }, + ["__debug__"] = new JObject { ["skip_broker_signature_validation"] = true }, + }; + File.WriteAllText(Path.Combine(root, "agent.json"), config.ToString()); + job = new AgentJob(); + } + + internal void Start() + { + Assert.Null(process); + stdout = null; + stderr = null; + ProcessStartInfo start = new(executable, "run") + { + UseShellExecute = false, + CreateNoWindow = true, + WorkingDirectory = root, + RedirectStandardOutput = true, + RedirectStandardError = true, + }; + start.EnvironmentVariables["DAGENT_CONFIG_PATH"] = root; + start.EnvironmentVariables["ProgramData"] = root; + process = Process.Start(start); + job.Assign(process); + stdout = process.StandardOutput.ReadToEndAsync(); + stderr = process.StandardError.ReadToEndAsync(); + Stopwatch timer = Stopwatch.StartNew(); + while (true) + { + Assert.False(process.HasExited, "Agent exited before its broker became ready"); + try + { + Get("/v1/health", 200); + return; + } + catch (TimeoutException) when (timer.Elapsed < TimeSpan.FromSeconds(20)) + { + Thread.Sleep(50); + } + catch (IOException) when (timer.Elapsed < TimeSpan.FromSeconds(20)) + { + Thread.Sleep(50); + } + } + } + + internal JObject Get(string path, int expectedStatus) + { + using NamedPipeClientStream pipe = new( + ".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous); + pipe.Connect(1000); + Task request = Exchange(pipe, path); + if (Task.WhenAny(request, Task.Delay(TimeSpan.FromSeconds(10))).GetAwaiter().GetResult() != request) + { + throw new TimeoutException($"timed out reading {path}"); + } + string response = request.GetAwaiter().GetResult(); + int headerEnd = response.IndexOf("\r\n\r\n", StringComparison.Ordinal); + Assert.True(headerEnd > 0, response); + Assert.Equal(expectedStatus.ToString(), response.Split(' ')[1]); + return JObject.Parse(response.Substring(headerEnd + 4)); + } + + private static async Task Exchange(NamedPipeClientStream pipe, string path) + { + byte[] request = Encoding.ASCII.GetBytes( + $"GET {path} HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\nContent-Length: 0\r\n\r\n"); + await pipe.WriteAsync(request, 0, request.Length).ConfigureAwait(false); + await pipe.FlushAsync().ConfigureAwait(false); + using MemoryStream response = new(); + await pipe.CopyToAsync(response).ConfigureAwait(false); + return Encoding.UTF8.GetString(response.ToArray()); + } + + internal void Stop() + { + if (process == null) + { + return; + } + try + { + if (!process.HasExited) + { + try + { + process.Kill(); + } + catch (InvalidOperationException) when (process.HasExited) + { + } + catch (Win32Exception) when (process.WaitForExit(10000)) + { + } + } + Assert.True(process.WaitForExit(10000), "Agent did not stop"); + if (stdout != null) + { + output.WriteLine(stdout.GetAwaiter().GetResult()); + output.WriteLine(stderr.GetAwaiter().GetResult()); + } + foreach (string log in Directory.GetFiles(root, "agent-installer-e2e*")) + { + output.WriteLine(File.ReadAllText(log)); + } + } + finally + { + if (process.HasExited) + { + process.Dispose(); + process = null; + } + } + } + + public void Dispose() + { + job.Dispose(); + Stop(); + } + } + + private sealed class AgentJob : IDisposable + { + private const uint JobObjectLimitKillOnJobClose = 0x2000; + private const int JobObjectExtendedLimitInformation = 9; + private readonly SafeFileHandle handle; + + internal AgentJob() + { + handle = CreateJobObjectW(IntPtr.Zero, null); + if (handle.IsInvalid) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + ExtendedLimitInformation limits = new() + { + BasicLimitInformation = new BasicLimitInformation { LimitFlags = JobObjectLimitKillOnJobClose }, + }; + if (!SetInformationJobObject( + handle, JobObjectExtendedLimitInformation, ref limits, Marshal.SizeOf())) + { + int error = Marshal.GetLastWin32Error(); + handle.Dispose(); + throw new Win32Exception(error); + } + } + + internal void Assign(Process process) + { + if (!AssignProcessToJobObject(handle, process.Handle)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } + + public void Dispose() => handle.Dispose(); + + [StructLayout(LayoutKind.Sequential)] + private struct BasicLimitInformation + { + internal long PerProcessUserTimeLimit; + internal long PerJobUserTimeLimit; + internal uint LimitFlags; + internal UIntPtr MinimumWorkingSetSize; + internal UIntPtr MaximumWorkingSetSize; + internal uint ActiveProcessLimit; + internal UIntPtr Affinity; + internal uint PriorityClass; + internal uint SchedulingClass; + } + + [StructLayout(LayoutKind.Sequential)] + private struct IoCounters + { + internal ulong ReadOperationCount; + internal ulong WriteOperationCount; + internal ulong OtherOperationCount; + internal ulong ReadTransferCount; + internal ulong WriteTransferCount; + internal ulong OtherTransferCount; + } + + [StructLayout(LayoutKind.Sequential)] + private struct ExtendedLimitInformation + { + internal BasicLimitInformation BasicLimitInformation; + internal IoCounters IoInfo; + internal UIntPtr ProcessMemoryLimit; + internal UIntPtr JobMemoryLimit; + internal UIntPtr PeakProcessMemoryUsed; + internal UIntPtr PeakJobMemoryUsed; + } + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern SafeFileHandle CreateJobObjectW(IntPtr attributes, string name); + + [DllImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool SetInformationJobObject( + SafeFileHandle job, int informationClass, ref ExtendedLimitInformation information, int length); + + [DllImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool AssignProcessToJobObject(SafeFileHandle job, IntPtr process); + } +} From 8059127ae47b7444fcc74e66a302ade4b6617436 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 16 Sep 2026 02:56:10 +0900 Subject: [PATCH 08/14] fix(agent): restore NuGet test imports for SYSTEM Pass the restored package root explicitly to the LocalSystem test invocation so generated imports discover the test SDK and produce its required privileged-test result. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 10 +++++++++- crates/agent-policy-tester/run-as-system.ps1 | 11 +++++++++-- .../agent-policy-tester/run-installer-tests.ps1 | 15 ++++++++++++++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a6b4e920..b11ead0cc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1413,7 +1413,15 @@ jobs: run: | $scriptPath = Resolve-Path -Path "./crates/agent-policy-tester/run-as-system.ps1" $dotnetPath = (Get-Command dotnet.exe).Source - psexec -accepteula -s pwsh.exe -NoProfile -File $scriptPath -DotnetPath $dotnetPath + $nuGetPackagesPath = ( + & $dotnetPath nuget locals global-packages --list | + Select-String '^global-packages:\s*(.+)$' + ).Matches[0].Groups[1].Value.Trim() + if ([string]::IsNullOrWhiteSpace($nuGetPackagesPath) -or -not (Test-Path -LiteralPath $nuGetPackagesPath)) { + throw "Could not resolve the restored NuGet package root for LocalSystem" + } + psexec -accepteula -s pwsh.exe -NoProfile -File $scriptPath ` + -DotnetPath $dotnetPath -NuGetPackagesPath $nuGetPackagesPath $exitCode = $LASTEXITCODE Get-Content -Path ./crates/agent-policy-tester/agent-policy-tester.out if ($exitCode -ne 0) { diff --git a/crates/agent-policy-tester/run-as-system.ps1 b/crates/agent-policy-tester/run-as-system.ps1 index 2e23d612f..ceeb8f71c 100644 --- a/crates/agent-policy-tester/run-as-system.ps1 +++ b/crates/agent-policy-tester/run-as-system.ps1 @@ -1,5 +1,6 @@ param( - [string] $DotnetPath = (Join-Path $env:ProgramFiles "dotnet\dotnet.exe") + [string] $DotnetPath = (Join-Path $env:ProgramFiles "dotnet\dotnet.exe"), + [Parameter(Mandatory)] [string] $NuGetPackagesPath ) $ErrorActionPreference = "Stop" @@ -15,6 +16,7 @@ $installerProject = Join-Path $workspacePath "package\AgentWindowsManaged.Tests\ $installerOutput = Join-Path $workspacePath "package\AgentWindowsManaged.Tests\bin\Debug\net48" $stagedInstallerOutput = Join-Path $stagingPath "installer-tests" $resultsPath = Join-Path $PSScriptRoot "installer-test-results" +$stagedResultsPath = Join-Path $stagingPath "installer-test-results" $exitCode = 1 $previousTemp = $env:TEMP $previousTmp = $env:TMP @@ -27,6 +29,7 @@ try { if ([System.IO.DriveInfo]::new([System.IO.Path]::GetPathRoot($workspacePath)).DriveType -ne 'Fixed') { throw "Use a local fixed-volume workspace path visible to LocalSystem, not a mapped drive" } + $NuGetPackagesPath = (Resolve-Path -LiteralPath $NuGetPackagesPath).Path Add-Type -TypeDefinition @' using System; using System.ComponentModel; @@ -79,6 +82,9 @@ public static class AgentPolicyTesterNativeDirectory if (Get-ChildItem -LiteralPath $stagingPath -Force) { throw "The atomically protected staged tester directory was not empty" } + if (Test-Path -LiteralPath $resultsPath) { + Remove-Item -LiteralPath $resultsPath -Recurse -Force + } $env:TEMP = Join-Path $stagingPath "scratch" $env:TMP = $env:TEMP New-Item -ItemType Directory -Path $env:TEMP | Out-Null @@ -103,7 +109,8 @@ public static class AgentPolicyTesterNativeDirectory $exitCode = $LASTEXITCODE & (Join-Path $PSScriptRoot "run-installer-tests.ps1") ` -ProjectPath $installerProject -TestOutputPath $stagedInstallerOutput ` - -AgentPath $stagedAgentPath -ResultsPath $resultsPath -DotnetPath $DotnetPath ` + -AgentPath $stagedAgentPath -ResultsPath $stagedResultsPath -ArtifactResultsPath $resultsPath ` + -DotnetPath $DotnetPath -NuGetPackagesPath $NuGetPackagesPath ` 2>&1 | Out-File $outputPath -Append if ($LASTEXITCODE -ne 0) { $exitCode = $LASTEXITCODE diff --git a/crates/agent-policy-tester/run-installer-tests.ps1 b/crates/agent-policy-tester/run-installer-tests.ps1 index e2c67c402..1e96f8090 100644 --- a/crates/agent-policy-tester/run-installer-tests.ps1 +++ b/crates/agent-policy-tester/run-installer-tests.ps1 @@ -3,17 +3,26 @@ param( [Parameter(Mandatory)] [string] $TestOutputPath, [Parameter(Mandatory)] [string] $AgentPath, [Parameter(Mandatory)] [string] $ResultsPath, - [Parameter(Mandatory)] [string] $DotnetPath + [Parameter(Mandatory)] [string] $ArtifactResultsPath, + [Parameter(Mandatory)] [string] $DotnetPath, + [Parameter(Mandatory)] [string] $NuGetPackagesPath ) $ErrorActionPreference = "Stop" $exitCode = 1 $previousAgent = $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE +$previousNuGetPackages = $env:NUGET_PACKAGES try { if (-not [System.Security.Principal.WindowsIdentity]::GetCurrent().IsSystem) { throw "Installer transaction tests must execute as LocalSystem" } + $nuGetPackageRoot = (Resolve-Path -LiteralPath $NuGetPackagesPath).Path + $separator = [System.IO.Path]::DirectorySeparatorChar.ToString() + if (-not $nuGetPackageRoot.EndsWith($separator, [System.StringComparison]::Ordinal)) { + $nuGetPackageRoot += $separator + } + $env:NUGET_PACKAGES = $nuGetPackageRoot $testClass = 'DevolutionsAgent.Installer.Tests.PackageBrokerInstallerTests' $expectedTests = @( "$testClass.TransactionTestsRunAsLocalSystem" @@ -39,6 +48,7 @@ try { $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE = (Resolve-Path -LiteralPath $AgentPath).Path & $DotnetPath test $ProjectPath --no-build --no-restore --configuration Debug --framework net48 ` "-p:OutputPath=$TestOutputPath\" -p:AppendTargetFrameworkToOutputPath=false ` + "-p:NuGetPackageRoot=$env:NUGET_PACKAGES" ` --filter $filter --logger "trx;LogFileName=installer-system.trx" --results-directory $ResultsPath $exitCode = $LASTEXITCODE if ($exitCode -ne 0) { @@ -61,6 +71,8 @@ try { throw "Required SYSTEM installer test did not pass exactly once: $name" } } + New-Item -ItemType Directory -Path $ArtifactResultsPath -Force | Out-Null + Copy-Item -LiteralPath $trxPath -Destination $ArtifactResultsPath -Force Write-Output "Verified all 12 installer SystemFacts and the installed-Agent migration E2E: 13 passed, zero skipped" } catch { Write-Output $_ @@ -69,6 +81,7 @@ try { } } finally { $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE = $previousAgent + $env:NUGET_PACKAGES = $previousNuGetPackages } exit $exitCode From c10b4bd774cab5be89276979211220b91b741046 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 16 Sep 2026 03:08:51 +0900 Subject: [PATCH 09/14] fix(agent): run installer tests as LocalSystem Preserve the restored NuGet imports and the product Agent filename when running the privileged installer lifecycle tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/run-as-system.ps1 | 2 +- .../InstalledAgentMigrationE2eTests.cs | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/agent-policy-tester/run-as-system.ps1 b/crates/agent-policy-tester/run-as-system.ps1 index ceeb8f71c..be1938f88 100644 --- a/crates/agent-policy-tester/run-as-system.ps1 +++ b/crates/agent-policy-tester/run-as-system.ps1 @@ -11,7 +11,7 @@ $agentPath = Join-Path $workspacePath "target/debug/devolutions-agent.exe" $outputPath = Join-Path $PSScriptRoot "agent-policy-tester.out" $stagingPath = Join-Path ([Environment]::GetFolderPath('CommonApplicationData')) "dgw-agent-policy-tester-$([guid]::NewGuid().ToString('N'))" $stagedTesterPath = Join-Path $stagingPath "agent-policy-tester.exe" -$stagedAgentPath = Join-Path $stagingPath "devolutions-agent.exe" +$stagedAgentPath = Join-Path $stagingPath "DevolutionsAgent.exe" $installerProject = Join-Path $workspacePath "package\AgentWindowsManaged.Tests\DevolutionsAgent.Installer.Tests.csproj" $installerOutput = Join-Path $workspacePath "package\AgentWindowsManaged.Tests\bin\Debug\net48" $stagedInstallerOutput = Join-Path $stagingPath "installer-tests" diff --git a/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs b/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs index e76e4153d..1d6c0b2a8 100644 --- a/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs +++ b/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs @@ -27,6 +27,14 @@ public sealed class InstalledAgentMigrationE2eTests public InstalledAgentMigrationE2eTests(ITestOutputHelper output) => this.output = output; + [Theory] + [InlineData("DevolutionsAgent.exe")] + [InlineData("devolutionsagent.exe")] + public void AgentExecutableNameComparisonIsCaseInsensitive(string fileName) + { + Assert.True(IsExpectedAgentExecutableName(fileName)); + } + [Fact] public void AgentJobTerminatesChildWhenScopeThrows() { @@ -67,7 +75,7 @@ public void ConvertedTransactionActivatesAndRetainsManagedAuthority() string agent = Environment.GetEnvironmentVariable("DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE"); Assert.False(string.IsNullOrWhiteSpace(agent), "The SYSTEM runner must supply the built Agent executable"); Assert.True(File.Exists(agent), agent); - Assert.Equal(Includes.EXECUTABLE_NAME, Path.GetFileName(agent), ignoreCase: true); + Assert.True(IsExpectedAgentExecutableName(Path.GetFileName(agent)), agent); using PackageBrokerPolicyActions.PinnedPath installedAgent = PackageBrokerPolicyActions.PinPathWithoutReparse( agent, leafIsDirectory: false, allowMissingLeaf: false, @@ -91,6 +99,7 @@ public void ConvertedTransactionActivatesAndRetainsManagedAuthority() PackageBrokerPolicyActions.CreateDirectoryWithSecurity( directory, Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); } + string source = Path.Combine(legacyDirectory, "package-broker-policy.json"); string destination = Path.Combine(managedDirectory, "package-broker-policy.json"); string marker = Path.Combine(managedDirectory, ".installer-e2e.migration"); @@ -205,6 +214,9 @@ private static JToken AssertActive(AgentProcess agent, string destination) return policy; } + private static bool IsExpectedAgentExecutableName(string fileName) => + string.Equals(Includes.EXECUTABLE_NAME, fileName, StringComparison.OrdinalIgnoreCase); + private sealed class AgentProcess : IDisposable { private readonly string executable; From 4b13e3f4e411d2cc61dd3f6e89470e04586bee22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 16 Sep 2026 03:23:43 +0900 Subject: [PATCH 10/14] fix(agent): authenticate installer E2E client Run migration lifecycle probes through the staged trusted test client instead of the hosted .NET test process, whose executable image cannot be retained by the package broker. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/run-as-system.ps1 | 3 ++ crates/agent-policy-tester/src/windows.rs | 26 +++++++++- .../InstalledAgentMigrationE2eTests.cs | 49 ++++++++++--------- 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/crates/agent-policy-tester/run-as-system.ps1 b/crates/agent-policy-tester/run-as-system.ps1 index be1938f88..00d872422 100644 --- a/crates/agent-policy-tester/run-as-system.ps1 +++ b/crates/agent-policy-tester/run-as-system.ps1 @@ -20,6 +20,7 @@ $stagedResultsPath = Join-Path $stagingPath "installer-test-results" $exitCode = 1 $previousTemp = $env:TEMP $previousTmp = $env:TMP +$previousInstallerTester = $env:AGENT_POLICY_TESTER_E2E_EXE try { Set-Content -LiteralPath $outputPath -Value "" @@ -107,6 +108,7 @@ public static class AgentPolicyTesterNativeDirectory Get-Acl -LiteralPath $stagedTesterPath | Format-List Owner, Sddl | Out-File $outputPath -Append & $stagedTesterPath $stagedAgentPath elevated 2>&1 | Out-File $outputPath -Append $exitCode = $LASTEXITCODE + $env:AGENT_POLICY_TESTER_E2E_EXE = $stagedTesterPath & (Join-Path $PSScriptRoot "run-installer-tests.ps1") ` -ProjectPath $installerProject -TestOutputPath $stagedInstallerOutput ` -AgentPath $stagedAgentPath -ResultsPath $stagedResultsPath -ArtifactResultsPath $resultsPath ` @@ -121,6 +123,7 @@ public static class AgentPolicyTesterNativeDirectory } finally { $env:TEMP = $previousTemp $env:TMP = $previousTmp + $env:AGENT_POLICY_TESTER_E2E_EXE = $previousInstallerTester for ($attempt = 0; $attempt -lt 20 -and (Test-Path -LiteralPath $stagingPath); $attempt++) { try { Remove-Item -LiteralPath $stagingPath -Recurse -Force diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index 1c136183e..c4bae6c78 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -194,6 +194,7 @@ enum Mode { StandardServer, StandardClient, Elevated, + Probe, } impl Mode { @@ -202,7 +203,8 @@ impl Mode { "standard-server" => Ok(Self::StandardServer), "standard-client" => Ok(Self::StandardClient), "elevated" => Ok(Self::Elevated), - _ => bail!("unknown mode '{value}'; expected 'standard-server', 'standard-client', or 'elevated'"), + "probe" => Ok(Self::Probe), + _ => bail!("unknown mode '{value}'; expected 'standard-server', 'standard-client', 'elevated', or 'probe'"), } } } @@ -247,6 +249,23 @@ pub(crate) async fn run() -> anyhow::Result<()> { managed_policy_lifecycle(&agent_path).await?; legacy_contract_and_interrupted_repair(&agent_path).await?; } + Mode::Probe => { + verify_local_system()?; + let pipe_name = next_string(&mut args, "pipe name")?; + let path = next_string(&mut args, "request path")?; + ensure!(args.next().is_none(), "unexpected probe arguments"); + probe(&pipe_name, &path).await?; + } + } + + async fn probe(pipe_name: &str, path: &str) -> anyhow::Result<()> { + let response = request(pipe_name, "GET", path).await?; + let body = response.json()?; + let mut stdout = std::io::stdout().lock(); + serde_json::to_writer(&mut stdout, &json!({ "Status": response.status, "Body": body }))?; + stdout.write_all(b"\n")?; + stdout.flush()?; + Ok(()) } Ok(()) @@ -1496,6 +1515,11 @@ mod tests { } } + #[test] + fn probe_mode_is_only_available_to_a_local_system_client() { + assert!(matches!(Mode::parse("probe"), Ok(Mode::Probe))); + } + #[test] fn standard_user_token_requires_medium_integrity() { validate_standard_user_token( diff --git a/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs b/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs index 1d6c0b2a8..0db6e859d 100644 --- a/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs +++ b/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs @@ -7,7 +7,6 @@ using System.ComponentModel; using System.Diagnostics; using System.IO; -using System.IO.Pipes; using System.Runtime.InteropServices; using System.Security.AccessControl; using System.Security.Principal; @@ -220,6 +219,7 @@ private static bool IsExpectedAgentExecutableName(string fileName) => private sealed class AgentProcess : IDisposable { private readonly string executable; + private readonly string tester; private readonly string root; private readonly ITestOutputHelper output; private readonly string pipeName = $"Devolutions.Now.PackageBroker.installer-e2e.{Guid.NewGuid():N}"; @@ -231,6 +231,9 @@ private sealed class AgentProcess : IDisposable internal AgentProcess(string executable, string root, ITestOutputHelper output) { this.executable = executable; + tester = Environment.GetEnvironmentVariable("AGENT_POLICY_TESTER_E2E_EXE"); + Assert.False(string.IsNullOrWhiteSpace(tester), "The SYSTEM runner must supply the policy tester executable"); + Assert.True(File.Exists(tester), tester); this.root = root; this.output = output; JObject config = new() @@ -288,30 +291,30 @@ internal void Start() internal JObject Get(string path, int expectedStatus) { - using NamedPipeClientStream pipe = new( - ".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous); - pipe.Connect(1000); - Task request = Exchange(pipe, path); - if (Task.WhenAny(request, Task.Delay(TimeSpan.FromSeconds(10))).GetAwaiter().GetResult() != request) + ProcessStartInfo start = new(tester, $"\"{tester}\" probe \"\\\\.\\pipe\\{pipeName}\" \"{path}\"") { - throw new TimeoutException($"timed out reading {path}"); + UseShellExecute = false, + CreateNoWindow = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + }; + using Process client = Process.Start(start); + Task stdout = client.StandardOutput.ReadToEndAsync(); + Task stderr = client.StandardError.ReadToEndAsync(); + if (!client.WaitForExit(10000)) + { + client.Kill(); + throw new TimeoutException($"timed out probing {path}"); } - string response = request.GetAwaiter().GetResult(); - int headerEnd = response.IndexOf("\r\n\r\n", StringComparison.Ordinal); - Assert.True(headerEnd > 0, response); - Assert.Equal(expectedStatus.ToString(), response.Split(' ')[1]); - return JObject.Parse(response.Substring(headerEnd + 4)); - } - - private static async Task Exchange(NamedPipeClientStream pipe, string path) - { - byte[] request = Encoding.ASCII.GetBytes( - $"GET {path} HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\nContent-Length: 0\r\n\r\n"); - await pipe.WriteAsync(request, 0, request.Length).ConfigureAwait(false); - await pipe.FlushAsync().ConfigureAwait(false); - using MemoryStream response = new(); - await pipe.CopyToAsync(response).ConfigureAwait(false); - return Encoding.UTF8.GetString(response.ToArray()); + string standardOutput = stdout.GetAwaiter().GetResult(); + string standardError = stderr.GetAwaiter().GetResult(); + if (client.ExitCode != 0) + { + throw new IOException($"policy tester probe failed for {path}: {standardError}"); + } + JObject response = JObject.Parse(standardOutput); + Assert.Equal(expectedStatus, (int)response["Status"]); + return response["Body"] as JObject; } internal void Stop() From 4300382963afa7186e2fdabf453dfafbeb1e9002 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 17 Sep 2026 09:39:08 +0900 Subject: [PATCH 11/14] test(agent): retain canonical policy E2E Remove installer migration and retired-policy compatibility coverage while preserving the canonical policy management lifecycle and its security boundaries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 27 +- crates/agent-policy-tester/run-as-system.ps1 | 42 +- .../run-installer-tests.ps1 | 87 ---- crates/agent-policy-tester/src/windows.rs | 211 ++------ .../InstalledAgentMigrationE2eTests.cs | 453 ------------------ 5 files changed, 57 insertions(+), 763 deletions(-) delete mode 100644 crates/agent-policy-tester/run-installer-tests.ps1 delete mode 100644 package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b11ead0cc..87550f9d6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1392,11 +1392,6 @@ jobs: exit $LASTEXITCODE } - - name: Build installer tests before entering LocalSystem - id: build-installer-tests - shell: pwsh - run: dotnet build package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj --configuration Debug --framework net48 - - name: Run Agent policy tester as standard user shell: pwsh run: | @@ -1408,35 +1403,17 @@ jobs: } - name: Run Agent policy tester as LocalSystem - if: ${{ !cancelled() && steps.build-policy-executables.outcome == 'success' && steps.build-installer-tests.outcome == 'success' }} + if: ${{ !cancelled() && steps.build-policy-executables.outcome == 'success' }} shell: pwsh run: | $scriptPath = Resolve-Path -Path "./crates/agent-policy-tester/run-as-system.ps1" - $dotnetPath = (Get-Command dotnet.exe).Source - $nuGetPackagesPath = ( - & $dotnetPath nuget locals global-packages --list | - Select-String '^global-packages:\s*(.+)$' - ).Matches[0].Groups[1].Value.Trim() - if ([string]::IsNullOrWhiteSpace($nuGetPackagesPath) -or -not (Test-Path -LiteralPath $nuGetPackagesPath)) { - throw "Could not resolve the restored NuGet package root for LocalSystem" - } - psexec -accepteula -s pwsh.exe -NoProfile -File $scriptPath ` - -DotnetPath $dotnetPath -NuGetPackagesPath $nuGetPackagesPath + psexec -accepteula -s pwsh.exe -NoProfile -File $scriptPath $exitCode = $LASTEXITCODE Get-Content -Path ./crates/agent-policy-tester/agent-policy-tester.out if ($exitCode -ne 0) { exit $exitCode } - - name: Upload SYSTEM installer test results - if: ${{ always() }} - uses: actions/upload-artifact@v7 - with: - name: agent-policy-installer-system-results - path: | - crates/agent-policy-tester/installer-test-results/*.trx - crates/agent-policy-tester/agent-policy-tester.out - - name: Run policy route authorization tests shell: pwsh run: cargo test --locked -p now-package-broker --features dev-skip-broker-signature diff --git a/crates/agent-policy-tester/run-as-system.ps1 b/crates/agent-policy-tester/run-as-system.ps1 index 00d872422..f26337f00 100644 --- a/crates/agent-policy-tester/run-as-system.ps1 +++ b/crates/agent-policy-tester/run-as-system.ps1 @@ -1,8 +1,3 @@ -param( - [string] $DotnetPath = (Join-Path $env:ProgramFiles "dotnet\dotnet.exe"), - [Parameter(Mandatory)] [string] $NuGetPackagesPath -) - $ErrorActionPreference = "Stop" $workspacePath = (Resolve-Path (Join-Path $PSScriptRoot "../..")).Path @@ -11,16 +6,7 @@ $agentPath = Join-Path $workspacePath "target/debug/devolutions-agent.exe" $outputPath = Join-Path $PSScriptRoot "agent-policy-tester.out" $stagingPath = Join-Path ([Environment]::GetFolderPath('CommonApplicationData')) "dgw-agent-policy-tester-$([guid]::NewGuid().ToString('N'))" $stagedTesterPath = Join-Path $stagingPath "agent-policy-tester.exe" -$stagedAgentPath = Join-Path $stagingPath "DevolutionsAgent.exe" -$installerProject = Join-Path $workspacePath "package\AgentWindowsManaged.Tests\DevolutionsAgent.Installer.Tests.csproj" -$installerOutput = Join-Path $workspacePath "package\AgentWindowsManaged.Tests\bin\Debug\net48" -$stagedInstallerOutput = Join-Path $stagingPath "installer-tests" -$resultsPath = Join-Path $PSScriptRoot "installer-test-results" -$stagedResultsPath = Join-Path $stagingPath "installer-test-results" $exitCode = 1 -$previousTemp = $env:TEMP -$previousTmp = $env:TMP -$previousInstallerTester = $env:AGENT_POLICY_TESTER_E2E_EXE try { Set-Content -LiteralPath $outputPath -Value "" @@ -30,7 +16,6 @@ try { if ([System.IO.DriveInfo]::new([System.IO.Path]::GetPathRoot($workspacePath)).DriveType -ne 'Fixed') { throw "Use a local fixed-volume workspace path visible to LocalSystem, not a mapped drive" } - $NuGetPackagesPath = (Resolve-Path -LiteralPath $NuGetPackagesPath).Path Add-Type -TypeDefinition @' using System; using System.ComponentModel; @@ -83,19 +68,10 @@ public static class AgentPolicyTesterNativeDirectory if (Get-ChildItem -LiteralPath $stagingPath -Force) { throw "The atomically protected staged tester directory was not empty" } - if (Test-Path -LiteralPath $resultsPath) { - Remove-Item -LiteralPath $resultsPath -Recurse -Force - } - $env:TEMP = Join-Path $stagingPath "scratch" - $env:TMP = $env:TEMP - New-Item -ItemType Directory -Path $env:TEMP | Out-Null - Copy-Item -LiteralPath $testerPath -Destination $stagedTesterPath - Copy-Item -LiteralPath $agentPath -Destination $stagedAgentPath - Copy-Item -LiteralPath $installerOutput -Destination $stagedInstallerOutput -Recurse - & icacls.exe $stagingPath /setowner '*S-1-5-18' /T /Q 2>&1 | Out-File $outputPath -Append + & icacls.exe $stagedTesterPath /setowner '*S-1-5-18' 2>&1 | Out-File $outputPath -Append if ($LASTEXITCODE -ne 0) { - throw "Failed to set the staged executable and installer test owners" + throw "Failed to set the staged tester owner" } & icacls.exe $stagedTesterPath /inheritance:r /grant:r '*S-1-5-18:(F)' '*S-1-5-32-544:(F)' 2>&1 | Out-File $outputPath -Append @@ -106,24 +82,12 @@ public static class AgentPolicyTesterNativeDirectory "Staged policy tester at $stagedTesterPath" | Out-File $outputPath -Append Get-Acl -LiteralPath $stagingPath | Format-List Owner, Sddl | Out-File $outputPath -Append Get-Acl -LiteralPath $stagedTesterPath | Format-List Owner, Sddl | Out-File $outputPath -Append - & $stagedTesterPath $stagedAgentPath elevated 2>&1 | Out-File $outputPath -Append + & $stagedTesterPath $agentPath elevated 2>&1 | Out-File $outputPath -Append $exitCode = $LASTEXITCODE - $env:AGENT_POLICY_TESTER_E2E_EXE = $stagedTesterPath - & (Join-Path $PSScriptRoot "run-installer-tests.ps1") ` - -ProjectPath $installerProject -TestOutputPath $stagedInstallerOutput ` - -AgentPath $stagedAgentPath -ResultsPath $stagedResultsPath -ArtifactResultsPath $resultsPath ` - -DotnetPath $DotnetPath -NuGetPackagesPath $NuGetPackagesPath ` - 2>&1 | Out-File $outputPath -Append - if ($LASTEXITCODE -ne 0) { - $exitCode = $LASTEXITCODE - } } catch { $_ | Out-File $outputPath -Append $exitCode = 1 } finally { - $env:TEMP = $previousTemp - $env:TMP = $previousTmp - $env:AGENT_POLICY_TESTER_E2E_EXE = $previousInstallerTester for ($attempt = 0; $attempt -lt 20 -and (Test-Path -LiteralPath $stagingPath); $attempt++) { try { Remove-Item -LiteralPath $stagingPath -Recurse -Force diff --git a/crates/agent-policy-tester/run-installer-tests.ps1 b/crates/agent-policy-tester/run-installer-tests.ps1 deleted file mode 100644 index 1e96f8090..000000000 --- a/crates/agent-policy-tester/run-installer-tests.ps1 +++ /dev/null @@ -1,87 +0,0 @@ -param( - [Parameter(Mandatory)] [string] $ProjectPath, - [Parameter(Mandatory)] [string] $TestOutputPath, - [Parameter(Mandatory)] [string] $AgentPath, - [Parameter(Mandatory)] [string] $ResultsPath, - [Parameter(Mandatory)] [string] $ArtifactResultsPath, - [Parameter(Mandatory)] [string] $DotnetPath, - [Parameter(Mandatory)] [string] $NuGetPackagesPath -) - -$ErrorActionPreference = "Stop" -$exitCode = 1 -$previousAgent = $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE -$previousNuGetPackages = $env:NUGET_PACKAGES - -try { - if (-not [System.Security.Principal.WindowsIdentity]::GetCurrent().IsSystem) { - throw "Installer transaction tests must execute as LocalSystem" - } - $nuGetPackageRoot = (Resolve-Path -LiteralPath $NuGetPackagesPath).Path - $separator = [System.IO.Path]::DirectorySeparatorChar.ToString() - if (-not $nuGetPackageRoot.EndsWith($separator, [System.StringComparison]::Ordinal)) { - $nuGetPackageRoot += $separator - } - $env:NUGET_PACKAGES = $nuGetPackageRoot - $testClass = 'DevolutionsAgent.Installer.Tests.PackageBrokerInstallerTests' - $expectedTests = @( - "$testClass.TransactionTestsRunAsLocalSystem" - "$testClass.InstalledAgentConverterUsesAuthoritativeContractBeforePublication" - "$testClass.ConvertedMigrationCommitAndRepeatPreserveOriginalAndEvidence" - "$testClass.RollbackRestoresLegacyArbitrationAndMigrationCanRepeat" - "$testClass.InterruptedPublicationRecoversOnlyOwnedAuthority" - "$testClass.InvalidLegacyPolicyFailsUpgradeAndPreservesSource" - "$testClass.ExistingNewDestinationAndPublicationCollisionArePreserved" - "$testClass.ChangedSourcePreventsDestructiveRollback" - "$testClass.PreexistingAuthorityPreventsLegacyResurrection" - "$testClass.AuthorityCollisionCannotCommitSourceDeletion" - "$testClass.DestinationCollisionCannotCommitSourceDeletion" - "$testClass.UnchangedInputRollbackPreservesLastSurvivingPolicy" - 'DevolutionsAgent.Installer.Tests.InstalledAgentMigrationE2eTests.ConvertedTransactionActivatesAndRetainsManagedAuthority' - ) - $filter = ($expectedTests | ForEach-Object { "FullyQualifiedName=$_" }) -join '|' - New-Item -ItemType Directory -Path $ResultsPath -Force | Out-Null - $trxPath = Join-Path $ResultsPath "installer-system.trx" - if (Test-Path -LiteralPath $trxPath) { - Remove-Item -LiteralPath $trxPath -Force - } - $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE = (Resolve-Path -LiteralPath $AgentPath).Path - & $DotnetPath test $ProjectPath --no-build --no-restore --configuration Debug --framework net48 ` - "-p:OutputPath=$TestOutputPath\" -p:AppendTargetFrameworkToOutputPath=false ` - "-p:NuGetPackageRoot=$env:NUGET_PACKAGES" ` - --filter $filter --logger "trx;LogFileName=installer-system.trx" --results-directory $ResultsPath - $exitCode = $LASTEXITCODE - if ($exitCode -ne 0) { - throw "SYSTEM installer tests exited with $exitCode" - } - [xml] $trx = Get-Content -LiteralPath $trxPath -Raw - $counters = $trx.TestRun.ResultSummary.Counters - $results = @($trx.TestRun.Results.UnitTestResult) - if ($trx.TestRun.ResultSummary.outcome -ne 'Completed' -or - [int] $counters.total -ne $expectedTests.Count -or - [int] $counters.executed -ne $expectedTests.Count -or - [int] $counters.passed -ne $expectedTests.Count -or - [int] $counters.notExecuted -ne 0 -or - $results.Count -ne $expectedTests.Count) { - throw "Expected exactly $($expectedTests.Count) executed, passed SYSTEM installer tests and zero skips: $($counters.OuterXml)" - } - foreach ($name in $expectedTests) { - $matching = @($results | Where-Object { $_.testName -eq $name }) - if ($matching.Count -ne 1 -or $matching[0].outcome -ne 'Passed') { - throw "Required SYSTEM installer test did not pass exactly once: $name" - } - } - New-Item -ItemType Directory -Path $ArtifactResultsPath -Force | Out-Null - Copy-Item -LiteralPath $trxPath -Destination $ArtifactResultsPath -Force - Write-Output "Verified all 12 installer SystemFacts and the installed-Agent migration E2E: 13 passed, zero skipped" -} catch { - Write-Output $_ - if ($exitCode -eq 0) { - $exitCode = 1 - } -} finally { - $env:DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE = $previousAgent - $env:NUGET_PACKAGES = $previousNuGetPackages -} - -exit $exitCode diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index c4bae6c78..d5284fb76 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -20,8 +20,6 @@ use windows::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken, PRO const FULL_POLICY: &str = include_str!("../../now-package-broker/src/assets/samples/corporate-allowlist.policy.json"); const MANAGED_POLICY_RELATIVE_PATH: &str = r"Devolutions\PackageBroker\package-broker-policy.json"; -const MANAGED_AUTHORITY_MARKER: &str = r"Devolutions\PackageBroker\.package-broker-managed-authority.v1"; -const LEGACY_POLICY_RELATIVE_PATH: &str = r"Devolutions\Agent\package-broker-policy.json"; #[cfg(test)] const SECURITY_MANDATORY_LOW_RID: u32 = 0x1000; const SECURITY_MANDATORY_MEDIUM_RID: u32 = 0x2000; @@ -194,7 +192,6 @@ enum Mode { StandardServer, StandardClient, Elevated, - Probe, } impl Mode { @@ -203,8 +200,7 @@ impl Mode { "standard-server" => Ok(Self::StandardServer), "standard-client" => Ok(Self::StandardClient), "elevated" => Ok(Self::Elevated), - "probe" => Ok(Self::Probe), - _ => bail!("unknown mode '{value}'; expected 'standard-server', 'standard-client', 'elevated', or 'probe'"), + _ => bail!("unknown mode '{value}'; expected 'standard-server', 'standard-client', or 'elevated'"), } } } @@ -247,25 +243,8 @@ pub(crate) async fn run() -> anyhow::Result<()> { redirected_policy_paths_fail_closed(&agent_path).await?; management_write_tokens_survive_watcher_reload(&agent_path).await?; managed_policy_lifecycle(&agent_path).await?; - legacy_contract_and_interrupted_repair(&agent_path).await?; + interrupted_malformed_repair(&agent_path).await?; } - Mode::Probe => { - verify_local_system()?; - let pipe_name = next_string(&mut args, "pipe name")?; - let path = next_string(&mut args, "request path")?; - ensure!(args.next().is_none(), "unexpected probe arguments"); - probe(&pipe_name, &path).await?; - } - } - - async fn probe(pipe_name: &str, path: &str) -> anyhow::Result<()> { - let response = request(pipe_name, "GET", path).await?; - let body = response.json()?; - let mut stdout = std::io::stdout().lock(); - serde_json::to_writer(&mut stdout, &json!({ "Status": response.status, "Body": body }))?; - stdout.write_all(b"\n")?; - stdout.flush()?; - Ok(()) } Ok(()) @@ -687,10 +666,8 @@ async fn validate_policy_by_pipe(pipe_name: &str, draft: &Value) -> anyhow::Resu "unexpected validator contract" ); ensure!( - validation["CanonicalDraft"].get("$schema").is_none() - && validation["CanonicalDraft"].get("PolicyVersion").is_none() - && validation["CanonicalDraft"]["PolicyFormatVersion"] == draft["PolicyFormatVersion"], - "canonical draft changed the format version or emitted legacy fields" + validation["CanonicalDraft"]["PolicyFormatVersion"] == draft["PolicyFormatVersion"], + "canonical draft changed the format version" ); Ok(validation) } @@ -936,28 +913,12 @@ async fn strict_contract_validation(pipe_name: &str) -> anyhow::Result<()> { draft["PolicyFormatVersion"] = json!(version); validate_policy_by_pipe(pipe_name, &draft).await?; } - for (field, value, expected_path) in [ - ( - "$schema", - "https://devolutions.net/schemas/now-policy.schema.1.0.json", - "/$schema", - ), - ("PolicyVersion", "1.0.0", "/PolicyVersion"), - ("PolicyFormatVersion", "2.0.0", "/PolicyFormatVersion"), - ("PolicyFormatVersion", "broken", "/PolicyFormatVersion"), - ] { + for (value, expected_path) in [("2.0.0", "/PolicyFormatVersion"), ("broken", "/PolicyFormatVersion")] { let mut draft = policy_draft("tests.contract", "Contract"); - draft[field] = json!(value); + draft["PolicyFormatVersion"] = json!(value); assert_invalid_draft(pipe_name, draft, expected_path).await?; } - let mut legacy = policy_draft("tests.contract", "Contract"); - legacy - .as_object_mut() - .context("draft is not an object")? - .remove("PolicyFormatVersion"); - legacy["PolicyVersion"] = json!("1.0.0"); - legacy["$schema"] = json!("https://devolutions.net/schemas/now-policy.schema.1.0.json"); - assert_invalid_draft(pipe_name, legacy, "/PolicyVersion").await + Ok(()) } async fn assert_invalid_draft(pipe_name: &str, draft: Value, expected_path: &str) -> anyhow::Result<()> { @@ -1022,11 +983,6 @@ async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { created["Policy"]["Metadata"]["Revision"] == 1, "Create did not assign revision 1" ); - let authority_marker = agent.data_dir.path().join(MANAGED_AUTHORITY_MARKER); - ensure!( - authority_marker.is_file() && std::fs::metadata(&authority_marker)?.len() == 0, - "Create did not establish durable managed authority" - ); wait_for_log(&agent, "Policy creation succeeded").await?; let updated = replace_policy( @@ -1143,30 +1099,6 @@ async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { restarted.json()?["Policy"] == confirmed["Policy"], "restart changed the active managed policy" ); - ensure!( - authority_marker.is_file() && policy_management(&agent).await?["Source"] == "DefaultPath", - "restart lost durable managed authority" - ); - - agent.stop().await?; - let legacy_path = agent.data_dir.path().join(LEGACY_POLICY_RELATIVE_PATH); - let legacy_dir = legacy_path.parent().context("legacy policy path has no parent")?; - std::fs::create_dir_all(legacy_dir).context("create isolated legacy policy directory")?; - secure_policy_path(legacy_dir, true)?; - std::fs::write(&legacy_path, serde_json::to_vec_pretty(&empty_policy())?) - .context("write isolated legacy policy")?; - secure_policy_path(&legacy_path, false)?; - std::fs::remove_file(&agent.policy_path).context("remove managed policy before authority restart")?; - agent.start_again(agent_path).await?; - let authority = policy_management(&agent).await?; - ensure!( - authority["State"] == "Missing" && authority["Source"] == "DefaultPath", - "durable managed authority allowed legacy policy rollback" - ); - ensure!( - request(&agent.pipe_name, "GET", "/v1/policy").await?.status == 404, - "legacy policy became active after managed authority was established" - ); Ok(()) } @@ -1175,7 +1107,7 @@ async fn warnings_identity_and_receipts( agent_path: &Path, current: &Value, ) -> anyhow::Result { - let mut draft = policy_draft("tests.replaced-identity", "Compatible contract"); + let mut draft = policy_draft("tests.replaced-identity", "Canonical contract"); draft["PolicyFormatVersion"] = json!("1.7.3"); draft["Enforcement"]["AuditMode"] = json!(true); let validation = validate_policy_by_pipe(&agent.pipe_name, &draft).await?; @@ -1225,10 +1157,8 @@ async fn warnings_identity_and_receipts( ensure!( replaced["Policy"]["Metadata"]["Id"] == "tests.replaced-identity" && replaced["Policy"]["Metadata"]["Revision"] == 1 - && replaced["Policy"]["PolicyFormatVersion"] == "1.7.3" - && replaced["Policy"].get("$schema").is_none() - && replaced["Policy"].get("PolicyVersion").is_none(), - "ReplaceIdentity did not preserve the compatible contract and reset revision" + && replaced["Policy"]["PolicyFormatVersion"] == "1.7.3", + "ReplaceIdentity did not preserve the canonical contract and reset revision" ); wait_for_log(agent, "Policy change succeeded").await?; wait_for_log(agent, "replace_identity").await?; @@ -1237,79 +1167,49 @@ async fn warnings_identity_and_receipts( Ok(replaced) } -async fn legacy_contract_and_interrupted_repair(agent_path: &Path) -> anyhow::Result<()> { - let mut legacy = empty_policy(); - legacy - .as_object_mut() - .context("policy is not an object")? - .remove("PolicyFormatVersion"); - legacy["$schema"] = json!("https://devolutions.net/schemas/now-policy.schema.1.0.json"); - legacy["PolicyVersion"] = json!("1.0.0"); - let mut mixed = legacy.clone(); - mixed["PolicyFormatVersion"] = json!("1.0.0"); - for original in [ - serde_json::to_vec(&legacy)?, - serde_json::to_vec(&mixed)?, - b"malformed-policy-secret-marker".to_vec(), - ] { - for marker_staging in [false, true] { - let data_dir = create_data_dir()?; - let policy_path = data_dir.path().join("policy.json"); - std::fs::write(&policy_path, &original)?; - secure_policy_path(&policy_path, false)?; - let prefix = ".policy.json.txn-11111111-2222-4333-8444-555555555555"; - let new_path = data_dir.path().join(format!("{prefix}.new")); - std::fs::write(&new_path, b"partial replacement")?; - secure_policy_path(&new_path, false)?; - let marker_path = data_dir.path().join(format!("{prefix}.marker.prepare")); - if marker_staging { - std::fs::write(&marker_path, br#"{"Version":"#)?; - secure_policy_path(&marker_path, false)?; - } - let mut agent = - AgentHarness::start_with_path(agent_path, data_dir, unique_pipe_name(), policy_path).await?; - let mut invalid = policy_management(&agent).await?; - ensure!( - invalid["State"] == "Invalid" && invalid["WriteCapability"] == "Writable", - "interrupted Repair did not retain a repairable invalid original: {invalid}" - ); - ensure!( - std::fs::read(&agent.policy_path)? == original && !new_path.exists() && !marker_path.exists(), - "recovery changed the original or retained prepublication remnants" - ); - ensure!( - request(&agent.pipe_name, "GET", "/v1/policy").await?.status == 404, - "legacy, mixed, or malformed policy became active" - ); - if original.starts_with(b"{") { - ensure!( - invalid["InvalidDiagnostics"]["Findings"] - .as_array() - .is_some_and(|findings| findings - .iter() - .any(|finding| finding["Code"] == "UnsupportedPolicyFormatVersion")), - "legacy contract did not produce its strict diagnostic" - ); - std::fs::write(&agent.policy_path, serde_json::to_vec(&empty_policy())?)?; - wait_for_management(&agent, |management| management["State"] == "Active").await?; - std::fs::write(&agent.policy_path, &original)?; - invalid = wait_for_management(&agent, |management| management["State"] == "Invalid").await?; - wait_for_log(&agent, "legacy_policy_contract").await?; - } - let repaired = replace_policy( - &agent, - "Repair", - invalid["StoreToken"].clone(), - policy_draft("tests.interrupted-repair", "Recovered"), - ) - .await?; - agent.restart(agent_path).await?; - ensure!( - policy_management(&agent).await?["Policy"] == repaired["Policy"], - "repaired policy did not survive restart" - ); - agent.stop().await?; +async fn interrupted_malformed_repair(agent_path: &Path) -> anyhow::Result<()> { + let original = b"malformed-policy-secret-marker"; + for marker_staging in [false, true] { + let data_dir = create_data_dir()?; + let policy_path = data_dir.path().join("policy.json"); + std::fs::write(&policy_path, original)?; + secure_policy_path(&policy_path, false)?; + let prefix = ".policy.json.txn-11111111-2222-4333-8444-555555555555"; + let new_path = data_dir.path().join(format!("{prefix}.new")); + std::fs::write(&new_path, b"partial replacement")?; + secure_policy_path(&new_path, false)?; + let marker_path = data_dir.path().join(format!("{prefix}.marker.prepare")); + if marker_staging { + std::fs::write(&marker_path, br#"{"Version":"#)?; + secure_policy_path(&marker_path, false)?; } + let mut agent = AgentHarness::start_with_path(agent_path, data_dir, unique_pipe_name(), policy_path).await?; + let invalid = policy_management(&agent).await?; + ensure!( + invalid["State"] == "Invalid" && invalid["WriteCapability"] == "Writable", + "interrupted Repair did not retain a repairable invalid original: {invalid}" + ); + ensure!( + std::fs::read(&agent.policy_path)? == original && !new_path.exists() && !marker_path.exists(), + "recovery changed the original or retained prepublication remnants" + ); + ensure!( + request(&agent.pipe_name, "GET", "/v1/policy").await?.status == 404, + "malformed policy became active" + ); + let repaired = replace_policy( + &agent, + "Repair", + invalid["StoreToken"].clone(), + policy_draft("tests.interrupted-repair", "Recovered"), + ) + .await?; + agent.restart(agent_path).await?; + ensure!( + policy_management(&agent).await?["Policy"] == repaired["Policy"], + "repaired policy did not survive restart" + ); + agent.stop().await?; } Ok(()) } @@ -1499,8 +1399,6 @@ mod tests { fn policy_fixtures_use_the_current_contract() { for policy in [full_policy(), empty_policy(), policy_draft("tests.contract", "Test")] { assert_eq!(policy["PolicyFormatVersion"], "1.0.0"); - assert!(policy.get("$schema").is_none()); - assert!(policy.get("PolicyVersion").is_none()); } } @@ -1515,11 +1413,6 @@ mod tests { } } - #[test] - fn probe_mode_is_only_available_to_a_local_system_client() { - assert!(matches!(Mode::parse("probe"), Ok(Mode::Probe))); - } - #[test] fn standard_user_token_requires_medium_integrity() { validate_standard_user_token( diff --git a/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs b/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs deleted file mode 100644 index 0db6e859d..000000000 --- a/package/AgentWindowsManaged.Tests/InstalledAgentMigrationE2eTests.cs +++ /dev/null @@ -1,453 +0,0 @@ -using DevolutionsAgent.Actions; -using DevolutionsAgent.Resources; -using Microsoft.Deployment.WindowsInstaller; -using Microsoft.Win32.SafeHandles; -using Newtonsoft.Json.Linq; -using System; -using System.ComponentModel; -using System.Diagnostics; -using System.IO; -using System.Runtime.InteropServices; -using System.Security.AccessControl; -using System.Security.Principal; -using System.Text; -using System.Threading; -using System.Threading.Tasks; -using Xunit; -using Xunit.Abstractions; - -namespace DevolutionsAgent.Installer.Tests; - -public sealed class InstalledAgentMigrationE2eTests -{ - private const string CurrentPolicy = - """{"PolicyFormatVersion":"1.7.3","PolicyType":"PackageBrokerPolicy","Metadata":{"Id":"installer-e2e","Publisher":"Test","Revision":17,"PublishedAt":"2026-01-01T00:00:00Z"},"Enforcement":{"DefaultDecision":"Deny","RulePrecedence":"PriorityThenDeny"},"Rules":[]}"""; - private readonly ITestOutputHelper output; - - public InstalledAgentMigrationE2eTests(ITestOutputHelper output) => this.output = output; - - [Theory] - [InlineData("DevolutionsAgent.exe")] - [InlineData("devolutionsagent.exe")] - public void AgentExecutableNameComparisonIsCaseInsensitive(string fileName) - { - Assert.True(IsExpectedAgentExecutableName(fileName)); - } - - [Fact] - public void AgentJobTerminatesChildWhenScopeThrows() - { - using Process child = Process.Start(new ProcessStartInfo( - Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), - @"WindowsPowerShell\v1.0\powershell.exe"), - "-NoProfile -NonInteractive -Command Start-Sleep -Seconds 60") - { - UseShellExecute = false, - CreateNoWindow = true, - }); - try - { - InvalidOperationException failure = new("simulate a failed Agent assertion"); - void FailWithAssignedChild() - { - using AgentJob job = new(); - job.Assign(child); - throw failure; - } - Assert.Same(failure, Assert.Throws(FailWithAssignedChild)); - Assert.True(child.WaitForExit(10000), "Job disposal left the child running"); - } - finally - { - if (!child.HasExited) - { - child.Kill(); - Assert.True(child.WaitForExit(10000), "Cleanup did not stop the child"); - } - } - } - - [PackageBrokerInstallerTests.SystemFact] - public void ConvertedTransactionActivatesAndRetainsManagedAuthority() - { - Assert.True(WindowsIdentity.GetCurrent().IsSystem); - string agent = Environment.GetEnvironmentVariable("DEVOLUTIONS_AGENT_MIGRATION_TEST_EXE"); - Assert.False(string.IsNullOrWhiteSpace(agent), "The SYSTEM runner must supply the built Agent executable"); - Assert.True(File.Exists(agent), agent); - Assert.True(IsExpectedAgentExecutableName(Path.GetFileName(agent)), agent); - using PackageBrokerPolicyActions.PinnedPath installedAgent = - PackageBrokerPolicyActions.PinPathWithoutReparse( - agent, leafIsDirectory: false, allowMissingLeaf: false, - leafAccess: WinAPI.GENERIC_READ | WinAPI.READ_CONTROL, verifyTrustedAncestors: true); - Assert.True( - PackageBrokerPolicyActions.TryVerifyLegacyPolicySourceSecurity( - File.GetAccessControl(agent), out string agentSecurityDiagnostic), - agentSecurityDiagnostic); - string root = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), - $"dgw-installer-e2e-{Guid.NewGuid():N}"); - PackageBrokerPolicyActions.CreateDirectoryWithSecurity(root, Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); - try - { - PackageBrokerPolicyActions.VerifyPackageBrokerSecurity(Directory.GetAccessControl(root)); - string vendor = Path.Combine(root, "Devolutions"); - string legacyDirectory = Path.Combine(vendor, "Agent"); - string managedDirectory = Path.Combine(vendor, "PackageBroker"); - foreach (string directory in new[] { vendor, legacyDirectory, managedDirectory }) - { - PackageBrokerPolicyActions.CreateDirectoryWithSecurity( - directory, Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); - } - - string source = Path.Combine(legacyDirectory, "package-broker-policy.json"); - string destination = Path.Combine(managedDirectory, "package-broker-policy.json"); - string marker = Path.Combine(managedDirectory, ".installer-e2e.migration"); - string authority = Path.Combine(managedDirectory, ".package-broker-managed-authority.v1"); - byte[] original = Encoding.UTF8.GetBytes(CurrentPolicy.Replace( - "\"PolicyFormatVersion\":", - "\"$schema\":\"https://devolutions.net/schemas/now-policy.schema.1.0.json\",\"PolicyVersion\":")); - File.WriteAllBytes(source, original); - FileSecurity security = new(); - security.SetSecurityDescriptorSddlForm(Includes.PROGRAM_DATA_PACKAGE_BROKER_FILE_SDDL); - File.SetAccessControl(source, security); - - int conversions = 0; - void Migrate() - { - Assert.Equal(ActionResult.Success, PackageBrokerPolicyActions.MigrateLegacyPolicy( - output.WriteLine, source, destination, marker, input => - { - Assert.False(File.Exists(destination)); - Assert.False(File.Exists(authority)); - Assert.Equal(original, input); - byte[] converted = PackageBrokerPolicyActions.ConvertWithInstalledAgent( - Path.GetDirectoryName(agent), input); - Assert.Equal(CurrentPolicy, Encoding.UTF8.GetString(converted)); - conversions++; - return converted; - })); - Assert.Equal(CurrentPolicy, File.ReadAllText(destination)); - Assert.True(File.Exists(authority)); - Assert.Empty(File.ReadAllBytes(authority)); - AssertPreserved(); - foreach (string path in new[] { destination, authority, marker, marker + ".original" }) - { - PackageBrokerPolicyActions.VerifyPackageBrokerSecurity(File.GetAccessControl(path)); - } - } - void AssertPreserved() - { - Assert.Equal(original, File.ReadAllBytes(source)); - Assert.Equal(original, File.ReadAllBytes(marker + ".original")); - Assert.True(File.Exists(marker)); - } - - Migrate(); - Assert.Equal(ActionResult.Success, PackageBrokerPolicyActions.RollbackLegacyPolicy( - output.WriteLine, source, destination, marker)); - Assert.False(File.Exists(destination)); - Assert.False(File.Exists(authority)); - AssertPreserved(); - Migrate(); - Assert.Equal(2, conversions); - PackageBrokerPolicyActions.CommitLegacyPolicy(output.WriteLine, source, destination, marker); - AssertPreserved(); - - using AgentProcess running = new(agent, root, output); - running.Start(); - JToken active = AssertActive(running, destination); - running.Stop(); - running.Start(); - Assert.True(JToken.DeepEquals(active, AssertActive(running, destination))); - AssertPreserved(); - running.Stop(); - File.Delete(destination); - running.Start(); - JObject management = running.Get("/v1/policy/management", 200)["Management"] as JObject; - Assert.NotNull(management); - Assert.Equal("DefaultPath", (string)management["Source"]); - Assert.Equal("Missing", (string)management["State"]); - Assert.Equal("active policy is unavailable", (string)running.Get("/v1/policy", 404)["Message"]); - Assert.True(File.Exists(authority)); - AssertPreserved(); - } - finally - { - for (int attempt = 0; ; attempt++) - { - try - { - Directory.Delete(root, recursive: true); - break; - } - catch (IOException) when (attempt < 19) - { - Thread.Sleep(250); - } - } - } - } - - private static JToken AssertActive(AgentProcess agent, string destination) - { - JObject response = agent.Get("/v1/policy", 200); - JToken policy = response["Policy"]; - Assert.NotNull(policy); - Assert.Equal("1.7.3", (string)policy["PolicyFormatVersion"]); - Assert.Equal("PackageBrokerPolicy", (string)policy["PolicyType"]); - Assert.Null(policy["PolicyVersion"]); - Assert.Null(policy["$schema"]); - Assert.Equal("installer-e2e", (string)policy["Metadata"]["Id"]); - Assert.Equal("Test", (string)policy["Metadata"]["Publisher"]); - Assert.Equal(17, (int)policy["Metadata"]["Revision"]); - Assert.Equal( - JObject.Parse(CurrentPolicy)["Metadata"]["PublishedAt"], - policy["Metadata"]["PublishedAt"]); - Assert.True(JToken.DeepEquals(JObject.Parse(CurrentPolicy)["Enforcement"], policy["Enforcement"])); - Assert.True(JToken.DeepEquals(new JArray(), policy["Rules"])); - Assert.Equal(CurrentPolicy, File.ReadAllText(destination)); - JObject management = agent.Get("/v1/policy/management", 200)["Management"] as JObject; - Assert.NotNull(management); - Assert.Equal("DefaultPath", (string)management["Source"]); - Assert.Equal("Active", (string)management["State"]); - return policy; - } - - private static bool IsExpectedAgentExecutableName(string fileName) => - string.Equals(Includes.EXECUTABLE_NAME, fileName, StringComparison.OrdinalIgnoreCase); - - private sealed class AgentProcess : IDisposable - { - private readonly string executable; - private readonly string tester; - private readonly string root; - private readonly ITestOutputHelper output; - private readonly string pipeName = $"Devolutions.Now.PackageBroker.installer-e2e.{Guid.NewGuid():N}"; - private readonly AgentJob job; - private Process process; - private Task stdout; - private Task stderr; - - internal AgentProcess(string executable, string root, ITestOutputHelper output) - { - this.executable = executable; - tester = Environment.GetEnvironmentVariable("AGENT_POLICY_TESTER_E2E_EXE"); - Assert.False(string.IsNullOrWhiteSpace(tester), "The SYSTEM runner must supply the policy tester executable"); - Assert.True(File.Exists(tester), tester); - this.root = root; - this.output = output; - JObject config = new() - { - ["LogFile"] = Path.Combine(root, "agent-installer-e2e"), - ["PackageBroker"] = new JObject - { - ["Enabled"] = true, - ["PipeName"] = @"\\.\pipe\" + pipeName, - }, - ["__debug__"] = new JObject { ["skip_broker_signature_validation"] = true }, - }; - File.WriteAllText(Path.Combine(root, "agent.json"), config.ToString()); - job = new AgentJob(); - } - - internal void Start() - { - Assert.Null(process); - stdout = null; - stderr = null; - ProcessStartInfo start = new(executable, "run") - { - UseShellExecute = false, - CreateNoWindow = true, - WorkingDirectory = root, - RedirectStandardOutput = true, - RedirectStandardError = true, - }; - start.EnvironmentVariables["DAGENT_CONFIG_PATH"] = root; - start.EnvironmentVariables["ProgramData"] = root; - process = Process.Start(start); - job.Assign(process); - stdout = process.StandardOutput.ReadToEndAsync(); - stderr = process.StandardError.ReadToEndAsync(); - Stopwatch timer = Stopwatch.StartNew(); - while (true) - { - Assert.False(process.HasExited, "Agent exited before its broker became ready"); - try - { - Get("/v1/health", 200); - return; - } - catch (TimeoutException) when (timer.Elapsed < TimeSpan.FromSeconds(20)) - { - Thread.Sleep(50); - } - catch (IOException) when (timer.Elapsed < TimeSpan.FromSeconds(20)) - { - Thread.Sleep(50); - } - } - } - - internal JObject Get(string path, int expectedStatus) - { - ProcessStartInfo start = new(tester, $"\"{tester}\" probe \"\\\\.\\pipe\\{pipeName}\" \"{path}\"") - { - UseShellExecute = false, - CreateNoWindow = true, - RedirectStandardOutput = true, - RedirectStandardError = true, - }; - using Process client = Process.Start(start); - Task stdout = client.StandardOutput.ReadToEndAsync(); - Task stderr = client.StandardError.ReadToEndAsync(); - if (!client.WaitForExit(10000)) - { - client.Kill(); - throw new TimeoutException($"timed out probing {path}"); - } - string standardOutput = stdout.GetAwaiter().GetResult(); - string standardError = stderr.GetAwaiter().GetResult(); - if (client.ExitCode != 0) - { - throw new IOException($"policy tester probe failed for {path}: {standardError}"); - } - JObject response = JObject.Parse(standardOutput); - Assert.Equal(expectedStatus, (int)response["Status"]); - return response["Body"] as JObject; - } - - internal void Stop() - { - if (process == null) - { - return; - } - try - { - if (!process.HasExited) - { - try - { - process.Kill(); - } - catch (InvalidOperationException) when (process.HasExited) - { - } - catch (Win32Exception) when (process.WaitForExit(10000)) - { - } - } - Assert.True(process.WaitForExit(10000), "Agent did not stop"); - if (stdout != null) - { - output.WriteLine(stdout.GetAwaiter().GetResult()); - output.WriteLine(stderr.GetAwaiter().GetResult()); - } - foreach (string log in Directory.GetFiles(root, "agent-installer-e2e*")) - { - output.WriteLine(File.ReadAllText(log)); - } - } - finally - { - if (process.HasExited) - { - process.Dispose(); - process = null; - } - } - } - - public void Dispose() - { - job.Dispose(); - Stop(); - } - } - - private sealed class AgentJob : IDisposable - { - private const uint JobObjectLimitKillOnJobClose = 0x2000; - private const int JobObjectExtendedLimitInformation = 9; - private readonly SafeFileHandle handle; - - internal AgentJob() - { - handle = CreateJobObjectW(IntPtr.Zero, null); - if (handle.IsInvalid) - { - throw new Win32Exception(Marshal.GetLastWin32Error()); - } - ExtendedLimitInformation limits = new() - { - BasicLimitInformation = new BasicLimitInformation { LimitFlags = JobObjectLimitKillOnJobClose }, - }; - if (!SetInformationJobObject( - handle, JobObjectExtendedLimitInformation, ref limits, Marshal.SizeOf())) - { - int error = Marshal.GetLastWin32Error(); - handle.Dispose(); - throw new Win32Exception(error); - } - } - - internal void Assign(Process process) - { - if (!AssignProcessToJobObject(handle, process.Handle)) - { - throw new Win32Exception(Marshal.GetLastWin32Error()); - } - } - - public void Dispose() => handle.Dispose(); - - [StructLayout(LayoutKind.Sequential)] - private struct BasicLimitInformation - { - internal long PerProcessUserTimeLimit; - internal long PerJobUserTimeLimit; - internal uint LimitFlags; - internal UIntPtr MinimumWorkingSetSize; - internal UIntPtr MaximumWorkingSetSize; - internal uint ActiveProcessLimit; - internal UIntPtr Affinity; - internal uint PriorityClass; - internal uint SchedulingClass; - } - - [StructLayout(LayoutKind.Sequential)] - private struct IoCounters - { - internal ulong ReadOperationCount; - internal ulong WriteOperationCount; - internal ulong OtherOperationCount; - internal ulong ReadTransferCount; - internal ulong WriteTransferCount; - internal ulong OtherTransferCount; - } - - [StructLayout(LayoutKind.Sequential)] - private struct ExtendedLimitInformation - { - internal BasicLimitInformation BasicLimitInformation; - internal IoCounters IoInfo; - internal UIntPtr ProcessMemoryLimit; - internal UIntPtr JobMemoryLimit; - internal UIntPtr PeakProcessMemoryUsed; - internal UIntPtr PeakJobMemoryUsed; - } - - [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] - private static extern SafeFileHandle CreateJobObjectW(IntPtr attributes, string name); - - [DllImport("kernel32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static extern bool SetInformationJobObject( - SafeFileHandle job, int informationClass, ref ExtendedLimitInformation information, int length); - - [DllImport("kernel32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static extern bool AssignProcessToJobObject(SafeFileHandle job, IntPtr process); - } -} From 9f6744a5255457b4e167f93758383f7334401386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 18 Sep 2026 00:25:08 +0900 Subject: [PATCH 12/14] test(agent): cover final policy contract Exercise the final canonical policy rule shapes and validation semantics through the end-to-end policy management harness. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/src/windows.rs | 75 +++++++++++++++++++++-- 1 file changed, 71 insertions(+), 4 deletions(-) diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index d5284fb76..bc5f51cb4 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -481,7 +481,49 @@ fn policy_draft(id: &str, publisher: &str) -> Value { "PolicyFormatVersion": "1.0.0", "Metadata": { "Id": id, "Publisher": publisher }, "Enforcement": { "DefaultDecision": "Deny" }, - "Rules": [] + "Rules": [ + { + "Id": "allow.exact", + "Priority": 100, + "Decision": "Allow", + "Match": { + "Operations": ["Install"], + "Managers": ["Winget"], + "SourceNames": ["winget"], + "PackageIdentifiers": { "Exact": ["Microsoft.PowerToys"] }, + "Version": { "Exact": ["0.86.0"] }, + "ExecutionElevation": ["Elevated"], + "Interactive": true, + "SkipHashCheck": false + }, + "Constraints": { + "AllowInteractive": true, + "AllowSkipHashCheck": false + } + }, + { + "Id": "allow.pattern", + "Priority": 101, + "Decision": "Allow", + "Match": { + "Managers": ["Winget"], + "SourceNames": ["winget"], + "PackageIdentifiers": { "Patterns": ["Contoso.*"] }, + "Version": { + "Range": { + "MinVersion": "1.0.0", + "MaxVersion": "2.0.0", + "IncludePrerelease": false + } + }, + "ExecutionElevation": ["Standard"], + "HasCustomParameters": false + }, + "Constraints": { + "AllowCustomParameters": false + } + } + ] }) } @@ -662,7 +704,7 @@ async fn validate_policy_by_pipe(pipe_name: &str, draft: &Value) -> anyhow::Resu let validation = validation_response.json()?["Validation"].clone(); ensure!(validation["IsValid"] == true, "policy validation failed"); ensure!( - validation["ValidatorVersion"] == "now-package-broker-policy-validator/9", + validation["ValidatorVersion"] == "now-package-broker-policy-validator/10", "unexpected validator contract" ); ensure!( @@ -911,13 +953,38 @@ async fn strict_contract_validation(pipe_name: &str) -> anyhow::Result<()> { for version in ["1.0.0", "1.7.3"] { let mut draft = policy_draft("tests.contract", "Contract"); draft["PolicyFormatVersion"] = json!(version); - validate_policy_by_pipe(pipe_name, &draft).await?; + let validation = validate_policy_by_pipe(pipe_name, &draft).await?; + let canonical = &validation["CanonicalDraft"]; + ensure!( + canonical["Rules"][0]["Match"]["Managers"] == json!(["Winget"]) + && canonical["Rules"][0]["Match"]["SourceNames"] == json!(["winget"]) + && canonical["Rules"][0]["Match"]["PackageIdentifiers"]["Exact"] == json!(["Microsoft.PowerToys"]) + && canonical["Rules"][0]["Match"]["Version"]["Exact"] == json!(["0.86.0"]) + && canonical["Rules"][0]["Match"]["ExecutionElevation"] == json!(["Elevated"]) + && canonical["Rules"][1]["Match"]["PackageIdentifiers"]["Patterns"] == json!(["Contoso.*"]) + && canonical["Rules"][1]["Match"]["Version"]["Range"]["MinVersion"] == "1.0.0" + && canonical["Rules"][1]["Match"]["ExecutionElevation"] == json!(["Standard"]), + "canonical draft did not preserve final rule shapes" + ); + ensure!( + canonical["Rules"][0]["Match"].get("PreRelease").is_none() + && canonical["Rules"][1]["Match"].get("Interactive").is_none(), + "canonical draft did not omit absent optional characteristics" + ); } for (value, expected_path) in [("2.0.0", "/PolicyFormatVersion"), ("broken", "/PolicyFormatVersion")] { let mut draft = policy_draft("tests.contract", "Contract"); draft["PolicyFormatVersion"] = json!(value); assert_invalid_draft(pipe_name, draft, expected_path).await?; } + let mut invalid_interval = policy_draft("tests.contract", "Contract"); + invalid_interval["Metadata"]["ValidFrom"] = json!("2026-01-01T00:00:00Z"); + invalid_interval["Metadata"]["ValidUntil"] = json!("2026-01-01T00:00:00Z"); + assert_invalid_draft(pipe_name, invalid_interval, "/Metadata/ValidUntil").await?; + + let mut duplicate_rule = policy_draft("tests.contract", "Contract"); + duplicate_rule["Rules"][1]["Id"] = duplicate_rule["Rules"][0]["Id"].clone(); + assert_invalid_draft(pipe_name, duplicate_rule, "/Rules/1/Id").await?; Ok(()) } @@ -945,7 +1012,7 @@ async fn assert_invalid_draft(pipe_name: &str, draft: Value, expected_path: &str ensure!( invalid_validation.get("CanonicalDraft").is_none() && invalid_validation.get("ValidationReceipt").is_none() - && invalid_validation["ValidatorVersion"] == "now-package-broker-policy-validator/9", + && invalid_validation["ValidatorVersion"] == "now-package-broker-policy-validator/10", "invalid draft returned a canonical draft, receipt, or wrong validator version" ); ensure!( From 8c553f585a2947a21ffd3fd815b7b3ec65aac84a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 18 Sep 2026 05:33:00 +0900 Subject: [PATCH 13/14] test(agent): cover policy review gaps Exercise standard-user read access, stale overwrite rejection, and LocalSystem shutdown verification after failed readiness. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/run-unelevated.ps1 | 128 ++++++++++++------ crates/agent-policy-tester/src/windows.rs | 22 ++- 2 files changed, 108 insertions(+), 42 deletions(-) diff --git a/crates/agent-policy-tester/run-unelevated.ps1 b/crates/agent-policy-tester/run-unelevated.ps1 index 5dc9b2b2b..b6615393c 100644 --- a/crates/agent-policy-tester/run-unelevated.ps1 +++ b/crates/agent-policy-tester/run-unelevated.ps1 @@ -108,6 +108,64 @@ function Remove-StagingPath { } } +function Complete-ServerShutdown { + param( + [bool] $ServerLaunchAttempted, + [bool] $ServerLaunchExplicitlyFailed, + [string] $StopPath, + [string] $StatusPath, + [string] $ServerOutputPath, + [string] $OutputPath, + [int] $ExitCode, + [scriptblock] $SignalServer + ) + + if (-not $ServerLaunchAttempted) { + return $ExitCode + } + + $signal = & $SignalServer + $signal.Output | Out-File $OutputPath -Append + if ($signal.ExitCode -ne 0 -or -not (Test-Path -LiteralPath $StopPath)) { + try { + New-Item -ItemType File -Path $StopPath -ErrorAction Stop | Out-Null + "Created the stop marker directly after LocalSystem signaling did not confirm it" | Out-File $OutputPath -Append + } catch { + $_ | Out-File $OutputPath -Append + } + } + if (-not (Test-Path -LiteralPath $StopPath)) { + "Failed to create the LocalSystem test server stop marker" | Out-File $OutputPath -Append + $ExitCode = 1 + } + + if (-not $ServerLaunchExplicitlyFailed) { + $deadline = [DateTime]::UtcNow.AddSeconds(30) + while (-not (Test-Path -LiteralPath $StatusPath) -and [DateTime]::UtcNow -lt $deadline) { + Start-Sleep -Milliseconds 100 + } + if (Test-Path -LiteralPath $ServerOutputPath) { + Get-Content -LiteralPath $ServerOutputPath | Out-File $OutputPath -Append + } + if (Test-Path -LiteralPath $StatusPath) { + try { + $serverExitCode = Read-ServerStatus -Path $StatusPath + if ($serverExitCode -ne 0 -and $ExitCode -eq 0) { + $ExitCode = $serverExitCode + } + } catch { + $_ | Out-File $OutputPath -Append + $ExitCode = 1 + } + } else { + "Timed out waiting for LocalSystem test server shutdown" | Out-File $OutputPath -Append + $ExitCode = 1 + } + } + + return $ExitCode +} + function New-RandomSecurePassword { $alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz23456789!@#$%^&*" $bytes = [byte[]]::new(32) @@ -271,6 +329,7 @@ function Invoke-RunnerSelfTests { try { $ready = Join-Path $root "ready.json" $status = Join-Path $root "status" + $serverOutput = Join-Path $root "server.out" foreach ($expected in @(0, 1, -1)) { Publish-ServerStatus -Path $status -ExitCode $expected if ((Read-ServerStatus -Path $status) -ne $expected) { @@ -383,6 +442,27 @@ function Invoke-RunnerSelfTests { } } + $stop = Join-Path $root "stop" + $shutdownOutput = Join-Path $root "shutdown.out" + $signalState = [pscustomobject]@{ Count = 0 } + [System.IO.File]::WriteAllText($status, "invalid") + $readinessFailureExitCode = Complete-ServerShutdown ` + -ServerLaunchAttempted $true -ServerLaunchExplicitlyFailed $false ` + -StopPath $stop -StatusPath $status -ServerOutputPath $serverOutput -OutputPath $shutdownOutput ` + -ExitCode 1 -SignalServer { + $signalState.Count++ + New-Item -ItemType File -Path $stop | Out-Null + [pscustomobject]@{ ExitCode = 0; Output = @("Simulated LocalSystem signal") } + } + if ( + $readinessFailureExitCode -ne 1 -or + $signalState.Count -ne 1 -or + -not (Test-Path -LiteralPath $stop) -or + (Get-Content -LiteralPath $shutdownOutput -Raw) -notmatch "published an invalid completion status" + ) { + throw "Readiness failure did not signal and verify LocalSystem server shutdown" + } + Remove-StagingPath -Path (Join-Path $root "already-absent") } finally { Remove-StagingPath -Path $root @@ -550,48 +630,14 @@ try { $_ | Out-File $outputPath -Append $exitCode = 1 } finally { - if ($serverLaunchAttempted) { - $signalOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` - -Action Signal -StopPath $stopPath 2>&1 - $signalExitCode = $LASTEXITCODE - $signalOutput | Out-File $outputPath -Append - if ($signalExitCode -ne 0 -and -not (Test-Path -LiteralPath $stopPath)) { - try { - New-Item -ItemType File -Path $stopPath -ErrorAction Stop | Out-Null - "Created the stop marker directly after SYSTEM signaling failed" | Out-File $outputPath -Append - } catch { - $_ | Out-File $outputPath -Append - } + $exitCode = Complete-ServerShutdown ` + -ServerLaunchAttempted $serverLaunchAttempted -ServerLaunchExplicitlyFailed $serverLaunchExplicitlyFailed ` + -StopPath $stopPath -StatusPath $statusPath -ServerOutputPath $serverOutputPath -OutputPath $outputPath ` + -ExitCode $exitCode -SignalServer { + $signalOutput = & psexec.exe -accepteula -s pwsh.exe -NoProfile -File $PSCommandPath ` + -Action Signal -StopPath $stopPath 2>&1 + [pscustomobject]@{ ExitCode = $LASTEXITCODE; Output = $signalOutput } } - if (-not (Test-Path -LiteralPath $stopPath) -and $exitCode -eq 0) { - "Failed to create the LocalSystem test server stop marker" | Out-File $outputPath -Append - $exitCode = 1 - } - - if (-not $serverLaunchExplicitlyFailed) { - $deadline = [DateTime]::UtcNow.AddSeconds(30) - while (-not (Test-Path -LiteralPath $statusPath) -and [DateTime]::UtcNow -lt $deadline) { - Start-Sleep -Milliseconds 100 - } - if (Test-Path -LiteralPath $serverOutputPath) { - Get-Content -LiteralPath $serverOutputPath | Out-File $outputPath -Append - } - if (Test-Path -LiteralPath $statusPath) { - try { - $serverExitCode = Read-ServerStatus -Path $statusPath - if ($serverExitCode -ne 0 -and $exitCode -eq 0) { - $exitCode = $serverExitCode - } - } catch { - $_ | Out-File $outputPath -Append - $exitCode = 1 - } - } elseif ($exitCode -eq 0) { - "Timed out waiting for LocalSystem test server shutdown" | Out-File $outputPath -Append - $exitCode = 1 - } - } - } if ($clientAccount) { try { diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index bc5f51cb4..aea53e539 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -918,6 +918,13 @@ async fn standard_user_management(ready_path: &Path, nonce: &str, client: &Proce let management = policy_management_by_pipe(pipe_name).await?; ensure!(management["State"] == "Missing", "expected a missing policy"); + let missing_policy = request(pipe_name, "GET", "/v1/policy").await?; + ensure!( + missing_policy.status == 404 + && missing_policy.json()?["Code"] == "NotFound" + && missing_policy.json()?["Message"] == "active policy is unavailable", + "standard-user missing policy read did not return the canonical not-found response" + ); let valid_draft = policy_draft("tests.standard-user", "Test"); let validation = validate_policy_by_pipe(pipe_name, &valid_draft).await?; @@ -1107,7 +1114,7 @@ async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { &agent, "Update", "Reject", - stale_token, + stale_token.clone(), policy_draft("tests.managed-external", "Stale"), ) .await?; @@ -1120,6 +1127,19 @@ async fn managed_policy_lifecycle(agent_path: &Path) -> anyhow::Result<()> { ); wait_for_log(&agent, "stale_conflict").await?; + let stale_confirm = replace_policy_response( + &agent, + "Update", + "ConfirmOverwrite", + stale_token, + policy_draft("tests.managed-external", "Stale confirmed overwrite"), + ) + .await?; + ensure!( + stale_confirm.status == 409 && stale_confirm.json()?["Code"] == "StalePolicyStoreToken", + "stale ConfirmOverwrite did not return a store token conflict" + ); + let current_token = stale["Management"]["StoreToken"].clone(); let confirmed = replace_policy_response( &agent, From 5eb089efe40e8feab28225da698b0cc096a69d53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Sun, 20 Sep 2026 17:50:53 +0900 Subject: [PATCH 14/14] fix(agent): accept advisory policy findings Remove retired warning acknowledgment requests while retaining receipt and conflict enforcement in policy management E2E coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/agent-policy-tester/src/windows.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/crates/agent-policy-tester/src/windows.rs b/crates/agent-policy-tester/src/windows.rs index aea53e539..c7511892d 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -756,7 +756,6 @@ async fn replace_policy_response_by_pipe( "ExpectedStoreToken": expected_store_token, "Operation": operation, "ConflictHandling": conflict_handling, - "WarningsAcknowledged": true, "Draft": validation["CanonicalDraft"], "ValidationReceipt": validation["ValidationReceipt"] }); @@ -1210,16 +1209,9 @@ async fn warnings_identity_and_receipts( "ExpectedStoreToken": current["Management"]["StoreToken"], "Operation": "ReplaceIdentity", "ConflictHandling": "Reject", - "WarningsAcknowledged": false, "Draft": validation["CanonicalDraft"], "ValidationReceipt": validation["ValidationReceipt"] }); - let warning = send_replacement(&agent.pipe_name, &replacement).await?; - ensure!( - warning.status == 409 && warning.json()?["Code"] == "WarningConfirmationRequired", - "unacknowledged warnings were not rejected" - ); - replacement["WarningsAcknowledged"] = json!(true); replacement["Draft"]["Metadata"]["Publisher"] = json!("Tampered after validation"); let tampered = send_replacement(&agent.pipe_name, &replacement).await?; ensure!( @@ -1245,12 +1237,11 @@ async fn warnings_identity_and_receipts( replaced["Policy"]["Metadata"]["Id"] == "tests.replaced-identity" && replaced["Policy"]["Metadata"]["Revision"] == 1 && replaced["Policy"]["PolicyFormatVersion"] == "1.7.3", - "ReplaceIdentity did not preserve the canonical contract and reset revision" + "advisory findings did not permit the canonical identity replacement" ); wait_for_log(agent, "Policy change succeeded").await?; wait_for_log(agent, "replace_identity").await?; wait_for_log(agent, "invalid_receipt").await?; - wait_for_log(agent, "warnings_not_acknowledged").await?; Ok(replaced) }