fix(builderbot-auth): default non-macOS session storage to a file

[corydouthat-sq]

Summary

The OS keyring backend for browser auth sessions is only implemented on macOS, yet default_session_storage_for_bb_home returned it on every platform, and stored_session_credential_header_value short-circuited to Ok(None) off macOS unless BB_AUTH_STORAGE or BB_AUTH_STORAGE_FILE was set. On Windows and Linux that meant kgoose requests left add_shared_session_credential without the session header, so Settings > Connections looked silently broken until each user exported BB_AUTH_STORAGE=file by hand.

This PR:

• defaults to the existing file store (<bb home>/auth-sessions.json) on non-macOS platforms and drops the early return; macOS behaviour is unchanged;
• advertises only the storage kinds a platform actually supports in the "unsupported BB_AUTH_STORAGE" error and the keyring-refusal message;
• says plainly, in code comments, bb-cli/docs/bb-auth-local-testing.md, and the doctor-windows warning, that the file is protected by file permissions rather than a keyring (0600 on Unix, a current-user-only ACL on Windows);
• makes the file store safe to share between the desktop app and the CLI: writers hold an exclusive OS lock on a sibling .lock file (std::fs::File::lock, so the crate MSRV becomes 1.89) and replace the document atomically through a temp file plus rename, so readers never see a partial file;
• resolves bb home from USERPROFILE on Windows when HOME is absent (native processes rarely have HOME), so the credential file lands in an absolute per-user location instead of a cwd-relative .bb, and resets the file ACL with icacls so only the current user has access (the 0600 equivalent);
• adds tests for the platform default, the explicit BB_AUTH_STORAGE=file:<path> and BB_AUTH_STORAGE_FILE overrides, the macOS-only keyring refusal, header lookup through the default store, set/get/delete round-tripping, the error text, concurrent writers in threads and in a second process, and (Windows-only) the USERPROFILE fallback and the single-user ACL. Environment access in the tests is serialized with a static mutex since the crate has no serial_test dependency.

restrict_permissions also takes &Path and marks the argument used on non-Unix so cargo clippy -D warnings passes for the crate on Windows (it was never linted there: the crate is a path dependency, not a workspace member).

Why now: an internal Windows build of Berd hit this in the wild; the internal repository carries this exact change as a patch until it lands here.

Related issue

None found.

Testing

• cargo fmt --check, cargo clippy --all-targets -- -D warnings, and cargo test for crates/builderbot-auth on Windows 11 (34 tests, including the 11 new ones; the cfg(not(target_os = "macos")) tests exercise the new default and the two cfg(windows) tests the home fallback and ACL).
• macOS: the default remains KeyringSessionCredentialStorage; the platform-default test asserts it. Not run locally (Windows machine); CI covers it.
• Windows app built from this change with no BB_AUTH_STORAGE in the environment: bb.exe auth status works and Connections send the session header.

[morgmart]

🤖 Automated code review

REQUEST_CHANGES: the new non-macOS default makes the existing file store part of the normal Berd and bb authentication path, but that store is not safe across processes and Windows can resolve its supposedly private location to a relative working-directory path. The change is non-UI and needs no screenshots. Supplied GitHub evidence is structurally available; ten checks passed and Transcript virtualization failed, so required checks still govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

[morgmart]

🤖 P1 · Make default storage process-safe (blocking)

This branch makes the shared JSON store the default for every non-macOS caller. That store performs unlocked read-modify-write updates and truncates the destination directly, so Berd and bb can overlap and either overwrite another credential update or read partially written JSON.

User effect: People can be signed out unexpectedly or see Connections and bb authentication fail intermittently when the desktop app and CLI use the session store at the same time.

Recommended fix: Coordinate access across processes and replace the file atomically so readers never observe partial content and concurrent updates cannot silently overwrite one another.

Test: Add a discriminating multi-process or equivalent concurrent-writer test that overlaps updates to different entries and proves both entries remain readable with valid JSON.

[corydouthat-sq]

Fixed in b038bda. set/delete now run under an exclusive OS lock on a sibling auth-sessions.json.lock (std::fs::File::lock; crate MSRV moves to 1.89, the repo toolchain is 1.94), and the document is written to a temp file in the same directory, restricted, then renamed over the store, so readers only ever see a complete document. Tests: concurrent_writers_keep_every_entry (8 handles in threads, distinct keys, 5 rounds each) and concurrent_processes_keep_every_entry (the test binary re-spawns itself as a second writer process); both assert every entry survives and parses, and that no temp files are left behind.

[morgmart]

🤖 P1 · Use a private Windows home (blocking)

The new Windows default writes beneath bb_home and claims protection from the user-profile ACL, but the canonical home resolver still reads HOME and falls back to relative .bb when HOME is absent. A native Windows process can therefore place the credential under its working directory, where the claimed profile ACL and stable location are not guaranteed.

User effect: Windows users may lose sign-in between launches, fail to sign in when the working directory is unwritable, or leave their browser session credential somewhere other users can access.

Recommended fix: Resolve the Windows default from a platform-native per-user private data location and ensure the credential directory and file have an explicitly private ACL before using file storage by default.

Test: Add a Windows-specific test with HOME absent that proves the resolved store is an absolute per-user path outside the working directory and has private access controls.

[corydouthat-sq]

Fixed in b038bda. default_bb_home now falls back to USERPROFILE on Windows when HOME is absent (only when both are missing does it still use a relative .bb), and restrict_permissions on Windows runs icacls <file> /inheritance:r /grant:r <user>:F on the temp file before the rename, so the credential file carries a current-user-only ACL (the 0600 equivalent) rather than inherited entries. Windows-only tests: windows_default_bb_home_uses_the_user_profile_when_home_is_absent (HOME removed, USERPROFILE = temp dir, asserts an absolute path under it) and windows_file_storage_grants_only_the_current_user (lists the ACL with icacls, asserts a single ACE for the current user and no BUILTIN/NT AUTHORITY entries).

[morgmart]

🤖 Automated code review

REQUEST_CHANGES: the earlier cross-process update and Windows home-location problems are addressed, but credential bytes are still written before the temporary file is made private, and the CLI still advertises a Rust version too old for its updated authentication dependency. This is a non-UI change and needs no screenshots. All ten supplied checks pass; required checks still independently govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

[morgmart]

🤖 P1 · Secure temporary file before writing (blocking)

The replacement path writes the complete credential document to a newly created temporary file before applying restrictive permissions. During that interval the file inherits its parent directory's access, so inherited principals can read session credentials, particularly when an explicit storage path points outside the private default directory. Unix has the same create-then-chmod window.

User effect: A person's browser session credential can briefly be readable by other accounts or inherited access groups whenever the session store is updated.

Recommended fix: Create and open the temporary file with current-user-only access before writing any credential bytes, then write, flush as appropriate, and atomically replace the destination.

Test: Add a discriminating test around the temporary-file creation boundary that verifies the file is private before credential content is written on Windows and Unix.

[morgmart]

🤖 P1 · Align the CLI Rust minimum (blocking)

The authentication crate now requires Rust 1.89, while the bb CLI package that depends on it still declares Rust 1.88. Building the CLI with its advertised minimum compiler will fail during dependency resolution even though CI passes on newer toolchains.

User effect: Contributors and packaging environments using the CLI's documented minimum Rust version will be unable to build bb.

Recommended fix: Raise the bb CLI package's declared minimum Rust version to at least 1.89, or implement locking without increasing the authentication crate's minimum.

Test: Add or update the minimum-supported-Rust validation so the CLI and its path dependencies are checked with the declared compiler version.

[kalvinnchau]

🤖 CI coverage gap

[kalvinnchau]

🤖 P2 · Execute the new storage tests in CI

This Windows ACL test and the new process-safety tests are not run by the repository's current CI recipes: builderbot-auth is not a src-tauri workspace member, and the Windows recipe does not invoke its manifest directly. The change therefore makes this backend the non-macOS default without CI executing the behavior that protects its credentials.

Please add cargo test --manifest-path crates/builderbot-auth/Cargo.toml to the platform matrix, including the Windows runner so this #[cfg(windows)] ACL path executes.