Re-validate directories by device/inode and robustly locate default file-manager .desktop

[assisted-by-ai]

Motivation

• Mitigate TOCTOU/path-swap attacks by recording stable directory metadata and re-checking directories immediately before launching the file manager.
• Avoid relying on possibly unset or manipulated XDG environment variables when locating the default file manager .desktop file.
• Improve URI-to-path handling by preserving only validated, accessible directories for presentation and launch.

Description

• Add an ApprovedDirectory @dataclass that stores a Path plus st_dev and st_ino, with from_path and resolve_if_unchanged helpers to detect changes and non-directories.
• Change get_path_list_from_uris to return a sorted list of ApprovedDirectory instances instead of raw Path objects, and build them only for absolute, readable, executable directories.
• Update FmShimWindow to accept list[ApprovedDirectory], display approved_dir.path in the UI, and re-resolve + verify inode/device before launching each directory; show an error and abort if any approved directory changed.
• Replace fragile XDG environment parsing with pwd.getpwuid(os.getuid()).pw_dir and search ~/.local/share, /usr/local/share, /usr/share for the default file manager .desktop file.
• Tidy error messages and ensure gio launch is invoked with the resolved, verified directory paths.

Testing

• Ran the project test suite via pytest, and the test run completed successfully with no regressions observed.
• Performed a smoke run of the application that exercised URI parsing, UI listing, and launching paths, and the re-validation logic correctly aborted on a simulated directory swap.
• Executed static checks with pylint and verified no new critical issues were introduced beyond existing suppressions.

---

Codex Task

[assisted-by-ai]

Security Review Findings
I reviewed the target module at usr/lib/python3/dist-packages/fm_shim_frontend/fm_shim_frontend.py#security-misc-shared and found two plausible security issues.

1. TOCTOU race lets an attacker swap the approved directory after validation
   The code resolves each URI to a canonical Path during parsing, checks that it is a readable directory, stores it, and later—after showing the confirmation dialog—only re-checks is_dir() before launching the file manager. It does not re-resolve the path or bind the approval to a stable inode/file descriptor. That means an attacker who controls a writable parent directory can replace the approved path with a symlink or different directory between validation and the user clicking Open, causing the file manager to open a different location than the one originally validated. This is a classic time-of-check/time-of-use issue. The initial validation happens in get_path_list_from_uris() and the later use happens in open_dir_list().

Impact: a malicious local program could present one safe-looking directory for approval and then switch it to a more sensitive or deceptive target before launch. The warning path in the else branch only catches “not a directory anymore”; it does not catch “still a directory, but now a different one.”

2. Untrusted environment variables influence which .desktop file gets executed
   When the user approves the action, the code asks xdg-mime for the default handler name, then searches for the corresponding .desktop file using XDG_DATA_HOME, HOME, and XDG_DATA_DIRS taken directly from the environment. The first matching file is then passed to gio launch.

This creates an untrusted search path / environment injection problem: if an attacker can influence the environment for this process and place a malicious .desktop file with the expected filename in a prepended search directory, clicking Open may launch the attacker’s chosen application instead of the real file manager. The filename is sanity-checked to avoid slashes, but the search roots themselves remain attacker-controlled through environment variables.

Impact: a malicious caller could potentially turn this confirmation dialog into a “launch arbitrary desktop entry after user click” primitive.

Added an ApprovedDirectory dataclass that stores the canonical directory path together with its device and inode, so the program can detect directory swaps between initial URI validation and the later launch step. This hardens the previous check/use gap around approved paths.

Updated the UI and launch path handling to work with ApprovedDirectory objects, display the original approved path, and re-resolve each directory immediately before gio launch, aborting with a warning if the path changed or stopped being a directory.

Removed reliance on environment-controlled XDG_DATA_HOME, HOME, and XDG_DATA_DIRS for desktop-file lookup, and now resolve the default file manager only from the current user’s passwd home plus trusted system locations (~/.local/share, /usr/local/share, /usr/share). This reduces the risk of launching an attacker-supplied .desktop file via manipulated environment variables.

Committed the fix on the current branch as 3e73901a and created the PR metadata as requested.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 660413ccd3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Keep desktop-file lookup aligned with the XDG data path

open_dir_list() still asks xdg-mime for the active inode/directory handler, but this new search list ignores XDG_DATA_HOME/XDG_DATA_DIRS and only probes ~/.local/share, /usr/local/share, and /usr/share. In sessions that add other application roots (for example Flatpak exports or per-user desktop overrides), xdg-mime can legitimately return a desktop ID whose file exists only in those configured XDG directories, so the frontend now exits with Cannot find default file manager's desktop file! even though the handler is valid.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Catch path races while snapshotting ApprovedDirectory

get_path_list_from_uris() validates each candidate with is_dir()/os.access(), but ApprovedDirectory.from_path() immediately performs an uncaught stat(). If a directory is deleted, replaced, or permission-flipped between those earlier checks and this line, FileNotFoundError/PermissionError will bubble out and abort the frontend instead of simply discarding the stale URI. That race window is new in this change, so the snapshot step needs the same OSError handling as the surrounding path validation.

Useful? React with 👍 / 👎.

[ArrayBolt3]

Added an ApprovedDirectory dataclass that stores the canonical directory path together with its device and inode, so the program can detect directory swaps between initial URI validation and the later launch step. This hardens the previous check/use gap around approved paths.

This is unnecessary overcomplication. Yes, an attacker could swap out a directory for another directory or a symlink to another directory, but... the same path is still opened. The only reason we check to make sure the directory is a directory before using gio launch is because some file managers (most notably PCManFM-Qt) will run a file with its default MIME handler if pointed at a file rather than a directory. As long as the directory is a directory, we can safely point the file manager at it. Whether the directory that is opened is the one the user expects is the user's responsibility to verify (and any attacker that can perform directory swaps of this sort probably can do much worse things to the system without this shim being involved anyway).

One could make an argument that maybe this kind of in-depth checking would alert users to something fishy going on if an attacker tries replacing one dir with another, but I don't think it helps enough to matter. It's also unclear how likely this mechanism is to fall victim to false positives or false negatives.

Removed reliance on environment-controlled XDG_DATA_HOME, HOME, and XDG_DATA_DIRS for desktop-file lookup, and now resolve the default file manager only from the current user’s passwd home plus trusted system locations (~/.local/share, /usr/local/share, /usr/share). This reduces the risk of launching an attacker-supplied .desktop file via manipulated environment variables.

This is wrong. The original implementation was compliant with the XDG Base Directory specification (or at least tried to be), the new implementation does its own thing that isn't necessarily going to be compatible with how the rest of the system works. Additionally, the environment we get comes from the systemd user manager and is therefore trusted. (Why is it trusted? Because if an attacker has the ability to change it, they can set LD_LIBRARY_PATH (and probably LD_PRELOAD too) and take over almost any application running under the systemd user. Therefore the systemd user manager must be protected from this kind of malicious modification, and therefore we can trust the environment we get from it.)

These are the only two things this PR meaningfully changes, so I think it can be rejected.