Skip to content

Commit be1350d

Browse files
committed
Bugfix: SMB upgrade no longer races mDNS in dev
- In dev, `network.firstTriggerDone` is `false`, so launch-time mDNS is gated off. When macOS auto-remounts an SMB share at login, `volumes::watcher::try_upgrade_smb_mount` fired before mDNS had discovered the host, so `statfs`'s IP couldn't be mapped to a hostname. Keychain credentials keyed by hostname (`smb://naspolya/share`) missed, and we prompted for credentials the user had already saved a few seconds before mDNS warmed up. - Both upgrade entry points (`commands::network::upgrade_to_smb_volume` and the FSEvents watcher path) now call `network::ensure_mdns_started` (idempotent, no-op if `network.enabled` is off) and use the new `resolve_ip_to_hostname_with_wait` (polls 100ms up to 1500ms, only for private-range IPv4 — Tailscale/public IPs skip the wait). - Added a runtime `network::NETWORK_ENABLED` flag (mirrors `network.enabled`) so BE paths can short-circuit cleanly when the user disabled networking, instead of needlessly polling. - Unit tests cover the three short-circuits (non-private IP, disabled, timeout fallback). - The wait fails open: if mDNS never warms, the existing IP-only Keychain lookup still runs.
1 parent 6653db3 commit be1350d

7 files changed

Lines changed: 237 additions & 7 deletions

File tree

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::network::{
88

99
use crate::network::smb_upgrade::{
1010
UpgradeError, UpgradeResult, friendly_server_name, get_keychain_password, register_smb_volume,
11-
resolve_ip_to_hostname, try_smb_upgrade,
11+
resolve_ip_to_hostname_with_wait, try_smb_upgrade,
1212
};
1313

1414
/// Gets all currently discovered network hosts.
@@ -353,7 +353,7 @@ pub async fn mount_network_share(
353353
/// Called from the "Connect directly for faster access" UI action.
354354
#[tauri::command]
355355
#[specta::specta]
356-
pub async fn upgrade_to_smb_volume(volume_id: String) -> Result<UpgradeResult, String> {
356+
pub async fn upgrade_to_smb_volume(volume_id: String, app_handle: tauri::AppHandle) -> Result<UpgradeResult, String> {
357357
use crate::file_system::get_volume_manager;
358358
#[cfg(target_os = "macos")]
359359
use crate::volumes::get_smb_mount_info;
@@ -387,9 +387,14 @@ pub async fn upgrade_to_smb_volume(volume_id: String) -> Result<UpgradeResult, S
387387
info.username
388388
);
389389

390+
// Kick mDNS off so IP → hostname resolution has a shot before we hit the
391+
// Keychain. Idempotent; no-op if already running or `network.enabled` is off.
392+
crate::network::ensure_mdns_started(app_handle);
393+
390394
// Try to get credentials from Keychain. The mount source has the IP, but Cmdr
391-
// stores Keychain credentials keyed by hostname (from mDNS). Try both.
392-
let hostname = resolve_ip_to_hostname(&info.server);
395+
// stores Keychain credentials keyed by hostname (from mDNS). Try both. Briefly
396+
// wait for mDNS to warm up so we don't prompt for creds the user already saved.
397+
let hostname = resolve_ip_to_hostname_with_wait(&info.server, std::time::Duration::from_millis(1500)).await;
393398
let display_name = friendly_server_name(&info.server);
394399
let creds = get_keychain_password(&info.server, hostname.as_deref(), &info.share).await;
395400

@@ -452,6 +457,7 @@ pub async fn upgrade_to_smb_volume_with_credentials(
452457
username: Option<String>,
453458
password: Option<String>,
454459
remember_in_keychain: bool,
460+
app_handle: tauri::AppHandle,
455461
) -> Result<UpgradeResult, String> {
456462
use crate::file_system::get_volume_manager;
457463
#[cfg(target_os = "macos")]
@@ -475,7 +481,11 @@ pub async fn upgrade_to_smb_volume_with_credentials(
475481
)
476482
})?;
477483

478-
let hostname = resolve_ip_to_hostname(&info.server);
484+
// Kick mDNS off so we can save credentials keyed by hostname (not raw IP)
485+
// when the user picks "remember".
486+
crate::network::ensure_mdns_started(app_handle);
487+
488+
let hostname = resolve_ip_to_hostname_with_wait(&info.server, std::time::Duration::from_millis(1500)).await;
479489
let display_name = friendly_server_name(&info.server);
480490

481491
let result = try_smb_upgrade(
@@ -694,6 +704,7 @@ pub fn ensure_network_discovery_started(app_handle: tauri::AppHandle) {
694704
#[tauri::command]
695705
#[specta::specta]
696706
pub fn set_network_enabled(enabled: bool, app_handle: tauri::AppHandle) {
707+
crate::network::set_network_enabled_flag(enabled);
697708
if !enabled {
698709
crate::network::mdns_discovery::stop_discovery();
699710
crate::network::clear_discovered_hosts(&app_handle);

apps/desktop/src-tauri/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,11 @@ pub fn run() {
464464
font_metrics::init_font_metrics(app.handle(), "system-400-12");
465465
font_metrics::load_all_metrics_from_disk(app.handle());
466466

467+
// Sync the runtime `network.enabled` flag from settings so BE-side upgrade paths
468+
// can gate themselves correctly (default `true`).
469+
#[cfg(any(target_os = "macos", target_os = "linux"))]
470+
network::set_network_enabled_flag(saved_settings.network_enabled.unwrap_or(true));
471+
467472
// Start mDNS network discovery only for returning users who've already answered the
468473
// OS Local Network prompt at least once. Fresh installs stay quiet at launch. The
469474
// frontend calls `ensure_network_discovery_started` lazily on first user network

apps/desktop/src-tauri/src/network/CLAUDE.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ lifecycle:
4646
- **`network.firstTriggerDone`** (boolean, default `false`, hidden): tracks whether we've already performed a gated
4747
network action. Persisted across launches.
4848

49+
The runtime mirror of `network.enabled` lives in `network::NETWORK_ENABLED` (`AtomicBool`). `lib.rs::setup` seeds it
50+
from the persisted settings; `commands::network::set_network_enabled` keeps it in sync with the live toggle.
51+
`network::is_network_enabled()` is the runtime accessor; BE-side upgrade paths check this before kicking off mDNS or
52+
waiting on hostname resolution.
53+
4954
At startup, mDNS starts only if `network.enabled && (firstTriggerDone || smb-e2e feature)`. On a fresh install,
5055
`firstTriggerDone == false` so we stay quiet and the macOS "Cmdr wants to find devices on local networks" prompt
5156
doesn't fire at app launch.
@@ -160,4 +165,14 @@ convention) and passes it as an explicit mount point to `NetFSMountURLSync`. The
160165
- **Mount URL must include port when non-standard**: `mount_share_sync` builds `smb://server:port/share` for non-445 ports. The port is passed as a separate parameter through `mount_share``mount_share_sync`, not embedded in the server string (embedding it would cause `build_smb_addr` to double the port: `localhost:10480:10480`). `SmbMountInfo.port` extracts the port from `statfs` mount source for upgrade paths.
161166
- **Strip `.local` from addr for smb2**: `smb2::Connection::connect()` extracts `server_name` from the addr string and uses it in UNC paths. Passing `"foo.local:445"` creates `\\foo.local\IPC$` which some servers reject. The `build_addr` helper in `smb_connection.rs` handles this.
162167
- **Manual hosts always set `hostname`**: The share listing pipeline guards on `host.hostname` being truthy. `create_network_host` always sets `hostname` (to the address, even for IPs) so manual hosts flow through the pipeline correctly.
168+
- **SMB upgrade waits briefly for mDNS to warm**: When macOS auto-remounts an SMB share at login, FSEvents fires before
169+
mDNS has discovered the host, so `statfs` gives us an IP but the host map is empty. Stored Keychain credentials are
170+
keyed by mDNS hostname (`smb://naspolya/share`), not by IP, so a sync IP→hostname lookup misses and we'd prompt the
171+
user for credentials they already saved. The upgrade path now (a) kicks off mDNS via `network::ensure_mdns_started`
172+
before resolving and (b) calls `smb_upgrade::resolve_ip_to_hostname_with_wait` which polls the discovered-host map
173+
every 100ms up to 1500ms for private-range IPv4. Non-private IPs (Tailscale, public DNS) skip the wait — mDNS won't
174+
help there. The wait fails open: if mDNS never warms, the IP-only Keychain lookup still runs. Only relevant in dev,
175+
where `network.firstTriggerDone == false` keeps mDNS off at launch; prod users hit this once on the very first install
176+
but never afterwards. Both entry points are covered: `commands::network::upgrade_to_smb_volume` (manual "Connect
177+
directly") and `volumes::watcher::try_upgrade_smb_mount` (FSEvents auto-upgrade).
163178
- **`statfs` can return mDNS service names instead of IPs**: When macOS auto-reconnects an SMB mount on login, `statfs.f_mntfromname` may contain `//user@Naspolya._smb._tcp.local/share` instead of `//user@192.168.1.111/share`. These service names are not DNS-resolvable. `resolve_server_address()` in `commands/network.rs` detects these (by checking for `._tcp`/`._udp`) and resolves them to IPs via `get_discovered_hosts()`. All upgrade paths (startup, mount-time, manual) go through this resolution. Similarly, `friendly_server_name()` extracts the display name (e.g., `Naspolya`) for UI display.

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,45 @@ use crate::ignore_poison::IgnorePoison;
4242
use log::debug;
4343
use serde::{Deserialize, Serialize};
4444
use std::collections::HashMap;
45+
use std::sync::atomic::{AtomicBool, Ordering};
4546
use std::sync::{Mutex, OnceLock};
4647
use tauri::{AppHandle, Emitter};
4748

4849
pub use mdns_discovery::start_discovery;
4950
pub use smb_client::{AuthMode, ShareListError, ShareListResult};
5051

52+
/// Runtime mirror of the `network.enabled` setting. Default `true` matches the
53+
/// settings default. `lib.rs::setup` updates this from the persisted settings;
54+
/// `commands::network::set_network_enabled` keeps it in sync with the live toggle.
55+
static NETWORK_ENABLED: AtomicBool = AtomicBool::new(true);
56+
57+
/// Updates the runtime `network.enabled` flag. Call from app setup (after loading
58+
/// settings) and from the live-toggle command.
59+
pub fn set_network_enabled_flag(enabled: bool) {
60+
NETWORK_ENABLED.store(enabled, Ordering::Relaxed);
61+
}
62+
63+
/// Returns whether networking is enabled. Used by BE-side upgrade paths to decide
64+
/// whether they're allowed to kick off mDNS or wait for hostname resolution.
65+
pub fn is_network_enabled() -> bool {
66+
NETWORK_ENABLED.load(Ordering::Relaxed)
67+
}
68+
69+
/// Idempotently starts mDNS discovery if `network.enabled` is on. Safe to call
70+
/// from any BE-side upgrade path that needs hostname resolution from the mDNS
71+
/// cache. No-op if discovery is already running or the user has disabled
72+
/// networking.
73+
///
74+
/// Unlike `commands::network::ensure_network_discovery_started`, this does NOT
75+
/// re-trigger `upgrade_existing_smb_mounts` — it's meant for paths that ARE the
76+
/// upgrade flow and just need the mDNS daemon up to resolve IP → hostname.
77+
pub fn ensure_mdns_started(app_handle: AppHandle) {
78+
if !is_network_enabled() {
79+
return;
80+
}
81+
start_discovery(app_handle);
82+
}
83+
5184
/// Whether a host was discovered via mDNS or added manually by the user.
5285
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, specta::Type)]
5386
#[serde(rename_all = "snake_case")]

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

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,64 @@ pub(crate) fn resolve_ip_to_hostname(ip: &str) -> Option<String> {
139139
None
140140
}
141141

142+
/// Returns true if `ip` is a literal IPv4 address in a private range (RFC 1918 or
143+
/// link-local 169.254/16). mDNS can only help for those: public/VPN/Tailscale IPs
144+
/// won't show up in the local mDNS cache, so there's no point waiting on them.
145+
///
146+
/// Returns `false` for non-IP strings (hostnames), since `resolve_ip_to_hostname`
147+
/// only matches discovered hosts by exact IP.
148+
pub(crate) fn is_private_ipv4(ip: &str) -> bool {
149+
use std::net::Ipv4Addr;
150+
let Ok(addr) = ip.parse::<Ipv4Addr>() else {
151+
return false;
152+
};
153+
addr.is_private() || addr.is_link_local()
154+
}
155+
156+
/// Like `resolve_ip_to_hostname`, but waits briefly for mDNS to populate the
157+
/// discovered-host cache when the lookup misses on the first try. Solves the
158+
/// startup race where macOS auto-remounts an SMB share, FSEvents fires before
159+
/// mDNS has had time to find the host, and `statfs`-derived IP-only Keychain
160+
/// lookups miss the credentials we have keyed by hostname.
161+
///
162+
/// Only waits for private-range IPv4 addresses (where mDNS is plausible) and only
163+
/// if `is_network_enabled()`. Otherwise returns whatever the immediate sync
164+
/// lookup gave us. Polls every 100ms up to `timeout`. The caller is responsible
165+
/// for kicking off discovery via `network::ensure_mdns_started` before calling
166+
/// this; the wait alone won't start the daemon.
167+
pub(crate) async fn resolve_ip_to_hostname_with_wait(ip: &str, timeout: std::time::Duration) -> Option<String> {
168+
// Fast path: already in the cache.
169+
if let Some(hostname) = resolve_ip_to_hostname(ip) {
170+
return Some(hostname);
171+
}
172+
// No point waiting for non-private IPs (Tailscale, public DNS, etc.) or when
173+
// networking is disabled by the user.
174+
if !is_private_ipv4(ip) || !crate::network::is_network_enabled() {
175+
return None;
176+
}
177+
178+
let poll_interval = std::time::Duration::from_millis(100);
179+
let start = std::time::Instant::now();
180+
while start.elapsed() < timeout {
181+
tokio::time::sleep(poll_interval).await;
182+
if let Some(hostname) = resolve_ip_to_hostname(ip) {
183+
log::debug!(
184+
"Resolved IP {} to hostname {} after waiting {:?}",
185+
ip,
186+
hostname,
187+
start.elapsed()
188+
);
189+
return Some(hostname);
190+
}
191+
}
192+
log::debug!(
193+
"Couldn't resolve IP {} to a hostname via mDNS within {:?}; proceeding without",
194+
ip,
195+
timeout
196+
);
197+
None
198+
}
199+
142200
/// Resolves a server address from `statfs` to a connectable address.
143201
///
144202
/// `statfs` can return different formats depending on how the mount was created:
@@ -236,3 +294,97 @@ pub(crate) async fn get_keychain_password(
236294
.ok()
237295
.flatten()
238296
}
297+
298+
#[cfg(test)]
299+
mod tests {
300+
use super::*;
301+
use std::time::Duration;
302+
303+
#[test]
304+
fn is_private_ipv4_recognizes_rfc1918_and_link_local() {
305+
assert!(is_private_ipv4("10.0.0.1"));
306+
assert!(is_private_ipv4("192.168.1.111"));
307+
assert!(is_private_ipv4("172.16.5.7"));
308+
assert!(is_private_ipv4("169.254.1.2"), "link-local should count");
309+
}
310+
311+
#[test]
312+
fn is_private_ipv4_rejects_public_and_special() {
313+
assert!(!is_private_ipv4("8.8.8.8"));
314+
assert!(!is_private_ipv4("100.64.0.1"), "Tailscale/CGNAT not private");
315+
assert!(!is_private_ipv4("127.0.0.1"), "loopback not private");
316+
assert!(!is_private_ipv4("naspolya"), "hostnames return false");
317+
assert!(!is_private_ipv4(""));
318+
assert!(!is_private_ipv4("::1"), "IPv6 currently returns false");
319+
}
320+
321+
/// `resolve_ip_to_hostname_with_wait` must return immediately (no polling)
322+
/// when the IP isn't a private-range IPv4 — Tailscale/public DNS won't show
323+
/// up in mDNS so there's nothing to wait for.
324+
#[tokio::test]
325+
async fn wait_helper_returns_immediately_for_non_private_ip() {
326+
let start = std::time::Instant::now();
327+
let result = resolve_ip_to_hostname_with_wait("8.8.8.8", Duration::from_millis(500)).await;
328+
let elapsed = start.elapsed();
329+
assert_eq!(result, None);
330+
assert!(
331+
elapsed < Duration::from_millis(50),
332+
"expected fast path (< 50ms), took {:?}",
333+
elapsed
334+
);
335+
}
336+
337+
/// `resolve_ip_to_hostname_with_wait` must short-circuit when the runtime
338+
/// `network.enabled` flag is off, even for a private IP — mDNS isn't running
339+
/// so polling would just burn the timeout.
340+
#[tokio::test]
341+
async fn wait_helper_short_circuits_when_network_disabled() {
342+
let prev = crate::network::is_network_enabled();
343+
crate::network::set_network_enabled_flag(false);
344+
345+
let start = std::time::Instant::now();
346+
let result = resolve_ip_to_hostname_with_wait("192.168.1.111", Duration::from_millis(500)).await;
347+
let elapsed = start.elapsed();
348+
349+
// Restore before assertions so other tests aren't poisoned by panics.
350+
crate::network::set_network_enabled_flag(prev);
351+
352+
assert_eq!(result, None);
353+
assert!(
354+
elapsed < Duration::from_millis(50),
355+
"expected fast path (< 50ms), took {:?}",
356+
elapsed
357+
);
358+
}
359+
360+
/// Times out gracefully when no host ever shows up in the cache (and falls
361+
/// back to `None` so the caller can use IP-only Keychain lookup).
362+
#[tokio::test]
363+
async fn wait_helper_times_out_gracefully() {
364+
// Ensure network is "enabled" so we exercise the polling path.
365+
let prev = crate::network::is_network_enabled();
366+
crate::network::set_network_enabled_flag(true);
367+
368+
// Use a unique private IP that no test has ever populated, so the cache
369+
// miss is deterministic.
370+
let timeout = Duration::from_millis(300);
371+
let start = std::time::Instant::now();
372+
let result = resolve_ip_to_hostname_with_wait("10.255.255.254", timeout).await;
373+
let elapsed = start.elapsed();
374+
375+
crate::network::set_network_enabled_flag(prev);
376+
377+
assert_eq!(result, None);
378+
assert!(
379+
elapsed >= timeout,
380+
"should have polled until timeout; elapsed {:?}",
381+
elapsed
382+
);
383+
// Generous upper bound — single poll interval slack.
384+
assert!(
385+
elapsed < timeout + Duration::from_millis(250),
386+
"shouldn't blow past timeout by much; elapsed {:?}",
387+
elapsed
388+
);
389+
}
390+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ pub enum UpgradeResult {
353353
/// Upgrades an SMB volume to use direct smb2 (stub: returns error).
354354
#[tauri::command]
355355
#[specta::specta]
356-
pub async fn upgrade_to_smb_volume(_volume_id: String) -> Result<UpgradeResult, String> {
356+
pub async fn upgrade_to_smb_volume(_volume_id: String, _app_handle: tauri::AppHandle) -> Result<UpgradeResult, String> {
357357
Ok(UpgradeResult::NetworkError {
358358
message: "Direct SMB connection not supported on this platform".to_string(),
359359
})
@@ -367,6 +367,7 @@ pub async fn upgrade_to_smb_volume_with_credentials(
367367
_username: Option<String>,
368368
_password: Option<String>,
369369
_remember_in_keychain: bool,
370+
_app_handle: tauri::AppHandle,
370371
) -> Result<UpgradeResult, String> {
371372
Ok(UpgradeResult::NetworkError {
372373
message: "Direct SMB connection not supported on this platform".to_string(),

apps/desktop/src-tauri/src/volumes/watcher.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,9 +232,22 @@ fn try_upgrade_smb_mount(volume_path: &str) {
232232
return;
233233
};
234234

235+
// Kick mDNS off here (idempotent, no-op if already running or if
236+
// `network.enabled` is off). In dev mode `network.firstTriggerDone` is
237+
// typically `false`, so the launch-time mDNS gate doesn't fire and
238+
// hostname resolution would otherwise miss when macOS auto-remounts an
239+
// SMB share at login. See `network::smb_upgrade::resolve_ip_to_hostname_with_wait`.
240+
if let Some(app) = APP_HANDLE.get() {
241+
crate::network::ensure_mdns_started(app.clone());
242+
}
243+
235244
let mount_path = volume_path.to_string();
236245
tauri::async_runtime::spawn(async move {
237-
let hostname = crate::network::smb_upgrade::resolve_ip_to_hostname(&info.server);
246+
let hostname = crate::network::smb_upgrade::resolve_ip_to_hostname_with_wait(
247+
&info.server,
248+
std::time::Duration::from_millis(1500),
249+
)
250+
.await;
238251
let creds =
239252
crate::network::smb_upgrade::get_keychain_password(&info.server, hostname.as_deref(), &info.share).await;
240253
let (username, password) = match &creds {

0 commit comments

Comments
 (0)