fix(shield-primary): engage on a lid-closed MacBook instead of erroring by antonmos · Pull Request #187 · clintcan/macrdp

antonmos · 2026-09-23T21:43:36Z

Split out of #181 at review request — this is an unrelated, unambiguous bug fix.

The bug

On a lid-closed MacBook with no external display, --shield-primary refused to engage on every connect:

could not engage headless overlay on client connect: no physical display to shield — the virtual display is the only online one already

ShieldedPrimary::install's existing single-panel fallback assumes the built-in panel is at worst online-but-inactive (mirrored into the vd) and finds it via CGGetOnlineDisplayList. A closed lid goes further: the panel is gone from the online list too (confirmed live — both CGGetActiveDisplayList and CGGetOnlineDisplayList returned only the vd's id). So the fallback came up empty and install returned its terminal error.

Because the overlay watcher's error arm only warn!s and the RDP session itself keeps working, this was silent: the shield, --restore-windows-on-disconnect, and the connect-time window gather were all disabled — they live in the same Ok(ovr) branch — with no user-visible symptom. It went unnoticed for days on a machine in daily use.

The fix

With the lid closed nothing is displaying a desktop, so there is nothing to shield. install now treats "no physical panel in any state" as success and degrades to a no-op shield instead of erroring.

Empty targets flows cleanly through every downstream step (checked by inspection before shipping):

broke_mirror derives to empty, so the mirror-break and displace are skipped
the SHOW-ack backoff loop's need = targets.len() = 0 is trivially satisfied
the arrangement tx is skipped under the default keep_physical_main
Drop already handles empty lists — lowered shields (no arrangement was moved) was already being logged in earlier successful sessions

Only the lid-closed case changes. The active-display and online-but-inactive paths are untouched.

Testing

cargo test 205 passing, cargo clippy --all-targets -- -D warnings clean, cargo fmt --check clean
Live-verified on macOS 26.5, lid-closed MacBook Air: the first connect after deploying logged RDP client connected — Mac is now headless via the virtual display with shielded_count=0, where previously only the warning ever fired.

docs/known-quirks.md gets an UPDATE on the existing single-panel shield note, including a diagnostic tip: grep macrdp.log for no physical display to shield first.

🤖 Generated with Claude Code

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 — CGGetActiveDisplayList AND CGGetOnlineDisplayList both return only the virtual display — a step beyond the online-but-inactive case the existing single-panel fallback handles. So every connect failed with "no physical display to shield". The overlay watcher's error arm only warns and the RDP session works regardless, so this silently disabled the shield, --restore-windows-on-disconnect, AND the connect-time window gather (all inside the same Ok(ovr) branch) with no user-visible symptom. With the lid closed nothing is displaying a desktop, so there is nothing to shield: treat it as success and let install degrade to a no-op shield. Empty targets flows cleanly through every downstream step — broke_mirror is empty (mirror-break/displace skipped), the SHOW ack's need is 0, the arrangement tx is skipped under the default keep_physical_main, and Drop already handles empty lists. Live-verified on macOS 26.5 (lid-closed MacBook Air): the first connect after deploying logged "RDP client connected" with shielded_count=0. Split out of clintcan#181 at review request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Reviewer asked for the lid-closed ShieldedPrimary::install fix to land on its own, since it is unrelated to lock/unlock. It now lives in clintcan#187; drop it (code + known-quirks note) from this branch so the two PRs don't conflict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

clintcan · 2026-09-24T18:45:59Z

tyvm. Since the shield fix is moved out, this is merged.

…ect (#181) * feat(session): --lock-on-disconnect + always-on auto-unlock on reconnect 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. --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 (same seam as --restore-windows-on-disconnect). SessionTracker also fires on blank-recovery self-heals, whose drop-then-ARC-reconnect can 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. A heuristic, documented as such. Mechanism note: the widely-cited `CGSession -suspend` trick does NOT work on macOS 26 — Menu Extras/User.menu no longer exists. Uses `open ScreenSaverEngine.app`, which is only a true password-required lock if the account's screenLock delay is "immediate" (not checked or changed; modifying security settings is out of scope). Auto-unlock on reconnect (always-on, no flag; MACRDP_AUTO_UNLOCK=0 opts out) Types the account password into the lock screen when a client connects while locked — any lock, any cause. Reuses the same credential PAM already validated at startup and that the connecting client proved via CredSSP, so no fresh keychain read; skipped entirely under --skip-auth. Legitimate mechanism, not a workaround: TN2150 scopes SecureEventInput to blocking *observation* (event taps), not synthetic injection, and this is what Apple's own Screen Sharing/ARD do to unlock a screensaver-locked Mac. 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 one from the physical keyboard. No settle delay or Return-retry can fix it. Fixed by typing real per-character keycode events, which needed 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 (password arrived one character short). Fixed with a bare Shift tap — a pure modifier can't insert text or submit. (Using Return as that wake submits an empty password and shakes the field.) Every character is resolved to a keystroke BEFORE typing anything, so an unmappable character aborts the attempt rather than submitting a partial password. Failures are capped at 2 consecutive attempts per lock (compile- time asserted below macOS's 3-free-attempt PAM throttle) and then alert loudly — sound + best-effort notification + error log — rather than silently retrying into a lockout. 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 its existing single-panel fallback handles — so every connect failed with "no physical display to shield". Since 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. Live-verified end-to-end on macOS 26.5 (lid-closed MacBook Air, --shield-primary): lock on disconnect, unlock on reconnect, and the shield path engaging for the first time on this hardware. 211 tests, clippy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: move the --shield-primary lid-closed fix out to #187 Reviewer asked for the lid-closed ShieldedPrimary::install fix to land on its own, since it is unrelated to lock/unlock. It now lives in #187; drop it (code + known-quirks note) from this branch so the two PRs don't conflict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(session): auto-unlock behind a flag + a real submission budget Addresses the two blocking review comments on #181. 1. The Return-retry loop defeated its own attempt cap. AUTO_UNLOCK_ MAX_CONSECUTIVE_FAILURES=2 (asserted < 3 to stay under macOS's PAM free-attempts margin) capped CALLS to attempt_auto_unlock, but each call internally retried Return up to 3 times, and the loop can't tell "ignored" from "submitted and rejected" apart — both leave the screen locked. Two reconnect attempts could submit the password against PAM up to 6 times against a promise of fewer than 3. Fixed by tracking the budget where it matters: AUTO_UNLOCK_ SUBMISSIONS counts real Return presses, shared across every call for the current lock cycle (not per-call), capped at AUTO_UNLOCK_MAX_SUBMISSIONS=2 via a small pure, unit-tested try_reserve_submission() helper. The budget (and the "gave up" alert latch) resets when the screen is observed unlocked, so a fresh lock cycle gets a fresh allowance. 2. Auto-unlock is now opt-in (--auto-unlock / config AUTO_UNLOCK=1), replacing the always-on-with-an-env-escape-hatch default. The effect lands on the physical machine — once it fires, anyone standing at that Mac has a live desktop, undoing a lock they may have set deliberately — so it should not be a quiet default, matching the rest of the project's opt-in-and-byte-identical-when- off convention. Also taken from the review's non-blocking list: - Caps Lock guard: reverse_map() always resolves assuming Caps is off, and whether a posted CGEvent honors the Mac's hardware Caps Lock state was flagged as unconfirmed (no rig to verify against here). Rather than guess, read the hardware state via CGEventSourceFlagsState (declared locally; core-graphics doesn't wrap it) and skip the attempt while Caps Lock is on — no submission spent, next reconnect tries again for free. - Startup warning when `sysadminctl -screenLock status` isn't Immediately (or the check itself fails), since --lock-on-disconnect's name promises a lock that ScreenSaverEngine.app won't actually enforce otherwise, and the failure was previously invisible. - Marked --lock-on-disconnect and --auto-unlock EXPERIMENTAL in the docs, alongside USB/camera redirection. - Rebased onto v0.9.7 (the branch predated it, per the review). 211 -> 212 tests (net: -2 auto_unlock_enabled tests, +1 flag-parsing test, +2 submission-budget tests). cargo clippy --all-targets -D warnings and cargo fmt --check both clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(session): warn when --auto-unlock is set without a headless mode Like --restore-windows-on-disconnect and --lock-on-disconnect, it rides the headless session watcher, so without detach/capture/shield it silently did nothing. Fold it into the existing shared warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(session): screen_is_locked() returns Option<bool> to fix budget-reset bug Addresses nit 2 from the #181 review. screen_is_locked() collapsed a lookup failure to false ("not locked"). That was fine for deciding whether to attempt an unlock (a false negative there just skips an attempt, safe) but became wrong once the submission-budget reset started keying off the same bare bool: an intermittently-failing private-API lookup would read as "confirmed unlocked" on every failure and reset AUTO_UNLOCK_SUBMISSIONS each time, silently defeating the cap AUTO_UNLOCK_MAX_SUBMISSIONS exists to enforce. Fixed by making the tri-state explicit: None = lookup failed (genuinely unknown), Some(true)/Some(false) = a confirmed read. Both attempt_auto_unlock call sites now match on it: - the top-of-function check only resets the budget on a confirmed Some(false), and treats None as "skip this attempt without touching the budget or the alert latch" (AutoUnlockOutcome::SkippedUnsafe) - the in-loop poll after each Return only counts a confirmed Some(false) as success, treating None the same as "still locked" so it can't falsely short-circuit the retry loop Still fails toward the safe direction throughout: every ambiguous case resolves to "don't act," never "assume unlocked." 212 tests pass, clippy -D warnings and fmt --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(session): widen the auto-unlock detection window + reset budget on lock Two bugs found in the reviewer's live pass on a Mac mini (M1, macOS 26.5) over ZeroTier: lock-on-disconnect fired reliably and the typed password worked 3/3, but detection of the successful unlock was flaky and a false give-up escalated into a remote lockout only an agent restart could clear. 1. RETURN_ATTEMPT_BUDGET (400ms) was too tight a window for screen_is_locked() to observe a real unlock on that link/hardware. With AUTO_UNLOCK_MAX_SUBMISSIONS=2 that's ~800ms total before giving up; when the unlock hadn't been observed by then, the loop spent its second submission on a Return that could land on the already-unlocked desktop and then falsely reported BudgetExhausted. Raised to 3s per Return, per the reviewer's suggestion, so a slower link/lookup has real room to register the flip before the next submission is spent. 2. The submission budget only reset on an OBSERVED Some(false) from inside attempt_auto_unlock, never on the act of locking. So after a false give-up, the real screen state was unlocked but nothing was polling it to notice — the budget stayed pinned at AUTO_UNLOCK_MAX_SUBMISSIONS, and the *next* lock-on-disconnect cycle started pre-exhausted: the reconnect after that refused to even attempt an unlock, indefinitely. Fixed per the reviewer's suggested root-cause fix: reset AUTO_UNLOCK_SUBMISSIONS/ AUTO_UNLOCK_GAVE_UP immediately before lock_session() in the lock_on_disconnect closure — that's always a point where the screen is known to be unlocked, so every lock cycle this project initiates is guaranteed a fresh budget regardless of how the previous cycle's detection turned out. 212 tests pass, clippy -D warnings and fmt --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(session): gate the pre-lock budget reset on a confirmed unlocked read Caught in review: the reset added in 5753ac8 zeroed AUTO_UNLOCK_SUBMISSIONS/AUTO_UNLOCK_GAVE_UP unconditionally right before lock_session(), assuming the screen was unlocked at that point without checking. That assumption breaks in a realistic case: the account password changes while macrdp keeps running. RDP auth still accepts the cached, now-stale password via NLA, so attempt_auto_unlock keeps typing that same wrong password and genuinely fails every time — a real rejection, not a detection miss, which the submission cap is supposed to stop retrying. Without the gate, every disconnect/reconnect cycle would hand that permanently- failing attempt a fresh 2-submission budget, burning real PAM submissions against a lock that will never clear — the same escalating-lockout risk the cap exists to prevent, just spread across cycles. Fixed: the reset now only fires when virtual_display::screen_is_locked() == Some(false) — a confirmed unlocked read right before lock_session() is called. Some(true) (still locked) or None (lookup failed) leave the budget/latch alone, so a persistently-wrong password stays given-up instead of getting a free retry allowance on every cycle. 212 tests pass, clippy -D warnings and fmt --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

@antonmos

A feature release for headless, remote-only Macs plus a --shield-primary fix. Everything new is opt-in; the default runtime path is unchanged. - #181 (@antonmos): --lock-on-disconnect locks the Mac ~25 s after the last client leaves; --auto-unlock types the PAM-validated password into the lock screen on reconnect, with a shared per-lock budget of 2 real submissions. Both experimental; live-verified on a Mac mini. - #188: GUI controller toggles for both (Display tab, "Lock while away"). - #187 (@antonmos): --shield-primary engages on a lid-closed MacBook instead of erroring, which had silently disabled the shield, restore-windows and the connect-time window gather. - Repo hardening: CODEOWNERS + code-owner review, fork-PR CI approval for all outside collaborators, enforced SHA pinning. Known issue: a reconnect ~12-22 s after leaving can be locked mid-handshake; self-corrects with --auto-unlock, strands a remote user without it. Pre-tag gates: fmt clean (stable + nightly), clippy -D warnings clean, 212 tests passing. Docs: release-history, README status, CLAUDE.md status.

antonmos mentioned this pull request Sep 23, 2026

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

Merged

clintcan approved these changes Sep 24, 2026

View reviewed changes

clintcan merged commit aa1ebaf into clintcan:main Sep 24, 2026
3 checks passed

Navigation Menu

fix(shield-primary): engage on a lid-closed MacBook instead of erroring - #187

fix(shield-primary): engage on a lid-closed MacBook instead of erroring#187
clintcan merged 1 commit into
clintcan:mainfrom
antonmos:fix/shield-primary-lid-closed

antonmos commented Sep 23, 2026

Uh oh!

Uh oh!

clintcan commented Sep 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

antonmos commented Sep 23, 2026

The bug

The fix

Testing

Uh oh!

Uh oh!

clintcan commented Sep 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants