From 9a067dd1e141717bad0e643c60a7d8a0d18e8c4c Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 31 Jul 2026 15:01:50 -0400 Subject: [PATCH 01/31] ci: log the silent install and dump service-setup logs on any outcome --- .github/workflows/build-windows-installer.yml | 18 +++++++++++++++++- tests/test_windows_service_security.py | 4 ++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-windows-installer.yml b/.github/workflows/build-windows-installer.yml index cada100..2f71cde 100644 --- a/.github/workflows/build-windows-installer.yml +++ b/.github/workflows/build-windows-installer.yml @@ -213,13 +213,29 @@ jobs: - name: Install HumWatch with default firewall task shell: pwsh + timeout-minutes: 20 run: | $installer = Get-ChildItem "${{ runner.temp }}\humwatch-installer" -Filter "HumWatch-Setup-v*.exe" | Select-Object -First 1 if (-not $installer) { throw "Downloaded HumWatch installer artifact is missing" } - $argumentLine = '/VERYSILENT /SUPPRESSMSGBOXES /NORESTART /SP- /DIR="C:\HumWatch" /TASKS="firewallrule"' + $installLog = Join-Path $env:RUNNER_TEMP "humwatch-install.log" + "HUMWATCH_INSTALL_LOG=$installLog" >> $env:GITHUB_ENV + $argumentLine = '/VERYSILENT /SUPPRESSMSGBOXES /NORESTART /SP- /DIR="C:\HumWatch" /TASKS="firewallrule" /LOG="' + $installLog + '"' $process = Start-Process -FilePath $installer.FullName -ArgumentList $argumentLine -Wait -PassThru if ($process.ExitCode -ne 0) { throw "HumWatch installer failed with exit code $($process.ExitCode)" } + - name: Dump installer and service setup logs + if: always() + shell: pwsh + run: | + foreach ($log in @($env:HUMWATCH_INSTALL_LOG, "$env:ProgramData\HumWatch\logs\service-setup.log")) { + if ($log -and (Test-Path -LiteralPath $log)) { + Write-Host "===== $log =====" + Get-Content -LiteralPath $log + } else { + Write-Host "Log not found: $log" + } + } + - name: Verify installed service and firewall state shell: pwsh env: diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index fea7ebb..5e873a2 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -230,6 +230,10 @@ def test_windows_security_workflow_runs_behavioral_contract_suite(): assert "needs.build.outputs.installer_artifact" in release_section assert "contents: write" in release_section assert "permissions:\n contents: read" in workflow + # A silent install that dies on the runner must leave a readable trail. + assert "/LOG=" in workflow + assert "if: always()" in workflow + assert "service-setup.log" in workflow def test_inno_service_setup_runs_are_gated_by_pascal_exit_handling(): From d958f542bc61ecf2ca982dbca41f96e951a521ec Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 31 Jul 2026 15:07:01 -0400 Subject: [PATCH 02/31] fix: flatten the application tree ACL with icacls instead of a per-file loop --- installer/service-setup.ps1 | 61 ++++++++++++-------------- scripts/provision-security.ps1 | 7 +-- tests/test_windows_service_security.py | 20 +++++++++ 3 files changed, 52 insertions(+), 36 deletions(-) diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index 093a3a4..62d8c5a 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -85,44 +85,39 @@ function Get-ServiceAcl { function Set-ProtectedApplicationAcl { param([Parameter(Mandatory)][string]$Path) + $aclTarget = $Path.TrimEnd('\') + # A per-file Get-Acl/Set-Acl loop over the bundled Python runtime takes + # tens of minutes. icacls flattens the children in one native pass and the + # protected root ACL below propagates to them through normal inheritance. + & icacls $aclTarget /reset /T /C /Q | Out-Null + if ($LASTEXITCODE -ne 0) { throw "The application tree could not be reset to inherited permissions." } + & icacls $aclTarget /setowner "*S-1-5-32-544" /T /C /Q | Out-Null + if ($LASTEXITCODE -ne 0) { throw "The application tree owner could not be set to Administrators." } + $systemSid = [Security.Principal.SecurityIdentifier]::new("S-1-5-18") $administratorsSid = [Security.Principal.SecurityIdentifier]::new("S-1-5-32-544") $usersSid = [Security.Principal.SecurityIdentifier]::new("S-1-5-32-545") $allow = [Security.AccessControl.AccessControlType]::Allow - $targets = @((Get-Item -LiteralPath $Path -ErrorAction Stop)) - if ($targets[0].PSIsContainer) { - $targets += Get-ChildItem -LiteralPath $Path -Force -Recurse - } - - foreach ($item in $targets) { - $acl = Get-ServiceAcl $item.FullName - $acl.SetAccessRuleProtection($true, $false) - $acl.SetOwner($administratorsSid) - foreach ($rule in @($acl.Access)) { - [void]$acl.RemoveAccessRuleAll($rule) - } - $inheritance = if ($item.PSIsContainer) { - [Security.AccessControl.InheritanceFlags]::ContainerInherit -bor [Security.AccessControl.InheritanceFlags]::ObjectInherit - } else { - [Security.AccessControl.InheritanceFlags]::None - } - $systemRule = [Security.AccessControl.FileSystemAccessRule]::new( - $systemSid, [Security.AccessControl.FileSystemRights]::FullControl, - $inheritance, [Security.AccessControl.PropagationFlags]::None, $allow - ) - $adminRule = [Security.AccessControl.FileSystemAccessRule]::new( - $administratorsSid, [Security.AccessControl.FileSystemRights]::FullControl, - $inheritance, [Security.AccessControl.PropagationFlags]::None, $allow - ) - $userRule = [Security.AccessControl.FileSystemAccessRule]::new( - $usersSid, [Security.AccessControl.FileSystemRights]::ReadAndExecute, - $inheritance, [Security.AccessControl.PropagationFlags]::None, $allow - ) - $acl.AddAccessRule($systemRule) - $acl.AddAccessRule($adminRule) - $acl.AddAccessRule($userRule) - Set-Acl -LiteralPath $item.FullName -AclObject $acl + $inheritance = [Security.AccessControl.InheritanceFlags]::ContainerInherit -bor [Security.AccessControl.InheritanceFlags]::ObjectInherit + $acl = Get-ServiceAcl $aclTarget + $acl.SetAccessRuleProtection($true, $false) + $acl.SetOwner($administratorsSid) + foreach ($rule in @($acl.Access)) { + [void]$acl.RemoveAccessRuleAll($rule) } + $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new( + $systemSid, [Security.AccessControl.FileSystemRights]::FullControl, + $inheritance, [Security.AccessControl.PropagationFlags]::None, $allow + )) + $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new( + $administratorsSid, [Security.AccessControl.FileSystemRights]::FullControl, + $inheritance, [Security.AccessControl.PropagationFlags]::None, $allow + )) + $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new( + $usersSid, [Security.AccessControl.FileSystemRights]::ReadAndExecute, + $inheritance, [Security.AccessControl.PropagationFlags]::None, $allow + )) + Set-Acl -LiteralPath $aclTarget -AclObject $acl } function Set-ProtectedWritableDirectoryAcl { diff --git a/scripts/provision-security.ps1 b/scripts/provision-security.ps1 index ac595c8..3c034f9 100644 --- a/scripts/provision-security.ps1 +++ b/scripts/provision-security.ps1 @@ -183,14 +183,15 @@ foreach ($directory in @($runtimeRoot, $tlsRoot, $logRoot)) { Set-RestrictedAcl -Path $directory -SystemPermission "FullControl" } -# Installed application files are executable by the service but never writable by ordinary users. -Set-RestrictedAcl -Path $AppDir -SystemPermission "ReadAndExecute" +# The application tree ACL is owned by installer/service-setup.ps1 +# (Set-ProtectedApplicationAcl), which runs immediately after provisioning. +# A second recursive pass here doubled a multi-thousand-file rewrite and was +# the cause of the installed-service CI hang. if ($TokenFile) { Copy-SecureFile -Source $TokenFile -Destination $tokenDestination -SystemPermission "FullControl" } elseif (-not (Test-Path -LiteralPath $tokenDestination -PathType Leaf)) { New-TokenFile -Path $tokenDestination - Set-RestrictedAcl -Path $tokenDestination -SystemPermission "FullControl" } if (-not (Test-TokenMaterial -Path $tokenDestination)) { throw "The authentication token must encode at least 32 random bytes." diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 5e873a2..3249871 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -391,3 +391,23 @@ def test_setup_preserves_elevated_child_exit_propagation(): assert "ExitCode" in source assert "service-setup.ps1" in source assert "install-service.ps1" not in source + + +def test_application_tree_acl_uses_native_icacls_not_a_per_file_loop(): + setup = read("installer/service-setup.ps1") + provision = read("scripts/provision-security.ps1") + + section = setup.split("function Set-ProtectedApplicationAcl", 1)[1].split( + "function Set-ProtectedWritableDirectoryAcl", 1 + )[0] + # The per-file Get-Acl/Set-Acl walk over the bundled Python runtime is the + # CI hang. The application tree must be flattened by icacls and receive a + # single inheritable root ACL instead. + assert "icacls" in section + assert "/reset /T /C /Q" in section + assert "/setowner" in section + assert "Get-ChildItem" not in section + assert "S-1-5-32-545" in section + + # The provisioner must not run its own second recursive pass over AppDir. + assert 'Set-RestrictedAcl -Path $AppDir' not in provision From 1d3a739f5907b3c7199e12297feefffbd1567c4f Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 31 Jul 2026 15:51:33 -0400 Subject: [PATCH 03/31] fix: fail closed when icacls reports per-file ACL failures --- installer/service-setup.ps1 | 31 ++++++++++++++++++++++---- tests/test_windows_service_security.py | 20 +++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index 62d8c5a..2287eb3 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -89,10 +89,33 @@ function Set-ProtectedApplicationAcl { # A per-file Get-Acl/Set-Acl loop over the bundled Python runtime takes # tens of minutes. icacls flattens the children in one native pass and the # protected root ACL below propagates to them through normal inheritance. - & icacls $aclTarget /reset /T /C /Q | Out-Null - if ($LASTEXITCODE -ne 0) { throw "The application tree could not be reset to inherited permissions." } - & icacls $aclTarget /setowner "*S-1-5-32-544" /T /C /Q | Out-Null - if ($LASTEXITCODE -ne 0) { throw "The application tree owner could not be set to Administrators." } + # /C keeps icacls going past per-file failures, so a zero exit code alone + # does not prove every file succeeded. Capture the output, log the + # "Successfully processed N files" and "Failed processing M files" + # summary line, and fail closed when M is greater than zero. If that + # summary line is missing or does not match, do not treat the parse + # failure itself as an ACL failure, just fall back to the exit code check. + $resetOutput = & icacls $aclTarget /reset /T /C /Q 2>&1 + $resetExitCode = $LASTEXITCODE + $resetText = ($resetOutput | Out-String) + $resetMatch = [regex]::Match($resetText, 'Successfully processed \d+ files?; Failed processing (\d+) files?') + $resetSummaryLine = if ($resetMatch.Success) { $resetMatch.Value.Trim() } else { "icacls /reset summary line not found" } + Write-Log "icacls /reset for ${aclTarget}: $resetSummaryLine" + $resetFailedCount = if ($resetMatch.Success) { [int]$resetMatch.Groups[1].Value } else { 0 } + if ($resetExitCode -ne 0 -or $resetFailedCount -gt 0) { + throw "The application tree could not be reset to inherited permissions (exit code $resetExitCode, $resetFailedCount file(s) reported failed)." + } + + $ownerOutput = & icacls $aclTarget /setowner "*S-1-5-32-544" /T /C /Q 2>&1 + $ownerExitCode = $LASTEXITCODE + $ownerText = ($ownerOutput | Out-String) + $ownerMatch = [regex]::Match($ownerText, 'Successfully processed \d+ files?; Failed processing (\d+) files?') + $ownerSummaryLine = if ($ownerMatch.Success) { $ownerMatch.Value.Trim() } else { "icacls /setowner summary line not found" } + Write-Log "icacls /setowner for ${aclTarget}: $ownerSummaryLine" + $ownerFailedCount = if ($ownerMatch.Success) { [int]$ownerMatch.Groups[1].Value } else { 0 } + if ($ownerExitCode -ne 0 -or $ownerFailedCount -gt 0) { + throw "The application tree owner could not be set to Administrators (exit code $ownerExitCode, $ownerFailedCount file(s) reported failed)." + } $systemSid = [Security.Principal.SecurityIdentifier]::new("S-1-5-18") $administratorsSid = [Security.Principal.SecurityIdentifier]::new("S-1-5-32-544") diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 3249871..fcf44ed 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -411,3 +411,23 @@ def test_application_tree_acl_uses_native_icacls_not_a_per_file_loop(): # The provisioner must not run its own second recursive pass over AppDir. assert 'Set-RestrictedAcl -Path $AppDir' not in provision + + +def test_application_tree_acl_fails_closed_on_icacls_per_file_failures(): + setup = read("installer/service-setup.ps1") + + section = setup.split("function Set-ProtectedApplicationAcl", 1)[1].split( + "function Set-ProtectedWritableDirectoryAcl", 1 + )[0] + # /C makes icacls continue past per-file ACL failures, so it can still + # exit 0 while its summary line reports "Failed processing M files" with + # M greater than zero. Piping the output straight to Out-Null (the + # original gap) throws that signal away and leaves exit code as the only + # check, which /C can make misleadingly clean. The output must be + # captured, the failed-file count parsed out of the summary line, logged, + # and checked so a nonzero count fails the install even when the exit + # code is 0. + assert "Out-Null" not in section + assert "Failed processing" in section + assert "FailedCount -gt 0" in section + assert "Write-Log" in section From b664f6653d0230859f1c6932b4d42a389934e349 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 31 Jul 2026 16:02:52 -0400 Subject: [PATCH 04/31] fix: fail fast and log the reason when installed service setup fails --- installer/HumWatch.iss | 4 ++-- installer/service-setup.ps1 | 10 ++++++++ tests/test_windows_service_security.py | 33 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/installer/HumWatch.iss b/installer/HumWatch.iss index 4b7fae2..9f7b900 100644 --- a/installer/HumWatch.iss +++ b/installer/HumWatch.iss @@ -189,7 +189,7 @@ begin InstallParameters := InstallParameters + ' -FullSensorMode'; if not ExecuteHumWatchSetup(InstallParameters) then begin - MsgBox('HumWatch service setup failed. Installation cannot continue.', mbError, MB_OK); + SuppressibleMsgBox('HumWatch service setup failed. Installation cannot continue.', mbError, MB_OK, IDOK); Abort; end; @@ -200,7 +200,7 @@ begin '" -Action firewall -AppDir "' + ExpandConstant('{app}') + '" -FirewallPort 9100 -FirewallProfiles Domain,Private,Public -AllowPublic'; if not ExecuteHumWatchSetup(FirewallParameters) then begin - MsgBox('HumWatch Public firewall setup failed. Installation cannot continue.', mbError, MB_OK); + SuppressibleMsgBox('HumWatch Public firewall setup failed. Installation cannot continue.', mbError, MB_OK, IDOK); Abort; end; end; diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index 2287eb3..8b895dc 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -60,6 +60,10 @@ if (-not $ProvisioningScript) { $LogFile = Join-Path $LogDir "service-setup.log" +# Write-Log is a no-op until this directory exists, and provisioning used to be +# what created it. A provisioning failure therefore left no log at all. +New-Item -ItemType Directory -Path $LogDir -Force -ErrorAction SilentlyContinue | Out-Null + function Write-Log([string]$msg) { $line = "$(Get-Date -Format 'yyyy-MM-dd HH:mm:ss') $msg" if (Test-Path -LiteralPath $LogDir) { @@ -67,6 +71,12 @@ function Write-Log([string]$msg) { } } +trap { + Write-Log "FATAL: $($_.Exception.Message)" + Write-Log "FATAL at: $($_.InvocationInfo.PositionMessage)" + exit 1 +} + function Set-NssmEnvironment { param([string[]]$Values) diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index fcf44ed..7de7218 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -413,6 +413,39 @@ def test_application_tree_acl_uses_native_icacls_not_a_per_file_loop(): assert 'Set-RestrictedAcl -Path $AppDir' not in provision +def test_installer_failure_paths_do_not_block_on_a_modal_dialog(): + iss = read("installer/HumWatch.iss") + + # Inno's /SUPPRESSMSGBOXES only suppresses SuppressibleMsgBox. A plain + # MsgBox blocks forever on a headless runner, which is what hung the + # installed-service CI job for 20 minutes. + assert "SuppressibleMsgBox" in iss + # "SuppressibleMsgBox(" ends in the literal characters "MsgBox(", so a + # plain substring check for "MsgBox('HumWatch..." would still match + # inside "SuppressibleMsgBox('HumWatch...". Compare counts instead: every + # occurrence of the plain-call substring must come from the Suppressible + # form, none may stand alone as a blocking plain MsgBox call. + assert iss.count("MsgBox('HumWatch service setup failed") == iss.count( + "SuppressibleMsgBox('HumWatch service setup failed" + ) + assert iss.count("MsgBox('HumWatch Public firewall setup failed") == iss.count( + "SuppressibleMsgBox('HumWatch Public firewall setup failed" + ) + + +def test_service_setup_logs_terminating_errors_before_provisioning(): + setup = read("installer/service-setup.ps1") + + # Write-Log is a no-op until $LogDir exists, and provisioning is what + # created it. A provisioning failure therefore left no log at all. + log_dir_index = setup.index('$LogDir = Join-Path $RuntimeRoot "logs"') + provision_index = setup.index("& $ProvisioningScript") + creation_index = setup.index("New-Item -ItemType Directory -Path $LogDir") + assert log_dir_index < creation_index < provision_index + + assert "trap" in setup + + def test_application_tree_acl_fails_closed_on_icacls_per_file_failures(): setup = read("installer/service-setup.ps1") From 30f0733cdbff06dc7a523abc269e6d55906083f3 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 31 Jul 2026 16:28:32 -0400 Subject: [PATCH 05/31] fix: rebuild the Windows PowerShell module path before the installer touches ACLs --- installer/service-setup.ps1 | 11 +++++++++++ scripts/provision-security.ps1 | 10 ++++++++++ tests/test_windows_service_security.py | 18 ++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index 8b895dc..1c3c7cb 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -33,6 +33,17 @@ param( $ErrorActionPreference = "Stop" +# PowerShell 7 exports its own PSModulePath. Inno passes the environment of +# whatever launched the installer straight through to powershell.exe, so 5.1 can +# inherit the 7.x module tree and fail to autoload Microsoft.PowerShell.Security. +# Rebuilding from the machine scope restores this host's own default. This must +# land before the trap block below, since the trap calls Write-Log, which calls +# Add-Content, which lives in Microsoft.PowerShell.Management. +$machineModulePath = [Environment]::GetEnvironmentVariable("PSModulePath", "Machine") +if ($machineModulePath) { + $env:PSModulePath = $machineModulePath +} + $ServiceName = "HumWatch" if (-not $NssmPath) { $NssmPath = Join-Path $AppDir "tools\nssm.exe" diff --git a/scripts/provision-security.ps1 b/scripts/provision-security.ps1 index 3c034f9..96509eb 100644 --- a/scripts/provision-security.ps1 +++ b/scripts/provision-security.ps1 @@ -14,6 +14,16 @@ param( $ErrorActionPreference = "Stop" +# PowerShell 7 exports its own PSModulePath. Inno passes the environment of +# whatever launched the installer straight through to powershell.exe, so 5.1 can +# inherit the 7.x module tree and fail to autoload Microsoft.PowerShell.Security. +# Rebuilding from the machine scope restores this host's own default. This must +# land before the first Get-Acl or Set-Acl usage below. +$machineModulePath = [Environment]::GetEnvironmentVariable("PSModulePath", "Machine") +if ($machineModulePath) { + $env:PSModulePath = $machineModulePath +} + $programDataRoot = [Environment]::GetFolderPath("CommonApplicationData") $runtimeRoot = Join-Path $programDataRoot "HumWatch" $tlsRoot = Join-Path $runtimeRoot "tls" diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 7de7218..ccc2c1c 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -464,3 +464,21 @@ def test_application_tree_acl_fails_closed_on_icacls_per_file_failures(): assert "Failed processing" in section assert "FailedCount -gt 0" in section assert "Write-Log" in section + + +def test_installer_scripts_rebuild_the_windows_powershell_module_path(): + setup = read("installer/service-setup.ps1") + provision = read("scripts/provision-security.ps1") + + # PowerShell 7 exports its own PSModulePath. When powershell.exe 5.1 + # inherits it, autoloading Microsoft.PowerShell.Security resolves to the + # incompatible 7.x copy and Get-Acl fails. Inno inherits the environment of + # whatever launched the installer, so each script rebuilds 5.1's own default. + for source in (setup, provision): + assert "PSModulePath" in source + assert 'GetEnvironmentVariable("PSModulePath", "Machine")' in source + + # The reset has to happen before the first ACL call, or it is useless. + reset_index = provision.index("PSModulePath") + first_acl_index = provision.index("Get-Acl") + assert reset_index < first_acl_index From a7ebee4083148dbddabfdd04d3fac727e4ed4e92 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 31 Jul 2026 16:34:28 -0400 Subject: [PATCH 06/31] fix: generate Windows TLS certificates with bundled Python, not host OpenSSL --- installer/HumWatch.iss | 1 + requirements-dev.txt | 1 + requirements.in | 1 + requirements.txt | 53 +++++++++++++++++++- run-no-admin.bat | 4 +- run.bat | 4 +- scripts/build-release.ps1 | 4 +- scripts/generate_certificate.py | 77 +++++++++++++++++++++++++++++ scripts/provision-security.ps1 | 48 +++++++----------- tests/test_security_provisioning.py | 46 +++++++++++++++++ 10 files changed, 202 insertions(+), 37 deletions(-) create mode 100644 scripts/generate_certificate.py diff --git a/installer/HumWatch.iss b/installer/HumWatch.iss index 9f7b900..1f11d8d 100644 --- a/installer/HumWatch.iss +++ b/installer/HumWatch.iss @@ -105,6 +105,7 @@ Source: "{#StageDir}\README.md"; DestDir: "{app}"; Flags: ignoreversion isreadme Source: "service-setup.ps1"; DestDir: "{app}\tools"; Flags: ignoreversion Source: "..\scripts\provision-security.ps1"; DestDir: "{app}\tools"; Flags: ignoreversion Source: "..\scripts\certificate_identities.py"; DestDir: "{app}\tools"; Flags: ignoreversion +Source: "..\scripts\generate_certificate.py"; DestDir: "{app}\tools"; Flags: ignoreversion [Icons] Name: "{group}\Open HumWatch Dashboard"; Filename: "{sys}\cmd.exe"; Parameters: "/c start {#MyDashboardURL}"; IconFilename: "{app}\static\img\icon.ico"; Comment: "Open the HumWatch dashboard in your browser" diff --git a/requirements-dev.txt b/requirements-dev.txt index 5cdce49..612535d 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,3 +2,4 @@ pytest==8.4.1 pytest-asyncio==1.1.0 PyYAML==6.0.2 +cryptography==50.0.0 diff --git a/requirements.in b/requirements.in index e1e1440..bbf2716 100644 --- a/requirements.in +++ b/requirements.in @@ -6,3 +6,4 @@ pythonnet>=3.0.3 aiosqlite>=0.19.0 pydantic>=2.0.0 httpx>=0.27.0 +cryptography>=43.0.0 diff --git a/requirements.txt b/requirements.txt index 4e69f84..e5ce4d8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -126,7 +126,9 @@ cffi==2.1.0 \ --hash=sha256:f9912624a0c0b834b7520d7769b3644453aabc0a7e1c839da7359f050750e9bc \ --hash=sha256:fb62edb5bb52cca65fab91a63afa7561607120d26090a7e8fda6fb9f064726da \ --hash=sha256:ff067a8d8d880e7809e4ac88eb009bb848870115317b306666502ccad30b147f - # via clr-loader + # via + # clr-loader + # cryptography click==8.4.2 \ --hash=sha256:9a6cea6e60b17ebe0a44c5cc636d94f09bd66142c1cd7d8b4cd731c4917a15f6 \ --hash=sha256:e6f9f66136c816745b9d65817da91d61d957fb16e02e4dcd0552553c5a197b76 @@ -139,6 +141,54 @@ colorama==0.4.6 ; sys_platform == 'win32' \ --hash=sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44 \ --hash=sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6 # via click +cryptography==50.0.0 \ + --hash=sha256:031e2d5dd4bb9caa3ca9c82e5a197fd8ae680232cee62603d1a813f3f07e3d03 \ + --hash=sha256:06a32a980526a6ab9a4b9bf8f7385800791e2bb960903cb6b530e4817509a3b7 \ + --hash=sha256:07479a1cb08219ab719147e742e76090c9c773321959bb94946fffdd397a6437 \ + --hash=sha256:07949c449a1abcf60d1ee6e88956d89404c7df3c8258f46589e912988e551987 \ + --hash=sha256:105110f43a471dbd0060b9c9516cb8a6a79233631a04cc2ba16f28323ac6e025 \ + --hash=sha256:11b74db56cdbe3cdee6e3f6982ecb70334fa10dce99ed58bf7894aaaa3b2a037 \ + --hash=sha256:12b9c6996425c76ea6c457ace4f3073e715b8c545add07cd1a8f3a4f90691269 \ + --hash=sha256:1489e263a8048bb8b6a8bac662eb2d402ea5d2b7b4699b72f385f1e2772db105 \ + --hash=sha256:19736989797678c6af1e55cd49055cdbcb55d8f6b5583ac5335f933aba9101dc \ + --hash=sha256:1b4a266766514614f8aa60416e71f2fc6e575d36e7bdc90f644fadb2f4b75b95 \ + --hash=sha256:2a8183b489dc1f7f80f135780fadc1108f14b31b8a40411c7a5b17425f65f28b \ + --hash=sha256:37fdb0d0111f1e2ff07139dfb79f1b49531f8e213c46f1163dd7642979b58c47 \ + --hash=sha256:3f5735ffe4996d28b809371756219f5354864902a3b9e7c0b9ee87041209fc9c \ + --hash=sha256:49e7d93abdbd2990caced757e5fade25302f719c3c8fb6e6fff2dde98999fc41 \ + --hash=sha256:5e34edd123674534acd70147f0ca331eaa2c74e6325fb2028c886aa26ba0b68c \ + --hash=sha256:62598a8a57f815db4c6259a4e97d857dab56697e7de8e8ab02352ab74da1995d \ + --hash=sha256:65c2c3add92b45fd0709db8594536aea39c2a67af0e27ffcf049c498501140b7 \ + --hash=sha256:6ba6a53445bd3cfa809ef3ef5f1589aa6ba08784a1d962bf47d0940e871dab1c \ + --hash=sha256:6e7d61120573a7f2cd94cc095f9e81f6967c61ccdf194285aa143ecec8e0b708 \ + --hash=sha256:7cec5b856506da6defb290f30c9ee687d5f5e8cb0bd3f6459dde43b0b4fa40ef \ + --hash=sha256:80b63928fa35083b33966ce1efb70e5b9607181e49dcd1c22c8c005e319f667f \ + --hash=sha256:82148ec5bddac30b51a5b3c1945075f896fa022cb93f8e4a01e9f6ee95292c5f \ + --hash=sha256:828743d939e9629bc267b8e2d08d8bb67cd4319c771a33d4b18b22dd8fb7440a \ + --hash=sha256:8d89f3976b10b4ce31118de72329025f70d2c6ead14a8217c5514dd2c6d5a78f \ + --hash=sha256:8eb5e1172eb569ea8a872796576e6a67c276351728b6455d5beb01242b027c6a \ + --hash=sha256:900131fafd8aead39ac7dd3a7e833be754c17a95cfd91221636949fe4eb0aa8a \ + --hash=sha256:910d11e1a385c654bf738bf3e6b8e6ed5de0f5610fcae2be9e5b398d8081d20e \ + --hash=sha256:910e1d2668e7de9648f2bcee30e180db2a6b15c30f887d7c4c93ddf96e3992e3 \ + --hash=sha256:9aa87839c383bdbab6ef865787a1fb877af8dd03464c4400322726feaaadfc6d \ + --hash=sha256:a1b30560f2acc95aa8b2e06e716a13dbfc97314747b80d9707e307f77b40d6b3 \ + --hash=sha256:a91296cb61e8df6f86d0c19cc4068228da256bf59bf86049fbd821084565327f \ + --hash=sha256:b42a28c1844fd9de8f3f7d540e36b66f3a9c83fceac7170ebc7a6a19edd9dcae \ + --hash=sha256:bd1c592e4d5974f0d08d4888e432157adba757c66da0246918e43677fafa2d30 \ + --hash=sha256:c87f62a3d3b9888ed0fdde100ec06aa61ca9cd44bad9057d1dff9a516b5f5bb9 \ + --hash=sha256:c99c003e088647b8a5b7c145d6f78c335f6348332b62e142d411c4b63d1460b9 \ + --hash=sha256:ccdc4a71a4dabae05de219404f9f4abc38e3b58422177ff93d0da05967dafa07 \ + --hash=sha256:d24fead1d4d076e1bfb006dcec392074a3cd8d7b4fc8a595aa64073b2b7a96ba \ + --hash=sha256:d58c3db7cd6eed54e6c06744db55456b65ebd7492ddeae9c1e93cfca7aa857d3 \ + --hash=sha256:d764dcf130c428ef66786f866dd750f53182bc608813489915e9fc106bb0c82f \ + --hash=sha256:df2a58a472f332225671c35b0a830208b86d004f82baa8530fa3782c85646533 \ + --hash=sha256:e722f16708d854fe924790e051061f6704a472c3bac347b6fd88033ea8dd0dc5 \ + --hash=sha256:ecfed7367f965a0328cfbdd70da860f15441f002f613185668c6e6ebf5a0ac11 \ + --hash=sha256:eeac2acb5a20ed25e0ad6d1df9891a520b78b404266b6d11778f25d5d691a6c9 \ + --hash=sha256:f59e38625469987d7ef6d495323c55e7db6c212eaf6112267e0d3b565a2e9c9f \ + --hash=sha256:f89831ef99dd7dd169ab06d63a831adb9e20a87aac6d380266bbda5823349169 \ + --hash=sha256:fd9192b7b70c573d7f214eb1ae35e00d359f6f5e4b27c7e21e30de1fc6204645 + # via -r requirements.in exceptiongroup==1.3.1 ; python_full_version < '3.11' \ --hash=sha256:8b412432c6055b0b7d14c310000ae93352ed6754f70fa8f7c34141f91c4e3219 \ --hash=sha256:a7a39a3bd276781e98394987d3a5701d0c4edffb633bb7a5144577f82c773598 @@ -467,6 +517,7 @@ typing-extensions==4.16.0 \ --hash=sha256:dc983d19a509c94dba722ee6abd33940f7c05a89e243c47e907eb4db6f1a43e5 # via # anyio + # cryptography # exceptiongroup # fastapi # pydantic diff --git a/run-no-admin.bat b/run-no-admin.bat index 2aee2c7..57a301c 100644 --- a/run-no-admin.bat +++ b/run-no-admin.bat @@ -56,9 +56,7 @@ set "HUMWATCH_DATA_DIR=%ROOT%\.humwatch" set "HUMWATCH_LOG_DIR=%ROOT%\.humwatch\logs" set "HUMWATCH_AUTH_TOKEN_FILE=%DEV_TOKEN_FILE%" set "HUMWATCH_HOST=127.0.0.1" -:: There is no reliable way to issue a PEM certificate pair on Windows -:: without openssl, so this is the explicit plaintext loopback exception, -:: the same fallback run.sh takes when openssl is unavailable. +:: The dev profile stays on the plaintext loopback exception. Provisioned installs get their certificate from scripts\generate_certificate.py. set "HUMWATCH_ALLOW_INSECURE_LOCALHOST=1" set "PLAINTEXT_FALLBACK=1" diff --git a/run.bat b/run.bat index 8052966..89a8b24 100644 --- a/run.bat +++ b/run.bat @@ -62,9 +62,7 @@ set "HUMWATCH_DATA_DIR=%ROOT%\.humwatch" set "HUMWATCH_LOG_DIR=%ROOT%\.humwatch\logs" set "HUMWATCH_AUTH_TOKEN_FILE=%DEV_TOKEN_FILE%" set "HUMWATCH_HOST=127.0.0.1" -:: There is no reliable way to issue a PEM certificate pair on Windows -:: without openssl, so this is the explicit plaintext loopback exception, -:: the same fallback run.sh takes when openssl is unavailable. +:: The dev profile stays on the plaintext loopback exception. Provisioned installs get their certificate from scripts\generate_certificate.py. set "HUMWATCH_ALLOW_INSECURE_LOCALHOST=1" set "PLAINTEXT_FALLBACK=1" diff --git a/scripts/build-release.ps1 b/scripts/build-release.ps1 index 67ef719..649a5e6 100644 --- a/scripts/build-release.ps1 +++ b/scripts/build-release.ps1 @@ -99,7 +99,9 @@ $RequiredReleaseFiles = @( "update.bat", "requirements.txt", "installer\service-setup.ps1", - "scripts\provision-security.ps1" + "scripts\provision-security.ps1", + "scripts\generate_certificate.py", + "scripts\certificate_identities.py" ) foreach ($dir in $RequiredReleaseDirs) { diff --git a/scripts/generate_certificate.py b/scripts/generate_certificate.py new file mode 100644 index 0000000..77289f5 --- /dev/null +++ b/scripts/generate_certificate.py @@ -0,0 +1,77 @@ +"""Generate the self-signed HumWatch TLS pair without an external OpenSSL.""" + +import argparse +import datetime +import ipaddress +from pathlib import Path + +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.x509.oid import NameOID + +from certificate_identities import certificate_subject_alt_names + +_VALIDITY_DAYS = 825 +_KEY_SIZE = 3072 + + +def _san_entries(names): + entries = [] + for name in names: + kind, _, value = name.partition(":") + if kind == "IP": + entries.append(x509.IPAddress(ipaddress.ip_address(value))) + else: + entries.append(x509.DNSName(value)) + return entries + + +def generate_certificate(certificate_path, key_path, hostname, bind_identity, advertised_identities): + key = rsa.generate_private_key(public_exponent=65537, key_size=_KEY_SIZE) + subject = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, hostname)]) + now = datetime.datetime.now(datetime.timezone.utc) + names = certificate_subject_alt_names(hostname, bind_identity, advertised_identities) + # BasicConstraints CA:TRUE mirrors the "openssl req -x509" output this + # replaces. Peer discovery loads the certificate as a trust anchor. + certificate = ( + x509.CertificateBuilder() + .subject_name(subject) + .issuer_name(subject) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now) + .not_valid_after(now + datetime.timedelta(days=_VALIDITY_DAYS)) + .add_extension(x509.SubjectAlternativeName(_san_entries(names)), critical=False) + .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) + .add_extension(x509.SubjectKeyIdentifier.from_public_key(key.public_key()), critical=False) + .sign(key, hashes.SHA256()) + ) + Path(key_path).write_bytes(key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + )) + Path(certificate_path).write_bytes(certificate.public_bytes(serialization.Encoding.PEM)) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--certificate", required=True) + parser.add_argument("--key", required=True) + parser.add_argument("--hostname", required=True) + parser.add_argument("--bind-identity") + parser.add_argument("--advertised-identity", action="append", default=[]) + arguments = parser.parse_args() + generate_certificate( + arguments.certificate, + arguments.key, + arguments.hostname, + arguments.bind_identity, + arguments.advertised_identity, + ) + print(f"HumWatch certificate written: {arguments.certificate}") + + +if __name__ == "__main__": + main() diff --git a/scripts/provision-security.ps1 b/scripts/provision-security.ps1 index 96509eb..2cf3d73 100644 --- a/scripts/provision-security.ps1 +++ b/scripts/provision-security.ps1 @@ -9,7 +9,7 @@ param( [string]$BindIdentity = "127.0.0.1", [string[]]$AdvertisedIdentity, [string]$PythonExecutable, - [string]$IdentityScript + [string]$CertificateScript ) $ErrorActionPreference = "Stop" @@ -141,33 +141,21 @@ function New-ServerCertificate { [Parameter(Mandatory)][string]$CertificatePath, [Parameter(Mandatory)][string]$KeyPath, [Parameter(Mandatory)][string]$PythonExecutable, - [Parameter(Mandatory)][string]$IdentityScript + [Parameter(Mandatory)][string]$CertificateScript ) - $bundledOpenSsl = Join-Path $AppDir "tools\openssl.exe" - if (Test-Path -LiteralPath $bundledOpenSsl -PathType Leaf) { - $openssl = Get-Item -LiteralPath $bundledOpenSsl - } else { - $openssl = Get-Command openssl.exe -ErrorAction SilentlyContinue - } - if (-not $openssl) { - throw "No bundled OpenSSL was found under AppDir tools and OpenSSL is not on PATH. Supply -CertificateFile and -PrivateKeyFile, or install bundled OpenSSL before service registration." - } - - $identityArguments = @($IdentityScript, "--hostname", $Hostname, "--bind-identity", $BindIdentity) + $certificateArguments = @( + $CertificateScript, + "--certificate", $CertificatePath, + "--key", $KeyPath, + "--hostname", $Hostname, + "--bind-identity", $BindIdentity + ) foreach ($identity in $AdvertisedIdentity) { - $identityArguments += @("--advertised-identity", $identity) - } - $subjectAltNames = & $PythonExecutable @identityArguments - if ($LASTEXITCODE -ne 0 -or -not $subjectAltNames) { - throw "TLS certificate identity generation failed." + $certificateArguments += @("--advertised-identity", $identity) } - - & $openssl.Source req -x509 -newkey rsa:3072 -sha256 -nodes -days 825 ` - -subj "/CN=$Hostname" ` - -addext ("subjectAltName=" + ($subjectAltNames -join ",")) ` - -keyout $KeyPath -out $CertificatePath | Out-Null - if ($LASTEXITCODE -ne 0) { + & $PythonExecutable @certificateArguments | Out-Null + if ($LASTEXITCODE -ne 0 -or -not (Test-Path -LiteralPath $CertificatePath -PathType Leaf) -or -not (Test-Path -LiteralPath $KeyPath -PathType Leaf)) { throw "TLS certificate generation failed." } } @@ -178,14 +166,16 @@ if (-not (Test-Path -LiteralPath $AppDir -PathType Container)) { if (-not $PythonExecutable) { $PythonExecutable = Join-Path $AppDir "python\python.exe" } -if (-not $IdentityScript) { - $IdentityScript = Join-Path (Split-Path -Parent $MyInvocation.MyCommand.Path) "certificate_identities.py" +if (-not $CertificateScript) { + $CertificateScript = Join-Path (Split-Path -Parent $MyInvocation.MyCommand.Path) "generate_certificate.py" } if (-not (Test-Path -LiteralPath $PythonExecutable -PathType Leaf)) { throw "Python was not found for TLS certificate identity generation." } -if (-not (Test-Path -LiteralPath $IdentityScript -PathType Leaf)) { - throw "TLS certificate identity helper was not found." +$certificateHelperDirectory = Split-Path -Parent $CertificateScript +$certificateIdentityScript = Join-Path $certificateHelperDirectory "certificate_identities.py" +if (-not (Test-Path -LiteralPath $CertificateScript -PathType Leaf) -or -not (Test-Path -LiteralPath $certificateIdentityScript -PathType Leaf)) { + throw "TLS certificate generation helpers were not found." } foreach ($directory in @($runtimeRoot, $tlsRoot, $logRoot)) { @@ -215,7 +205,7 @@ if ($CertificateFile) { Copy-SecureFile -Source $CertificateFile -Destination $certificateDestination -SystemPermission "ReadAndExecute" Copy-SecureFile -Source $PrivateKeyFile -Destination $privateKeyDestination -SystemPermission "FullControl" } elseif (-not (Test-Path -LiteralPath $certificateDestination -PathType Leaf) -or -not (Test-Path -LiteralPath $privateKeyDestination -PathType Leaf)) { - New-ServerCertificate -CertificatePath $certificateDestination -KeyPath $privateKeyDestination -PythonExecutable $PythonExecutable -IdentityScript $IdentityScript + New-ServerCertificate -CertificatePath $certificateDestination -KeyPath $privateKeyDestination -PythonExecutable $PythonExecutable -CertificateScript $CertificateScript Set-RestrictedAcl -Path $certificateDestination -SystemPermission "ReadAndExecute" Set-RestrictedAcl -Path $privateKeyDestination -SystemPermission "FullControl" } diff --git a/tests/test_security_provisioning.py b/tests/test_security_provisioning.py index 2227dfa..38407bb 100644 --- a/tests/test_security_provisioning.py +++ b/tests/test_security_provisioning.py @@ -1,8 +1,14 @@ +import subprocess +import sys from pathlib import Path +import pytest + from agent.config import HumWatchConfig from scripts.certificate_identities import certificate_subject_alt_names +ROOT = Path(__file__).resolve().parents[1] + def extracted_postinst() -> str: source = Path("scripts/build-deb.sh").read_text(encoding="utf-8") @@ -206,3 +212,43 @@ def test_certificate_sans_exclude_wildcard_bind_addresses(): assert "IP:0.0.0.0" not in certificate_subject_alt_names( hostname="humwatch-lan", bind_identity="0.0.0.0" ) + + +def test_generate_certificate_writes_a_parity_self_signed_pair(tmp_path): + cryptography = pytest.importorskip("cryptography") + from cryptography import x509 + from cryptography.hazmat.primitives.serialization import load_pem_private_key + from cryptography.x509.oid import NameOID + + cert_path = tmp_path / "cert.pem" + key_path = tmp_path / "key.pem" + completed = subprocess.run( + [ + sys.executable, + str(ROOT / "scripts" / "generate_certificate.py"), + "--certificate", str(cert_path), + "--key", str(key_path), + "--hostname", "hum-test", + "--bind-identity", "0.0.0.0", + "--advertised-identity", "100.64.0.7", + ], + capture_output=True, + text=True, + ) + assert completed.returncode == 0, completed.stderr + + certificate = x509.load_pem_x509_certificate(cert_path.read_bytes()) + common_names = certificate.subject.get_attributes_for_oid(NameOID.COMMON_NAME) + assert common_names[0].value == "hum-test" + sans = certificate.extensions.get_extension_for_class(x509.SubjectAlternativeName).value + dns_names = set(sans.get_values_for_type(x509.DNSName)) + ip_names = {str(ip) for ip in sans.get_values_for_type(x509.IPAddress)} + assert dns_names == {"hum-test", "localhost"} + assert ip_names == {"127.0.0.1", "::1", "100.64.0.7"} + basic = certificate.extensions.get_extension_for_class(x509.BasicConstraints) + assert basic.value.ca is True and basic.critical is True + + key = load_pem_private_key(key_path.read_bytes(), password=None) + assert key.key_size == 3072 + validity = certificate.not_valid_after_utc - certificate.not_valid_before_utc + assert validity.days == 825 From 57621dc66691ae9f19fd70dd976d9b7cb2e9df99 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 31 Jul 2026 16:40:16 -0400 Subject: [PATCH 07/31] fix: keep Users read access when update.bat re-hardens the portable tree --- tests/test_installation_docs.py | 6 ++++++ update.bat | 2 ++ 2 files changed, 8 insertions(+) diff --git a/tests/test_installation_docs.py b/tests/test_installation_docs.py index ee76a34..871468a 100644 --- a/tests/test_installation_docs.py +++ b/tests/test_installation_docs.py @@ -152,6 +152,12 @@ def test_update_is_elevated_stopped_fail_closed_and_acl_verified(): assert "'updated'" in lower assert "start-process pip" not in lower + # LocalService reads the application tree through BUILTIN\Users. The + # hardened ACL must keep Users read-and-execute or the service cannot + # restart after an update. + assert "s-1-5-32-545" in lower + assert "readandexecute" in lower + def test_update_requires_the_named_release_and_verified_digest(): source = read("update.bat") diff --git a/update.bat b/update.bat index d4aed57..07630bc 100644 --- a/update.bat +++ b/update.bat @@ -89,6 +89,7 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ " if ($LASTEXITCODE -ne 0) { throw 'The application tree could not be reset to inherited permissions' }; " ^ " $systemSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-18'); " ^ " $administratorsSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-32-544'); " ^ + " $usersSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-32-545'); " ^ " $inherit = [Security.AccessControl.InheritanceFlags]::ContainerInherit -bor [Security.AccessControl.InheritanceFlags]::ObjectInherit; " ^ " $noPropagation = [Security.AccessControl.PropagationFlags]::None; " ^ " $allow = [Security.AccessControl.AccessControlType]::Allow; " ^ @@ -98,6 +99,7 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ " foreach ($rule in @($acl.Access)) { [void]$acl.RemoveAccessRuleAll($rule) }; " ^ " $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($systemSid, [Security.AccessControl.FileSystemRights]::ReadAndExecute, $inherit, $noPropagation, $allow)); " ^ " $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($administratorsSid, [Security.AccessControl.FileSystemRights]::FullControl, $inherit, $noPropagation, $allow)); " ^ + " $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($usersSid, [Security.AccessControl.FileSystemRights]::ReadAndExecute, $inherit, $noPropagation, $allow)); " ^ " Set-Acl -LiteralPath $path -AclObject $acl " ^ "}; " ^ "function Stop-HumWatch { " ^ From 533abea67a12bd04f72d4c3b6a99f6ac0759b5c3 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 31 Jul 2026 16:51:26 -0400 Subject: [PATCH 08/31] fix: make certificate generation import and fail loudly under embedded Python --- scripts/generate_certificate.py | 11 ++++++ scripts/provision-security.ps1 | 13 +++++-- tests/test_security_provisioning.py | 55 +++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/scripts/generate_certificate.py b/scripts/generate_certificate.py index 77289f5..fef8503 100644 --- a/scripts/generate_certificate.py +++ b/scripts/generate_certificate.py @@ -3,6 +3,7 @@ import argparse import datetime import ipaddress +import sys from pathlib import Path from cryptography import x509 @@ -10,6 +11,16 @@ from cryptography.hazmat.primitives.asymmetric import rsa from cryptography.x509.oid import NameOID +# The bundled Windows runtime is the CPython embeddable distribution, which +# ships a python312._pth file. That file puts the interpreter in isolated +# mode, so the script's own directory is not prepended to sys.path and +# PYTHONPATH is ignored. Without this, the sibling import below raises +# ModuleNotFoundError. This is a no-op on a normal Python, where the +# directory is already present. +_SCRIPT_DIRECTORY = str(Path(__file__).resolve().parent) +if _SCRIPT_DIRECTORY not in sys.path: + sys.path.insert(0, _SCRIPT_DIRECTORY) + from certificate_identities import certificate_subject_alt_names _VALIDITY_DAYS = 825 diff --git a/scripts/provision-security.ps1 b/scripts/provision-security.ps1 index 2cf3d73..e0966a4 100644 --- a/scripts/provision-security.ps1 +++ b/scripts/provision-security.ps1 @@ -154,9 +154,16 @@ function New-ServerCertificate { foreach ($identity in $AdvertisedIdentity) { $certificateArguments += @("--advertised-identity", $identity) } - & $PythonExecutable @certificateArguments | Out-Null - if ($LASTEXITCODE -ne 0 -or -not (Test-Path -LiteralPath $CertificatePath -PathType Leaf) -or -not (Test-Path -LiteralPath $KeyPath -PathType Leaf)) { - throw "TLS certificate generation failed." + # Capture stdout and stderr instead of discarding them. On failure this + # output is the only clue in service-setup.log about what Python did. + $certificateOutput = & $PythonExecutable @certificateArguments 2>&1 + $certificateExitCode = $LASTEXITCODE + if ($certificateExitCode -ne 0 -or -not (Test-Path -LiteralPath $CertificatePath -PathType Leaf) -or -not (Test-Path -LiteralPath $KeyPath -PathType Leaf)) { + $certificateOutputText = ($certificateOutput | Out-String).Trim() + if (-not $certificateOutputText) { + $certificateOutputText = "(no output was captured)" + } + throw "TLS certificate generation failed. $certificateOutputText" } } diff --git a/tests/test_security_provisioning.py b/tests/test_security_provisioning.py index 38407bb..e4cdebc 100644 --- a/tests/test_security_provisioning.py +++ b/tests/test_security_provisioning.py @@ -1,3 +1,4 @@ +import os import subprocess import sys from pathlib import Path @@ -252,3 +253,57 @@ def test_generate_certificate_writes_a_parity_self_signed_pair(tmp_path): assert key.key_size == 3072 validity = certificate.not_valid_after_utc - certificate.not_valid_before_utc assert validity.days == 825 + + +def test_generate_certificate_works_when_its_own_directory_is_not_on_sys_path(tmp_path): + pytest.importorskip("cryptography") + + cert_path = tmp_path / "cert.pem" + key_path = tmp_path / "key.pem" + script = ROOT / "scripts" / "generate_certificate.py" + + # -I reproduces the embedded Windows runtime closely enough for this + # import: it disables the automatic script-directory sys.path entry + # (the actual defect) and ignores PYTHONPATH, same as the python312._pth + # isolated mode on the bundled interpreter. It also drops user + # site-packages, which is fine here because this environment has a + # second, non-user install of cryptography, so the test still exercises + # the real import failure instead of masking it behind a missing + # dependency. Running from tmp_path (not the repo) and with the + # environment's own PYTHONPATH removed rules out both of those as + # accidental sources of the script directory landing on sys.path. + environment = dict(os.environ) + environment.pop("PYTHONPATH", None) + completed = subprocess.run( + [ + sys.executable, + "-I", + str(script), + "--certificate", str(cert_path), + "--key", str(key_path), + "--hostname", "hum-test", + "--bind-identity", "0.0.0.0", + ], + cwd=str(tmp_path), + env=environment, + capture_output=True, + text=True, + ) + assert completed.returncode == 0, completed.stderr + assert "ModuleNotFoundError" not in completed.stderr + assert cert_path.exists() + assert key_path.exists() + + from cryptography import x509 + + certificate = x509.load_pem_x509_certificate(cert_path.read_bytes()) + assert certificate.subject.rfc4514_string() == "CN=hum-test" + + +def test_provisioning_captures_certificate_generation_output_on_failure(): + source = Path("scripts/provision-security.ps1").read_text(encoding="utf-8") + + assert "& $PythonExecutable @certificateArguments | Out-Null" not in source + assert "& $PythonExecutable @certificateArguments 2>&1" in source + assert "$certificateExitCode = $LASTEXITCODE" in source + assert 'throw "TLS certificate generation failed. $certificateOutputText"' in source From 28e377ca7d7a1f491d13aaa511c98de67ac0f5b8 Mon Sep 17 00:00:00 2001 From: Static Date: Sun, 2 Aug 2026 13:51:08 -0400 Subject: [PATCH 09/31] docs: stop claiming the installer ships PawnIO and fix release-note scheme --- README.md | 4 ++-- scripts/build-installer.ps1 | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index f238473..4d21f21 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ A self-hosted, local-first hardware monitoring system for Windows and Linux PCs. 2. Run the installer — click **Yes** when Windows asks for admin access 3. Follow the prompts (default install path `C:\HumWatch` is fine) -The installer handles everything: bundled Python 3.12, all dependencies, LibreHardwareMonitor (v0.9.6 + PawnIO driver), Windows service registration, firewall rule, and auto-start on boot. +The installer handles everything except one optional driver: bundled Python 3.12, all dependencies, LibreHardwareMonitor v0.9.6, Windows service registration, firewall rule, and auto-start on boot. For temperatures, fans, voltages, and GPU metrics, also install the PawnIO driver once: `winget install PawnIO.PawnIO`. Without it HumWatch runs in psutil-only mode (CPU load, memory, disk, network, battery). Once installed, open `https://localhost:9100` in your browser. The first visit may require certificate trust setup... trust the HumWatch certificate or its @@ -70,7 +70,7 @@ Running `python -m agent.main` directly skips that provisioning, so it requires Without LibreHardwareMonitor, HumWatch runs in psutil-only mode — you get CPU load, memory, disk, network, and battery basics. With LHM, you also get temperatures, voltages, GPU metrics, fan speeds, and more. -> **Note:** LHM v0.9.5+ requires the [PawnIO](https://github.com/PawnIO/PawnIO) driver (replaces the deprecated WinRing0 driver). The `download-lhm.ps1` script installs it automatically via `winget`. To install manually: `winget install PawnIO.PawnIO` +> **Note:** LHM v0.9.5+ requires the [PawnIO](https://github.com/PawnIO/PawnIO) driver (replaces the deprecated WinRing0 driver). The download-lhm.ps1 script installs it via winget on development machines. The Windows installer does not install drivers, so run the winget command once on installed machines that need full sensors. ### Install as a Windows Service (from source) diff --git a/scripts/build-installer.ps1 b/scripts/build-installer.ps1 index 538e994..a3ccb99 100644 --- a/scripts/build-installer.ps1 +++ b/scripts/build-installer.ps1 @@ -485,9 +485,9 @@ if ($CreateGitHubRelease) { "2. Run it (SmartScreen warning -- click **More info -> Run anyway**)`n" + "3. Follow the installer. It handles everything automatically:`n" + " - Installs a self-contained Python runtime (no Python required)`n" + - " - Installs LibreHardwareMonitor for full CPU/GPU sensor access`n" + + " - Installs LibreHardwareMonitor (run 'winget install PawnIO.PawnIO' once for full sensors)`n" + " - Registers a Windows service that starts automatically on every boot`n" + - "4. Open **http://localhost:9100**`n`n" + + "4. Open **https://localhost:9100** and trust the HumWatch certificate`n`n" + "### Requirements`n" + "- Windows 10 version 1809 or later (64-bit)`n`n" + "---`n" + From 1f0c3a20d623f0f42740e2f0fd062a7646467bd7 Mon Sep 17 00:00:00 2001 From: Static Date: Sun, 2 Aug 2026 14:03:28 -0400 Subject: [PATCH 10/31] fix: stop emitting empty native command arguments and report failed installs --- installer/HumWatch.iss | 10 +++++-- installer/service-setup.ps1 | 8 +++++- scripts/provision-security.ps1 | 23 +++++++++++++--- tests/test_windows_service_security.py | 37 +++++++++++++++++++++++++- 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/installer/HumWatch.iss b/installer/HumWatch.iss index 1f11d8d..42cd2f6 100644 --- a/installer/HumWatch.iss +++ b/installer/HumWatch.iss @@ -191,7 +191,13 @@ begin if not ExecuteHumWatchSetup(InstallParameters) then begin SuppressibleMsgBox('HumWatch service setup failed. Installation cannot continue.', mbError, MB_OK, IDOK); - Abort; + // Abort alone let a silent (/VERYSILENT) install finish reporting exit + // code 0 even though the service was never configured -- any caller, + // CI or a user's script, was told a broken install succeeded. + // RaiseException forces a fatal, untrapped error, which Setup reports + // with a nonzero exit code while still running its own rollback and + // cleanup, unlike the ExitProcess kernel32 trick. + RaiseException('HumWatch service setup failed. Installation cannot continue.'); end; if WizardIsTaskSelected('firewallpublic') then begin @@ -202,7 +208,7 @@ begin '" -FirewallPort 9100 -FirewallProfiles Domain,Private,Public -AllowPublic'; if not ExecuteHumWatchSetup(FirewallParameters) then begin SuppressibleMsgBox('HumWatch Public firewall setup failed. Installation cannot continue.', mbError, MB_OK, IDOK); - Abort; + RaiseException('HumWatch Public firewall setup failed. Installation cannot continue.'); end; end; end; diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index 1c3c7cb..2d83a96 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -466,7 +466,13 @@ if (Test-Path -LiteralPath $configPath -PathType Leaf) { } if (-not $Hostname) { $Hostname = $env:COMPUTERNAME } if (-not $BindIdentity) { $BindIdentity = "127.0.0.1" } -$AdvertisedIdentity = @($AdvertisedIdentity) + $configuredIdentities +# @($AdvertisedIdentity) alone would be a one-element array holding $null when +# the installer never passes -AdvertisedIdentity, not an empty array. Filtering +# with Where-Object after concatenation drops that null (and any blank +# configured identity) before it becomes a dangling native argument. Wrapping +# the whole pipeline in @(...) keeps a single surviving element an array +# instead of collapsing it to a scalar. +$AdvertisedIdentity = @(@($AdvertisedIdentity) + @($configuredIdentities) | Where-Object { $_ }) # Do not retain an earlier, less restricted service if provisioning fails. Remove-HumWatchService diff --git a/scripts/provision-security.ps1 b/scripts/provision-security.ps1 index e0966a4..8f9ccfd 100644 --- a/scripts/provision-security.ps1 +++ b/scripts/provision-security.ps1 @@ -144,14 +144,27 @@ function New-ServerCertificate { [Parameter(Mandatory)][string]$CertificateScript ) + # This function cannot trust its caller's arrays to be free of $null or + # empty entries (installer/service-setup.ps1 is not the only caller). + # Windows PowerShell 5.1 silently drops empty-string arguments when + # invoking a native executable, so a blank value here does not vanish + # cleanly, it leaves the preceding flag dangling with no value and + # argparse rejects the whole command line. --hostname is a required + # argparse argument, so an empty $Hostname would fail the same way. + if (-not $Hostname) { + throw "A certificate hostname is required to generate the HumWatch TLS certificate." + } $certificateArguments = @( $CertificateScript, "--certificate", $CertificatePath, "--key", $KeyPath, - "--hostname", $Hostname, - "--bind-identity", $BindIdentity + "--hostname", $Hostname ) + if ($BindIdentity) { + $certificateArguments += @("--bind-identity", $BindIdentity) + } foreach ($identity in $AdvertisedIdentity) { + if (-not $identity) { continue } $certificateArguments += @("--advertised-identity", $identity) } # Capture stdout and stderr instead of discarding them. On failure this @@ -159,7 +172,11 @@ function New-ServerCertificate { $certificateOutput = & $PythonExecutable @certificateArguments 2>&1 $certificateExitCode = $LASTEXITCODE if ($certificateExitCode -ne 0 -or -not (Test-Path -LiteralPath $CertificatePath -PathType Leaf) -or -not (Test-Path -LiteralPath $KeyPath -PathType Leaf)) { - $certificateOutputText = ($certificateOutput | Out-String).Trim() + # Out-String defaults to an 80 column width and truncates anything + # wider, which previously cut off the argparse "error:" line and left + # only the usage banner in the log. A wide, explicit width keeps the + # full captured error intact. + $certificateOutputText = ($certificateOutput | Out-String -Width 4096).Trim() if (-not $certificateOutputText) { $certificateOutputText = "(no output was captured)" } diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index ccc2c1c..4ec898f 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -247,7 +247,10 @@ def test_inno_service_setup_runs_are_gated_by_pascal_exit_handling(): assert "ssPostInstall" in code_section assert "ewWaitUntilTerminated" in code_section assert "ResultCode" in code_section - assert "Abort" in code_section + # Abort alone let a silent install finish with exit code 0 even though + # the service setup failed (task 3.5). RaiseException is the mechanism + # that now forces a nonzero exit code while preserving Inno's rollback. + assert "RaiseException" in code_section assert "WizardIsTaskSelected('firewallrule')" in code_section assert "WizardIsTaskSelected('firewallpublic')" in code_section for argument in ( @@ -482,3 +485,35 @@ def test_installer_scripts_rebuild_the_windows_powershell_module_path(): reset_index = provision.index("PSModulePath") first_acl_index = provision.index("Get-Acl") assert reset_index < first_acl_index + + +def test_advertised_identity_list_cannot_contain_empty_entries(): + setup = read("installer/service-setup.ps1") + provision = read("scripts/provision-security.ps1") + + # @($null) is a one-element array holding $null, not an empty array. + # PowerShell 5.1 drops empty string arguments to native executables, which + # leaves python staring at a dangling --advertised-identity flag. + assert "$AdvertisedIdentity = @($AdvertisedIdentity) + $configuredIdentities" not in setup + assert "Where-Object" in setup + + certificate_section = provision.split("function New-ServerCertificate", 1)[1].split( + "\nfunction ", 1 + )[0] + assert "if (-not $identity" in certificate_section or "Where-Object" in certificate_section + + +def test_captured_subprocess_output_is_not_column_truncated(): + provision = read("scripts/provision-security.ps1") + + # Out-String defaults to 80 columns and silently truncated the real + # argparse error, leaving only the usage banner in the log. + assert "Out-String" in provision + assert "-Width" in provision + + +def test_installer_reports_a_failed_post_install_to_its_caller(): + iss = read("installer/HumWatch.iss") + + # A silent install that leaves no working service must not exit 0. + assert "SetupExitCode" in iss or "ExitProcess" in iss or "RaiseException" in iss From 6f478a90e76996869e14d96f284cc80ea220ef11 Mon Sep 17 00:00:00 2001 From: Static Date: Sun, 2 Aug 2026 14:10:13 -0400 Subject: [PATCH 11/31] fix: validate security config in the server lifespan, not only under __main__ --- agent/main.py | 4 ++++ tests/test_security_config.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/agent/main.py b/agent/main.py index 0c0ad08..4adbb43 100644 --- a/agent/main.py +++ b/agent/main.py @@ -88,6 +88,10 @@ async def lifespan(app: FastAPI): ], ) + # Refuse to serve an unsafe transport/auth combination no matter how the + # ASGI app was launched (python -m, uvicorn CLI, or a direct import). + validate_security_config(config) + scheme = "https" if config.resolved_tls_certfile and config.resolved_tls_keyfile else "http" logger.info("HumWatch v%s starting on port %d", __version__, config.port) diff --git a/tests/test_security_config.py b/tests/test_security_config.py index d6d3335..70f534c 100644 --- a/tests/test_security_config.py +++ b/tests/test_security_config.py @@ -227,3 +227,18 @@ def test_non_loopback_hosts_are_not_loopback(host): @pytest.mark.parametrize("host", ["127.0.0.1", "::1", "localhost"]) def test_loopback_hosts_are_detected(host): assert is_loopback_host(host) is True + + +def test_server_lifespan_refuses_unsafe_transport(monkeypatch, tmp_path): + from fastapi.testclient import TestClient + + monkeypatch.setenv("HUMWATCH_HOST", "0.0.0.0") + monkeypatch.setenv("HUMWATCH_DATA_DIR", str(tmp_path)) + monkeypatch.delenv("HUMWATCH_AUTH_TOKEN", raising=False) + import agent.config + import agent.main + + monkeypatch.setattr(agent.config, "_config", None) + with pytest.raises(SecurityConfigurationError): + with TestClient(agent.main.create_app()): + pass From 81be3a8721e54b49aff70b097816d78f1563799a Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 00:37:43 -0400 Subject: [PATCH 12/31] fix: stop the installed-state ACL check from flagging read-only grants Test-NoUnauthorizedWriteAccess built its write mask by ORing in Modify and FullControl, both composite FileSystemRights values that also carry the read bits, so the check flagged the deliberate BUILTIN\Users ReadAndExecute grant as unauthorized write access. Build the mask from atomic write-only rights instead (Write, Delete, DeleteSubdirectoriesAndFiles, ChangePermissions, TakeOwnership) in both the real check and the duplicated mask in the installed-state CI harness, and rescope the static assertion test to the function body so it actually pins the atomic rights instead of matching Modify and FullControl elsewhere in the file. Co-Authored-By: Claude Opus 5 --- installer/service-setup.ps1 | 7 +-- tests/test_windows_service_security.py | 70 ++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index 2d83a96..cb7a1c2 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -226,9 +226,10 @@ function Test-NoUnauthorizedWriteAccess { ) $writeRights = [Security.AccessControl.FileSystemRights]::Write -bor - [Security.AccessControl.FileSystemRights]::Modify -bor - [Security.AccessControl.FileSystemRights]::FullControl -bor - [Security.AccessControl.FileSystemRights]::Delete + [Security.AccessControl.FileSystemRights]::Delete -bor + [Security.AccessControl.FileSystemRights]::DeleteSubdirectoriesAndFiles -bor + [Security.AccessControl.FileSystemRights]::ChangePermissions -bor + [Security.AccessControl.FileSystemRights]::TakeOwnership $blockedSids = @("S-1-1-0", "S-1-5-11", "S-1-5-32-545") foreach ($path in $Paths) { if (-not (Test-Path -LiteralPath $path)) { diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 4ec898f..1c44512 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -1,6 +1,7 @@ import json import os from pathlib import Path +import re import shutil import subprocess @@ -11,6 +12,33 @@ def read(path: str) -> str: return Path(path).read_text(encoding="utf-8") +# Documented .NET System.Security.AccessControl.FileSystemRights enum values. +# Modify and FullControl are composites that also OR in the read bits +# (ReadData, ReadAttributes, ReadPermissions, ...), which is the bug this +# module pins: building a "does this grant write access" mask out of either +# composite makes a read-only ReadAndExecute grant match it too. +FILE_SYSTEM_RIGHTS = { + "Write": 0x116, + "Delete": 0x10000, + "DeleteSubdirectoriesAndFiles": 0x40, + "ChangePermissions": 0x40000, + "TakeOwnership": 0x80000, + "Modify": 0x301BF, + "FullControl": 0x1F01FF, + "ReadAndExecute": 0x200A9, +} + + +def _mask_from_bor_expression(expression: str) -> int: + """OR together the FileSystemRights values named in a `-bor`-joined expression.""" + names = re.findall(r"FileSystemRights\]::(\w+)", expression) + assert names, "no FileSystemRights]:: tokens found" + mask = 0 + for name in names: + mask |= FILE_SYSTEM_RIGHTS[name] + return mask + + def _powershell_contract_harness() -> str: script_path = str((Path.cwd() / "installer/service-setup.ps1").resolve()).replace("'", "''") return rf""" @@ -154,7 +182,7 @@ def test_windows_installed_state_acl_and_firewall_contract(): $appDir = $params.AppDirectory $paths = @($appDir, (Join-Path $appDir 'tools\nssm.exe'), (Join-Path $appDir 'python\python.exe'), (Join-Path $appDir 'agent'), "$env:ProgramData\HumWatch\auth-token", "$env:ProgramData\HumWatch\tls\humwatch-key.pem") $blocked = @('S-1-1-0', 'S-1-5-11', 'S-1-5-32-545') -$write = [Security.AccessControl.FileSystemRights]::Write -bor [Security.AccessControl.FileSystemRights]::Modify -bor [Security.AccessControl.FileSystemRights]::FullControl -bor [Security.AccessControl.FileSystemRights]::Delete +$write = [Security.AccessControl.FileSystemRights]::Write -bor [Security.AccessControl.FileSystemRights]::Delete -bor [Security.AccessControl.FileSystemRights]::DeleteSubdirectoriesAndFiles -bor [Security.AccessControl.FileSystemRights]::ChangePermissions -bor [Security.AccessControl.FileSystemRights]::TakeOwnership foreach ($path in $paths) { if (-not (Test-Path -LiteralPath $path)) { throw "Missing installed path: $path" } foreach ($rule in (Get-Acl -LiteralPath $path).Access) { @@ -348,14 +376,46 @@ def test_firewall_is_only_configured_when_the_installer_task_is_selected(): assert 'if ($ConfigureFirewall)' in setup +def test_write_check_mask_excludes_read_bits_pulled_in_by_composite_rights(): + source = read("installer/service-setup.ps1") + function_body = source.split("function Test-NoUnauthorizedWriteAccess", 1)[1].split( + "function Verify-InstalledState", 1 + )[0] + assignment = function_body.split("$writeRights =", 1)[1].split("$blockedSids", 1)[0] + mask = _mask_from_bor_expression(assignment) + + assert FILE_SYSTEM_RIGHTS["ReadAndExecute"] & mask == 0, ( + "a read-only ReadAndExecute grant must not be flagged as write access" + ) + for right in ("Write", "Modify", "FullControl", "Delete"): + assert FILE_SYSTEM_RIGHTS[right] & mask != 0, f"{right} must still be flagged as write access" + + own_source = read(__file__) + harness_assignment = own_source.split("$write = ", 1)[1].split("\n", 1)[0] + harness_mask = _mask_from_bor_expression(harness_assignment) + + assert FILE_SYSTEM_RIGHTS["ReadAndExecute"] & harness_mask == 0, ( + "the installed-state harness must not flag a read-only ReadAndExecute grant either" + ) + for right in ("Write", "Modify", "FullControl", "Delete"): + assert FILE_SYSTEM_RIGHTS[right] & harness_mask != 0, ( + f"the installed-state harness must still flag {right} as write access" + ) + + def test_acl_verification_checks_effective_write_bits_for_world_and_users(): source = read("installer/service-setup.ps1") + function_body = source.split("function Test-NoUnauthorizedWriteAccess", 1)[1].split( + "function Verify-InstalledState", 1 + )[0] for sid in ("S-1-1-0", "S-1-5-11", "S-1-5-32-545"): - assert sid in source - for right in ("Write", "Modify", "FullControl", "Delete"): - assert right in source - assert "-band $writeRights" in source + assert sid in function_body + for right in ("Write", "Delete", "DeleteSubdirectoriesAndFiles", "ChangePermissions", "TakeOwnership"): + assert right in function_body + for composite in ("Modify", "FullControl"): + assert composite not in function_body + assert "-band $writeRights" in function_body def test_installed_state_verification_covers_executable_and_secret_paths(): From 890ccf2d9eae65027021ef4befa7abd7564f2046 Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 00:41:16 -0400 Subject: [PATCH 13/31] fix: cache the bearer token file between requests with mtime invalidation --- agent/security/auth.py | 23 ++++++++++++++++++----- tests/test_security_config.py | 25 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/agent/security/auth.py b/agent/security/auth.py index fb34294..66d0ca0 100644 --- a/agent/security/auth.py +++ b/agent/security/auth.py @@ -4,6 +4,7 @@ import secrets from collections import Counter from math import log2 +from pathlib import Path from fastapi import HTTPException, Request @@ -17,6 +18,22 @@ MAX_TOKEN_CHARACTER_RATIO = 0.75 MAX_REPEATED_CYCLE_LENGTH = 16 +_TOKEN_CACHE: dict[Path, tuple[int, int, str]] = {} + + +def _read_token_file(token_file: Path) -> str: + stat = token_file.stat() + cached = _TOKEN_CACHE.get(token_file) + if cached is not None and cached[0] == stat.st_mtime_ns and cached[1] == stat.st_size: + return cached[2] + token = token_file.read_bytes().decode("utf-8") + if token.endswith("\r\n"): + token = token[:-2] + elif token.endswith("\n"): + token = token[:-1] + _TOKEN_CACHE[token_file] = (stat.st_mtime_ns, stat.st_size, token) + return token + def _has_repeated_cycle(token: str) -> bool: """Reject short repeating token cycles that meet simple entropy metrics.""" @@ -60,11 +77,7 @@ def load_auth_token(config: HumWatchConfig) -> str: token_file = config.resolved_auth_token_file if token_file is not None: try: - token = token_file.read_bytes().decode("utf-8") - if token.endswith("\r\n"): - token = token[:-2] - elif token.endswith("\n"): - token = token[:-1] + token = _read_token_file(token_file) except OSError as exc: raise SecurityConfigurationError("Auth token file could not be read") from exc elif os.environ.get("HUMWATCH_AUTH_TOKEN") is not None: diff --git a/tests/test_security_config.py b/tests/test_security_config.py index 70f534c..6458168 100644 --- a/tests/test_security_config.py +++ b/tests/test_security_config.py @@ -1,5 +1,6 @@ import json import os +import time from pathlib import Path import pytest @@ -138,6 +139,30 @@ def test_token_file_takes_precedence_and_normalizes_one_final_crlf(tmp_path, mon ) == token +def test_token_file_reads_are_cached_until_the_file_changes(tmp_path, monkeypatch): + token_a = "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8S9t0" + token_b = "Z9y8X7w6V5u4T3s2R1q0P9o8N7m6L5k4J3i2H1g0" + token_file = tmp_path / "auth-token" + token_file.write_text(token_a + "\n", encoding="utf-8") + config = HumWatchConfig(auth_token_file=str(token_file)) + + reads = {"count": 0} + original = Path.read_bytes + + def counting_read_bytes(self): + reads["count"] += 1 + return original(self) + + monkeypatch.setattr(Path, "read_bytes", counting_read_bytes) + assert load_auth_token(config) == token_a + assert load_auth_token(config) == token_a + assert reads["count"] == 1 + + time.sleep(0.01) + token_file.write_text(token_b + "\n", encoding="utf-8") + assert load_auth_token(config) == token_b + + def test_environment_token_is_used_when_file_is_not_configured(monkeypatch): token = "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8S9t0" monkeypatch.setenv("HUMWATCH_AUTH_TOKEN", token) From b3af4fc20b4ce73626e14341bed55c6df9992f36 Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 00:49:53 -0400 Subject: [PATCH 14/31] fix: assert the installer and harness ACL masks cannot drift apart Co-Authored-By: Claude Opus 5 --- tests/test_windows_service_security.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 1c44512..e44a8aa 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -402,6 +402,12 @@ def test_write_check_mask_excludes_read_bits_pulled_in_by_composite_rights(): f"the installed-state harness must still flag {right} as write access" ) + assert mask == harness_mask, ( + "the installer's $writeRights mask and the test harness's duplicated $write mask " + "have drifted apart: a right present in one but not the other would pass every " + "assertion above while still breaking the contract that the two copies stay identical" + ) + def test_acl_verification_checks_effective_write_bits_for_world_and_users(): source = read("installer/service-setup.ps1") From dc4d8e7c3e064fdc8e399b53f1c2f5aa6176e758 Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 00:49:53 -0400 Subject: [PATCH 15/31] fix: guard the bearer token cache against same-tick mtime collisions Co-Authored-By: Claude Opus 5 --- agent/security/auth.py | 30 ++++++++++++++++-- tests/test_security_config.py | 59 ++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/agent/security/auth.py b/agent/security/auth.py index 66d0ca0..469b734 100644 --- a/agent/security/auth.py +++ b/agent/security/auth.py @@ -2,6 +2,7 @@ import os import secrets +import time from collections import Counter from math import log2 from pathlib import Path @@ -20,18 +21,43 @@ _TOKEN_CACHE: dict[Path, tuple[int, int, str]] = {} +# A cached entry is only trusted once its mtime is at least this far in the +# past. Some filesystems (FAT-family in particular, with a 2-second mtime +# resolution) can make two different writes look identical: same mtime, same +# size. Without this guard a same-tick token rotation could be served the +# stale cached value. 3 seconds gives FAT's 2-second granularity a margin. +_MTIME_GUARD_WINDOW_NS = 3_000_000_000 + + +def _now_ns() -> int: + return time.time_ns() + def _read_token_file(token_file: Path) -> str: stat = token_file.stat() + trust_cache = (_now_ns() - stat.st_mtime_ns) >= _MTIME_GUARD_WINDOW_NS cached = _TOKEN_CACHE.get(token_file) - if cached is not None and cached[0] == stat.st_mtime_ns and cached[1] == stat.st_size: + if ( + trust_cache + and cached is not None + and cached[0] == stat.st_mtime_ns + and cached[1] == stat.st_size + ): return cached[2] + token = token_file.read_bytes().decode("utf-8") if token.endswith("\r\n"): token = token[:-2] elif token.endswith("\n"): token = token[:-1] - _TOKEN_CACHE[token_file] = (stat.st_mtime_ns, stat.st_size, token) + + if trust_cache: + _TOKEN_CACHE[token_file] = (stat.st_mtime_ns, stat.st_size, token) + else: + # Recent mtimes are never safe to cache: a follow-up rewrite could + # still land on the same truncated tick, so keep re-reading until + # the file's mtime is safely in the past. + _TOKEN_CACHE.pop(token_file, None) return token diff --git a/tests/test_security_config.py b/tests/test_security_config.py index 6458168..8b4d1c8 100644 --- a/tests/test_security_config.py +++ b/tests/test_security_config.py @@ -144,6 +144,10 @@ def test_token_file_reads_are_cached_until_the_file_changes(tmp_path, monkeypatc token_b = "Z9y8X7w6V5u4T3s2R1q0P9o8N7m6L5k4J3i2H1g0" token_file = tmp_path / "auth-token" token_file.write_text(token_a + "\n", encoding="utf-8") + # Backdate well outside the mtime guard window so the initial read is + # cacheable, matching an install-time token file that is never touched. + old_mtime = time.time() - 3600 + os.utime(token_file, times=(old_mtime, old_mtime)) config = HumWatchConfig(auth_token_file=str(token_file)) reads = {"count": 0} @@ -158,11 +162,64 @@ def counting_read_bytes(self): assert load_auth_token(config) == token_a assert reads["count"] == 1 - time.sleep(0.01) token_file.write_text(token_b + "\n", encoding="utf-8") assert load_auth_token(config) == token_b +def test_token_rotation_inside_the_mtime_guard_window_is_not_served_stale(tmp_path, monkeypatch): + """A same-length rotation that lands on an identical mtime/size must never be served stale. + + This reproduces the exact collision the mtime+size cache key cannot see on + its own: rewriting the file with a different token of the same byte length, + stamped with the same mtime as the cached read. Only the guard window (which + refuses to trust a cache entry whose mtime is still close to "now") prevents + this from returning the old token. + """ + token_a = "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8S9t0" + token_b = "Z9y8X7w6V5u4T3s2R1q0P9o8N7m6L5k4J3i2H1g0" + assert len(token_a) == len(token_b) + token_file = tmp_path / "auth-token" + token_file.write_text(token_a + "\n", encoding="utf-8") + + fixed_now = 1_700_000_000.0 + monkeypatch.setattr(auth, "_now_ns", lambda: int(fixed_now * 1_000_000_000)) + same_mtime = fixed_now - 0.001 # 1ms before "now": deep inside the guard window + os.utime(token_file, times=(same_mtime, same_mtime)) + config = HumWatchConfig(auth_token_file=str(token_file)) + + assert load_auth_token(config) == token_a + + token_file.write_text(token_b + "\n", encoding="utf-8") + os.utime(token_file, times=(same_mtime, same_mtime)) # identical mtime/size to the cached read + + assert load_auth_token(config) == token_b + + +def test_old_mtime_token_file_is_still_served_from_cache_without_reread(tmp_path, monkeypatch): + """A token file that is old relative to 'now' must not be re-read on every request.""" + token = "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8S9t0" + token_file = tmp_path / "auth-token" + token_file.write_text(token + "\n", encoding="utf-8") + + fixed_now = 1_700_000_000.0 + monkeypatch.setattr(auth, "_now_ns", lambda: int(fixed_now * 1_000_000_000)) + old_mtime = fixed_now - 3600 # an hour before "now": well outside the guard window + os.utime(token_file, times=(old_mtime, old_mtime)) + config = HumWatchConfig(auth_token_file=str(token_file)) + + reads = {"count": 0} + original = Path.read_bytes + + def counting_read_bytes(self): + reads["count"] += 1 + return original(self) + + monkeypatch.setattr(Path, "read_bytes", counting_read_bytes) + assert load_auth_token(config) == token + assert load_auth_token(config) == token + assert reads["count"] == 1 + + def test_environment_token_is_used_when_file_is_not_configured(monkeypatch): token = "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8S9t0" monkeypatch.setenv("HUMWATCH_AUTH_TOKEN", token) From 8e1857a38002f5c40c73c2fe6be07e95bb063e78 Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 00:57:49 -0400 Subject: [PATCH 16/31] fix: key the bearer token cache on file identity, not pathname --- agent/security/auth.py | 30 +++++++++++++++++++------- tests/test_security_config.py | 40 +++++++++++++++++------------------ 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/agent/security/auth.py b/agent/security/auth.py index 469b734..00c3243 100644 --- a/agent/security/auth.py +++ b/agent/security/auth.py @@ -19,13 +19,26 @@ MAX_TOKEN_CHARACTER_RATIO = 0.75 MAX_REPEATED_CYCLE_LENGTH = 16 -_TOKEN_CACHE: dict[Path, tuple[int, int, str]] = {} +# Cache key is real file identity (device, inode), not the pathname. Value is +# (mtime_ns, size, token). Keying on identity means a rename-into-place +# rotation (the standard admin pattern) is detected immediately on any +# filesystem, regardless of timestamps, because it always produces a new +# inode: the (dev, ino) lookup simply misses. +_TOKEN_CACHE: dict[tuple[int, int], tuple[int, int, str]] = {} # A cached entry is only trusted once its mtime is at least this far in the -# past. Some filesystems (FAT-family in particular, with a 2-second mtime -# resolution) can make two different writes look identical: same mtime, same -# size. Without this guard a same-tick token rotation could be served the -# stale cached value. 3 seconds gives FAT's 2-second granularity a margin. +# past. Rename-into-place rotation (the standard admin pattern) is already +# caught by the (dev, ino) identity check above, regardless of this window, +# since it always produces a new inode. This window exists only for the one +# case identity cannot see: an in-place, same-length, timestamp-preserving +# overwrite of the SAME inode (no rename, size unchanged, mtime unchanged or +# landing on the same truncated tick) on a filesystem with coarse mtime +# resolution (FAT-family's 2-second resolution is the reference case; 3 +# seconds gives it margin). That residual is accepted, not eliminated: an +# attacker able to overwrite the token file in place while preserving its +# size and timestamp already has write access to the token file itself, +# which is an independent compromise no caching strategy for reading that +# same file can defend against. _MTIME_GUARD_WINDOW_NS = 3_000_000_000 @@ -35,8 +48,9 @@ def _now_ns() -> int: def _read_token_file(token_file: Path) -> str: stat = token_file.stat() + identity = (stat.st_dev, stat.st_ino) trust_cache = (_now_ns() - stat.st_mtime_ns) >= _MTIME_GUARD_WINDOW_NS - cached = _TOKEN_CACHE.get(token_file) + cached = _TOKEN_CACHE.get(identity) if ( trust_cache and cached is not None @@ -52,12 +66,12 @@ def _read_token_file(token_file: Path) -> str: token = token[:-1] if trust_cache: - _TOKEN_CACHE[token_file] = (stat.st_mtime_ns, stat.st_size, token) + _TOKEN_CACHE[identity] = (stat.st_mtime_ns, stat.st_size, token) else: # Recent mtimes are never safe to cache: a follow-up rewrite could # still land on the same truncated tick, so keep re-reading until # the file's mtime is safely in the past. - _TOKEN_CACHE.pop(token_file, None) + _TOKEN_CACHE.pop(identity, None) return token diff --git a/tests/test_security_config.py b/tests/test_security_config.py index 8b4d1c8..5ae8630 100644 --- a/tests/test_security_config.py +++ b/tests/test_security_config.py @@ -166,31 +166,34 @@ def counting_read_bytes(self): assert load_auth_token(config) == token_b -def test_token_rotation_inside_the_mtime_guard_window_is_not_served_stale(tmp_path, monkeypatch): - """A same-length rotation that lands on an identical mtime/size must never be served stale. - - This reproduces the exact collision the mtime+size cache key cannot see on - its own: rewriting the file with a different token of the same byte length, - stamped with the same mtime as the cached read. Only the guard window (which - refuses to trust a cache entry whose mtime is still close to "now") prevents - this from returning the old token. +def test_token_rotation_via_atomic_replace_is_not_served_stale_by_identity(tmp_path): + """Rename-into-place rotation must be detected by file identity, not just mtime/size. + + This is the standard admin rotation pattern: write the replacement to a + staging file, then atomically replace the original. That always produces a + new inode, but here we also force the replacement to carry the exact same + mtime and byte size as the file it replaces (same-length token, timestamp + deliberately preserved, and both are backdated outside the mtime guard + window so the guard's recency check cannot be what saves this test). A + cache keyed on pathname plus mtime plus size alone cannot tell this apart + from the original file and would serve the stale token. Only checking the + real file identity (device and inode) from the stat catches it. """ token_a = "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8S9t0" token_b = "Z9y8X7w6V5u4T3s2R1q0P9o8N7m6L5k4J3i2H1g0" assert len(token_a) == len(token_b) token_file = tmp_path / "auth-token" token_file.write_text(token_a + "\n", encoding="utf-8") - - fixed_now = 1_700_000_000.0 - monkeypatch.setattr(auth, "_now_ns", lambda: int(fixed_now * 1_000_000_000)) - same_mtime = fixed_now - 0.001 # 1ms before "now": deep inside the guard window - os.utime(token_file, times=(same_mtime, same_mtime)) + old_mtime = time.time() - 3600 # well outside the mtime guard window + os.utime(token_file, times=(old_mtime, old_mtime)) config = HumWatchConfig(auth_token_file=str(token_file)) - assert load_auth_token(config) == token_a + assert load_auth_token(config) == token_a # populates the cache - token_file.write_text(token_b + "\n", encoding="utf-8") - os.utime(token_file, times=(same_mtime, same_mtime)) # identical mtime/size to the cached read + staging_file = tmp_path / "auth-token.new" + staging_file.write_text(token_b + "\n", encoding="utf-8") + os.utime(staging_file, times=(old_mtime, old_mtime)) # same mtime as the cached read + os.replace(staging_file, token_file) # atomic rename-into-place: new inode, same path assert load_auth_token(config) == token_b @@ -200,10 +203,7 @@ def test_old_mtime_token_file_is_still_served_from_cache_without_reread(tmp_path token = "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8S9t0" token_file = tmp_path / "auth-token" token_file.write_text(token + "\n", encoding="utf-8") - - fixed_now = 1_700_000_000.0 - monkeypatch.setattr(auth, "_now_ns", lambda: int(fixed_now * 1_000_000_000)) - old_mtime = fixed_now - 3600 # an hour before "now": well outside the guard window + old_mtime = time.time() - 3600 # well outside the mtime guard window os.utime(token_file, times=(old_mtime, old_mtime)) config = HumWatchConfig(auth_token_file=str(token_file)) From e91e728e9bc5bac7567b093168e5bca037d2fc8e Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 01:02:30 -0400 Subject: [PATCH 17/31] fix: pin get-pip to an immutable commit URL and reject floating hosts Co-Authored-By: Claude Opus 5 --- scripts/asset-versions.ps1 | 2 +- scripts/verify-downloads.ps1 | 2 +- tests/test_supply_chain_policy.py | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/asset-versions.ps1 b/scripts/asset-versions.ps1 index 4663aa3..48e073c 100644 --- a/scripts/asset-versions.ps1 +++ b/scripts/asset-versions.ps1 @@ -36,7 +36,7 @@ $AssetManifest = @( [pscustomobject]@{ Name = "get-pip" Version = "26.2" - Url = "https://bootstrap.pypa.io/get-pip.py" + Url = "https://raw.githubusercontent.com/pypa/get-pip/47fcb6bb638930c19dcb8cb9e53f7a16e6186e7d/public/get-pip.py" Sha256 = "25b5c39ade96bab5eabe6404ce83cab6da2deb5fe3c07d9881f43803edb6f9c8" MinimumBytes = 2000000 } diff --git a/scripts/verify-downloads.ps1 b/scripts/verify-downloads.ps1 index 1549ce1..64e9546 100644 --- a/scripts/verify-downloads.ps1 +++ b/scripts/verify-downloads.ps1 @@ -38,7 +38,7 @@ foreach ($asset in $AssetManifest) { if ([int64]$asset.MinimumBytes -le 0) { throw "Asset $($asset.Name) must have a positive minimum size" } - if ($asset.Url -match "(?i)(latest|/master(?:/|$)|/main(?:/|$))") { + if ($asset.Url -match "(?i)(latest|bootstrap\.pypa\.io|/master(?:/|$)|/main(?:/|$))") { throw "Asset $($asset.Name) uses a floating URL" } } diff --git a/tests/test_supply_chain_policy.py b/tests/test_supply_chain_policy.py index 7968174..ffbf8bf 100644 --- a/tests/test_supply_chain_policy.py +++ b/tests/test_supply_chain_policy.py @@ -66,6 +66,9 @@ def test_asset_manifest_has_exact_pins_and_digests(): assert len(digests) >= 5 assert all(set(digest.lower()) <= set("0123456789abcdef") for digest in digests) assert "latest" not in source.lower() + # bootstrap.pypa.io/get-pip.py is a floating URL whose content changes on + # every pip release. Only immutable commit-pinned copies are allowed. + assert "bootstrap.pypa.io" not in source def test_download_and_build_scripts_verify_before_use(): From 61f95d4b53a7a13701c7678d567f0a4716dc203e Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 01:04:44 -0400 Subject: [PATCH 18/31] fix: mark config.json as a dpkg conffile so upgrades keep operator edits Co-Authored-By: Claude Opus 5 --- scripts/build-deb.sh | 5 +++++ tests/test_linux_service_security.py | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/scripts/build-deb.sh b/scripts/build-deb.sh index 1c83ba5..88c8149 100755 --- a/scripts/build-deb.sh +++ b/scripts/build-deb.sh @@ -36,6 +36,11 @@ Description: Self-hosted hardware monitoring agent What hums beneath the shell. EOF +# Operator-owned configuration survives package upgrades. +cat < "$STAGE_DIR/DEBIAN/conffiles" +/opt/HumWatch/config.json +EOF + # Pre-install script (optional) # Post-install script cat > "$STAGE_DIR/DEBIAN/postinst" <<'POSTINST' diff --git a/tests/test_linux_service_security.py b/tests/test_linux_service_security.py index 4454c1f..b6090f8 100644 --- a/tests/test_linux_service_security.py +++ b/tests/test_linux_service_security.py @@ -110,3 +110,11 @@ def test_generated_package_removal_preserves_runtime_and_secret_state(built_deb) assert "/etc/humwatch" in postrm assert "rm -r" not in postrm assert "systemctl stop humwatch.service || true" in prerm + + +def test_operator_config_is_a_conffile(built_deb): + _package, control_dir = built_deb + conffiles = control_dir / "conffiles" + assert conffiles.exists() + entries = conffiles.read_text(encoding="utf-8").split() + assert "/opt/HumWatch/config.json" in entries From 889b19d9ac13ae09ec7832a68fb01d2bee6f5800 Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 15:53:59 -0400 Subject: [PATCH 19/31] fix: make a failed post-install exit nonzero instead of reporting success RaiseException at ssPostInstall never changed Setup's exit code. By that step Inno has already copied the files and written the uninstall key, so the exception is caught, logged, shown as a runtime error, and Setup deinitializes with exit code 0. CI run 30760330669 recorded exactly that: service setup exited 1, the installer reported success, and no rollback ran. CurStepChanged now records the failure in a module-level flag and DeinitializeSetup, which runs late enough to control the real process exit code, calls kernel32 ExitProcess(1). The install is already committed at that point, so both failure messages tell the operator to uninstall before retrying. The guard test was a three-way substring or over the whole file, satisfied by the comment text alone, and would have passed with both RaiseException calls deleted. It now strips comments, pins the flag, the kernel32 import, the CurStepChanged assignment, and the exit code, and it fails when any of them is removed. Co-Authored-By: Claude Opus 5 --- installer/HumWatch.iss | 52 +++++++++++++++---- tests/test_windows_service_security.py | 69 ++++++++++++++++++++++---- 2 files changed, 102 insertions(+), 19 deletions(-) diff --git a/installer/HumWatch.iss b/installer/HumWatch.iss index 42cd2f6..2da57b6 100644 --- a/installer/HumWatch.iss +++ b/installer/HumWatch.iss @@ -132,6 +132,24 @@ Type: filesandordirs; Name: "{app}\__pycache__" ; User can manually delete {app} after uninstall if they want a clean removal [Code] +// ── Failed-install reporting ─────────────────────────────────────────────── +// By ssPostInstall, Inno has already copied every file and written the +// uninstall key, so an exception raised from there is caught, logged, and +// Setup still deinitializes with exit code 0. CI run 30760330669 recorded +// exactly that: a service setup that exited 1, a runtime error dialog, and a +// Setup process that reported success to its caller. Inno only rolls back and +// returns a fatal exit code for failures during the file-copy phase. +// +// DeinitializeSetup runs last, after Setup has finished its own cleanup, which +// is late enough for ExitProcess to decide the real process exit code. The +// install itself is already committed at that point, so the operator has to +// uninstall before retrying. Both failure messages say so. +procedure ExitProcess(uExitCode: Integer); + external 'ExitProcess@kernel32.dll stdcall'; + +var + HumWatchSetupFailed: Boolean; + // ── Service control ──────────────────────────────────────────────────────── // Stop the HumWatch service before file copy so files aren't locked. // Pascal post-install code reinstalls and starts the service after copy. @@ -171,11 +189,12 @@ begin Result := ResultCode = 0; end; -procedure RunHumWatchPostInstall(); +function RunHumWatchPostInstall(): Boolean; var InstallParameters: string; FirewallParameters: string; begin + Result := True; InstallParameters := '-ExecutionPolicy Bypass -NonInteractive -File "' + ExpandConstant('{app}\tools\service-setup.ps1') + @@ -190,14 +209,16 @@ begin InstallParameters := InstallParameters + ' -FullSensorMode'; if not ExecuteHumWatchSetup(InstallParameters) then begin - SuppressibleMsgBox('HumWatch service setup failed. Installation cannot continue.', mbError, MB_OK, IDOK); // Abort alone let a silent (/VERYSILENT) install finish reporting exit // code 0 even though the service was never configured -- any caller, - // CI or a user's script, was told a broken install succeeded. - // RaiseException forces a fatal, untrapped error, which Setup reports - // with a nonzero exit code while still running its own rollback and - // cleanup, unlike the ExitProcess kernel32 trick. - RaiseException('HumWatch service setup failed. Installation cannot continue.'); + // CI or a user's script, was told a broken install succeeded. So did the + // RaiseException that replaced it. The failure is reported to the caller + // through HumWatchSetupFailed and DeinitializeSetup instead. Interactive + // users still get the suppressible dialog. + SuppressibleMsgBox('HumWatch service setup failed. Installation cannot continue. Uninstall HumWatch from Apps and Features before retrying the installer.', mbError, MB_OK, IDOK); + Log('HumWatch service setup failed. Setup will report a nonzero exit code.'); + Result := False; + Exit; end; if WizardIsTaskSelected('firewallpublic') then begin @@ -207,8 +228,10 @@ begin '" -Action firewall -AppDir "' + ExpandConstant('{app}') + '" -FirewallPort 9100 -FirewallProfiles Domain,Private,Public -AllowPublic'; if not ExecuteHumWatchSetup(FirewallParameters) then begin - SuppressibleMsgBox('HumWatch Public firewall setup failed. Installation cannot continue.', mbError, MB_OK, IDOK); - RaiseException('HumWatch Public firewall setup failed. Installation cannot continue.'); + SuppressibleMsgBox('HumWatch Public firewall setup failed. Installation cannot continue. Uninstall HumWatch from Apps and Features before retrying the installer.', mbError, MB_OK, IDOK); + Log('HumWatch Public firewall setup failed. Setup will report a nonzero exit code.'); + Result := False; + Exit; end; end; end; @@ -218,7 +241,16 @@ begin if CurStep = ssInstall then begin StopHumWatchService(); end else if CurStep = ssPostInstall then begin - RunHumWatchPostInstall(); + if not RunHumWatchPostInstall() then + HumWatchSetupFailed := True; + end; +end; + +procedure DeinitializeSetup(); +begin + if HumWatchSetupFailed then begin + Log('HumWatch post-install failed. Exiting Setup with code 1.'); + ExitProcess(1); end; end; diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index e44a8aa..acf7a8b 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -39,6 +39,26 @@ def _mask_from_bor_expression(expression: str) -> int: return mask +def _pascal_code_without_comments(source: str) -> str: + """Drop `//` comment lines from an Inno script. + + Every substring assertion against `HumWatch.iss` has to run through this + first. The old exit-code guard was a three-way `or` over the whole file and + both of the mechanisms it named also appeared in the comments explaining + them, so it passed on prose alone and would have kept passing with the real + calls deleted. + """ + return "\n".join( + line for line in source.splitlines() if not line.strip().startswith("//") + ) + + +def _pascal_block(code: str, header: str) -> str: + """Return a single Pascal routine, from its header to the next routine.""" + assert header in code, header + return re.split(r"\n(?=(?:procedure|function)\s)", code.split(header, 1)[1])[0] + + def _powershell_contract_harness() -> str: script_path = str((Path.cwd() / "installer/service-setup.ps1").resolve()).replace("'", "''") return rf""" @@ -267,18 +287,24 @@ def test_windows_security_workflow_runs_behavioral_contract_suite(): def test_inno_service_setup_runs_are_gated_by_pascal_exit_handling(): source = read("installer/HumWatch.iss") run_section = source.split("[Run]", 1)[1].split("[UninstallRun]", 1)[0] - code_section = source.split("[Code]", 1)[1] + code_section = _pascal_code_without_comments(source.split("[Code]", 1)[1]) assert "-Action install" not in run_section assert "-Action firewall" not in run_section - assert "procedure RunHumWatchPostInstall" in code_section + assert "function RunHumWatchPostInstall" in code_section assert "ssPostInstall" in code_section assert "ewWaitUntilTerminated" in code_section assert "ResultCode" in code_section # Abort alone let a silent install finish with exit code 0 even though - # the service setup failed (task 3.5). RaiseException is the mechanism - # that now forces a nonzero exit code while preserving Inno's rollback. - assert "RaiseException" in code_section + # the service setup failed (task 3.5), and so did RaiseException, which + # replaced it: at ssPostInstall the exception is caught and Setup still + # exits 0. The post-install result now has to reach the module-level flag + # that DeinitializeSetup turns into a nonzero process exit code. + post_install = _pascal_block(code_section, "function RunHumWatchPostInstall") + assert "Result := False;" in post_install + step = _pascal_block(code_section, "procedure CurStepChanged") + assert "if not RunHumWatchPostInstall() then" in step + assert "HumWatchSetupFailed := True;" in step assert "WizardIsTaskSelected('firewallrule')" in code_section assert "WizardIsTaskSelected('firewallpublic')" in code_section for argument in ( @@ -579,7 +605,32 @@ def test_captured_subprocess_output_is_not_column_truncated(): def test_installer_reports_a_failed_post_install_to_its_caller(): - iss = read("installer/HumWatch.iss") - - # A silent install that leaves no working service must not exit 0. - assert "SetupExitCode" in iss or "ExitProcess" in iss or "RaiseException" in iss + code = _pascal_code_without_comments(read("installer/HumWatch.iss")) + + # By ssPostInstall, Inno has already committed the install and written the + # uninstall key. An exception raised there is caught, logged, shown as a + # runtime error, and Setup still deinitializes with exit code 0. CI run + # 30760330669 proved exactly that. DeinitializeSetup runs late enough to + # control the real process exit code, so the kernel32 ExitProcess call + # placed there is the mechanism, and nothing else in this file may claim + # to be it. + assert "RaiseException" not in code + assert "Abort;" not in code + assert code.count("external 'ExitProcess@kernel32.dll stdcall'") == 1 + + # The flag has to outlive CurStepChanged, so it must be module level. + declaration = code.split("procedure StopHumWatchService", 1)[0] + assert re.search(r"^var\s*$", declaration, re.M) + assert "HumWatchSetupFailed: Boolean;" in declaration + + step = _pascal_block(code, "procedure CurStepChanged") + assert "ssPostInstall" in step + assert "HumWatchSetupFailed := True;" in step + + deinitialize = _pascal_block(code, "procedure DeinitializeSetup") + assert "if HumWatchSetupFailed then" in deinitialize + assert re.findall(r"ExitProcess\((\d+)\)", deinitialize) == ["1"] + + # The install is committed before this can fire, so both failure messages + # have to tell the operator to uninstall before retrying. + assert code.count("Uninstall HumWatch") == 2 From dc14059e49e068d8b5c828efe51e64810e43ebd4 Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 15:55:26 -0400 Subject: [PATCH 20/31] fix: check the secret paths for any access, not just write access Narrowing the shared ACL mask to write-conferring bits was correct for the executable tree, which deliberately grants Users ReadAndExecute. It was wrong for the two secret paths that used the same helper. ReadAndExecute (0x200A9) -band the new mask (0xD0156) is 0, so a BUILTIN\Users read grant on the bearer token or the TLS private key started passing verification silently. Read access to either file is complete authentication or transport compromise, and catching a provisioning regression that leaks them is the whole job of this control. Test-NoUnauthorizedWriteAccess keeps the write mask for the executable paths. A new Test-NoUnauthorizedAccess rejects any allow ACE for Everyone, Authenticated Users, or Users on the token and the private key, with no rights mask at all. The duplicated block in the CI harness gets the same split, and the mask drift test now pins which call site uses which form. Co-Authored-By: Claude Opus 5 --- installer/service-setup.ps1 | 28 ++++++++++++++++- tests/test_windows_service_security.py | 42 ++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index cb7a1c2..b6af462 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -245,9 +245,35 @@ function Test-NoUnauthorizedWriteAccess { } } +# The executable tree deliberately grants Users ReadAndExecute, so it can only +# be checked for write access. The bearer token and the TLS private key are a +# different question: read access to either one is complete authentication or +# transport compromise, so no world or regular-user principal may hold any +# access at all. There is no rights mask here on purpose. +function Test-NoUnauthorizedAccess { + param( + [Parameter(Mandatory)][string[]]$Paths, + [Parameter(Mandatory)][string]$Description + ) + + $blockedSids = @("S-1-1-0", "S-1-5-11", "S-1-5-32-545") + foreach ($path in $Paths) { + if (-not (Test-Path -LiteralPath $path)) { + throw "Installed state verification could not find $Description path: $path" + } + $acl = Get-ServiceAcl $path + foreach ($rule in @($acl.Access)) { + $sid = $rule.IdentityReference.Translate([Security.Principal.SecurityIdentifier]).Value + if ($rule.AccessControlType -eq "Allow" -and $blockedSids -contains $sid) { + throw "A regular user or world principal still has access to $Description path: $path" + } + } + } +} + function Verify-InstalledState { Test-NoUnauthorizedWriteAccess -Paths @($AppDir, $NssmPath, $PythonPath, (Join-Path $AppDir "agent")) -Description "executable" - Test-NoUnauthorizedWriteAccess -Paths @($TokenPath, $PrivateKeyPath) -Description "secret" + Test-NoUnauthorizedAccess -Paths @($TokenPath, $PrivateKeyPath) -Description "secret" Write-Log "Verified ACLs for application root, service binary, Python, agent, token, and TLS key." } diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index acf7a8b..c08afeb 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -200,16 +200,24 @@ def test_windows_installed_state_acl_and_firewall_contract(): if ($service.State -ne 'Running') { throw "HumWatch service is not Running: $($service.State)" } $params = Get-ItemProperty 'HKLM:\SYSTEM\CurrentControlSet\Services\HumWatch\Parameters' -ErrorAction Stop $appDir = $params.AppDirectory -$paths = @($appDir, (Join-Path $appDir 'tools\nssm.exe'), (Join-Path $appDir 'python\python.exe'), (Join-Path $appDir 'agent'), "$env:ProgramData\HumWatch\auth-token", "$env:ProgramData\HumWatch\tls\humwatch-key.pem") +$executablePaths = @($appDir, (Join-Path $appDir 'tools\nssm.exe'), (Join-Path $appDir 'python\python.exe'), (Join-Path $appDir 'agent')) +$secretPaths = @("$env:ProgramData\HumWatch\auth-token", "$env:ProgramData\HumWatch\tls\humwatch-key.pem") $blocked = @('S-1-1-0', 'S-1-5-11', 'S-1-5-32-545') $write = [Security.AccessControl.FileSystemRights]::Write -bor [Security.AccessControl.FileSystemRights]::Delete -bor [Security.AccessControl.FileSystemRights]::DeleteSubdirectoriesAndFiles -bor [Security.AccessControl.FileSystemRights]::ChangePermissions -bor [Security.AccessControl.FileSystemRights]::TakeOwnership -foreach ($path in $paths) { +foreach ($path in $executablePaths) { if (-not (Test-Path -LiteralPath $path)) { throw "Missing installed path: $path" } foreach ($rule in (Get-Acl -LiteralPath $path).Access) { $sid = $rule.IdentityReference.Translate([Security.Principal.SecurityIdentifier]).Value if ($rule.AccessControlType -eq 'Allow' -and $blocked -contains $sid -and (($rule.FileSystemRights -band $write) -ne 0)) { throw "Unauthorized write access: $path" } } } +foreach ($path in $secretPaths) { + if (-not (Test-Path -LiteralPath $path)) { throw "Missing installed secret path: $path" } + foreach ($rule in (Get-Acl -LiteralPath $path).Access) { + $sid = $rule.IdentityReference.Translate([Security.Principal.SecurityIdentifier]).Value + if ($rule.AccessControlType -eq 'Allow' -and $blocked -contains $sid) { throw "Unauthorized access to secret: $path" } + } +} $rule = Get-NetFirewallRule -DisplayName 'HumWatch' -ErrorAction SilentlyContinue if (-not $rule) { if ($env:HUMWATCH_REQUIRE_INSTALLED_STATE -eq '1') { throw 'HumWatch firewall rule is required but is not installed' } @@ -434,11 +442,39 @@ def test_write_check_mask_excludes_read_bits_pulled_in_by_composite_rights(): "assertion above while still breaking the contract that the two copies stay identical" ) + # The two calls in Verify-InstalledState ask different questions. The + # executable tree deliberately grants Users ReadAndExecute, so it gets the + # write-only mask above. The bearer token and the TLS private key must be + # unreachable by any regular user, so they get an any-access check with no + # rights mask at all. Narrowing one shared mask to write bits silently + # stopped the secret paths from being checked for read access, and read + # access to those two files is full authentication and transport bypass. + verify = source.split("function Verify-InstalledState", 1)[1].split("\nfunction ", 1)[0] + executable_calls = [line for line in verify.splitlines() if '-Description "executable"' in line] + secret_calls = [line for line in verify.splitlines() if '-Description "secret"' in line] + assert len(executable_calls) == 1 and len(secret_calls) == 1 + assert "Test-NoUnauthorizedWriteAccess " in executable_calls[0] + assert "Test-NoUnauthorizedAccess " in secret_calls[0] + assert "Test-NoUnauthorizedWriteAccess" not in secret_calls[0] + assert "$TokenPath" in secret_calls[0] and "$PrivateKeyPath" in secret_calls[0] + + any_access = source.split("function Test-NoUnauthorizedAccess", 1)[1].split("\nfunction ", 1)[0] + assert "-band" not in any_access, "the secret-path check must not filter by a rights mask" + assert "FileSystemRights" not in any_access + for sid in ("S-1-1-0", "S-1-5-11", "S-1-5-32-545"): + assert sid in any_access, sid + + harness_secret_loop = own_source.split("foreach ($path in $secretPaths) {", 1)[1].split( + "\n}", 1 + )[0] + assert "-band" not in harness_secret_loop + assert "$blocked -contains $sid" in harness_secret_loop + def test_acl_verification_checks_effective_write_bits_for_world_and_users(): source = read("installer/service-setup.ps1") function_body = source.split("function Test-NoUnauthorizedWriteAccess", 1)[1].split( - "function Verify-InstalledState", 1 + "\nfunction ", 1 )[0] for sid in ("S-1-1-0", "S-1-5-11", "S-1-5-32-545"): From 8469ac3bc797a2a00a3525846724ef5fc3f7ce7b Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 15:57:07 -0400 Subject: [PATCH 21/31] fix: harden the application tree ACL in every locale and close the reset window Two defects in Set-ProtectedApplicationAcl. The failed-file count was parsed out of the icacls summary line with an English-only regex. On a German, French, Japanese, Spanish, or Portuguese Windows the match missed, the count fell back to 0, and the fail-closed check degraded to exactly the exit-code-only test that /C makes unreliable. Dropping /C makes icacls stop at the first per-file failure and return nonzero in every locale, so the exit code alone is authoritative and the locale-dependent parse is removed rather than left as dead code. The trade is that one locked file now aborts the install instead of being reported at the end, which is the intended fail-closed behavior. The tree was also reset to inherited before the root was protected. C:\ grants Authenticated Users an inheritable Modify and the default install path sits directly under it, so for the length of two passes over roughly 3000 files every file in the tree was writable by any local user. On a fresh install that is not a regression. On an upgrade it un-hardens a protected tree that the service executes from. The root ACL now goes on first and the reset is scoped to the children, since resetting the root itself would drop the protection just applied. Co-Authored-By: Claude Opus 5 --- installer/service-setup.ps1 | 67 ++++++++++++++------------ tests/test_windows_service_security.py | 43 ++++++++++++----- 2 files changed, 68 insertions(+), 42 deletions(-) diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index b6af462..a3e4ad7 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -107,37 +107,14 @@ function Set-ProtectedApplicationAcl { param([Parameter(Mandatory)][string]$Path) $aclTarget = $Path.TrimEnd('\') - # A per-file Get-Acl/Set-Acl loop over the bundled Python runtime takes - # tens of minutes. icacls flattens the children in one native pass and the - # protected root ACL below propagates to them through normal inheritance. - # /C keeps icacls going past per-file failures, so a zero exit code alone - # does not prove every file succeeded. Capture the output, log the - # "Successfully processed N files" and "Failed processing M files" - # summary line, and fail closed when M is greater than zero. If that - # summary line is missing or does not match, do not treat the parse - # failure itself as an ACL failure, just fall back to the exit code check. - $resetOutput = & icacls $aclTarget /reset /T /C /Q 2>&1 - $resetExitCode = $LASTEXITCODE - $resetText = ($resetOutput | Out-String) - $resetMatch = [regex]::Match($resetText, 'Successfully processed \d+ files?; Failed processing (\d+) files?') - $resetSummaryLine = if ($resetMatch.Success) { $resetMatch.Value.Trim() } else { "icacls /reset summary line not found" } - Write-Log "icacls /reset for ${aclTarget}: $resetSummaryLine" - $resetFailedCount = if ($resetMatch.Success) { [int]$resetMatch.Groups[1].Value } else { 0 } - if ($resetExitCode -ne 0 -or $resetFailedCount -gt 0) { - throw "The application tree could not be reset to inherited permissions (exit code $resetExitCode, $resetFailedCount file(s) reported failed)." - } - - $ownerOutput = & icacls $aclTarget /setowner "*S-1-5-32-544" /T /C /Q 2>&1 - $ownerExitCode = $LASTEXITCODE - $ownerText = ($ownerOutput | Out-String) - $ownerMatch = [regex]::Match($ownerText, 'Successfully processed \d+ files?; Failed processing (\d+) files?') - $ownerSummaryLine = if ($ownerMatch.Success) { $ownerMatch.Value.Trim() } else { "icacls /setowner summary line not found" } - Write-Log "icacls /setowner for ${aclTarget}: $ownerSummaryLine" - $ownerFailedCount = if ($ownerMatch.Success) { [int]$ownerMatch.Groups[1].Value } else { 0 } - if ($ownerExitCode -ne 0 -or $ownerFailedCount -gt 0) { - throw "The application tree owner could not be set to Administrators (exit code $ownerExitCode, $ownerFailedCount file(s) reported failed)." - } + # The protected root ACL goes on first. C:\ grants Authenticated Users an + # inheritable Modify, and the default install path sits directly under it, + # so resetting the tree before the root is protected leaves every file, + # including python\python.exe and the whole agent package, writable by any + # local user until the root ACL lands. Harmless on a fresh install, where + # the tree already inherits those grants, but on an upgrade it un-hardens a + # protected tree that NT AUTHORITY\LocalService executes from. $systemSid = [Security.Principal.SecurityIdentifier]::new("S-1-5-18") $administratorsSid = [Security.Principal.SecurityIdentifier]::new("S-1-5-32-544") $usersSid = [Security.Principal.SecurityIdentifier]::new("S-1-5-32-545") @@ -162,6 +139,36 @@ function Set-ProtectedApplicationAcl { $inheritance, [Security.AccessControl.PropagationFlags]::None, $allow )) Set-Acl -LiteralPath $aclTarget -AclObject $acl + + # A per-file Get-Acl/Set-Acl loop over the bundled Python runtime takes + # tens of minutes. icacls flattens the children in one native pass and they + # pick the rules above up through normal inheritance. The reset is scoped + # to the children ("$aclTarget\*") on purpose: running it on the root would + # drop the protection just applied and re-inherit the C:\ grants. + # + # icacls runs without /C. With /C it continues past per-file failures and + # can still exit 0, and its failed-file summary line is localized, so + # parsing that count only worked on English Windows and degraded to a + # fail-open exit-code check everywhere else. Without /C, icacls stops at + # the first failure and returns nonzero in every locale. + $resetOutput = & icacls "$aclTarget\*" /reset /T /Q 2>&1 + $resetExitCode = $LASTEXITCODE + Write-Log "icacls /reset for ${aclTarget}: exit code $resetExitCode" + if ($resetExitCode -ne 0) { + Write-Log "icacls /reset output: $(($resetOutput | Out-String -Width 4096).Trim())" + throw "The application tree could not be reset to inherited permissions (exit code $resetExitCode)." + } + + # An owner always carries implicit READ_CONTROL and WRITE_DAC, so a file + # still owned by whoever unpacked the tree can re-grant itself write access + # regardless of the DACL above. + $ownerOutput = & icacls $aclTarget /setowner "*S-1-5-32-544" /T /Q 2>&1 + $ownerExitCode = $LASTEXITCODE + Write-Log "icacls /setowner for ${aclTarget}: exit code $ownerExitCode" + if ($ownerExitCode -ne 0) { + Write-Log "icacls /setowner output: $(($ownerOutput | Out-String -Width 4096).Trim())" + throw "The application tree owner could not be set to Administrators (exit code $ownerExitCode)." + } } function Set-ProtectedWritableDirectoryAcl { diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index c08afeb..54a752e 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -535,11 +535,22 @@ def test_application_tree_acl_uses_native_icacls_not_a_per_file_loop(): # CI hang. The application tree must be flattened by icacls and receive a # single inheritable root ACL instead. assert "icacls" in section - assert "/reset /T /C /Q" in section + assert "/reset /T /Q" in section assert "/setowner" in section assert "Get-ChildItem" not in section assert "S-1-5-32-545" in section + # C:\ grants Authenticated Users inheritable Modify and the default install + # path C:\HumWatch sits directly under it, so resetting the tree before the + # root is protected leaves every file, python\python.exe included, writable + # by any local user until the final Set-Acl lands. On an upgrade that is a + # new privilege-escalation window into a tree the service executes from. + # The root ACL goes on first, and the reset is scoped to the children so it + # cannot drop the protection that was just applied to the root. + assert section.index("Set-Acl -LiteralPath $aclTarget") < section.index("& icacls") + reset_call = [line for line in section.splitlines() if "/reset" in line][0] + assert r'"$aclTarget\*"' in reset_call, reset_call + # The provisioner must not run its own second recursive pass over AppDir. assert 'Set-RestrictedAcl -Path $AppDir' not in provision @@ -583,17 +594,25 @@ def test_application_tree_acl_fails_closed_on_icacls_per_file_failures(): section = setup.split("function Set-ProtectedApplicationAcl", 1)[1].split( "function Set-ProtectedWritableDirectoryAcl", 1 )[0] - # /C makes icacls continue past per-file ACL failures, so it can still - # exit 0 while its summary line reports "Failed processing M files" with - # M greater than zero. Piping the output straight to Out-Null (the - # original gap) throws that signal away and leaves exit code as the only - # check, which /C can make misleadingly clean. The output must be - # captured, the failed-file count parsed out of the summary line, logged, - # and checked so a nonzero count fails the install even when the exit - # code is 0. - assert "Out-Null" not in section - assert "Failed processing" in section - assert "FailedCount -gt 0" in section + # /C makes icacls continue past per-file ACL failures, so it can exit 0 + # while its summary line reports "Failed processing M files" with M greater + # than zero. Piping the output to Out-Null threw that signal away entirely. + # Parsing the summary line was the next attempt, but icacls localizes it, + # so on a German, Japanese, or Spanish Windows the regex missed, the count + # fell back to 0, and the check degraded to exactly the fail-open exit-code + # test it replaced. Dropping /C makes icacls stop at the first failure and + # return nonzero in every locale, so the exit code alone is authoritative + # and the locale-dependent parse is gone rather than left as dead code. + icacls_calls = [line.strip() for line in section.splitlines() if "& icacls" in line] + assert len(icacls_calls) == 2 + for call in icacls_calls: + assert "/C" not in call, call + assert "2>&1" in call, call + assert "Out-Null" not in call, call + assert "Failed processing" not in section + assert "FailedCount" not in section + assert section.count("ExitCode -ne 0") == 2 + assert section.count("throw ") == 2 assert "Write-Log" in section From 2632267ebc237b0ea2335852a87c022efd237aaf Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 15:59:42 -0400 Subject: [PATCH 22/31] fix: bring update.bat up to the installer's ACL and module path handling update.bat re-hardens the same tree the service executes from, so a user-writable file left anywhere under it is local privilege escalation. Two fixes this branch made in service-setup.ps1 never reached this copy. Set-HardenedAppAcl kept the fail-open icacls pattern: /C plus an exit-code-only check, with the output piped to Out-Null so a per-file failure was invisible. It also never set the owner, and an owner carries implicit WRITE_DAC and can re-grant itself write regardless of the DACL, which matters because a portable tree is unpacked by whatever account ran the unzip. It also reset the tree before protecting the root. All three now match Set-ProtectedApplicationAcl, and a new test pins the two copies together the way the ACL mask drift test already does. The script also called Get-Acl and Set-Acl with no PSModulePath rebuild. cmd hands powershell.exe the caller's environment, so running the updater from a pwsh 7 prompt gives 5.1 the 7.x module tree and Get-Acl fails to autoload Microsoft.PowerShell.Security. Same root cause the branch already fixed twice, left live in the third script with the pattern. Co-Authored-By: Claude Opus 5 --- tests/test_windows_service_security.py | 41 ++++++++++++++++++++++++++ update.bat | 23 ++++++++++++--- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 54a752e..4491b21 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -619,6 +619,7 @@ def test_application_tree_acl_fails_closed_on_icacls_per_file_failures(): def test_installer_scripts_rebuild_the_windows_powershell_module_path(): setup = read("installer/service-setup.ps1") provision = read("scripts/provision-security.ps1") + update = read("update.bat") # PowerShell 7 exports its own PSModulePath. When powershell.exe 5.1 # inherits it, autoloading Microsoft.PowerShell.Security resolves to the @@ -633,6 +634,46 @@ def test_installer_scripts_rebuild_the_windows_powershell_module_path(): first_acl_index = provision.index("Get-Acl") assert reset_index < first_acl_index + # update.bat launches powershell.exe from cmd and inherits the caller's + # environment exactly the way Inno did. Run from a pwsh 7 prompt it hands + # 5.1 the 7.x module tree and Get-Acl fails for the same reason. Same bug, + # third script with the vulnerable pattern. + assert "GetEnvironmentVariable('PSModulePath','Machine')" in update + assert update.index("PSModulePath") < update.index("Get-Acl") + + +def test_update_batch_hardens_the_application_tree_the_way_the_installer_does(): + setup = read("installer/service-setup.ps1") + update = read("update.bat") + + installer_section = setup.split("function Set-ProtectedApplicationAcl", 1)[1].split( + "function Set-ProtectedWritableDirectoryAcl", 1 + )[0] + update_section = update.split("function Set-HardenedAppAcl", 1)[1].split( + "function Stop-HumWatch", 1 + )[0] + + # update.bat re-hardens the same tree that the SYSTEM or LocalService + # account executes from, so a user-writable file left anywhere under it is + # local privilege escalation. The installer's fixes for that never reached + # this copy: it kept /C plus an exit-code-only check (fail open on per-file + # ACL failures), never set the owner (an owner carries implicit WRITE_DAC + # and can re-grant itself write), and reset the tree before protecting the + # root. All three now have to match. + for section in (installer_section, update_section): + icacls_calls = [line for line in section.splitlines() if "& icacls" in line] + assert len(icacls_calls) == 2, section + for call in icacls_calls: + assert "/C" not in call, call + assert "Out-Null" not in call, call + assert "/setowner" in section + assert "*S-1-5-32-544" in section + assert section.index("Set-Acl -LiteralPath") < section.index("& icacls") + + assert update_section.count("$LASTEXITCODE -ne 0") == 2 + reset_call = [line for line in update_section.splitlines() if "/reset" in line][0] + assert r"($aclTarget + '\*')" in reset_call, reset_call + def test_advertised_identity_list_cannot_contain_empty_entries(): setup = read("installer/service-setup.ps1") diff --git a/update.bat b/update.bat index 07630bc..c3075d8 100644 --- a/update.bat +++ b/update.bat @@ -35,9 +35,22 @@ if errorlevel 1 ( echo Running with administrator privileges. echo. +:: PowerShell 7 exports its own PSModulePath and cmd hands this child process +:: the caller's environment, so Windows PowerShell 5.1 can inherit the 7.x +:: module tree and fail to autoload Microsoft.PowerShell.Security for Get-Acl +:: and Set-Acl. The first statement below rebuilds 5.1's own default, the same +:: way installer\service-setup.ps1 and scripts\provision-security.ps1 do. +:: +:: Set-HardenedAppAcl mirrors Set-ProtectedApplicationAcl: the protected root +:: ACL is applied before anything is reset, the reset is scoped to the children +:: so it cannot drop that protection, icacls runs without /C so a per-file +:: failure returns nonzero in every locale, and ownership is forced to +:: Administrators because an owner carries implicit WRITE_DAC regardless of the +:: DACL. tests/test_windows_service_security.py pins the two copies together. powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ "$ErrorActionPreference = 'Stop'; Set-StrictMode -Version Latest; " ^ "$ProgressPreference = 'SilentlyContinue'; " ^ + "$machineModulePath = [Environment]::GetEnvironmentVariable('PSModulePath','Machine'); if ($machineModulePath) { $env:PSModulePath = $machineModulePath }; " ^ "$root = [IO.Path]::GetFullPath($env:HUMWATCH_UPDATE_ROOT); " ^ "$runtimeRoot = Join-Path $env:ProgramData 'HumWatch'; " ^ "$statusPath = Join-Path $root '_update_status.txt'; " ^ @@ -85,22 +98,24 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ " # Apply the hardened ACL inside the transaction so the first hardened " ^ " # release is reachable, then let the verification below confirm it. " ^ " $aclTarget = $path.TrimEnd('\'); " ^ - " & icacls $aclTarget /reset /T /C /Q | Out-Null; " ^ - " if ($LASTEXITCODE -ne 0) { throw 'The application tree could not be reset to inherited permissions' }; " ^ " $systemSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-18'); " ^ " $administratorsSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-32-544'); " ^ " $usersSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-32-545'); " ^ " $inherit = [Security.AccessControl.InheritanceFlags]::ContainerInherit -bor [Security.AccessControl.InheritanceFlags]::ObjectInherit; " ^ " $noPropagation = [Security.AccessControl.PropagationFlags]::None; " ^ " $allow = [Security.AccessControl.AccessControlType]::Allow; " ^ - " $acl = Get-Acl -LiteralPath $path -ErrorAction Stop; " ^ + " $acl = Get-Acl -LiteralPath $aclTarget -ErrorAction Stop; " ^ " $acl.SetAccessRuleProtection($true, $false); " ^ " $acl.SetOwner($administratorsSid); " ^ " foreach ($rule in @($acl.Access)) { [void]$acl.RemoveAccessRuleAll($rule) }; " ^ " $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($systemSid, [Security.AccessControl.FileSystemRights]::ReadAndExecute, $inherit, $noPropagation, $allow)); " ^ " $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($administratorsSid, [Security.AccessControl.FileSystemRights]::FullControl, $inherit, $noPropagation, $allow)); " ^ " $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($usersSid, [Security.AccessControl.FileSystemRights]::ReadAndExecute, $inherit, $noPropagation, $allow)); " ^ - " Set-Acl -LiteralPath $path -AclObject $acl " ^ + " Set-Acl -LiteralPath $aclTarget -AclObject $acl; " ^ + " $resetOutput = & icacls ($aclTarget + '\*') /reset /T /Q 2>&1; " ^ + " if ($LASTEXITCODE -ne 0) { throw ('The application tree could not be reset to inherited permissions: ' + (($resetOutput | Out-String -Width 4096).Trim())) }; " ^ + " $ownerOutput = & icacls $aclTarget /setowner '*S-1-5-32-544' /T /Q 2>&1; " ^ + " if ($LASTEXITCODE -ne 0) { throw ('The application tree owner could not be set to Administrators: ' + (($ownerOutput | Out-String -Width 4096).Trim())) } " ^ "}; " ^ "function Stop-HumWatch { " ^ " $current = Get-Service -Name HumWatch -ErrorAction SilentlyContinue; if (-not $current -or $current.Status -eq 'Stopped') { return }; " ^ From a223413c63d7ae590a3119f074ab9826410a5e56 Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 16:00:55 -0400 Subject: [PATCH 23/31] fix: source every build download URL from the asset manifest Repinning get-pip to an immutable commit URL pinned nothing. verify-downloads.ps1 reads Sha256 and MinimumBytes out of the manifest and lints the shape of Url, but no download path ever used Url, so the field was documentation and build-installer.ps1 kept fetching https://bootstrap.pypa.io/get-pip.py. That is a floating endpoint whose content changes on roughly every pip release, so the next rotation breaks the build with a digest mismatch. Neither guard could see it: the PowerShell floating-URL regex scans only the manifest, and the Python assertion read only asset-versions.ps1. The whole class is fixed rather than the one entry. build-installer.ps1 now dot-sources the manifest and resolves every download URL, and the Python, NSSM, LHM, and Inno Setup version strings, through Get-AssetUrl and Get-Asset. A -PythonVersion that disagrees with the pinned entry now fails immediately instead of several minutes later on a digest mismatch. A new test walks every Get-FileFromUrl call site and fails if the first argument is a literal http URL rather than a manifest-sourced value. Co-Authored-By: Claude Opus 5 --- scripts/build-installer.ps1 | 43 ++++++++++++++++++++++++++----- tests/test_supply_chain_policy.py | 43 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/scripts/build-installer.ps1 b/scripts/build-installer.ps1 index a3ccb99..a3493d7 100644 --- a/scripts/build-installer.ps1 +++ b/scripts/build-installer.ps1 @@ -58,6 +58,25 @@ $StageDir = Join-Path $DistDir "stage" $ManifestPath = Join-Path $ProjectRoot "scripts\asset-versions.ps1" $VerifyScript = Join-Path $ProjectRoot "scripts\verify-downloads.ps1" +# The manifest is the single authority for what this build fetches. Its Sha256 +# is verified after every download, but nothing used to read its Url, so a +# repin there changed the documentation and not the download. Every URL below +# comes out of $AssetManifest, so a manifest entry and the bytes on disk cannot +# describe different upstreams. +. $ManifestPath + +function Get-Asset([string]$assetName) { + $asset = $AssetManifest | Where-Object { $_.Name -eq $assetName } | Select-Object -First 1 + if (-not $asset) { throw "No entry named '$assetName' in $ManifestPath" } + return $asset +} + +function Get-AssetUrl([string]$assetName) { + $asset = Get-Asset $assetName + if (-not $asset.Url) { throw "Manifest entry '$assetName' has no Url" } + return [string]$asset.Url +} + # ---- Helpers --------------------------------------------------------------- function Write-Step([string]$msg) { @@ -207,9 +226,17 @@ $PyStageDirPath = Join-Path $StageDir "python" $PyEmbedZip = Join-Path $DistDir "python-$PythonVersion-embed-amd64.zip" $PyMajorMinor = ($PythonVersion -split '\.')[0..1] -join "" # e.g. "312" +# -PythonVersion cannot silently disagree with the pinned entry. The digest +# would reject the download anyway, several minutes later and with a much worse +# error message. +$PyManifestVersion = (Get-Asset "python-embed").Version +if ($PythonVersion -ne $PyManifestVersion) { + Write-Fail "Requested Python $PythonVersion but scripts\asset-versions.ps1 pins $PyManifestVersion. Update the manifest first." +} + if (-not (Test-Path $PyEmbedZip)) { New-Item -ItemType Directory -Path $DistDir -Force | Out-Null - $PyUrl = "https://www.python.org/ftp/python/$PythonVersion/python-$PythonVersion-embed-amd64.zip" + $PyUrl = Get-AssetUrl "python-embed" $ok = Get-FileFromUrl $PyUrl $PyEmbedZip "Python $PythonVersion embeddable" "python-embed" if (-not $ok) { Write-Fail "Python download failed." } } else { @@ -258,7 +285,7 @@ Write-Ok "Extracted Python $PythonVersion" # Bootstrap pip Write-Info "Bootstrapping pip..." $getPipPath = Join-Path $env:TEMP "get-pip.py" -$ok = Get-FileFromUrl "https://bootstrap.pypa.io/get-pip.py" $getPipPath "get-pip.py" "get-pip" +$ok = Get-FileFromUrl (Get-AssetUrl "get-pip") $getPipPath "get-pip.py" "get-pip" if (-not $ok) { Write-Fail "Could not download get-pip.py." } $pyExe = Join-Path $PyStageDirPath "python.exe" @@ -324,7 +351,8 @@ if (Test-Path $NssmSource) { } else { $nssmZip = Join-Path $env:TEMP "nssm.zip" $nssmExtract = Join-Path $env:TEMP "nssm-extract" - $ok = Get-FileFromUrl "https://nssm.cc/release/nssm-2.24.zip" $nssmZip "NSSM 2.24" "nssm" + $nssmVersion = (Get-Asset "nssm").Version + $ok = Get-FileFromUrl (Get-AssetUrl "nssm") $nssmZip "NSSM $nssmVersion" "nssm" if (-not $ok) { Write-Fail "NSSM download failed." } & $VerifyScript -Manifest $ManifestPath -AssetName "nssm" -Path $nssmZip if ($LASTEXITCODE -ne 0) { throw "NSSM verification failed" } @@ -359,10 +387,10 @@ if (Test-Path $LhmDll) { Copy-Item "$LhmLibDir\*" $LhmStageDir -Force Write-Ok "Copied LHM DLLs from lib/" } else { - $lhmVersion = "0.9.6" + $lhmVersion = (Get-Asset "lhm").Version $lhmZip = Join-Path $env:TEMP "lhm.zip" $lhmExtract = Join-Path $env:TEMP "lhm-extract" - $lhmUrl = "https://github.com/LibreHardwareMonitor/LibreHardwareMonitor/releases/download/v$lhmVersion/LibreHardwareMonitor.zip" + $lhmUrl = Get-AssetUrl "lhm" $ok = Get-FileFromUrl $lhmUrl $lhmZip "LibreHardwareMonitor $lhmVersion" "lhm" if (-not $ok) { Write-Fail "LHM download failed." } & $VerifyScript -Manifest $ManifestPath -AssetName "lhm" -Path $lhmZip @@ -411,8 +439,9 @@ $iscc = Find-Iscc if (-not $iscc) { Write-Warn "Inno Setup not found. Downloading and installing silently..." $isSetupExe = Join-Path $env:TEMP "innosetup.exe" - $isSetupUrl = "https://github.com/jrsoftware/issrc/releases/download/is-6_7_3/innosetup-6.7.3.exe" - $ok = Get-FileFromUrl $isSetupUrl $isSetupExe "Inno Setup 6.7.3" "inno-setup" + $isSetupVersion = (Get-Asset "inno-setup").Version + $isSetupUrl = Get-AssetUrl "inno-setup" + $ok = Get-FileFromUrl $isSetupUrl $isSetupExe "Inno Setup $isSetupVersion" "inno-setup" if (-not $ok) { Write-Fail "Could not download Inno Setup. Install from https://jrsoftware.org/isdl.php" } Start-Process -FilePath $isSetupExe -ArgumentList "/VERYSILENT /SUPPRESSMSGBOXES /NORESTART" -Wait Remove-Item $isSetupExe -Force -ErrorAction SilentlyContinue diff --git a/tests/test_supply_chain_policy.py b/tests/test_supply_chain_policy.py index ffbf8bf..5c8b96c 100644 --- a/tests/test_supply_chain_policy.py +++ b/tests/test_supply_chain_policy.py @@ -82,6 +82,49 @@ def test_download_and_build_scripts_verify_before_use(): assert "AssetName" in source or "-CheckOnly" in source or "Verify-Download" in source +def test_build_installer_downloads_only_manifest_sourced_urls(): + source = read("scripts/build-installer.ps1") + + # Repinning get-pip to an immutable commit URL in the manifest changed + # nothing on its own: verify-downloads.ps1 only ever reads Sha256 and + # MinimumBytes out of the manifest, so the Url field was documentation and + # the build kept fetching the floating bootstrap.pypa.io copy. Every + # download URL has to come out of the manifest for the pin to mean + # anything, and a hardcoded literal at any call site puts it back. + assert ". $ManifestPath" in source + assert "function Get-AssetUrl" in source + assert "$AssetManifest" in source + + call_lines = [ + line.strip() + for line in source.splitlines() + if "Get-FileFromUrl" in line and not line.strip().startswith("function") + ] + assert len(call_lines) >= 5 + for line in call_lines: + assert not re.search(r"https?://", line), line + first_argument = line.split("Get-FileFromUrl", 1)[1].lstrip() + if first_argument.startswith("(Get-AssetUrl"): + continue + match = re.match(r"\$\w+", first_argument) + assert match, line + variable = match.group(0) + assignment = re.search(rf"^\s*{re.escape(variable)}\s*=\s*(.+)$", source, re.M) + assert assignment, line + assert "Get-AssetUrl" in assignment.group(1), line + + +def test_no_download_script_hardcodes_a_floating_upstream_url(): + for path in ( + "scripts/build-installer.ps1", + "scripts/download-lhm.ps1", + "scripts/build-release.ps1", + ): + source = read(path) + assert "bootstrap.pypa.io" not in source, path + assert not re.search(r"https?://\S*/latest\b", source), path + + def test_installer_cannot_skip_verified_python_rebuild(): source = read("scripts/build-installer.ps1") From 2e184e5486e70e0e843daeda56e71ed6a4d47693 Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 16:01:41 -0400 Subject: [PATCH 24/31] fix: create the TLS private key owner-only instead of at the umask Path.write_bytes creates the key at the process umask, which is 0644 on a normal Linux box. The Windows installed path is covered, since provision-security.ps1 hardens the TLS directory before calling this and re-restricts the key right after, but the script also ships in {app}\tools and in the portable release as a standalone entry point. Running it directly produced a world-readable RSA private key. The key is now created through os.open with mode 0600, so there is no window where it exists with wider permissions rather than a mode fixed after the fact. Any existing key file is unlinked first, because the mode argument only applies to a file the call creates and a stale 0644 key would otherwise keep its permissions through the rewrite. Co-Authored-By: Claude Opus 5 --- scripts/generate_certificate.py | 22 +++++++++++++++++- tests/test_security_provisioning.py | 35 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/scripts/generate_certificate.py b/scripts/generate_certificate.py index fef8503..79448f4 100644 --- a/scripts/generate_certificate.py +++ b/scripts/generate_certificate.py @@ -3,6 +3,7 @@ import argparse import datetime import ipaddress +import os import sys from pathlib import Path @@ -58,7 +59,7 @@ def generate_certificate(certificate_path, key_path, hostname, bind_identity, ad .add_extension(x509.SubjectKeyIdentifier.from_public_key(key.public_key()), critical=False) .sign(key, hashes.SHA256()) ) - Path(key_path).write_bytes(key.private_bytes( + _write_private_key(key_path, key.private_bytes( serialization.Encoding.PEM, serialization.PrivateFormat.PKCS8, serialization.NoEncryption(), @@ -66,6 +67,25 @@ def generate_certificate(certificate_path, key_path, hostname, bind_identity, ad Path(certificate_path).write_bytes(certificate.public_bytes(serialization.Encoding.PEM)) +def _write_private_key(key_path, key_bytes): + """Create the key file owner-only, never wider and then narrowed. + + A plain write_bytes creates the file at the process umask, so 0644 on a + normal Linux box. The Windows installed path is covered (the TLS directory + is hardened before this runs and the key is re-restricted right after), but + this script is a standalone entry point shipped in the installer and the + portable release, so running it directly handed out a world-readable RSA + private key. Any existing file is removed rather than truncated, because + the mode argument below only applies to a file this call creates, and an + old key sitting at 0644 would otherwise keep its permissions. + """ + key_path = Path(key_path) + key_path.unlink(missing_ok=True) + descriptor = os.open(key_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + with os.fdopen(descriptor, "wb") as handle: + handle.write(key_bytes) + + def main(): parser = argparse.ArgumentParser() parser.add_argument("--certificate", required=True) diff --git a/tests/test_security_provisioning.py b/tests/test_security_provisioning.py index e4cdebc..5fe0e8f 100644 --- a/tests/test_security_provisioning.py +++ b/tests/test_security_provisioning.py @@ -1,4 +1,5 @@ import os +import stat import subprocess import sys from pathlib import Path @@ -300,6 +301,40 @@ def test_generate_certificate_works_when_its_own_directory_is_not_on_sys_path(tm assert certificate.subject.rfc4514_string() == "CN=hum-test" +@pytest.mark.skipif(os.name == "nt", reason="POSIX file modes do not describe Windows ACLs") +def test_generate_certificate_never_leaves_the_private_key_world_readable(tmp_path): + pytest.importorskip("cryptography") + + cert_path = tmp_path / "cert.pem" + key_path = tmp_path / "key.pem" + + # The Windows installed path hardens the TLS directory before this runs and + # re-restricts the key right after. This script is also a standalone entry + # point, shipped in {app}\tools and in the portable release, so anyone + # running it directly used to get an RSA private key at the process umask, + # which is 0644 on a normal Linux box. A pre-existing key file is replaced + # rather than truncated in place so a permissive mode cannot be inherited, + # and there is no window where the new key exists with wider permissions. + key_path.write_bytes(b"stale key\n") + key_path.chmod(0o644) + + completed = subprocess.run( + [ + sys.executable, + str(ROOT / "scripts" / "generate_certificate.py"), + "--certificate", str(cert_path), + "--key", str(key_path), + "--hostname", "hum-test", + "--bind-identity", "127.0.0.1", + ], + capture_output=True, + text=True, + ) + assert completed.returncode == 0, completed.stderr + assert stat.S_IMODE(key_path.stat().st_mode) == 0o600 + assert b"PRIVATE KEY" in key_path.read_bytes() + + def test_provisioning_captures_certificate_generation_output_on_failure(): source = Path("scripts/provision-security.ps1").read_text(encoding="utf-8") From 73e026f23f4076e4e027db22f32267ba4e58de1b Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 16:03:29 -0400 Subject: [PATCH 25/31] fix: stop four source-level tests from passing against a broken implementation Each of these asserted something weaker than it read. The advertised-identity test asserted "Where-Object" appears anywhere in service-setup.ps1, which Resolve-FirewallProfiles satisfies on its own, so it held with the fix reverted as long as the old line was not restored verbatim. It now pins the single $AdvertisedIdentity assignment. Its second half was an or over two mechanisms, pinning neither. Both filters are now required. The trap test asserted the bare word "trap", which a comment satisfies. It now requires the trap block itself. The workflow test asserted "if: always()" appeared somewhere in the file rather than on the step that dumps the installer and service setup logs, which is the step that has to survive a failed install. It now scopes to that step. The updater ACL test asserted the substring "readandexecute", which cannot tell the Users grant apart from the SYSTEM grant that predates it. It now pins the whole access rule. Co-Authored-By: Claude Opus 5 --- tests/test_installation_docs.py | 10 ++++++-- tests/test_windows_service_security.py | 33 +++++++++++++++++++++----- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/tests/test_installation_docs.py b/tests/test_installation_docs.py index 871468a..a36c812 100644 --- a/tests/test_installation_docs.py +++ b/tests/test_installation_docs.py @@ -154,9 +154,15 @@ def test_update_is_elevated_stopped_fail_closed_and_acl_verified(): # LocalService reads the application tree through BUILTIN\Users. The # hardened ACL must keep Users read-and-execute or the service cannot - # restart after an update. + # restart after an update. A bare "readandexecute" substring cannot tell + # the Users grant apart from the SYSTEM grant that was always there, so + # pin the whole rule. assert "s-1-5-32-545" in lower - assert "readandexecute" in lower + assert ( + "$acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($usersSid, " + "[Security.AccessControl.FileSystemRights]::ReadAndExecute, $inherit, $noPropagation, " + "$allow))" + ) in source def test_update_requires_the_named_release_and_verified_digest(): diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 4491b21..a06a40e 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -287,9 +287,16 @@ def test_windows_security_workflow_runs_behavioral_contract_suite(): assert "contents: write" in release_section assert "permissions:\n contents: read" in workflow # A silent install that dies on the runner must leave a readable trail. + # "if: always()" anywhere in the file says nothing about the step that + # actually dumps the logs, which is the one that has to survive a failed + # install step. assert "/LOG=" in workflow - assert "if: always()" in workflow - assert "service-setup.log" in workflow + dump_step = workflow.split("- name: Dump installer and service setup logs", 1)[1].split( + " - name:", 1 + )[0] + assert "if: always()" in dump_step + assert "service-setup.log" in dump_step + assert "HUMWATCH_INSTALL_LOG" in dump_step def test_inno_service_setup_runs_are_gated_by_pascal_exit_handling(): @@ -585,7 +592,8 @@ def test_service_setup_logs_terminating_errors_before_provisioning(): creation_index = setup.index("New-Item -ItemType Directory -Path $LogDir") assert log_dir_index < creation_index < provision_index - assert "trap" in setup + # "trap" as a bare substring passes on the word appearing in a comment. + assert "\ntrap {" in setup def test_application_tree_acl_fails_closed_on_icacls_per_file_failures(): @@ -682,13 +690,26 @@ def test_advertised_identity_list_cannot_contain_empty_entries(): # @($null) is a one-element array holding $null, not an empty array. # PowerShell 5.1 drops empty string arguments to native executables, which # leaves python staring at a dangling --advertised-identity flag. - assert "$AdvertisedIdentity = @($AdvertisedIdentity) + $configuredIdentities" not in setup - assert "Where-Object" in setup + # + # A bare "Where-Object in setup" assertion proves nothing here: + # Resolve-FirewallProfiles uses one too, so it holds with the fix reverted + # as long as the old line is not restored character for character. Pin the + # one assignment that matters instead. + assignments = [ + line for line in setup.splitlines() + if line.strip().startswith("$AdvertisedIdentity = ") + ] + assert len(assignments) == 1, assignments + assert "@($AdvertisedIdentity) + @($configuredIdentities)" in assignments[0] + assert "Where-Object { $_ }" in assignments[0] + # The provisioner cannot trust its callers either, and an "a or b" over two + # mechanisms pins neither. Both filters have to be present. certificate_section = provision.split("function New-ServerCertificate", 1)[1].split( "\nfunction ", 1 )[0] - assert "if (-not $identity" in certificate_section or "Where-Object" in certificate_section + assert "if (-not $identity) { continue }" in certificate_section + assert "if (-not $Hostname) {" in certificate_section def test_captured_subprocess_output_is_not_column_truncated(): From 309edb0c91487c797c8f837c11971d9225dc207e Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 23:24:40 -0400 Subject: [PATCH 26/31] fix: convert update.bat line comments to PowerShell block comments cmd's caret continuation on the -Command fragments feeds each quoted line to powershell.exe as a separate argument, which PowerShell then joins with spaces into one logical line. A bare `#` inside any of those fragments comments out every fragment joined after it, and because the enclosing function's closing brace never runs, it is a parse error rather than a silent no-op. Convert the four bare `#` comments (three inside Set-HardenedAppAcl, one before the release secret scan) to `<# ... #>` block comment form, which is safe on a single joined line. No PowerShell logic or batch structure changed. Adds a regression test that reconstructs the joined -Command source and walks it tracking single-quote and block-comment state, so it flags a bare line-comment `#` without false-positiving on `#` inside a quoted string or regex character class. Co-Authored-By: Claude Opus 5 --- tests/test_windows_service_security.py | 83 ++++++++++++++++++++++++++ update.bat | 8 +-- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index a06a40e..f356061 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -683,6 +683,89 @@ def test_update_batch_hardens_the_application_tree_the_way_the_installer_does(): assert r"($aclTarget + '\*')" in reset_call, reset_call +def _update_bat_powershell_source(update: str) -> str: + """Reconstruct the single logical line powershell.exe actually receives. + + update.bat builds its -Command argument out of many double-quoted + fragments, each line ending in a cmd caret continuation. cmd's trailing + caret eats the newline, so each fragment reaches powershell.exe as a + separate -Command argument, and PowerShell joins multiple -Command + arguments with spaces into one logical line before parsing it. A bare `#` + inside any fragment therefore comments out everything joined after it, + across every later fragment, not just the rest of the line it was + written on. + """ + block = update.split("-Command ^\n", 1)[1].split( + '\nset "UPDATE_EXIT=%errorlevel%"', 1 + )[0] + fragments = [] + for line in block.splitlines(): + stripped = line.strip() + if stripped.endswith("^"): + stripped = stripped[:-1].rstrip() + assert stripped.startswith('"') and stripped.endswith('"'), stripped + fragments.append(stripped[1:-1]) + return " ".join(fragments) + + +def test_update_batch_quoted_powershell_has_no_line_ending_comment(): + """A bare `#` anywhere in update.bat's -Command fragments is a parse-time + bug, not a style nit: joined onto one logical line (see + _update_bat_powershell_source), it comments out every fragment after it, + including the closing braces that end open functions and the try block, + which is a parse error rather than a silent no-op. PR #4 introduced four + such comments and broke the portable updater. + + This walks the reconstructed line tracking single-quote and `<# #>` + block-comment state character by character, so a `#` written inside a + PowerShell string or regex character class (single-quoted throughout this + script) does not false-positive, and only a `#` that would actually start + a line comment does. cmd `::`/`rem` lines elsewhere in update.bat are not + part of the reconstructed source at all, so they cannot false-positive + either. + """ + update = read("update.bat") + source = _update_bat_powershell_source(update) + + in_single_quote = False + in_block_comment = False + i = 0 + n = len(source) + while i < n: + ch = source[i] + if in_block_comment: + if source[i : i + 2] == "#>": + in_block_comment = False + i += 2 + else: + i += 1 + continue + if in_single_quote: + if ch == "'": + in_single_quote = False + i += 1 + continue + if ch == "'": + in_single_quote = True + i += 1 + continue + if source[i : i + 2] == "<#": + in_block_comment = True + i += 2 + continue + if ch == "#": + context = source[max(0, i - 40) : i + 40] + pytest.fail( + "bare '#' line comment found in update.bat's joined " + f"-Command source (would comment out everything after it): " + f"...{context}..." + ) + i += 1 + + assert not in_single_quote, "unterminated PowerShell single-quoted string" + assert not in_block_comment, "unterminated PowerShell <# #> block comment" + + def test_advertised_identity_list_cannot_contain_empty_entries(): setup = read("installer/service-setup.ps1") provision = read("scripts/provision-security.ps1") diff --git a/update.bat b/update.bat index c3075d8..c267e7f 100644 --- a/update.bat +++ b/update.bat @@ -94,9 +94,9 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ " } " ^ "}; " ^ "function Set-HardenedAppAcl([string]$path) { " ^ - " # A pre-hardening install still carries inherited or user writable grants. " ^ - " # Apply the hardened ACL inside the transaction so the first hardened " ^ - " # release is reachable, then let the verification below confirm it. " ^ + " <# A pre-hardening install still carries inherited or user writable grants. " ^ + " Apply the hardened ACL inside the transaction so the first hardened " ^ + " release is reachable, then let the verification below confirm it. #> " ^ " $aclTarget = $path.TrimEnd('\'); " ^ " $systemSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-18'); " ^ " $administratorsSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-32-544'); " ^ @@ -146,7 +146,7 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ " Expand-Archive -LiteralPath $zipPath -DestinationPath $extractPath -Force; $innerDirs = @(Get-ChildItem -LiteralPath $extractPath -Directory); $srcRoot = if ($innerDirs.Count -eq 1 -and (Test-Path -LiteralPath (Join-Path $innerDirs[0].FullName 'agent'))) { $innerDirs[0].FullName } else { $extractPath }; " ^ " $requiredEntries = @('agent','static','scripts','installer','tools','requirements.txt','setup.bat','run.bat','update.bat'); foreach ($entry in $requiredEntries) { if (-not (Test-Path -LiteralPath (Join-Path $srcRoot $entry))) { throw 'The release archive is missing a required setup asset' } }; " ^ " New-Item -ItemType Directory -Path $replacementRoot -Force | Out-Null; foreach ($directory in $immutableDirs) { Sync-Directory (Join-Path $srcRoot $directory) (Join-Path $replacementRoot $directory) }; foreach ($file in $rootFiles) { $sourceFile = Join-Path $srcRoot $file; if (-not (Test-Path -LiteralPath $sourceFile -PathType Leaf)) { throw 'The release archive is missing an immutable root file' }; Copy-Item -LiteralPath $sourceFile -Destination (Join-Path $replacementRoot $file) -Force }; " ^ - " # Scan only release supplied content before adding the staged venv. " ^ + " <# Scan only release supplied content before adding the staged venv. #> " ^ " $releaseSecretPattern = '(?i)^(auth-token(?:\.txt)?|token(?:\.txt)?|\.env(?:\..*)?|private(?:\.key|_key)|.*\.db(?:-.*)?|.*\.(?:sqlite|sqlite3|log|key|pem|pfx|p12|crt|csr))$'; " ^ " $forbidden = @(Get-ChildItem -LiteralPath $replacementRoot -Recurse -File | Where-Object { $_.Name -match $releaseSecretPattern }); if ($forbidden.Count -gt 0) { throw 'The staged release contains protected runtime state' }; " ^ " $requirements = Join-Path $replacementRoot 'requirements.txt'; & $livePython -m venv $stagedVenv; if ($LASTEXITCODE -ne 0) { throw 'The replacement Python venv could not be created' }; " ^ From d2008052502945d248b0f7726578bc06b530659c Mon Sep 17 00:00:00 2001 From: Static Date: Mon, 3 Aug 2026 23:35:00 -0400 Subject: [PATCH 27/31] fix: refuse to harden a volume root in both ACL routines Set-ProtectedApplicationAcl trimmed the trailing separator off its target with $Path.TrimEnd('\'). For a drive root like C:\ that yields the drive-relative C:, so the recursive child reset expanded to "icacls C:\* /reset /T" and reset ACLs across the entire system drive, and Get-Acl/Set-Acl against C: landed on the process current directory instead of the drive root. HumWatch.iss leaves DisableDirPage=no and Inno honours /DIR="C:\", so a scripted install could reach it. update.bat's copied Set-HardenedAppAcl had the identical defect. Both copies now normalize the path (fold forward slashes, strip an extended-length prefix, trim the trailing separator) and refuse a volume root before any Get-Acl, Set-Acl, or icacls call, so nothing is mutated on the way to the error. Rejected: C:\, C:, c:/, anything that trims to a bare drive, and a UNC share root such as \\server\share. Accepted: C:\HumWatch, C:\HumWatch\, D:\Program Files\HumWatch, and \\server\share\HumWatch. The root-ACL-before-child-reset ordering is unchanged. Covered behaviorally in the pwsh contract harness for both copies, with a fake icacls so a regression can never reach the real binary, and at the source level so the guard cannot be deleted without a failure on Linux. The icacls fail-closed throw count is now scoped past the first icacls call so the new refusal does not dilute it. Co-Authored-By: Claude Opus 5 --- installer/service-setup.ps1 | 30 +++- tests/test_windows_service_security.py | 211 ++++++++++++++++++++++++- update.bat | 12 +- 3 files changed, 250 insertions(+), 3 deletions(-) diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index a3e4ad7..ee25c40 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -106,7 +106,35 @@ function Get-ServiceAcl { function Set-ProtectedApplicationAcl { param([Parameter(Mandatory)][string]$Path) - $aclTarget = $Path.TrimEnd('\') + # A volume root is never a valid install location, and hardening one is + # catastrophic rather than merely wrong. TrimEnd('\') turns "C:\" into the + # drive-relative "C:", so the child reset below expands to "C:\*" and + # recursively resets ACLs across the entire system drive, while Get-Acl and + # Set-Acl against "C:" land on the process current directory instead of the + # drive root. A UNC share root carries the same recursive blast radius. + # HumWatch.iss leaves DisableDirPage=no and Inno honours /DIR="C:\", so a + # scripted install can reach here with a root. Refuse before anything is + # read or mutated. Forward slashes fold to backslashes first, and the + # extended-length prefixes are stripped, so "c:/" and "\\?\C:\" are judged + # as the roots they name. + $normalizedPath = $Path.Trim().Replace('/', '\') + if ($normalizedPath.StartsWith('\\?\UNC\', [System.StringComparison]::OrdinalIgnoreCase)) { + $normalizedPath = '\\' + $normalizedPath.Substring(8) + } elseif ($normalizedPath.StartsWith('\\?\', [System.StringComparison]::OrdinalIgnoreCase)) { + $normalizedPath = $normalizedPath.Substring(4) + } + $aclTarget = $normalizedPath.TrimEnd('\') + $isVolumeRoot = -not $aclTarget + if ($aclTarget.StartsWith('\\')) { + # A UNC target needs a server, a share, and at least one directory + # below the share. Anything shorter is the share root itself. + $isVolumeRoot = @($aclTarget.Substring(2).Split('\') | Where-Object { $_ }).Count -lt 3 + } elseif ($aclTarget -match '^[A-Za-z]:$') { + $isVolumeRoot = $true + } + if ($isVolumeRoot) { + throw "A volume root is not a permitted HumWatch install location: $Path" + } # The protected root ACL goes on first. C:\ grants Authenticated Users an # inheritable Modify, and the default install path sits directly under it, diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index f356061..927e2ed 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -95,6 +95,31 @@ def _powershell_contract_harness() -> str: Grant-ServiceRuntimeAccess -ServiceSid 'S-1-5-19' Grant-ServiceReadAccess -ServiceSid 'S-1-5-19' +$script:icaclsCalls = @() +function icacls {{ $script:icaclsCalls += ($args -join ' '); $global:LASTEXITCODE = 0 }} + +$volumeRootResults = [ordered]@{{}} +foreach ($candidate in @('C:\', 'C:', 'c:/', 'C:\\', 'D:/', '\\server\share', '\\server\share\')) {{ + $beforeAcl = $script:aclStore.Keys.Count + $beforeIcacls = $script:icaclsCalls.Count + $rejected = $false + $message = '' + try {{ Set-ProtectedApplicationAcl -Path $candidate }} catch {{ $rejected = $true; $message = $_.Exception.Message }} + $volumeRootResults[$candidate] = [ordered]@{{ + rejected = $rejected + message = $message + mutated = (($script:aclStore.Keys.Count -ne $beforeAcl) -or ($script:icaclsCalls.Count -ne $beforeIcacls)) + }} +}} + +$acceptedResults = [ordered]@{{}} +foreach ($candidate in @('C:\HumWatch', 'C:\HumWatch\', 'D:\Program Files\HumWatch', '\\server\share\HumWatch')) {{ + $script:icaclsCalls = @() + $failure = '' + try {{ Set-ProtectedApplicationAcl -Path $candidate }} catch {{ $failure = $_.Exception.Message }} + $acceptedResults[$candidate] = [ordered]@{{ failure = $failure; icacls = @($script:icaclsCalls) }} +}} + $FirewallPort = 9123 $FirewallProfiles = 'Domain,Private' $AllowPublic = $false @@ -125,6 +150,8 @@ def _powershell_contract_harness() -> str: certificate = @($script:aclStore[$CertificatePath].Rules) privateKey = @($script:aclStore[$PrivateKeyPath].Rules) }} + volumeRoots = $volumeRootResults + accepted = $acceptedResults defaultPort = $defaultFirewall.LocalPort defaultProfiles = $defaultProfiles publicDenied = $publicDenied @@ -176,6 +203,38 @@ def test_windows_service_contract_executes_acl_and_firewall_behavior_without_win service_rules = [rule for rule in result["acl"][path] if rule["Sid"] == "S-1-5-19"] assert _granted_rights(service_rules) == ["ReadAndExecute"] + # A volume root has to be refused outright. TrimEnd('\') turns "C:\" into + # the drive-relative "C:", which expands the child reset to "C:\*" and + # recursively resets ACLs across the whole system drive. Nothing may be read + # or written on the way to the error either, so the fake Get-Acl/Set-Acl + # store and the fake icacls call log both have to be untouched. + volume_roots = result["volumeRoots"] + assert sorted(volume_roots) == sorted( + ["C:\\", "C:", "c:/", "C:\\\\", "D:/", "\\\\server\\share", "\\\\server\\share\\"] + ) + for candidate, outcome in volume_roots.items(): + assert outcome["rejected"] is True, candidate + assert "volume root is not a permitted" in outcome["message"], candidate + assert candidate in outcome["message"], candidate + assert outcome["mutated"] is False, candidate + + # Normal install paths still go through, with the trailing separator + # normalized away and the reset scoped to the children of that exact target. + expected_targets = { + "C:\\HumWatch": "C:\\HumWatch", + "C:\\HumWatch\\": "C:\\HumWatch", + "D:\\Program Files\\HumWatch": "D:\\Program Files\\HumWatch", + "\\\\server\\share\\HumWatch": "\\\\server\\share\\HumWatch", + } + assert sorted(result["accepted"]) == sorted(expected_targets) + for candidate, target in expected_targets.items(): + outcome = result["accepted"][candidate] + assert outcome["failure"] == "", candidate + assert outcome["icacls"] == [ + target + "\\* /reset /T /Q", + target + " /setowner *S-1-5-32-544 /T /Q", + ], candidate + assert result["defaultPort"] == 9123 assert result["defaultProfiles"] == ["Domain", "Private"] assert result["publicDenied"] is True @@ -620,7 +679,12 @@ def test_application_tree_acl_fails_closed_on_icacls_per_file_failures(): assert "Failed processing" not in section assert "FailedCount" not in section assert section.count("ExitCode -ne 0") == 2 - assert section.count("throw ") == 2 + # Scoped past the first icacls call so the volume-root refusal at the top of + # the function, pinned by + # test_application_tree_acl_refuses_to_harden_a_volume_root, does not inflate + # this count. Both native passes still have to fail closed. + icacls_region = section[section.index("& icacls"):] + assert icacls_region.count("throw ") == 2 assert "Write-Log" in section @@ -683,6 +747,54 @@ def test_update_batch_hardens_the_application_tree_the_way_the_installer_does(): assert r"($aclTarget + '\*')" in reset_call, reset_call +def test_application_tree_acl_refuses_to_harden_a_volume_root(): + """Both copies of the hardening routine must refuse a volume root. + + `$Path.TrimEnd('\\')` turns "C:\\" into the drive-relative "C:". The child + reset then expands to "C:\\*" and recursively resets ACLs across the entire + system drive, and Get-Acl/Set-Acl against "C:" land on the process current + directory rather than the drive root. A UNC share root carries the same + recursive blast radius. HumWatch.iss leaves DisableDirPage=no and Inno + honours /DIR="C:\\", so this is reachable from a scripted install. + """ + setup = read("installer/service-setup.ps1") + update = read("update.bat") + + installer_section = setup.split("function Set-ProtectedApplicationAcl", 1)[1].split( + "function Set-ProtectedWritableDirectoryAcl", 1 + )[0] + update_section = update.split("function Set-HardenedAppAcl", 1)[1].split( + "function Stop-HumWatch", 1 + )[0] + + # The vulnerable assignment cannot survive in either copy under any casing + # of the parameter name. + assert "$aclTarget = $Path.TrimEnd" not in installer_section + assert "$aclTarget = $path.TrimEnd" not in update_section + + for section, acl_read in ( + (installer_section, "Get-ServiceAcl"), + (update_section, "Get-Acl -LiteralPath"), + ): + # Forward slashes have to be folded first, or "c:/" walks straight past + # a backslash-only check. + assert ".Replace('/', '\\')" in section, section + # The target the rest of the function uses is derived from the + # normalized path, not from the raw parameter. + assert "$aclTarget = $normalizedPath.TrimEnd('\\')" in section, section + # A bare drive, with or without the trailing separator already trimmed. + assert "'^[A-Za-z]:$'" in section, section + # A UNC target needs a server, a share, and at least one directory + # below the share before it is safe to reset recursively. + assert "$aclTarget.Substring(2).Split('\\')" in section, section + assert "-lt 3" in section, section + + # Nothing may be read or mutated on the way to the refusal. + refusal = section.index("A volume root is not a permitted") + for call in (acl_read, "Set-Acl -LiteralPath", "icacls"): + assert refusal < section.index(call), (call, section) + + def _update_bat_powershell_source(update: str) -> str: """Reconstruct the single logical line powershell.exe actually receives. @@ -766,6 +878,103 @@ def test_update_batch_quoted_powershell_has_no_line_ending_comment(): assert not in_block_comment, "unterminated PowerShell <# #> block comment" +def _update_bat_acl_contract_harness(update: str) -> str: + """Lift update.bat's Set-HardenedAppAcl out of cmd and run it under pwsh. + + The installer copy is exercised through the shared contract harness. This + one cannot be dot-sourced, since it only exists as fragments of a cmd + double-quoted string, so reconstruct the logical PowerShell line, cut the + one function out of it, and drive it against fake ACL primitives. + """ + source = _update_bat_powershell_source(update) + definition = "function Set-HardenedAppAcl" + source.split( + "function Set-HardenedAppAcl", 1 + )[1].split("function Stop-HumWatch", 1)[0] + return rf""" +$ErrorActionPreference = 'Stop' +$script:aclStore = @{{}} +$script:icaclsCalls = @() + +function New-FakeAcl([string]$Path) {{ + $acl = [pscustomobject]@{{ Path = $Path; Access = @(); Rules = @() }} + $acl | Add-Member ScriptMethod SetAccessRuleProtection {{ param($a, $b) }} + $acl | Add-Member ScriptMethod SetOwner {{ param($owner) }} + $acl | Add-Member ScriptMethod RemoveAccessRuleAll {{ param($rule) }} + $acl | Add-Member ScriptMethod AddAccessRule {{ param($rule) $this.Rules += $rule }} + return $acl +}} +function Get-Acl {{ param([string]$LiteralPath, [string]$ErrorAction) return New-FakeAcl $LiteralPath }} +function Set-Acl {{ param([string]$LiteralPath, $AclObject) $script:aclStore[$LiteralPath] = $AclObject }} +function icacls {{ $script:icaclsCalls += ($args -join ' '); $global:LASTEXITCODE = 0 }} + +{definition} + +$volumeRootResults = [ordered]@{{}} +foreach ($candidate in @('C:\', 'C:', 'c:/', 'C:\\', 'D:/', '\\server\share', '\\server\share\')) {{ + $beforeAcl = $script:aclStore.Keys.Count + $beforeIcacls = $script:icaclsCalls.Count + $rejected = $false + $message = '' + try {{ Set-HardenedAppAcl $candidate }} catch {{ $rejected = $true; $message = $_.Exception.Message }} + $volumeRootResults[$candidate] = [ordered]@{{ + rejected = $rejected + message = $message + mutated = (($script:aclStore.Keys.Count -ne $beforeAcl) -or ($script:icaclsCalls.Count -ne $beforeIcacls)) + }} +}} + +$acceptedResults = [ordered]@{{}} +foreach ($candidate in @('C:\HumWatch', 'C:\HumWatch\', 'D:\Program Files\HumWatch', '\\server\share\HumWatch')) {{ + $script:icaclsCalls = @() + $failure = '' + try {{ Set-HardenedAppAcl $candidate }} catch {{ $failure = $_.Exception.Message }} + $acceptedResults[$candidate] = [ordered]@{{ failure = $failure; icacls = @($script:icaclsCalls) }} +}} + +[ordered]@{{ volumeRoots = $volumeRootResults; accepted = $acceptedResults }} | ConvertTo-Json -Compress -Depth 6 +""" + + +def test_update_batch_acl_contract_refuses_a_volume_root_and_accepts_a_normal_path(): + pwsh = shutil.which("pwsh") or shutil.which("powershell") + if not pwsh: + pytest.skip("PowerShell behavioral contract requires pwsh or Windows PowerShell") + + completed = subprocess.run( + [ + pwsh, + "-NoProfile", + "-NonInteractive", + "-Command", + _update_bat_acl_contract_harness(read("update.bat")), + ], + check=True, + capture_output=True, + text=True, + ) + result = json.loads(completed.stdout.strip().splitlines()[-1]) + + for candidate, outcome in result["volumeRoots"].items(): + assert outcome["rejected"] is True, candidate + assert "volume root is not a permitted" in outcome["message"], candidate + assert candidate in outcome["message"], candidate + assert outcome["mutated"] is False, candidate + + expected_targets = { + "C:\\HumWatch": "C:\\HumWatch", + "C:\\HumWatch\\": "C:\\HumWatch", + "D:\\Program Files\\HumWatch": "D:\\Program Files\\HumWatch", + "\\\\server\\share\\HumWatch": "\\\\server\\share\\HumWatch", + } + for candidate, target in expected_targets.items(): + outcome = result["accepted"][candidate] + assert outcome["failure"] == "", candidate + assert outcome["icacls"] == [ + target + "\\* /reset /T /Q", + target + " /setowner *S-1-5-32-544 /T /Q", + ], candidate + + def test_advertised_identity_list_cannot_contain_empty_entries(): setup = read("installer/service-setup.ps1") provision = read("scripts/provision-security.ps1") diff --git a/update.bat b/update.bat index c267e7f..076e972 100644 --- a/update.bat +++ b/update.bat @@ -97,7 +97,17 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ " <# A pre-hardening install still carries inherited or user writable grants. " ^ " Apply the hardened ACL inside the transaction so the first hardened " ^ " release is reachable, then let the verification below confirm it. #> " ^ - " $aclTarget = $path.TrimEnd('\'); " ^ + " <# A volume root is never a valid install location. TrimEnd on 'C:\' " ^ + " yields the drive relative 'C:', which turns the child reset below into " ^ + " 'C:\*' and recursively resets ACLs across the entire system drive. A UNC " ^ + " share root carries the same blast radius. Refuse before anything is read " ^ + " or mutated. #> " ^ + " $normalizedPath = $path.Trim().Replace('/', '\'); " ^ + " if ($normalizedPath.StartsWith('\\?\UNC\', [System.StringComparison]::OrdinalIgnoreCase)) { $normalizedPath = '\\' + $normalizedPath.Substring(8) } elseif ($normalizedPath.StartsWith('\\?\', [System.StringComparison]::OrdinalIgnoreCase)) { $normalizedPath = $normalizedPath.Substring(4) }; " ^ + " $aclTarget = $normalizedPath.TrimEnd('\'); " ^ + " $isVolumeRoot = -not $aclTarget; " ^ + " if ($aclTarget.StartsWith('\\')) { $isVolumeRoot = @($aclTarget.Substring(2).Split('\') | Where-Object { $_ }).Count -lt 3 } elseif ($aclTarget -match '^[A-Za-z]:$') { $isVolumeRoot = $true }; " ^ + " if ($isVolumeRoot) { throw ('A volume root is not a permitted HumWatch install location: ' + $path) }; " ^ " $systemSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-18'); " ^ " $administratorsSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-32-544'); " ^ " $usersSid = [Security.Principal.SecurityIdentifier]::new('S-1-5-32-545'); " ^ From a99119dbb11b6ab2b52e6e20fb41cdd93430d112 Mon Sep 17 00:00:00 2001 From: Static Date: Tue, 4 Aug 2026 21:17:42 -0400 Subject: [PATCH 28/31] fix: silence the Stop preference around the certificate stderr capture Under Windows PowerShell 5.1 the 2>&1 redirection converts native stderr into error records, and the file-level $ErrorActionPreference = "Stop" turns the first one into a terminating error. The assignment died before $LASTEXITCODE was read, so the wide Out-String diagnostic never ran and the trap logged a bare NativeCommandError instead of the captured Python output, defeating the stderr capture this exists for. Bracket the one native call with the same save, silence, capture, restore sequence Invoke-Nssm and the pip bootstrap already use. Co-Authored-By: Claude Fable 5 --- scripts/provision-security.ps1 | 6 +++++ tests/test_windows_service_security.py | 31 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/scripts/provision-security.ps1 b/scripts/provision-security.ps1 index 8f9ccfd..f7877bb 100644 --- a/scripts/provision-security.ps1 +++ b/scripts/provision-security.ps1 @@ -169,8 +169,14 @@ function New-ServerCertificate { } # Capture stdout and stderr instead of discarding them. On failure this # output is the only clue in service-setup.log about what Python did. + # Under Windows PowerShell 5.1 the 2>&1 redirection turns native stderr + # into error records, and the file-level Stop preference would terminate + # this assignment on the first one, so silence it around the call only. + $prevEAP = $ErrorActionPreference + $ErrorActionPreference = "SilentlyContinue" $certificateOutput = & $PythonExecutable @certificateArguments 2>&1 $certificateExitCode = $LASTEXITCODE + $ErrorActionPreference = $prevEAP if ($certificateExitCode -ne 0 -or -not (Test-Path -LiteralPath $CertificatePath -PathType Leaf) -or -not (Test-Path -LiteralPath $KeyPath -PathType Leaf)) { # Out-String defaults to an 80 column width and truncates anything # wider, which previously cut off the argparse "error:" line and left diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 927e2ed..3500d9b 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -1013,6 +1013,37 @@ def test_captured_subprocess_output_is_not_column_truncated(): assert "-Width" in provision +def test_certificate_stderr_capture_survives_strict_error_preference(): + provision = read("scripts/provision-security.ps1") + + # The file runs under $ErrorActionPreference = "Stop". On Windows + # PowerShell 5.1, 2>&1 converts native stderr into error records, and Stop + # turns the first one into a terminating error. The assignment dies before + # $LASTEXITCODE is read, so the wide Out-String diagnostic never runs and + # the trap logs a bare NativeCommandError instead of the captured Python + # output. The invocation must be bracketed by a save, silence, capture, + # restore sequence, in that exact order, matching Invoke-Nssm in + # installer/service-setup.ps1 and the pip bootstrap in + # scripts/build-installer.ps1. Substring presence alone proves nothing + # (SilentlyContinue appears elsewhere), so pin the ordered lines around + # the one native call. + certificate_section = provision.split("function New-ServerCertificate", 1)[1].split( + "\nfunction ", 1 + )[0] + lines = [line.strip() for line in certificate_section.splitlines()] + invocations = [ + index + for index, line in enumerate(lines) + if line.startswith("$certificateOutput = & $PythonExecutable") and "2>&1" in line + ] + assert len(invocations) == 1, invocations + at = invocations[0] + assert lines[at - 2] == "$prevEAP = $ErrorActionPreference", lines[at - 2] + assert lines[at - 1] == '$ErrorActionPreference = "SilentlyContinue"', lines[at - 1] + assert lines[at + 1] == "$certificateExitCode = $LASTEXITCODE", lines[at + 1] + assert lines[at + 2] == "$ErrorActionPreference = $prevEAP", lines[at + 2] + + def test_installer_reports_a_failed_post_install_to_its_caller(): code = _pascal_code_without_comments(read("installer/HumWatch.iss")) From 5d511a1a7965a6f015f16af9f01bd92921a312b5 Mon Sep 17 00:00:00 2001 From: Static Date: Tue, 4 Aug 2026 21:20:47 -0400 Subject: [PATCH 29/31] docs: add the v2 security hardening story to the README Co-Authored-By: Claude Fable 5 --- README.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/README.md b/README.md index 4d21f21..52d184f 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,30 @@ sudo systemctl enable --now humwatch Check status with `systemctl status humwatch` and logs with `journalctl -u humwatch -f`. +## Security Hardening + +HumWatch v2 treats an install as a security boundary, not a file copy. The +highlights, all on by default: + +- **Bearer token auth and TLS everywhere.** Every listener requires HTTPS and + every protected endpoint requires a bearer token. Install-time provisioning + generates the certificate and token rather than leaving them as homework. +- **Locked-down install ACLs on Windows.** The runtime data under + `C:\ProgramData\HumWatch` (token, TLS material, database, logs) is stripped + to SYSTEM and Administrators only. The application tree grants ordinary + users read and execute, nothing more. Both routines resolve their target + first and refuse to harden a volume root, so a scripted `/DIR="C:\"` aborts + instead of rewriting ACLs across the whole drive. +- **Owner-only private keys.** The TLS private key is created with a + restricted mode from the first byte, never at the default umask. +- **A pinned supply chain.** Everything the Windows build downloads comes + from a single asset manifest with pinned URLs and SHA-256 digests, verified + before use. No floating URLs, no unverified fetches. +- **Honest exit codes.** A failed Windows install now exits nonzero instead + of reporting success, so silent and scripted installs can trust the result. +- **Upgrades keep your config.** On Debian, `config.json` is a dpkg conffile, + so package upgrades preserve operator edits instead of overwriting them. + ## Dashboard The dashboard sidebar contains these pages: From 46c3e19f90d4533a73898a61e4b4021fe0374594 Mon Sep 17 00:00:00 2001 From: Static Date: Tue, 4 Aug 2026 21:32:51 -0400 Subject: [PATCH 30/31] fix: stop recursive ACL hardening from following reparse points A pre-hardening tree is writable by ordinary users, so a local user can plant a junction or symlink inside it before the elevated installer, updater, or provisioner runs. The recursive icacls passes lacked /L and would follow the reparse point, resetting ACLs and ownership outside the install directory. Both passes in both copies now carry /L. Set-RestrictedAcl had the same hole twice over: Get-ChildItem -Recurse traverses junctions under PowerShell 5.1, and Get-Acl/Set-Acl resolve against the reparse target rather than the link. The walk is now a manual queue that refuses a reparse point as its root and neither descends into nor touches reparse-point children. Co-Authored-By: Claude Fable 5 --- installer/service-setup.ps1 | 10 ++++-- scripts/provision-security.ps1 | 29 +++++++++++++-- tests/test_security_provisioning.py | 4 ++- tests/test_windows_service_security.py | 49 +++++++++++++++++++++++--- update.bat | 4 +-- 5 files changed, 83 insertions(+), 13 deletions(-) diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index ee25c40..5e60de2 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -179,7 +179,13 @@ function Set-ProtectedApplicationAcl { # parsing that count only worked on English Windows and degraded to a # fail-open exit-code check everywhere else. Without /C, icacls stops at # the first failure and returns nonzero in every locale. - $resetOutput = & icacls "$aclTarget\*" /reset /T /Q 2>&1 + # + # Both recursive passes carry /L. A pre-hardening tree is writable by + # ordinary users, so a local user can plant a junction or symlink inside + # it before this elevated pass runs, and without /L icacls follows the + # reparse point and resets ACLs and ownership outside the install + # directory. With /L the link itself is processed and never traversed. + $resetOutput = & icacls "$aclTarget\*" /reset /T /L /Q 2>&1 $resetExitCode = $LASTEXITCODE Write-Log "icacls /reset for ${aclTarget}: exit code $resetExitCode" if ($resetExitCode -ne 0) { @@ -190,7 +196,7 @@ function Set-ProtectedApplicationAcl { # An owner always carries implicit READ_CONTROL and WRITE_DAC, so a file # still owned by whoever unpacked the tree can re-grant itself write access # regardless of the DACL above. - $ownerOutput = & icacls $aclTarget /setowner "*S-1-5-32-544" /T /Q 2>&1 + $ownerOutput = & icacls $aclTarget /setowner "*S-1-5-32-544" /T /L /Q 2>&1 $ownerExitCode = $LASTEXITCODE Write-Log "icacls /setowner for ${aclTarget}: exit code $ownerExitCode" if ($ownerExitCode -ne 0) { diff --git a/scripts/provision-security.ps1 b/scripts/provision-security.ps1 index f7877bb..1fd060a 100644 --- a/scripts/provision-security.ps1 +++ b/scripts/provision-security.ps1 @@ -39,9 +39,32 @@ function Set-RestrictedAcl { [Parameter(Mandatory)][string]$SystemPermission ) - $targets = @((Get-Item -LiteralPath $Path)) - if ($targets[0].PSIsContainer) { - $targets += Get-ChildItem -LiteralPath $Path -Force -Recurse + # A pre-hardening runtime tree is creatable by ordinary users (ProgramData + # grants Users create rights), so it can contain a planted junction or + # symlink. PowerShell 5.1 both recurses through reparse points and + # resolves Get-Acl and Set-Acl against the reparse TARGET rather than the + # link, so following one here would rewrite ACLs and ownership outside + # this tree. Refuse a reparse point as the root outright, and neither + # descend into nor touch reparse-point children. The hardened parent DACL + # denies ordinary users the delete right needed to swap in new ones later. + $reparsePoint = [IO.FileAttributes]::ReparsePoint + $root = Get-Item -LiteralPath $Path -Force + if ($root.Attributes -band $reparsePoint) { + throw "A reparse point is not a permitted hardening target: $Path" + } + $targets = @($root) + if ($root.PSIsContainer) { + $pending = [Collections.Generic.Queue[string]]::new() + $pending.Enqueue($root.FullName) + while ($pending.Count -gt 0) { + foreach ($child in @(Get-ChildItem -LiteralPath $pending.Dequeue() -Force)) { + if ($child.Attributes -band $reparsePoint) { continue } + $targets += $child + if ($child.PSIsContainer) { + $pending.Enqueue($child.FullName) + } + } + } } # Reset all existing grants, including Everyone, Authenticated Users, and diff --git a/tests/test_security_provisioning.py b/tests/test_security_provisioning.py index 5fe0e8f..189aff2 100644 --- a/tests/test_security_provisioning.py +++ b/tests/test_security_provisioning.py @@ -150,7 +150,9 @@ def test_windows_provisioner_resets_all_inherited_world_write_grants(): for sid in ("S-1-1-0", "S-1-5-11", "S-1-5-32-545"): assert sid in source assert "RemoveAccessRuleAll" in source - assert "Get-ChildItem -LiteralPath $Path -Force -Recurse" in source + # The walk is a manual queue rather than -Recurse, because PowerShell 5.1 + # recurses through reparse points and Set-Acl follows them to their target. + assert "Get-ChildItem -LiteralPath $pending.Dequeue() -Force" in source assert "SetOwner($administratorsSid)" in source diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 3500d9b..15a3c3a 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -231,8 +231,8 @@ def test_windows_service_contract_executes_acl_and_firewall_behavior_without_win outcome = result["accepted"][candidate] assert outcome["failure"] == "", candidate assert outcome["icacls"] == [ - target + "\\* /reset /T /Q", - target + " /setowner *S-1-5-32-544 /T /Q", + target + "\\* /reset /T /L /Q", + target + " /setowner *S-1-5-32-544 /T /L /Q", ], candidate assert result["defaultPort"] == 9123 @@ -601,7 +601,7 @@ def test_application_tree_acl_uses_native_icacls_not_a_per_file_loop(): # CI hang. The application tree must be flattened by icacls and receive a # single inheritable root ACL instead. assert "icacls" in section - assert "/reset /T /Q" in section + assert "/reset /T /L /Q" in section assert "/setowner" in section assert "Get-ChildItem" not in section assert "S-1-5-32-545" in section @@ -970,8 +970,8 @@ def test_update_batch_acl_contract_refuses_a_volume_root_and_accepts_a_normal_pa outcome = result["accepted"][candidate] assert outcome["failure"] == "", candidate assert outcome["icacls"] == [ - target + "\\* /reset /T /Q", - target + " /setowner *S-1-5-32-544 /T /Q", + target + "\\* /reset /T /L /Q", + target + " /setowner *S-1-5-32-544 /T /L /Q", ], candidate @@ -1013,6 +1013,45 @@ def test_captured_subprocess_output_is_not_column_truncated(): assert "-Width" in provision +def test_recursive_acl_hardening_does_not_follow_reparse_points(): + setup = read("installer/service-setup.ps1") + update = read("update.bat") + provision = read("scripts/provision-security.ps1") + + # A pre-hardening install tree is writable by ordinary users, so a local + # user can plant a directory junction or symlink inside it before the + # elevated installer or updater runs. A recursive icacls pass without /L + # follows the reparse point and resets ACLs and ownership on whatever it + # targets, outside the install directory. Every recursive icacls call in + # both copies of the hardening routine must carry /L. + installer_section = setup.split("function Set-ProtectedApplicationAcl", 1)[1].split( + "function Set-ProtectedWritableDirectoryAcl", 1 + )[0] + update_section = update.split("function Set-HardenedAppAcl", 1)[1].split( + "function Stop-HumWatch", 1 + )[0] + for section in (installer_section, update_section): + icacls_calls = [line.strip() for line in section.splitlines() if "& icacls" in line] + assert len(icacls_calls) == 2, section + for call in icacls_calls: + assert "/T" in call, call + assert "/L" in call.split("2>&1")[0], call + + # Set-RestrictedAcl walks the runtime secrets tree with Get-ChildItem, and + # PowerShell 5.1 both recurses through junctions and resolves Get-Acl and + # Set-Acl against the reparse TARGET rather than the link. A blanket + # -Recurse is therefore never safe here. The walk must refuse a reparse + # point as its root outright and must neither descend into nor touch + # reparse-point children. + acl_function = provision.split("function Set-RestrictedAcl", 1)[1].split( + "\nfunction ", 1 + )[0] + assert "-Recurse" not in acl_function + assert "$reparsePoint = [IO.FileAttributes]::ReparsePoint" in acl_function + assert "if ($root.Attributes -band $reparsePoint) {" in acl_function + assert "if ($child.Attributes -band $reparsePoint) { continue }" in acl_function + + def test_certificate_stderr_capture_survives_strict_error_preference(): provision = read("scripts/provision-security.ps1") diff --git a/update.bat b/update.bat index 076e972..6022a91 100644 --- a/update.bat +++ b/update.bat @@ -122,9 +122,9 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ " $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($administratorsSid, [Security.AccessControl.FileSystemRights]::FullControl, $inherit, $noPropagation, $allow)); " ^ " $acl.AddAccessRule([Security.AccessControl.FileSystemAccessRule]::new($usersSid, [Security.AccessControl.FileSystemRights]::ReadAndExecute, $inherit, $noPropagation, $allow)); " ^ " Set-Acl -LiteralPath $aclTarget -AclObject $acl; " ^ - " $resetOutput = & icacls ($aclTarget + '\*') /reset /T /Q 2>&1; " ^ + " $resetOutput = & icacls ($aclTarget + '\*') /reset /T /L /Q 2>&1; " ^ " if ($LASTEXITCODE -ne 0) { throw ('The application tree could not be reset to inherited permissions: ' + (($resetOutput | Out-String -Width 4096).Trim())) }; " ^ - " $ownerOutput = & icacls $aclTarget /setowner '*S-1-5-32-544' /T /Q 2>&1; " ^ + " $ownerOutput = & icacls $aclTarget /setowner '*S-1-5-32-544' /T /L /Q 2>&1; " ^ " if ($LASTEXITCODE -ne 0) { throw ('The application tree owner could not be set to Administrators: ' + (($ownerOutput | Out-String -Width 4096).Trim())) } " ^ "}; " ^ "function Stop-HumWatch { " ^ From a5373a4bf9436a9ba4c01a12e6b95f7b8ccdee32 Mon Sep 17 00:00:00 2001 From: Static Date: Tue, 4 Aug 2026 21:41:03 -0400 Subject: [PATCH 31/31] fix: fail provisioning on a planted reparse point instead of skipping it A skipped link stays in the runtime tree, and the later already-provisioned branches would then let the service read its TLS material or token through a link a local user planted before the upgrade. Refusing the whole pass fails closed and surfaces the planted entry in the log. Co-Authored-By: Claude Fable 5 --- scripts/provision-security.ps1 | 12 ++++++++---- tests/test_windows_service_security.py | 12 +++++++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/scripts/provision-security.ps1 b/scripts/provision-security.ps1 index 1fd060a..85a1629 100644 --- a/scripts/provision-security.ps1 +++ b/scripts/provision-security.ps1 @@ -44,9 +44,11 @@ function Set-RestrictedAcl { # symlink. PowerShell 5.1 both recurses through reparse points and # resolves Get-Acl and Set-Acl against the reparse TARGET rather than the # link, so following one here would rewrite ACLs and ownership outside - # this tree. Refuse a reparse point as the root outright, and neither - # descend into nor touch reparse-point children. The hardened parent DACL - # denies ordinary users the delete right needed to swap in new ones later. + # this tree. Refuse a reparse point as the root outright, and fail the + # pass on a reparse-point child rather than skip it: a skipped link stays + # in place, and the later already-provisioned branches would then let the + # service read its TLS material or token through a link a local user + # planted before the upgrade. $reparsePoint = [IO.FileAttributes]::ReparsePoint $root = Get-Item -LiteralPath $Path -Force if ($root.Attributes -band $reparsePoint) { @@ -58,7 +60,9 @@ function Set-RestrictedAcl { $pending.Enqueue($root.FullName) while ($pending.Count -gt 0) { foreach ($child in @(Get-ChildItem -LiteralPath $pending.Dequeue() -Force)) { - if ($child.Attributes -band $reparsePoint) { continue } + if ($child.Attributes -band $reparsePoint) { + throw "The runtime tree contains a reparse point and cannot be hardened: $($child.FullName)" + } $targets += $child if ($child.PSIsContainer) { $pending.Enqueue($child.FullName) diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 15a3c3a..d923fca 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -1041,15 +1041,21 @@ def test_recursive_acl_hardening_does_not_follow_reparse_points(): # PowerShell 5.1 both recurses through junctions and resolves Get-Acl and # Set-Acl against the reparse TARGET rather than the link. A blanket # -Recurse is therefore never safe here. The walk must refuse a reparse - # point as its root outright and must neither descend into nor touch - # reparse-point children. + # point as its root outright, and a reparse-point child must fail the + # provisioning pass rather than be skipped: a skipped link stays in + # place, and the later already-provisioned branches would then let the + # service read its TLS material or token through a link a local user + # planted before the upgrade. acl_function = provision.split("function Set-RestrictedAcl", 1)[1].split( "\nfunction ", 1 )[0] assert "-Recurse" not in acl_function assert "$reparsePoint = [IO.FileAttributes]::ReparsePoint" in acl_function assert "if ($root.Attributes -band $reparsePoint) {" in acl_function - assert "if ($child.Attributes -band $reparsePoint) { continue }" in acl_function + assert "{ continue }" not in acl_function + child_guard = acl_function.split("if ($child.Attributes -band $reparsePoint) {", 1) + assert len(child_guard) == 2, acl_function + assert child_guard[1].lstrip().startswith("throw"), child_guard[1][:120] def test_certificate_stderr_capture_survives_strict_error_preference():