feat(session): --lock-on-disconnect + always-on auto-unlock on reconnect

[antonmos]

Makes a headless macrdp Mac lock itself when the last RDP client leaves and unlock itself when a client comes back — so a remote-only Mac isn't left sitting unlocked between sessions, without stranding the operator outside it.

Default runtime path is unchanged when the new flag is off; auto-unlock only ever acts on a session that is already locked.

--lock-on-disconnect (opt-in, config LOCK_ON_DISCONNECT=1)

Locks the local session on a genuine last-client-disconnect, hooked into spawn_primary_overlay_watcher's existing disconnect edge — the same seam --restore-windows-on-disconnect uses.

The subtlety worth reviewing: SessionTracker also fires on blank-recovery self-heals, and its drop-then-ARC-reconnect can legitimately take ~12–15s — far longer than the watcher's 2.5s REACTIVATION_GRACE can distinguish from a real disconnect. So this adds its own cancellable safety buffer (default 22.5s, ~25s total, MACRDP_LOCK_ON_DISCONNECT_DELAY_MS) and skips the lock if the session comes back. It's a heuristic timer, documented as such — same posture as the auth_guard lockout and the blank-recovery detector.

Mechanism note: the widely-cited CGSession -suspend trick does not work on macOS 26 — Menu Extras/User.menu no longer exists (the first deployed build silently failed every lock). Uses open ScreenSaverEngine.app instead, which is only a true password-required lock if the account's screenLock delay is "immediate". macrdp deliberately does not check or change that setting.

Auto-unlock on reconnect (always-on, MACRDP_AUTO_UNLOCK=0 / AUTO_UNLOCK=0 opts out)

Types the account password into the lock screen when a client connects while locked — any lock, any cause, not just one this feature set. That is a deliberate security-posture choice worth an explicit look from a reviewer: an authenticated RDP connection can undo a lock someone set for an unrelated reason.

Reuses the credential PAM already validated at startup and that the connecting client independently proved via CredSSP — so no fresh keychain read, and it's skipped entirely under --skip-auth (where nothing was PAM-validated).

The mechanism is legitimate rather than a workaround: Apple's TN2150 scopes SecureEventInput to blocking observation (event taps / keyloggers), not synthetic injection, and this is the same thing Apple's own Screen Sharing / ARD do to unlock a screensaver-locked Mac remotely. It also resolves an apparent contradiction in our own docs — the "posting to the login window is blocked" note refers to the pre-session loginwindow.app, where no Accessibility grant exists yet, not the in-session screensaver lock this targets.

Two non-obvious behaviours drove the implementation (both live-diagnosed)

1. A bulk CGEventKeyboardSetUnicodeString fill leaves a secure password field INERT. The text displays, but the field's text-change bookkeeping never fires, so it ignores every subsequent Return — including a real Return from the physical keyboard. No settle delay or Return-retry can fix this. Fixed by typing real per-character keycode events, which required a reverse UCKeyTranslate lookup: KeyboardLayout::current() + reverse_map() in keyboard_layout.rs.
2. The lock screen consumes the first keystroke as a wake/focus event — this showed up as the password arriving one character short and being rejected. Fixed with a bare Shift tap, which can neither insert text nor submit. (Using Return as that wake submits an empty password and shakes the field.)

Safety properties

• Every character is resolved to a keystroke before typing anything, so an unmappable character aborts the attempt rather than submitting a partial password.
• Failures cap at 2 consecutive attempts per lock, with a compile-time assertion keeping that below macOS's 3-free-attempt PAM throttle (which escalates to multi-hour lockouts).
• On giving up it alerts loudly — sound + best-effort notification + error! log — rather than silently retrying into a lockout.
• Password material is never logged.

Also fixes: --shield-primary never engaged on a lid-closed MacBook

ShieldedPrimary::install treated "no physical panel in any state" as an error. A closed lid removes the built-in panel from the display list entirely — a step beyond the online-but-inactive case the existing single-panel fallback handles — so every connect failed with no physical display to shield.

Because the overlay watcher's error arm only warns and the RDP session works regardless, this was silently disabling the shield, --restore-windows-on-disconnect, AND the connect-time window gather (all live inside the same Ok(ovr) branch) with no user-visible symptom. Now treated as success: nothing is displaying a desktop, so there is nothing to shield, and install degrades to a no-op shield. Verified by inspection that empty targets flows cleanly through the mirror-break, SHOW-ack, arrangement, and Drop paths.

Testing

• cargo test 211 passing, cargo clippy --all-targets -- -D warnings clean.
• New unit tests: reverse_map_round_trips_password_characters (asserts letters/digits/symbols/space map on the US layout, and that every entry translates back to the character it's filed under — a wrong entry would mean typing a wrong password), plus auto-unlock env-parsing and lock-delay parsing tests.
• Live-verified end-to-end on macOS 26.5, lid-closed MacBook Air with --shield-primary: lock fires on disconnect, unlock succeeds on reconnect, and the shield path engages for the first time on this hardware.

Notes for the reviewer

• screen_is_locked() uses the undocumented CGSessionCopyCurrentDictionary() / CGSSessionScreenIsLocked. It's quarantined in virtual_display/private_api.rs alongside the existing CGS/SkyLight private calls, fails toward "not locked" (the safe direction), and the doc comment names the com.apple.screenIsLocked distributed-notification fallback if Apple ever removes it. Given CGSession itself vanished in macOS 26, this risk is real and called out deliberately.
• The three load-bearing pieces of the typing sequence (per-character keycodes, Shift wake, Backspace clear) each look redundant and each silently break the feature by submitting wrong passwords if removed. Both the code comments and docs/known-quirks.md carry the full five-round failure history and explicit "do not simplify this back" warnings.
• There is a documented, unavoidable residual race: between the screen_is_locked() check and the keystrokes, someone could unlock locally, and the password would land on the desktop instead. Mitigated by checking as late as possible, not eliminated.

🤖 Generated with Claude Code

[clintcan]

Thanks for this, and sorry it sat so long.

Verified before writing: cargo test 211 passed / 0 failed, cargo clippy --all-targets -- -D warnings clean, cargo fmt --check clean on both stable and nightly. Everything the description claims checks out, and the write-up made this much faster to review — the five-round failure history on the typing sequence especially.

Things I checked specifically and found right:

• the password stays Arc<Zeroizing<String>> end to end and is never cloned into a plain String
• the lock fires after drop(ovr) plus the safety buffer, so it sidesteps the documented "a --capture-primary Mac cannot be locked" trap rather than tripping over it
• reverse_map's or_insert ordering picks the simplest modifier combination, and dead keys are correctly excluded (they return Some(""), which the (Some(ch), None) pattern rejects)
• auto-unlock is skipped under --skip-auth

Request: split the --shield-primary lid-closed fix into its own PR

It's unrelated to lock/unlock and it's an unambiguous bug — on a lid-closed MacBook it silently disables the shield, --restore-windows-on-disconnect and the connect-time gather, all three living inside the same Ok(ovr) branch, with no user-visible symptom. I'd rather merge that on its own than have it wait on the discussion below.

Two things before the lock/unlock half merges

1. The Return retry defeats the attempt cap it is meant to respect.

AUTO_UNLOCK_MAX_CONSECUTIVE_FAILURES = 2 carries const _: () = assert!(… < 3), commented as keeping below macOS's three free attempts. But that counts calls to attempt_auto_unlock, and each call sends up to RETURN_ATTEMPTS = 3 Returns. The loop cannot distinguish "Return was ignored" from "Return submitted and was rejected" — both leave screen_is_locked() == true — so on a rejection it retries straight into the rejection. Worst case is six submissions where the assertion promises fewer than three.

That failure mode is the one I care about: a multi-hour PAM lockout on a machine whose whole purpose is being reachable remotely. Could the budget count submissions, shared across the lock, rather than calls?

2. Please put auto-unlock behind a flag.

Everything else in the project is opt-in and byte-identical when off — conventions.md says so, and its own sibling --lock-on-disconnect follows it. It matters more than usual here because the effect lands on the physical machine: once it fires, anyone standing at that Mac has a live desktop, and it will undo a lock it did not set and that someone may have set deliberately. MACRDP_AUTO_UNLOCK=0 is a quiet escape hatch for something with that blast radius.

Your reasoning for always-on is sound for the headless-mini case and I don't want to lose it — an --auto-unlock flag (or AUTO_UNLOCK=1 in config.env) keeps exactly that behaviour one line away for anyone who wants it.

Non-blocking

• Caps Lock looks unmodelled. reverse_map translates with caps: false, while translate() takes a caps parameter the RDP path uses. If the Mac's Caps Lock happens to be on, letters invert, the password is wrong, and an attempt burns. I have not confirmed whether a posted CGEvent inherits the hardware caps state — you have the live rig, so could you check? If it does, aborting fits your existing "resolve everything before typing, or abort" rule.
• A startup warning when sysadminctl -screenLock status isn't immediate. The caveat is documented, but the flag's name promises a lock and the failure is invisible. macrdp already warns about expired certs, --password in ps, and --restore-windows-on-disconnect without a headless mode.
• Mark it EXPERIMENTAL in the docs, like USB and camera redirection. It is verified on one machine, one macOS version, one keyboard layout, and rests on a private API plus a behaviour Apple can change — CGSession -suspend vanishing in macOS 26 is exactly that precedent, and you hit it.
• Rebase. The branch predates v0.9.7, and both docs/cli.md and docs/known-quirks.md have moved since, so merging as-is risks quietly reverting text.

[antonmos]

Split done: the --shield-primary lid-closed fix is now its own PR, #187, and has been removed from this branch (b0660a0) so the two don't conflict. The two blocking items here (counting submissions rather than calls, and putting auto-unlock behind a flag) are still open.

🤖 Addressed by Claude Code

[antonmos]

Both blocking items fixed in 1092ad9, plus the rebase and the three non-blocking asks I could act on without live hardware:

1. Submission budget, not call budget. AUTO_UNLOCK_SUBMISSIONS now counts actual Return presses, shared across every call for the current lock cycle, capped at AUTO_UNLOCK_MAX_SUBMISSIONS = 2 via a small pure try_reserve_submission() helper (unit-tested: reserves_up_to_the_max_then_refuses, zero_budget_never_reserves). A single call's internal retry loop now draws from the same shared pool as every other call for that lock and stops the instant it's empty — so the total across every reconnect attempt is provably under 3, not just under 3 per call. The budget resets when the screen is observed unlocked.

2. --auto-unlock is now an opt-in flag (config AUTO_UNLOCK=1), off by default, matching --lock-on-disconnect and the rest of the project's convention. The old MACRDP_AUTO_UNLOCK=0 escape hatch is gone.

Non-blocking, taken:

• Caps Lock: rather than verify whether a posted CGEvent inherits hardware Caps Lock state (no rig here to check), I read the hardware state directly via CGEventSourceFlagsState and skip the whole attempt while it's on — no submission spent, tries again next reconnect. Sidesteps the question instead of answering it, but keeps the "resolve everything before typing, or abort" invariant.
• Startup warning when sysadminctl -screenLock status isn't Immediately (or the check itself fails), gated on --lock-on-disconnect.
• Marked EXPERIMENTAL in docs/cli.md.
• Rebased onto v0.9.7 (main tip).

cargo test 212 passed / 0 failed, cargo clippy --all-targets -- -D warnings clean, cargo fmt --check clean. I don't have the live rig to re-verify end-to-end lock/unlock behavior after these changes — the logic changes are mechanical (budget accounting and a pre-flight guard) and shouldn't touch the parts that were already live-verified, but flagging that explicitly since it matters here.

🤖 Generated with Claude Code

[clintcan]

Thanks — verified both blockers on 1092ad9: the shared, atomically-reserved budget does cap it at 2 per lock, and --auto-unlock is opt-in now. 212 pass, clippy/fmt clean. #187 is merged.

Two small nits before merge:

1. --auto-unlock isn't in the "has no effect without a headless mode" warning, so without detach/capture/shield it silently does nothing.
2. screen_is_locked() returns false when the read fails, and the new reset-on-unlocked turns that into a budget reset. Returning Option<bool> and resetting only on Some(false) closes it.

After that, the last thing is one live lock/unlock pass. You're right that nobody has run it since these changes, so I'll do it on my side.

[antonmos]

Nit 1 fixed in 6c52d6f: --auto-unlock now joins the shared "has no effect without --detach-primary / --capture-primary / --shield-primary" startup warning. fmt/clippy clean, 212 pass. Nit 2 (screen_is_locked() → Option<bool>, reset the budget only on Some(false)) isn't in this push; it will come separately.

🤖 Addressed by Claude Code

[antonmos]

Nit 2 fixed in 8458508: `screen_is_locked()` now returns `Option` instead of a bare `bool`.

`None` = the private-API lookup itself failed (genuinely unknown), `Some(true)`/`Some(false)` = a confirmed read. Both call sites in `attempt_auto_unlock` now match on it:

• the top-of-function check only resets `AUTO_UNLOCK_SUBMISSIONS`/`AUTO_UNLOCK_GAVE_UP` on a confirmed `Some(false)`, and treats `None` as "skip this attempt, don't touch the budget" (`AutoUnlockOutcome::SkippedUnsafe`)
• the in-loop poll after each Return only counts a confirmed `Some(false)` as success — `None` is treated the same as "still locked" so it can't falsely claim victory or reset the budget mid-loop

So a flaky lookup can no longer launder its way into an unlimited retry budget. Still fails toward the safe direction throughout (every ambiguous case → "don't act," never "assume unlocked").

212 tests pass, clippy `-D warnings` and `fmt --check` clean. Ready for your live lock/unlock pass whenever convenient — no behavior change expected there since this only touches the failure-path accounting, not the happy path.

🤖 Generated with Claude Code

[clintcan]

Ran the live pass on a Mac mini (M1, macOS 26.5) over ZeroTier with nobody physically at it: main (incl. #187) + 8458508, --shield-primary --lock-on-disconnect --auto-unlock, screen-lock delay Immediate.

• Lock-on-disconnect: ✅ — locked 22.5 s after the last client left, every time.
• The typed password works 3/3. Nobody touched the mini; it reached the desktop on its own each time.
• Detection is flaky: 1 succeeded, 2 false exhausted its submission budget … giving up. With 2 submissions × a 400 ms settle there's ~800 ms for screen_is_locked() to flip, and the mini is right at that edge. When it misses, it presses Return a second time mid-unlock (that one may land on the unlocked desktop) and fires a false alarm.
• The bigger problem — a false give-up becomes a remote lockout. The budget only resets when an attempt observes Some(false). After a false give-up nothing observes the unlock, so the next disconnect re-locks with the budget still spent, and the next connect refuses to try. Reproduced twice; only an agent restart recovered it. For a remote-only Mac that's the worst case this feature can hit.

Suggested fixes:

1. After each Return, wait up to ~3 s for Some(false) before spending the next submission.
2. Reset the budget at the start of each new lock cycle — e.g. in the disconnect path right before lock_session(), where the screen is known to be unlocked.

Happy to re-run it on the mini once those are in.

[antonmos]

Both issues from the Mac mini pass fixed in 5753ac8, following your suggested fixes directly:

1. Widened the detection window. RETURN_ATTEMPT_BUDGET was 400ms; raised to 3s per Return. With the 2-submission budget that's up to ~6s before giving up instead of ~800ms — real room for screen_is_locked() to observe a slower unlock before the loop spends its next submission on a Return that might land on an already-unlocked desktop.

2. Reset the budget at lock time, not just on observed unlock. The lock_on_disconnect closure now zeroes AUTO_UNLOCK_SUBMISSIONS/AUTO_UNLOCK_GAVE_UP immediately before calling lock_session(). That's always a moment where the screen is known to be unlocked (we're the ones locking it), so every lock cycle this feature itself initiates gets a fresh budget no matter how the previous cycle's detection turned out — closing the path where a false give-up permanently pinned the budget and the next reconnect refused to even try.

cargo test 212 passed, clippy -D warnings and fmt --check clean. Ready for another live pass on the mini whenever you get a chance — thanks for catching both of these, they're exactly the kind of thing that's invisible without real hardware and a real link.

🤖 Generated with Claude Code

[clintcan]

Thanks — both look right: 3 s per Return and the reset-before-lock close what I saw on the mini. 212 pass, clippy/fmt clean.

One tightening on the reset: it assumes the screen is unlocked at that point, but nothing checks it. If it's already locked when lock-on-disconnect fires, the same lock gets a fresh budget. Realistic case: the Mac's password is changed while macrdp keeps running. RDP still accepts the old password, auto-unlock types the old one, and every disconnect/reconnect then burns 2 more attempts against a lock that never clears — exactly the macOS escalation the cap exists to prevent. Gating the reset on screen_is_locked() == Some(false) makes the assumption true.

After that I'll re-run the pass on the mini.

[antonmos]

Good catch — fixed in f68113a. The reset before `lock_session()` is now gated on `virtual_display::screen_is_locked() == Some(false)`, so it only fires on a confirmed unlocked read. `Some(true)` (still locked, e.g. your stale-password scenario) or `None` (lookup failed) leave the budget and the give-up latch alone — a persistently-wrong password now stays given-up instead of getting a fresh 2-submission allowance every disconnect/reconnect cycle.

`cargo test` 212 passed, clippy `-D warnings` and `fmt --check` clean. Ready whenever you want to re-run the mini.

🤖 Generated with Claude Code

[clintcan]

Final live pass on the mini with main + f68113a: 4 lock/unlock cycles, locked 22.5 s after each disconnect, auto-unlock: succeeded 4/4 about 3 s after connecting, zero false give-ups. Merging — thanks for turning every round around so fast.

One small follow-up for a separate PR (not blocking): the lock can fire mid-handshake. A connection only counts as "back" once it's fully connected, which takes about 10 s here, so a reconnect started 12–22 s after disconnecting gets locked underneath it. With --auto-unlock that's invisible (it unlocked 3 s later), but with --lock-on-disconnect alone it would strand a remote user on the lock screen. Counting accepted-but-not-yet-connected sessions as "came back" would close it.

[antonmos]

Great to hear — thanks for running all of these down on real hardware, that's exactly the kind of testing this feature needed.

Filed the mid-handshake lock follow-up as a separate tracked task, to be picked up as its own PR against main once this one's merged, per your note that it's non-blocking.

🤖 Generated with Claude Code

[clintcan]

Heads-up on the mid-handshake lock follow-up you flagged in your final review: I picked it up and it's merged as #190, so no need to spend time on it.

It holds the pending lock while there's been connection activity since the disconnect: an authenticated reconnect within 30 s, or an accepted connection within 10 s, since the timer can end mid-CredSSP. The total hold is capped at 30 s past the normal firing time, so an unauthenticated peer can only delay the lock, never prevent it. I used timestamps rather than a per-connection counter because preemption candidates can get on_accept without ever reaching on_disconnected.

Live-tested with a ~5.5 s timer, so a normal reconnect lands mid-handshake. The lock held and was then skipped once the session came up, and the negative control still locked on time. mstsc's spare pre-auth connection used ~7.7 s of the 10 s pre-auth window before the real one arrived. If you see that margin get tight on your ZeroTier setup, I'd like to know.

Thanks again for catching it. It'll ship in the next release (just after v0.9.9).