full read: fix the set -e footguns a full run would have hit
Read the whole thing — 2392 lines of entry point and 2700 of libraries — looking for what shellcheck cannot see. shellcheck itself is clean at error level; its warnings are cross-file false positives and one deliberate tilde in a display string. Everything below is a real defect. ── The Git section aborted on any machine where git was not already configured ── `git config --global --get <key>` exits NON-ZERO when the key is simply unset, and `VAR="$(git_get …)"` propagates that under `set -e`. So on a fresh machine — the case this script exists for — the section died at its first assignment, before printing anything, and took the remaining nine sections with it. It passed every earlier test because those harnesses sourced the section under a `bash -c` with no `set -e`. Verified now against a genuinely fresh account with the real script: the section completes and writes a correct .gitconfig. ── An optional step failing aborted the whole run ── Twelve functions ended on a command that can fail — `systemctl enable --now earlyoom`, `systemctl restart systemd-logind`, `chsh`, `sysctl -w`, `chown -R`, the oh-my-zsh installer, and others. Called as plain commands under `set -e`, any one of them failing ends the script, so a masked unit or a container without systemd would abort a 28-section run over an optional improvement. They now return 0 explicitly and the callers verify the outcome instead — which also fixed a lie: the sleep section printed "sleep disabled, logind reloaded" whether or not the restart had worked. It now checks the targets and the logind values and reports honestly. ── chown user:user assumed the primary group is named after the user ── True on Debian and Ubuntu, which create a group per user. Not true for an account from LDAP, or made with `useradd -g users`, or on an image with a shared group — there `install -g <user>` fails with "invalid group" and the step aborts. Proved it against an account whose primary group is `oddgroup`: the old form fails, the new one gets ownership right. Eight call sites now ask `id -gn`. ── Also hardened ── agent_path and current_editor gained `|| true` for the same reason git_get needed it: "nothing is set" is an answer, not a failure. Verified afterwards: shellcheck clean at error level, every section runs standalone without aborting, and the two apparent failures in that sweep are correct behaviour — Timezone and Git refusing an empty answer from /dev/null. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -421,6 +421,16 @@ ask_officer_root() {
|
||||
OFFICER_ROOT="${answer%/}"
|
||||
}
|
||||
|
||||
# The account's PRIMARY GROUP, asked of the system rather than assumed to be
|
||||
# named after the user.
|
||||
#
|
||||
# Debian and Ubuntu create a group per user, so "pastilhas:pastilhas" is right on
|
||||
# most machines — but not on one where the account came from LDAP, or was made
|
||||
# with `useradd -g users`, or is a cloud image with a shared group. There
|
||||
# `chown user:user` fails with "invalid group" and `install -g user` refuses,
|
||||
# both of which abort the step.
|
||||
user_group() { id -gn "${1:-$USERNAME}" 2>/dev/null || echo "${1:-$USERNAME}"; }
|
||||
|
||||
# Run a block as the created user (login shell, inherits HOME)
|
||||
as_user() {
|
||||
sudo -u "$USERNAME" -i bash -c "$1"
|
||||
|
||||
@@ -42,7 +42,12 @@ MACHINE_SETUP_DEV_LOADED=1
|
||||
# Found because the writes failed silently: the section reported "written" while
|
||||
# nothing had been. Both wrappers now run in a subshell from /, which every
|
||||
# account can stat, and their exit status is checked by the caller.
|
||||
git_get() { (cd / && sudo -H -u "$USERNAME" git config --global --get "$1" 2>/dev/null); }
|
||||
# `git config --get` exits NON-ZERO when the key is simply unset, and
|
||||
# `VAR="$(git_get …)"` propagates that under `set -e`. So on a machine where git
|
||||
# has never been configured — the fresh machine this script exists for — reading
|
||||
# the current value aborted the run before the section had printed anything.
|
||||
# Missing a value is an answer here, not a failure.
|
||||
git_get() { (cd / && sudo -H -u "$USERNAME" git config --global --get "$1" 2>/dev/null) || true; }
|
||||
git_set() { (cd / && sudo -H -u "$USERNAME" git config --global "$1" "$2"); }
|
||||
|
||||
# Is there anything configured at all?
|
||||
@@ -89,6 +94,11 @@ install_oh_my_zsh() {
|
||||
# only ever called when there is none.
|
||||
sudo -H -u "$USERNAME" sh -c \
|
||||
"$(curl -fsSL https://raw.githubusercontent.com/ohmyzsh/ohmyzsh/master/tools/install.sh)" "" --unattended >/dev/null 2>&1
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# `chsh` is what actually changes the login shell. Asked separately from
|
||||
@@ -98,6 +108,11 @@ set_login_shell() {
|
||||
local shell="$1"
|
||||
grep -qxF "$shell" /etc/shells || echo "$shell" >>/etc/shells
|
||||
chsh -s "$shell" "$USERNAME"
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
@@ -162,7 +177,7 @@ nvim_install() {
|
||||
nvim_clone_config() {
|
||||
local repo="$1" dest="${USER_HOME}/.config/nvim"
|
||||
|
||||
install -d -m 0755 -o "$USERNAME" -g "$USERNAME" "${USER_HOME}/.config"
|
||||
install -d -m 0755 -o "$USERNAME" -g "$(user_group)" "${USER_HOME}/.config"
|
||||
(cd / && sudo -H -u "$USERNAME" git clone --depth 1 "$repo" "$dest" >/dev/null 2>&1) || return 1
|
||||
|
||||
# The starter is a template, not something to track. Left in place for a
|
||||
@@ -325,7 +340,7 @@ agent_path() {
|
||||
echo "$bin"
|
||||
return
|
||||
}
|
||||
command -v "$name" 2>/dev/null
|
||||
command -v "$name" 2>/dev/null || true
|
||||
}
|
||||
|
||||
agent_is_npm_install() { [[ "$(readlink -f "$(agent_path "$1")" 2>/dev/null)" == */node_modules/* ]]; }
|
||||
@@ -368,7 +383,9 @@ editor_candidates() {
|
||||
for e in nvim vim nano; do command -v "$e" &>/dev/null && echo "$e"; done
|
||||
}
|
||||
|
||||
current_editor() { (cd / && sudo -H -u "$USERNAME" bash -lc 'echo "${EDITOR:-}"' 2>/dev/null); }
|
||||
# `|| true` for the same reason git_get has it: "not set" is an answer, and an
|
||||
# assignment from a function that exits non-zero aborts the run under `set -e`.
|
||||
current_editor() { (cd / && sudo -H -u "$USERNAME" bash -lc 'echo "${EDITOR:-}"' 2>/dev/null) || true; }
|
||||
|
||||
set_system_editor() {
|
||||
local editor="$1" path
|
||||
|
||||
@@ -42,7 +42,7 @@ install_config() {
|
||||
local src="$1" dest="$2" owner="$3" answer
|
||||
|
||||
if [[ ! -f "$dest" ]]; then
|
||||
install -D -m 0644 -o "$owner" -g "$owner" "$src" "$dest"
|
||||
install -D -m 0644 -o "$owner" -g "$(user_group "$owner")" "$src" "$dest"
|
||||
return 0
|
||||
fi
|
||||
|
||||
@@ -76,7 +76,7 @@ install_config() {
|
||||
;;
|
||||
2)
|
||||
cp -a "$dest" "${dest}.before-machine-setup"
|
||||
install -D -m 0644 -o "$owner" -g "$owner" "$src" "$dest"
|
||||
install -D -m 0644 -o "$owner" -g "$(user_group "$owner")" "$src" "$dest"
|
||||
ok "replaced — yours is at ${dest}.before-machine-setup"
|
||||
return 0
|
||||
;;
|
||||
|
||||
@@ -172,6 +172,11 @@ network:
|
||||
dhcp-identifier: mac
|
||||
EOF
|
||||
chmod 600 "$NETPLAN_DHCP_ID"
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# Freeze the current lease into a static address.
|
||||
@@ -192,6 +197,11 @@ network:
|
||||
via: ${gateway}
|
||||
EOF
|
||||
chmod 600 "$NETPLAN_STATIC"
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
netplan_check() { netplan generate 2>&1; }
|
||||
|
||||
@@ -65,11 +65,16 @@ has_authorized_key() { (($(authorized_key_count) > 0)); }
|
||||
fix_ssh_permissions() {
|
||||
local dir
|
||||
dir="$(user_ssh_dir)"
|
||||
[[ -d "$dir" ]] || install -d -m 0700 -o "$USERNAME" -g "$USERNAME" "$dir"
|
||||
[[ -d "$dir" ]] || install -d -m 0700 -o "$USERNAME" -g "$(user_group)" "$dir"
|
||||
chmod 700 "$dir"
|
||||
[[ -f "$dir/authorized_keys" ]] && chmod 600 "$dir/authorized_keys"
|
||||
find "$dir" -maxdepth 1 -type f -name 'id_*' ! -name '*.pub' -exec chmod 600 {} +
|
||||
chown -R "$USERNAME:$USERNAME" "$dir"
|
||||
chown -R "${USERNAME}:$(user_group)" "$dir"
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# Add a public key, once. Appending blindly is how authorized_keys ends up with
|
||||
@@ -85,7 +90,7 @@ add_authorized_key() {
|
||||
return 1
|
||||
fi
|
||||
|
||||
install -d -m 0700 -o "$USERNAME" -g "$USERNAME" "$(user_ssh_dir)"
|
||||
install -d -m 0700 -o "$USERNAME" -g "$(user_group)" "$(user_ssh_dir)"
|
||||
touch "$file"
|
||||
|
||||
# Compare on the key body, not the whole line: the trailing comment differs
|
||||
@@ -106,7 +111,7 @@ generate_user_key() {
|
||||
local comment="$1" key
|
||||
key="$(user_ssh_dir)/id_ed25519"
|
||||
|
||||
install -d -m 0700 -o "$USERNAME" -g "$USERNAME" "$(user_ssh_dir)"
|
||||
install -d -m 0700 -o "$USERNAME" -g "$(user_group)" "$(user_ssh_dir)"
|
||||
sudo -u "$USERNAME" ssh-keygen -t ed25519 -C "$comment" -f "$key" -N "" >/dev/null
|
||||
add_authorized_key "$(cat "${key}.pub")"
|
||||
}
|
||||
|
||||
@@ -171,6 +171,11 @@ swap_create() {
|
||||
swappiness_set() {
|
||||
echo "vm.swappiness=$1" >/etc/sysctl.d/99-machine-setup-swappiness.conf
|
||||
sysctl -q -w "vm.swappiness=$1"
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
@@ -322,6 +327,11 @@ earlyoom_is_active() { systemctl is-active --quiet earlyoom 2>/dev/null; }
|
||||
earlyoom_install() {
|
||||
pkg_is_installed earlyoom || pkg_install_now earlyoom
|
||||
systemctl enable --now earlyoom >/dev/null 2>&1
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
@@ -353,6 +363,11 @@ fs.inotify.max_user_instances=${INOTIFY_INSTANCES}
|
||||
EOF
|
||||
sysctl -q -w "fs.inotify.max_user_watches=${INOTIFY_WATCHES}"
|
||||
sysctl -q -w "fs.inotify.max_user_instances=${INOTIFY_INSTANCES}"
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
@@ -436,6 +451,11 @@ disable_sleep() {
|
||||
# Only restart when something actually changed — a needless restart of logind
|
||||
# disturbs live sessions, and this step runs on every pass.
|
||||
systemctl restart systemd-logind
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
@@ -482,7 +502,12 @@ wait_online_boot_time() {
|
||||
systemd-analyze blame 2>/dev/null | awk -v u="$WAIT_ONLINE_UNIT" '$NF == u { $NF = ""; sub(/[[:space:]]+$/, ""); print; exit }'
|
||||
}
|
||||
|
||||
mask_wait_online() { systemctl mask --now "$WAIT_ONLINE_UNIT" >/dev/null 2>&1; }
|
||||
# Returns 0 whatever happens — see swappiness_set for why an optional step must
|
||||
# not be able to abort the run.
|
||||
mask_wait_online() {
|
||||
systemctl mask --now "$WAIT_ONLINE_UNIT" >/dev/null 2>&1
|
||||
return 0
|
||||
}
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# Timezone
|
||||
|
||||
@@ -246,6 +246,11 @@ net.ipv4.ip_forward = 1
|
||||
net.ipv6.conf.all.forwarding = 1
|
||||
EOF
|
||||
sysctl --system >/dev/null 2>&1
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# UDP GRO forwarding, which Tailscale documents as roughly doubling throughput on
|
||||
@@ -281,6 +286,11 @@ EOF
|
||||
|
||||
# And once now, for the interface that is already up.
|
||||
IFACE="$(default_iface)" bash "$TS_DISPATCHER" >/dev/null 2>&1 || true
|
||||
# Returns 0 whatever happens. This is an optional improvement, and a
|
||||
# function that ends on a failing command is fatal under `set -e` when it
|
||||
# is called as a plain command — which would abort the remaining sections
|
||||
# over something the run could simply report. The caller checks the outcome.
|
||||
return 0
|
||||
}
|
||||
|
||||
# The LAN this machine sits on, as a CIDR — the useful default for a subnet
|
||||
|
||||
@@ -764,7 +764,7 @@ if ! skip; then
|
||||
echo " already has ${ACTIVE_SWAP_GB}G of swap, leaving it alone"
|
||||
if [[ "$CURRENT_SWAPPINESS" != "$SWAPPINESS" ]] && confirm "Set swappiness to ${SWAPPINESS}?"; then
|
||||
swappiness_set "$SWAPPINESS"
|
||||
ok "swappiness set to ${SWAPPINESS}"
|
||||
ok "swappiness is now $(sysctl -n vm.swappiness)"
|
||||
SUMMARY+=("Swap: kept ${ACTIVE_SWAP_GB}G, swappiness ${SWAPPINESS}")
|
||||
else
|
||||
SUMMARY+=("Swap: kept ${ACTIVE_SWAP_GB}G")
|
||||
@@ -778,7 +778,7 @@ if ! skip; then
|
||||
if confirm "Proceed?"; then
|
||||
swap_create "$WANT_SWAP_GB"
|
||||
swappiness_set "$SWAPPINESS"
|
||||
ok "${WANT_SWAP_GB}G swap active, swappiness ${SWAPPINESS}"
|
||||
ok "$(swap_active_gb)G swap active, swappiness $(sysctl -n vm.swappiness)"
|
||||
SUMMARY+=("Swap: ${WANT_SWAP_GB}G created, swappiness ${SWAPPINESS}")
|
||||
else
|
||||
warn "skipped by request"
|
||||
@@ -1044,8 +1044,17 @@ elif ! skip; then
|
||||
echo " clean shutdown is 'sudo poweroff' rather than the button."
|
||||
if confirm "Proceed?"; then
|
||||
disable_sleep
|
||||
# Verified rather than asserted: disable_sleep returns 0 whatever happens,
|
||||
# so that a failed logind restart cannot abort the remaining sections. The
|
||||
# check is what turns that into an honest report.
|
||||
if sleep_targets_masked && logind_is_configured; then
|
||||
ok "sleep disabled, logind reloaded"
|
||||
SUMMARY+=("Sleep: disabled (targets masked, logind handlers ignored)")
|
||||
else
|
||||
warn "sleep settings were written but are not all in force — check: systemctl status systemd-logind"
|
||||
ERRORS+=("Sleep: settings written but not in force")
|
||||
SUMMARY+=("Sleep: written, NOT fully in force")
|
||||
fi
|
||||
else
|
||||
warn "skipped by request"
|
||||
SUMMARY+=("Sleep: SKIPPED by request")
|
||||
@@ -2141,7 +2150,7 @@ if ! skip; then
|
||||
# unguarded, so a re-run duplicated the lot.
|
||||
ZSHRC="${USER_HOME}/.zshrc"
|
||||
touch "$ZSHRC"
|
||||
chown "$USERNAME:$USERNAME" "$ZSHRC"
|
||||
chown "${USERNAME}:$(user_group)" "$ZSHRC"
|
||||
|
||||
if command -v starship &>/dev/null; then
|
||||
if append_once "$ZSHRC" starship <<'EOF'
|
||||
|
||||
Reference in New Issue
Block a user