Skip to content
79 changes: 74 additions & 5 deletions src/deadline/client/cli/_deadline_web_url.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

import configparser
import os
import re
import sys
Expand Down Expand Up @@ -215,13 +216,81 @@
MimeType=x-scheme-handler/{DEADLINE_URL_SCHEME_NAME}
"""

mimeapps_file_content = f"""[Default Applications]
x-scheme-handler/{DEADLINE_URL_SCHEME_NAME}={DEADLINE_URL_SCHEME_NAME}.desktop;
"""
with open(desktop_file_path, "w") as desktop_file:
desktop_file.write(desktop_file_content)
with open(mimeapps_list_file_path, "w") as mimeapps_list_file:
mimeapps_list_file.write(mimeapps_file_content)

# Read/parse any existing mimeapps.list and add/update ONLY the deadline
# handler entry, preserving all other default-application associations.
# Opening in "w" mode would truncate the file and destroy unrelated
# associations (browser, PDF, mailto, etc.), causing permanent data loss.
# interpolation=None avoids treating "%" in values specially, and
# optionxform=str preserves the case of mime-type/scheme keys.
# strict=False tolerates duplicate keys/sections (last value wins),
# which real-world mimeapps.list files written by other desktop tools
# have historically contained; the default strict=True would turn an
# otherwise-usable file into a hard install failure.
mimeapps = configparser.ConfigParser(interpolation=None, strict=False)
mimeapps.optionxform = str # type: ignore[assignment,method-assign]

if os.path.isfile(mimeapps_list_file_path):
# A pre-existing mimeapps.list that is not valid INI can still make
# configparser raise even with strict=False (MissingSectionHeaderError
# for a key before any section header, UnicodeDecodeError for a
# non-text file, ...). Surface these as a DeadlineOperationError for
# consistency with the rest of this function rather than crashing the
# CLI with a raw traceback.
try:
mimeapps.read(mimeapps_list_file_path)
except (configparser.Error, UnicodeDecodeError) as e:
raise DeadlineOperationError(
f"Failed to install the handler for {DEADLINE_URL_SCHEME_NAME} URLs: "
f"could not parse existing {mimeapps_list_file_path}:\n{e}"
) from e

if not mimeapps.has_section("Default Applications"):
mimeapps.add_section("Default Applications")
mimeapps.set(
"Default Applications",
f"x-scheme-handler/{DEADLINE_URL_SCHEME_NAME}",
f"{DEADLINE_URL_SCHEME_NAME}.desktop;",
)

# Write atomically: rendering to a temp file in the same directory and
# os.replace()-ing it into place means the original mimeapps.list is only
# replaced once the new content is fully and successfully written. Opening
# the destination directly in "w" mode would truncate it before write()
# runs, so a crash/kill/disk-full mid-write would leave it empty or partial
# -- losing exactly the unrelated associations this change protects.
import stat
import tempfile

# tempfile.mkstemp() creates the temp file with mode 0600, and os.replace()
# keeps that mode on the final file. That would silently drop the
# permissions of a pre-existing mimeapps.list, and for the all_users case
# (/usr/share/applications/mimeapps.list, written as root) would leave a
# system-wide file that other users' desktop environments cannot read.
# Preserve the existing file's mode, or default to 0644 for a new file.
if os.path.isfile(mimeapps_list_file_path):
mimeapps_list_file_mode = stat.S_IMODE(os.stat(mimeapps_list_file_path).st_mode)
else:
mimeapps_list_file_mode = 0o644

dir_name = os.path.dirname(mimeapps_list_file_path)
fd, tmp_path = tempfile.mkstemp(dir=dir_name, prefix=".mimeapps.list.", suffix=".tmp")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The atomic-write here does not preserve the destination file's permissions. tempfile.mkstemp() creates the temp file with mode 0600 (owner read/write only), and os.replace() keeps that mode on the final mimeapps.list.

For the all_users case (/usr/share/applications/mimeapps.list, written as root) this is a functional regression: the system-wide file needs to be world-readable so every user's desktop environment can resolve associations, but after this change it becomes root-only (0600) — and it silently drops the permissions on a pre-existing file that was previously e.g. 0644.

Consider stat-ing the existing file first (if present) and applying its mode to the temp file before os.replace(), or applying a sane default honoring umask (e.g. 0644) when the file is new. mkstemp's hardcoded 0600 is rarely what you want for a shared config file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 08ec80f: the existing file's mode is stat()ed before the rewrite and applied to the temp file via os.chmod() before os.replace(); a newly created mimeapps.list defaults to 0644. Covered by test_linux_install_preserves_mimeapps_permissions and test_linux_install_creates_mimeapps_with_default_permissions.

try:
with os.fdopen(fd, "w") as tmp_file:
mimeapps.write(tmp_file, space_around_delimiters=False)
os.chmod(tmp_path, mimeapps_list_file_mode)
Comment thread
crowecawcaw marked this conversation as resolved.
Dismissed
os.replace(tmp_path, mimeapps_list_file_path)
except BaseException:
try:
os.unlink(tmp_path)
except OSError:
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
# Best-effort temp file cleanup; the original exception below is
# the actual failure to surface, and a leftover temp file is
# harmless compared to masking it.
pass
raise

try:
subprocess.run(["update-desktop-database", entry_dir], check=True)
Expand Down
Loading
Loading