mirror of
https://github.com/Routstr/routstrd.git
synced 2026-10-05 12:28:23 +00:00
fix(install): harden install.sh after review
Review follow-ups on the installer: - --print-asset no longer requires curl/wget or HOME: the HTTP client is resolved lazily and the install directory only after the early exit, so printing the asset name works on a machine with neither, which is the point of that flag. - Trap INT/TERM with `exit 1` instead of `cleanup` alone. A trapped signal previously resumed execution after the handler, which (with the temp dir already removed by the handler) surfaced as a misleading follow-on download error. The installer now reports the interrupt and exits. - Bound every HTTP transfer: 10s connect timeout, 60s for the API, 15min for the archive, with retries. A stalled connection previously hung the installer forever; the standalone updater already caps this at 30s. - Prefer the native arm64 build on Apple Silicon when a Rosetta shell reports x86_64, instead of installing the x64 build under emulation. - Validate the staged binary against the requested version before swapping it in, as installStandaloneRelease already does, and refuse the install on a mismatch instead of replacing a working install and warning afterwards. Skipped when --platform/--arch are overridden, where the binary may not be able to run on this machine at all. - Cap SHA256SUMS at 1MB, matching MAX_CHECKSUM_BYTES in the updater. - Use tr '[:upper:]' '[:lower:]' (shellcheck SC2018/SC2019) and document the --install-dir alias in --help. Tests cover each fix, and the four new failure-path tests fail against the previous script.
This commit is contained in:
@@ -44,7 +44,8 @@ curl -fsSL https://github.com/Routstr/routstrd/releases/latest/download/install.
|
||||
```
|
||||
|
||||
The installer downloads the release archive, verifies it against the release
|
||||
`SHA256SUMS`, and only replaces `routstrd` after the checksum matches.
|
||||
`SHA256SUMS`, and only replaces an existing `routstrd` once the checksum matches
|
||||
and the extracted binary reports the expected version.
|
||||
|
||||
<details>
|
||||
<summary>Manual install</summary>
|
||||
|
||||
+66
-23
@@ -21,7 +21,13 @@ PLATFORM="${ROUTSTRD_PLATFORM:-}"
|
||||
ARCH="${ROUTSTRD_ARCH:-}"
|
||||
INSTALL_DIR="${ROUTSTRD_INSTALL_DIR:-}"
|
||||
PRINT_ASSET=0
|
||||
PLATFORM_DETECTED=0
|
||||
ARCH_DETECTED=0
|
||||
MAX_ARCHIVE_BYTES=262144000
|
||||
MAX_CHECKSUMS_BYTES=1048576
|
||||
CONNECT_TIMEOUT_SECS=10
|
||||
API_TIMEOUT_SECS=60
|
||||
DOWNLOAD_TIMEOUT_SECS=900
|
||||
|
||||
say() { printf '%s\n' "$*" >&2; }
|
||||
die() { printf 'error: %s\n' "$*" >&2; exit 1; }
|
||||
@@ -35,6 +41,7 @@ Usage: sh install.sh [options]
|
||||
Options:
|
||||
--version <version> Install a specific version (default: latest)
|
||||
--dir <path> Install directory (default: $HOME/.local/bin)
|
||||
--install-dir <path> Alias for --dir
|
||||
--platform <platform> Override platform detection (linux|darwin)
|
||||
--arch <arch> Override architecture detection (x64|arm64)
|
||||
--repo <owner/name> GitHub repository (default: Routstr/routstrd)
|
||||
@@ -70,25 +77,29 @@ else
|
||||
DOWNLOAD_BASE_URL="https://github.com/${REPO}/releases/download"
|
||||
fi
|
||||
|
||||
if [ -z "$INSTALL_DIR" ]; then
|
||||
[ -n "${HOME:-}" ] || die "HOME is not set; pass --dir or set ROUTSTRD_INSTALL_DIR."
|
||||
INSTALL_DIR="$HOME/.local/bin"
|
||||
fi
|
||||
|
||||
if [ -z "$PLATFORM" ]; then
|
||||
case "$(uname -s)" in
|
||||
Linux) PLATFORM=linux ;;
|
||||
Darwin) PLATFORM=darwin ;;
|
||||
*) die "unsupported operating system '$(uname -s)'; releases cover Linux and macOS." ;;
|
||||
esac
|
||||
PLATFORM_DETECTED=1
|
||||
fi
|
||||
|
||||
if [ -z "$ARCH" ]; then
|
||||
case "$(uname -m)" in
|
||||
x86_64|amd64) ARCH=x64 ;;
|
||||
x86_64|amd64)
|
||||
ARCH=x64
|
||||
# Under Rosetta, uname reports x86_64 on arm64 Macs; prefer the native build.
|
||||
if [ "$PLATFORM" = darwin ] \
|
||||
&& [ "$(sysctl -n hw.optional.arm64 2>/dev/null)" = "1" ]; then
|
||||
ARCH=arm64
|
||||
fi
|
||||
;;
|
||||
arm64|aarch64) ARCH=arm64 ;;
|
||||
*) die "unsupported architecture '$(uname -m)'; releases cover x64 and arm64." ;;
|
||||
esac
|
||||
ARCH_DETECTED=1
|
||||
fi
|
||||
|
||||
case "$PLATFORM" in
|
||||
@@ -101,29 +112,43 @@ case "$ARCH" in
|
||||
*) die "unsupported architecture '$ARCH'; expected x64 or arm64." ;;
|
||||
esac
|
||||
|
||||
if command -v curl >/dev/null 2>&1; then
|
||||
# Resolved lazily by require_http_client, so --print-asset does not need either.
|
||||
HTTP_CLIENT=""
|
||||
require_http_client() {
|
||||
if [ -n "$HTTP_CLIENT" ]; then
|
||||
return 0
|
||||
fi
|
||||
if command -v curl >/dev/null 2>&1; then
|
||||
HTTP_CLIENT=curl
|
||||
elif command -v wget >/dev/null 2>&1; then
|
||||
elif command -v wget >/dev/null 2>&1; then
|
||||
HTTP_CLIENT=wget
|
||||
else
|
||||
else
|
||||
die "curl or wget is required to download routstrd."
|
||||
fi
|
||||
fi
|
||||
}
|
||||
|
||||
fetch_stdout() {
|
||||
require_http_client
|
||||
if [ "$HTTP_CLIENT" = curl ]; then
|
||||
curl -fsSL -H "Accept: application/vnd.github+json" \
|
||||
curl -fsSL --connect-timeout "$CONNECT_TIMEOUT_SECS" \
|
||||
--max-time "$API_TIMEOUT_SECS" --retry 3 \
|
||||
-H "Accept: application/vnd.github+json" \
|
||||
${GITHUB_TOKEN:+-H "Authorization: Bearer ${GITHUB_TOKEN}"} "$1"
|
||||
else
|
||||
wget -qO- --header="Accept: application/vnd.github+json" \
|
||||
wget -qO- --timeout="$API_TIMEOUT_SECS" --tries=3 \
|
||||
--header="Accept: application/vnd.github+json" \
|
||||
${GITHUB_TOKEN:+--header="Authorization: Bearer ${GITHUB_TOKEN}"} "$1"
|
||||
fi
|
||||
}
|
||||
|
||||
fetch_file() {
|
||||
require_http_client
|
||||
if [ "$HTTP_CLIENT" = curl ]; then
|
||||
curl -fsSL -o "$2" "$1"
|
||||
curl -fsSL --connect-timeout "$CONNECT_TIMEOUT_SECS" \
|
||||
--max-time "$DOWNLOAD_TIMEOUT_SECS" --retry 3 \
|
||||
-o "$2" "$1"
|
||||
else
|
||||
wget -qO "$2" "$1"
|
||||
wget -qO "$2" --timeout="$DOWNLOAD_TIMEOUT_SECS" --tries=3 "$1"
|
||||
fi
|
||||
}
|
||||
|
||||
@@ -155,6 +180,11 @@ if [ "$PRINT_ASSET" = 1 ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
if [ -z "$INSTALL_DIR" ]; then
|
||||
[ -n "${HOME:-}" ] || die "HOME is not set; pass --dir or set ROUTSTRD_INSTALL_DIR."
|
||||
INSTALL_DIR="$HOME/.local/bin"
|
||||
fi
|
||||
|
||||
if command -v sha256sum >/dev/null 2>&1; then
|
||||
SHA256_CMD="sha256sum"
|
||||
elif command -v shasum >/dev/null 2>&1; then
|
||||
@@ -167,10 +197,10 @@ fi
|
||||
|
||||
hash_file() {
|
||||
if [ "$SHA256_CMD" = openssl_sha256 ]; then
|
||||
openssl dgst -sha256 -r "$1" | awk '{print $1}' | tr 'A-Z' 'a-z'
|
||||
openssl dgst -sha256 -r "$1" | awk '{print $1}' | tr '[:upper:]' '[:lower:]'
|
||||
else
|
||||
# shellcheck disable=SC2086
|
||||
$SHA256_CMD "$1" | awk '{print $1}' | tr 'A-Z' 'a-z'
|
||||
$SHA256_CMD "$1" | awk '{print $1}' | tr '[:upper:]' '[:lower:]'
|
||||
fi
|
||||
}
|
||||
|
||||
@@ -181,7 +211,8 @@ cleanup() {
|
||||
if [ -n "$STAGED" ]; then rm -f "$STAGED" 2>/dev/null || true; fi
|
||||
if [ -n "$WORKDIR" ]; then rm -rf "$WORKDIR" 2>/dev/null || true; fi
|
||||
}
|
||||
trap cleanup EXIT INT TERM
|
||||
trap cleanup EXIT
|
||||
trap 'say "Interrupted."; exit 1' INT TERM
|
||||
|
||||
RELEASE_BASE_URL="${DOWNLOAD_BASE_URL}/v${VERSION}"
|
||||
ARCHIVE="${WORKDIR}/${ASSET}"
|
||||
@@ -200,10 +231,14 @@ archive_bytes="$(wc -c < "$ARCHIVE" | tr -d '[:space:]')"
|
||||
[ "$archive_bytes" -le "$MAX_ARCHIVE_BYTES" ] \
|
||||
|| die "${ASSET} is unexpectedly large (${archive_bytes} bytes)."
|
||||
|
||||
checksums_bytes="$(wc -c < "$CHECKSUMS" | tr -d '[:space:]')"
|
||||
[ "$checksums_bytes" -le "$MAX_CHECKSUMS_BYTES" ] \
|
||||
|| die "SHA256SUMS is unexpectedly large (${checksums_bytes} bytes)."
|
||||
|
||||
expected="$(grep -F "$ASSET" "$CHECKSUMS" 2>/dev/null \
|
||||
| awk -v name="$ASSET" '$2 == name || $2 == "*" name { print $1 }' \
|
||||
| head -n 1 \
|
||||
| tr 'A-Z' 'a-z')"
|
||||
| tr '[:upper:]' '[:lower:]')"
|
||||
[ -n "$expected" ] || die "SHA256SUMS does not contain ${ASSET}."
|
||||
|
||||
actual="$(hash_file "$ARCHIVE")"
|
||||
@@ -220,15 +255,23 @@ STAGED="${INSTALL_DIR}/.routstrd.tmp.$$"
|
||||
|
||||
cp "${WORKDIR}/routstrd" "$STAGED" || die "could not write to ${INSTALL_DIR}."
|
||||
chmod 755 "$STAGED"
|
||||
|
||||
# Validate the staged binary before replacing the target, mirroring
|
||||
# installStandaloneRelease: refuse a release whose binary reports the wrong
|
||||
# version rather than replacing a working install and warning about it.
|
||||
# Skipped for cross-installs (--platform/--arch overrides), where the binary may
|
||||
# not be able to run on this machine at all.
|
||||
if [ "$PLATFORM_DETECTED" = 1 ] && [ "$ARCH_DETECTED" = 1 ]; then
|
||||
staged_version="$("$STAGED" --version 2>/dev/null || true)"
|
||||
if [ "$staged_version" != "$VERSION" ] && [ "$staged_version" != "v$VERSION" ]; then
|
||||
die "staged routstrd reported version '${staged_version:-unknown}' instead of '${VERSION}'."
|
||||
fi
|
||||
fi
|
||||
|
||||
# Rename over the target so a running daemon keeps its old inode.
|
||||
mv -f "$STAGED" "$TARGET" || die "could not install to ${TARGET}."
|
||||
STAGED=""
|
||||
|
||||
installed_version="$("$TARGET" --version 2>/dev/null || true)"
|
||||
if [ "$installed_version" != "$VERSION" ]; then
|
||||
say "warning: ${TARGET} reported version '${installed_version:-unknown}' instead of '${VERSION}'."
|
||||
fi
|
||||
|
||||
say "Installed routstrd v${VERSION} to ${TARGET}"
|
||||
|
||||
case ":${PATH:-}:" in
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import { createHash } from "crypto";
|
||||
import { mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "fs";
|
||||
import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { releaseArchiveName } from "../src/utils/standalone-update";
|
||||
@@ -51,6 +51,8 @@ type FakeReleaseOptions = {
|
||||
archive?: Uint8Array;
|
||||
checksums?: string;
|
||||
omitAsset?: boolean;
|
||||
/** Delays the archive response so a test can signal while the download is in flight. */
|
||||
assetDelayMs?: number;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -62,7 +64,7 @@ function serveFakeRelease(options: FakeReleaseOptions = {}): string {
|
||||
const asset = options.asset ?? releaseArchiveName(version, "linux", "x64");
|
||||
const server = Bun.serve({
|
||||
port: 0,
|
||||
fetch(request) {
|
||||
async fetch(request) {
|
||||
const { pathname } = new URL(request.url);
|
||||
if (pathname === `/repos/${REPO}/releases/latest`) {
|
||||
return Response.json({
|
||||
@@ -75,6 +77,7 @@ function serveFakeRelease(options: FakeReleaseOptions = {}): string {
|
||||
}
|
||||
if (pathname === `/dl/v${version}/${asset}`) {
|
||||
if (options.omitAsset) return new Response("not found", { status: 404 });
|
||||
if (options.assetDelayMs) await Bun.sleep(options.assetDelayMs);
|
||||
return new Response(options.archive ?? new Uint8Array());
|
||||
}
|
||||
return new Response("not found", { status: 404 });
|
||||
@@ -308,4 +311,150 @@ describe("install.sh", () => {
|
||||
expect(statSync(installed).isFile()).toBe(true);
|
||||
expect(statSync(installed).mode & 0o111).not.toBe(0);
|
||||
});
|
||||
|
||||
test("prints the asset without curl, wget or HOME", () => {
|
||||
// --print-asset performs no install and, with --version, no network access,
|
||||
// so it must not require an HTTP client or an install directory. /bin/sh is
|
||||
// addressed absolutely because PATH is deliberately stripped.
|
||||
const result = Bun.spawnSync(
|
||||
[
|
||||
"/bin/sh",
|
||||
INSTALL_SCRIPT,
|
||||
"--print-asset",
|
||||
"--version",
|
||||
VERSION,
|
||||
"--platform",
|
||||
"linux",
|
||||
"--arch",
|
||||
"x64",
|
||||
],
|
||||
{ env: { PATH: "/nonexistent" }, stdout: "pipe", stderr: "pipe" },
|
||||
);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stdout.toString().trim()).toBe(releaseArchiveName(VERSION, "linux", "x64"));
|
||||
});
|
||||
|
||||
test("refuses a release whose binary reports the wrong version", async () => {
|
||||
const dir = tempDir("routstrd-install-wrong-version-");
|
||||
const installDir = join(dir, "bin");
|
||||
mkdirSync(installDir, { recursive: true });
|
||||
writeFileSync(join(installDir, "routstrd"), "#!/bin/sh\necho old\n");
|
||||
const asset = releaseArchiveName(VERSION, process.platform, process.arch);
|
||||
const archive = buildArchive(dir, "0.0.1");
|
||||
const origin = serveFakeRelease({
|
||||
asset,
|
||||
archive,
|
||||
checksums: `${sha256Hex(archive)} ${asset}\n`,
|
||||
});
|
||||
|
||||
const result = await runInstallerAsync([
|
||||
"--dir",
|
||||
installDir,
|
||||
"--api-base-url",
|
||||
origin,
|
||||
"--download-base-url",
|
||||
`${origin}/dl`,
|
||||
]);
|
||||
|
||||
expect(result.exitCode).not.toBe(0);
|
||||
expect(result.stderr).toContain("reported version");
|
||||
// The previous install must survive a rejected update.
|
||||
expect(readFileSync(join(installDir, "routstrd"), "utf8")).toContain("old");
|
||||
});
|
||||
|
||||
test("accepts a staged binary that reports a v-prefixed version", async () => {
|
||||
const dir = tempDir("routstrd-install-vprefixed-binary-");
|
||||
const installDir = join(dir, "bin");
|
||||
const asset = releaseArchiveName(VERSION, process.platform, process.arch);
|
||||
const archive = buildArchive(dir, `v${VERSION}`);
|
||||
const origin = serveFakeRelease({
|
||||
asset,
|
||||
archive,
|
||||
checksums: `${sha256Hex(archive)} ${asset}\n`,
|
||||
});
|
||||
|
||||
const result = await runInstallerAsync([
|
||||
"--dir",
|
||||
installDir,
|
||||
"--api-base-url",
|
||||
origin,
|
||||
"--download-base-url",
|
||||
`${origin}/dl`,
|
||||
]);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(statSync(join(installDir, "routstrd")).isFile()).toBe(true);
|
||||
});
|
||||
|
||||
test("rejects an oversized SHA256SUMS", async () => {
|
||||
const dir = tempDir("routstrd-install-big-sums-");
|
||||
const asset = releaseArchiveName(VERSION, process.platform, process.arch);
|
||||
const archive = buildArchive(dir, VERSION);
|
||||
const origin = serveFakeRelease({
|
||||
asset,
|
||||
archive,
|
||||
checksums: `${sha256Hex(archive)} ${asset}\n${"a".repeat(2 * 1024 * 1024)}\n`,
|
||||
});
|
||||
|
||||
const result = await runInstallerAsync([
|
||||
"--dir",
|
||||
join(dir, "bin"),
|
||||
"--api-base-url",
|
||||
origin,
|
||||
"--download-base-url",
|
||||
`${origin}/dl`,
|
||||
]);
|
||||
|
||||
expect(result.exitCode).not.toBe(0);
|
||||
expect(result.stderr).toContain("SHA256SUMS is unexpectedly large");
|
||||
});
|
||||
|
||||
test(
|
||||
"exits without installing when interrupted mid-download",
|
||||
async () => {
|
||||
const dir = tempDir("routstrd-install-interrupt-");
|
||||
const installDir = join(dir, "bin");
|
||||
const tmpParent = join(dir, "tmp");
|
||||
mkdirSync(tmpParent, { recursive: true });
|
||||
const asset = releaseArchiveName(VERSION, process.platform, process.arch);
|
||||
const archive = buildArchive(dir, VERSION);
|
||||
const origin = serveFakeRelease({
|
||||
asset,
|
||||
archive,
|
||||
checksums: `${sha256Hex(archive)} ${asset}\n`,
|
||||
assetDelayMs: 2000,
|
||||
});
|
||||
|
||||
const proc = Bun.spawn(
|
||||
[
|
||||
"sh",
|
||||
INSTALL_SCRIPT,
|
||||
"--dir",
|
||||
installDir,
|
||||
"--api-base-url",
|
||||
origin,
|
||||
"--download-base-url",
|
||||
`${origin}/dl`,
|
||||
],
|
||||
{ env: { ...process.env, TMPDIR: tmpParent }, stdout: "pipe", stderr: "pipe" },
|
||||
);
|
||||
|
||||
// Signal while the archive download is still in flight. The installer must
|
||||
// report the interrupt, exit non-zero and clean up after itself, rather
|
||||
// than resuming into a misleading follow-on failure (or an install).
|
||||
await Bun.sleep(300);
|
||||
proc.kill();
|
||||
|
||||
const [exitCode, stderr] = await Promise.all([
|
||||
Promise.race([proc.exited, Bun.sleep(8000).then(() => -1)]),
|
||||
new Response(proc.stderr).text(),
|
||||
]);
|
||||
expect(exitCode).toBe(1);
|
||||
expect(stderr).toContain("Interrupted.");
|
||||
expect(readdirSync(tmpParent)).toEqual([]);
|
||||
expect(() => statSync(join(installDir, "routstrd"))).toThrow();
|
||||
},
|
||||
15000,
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user