diff --git a/scripts/install.sh b/scripts/install.sh index 49ab7009c293..312517544f9a 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -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 diff --git a/scripts/test-install-validation.sh b/scripts/test-install-validation.sh deleted file mode 100755 index aaa3aa99eed1..000000000000 --- a/scripts/test-install-validation.sh +++ /dev/null @@ -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' - -
The requested URL was not found on this server.
- - -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 diff --git a/test/scripts/install-sh.test.ts b/test/scripts/install-sh.test.ts index 2ab3e50225ca..a6924d15491f 100644 --- a/test/scripts/install-sh.test.ts +++ b/test/scripts/install-sh.test.ts @@ -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"), "unexpected response\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 'unexpected response\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' "$*"; }