From da01738ce9ae2ea0a70bc21b83fff8288c1b6928 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 1/6] 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 | 457 +++++++++++++++++- 5 files changed, 576 insertions(+), 26 deletions(-) create mode 100644 crates/agent-policy-tester/run-unelevated.ps1 diff --git a/Cargo.lock b/Cargo.lock index 7878061d7..61d10d1ea 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 82eb6b4bf..4e15716aa 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 } @@ -363,12 +502,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", @@ -389,12 +523,23 @@ 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"] @@ -407,6 +552,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() } @@ -463,6 +618,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 9019d98532b06c0174399abbf8b926b0bdb4da8b 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 2/6] 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 50c653eef..40e2c2121 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1377,6 +1377,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: | @@ -1388,6 +1398,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 ff010a5e593ceaec37f661b2d572b8f22d4a05da 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 3/6] 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 4e15716aa..5766ed176 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 { @@ -493,7 +555,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 {}", @@ -502,14 +568,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"), @@ -533,7 +599,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", @@ -545,7 +628,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"), @@ -618,13 +701,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" @@ -638,7 +802,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"), @@ -657,8 +821,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(), @@ -674,7 +838,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 27392eafb35092d3616ae2c1a0f413da894c1be6 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 4/6] 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 5766ed176..72a7dbe27 100644 --- a/crates/agent-policy-tester/src/windows.rs +++ b/crates/agent-policy-tester/src/windows.rs @@ -727,15 +727,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 344a22bf84360269989cdd020f7b131292d65051 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 5/6] 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 72a7dbe27..c2d3797b0 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 { @@ -1176,6 +1252,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; } @@ -1191,3 +1268,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 3867fa32edd9052a898f9d4d5700211717292d75 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 6/6] 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 } }