fix(update): honor outer archive timeout (#4790)

Author: Duang777

loopx/self_update_download.py

@@ -67,13 +67,17 @@ def run_archive_installer(
                     code = 28
                     break
                 observation["stage"] = "installer_execution"
+                installer_env = dict(env)
+                installer_env["LOOPX_INSTALLER_TIMEOUT_SECONDS"] = str(
+                    max(1, int(remaining))
+                )
                 try:
                     return subprocess.run(
                         ["bash", str(script)],
                         check=False,
                         text=True, encoding="utf-8", errors="replace",
                         capture_output=True,
-                        env=env,
+                        env=installer_env,
                         timeout=remaining,
                     ), observation
                 except subprocess.TimeoutExpired:

scripts/install-from-github.sh

@@ -6,6 +6,7 @@ ref="${LOOPX_REF:-stable}"
 archive_url_override="${LOOPX_ARCHIVE_URL:-}"
 archive_url="$archive_url_override"
 python_bin="${LOOPX_PYTHON:-python3}"
+installer_timeout_seconds="${LOOPX_INSTALLER_TIMEOUT_SECONDS:-}"
 export LOOPX_REPO="$repo"
 export LOOPX_REF="$ref"
 
@@ -25,6 +26,11 @@ if [[ -n "${LOOPX_RESOLVED_SOURCE_GIT_COMMIT:-}" \
   echo "loopx installer error: LOOPX_RESOLVED_SOURCE_GIT_COMMIT must be a full Git commit SHA" >&2
   exit 2
 fi
+if [[ -n "$installer_timeout_seconds" \
+  && ! "$installer_timeout_seconds" =~ ^[1-9][0-9]*$ ]]; then
+  echo "loopx installer error: LOOPX_INSTALLER_TIMEOUT_SECONDS must be a positive integer" >&2
+  exit 2
+fi
 
 tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/loopx-install.XXXXXX")"
 cleanup() {
@@ -112,8 +118,65 @@ extract_dir="$tmp_dir/extract"
 mkdir -p "$extract_dir"
 
 echo "loopx installer: downloading $archive_url" >&2
-curl -fsSL --connect-timeout 10 --max-time 120 --retry 2 --retry-max-time 150 \
-  "$archive_url" -o "$archive_path"
+archive_deadline=$((SECONDS + ${installer_timeout_seconds:-150}))
+archive_attempt=1
+archive_max_attempts=3
+archive_attempts_completed=0
+archive_downloaded=0
+last_curl_code=28
+last_http_status=0
+while [[ "$archive_attempt" -le "$archive_max_attempts" ]]; do
+  remaining=$((archive_deadline - SECONDS))
+  if [[ "$remaining" -le 0 ]]; then
+    break
+  fi
+  attempt_timeout="$remaining"
+  if [[ "$archive_attempt" -lt "$archive_max_attempts" ]]; then
+    reserved_attempts=$((archive_max_attempts - archive_attempt))
+    attempt_timeout=$((remaining - reserved_attempts))
+    if [[ "$attempt_timeout" -gt 120 ]]; then
+      attempt_timeout=120
+    elif [[ "$attempt_timeout" -lt 1 ]]; then
+      attempt_timeout=1
+    fi
+  fi
+  archive_attempts_completed="$archive_attempt"
+  if http_status="$(curl --silent --show-error --fail --location \
+    --connect-timeout 10 --max-time "$attempt_timeout" \
+    --continue-at - --write-out '%{http_code}' \
+    "$archive_url" -o "$archive_path")"; then
+    archive_downloaded=1
+    break
+  else
+    last_curl_code=$?
+  fi
+  if [[ "$http_status" =~ ^[0-9]{3}$ ]]; then
+    last_http_status="$http_status"
+  else
+    last_http_status=0
+  fi
+  retryable=0
+  case "$last_curl_code" in
+    5|6|7|18|28|35|52|55|56)
+      retryable=1
+      ;;
+    22)
+      case "$last_http_status" in
+        403|408|429|500|502|503|504)
+          retryable=1
+          ;;
+      esac
+      ;;
+  esac
+  if [[ "$retryable" -ne 1 ]]; then
+    break
+  fi
+  archive_attempt=$((archive_attempt + 1))
+done
+if [[ "$archive_downloaded" -ne 1 ]]; then
+  echo "loopx installer error: archive download failed after $archive_attempts_completed attempt(s) (curl $last_curl_code, HTTP $last_http_status)" >&2
+  exit "$last_curl_code"
+fi
 archive_sha256="$("$python_bin" - "$archive_path" <<'PY'
 from pathlib import Path
 import hashlib

tests/test_archive_installer_commit_response.py

@@ -1,16 +1,66 @@
 """Exercise the shipped shell installer with a large GitHub commit response."""
 
+import hashlib
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
 import json
 import os
 from pathlib import Path
 import shlex
 import subprocess
 import sys
 import tarfile
+import threading
+import time
 
 import pytest
 
 
+def _start_partial_archive_server(payload, *, complete_resume):
+    requests = []
+    partial_size = max(1, len(payload) // 3)
+
+    class Handler(BaseHTTPRequestHandler):
+        def do_GET(self):
+            range_header = self.headers.get("Range")
+            requests.append(range_header)
+            if len(requests) == 1:
+                self.send_response(200)
+                self.send_header("Content-Length", str(len(payload)))
+                self.end_headers()
+                self.wfile.write(payload[:partial_size])
+                self.wfile.flush()
+                time.sleep(3)
+                return
+            if not complete_resume:
+                self.send_response(503)
+                self.send_header("Content-Length", "0")
+                self.end_headers()
+                return
+            expected_range = f"bytes={partial_size}-"
+            if range_header != expected_range:
+                self.send_response(400)
+                self.send_header("Content-Length", "0")
+                self.end_headers()
+                return
+            self.send_response(206)
+            self.send_header("Content-Length", str(len(payload) - partial_size))
+            self.send_header(
+                "Content-Range",
+                f"bytes {partial_size}-{len(payload) - 1}/{len(payload)}",
+            )
+            self.end_headers()
+            self.wfile.write(payload[partial_size:])
+
+        def log_message(self, _format, *_args):
+            pass
+
+    server = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
+    server.daemon_threads = True
+    thread = threading.Thread(target=server.serve_forever, daemon=True)
+    thread.start()
+    return server, thread, requests, partial_size
+
+
 @pytest.mark.skipif(os.name == "nt", reason="POSIX archive installer")
 def test_invalid_commit_override_precedes_temp_directory_failure(tmp_path):
     source = Path(__file__).resolve().parents[1]
@@ -67,10 +117,11 @@ def test_commit_response_uses_file_transport_and_cleans_up(tmp_path, valid, ref_
         + shlex.quote(sys.executable)
         + " -c "
         + shlex.quote(
-            "import os,sys,shutil; "
+            "import json,os,sys,shutil; "
             "args=sys.argv[1:]; "
             "is_api=any('api.github.com' in a for a in args); "
             "open(os.environ['TEST_CALLS'],'a').write('api\\n' if is_api else 'archive\\n'); "
+            "open(os.environ['TEST_ARCHIVE_ARGS'],'w').write(json.dumps(args)) if not is_api else None; "
             "sys.exit(22) if is_api and (os.environ['TEST_REF_KIND']=='sha' or os.environ['TEST_API_MODE']!='public') else None; "
             "source=os.environ['TEST_RESPONSE'] if any('api.github.com' in a for a in args) "
             "else os.environ['TEST_ARCHIVE']; "
@@ -98,8 +149,10 @@ def test_commit_response_uses_file_transport_and_cleans_up(tmp_path, valid, ref_
         TMPDIR=str(scratch),
         LOOPX_PYTHON=sys.executable,
         LOOPX_REF=sha if ref_kind == "sha" else "stable",
+        LOOPX_INSTALLER_TIMEOUT_SECONDS="480",
         TEST_REF_KIND=ref_kind,
         TEST_API_MODE=api_mode,
+        TEST_ARCHIVE_ARGS=str(tmp_path / "archive-args.json"),
         TEST_CALLS=str(tmp_path / "calls"),
         TEST_RESPONSE=str(fixture),
         TEST_ARCHIVE=str(archive),
@@ -130,3 +183,102 @@ def test_commit_response_uses_file_transport_and_cleans_up(tmp_path, valid, ref_
     else:
         assert calls[0] == "api"
         assert ("authenticated" in calls) == (api_mode != "public")
+    if "archive" in calls:
+        archive_args = json.loads((tmp_path / "archive-args.json").read_text())
+        assert archive_args[archive_args.index("--max-time") + 1] == "120"
+        assert "--retry" not in archive_args
+        assert archive_args[archive_args.index("--continue-at") + 1] == "-"
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX archive installer")
+def test_archive_download_resumes_after_attempt_timeout(tmp_path):
+    source = Path(__file__).resolve().parents[1]
+    package = tmp_path / "package"
+    scripts = package / "scripts"
+    scripts.mkdir(parents=True)
+    installer = scripts / "install-local.sh"
+    installer.write_text(
+        '#!/bin/sh\nprintf "%s\\n" "$LOOPX_ARCHIVE_SHA256" > "$TEST_RECEIPT"\n'
+    )
+    installer.chmod(0o755)
+    (package / "payload.bin").write_bytes(bytes(range(256)) * 4096)
+    archive = tmp_path / "package.tar.gz"
+    with tarfile.open(archive, "w:gz") as handle:
+        handle.add(package, arcname="package")
+    payload = archive.read_bytes()
+    server, thread, requests, partial_size = _start_partial_archive_server(
+        payload, complete_resume=True
+    )
+    scratch = tmp_path / "scratch"
+    scratch.mkdir()
+    receipt = tmp_path / "receipt"
+    env = {k: v for k, v in os.environ.items() if not k.startswith("LOOPX_")}
+    env.update(
+        TMPDIR=str(scratch),
+        LOOPX_PYTHON=sys.executable,
+        LOOPX_ARCHIVE_URL=f"http://127.0.0.1:{server.server_port}/archive.tar.gz",
+        LOOPX_INSTALLER_TIMEOUT_SECONDS="4",
+        TEST_RECEIPT=str(receipt),
+    )
+    try:
+        result = subprocess.run(
+            ["bash", str(source / "scripts/install-from-github.sh")],
+            env=env,
+            capture_output=True,
+            text=True,
+            timeout=15,
+        )
+    finally:
+        server.shutdown()
+        server.server_close()
+        thread.join(timeout=5)
+
+    assert result.returncode == 0, result.stderr
+    assert requests[:2] == [None, f"bytes={partial_size}-"]
+    assert receipt.read_text().strip() == hashlib.sha256(payload).hexdigest()
+    assert list(scratch.iterdir()) == []
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX archive installer")
+def test_archive_timeout_failure_never_extracts_partial_file(tmp_path):
+    source = Path(__file__).resolve().parents[1]
+    payload = bytes(range(256)) * 4096
+    server, thread, requests, partial_size = _start_partial_archive_server(
+        payload, complete_resume=False
+    )
+    binary = tmp_path / "bin"
+    binary.mkdir()
+    tar_marker = tmp_path / "tar-called"
+    tar = binary / "tar"
+    tar.write_text(
+        '#!/bin/sh\nprintf called > "$TEST_TAR_MARKER"\nexit 99\n'
+    )
+    tar.chmod(0o755)
+    scratch = tmp_path / "scratch"
+    scratch.mkdir()
+    env = {k: v for k, v in os.environ.items() if not k.startswith("LOOPX_")}
+    env.update(
+        PATH=f"{binary}{os.pathsep}{env['PATH']}",
+        TMPDIR=str(scratch),
+        LOOPX_PYTHON=sys.executable,
+        LOOPX_ARCHIVE_URL=f"http://127.0.0.1:{server.server_port}/archive.tar.gz",
+        LOOPX_INSTALLER_TIMEOUT_SECONDS="4",
+        TEST_TAR_MARKER=str(tar_marker),
+    )
+    try:
+        result = subprocess.run(
+            ["bash", str(source / "scripts/install-from-github.sh")],
+            env=env,
+            capture_output=True,
+            text=True,
+            timeout=15,
+        )
+    finally:
+        server.shutdown()
+        server.server_close()
+        thread.join(timeout=5)
+
+    assert result.returncode != 0
+    assert requests[:2] == [None, f"bytes={partial_size}-"]
+    assert not tar_marker.exists()
+    assert list(scratch.iterdir()) == []

tests/test_self_update_download.py

@@ -91,6 +91,27 @@ def run(args, **kwargs):
     assert diagnostic["attempts"][0]["http_status"] == 0
 
 
+def test_installer_receives_remaining_outer_timeout_budget(monkeypatch):
+    calls = []
+
+    def run(args, **kwargs):
+        calls.append((args, kwargs))
+        if args[0] == "curl":
+            Path(args[args.index("--output") + 1]).write_text("exit 0")
+            return subprocess.CompletedProcess(args, 0, "200", "")
+        return subprocess.CompletedProcess(args, 0, "", "")
+
+    monkeypatch.setattr("loopx.self_update_download.time.monotonic", lambda: 0.0)
+    monkeypatch.setattr("loopx.self_update_download.subprocess.run", run)
+    result, diagnostic = run_archive_installer(
+        "https://example.invalid/", env={}, timeout_seconds=600
+    )
+
+    assert result.returncode == 0
+    assert diagnostic["stage"] == "installer_execution"
+    assert calls[-1][1]["env"]["LOOPX_INSTALLER_TIMEOUT_SECONDS"] == "600"
+
+
 @pytest.mark.parametrize("timeout", [False, True])
 def test_installer_failure_is_not_retried(monkeypatch, timeout):
     calls = []