Skip to content

[WIP] Fix exec quotes #361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Itsskiip wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] Fix exec quotes #361

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3bee83f
fixed quotation marks for non-sh exec lines
Itsskiip Aug 22, 2025
ca0013c
fixed private browsing opening extra windows on firefox browsers
Itsskiip Aug 22, 2025
e32dd85
Merge branch 'linuxmint:master' into fix-exec-quotes
Itsskiip Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 75 additions & 95 deletions usr/lib/webapp-manager/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,121 +319,101 @@ def create_webapp(self, name, desc, url, icon, category, browser, custom_paramet


def get_exec_string(self, browser, codename, custom_parameters, icon, isolate_profile, navbar, privatewindow, url):
if browser.browser_type in [BROWSER_TYPE_FIREFOX, BROWSER_TYPE_FIREFOX_FLATPAK, BROWSER_TYPE_FIREFOX_SNAP, BROWSER_TYPE_ZEN_FLATPAK]:
# Firefox based
if browser.browser_type == BROWSER_TYPE_FIREFOX:
firefox_profiles_dir = FIREFOX_PROFILES_DIR
elif browser.browser_type == BROWSER_TYPE_FIREFOX_FLATPAK:
firefox_profiles_dir = FIREFOX_FLATPAK_PROFILES_DIR
elif browser.browser_type == BROWSER_TYPE_ZEN_FLATPAK:
firefox_profiles_dir = ZEN_FLATPAK_PROFILES_DIR
else:
firefox_profiles_dir = FIREFOX_SNAP_PROFILES_DIR
firefox_profile_path = os.path.join(firefox_profiles_dir, codename)
exec_string = ("sh -c 'XAPP_FORCE_GTKWINDOW_ICON=\"" + icon + "\" " + browser.exec_path +
" --class WebApp-" + codename +
" --name WebApp-" + codename +
" --profile " + firefox_profile_path +
" --no-remote")
if privatewindow:
exec_string += " --private-window"
if custom_parameters:
exec_string += " {}".format(custom_parameters)
exec_string += " \"" + url + "\"" + "'"
# Create a Firefox profile
shutil.copytree('/usr/share/webapp-manager/firefox/profile', firefox_profile_path, dirs_exist_ok = True)
if navbar:
shutil.copy('/usr/share/webapp-manager/firefox/userChrome-with-navbar.css',
os.path.join(firefox_profile_path, "chrome", "userChrome.css"))
elif browser.browser_type == BROWSER_TYPE_LIBREWOLF_FLATPAK:
# LibreWolf flatpak
firefox_profiles_dir = LIBREWOLF_FLATPAK_PROFILES_DIR
firefox_profile_path = os.path.join(firefox_profiles_dir, codename)
exec_string = ("sh -c 'XAPP_FORCE_GTKWINDOW_ICON=\"" + icon + "\" " + browser.exec_path +
" --class WebApp-" + codename +
" --name WebApp-" + codename +
" --profile " + firefox_profile_path +
" --no-remote")
if privatewindow:
exec_string += " --private-window"
if custom_parameters:
exec_string += " {}".format(custom_parameters)
exec_string += " \"" + url + "\"" + "'"
# Create a Firefox profile
shutil.copytree('/usr/share/webapp-manager/firefox/profile', firefox_profile_path, dirs_exist_ok = True)
if navbar:
shutil.copy('/usr/share/webapp-manager/firefox/userChrome-with-navbar.css',
os.path.join(firefox_profile_path, "chrome", "userChrome.css"))
elif browser.browser_type == BROWSER_TYPE_FLOORP_FLATPAK:
# Floorp flatpak
firefox_profiles_dir = FLOORP_FLATPAK_PROFILES_DIR
firefox_profile_path = os.path.join(firefox_profiles_dir, codename)
exec_string = ("sh -c 'XAPP_FORCE_GTKWINDOW_ICON=\"" + icon + "\" " + browser.exec_path +
" --class WebApp-" + codename +
" --name WebApp-" + codename +
" --profile " + firefox_profile_path +
" --no-remote")
exec_args = []
firefox_type_map = {
BROWSER_TYPE_FIREFOX: FIREFOX_PROFILES_DIR,
BROWSER_TYPE_FIREFOX_FLATPAK: FIREFOX_FLATPAK_PROFILES_DIR,
BROWSER_TYPE_FIREFOX_SNAP: FIREFOX_SNAP_PROFILES_DIR,
BROWSER_TYPE_ZEN_FLATPAK: ZEN_FLATPAK_PROFILES_DIR,
BROWSER_TYPE_LIBREWOLF_FLATPAK: LIBREWOLF_FLATPAK_PROFILES_DIR,
BROWSER_TYPE_FLOORP_FLATPAK: FLOORP_FLATPAK_PROFILES_DIR
}
name = "Webapp-" + codename
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent capitalization: "Webapp" should be "WebApp" to match the existing convention used throughout the codebase. The old code uses "WebApp-" (line 289 shows StartupWMClass=WebApp-%s), and the delete function uses "WebApp-" (lines 255, 258, 300, 301 in the full file context). This inconsistency will break profile/directory matching in Epiphany and potentially cause issues with window management class names.

Suggested change
name = "Webapp-" + codename
name = "WebApp-" + codename

Copilot uses AI. Check for mistakes.

if profile_dir := firefox_type_map.get(browser.browser_type):
# Firefox-based browsers
profile_path = os.path.join(profile_dir, codename)
exec_args += [
"env",
"XAPP_FORCE_GTKWINDOW_ICON=" + icon,
browser.exec_path,
]

# This needs to appear before the url
if privatewindow:
exec_string += " --private-window"
if custom_parameters:
exec_string += " {}".format(custom_parameters)
exec_string += " \"" + url + "\"" + "'"
exec_args += ["--private-window"]

exec_args += [
url,
"--class", name,
"--name", name,
"--profile", profile_path,
"--no-remote",
Comment on lines +347 to +351
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL is placed in the middle of the Firefox arguments (before --class, --name, --profile, and --no-remote). In the old code, the URL was the last argument. Firefox expects the URL to be the final positional argument after all flags. The current ordering will likely cause Firefox to misinterpret the command line. Move the url to be the last element in exec_args for Firefox-based browsers.

Suggested change
url,
"--class", name,
"--name", name,
"--profile", profile_path,
"--no-remote",
"--class", name,
"--name", name,
"--profile", profile_path,
"--no-remote",
url,

Copilot uses AI. Check for mistakes.
]

# Create a Firefox profile
shutil.copytree('/usr/share/webapp-manager/firefox/profile', firefox_profile_path, dirs_exist_ok = True)
shutil.copytree('/usr/share/webapp-manager/firefox/profile', profile_path, dirs_exist_ok = True)
if navbar:
shutil.copy('/usr/share/webapp-manager/firefox/userChrome-with-navbar.css',
os.path.join(firefox_profile_path, "chrome", "userChrome.css"))
os.path.join(profile_path, "chrome", "userChrome.css"))
elif browser.browser_type == BROWSER_TYPE_EPIPHANY:
# Epiphany based
epiphany_profile_path = os.path.join(EPIPHANY_PROFILES_DIR, "org.gnome.Epiphany.WebApp-" + codename)
epiphany_profile_path = os.path.join(EPIPHANY_PROFILES_DIR, "org.gnome.Epiphany." + name)
# Create symlink of profile dir at ~/.local/share
epiphany_orig_prof_dir = os.path.join(os.path.expanduser("~/.local/share"),
"org.gnome.Epiphany.WebApp-" + codename)
"org.gnome.Epiphany." + name)
os.symlink(epiphany_profile_path, epiphany_orig_prof_dir)
exec_string = browser.exec_path
exec_string += " --application-mode "
exec_string += " --profile=\"" + epiphany_orig_prof_dir + "\""
exec_string += " \"" + url + "\""
if custom_parameters:
exec_string += " {}".format(custom_parameters)

exec_args += [
browser.exec_path,
url,
"--application-mode",
"--profile", epiphany_orig_prof_dir,
Comment on lines +369 to +371
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The argument order for Epiphany has changed significantly. In the old code, the order was: --application-mode --profile="..." "url". In the new code, it's: url --application-mode --profile .... Placing the URL before the flags may cause Epiphany to misinterpret the command line arguments. The URL should typically be the last positional argument after all flags.

Suggested change
url,
"--application-mode",
"--profile", epiphany_orig_prof_dir,
"--application-mode",
"--profile", epiphany_orig_prof_dir,
url,

Copilot uses AI. Check for mistakes.
]
elif browser.browser_type == BROWSER_TYPE_FALKON:
# KDE Falkon
exec_string = browser.exec_path
exec_string += " --wmclass=WebApp-" + codename
exec_args += [
browser.exec_path,
"--no-remote", url,
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of arguments for Falkon appears incorrect. The --no-remote flag and url are placed before --wmclass, but flags should typically come after positional arguments or at least be consistently ordered. More importantly, looking at the old code, url was the last argument, not in the middle of the flags. This reordering may cause the URL to be misinterpreted or the flags to not work correctly.

Copilot uses AI. Check for mistakes.
"--wmclass=" + name,
]
if isolate_profile:
exec_string += " --profile=" + codename
exec_args += ["--profile=" + codename]
if privatewindow:
exec_string += " --private-browsing"
if custom_parameters:
exec_string += " {}".format(custom_parameters)
exec_string += " --no-remote " + url
exec_args += ["--private-browsing"]
else:
# Chromium based
exec_args += [
browser.exec_path,
"--app=" + url,
"--class=" + name,
"--name=" + name,
]
if isolate_profile:
profile_path = os.path.join(PROFILES_DIR, codename)
exec_string = (browser.exec_path +
" --app=" + "\"" + url + "\"" +
" --class=WebApp-" + codename +
" --name=WebApp-" + codename +
" --user-data-dir=" + profile_path)
else:
exec_string = (browser.exec_path +
" --app=" + "\"" + url + "\"" +
" --class=WebApp-" + codename +
" --name=WebApp-" + codename)
exec_args += ["--user-data-dir=" + profile_path]

if privatewindow:
if browser.name == "Microsoft Edge":
exec_string += " --inprivate"
elif browser.name == "Microsoft Edge Beta":
exec_string += " --inprivate"
elif browser.name == "Microsoft Edge Dev":
exec_string += " --inprivate"
if browser.name in ["Microsoft Edge", "Microsoft Edge Beta", "Microsoft Edge Dev"]:
exec_args += ["--inprivate"]
else:
exec_string += " --incognito"

if custom_parameters:
exec_string += " {}".format(custom_parameters)
exec_args += ["--incognito"]

def escape_arg(arg: str):
"""Apply quoting rules according to desktop-entry-spec 1.1"""
escapes = {
"\\": "\\\\\\\\", # 4 backslashes
"$": "\\\\$", # \\$
"`": "\\\\`", # \\`
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical escaping bug: The escapes dictionary is missing the double quote character ("). When an argument contains a double quote, line 409 detects it and triggers quoting, but line 410 wraps the argument in double quotes without escaping the internal double quotes. This will result in broken command lines. According to the desktop-entry-spec, double quotes inside a quoted string should be escaped. Add '"': '\\\\"' to the escapes dictionary.

Suggested change
"`": "\\\\`", # \\`
"`": "\\\\`", # \\`
'"': '\\\\"', # \"

Copilot uses AI. Check for mistakes.
}
if any(c in " \t\n\"'\\><~|&;$*?#()`" for c in arg):
return '"{}"'.format("".join(escapes.get(c, c) for c in arg))
return arg
Comment on lines +402 to +411
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The escape_arg function is defined inside get_exec_string, which means it's recreated every time the function is called. This is inefficient and unconventional. Consider moving it to module level or as a static method of the class for better performance and reusability.

Suggested change
def escape_arg(arg: str):
"""Apply quoting rules according to desktop-entry-spec 1.1"""
escapes = {
"\\": "\\\\\\\\", # 4 backslashes
"$": "\\\\$", # \\$
"`": "\\\\`", # \\`
}
if any(c in " \t\n\"'\\><~|&;$*?#()`" for c in arg):
return '"{}"'.format("".join(escapes.get(c, c) for c in arg))
return arg
# Module-level helper function
def escape_arg(arg: str):
"""Apply quoting rules according to desktop-entry-spec 1.1"""
escapes = {
"\\": "\\\\\\\\", # 4 backslashes
"$": "\\\\$", # \\$
"`": "\\\\`", # \\`
}
if any(c in " \t\n\"'\\><~|&;$*?#()`" for c in arg):
return '"{}"'.format("".join(escapes.get(c, c) for c in arg))
return arg

Copilot uses AI. Check for mistakes.

exec_string = " ".join(map(escape_arg, exec_args))

if custom_parameters:
exec_string += " " + custom_parameters # No automatic escaping for these done
Comment on lines +415 to +416
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security/correctness concern: custom_parameters are appended without escaping. If custom_parameters contain quotes, backslashes, or other special characters, they could break the desktop entry parsing or cause unintended argument splitting. For example, a parameter like --foo="bar baz" might be incorrectly parsed. While the comment notes this limitation, it would be better to either apply the same escaping rules to custom_parameters or at least validate that they don't contain problematic characters.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment: "No automatic escaping for these done" should be "No automatic escaping done for these" for better grammar.

Suggested change
exec_string += " " + custom_parameters # No automatic escaping for these done
exec_string += " " + custom_parameters # No automatic escaping done for these

Copilot uses AI. Check for mistakes.

return exec_string

Expand Down
Loading