Conversation
| Write-Error-Custom "An unexpected script error occurred: $($_.Exception.Message)" | ||
| } | ||
| finally { |
There was a problem hiding this comment.
🔴 The temp directory created on line 20 (containing a full git clone plus Go build artifacts) is never cleaned up. The finally block only restores the working directory but does not delete $tmp_dir, leaking potentially hundreds of MB per install. Add Remove-Item -Recurse -Force $tmp_dir -ErrorAction SilentlyContinue to the finally block.
Extended reasoning...
What the bug is
The PowerShell install script creates a temporary directory on line 20-21 and clones the entire repository into it, then downloads Go modules and builds the binary. However, the finally block (lines 58-60) only runs Set-Location $env:USERPROFILE to restore the working directory. It never removes the temporary directory.
How it manifests
Every time a user runs install.ps1 (whether for a fresh install or an upgrade), a new uniquely-named directory is created under %TEMP% containing a full shallow git clone of the repository, Go module cache and build artifacts, and potentially the compiled binary. This could easily be 100+ MB per run, and the directories accumulate indefinitely since each has a unique GUID-based name.
Why existing code does not prevent it
The finally block is the correct place for cleanup in PowerShell try/catch/finally, but it only contains Set-Location $env:USERPROFILE. There is no Remove-Item call anywhere in the script. The catch block also does not attempt cleanup.
Proof by comparison with bash script
The bash equivalent scripts/install.sh correctly handles this at line 143 with trap 'rm -rf "$tmp_dir"' EXIT, which ensures the temp directory is removed regardless of how the script exits. The PowerShell script is missing the equivalent cleanup.
Step-by-step example
- User runs
install.ps1 - Line 20:
$tmp_diris set to a unique path under TEMP - Line 21: Directory is created
- Lines 24-25: Repo is cloned into it (50+ MB)
- Line 28:
go mod downloadfetches dependencies (50+ MB) - Lines 32-36: Binary is built
- Line 40: Binary is moved to install dir
- Script reaches
finallyblock (lines 58-60) Set-Location $env:USERPROFILEruns, but$tmp_dirstill exists with the clone and build artifacts- The temp directory persists indefinitely, leaking disk space
Fix
Add cleanup to the finally block:
finally {
Set-Location $env:USERPROFILE
Remove-Item -Recurse -Force $tmp_dir -ErrorAction SilentlyContinue
}| Write-Info "Updating PATH..." | ||
| $current_path = [Environment]::GetEnvironmentVariable("Path", "User") | ||
| if ($current_path -notlike "*$INSTALL_DIR*") { | ||
| $new_path = "$current_path;$INSTALL_DIR" |
There was a problem hiding this comment.
🔴 The PATH update on lines 43-46 has two issues: (1) [Environment]::SetEnvironmentVariable writes PATH back as REG_SZ, permanently converting any existing REG_EXPAND_SZ entries (like %USERPROFILE%\AppData\Local\Microsoft\WindowsApps) into hardcoded literal paths, destroying expandable variable references for all PATH entries. (2) If no user-level PATH exists (common on fresh installs), GetEnvironmentVariable returns $null and the concatenation produces ";C:\...\bin" with a leading semicolon, creating a malformed empty path entry. Fix by using [Microsoft.Win32.Registry]::SetValue with RegistryValueKind.ExpandString to preserve registry types, and adding a null check before concatenation.
Extended reasoning...
Registry value type corruption (REG_EXPAND_SZ → REG_SZ)
On lines 43-46, the script reads the user-level PATH using [Environment]::GetEnvironmentVariable("Path", "User") and writes it back using [Environment]::SetEnvironmentVariable("Path", $new_path, "User"). This roundtrip has a well-documented destructive side effect on Windows.
On default Windows 10/11 installations, the user-level PATH (stored at HKCU\Environment\Path) is typically of type REG_EXPAND_SZ and contains unexpanded variable references like %USERPROFILE%\AppData\Local\Microsoft\WindowsApps. The .NET GetEnvironmentVariable method expands all %VAR% references when reading, returning literal paths like C:\Users\alice\AppData\Local\Microsoft\WindowsApps. Then SetEnvironmentVariable writes the value back as REG_SZ (not REG_EXPAND_SZ), permanently converting all variable references to hardcoded literal strings.
Step-by-step proof of the registry type corruption:
- User has
HKCU\Environment\Path=%USERPROFILE%\AppData\Local\Microsoft\WindowsApps(type:REG_EXPAND_SZ) [Environment]::GetEnvironmentVariable("Path", "User")returns"C:\Users\alice\AppData\Local\Microsoft\WindowsApps"(expanded)- The
-notlikecheck passes, so:$new_path = "C:\Users\alice\AppData\Local\Microsoft\WindowsApps;C:\Users\alice\.entire\bin" [Environment]::SetEnvironmentVariable("Path", $new_path, "User")writes this asREG_SZ- The registry now contains literal paths instead of
%USERPROFILE%references, and the value type is permanently changed fromREG_EXPAND_SZtoREG_SZ
This is irreversible without manual registry editing and affects ALL existing PATH entries, not just the one being added. The fix is to use [Microsoft.Win32.Registry]::SetValue("HKEY_CURRENT_USER\Environment", "Path", $new_path, [Microsoft.Win32.RegistryValueKind]::ExpandString) to preserve the registry value type.
Null PATH leading semicolon
On fresh Windows installs, the user-level PATH may not be set at all (PATH is typically only defined at the Machine level). In this case, [Environment]::GetEnvironmentVariable("Path", "User") returns $null. PowerShell interpolates $null as an empty string, so "$current_path;$INSTALL_DIR" on line 45 becomes ";C:\Users\alice\.entire\bin" with a leading semicolon.
Step-by-step proof:
- Fresh Windows install, no user-level PATH set
$current_path=$null$null -notlike "*$INSTALL_DIR*"evaluates to$true(PowerShell null comparison)"$current_path;$INSTALL_DIR"→";C:\Users\alice\.entire\bin"(leading semicolon)- This malformed value is written permanently to the user-level PATH registry key
While Windows generally tolerates leading semicolons, an empty path entry can resolve to the current directory, which is a minor security concern. The fix is straightforward: if ([string]::IsNullOrEmpty($current_path)) { $new_path = $INSTALL_DIR } else { $new_path = "$current_path;$INSTALL_DIR" }.
| @@ -0,0 +1,62 @@ | |||
| $ErrorActionPreference = "Continue" | |||
There was a problem hiding this comment.
🔴 $ErrorActionPreference = "Continue" on line 1 means non-terminating errors from cmdlets (Move-Item, Set-Location, New-Item) will not trigger the catch block — the script silently proceeds through failed steps (e.g., if git clone fails, it continues to go mod download and go build in a wrong/empty directory). Should be "Stop". Additionally, Write-Error-Custom on lines 52 and 56 only prints colored text without setting an exit code or throwing, so those error paths exit with code 0 (success), misleading CI/automation. The bash install.sh correctly uses set -euo pipefail and exit 1 in its error() function.
Extended reasoning...
Bug Analysis: ErrorActionPreference and missing exit codes
The ErrorActionPreference problem
The script sets $ErrorActionPreference = "Continue" on line 1. In PowerShell, "Continue" is the default — non-terminating errors are displayed but execution continues. This completely defeats the try/catch block that wraps the build pipeline (lines 23-57), because catch only catches terminating errors. With "Continue", cmdlet failures from New-Item, Set-Location, and Move-Item are silently swallowed.
Concrete failure scenario
Consider what happens when git clone fails due to a network error:
- Line 25:
git clonefails. Since it is an external command, it sets$LASTEXITCODEto non-zero but does not throw a terminating error regardless of$ErrorActionPreference. - Line 26:
Set-Location $tmp_dir. With"Continue", if the directory is empty or partially created, the cmdlet may fail but execution continues. - Line 28:
go mod downloadruns in the wrong directory, producing confusing errors or silently doing nothing. - Lines 33-36:
go buildruns and likely fails or produces no binary. - Line 39:
Test-Path $BINARY_NAMEis false, so we fall to line 52. - Line 52:
Write-Error-Customprints a red message, but the script exits with code 0.
The user sees red text about a missing binary but has no idea the root cause was a failed git clone. Any automation checking exit codes thinks the install succeeded.
The Write-Error-Custom problem
The function on line 9 is just a display helper. It does not call exit 1, throw, or set any exit code. Compare with the bash install.sh (lines 37-39) which properly calls exit 1 after printing to stderr. The first call site (line 12) is fine because it is manually followed by exit 1, but the other two are not:
- Line 52: Binary not found after build. Script falls through to
finallyand exits 0. - Line 56: Catch block for unexpected errors. Script falls through to
finallyand exits 0.
This is a distinct issue from the ErrorActionPreference bug. Even if ErrorActionPreference were fixed to "Stop", the catch block on line 56 would still exit with code 0.
Impact
For interactive users, the red text is visible but misleading (wrong root cause shown). For CI/CD pipelines or automation that checks $LASTEXITCODE, both error paths report success (exit code 0) despite failure. The PR description even shows a iwr | iex remote install pattern, where silent failure is especially dangerous.
Recommended fix
- Change line 1 to
$ErrorActionPreference = "Stop"to make cmdlet errors terminating (the PowerShell equivalent ofset -e). - Add
$LASTEXITCODEchecks after external commands (git,go) since even with"Stop", native executables do not throw terminating errors in PowerShell 5.1. - Add
exit 1afterWrite-Error-Customon lines 52 and 56, or modify the function itself to includeexit 1(and remove the manualexit 1on line 13).
|
|
||
| if (!(Get-Command go -ErrorAction SilentlyContinue)) { | ||
| Write-Error-Custom "Go is not installed. Download it here: https://go.dev/dl/" | ||
| exit 1 |
There was a problem hiding this comment.
🔴 exit 1 on line 13 will terminate the entire PowerShell session when the script is run via the documented iwr ... | iex pattern. The user's terminal window closes immediately, and the "Go is not installed" error message is lost. Replace exit 1 with return or wrap the script body in a function to scope the exit.
Extended reasoning...
The Bug
When PowerShell executes code via Invoke-Expression (aliased as iex), there is no script scope boundary — the code runs directly in the caller's scope. In this context, exit 1 does not exit "the script"; it terminates the entire PowerShell host process. This is a well-documented PowerShell behavior.
How It Triggers
The PR description explicitly advertises remote installation via:
Set-ExecutionPolicy RemoteSigned -Scope Process; iwr https://entire.io/install.ps1 | iexWhen a user without Go installed runs this command, execution reaches line 11-13:
if (!(Get-Command go -ErrorAction SilentlyContinue)) {
Write-Error-Custom "Go is not installed. Download it here: https://go.dev/dl/"
exit 1
}The Write-Error-Custom message briefly appears, then exit 1 immediately terminates the PowerShell host. The user sees their terminal window close with no chance to read the error.
Step-by-Step Proof
- User opens PowerShell (does not have Go installed)
- User runs:
iwr https://entire.io/install.ps1 | iex - PowerShell downloads the script content and passes it to
Invoke-Expression iexevaluates the script in the current session scope (no child process, no script scope)Get-Command gofails silently, theifcondition is trueWrite-Error-Customprints the red error textexit 1executes — since there is no script scope to exit from, PowerShell exits the host process- The terminal window closes immediately; the error message is gone
Impact
This affects the primary documented remote install path. Users who don't have Go installed — arguably the most common failure case — get the worst possible experience: their terminal disappears with no explanation.
Fix
Replace exit 1 with return on line 13. When run via iex, return exits the current scope without terminating the host. When run via powershell -File, return also works correctly (exits the script with code 0 rather than 1, but the error message is still displayed). Alternatively, wrap the entire script body in a function and invoke it, which creates a proper scope boundary:
function Install-EntireCLI {
# ... script body ...
if (!(Get-Command go -ErrorAction SilentlyContinue)) {
Write-Error-Custom "Go is not installed."
return
}
# ...
}
Install-EntireCLIThe function-wrapper approach is the most robust since it works identically in both -File and iex invocation modes.
| go build -v -ldflags="-s -w" -o $BINARY_NAME ./cmd/entire | ||
| } else { | ||
| go build -v -ldflags="-s -w" -o $BINARY_NAME . |
There was a problem hiding this comment.
🟡 The go build commands diverge from the official goreleaser config in three ways: missing CGO_ENABLED=0 (risking build failures on Windows with MinGW/MSYS2), missing -X ldflags for version info (so entire version always shows "dev"/"unknown" despite line 49 telling users to verify with it), and always cloning HEAD instead of a release tag (users may get unreleased code). Consider fetching the latest release tag via the GitHub API, setting $env:CGO_ENABLED = "0", and adding -X ldflags to match the official build.
Extended reasoning...
Build configuration divergence from official goreleaser
The new install.ps1 builds the binary with go build -v -ldflags="-s -w", but the official build configuration in .goreleaser.yaml and mise-tasks/build includes several additional settings that are absent here.
Missing CGO_ENABLED=0
The .goreleaser.yaml (line 14) explicitly sets CGO_ENABLED=0 for all official builds. On Windows, Go defaults to CGO_ENABLED=1 when a C compiler is detected in PATH. This is common among Windows Go developers who have MinGW or MSYS2 installed. Without $env:CGO_ENABLED = "0", the build could fail with confusing gcc errors if any transitive dependency has platform-incompatible C code, or produce a binary with subtly different behavior (e.g., net and os/user packages use different implementations with CGO enabled vs disabled). This is especially relevant since goreleaser only targets darwin/linux — install.ps1 is the sole path to a Windows binary.
Missing -X ldflags for version info
The project embeds version info at build time. Both .goreleaser.yaml (lines 23-24) and mise-tasks/build (line 8) pass -X github.com/entireio/cli/cmd/entire/cli/versioninfo.Version=... and -X ...Commit=... via ldflags. The defaults in versioninfo.go are Version = "dev" and Commit = "unknown". Without the -X flags, the installed binary will always report version "dev" and commit "unknown". Line 49 of the script tells users: "Restart your terminal and run entire version to verify" — but this will show confusing dev/unknown output, making it look like the install failed. Even if version pinning (bug_010) were fixed to clone a tagged release, the binary would still show "dev" without the -X flags.
No version pinning
The bash install.sh fetches the latest release tag via the GitHub API and downloads a versioned binary. The PowerShell script clones HEAD of the default branch (git clone --depth 1), which could include unreleased or broken code. One verifier argues this is intentional since no pre-built Windows binaries exist, and building from source is the deliberate MVP approach — which is a fair point. However, the script could still fetch the latest release tag and use git clone --branch <tag> --depth 1 to ensure users get tested, released code.
Concrete walkthrough
- A Windows developer with MinGW installed runs the install script.
git clone --depth 1fetches HEAD of the default branch (possibly unreleased code).go buildruns. Go detectsgccin PATH, setsCGO_ENABLED=1internally.- If the build succeeds, the binary is installed to
~/.entire/bin/entire.exe. - The user follows the script output: restarts terminal and runs
entire version. - Output shows
Version: dev, Commit: unknown— the user has no idea what version they actually built. - If the build fails due to CGO-related issues, the error messages reference gcc/cc, which is confusing for a pure Go project.
Suggested fix
Before the build commands, add:
$env:CGO_ENABLED = "0"
$version = (git describe --tags --abbrev=0 2>$null) -replace ^v,
if (-not $version) { $version = "dev" }
$commit = (git rev-parse --short HEAD)Then update the ldflags to:
-ldflags="-s -w -X github.com/entireio/cli/cmd/entire/cli/versioninfo.Version=$version -X github.com/entireio/cli/cmd/entire/cli/versioninfo.Commit=$commit"And ideally fetch the latest release tag before cloning.
Adds native Windows install (using Golang build) script.
Usage: