Draft
Conversation
- mklink via cmd.exe → New-Item -ItemType SymbolicLink - netsh advfirewall → New-NetFirewallRule / Remove-NetFirewallRule - sc create → New-Service - sc start → Start-Service - sc delete → direct & sc.exe (no PS builtin on 5.1) - sc failure/failureflag → direct & sc.exe (no PS builtin) Start-Process didn't propagate exit codes, so try/catch blocks around it were decorative. Native cmdlets throw on failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces hardcoded C:\opt\viam with a configurable parameter (default unchanged). The root path is: - Forwarded through self-elevation - Used to derive cache/bin paths - Passed as --viam-dir in the service BinaryPathName This enables installing to non-C: drives (e.g. D:\viam for UWF-protected LTSC devices). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The function logged "Ignoring netsh error on non-SYSTEM windows" but still returned the error, causing DownloadBinary to fail entirely even though the file was already downloaded. Changes: - On non-elevated processes, warn and return nil (firewall rule is best-effort when running as a non-admin service account) - On elevated processes, return the error (unexpected failure) - Replace fragile user.Name=="SYSTEM" string check with isElevated() using Windows token elevation API, which is locale-independent and covers any admin account - Add isElevated() helpers in platform-specific files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When -UserAccount is specified, the installer: - Validates the local user exists - Prompts for password via Get-Credential (no plaintext on CLI) - Grants the account full control of the viam directory (ACL) - Creates the service with -Credential (auto-grants SeServiceLogonRight) - Grants service self-management SDDL rights (query/start/stop) - Pre-registers the event log source When omitted, behavior is unchanged (runs as LocalSystem). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When set, enables Windows security audit policies via auditpol: - Object Access failures (ACL denials on files/registry/services) - Privilege Use failures (firewall, service control) - Process Creation (success + failure, to trace agent child processes) Events appear in Event Viewer > Security log. Defaults to off. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mirrors uninstall.sh for Windows. Removes: - viam-agent service (stop + delete) - Firewall rule - Event log source - Viam directory tree Takes the same -RootPath and -UserAccount args as the installer for consistency. Does NOT delete user accounts (prints a reminder). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two compatibility issues: 1. Invoke-WebRequest without -UseBasicParsing depends on IE's DOM parser. Fails on LTSC/Server Core (no IE), and after the Dec 2025 CVE-2025-54100 security update it shows an interactive confirmation prompt that breaks -Silent mode. 2. Older Win10 builds may negotiate TLS 1.0/1.1 by default, which GCS (and most CDNs) no longer accepts. Force TLS 1.2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On LTSC devices with UWF protecting C:, registry changes (service registration, firewall rules, event log source, etc.) are lost on reboot. The -UwfCommit flag commits known registry paths via uwfmgr.exe after installation completes. The commit list is a best-known set pending verification with procmon on an actual LTSC device. A TODO comment notes that the list must be rechecked whenever the installer changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review of the full diff against main for Win10/LTSC compatibility: Verified compatible: - New-Service -Credential: available in PS 5.1 (confirmed) - Invoke-WebRequest -UseBasicParsing: PS 3.0+ - New-Item -ItemType SymbolicLink: PS 5.0+ (all Win10) - New-NetFirewallRule: Win8+ (NetSecurity module) - Get-LocalUser: Win10 1607+ (LTSC 2016+, LTSC 2015 is EOL) - TLS 1.2 ServicePointManager: .NET 4.5+ (all Win10) - isElevated() Go API: GetTokenInformation available since Vista Fixed: - New-Service -Credential does NOT auto-grant SeServiceLogonRight (only DSC's Service resource does). Added explicit grant via secedit so the service doesn't fail with error 1069 on start. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract logical blocks into named functions: - Assert-AdminPrivileges, Enable-AuditLogging - Remove-ExistingInstallation, Install-AgentBinary - Set-FirewallRule, Set-UserAccountPermissions - Grant-ServiceLogonRight, Grant-ServiceSelfManagement - Install-AgentService, Invoke-UwfCommit Main flow at the bottom is now ~20 lines and reads as a summary. Stays as a single file for ps2exe compilation. Each function is independently testable with Pester. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Uses PSScriptAnalyzer via pwsh (PowerShell Core), which runs on mac/linux/windows. Reports warnings and errors, exits non-zero on findings. Lints both installer and uninstaller scripts. Requires: pwsh (install via brew/apt/snap) Usage: make lint-ps Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The agent downloads binaries at runtime (viam-server, subsystems) and creates firewall exceptions for each via netsh. netsh goes through the BFE (Base Filtering Engine) service, which is admin-only by default. Changes: - Add Grant-FirewallManagement to installer: grants the service account CCLCRPWPRC on the BFE service DACL so netsh works at runtime without elevation - Add BFE service key to UWF commit list - Update allowFirewall comment to reflect BFE access pattern - Also picks up user's Makefile comment for lint-ps target Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The existing installer on main works on our oldest Win10 target without this. TLS 1.2 is the default on all Win10 builds we ship to. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both scripts pass lint with two rules suppressed: - PSAvoidUsingWriteHost: intentional for interactive installer output - PSUseBOMForUnicodeEncodedFile: UTF-8 without BOM is standard Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Grant-FirewallManagement called Write-Status (from reverted refactor) which doesn't exist. Replaced with Write-Host guard. - secedit username check matched anywhere in the policy file. Scoped to SeServiceLogonRight line only. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When -UserAccount is specified, the installer grants the account firewall management rights on the BFE service. The uninstaller now removes that ACE by matching the exact SID in the SDDL string. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The "get SDDL, append/remove ACE, handle missing SACL" pattern appeared 3 times with the same structure. Extracted into: - Add-ServiceDaclAce: used for BFE and viam-agent service DACLs - Remove-ServiceDaclAce: used in uninstaller for BFE revert Both helpers add null checks on sc.exe sdshow output, LASTEXITCODE checks on sdset, and skip if the ACE is already present/absent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
abe-winter
force-pushed
the
aw/win-non-system
branch
from
5cfb5a3 to
6b1e449
Compare
PowerShell 5.1 on Windows reads UTF-8-without-BOM files using the system default encoding (Windows-1252). The multi-byte em dash characters caused the parser to miscount bytes, producing cascading "missing closing '}'" syntax errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- lint-ps now fails fast if .ps1 files contain non-ASCII chars, which cause parse errors on PS 5.1 without BOM - Suppress Invoke-WebRequest progress bar during download -- PS 5.1 redraws it on every chunk, making downloads very slow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of failing when -UserAccount doesn't exist, the installer now creates it with New-LocalUser. Password prompt is moved before the existence check so the same credential is used for both account creation and service configuration. Idempotent on re-run. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Get-Credential shows a GUI dialog that: - Displays .\username in an editable field (users remove the .\ prefix, then New-Service rejects it) - Breaks over SSH/remote sessions (no GUI) - Blocks -Silent mode Replaced with Read-Host -AsSecureString + manual PSCredential construction. The .\ prefix for local accounts is now hardcoded and not user-editable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use Windows Virtual Service Account (NT SERVICE\viam-agent) instead of a named local user account. This eliminates: - Password prompts (no password to manage) - User creation (Windows auto-creates the identity) - SeServiceLogonRight grant (implicit for virtual accounts) - ps2exe/SSH compatibility issues with Get-Credential/Read-Host The -ServiceAccount switch configures the service to run as NT SERVICE\viam-agent after creation, then grants it: - Full control of the viam directory (ACL) - Firewall management rights (BFE service DACL) - Service self-management rights (viam-agent service DACL) - Event log source registration Uninstaller updated to match: -ServiceAccount reverts BFE DACL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When provided, -Url replaces the default GCS download URL entirely. The cache filename is derived from the URL tail, so custom builds get distinct cache entries. Forwarded through self-elevation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
"Object Access" and "Privilege Use" are category names, not subcategory names. auditpol /subcategory rejects them with error 0x0057 (invalid parameter). Changed to /category. "Process Creation" is a valid subcategory (under Detailed Tracking), so it stays as /subcategory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without this, the agent defaults to C:\etc\viam.json regardless of -RootPath, which breaks on UWF-protected C: drives and may not be readable by the virtual service account. Usage: -ConfigPath "D:\viam\etc\viam.json" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The virtual service account needs read access to the directory containing viam.json. When -ConfigPath is set, grants access to its parent directory. Otherwise grants access to the default C:\etc. Warns if the directory doesn't exist yet. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
C:\etc doesn't exist by default on Windows. Create it (or the custom -ConfigPath parent) before setting the ACL rather than warning and skipping. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The agent creates symlinks at runtime when downloading new versions of itself and viam-server (utils.ForceSymlink). Without this privilege, the virtual service account gets "A required privilege is not held by the client" when symlinking in the cache/bin dirs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
secedit exports user rights as *SID entries (e.g. *S-1-5-32-544), not account names. The previous code appended the account name literally, which secedit silently ignored. Now resolves NT SERVICE\viam-agent to its SID and uses *SID format. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two bugs in the secedit manipulation: 1. .* captured \r from CRLF line endings, appending the SID after the carriage return and garbling the line. Fixed with [^\r\n]* to match only the line content. 2. Set-Content defaulted to system encoding, but secedit exports UTF-16 LE. The encoding mismatch caused secedit /configure to reject the file (exit code 1). Fixed with -Encoding Unicode on both Get-Content and Set-Content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The try/finally was silently swallowing errors. Added: - catch block that warns on failure - LASTEXITCODE check after secedit /configure - Success/already-granted status messages This will surface the root cause if the privilege still isn't being granted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three issues prevented SeCreateSymbolicLinkPrivilege from being
granted via secedit:
1. secedit exports ANSI on Win11 (not UTF-16 as older docs say).
Use [Text.Encoding]::Default and .NET File APIs to preserve
exact encoding on read/write.
2. [IO.File]::WriteAllText without explicit encoding adds a UTF-8
BOM that secedit rejects ("incorrect format").
3. GetTempFileName creates a 0-byte .sdb file that secedit tries
to open as an existing database. Delete it before /configure.
Also added error reporting: warns on secedit failure, logs
success with SID, and reports "already granted" when idempotent.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
allowFirewall()(need rework before review)make lint-pstarget (doesn't work super well)Why
Support more windows targets