fix(install.sh): stage payload as .ps1 file + ssh-keygen -A for hostkey ACLs (continuum's catches)

[joelteply]

Builds on #195. Two Windows install bugs caught by continuum-b69f testing on real Windows MINGW64 (issue #196):

1. Inline payload mangled by 4-layer quote escaping → silently breaks Start-Transcript. Fix: stage as .ps1 file, run via Start-Process -File.
2. sshd Start-Service fails with WIN32_EXIT_CODE 1067 on every fresh Windows install because OpenSSH host-key files have overly-permissive ACLs. Fix: ssh-keygen -A between capability install and Start-Service (idempotent, generates missing keys + restores correct ACLs).
3. Bash side re-queries sshd state post-elevation as belt-and-suspenders; surfaces 'partial install' warning when elevated exit=0 but service isn't actually Running.

continuum-b69f verified the .ps1 file approach gets the transcript every time on his real Windows. ssh-keygen -A is the documented standard fix for the OpenSSH-on-Windows post-install ACL bug.

[Copilot]

Pull request overview

Improves the Windows Git Bash install path in install.sh by making the UAC-elevated OpenSSH setup more reliable (avoids PowerShell quoting/escaping issues, fixes host key ACL problems that prevent sshd from starting, and adds a post-elevation service-state verification).

Changes:

• Stage the elevated PowerShell payload as a .ps1 file (with UTF-8 BOM + preflight parse-check) and run it via Start-Process ... -File.
• Add OpenSSH host-key generation + ACL reset (ssh-keygen -A + icacls) to prevent sshd start failures (1067) on fresh installs.
• Re-query sshd status after elevation to detect “partial install” even when the elevated launcher returns success.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Start-Process -Wait does not propagate the child process exit code back to the current powershell.exe process. As written, _elev_rc will usually stay 0 even if the elevated powershell -File fails, so bash can incorrectly treat the payload as successful. Capture the exit code explicitly (e.g., use Start-Process ... -PassThru, then Wait-Process and exit $p.ExitCode, or avoid Start-Process and invoke elevation in a way that returns the child exit code).

Suggested change

	# Run the elevated payload via -File (no quoting hell). Start-
	# Process -Wait propagates the elevated process's exit code.
	# -ExecutionPolicy Bypass so the elevated PS doesn't refuse
	# the unsigned .ps1.
	powershell.exe -NoProfile -Command "Start-Process powershell -Verb RunAs -Wait -ArgumentList @('-NoProfile','-ExecutionPolicy','Bypass','-File','$_elevated_ps1_win')" 2>&1 \
	# Run the elevated payload via -File (no quoting hell). Capture
	# the Process object, wait for it, then explicitly exit this
	# non-elevated PowerShell with the elevated process's exit code.
	# -ExecutionPolicy Bypass so the elevated PS doesn't refuse
	# the unsigned .ps1.
	powershell.exe -NoProfile -Command "\$p = Start-Process powershell -Verb RunAs -PassThru -ArgumentList @('-NoProfile','-ExecutionPolicy','Bypass','-File','$_elevated_ps1_win'); Wait-Process -InputObject \$p; exit \$p.ExitCode" 2>&1 \

[Copilot]

$_elevated_ps1_win is embedded inside a single-quoted PowerShell string in the ParseFile('...') call. If the Windows path contains a ' (apostrophe) (e.g., a username like O'Connor), the parse-check command will break and may produce misleading errors. Prefer passing the path as an argument (or via an env var) and using -LiteralPath, or escape single quotes before embedding.

Suggested change

	_parse_errs=$(powershell.exe -NoProfile -Command "
	\$tokens = \$null; \$errors = \$null;
	[System.Management.Automation.Language.Parser]::ParseFile('$_elevated_ps1_win', [ref]\$tokens, [ref]\$errors) | Out-Null;
	_parse_errs=$(AIRC_ELEVATED_PS1_WIN="$_elevated_ps1_win" powershell.exe -NoProfile -Command "
	\$tokens = \$null; \$errors = \$null;
	[System.Management.Automation.Language.Parser]::ParseFile(\$env:AIRC_ELEVATED_PS1_WIN, [ref]\$tokens, [ref]\$errors) | Out-Null;

[Copilot]

The staged script path is passed to PowerShell as a single-quoted string element ('$_elevated_ps1_win') inside -ArgumentList. This will break if the Windows path contains a ' character. Consider passing the path via $env:... / -ArgumentList @(..., $path) where $path is built inside PowerShell, or escape quotes when interpolating from bash.

Suggested change

	# the unsigned .ps1.
	powershell.exe -NoProfile -Command "Start-Process powershell -Verb RunAs -Wait -ArgumentList @('-NoProfile','-ExecutionPolicy','Bypass','-File','$_elevated_ps1_win')" 2>&1 \
	# the unsigned .ps1. Pass the path via env so PowerShell
	# doesn't have to parse an interpolated single-quoted literal.
	AIRC_ELEVATED_PS1_WIN="$_elevated_ps1_win" \
	powershell.exe -NoProfile -Command '$p=$env:AIRC_ELEVATED_PS1_WIN; Start-Process powershell -Verb RunAs -Wait -ArgumentList @("-NoProfile","-ExecutionPolicy","Bypass","-File",$p)' 2>&1 \

[Copilot]

The comment says the elevated log lives at $env:TEMP\airc-install-elevated.log, but the code now consistently uses [System.IO.Path]::GetTempPath() to avoid the Git Bash TEMP=/tmp mismatch. Please update this comment to reflect the actual mechanism/path so it doesn't reintroduce confusion during debugging.

Suggested change

	# blinks for a half second so i have no idea"). Log lives at
	# $env:TEMP\airc-install-elevated.log; bash side surfaces it
	# below regardless of success/failure.
	# blinks for a half second so i have no idea"). Log is written to
	# [System.IO.Path]::GetTempPath() + "airc-install-elevated.log";
	# bash side surfaces it below regardless of success/failure.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

install.sh:454

• Start-Process -Wait does not propagate the launched process’ exit code by default; powershell.exe typically exits 0 as long as Start-Process succeeded. As written, _elev_rc won’t reflect failures inside the elevated script. If you need the elevated script’s exit code, use -PassThru and then exit $proc.ExitCode (or equivalent) and update the comment accordingly.

          ok "sshd running (Windows OpenSSH.Server)"
          return 0
          ;;
        Stopped|StopPending|StartPending|Paused|"")
          info "Configuring OpenSSH.Server + HNS port-22 reservation (UAC prompt incoming)."

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The parse preflight embeds $_elevated_ps1_win inside a PowerShell single-quoted string. If the Windows path contains an apostrophe (e.g. in the username), the PowerShell snippet will fail to parse and the install will abort. Pass the path via -ArgumentList/$args[0], or escape ' for PowerShell string literals before interpolating it.

[Copilot]

This ACL fix grants SYSTEM+Administrators and removes the creator ACE, but it never sets the file owner. If OpenSSH’s permission check enforces owner=SYSTEM/Administrators, sshd may still fail. Consider adding an explicit owner fix (e.g. icacls ... /setowner) as part of the remediation.

[Copilot]

The staged script avoids global error handling and bases success/failure on a post-check outside the script. With this approach, failures in earlier required steps (capability/HNS/firewall/DefaultShell) may be silent from the caller’s perspective if sshd happens to end up Running. Consider emitting a stable failure marker and/or a non-zero exit when a required step fails, while still keeping the transcript for diagnostics.

Suggested change

	# No global try/catch, no $ErrorActionPreference = "Stop". Each step
	# runs plainly; if a cmdlet errors, PowerShell prints the error to the
	# transcript and execution continues. Bash side detects success/failure
	# from Get-Service sshd post-check, not from this script's exit code.
	$failureMarker = "AIRC_ELEVATED_INSTALL_FAILED";
	$ErrorActionPreference = "Stop";
	trap {
	Write-Host $failureMarker;
	Write-Error $_;
	try { Stop-Transcript | Out-Null } catch {}
	exit 1;
	}
	# Required setup steps must fail fast so the caller does not treat the
	# install as successful based only on a later sshd status check. Keep
	# the transcript for diagnostics and emit a stable marker on failure.

[Copilot]

Deleting any existing C:\ProgramData\ssh\ssh_host_* files will rotate host key fingerprints on re-runs (e.g. if sshd is merely stopped), causing client trust/MITM warnings. Prefer leaving existing host keys in place; run ssh-keygen -A to generate missing keys and repair ACLs/ownership in-place, and only regenerate keys as a last resort when sshd cannot start due to permission issues.

[Copilot]

The comment here says the payload runs in a SYSTEM context, but Start-Process ... -Verb RunAs elevates to the current user’s admin token (not SYSTEM). This matters for host-key ownership/ACL expectations. Please adjust the comment to reflect the actual execution context (or change the launch mechanism if SYSTEM is truly required).

Suggested change

	# from this elevated SYSTEM-context process. ssh-keygen -A sets the
	# right ACLs at creation time (owner = SYSTEM, ACEs = SYSTEM + Admins).
	# from this elevated administrative process. That recreates the keys with
	# fresh default ACLs at creation time, but this path should not assume
	# SYSTEM ownership unless the installer is actually launched as SYSTEM.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The directory ACL “fix” uses icacls ... /inheritance:r plus /grant, which only adds ACEs and can leave pre-existing explicit permissions in place. That undermines the goal of ensuring C:\ProgramData\ssh and ...\logs are not writable by regular users, and may also fail OpenSSH’s strict-perms checks if extra ACEs remain. Consider using icacls in a way that replaces the DACL (e.g., /grant:r plus explicit /remove of broad principals, or icacls ... /reset followed by a known-good set) so the resulting ACL matches the documented required set.

Suggested change

	icacls $d /inheritance:r 2>&1 | Out-Null
	icacls $d /grant 'NT AUTHORITY\SYSTEM:(OI)(CI)(F)' 'BUILTIN\Administrators:(OI)(CI)(F)' 'NT AUTHORITY\Authenticated Users:(OI)(CI)(RX)' 2>&1 | Out-Null
	icacls $d /reset 2>&1 | Out-Null
	icacls $d /inheritance:r 2>&1 | Out-Null
	icacls $d /grant:r 'NT AUTHORITY\SYSTEM:(OI)(CI)(F)' 'BUILTIN\Administrators:(OI)(CI)(F)' 'NT AUTHORITY\Authenticated Users:(OI)(CI)(RX)' 2>&1 | Out-Null

[Copilot]

$regChanged is set but never used, and the script restarts the HNS service whenever it is Running. Restarting HNS can disrupt WSL2/Docker/Hyper-V networking; it should ideally be done only when it’s actually needed (e.g., when EnableExcludedPortRange was changed or when the port-22 excluded range was newly added). Consider using $regChanged (and a similar flag for the netsh add excludedportrange call) to conditionally restart HNS, or remove the restart if no state changed.

[Copilot]

The comment and logic here assume Start-Process -Wait will propagate the elevated PowerShell process’s exit code into the non-elevated powershell.exe exit status, but Start-Process does not do that by default. As written, _elev_rc is effectively only detecting “Start-Process/UAC failed”, not “payload failed”. If you want a reliable payload exit code, capture it via Start-Process -PassThru -Wait and exit with the returned process’s ExitCode; otherwise, adjust the comment/logic to reflect that the post-check is the authoritative success signal.

[Copilot]

The staged PowerShell payload now intentionally avoids a global try/catch and always ends with exit 0, which means the script no longer emits a stable failure marker (the prior airc-elevated-error: string) and cannot communicate step failures via exit code. Since the bash side still tries to infer “payload failed” from the transcript contents / exit status, consider reintroducing a consistent, parseable failure signal (or updating the bash-side detection logic accordingly) so failures don’t get silently treated as success when sshd happens to be Running.

Suggested change

	# No global try/catch, no $ErrorActionPreference = "Stop". Each step
	# runs plainly; if a cmdlet errors, PowerShell prints the error to the
	# transcript and execution continues. Bash side detects success/failure
	# from Get-Service sshd post-check, not from this script's exit code.
	# Anything wrapped in try/catch below is wrapped because the failure is
	# *expected* and *recoverable* (e.g. ssh-keygen missing -> warn + skip).
	$ErrorActionPreference = "Stop"
	trap {
	$msg = $_.Exception.Message
	if (-not $msg) { $msg = $_.ToString() }
	Write-Host ("airc-elevated-error: " + $msg)
	try { Stop-Transcript | Out-Null } catch {}
	exit 1
	}
	# Unexpected failures must be visible to the bash caller via both a
	# stable transcript marker and a non-zero exit status. Steps that are
	# intentionally recoverable should continue to use local try/catch.

[Copilot]

The elevated PowerShell payload deletes any existing C:\ProgramData\ssh\ssh_host_* files before running ssh-keygen -A. If a user re-runs install on a machine where OpenSSH was already in use (e.g., sshd installed but currently Stopped), this will rotate host keys and break existing client trust (host key mismatch) / potentially disrupt automation. Consider gating deletion/regeneration to the “fresh install” path only (e.g., only when $cap.State was not Installed before Add-WindowsCapability, or only when keys are missing), and otherwise just repair permissions / generate missing keys without rotating existing ones.

Suggested change

	# Since this is install-time setup and the host hasn't published any
	# fingerprint yet, regenerating is safe -- nobody is trusting these
	# keys yet from a client.
	$sshKeygen = Join-Path $env:WINDIR "System32\OpenSSH\ssh-keygen.exe";
	if (-not (Test-Path $sshKeygen)) {
	Write-Host " WARN: ssh-keygen.exe not found at $sshKeygen -- sshd will fail to start"
	} else {
	$sshDir = 'C:\ProgramData\ssh';
	if (-not (Test-Path $sshDir)) { New-Item -Path $sshDir -ItemType Directory -Force | Out-Null }
	$existing = Get-ChildItem (Join-Path $sshDir 'ssh_host_*') -ErrorAction SilentlyContinue
	if ($existing) {
	Write-Host " removing $($existing.Count) existing host key file(s)"
	$existing | Remove-Item -Force -ErrorAction SilentlyContinue
	# Preserve any existing host keys so a reinstall does not rotate the
	# server identity and break existing client trust. ssh-keygen -A only
	# generates missing host keys, so it is safe to run without deleting.
	$sshKeygen = Join-Path $env:WINDIR "System32\OpenSSH\ssh-keygen.exe";
	if (-not (Test-Path $sshKeygen)) {
	Write-Host " WARN: ssh-keygen.exe not found at $sshKeygen -- sshd will fail to start"
	} else {
	$sshDir = 'C:\ProgramData\ssh';
	if (-not (Test-Path $sshDir)) { New-Item -Path $sshDir -ItemType Directory -Force | Out-Null }
	$existingKeys = Get-ChildItem (Join-Path $sshDir 'ssh_host_*_key') -ErrorAction SilentlyContinue
	if ($existingKeys) {
	Write-Host " preserving $($existingKeys.Count) existing host key file(s)"
	} else {
	Write-Host " no existing host keys found; generating host keys"