From 0a65b45ab145c5bc134b6846e72e12dbf459de53 Mon Sep 17 00:00:00 2001 From: Adolanium <94890352+Adolanium@users.noreply.github.com> Date: Tue, 25 Aug 2026 08:48:49 +0300 Subject: [PATCH] Refuse hook and state names that are paths omarchy-hook and omarchy-state set join a name straight into a path. A name with a slash, or a bare . or .., points outside the hooks or state directory. Every caller in the repo passes a fixed label, so this is a footgun guard for future callers, not a fix for anything that ships today. Names with dots in the middle (a..b) stay allowed. omarchy-state clear is untouched: find -name matches basenames only. --- bin/omarchy-hook | 12 +++ bin/omarchy-state | 15 ++- test/shell.d/hook-state-name-guard-test.sh | 109 +++++++++++++++++++++ 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 test/shell.d/hook-state-name-guard-test.sh diff --git a/bin/omarchy-hook b/bin/omarchy-hook index 8c2a59d9..499cdf14 100755 --- a/bin/omarchy-hook +++ b/bin/omarchy-hook @@ -11,6 +11,18 @@ if (( $# < 1 )); then fi HOOK=$1 + +# Hook names are fixed labels chosen by Omarchy code (post-update, theme-set, +# font-set). The name becomes a filename under the hooks directory. A slash +# would turn it into directory levels, and a bare `.` or `..` would point bash +# at the directory itself or its parent. Refuse those rather than follow them. +# Dots inside a name (a..b) are fine; once slashes are out, only the whole +# name being `.` or `..` can leave the directory. +if [[ -z $HOOK || $HOOK == */* || $HOOK == "." || $HOOK == ".." ]]; then + echo "Invalid hook name: $HOOK" >&2 + exit 2 +fi + HOOK_PATH="$HOME/.config/omarchy/hooks/$1" HOOK_DIR="$HOOK_PATH.d" shift diff --git a/bin/omarchy-state b/bin/omarchy-state index 4fda5b3a..3b2f5e17 100755 --- a/bin/omarchy-state +++ b/bin/omarchy-state @@ -21,6 +21,19 @@ if [[ -z $STATE_NAME ]]; then fi case "$COMMAND" in -set) touch "$STATE_DIR/$STATE_NAME" ;; +set) + # State names are fixed labels (reboot-required, restart-*-required). The + # name becomes a filename under the state directory. A slash would turn it + # into directory levels, and a bare `.` or `..` would touch the directory + # itself or its parent. Refuse those. Dots inside a name (a..b) are fine; + # once slashes are out, only the whole name being `.` or `..` can leave the + # directory. clear needs no such guard: find -name matches basenames only, + # so a pattern can never walk out of the directory. + if [[ $STATE_NAME == */* || $STATE_NAME == "." || $STATE_NAME == ".." ]]; then + echo "Invalid state name: $STATE_NAME" >&2 + exit 2 + fi + touch "$STATE_DIR/$STATE_NAME" + ;; clear) find "$STATE_DIR" -maxdepth 1 -type f -name "$STATE_NAME" -delete ;; esac diff --git a/test/shell.d/hook-state-name-guard-test.sh b/test/shell.d/hook-state-name-guard-test.sh new file mode 100644 index 00000000..2955ad27 --- /dev/null +++ b/test/shell.d/hook-state-name-guard-test.sh @@ -0,0 +1,109 @@ +#!/bin/bash + +set -euo pipefail + +source "$(dirname "${BASH_SOURCE[0]}")/base-test.sh" + +work_dir=$(mktemp -d) +trap 'rm -rf "$work_dir"' EXIT + +fake_home="$work_dir/home" +mkdir -p "$fake_home/.config/omarchy/hooks" "$fake_home/.local/state/omarchy" + +# --- omarchy-hook -------------------------------------------------------------- + +# A hook name is a label, not a path. One carrying a slash, or one that is a +# bare `.` or `..`, would run a script from outside the hooks directory. + +cat >"$fake_home/.config/omarchy/hooks/test-hook" <<'SH' +touch "$HOME/hook-ran" +SH + +HOME="$fake_home" "$ROOT/bin/omarchy-hook" test-hook +[[ -f $fake_home/hook-ran ]] || + fail "omarchy hook runs a named hook from the hooks directory" +pass "omarchy hook runs a named hook from the hooks directory" + +# Dots inside a name are not a path. a..b stays inside the hooks directory. +cat >"$fake_home/.config/omarchy/hooks/a..b" <<'SH' +touch "$HOME/dotted-hook-ran" +SH + +HOME="$fake_home" "$ROOT/bin/omarchy-hook" a..b +[[ -f $fake_home/dotted-hook-ran ]] || + fail "omarchy hook accepts a hook name with dots in the middle" +pass "omarchy hook accepts a hook name with dots in the middle" + +for name in . ..; do + status=0 + HOME="$fake_home" "$ROOT/bin/omarchy-hook" "$name" >/dev/null 2>&1 || status=$? + (( status == 2 )) || + fail "omarchy hook refuses a hook name of $name" "exit: $status" + pass "omarchy hook refuses a hook name of $name" +done + +# This file sits where a name of ../../evil would resolve: hooks/../.. is +# ~/.config. +cat >"$fake_home/.config/evil" <<'SH' +touch "$HOME/escape-ran" +SH +chmod +x "$fake_home/.config/evil" + +status=0 +HOME="$fake_home" "$ROOT/bin/omarchy-hook" "../../evil" >/dev/null 2>&1 || status=$? +(( status == 2 )) || + fail "omarchy hook refuses a hook name with a dot-dot" "exit: $status" +[[ ! -e $fake_home/escape-ran ]] || + fail "omarchy hook runs nothing when it refuses the name" +pass "omarchy hook refuses a hook name with a dot-dot" + +status=0 +HOME="$fake_home" "$ROOT/bin/omarchy-hook" "sub/dir" >/dev/null 2>&1 || status=$? +(( status == 2 )) || + fail "omarchy hook refuses a hook name with a slash" "exit: $status" +pass "omarchy hook refuses a hook name with a slash" + +# --- omarchy-state ------------------------------------------------------------- + +state_dir="$fake_home/.local/state/omarchy" + +HOME="$fake_home" "$ROOT/bin/omarchy-state" set reboot-required +[[ -f $state_dir/reboot-required ]] || + fail "omarchy state set still creates a plain state file" +pass "omarchy state set still creates a plain state file" + +HOME="$fake_home" "$ROOT/bin/omarchy-state" set v1..2 +[[ -f $state_dir/v1..2 ]] || + fail "omarchy state set accepts a state name with dots in the middle" +pass "omarchy state set accepts a state name with dots in the middle" + +for name in . ..; do + status=0 + HOME="$fake_home" "$ROOT/bin/omarchy-state" set "$name" >/dev/null 2>&1 || status=$? + (( status == 2 )) || + fail "omarchy state set refuses a state name of $name" "exit: $status" + pass "omarchy state set refuses a state name of $name" +done + +# state/../.. is ~/.local. The guard must fire before touch gets there. +status=0 +HOME="$fake_home" "$ROOT/bin/omarchy-state" set "../../escape" >/dev/null 2>&1 || status=$? +(( status == 2 )) || + fail "omarchy state set refuses a state name with a dot-dot" "exit: $status" +[[ ! -e $fake_home/.local/escape ]] || + fail "omarchy state set creates nothing outside the state directory" +pass "omarchy state set refuses a state name with a dot-dot" + +status=0 +HOME="$fake_home" "$ROOT/bin/omarchy-state" set "sub/dir" >/dev/null 2>&1 || status=$? +(( status == 2 )) || + fail "omarchy state set refuses a state name with a slash" "exit: $status" +pass "omarchy state set refuses a state name with a slash" + +# clear takes patterns by design ("state-name-or-pattern") and matches +# basenames through find -name, so it can never walk out of the directory. +touch "$state_dir/restart-a-required" "$state_dir/restart-b-required" "$state_dir/keep-me" +HOME="$fake_home" "$ROOT/bin/omarchy-state" clear "restart-*-required" +[[ ! -e $state_dir/restart-a-required && ! -e $state_dir/restart-b-required && -f $state_dir/keep-me ]] || + fail "omarchy state clear still clears matching patterns only" +pass "omarchy state clear still clears matching patterns only"