From f663ad3d5e374d7123b9a24764e13b75345860aa Mon Sep 17 00:00:00 2001 From: Jensen Bernard Date: Fri, 27 Mar 2026 14:45:00 +0100 Subject: [PATCH] fix: harden scripts against injection, race conditions, and silent failures Full security audit and remediation across 14 files: replace unsafe eval with safe arg parser in audit.sh, add module name validation, fix TOCTOU race in dotfile symlinks, quote SSH config variables, add temp file cleanup traps, surface masked errors in brew/mise, pin bun version, add SSH key strength warnings, enable macOS firewall and screen lock defaults. Co-Authored-By: Claude Opus 4.6 --- VERSION | 2 +- install.sh | 8 ++++++++ lib/audit.sh | 24 +++++++++++++++++++++--- lib/profile.sh | 8 +++++++- lib/state.sh | 27 ++++++++++++++++++++------- modules/01-xcode.sh | 4 +++- modules/02-homebrew.sh | 6 ++++-- modules/03-shell.sh | 5 +++-- modules/04-mise.sh | 6 ++++-- modules/05-dotfiles.sh | 15 ++++++++++----- modules/07-ssh.sh | 22 ++++++++++++++++++++-- modules/10-ai-tools.sh | 2 +- modules/11-macos-defaults.sh | 7 +++++++ profiles/devizer-full.conf | 2 +- profiles/personal.conf | 2 +- 15 files changed, 111 insertions(+), 29 deletions(-) diff --git a/VERSION b/VERSION index 21e8796..ee90284 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.0.3 +1.0.4 diff --git a/install.sh b/install.sh index af53631..3d24612 100755 --- a/install.sh +++ b/install.sh @@ -1,6 +1,14 @@ #!/usr/bin/env bash # install.sh — Bootstrap mbp on a fresh Mac # Usage: bash <(curl -fsSL https://raw.githubusercontent.com/devizerio/mbp/main/install.sh) +# +# Security note: This script is fetched over HTTPS from GitHub. It trusts: +# - GitHub's TLS infrastructure for transport security +# - The devizerio/mbp repository for code integrity +# For additional verification, download first and inspect: +# curl -fsSL https://raw.githubusercontent.com/devizerio/mbp/main/install.sh -o install.sh +# shasum -a 256 install.sh # compare with published hash +# bash install.sh set -euo pipefail MBP_REPO="${MBP_REPO:-$HOME/.mbp/repo}" diff --git a/lib/audit.sh b/lib/audit.sh index 22c837e..1d73ffc 100644 --- a/lib/audit.sh +++ b/lib/audit.sh @@ -158,9 +158,27 @@ audit_macos_defaults() { # Parse apply_default lines: apply_default "domain" "key" "type" "value" while IFS= read -r line; do local domain key type expected_value - # Extract quoted args using parameter expansion - eval "set -- $(echo "$line" | sed 's/apply_default //')" 2>/dev/null || continue - domain="$1" key="$2" type="$3" expected_value="$4" + # Safely extract quoted args without eval + local args_str + args_str=$(echo "$line" | sed 's/^[[:space:]]*apply_default[[:space:]]*//') + # Parse space-separated quoted arguments safely using read + local -a args=() + while [[ -n "$args_str" ]]; do + args_str="${args_str#"${args_str%%[![:space:]]*}"}" # trim leading space + [ -z "$args_str" ] && break + if [[ "$args_str" == \"* ]]; then + # Quoted argument — extract up to closing quote + args_str="${args_str#\"}" + args+=("${args_str%%\"*}") + args_str="${args_str#*\"}" + else + # Unquoted argument + args+=("${args_str%% *}") + args_str="${args_str#* }" + [[ "$args_str" == "${args[${#args[@]}-1]}" ]] && args_str="" + fi + done + domain="${args[0]:-}" key="${args[1]:-}" type="${args[2]:-}" expected_value="${args[3]:-}" [ -z "$domain" ] || [ -z "$key" ] && continue diff --git a/lib/profile.sh b/lib/profile.sh index b22345d..96f44fb 100644 --- a/lib/profile.sh +++ b/lib/profile.sh @@ -34,9 +34,15 @@ profile_load() { fi ;; modules) - # Split on commas, trim each value + # Split on commas, trim each value, validate names (alphanumeric + hyphens only) MBP_PROFILE_MODULES=$(echo "$raw_val" | tr ',' '\n' | \ sed 's/^[[:space:]]*//;s/[[:space:]]*$//' | tr '\n' ' ' | sed 's/ $//') + for _mod in $MBP_PROFILE_MODULES; do + if ! echo "$_mod" | grep -qE '^[a-zA-Z0-9_-]+$'; then + printf "profile: invalid module name '%s' — only alphanumeric, hyphens, underscores allowed\n" "$_mod" >&2 + return 1 + fi + done ;; brewfiles) MBP_PROFILE_BREWFILES=$(echo "$raw_val" | tr ',' '\n' | \ diff --git a/lib/state.sh b/lib/state.sh index a265ce7..263b411 100644 --- a/lib/state.sh +++ b/lib/state.sh @@ -9,6 +9,19 @@ MBP_STATE_TXT="${MBP_STATE_DIR}/state.txt" MBP_STATE_SCHEMA_VERSION=1 mkdir -p "$MBP_STATE_DIR" +chmod 700 "$MBP_STATE_DIR" + +# Track temp files for cleanup on exit +_MBP_STATE_TMPFILES=() +_mbp_state_cleanup() { rm -f "${_MBP_STATE_TMPFILES[@]}" 2>/dev/null; } +trap _mbp_state_cleanup EXIT + +# Safe mktemp that registers for cleanup +_mbp_mktemp() { + local f; f=$(mktemp) + _MBP_STATE_TMPFILES+=("$f") + echo "$f" +} # === Plain-text state (modules 01-02) === # Format: module=status:exit_code:timestamp @@ -93,7 +106,7 @@ state_migrate_from_txt() { extra=$(jq -n --argjson count "$pkg_count" '{"packages": $count}') fi - local tmp; tmp=$(mktemp) + local tmp; tmp=$(_mbp_mktemp) jq --arg module "$module" \ --arg status "$status" \ --argjson exit_code "${exit_code:-0}" \ @@ -113,7 +126,7 @@ state_set_module_ok() { command -v jq >/dev/null 2>&1 || return 1 [ -f "$MBP_STATE_JSON" ] || state_init_json local ts; ts=$(date -u +%Y-%m-%dT%H:%M:%SZ) - local tmp; tmp=$(mktemp) + local tmp; tmp=$(_mbp_mktemp) jq --arg module "$module" --arg ts "$ts" \ '.modules[$module] = (.modules[$module] // {} | . + {status: "ok", exit_code: 0, ran_at: $ts})' \ "$MBP_STATE_JSON" > "$tmp" && mv "$tmp" "$MBP_STATE_JSON" @@ -124,7 +137,7 @@ state_set_module_error() { command -v jq >/dev/null 2>&1 || return 1 [ -f "$MBP_STATE_JSON" ] || state_init_json local ts; ts=$(date -u +%Y-%m-%dT%H:%M:%SZ) - local tmp; tmp=$(mktemp) + local tmp; tmp=$(_mbp_mktemp) jq --arg module "$module" --arg ts "$ts" \ --argjson exit_code "$exit_code" --arg error "$error_msg" \ '.modules[$module] = (.modules[$module] // {} | . + {status: "error", exit_code: $exit_code, ran_at: $ts, error: $error})' \ @@ -135,7 +148,7 @@ state_set_module_meta() { local module="$1" key="$2" value="$3" command -v jq >/dev/null 2>&1 || return 1 [ -f "$MBP_STATE_JSON" ] || return 1 - local tmp; tmp=$(mktemp) + local tmp; tmp=$(_mbp_mktemp) # value may be a JSON fragment (array, number) or a plain string if echo "$value" | jq . >/dev/null 2>&1; then jq --arg module "$module" --arg key "$key" --argjson val "$value" \ @@ -165,7 +178,7 @@ state_set_profile() { local profile="$1" command -v jq >/dev/null 2>&1 || return 1 [ -f "$MBP_STATE_JSON" ] || state_init_json "$profile" - local tmp; tmp=$(mktemp) + local tmp; tmp=$(_mbp_mktemp) jq --arg profile "$profile" '.profile = $profile' "$MBP_STATE_JSON" > "$tmp" && mv "$tmp" "$MBP_STATE_JSON" } @@ -173,7 +186,7 @@ state_set_last_run() { command -v jq >/dev/null 2>&1 || return 1 [ -f "$MBP_STATE_JSON" ] || return 1 local ts; ts=$(date -u +%Y-%m-%dT%H:%M:%SZ) - local tmp; tmp=$(mktemp) + local tmp; tmp=$(_mbp_mktemp) jq --arg ts "$ts" '.last_run = $ts' "$MBP_STATE_JSON" > "$tmp" && mv "$tmp" "$MBP_STATE_JSON" } @@ -207,7 +220,7 @@ state_check_schema() { printf "state: schema v%s detected, expected v%s — migrating...\n" \ "$current_schema" "$MBP_STATE_SCHEMA_VERSION" # Future migration functions go here - local tmp; tmp=$(mktemp) + local tmp; tmp=$(_mbp_mktemp) jq --argjson v "$MBP_STATE_SCHEMA_VERSION" '.schema_version = $v' \ "$MBP_STATE_JSON" > "$tmp" && mv "$tmp" "$MBP_STATE_JSON" fi diff --git a/modules/01-xcode.sh b/modules/01-xcode.sh index fd82801..03d985f 100755 --- a/modules/01-xcode.sh +++ b/modules/01-xcode.sh @@ -30,7 +30,9 @@ until xcode-select -p >/dev/null 2>&1; do done # Accept license -sudo xcodebuild -license accept 2>/dev/null || true +if ! sudo xcodebuild -license accept 2>/dev/null; then + mbp_log_warn "Could not auto-accept Xcode license — you may need to run: sudo xcodebuild -license accept" +fi mbp_log_ok "Xcode Command Line Tools installed: $(xcode-select -p)" state_txt_set "xcode" "ok" "0" diff --git a/modules/02-homebrew.sh b/modules/02-homebrew.sh index 8bd2730..632a7d4 100755 --- a/modules/02-homebrew.sh +++ b/modules/02-homebrew.sh @@ -37,8 +37,10 @@ for bf in $BREWFILES; do BFPATH="$BREWFILE_DIR/Brewfile.$bf" if [ -f "$BFPATH" ]; then mbp_log_step "Bundling: Brewfile.$bf" - brew bundle --file="$BFPATH" --no-upgrade 2>&1 | \ - grep -v "^Using " | grep -v "^Homebrew Bundle complete" || true + if ! brew bundle --file="$BFPATH" --no-upgrade 2>&1 | \ + grep -v "^Using " | grep -v "^Homebrew Bundle complete"; then + mbp_log_warn "brew bundle returned non-zero for Brewfile.$bf — some packages may have failed" + fi else mbp_log_warn "Brewfile.$bf not found, skipping" fi diff --git a/modules/03-shell.sh b/modules/03-shell.sh index 8ec5551..d1c4bd2 100755 --- a/modules/03-shell.sh +++ b/modules/03-shell.sh @@ -21,9 +21,10 @@ fi # Ensure Homebrew zsh is in /etc/shells ZSH_BIN="$(brew --prefix 2>/dev/null)/bin/zsh" if [ -f "$ZSH_BIN" ]; then - if ! grep -qF "$ZSH_BIN" /etc/shells 2>/dev/null; then + # Validate the path contains no newlines or special characters before writing to /etc/shells + if [[ "$ZSH_BIN" =~ ^[a-zA-Z0-9/_.-]+$ ]] && ! grep -qF "$ZSH_BIN" /etc/shells 2>/dev/null; then mbp_log_step "Adding Homebrew zsh to /etc/shells..." - echo "$ZSH_BIN" | sudo tee -a /etc/shells >/dev/null + printf '%s\n' "$ZSH_BIN" | sudo tee -a /etc/shells >/dev/null fi # Change default shell if needed diff --git a/modules/04-mise.sh b/modules/04-mise.sh index 019f63f..41a12ae 100755 --- a/modules/04-mise.sh +++ b/modules/04-mise.sh @@ -34,9 +34,11 @@ for tool_spec in $MISE_TOOLS; do tool_name="${tool_spec%%@*}" tool_version="${tool_spec#*@}" mbp_log_step "Ensuring ${tool_name}@${tool_version}..." - mise install "${tool_spec}" 2>&1 | grep -v "^mise " | tail -3 | while IFS= read -r line; do + if ! mise install "${tool_spec}" 2>&1 | grep -v "^mise " | tail -3 | while IFS= read -r line; do [ -n "$line" ] && mbp_log_step "$line" - done + done; then + mbp_log_warn "mise install ${tool_spec} may have failed" + fi mise use --global "${tool_spec}" 2>/dev/null || true done diff --git a/modules/05-dotfiles.sh b/modules/05-dotfiles.sh index fde51ca..705e839 100755 --- a/modules/05-dotfiles.sh +++ b/modules/05-dotfiles.sh @@ -34,17 +34,22 @@ link_dotfile() { return fi - # Backup existing non-symlink file + # Ensure parent directory exists + mkdir -p "$(dirname "$dst")" + + # Backup existing regular file atomically (copy then link in one step) if [ -f "$dst" ] && [ ! -L "$dst" ]; then mkdir -p "$BACKUP_DIR" cp "$dst" "$BACKUP_DIR/$(basename "$dst")" mbp_log_step "backed up: ~/${dst_name} → $BACKUP_DIR" fi - # Ensure parent directory exists - mkdir -p "$(dirname "$dst")" - - ln -sf "$src" "$dst" + # Atomic symlink: create temp link then rename over destination + local tmp_link + tmp_link=$(mktemp "${dst}.mbp.XXXXXX") + rm -f "$tmp_link" + ln -s "$src" "$tmp_link" + mv -f "$tmp_link" "$dst" mbp_log_ok "linked: ~/${dst_name}" LINKED=$((LINKED + 1)) } diff --git a/modules/07-ssh.sh b/modules/07-ssh.sh index 442e298..02f2d75 100755 --- a/modules/07-ssh.sh +++ b/modules/07-ssh.sh @@ -19,7 +19,9 @@ for key in "$SSH_DIR"/*; do [[ "$(basename "$key")" == "config"* ]] && continue [[ "$(basename "$key")" == "environment" ]] && continue - chmod 600 "$key" + if ! chmod 600 "$key"; then + mbp_log_warn "failed to set permissions on $(basename "$key")" + fi KEY_COUNT=$((KEY_COUNT + 1)) mbp_log_step "permissions 600: $(basename "$key")" @@ -36,6 +38,22 @@ for key in "$SSH_DIR"/*; do esac done +# Warn about weak key types +if [ -n "$GITHUB_KEY" ] && [ -f "${GITHUB_KEY}.pub" ]; then + KEY_TYPE=$(ssh-keygen -l -f "${GITHUB_KEY}.pub" 2>/dev/null | awk '{print $4}' | tr -d '()') + KEY_BITS=$(ssh-keygen -l -f "${GITHUB_KEY}.pub" 2>/dev/null | awk '{print $1}') + case "$KEY_TYPE" in + DSA) + mbp_log_warn "SSH key $(basename "$GITHUB_KEY") uses DSA which is deprecated — generate an ed25519 key" + ;; + RSA) + if [ "${KEY_BITS:-0}" -lt 3072 ]; then + mbp_log_warn "SSH key $(basename "$GITHUB_KEY") is RSA ${KEY_BITS}-bit — consider ed25519 or RSA 4096" + fi + ;; + esac +fi + # Ensure config.d directory for per-client/per-project includes mkdir -p "${SSH_DIR}/config.d" @@ -47,7 +65,7 @@ if [ -n "$GITHUB_KEY" ]; then Host github.com HostName github.com User git - IdentityFile ${GITHUB_KEY} + IdentityFile "${GITHUB_KEY}" EOF mbp_log_ok "GitHub SSH: using $(basename "$GITHUB_KEY")" elif [ ! -f "$GITHUB_CONF" ]; then diff --git a/modules/10-ai-tools.sh b/modules/10-ai-tools.sh index 1e32a1f..9333945 100755 --- a/modules/10-ai-tools.sh +++ b/modules/10-ai-tools.sh @@ -28,7 +28,7 @@ else else mbp_log_warn "npm not found — install node (module 04) and re-run" state_set_module_error "ai-tools" "1" "npm not available for Claude Code install" - exit 1 + return 1 fi fi diff --git a/modules/11-macos-defaults.sh b/modules/11-macos-defaults.sh index f6a3f9f..d01f98d 100755 --- a/modules/11-macos-defaults.sh +++ b/modules/11-macos-defaults.sh @@ -74,6 +74,13 @@ mbp_log_step "Activity Monitor..." apply_default "com.apple.ActivityMonitor" "OpenMainWindow" "bool" "true" apply_default "com.apple.ActivityMonitor" "ShowCategory" "int" "0" +mbp_log_step "Security..." +# Enable macOS firewall +sudo /usr/libexec/ApplicationFirewall/socketfilterfw --setglobalstate on 2>/dev/null || true +# Require password immediately after sleep/screen saver +apply_default "com.apple.screensaver" "askForPassword" "int" "1" +apply_default "com.apple.screensaver" "askForPasswordDelay" "int" "0" + # Restart affected system processes mbp_log_step "Restarting system services..." killall Dock 2>/dev/null || true diff --git a/profiles/devizer-full.conf b/profiles/devizer-full.conf index c6760a4..3e3a5df 100644 --- a/profiles/devizer-full.conf +++ b/profiles/devizer-full.conf @@ -5,4 +5,4 @@ format = 1 modules = xcode,homebrew,shell,mise,dotfiles,git,ssh,secrets,docker,ai-tools,macos-defaults,apps,dev-dirs brewfiles = core,dev,ai,apps,personal -mise_tools = node@22,ruby@3.3,python@3.12,bun@latest +mise_tools = node@22,ruby@3.3,python@3.12,bun@1.2 diff --git a/profiles/personal.conf b/profiles/personal.conf index 076d6a8..977a547 100644 --- a/profiles/personal.conf +++ b/profiles/personal.conf @@ -4,4 +4,4 @@ format = 1 modules = xcode,homebrew,shell,mise,dotfiles,git,ssh,secrets,docker,ai-tools,macos-defaults,apps,dev-dirs brewfiles = core,dev,ai,apps,personal -mise_tools = node@22,ruby@3.3,python@3.12,bun@latest +mise_tools = node@22,ruby@3.3,python@3.12,bun@1.2