Skip to content

Audit fixes and polish#5

Merged
MegaManSec merged 8 commits into
mainfrom
audit-fixes-and-polish
May 28, 2026
Merged

Audit fixes and polish#5
MegaManSec merged 8 commits into
mainfrom
audit-fixes-and-polish

Conversation

@MegaManSec
Copy link
Copy Markdown
Owner

Summary

  • Address the P0/P1 findings from the recent audit: handshake double-completion, command/data frame ambiguity, monotonic rate limiter, missing NSBonjourServices (broke discovery on macOS 14+), unchecked-in Xcode scheme, release pipeline gating against a v0.0.0 overwrite, Bonjour TOFU pin, and BluetoothManager state surfacing.
  • Fix two user-reported bugs: peripheral disconnects automatically a moment after "Connect to PC" (we weren't retaining IOBluetoothDevicePair), and the button title didn't update until you switched tabs (no observable state).
  • Add hover tooltips on every Settings button + tab label, gate the misleading "Notification Sent" toast on pair state, and add a Launch at Login toggle.

…ring

Every Settings button now has a `.help` tooltip explaining what it does,
including the four tab labels. The Notify and Sync buttons on the Device
tab are also disabled while unpaired, with a tooltip pointing the user at
the Pairing tab.

Previously, clicking Notify while unpaired called sendNotificationOverSecure
(which silently no-op'd on its isPaired guard) but the local "Notification
Sent" toast fired unconditionally, making the UI claim success. The toast
is now gated on isPaired, and a "Not Paired" toast is shown otherwise.
…ively

Two related bugs:

1. Clicking "Connect to PC" briefly connected the peripheral (correctly
   disconnecting it from the other Mac) but the local Mac dropped it a
   moment later. IOBluetoothDevicePair.start() is asynchronous, but the
   previous code treated it as sync and called openConnection() right
   after. The pair instance then went out of scope at end of closure and
   ARC freed it mid-flight, so macOS aborted the in-progress bond.

2. The button title ("Connect to PC" vs "Remove from PC") only refreshed
   when switching tabs. BluetoothPeripheral.isConnected was a plain
   computed property querying IOBluetoothDevice on each access -- not
   observable, so SwiftUI never knew to re-render.

Replace the computed isConnected with a published connectionStates dict on
BluetoothPeripheralStore, driven by the IOBluetoothDevicePairDelegate
callback and IOBluetoothDevice disconnect notifications. The store now
retains each in-flight IOBluetoothDevicePair in a pendingPairs dict until
devicePairingFinished fires, then calls openConnection() from there.
Peripherals that drop on their own (battery, range) flip back to
.disconnected reactively via the registered disconnect notification.

The row shows "Pairing..." (disabled) during the connecting state and
"Remove from PC" once connected -- both update without a tab switch.
…reading

Five P0/P1 correctness fixes from the earlier audit:

1. SecureChannel.performHandshake: single-shot completion latch.
   The handshake timeout calls connection.cancel(), which propagates as a
   connection error to in-flight sendRaw/receiveRaw and triggers their own
   completion path. Without the latch, the outer completion fires twice.

2. IncomingConnection.handleIncoming: when a command is awaiting its data
   frame, consume the next frame as data first. Previously the parser
   tried DeviceCommand(rawValue:) before checking lastReceivedCommand, so
   a payload that happens to be a valid command raw value (e.g.
   "OP_SUCCESS" as a notification body) was misinterpreted as a new
   command and the sync data was silently dropped.

3. RateLimiter: switch in-memory failure/block tracking to
   CACurrentMediaTime (monotonic) so a wall-clock jump forward cannot
   shorten an active block or reset the failure window. Persistence still
   uses Date, converted at the boundary on save/load.

4. PairingSettingsView.GenerateCodeSheet: regenerate the code on
   .onAppear so a cancelled-and-reopened sheet shows a fresh code, never
   a previously-displayed-but-discarded one.

5. AppDelegate (handleLeftClick / waitForDisconnection): replace the
   synchronous checkActualConnectionStatus (which queried IOBluetooth on
   the main thread and read peripherals across queues) with the new
   checkActualConnectionStatusAsync, which snapshots the peripheral list
   on main and runs the IOBluetooth queries on bluetoothQueue.
… scheme

macOS 14 requires apps that advertise or browse Bonjour services to
enumerate the service types in Info.plist under NSBonjourServices, or
discovery silently returns nothing. The project was relying on Xcode's
GENERATE_INFOPLIST_FILE + INFOPLIST_KEY_* build settings, which only
support scalar values; NSBonjourServices needs an array.

- Add a real Blue Switch/Info.plist containing NSBonjourServices as an
  array, the existing usage descriptions, and LSUIElement = true (the
  app already calls setActivationPolicy(.accessory) at launch — this
  marks it up front and avoids the dock-icon flash).
- Set INFOPLIST_FILE and GENERATE_INFOPLIST_FILE = NO in both Debug and
  Release configurations.

Also check in Blue Switch.xcodeproj/xcshareddata/xcschemes/Blue Switch.xcscheme.
The release workflow runs xcodebuild -scheme "Blue Switch", which until
now only worked because Xcode auto-generated a scheme on first GUI
launch. A fresh CI runner that has never opened the project would have
failed.
Two CI fixes from the audit:

1. release.yml: when there are no releasable commits, the version job
   used to fall back to "0.0.0", which then ran the full build/release
   pipeline and overwrote tag history with a v0.0.0 release. Now the
   build and release jobs short-circuit via `if:` when the dry-run
   produces no version. Also:
   - Add fetch-depth: 0 to the version checkout so semantic-release
     can see the full history.
   - Tighten the version extraction to match exactly X.Y.Z.
   - Add persist-credentials: false to the release checkout.

2. New ci.yml that builds Debug on every PR and on push to main.
   Previously only check-format ran on PRs; a PR that broke xcodebuild
   would only fail on the post-merge release pipeline. CODE_SIGNING is
   disabled since the CI runner has no Developer ID identity.
BluetoothManager was a stub: it only print()'d state transitions, so a
Mac that had Bluetooth off or had denied Bluetooth permission would
silently fail to switch peripherals with no user-visible feedback.

- Make BluetoothManager.state @published.
- AppDelegate subscribes via Combine and posts a UNNotification on
  transitions to .poweredOff, .unauthorized, or .unsupported.
- Hop CB delegate callbacks to main before mutating @published.

The transition filter avoids re-notifying on every delegate fire (the
initial .unknown → .poweredOn would otherwise spam at startup).
Previously a NetworkDevice was keyed only by its Bonjour name. An
attacker on the LAN who knew the PSK could publish a colliding name
and redirect outgoing commands to themselves; even without knowing the
PSK, a name collision would create UX confusion (whichever resolved
last won).

- ServicePublisher writes a `fp=<fingerprint>` TXT record on publish
  and re-publishes it whenever PairingStore.$fingerprint changes
  (pair/unpair).
- ServiceBrowser reads the TXT record on resolve and attaches the
  fingerprint to the discovered NetworkDevice.
- NetworkDevice gains an optional `fingerprint` field (back-compat
  decode via Codable's automatic Optional handling — old persisted
  records decode with fingerprint = nil).
- NetworkDevice.update applies the TOFU pin: if our stored fingerprint
  differs from the incoming one, the device is marked inactive and the
  routing info (host/port) is *not* updated.
- NetworkDeviceStore posts a user-visible "Identity Mismatch"
  notification when a registered device's fingerprint changes.

Since both legitimate peers share the same PSK they share the same
fingerprint; the defense is against attackers that don't know the PSK
but try to redirect by name collision.
Adds a "Launch at Login" toggle to the Other tab on macOS 13+ via
SMAppService.mainApp. The toggle reflects the current registration
state on view appearance and reverts itself if register/unregister
throws.
@MegaManSec MegaManSec merged commit 0f5a415 into main May 28, 2026
2 checks passed
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 2.0.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant