Skip to content

Commit 9ece53c

Browse files
dhhclaude
authored andcommitted
Prove the web app name guard, and reject before the icon is fetched
The slash guard was the only thing keeping a name out of the directory structure, and nothing tested it: deleting it left the suite green, because creating the launcher directly in the applications directory already makes the redirect fail on its own, with a raw bash error instead of the message. The assertion is on the message now, alongside the traversal case the guard actually closes -- on quattro a name of `../../../../escaped` writes its launcher clean outside the applications directory. The interactive prompt read the name, fetched the favicon, wrote it and updated the icon cache before the name was ever checked, so a URL typed into the Name field left an icon behind on every attempt. Validating as soon as the name is read covers both paths from one place. Removing by name also scanned unconditionally, so a machine with no applications directory printed a find error where omarchy-remove-gaming-xbox-cloud does not hide stderr. 🤖 Generated by Opus 5 in Claude Code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7c896d3 commit 9ece53c

3 files changed

Lines changed: 83 additions & 12 deletions

File tree

bin/omarchy-webapp-install

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@ safe_icon_name() {
1313
| sed 's/[^[:alnum:]]\+/-/g; s/^-//; s/-$//'
1414
}
1515

16+
require_plain_name() {
17+
# The name becomes a filename. A slash would turn it into directory levels, so
18+
# the launcher lands somewhere omarchy-webapp-remove cannot address and the app
19+
# is stuck in the launcher; a leading ../ leaves the applications directory
20+
# altogether. Refuse rather than silently renaming what the user typed -- most
21+
# often it is a URL entered in the name field.
22+
if [[ $1 == */* ]]; then
23+
echo "App name cannot contain '/': $1"
24+
exit 1
25+
fi
26+
}
27+
1628
icon_name_from_ref() {
1729
local ref="$1"
1830
local name
@@ -68,6 +80,7 @@ fetch_site_icon() {
6880
if (( $# < 3 )); then
6981
echo -e "\e[32mLet's create a new web app you can start with the app launcher.\n\e[0m"
7082
APP_NAME=$(gum input --prompt "Name> " --placeholder "My favorite web app")
83+
require_plain_name "$APP_NAME"
7184
APP_URL=$(gum input --prompt "URL> " --placeholder "https://example.com")
7285
if [[ ! $APP_URL =~ ^[a-zA-Z][a-zA-Z0-9+.-]*: ]]; then
7386
APP_URL="https://$APP_URL"
@@ -104,14 +117,7 @@ if [[ -z $APP_NAME || -z $APP_URL ]]; then
104117
exit 1
105118
fi
106119

107-
# The name becomes a filename. A slash would turn it into directory levels, so
108-
# the launcher lands somewhere omarchy-webapp-remove cannot address and the app
109-
# is stuck in the launcher. Refuse rather than silently renaming what the user
110-
# typed -- most often it is a URL entered in the name field.
111-
if [[ $APP_NAME == */* ]]; then
112-
echo "App name cannot contain '/': $APP_NAME"
113-
exit 1
114-
fi
120+
require_plain_name "$APP_NAME"
115121

116122
if [[ -z $ICON_REF ]]; then
117123
ICON_VALUE=$(safe_icon_name "$APP_NAME")

bin/omarchy-webapp-remove

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ while IFS= read -r -d '' file; do
1919
WEB_APPS+=("$(basename "${file%.desktop}")")
2020
WEB_APP_PATHS+=("$file")
2121
fi
22-
done < <(find "$DESKTOP_DIR" -name '*.desktop' -print0)
22+
done < <(find "$DESKTOP_DIR" -name '*.desktop' -print0 2>/dev/null)
2323

2424
# The launcher matching a chosen name, or empty when nothing was indexed under
2525
# it (an app removed between the scan and the pick, say).

test/shell.d/webapp-name-test.sh

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,73 @@ run_remove() {
2424
}
2525

2626
apps_dir="$tmp_dir/home/.local/share/applications"
27+
icons_dir="$tmp_dir/home/.local/share/icons/hicolor/256x256/apps"
2728

2829
# A URL typed into the name field is the reported way in. Every slash used to
29-
# become a directory level, leaving a launcher nothing could address.
30-
if run_install "http://example.test/oops" "https://example.com" hey >/dev/null 2>&1; then
30+
# become a directory level, leaving a launcher nothing could address. Assert on
31+
# the message: creating the launcher directly in the applications directory
32+
# already makes the redirect fail on its own, so a bare non-zero exit would pass
33+
# just as well with no validation at all.
34+
output=$(run_install "http://example.test/oops" "https://example.com" hey 2>&1) &&
3135
fail "webapp install rejects a name containing a slash"
32-
fi
36+
[[ $output == *"App name cannot contain '/'"* ]] ||
37+
fail "webapp install says why it refused a slashed name" "$output"
3338
[[ -e "$apps_dir/http:" ]] &&
3439
fail "webapp install does not create a directory from a slashed name"
3540
pass "webapp install rejects a name that would nest the launcher"
3641

42+
# The name was a path fragment until something said otherwise, so ../ climbed
43+
# out of the applications directory entirely and wrote wherever it landed.
44+
if run_install "../../../../escaped" "https://example.com" hey >/dev/null 2>&1; then
45+
fail "webapp install rejects a name that climbs out of the applications directory"
46+
fi
47+
[[ -e "$tmp_dir/escaped.desktop" ]] &&
48+
fail "webapp install writes no launcher outside the applications directory"
49+
pass "webapp install refuses a name that would escape the applications directory"
50+
51+
# The interactive prompt reads the name long before it is used as a path, and
52+
# fetches the site icon in between. Rejecting only at the write leaves that icon
53+
# behind in the user's icon theme, once per attempt.
54+
mkdir -p "$tmp_dir/ibin"
55+
cp "$tmp_dir/bin"/* "$tmp_dir/ibin/"
56+
cat >"$tmp_dir/ibin/gum" <<'STUB'
57+
#!/bin/bash
58+
count_file="${GUM_STUB_COUNT:?}"
59+
count=$(cat "$count_file" 2>/dev/null || echo 0)
60+
count=$((count + 1))
61+
echo "$count" >"$count_file"
62+
if (( count == 1 )); then
63+
echo "http://example.test/oops"
64+
else
65+
echo "https://example.com"
66+
fi
67+
STUB
68+
cat >"$tmp_dir/ibin/curl" <<'STUB'
69+
#!/bin/bash
70+
# Answer any download with a real PNG so the icon fetch reports success.
71+
out=""
72+
prev=""
73+
for arg in "$@"; do
74+
[[ $prev == "-o" ]] && out="$arg"
75+
prev="$arg"
76+
done
77+
if [[ -n $out ]]; then
78+
printf '%s' 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==' | base64 -d >"$out"
79+
fi
80+
STUB
81+
chmod +x "$tmp_dir/ibin/gum" "$tmp_dir/ibin/curl"
82+
83+
if HOME="$tmp_dir/home" PATH="$tmp_dir/ibin:$PATH" \
84+
GUM_STUB_COUNT="$tmp_dir/gum-count" \
85+
"$ROOT/bin/omarchy-webapp-install" >/dev/null 2>&1; then
86+
fail "interactive webapp install rejects a name containing a slash"
87+
fi
88+
if compgen -G "$icons_dir/*.png" >/dev/null; then
89+
fail "interactive webapp install downloads no icon for a name it refuses" \
90+
"$(ls "$icons_dir")"
91+
fi
92+
pass "webapp install refuses a slashed name before fetching its icon"
93+
3794
# A normal name still installs and removes.
3895
run_install "Example App" "https://example.com" hey >/dev/null
3996
[[ -f "$apps_dir/Example App.desktop" ]] ||
@@ -59,3 +116,11 @@ run_remove "127.0.0.1:4000" >/dev/null
59116
[[ -f "$apps_dir/http:/127.0.0.1:4000/.desktop" ]] &&
60117
fail "webapp remove deletes a launcher left nested by an older install"
61118
pass "webapp remove reaches a nested legacy launcher"
119+
120+
# Removing by name on a machine with no applications directory yet must stay
121+
# quiet: omarchy-remove-gaming-xbox-cloud calls it without hiding stderr.
122+
noise=$(HOME="$tmp_dir/empty" PATH="$tmp_dir/bin:$PATH" OMARCHY_REMOVE_NOTIFY=false \
123+
"$ROOT/bin/omarchy-webapp-remove" "Xbox Cloud Gaming" 2>&1 >/dev/null)
124+
[[ -n $noise ]] &&
125+
fail "webapp remove stays quiet with no applications directory" "$noise"
126+
pass "webapp remove stays quiet when there is no applications directory"

0 commit comments

Comments
 (0)