fix(installer): harden managed script downloads

This commit is contained in:
joshavant
2026-07-24 15:45:35 -05:00
committed by Josh Avant
parent 34cc32fa41
commit 09c26e24eb
3 changed files with 184 additions and 280 deletions

View File

@@ -110,10 +110,18 @@ detect_downloader() {
download_file() {
local url="$1"
local output="$2"
local redirect_mode="${3:-follow}"
if [[ -z "$DOWNLOADER" ]]; then
detect_downloader
fi
if [[ "$DOWNLOADER" == "curl" ]]; then
if [[ "$redirect_mode" == "deny" ]]; then
curl -fsSL --max-redirs 0 --proto '=https' --tlsv1.2 \
--speed-limit 1 --speed-time 30 \
--retry 3 --retry-delay 1 --retry-connrefused \
-o "$output" "$url"
return
fi
# Bound post-connect stalls without imposing a total download duration.
curl -fsSL --proto '=https' --tlsv1.2 \
--speed-limit 1 --speed-time 30 \
@@ -121,14 +129,15 @@ download_file() {
-o "$output" "$url"
return
fi
if [[ "$redirect_mode" == "deny" ]]; then
wget -q --max-redirect=0 --https-only --secure-protocol=TLSv1_2 --tries=3 --timeout=20 -O "$output" "$url"
return
fi
wget -q --https-only --secure-protocol=TLSv1_2 --tries=3 --timeout=20 -O "$output" "$url"
}
# Validate a downloaded file is a non-empty shell script before execution.
# Used by run_remote_bash and the NodeSource setup-script paths.
# A full checksum pin is impractical for third-party scripts that update
# frequently (e.g., Homebrew installer), but structural validation catches
# truncated downloads, HTML error pages, and empty responses.
# Managed setup endpoints must return a non-empty script with a raw shebang.
# This is a response-shape check, not an authenticity or completeness check.
validate_downloaded_script() {
local file="$1" url="$2"
if [[ ! -s "$file" ]]; then
@@ -141,26 +150,24 @@ validate_downloaded_script() {
local raw_magic
raw_magic="$(od -An -tx1 -N2 "$file" | tr -d ' ')"
if [[ "$raw_magic" != "2321" ]]; then
local first_line
first_line="$(head -c 256 "$file" | head -1)"
# Sanitize before logging: strip C0 controls (\000-\037), DEL (\177),
# and C1 controls (\200-\237) so untrusted content cannot inject
# terminal escapes through ui_error's echo -e path.
local safe_line
safe_line="$(printf '%s' "${first_line:0:80}" | LC_ALL=C tr -d '\000-\037\177\200-\237')"
safe_line="${safe_line//\\/\\\\}"
ui_error "Downloaded file does not look like a shell script (no shebang): ${url}"
ui_error "First line: ${safe_line}"
return 1
fi
}
download_validated_script() {
local url="$1" output="$2"
# These fixed executable-script endpoints must not redirect: Wget's
# --https-only only filters recursive traversal, not ordinary redirects.
download_file "$url" "$output" deny || return 1
validate_downloaded_script "$output" "$url"
}
run_remote_bash() {
local url="$1"
local tmp
mktempfile tmp
download_file "$url" "$tmp" || return 1
validate_downloaded_script "$tmp" "$url" || return 1
download_validated_script "$url" "$tmp" || return 1
/bin/bash "$tmp"
}
@@ -2022,10 +2029,10 @@ install_node() {
ui_info "Installing Node.js via NodeSource"
if command -v apt-get &> /dev/null; then
local tmp
local tmp setup_url
setup_url="https://deb.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x"
mktempfile tmp
run_required_step "Downloading NodeSource setup script" download_file "https://deb.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
validate_downloaded_script "$tmp" "https://deb.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" || return 1
run_required_step "Downloading NodeSource setup script" download_validated_script "$setup_url" "$tmp"
if is_root; then
run_required_step "Configuring NodeSource repository" bash "$tmp"
run_required_step "Installing Node.js" apt_get_install nodejs
@@ -2034,10 +2041,10 @@ install_node() {
run_required_step "Installing Node.js" apt_get_install nodejs
fi
elif command -v dnf &> /dev/null; then
local tmp
local tmp setup_url
setup_url="https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x"
mktempfile tmp
run_required_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
validate_downloaded_script "$tmp" "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" || return 1
run_required_step "Downloading NodeSource setup script" download_validated_script "$setup_url" "$tmp"
if is_root; then
run_required_step "Configuring NodeSource repository" bash "$tmp"
run_required_step "Installing Node.js" dnf install -y -q nodejs
@@ -2046,10 +2053,10 @@ install_node() {
run_required_step "Installing Node.js" sudo dnf install -y -q nodejs
fi
elif command -v yum &> /dev/null; then
local tmp
local tmp setup_url
setup_url="https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x"
mktempfile tmp
run_required_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
validate_downloaded_script "$tmp" "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" || return 1
run_required_step "Downloading NodeSource setup script" download_validated_script "$setup_url" "$tmp"
if is_root; then
run_required_step "Configuring NodeSource repository" bash "$tmp"
run_required_step "Installing Node.js" yum install -y -q nodejs

View File

@@ -1,249 +0,0 @@
#!/bin/bash
# Test harness for run_remote_bash download validation (PR #82955).
# Sources the production install.sh (via OPENCLAW_INSTALL_SH_NO_RUN=1)
# and exercises validate_downloaded_script directly, proving the real
# code path rejects HTML error pages, JSON responses, binary blobs,
# and empty files while accepting valid scripts.
set -euo pipefail
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# ---------------------------------------------------------------------------
# Source the production install.sh without running it.
# This gives us the real validate_downloaded_script, ui_error, etc.
# ---------------------------------------------------------------------------
export OPENCLAW_INSTALL_SH_NO_RUN=1
# shellcheck source=install.sh
source "${SCRIPT_DIR}/install.sh"
BOLD='\033[1m'
NC='\033[0m'
GREEN='\033[38;2;0;229;204m'
RED='\033[38;2;230;57;70m'
MUTED='\033[38;2;90;100;128m'
pass=0
fail=0
announce() { echo -e "\n${BOLD}── $1 ──${NC}"; }
ok() { pass=$((pass+1)); echo -e " ${GREEN}✓ PASS${NC} $1"; }
ko() { fail=$((fail+1)); echo -e " ${RED}✗ FAIL${NC} $1"; }
# The OLD behavior: no validation at all (always succeeds)
no_validate() {
return 0
}
# ---------------------------------------------------------------------------
# Build test fixtures
# ---------------------------------------------------------------------------
TMPDIR_TEST="$(mktemp -d)"
trap 'rm -rf "$TMPDIR_TEST"' EXIT
# (a) Empty file
touch "$TMPDIR_TEST/empty.sh"
# (b) HTML error page
cat > "$TMPDIR_TEST/html_error.html" << 'HTML'
<html>
<head><title>404 Not Found</title></head>
<body>
<h1>Not Found</h1>
<p>The requested URL was not found on this server.</p>
</body>
</html>
HTML
# (c) JSON error response
cat > "$TMPDIR_TEST/json_error.json" << 'JSON'
{"error":"not_found","message":"The requested resource does not exist","status":404}
JSON
# (d) Binary file (deterministic non-shebang bytes)
# Fixed bytes ensure the first two bytes are never 0x23 0x21 (#!),
# so the test is not probabilistic like /dev/urandom would be.
printf '\x89\x50\x4e\x47\x0d\x0a\x1a\x0a' > "$TMPDIR_TEST/binary.bin"
printf '\x00\x00\x00\x0d\x49\x48\x44\x52' >> "$TMPDIR_TEST/binary.bin"
# (e) Valid shell script
cat > "$TMPDIR_TEST/valid.sh" << 'VALID'
#!/bin/bash
echo "Hello from a valid script"
VALID
# (f) Valid env-style shebang
cat > "$TMPDIR_TEST/valid_env.sh" << 'ENVSH'
#!/usr/bin/env bash
echo "Hello from env-bash script"
ENVSH
# (g) File with C1 control bytes (0x9B = CSI, could inject terminal escapes)
printf '\x9b\x33\x31\x6dPWNED\x9b\x30\x6d rest of line\n' > "$TMPDIR_TEST/c1_escape.txt"
# (h) NUL-prefixed shebang: raw bytes are \x00\x00#!/bin/bash but command
# substitution strips the NULs, so the old string check would false-accept.
printf '\x00\x00#!/bin/bash\necho pwned\n' > "$TMPDIR_TEST/nul_prefix.sh"
# (i) Partial download: starts with a valid shebang but is truncated mid-line.
# A failed curl/wget can leave behind a partial file that passes the shebang
# check. The download_file failure guard (|| return 1) must prevent this
# from ever reaching validate_downloaded_script.
printf '#!/bin/bash\nset -e\necho "starting install"\napt-get ' > "$TMPDIR_TEST/partial.sh"
# ===========================================================================
# GREEN tests — WITH validation (new behavior), bad files are REJECTED
# ===========================================================================
announce "GREEN: validation rejects bad downloads"
if ! validate_downloaded_script "$TMPDIR_TEST/empty.sh" "https://test/empty" 2>/dev/null; then
ok "empty file → rejected"
else
ko "empty file → should have been rejected"
fi
if ! validate_downloaded_script "$TMPDIR_TEST/html_error.html" "https://test/html" 2>/dev/null; then
ok "HTML error → rejected"
else
ko "HTML error → should have been rejected"
fi
if ! validate_downloaded_script "$TMPDIR_TEST/json_error.json" "https://test/json" 2>/dev/null; then
ok "JSON error → rejected"
else
ko "JSON error → should have been rejected"
fi
if ! validate_downloaded_script "$TMPDIR_TEST/binary.bin" "https://test/binary" 2>/dev/null; then
ok "binary blob → rejected"
else
ko "binary blob → should have been rejected"
fi
if ! validate_downloaded_script "$TMPDIR_TEST/c1_escape.txt" "https://test/c1" 2>/dev/null; then
ok "C1 escape → rejected"
else
ko "C1 escape → should have been rejected"
fi
# NUL-prefix bypass: command substitution strips NULs, making the content
# look like it starts with '#!' — the raw byte check catches this.
if ! validate_downloaded_script "$TMPDIR_TEST/nul_prefix.sh" "https://test/nul" 2>/dev/null; then
ok "NUL prefix → rejected (raw byte check caught NUL before #!)"
else
ko "NUL prefix → false-accepted (raw byte check failed)"
fi
# Verify C1 bytes are stripped from the "First line:" diagnostic
c1_first_line="$(validate_downloaded_script "$TMPDIR_TEST/c1_escape.txt" "https://example.com/c1" 2>&1 \
| grep 'First line:' | sed 's/.*First line: //' || true)"
c1_cleaned="$(printf '%s' "$c1_first_line" | LC_ALL=C tr -d '\000-\037\177\200-\237')"
if [ -n "$c1_first_line" ] && [ "$c1_cleaned" = "$c1_first_line" ]; then
ok "C1 diagnostic → C1 bytes stripped from error output"
else
ko "C1 diagnostic → C1 bytes leaked into error output"
fi
# Valid scripts should PASS
if validate_downloaded_script "$TMPDIR_TEST/valid.sh" "https://test/valid" 2>/dev/null; then
ok "valid script (#!bash) → accepted"
else
ko "valid script (#!bash) → should have been accepted"
fi
if validate_downloaded_script "$TMPDIR_TEST/valid_env.sh" "https://test/valid-env" 2>/dev/null; then
ok "valid script (#!env bash) → accepted"
else
ko "valid script (#!env bash) → should have been accepted"
fi
# Partial download: has a valid shebang so validate_downloaded_script ACCEPTS it.
# This proves the download_file failure guard (|| return 1) is essential:
# without it, a failed curl that left a partial file would pass validation.
if validate_downloaded_script "$TMPDIR_TEST/partial.sh" "https://test/partial" 2>/dev/null; then
ok "partial download (valid shebang) → accepted by validation alone"
else
ko "partial download → should have been accepted by validation alone"
fi
# Simulate the full download-then-validate pipeline with a failing downloader.
# download_file returns non-zero but leaves the partial file on disk.
simulate_failed_download() {
local tmp="$1" url="$2"
# Downloader "fails" (returns 1) after writing partial content
cp "$TMPDIR_TEST/partial.sh" "$tmp"
return 1
}
run_remote_bash_guarded() {
local url="$1"
local tmp
tmp="$(mktemp)"
simulate_failed_download "$tmp" "$url" || return 1
validate_downloaded_script "$tmp" "$url" || return 1
/bin/bash "$tmp"
}
if ! run_remote_bash_guarded "https://example.com/partial-download" 2>/dev/null; then
ok "failed download (partial) → pipeline rejected before validation"
else
ko "failed download (partial) → pipeline should have rejected (download_file guard missing)"
fi
# ===========================================================================
# RED tests — WITHOUT validation (old behavior), everything is "accepted"
# ===========================================================================
announce "RED: no validation lets bad downloads through"
if no_validate "$TMPDIR_TEST/empty.sh"; then
ok "empty file → accepted (DANGEROUS without validation)"
else
ko "empty file → unexpectedly rejected"
fi
if no_validate "$TMPDIR_TEST/html_error.html"; then
ok "HTML error → accepted (DANGEROUS without validation)"
else
ko "HTML error → unexpectedly rejected"
fi
if no_validate "$TMPDIR_TEST/json_error.json"; then
ok "JSON error → accepted (DANGEROUS without validation)"
else
ko "JSON error → unexpectedly rejected"
fi
if no_validate "$TMPDIR_TEST/binary.bin"; then
ok "binary blob → accepted (DANGEROUS without validation)"
else
ko "binary blob → unexpectedly rejected"
fi
# ===========================================================================
# Error message demo — show what the user sees when validation fires
# ===========================================================================
announce "Error messages shown to the user"
echo -e "${MUTED}--- empty file ---${NC}"
validate_downloaded_script "$TMPDIR_TEST/empty.sh" "https://example.com/missing.sh" 2>&1 || true
echo -e "${MUTED}--- HTML error page ---${NC}"
validate_downloaded_script "$TMPDIR_TEST/html_error.html" "https://raw.githubusercontent.com/example/install.sh" 2>&1 || true
echo -e "${MUTED}--- JSON error ---${NC}"
validate_downloaded_script "$TMPDIR_TEST/json_error.json" "https://api.example.com/script" 2>&1 || true
echo -e "${MUTED}--- binary blob ---${NC}"
validate_downloaded_script "$TMPDIR_TEST/binary.bin" "https://example.com/corrupted.sh" 2>&1 || true
# ===========================================================================
# Summary
# ===========================================================================
echo ""
echo -e "${BOLD}Results: ${GREEN}${pass} passed${NC}, ${RED}${fail} failed${NC}"
if [[ $fail -eq 0 ]]; then
echo -e "${GREEN}All checks passed — validation works as intended.${NC}"
else
echo -e "${RED}Some checks failed!${NC}"
exit 1
fi

View File

@@ -82,6 +82,97 @@ describe("install.sh", () => {
}
});
it("rejects malformed managed scripts without rendering their content", () => {
const tmp = mkdtempSync(join(tmpdir(), "openclaw-install-script-validation-"));
writeFileSync(join(tmp, "empty.sh"), "");
writeFileSync(join(tmp, "html.sh"), "<html><body>unexpected response</body></html>\n");
writeFileSync(join(tmp, "nul-prefix.sh"), Buffer.from("\0#!/bin/bash\necho unexpected\n"));
writeFileSync(join(tmp, "valid.sh"), "#!/bin/bash\necho valid\n");
try {
const result = runInstallShell(
[
"set -euo pipefail",
`source ${JSON.stringify(SCRIPT_PATH)}`,
'for fixture in empty.sh html.sh nul-prefix.sh; do',
' if validate_downloaded_script "$FIXTURE_DIR/$fixture" "https://example.invalid/$fixture"; then',
' printf "unexpectedly accepted: %s\\n" "$fixture"',
" exit 91",
" fi",
"done",
'validate_downloaded_script "$FIXTURE_DIR/valid.sh" "https://example.invalid/valid.sh"',
].join("\n"),
{ FIXTURE_DIR: tmp },
);
expect(result.status).toBe(0);
expect(result.stdout + result.stderr).not.toContain("unexpected response");
expect(result.stdout + result.stderr).not.toContain("echo unexpected");
} finally {
rmSync(tmp, { force: true, recursive: true });
}
});
it("does not execute a shebang-prefixed partial file after download failure", () => {
const tmp = mkdtempSync(join(tmpdir(), "openclaw-install-partial-download-"));
const marker = join(tmp, "executed");
try {
const result = runInstallShell(
[
"set -euo pipefail",
`source ${JSON.stringify(SCRIPT_PATH)}`,
'download_file() { printf \'#!/bin/bash\\n: > "$EXECUTION_MARKER"\\n\' > "$2"; return 23; }',
"set +e",
'run_remote_bash "https://example.invalid/partial.sh"',
"status=$?",
"set -e",
'printf "status=%s\\n" "$status"',
'[[ "$status" -ne 0 ]]',
].join("\n"),
{ EXECUTION_MARKER: marker },
);
expect(result.status).toBe(0);
expect(result.stdout).toContain("status=1");
expect(existsSync(marker)).toBe(false);
} finally {
rmSync(tmp, { force: true, recursive: true });
}
});
it("denies redirects for managed script downloads", () => {
const tmp = mkdtempSync(join(tmpdir(), "openclaw-install-managed-download-"));
try {
const result = runInstallShell(
`
set -euo pipefail
source "${SCRIPT_PATH}"
curl() { printf 'curl=%s\n' "$*"; }
wget() { printf 'wget=%s\n' "$*"; }
DOWNLOADER=curl
download_file "https://example.invalid/setup.sh" "$DOWNLOAD_DIR/curl-setup.sh" deny
DOWNLOADER=wget
download_file "https://example.invalid/setup.sh" "$DOWNLOAD_DIR/wget-setup.sh" deny
download_file() {
printf 'managed-mode=%s\n' "\${3:-}"
printf '#!/bin/bash\n' > "$2"
}
download_validated_script "https://example.invalid/setup.sh" "$DOWNLOAD_DIR/managed-setup.sh"
`,
{ DOWNLOAD_DIR: tmp },
);
expect(result.status).toBe(0);
expect(result.stdout).toContain("curl=-fsSL --max-redirs 0");
expect(result.stdout).toContain("wget=-q --max-redirect=0");
expect(result.stdout).toContain("managed-mode=deny");
} finally {
rmSync(tmp, { force: true, recursive: true });
}
});
it("bounds stalled curl downloads and propagates timeout failures", () => {
const result = runInstallShell(`
set -euo pipefail
@@ -99,10 +190,68 @@ describe("install.sh", () => {
expect(result.status).toBe(0);
expect(result.stdout).toContain("--speed-limit 1 --speed-time 30");
expect(result.stdout).not.toContain("--connect-timeout");
expect(result.stdout).not.toContain("--max-redirs");
expect(result.stdout).toContain("--retry 3 --retry-delay 1 --retry-connrefused");
expect(result.stdout).toContain("status=28");
});
it.each(["apt-get", "dnf", "yum"])(
"rejects an invalid NodeSource response before %s repository setup",
(packageManager) => {
const tmp = mkdtempSync(join(tmpdir(), "openclaw-install-nodesource-validation-"));
const marker = join(tmp, "configured");
try {
const result = runInstallShell(
`
set -euo pipefail
source "${SCRIPT_PATH}"
OS=linux
PACKAGE_MANAGER="$PACKAGE_MANAGER_UNDER_TEST"
require_sudo() { :; }
install_build_tools_linux() { return 0; }
is_root() { return 0; }
command() {
if [[ "\${1:-}" == "-v" ]]; then
case "\${2:-}" in
pacman|apk) return 1 ;;
apt-get|dnf|yum) [[ "$PACKAGE_MANAGER" == "$2" ]]; return ;;
esac
fi
builtin command "$@"
}
download_file() {
printf '<html>unexpected response</html>\n' > "$2"
}
ui_info() { printf 'info:%s\n' "$*"; }
ui_success() { :; }
ui_error() { printf 'error:%s\n' "$*"; }
run_quiet_step() {
local title="$1"
shift
printf 'step:%s|%s\n' "$title" "$*"
if [[ "$title" == "Downloading NodeSource setup script" ]]; then
"$@"
return
fi
: > "$EXECUTION_MARKER"
return 0
}
install_node
`,
{ EXECUTION_MARKER: marker, PACKAGE_MANAGER_UNDER_TEST: packageManager },
);
expect(result.status).toBe(1);
expect(result.stdout).toContain("step:Downloading NodeSource setup script");
expect(result.stdout).not.toContain("unexpected response");
expect(existsSync(marker)).toBe(false);
} finally {
rmSync(tmp, { force: true, recursive: true });
}
},
);
it("runs apt-get through noninteractive wrappers", () => {
expect(script).toContain("apt_get()");
expect(script).toContain('DEBIAN_FRONTEND="${DEBIAN_FRONTEND:-noninteractive}"');
@@ -259,8 +408,7 @@ NODE
is_alpine_linux() { return 1; }
pacman() { printf 'pacman:%s\\n' "$*"; }
apt-get() { :; }
download_file() { :; }
validate_downloaded_script() { return 0; }
download_validated_script() { :; }
ui_info() { printf 'info:%s\\n' "$*"; }
ui_success() { :; }
run_required_step() { printf 'step:%s|%s\\n' "$1" "\${*:2}"; }
@@ -399,8 +547,7 @@ NODE
is_root() { return 0; }
is_alpine_linux() { return 1; }
apt-get() { :; }
download_file() { :; }
validate_downloaded_script() { return 0; }
download_validated_script() { return 0; }
ui_info() { printf 'info:%s\\n' "$*"; }
ui_success() { printf 'success:%s\\n' "$*"; }
ui_error() { printf 'error:%s\\n' "$*"; }
@@ -439,8 +586,7 @@ NODE
is_root() { return 0; }
is_alpine_linux() { return 1; }
apt-get() { :; }
download_file() { :; }
validate_downloaded_script() { return 0; }
download_validated_script() { return 0; }
ui_info() { printf 'info:%s\\n' "$*"; }
ui_success() { printf 'success:%s\\n' "$*"; }
ui_error() { printf 'error:%s\\n' "$*"; }