Skip to content

Commit 9211946

Browse files
committed
SMB: Stop NetFS guest mount from popping macOS auth dialog
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 `<NetFS/NetFS.h>` 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.
1 parent 0f8c9ff commit 9211946

1 file changed

Lines changed: 119 additions & 12 deletions

File tree

  • apps/desktop/src-tauri/src/network

apps/desktop/src-tauri/src/network/mount.rs

Lines changed: 119 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,28 @@
11
//! SMB share mounting using macOS NetFS.framework.
22
//!
33
//! Provides async mount operations with proper error handling and credential support.
4+
//!
5+
//! ## Credential handling: why we pass creds explicitly to NetFS
6+
//!
7+
//! `NetFSMountURLSync` accepts `user`, `passwd`, and an `openOptions` CFDictionary.
8+
//! When `user`/`passwd` are both `NULL` and `openOptions` doesn't say otherwise,
9+
//! NetFS falls back to looking up credentials in the system Keychain. If the lookup
10+
//! misses (fresh host, fresh Docker container, brand-new NAS), the kernel `smbfs`
11+
//! kext pops a credential dialog with the current OS user prefilled. That dialog
12+
//! steals focus, blocks the caller, and looks like the app has frozen.
13+
//!
14+
//! Cmdr already collects credentials (or "guest") in the frontend. We pass them
15+
//! down so NetFS never reaches the Keychain fallback:
16+
//!
17+
//! - **Credentialed mount**: build CFStrings from the supplied user + password and
18+
//! pass them as `user`/`passwd`. NetFS uses them directly.
19+
//! - **Guest mount**: set `kNetFSUseGuestKey` (literal key `"Guest"`) to
20+
//! `kCFBooleanTrue` in `openOptions`. NetFS skips the Keychain and authenticates
21+
//! as guest. `user`/`passwd` stay `NULL` in this case, per Apple's NetFS docs.
22+
//!
23+
//! The constant `kNetFSUseGuestKey` is a `#define` in `<NetFS/NetFS.h>` (not an
24+
//! exported symbol), so we recreate the CFString from the literal `"Guest"` at the
25+
//! call site rather than linking to an `extern "C"` static.
426
527
use core_foundation::base::TCFType;
628
use core_foundation::string::CFString;
@@ -165,23 +187,42 @@ pub fn mount_share_sync(
165187
// If so, pick a disambiguated path (public-1, public-2, ...) like Finder does.
166188
let explicit_mount_path = disambiguated_mount_path(server, share, port);
167189

168-
// When disambiguating, force a new SMB session so macOS doesn't reuse
169-
// the existing session to the same hostname (different port = different server).
170-
let open_options = if explicit_mount_path.is_some() {
190+
// Build openOptions. Two reasons we may need a dict:
191+
// 1. Guest mount (no credentials): set `kNetFSUseGuestKey = true` so NetFS
192+
// doesn't consult the Keychain and pop a credential dialog.
193+
// 2. Disambiguating against an existing same-hostname mount: set
194+
// `ForceNewSession = true` so macOS opens a fresh SMB session instead of
195+
// reusing the existing one (different port = different server).
196+
// If neither applies, pass NULL (NetFS uses default behavior).
197+
let want_guest = cf_user.is_none() && cf_pass.is_none();
198+
let want_force_new_session = explicit_mount_path.is_some();
199+
let open_options = if want_guest || want_force_new_session {
171200
unsafe {
172201
let dict = core_foundation::dictionary::CFDictionaryCreateMutable(
173202
ptr::null(),
174-
1,
203+
2,
175204
&core_foundation::dictionary::kCFTypeDictionaryKeyCallBacks,
176205
&core_foundation::dictionary::kCFTypeDictionaryValueCallBacks,
177206
);
178-
let key = CFString::new("ForceNewSession");
179-
let value = core_foundation::boolean::kCFBooleanTrue;
180-
core_foundation::dictionary::CFDictionarySetValue(
181-
dict,
182-
key.as_concrete_TypeRef() as *const c_void,
183-
value as *const c_void,
184-
);
207+
let true_value = core_foundation::boolean::kCFBooleanTrue;
208+
if want_guest {
209+
// `kNetFSUseGuestKey` is a `#define` in <NetFS/NetFS.h>, not an
210+
// exported symbol. Build the CFString from its literal value.
211+
let guest_key = CFString::new("Guest");
212+
core_foundation::dictionary::CFDictionarySetValue(
213+
dict,
214+
guest_key.as_concrete_TypeRef() as *const c_void,
215+
true_value as *const c_void,
216+
);
217+
}
218+
if want_force_new_session {
219+
let force_key = CFString::new("ForceNewSession");
220+
core_foundation::dictionary::CFDictionarySetValue(
221+
dict,
222+
force_key.as_concrete_TypeRef() as *const c_void,
223+
true_value as *const c_void,
224+
);
225+
}
185226
dict as *const c_void
186227
}
187228
} else {
@@ -193,8 +234,9 @@ pub fn mount_share_sync(
193234

194235
// Call NetFSMountURLSync. Mount path is NULL even when disambiguating;
195236
// NetFS auto-creates the mount point in /Volumes/ (we can't mkdir there).
196-
// With ForceNewSession, NetFS treats this as a separate server and picks
237+
// With `ForceNewSession`, NetFS treats this as a separate server and picks
197238
// a disambiguated name (public-1, public-2, etc.) automatically.
239+
// With `Guest`, NetFS authenticates as guest without consulting Keychain.
198240
let result = unsafe {
199241
NetFSMountURLSync(
200242
cf_url.as_concrete_TypeRef() as *const c_void,
@@ -468,4 +510,69 @@ mod tests {
468510
const { assert!(DEFAULT_MOUNT_TIMEOUT_MS >= 10_000) };
469511
const { assert!(DEFAULT_MOUNT_TIMEOUT_MS <= 60_000) };
470512
}
513+
514+
/// Regression test for the macOS NetFS guest-mount credential dialog.
515+
///
516+
/// Asserts a guest mount completes within a tight wall-clock budget. A
517+
/// blocking kernel `smbfs` prompt waits for user input indefinitely, so a
518+
/// sub-budget completion is the proxy for "no dialog appeared." Gated to
519+
/// macOS because Linux uses gvfs, which has neither the dialog nor this
520+
/// mount path.
521+
///
522+
/// We don't add a paired auth-success / auth-failure test here because
523+
/// NetFS caches SMB sessions across calls — once `testuser`+`testpass`
524+
/// authenticates once, subsequent calls (even with wrong creds) ride the
525+
/// cached session, so a tight harness can't reliably distinguish "creds
526+
/// passed correctly" from "session reused" without forcibly tearing down
527+
/// the session. The guest path is what regressed in real use and is what
528+
/// this test guards. Manual end-to-end coverage for the auth path runs
529+
/// via `pnpm dev` against the same Docker containers.
530+
#[cfg(target_os = "macos")]
531+
#[tokio::test]
532+
#[ignore = "Requires Docker SMB containers (./apps/desktop/test/smb-servers/start.sh)"]
533+
async fn smb_integration_mount_guest_no_dialog() {
534+
use std::time::{Duration, Instant};
535+
536+
let port: u16 = std::env::var("SMB_CONSUMER_GUEST_PORT")
537+
.ok()
538+
.and_then(|v| v.parse().ok())
539+
.unwrap_or(10480);
540+
// Use `localhost` rather than `127.0.0.1`: NetFS itself handles either,
541+
// but the wider SMB test harness uses `localhost` to dodge the smbutil
542+
// loopback quirk on non-standard ports.
543+
let host = "localhost".to_string();
544+
545+
// Pre-clean any stale mount from a previous run so we exercise the
546+
// real first-mount path (the one that pops the dialog when broken).
547+
let _ = std::process::Command::new("diskutil")
548+
.args(["unmount", "force", "/Volumes/public"])
549+
.output();
550+
551+
// 10 s budget: a real credential dialog blocks the call indefinitely,
552+
// so this picks up the regression even under cold Docker startup.
553+
let budget = Duration::from_secs(10);
554+
let start = Instant::now();
555+
let result = mount_share(host.clone(), "public".to_string(), None, None, port, Some(8_000)).await;
556+
let elapsed = start.elapsed();
557+
558+
// Always try to unmount so a successful mount doesn't linger between runs.
559+
if let Ok(ref ok) = result {
560+
let _ = std::process::Command::new("diskutil")
561+
.args(["unmount", "force", &ok.mount_path])
562+
.output();
563+
}
564+
565+
assert!(
566+
elapsed < budget,
567+
"guest mount took {:?} (budget {:?}); a credential dialog probably blocked NetFS",
568+
elapsed,
569+
budget
570+
);
571+
let mount_result = result.unwrap_or_else(|e| panic!("guest mount against {host}:{port} failed: {e:?}"));
572+
assert!(
573+
mount_result.mount_path.starts_with("/Volumes/"),
574+
"expected /Volumes/* mount path, got {}",
575+
mount_result.mount_path
576+
);
577+
}
471578
}

0 commit comments

Comments
 (0)