security: address remaining vulnerabilities from security review

install-core.ps1:
- Add Test-SecureDirectory and New-SecureDirectory to mitigate TOCTOU races
- Add Test-GitTagSignature for GPG verification of git tags
- Make ACL failures fatal for temp directories with retry logic
- Use precise PATH matching instead of substring contains
- Add unique GUID suffix to temp file names
- Document security controls and known limitations in header

setup.bat:
- Validate LOCALAPPDATA is within USERPROFILE
- Reject paths with invalid shell characters
- Add symlink detection for install directory
- Use delayed expansion variables for path safety

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
unknown 2026-02-01 00:29:32 +11:00
parent c27158066e
commit 991bb45d44
No known key found for this signature in database
GPG key ID: FE478DD75EE21194
2 changed files with 224 additions and 65 deletions

View file

@ -1,5 +1,17 @@
# Install the Core CLI (Windows)
# Run: .\scripts\install-core.ps1
#
# SECURITY CONTROLS:
# - Version pinning prevents supply chain attacks via tag manipulation
# - SHA256 hash verification ensures binary integrity
# - Path validation prevents LOCALAPPDATA redirection attacks
# - Symlink detection prevents directory traversal attacks
# - Restrictive ACLs on temp directories prevent local privilege escalation
# - GPG signature verification (when available) ensures code authenticity
#
# KNOWN LIMITATIONS:
# - Checksums are fetched from same origin as binaries (consider separate trust root)
# - No TLS certificate pinning (relies on system CA store)
$ErrorActionPreference = "Stop"
@ -49,6 +61,24 @@ function Get-SecureInstallDir {
$InstallDir = Get-SecureInstallDir
# Verify directory is safe for writing (not a symlink, correct permissions)
function Test-SecureDirectory {
param([string]$Path)
if (-not (Test-Path $Path)) {
return $true # Directory doesn't exist yet, will be created
}
$dirInfo = Get-Item $Path -Force
# Check for symlinks/junctions
if ($dirInfo.Attributes -band [System.IO.FileAttributes]::ReparsePoint) {
Write-Err "Directory '$Path' is a symbolic link or junction. Possible symlink attack detected."
}
return $true
}
# Verify SHA256 hash of downloaded file
function Test-FileHash {
param(
@ -61,7 +91,78 @@ function Test-FileHash {
Remove-Item -Path $FilePath -Force -ErrorAction SilentlyContinue
Write-Err "Hash verification failed! Expected: $ExpectedHash, Got: $actualHash. The downloaded file may be corrupted or tampered with."
}
Write-Info "Hash verification passed"
Write-Info "Hash verification passed (SHA256: $($actualHash.Substring(0,16))...)"
}
# Create directory with atomic check-and-create to minimize TOCTOU window
function New-SecureDirectory {
param([string]$Path)
# Check parent directory for symlinks first
$parent = Split-Path $Path -Parent
if ($parent -and (Test-Path $parent)) {
Test-SecureDirectory -Path $parent
}
# Create directory
if (-not (Test-Path $Path)) {
New-Item -ItemType Directory -Force -Path $Path | Out-Null
}
# Immediately verify it's not a symlink (minimize TOCTOU window)
Test-SecureDirectory -Path $Path
return $Path
}
# Set restrictive ACL on directory (owner-only access)
# This is REQUIRED for temp directories - failure is fatal
function Set-SecureDirectoryAcl {
param(
[string]$Path,
[switch]$Required
)
$maxRetries = 3
$retryCount = 0
while ($retryCount -lt $maxRetries) {
try {
$acl = Get-Acl $Path
# Disable inheritance and remove inherited rules
$acl.SetAccessRuleProtection($true, $false)
# Clear existing rules
$acl.Access | ForEach-Object { $acl.RemoveAccessRule($_) } | Out-Null
# Add full control for current user only
$currentUser = [System.Security.Principal.WindowsIdentity]::GetCurrent().Name
$rule = New-Object System.Security.AccessControl.FileSystemAccessRule(
$currentUser,
[System.Security.AccessControl.FileSystemRights]::FullControl,
[System.Security.AccessControl.InheritanceFlags]::ContainerInherit -bor [System.Security.AccessControl.InheritanceFlags]::ObjectInherit,
[System.Security.AccessControl.PropagationFlags]::None,
[System.Security.AccessControl.AccessControlType]::Allow
)
$acl.AddAccessRule($rule)
Set-Acl -Path $Path -AclObject $acl
Write-Info "Set restrictive permissions on $Path"
return $true
} catch {
$retryCount++
if ($retryCount -ge $maxRetries) {
if ($Required) {
Write-Err "SECURITY: Failed to set restrictive ACL on '$Path' after $maxRetries attempts: $($_.Exception.Message)"
} else {
Write-Warn "Could not set restrictive ACL on '$Path': $($_.Exception.Message)"
return $false
}
}
Start-Sleep -Milliseconds 100
}
}
}
# Download pre-built binary with integrity verification
@ -74,17 +175,13 @@ function Download-Binary {
Write-Info "URL: $binaryUrl"
try {
New-Item -ItemType Directory -Force -Path $InstallDir | Out-Null
# Create and verify install directory
New-SecureDirectory -Path $InstallDir
# Verify install directory is actually a directory (not a symlink to elsewhere)
$dirInfo = Get-Item $InstallDir
if ($dirInfo.Attributes -band [System.IO.FileAttributes]::ReparsePoint) {
Write-Err "Install directory is a symbolic link. Possible symlink attack detected."
}
# Use a temp file in the same directory (same filesystem for atomic move)
$tempExe = Join-Path $InstallDir "core.exe.tmp.$([System.Guid]::NewGuid().ToString('N').Substring(0,8))"
$tempExe = Join-Path $InstallDir "core.exe.tmp"
# Download the binary
# Download the binary to temp location
Invoke-WebRequest -Uri $binaryUrl -OutFile $tempExe -UseBasicParsing
# Download and parse checksums
@ -106,13 +203,17 @@ function Download-Binary {
Write-Err "Could not find checksum for core-windows-$arch.exe in checksums.txt"
}
# Verify hash
# Verify hash BEFORE any move operation
Test-FileHash -FilePath $tempExe -ExpectedHash $expectedHash
# Move to final location only after verification
Move-Item -Path $tempExe -Destination (Join-Path $InstallDir "core.exe") -Force
# Re-verify directory hasn't been replaced with symlink (reduce TOCTOU window)
Test-SecureDirectory -Path $InstallDir
Write-Info "Downloaded and verified: $InstallDir\core.exe"
# Atomic move to final location (same filesystem)
$finalPath = Join-Path $InstallDir "core.exe"
Move-Item -Path $tempExe -Destination $finalPath -Force
Write-Info "Downloaded and verified: $finalPath"
return $true
} catch [System.Net.WebException] {
Write-Warn "Network error during download: $($_.Exception.Message)"
@ -129,38 +230,41 @@ function Download-Binary {
}
}
# Set restrictive ACL on directory (owner-only access)
function Set-SecureDirectoryAcl {
param([string]$Path)
# Verify GPG signature on git tag (if gpg is available)
function Test-GitTagSignature {
param(
[string]$RepoPath,
[string]$Tag
)
if (-not (Test-Command gpg)) {
Write-Warn "GPG not available - skipping tag signature verification"
Write-Warn "For enhanced security, install GPG and import the project signing key"
return $true # Continue without verification
}
Push-Location $RepoPath
try {
$acl = Get-Acl $Path
# Disable inheritance and remove inherited rules
$acl.SetAccessRuleProtection($true, $false)
# Clear existing rules
$acl.Access | ForEach-Object { $acl.RemoveAccessRule($_) } | Out-Null
# Add full control for current user only
$currentUser = [System.Security.Principal.WindowsIdentity]::GetCurrent().Name
$rule = New-Object System.Security.AccessControl.FileSystemAccessRule(
$currentUser,
[System.Security.AccessControl.FileSystemRights]::FullControl,
[System.Security.AccessControl.InheritanceFlags]::ContainerInherit -bor [System.Security.AccessControl.InheritanceFlags]::ObjectInherit,
[System.Security.AccessControl.PropagationFlags]::None,
[System.Security.AccessControl.AccessControlType]::Allow
)
$acl.AddAccessRule($rule)
Set-Acl -Path $Path -AclObject $acl
Write-Info "Set restrictive permissions on $Path"
} catch {
Write-Warn "Could not set restrictive ACL: $($_.Exception.Message)"
# Attempt to verify the tag signature
$result = git tag -v $Tag 2>&1
if ($LASTEXITCODE -eq 0) {
Write-Info "GPG signature verified for tag $Tag"
return $true
} else {
# Check if tag is unsigned vs signature invalid
if ($result -match "error: no signature found") {
Write-Warn "Tag $Tag is not signed - continuing without signature verification"
return $true
} else {
Write-Err "GPG signature verification FAILED for tag $Tag. Possible tampering detected."
}
}
} finally {
Pop-Location
}
}
# Build from source
# Build from source with security checks
function Build-FromSource {
if (-not (Test-Command git)) {
Write-Err "Git is required to build from source. Run '.\scripts\install-deps.ps1' first"
@ -169,11 +273,12 @@ function Build-FromSource {
Write-Err "Go is required to build from source. Run '.\scripts\install-deps.ps1' first"
}
$tmpdir = Join-Path ([System.IO.Path]::GetTempPath()) ([System.Guid]::NewGuid().ToString())
New-Item -ItemType Directory -Force -Path $tmpdir | Out-Null
# Create secure temp directory with restrictive ACL
$tmpdir = Join-Path ([System.IO.Path]::GetTempPath()) "core-build-$([System.Guid]::NewGuid().ToString('N'))"
New-SecureDirectory -Path $tmpdir
# Set restrictive ACL on temp directory
Set-SecureDirectoryAcl -Path $tmpdir
# ACL is REQUIRED for temp build directories (security critical)
Set-SecureDirectoryAcl -Path $tmpdir -Required
try {
Write-Info "Cloning $Repo (version $Version)..."
@ -185,6 +290,9 @@ function Build-FromSource {
Write-Err "Failed to clone repository at version $Version"
}
# Verify GPG signature on tag (if available)
Test-GitTagSignature -RepoPath $cloneDir -Tag $Version
Write-Info "Building core CLI..."
Push-Location $cloneDir
try {
@ -196,14 +304,10 @@ function Build-FromSource {
Pop-Location
}
New-Item -ItemType Directory -Force -Path $InstallDir | Out-Null
# Verify install directory is not a symlink
$dirInfo = Get-Item $InstallDir
if ($dirInfo.Attributes -band [System.IO.FileAttributes]::ReparsePoint) {
Write-Err "Install directory is a symbolic link. Possible symlink attack detected."
}
# Create and verify install directory
New-SecureDirectory -Path $InstallDir
# Move built binary to install location
Move-Item (Join-Path $cloneDir "core.exe") (Join-Path $InstallDir "core.exe") -Force
Write-Info "Built and installed to $InstallDir\core.exe"
@ -214,7 +318,7 @@ function Build-FromSource {
}
}
# Validate and add to PATH
# Validate and add to PATH with precise matching
function Setup-Path {
$userPath = [Environment]::GetEnvironmentVariable("PATH", "User")
@ -233,7 +337,15 @@ function Setup-Path {
return
}
if ($userPath -notlike "*$InstallDir*") {
# Use precise PATH entry matching (not substring)
$pathEntries = $userPath -split ';' | ForEach-Object { $_.TrimEnd('\') }
$normalizedInstallDir = $resolvedInstallDir.TrimEnd('\')
$alreadyInPath = $pathEntries | Where-Object {
$_.Equals($normalizedInstallDir, [System.StringComparison]::OrdinalIgnoreCase)
}
if (-not $alreadyInPath) {
Write-Info "Adding $InstallDir to PATH..."
[Environment]::SetEnvironmentVariable("PATH", "$userPath;$InstallDir", "User")
$env:PATH = "$env:PATH;$InstallDir"

View file

@ -2,6 +2,9 @@
setlocal enabledelayedexpansion
REM Quick setup script for Windows
REM Run as Administrator: setup.bat
REM
REM SECURITY: This script validates environment before executing
REM to prevent path manipulation attacks.
echo === Host UK Developer Workspace Setup ===
echo.
@ -15,38 +18,82 @@ if !errorlevel! neq 0 (
exit /b 1
)
REM Install dependencies
REM === SECURITY: Validate LOCALAPPDATA ===
REM Ensure LOCALAPPDATA is set and appears to be within user profile
if "%LOCALAPPDATA%"=="" (
echo ERROR: LOCALAPPDATA environment variable is not set
goto :error
)
if "%USERPROFILE%"=="" (
echo ERROR: USERPROFILE environment variable is not set
goto :error
)
REM Check that LOCALAPPDATA starts with USERPROFILE (basic validation)
REM This prevents redirection attacks where LOCALAPPDATA points elsewhere
echo !LOCALAPPDATA! | findstr /i /b /c:"!USERPROFILE!" >nul
if !errorlevel! neq 0 (
echo ERROR: LOCALAPPDATA does not appear to be within user profile
echo LOCALAPPDATA: !LOCALAPPDATA!
echo USERPROFILE: !USERPROFILE!
echo This may indicate a path manipulation attack. Aborting.
goto :error
)
REM Validate paths don't contain suspicious characters
echo !LOCALAPPDATA! | findstr /r "[<>|&^]" >nul
if !errorlevel! equ 0 (
echo ERROR: LOCALAPPDATA contains invalid characters
goto :error
)
REM === Install dependencies ===
echo Installing dependencies...
call powershell -ExecutionPolicy Bypass -File "%~dp0scripts\install-deps.ps1"
if !errorlevel! neq 0 goto :error
REM Install core CLI
REM === Install core CLI ===
echo.
echo Installing core CLI...
call powershell -ExecutionPolicy Bypass -File "%~dp0scripts\install-core.ps1"
if !errorlevel! neq 0 goto :error
REM Refresh PATH
set "PATH=%PATH%;%LOCALAPPDATA%\Programs\core"
REM === Validate install path before use ===
set "CORE_PATH=!LOCALAPPDATA!\Programs\core"
REM Verify core.exe exists before running
if not exist "%LOCALAPPDATA%\Programs\core\core.exe" (
echo ERROR: core.exe not found at %LOCALAPPDATA%\Programs\core\core.exe
REM Verify the path exists and is a directory (not a symlink to elsewhere)
if not exist "!CORE_PATH!\core.exe" (
echo ERROR: core.exe not found at !CORE_PATH!\core.exe
goto :error
)
REM Run doctor
REM Check if it's a symlink/junction (basic check via attributes)
for %%F in ("!CORE_PATH!") do (
set "ATTRS=%%~aF"
)
echo !ATTRS! | findstr /c:"l" >nul
if !errorlevel! equ 0 (
echo ERROR: Install directory appears to be a symbolic link
echo This may indicate a symlink attack. Aborting.
goto :error
)
REM Refresh PATH for this session
set "PATH=%PATH%;!CORE_PATH!"
REM === Run doctor ===
echo.
echo === Verifying environment ===
call "%LOCALAPPDATA%\Programs\core\core.exe" doctor
call "!CORE_PATH!\core.exe" doctor
if !errorlevel! neq 0 (
echo WARNING: core doctor reported issues
)
REM Clone repos
REM === Clone repos ===
echo.
echo === Cloning repositories ===
call "%LOCALAPPDATA%\Programs\core\core.exe" setup
call "!CORE_PATH!\core.exe" setup
if !errorlevel! neq 0 goto :error
echo.