Fix: SMB share-open no longer triggers macOS auth dialog#17
Closed
vdavid wants to merge 5 commits into
Closed
Conversation
`mount_network_share` now opens an SMB share with a direct smb2 session and synthesizes a `/Volumes/<share>` path. The old NetFS path (and `gio mount` on Linux) is gone from the default flow, so macOS no longer pops the kernel `smbfs` credentials dialog when Cmdr already has working credentials. `SmbVolume::supports_local_fs_access() = false`, so the logical path never gets statted. - New `network::smb_upgrade::connect_smb_volume_direct` does the connect, registers the `SmbVolume`, and maps `smb2::Error` to typed `MountError` variants by `ErrorKind` (no string matching) so the FE can re-prompt inline. - `network.directSmbConnection` now also gates this path: when off, falls back to the legacy OS-mount-then-upgrade flow so users who want Finder to see the same mount keep that option. - Watcher + `upgrade_to_smb_volume` paths unchanged (still handle Finder/Cmd+K-mounted shares). - Adds Rust integration tests against `smb-consumer-guest`/`-auth`: asserts no `smbfs` entry shows up at `/Volumes/<share>` after the command, plus a typed-error guard for bad-password and a unit test for the unreachable-host fast path.
- Regenerated `bindings.ts` to pick up the new doc comments on `mount_network_share`. - `network.directSmbConnection` description now explains the no-OS-mount semantics (no macOS keychain dialog) and when to turn it off. - Two extra IPC contract tests in `network-smb.test.ts`: `auth_required` from the direct-smb2 path (for inline re-prompting) and the synthesized `/Volumes/<share>` happy-path shape.
Decision entry in `volume/CLAUDE.md` explains why the FE-initiated share-open uses direct smb2 only (no OS mount). Captures the trade-off the setting toggle preserves and notes that the watcher / `upgrade_to_smb_volume` paths still cover external-mount upgrades.
The kernel `smbfs` credentials dialog we're avoiding is a macOS-only behavior. On Linux, `mount_network_share` goes through `gio mount` / gvfs, which is transparent and doesn't pop a kernel prompt — and the gvfs path (`/run/user/<uid>/gvfs/...`) is what the FE and existing Playwright suite (smb.spec.ts) expect. Cutting that on Linux broke the Linux E2E `mounting guest share navigates to mounted path` test. Make `use_direct = is_direct_smb_enabled()` only on macOS, and `false` unconditionally on Linux so it keeps the legacy gvfs path. Also gates the new macOS-specific Docker regression tests with `cfg(target_os = "macos")` since their `mount_path == /Volumes/<share>` assertion is the synthesized macOS shape — Linux's gvfs path has a different format. Net behavior: - macOS, setting on (default): direct smb2 only, no NetFS, no dialog. - macOS, setting off: legacy NetFS + smb2 upgrade. - Linux: legacy gvfs + smb2 upgrade (unchanged).
After the previous commit made the direct-smb2 share-open path macOS-only, the macOS-only test bodies sit inside a `#[cfg(test)]` module whose helpers (`guest_port`, `auth_port`, `is_os_smb_mount`, `cleanup_registered`) only exist for those tests. On Linux CI clippy flagged every helper and every test fn as unused, dead code, or stale import. Switch the module guard to `#[cfg(all(test, target_os = "macos"))]` and drop the now-redundant per-fn `cfg(target_os = "macos")` / platform-split `is_os_smb_mount` variants. The module compiles only on macOS; clippy on Linux no longer sees it; the tests still run as before on the macOS Rust-tests job.
Owner
Author
|
Approach was structurally wrong — synthesizing /Volumes/ bypasses path-to-volume resolution and lands the pane back at /Volumes. Restarting with a cleaner approach: pass user/passwd explicitly to NetFSMountURLSync so the system never falls back to the Keychain prompt. New PR incoming. |
vdavid
added a commit
that referenced
this pull request
May 18, 2026
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.
vdavid
added a commit
that referenced
this pull request
May 18, 2026
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opening an SMB share from Cmdr's Network view used to pop the macOS kernel
smbfscredentials dialog, even when Cmdr had already authenticated and listed the share. Root cause:mount_network_shareunconditionally calledNetFSMountURLSyncbefore establishing its own smb2 session. The dialog hijacked focus and the OS mount was wasted work, because Cmdr's data path already uses the direct smb2 session.mount_network_sharenow goes direct-smb2-only by default: connects, registers theSmbVolume, returns a synthesized/Volumes/<share>(logical address, never statted sinceSmbVolume::supports_local_fs_access() = false).MountErrorvariants bysmb2::ErrorKind(no string matching), soNetworkMountViewre-prompts inline on auth failure.network.directSmbConnection(defaulttrue) now gates this path too: whenfalse, the command falls back to the legacy OS-mount-then-upgrade flow for users who want Finder to see the same mount. The watcher (try_upgrade_smb_mount) and the manualupgrade_to_smb_volumecommand still upgrade shares mounted via Finder /Cmd+K.volume/CLAUDE.mdrecords the decision and the trade-off.smb-consumer-guestandsmb-consumer-auth: assert that aftermount_network_sharesucceeds,/Volumes/<share>has nosmbfsentry viastatfs. Plus a typed-error guard for the bad-password path and a unit test for the unreachable-host fast path. Two extra FE IPC contract tests coverauth_requiredand the synthesized happy-path shape.Test plan
./scripts/check.sh --rust(withsmb-consumercore stack up): all 13 checks pass, includingrust-integration-tests(32 tests).cd apps/desktop && pnpm check: 0 errors, 0 warnings.cd apps/desktop && pnpm vitest run src/lib/ipc/network-smb.test.ts: 8/8 pass.pnpm exec oxlint test/ src/lib/ipc/: clean../scripts/check.sh --check oxfmt: clean.cargo nextest run --release --run-ignored only -E 'test(mount_network_share)': 3 integration tests pass (skips_os_mount_guest,skips_os_mount_auth,bad_password_is_typed_auth_failure).cargo nextest run -E 'test(mount_network_share_unreachable)': 1 unit test passes.statfscheck in the integration tests, which would fail today against the old code (NetFS would create ansmbfsmount in CI's macOS runner).