From 588ab3a5bf35a1d634d92ae56e7ed930389f5f83 Mon Sep 17 00:00:00 2001 From: Jon Hadfield Date: Sat, 5 Sep 2026 02:00:36 +0100 Subject: [PATCH 1/3] make the install script refuse what it cannot verify. From the review of the merged change, and the first of these was a fault in the line the README tells people to run. CERTREADER_INSTALL_DIR pointing somewhere that does not exist yet, which is the ordinary state of ~/.local/bin, failed. Worse, it failed saying the directory was not writable and to pick one that was, which is not what was wrong with it. The directory is created now, with sudo only if it cannot be created otherwise. The earlier test of this passed because it ran mkdir -p first, so it tested a case the README does not describe. Without sha256sum or shasum the download went unchecked and was installed anyway, on a shrug and a message to stderr. A script that people are told to pipe into a shell does not get to skip the one step that makes that defensible. It stops instead. Which then went wrong in its own way: the failure was raised inside a subshell, so what reached the user was the mismatch message from the caller rather than the missing tool. The command is settled before anything is downloaded now, and the three ways verification can fail say which one happened: no tool to check with, no entry in the sums file for this archive, or a file that does not match the entry. None of them install anything. Matching the archive in the sums file is by whole field rather than by grep, where the dots in a filename are a pattern that could match a longer name. The note about brew now spells macOS the way apple does, being a line somebody reads. The comments keep the lowercase this repo writes them in. Tested on amd64 and arm64: the README's own command with no directory there, no checksum tool, a sums file with no matching entry, and a tampered archive served through GITHUB_URL. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WcPAJjNzG6bqy2FKqKKY1v --- README.md | 5 +++-- install | 43 ++++++++++++++++++++++++++++++++----------- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 58cc516..85585ee 100644 --- a/README.md +++ b/README.md @@ -593,8 +593,9 @@ curl -sL https://raw.githubusercontent.com/jonhadfield/certreader/main/install | ``` This works out the latest release, downloads the archive for the machine it is run on, checks it -against the sums published beside it, and installs to `/usr/local/bin`, asking `sudo` only if that -directory is not already writable. It reads three optional variables: +against the sums published beside it, and installs to `/usr/local/bin`. The directory is created if +it is not there, and `sudo` is used only if it cannot be written to otherwise. A download that does +not match its checksum is refused, and nothing is installed. It reads three optional variables: | variable | | | --- | --- | diff --git a/install b/install index 35cd3ec..4f73aa2 100755 --- a/install +++ b/install @@ -44,16 +44,15 @@ resolve_version() { sed 's#.*/tag/##' } -# checksum verifies the download against the sums published beside it. The two -# commands take the same arguments and only one of them is usually present. -checksum() { +# checksum_command is settled before anything is downloaded, so that not having +# one is reported as that rather than reaching the verification and being +# mistaken there for a download that does not match. The two commands take the +# same arguments and only one of them is usually present. +checksum_command() { if command -v sha256sum >/dev/null 2>&1; then - sha256sum -c "$1" + echo "sha256sum -c" elif command -v shasum >/dev/null 2>&1; then - shasum -a 256 -c "$1" - else - echo "neither sha256sum nor shasum found, skipping checksum" >&2 - return 0 + echo "shasum -a 256 -c" fi } @@ -67,9 +66,13 @@ esac [ -n "${arch}" ] || fail "no build for $(uname -m); see ${github_url}/${owner}/${repo}/releases" if [ "${os}" = "darwin" ]; then - echo "note: on macos, brew install ${owner}/${repo}/${repo} is the supported route" >&2 + echo "note: on macOS, brew install ${owner}/${repo}/${repo} is the supported route" >&2 fi +checksum="$(checksum_command)" +[ -n "${checksum}" ] || + fail "neither sha256sum nor shasum is available to check the download with" + version="$(resolve_version)" [ -n "${version}" ] || fail "could not work out the latest version" # the archives carry the version without its leading v @@ -94,12 +97,30 @@ curl -fsSL -o "${work}/${archive}" "${base}/${archive}" || echo "[2/4] Verify against ${sums}" curl -fsSL -o "${work}/${sums}" "${base}/${sums}" || fail "could not download ${sums}" # the sums file names every archive of that platform, and -c fails on a line it -# has no file for, so check the one that was downloaded -(cd "${work}" && grep " ${archive}\$" "${sums}" > wanted && checksum wanted) || +# has no file for, so pick out the line for the one that was downloaded. awk +# compares the whole field, where a grep would read the dots in the name as a +# pattern and could match a longer one +awk -v want="${archive}" '$2 == want' "${work}/${sums}" > "${work}/wanted" +[ -s "${work}/wanted" ] || fail "${sums} has no entry for ${archive}" +# unquoted, because the command is two or four words +# shellcheck disable=SC2086 +(cd "${work}" && ${checksum} wanted) || fail "${archive} does not match its published checksum" echo "[3/4] Install ${repo} to ${install_dir}" tar -xzf "${work}/${archive}" -C "${work}" "${repo}" || fail "could not extract ${repo}" + +# a directory that is not there is the ordinary case for something like +# ~/.local/bin, and is not the same as one that cannot be written to +if [ ! -d "${install_dir}" ]; then + echo "${install_dir} does not exist, creating it" + if ! mkdir -p "${install_dir}" 2>/dev/null; then + command -v sudo >/dev/null 2>&1 || + fail "${install_dir} does not exist and sudo is not available to create it" + sudo mkdir -p "${install_dir}" || fail "could not create ${install_dir}" + fi +fi + if [ -w "${install_dir}" ]; then install -m 0755 "${work}/${repo}" "${install_dir}/${repo}" || fail "could not write ${install_dir}/${repo}" From 0512b3a4ec0bb46178a1b8c83937703c20781ebc Mon Sep 17 00:00:00 2001 From: Jon Hadfield Date: Sat, 5 Sep 2026 02:04:56 +0100 Subject: [PATCH 2/3] say what to do about a directory that cannot be created. The other install directory failures name the variable to set and this one did not, so it told somebody what was wrong without telling them what to do about it. The comment on checksum_command now says why finding neither tool returns zero rather than tripping set -e at the assignment, which is the other thing the review raised. An if whose conditions are all false and which has no else exits zero, so the empty answer reaches the caller's check, which is what reports it. Confirmed in sh, bash and dash, and by running the script with both tools hidden: it prints the missing tool message and exits 1. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WcPAJjNzG6bqy2FKqKKY1v --- install | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/install b/install index 4f73aa2..12f65d1 100755 --- a/install +++ b/install @@ -48,6 +48,10 @@ resolve_version() { # one is reported as that rather than reaching the verification and being # mistaken there for a download that does not match. The two commands take the # same arguments and only one of them is usually present. +# +# Finding neither returns zero and prints nothing, rather than tripping set -e +# at the assignment: an if whose conditions are all false and which has no else +# is defined to exit zero. The caller checks for the empty answer. checksum_command() { if command -v sha256sum >/dev/null 2>&1; then echo "sha256sum -c" @@ -116,7 +120,7 @@ if [ ! -d "${install_dir}" ]; then echo "${install_dir} does not exist, creating it" if ! mkdir -p "${install_dir}" 2>/dev/null; then command -v sudo >/dev/null 2>&1 || - fail "${install_dir} does not exist and sudo is not available to create it" + fail "${install_dir} does not exist and sudo is not available to create it; set CERTREADER_INSTALL_DIR to a directory you can create" sudo mkdir -p "${install_dir}" || fail "could not create ${install_dir}" fi fi From e1fdd7f38db2b08d307fb4f8594ab501bb62fc72 Mon Sep 17 00:00:00 2001 From: Jon Hadfield Date: Sat, 5 Sep 2026 02:07:22 +0100 Subject: [PATCH 3/3] return zero from checksum_command outright. The behaviour is unchanged: an if whose conditions are all false and which has no else already exits zero, so the empty answer reached the caller's check and the missing tool was reported there. Two passes of review read the function as though it could end the script at the assignment instead, which is reason enough to stop asking a reader to know the rule. The return states it. Still prints the missing tool message and exits 1 with both tools hidden, and installs as before with them present. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WcPAJjNzG6bqy2FKqKKY1v --- install | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/install b/install index 12f65d1..ec2404a 100755 --- a/install +++ b/install @@ -47,17 +47,18 @@ resolve_version() { # checksum_command is settled before anything is downloaded, so that not having # one is reported as that rather than reaching the verification and being # mistaken there for a download that does not match. The two commands take the -# same arguments and only one of them is usually present. -# -# Finding neither returns zero and prints nothing, rather than tripping set -e -# at the assignment: an if whose conditions are all false and which has no else -# is defined to exit zero. The caller checks for the empty answer. +# same arguments and only one of them is usually present. Finding neither prints +# nothing and returns zero, so the caller's check on the empty answer is what +# reports it, rather than set -e ending the script at the assignment without a +# word. The return says so outright, an if with no else being defined to exit +# zero anyway but not obviously. checksum_command() { if command -v sha256sum >/dev/null 2>&1; then echo "sha256sum -c" elif command -v shasum >/dev/null 2>&1; then echo "shasum -a 256 -c" fi + return 0 } os="$(get_os)"