Skip to content

feat(app): LaunchDaemon mode with SMAppService XPC privileged helper#103

Open
martinmarsian wants to merge 93 commits into
nark:mainfrom
martinmarsian:feat/launch-daemon-xpc
Open

feat(app): LaunchDaemon mode with SMAppService XPC privileged helper#103
martinmarsian wants to merge 93 commits into
nark:mainfrom
martinmarsian:feat/launch-daemon-xpc

Conversation

@martinmarsian

Copy link
Copy Markdown

Summary

This PR adds LaunchDaemon support to WiredServerApp, allowing the Wired 3 server to run as a system service that starts at boot — independently of any user session. The centrepiece is a new WiredServerHelper privileged XPC tool installed via SMAppService.daemon, which eliminates the need for password prompts or Touch ID for any admin operation.


What's included

1. WiredServerHelper — new SMAppService XPC daemon target

A new WiredServerHelper SPM executableTarget (Sources/WiredServerHelper/) is a lightweight privileged XPC helper that the app installs once via SMAppService. It handles every operation that requires elevated access:

XPC method What it does
createSystemDirectory Creates /Library/Wired3/ with correct ownership
migrateDataToSystemDirectory Copies user data to /Library/Wired3/ (upgrade path)
writeLaunchDaemonPlist / removeLaunchDaemonPlist Manages /Library/LaunchDaemons/fr.read-write.WiredServer3.plist
startDaemon / stopDaemon Calls launchctl bootstrap/bootout
reloadDaemonConfig Sends SIGHUP to the running daemon process
runFDACheck Tests read access to the files directory from the helper's context
setPosixPermissions Sets ownership/mode on server-managed paths
getDaemonPid Reads the PID file as the privileged user

The helper's launchd plist is embedded in the Mach-O __TEXT __info_plist section (via linkerSettings in Package.swift) so SMD can read MachServices and SMAuthorizedClients without a separate file on disk.

Signing note: SMAuthorizedClients and SMPrivilegedExecutables use Team ID VGB467J8DZ. The helper binary must be signed with the same Team ID and placed under Contents/Library/LaunchServices/ in the app bundle — standard Xcode project setup.

2. HelperConnection — async/await XPC lifecycle manager

Sources/WiredServerApp/HelperConnection.swift manages the full SMAppService lifecycle: detects whether the helper is installed, installs/upgrades it automatically, and exposes all XPC calls as async throws functions. A version mismatch triggers an automatic reinstall.

3. LaunchDaemon mode in the app

  • New installMode preference: .launchAgent (default, unchanged behaviour) or .launchDaemon (new).
  • In daemon mode, data lives at /Library/Wired3/; a one-time migration copies existing user data.
  • New Advanced settings pane: mode toggle, daemon user/group fields, Full Disk Access status + re-check, daemon start/stop controls.
  • GeneralTabView simplified — no duplicate controls.
  • External-volume warning when the files directory is on a volume that may not be mounted at boot.

4. Touch ID removed

Touch ID was added before the XPC helper as a workaround for password prompts. With the helper in place it is entirely obsolete and has been removed.

5. ServerIdentity — world-readable .pub companion file

On key generation or first load, wired-identity.pub (0644) is written alongside the private key (0600), containing the raw 64-byte P256 public key. New fingerprintFromPublicKeyFile(at:) lets the GUI read the fingerprint as a different OS user (logged-in user vs. _wired daemon) without accessing the private key.

6. Localisation

~55 new keys in en, de, and fr Localizable.strings for all new UI strings.


Test plan

  • Build: both WiredServerApp and WiredServerHelper targets compile cleanly
  • LaunchAgent mode (default): start/stop works as before — no regression
  • LaunchDaemon mode: toggle in Advanced, migrate data, confirm server starts at boot as _wired
  • Helper version bump: increment kHelperVersion, confirm automatic reinstall
  • FDA check: verify status row with and without Full Disk Access granted
  • .pub file: delete it, restart server, confirm re-created with 0644
  • Fingerprint display: in daemon mode (different user), fingerprint shows via .pub
  • Localisations: switch to DE and FR, verify no missing strings

🤖 Generated with Claude Code

Professor© and others added 30 commits April 24, 2026 14:24
Adds a complete migration path from Wired 2.5 SQLite databases into
the Wired 3 GRDB schema, covering accounts, privileges, bans, boards,
threads and posts.

Server CLI:
- New --migrate-from <path> flag in main.swift
- MigrationController: migrates groups+privileges, users+privileges,
  bans, boards (Unix mode → 6 permission booleans), threads, posts
  (BLOB icon support, date string → Unix epoch conversion)
- DatabaseController: expose dbQueue as public for migration access

WiredServerApp (macOS GUI):
- New "Migration" tab in preferences
- Source path picker + overwrite toggle + progress output view
- Subprocess runs wired3 --migrate-from via temp-file output capture
  (avoids pipe buffering / race-condition issues)
- Localizable strings added (de/en/fr)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Accounts migrated from Wired 2.5 have password_salt = NULL and a 40-char
SHA1 hash stored in the password column. The P7 key exchange previously
always derived the auth proof from SHA256(plaintext), causing these accounts
to fail authentication entirely.

P7 protocol (P7 v1.4 additions):
- New field p7.encryption.legacy_password (bool, id=22) in server_challenge:
  server sets this when password.count==40 && salt==nil
- New field wired.user.password_must_change (bool, id=3016) in wired.login:
  signals the client to prompt for a password change immediately

P7Socket (server side):
- Detect legacy users after username_request; send legacy_password=true
  in server_challenge and set isLegacyAuth=true after ECDSA verify passes
- Server-side base_hash uses the stored SHA1 string unchanged (no re-derive)

P7Socket (client side):
- Defer password hashing until after server_challenge is received
- Use SHA1 when legacy_password=true, SHA256 otherwise
- Set isLegacyAuth=true for downstream consumers

ServerController+Auth (wired.send_login handler):
- For legacy sessions (socket.isLegacyAuth): skip the redundant SHA256
  equality check (P7 ECDSA already verified the SHA1 credential)
- Lazily assign a new per-user stored_salt on first legacy login
- Include wired.user.password_must_change=true in wired.login reply

Connection (client library):
- New connectionRequiresPasswordChange(connection:) delegate method
- Called after login when wired.user.password_must_change is true

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The FTS5 table uses content='index' (external content). On restart after
a crash, old rows in the `index` table may have no matching FTS5 entries.
The row-level delete trigger then sends a 'delete' command for a non-
existent FTS5 rowid, causing SQLITE_IOERR (error 10) during cleanup.

Fix: in performFullRebuild, drop the index_ad trigger before the bulk
delete, then call FTS5 'rebuild' to re-sync the index from the current
content table, then restore the trigger. This is more correct than
relying on row-level triggers for bulk operations.

Add recoverIndexAndFTS5() as a fallback: drops FTS5 and all triggers,
clears the index table, and recreates everything from scratch. A
pendingRebuild flag ensures a fresh traversal follows automatically.

Also: auto-load notary profile from ~/.wired-notary in release scripts
so notarization works without manually exporting NOTARY_PROFILE each time.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two changes to prevent SQLITE_IOERR on the first index rebuild after a
crash or unclean shutdown:

1. DatabaseController: run PRAGMA wal_checkpoint(TRUNCATE) right after
   opening the database. On a fresh start there are no active readers,
   so TRUNCATE safely merges and clears the WAL. This eliminates the
   root cause: accumulated partial FTS5 shadow-table writes from failed
   previous rebuild cycles sitting in the WAL.

2. IndexController: before the first rebuild traversal, probe FTS5 with
   an integrity-check write. If it throws (SQLITE_IOERR or any other
   error), recoverIndexAndFTS5() is called to drop and recreate the FTS5
   virtual table and triggers from scratch, then a fresh rebuild follows.
   This acts as a belt-and-suspenders guard for cases where the main DB
   file itself has FTS5 inconsistencies.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The regex previously rejected versions like "3.0-beta.21" because it
only matched numeric dot-separated segments. Allow an optional
pre-release suffix (-beta.N, -rc.N, -alpha.N, etc.).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Assigning the salt on the very first legacy login caused a permanent
lockout on the second login: passwordSalt != nil triggered the SHA-256
auth path, which failed against the still-SHA1 stored hash.

The salt is now written only in receiveAccountChangePassword, i.e. when
the user actually completes the password-change flow.

Fixes: nark#72

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…B column

The old detection (password.count == 40 && passwordSalt == nil) was fragile
and would produce false positives in edge cases. A dedicated is_legacy boolean
column on the users table is the authoritative source of truth:

- DB migration v12: adds is_legacy (default false), back-fills existing SHA1
  accounts (no salt + 40-char hash) automatically
- User model: new isLegacy property with CodingKey is_legacy
- SocketPasswordDelegate: new isLegacyUser(username:) method replaces the
  length check in P7Socket.serverKeyExchange
- UsersController: implements isLegacyUser(username:)
- MigrationController: sets is_legacy=1 for all Wired 2.5 imported accounts
- receiveAccountChangePassword: clears is_legacy=false on successful upgrade

Addresses nark's review of PR nark#72.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- MigrationController: replace raw table/column string interpolation in
  migratePrivileges() with a closed PrivilegeTarget enum (allowlist)
- WiredServerViewModel.runMigration(): replace DispatchQueue.global/main
  mixing with withCheckedContinuation + withTaskGroup; add 5-minute timeout
  so isMigrating can never be stuck true forever if the subprocess hangs
- migrate-from-wired25.py: document that boards/threads/posts are out of
  scope; point users to wired3 --migrate-from for full content migration
- Add MigrationControllerTests: 8 tests covering is_legacy flag, skip/
  overwrite behaviour, legacy auth detection, and password upgrade flow

Addresses nark's minor review points on PR nark#72.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When recoverIndexAndFTS5() fails, hasFTS5 is set to false but the
index_ad trigger remains in the database. On the next rebuild the
trigger drop was skipped (gated on hasFTS5), the trigger fired during
the bulk DELETE, hit the corrupt FTS5 shadow tables, and propagated
SQLITE_IOERR (10) back to the DELETE statement.

DROP TRIGGER IF EXISTS index_ad is now unconditional so the trigger
is always gone before the bulk delete, regardless of hasFTS5 state.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…offline user list

- wired.xml: add fields wired.user.is_offline (3017), wired.message.offline.{sender_login,recipient_login,date} (5010-5012); add messages wired.user.offline_list/done (3011-3012), wired.message.send_offline_message/offline_message (5010-5011)
- DB migration v13: replace speculative E2E-crypto offline_messages schema with a simple plaintext table (sender_login, recipient_login, body, sent_at) with per-recipient limit trigger
- OfflineMessage.swift: new GRDB model for the offline_messages table
- ServerController+Chat: receiveMessageSendOfflineMessage, deliverOfflineMessages, sendOfflineUserList
- ServerController+Auth: call sendOfflineUserList + deliverOfflineMessages after successful login
- ServerController: route wired.message.send_offline_message to new handler
- ClientsController: add allConnectedLogins() and user(withLogin:)
- UsersController: add userExists(withUsername:)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, privilege backfill

- Migration v14: add last_login_at to users; update on every successful login
- sendOfflineUserList: filter to users active in last 30 days; send full_name as nick
- broadcastNewOfflineUser: send full_name as nick when new account is created
- migrateSendOfflineMessagesPrivilegeIfNeeded: backfill privilege to existing accounts
  that already have send_messages, so upgrading installations get it automatically

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… list

- Migration v15: add last_nick TEXT column to users table
- receiveUserSetNick: persist nick to DB whenever a logged-in user changes it
- Login path: also persist nick at login time (nick is sent before login completes)
- sendOfflineUserList: prefer last_nick, fall back to full_name, then username

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… user list

The IS NULL exception caused all accounts without a last_login_at (everyone
after a fresh upgrade) to appear. Now only accounts with a real login within
the last 30 days are shown.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gin or full_name

Using login or full_name as fallback was a security issue — it revealed account
names to other users. Now only users who have connected at least once (and thus
have a last_nick persisted) appear in the offline list. New accounts are excluded
until their first login, at which point their self-chosen nick is saved.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ients

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Poly1305

- Migration v16: adds is_encrypted column to offline_messages
- wired.xml: fields 5014/5015 and messages 3013/3014/3015 for key exchange
- Server stores and delivers is_encrypted flag with each offline message
- Handler for wired.user.set_public_key stores X25519 public key in users.offline_public_key
- Handler for wired.user.get_public_key returns stored public key by login

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GRDB stores Date values as ISO 8601 text ("yyyy-MM-dd HH:mm:ss.SSS"),
not as Unix timestamps. Reading sent_at via Date(timeIntervalSince1970:)
forced a Double conversion that fatalError'd on text values, crashing
the server whenever a user with pending offline messages logged in.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ry/Wired3/

- Persist workingDirectory in UserDefaults so it survives restarts after migration
- Add migrateToSystemDirectory() — stops server, creates /Library/Wired3/ with
  admin privileges (osascript), copies all data, verifies DB integrity (PRAGMA
  integrity_check), switches active path, rewrites LaunchAgent plist if needed
- Original data preserved as backup at ~/Library/Application Support/Wired3/
- Add systemMigrationAvailable, isUsingSystemDirectory computed properties
- Add WiredServerError cases: systemDirectoryCreationFailed, systemMigrationFailed
- Dashboard General tab: new "System Data Directory" section with migration button,
  progress indicator, and status line; shows green checkmark when already migrated

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add ServerInstallMode enum (launchAgent / launchDaemon), persisted in UserDefaults
- Add daemonUserName (default: _wired) and daemonStartAtBoot, both persisted
- switchInstallMode(): stops server, creates hidden system user via dscl (osascript),
  sets /Library/Wired3/ ownership, writes LaunchDaemon plist to /Library/LaunchDaemons/
  (via temp file + osascript cp with admin), or reverses all steps when switching back
- startDaemon() / stopDaemon(): launchctl bootstrap/bootout for system domain
- toggleDaemonStartAtBoot(): rewrites plist with updated RunAtLoad flag
- findFreeSystemUID(): auto-assigns UID from range 400-499
- Dashboard General tab: new "Install Mode" section with Picker, system user field
  with existence indicator, progress/status display; Execution section switches
  between LaunchAgent (Start at Login) and LaunchDaemon (Start at Boot) UI
- New WiredServerError cases: systemDirectoryOwnershipFailed, daemonUserCreationFailed,
  launchDaemonWriteFailed, launchDaemonInstallFailed, launchDaemonRemoveFailed

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- activateLaunchDaemon(): single osascript call — user creation (optional) +
  chown /Library/Wired3 + plist copy to /Library/LaunchDaemons/ (one admin dialog)
- deactivateLaunchDaemon(): single osascript call — bootout + plist removal +
  chown back to current user (one admin dialog)
- reinstallDaemonPlist(): single osascript call for updating existing plist
  (used by toggleDaemonStartAtBoot)
- Extract runPrivileged() helper to avoid repeating osascript boilerplate

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add daemonGroupName (default: staff), persisted in UserDefaults
- Add daemonGroupExists computed property
- findFreeSystemGID(): auto-assigns GID from range 400-499
- groupGID(for:): reads PrimaryGroupID from dscl for existing groups
- activateLaunchDaemon: creates custom group via dscl if it doesn't exist,
  uses correct GID for user's PrimaryGroupID, uses group name in chown
- saveDaemonSettings() persists group name alongside user name
- Dashboard: side-by-side User/Group text fields each with existence indicator

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- filesDirectoryIsOnExternalVolume: detects if files dir is under /Volumes/
  using URLResourceKey.volumeURLKey
- wired3HasFullDiskAccess: queries TCC.db via sqlite3 for the wired3 binary's
  kTCCServiceSystemPolicyAllFiles entry (readable only if WiredServerApp also
  has FDA)
- openFullDiskAccessSettings(): opens System Settings Privacy_AllFiles pane
- ExternalVolumeWarningView: reusable warning banner — orange with
  "Open Settings…" button when FDA missing, green checkmark when granted
- Warning shown in "Install Mode" section (General tab) and in "Files" tab
  whenever LaunchDaemon mode is active and files dir is on external volume

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Use Color.primary/Color.orange explicitly in foregroundStyle ternary to
  resolve HierarchicalShapeStyle vs Color type mismatch
- Replace URLResourceValues.volumeURL (unavailable in SDK) with simple
  /Volumes/ path prefix check for external volume detection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fields were disabled when launchDaemonInstalled=true, making it
impossible to change user/group after the first mode switch. Now
the fields are always editable and shown regardless of install mode,
so user can configure them before or after switching to LaunchDaemon.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace daemonRunning/daemonUserExists/daemonGroupExists computed
  properties (which ran launchctl/dscl synchronously on the main thread
  during every SwiftUI view render) with cached @published properties
  isDaemonRunning/isDaemonUserExists/isDaemonGroupExists
- Update these via refreshDaemonStatus() called from pollState() only
  when in LaunchDaemon mode (once per second, not on every render)
- Fix startDaemon()/stopDaemon() to use admin privileges via osascript
  since launchctl bootstrap/bootout system requires root

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When in LaunchDaemon mode and the daemon is running, skip the automatic
binary synchronization in refreshAll(). launchd detects when the binary
file is replaced on disk and kills the running daemon process, which
appeared as a spurious server crash whenever the General Tab was opened.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Set chmod 775 on /Library/Wired3/bin/ after chown so admin user
  (in staff group) can update the wired3 binary without sudo
- Guard auto-update in refreshAll() with isWritableFile check so it
  silently skips when permissions aren't available (e.g. before the
  first mode switch has set the correct group-write bit)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e for admin

Data dirs under /Library/Wired3 are chowned to _wired:daemon.
bin/ is explicitly re-chowned to _wired:staff with 775 so the admin
user (member of staff, not daemon) can still update the binary.
Default daemonGroupName changed from staff to daemon.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nark nark closed this May 13, 2026
@nark nark reopened this May 13, 2026
@nark nark closed this May 13, 2026
@nark nark reopened this May 13, 2026
@nark nark closed this May 13, 2026
@nark nark reopened this May 13, 2026
@nark

nark commented May 13, 2026

Copy link
Copy Markdown
Owner

Hi @nark,

just a heads-up: I've reorganized all my open pull requests. I noticed you're using an automated Claude review system, so I wanted to make sure each PR is clean, atomic, and easy to review — one logical unit per PR, all based on current main.

The older PRs (#77, #81, #83, #84, #94, #95) were getting messy and hard to review, so I closed them and replaced them with six focused PRs (#98#103).

I'm sorry if this caused a flood of GitHub notifications — that wasn't my intention, and I appreciate your patience!

Hi @martinmarsian,

Thank you for cleaning this up — the cleaner it is, the easier the review will be.

And yes, I’m currently trying to automate part of the process using AI to prepare the review work and make things easier to move forward. It’s still not working as smoothly as I’d like, though, so I’m running a few tests at the moment. Sorry for the noise caused by all the PR open/close notifications.

@nark nark closed this May 13, 2026
@nark nark reopened this May 13, 2026
@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @nark's task in 2m 49s —— View job


Review PR #103 — feat(app): LaunchDaemon mode with SMAppService XPC privileged helper

  • Read context and key files
  • Security analysis: XPC helper, privileged operations, paths
  • Protocol compatibility analysis (wired.xml, COMPATIBILITY.md)
  • General quality and Swift code review
  • Wrote the final review

Hey @martinmarsian, nice PR! The XPC architecture is clean, the helper is well isolated, and parameter validation on the root side is solid. Here is my feedback.


🔴 Blocking

try! in ServerIdentity.signWithIdentity(data:) — daemon crash risk

Sources/WiredSwift/Crypto/ServerIdentity.swift:44:

let signature = try! privateKey.signature(for: data)

P256.Signing.PrivateKey.signature(for:) can throw in CryptoKit (failing secure element, OOM, etc.). A try! will crash the whole process — and this is the production daemon. The function already returns Data?, so the fix is immediate:

guard let sig = try? privateKey.signature(for: data) else { return nil }
return sig.rawRepresentation

Fix this →


🟡 Should fix

1. Info.plist version (5) ≠ kHelperVersion (6) — version mismatch

Sources/WiredServerHelper/Info.plist:7 has CFBundleVersion = 5 while kHelperVersion = "6" in WiredHelperProtocol.swift:4. SMAppService uses the bundle version to detect helper changes: without bumping it, launchd will not auto-replace the existing binary. These need to stay in sync.
Fix this →

2. deactivateDaemon always returns (true, "") even on error

Sources/WiredServerHelper/HelperDelegate.swift:175 — all intermediate commands (launchctl bootout, dscl -delete, chown -R) are invoked with @discardableResult and their errors are ignored. If dscl -delete fails (user already removed, or insufficient rights), the function still reports success. The caller deactivateDaemon in HelperConnection therefore cannot detect a partial deactivation.

3. Race condition when writing the private key

Sources/WiredSwift/Crypto/ServerIdentity.swift:74-79: Data.write(to:options:.atomic) first creates a temp file with permissions inherited from the umask (often 0644 on macOS), then setAttributes([.posixPermissions: 0o600]) is called separately. There is a brief window during which the private key is world-readable. The correct mitigation is to set permissions before writing, or to use a file descriptor opened with O_EXCL | O_CREAT and the right permissions from the start.


💡 Suggestions / questions

4. connectKeyExchange hardcodes false for isNewKey

Sources/WiredSwift/P7/P7Socket.swift:1382:

if !trustHandler(fp, false, strictIdentity) {

The isNewKey parameter of serverTrustHandler is documented as "true on the first connection" but is always false. The handler has to manage that logic itself via its own TOFU store — so the API is misleading. This is not a security bug (the ECDSA verification stays correct) but it deserves a clarifying comment or a reply here.

5. stopDaemonpkill -x wired3 kills every wired3 process

Sources/WiredServerHelper/HelperDelegate.swift:79: killing by process name can affect unrelated wired3 instances. Prefer killing by PID (via the PID file) when available.

6. copyBinary without code-signature verification

Sources/WiredServerHelper/HelperDelegate.swift:216-232: the source is only validated by absolute path with no ... The validity (code signing, expected hash) of the binary copied to /Library/Wired3/bin/wired3 is not checked. This is acceptable under the current trust model (only the app signed with the right Team ID can call the helper), but worth documenting.


✨ Nice work

  • Defensive XPC validation: isValidAccount, isValidLabel, mandatory path prefixes (/Library/, /Library/LaunchDaemons/) — excellent for a helper running as root.
  • Anti-enumeration: the dummy password UUID().uuidString.sha256() for non-existent accounts (P7Socket:1649) is a good practice.
  • Handshake timeouts: handshakeTimeout = 30s with enforceDeadline: true on every critical step — prevents thread-pool exhaustion.
  • Touch ID removal: the workaround is cleanly dropped, replaced by the XPC architecture that makes it obsolete.
  • wired-identity.pub (0644): elegant design for letting the GUI user display the fingerprint without exposing the private key to the daemon.
  • Protocol compatibility: every new wired.xml entry carries version="3.2", the root version is bumped, the CompatibilityTests.swift tests are in place — COMPATIBILITY.md is respected.
  • CompatibilityDiff: clean architecture for handling drift between minor versions without breaking sessions.
  • Wired 2.5 → 3 migration: well documented, with a clear warning about the SHA1 → SHA256 issue.

Great work overall @martinmarsian — the try! is the only real blocker, the rest is robustness polish. Once that's sorted, this PR is in great shape.

@nark

nark commented May 13, 2026

Copy link
Copy Markdown
Owner

@martinmarsian sorry if some of the email notifications came through in French — everything has been translated to English on GitHub now.

@nark nark added the claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) label May 13, 2026
Professor© and others added 2 commits May 13, 2026 11:38
Introduces WiredServerHelper, a privileged XPC daemon registered via
SMAppService (replacing the SMJobBless approach). The helper performs
all root-level operations on behalf of WiredServerApp:

- Create /Library/Wired3 system directory with correct ownership
- Create/delete daemon user and group via dscl
- Install the LaunchDaemon plist and set ownership/permissions
- Bootstrap and kickstart the LaunchDaemon via launchctl
- Bootout the LaunchDaemon; falls back to pkill for orphaned processes
- Copy the wired3 binary to /Library/Wired3/bin/wired3
- Run an FDA access check as the daemon user to validate TCC grants

Key changes vs the SMJobBless prototype:
- Removes SMAuthorizedClients restricted entitlement from helper (required
  an Apple provisioning profile; AMFI rejected the helper binary without one)
- Removes Label/MachServices/SMAuthorizedClients from helper Info.plist
- Authorization is now done via AssociatedBundleIdentifiers in SMAppService.plist
- $(AppBundlePath) in SMAppService.plist is expanded at build time via
  PlistBuddy before signing (macOS 26 SMD does not expand the variable)
- Bumps kHelperVersion to 6 to force reinstallation of running helpers
- Dashboard Start button calls startDaemon() in LaunchDaemon mode (was
  always calling startServer() / direct child process, making Stop a no-op)
- Adds ExternalVolumeWarningView with FDA status indicator and re-check button
- Adds German/English/French localizations for all new strings

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds --check-access <path> to the wired3 CLI. The option tests whether
the process can read the given directory (via contentsOfDirectory) and
exits 0 on success or 1 on failure, with no server startup.

WiredServerHelper uses this to verify Full Disk Access grants under the
daemon user's identity: it runs wired3 as the daemon user via sudo so
macOS TCC evaluates the grant for wired3's code signature, not root.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@martinmarsian martinmarsian force-pushed the feat/launch-daemon-xpc branch from e936718 to 4744780 Compare May 13, 2026 09:38
@github-actions github-actions Bot added claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) and removed claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) labels May 13, 2026
URL(fileURLWithPath:) is used to resolve the Info.plist path for the
WiredServerHelper linker flag. Foundation is not implicitly available
in Package.swift manifests — the explicit import is required.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) and removed claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) labels May 13, 2026
…nfo.plist version

- ServerIdentity.signWithIdentity: P256.Signing.PrivateKey.signature(for:) can throw
  (failing secure element, OOM); the force-try would crash the daemon. Use guard+try?
  instead — the function already returns Data? so nil on failure is the correct contract.
- Info.plist CFBundleVersion bumped from 5 → 6 to match kHelperVersion = "6".
  SMAppService uses the bundle version to detect helper binary changes; a mismatch
  means launchd will not replace the running helper after an app update.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) and removed claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) labels May 13, 2026
… errors

ServerIdentity: replace Data.write(.atomic)+setAttributes with FileManager.createFile
so the 0600 permission is set at inode creation time, eliminating the brief window
during which the temp file is world-readable at the default umask.

deactivateDaemon: launchctl bootout exit code was silently discarded, making it
impossible for the caller to detect a failed stop. Now returns (false, …) when
bootout exits non-zero and the job is still registered in the system domain.
dscl cleanup exits are logged via diagLog rather than dropped entirely.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) and removed claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug) labels May 13, 2026
@martinmarsian

Copy link
Copy Markdown
Author

Merge note for maintainer: This PR includes the snapshot trigger functionality from PR #102 (feat/snapshot-sigusr2) and extends it with XPC helper support for LaunchDaemon mode. If you merge both PRs, you will get a merge conflict in Tabs.swift and WiredServerViewModel.swift.

Resolution: keep the triggerManualSnapshot() implementation from this PR (PR #103) and discard the triggerSnapshotNow() from PR #102. The triggerManualSnapshot() function is a superset — it sends SIGUSR2 directly in app-mode and delegates to the XPC helper in daemon-mode.

In Tabs.swift keep the HStack with the inline confirmation text (from this PR); in WiredServerViewModel.swift keep triggerManualSnapshot() + flashSnapshotConfirmed() and remove triggerSnapshotNow().

Suggested merge order: #102 first, then #103 (so git sees #103 as the later, superseding change).

@martinmarsian

Copy link
Copy Markdown
Author

Note on the Claude PR Review workflow: The claude-review.yml from this PR is now in your repo but the CLAUDE_CODE_OAUTH_TOKEN secret isn't configured there, which is why every PR triggers a failing CI run.

To fix this you have two options:

  1. Configure the secret — go to your repo → Settings → Secrets and variables → Actions → New repository secret, name it CLAUDE_CODE_OAUTH_TOKEN, and paste a valid Claude Code OAuth token (requires a Claude Pro or Teams subscription at claude.ai).

  2. Remove the workflow — if you don't want automated Claude reviews, simply delete .github/workflows/claude-review.yml and .github/workflows/claude.yml from your repo.

Sorry for the noise — I should have flagged this before submitting the PR.

@martinmarsian

Copy link
Copy Markdown
Author

Follow-up on the Claude review findings — all flagged items have been addressed in commits pushed after the review ran.

Issue Status Commit
🔴 try! in ServerIdentity.signWithIdentity(data:) ✅ Fixed — replaced with guard let sig = try? … b620b7d
🟡 Info.plist CFBundleVersion 5 ≠ kHelperVersion 6 ✅ Fixed — both now "6" b620b7d
🟡 Race condition — private key briefly world-readable ✅ Fixed — FileManager.createFile sets 0600 at inode creation 1ff9bf1
🟡 deactivateDaemon always returns (true, "") ✅ Improved — returns (false, …) when launchctl bootout fails with the job still registered; dscl exits are logged 1ff9bf1

The automated re-review can't fire (the CLAUDE_CODE_OAUTH_TOKEN secret isn't configured on this repo), so leaving this note manually. The branch is ready for another look when you get the chance.

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

Labels

claude: 🔴 blocking Claude review: à corriger avant merge (sécurité, bug)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants