From 919137d891dc9cf91cd403adc0a966bb47289a0e Mon Sep 17 00:00:00 2001 From: Franz Hemmer Date: Thu, 5 Nov 2026 09:10:32 -0500 Subject: [PATCH 1/4] ci: capture every view saving under a filtered token (#83) Runs the release exe on the hosted Windows desktop with an Administrators-owned config.toml and Administrators deny-only, clicks Project, Backlog and Fleet, and photographs each one. A view fails if the save-error banner shows or config.toml doesn't change. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 18 ++ AGENTS.md | 4 + scripts/native-capture/FilteredToken.cs | 88 +++++++ .../native-capture/Invoke-ViewSaveCapture.ps1 | 239 ++++++++++++++++++ scripts/native-capture/Start-LoopbackPmd.ps1 | 14 + 5 files changed, 363 insertions(+) create mode 100644 scripts/native-capture/FilteredToken.cs create mode 100644 scripts/native-capture/Invoke-ViewSaveCapture.ps1 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 023c0c2..c5d57f1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,6 +16,7 @@ on: - crates/dashboard-app/build.rs - crates/dashboard-app/src/mark.rs - crates/dashboard-app/src/main.rs + - crates/dashboard-core/src/config.rs - Cargo.toml - Cargo.lock - crates/*/Cargo.toml @@ -89,6 +90,23 @@ jobs: path: native-captures/ retention-days: 14 + # Issue 83: real clicks on the release exe, an Administrators-owned config and a filtered token. + - name: Native Windows view-save capture (issue 83) + if: github.event_name == 'pull_request' + timeout-minutes: 10 + shell: pwsh + env: + CAPTURE_HEAD: ${{ github.event.pull_request.head.sha }} + run: ./scripts/native-capture/Invoke-ViewSaveCapture.ps1 -ExpectedHead $env:CAPTURE_HEAD + + - name: Upload view-save evidence + if: always() && github.event_name == 'pull_request' + uses: actions/upload-artifact@v4 + with: + name: view-save-windows-${{ github.event.pull_request.head.sha }} + path: view-save-captures/ + retention-days: 14 + - name: Upload the package if: always() uses: actions/upload-artifact@v4 diff --git a/AGENTS.md b/AGENTS.md index 7b2d9d7..3ab79bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,10 @@ cargo deny --locked check advisories # needs cargo inst CAPTURE_DIR=$PWD/captures cargo test -p dashboard-app captures -- --ignored # PNGs (Linux or macOS; absolute path) ``` +The Release workflow's PR runs also start the release exe on the hosted Windows desktop under a filtered token, with an +Administrators-owned `config.toml`, and click Project, Backlog and Fleet (`scripts/native-capture/Invoke-ViewSaveCapture.ps1`, +#83). Its screenshots are in the `view-save-windows-` artifact. + On Linux, building needs GPUI's headers: `sudo apt install libxkbcommon-dev libxkbcommon-x11-dev libwayland-dev libfontconfig-dev libfreetype-dev libxcb1-dev libx11-xcb-dev`. diff --git a/scripts/native-capture/FilteredToken.cs b/scripts/native-capture/FilteredToken.cs new file mode 100644 index 0000000..3733766 --- /dev/null +++ b/scripts/native-capture/FilteredToken.cs @@ -0,0 +1,88 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Runtime.InteropServices; +using System.Text; + +// Starts a process the way UAC starts a non-elevated one from an administrator's logon: Administrators held +// deny-only, no extra privileges, medium integrity. Such a token can't name Administrators as a file's owner. +public static class FilteredToken { + const string Administrators = "S-1-5-32-544"; + const string MediumIntegrity = "S-1-16-8192"; + const uint TOKEN_ALL_ACCESS = 0xF01FF; + const uint DISABLE_MAX_PRIVILEGE = 1, LUA_TOKEN = 4; + const uint SE_GROUP_USE_FOR_DENY_ONLY = 0x10, SE_GROUP_INTEGRITY = 0x20; + const int TokenGroups = 2, TokenIntegrityLevel = 25; + const uint CREATE_UNICODE_ENVIRONMENT = 0x400, CREATE_NO_WINDOW = 0x08000000; + + [StructLayout(LayoutKind.Sequential)] struct SID_AND_ATTRIBUTES { public IntPtr Sid; public uint Attributes; } + [StructLayout(LayoutKind.Sequential)] struct PROCESS_INFORMATION { public IntPtr Process, Thread; public uint ProcessId, ThreadId; } + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] struct STARTUPINFO { + public int Size; public string Reserved, Desktop, Title; + public uint X, Y, Width, Height, CountChars, CountLines, Fill, Flags; + public ushort Show, Reserved2; public IntPtr Reserved3, Input, Output, Error; + } + + [DllImport("kernel32.dll")] static extern IntPtr GetCurrentProcess(); + [DllImport("kernel32.dll", SetLastError = true)] static extern bool CloseHandle(IntPtr handle); + [DllImport("advapi32.dll", SetLastError = true)] static extern bool OpenProcessToken(IntPtr process, uint access, out IntPtr token); + [DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)] static extern bool ConvertStringSidToSidW(string text, out IntPtr sid); + [DllImport("advapi32.dll")] static extern int GetLengthSid(IntPtr sid); + [DllImport("advapi32.dll")] static extern bool EqualSid(IntPtr a, IntPtr b); + [DllImport("advapi32.dll", SetLastError = true)] static extern bool CreateRestrictedToken(IntPtr existing, uint flags, uint disableCount, SID_AND_ATTRIBUTES[] disable, uint deleteCount, IntPtr delete, uint restrictCount, IntPtr restrict, out IntPtr result); + [DllImport("advapi32.dll", SetLastError = true)] static extern bool SetTokenInformation(IntPtr token, int kind, ref SID_AND_ATTRIBUTES label, int length); + [DllImport("advapi32.dll", SetLastError = true)] static extern bool GetTokenInformation(IntPtr token, int kind, IntPtr buffer, int length, out int needed); + [DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)] static extern bool CreateProcessAsUserW(IntPtr token, string application, StringBuilder commandLine, IntPtr processAttributes, IntPtr threadAttributes, bool inherit, uint flags, string environment, string directory, ref STARTUPINFO startup, out PROCESS_INFORMATION information); + + static void Check(bool ok, string what) { + if (!ok) throw new InvalidOperationException(what + " failed: Windows error " + Marshal.GetLastWin32Error()); + } + static IntPtr Sid(string text) { + IntPtr sid; + Check(ConvertStringSidToSidW(text, out sid), "ConvertStringSidToSid"); + return sid; + } + + // True only when the token holds Administrators and may use it for nothing but deny entries. + static bool AdministratorsIsDenyOnly(IntPtr token) { + int needed; + GetTokenInformation(token, TokenGroups, IntPtr.Zero, 0, out needed); + IntPtr buffer = Marshal.AllocHGlobal(needed); + try { + Check(GetTokenInformation(token, TokenGroups, buffer, needed, out needed), "GetTokenInformation"); + int count = Marshal.ReadInt32(buffer); + int size = Marshal.SizeOf(typeof(SID_AND_ATTRIBUTES)); + IntPtr administrators = Sid(Administrators); + for (int i = 0; i < count; i++) { + var group = (SID_AND_ATTRIBUTES)Marshal.PtrToStructure(buffer + IntPtr.Size + i * size, typeof(SID_AND_ATTRIBUTES)); + if (EqualSid(group.Sid, administrators)) return (group.Attributes & SE_GROUP_USE_FOR_DENY_ONLY) != 0; + } + return false; + } finally { Marshal.FreeHGlobal(buffer); } + } + + // A restricted copy of the caller's own token needs no SeAssignPrimaryTokenPrivilege. + public static int Start(string application, string directory, IDictionary environment) { + IntPtr own, filtered; + Check(OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, out own), "OpenProcessToken"); + try { + var disable = new[] { new SID_AND_ATTRIBUTES { Sid = Sid(Administrators) } }; + Check(CreateRestrictedToken(own, DISABLE_MAX_PRIVILEGE | LUA_TOKEN, 1, disable, 0, IntPtr.Zero, 0, IntPtr.Zero, out filtered), "CreateRestrictedToken"); + } finally { CloseHandle(own); } + try { + var label = new SID_AND_ATTRIBUTES { Sid = Sid(MediumIntegrity), Attributes = SE_GROUP_INTEGRITY }; + Check(SetTokenInformation(filtered, TokenIntegrityLevel, ref label, Marshal.SizeOf(typeof(SID_AND_ATTRIBUTES)) + GetLengthSid(label.Sid)), "SetTokenInformation"); + if (!AdministratorsIsDenyOnly(filtered)) throw new InvalidOperationException("Administrators is not deny-only in the filtered token."); + var names = new List(); + foreach (DictionaryEntry entry in environment) names.Add(entry.Key + "=" + entry.Value); + names.Sort(StringComparer.OrdinalIgnoreCase); + var startup = new STARTUPINFO { Size = Marshal.SizeOf(typeof(STARTUPINFO)) }; + PROCESS_INFORMATION information; + Check(CreateProcessAsUserW(filtered, application, new StringBuilder("\"" + application + "\""), IntPtr.Zero, IntPtr.Zero, false, + CREATE_UNICODE_ENVIRONMENT | CREATE_NO_WINDOW, string.Join("\0", names) + "\0", directory, ref startup, out information), "CreateProcessAsUser"); + CloseHandle(information.Thread); + CloseHandle(information.Process); + return (int)information.ProcessId; + } finally { CloseHandle(filtered); } + } +} diff --git a/scripts/native-capture/Invoke-ViewSaveCapture.ps1 b/scripts/native-capture/Invoke-ViewSaveCapture.ps1 new file mode 100644 index 0000000..259818b --- /dev/null +++ b/scripts/native-capture/Invoke-ViewSaveCapture.ps1 @@ -0,0 +1,239 @@ +# Issue 83 on the hosted Windows desktop: the release exe, a real Administrators-owned config.toml and a token with +# Administrators deny-only, which is what Franz's non-elevated app has. Each real click must save its view with no banner. +# Every view is photographed even when an earlier one fails, so a failed run shows the whole problem. +param( + [string]$Exe = 'target/release/project-dashboard.exe', + [string]$OutDir = 'view-save-captures', + [Parameter(Mandatory)][string]$ExpectedHead +) +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version Latest +New-Item -ItemType Directory -Force $OutDir | Out-Null +$OutDir = (Resolve-Path $OutDir).Path +$root = Join-Path ([IO.Path]::GetTempPath()) "dashboard-view-save-$([Guid]::NewGuid().ToString('N'))" +$app = $null; $fixture = $null; $ocr = $null +$failures = [Collections.Generic.List[string]]::new() +$readId = 0 +. (Join-Path $PSScriptRoot 'OcrPipe.ps1') +$manifest = [ordered]@{ + head = (git rev-parse HEAD); expectedHead = $ExpectedHead + runner = 'GitHub-hosted windows-latest'; run = "https://github.com/$env:GITHUB_REPOSITORY/actions/runs/$env:GITHUB_RUN_ID" + syntheticLoopbackFixture = $true; success = $false; views = @(); failures = @() + startedAt = [DateTimeOffset]::UtcNow.ToString('o') +} + +$essentials = @('PATH','SystemRoot','WINDIR','USERPROFILE','APPDATA','LOCALAPPDATA','TEMP','TMP','COMSPEC','PATHEXT','ProgramFiles','ProgramFiles(x86)','ProgramData','SystemDrive') +function Get-Essentials { + $values = @{} + foreach ($name in $essentials) { + $value = [Environment]::GetEnvironmentVariable($name) + if ($value) { $values[$name] = $value } + } + $values +} +function New-HelperStart([string]$File, [string[]]$Arguments) { + $start = [Diagnostics.ProcessStartInfo]::new($File) + $start.UseShellExecute = $false + $start.CreateNoWindow = $true + $start.Environment.Clear() + $values = Get-Essentials + foreach ($name in $values.Keys) { $start.Environment[$name] = $values[$name] } + foreach ($argument in $Arguments) { $start.ArgumentList.Add($argument) } + $start +} +function Stop-Helper($Process) { + if ($null -ne $Process) { + if (-not $Process.HasExited) { $Process.Kill($true); [void]$Process.WaitForExit(5000) } + if (-not $Process.HasExited) { throw "Owned process $($Process.Id) did not stop." } + $Process.Dispose() + } +} +function Get-WindowBounds { + $app.Refresh() + if ($app.HasExited) { throw 'Dashboard exited during the attempt.' } + if ($app.MainWindowTitle -cne 'Project Dashboard') { throw 'Owned window title changed.' } + if ([NativeDesktop]::InputDesktop() -ne 'Default') { throw 'Locked/noninteractive input desktop.' } + [void][NativeDesktop]::SetForegroundWindow($app.MainWindowHandle) + $deadline = [DateTime]::UtcNow.AddSeconds(2) + while ([NativeDesktop]::GetForegroundWindow() -ne $app.MainWindowHandle -and [DateTime]::UtcNow -lt $deadline) { + Start-Sleep -Milliseconds 50 + } + if ([NativeDesktop]::GetForegroundWindow() -ne $app.MainWindowHandle) { throw 'Dashboard is not foreground.' } + $rect = [NativeDesktop+RECT]::new() + if (-not [NativeDesktop]::GetWindowRect($app.MainWindowHandle,[ref]$rect)) { throw 'GetWindowRect failed.' } + [Drawing.Rectangle]::FromLTRB($rect.Left,$rect.Top,$rect.Right,$rect.Bottom) +} +function Read-Image([string]$Path) { + $script:readId++ + # The first read also starts the OCR engine, which takes longer. + (Invoke-OcrRead $ocr $Path $readId $(if ($readId -eq 1) { 120000 } else { 60000 })).result +} +function Save-Window([string]$Name) { + $bounds = Get-WindowBounds + $path = Join-Path $OutDir "$Name.png" + $bitmap = [Drawing.Bitmap]::new($bounds.Width,$bounds.Height) + $graphics = [Drawing.Graphics]::FromImage($bitmap) + try { + # Only GetWindowRect of this owned foreground process, never the desktop. + $graphics.CopyFromScreen($bounds.Location,[Drawing.Point]::Empty,$bounds.Size) + $bitmap.Save($path,[Drawing.Imaging.ImageFormat]::Png) + } finally { $graphics.Dispose(); $bitmap.Dispose() } + @{bounds=$bounds; path=$path; file="$Name.png"; text=(Read-Image $path)} +} +function Find-Tab($Shot, [string]$Name) { + # The title bar also says "Project", so anchor on Backlog, the one tab whose word is unique near the top. + $anchors = @($Shot.text.words | Where-Object { $_.text -ceq 'Backlog' } | Sort-Object y) + if ($anchors.Count -lt 1) { throw "Cannot find the Backlog tab. OCR: $($Shot.text.text)" } + $anchor = $anchors[0] + if ($Name -ceq 'Backlog') { return $anchor } + $row = @($Shot.text.words | Where-Object { $_.text -ceq $Name -and [Math]::Abs($_.y-$anchor.y) -lt $anchor.height }) + if ($row.Count -ne 1) { throw "Cannot find one $Name tab beside Backlog. OCR: $($Shot.text.text)" } + $row[0] +} +function Read-SavedView { + $text = [IO.File]::ReadAllText($configPath) + if ($text -notmatch "(?m)^view\s*=\s*['""](\w+)['""]") { throw "config.toml has no view line: $text" } + $Matches[1] +} +function Test-Saved([string]$View) { + $deadline = [DateTime]::UtcNow.AddSeconds(5) + while ((Read-SavedView) -cne $View -and [DateTime]::UtcNow -lt $deadline) { Start-Sleep -Milliseconds 100 } + (Read-SavedView) -ceq $View +} +function Assert-View([string]$Step, [string]$View, [string]$Marker, $Shot) { + $visible = [regex]::Replace($Shot.text.text,'\s+',' ') + $problem = @() + if ($visible -match "can.?t write|security ID|os error|1307") { $problem += 'the save-error banner is showing' } + if (-not (Test-Saved $View)) { $problem += "config.toml still says view = '$(Read-SavedView)'" } + if ($visible -notmatch $Marker) { $problem += "the $View view's content ($Marker) isn't visible" } + $security = Get-Acl -LiteralPath $configPath + $manifest.views += @{ + step=$Step; view=$View; file=$Shot.file; sha256=(Get-FileHash $Shot.path -Algorithm SHA256).Hash.ToLowerInvariant() + savedView=(Read-SavedView); owner=$security.Owner; visibleText=$Shot.text.text; problems=$problem + } + foreach ($item in $problem) { $failures.Add("${Step}: $item") } + Write-Host "$Step ($View): owner $($security.Owner), $(if ($problem) { $problem -join '; ' } else { 'saved, no banner' })" +} +function Switch-View($From, [string]$Tab, [string]$Step, [string]$View, [string]$Marker, [int]$SettleSeconds) { + $word = Find-Tab $From $Tab + $bounds = Get-WindowBounds + $x = $bounds.X+[int]($word.x+$word.width/2); $y = $bounds.Y+[int]($word.y+$word.height/2) + if (-not [NativeDesktop]::OwnsPoint($app.MainWindowHandle,$x,$y)) { throw "The $Tab tab isn't inside the Dashboard window." } + [NativeDesktop]::Click($x,$y) + Start-Sleep -Seconds $SettleSeconds + $shot = Save-Window $Step + Assert-View $Step $View $Marker $shot + $shot +} + +try { + if (-not $IsWindows) { throw 'This capture requires Windows.' } + if ($manifest.head -ne $ExpectedHead) { throw 'Checkout is not the PR head.' } + $identity = [Security.Principal.WindowsIdentity]::GetCurrent() + if (-not [Security.Principal.WindowsPrincipal]::new($identity).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)) { + throw 'Creating an Administrators-owned config needs an elevated runner.' + } + $Exe = (Resolve-Path $Exe).Path + $manifest.exeSha256 = (Get-FileHash $Exe -Algorithm SHA256).Hash.ToLowerInvariant() + Add-Type -Path (Join-Path $PSScriptRoot 'NativeDesktop.cs') + Add-Type -Path (Join-Path $PSScriptRoot 'FilteredToken.cs') + Add-Type -AssemblyName System.Windows.Forms,System.Drawing + [void][NativeDesktop]::SetThreadDpiAwarenessContext([IntPtr](-4)) + $manifest.inputDesktop = [NativeDesktop]::InputDesktop() + if (-not [Environment]::UserInteractive -or $manifest.inputDesktop -ne 'Default') { throw 'Locked/noninteractive runner desktop.' } + $area = [Windows.Forms.Screen]::PrimaryScreen.WorkingArea + + New-Item -ItemType Directory -Path (Join-Path $root 'checkout/.git') -Force | Out-Null + [IO.File]::WriteAllText((Join-Path $root 'checkout/.git/config'),"[remote `"origin`"]`nurl = https://github.com/fixture-org/synthetic-dashboard.git`n") + [IO.File]::WriteAllText((Join-Path $root 'read-token'),'synthetic-issue31-token') + [IO.File]::WriteAllText((Join-Path $root 'state.txt'),'normal') + + $ocrStart = New-HelperStart (Join-Path $env:SystemRoot 'System32/WindowsPowerShell/v1.0/powershell.exe') @('-NoLogo','-NoProfile','-NonInteractive','-File',(Join-Path $PSScriptRoot 'Read-WindowText.ps1')) + $ocrStart.RedirectStandardInput=$true; $ocrStart.RedirectStandardOutput=$true; $ocrStart.RedirectStandardError=$true + $ocrStart.StandardInputEncoding=[Text.UTF8Encoding]::new($false) + $ocrStart.StandardOutputEncoding=[Text.UTF8Encoding]::new($false) + $ocr = [Diagnostics.Process]::Start($ocrStart) + $null = $ocr.StandardError.ReadToEndAsync() + + $fixture = [Diagnostics.Process]::Start((New-HelperStart (Join-Path $PSHOME 'pwsh.exe') @('-NoProfile','-File',(Join-Path $PSScriptRoot 'Start-LoopbackPmd.ps1'),'-Root',$root))) + $deadline = [DateTime]::UtcNow.AddSeconds(15) + while (-not (Test-Path (Join-Path $root 'port.txt'))) { + if ($fixture.HasExited -or [DateTime]::UtcNow -ge $deadline) { throw 'Synthetic loopback fixture did not start within 15 seconds.' } + Start-Sleep -Milliseconds 100 + } + $port = (Get-Content (Join-Path $root 'port.txt') -Raw).Trim() + + # The reported shape: Administrators owns the file, the user has full control through inherited entries. + $configPath = Join-Path $root 'config.toml' + $configText = @" +# keep this comment +view = 'project' +repositories = ['$(Join-Path $root 'checkout')'] +refresh_minutes = 0 +parallelism = 1 +pmd_url = 'http://127.0.0.1:$port' +pmd_token_path = 'read-token' +t3_home = '$root' +"@ + [IO.File]::WriteAllText($configPath,$configText) + $administrators = [Security.Principal.SecurityIdentifier]::new([Security.Principal.WellKnownSidType]::BuiltinAdministratorsSid,$null) + $security = Get-Acl -LiteralPath $configPath + $security.SetOwner($administrators) + Set-Acl -LiteralPath $configPath -AclObject $security + $before = Get-Acl -LiteralPath $configPath + if ($before.GetOwner([Security.Principal.SecurityIdentifier]) -ne $administrators) { throw "Fixture owner is $($before.Owner), not Administrators." } + $manifest.ownerBefore = $before.Owner + $manifest.accessBefore = $before.AccessToString + + $environment = Get-Essentials + $environment['PROJECT_DASHBOARD_CONFIG_DIR'] = $root + $app = [Diagnostics.Process]::GetProcessById([FilteredToken]::Start($Exe,$root,$environment)) + $manifest.dashboardProcessId = $app.Id + $deadline = [DateTime]::UtcNow.AddSeconds(60) + do { + Start-Sleep -Milliseconds 200; $app.Refresh() + if ($app.HasExited) { throw 'Dashboard exited before showing a window.' } + } until ($app.MainWindowTitle -eq 'Project Dashboard' -or [DateTime]::UtcNow -ge $deadline) + if ($app.MainWindowTitle -ne 'Project Dashboard') { throw 'No Dashboard window within 60 seconds.' } + [void][NativeDesktop]::ShowWindow($app.MainWindowHandle,9) + if (-not [NativeDesktop]::MoveWindow($app.MainWindowHandle,$area.X+8,$area.Y+8,[Math]::Min(1400,$area.Width-16),[Math]::Min(1000,$area.Height-16),$true)) { throw 'MoveWindow failed.' } + $deadline = [DateTime]::UtcNow.AddSeconds(30) + while (-not (Test-Path (Join-Path $root 'completed-normal')) -and [DateTime]::UtcNow -lt $deadline) { Start-Sleep -Milliseconds 100 } + if (-not (Test-Path (Join-Path $root 'completed-normal'))) { throw 'No production fetch completed within 30 seconds.' } + Start-Sleep -Seconds 3 + + $projectMarker = 'fixture.?[o0]rg' + $shot = Save-Window '1-project-at-start' + Assert-View '1-project-at-start' 'project' $projectMarker $shot + $shot = Switch-View $shot 'Backlog' '2-backlog' 'backlog' 'SYNTHETIC FIXTURE' 2 + # Fleet fetches on selection, and needs time to show its rows. + $shot = Switch-View $shot 'Fleet' '3-fleet' 'fleet' 'synthetic-windows' 5 + $shot = Switch-View $shot 'Project' '4-project' 'project' $projectMarker 2 + + $after = Get-Acl -LiteralPath $configPath + $manifest.ownerAfter = $after.Owner + $manifest.accessAfter = $after.AccessToString + $manifest.groupUnchanged = $after.Group -eq $before.Group + if ($after.GetOwner([Security.Principal.SecurityIdentifier]) -eq $administrators) { $failures.Add('config.toml is still owned by Administrators, so no save replaced it.') } + if ($after.AccessToString -cne $before.AccessToString) { $failures.Add('The save changed who can access config.toml.') } + if ($after.Group -ne $before.Group) { $failures.Add('The save changed the group on config.toml.') } + # One round trip back to the starting view must leave every byte as it was. + $saved = [IO.File]::ReadAllText($configPath) + if ($saved -cne $configText) { $failures.Add("config.toml changed outside its view value: $saved") } + if ($failures.Count) { throw "Views failed: $($failures -join ' | ')" } + $manifest.success = $true +} catch { + $manifest.failure = $_.Exception.Message + throw +} finally { + $manifest.failures = @($failures) + $cleanupErrors = @() + foreach ($owned in @($app,$fixture,$ocr)) { + try { Stop-Helper $owned } catch { $cleanupErrors += $_.Exception.Message } + } + try { if (Test-Path $root) { Remove-Item $root -Recurse -Force } } catch { $cleanupErrors += $_.Exception.Message } + if ($cleanupErrors.Count) { $manifest.success=$false; $manifest.cleanupErrors=$cleanupErrors } + $manifest.finishedAt = [DateTimeOffset]::UtcNow.ToString('o') + $manifest | ConvertTo-Json -Depth 8 | Set-Content (Join-Path $OutDir 'manifest.json') + if ($cleanupErrors.Count) { throw "Cleanup failed: $($cleanupErrors -join '; ')" } +} diff --git a/scripts/native-capture/Start-LoopbackPmd.ps1 b/scripts/native-capture/Start-LoopbackPmd.ps1 index 9b24147..865a614 100644 --- a/scripts/native-capture/Start-LoopbackPmd.ps1 +++ b/scripts/native-capture/Start-LoopbackPmd.ps1 @@ -57,6 +57,20 @@ try { if ($uri.Query -match 'cursor=') { $body=@{items=@();nextCursor=$null} } else { $body=@{items=@($item);nextCursor='synthetic-next-page'} } } + # Backlog and Fleet, so the view-save capture shows them populated. + '/v1/dispatch/plan' { $body=@{recommendations=@(@{workItem=@{id=$item.id};reason='SYNTHETIC FIXTURE recommendation'});skipped=@();projects=@()} } + '/v1/portfolio' { $body=@{} } + '/v1/assignments' { $body=@{assignments=@();truncated=$false} } + '/v1/machines' { $body=@{machines=@(@{name='synthetic-windows';os='windows';maxSessions=6},@{name='synthetic-linux';os='linux';maxSessions=3})} } + '/v1/capacity' { $body=@{machines=@(@{name='synthetic-windows';used=0;chiefStarted=0;free=6},@{name='synthetic-linux';used=0;chiefStarted=0;free=3})} } + '/v1/fleet/status' { + $at=[DateTimeOffset]::UtcNow.ToString('o') + $body=@{schema=1;machines=@(@{name='synthetic-windows';online=$true;probeOk=$true;checkedAt=$at},@{name='synthetic-linux';online=$true;probeOk=$true;checkedAt=$at})} + } + '/v1/fleet/activity' { + $at=[DateTimeOffset]::UtcNow.ToString('o') + $body=@{schema=1;observedAt=$at;machines=@(@{name='synthetic-windows';observedAt=$at;state='ok';activeCount=0;sessions=@()},@{name='synthetic-linux';observedAt=$at;state='ok';activeCount=0;sessions=@()})} + } default { $code=404; $body=@{error=@{code='not_found';message='synthetic fixture unknown route'}} } } } From 4b85ec778a48aa536199fc6c170c33ff3902e51a Mon Sep 17 00:00:00 2001 From: Franz Hemmer Date: Thu, 5 Nov 2026 09:10:32 -0500 Subject: [PATCH 2/4] TEMPORARY: put the pre-#84 save code back to show the capture failing Removed by the next commit. This head should show the banner on every view. Co-Authored-By: Claude Opus 5.5 --- crates/dashboard-core/src/config.rs | 143 +----- crates/dashboard-core/src/config_atomic_73.rs | 432 ------------------ 2 files changed, 25 insertions(+), 550 deletions(-) diff --git a/crates/dashboard-core/src/config.rs b/crates/dashboard-core/src/config.rs index 42739dd..f862fd5 100644 --- a/crates/dashboard-core/src/config.rs +++ b/crates/dashboard-core/src/config.rs @@ -463,20 +463,10 @@ fn replace_config_with( path: &Path, bytes: &[u8], write: impl FnOnce(&mut std::fs::File, &[u8]) -> std::io::Result<()>, -) -> Result<(), ConfigError> { - let security = ConfigSecurity::read(path).map_err(|err| ConfigError::Write(path.to_owned(), err))?; - replace_config_using(&security, path, bytes, write) -} - -#[cfg(any(windows, target_os = "linux"))] -fn replace_config_using( - security: &ConfigSecurity, - path: &Path, - bytes: &[u8], - write: impl FnOnce(&mut std::fs::File, &[u8]) -> std::io::Result<()>, ) -> Result<(), ConfigError> { use std::sync::atomic::{AtomicU64, Ordering}; static NEXT: AtomicU64 = AtomicU64::new(0); + let security = ConfigSecurity::read(path).map_err(|err| ConfigError::Write(path.to_owned(), err))?; let parent = path.parent().unwrap_or(Path::new(".")); let (temporary, mut file) = loop { let temporary = parent.join(format!( @@ -596,32 +586,21 @@ use config_security_windows::ConfigSecurity; mod config_security_windows { use std::os::windows::{ffi::OsStrExt as _, io::FromRawHandle as _}; use std::path::Path; - use std::ptr::{null, null_mut}; + use std::ptr::null_mut; use windows_sys::Win32::Foundation::{ - ERROR_FILE_NOT_FOUND, ERROR_INVALID_OWNER, ERROR_SUCCESS, GENERIC_WRITE, INVALID_HANDLE_VALUE, LocalFree, + ERROR_FILE_NOT_FOUND, ERROR_SUCCESS, GENERIC_WRITE, INVALID_HANDLE_VALUE, LocalFree, }; use windows_sys::Win32::Security::Authorization::{ ConvertStringSecurityDescriptorToSecurityDescriptorW, GetNamedSecurityInfoW, SDDL_REVISION_1, SE_FILE_OBJECT, }; use windows_sys::Win32::Security::{ - DACL_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION, GetSecurityDescriptorDacl, GetSecurityDescriptorGroup, - InitializeSecurityDescriptor, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, SE_DACL_PROTECTED, - SECURITY_ATTRIBUTES, SECURITY_DESCRIPTOR, SetSecurityDescriptorControl, SetSecurityDescriptorDacl, - SetSecurityDescriptorGroup, + DACL_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, + SE_DACL_PROTECTED, SECURITY_ATTRIBUTES, SetSecurityDescriptorControl, }; - use windows_sys::Win32::Storage::FileSystem::{ - CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, - FILE_WRITE_DATA, OPEN_EXISTING, READ_CONTROL, WRITE_DAC, - }; - - /// `SECURITY_DESCRIPTOR_REVISION`, which windows-sys keeps behind a feature nothing else here needs. - const DESCRIPTOR_REVISION: u32 = 1; + use windows_sys::Win32::Storage::FileSystem::{CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_NORMAL}; // Own the snapshot until CreateFileW has copied it. Never create a readable temporary and tighten it later. - pub(super) struct ConfigSecurity { - descriptor: PSECURITY_DESCRIPTOR, - destination: Vec, - } + pub(super) struct ConfigSecurity(PSECURITY_DESCRIPTOR); impl ConfigSecurity { pub(super) fn read(path: &Path) -> std::io::Result { @@ -660,86 +639,40 @@ mod config_security_windows { } else if status != ERROR_SUCCESS { return Err(std::io::Error::from_raw_os_error(status as i32)); } - Self::protected(descriptor, path) - } - - fn protected(descriptor: PSECURITY_DESCRIPTOR, destination: Vec) -> std::io::Result { - let security = Self { - descriptor, - destination, - }; + let security = Self(descriptor); // Freeze the existing access rules: inheriting new parent ACEs could broaden access on replacement. // SAFETY: descriptor was returned by the security API and remains owned by security. - if unsafe { SetSecurityDescriptorControl(security.descriptor, SE_DACL_PROTECTED, SE_DACL_PROTECTED) } == 0 { + if unsafe { SetSecurityDescriptorControl(security.0, SE_DACL_PROTECTED, SE_DACL_PROTECTED) } == 0 { return Err(std::io::Error::last_os_error()); } Ok(security) } - /// A snapshot as [`ConfigSecurity::read`] would take it, for an owner a test can't give a real file. Takes - /// ownership of a descriptor allocated by the security API. - #[cfg(test)] - pub(super) fn from_descriptor(descriptor: PSECURITY_DESCRIPTOR, destination: &Path) -> Self { - let destination = destination.as_os_str().encode_wide().chain(Some(0)).collect(); - Self::protected(descriptor, destination).unwrap() - } - - /// Windows refuses to create a file with an owner the token can't assign (1307), as with an - /// Administrators-owned config under UAC's filtered token. The temporary then keeps the group and DACL and takes - /// the token's default owner, but only for a caller already able to rewrite the destination's bytes and DACL, - /// which is what owning it would add. Anyone else still gets the 1307. pub(super) fn create(&self, path: &Path) -> std::io::Result { let path: Vec = path.as_os_str().encode_wide().chain(Some(0)).collect(); - match create_temporary(&path, self.descriptor) { - Err(err) if err.raw_os_error() == Some(ERROR_INVALID_OWNER as i32) && self.caller_can_rewrite() => { - self.create_with_default_owner(&path) - } - result => result, - } - } - - /// Whether the destination opens for writing its bytes and its DACL. Opening changes nothing. - fn caller_can_rewrite(&self) -> bool { - // SAFETY: the destination is terminated and no security attributes or template are passed. + let attributes = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: self.0, + bInheritHandle: 0, + }; + // SAFETY: path and security descriptor remain valid for the call. CREATE_NEW never opens another file. let handle = unsafe { CreateFileW( - self.destination.as_ptr(), - FILE_WRITE_DATA | READ_CONTROL | WRITE_DAC, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - null(), - OPEN_EXISTING, + path.as_ptr(), + GENERIC_WRITE, + 0, + &attributes, + CREATE_NEW, FILE_ATTRIBUTE_NORMAL, null_mut(), ) }; if handle == INVALID_HANDLE_VALUE { - return false; - } - // SAFETY: the valid handle has no other owner. File closes it. - drop(unsafe { std::fs::File::from_raw_handle(handle as _) }); - true - } - - fn create_with_default_owner(&self, path: &[u16]) -> std::io::Result { - // SAFETY: all-zero is a valid SECURITY_DESCRIPTOR, and InitializeSecurityDescriptor fills it in below. - let mut storage: SECURITY_DESCRIPTOR = unsafe { std::mem::zeroed() }; - let descriptor: PSECURITY_DESCRIPTOR = (&raw mut storage).cast(); - let (mut group, mut group_defaulted) = (null_mut(), 0); - let (mut dacl_present, mut dacl, mut dacl_defaulted) = (0, null_mut(), 0); - // SAFETY: the outputs are valid. The new descriptor only points into self.descriptor, which outlives it. - let built = unsafe { - InitializeSecurityDescriptor(descriptor, DESCRIPTOR_REVISION) != 0 - && GetSecurityDescriptorGroup(self.descriptor, &mut group, &mut group_defaulted) != 0 - && SetSecurityDescriptorGroup(descriptor, group, group_defaulted) != 0 - && GetSecurityDescriptorDacl(self.descriptor, &mut dacl_present, &mut dacl, &mut dacl_defaulted) - != 0 - && SetSecurityDescriptorDacl(descriptor, dacl_present, dacl, dacl_defaulted) != 0 - && SetSecurityDescriptorControl(descriptor, SE_DACL_PROTECTED, SE_DACL_PROTECTED) != 0 - }; - if !built { - return Err(std::io::Error::last_os_error()); + Err(std::io::Error::last_os_error()) + } else { + // SAFETY: the valid handle has no other owner. File closes it. + Ok(unsafe { std::fs::File::from_raw_handle(handle as _) }) } - create_temporary(path, descriptor) } pub(super) fn apply(&self, _file: &std::fs::File) -> std::io::Result<()> { @@ -747,36 +680,10 @@ mod config_security_windows { } } - fn create_temporary(path: &[u16], descriptor: PSECURITY_DESCRIPTOR) -> std::io::Result { - let attributes = SECURITY_ATTRIBUTES { - nLength: std::mem::size_of::() as u32, - lpSecurityDescriptor: descriptor, - bInheritHandle: 0, - }; - // SAFETY: path and security descriptor remain valid for the call. CREATE_NEW never opens another file. - let handle = unsafe { - CreateFileW( - path.as_ptr(), - GENERIC_WRITE, - 0, - &attributes, - CREATE_NEW, - FILE_ATTRIBUTE_NORMAL, - null_mut(), - ) - }; - if handle == INVALID_HANDLE_VALUE { - Err(std::io::Error::last_os_error()) - } else { - // SAFETY: the valid handle has no other owner. File closes it. - Ok(unsafe { std::fs::File::from_raw_handle(handle as _) }) - } - } - impl Drop for ConfigSecurity { fn drop(&mut self) { // SAFETY: allocated by GetNamedSecurityInfoW or ConvertStringSecurityDescriptorToSecurityDescriptorW. - unsafe { LocalFree(self.descriptor as _) }; + unsafe { LocalFree(self.0 as _) }; } } } diff --git a/crates/dashboard-core/src/config_atomic_73.rs b/crates/dashboard-core/src/config_atomic_73.rs index d79ed18..002fd98 100644 --- a/crates/dashboard-core/src/config_atomic_73.rs +++ b/crates/dashboard-core/src/config_atomic_73.rs @@ -404,435 +404,3 @@ fn compatibility_in_place_helper_preserves_existing_file_and_write_errors() { assert!(matches!(write_config_in_place(&invalid, b"new"), Err(ConfigError::Write(path, _)) if path == invalid)); dir.assert_clean(); } - -// A process can name itself or an owner-capable group as a file's owner, and nothing else. A UAC-filtered token holds -// Administrators as deny-only, so it can read an Administrators-owned file but can't create one. -#[cfg(windows)] -mod owner_seam { - use super::*; - use std::os::windows::ffi::OsStrExt as _; - use std::os::windows::io::FromRawHandle as _; - use std::ptr::{null, null_mut}; - use windows_sys::Win32::Foundation::{ - CloseHandle, ERROR_SUCCESS, GENERIC_WRITE, HANDLE, INVALID_HANDLE_VALUE, LocalFree, - }; - use windows_sys::Win32::Security::Authorization::{ - ConvertSidToStringSidW, ConvertStringSecurityDescriptorToSecurityDescriptorW, ConvertStringSidToSidW, - SDDL_REVISION_1, SE_FILE_OBJECT, SetNamedSecurityInfoW, - }; - use windows_sys::Win32::Security::{ - ACL, CreateRestrictedToken, DACL_SECURITY_INFORMATION, GetSecurityDescriptorDacl, GetTokenInformation, - ImpersonateLoggedOnUser, PROTECTED_DACL_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, PSID, RevertToSelf, - SECURITY_ATTRIBUTES, SID_AND_ATTRIBUTES, TOKEN_ASSIGN_PRIMARY, TOKEN_DUPLICATE, TOKEN_INFORMATION_CLASS, - TOKEN_QUERY, TokenPrimaryGroup, TokenUser, - }; - use windows_sys::Win32::Storage::FileSystem::{CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_NORMAL}; - - // Win32_System_Threading is not a dashboard-core feature, and tests alone need this one call. - #[link(name = "advapi32")] - unsafe extern "system" { - fn OpenProcessToken(process: HANDLE, access: u32, token: *mut HANDLE) -> i32; - } - - fn wide(text: impl AsRef) -> Vec { - text.as_ref().encode_wide().chain(Some(0)).collect() - } - - fn process_token() -> HANDLE { - let mut token = null_mut(); - // SAFETY: -1 is the documented current-process pseudo handle, and token is a valid output. - let opened = unsafe { - OpenProcessToken( - -1isize as HANDLE, - TOKEN_QUERY | TOKEN_DUPLICATE | TOKEN_ASSIGN_PRIMARY, - &mut token, - ) - }; - assert_ne!(opened, 0, "{}", std::io::Error::last_os_error()); - token - } - - /// The string form of a SID the process token holds; each information class used here leads with a SID pointer. - fn token_sid(class: TOKEN_INFORMATION_CLASS) -> String { - let token = process_token(); - let mut buffer = vec![0u64; 128]; - let mut length = 0; - // SAFETY: the buffer is aligned, its byte size is passed, and length is a valid output. - let read = unsafe { - GetTokenInformation( - token, - class, - buffer.as_mut_ptr().cast(), - (buffer.len() * 8) as u32, - &mut length, - ) - }; - assert_ne!(read, 0, "{}", std::io::Error::last_os_error()); - let sid = buffer[0] as usize as PSID; - let mut text = null_mut(); - // SAFETY: sid points into buffer; LocalFree releases the returned text. - assert_ne!(unsafe { ConvertSidToStringSidW(sid, &mut text) }, 0); - // SAFETY: the API returned a terminated string; the text and the token handle are owned here. - unsafe { - let length = (0..).take_while(|&i| *text.add(i) != 0).count(); - let value = String::from_utf16_lossy(std::slice::from_raw_parts(text, length)); - LocalFree(text as _); - CloseHandle(token); - value - } - } - - pub(super) fn user() -> String { - token_sid(TokenUser) - } - - pub(super) fn primary_group() -> String { - token_sid(TokenPrimaryGroup) - } - - pub(super) fn descriptor(sddl: &str) -> PSECURITY_DESCRIPTOR { - let text = wide(sddl); - let mut descriptor = null_mut(); - // SAFETY: terminated SDDL and valid output; the caller releases the descriptor with LocalFree. - let converted = unsafe { - ConvertStringSecurityDescriptorToSecurityDescriptorW( - text.as_ptr(), - SDDL_REVISION_1, - &mut descriptor, - null_mut(), - ) - }; - assert_ne!(converted, 0, "{sddl}: {}", std::io::Error::last_os_error()); - descriptor - } - - /// Replaces a fixture's DACL with a protected one, leaving its owner alone. - pub(super) fn set_dacl(path: &Path, sddl: &str) { - let descriptor = descriptor(sddl); - let (mut present, mut defaulted, mut dacl): (i32, i32, *mut ACL) = (0, 0, null_mut()); - // SAFETY: valid descriptor and outputs; dacl points into descriptor, which outlives its use. - let status = unsafe { - assert_ne!( - GetSecurityDescriptorDacl(descriptor, &mut present, &mut dacl, &mut defaulted), - 0 - ); - let status = SetNamedSecurityInfoW( - wide(path).as_ptr(), - SE_FILE_OBJECT, - DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION, - null_mut(), - null_mut(), - dacl, - null(), - ); - LocalFree(descriptor as _); - status - }; - assert_eq!(status, ERROR_SUCCESS, "{}", path.display()); - } - - /// CREATE_NEW with exactly this descriptor and the thread's effective token. - pub(super) fn create_with(path: &Path, sddl: &str) -> std::io::Result { - let descriptor = descriptor(sddl); - let attributes = SECURITY_ATTRIBUTES { - nLength: std::mem::size_of::() as u32, - lpSecurityDescriptor: descriptor, - bInheritHandle: 0, - }; - let path = wide(path); - // SAFETY: path and descriptor stay valid for the call; a valid returned handle has no other owner. - unsafe { - let handle = CreateFileW( - path.as_ptr(), - GENERIC_WRITE, - 0, - &attributes, - CREATE_NEW, - FILE_ATTRIBUTE_NORMAL, - null_mut(), - ); - let result = if handle == INVALID_HANDLE_VALUE { - Err(std::io::Error::last_os_error()) - } else { - Ok(std::fs::File::from_raw_handle(handle as _)) - }; - LocalFree(descriptor as _); - result - } - } - - /// While alive, the current thread uses a copy of the process token with `deny_only` SIDs disabled and access - /// limited by `restrict_to`. Reverts on drop, so a failing assert can't leak it into the next test on this thread. - pub(super) struct Impersonation(HANDLE); - - impl Impersonation { - pub(super) fn begin(deny_only: &[&str], restrict_to: &[&str]) -> Self { - let sids = |list: &[&str]| -> Vec { - list.iter() - .map(|sid| { - let mut value = null_mut(); - // SAFETY: terminated text and valid output; released below, after the token exists. - let converted = unsafe { ConvertStringSidToSidW(wide(sid).as_ptr(), &mut value) }; - assert_ne!(converted, 0, "{sid}"); - SID_AND_ATTRIBUTES { - Sid: value, - Attributes: 0, - } - }) - .collect() - }; - let (disable, restrict) = (sids(deny_only), sids(restrict_to)); - let source = process_token(); - let mut token = null_mut(); - // SAFETY: both arrays and their counts are valid for the call, and token is a valid output. - let created = unsafe { - CreateRestrictedToken( - source, - 0, - disable.len() as u32, - disable.as_ptr(), - 0, - null(), - restrict.len() as u32, - restrict.as_ptr(), - &mut token, - ) - }; - let error = std::io::Error::last_os_error(); - // SAFETY: the SIDs came from ConvertStringSidToSidW and the handle from OpenProcessToken. - unsafe { - for sid in disable.iter().chain(&restrict) { - LocalFree(sid.Sid as _); - } - CloseHandle(source); - } - assert_ne!(created, 0, "{error}"); - // SAFETY: token is a valid primary token for this user; RevertToSelf ends it in Drop. - let impersonating = unsafe { ImpersonateLoggedOnUser(token) }; - assert_ne!(impersonating, 0, "{}", std::io::Error::last_os_error()); - Self(token) - } - } - - impl Drop for Impersonation { - fn drop(&mut self) { - // SAFETY: reverts this thread's own impersonation and closes the handle this guard owns. - unsafe { - RevertToSelf(); - CloseHandle(self.0); - } - } - } -} - -#[cfg(windows)] -mod unassignable_owner { - use super::owner_seam::{Impersonation, create_with, descriptor, primary_group, set_dacl, user}; - use super::*; - - const ADMINISTRATORS: &str = "S-1-5-32-544"; - /// What Windows reports for an owner it won't assign: "This security ID may not be assigned as the owner". - const INVALID_OWNER: i32 = 1307; - - /// The descriptor shape read from the reported config: an owner the logged-in user may not hold, inherited ACEs, - /// and full control for that user. - fn snapshot(owner: &str, destination: &Path) -> ConfigSecurity { - snapshot_with_group(owner, &primary_group(), destination) - } - - fn snapshot_with_group(owner: &str, group: &str, destination: &Path) -> ConfigSecurity { - let user = user(); - ConfigSecurity::from_descriptor( - descriptor(&format!( - "O:{owner}G:{group}D:AI(A;ID;FA;;;SY)(A;ID;FA;;;BA)(A;ID;FA;;;{user})" - )), - destination, - ) - } - - /// The group and DACL, without the owner. - fn group_and_dacl(path: &Path) -> String { - let security = windows_security(path); - security.split_once("G:").unwrap().1.to_owned() - } - - fn owner(path: &Path) -> String { - let security = windows_security(path); - security.split_once("G:").unwrap().0.to_owned() - } - - fn save(security: &ConfigSecurity, path: &Path, bytes: &[u8]) -> Result<(), ConfigError> { - replace_config_using(security, path, bytes, |file, bytes| file.write_all(bytes)) - } - - fn os_error(result: Result<(), ConfigError>) -> Option { - match result { - Err(ConfigError::Write(_, err)) => err.raw_os_error(), - other => panic!("expected a write error, got {other:?}"), - } - } - - #[test] - fn a_filtered_token_replaces_a_config_snapshotted_with_an_administrators_owner() { - let (dir, control) = (Directory::new(), Directory::new()); - std::fs::write(dir.path(), "view = 'project'\n").unwrap(); - std::fs::write(control.path(), "view = 'project'\n").unwrap(); - let mut poll = ConfigFile::new(&dir.0); - assert!(poll.changed().is_none()); - assert_eq!(poll.changed().unwrap().unwrap().view, View::Project); - - let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); - let replacement = b"# new\nview = 'fleet'\n"; - save(&snapshot("BA", &dir.path()), &dir.path(), replacement).unwrap(); - // The same snapshot with an owner Windows accepts: only the owner may differ. - save(&snapshot(&user(), &control.path()), &control.path(), replacement).unwrap(); - - assert_eq!(std::fs::read(dir.path()).unwrap(), replacement); - assert_eq!(group_and_dacl(&dir.path()), group_and_dacl(&control.path())); - // SDDL prints an alias for some accounts, so compare what Windows reports: the owner ends up as the user. - assert_eq!(owner(&dir.path()), owner(&control.path())); - assert_ne!(owner(&dir.path()), "O:BA"); - assert!(poll.changed().is_none()); - assert_eq!(poll.changed().unwrap().unwrap().view, View::Fleet); - dir.assert_clean(); - } - - #[test] - fn the_snapshot_group_survives_when_the_owner_cannot() { - let dir = Directory::new(); - std::fs::write( - dir.path(), - "view = 'project' -", - ) - .unwrap(); - let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); - // SYSTEM is not the token's own primary group, so a dropped group would show. - let security = snapshot_with_group("BA", "SY", &dir.path()); - save( - &security, - &dir.path(), - b"view = 'fleet' -", - ) - .unwrap(); - assert!(group_and_dacl(&dir.path()).starts_with("SYD:")); - } - - #[test] - fn an_owner_it_cannot_assign_is_still_refused_without_the_right_to_change_the_dacl() { - let dir = Directory::new(); - let original = b"view = 'project'\n"; - std::fs::write(dir.path(), original).unwrap(); - // Read, write and delete, but no WRITE_DAC: OWNER RIGHTS removes the owner's implicit one. An elevated host owns - // this file as Administrators, which the filtered token below can't use, so the user needs its own entry. - set_dacl(&dir.path(), &format!("D:P(A;;0x1301bf;;;OW)(A;;0x1301bf;;;{})", user())); - let security_before = windows_security(&dir.path()); - - let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); - let result = save(&snapshot("BA", &dir.path()), &dir.path(), b"view = 'fleet'\n"); - - assert_eq!(os_error(result), Some(INVALID_OWNER)); - assert_eq!(std::fs::read(dir.path()).unwrap(), original); - assert_eq!(windows_security(&dir.path()), security_before); - dir.assert_clean(); - } - - #[test] - fn a_failed_replacement_past_the_owner_keeps_the_original_bytes_and_security() { - use std::os::windows::fs::OpenOptionsExt as _; - use windows_sys::Win32::Storage::FileSystem::{FILE_SHARE_READ, FILE_SHARE_WRITE}; - let dir = Directory::new(); - let original = b"view = 'project'\n"; - std::fs::write(dir.path(), original).unwrap(); - let security_before = windows_security(&dir.path()); - let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); - let security = snapshot("BA", &dir.path()); - - // The write fails after a partial prefix, with polling running against the file meanwhile. - let mut poll = ConfigFile::new(&dir.0); - assert!(poll.changed().is_none()); - assert!(poll.changed().unwrap().is_ok()); - let result = replace_config_using(&security, &dir.path(), b"view = 'fleet'\n", |file, _| { - file.write_all(b"view = ")?; - assert!(poll.changed().is_none()); - Err(std::io::Error::other("forced failure after partial write")) - }); - assert!(matches!(result, Err(ConfigError::Write(_, err)) if err.to_string().contains("forced failure"))); - assert_eq!(std::fs::read(dir.path()).unwrap(), original); - assert_eq!(windows_security(&dir.path()), security_before); - dir.assert_clean(); - - // The rename fails: this handle doesn't share delete access. - let held = std::fs::OpenOptions::new() - .read(true) - .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE) - .open(dir.path()) - .unwrap(); - let result = save(&security, &dir.path(), b"view = 'fleet'\n"); - assert_ne!(os_error(result), Some(INVALID_OWNER)); - drop(held); - assert_eq!(std::fs::read(dir.path()).unwrap(), original); - assert_eq!(windows_security(&dir.path()), security_before); - assert!(poll.changed().is_none()); - dir.assert_clean(); - } - - /// The reported state in full, which only an elevated process can set up: a real file owned by Administrators, - /// saved with Administrators deny-only, as under UAC. GitHub's Windows runners are elevated. A non-elevated host - /// gets no fixture and skips; the snapshot tests above still run there, on its real filtered token. - #[test] - fn every_save_replaces_a_real_administrators_owned_config_under_a_filtered_token() { - type Save = Box Result<(), ConfigError>>; - let user = user(); - let text = "# keep\nview = 'project'\nrepositories = []\n"; - let sddl = format!("O:BAD:P(A;;FA;;;{user})(A;;FA;;;SY)"); - let saves: [(&str, Save); 3] = [ - ("save_view", Box::new(|dir| Config::save_view(dir, View::Fleet))), - ( - "add_repository", - Box::new(|dir| Config::add_repository(dir, Path::new("a")).map(|_| ())), - ), - ("save", Box::new(|dir| Config::default().save(dir))), - ]; - for (name, entry_point) in saves { - let dir = Directory::new(); - match create_with(&dir.path(), &sddl) { - Ok(mut file) => file.write_all(text.as_bytes()).unwrap(), - Err(err) if err.raw_os_error() == Some(INVALID_OWNER) => { - assert!( - std::env::var_os("GITHUB_ACTIONS").is_none(), - "CI must run this test elevated: {err}" - ); - eprintln!("skipped: creating an Administrators-owned fixture needs an elevated process"); - return; - } - Err(err) => panic!("{err}"), - } - let before = windows_security(&dir.path()); - assert!(before.starts_with("O:BA"), "{before}"); - let dacl = before.split_once("D:").unwrap().1.to_owned(); - let mut poll = ConfigFile::new(&dir.0); - assert!(poll.changed().is_none()); - assert!(poll.changed().unwrap().is_ok()); - - let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); - // The token now behaves as the reported one: it can't name Administrators as an owner. - let probe = dir.0.join("probe"); - assert_eq!( - create_with(&probe, &sddl).unwrap_err().raw_os_error(), - Some(INVALID_OWNER) - ); - - entry_point(&dir.0).unwrap_or_else(|err| panic!("{name}: {err}")); - - let after = windows_security(&dir.path()); - assert_ne!(std::fs::read_to_string(dir.path()).unwrap(), text, "{name}"); - assert_eq!(after.split_once("D:").unwrap().1, dacl, "{name}"); - assert!(!after.starts_with("O:BA"), "{name}: {after}"); - assert!(poll.changed().is_none()); - assert!(poll.changed().unwrap().is_ok()); - dir.assert_clean(); - } - } -} From 86833c33ce14fac1ae310645504e0497a72c93a4 Mon Sep 17 00:00:00 2001 From: Franz Hemmer Date: Thu, 5 Nov 2026 09:24:10 -0500 Subject: [PATCH 3/4] Restore the #84 save code and fill the Fleet fixture Removes the temporary commit, which showed the banner on every view. The Fleet fixture was missing fields, so its rows read Stale and Unknown. Co-Authored-By: Claude Opus 5.5 --- crates/dashboard-core/src/config.rs | 143 +++++- crates/dashboard-core/src/config_atomic_73.rs | 432 ++++++++++++++++++ .../native-capture/Invoke-ViewSaveCapture.ps1 | 2 +- scripts/native-capture/Start-LoopbackPmd.ps1 | 4 +- 4 files changed, 553 insertions(+), 28 deletions(-) diff --git a/crates/dashboard-core/src/config.rs b/crates/dashboard-core/src/config.rs index f862fd5..42739dd 100644 --- a/crates/dashboard-core/src/config.rs +++ b/crates/dashboard-core/src/config.rs @@ -463,10 +463,20 @@ fn replace_config_with( path: &Path, bytes: &[u8], write: impl FnOnce(&mut std::fs::File, &[u8]) -> std::io::Result<()>, +) -> Result<(), ConfigError> { + let security = ConfigSecurity::read(path).map_err(|err| ConfigError::Write(path.to_owned(), err))?; + replace_config_using(&security, path, bytes, write) +} + +#[cfg(any(windows, target_os = "linux"))] +fn replace_config_using( + security: &ConfigSecurity, + path: &Path, + bytes: &[u8], + write: impl FnOnce(&mut std::fs::File, &[u8]) -> std::io::Result<()>, ) -> Result<(), ConfigError> { use std::sync::atomic::{AtomicU64, Ordering}; static NEXT: AtomicU64 = AtomicU64::new(0); - let security = ConfigSecurity::read(path).map_err(|err| ConfigError::Write(path.to_owned(), err))?; let parent = path.parent().unwrap_or(Path::new(".")); let (temporary, mut file) = loop { let temporary = parent.join(format!( @@ -586,21 +596,32 @@ use config_security_windows::ConfigSecurity; mod config_security_windows { use std::os::windows::{ffi::OsStrExt as _, io::FromRawHandle as _}; use std::path::Path; - use std::ptr::null_mut; + use std::ptr::{null, null_mut}; use windows_sys::Win32::Foundation::{ - ERROR_FILE_NOT_FOUND, ERROR_SUCCESS, GENERIC_WRITE, INVALID_HANDLE_VALUE, LocalFree, + ERROR_FILE_NOT_FOUND, ERROR_INVALID_OWNER, ERROR_SUCCESS, GENERIC_WRITE, INVALID_HANDLE_VALUE, LocalFree, }; use windows_sys::Win32::Security::Authorization::{ ConvertStringSecurityDescriptorToSecurityDescriptorW, GetNamedSecurityInfoW, SDDL_REVISION_1, SE_FILE_OBJECT, }; use windows_sys::Win32::Security::{ - DACL_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, - SE_DACL_PROTECTED, SECURITY_ATTRIBUTES, SetSecurityDescriptorControl, + DACL_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION, GetSecurityDescriptorDacl, GetSecurityDescriptorGroup, + InitializeSecurityDescriptor, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, SE_DACL_PROTECTED, + SECURITY_ATTRIBUTES, SECURITY_DESCRIPTOR, SetSecurityDescriptorControl, SetSecurityDescriptorDacl, + SetSecurityDescriptorGroup, }; - use windows_sys::Win32::Storage::FileSystem::{CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_NORMAL}; + use windows_sys::Win32::Storage::FileSystem::{ + CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, + FILE_WRITE_DATA, OPEN_EXISTING, READ_CONTROL, WRITE_DAC, + }; + + /// `SECURITY_DESCRIPTOR_REVISION`, which windows-sys keeps behind a feature nothing else here needs. + const DESCRIPTOR_REVISION: u32 = 1; // Own the snapshot until CreateFileW has copied it. Never create a readable temporary and tighten it later. - pub(super) struct ConfigSecurity(PSECURITY_DESCRIPTOR); + pub(super) struct ConfigSecurity { + descriptor: PSECURITY_DESCRIPTOR, + destination: Vec, + } impl ConfigSecurity { pub(super) fn read(path: &Path) -> std::io::Result { @@ -639,40 +660,86 @@ mod config_security_windows { } else if status != ERROR_SUCCESS { return Err(std::io::Error::from_raw_os_error(status as i32)); } - let security = Self(descriptor); + Self::protected(descriptor, path) + } + + fn protected(descriptor: PSECURITY_DESCRIPTOR, destination: Vec) -> std::io::Result { + let security = Self { + descriptor, + destination, + }; // Freeze the existing access rules: inheriting new parent ACEs could broaden access on replacement. // SAFETY: descriptor was returned by the security API and remains owned by security. - if unsafe { SetSecurityDescriptorControl(security.0, SE_DACL_PROTECTED, SE_DACL_PROTECTED) } == 0 { + if unsafe { SetSecurityDescriptorControl(security.descriptor, SE_DACL_PROTECTED, SE_DACL_PROTECTED) } == 0 { return Err(std::io::Error::last_os_error()); } Ok(security) } + /// A snapshot as [`ConfigSecurity::read`] would take it, for an owner a test can't give a real file. Takes + /// ownership of a descriptor allocated by the security API. + #[cfg(test)] + pub(super) fn from_descriptor(descriptor: PSECURITY_DESCRIPTOR, destination: &Path) -> Self { + let destination = destination.as_os_str().encode_wide().chain(Some(0)).collect(); + Self::protected(descriptor, destination).unwrap() + } + + /// Windows refuses to create a file with an owner the token can't assign (1307), as with an + /// Administrators-owned config under UAC's filtered token. The temporary then keeps the group and DACL and takes + /// the token's default owner, but only for a caller already able to rewrite the destination's bytes and DACL, + /// which is what owning it would add. Anyone else still gets the 1307. pub(super) fn create(&self, path: &Path) -> std::io::Result { let path: Vec = path.as_os_str().encode_wide().chain(Some(0)).collect(); - let attributes = SECURITY_ATTRIBUTES { - nLength: std::mem::size_of::() as u32, - lpSecurityDescriptor: self.0, - bInheritHandle: 0, - }; - // SAFETY: path and security descriptor remain valid for the call. CREATE_NEW never opens another file. + match create_temporary(&path, self.descriptor) { + Err(err) if err.raw_os_error() == Some(ERROR_INVALID_OWNER as i32) && self.caller_can_rewrite() => { + self.create_with_default_owner(&path) + } + result => result, + } + } + + /// Whether the destination opens for writing its bytes and its DACL. Opening changes nothing. + fn caller_can_rewrite(&self) -> bool { + // SAFETY: the destination is terminated and no security attributes or template are passed. let handle = unsafe { CreateFileW( - path.as_ptr(), - GENERIC_WRITE, - 0, - &attributes, - CREATE_NEW, + self.destination.as_ptr(), + FILE_WRITE_DATA | READ_CONTROL | WRITE_DAC, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + null(), + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, null_mut(), ) }; if handle == INVALID_HANDLE_VALUE { - Err(std::io::Error::last_os_error()) - } else { - // SAFETY: the valid handle has no other owner. File closes it. - Ok(unsafe { std::fs::File::from_raw_handle(handle as _) }) + return false; } + // SAFETY: the valid handle has no other owner. File closes it. + drop(unsafe { std::fs::File::from_raw_handle(handle as _) }); + true + } + + fn create_with_default_owner(&self, path: &[u16]) -> std::io::Result { + // SAFETY: all-zero is a valid SECURITY_DESCRIPTOR, and InitializeSecurityDescriptor fills it in below. + let mut storage: SECURITY_DESCRIPTOR = unsafe { std::mem::zeroed() }; + let descriptor: PSECURITY_DESCRIPTOR = (&raw mut storage).cast(); + let (mut group, mut group_defaulted) = (null_mut(), 0); + let (mut dacl_present, mut dacl, mut dacl_defaulted) = (0, null_mut(), 0); + // SAFETY: the outputs are valid. The new descriptor only points into self.descriptor, which outlives it. + let built = unsafe { + InitializeSecurityDescriptor(descriptor, DESCRIPTOR_REVISION) != 0 + && GetSecurityDescriptorGroup(self.descriptor, &mut group, &mut group_defaulted) != 0 + && SetSecurityDescriptorGroup(descriptor, group, group_defaulted) != 0 + && GetSecurityDescriptorDacl(self.descriptor, &mut dacl_present, &mut dacl, &mut dacl_defaulted) + != 0 + && SetSecurityDescriptorDacl(descriptor, dacl_present, dacl, dacl_defaulted) != 0 + && SetSecurityDescriptorControl(descriptor, SE_DACL_PROTECTED, SE_DACL_PROTECTED) != 0 + }; + if !built { + return Err(std::io::Error::last_os_error()); + } + create_temporary(path, descriptor) } pub(super) fn apply(&self, _file: &std::fs::File) -> std::io::Result<()> { @@ -680,10 +747,36 @@ mod config_security_windows { } } + fn create_temporary(path: &[u16], descriptor: PSECURITY_DESCRIPTOR) -> std::io::Result { + let attributes = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: descriptor, + bInheritHandle: 0, + }; + // SAFETY: path and security descriptor remain valid for the call. CREATE_NEW never opens another file. + let handle = unsafe { + CreateFileW( + path.as_ptr(), + GENERIC_WRITE, + 0, + &attributes, + CREATE_NEW, + FILE_ATTRIBUTE_NORMAL, + null_mut(), + ) + }; + if handle == INVALID_HANDLE_VALUE { + Err(std::io::Error::last_os_error()) + } else { + // SAFETY: the valid handle has no other owner. File closes it. + Ok(unsafe { std::fs::File::from_raw_handle(handle as _) }) + } + } + impl Drop for ConfigSecurity { fn drop(&mut self) { // SAFETY: allocated by GetNamedSecurityInfoW or ConvertStringSecurityDescriptorToSecurityDescriptorW. - unsafe { LocalFree(self.0 as _) }; + unsafe { LocalFree(self.descriptor as _) }; } } } diff --git a/crates/dashboard-core/src/config_atomic_73.rs b/crates/dashboard-core/src/config_atomic_73.rs index 002fd98..d79ed18 100644 --- a/crates/dashboard-core/src/config_atomic_73.rs +++ b/crates/dashboard-core/src/config_atomic_73.rs @@ -404,3 +404,435 @@ fn compatibility_in_place_helper_preserves_existing_file_and_write_errors() { assert!(matches!(write_config_in_place(&invalid, b"new"), Err(ConfigError::Write(path, _)) if path == invalid)); dir.assert_clean(); } + +// A process can name itself or an owner-capable group as a file's owner, and nothing else. A UAC-filtered token holds +// Administrators as deny-only, so it can read an Administrators-owned file but can't create one. +#[cfg(windows)] +mod owner_seam { + use super::*; + use std::os::windows::ffi::OsStrExt as _; + use std::os::windows::io::FromRawHandle as _; + use std::ptr::{null, null_mut}; + use windows_sys::Win32::Foundation::{ + CloseHandle, ERROR_SUCCESS, GENERIC_WRITE, HANDLE, INVALID_HANDLE_VALUE, LocalFree, + }; + use windows_sys::Win32::Security::Authorization::{ + ConvertSidToStringSidW, ConvertStringSecurityDescriptorToSecurityDescriptorW, ConvertStringSidToSidW, + SDDL_REVISION_1, SE_FILE_OBJECT, SetNamedSecurityInfoW, + }; + use windows_sys::Win32::Security::{ + ACL, CreateRestrictedToken, DACL_SECURITY_INFORMATION, GetSecurityDescriptorDacl, GetTokenInformation, + ImpersonateLoggedOnUser, PROTECTED_DACL_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, PSID, RevertToSelf, + SECURITY_ATTRIBUTES, SID_AND_ATTRIBUTES, TOKEN_ASSIGN_PRIMARY, TOKEN_DUPLICATE, TOKEN_INFORMATION_CLASS, + TOKEN_QUERY, TokenPrimaryGroup, TokenUser, + }; + use windows_sys::Win32::Storage::FileSystem::{CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_NORMAL}; + + // Win32_System_Threading is not a dashboard-core feature, and tests alone need this one call. + #[link(name = "advapi32")] + unsafe extern "system" { + fn OpenProcessToken(process: HANDLE, access: u32, token: *mut HANDLE) -> i32; + } + + fn wide(text: impl AsRef) -> Vec { + text.as_ref().encode_wide().chain(Some(0)).collect() + } + + fn process_token() -> HANDLE { + let mut token = null_mut(); + // SAFETY: -1 is the documented current-process pseudo handle, and token is a valid output. + let opened = unsafe { + OpenProcessToken( + -1isize as HANDLE, + TOKEN_QUERY | TOKEN_DUPLICATE | TOKEN_ASSIGN_PRIMARY, + &mut token, + ) + }; + assert_ne!(opened, 0, "{}", std::io::Error::last_os_error()); + token + } + + /// The string form of a SID the process token holds; each information class used here leads with a SID pointer. + fn token_sid(class: TOKEN_INFORMATION_CLASS) -> String { + let token = process_token(); + let mut buffer = vec![0u64; 128]; + let mut length = 0; + // SAFETY: the buffer is aligned, its byte size is passed, and length is a valid output. + let read = unsafe { + GetTokenInformation( + token, + class, + buffer.as_mut_ptr().cast(), + (buffer.len() * 8) as u32, + &mut length, + ) + }; + assert_ne!(read, 0, "{}", std::io::Error::last_os_error()); + let sid = buffer[0] as usize as PSID; + let mut text = null_mut(); + // SAFETY: sid points into buffer; LocalFree releases the returned text. + assert_ne!(unsafe { ConvertSidToStringSidW(sid, &mut text) }, 0); + // SAFETY: the API returned a terminated string; the text and the token handle are owned here. + unsafe { + let length = (0..).take_while(|&i| *text.add(i) != 0).count(); + let value = String::from_utf16_lossy(std::slice::from_raw_parts(text, length)); + LocalFree(text as _); + CloseHandle(token); + value + } + } + + pub(super) fn user() -> String { + token_sid(TokenUser) + } + + pub(super) fn primary_group() -> String { + token_sid(TokenPrimaryGroup) + } + + pub(super) fn descriptor(sddl: &str) -> PSECURITY_DESCRIPTOR { + let text = wide(sddl); + let mut descriptor = null_mut(); + // SAFETY: terminated SDDL and valid output; the caller releases the descriptor with LocalFree. + let converted = unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + text.as_ptr(), + SDDL_REVISION_1, + &mut descriptor, + null_mut(), + ) + }; + assert_ne!(converted, 0, "{sddl}: {}", std::io::Error::last_os_error()); + descriptor + } + + /// Replaces a fixture's DACL with a protected one, leaving its owner alone. + pub(super) fn set_dacl(path: &Path, sddl: &str) { + let descriptor = descriptor(sddl); + let (mut present, mut defaulted, mut dacl): (i32, i32, *mut ACL) = (0, 0, null_mut()); + // SAFETY: valid descriptor and outputs; dacl points into descriptor, which outlives its use. + let status = unsafe { + assert_ne!( + GetSecurityDescriptorDacl(descriptor, &mut present, &mut dacl, &mut defaulted), + 0 + ); + let status = SetNamedSecurityInfoW( + wide(path).as_ptr(), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION, + null_mut(), + null_mut(), + dacl, + null(), + ); + LocalFree(descriptor as _); + status + }; + assert_eq!(status, ERROR_SUCCESS, "{}", path.display()); + } + + /// CREATE_NEW with exactly this descriptor and the thread's effective token. + pub(super) fn create_with(path: &Path, sddl: &str) -> std::io::Result { + let descriptor = descriptor(sddl); + let attributes = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: descriptor, + bInheritHandle: 0, + }; + let path = wide(path); + // SAFETY: path and descriptor stay valid for the call; a valid returned handle has no other owner. + unsafe { + let handle = CreateFileW( + path.as_ptr(), + GENERIC_WRITE, + 0, + &attributes, + CREATE_NEW, + FILE_ATTRIBUTE_NORMAL, + null_mut(), + ); + let result = if handle == INVALID_HANDLE_VALUE { + Err(std::io::Error::last_os_error()) + } else { + Ok(std::fs::File::from_raw_handle(handle as _)) + }; + LocalFree(descriptor as _); + result + } + } + + /// While alive, the current thread uses a copy of the process token with `deny_only` SIDs disabled and access + /// limited by `restrict_to`. Reverts on drop, so a failing assert can't leak it into the next test on this thread. + pub(super) struct Impersonation(HANDLE); + + impl Impersonation { + pub(super) fn begin(deny_only: &[&str], restrict_to: &[&str]) -> Self { + let sids = |list: &[&str]| -> Vec { + list.iter() + .map(|sid| { + let mut value = null_mut(); + // SAFETY: terminated text and valid output; released below, after the token exists. + let converted = unsafe { ConvertStringSidToSidW(wide(sid).as_ptr(), &mut value) }; + assert_ne!(converted, 0, "{sid}"); + SID_AND_ATTRIBUTES { + Sid: value, + Attributes: 0, + } + }) + .collect() + }; + let (disable, restrict) = (sids(deny_only), sids(restrict_to)); + let source = process_token(); + let mut token = null_mut(); + // SAFETY: both arrays and their counts are valid for the call, and token is a valid output. + let created = unsafe { + CreateRestrictedToken( + source, + 0, + disable.len() as u32, + disable.as_ptr(), + 0, + null(), + restrict.len() as u32, + restrict.as_ptr(), + &mut token, + ) + }; + let error = std::io::Error::last_os_error(); + // SAFETY: the SIDs came from ConvertStringSidToSidW and the handle from OpenProcessToken. + unsafe { + for sid in disable.iter().chain(&restrict) { + LocalFree(sid.Sid as _); + } + CloseHandle(source); + } + assert_ne!(created, 0, "{error}"); + // SAFETY: token is a valid primary token for this user; RevertToSelf ends it in Drop. + let impersonating = unsafe { ImpersonateLoggedOnUser(token) }; + assert_ne!(impersonating, 0, "{}", std::io::Error::last_os_error()); + Self(token) + } + } + + impl Drop for Impersonation { + fn drop(&mut self) { + // SAFETY: reverts this thread's own impersonation and closes the handle this guard owns. + unsafe { + RevertToSelf(); + CloseHandle(self.0); + } + } + } +} + +#[cfg(windows)] +mod unassignable_owner { + use super::owner_seam::{Impersonation, create_with, descriptor, primary_group, set_dacl, user}; + use super::*; + + const ADMINISTRATORS: &str = "S-1-5-32-544"; + /// What Windows reports for an owner it won't assign: "This security ID may not be assigned as the owner". + const INVALID_OWNER: i32 = 1307; + + /// The descriptor shape read from the reported config: an owner the logged-in user may not hold, inherited ACEs, + /// and full control for that user. + fn snapshot(owner: &str, destination: &Path) -> ConfigSecurity { + snapshot_with_group(owner, &primary_group(), destination) + } + + fn snapshot_with_group(owner: &str, group: &str, destination: &Path) -> ConfigSecurity { + let user = user(); + ConfigSecurity::from_descriptor( + descriptor(&format!( + "O:{owner}G:{group}D:AI(A;ID;FA;;;SY)(A;ID;FA;;;BA)(A;ID;FA;;;{user})" + )), + destination, + ) + } + + /// The group and DACL, without the owner. + fn group_and_dacl(path: &Path) -> String { + let security = windows_security(path); + security.split_once("G:").unwrap().1.to_owned() + } + + fn owner(path: &Path) -> String { + let security = windows_security(path); + security.split_once("G:").unwrap().0.to_owned() + } + + fn save(security: &ConfigSecurity, path: &Path, bytes: &[u8]) -> Result<(), ConfigError> { + replace_config_using(security, path, bytes, |file, bytes| file.write_all(bytes)) + } + + fn os_error(result: Result<(), ConfigError>) -> Option { + match result { + Err(ConfigError::Write(_, err)) => err.raw_os_error(), + other => panic!("expected a write error, got {other:?}"), + } + } + + #[test] + fn a_filtered_token_replaces_a_config_snapshotted_with_an_administrators_owner() { + let (dir, control) = (Directory::new(), Directory::new()); + std::fs::write(dir.path(), "view = 'project'\n").unwrap(); + std::fs::write(control.path(), "view = 'project'\n").unwrap(); + let mut poll = ConfigFile::new(&dir.0); + assert!(poll.changed().is_none()); + assert_eq!(poll.changed().unwrap().unwrap().view, View::Project); + + let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); + let replacement = b"# new\nview = 'fleet'\n"; + save(&snapshot("BA", &dir.path()), &dir.path(), replacement).unwrap(); + // The same snapshot with an owner Windows accepts: only the owner may differ. + save(&snapshot(&user(), &control.path()), &control.path(), replacement).unwrap(); + + assert_eq!(std::fs::read(dir.path()).unwrap(), replacement); + assert_eq!(group_and_dacl(&dir.path()), group_and_dacl(&control.path())); + // SDDL prints an alias for some accounts, so compare what Windows reports: the owner ends up as the user. + assert_eq!(owner(&dir.path()), owner(&control.path())); + assert_ne!(owner(&dir.path()), "O:BA"); + assert!(poll.changed().is_none()); + assert_eq!(poll.changed().unwrap().unwrap().view, View::Fleet); + dir.assert_clean(); + } + + #[test] + fn the_snapshot_group_survives_when_the_owner_cannot() { + let dir = Directory::new(); + std::fs::write( + dir.path(), + "view = 'project' +", + ) + .unwrap(); + let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); + // SYSTEM is not the token's own primary group, so a dropped group would show. + let security = snapshot_with_group("BA", "SY", &dir.path()); + save( + &security, + &dir.path(), + b"view = 'fleet' +", + ) + .unwrap(); + assert!(group_and_dacl(&dir.path()).starts_with("SYD:")); + } + + #[test] + fn an_owner_it_cannot_assign_is_still_refused_without_the_right_to_change_the_dacl() { + let dir = Directory::new(); + let original = b"view = 'project'\n"; + std::fs::write(dir.path(), original).unwrap(); + // Read, write and delete, but no WRITE_DAC: OWNER RIGHTS removes the owner's implicit one. An elevated host owns + // this file as Administrators, which the filtered token below can't use, so the user needs its own entry. + set_dacl(&dir.path(), &format!("D:P(A;;0x1301bf;;;OW)(A;;0x1301bf;;;{})", user())); + let security_before = windows_security(&dir.path()); + + let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); + let result = save(&snapshot("BA", &dir.path()), &dir.path(), b"view = 'fleet'\n"); + + assert_eq!(os_error(result), Some(INVALID_OWNER)); + assert_eq!(std::fs::read(dir.path()).unwrap(), original); + assert_eq!(windows_security(&dir.path()), security_before); + dir.assert_clean(); + } + + #[test] + fn a_failed_replacement_past_the_owner_keeps_the_original_bytes_and_security() { + use std::os::windows::fs::OpenOptionsExt as _; + use windows_sys::Win32::Storage::FileSystem::{FILE_SHARE_READ, FILE_SHARE_WRITE}; + let dir = Directory::new(); + let original = b"view = 'project'\n"; + std::fs::write(dir.path(), original).unwrap(); + let security_before = windows_security(&dir.path()); + let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); + let security = snapshot("BA", &dir.path()); + + // The write fails after a partial prefix, with polling running against the file meanwhile. + let mut poll = ConfigFile::new(&dir.0); + assert!(poll.changed().is_none()); + assert!(poll.changed().unwrap().is_ok()); + let result = replace_config_using(&security, &dir.path(), b"view = 'fleet'\n", |file, _| { + file.write_all(b"view = ")?; + assert!(poll.changed().is_none()); + Err(std::io::Error::other("forced failure after partial write")) + }); + assert!(matches!(result, Err(ConfigError::Write(_, err)) if err.to_string().contains("forced failure"))); + assert_eq!(std::fs::read(dir.path()).unwrap(), original); + assert_eq!(windows_security(&dir.path()), security_before); + dir.assert_clean(); + + // The rename fails: this handle doesn't share delete access. + let held = std::fs::OpenOptions::new() + .read(true) + .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE) + .open(dir.path()) + .unwrap(); + let result = save(&security, &dir.path(), b"view = 'fleet'\n"); + assert_ne!(os_error(result), Some(INVALID_OWNER)); + drop(held); + assert_eq!(std::fs::read(dir.path()).unwrap(), original); + assert_eq!(windows_security(&dir.path()), security_before); + assert!(poll.changed().is_none()); + dir.assert_clean(); + } + + /// The reported state in full, which only an elevated process can set up: a real file owned by Administrators, + /// saved with Administrators deny-only, as under UAC. GitHub's Windows runners are elevated. A non-elevated host + /// gets no fixture and skips; the snapshot tests above still run there, on its real filtered token. + #[test] + fn every_save_replaces_a_real_administrators_owned_config_under_a_filtered_token() { + type Save = Box Result<(), ConfigError>>; + let user = user(); + let text = "# keep\nview = 'project'\nrepositories = []\n"; + let sddl = format!("O:BAD:P(A;;FA;;;{user})(A;;FA;;;SY)"); + let saves: [(&str, Save); 3] = [ + ("save_view", Box::new(|dir| Config::save_view(dir, View::Fleet))), + ( + "add_repository", + Box::new(|dir| Config::add_repository(dir, Path::new("a")).map(|_| ())), + ), + ("save", Box::new(|dir| Config::default().save(dir))), + ]; + for (name, entry_point) in saves { + let dir = Directory::new(); + match create_with(&dir.path(), &sddl) { + Ok(mut file) => file.write_all(text.as_bytes()).unwrap(), + Err(err) if err.raw_os_error() == Some(INVALID_OWNER) => { + assert!( + std::env::var_os("GITHUB_ACTIONS").is_none(), + "CI must run this test elevated: {err}" + ); + eprintln!("skipped: creating an Administrators-owned fixture needs an elevated process"); + return; + } + Err(err) => panic!("{err}"), + } + let before = windows_security(&dir.path()); + assert!(before.starts_with("O:BA"), "{before}"); + let dacl = before.split_once("D:").unwrap().1.to_owned(); + let mut poll = ConfigFile::new(&dir.0); + assert!(poll.changed().is_none()); + assert!(poll.changed().unwrap().is_ok()); + + let _filtered = Impersonation::begin(&[ADMINISTRATORS], &[]); + // The token now behaves as the reported one: it can't name Administrators as an owner. + let probe = dir.0.join("probe"); + assert_eq!( + create_with(&probe, &sddl).unwrap_err().raw_os_error(), + Some(INVALID_OWNER) + ); + + entry_point(&dir.0).unwrap_or_else(|err| panic!("{name}: {err}")); + + let after = windows_security(&dir.path()); + assert_ne!(std::fs::read_to_string(dir.path()).unwrap(), text, "{name}"); + assert_eq!(after.split_once("D:").unwrap().1, dacl, "{name}"); + assert!(!after.starts_with("O:BA"), "{name}: {after}"); + assert!(poll.changed().is_none()); + assert!(poll.changed().unwrap().is_ok()); + dir.assert_clean(); + } + } +} diff --git a/scripts/native-capture/Invoke-ViewSaveCapture.ps1 b/scripts/native-capture/Invoke-ViewSaveCapture.ps1 index 259818b..862a689 100644 --- a/scripts/native-capture/Invoke-ViewSaveCapture.ps1 +++ b/scripts/native-capture/Invoke-ViewSaveCapture.ps1 @@ -207,7 +207,7 @@ t3_home = '$root' Assert-View '1-project-at-start' 'project' $projectMarker $shot $shot = Switch-View $shot 'Backlog' '2-backlog' 'backlog' 'SYNTHETIC FIXTURE' 2 # Fleet fetches on selection, and needs time to show its rows. - $shot = Switch-View $shot 'Fleet' '3-fleet' 'fleet' 'synthetic-windows' 5 + $shot = Switch-View $shot 'Fleet' '3-fleet' 'fleet' 'synthetic-' 5 $shot = Switch-View $shot 'Project' '4-project' 'project' $projectMarker 2 $after = Get-Acl -LiteralPath $configPath diff --git a/scripts/native-capture/Start-LoopbackPmd.ps1 b/scripts/native-capture/Start-LoopbackPmd.ps1 index 865a614..3ec153e 100644 --- a/scripts/native-capture/Start-LoopbackPmd.ps1 +++ b/scripts/native-capture/Start-LoopbackPmd.ps1 @@ -65,11 +65,11 @@ try { '/v1/capacity' { $body=@{machines=@(@{name='synthetic-windows';used=0;chiefStarted=0;free=6},@{name='synthetic-linux';used=0;chiefStarted=0;free=3})} } '/v1/fleet/status' { $at=[DateTimeOffset]::UtcNow.ToString('o') - $body=@{schema=1;machines=@(@{name='synthetic-windows';online=$true;probeOk=$true;checkedAt=$at},@{name='synthetic-linux';online=$true;probeOk=$true;checkedAt=$at})} + $body=@{schema=1;machines=@(@{name='synthetic-windows';online=$true;probeOk=$true;checkedAt=$at;attemptedAt=$at;serverVersion='0.0.0-fixture';latestVersion='0.0.0-fixture';clientStatus='not_applicable'},@{name='synthetic-linux';online=$true;probeOk=$true;checkedAt=$at;attemptedAt=$at;serverVersion='0.0.0-fixture';latestVersion='0.0.0-fixture';clientStatus='not_applicable'})} } '/v1/fleet/activity' { $at=[DateTimeOffset]::UtcNow.ToString('o') - $body=@{schema=1;observedAt=$at;machines=@(@{name='synthetic-windows';observedAt=$at;state='ok';activeCount=0;sessions=@()},@{name='synthetic-linux';observedAt=$at;state='ok';activeCount=0;sessions=@()})} + $body=@{schema=1;observedAt=$at;machines=@(@{name='synthetic-windows';observedAt=$at;state='ok';activeCount=0;chiefActive=$false;sessions=@()},@{name='synthetic-linux';observedAt=$at;state='ok';activeCount=0;chiefActive=$false;sessions=@()})} } default { $code=404; $body=@{error=@{code='not_found';message='synthetic fixture unknown route'}} } } From 3d66be08167cb4c4b3381abd5c6483b674b1ddbd Mon Sep 17 00:00:00 2001 From: Franz Hemmer Date: Thu, 5 Nov 2026 09:32:21 -0500 Subject: [PATCH 4/4] Start the view-save capture with the quotes a save writes Saves write the view value in double quotes, so the round-trip byte check failed on the fixture's single quotes. All four views had already passed. Co-Authored-By: Claude Opus 5.5 --- scripts/native-capture/Invoke-ViewSaveCapture.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/native-capture/Invoke-ViewSaveCapture.ps1 b/scripts/native-capture/Invoke-ViewSaveCapture.ps1 index 862a689..2938442 100644 --- a/scripts/native-capture/Invoke-ViewSaveCapture.ps1 +++ b/scripts/native-capture/Invoke-ViewSaveCapture.ps1 @@ -167,7 +167,7 @@ try { $configPath = Join-Path $root 'config.toml' $configText = @" # keep this comment -view = 'project' +view = "project" repositories = ['$(Join-Path $root 'checkout')'] refresh_minutes = 0 parallelism = 1 @@ -217,7 +217,7 @@ t3_home = '$root' if ($after.GetOwner([Security.Principal.SecurityIdentifier]) -eq $administrators) { $failures.Add('config.toml is still owned by Administrators, so no save replaced it.') } if ($after.AccessToString -cne $before.AccessToString) { $failures.Add('The save changed who can access config.toml.') } if ($after.Group -ne $before.Group) { $failures.Add('The save changed the group on config.toml.') } - # One round trip back to the starting view must leave every byte as it was. + # One round trip back to the starting view must leave every byte as it was. Saves write the value in double quotes. $saved = [IO.File]::ReadAllText($configPath) if ($saved -cne $configText) { $failures.Add("config.toml changed outside its view value: $saved") } if ($failures.Count) { throw "Views failed: $($failures -join ' | ')" }