Skip to content

Commit 69f6852

Browse files
fullstackjamfullstackjam
committed
fix: resolve multi-channel install conflicts and critical security vulnerabilities in install.sh
- Add conflict detection for curl vs brew installations - Fix P0-1: atomic file operations with temp file and trap cleanup - Fix P0-2: remove unsafe eval, manually set PATH to prevent injection - Fix P0-3: harden curl with --max-time, --retry, --proto=https, --tlsv1.2 - Fix P1-1: cleanup corrupted files on checksum failure - Fix P1-2: add file size validation to detect download errors - Fix P1-3: backup existing binary before installation for rollback - Add checkInstallationConflicts() to doctor command Security improvements address MITM attacks, command injection (CVE-2026-1665 pattern), race conditions, and data integrity issues. Script now follows industry best practices from mise, Deno, and rustup.
1 parent 53299d4 commit 69f6852

2 files changed

Lines changed: 190 additions & 9 deletions

File tree

‎internal/cli/doctor.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ func runDoctor() error {
4444

4545
results = append(results, checkNetwork()...)
4646
results = append(results, checkDiskSpace()...)
47+
results = append(results, checkInstallationConflicts()...)
4748
results = append(results, checkHomebrew()...)
4849
results = append(results, checkGit()...)
4950
results = append(results, checkShell()...)
@@ -268,3 +269,62 @@ func checkDiskSpace() []checkResult {
268269
status: "ok",
269270
}}
270271
}
272+
273+
func checkInstallationConflicts() []checkResult {
274+
var results []checkResult
275+
var installations []string
276+
277+
home, err := os.UserHomeDir()
278+
if err != nil {
279+
return nil
280+
}
281+
282+
curlInstallPath := filepath.Join(home, ".openboot", "bin", "openboot")
283+
if _, err := os.Stat(curlInstallPath); err == nil {
284+
installations = append(installations, curlInstallPath+" (curl install)")
285+
}
286+
287+
if brewPath, err := exec.LookPath("brew"); err == nil {
288+
cmd := exec.Command(brewPath, "list", "openboot")
289+
if err := cmd.Run(); err == nil {
290+
if openbootPath, err := exec.LookPath("openboot"); err == nil {
291+
if realPath, err := filepath.EvalSymlinks(openbootPath); err == nil {
292+
if strings.Contains(realPath, "Cellar/openboot") || strings.Contains(realPath, "opt/homebrew") {
293+
installations = append(installations, openbootPath+" (Homebrew)")
294+
}
295+
}
296+
}
297+
}
298+
}
299+
300+
if len(installations) == 0 {
301+
return []checkResult{{
302+
name: "Installation",
303+
status: "error",
304+
message: "OpenBoot not found in standard locations",
305+
}}
306+
}
307+
308+
if len(installations) > 1 {
309+
results = append(results, checkResult{
310+
name: "Multiple installations",
311+
status: "warn",
312+
message: fmt.Sprintf("found at: %s", strings.Join(installations, ", ")),
313+
})
314+
results = append(results, checkResult{
315+
name: "Recommendation",
316+
status: "info",
317+
message: "keep only one installation method to avoid conflicts",
318+
})
319+
return results
320+
}
321+
322+
if whichPath, err := exec.LookPath("openboot"); err == nil {
323+
results = append(results, checkResult{
324+
name: fmt.Sprintf("Single installation: %s", whichPath),
325+
status: "ok",
326+
})
327+
}
328+
329+
return results
330+
}

‎scripts/install.sh‎

Lines changed: 130 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,73 @@ install_homebrew() {
3737

3838
/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"
3939

40-
if [[ $(uname -m) == "arm64" ]]; then
41-
eval "$(/opt/homebrew/bin/brew shellenv)"
42-
else
43-
eval "$(/usr/local/bin/brew shellenv)"
44-
fi
40+
# Manually set PATH instead of using eval for security
41+
local arch
42+
arch=$(uname -m)
43+
case "$arch" in
44+
arm64)
45+
if [[ -x "/opt/homebrew/bin/brew" ]]; then
46+
export PATH="/opt/homebrew/bin:/opt/homebrew/sbin:$PATH"
47+
export HOMEBREW_PREFIX="/opt/homebrew"
48+
export HOMEBREW_CELLAR="/opt/homebrew/Cellar"
49+
export HOMEBREW_REPOSITORY="/opt/homebrew"
50+
fi
51+
;;
52+
x86_64)
53+
if [[ -x "/usr/local/bin/brew" ]]; then
54+
export PATH="/usr/local/bin:/usr/local/sbin:$PATH"
55+
export HOMEBREW_PREFIX="/usr/local"
56+
export HOMEBREW_CELLAR="/usr/local/Cellar"
57+
export HOMEBREW_REPOSITORY="/usr/local/Homebrew"
58+
fi
59+
;;
60+
*)
61+
echo "Error: Unsupported architecture: $arch" >&2
62+
exit 1
63+
;;
64+
esac
4565

4666
echo ""
4767
echo "Homebrew installed!"
4868
echo ""
4969
}
5070

71+
check_existing_installation() {
72+
if ! command -v brew &>/dev/null; then
73+
return 0
74+
fi
75+
76+
if ! brew list openboot &>/dev/null 2>&1; then
77+
return 0
78+
fi
79+
80+
local existing_path
81+
existing_path=$(command -v openboot 2>/dev/null || true)
82+
83+
if [[ -z "$existing_path" ]]; then
84+
return 0
85+
fi
86+
87+
if [[ -L "$existing_path" ]]; then
88+
local link_target
89+
link_target=$(readlink "$existing_path" 2>/dev/null || true)
90+
if [[ "$link_target" == *"/Cellar/openboot"* ]] || [[ "$link_target" == *"/opt/homebrew"*"/openboot"* ]]; then
91+
echo ""
92+
echo "⚠️ OpenBoot is already installed via Homebrew"
93+
echo " Location: $existing_path"
94+
echo ""
95+
echo "Choose one:"
96+
echo " 1. Update via Homebrew: brew upgrade openboot"
97+
echo " 2. Switch to curl: brew uninstall openboot && curl -fsSL https://openboot.dev/install.sh | bash"
98+
echo ""
99+
exit 1
100+
fi
101+
fi
102+
103+
echo "Cleaning up stale Homebrew registration..."
104+
brew uninstall --force openboot &>/dev/null || true
105+
}
106+
51107
detect_arch() {
52108
local arch
53109
arch=$(uname -m)
@@ -96,7 +152,13 @@ verify_checksum() {
96152
fi
97153

98154
local checksums
99-
if ! checksums=$(curl -fsSL "$checksum_url" 2>/dev/null); then
155+
if ! checksums=$(curl -fsSL \
156+
--max-time 30 \
157+
--retry 3 \
158+
--retry-delay 2 \
159+
--proto '=https' \
160+
--tlsv1.2 \
161+
"$checksum_url" 2>/dev/null); then
100162
echo "Warning: Could not download checksums file. Skipping verification."
101163
return 0
102164
fi
@@ -121,8 +183,12 @@ verify_checksum() {
121183
if [[ "$actual_checksum" != "$expected_checksum" ]]; then
122184
echo ""
123185
echo "Error: Downloaded file appears corrupted."
186+
echo "Expected: $expected_checksum"
187+
echo "Got: $actual_checksum"
188+
echo ""
124189
echo "Please try again or download manually from:"
125190
echo " https://github.com/${REPO}/releases"
191+
rm -f "$binary_path"
126192
exit 1
127193
fi
128194
}
@@ -169,6 +235,19 @@ EOF
169235
}
170236

171237
add_to_path() {
238+
if command -v openboot &>/dev/null; then
239+
local existing_path
240+
existing_path=$(command -v openboot)
241+
if [[ "$existing_path" != "$INSTALL_DIR/openboot" ]]; then
242+
echo "OpenBoot already available at: $existing_path"
243+
echo "Skipping PATH modification to avoid conflicts"
244+
echo ""
245+
echo "If you want to use the version at $INSTALL_DIR/openboot instead,"
246+
echo "remove the existing installation first or adjust your PATH manually."
247+
return 0
248+
fi
249+
fi
250+
172251
local shell_type
173252
shell_type=$(detect_shell)
174253
local rc_file
@@ -233,6 +312,7 @@ main() {
233312
if [[ "$os" == "darwin" && "$snapshot_mode" == false ]]; then
234313
install_xcode_clt
235314
install_homebrew
315+
check_existing_installation
236316
fi
237317

238318
url=$(get_download_url "$os" "$arch")
@@ -259,7 +339,23 @@ main() {
259339
echo "Downloading OpenBoot..."
260340
mkdir -p "$INSTALL_DIR"
261341

262-
if ! curl -fsSL "$url" -o "$binary_path"; then
342+
local temp_binary="${INSTALL_DIR}/.openboot.tmp.$$"
343+
trap 'rm -f "$temp_binary"' EXIT INT TERM
344+
345+
if [[ -f "$binary_path" ]]; then
346+
local backup_path="${binary_path}.backup.$(date +%s)"
347+
echo "Backing up existing binary to: ${backup_path##*/}"
348+
mv "$binary_path" "$backup_path"
349+
fi
350+
351+
if ! curl -fsSL \
352+
--max-time 60 \
353+
--retry 3 \
354+
--retry-delay 2 \
355+
--proto '=https' \
356+
--tlsv1.2 \
357+
"$url" \
358+
-o "$temp_binary"; then
263359
echo ""
264360
echo "Error: Failed to download OpenBoot"
265361
echo "URL: $url"
@@ -268,9 +364,34 @@ main() {
268364
exit 1
269365
fi
270366

271-
verify_checksum "$binary_path" "$os" "$arch"
367+
local file_size
368+
if command -v stat &>/dev/null; then
369+
if stat -f%z "$temp_binary" &>/dev/null 2>&1; then
370+
file_size=$(stat -f%z "$temp_binary")
371+
else
372+
file_size=$(stat -c%s "$temp_binary" 2>/dev/null || echo "0")
373+
fi
374+
else
375+
file_size="0"
376+
fi
377+
378+
if [[ "$file_size" -lt 1000000 ]]; then
379+
echo ""
380+
echo "Error: Downloaded file is too small (${file_size} bytes)"
381+
echo "This may indicate a download error or invalid release"
382+
rm -f "$temp_binary"
383+
exit 1
384+
fi
272385

273-
chmod +x "$binary_path"
386+
verify_checksum "$temp_binary" "$os" "$arch"
387+
388+
chmod 755 "$temp_binary"
389+
390+
if ! mv "$temp_binary" "$binary_path"; then
391+
echo ""
392+
echo "Error: Failed to install binary"
393+
exit 1
394+
fi
274395

275396
add_to_path
276397
export PATH="$INSTALL_DIR:$PATH"

0 commit comments

Comments
 (0)