TransferService: harden OBEX incoming-file handling (7 audit findings)

[geraldo-netto]

Summary

Hardens blueman/plugins/applet/TransferService.py — the OBEX incoming-file
(OPP) plugin. Started from 7 TODO.md rescan findings scoped to this file; a
follow-up review surfaced 4 more. 11 findings total, one commit per finding,
each with tests. Module statement coverage is now 100% (50 unit + fuzz cases).

No behavioral change for the happy path; every change either closes a security/
correctness gap or makes existing intent actually hold.

Findings

sec-1 — Pango markup injection in the invalid-dir notification (security)

• What: the "incoming files directory does not exist" notification interpolated the user-configured shared-path and the fallback path into <b>%s</b> markup without escaping.
• Why it matters: a path containing markup/entities (<, &, quotes) could alter the notification body; daemons that render body markup could misinterpret it.
• Fix: html.escape both interpolated paths before formatting. Regression tests with <b>, &, and quote characters.

i18n-3 — Untranslated action label (i18n)

• What: the notification's "Reset to default" action used a bare English string.
• Why: it never localized.
• Fix: wrap in _() (file already in po/POTFILES.in, so it's extracted). Test asserts the label routes through gettext.

rel-10 — Wrong device revoked by removal timeout (reliability)

• What: the 60s timeout revoking a one-shot OPP authorization read self._pending_transfer['address'] when it fired, not when scheduled.
• Why: an overlapping push or cleared state could revoke the wrong device or trip the assert.
• Fix: capture the accepted address as a closure default argument bound at schedule time; _allowed_devices is now a set revoked with discard so a double-fire can't raise. Tests cover correct revoke, idempotency, and overlap.

sm-8 — Single pending slot clobbered by overlapping pushes (scalability/concurrency)

• What: one _pending_transfer slot held the in-flight authorization, so a second push arriving before the user answered overwrote the state the first notification's callback read.
• Why: the wrong file could be accepted, or the action could fail.
• Fix: key pending records by transfer_path in a dict; bind each notification callback to its own immutable record (default-arg capture); the callback pops only its own record. Test drives two overlapping requests and asserts independence.

data-1 — Same-second filename collision can overwrite (data integrity)

• What: collision handling prefixed only a second-resolution timestamp, then moved without rechecking the timestamped destination.
• Why: two same-named transfers completing in the same second could collide and overwrite/fail.
• Fix: new reserve_destination walks name, timestamp_name, timestamp_1_name, … and reserves the first free candidate with an O_EXCL create, then moves the source onto the reservation; the placeholder is cleaned up if the move fails. Same-second and fuzz tests over odd filenames (spaces, unicode, dotfiles, long names).

dec-1 — Agent coupled to the whole applet (decoupling/SOLID)

• What: the OBEX Agent took the entire BluemanApplet only to reach parent.Manager.get_adapter/find_device for the device name + trust state.
• Why: unnecessary coupling to the applet object graph; hard to test.
• Fix: inject a DeviceResolver callable (source, address) -> (name, trusted); the plugin supplies one bound to its parent.Manager, and the Agent depends only on that. Drops the BluemanApplet import. The resolver's raise→untrusted-fallback contract is documented on the type alias. Authorization is now testable with a 1-line stub.

test-3 — No focused tests for authorization/completion (test coverage)

• What: the authorization and completion paths had no targeted tests.
• Fix: unit + fuzz tests for overlapping requests, allowed-device expiry, filename collisions, failed moves, counters, session summaries, auto-accept, and share-path resolution.

---

A — Transfer counters never reset (reliability) [follow-up]

• What: _on_session_removed reported _silent_transfers/_normal_transfers but never zeroed them.
• Why: every session after the first double-counted background transfers and reported wrong totals.
• Fix: reset both counters once the summary has consumed them. Regression test.

B — Class-level mutable _handlerids (correctness) [follow-up]

• What: _handlerids = [] was a class attribute shared by all instances; _on_dbus_name_appeared appended to the class list before any instance copy existed.
• Why: latent cross-instance state bleed.
• Fix: initialize self._handlerids = [] per-instance in on_load. Test asserts the instance list is distinct from the class attribute.

C — Redundant elif not success (code clarity) [follow-up]

• What/Fix: success is a bool; if success / elif not success collapsed to if/else.

D — Path(None) crash when XDG has no download dir (robustness) [follow-up]

• What: _make_share_path built Path(GLib.get_user_special_dir(DOWNLOAD)) unconditionally; when XDG returns no dir this raised TypeError on Path(None) before the existing ~ fallback could run — making that fallback dead code.
• Why: crash on systems without an XDG download dir.
• Fix: build default_path only when XDG yields a value, so the ~ fallback applies. Test patches XDG to None.

Unrelated CI fix — flaky test_resolver_changed

Adding this PR's tests pushed the suite to 392 tests and shifted timing enough to expose a pre-existing flake in test/main/test_dns_server_provider.py (not part of the OPP hardening). CI failed on it for test (3.12) and test (3.14):

FAIL: test_resolver_changed
AssertionError: expected call not found.
Expected: mock(<DNSServerProvider ...>)
  Actual: not called.

• Root cause: _test_changed truncated the watched resolver file, then drained only already-queued GLib events (while context.pending(): context.iteration()). The Gio.FileMonitor CHANGED event is delivered asynchronously — it was frequently not queued yet when the drain ran, so the changed signal looked like it never fired. The systemd-resolved variant emits synchronously over D-Bus, so only the file-monitor variant raced.
• Fix: block the main loop until the changed signal actually fires (context.iteration(may_block=True)), bounded by a 5s timeout backstop so a genuine miss still fails fast instead of hanging. Deterministic for both the file-monitor and D-Bus variants.

Kept as a standalone commit so the OPP hardening history stays per-finding.

Testing

• 50 unit/fuzz tests, all passing
• Module statement coverage 51% → 100%
• pycodestyle + pyflakes clean; mypy --strict clean on the module
• Test seam: BLUEMAN_SOURCE=1 python3 -m unittest test.plugins.applet.test_transfer_service

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud