From 6926bd706f22db9a1be9582ea65c74ae319aecc5 Mon Sep 17 00:00:00 2001 From: David Veszelovszki Date: Mon, 18 May 2026 21:55:57 +0200 Subject: [PATCH] SMB: Stop NetFS guest mount from popping macOS auth dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening an SMB share for a host not in Keychain (fresh Docker container, new NAS, colleague's laptop) used to pop the kernel `smbfs` credential dialog with the current OS user prefilled. Root cause: `NetFSMountURLSync` got NULL user/passwd plus an empty `openOptions`, so NetFS fell through to Keychain lookup → miss → prompt. Cmdr already knows whether the user picked "guest" vs typed credentials in `NetworkMountView.svelte`. Plumb that intent into NetFS: - Guest mount: set `kNetFSUseGuestKey = true` (the literal `"Guest"` key, since `kNetFSUseGuestKey` is a `#define` in `` rather than an exported symbol) in `openOptions`. NetFS authenticates as guest, skips Keychain, no prompt. - Credentialed mount: already worked — we already pass user/passwd as CFStrings. Unchanged. - The existing `ForceNewSession` flag for path disambiguation (different-server same-share-name case) coexists with the new `Guest` flag in the same dictionary. Tests: - New `smb_integration_mount_guest_no_dialog` (gated to macOS, requires Docker SMB) asserts a guest mount against `smb-consumer-guest` returns in < 10 s. A real dialog blocks indefinitely, so the tight wall-clock budget is the regression signal. - No paired auth-success/failure test: NetFS aggressively caches SMB sessions across calls, so a tight harness can't reliably distinguish "creds passed correctly" from "session reused" without forcibly tearing down the session. The auth path is exercised manually via `pnpm dev`. - Module doc-comment now explains the credential-passing contract and why each branch matters. Cleaner take on #17, which tried to skip the OS mount entirely and broke path resolution. ## Test plan - `./scripts/check.sh --check clippy --check rust-tests --check rust-integration-tests --check rustfmt --check svelte-check`: green (1892 unit tests, 31 integration tests). - `cargo nextest run --run-ignored only -E 'test(smb_integration_mount_guest_no_dialog)'` against the running Docker SMB containers: green in 2.5 s on a 10 s budget. - Manual: open an SMB share via Network view against a fresh Docker container without Keychain creds → no dialog, share mounts. --- apps/desktop/src-tauri/src/network/mount.rs | 131 ++++++++++++++++++-- 1 file changed, 119 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src-tauri/src/network/mount.rs b/apps/desktop/src-tauri/src/network/mount.rs index 190c85a6..c77c51f7 100644 --- a/apps/desktop/src-tauri/src/network/mount.rs +++ b/apps/desktop/src-tauri/src/network/mount.rs @@ -1,6 +1,28 @@ //! SMB share mounting using macOS NetFS.framework. //! //! Provides async mount operations with proper error handling and credential support. +//! +//! ## Credential handling: why we pass creds explicitly to NetFS +//! +//! `NetFSMountURLSync` accepts `user`, `passwd`, and an `openOptions` CFDictionary. +//! When `user`/`passwd` are both `NULL` and `openOptions` doesn't say otherwise, +//! NetFS falls back to looking up credentials in the system Keychain. If the lookup +//! misses (fresh host, fresh Docker container, brand-new NAS), the kernel `smbfs` +//! kext pops a credential dialog with the current OS user prefilled. That dialog +//! steals focus, blocks the caller, and looks like the app has frozen. +//! +//! Cmdr already collects credentials (or "guest") in the frontend. We pass them +//! down so NetFS never reaches the Keychain fallback: +//! +//! - **Credentialed mount**: build CFStrings from the supplied user + password and +//! pass them as `user`/`passwd`. NetFS uses them directly. +//! - **Guest mount**: set `kNetFSUseGuestKey` (literal key `"Guest"`) to +//! `kCFBooleanTrue` in `openOptions`. NetFS skips the Keychain and authenticates +//! as guest. `user`/`passwd` stay `NULL` in this case, per Apple's NetFS docs. +//! +//! The constant `kNetFSUseGuestKey` is a `#define` in `` (not an +//! exported symbol), so we recreate the CFString from the literal `"Guest"` at the +//! call site rather than linking to an `extern "C"` static. use core_foundation::base::TCFType; use core_foundation::string::CFString; @@ -165,23 +187,42 @@ pub fn mount_share_sync( // If so, pick a disambiguated path (public-1, public-2, ...) like Finder does. let explicit_mount_path = disambiguated_mount_path(server, share, port); - // When disambiguating, force a new SMB session so macOS doesn't reuse - // the existing session to the same hostname (different port = different server). - let open_options = if explicit_mount_path.is_some() { + // Build openOptions. Two reasons we may need a dict: + // 1. Guest mount (no credentials): set `kNetFSUseGuestKey = true` so NetFS + // doesn't consult the Keychain and pop a credential dialog. + // 2. Disambiguating against an existing same-hostname mount: set + // `ForceNewSession = true` so macOS opens a fresh SMB session instead of + // reusing the existing one (different port = different server). + // If neither applies, pass NULL (NetFS uses default behavior). + let want_guest = cf_user.is_none() && cf_pass.is_none(); + let want_force_new_session = explicit_mount_path.is_some(); + let open_options = if want_guest || want_force_new_session { unsafe { let dict = core_foundation::dictionary::CFDictionaryCreateMutable( ptr::null(), - 1, + 2, &core_foundation::dictionary::kCFTypeDictionaryKeyCallBacks, &core_foundation::dictionary::kCFTypeDictionaryValueCallBacks, ); - let key = CFString::new("ForceNewSession"); - let value = core_foundation::boolean::kCFBooleanTrue; - core_foundation::dictionary::CFDictionarySetValue( - dict, - key.as_concrete_TypeRef() as *const c_void, - value as *const c_void, - ); + let true_value = core_foundation::boolean::kCFBooleanTrue; + if want_guest { + // `kNetFSUseGuestKey` is a `#define` in , not an + // exported symbol. Build the CFString from its literal value. + let guest_key = CFString::new("Guest"); + core_foundation::dictionary::CFDictionarySetValue( + dict, + guest_key.as_concrete_TypeRef() as *const c_void, + true_value as *const c_void, + ); + } + if want_force_new_session { + let force_key = CFString::new("ForceNewSession"); + core_foundation::dictionary::CFDictionarySetValue( + dict, + force_key.as_concrete_TypeRef() as *const c_void, + true_value as *const c_void, + ); + } dict as *const c_void } } else { @@ -193,8 +234,9 @@ pub fn mount_share_sync( // Call NetFSMountURLSync. Mount path is NULL even when disambiguating; // NetFS auto-creates the mount point in /Volumes/ (we can't mkdir there). - // With ForceNewSession, NetFS treats this as a separate server and picks + // With `ForceNewSession`, NetFS treats this as a separate server and picks // a disambiguated name (public-1, public-2, etc.) automatically. + // With `Guest`, NetFS authenticates as guest without consulting Keychain. let result = unsafe { NetFSMountURLSync( cf_url.as_concrete_TypeRef() as *const c_void, @@ -468,4 +510,69 @@ mod tests { const { assert!(DEFAULT_MOUNT_TIMEOUT_MS >= 10_000) }; const { assert!(DEFAULT_MOUNT_TIMEOUT_MS <= 60_000) }; } + + /// Regression test for the macOS NetFS guest-mount credential dialog. + /// + /// Asserts a guest mount completes within a tight wall-clock budget. A + /// blocking kernel `smbfs` prompt waits for user input indefinitely, so a + /// sub-budget completion is the proxy for "no dialog appeared." Gated to + /// macOS because Linux uses gvfs, which has neither the dialog nor this + /// mount path. + /// + /// We don't add a paired auth-success / auth-failure test here because + /// NetFS caches SMB sessions across calls — once `testuser`+`testpass` + /// authenticates once, subsequent calls (even with wrong creds) ride the + /// cached session, so a tight harness can't reliably distinguish "creds + /// passed correctly" from "session reused" without forcibly tearing down + /// the session. The guest path is what regressed in real use and is what + /// this test guards. Manual end-to-end coverage for the auth path runs + /// via `pnpm dev` against the same Docker containers. + #[cfg(target_os = "macos")] + #[tokio::test] + #[ignore = "Requires Docker SMB containers (./apps/desktop/test/smb-servers/start.sh)"] + async fn smb_integration_mount_guest_no_dialog() { + use std::time::{Duration, Instant}; + + let port: u16 = std::env::var("SMB_CONSUMER_GUEST_PORT") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(10480); + // Use `localhost` rather than `127.0.0.1`: NetFS itself handles either, + // but the wider SMB test harness uses `localhost` to dodge the smbutil + // loopback quirk on non-standard ports. + let host = "localhost".to_string(); + + // Pre-clean any stale mount from a previous run so we exercise the + // real first-mount path (the one that pops the dialog when broken). + let _ = std::process::Command::new("diskutil") + .args(["unmount", "force", "/Volumes/public"]) + .output(); + + // 10 s budget: a real credential dialog blocks the call indefinitely, + // so this picks up the regression even under cold Docker startup. + let budget = Duration::from_secs(10); + let start = Instant::now(); + let result = mount_share(host.clone(), "public".to_string(), None, None, port, Some(8_000)).await; + let elapsed = start.elapsed(); + + // Always try to unmount so a successful mount doesn't linger between runs. + if let Ok(ref ok) = result { + let _ = std::process::Command::new("diskutil") + .args(["unmount", "force", &ok.mount_path]) + .output(); + } + + assert!( + elapsed < budget, + "guest mount took {:?} (budget {:?}); a credential dialog probably blocked NetFS", + elapsed, + budget + ); + let mount_result = result.unwrap_or_else(|e| panic!("guest mount against {host}:{port} failed: {e:?}")); + assert!( + mount_result.mount_path.starts_with("/Volumes/"), + "expected /Volumes/* mount path, got {}", + mount_result.mount_path + ); + } }