Skip to content

Add macOS menu bar application support#11

Open
Yeraze wants to merge 1 commit into
mainfrom
feature/macos-support
Open

Add macOS menu bar application support#11
Yeraze wants to merge 1 commit into
mainfrom
feature/macos-support

Conversation

@Yeraze

@Yeraze Yeraze commented Apr 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds a native macOS menu bar app (pystray) for the BLE bridge, with AppleScript-based settings dialogs and BLE device scan/select
  • Ships full packaging: PyInstaller spec, Info.plist, entitlements, and build.sh / notarize.sh for codesigning + notarization
  • Adds release-macos.yml workflow (signed/notarized releases) and extends test.yml with macOS build checks; includes docs/MACOS_GUI.md user guide

Test plan

  • CI macOS build job passes on this PR
  • Local build/macos/build.sh produces a working .app bundle
  • Menu bar icon appears and responds to clicks
  • Settings dialog: BLE scan lists devices, selection persists
  • Bridge connects to selected BLE device and serves TCP on 0.0.0.0:4403
  • Icon turns red on connection failure (parity with Windows GUI behavior)
  • notarize.sh dry-run validates signing config (smoke test only unless creds available)

🤖 Generated with Claude Code

- Add macOS GUI with menu bar icon using pystray
- Native AppleScript dialogs for settings (Tkinter threading workaround)
- BLE device scanning with selection list
- Build system: PyInstaller spec, entitlements, Info.plist
- Build scripts: build.sh, notarize.sh for signing/notarization
- CI/CD: PR build tests, release workflow with code signing
- Documentation: MACOS_GUI.md user guide, build README
- Update main README with macOS platform support

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Apr 22, 2026

Copy link
Copy Markdown

Code Review: Add macOS Menu Bar Application Support

Great addition overall — macOS parity with the Windows GUI is a meaningful feature, and the packaging (DMG, signing, notarization CI) is thorough. Several issues worth addressing before merge.

Security: AppleScript Injection

src/gui/macos_tray_app.py

User-controlled strings are interpolated directly into AppleScript via f-strings with no escaping. If a message or title contains double-quotes, the AppleScript string breaks. More critically, device names from BLE scan are inserted into AppleScript like this:

device_list = join([name + " (" + addr + ")" for name, addr in devices])

A maliciously named BLE device could inject into the AppleScript string and potentially execute arbitrary shell commands via do shell script.

Fix: Sanitize all strings before AppleScript interpolation. At minimum escape backslashes and double-quotes before inserting any variable into an AppleScript string.


Dead Code: SettingsDialog (Tkinter) Is Never Used

src/gui/macos_settings_dialog.py (454 lines) and TrayApplication._on_settings_save() are unreachable. _show_settings() always spawns _show_settings_applescript() in a thread — the Tkinter dialog is never instantiated.

This is presumably because Tkinter needs the main thread, which pystray owns on macOS. Either remove macos_settings_dialog.py if the AppleScript path is the intended approach, or document why both exist.


Missing Cache Settings in AppleScript Flow

_show_settings_applescript() updates ble_address, tcp_port, and autostart — but silently omits cache_enabled and max_cache_nodes. There is no way to configure caching through the live code path.


CI: Massive Duplication in release-macos.yml

build-macos-arm64 and build-macos-x64 are approximately 250 line jobs differing only in runs-on and the architecture suffix. A matrix strategy would eliminate this:

strategy:
  matrix:
    include:
      - arch: arm64
        runner: macos-latest
      - arch: x64
        runner: macos-13

pyinstaller Not Version-Pinned

The workflow runs pip install pyinstaller without a version. PyInstaller major versions have broken existing specs before. Pin it in requirements-macos.txt.


Third-Party Action Not Pinned to SHA

softprops/action-gh-release@v2 uses a floating tag. Pinning to a full commit SHA is the security best practice for GitHub Actions workflows.


Redundant Imports Inside Functions

subprocess is imported at module level in macos_tray_app.py but re-imported inside two functions. re is also imported inside _show_settings_applescript(). Move these to module level.


What Works Well

  • Keychain management in CI is correct (ephemeral keychain, runner-temp path).
  • Async/threading split — event loop in daemon thread with run_coroutine_threadsafe — is the right pattern for this kind of GUI.
  • _scan_devices_thread() in the settings dialog correctly uses root.after(0, ...) for thread-safe UI updates.
  • UTF-8 logging to ~/.meshtastic-bridge/bridge.log is consistent with the rest of the project.
  • entitlements.plist correctly covers Bluetooth, network server/client, and Python runtime flags.
  • Icon state (green/gray/red) and reconnection-failure callback are well-wired.

Summary

Priority Issue
High AppleScript injection via unescaped user/device data
Medium Dead code: Tkinter SettingsDialog is never shown
Medium Cache settings not configurable through any live code path
Medium ~250 line CI duplication; pyinstaller unpinned; action not SHA-pinned
Low Redundant imports inside functions

The core bridge logic and async architecture look solid. The injection risk in AppleScript interpolation is the main item to fix before merge.

Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant