Merge pull request #8473 from bastidotnet/fix/webapp-desktop-value-escaping

bin/omarchy-webapp-install

@@ -77,6 +77,37 @@ fetch_site_icon() {
     download_icon "https://www.google.com/s2/favicons?domain=${site_url}&sz=256" "$dest"
 }
 
+desktop_string_escape() {
+  # Desktop Entry "string" value (freedesktop Desktop Entry Spec, "Value types"):
+  # a raw newline would start a new key line and let a value inject a second
+  # Exec=. Escape backslash first, then tab/CR/LF and a leading space. Every value
+  # written into the .desktop file passes through here.
+  #
+  # Parameter expansion rather than sed: GNU sed's N auto-prints the pattern space
+  # and exits at end of input, so a `:a;N;$!ba` slurp skips every following s///
+  # for a value with no newline in it - which is every value except the injection
+  # attempt this exists to stop.
+  local value="$1"
+
+  value=${value//\\/\\\\}
+  value=${value//$'\t'/\\t}
+  value=${value//$'\r'/\\r}
+  value=${value//$'\n'/\\n}
+  [[ $value == " "* ]] && value="\\s${value# }"
+
+  printf '%s' "$value"
+}
+
+desktop_exec_arg() {
+  # One Exec argument, double-quoted per the freedesktop Exec spec: inside quotes
+  # " ` $ \ take a backslash and a literal % becomes %%. Only the default Exec's
+  # URL needs this; $CUSTOM_EXEC stays a whole command line (file-syntax only).
+  local escaped
+  escaped=$(printf '%s' "$1" \
+    | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e 's/`/\\`/g' -e 's/\$/\\$/g' -e 's/%/%%/g')
+  printf '"%s"' "$escaped"
+}
+
 if (( $# < 3 )); then
   echo -e "\e[32mLet's create a new web app you can start with the app launcher.\n\e[0m"
   APP_NAME=$(gum input --prompt "Name> " --placeholder "My favorite web app")
@@ -143,29 +174,39 @@ else
   ICON_VALUE=$(icon_name_from_ref "$ICON_REF")
 fi
 
-# Use custom exec if provided, otherwise default behavior
+# Default Exec quotes the URL as one Exec-spec argument; the whole line then gets
-EXEC_COMMAND="${CUSTOM_EXEC:-omarchy-launch-webapp $APP_URL}"
+# the file-syntax escaping below (unescaped first at read time per spec, so the
+# layers compose). $CUSTOM_EXEC is a full command line, so it gets file-syntax only.
+if [[ -n $CUSTOM_EXEC ]]; then
+  EXEC_COMMAND=$CUSTOM_EXEC
+else
+  EXEC_COMMAND="omarchy-launch-webapp $(desktop_exec_arg "$APP_URL")"
+fi
 
 # Create application .desktop file
 DESKTOP_DIR="$HOME/.local/share/applications"
 DESKTOP_FILE="$DESKTOP_DIR/$APP_NAME.desktop"
 mkdir -p "$DESKTOP_DIR"
 
+name_field=$(desktop_string_escape "$APP_NAME")
+exec_field=$(desktop_string_escape "$EXEC_COMMAND")
+icon_field=$(desktop_string_escape "$ICON_VALUE")
+
 cat >"$DESKTOP_FILE" <<EOF
 [Desktop Entry]
 Version=1.0
-Name=$APP_NAME
+Name=$name_field
-Comment=$APP_NAME
+Comment=$name_field
-Exec=$EXEC_COMMAND
+Exec=$exec_field
 Terminal=false
 Type=Application
-Icon=$ICON_VALUE
+Icon=$icon_field
 StartupNotify=true
 EOF
 
 # Add mime types if provided
 if [[ -n $MIME_TYPES ]]; then
-  echo "MimeType=$MIME_TYPES" >>"$DESKTOP_FILE"
+  printf 'MimeType=%s\n' "$(desktop_string_escape "$MIME_TYPES")" >>"$DESKTOP_FILE"
 fi
 
 chmod +x "$DESKTOP_FILE"

test/shell.d/webapp-install-escaping-test.sh

@@ -0,0 +1,92 @@
+#!/bin/bash
+
+set -euo pipefail
+
+source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh"
+
+require_command gio
+
+test_tmp=$(mktemp -d)
+trap 'rm -rf "$test_tmp"' EXIT
+
+mock_bin="$test_tmp/bin"
+mkdir -p "$mock_bin"
+
+cat >"$mock_bin/omarchy-launch-webapp" <<'SH'
+#!/bin/bash
+printf '%s\n' "$@" >>"$OMARCHY_TEST_ARGV"
+SH
+chmod +x "$mock_bin"/*
+
+export HOME="$test_tmp/home"
+export PATH="$mock_bin:$PATH"
+export OMARCHY_TEST_ARGV="$test_tmp/argv"
+
+applications="$HOME/.local/share/applications"
+
+install_webapp() {
+  bash "$ROOT/bin/omarchy-webapp-install" "$@" >/dev/null
+}
+
+desktop_value() {
+  sed -n "s/^$2=//p" "$1" | head -1
+}
+
+# gio launch returns before the entry it spawned has run, so poll for the argv the
+# stub records rather than reading the log once.
+launched_argument() {
+  local file="$1" attempt
+
+  : >"$OMARCHY_TEST_ARGV"
+  gio launch "$file" >/dev/null 2>&1 || return 1
+  for ((attempt = 0; attempt < 200; attempt++)); do
+    [[ -s $OMARCHY_TEST_ARGV ]] && break
+    sleep 0.01
+  done
+
+  head -1 "$OMARCHY_TEST_ARGV"
+}
+
+# The Exec quoting escapes a dollar sign with a backslash, and the file syntax has
+# to escape that backslash in turn. Left single, GLib reads \$ as an invalid escape
+# and refuses the whole entry, so the web app vanishes from the launcher.
+install_webapp 'Dollar App' 'https://example.com/a$b' someicon
+dollar_file="$applications/Dollar App.desktop"
+
+[[ -f $dollar_file ]] || fail "web app install writes a desktop entry"
+
+[[ $(desktop_value "$dollar_file" Exec) == 'omarchy-launch-webapp "https://example.com/a\\$b"' ]] ||
+  fail "Exec escapes the backslash its own quoting introduced" "$(desktop_value "$dollar_file" Exec)"
+pass "Exec escapes the backslash its own quoting introduced"
+
+[[ $(launched_argument "$dollar_file") == 'https://example.com/a$b' ]] ||
+  fail "a URL containing a dollar sign reaches the browser unchanged"
+pass "a URL containing a dollar sign reaches the browser unchanged"
+
+# An unescaped % is read as a Desktop Entry field code and eaten, so ?q=a%20b used
+# to arrive as ?q=a0b.
+install_webapp 'Percent App' 'https://example.com/s?q=a%20b' someicon
+percent_file="$applications/Percent App.desktop"
+
+[[ $(launched_argument "$percent_file") == 'https://example.com/s?q=a%20b' ]] ||
+  fail "a percent-encoded URL reaches the browser unchanged"
+pass "a percent-encoded URL reaches the browser unchanged"
+
+# A lone backslash is not a Desktop Entry escape sequence, so GLib cannot interpret
+# a value that contains one.
+install_webapp 'Back\slash App' 'https://example.com' someicon
+backslash_file="$applications/Back\slash App.desktop"
+
+[[ $(desktop_value "$backslash_file" Name) == 'Back\\slash App' ]] ||
+  fail "a backslash in the app name is escaped" "$(desktop_value "$backslash_file" Name)"
+pass "a backslash in the app name is escaped"
+
+# The property the escaping exists for: a newline in a value must not be able to
+# start a second key line.
+inject_name=$(printf 'Inject\nExec=evil')
+install_webapp "$inject_name" 'https://example.com' someicon
+inject_file="$applications/$inject_name.desktop"
+
+(( $(grep -c '^Exec=' "$inject_file") == 1 )) ||
+  fail "a newline in the app name cannot inject a second Exec" "$(cat "$inject_file")"
+pass "a newline in the app name cannot inject a second Exec"