Merge pull request #8416 from mdisec/theme-name-shell-syntax

Refuse a theme name that is shell syntax, and quote the one the unlock picker returns

bin/omarchy-theme-install

@@ -23,16 +23,26 @@ omarchy-git-url-check "$REPO_URL" || exit 1
 
 THEMES_DIR="$HOME/.config/omarchy/themes"
 
-# Strip user@host: prefix from scp-style SSH URLs so basename sees just the path
+# Strip user@host: prefix from scp-style SSH URLs so basename sees just the path.
+# git reads a URL as scp-style when a colon appears before any slash, so the path
+# after it need not hold one: `git@host:omarchy-blue-theme.git` is a repo in that
+# user's home, and leaving its prefix on names the theme after the whole URL.
 REPO_PATH="$REPO_URL"
-[[ $REPO_PATH != *"://"* && $REPO_PATH == *:*/* ]] && REPO_PATH="${REPO_PATH#*:}"
+[[ $REPO_PATH != *"://"* && $REPO_PATH == *:* && ${REPO_PATH%%:*} != */* ]] && REPO_PATH="${REPO_PATH#*:}"
 THEME_NAME=$(basename -- "$REPO_PATH" .git | sed -E 's/^omarchy-//; s/-theme$//' | tr '[:upper:]' '[:lower:]')
 THEME_PATH="$THEMES_DIR/$THEME_NAME"
 
-# The name comes from the URL and is joined into a path that is about to be
+# The name comes from the URL, is joined into a path that is about to be
-# removed, so a repo called `..` would take ~/.config/omarchy with it. A leading
+# removed, and then names a directory the rest of Omarchy passes around by
-# dot is refused with it: `host:-s/foo.git` leaves basename with `.git`.
+# name: Style > Unlock builds a command line out of the one the picker
-if [[ -z $THEME_NAME || $THEME_NAME == .* || $THEME_NAME == */* ]]; then
+# returned. So it is held to the characters a theme name needs rather than
+# screened for the harm of the day -- a repo called `..` would take
+# ~/.config/omarchy with it, and one called `a';'id` would carry its own
+# command into that picker. The leading character is kept out of `.` and `-`,
+# which also covers `host:-s/foo.git` leaving basename with `.git`.
+# A bracket range follows the locale's collation, not ASCII: `[a-z]` takes in
+# `é` under en_US.UTF-8. Pin the locale so the set is the one written here.
+if ! (LC_ALL=C; [[ $THEME_NAME =~ ^[a-z0-9_][a-z0-9._+-]*$ ]]); then
   echo "Error: '$REPO_URL' does not give a usable theme name."
   exit 1
 fi

default/omarchy/omarchy-menu.jsonc

@@ -103,7 +103,7 @@
   // Style
   "style.theme": {"icon":"󰸌","label":"Theme","aliases":["theme","themes"],"action":"theme=$(omarchy-theme-switcher); [[ -n $theme ]] && omarchy-theme-set \"$theme\""},
   "style.background": {"icon":"","label":"Background","aliases":["background","wallpaper"],"action":"background=$(omarchy-theme-bg-switcher); [[ -n $background ]] && omarchy-theme-bg-set \"$background\""},
-  "style.unlock": {"icon":"󰟵","label":"Unlock","aliases":["unlock"],"action":"unlock=$(omarchy-plymouth-switcher); if [[ $unlock == default ]]; then omarchy-launch-floating-terminal-with-presentation omarchy-plymouth-reset; elif [[ -n $unlock ]]; then omarchy-launch-floating-terminal-with-presentation \"omarchy-plymouth-set-by-theme '$unlock'\"; fi"},
+  "style.unlock": {"icon":"󰟵","label":"Unlock","aliases":["unlock"],"action":"unlock=$(omarchy-plymouth-switcher); if [[ $unlock == default ]]; then omarchy-launch-floating-terminal-with-presentation omarchy-plymouth-reset; elif [[ -n $unlock ]]; then omarchy-launch-floating-terminal-with-presentation \"omarchy-plymouth-set-by-theme $(printf %q \"$unlock\")\"; fi"},
   "style.font": {"icon":"","label":"Font","provider":"fonts"},
   "style.bar": {"icon":"󰍜","label":"Menu Bar"},
   "style.bar.position": {"icon":"","label":"Position"},

manual/43-making-your-own-theme.md

@@ -38,6 +38,8 @@ There's a fully commented `alacritty.toml.tpl.sample` in that folder to copy fro
 
 If you want to distribute your theme so others can use it, you need to put it on a public git server, like GitHub. Then people can install it using _Install > Style > Theme_ in the Omarchy menu using that URL. It's recommended that you follow the naming convention of `omarchy-[themename]-theme`, as the theme will show correctly as just `[themename]` in the theme selection menu after installation.
 
+That leftover `[themename]` becomes the theme's directory name, so it has to be one Omarchy can hand around safely: it must start with a letter, a digit, or an underscore, and the rest may hold letters, digits, `.`, `_`, `+`, and `-`. Capitals are lowercased for you, but anything else — a space, a quote, a non-English character — is refused at install time rather than turned into a directory name. So `omarchy-tokyo-night-theme`, `omarchy-flexoki_light-theme`, and `omarchy-c++-theme` all install fine.
+
 Remember that once it's installed from a repo, any `.lua`, terminal config or `vscode.json` it ships is dropped, so don't build the theme around those.
 
 You can have your theme added to [the extra themes page](https://omarchy.org/themes/) by sending a pull request to [the omarchy-site repo](https://github.com/omacom-io/omarchy-site).

test/shell.d/plymouth-set-test.sh

@@ -41,3 +41,103 @@ grep -Fq 'sudo cp "$staging_dir/logo.png" "$sddm_dir/logo.png"' "$ROOT/bin/omarc
   fail "omarchy-plymouth-set copies the staged logo to SDDM rather than rereading the caller's path as root"
 
 pass "a themed logo cannot republish a file it merely points at"
+
+# Style > Unlock picks a theme by name and hands the answer to
+# omarchy-launch-floating-terminal-with-presentation, which joins its arguments
+# into a script and runs that with `bash -c`. So the name is shell source
+# unless the action quotes it -- and the name is a directory name under
+# ~/.config/omarchy/themes, which a theme installed from a git repo gets from
+# the repo URL. `a';id;'b` is a legal directory name.
+require_command node
+
+unlock_action=$(node -e '
+  const fs = require("fs")
+  const path = require("path")
+  const menu = require(path.join(process.env.ROOT, "shell/plugins/menu/MenuModel.js"))
+  const items = menu.parseMenuJsonc(fs.readFileSync(path.join(process.env.ROOT, "default/omarchy/omarchy-menu.jsonc"), "utf8"))
+  process.stdout.write(items.find(item => item.id === "style.unlock").action)
+')
+
+[[ -n $unlock_action ]] || fail "the shipped menu still carries a style.unlock action"
+
+stub_dir="$test_tmp/stubs"
+mkdir -p "$stub_dir"
+
+canary="$test_tmp/canary"
+set_args="$test_tmp/set-args"
+reset_marker="$test_tmp/reset-ran"
+
+# What a name that got reparsed would reach. It is a command rather than a
+# `touch` so that no quoting of the test's own paths is involved.
+cat >"$stub_dir/omarchy-test-canary" <<STUB
+#!/bin/bash
+printf 'ran\n' >"$canary"
+STUB
+
+cat >"$stub_dir/omarchy-plymouth-switcher" <<'STUB'
+#!/bin/bash
+printf '%s\n' "$OMARCHY_TEST_UNLOCK_NAME"
+STUB
+
+# Stands in for the real wrapper, which is a shell-string API: it interpolates
+# "$*" into a script and hands that to `bash -c`. The grep below is what keeps
+# this stub honest if the wrapper ever stops working that way.
+cat >"$stub_dir/omarchy-launch-floating-terminal-with-presentation" <<'STUB'
+#!/bin/bash
+exec bash -c "omarchy-show-logo; $*; omarchy-show-done"
+STUB
+
+grep -Fq 'bash -c "$presentation_script"' "$ROOT/bin/omarchy-launch-floating-terminal-with-presentation" ||
+  fail "the presentation wrapper still runs its argument as a shell string, as the stub above assumes"
+
+# Records what actually arrived, so a name that survived as data is told apart
+# from one that arrived split or partly eaten.
+cat >"$stub_dir/omarchy-plymouth-set-by-theme" <<'STUB'
+#!/bin/bash
+printf '%s\n' "$#" "$@" >"$OMARCHY_TEST_SET_ARGS"
+STUB
+
+cat >"$stub_dir/omarchy-plymouth-reset" <<'STUB'
+#!/bin/bash
+printf 'ran\n' >"$OMARCHY_TEST_RESET_MARKER"
+STUB
+
+for command in omarchy-show-logo omarchy-show-done; do
+  printf '#!/bin/bash\nexit 0\n' >"$stub_dir/$command"
+done
+
+chmod +x "$stub_dir"/*
+
+run_unlock_action() {
+  rm -f "$canary" "$set_args" "$reset_marker"
+
+  PATH="$stub_dir:$PATH" \
+    OMARCHY_TEST_UNLOCK_NAME="$1" \
+    OMARCHY_TEST_SET_ARGS="$set_args" \
+    OMARCHY_TEST_RESET_MARKER="$reset_marker" \
+    bash -c "$unlock_action" >/dev/null 2>&1
+}
+
+# A directory name cannot hold a slash or a NUL, and everything else is fair
+# game -- these are the shapes that would run on the way to the picker.
+for name in "a';omarchy-test-canary;'b" 'a$(omarchy-test-canary)b' 'a`omarchy-test-canary`b' 'a b' '-a'; do
+  run_unlock_action "$name"
+
+  [[ ! -e $canary ]] || fail "a theme name reaches the unlock screen as data, not as shell" "ran for: $name"
+  [[ $(cat "$set_args" 2>/dev/null) == $'1\n'"$name" ]] ||
+    fail "the unlock screen gets the theme name whole" "$name: $(cat "$set_args" 2>/dev/null)"
+done
+
+pass "a theme name cannot carry a command into the unlock screen"
+
+# The two ordinary paths still work: a named theme is applied, and `default`
+# resets rather than being looked up as a theme.
+run_unlock_action "tokyo-night"
+[[ $(cat "$set_args" 2>/dev/null) == $'1\ntokyo-night' ]] ||
+  fail "an ordinary theme name still reaches omarchy-plymouth-set-by-theme" "$(cat "$set_args" 2>/dev/null)"
+
+run_unlock_action "default"
+[[ -e $reset_marker ]] || fail "picking default still resets the unlock screen"
+[[ ! -e $set_args ]] || fail "picking default does not look up a theme named default" "$(cat "$set_args")"
+
+pass "the unlock picker still applies a theme and still resets on default"

test/shell.d/theme-install-guards-test.sh

@@ -92,6 +92,83 @@ done
 
 pass "a URL whose name would climb out of the themes directory never reaches git"
 
+# The derived name outlives the clone: it is the theme's directory name, and
+# Style > Unlock builds a command line out of the name the picker returned. A
+# repo whose name carries shell syntax would hand that picker its own command,
+# so the name is refused here rather than quoted at each place it lands.
+for url in \
+  "https://example.com/omarchy-a';id;'b-theme.git" \
+  'https://example.com/a$(id).git' \
+  'https://example.com/a`id`.git' \
+  "https://example.com/a b.git" \
+  "https://example.com/-a.git"; do
+  if install_theme "$url"; then
+    fail "omarchy-theme-install refuses the derived name from '$url'"
+  fi
+
+  [[ ! -s $git_calls ]] || fail "omarchy-theme-install refuses '$url' before running git" "$(cat "$git_calls")"
+done
+
+pass "a URL whose name would be shell syntax never reaches git"
+
+# And the check is an allowlist, so the punctuation a real theme name uses has
+# to keep working.
+install_theme "https://github.com/example/omarchy-tokyo_night.2-theme.git" ||
+  fail "omarchy-theme-install accepts the punctuation a theme name uses"
+grep -Fq "/themes/tokyo_night.2" "$git_calls" ||
+  fail "omarchy-theme-install derives a name carrying an underscore and a dot" "$(cat "$git_calls")"
+
+pass "a theme name may still hold an underscore, a dot, and a dash"
+
+# A plus is neither path-climb nor shell syntax, and a leading underscore is
+# neither the `..` climb nor the dash that reads as an option, so the allowlist
+# keeps both rather than stranding a repo that names itself with them.
+install_theme "https://github.com/example/omarchy-c++-theme.git" ||
+  fail "omarchy-theme-install accepts a name holding a plus"
+grep -Fq "/themes/c++" "$git_calls" ||
+  fail "omarchy-theme-install derives a name carrying a plus" "$(cat "$git_calls")"
+
+install_theme "https://github.com/example/_private.git" ||
+  fail "omarchy-theme-install accepts a name starting with an underscore"
+grep -Fq "/themes/_private" "$git_calls" ||
+  fail "omarchy-theme-install derives a name starting with an underscore" "$(cat "$git_calls")"
+
+pass "a plus and a leading underscore are still usable theme names"
+
+# git reads a colon before any slash as the scp-style separator, so the path
+# after it does not have to hold one. Without that reading, the whole URL becomes
+# the theme name and the allowlist above refuses a repo that clones fine.
+install_theme "git@example.com:omarchy-blue-theme.git" ||
+  fail "omarchy-theme-install accepts a home-relative scp-style URL"
+grep -Fq "/themes/blue" "$git_calls" ||
+  fail "omarchy-theme-install names the theme after the repo, not the whole URL" "$(cat "$git_calls")"
+
+# A colon that is part of a local path, not an scp separator, keeps its prefix.
+install_theme "/srv/git:mirrors/omarchy-blue-theme.git" ||
+  fail "omarchy-theme-install accepts a local path holding a colon"
+grep -Fq "/themes/blue" "$git_calls" ||
+  fail "omarchy-theme-install reads a colon after a slash as part of the path" "$(cat "$git_calls")"
+
+pass "an scp-style URL with no slash after the colon still names the theme"
+
+# The allowlist is a bracket range, and a range follows the locale's collation
+# rather than ASCII: under en_US.UTF-8 an unpinned `[a-z]` takes in `é`, so the
+# same URL would install on one desktop and be refused on the next.
+if locale -a 2>/dev/null | grep -qix 'en_US.utf-\?8'; then
+  for locale_name in C en_US.UTF-8; do
+    if LC_ALL=$locale_name install_theme "https://github.com/example/omarchy-café-theme.git"; then
+      fail "omarchy-theme-install refuses a non-ASCII theme name under LC_ALL=$locale_name" "$(cat "$git_calls")"
+    fi
+
+    [[ ! -s $git_calls ]] ||
+      fail "omarchy-theme-install refuses a non-ASCII name before running git" "$(cat "$git_calls")"
+  done
+
+  pass "the accepted set does not move with the desktop's locale"
+else
+  pass "no en_US.UTF-8 locale; skipping the locale-pinning check"
+fi
+
 # basename reads a leading dash as an option once the scp-style prefix is gone.
 install_theme "host:-s/foo.git" || fail "omarchy-theme-install accepts a normal scp-style URL"
 grep -Fq -- "-- host:-s/foo.git" "$git_calls" || fail "omarchy-theme-install passes the URL after --" "$(cat "$git_calls")"