diff --git a/Cargo.lock b/Cargo.lock index f1a5fd9..256270d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -162,6 +162,12 @@ dependencies = [ "serde", ] +[[package]] +name = "ambient-authority" +version = "0.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" + [[package]] name = "android_system_properties" version = "0.1.5" @@ -1146,6 +1152,48 @@ dependencies = [ "wayland-client", ] +[[package]] +name = "cap-fs-ext" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d78e5a3368ae89b7cb68186411452b4b9fac8b41be9c19bf3f47c2d2c8e36e6b" +dependencies = [ + "cap-primitives", + "cap-std", + "io-lifetimes 3.0.1", + "windows-sys 0.61.2", +] + +[[package]] +name = "cap-primitives" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdadbd7c002d3a484b35243669abdae85a0ebaded5a61117169dc3400f9a7ff0" +dependencies = [ + "ambient-authority", + "fs-set-times", + "io-extras", + "io-lifetimes 3.0.1", + "ipnet", + "maybe-owned", + "rustix 1.1.4", + "rustix-linux-procfs", + "windows-sys 0.61.2", + "winx", +] + +[[package]] +name = "cap-std" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7281235d6e96d3544ca18bba9049be92f4190f8d923e3caef1b5f66cfa752608" +dependencies = [ + "cap-primitives", + "io-extras", + "io-lifetimes 3.0.1", + "rustix 1.1.4", +] + [[package]] name = "cbc" version = "0.1.2" @@ -2445,6 +2493,8 @@ name = "ely_app" version = "0.1.0" dependencies = [ "ahash", + "cap-fs-ext", + "cap-std", "core-foundation 0.10.0", "core-video", "directories", @@ -2454,6 +2504,7 @@ dependencies = [ "ely_domain", "ely_servo_host", "ely_sync_client", + "fs2", "gpui", "gpui-component", "gpui-component-assets", @@ -2532,7 +2583,10 @@ dependencies = [ name = "ely_sync_client" version = "0.1.0" dependencies = [ + "apple-native-keyring-store", "base64", + "cap-fs-ext", + "cap-std", "chacha20poly1305", "ed25519-dalek 2.2.0", "ely_domain", @@ -2541,13 +2595,17 @@ dependencies = [ "hkdf 0.12.4", "hmac 0.12.1", "hpke", - "keyring", + "keyring-core", + "rustix 1.1.4", "serde", "serde_json", "sha2 0.10.9", + "tempfile", "thiserror 2.0.18", "ureq", "uuid", + "windows-native-keyring-store", + "zbus-secret-service-keyring-store", "zeroize", ] @@ -3108,6 +3166,17 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "fs-set-times" +version = "0.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94e7099f6313ecacbe1256e8ff9d617b75d1bcb16a6fddef94866d225a01a14a" +dependencies = [ + "io-lifetimes 2.0.4", + "rustix 1.1.4", + "windows-sys 0.59.0", +] + [[package]] name = "fs2" version = "0.4.3" @@ -4999,6 +5068,28 @@ dependencies = [ "rustversion", ] +[[package]] +name = "io-extras" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20fd6de4ccfcc187e38bc21cfa543cb5a302cb86a8b114eb7f0bf0dc9f8ac00f" +dependencies = [ + "io-lifetimes 3.0.1", + "windows-sys 0.59.0", +] + +[[package]] +name = "io-lifetimes" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06432fb54d3be7964ecd3649233cddf80db2832f47fec34c01f65b3d9d774983" + +[[package]] +name = "io-lifetimes" +version = "3.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f0fb0570afe1fed943c5c3d4102d5358592d8625fda6a0007fdbe65a92fba96" + [[package]] name = "io-surface" version = "0.16.1" @@ -5241,18 +5332,6 @@ dependencies = [ "unicode-segmentation", ] -[[package]] -name = "keyring" -version = "4.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6ee8d4dae108d4177d0a0ce241f98acc1ef28e20837bdef43cff4d160cf70fe" -dependencies = [ - "apple-native-keyring-store", - "keyring-core", - "windows-native-keyring-store", - "zbus-secret-service-keyring-store", -] - [[package]] name = "keyring-core" version = "1.0.0" @@ -5662,6 +5741,12 @@ dependencies = [ "regex-automata 0.4.14", ] +[[package]] +name = "maybe-owned" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4facc753ae494aeb6e3c22f839b158aebd4f9270f55cd3c79906c45476c47ab4" + [[package]] name = "maybe-rayon" version = "0.1.1" @@ -7872,6 +7957,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustix-linux-procfs" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fc84bf7e9aa16c4f2c758f27412dc9841341e16aa682d9c7ac308fe3ee12056" +dependencies = [ + "once_cell", + "rustix 1.1.4", +] + [[package]] name = "rustls" version = "0.23.40" @@ -12608,6 +12703,16 @@ version = "0.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904" +[[package]] +name = "winx" +version = "0.36.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f3fd376f71958b862e7afb20cfe5a22830e1963462f3a17f49d82a6c1d1f42d" +dependencies = [ + "bitflags 2.11.1", + "windows-sys 0.59.0", +] + [[package]] name = "wio" version = "0.2.2" diff --git a/Cargo.toml b/Cargo.toml index bef273a..65b44ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,8 @@ rust-version = "1.95" [workspace.dependencies] base64 = "0.22.1" +cap-fs-ext = "4.0.2" +cap-std = "4.0.2" chacha20poly1305 = "0.11.0" directories = "6.0.0" dpi = "0.1" @@ -32,7 +34,8 @@ hkdf = "0.12.4" hmac = "0.12.1" hpke = { version = "0.14.0", default-features = false, features = ["alloc", "chacha", "getrandom", "x25519"] } image = "0.25.10" -keyring = "4.1.4" +keyring-core = "1.0.0" +rustix = "1.1.4" servo = { version = "0.4.0", git = "https://github.com/servo/servo.git", rev = "c983c232385edf1170118b24637d2eb2564674d7" } sha2 = "0.10.9" semver = "1.0.28" diff --git a/PRD.md b/PRD.md index 7cc4e9e..834e8fa 100644 --- a/PRD.md +++ b/PRD.md @@ -960,6 +960,7 @@ Better Auth 在 Cloudflare Workers 中初始化，D1 binding 作为 database 传 - Google/GitHub OAuth 按环境启用。 - D1 session validation 与自定义 device/session binding。 - Bearer logout 精确删除当前 D1 session，并级联清理 session device context 与 rebind challenge。 +- Desktop bearer 以 stable `ProfileId` 作为系统凭据 account，Windows 使用 Local persistence；旧明文文件在 credential read-back 与 durable marker 提交后清理，系统凭据不可用时阻断设备加载与 Sync upload。 - 设备注册、rebind、批准、撤销与 Vault rotation。 - Signed Sync reset 和 signed account deletion。 - 管理所有 `/api/auth/*` 路由。 @@ -1571,7 +1572,7 @@ Servo Host 需要实现： | ID | 模块 | 验收标准 | |---|---|---| | S-001 | Sync encryption | 服务端无法解密 Sync payload | -| S-002 | Keychain | session token 和本地数据库密钥存入系统钥匙串 | +| S-002 | Keychain | session token 按 stable ProfileId 隔离存入原生系统凭据；明文 legacy token 完成可恢复的一次迁移后删除；凭据不可用时进入独立错误状态并暂停云端动作 | | S-003 | Device revoke | 撤销设备后目标 sessions 失效、后续 Sync API 被拒绝、approved revoke 原子轮换 Vault generation | | S-004 | Plugin sandbox | 插件无声明权限时无法访问 tabs/bookmarks/history/page | | S-005 | Plugin signature | 市场插件必须签名验证通过 | diff --git a/cloudflare/src/auth.ts b/cloudflare/src/auth.ts index 76765c5..91a3ee5 100644 --- a/cloudflare/src/auth.ts +++ b/cloudflare/src/auth.ts @@ -3,7 +3,7 @@ import { primaryD1Session } from "./bindings.js"; import { prefixedKvKey } from "./kv_keys.js"; const AUTH_SESSION_CACHE_NAMESPACE = "auth_session_cache"; -const BEARER_TOKEN_PATTERN = /^[A-Za-z0-9._~+/=-]{32,4096}$/; +const BEARER_TOKEN_PATTERN = /^[A-Za-z0-9._~+/=-]{32,2560}$/; const SUBJECT_ID_PATTERN = /^[a-zA-Z0-9._:-]{3,128}$/; const DEVICE_ID_PATTERN = /^[a-zA-Z0-9._:-]{3,128}$/; const BETTER_AUTH_SESSION_QUERY = ` diff --git a/cloudflare/tests/api_controls.test.ts b/cloudflare/tests/api_controls.test.ts index 9ff0540..407d74f 100644 --- a/cloudflare/tests/api_controls.test.ts +++ b/cloudflare/tests/api_controls.test.ts @@ -149,6 +149,23 @@ describe("api controls", () => { ]); }); + it("rejects bearer tokens larger than the native credential contract", async () => { + const d1Queries: string[] = []; + const response = await withAuthenticatedApiControls( + new Request("https://elydora.test/api/devices", { + headers: { authorization: `Bearer ${"A".repeat(2561)}` }, + }), + testEnv({ d1: testD1Database({ queries: d1Queries }) }), + "devices.list", + ["GET"], + () => Promise.resolve(jsonResponse({ ok: true }, 200)), + ); + + assert.equal(response.status, 401); + assert.deepEqual(await response.json(), { error: "authorization_invalid" }); + assert.deepEqual(d1Queries, []); + }); + it("passes authenticated session context and records subject audit fields", async () => { const tokenHash = await authTokenHash(ACCESS_TOKEN); const auditEvents: ElyAnalyticsDataPoint[] = []; diff --git a/crates/ely_app/Cargo.toml b/crates/ely_app/Cargo.toml index a11a5d0..a4a46e4 100644 --- a/crates/ely_app/Cargo.toml +++ b/crates/ely_app/Cargo.toml @@ -10,12 +10,15 @@ live-site-smoke = [] [dependencies] ahash = "0.8" +cap-fs-ext.workspace = true +cap-std.workspace = true directories.workspace = true ed25519-dalek.workspace = true ely_browser_core = { path = "../ely_browser_core" } ely_design_system = { path = "../ely_design_system" } ely_domain = { path = "../ely_domain" } ely_sync_client = { path = "../ely_sync_client" } +fs2.workspace = true gpui.workspace = true gpui-component.workspace = true gpui-component-assets.workspace = true @@ -29,6 +32,7 @@ tracing.workspace = true tracing-subscriber.workspace = true ureq.workspace = true url.workspace = true +uuid.workspace = true [target.'cfg(target_os = "macos")'.dependencies] core-foundation = "0.10" @@ -37,7 +41,6 @@ io-surface = "0.16" mach2 = "0.6" objc2-core-foundation = { version = "0.3.2", features = ["CFBase", "CFDictionary", "CFNumber", "CFString"] } objc2-io-surface = "0.3.2" -uuid.workspace = true [dev-dependencies] ely_servo_host = { path = "../ely_servo_host" } diff --git a/crates/ely_app/src/shell/auth.rs b/crates/ely_app/src/shell/auth.rs index 2786fb7..e3ee33b 100644 --- a/crates/ely_app/src/shell/auth.rs +++ b/crates/ely_app/src/shell/auth.rs @@ -139,29 +139,54 @@ impl ElyShell { } /// Drop the persisted bearer token and reset the local form. - /// The bearer file is removed synchronously — there is no network - /// call to make, the token is the only artefact we own. pub(crate) fn submit_sign_out(&mut self, _cx: &mut Context) { - self.auth_flow_phase = AuthFlowPhase::Idle; - self.sync_devices.reset(); - self.sync_retry_at = None; - self.clear_pending_cloud_sync_upload(); let active_profile_id = match active_profile_id_for(&self.state) { Some(profile_id) => profile_id, None => return, }; + self.auth_flow_phase = AuthFlowPhase::Idle; + self.sync_devices.reset(); + self.sync_upload_scheduled = false; + self.sync_retry_at = None; + self.clear_pending_cloud_sync_upload(); let Some(profile_root) = default_profile_data_root() else { + self.set_sign_out_error( + active_profile_id, + "Profile data root is unavailable. Retry sign out.", + ); return; }; let profile_dir = sync_profile_data_dir(&profile_root, &active_profile_id); - if let Err(error) = clear_persisted_bearer(&profile_dir) { + if let Err(error) = clear_persisted_bearer( + &active_profile_id, + &profile_dir, + &profile_root, + self.default_profile_id.as_ref(), + ) { tracing::warn!(target: "ely::sync", error = %error, "sign-out failed to clear bearer"); + self.set_sign_out_error( + active_profile_id, + "System credential access failed. Retry sign out.", + ); + return; } if let ShellState::Ready(core) = &mut self.state { core.set_sync_connection_state(ely_domain::SyncConnectionState::SignedOut); } } + fn set_sign_out_error(&mut self, profile_id: ProfileId, message: &str) { + self.auth_flow_phase = + AuthFlowPhase::Error { profile_id, email: String::new(), message: message.to_string() }; + if let ShellState::Ready(core) = &mut self.state { + core.set_sync_connection_state( + ely_domain::SyncConnectionState::CredentialUnavailable { + message: message.to_string(), + }, + ); + } + } + fn read_auth_email_input(&self, cx: &Context) -> String { self.auth_email_input.read(cx).value().to_string() } @@ -201,8 +226,28 @@ fn active_profile_id_for(state: &ShellState) -> Option { core.snapshot().ok().map(|snapshot| snapshot.active_profile_id) } -pub(super) fn clear_persisted_bearer(profile_dir: &Path) -> Result<(), SyncClientError> { - BearerTokenStore::new(profile_dir.join("sync").join("bearer.token")).clear() +pub(super) fn clear_persisted_bearer( + profile_id: &ProfileId, + profile_dir: &Path, + profile_root: &Path, + default_profile_id: Option<&ProfileId>, +) -> Result<(), SyncClientError> { + bearer_store_for_profile(profile_id, profile_dir, profile_root, default_profile_id).clear() +} + +pub(super) fn bearer_store_for_profile( + profile_id: &ProfileId, + profile_dir: &Path, + profile_root: &Path, + default_profile_id: Option<&ProfileId>, +) -> BearerTokenStore { + let store = BearerTokenStore::new(profile_id, profile_dir); + if default_profile_id != Some(profile_id) { + return store; + } + store.with_legacy_path( + profile_root.join("default").join("servo").join("sync").join("bearer.token"), + ) } fn spawn_send_otp(profile_id: ProfileId, email: String, tx: Sender) { @@ -252,7 +297,12 @@ fn spawn_verify_otp( } }; let mut engine = - match SyncEngine::for_profile_dir(&profile_dir, "ELY", sync_platform_label()) { + match SyncEngine::for_profile_dir( + &profile_id, + &profile_dir, + "ELY", + sync_platform_label(), + ) { Ok(engine) => engine, Err(error) => { let _ = tx.send(SyncStateUpdate::AuthError { @@ -284,7 +334,9 @@ mod tests { use ely_browser_core::{BrowserCore, InitialBrowserConfig}; use ely_domain::ProfileId; - use super::{AuthFlowPhase, active_profile_sync_context_for, normalize_email}; + use super::{ + AuthFlowPhase, active_profile_sync_context_for, bearer_store_for_profile, normalize_email, + }; use crate::shell::ShellState; #[test] @@ -334,4 +386,57 @@ mod tests { Ok(()) } + + #[test] + fn default_profile_store_cleans_stable_and_old_legacy_paths() + -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let profile_dir = directory.path().join(profile_id.as_str()).join("servo"); + let stable = profile_dir.join("sync/bearer.token"); + let old = directory.path().join("default/servo/sync/bearer.token"); + std::fs::create_dir_all(stable.parent().ok_or("missing stable parent")?)?; + std::fs::create_dir_all(old.parent().ok_or("missing old parent")?)?; + std::fs::write(&stable, "a".repeat(64))?; + std::fs::write(&old, "b".repeat(64))?; + let store = bearer_store_for_profile( + &profile_id, + &profile_dir, + directory.path(), + Some(&profile_id), + ); + + store.clear_legacy_files()?; + + assert!(!stable.exists()); + assert!(!old.exists()); + Ok(()) + } + + #[test] + fn custom_profile_store_leaves_default_legacy_credentials_untouched() + -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let default_profile_id = ProfileId::new(); + let profile_dir = directory.path().join(profile_id.as_str()).join("servo"); + let stable = profile_dir.join("sync/bearer.token"); + let old_default = directory.path().join("default/servo/sync/bearer.token"); + std::fs::create_dir_all(stable.parent().ok_or("missing stable parent")?)?; + std::fs::create_dir_all(old_default.parent().ok_or("missing default parent")?)?; + std::fs::write(&stable, "a".repeat(64))?; + std::fs::write(&old_default, "b".repeat(64))?; + let store = bearer_store_for_profile( + &profile_id, + &profile_dir, + directory.path(), + Some(&default_profile_id), + ); + + store.clear_legacy_files()?; + + assert!(!stable.exists()); + assert!(old_default.exists()); + Ok(()) + } } diff --git a/crates/ely_app/src/shell/internal_pages/sync.rs b/crates/ely_app/src/shell/internal_pages/sync.rs index ff7f3a1..a03bde8 100644 --- a/crates/ely_app/src/shell/internal_pages/sync.rs +++ b/crates/ely_app/src/shell/internal_pages/sync.rs @@ -15,7 +15,6 @@ use super::sync_controls::{ render_reset_button, render_secondary_button, render_sign_out_button, }; use super::{ElyShell, render_canvas_surface}; - impl ElyShell { pub(super) fn render_sync_page( &mut self, @@ -23,7 +22,10 @@ impl ElyShell { cx: &mut Context, ) -> AnyElement { if profile_allows_sync_controls(&snapshot.active_profile_kind) - && !matches!(snapshot.sync_status.connection(), SyncConnectionState::SignedOut) + && !matches!( + snapshot.sync_status.connection(), + SyncConnectionState::SignedOut | SyncConnectionState::CredentialUnavailable { .. } + ) { self.ensure_sync_devices_loaded(cx); } @@ -37,7 +39,6 @@ impl ElyShell { ) } } - fn render_sync_body( shell: &mut ElyShell, snapshot: &BrowserSnapshot, @@ -63,11 +64,9 @@ fn render_sync_body( ) .into_any_element() } - fn profile_allows_sync_controls(profile_kind: &ProfileKind) -> bool { profile_kind == &ProfileKind::Standard } - fn render_private_profile_card() -> AnyElement { div() .p(px(16.0)) @@ -88,7 +87,6 @@ fn render_private_profile_card() -> AnyElement { ) .into_any_element() } - fn render_account_card( shell: &mut ElyShell, snapshot: &BrowserSnapshot, @@ -102,6 +100,11 @@ fn render_account_card( .child(render_card_heading("Account")) .children(account_form(shell, &snapshot.active_profile_id, cx)) .into_any_element(), + SyncConnectionState::CredentialUnavailable { message } => card + .child(render_card_heading("Account")) + .child(render_inline_error(message)) + .children(account_form(shell, &snapshot.active_profile_id, cx)) + .into_any_element(), SyncConnectionState::SignedIn | SyncConnectionState::AwaitingDeviceApproval | SyncConnectionState::SyncReady { .. } @@ -120,11 +123,13 @@ fn render_account_card( .text_color(rgb(colors::ink_3())) .child("End-to-end encrypted"), ) + .when_some(shell.auth_flow_phase.error_message(), |card, message| { + card.child(render_inline_error(message)) + }) .child(render_devices(shell, cx)) .into_any_element(), } } - fn render_devices(shell: &mut ElyShell, cx: &mut Context) -> AnyElement { let loading = shell.sync_devices.is_loading(); let header = diff --git a/crates/ely_app/src/shell/mod.rs b/crates/ely_app/src/shell/mod.rs index 2d996b5..49f908e 100644 --- a/crates/ely_app/src/shell/mod.rs +++ b/crates/ely_app/src/shell/mod.rs @@ -115,6 +115,7 @@ pub struct ElyShell { sync_upload_pending: bool, sync_upload_pending_merge: Option, sync_retry_at: Option, + default_profile_id: Option, pub(crate) sync_devices: SyncDeviceUiState, pub(crate) sync_verification_input: Entity, pub(crate) auth_email_input: Entity, @@ -126,21 +127,23 @@ pub struct ElyShell { impl ElyShell { pub fn new(window: &mut Window, cx: &mut Context) -> Self { + let mut default_profile_id = None; let config = InitialBrowserConfig::ely_defaults() .map_err(|error| error.to_string()) .and_then(|mut config| { - config.profile_id = Some( - crate::services::profile_identity::default_standard_profile_id() - .map_err(|error| error.to_string())?, - ); + let profile_id = crate::services::profile_identity::default_standard_profile_id() + .map_err(|error| error.to_string())?; + config.profile_id = Some(profile_id.clone()); + default_profile_id = Some(profile_id); Ok(config) }); - Self::new_with_config(config, window, cx) + Self::new_with_config(config, default_profile_id, window, cx) } pub fn new_private(window: &mut Window, cx: &mut Context) -> Self { Self::new_with_config( InitialBrowserConfig::private_window().map_err(|error| error.to_string()), + None, window, cx, ) @@ -148,6 +151,7 @@ impl ElyShell { fn new_with_config( config: Result, + default_profile_id: Option, window: &mut Window, cx: &mut Context, ) -> Self { @@ -261,6 +265,7 @@ impl ElyShell { sync_upload_pending: false, sync_upload_pending_merge: None, sync_retry_at: None, + default_profile_id, sync_devices: SyncDeviceUiState::default(), sync_verification_input, auth_email_input, diff --git a/crates/ely_app/src/shell/settings_actions.rs b/crates/ely_app/src/shell/settings_actions.rs index 40dd9a1..4493fd5 100644 --- a/crates/ely_app/src/shell/settings_actions.rs +++ b/crates/ely_app/src/shell/settings_actions.rs @@ -9,7 +9,9 @@ use gpui_component::slider::SliderValue; use crate::services::servo_profile_data::{default_profile_data_root, sync_profile_data_dir}; -use super::sync_state::{PendingMergeUpload, SyncStateUpdate, sync_platform_label}; +use super::sync_state::{ + PendingMergeUpload, SyncStateUpdate, sync_failure_update, sync_platform_label, +}; use super::{ElyShell, ShellState}; impl ElyShell { @@ -255,11 +257,11 @@ impl ElyShell { } fn trigger_cloud_sync_upload_with_merge(&mut self, mut merge: Option) { - let active_profile_allows_sync = match &self.state { - ShellState::Ready(core) => core.active_profile_allows_sync(), + let cloud_sync_enabled = match &self.state { + ShellState::Ready(core) => core.cloud_sync_upload_enabled(), ShellState::StartupError(_) => false, }; - if !active_profile_allows_sync { + if !cloud_sync_enabled { self.sync_upload_scheduled = false; self.clear_pending_cloud_sync_upload(); return; @@ -352,6 +354,7 @@ fn run_sync_upload( inbox: std::sync::mpsc::Sender, ) { let mut engine = match SyncEngine::for_profile_dir( + &profile_id, &profile_dir, device_name, sync_platform_label(), @@ -360,7 +363,7 @@ fn run_sync_upload( Err(error) => { let message = error.to_string(); tracing::warn!(target: "ely::sync", error = %message, "could not initialise sync engine"); - let _ = inbox.send(SyncStateUpdate::SyncError { profile_id, message }); + let _ = inbox.send(sync_failure_update(profile_id, error)); return; } }; @@ -380,7 +383,10 @@ fn run_sync_upload( device_id = %device_id, "sync device is awaiting approval", ); - let _ = inbox.send(SyncStateUpdate::AwaitingDeviceApproval { profile_id }); + let _ = inbox.send(SyncStateUpdate::AwaitingDeviceApproval { + profile_id, + finishes_upload: true, + }); } Ok(ely_browser_core::SyncOutcome::RemoteSnapshot { snapshot_id, @@ -454,12 +460,7 @@ fn run_sync_upload( Err(error) => { let message = error.to_string(); tracing::warn!(target: "ely::sync", error = %message, "snapshot upload failed"); - let update = if message.contains("device_not_approved") { - SyncStateUpdate::AwaitingDeviceApproval { profile_id } - } else { - SyncStateUpdate::SyncError { profile_id, message } - }; - let _ = inbox.send(update); + let _ = inbox.send(sync_failure_update(profile_id, error)); } } } diff --git a/crates/ely_app/src/shell/sync_devices.rs b/crates/ely_app/src/shell/sync_devices.rs index 3bf7fc1..7ba9b07 100644 --- a/crates/ely_app/src/shell/sync_devices.rs +++ b/crates/ely_app/src/shell/sync_devices.rs @@ -7,7 +7,7 @@ use gpui::Context; use crate::services::servo_profile_data::{default_profile_data_root, sync_profile_data_dir}; -use super::sync_state::sync_platform_label; +use super::sync_state::{device_failure_update, sync_platform_label}; use super::{ElyShell, ShellState, sync_state::SyncStateUpdate}; #[derive(Clone, Debug, Default)] @@ -141,8 +141,12 @@ impl ElyShell { } let tx = self.sync_inbox_tx.clone(); spawn_device_task("ely-sync-device-approve", profile_id.clone(), tx, move || { - let engine = - SyncEngine::for_profile_dir(&profile_dir, device_name, sync_platform_label())?; + let engine = SyncEngine::for_profile_dir( + &profile_id, + &profile_dir, + device_name, + sync_platform_label(), + )?; engine.approve_cloud_device(&device_id, &verification_code)?; load_devices(profile_id, engine) }); @@ -163,8 +167,12 @@ impl ElyShell { } let tx = self.sync_inbox_tx.clone(); spawn_device_task("ely-sync-device-revoke", profile_id.clone(), tx, move || { - let engine = - SyncEngine::for_profile_dir(&profile_dir, device_name, sync_platform_label())?; + let engine = SyncEngine::for_profile_dir( + &profile_id, + &profile_dir, + device_name, + sync_platform_label(), + )?; engine.revoke_cloud_device(&device_id)?; load_devices(profile_id, engine) }); @@ -181,8 +189,12 @@ impl ElyShell { } let tx = self.sync_inbox_tx.clone(); spawn_device_task("ely-sync-device-list", profile_id.clone(), tx, move || { - let engine = - SyncEngine::for_profile_dir(&profile_dir, device_name, sync_platform_label())?; + let engine = SyncEngine::for_profile_dir( + &profile_id, + &profile_dir, + device_name, + sync_platform_label(), + )?; load_devices(profile_id, engine) }); } @@ -191,7 +203,7 @@ impl ElyShell { let ShellState::Ready(core) = &self.state else { return None; }; - if !core.active_profile_allows_sync() { + if !core.cloud_sync_upload_enabled() { return None; } let snapshot = core.snapshot().ok()?; @@ -225,10 +237,7 @@ fn spawn_device_task( let worker_tx = tx.clone(); let worker_profile_id = profile_id.clone(); let spawn_result = std::thread::Builder::new().name(thread_name).spawn(move || { - let update = task().unwrap_or_else(|error| SyncStateUpdate::DevicesError { - profile_id: worker_profile_id, - message: error.to_string(), - }); + let update = task().unwrap_or_else(|error| device_failure_update(worker_profile_id, error)); let _ = worker_tx.send(update); }); if let Err(error) = spawn_result { @@ -257,4 +266,67 @@ mod tests { assert!(state.current_code().is_none()); assert!(state.error().is_none()); } + + #[test] + fn credential_failures_do_not_finish_an_upload() { + let profile_id = ProfileId::new(); + let update = device_failure_update( + profile_id.clone(), + ely_sync_client::SyncClientError::BearerCredentialStorage("locked".to_string()), + ); + + assert!(matches!( + update, + SyncStateUpdate::CredentialUnavailable { + profile_id: owner, + finishes_upload: false, + .. + } if owner == profile_id + )); + } + + #[test] + fn approval_failures_do_not_finish_an_upload() { + let profile_id = ProfileId::new(); + let errors = [ + ely_sync_client::SyncClientError::DeviceApprovalStatus { + device_id: "device-id".to_string(), + status: "pending".to_string(), + }, + ely_sync_client::SyncClientError::HttpStatus { + endpoint: "/api/sync/devices".to_string(), + status: 403, + body: r#"{"error":"device_not_approved"}"#.to_string(), + }, + ]; + + for error in errors { + let update = device_failure_update(profile_id.clone(), error); + assert!(matches!( + update, + SyncStateUpdate::AwaitingDeviceApproval { + profile_id: owner, + finishes_upload: false, + } if owner == profile_id + )); + } + } + + #[test] + fn operational_failures_stay_in_device_ui_state() { + let profile_id = ProfileId::new(); + let update = device_failure_update( + profile_id.clone(), + ely_sync_client::SyncClientError::HttpStatus { + endpoint: "/api/sync/devices".to_string(), + status: 503, + body: "unavailable".to_string(), + }, + ); + + assert!(matches!( + update, + SyncStateUpdate::DevicesError { profile_id: owner, .. } if owner == profile_id + )); + } } diff --git a/crates/ely_app/src/shell/sync_state.rs b/crates/ely_app/src/shell/sync_state.rs index 48c38b1..7ba369c 100644 --- a/crates/ely_app/src/shell/sync_state.rs +++ b/crates/ely_app/src/shell/sync_state.rs @@ -1,11 +1,13 @@ use std::{path::Path, time::Duration}; use ely_domain::{ProfileId, ProfileKind, SyncConnectionState}; -use ely_sync_client::{AuthenticatedSnapshotHead, DeviceRecord}; +use ely_sync_client::{AuthenticatedSnapshotHead, BearerTokenStore, DeviceRecord}; use gpui::{Context, Timer}; use super::{ElyShell, ShellState, auth}; +mod legacy_sync_migration; + const CLOUD_SYNC_UPLOAD_DEBOUNCE: Duration = Duration::from_millis(750); const CAS_RETRY_LIMIT: u8 = 3; const CAS_RETRY_DELAY: Duration = Duration::from_secs(2); @@ -25,11 +27,12 @@ pub(crate) struct PendingMergeUpload { #[derive(Clone, Debug)] pub(crate) enum SyncStateUpdate { SignedOut { profile_id: ProfileId }, - AwaitingDeviceApproval { profile_id: ProfileId }, + AwaitingDeviceApproval { profile_id: ProfileId, finishes_upload: bool }, RemoteSnapshot { profile_id: ProfileId, bytes: Vec, merge: PendingMergeUpload }, SyncReady { profile_id: ProfileId, last_synced_at_secs: u64 }, SyncBusy { profile_id: ProfileId }, SyncError { profile_id: ProfileId, message: String }, + CredentialUnavailable { profile_id: ProfileId, message: String, finishes_upload: bool }, DevicesLoaded { profile_id: ProfileId, devices: Vec, current_code: String }, DevicesError { profile_id: ProfileId, message: String }, AuthOtpSent { profile_id: ProfileId, email: String }, @@ -37,6 +40,45 @@ pub(crate) enum SyncStateUpdate { AuthError { profile_id: ProfileId, email: String, message: String }, } +pub(super) fn sync_failure_update( + profile_id: ProfileId, + error: ely_sync_client::SyncClientError, +) -> SyncStateUpdate { + let message = error.to_string(); + match error { + ely_sync_client::SyncClientError::BearerCredentialStorage(_) + | ely_sync_client::SyncClientError::AccountKeyStorage(_) + | ely_sync_client::SyncClientError::DeviceKeyStorage(_) => { + SyncStateUpdate::CredentialUnavailable { profile_id, message, finishes_upload: true } + } + ely_sync_client::SyncClientError::DeviceApprovalStatus { .. } => { + SyncStateUpdate::AwaitingDeviceApproval { profile_id, finishes_upload: true } + } + _ if message.contains("device_not_approved") => { + SyncStateUpdate::AwaitingDeviceApproval { profile_id, finishes_upload: true } + } + _ => SyncStateUpdate::SyncError { profile_id, message }, + } +} + +pub(super) fn device_failure_update( + profile_id: ProfileId, + error: ely_sync_client::SyncClientError, +) -> SyncStateUpdate { + match sync_failure_update(profile_id, error) { + SyncStateUpdate::CredentialUnavailable { profile_id, message, .. } => { + SyncStateUpdate::CredentialUnavailable { profile_id, message, finishes_upload: false } + } + SyncStateUpdate::AwaitingDeviceApproval { profile_id, .. } => { + SyncStateUpdate::AwaitingDeviceApproval { profile_id, finishes_upload: false } + } + SyncStateUpdate::SyncError { profile_id, message } => { + SyncStateUpdate::DevicesError { profile_id, message } + } + update => update, + } +} + /// Stable label for the current OS used by the device registration /// payload. Defined once here so every off-thread call site agrees. pub(crate) const fn sync_platform_label() -> &'static str { @@ -128,7 +170,7 @@ impl ElyShell { let ShellState::Ready(core) = &mut self.state else { return false; }; - probe_initial_sync_state_at(core, &profile_root) + probe_initial_sync_state_at(core, &profile_root, self.default_profile_id.as_ref()) } /// Drain any sync upload outcomes the off-thread worker pushed @@ -155,8 +197,8 @@ impl ElyShell { latest_connection = Some(SyncConnectionState::SignedOut); } } - SyncStateUpdate::AwaitingDeviceApproval { profile_id } => { - upload_finished = true; + SyncStateUpdate::AwaitingDeviceApproval { profile_id, finishes_upload } => { + upload_finished |= finishes_upload; if active_profile_id(&self.state).as_ref() == Some(&profile_id) { latest_connection = Some(SyncConnectionState::AwaitingDeviceApproval); } @@ -216,6 +258,20 @@ impl ElyShell { latest_connection = Some(SyncConnectionState::SyncError { message }); } } + SyncStateUpdate::CredentialUnavailable { profile_id, message, finishes_upload } => { + upload_finished |= finishes_upload; + if active_profile_id(&self.state).as_ref() == Some(&profile_id) { + latest_connection = + Some(SyncConnectionState::CredentialUnavailable { message }); + self.sync_devices.reset(); + self.sync_upload_scheduled = false; + self.sync_retry_at = None; + self.clear_pending_cloud_sync_upload(); + trigger_initial_sync = false; + trigger_merged_upload = None; + devices_changed = true; + } + } SyncStateUpdate::DevicesLoaded { profile_id, devices, current_code } => { if active_profile_id(&self.state).as_ref() == Some(&profile_id) { self.sync_devices.set_ready(profile_id, devices, current_code); @@ -296,6 +352,7 @@ fn active_profile_id(state: &ShellState) -> Option { fn probe_initial_sync_state_at( core: &mut ely_browser_core::BrowserCore, profile_root: &Path, + default_profile_id: Option<&ProfileId>, ) -> bool { let Some(snapshot) = core.snapshot().ok() else { return false; @@ -305,65 +362,46 @@ fn probe_initial_sync_state_at( &snapshot.active_profile_id, ); if !core.active_profile_allows_sync() { - if let Err(error) = auth::clear_persisted_bearer(&profile_dir) { + let store = BearerTokenStore::new(&snapshot.active_profile_id, &profile_dir); + if let Err(error) = store.clear_legacy_files() { tracing::warn!(target: "ely::sync", error = %error, "private bearer cleanup failed"); } core.set_sync_connection_state(SyncConnectionState::SignedOut); return false; } - if snapshot.active_profile_name == "Default" - && matches!(snapshot.active_profile_kind, ProfileKind::Standard) + let is_default_profile = matches!(snapshot.active_profile_kind, ProfileKind::Standard) + && default_profile_id == Some(&snapshot.active_profile_id); + if is_default_profile + && let Err(error) = + legacy_sync_migration::migrate_default_sync_device(profile_root, &profile_dir) { - migrate_legacy_default_sync_dir(profile_root, &profile_dir); - } - let bearer_path = profile_dir.join("sync").join("bearer.token"); - let bearer_present = bearer_token_file_present(&bearer_path); - let state = if bearer_present { - ely_domain::SyncConnectionState::SignedIn - } else { - ely_domain::SyncConnectionState::SignedOut - }; - core.set_sync_connection_state(state); - bearer_present -} - -fn bearer_token_file_present(path: &Path) -> bool { - std::fs::metadata(path).map(|metadata| metadata.len() > 0).unwrap_or(false) -} - -fn migrate_legacy_default_sync_dir(profile_root: &Path, stable_profile_dir: &Path) { - let stable_sync_dir = stable_profile_dir.join("sync"); - if stable_sync_dir.exists() { - return; - } - - let candidate = profile_root.join("default").join("servo").join("sync"); - if !bearer_token_file_present(&candidate.join("bearer.token")) { - return; - } - if let Err(error) = copy_dir_recursive(&candidate, &stable_sync_dir) { tracing::warn!( target: "ely::sync", error = %error, - source = %candidate.display(), "legacy sync profile migration failed", ); } -} - -fn copy_dir_recursive(source: &Path, destination: &Path) -> std::io::Result<()> { - std::fs::create_dir_all(destination)?; - for entry in std::fs::read_dir(source)? { - let entry = entry?; - let file_type = entry.file_type()?; - let destination_path = destination.join(entry.file_name()); - if file_type.is_dir() { - copy_dir_recursive(&entry.path(), &destination_path)?; - } else if file_type.is_file() { - std::fs::copy(entry.path(), destination_path)?; + let store = auth::bearer_store_for_profile( + &snapshot.active_profile_id, + &profile_dir, + profile_root, + default_profile_id, + ); + let (bearer_present, state) = match store.load() { + Ok(Some(_)) => (true, SyncConnectionState::SignedIn), + Ok(None) => (false, SyncConnectionState::SignedOut), + Err(error) => { + tracing::warn!(target: "ely::sync", error = %error, "bearer credential probe failed"); + ( + false, + SyncConnectionState::CredentialUnavailable { + message: "System credential access failed.".to_string(), + }, + ) } - } - Ok(()) + }; + core.set_sync_connection_state(state); + bearer_present } #[cfg(test)] @@ -371,59 +409,9 @@ mod tests { use ely_browser_core::{BrowserCore, InitialBrowserConfig}; use ely_domain::SyncConnectionState; - use super::{ - bearer_token_file_present, migrate_legacy_default_sync_dir, probe_initial_sync_state_at, - }; + use super::{SyncStateUpdate, probe_initial_sync_state_at, sync_failure_update}; use crate::services::servo_profile_data::sync_profile_data_dir; - #[test] - fn bearer_token_file_presence_requires_bytes() -> Result<(), Box> { - let suffix = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH)?.as_nanos(); - let dir = std::env::temp_dir().join(format!("ely-sync-token-probe-{}", suffix)); - std::fs::create_dir_all(&dir)?; - let path = dir.join("bearer.token"); - - assert!(!bearer_token_file_present(&path)); - - std::fs::write(&path, "")?; - assert!(!bearer_token_file_present(&path)); - - std::fs::write(&path, "session-token")?; - assert!(bearer_token_file_present(&path)); - - std::fs::remove_dir_all(dir)?; - Ok(()) - } - - #[test] - fn known_default_sync_directory_is_migrated() -> Result<(), Box> { - let directory = tempfile::tempdir()?; - let legacy = directory.path().join("default/servo/sync"); - let stable = directory.path().join("profile_stable/servo"); - std::fs::create_dir_all(&legacy)?; - std::fs::write(legacy.join("bearer.token"), "default-token")?; - - migrate_legacy_default_sync_dir(directory.path(), &stable); - - assert_eq!(std::fs::read_to_string(stable.join("sync/bearer.token"))?, "default-token"); - Ok(()) - } - - #[test] - fn custom_profile_bearer_is_ignored_during_default_migration() - -> Result<(), Box> { - let directory = tempfile::tempdir()?; - let custom = directory.path().join("profile_custom/servo/sync"); - let stable = directory.path().join("profile_stable/servo"); - std::fs::create_dir_all(&custom)?; - std::fs::write(custom.join("bearer.token"), "custom-token")?; - - migrate_legacy_default_sync_dir(directory.path(), &stable); - - assert!(!stable.join("sync/bearer.token").exists()); - Ok(()) - } - #[test] fn private_startup_clears_a_persisted_bearer() -> Result<(), Box> { let directory = tempfile::tempdir()?; @@ -436,7 +424,7 @@ mod tests { std::fs::write(bearer_path.with_extension("tmp"), "b".repeat(64))?; core.set_sync_connection_state(SyncConnectionState::SignedIn); - assert!(!probe_initial_sync_state_at(&mut core, directory.path())); + assert!(!probe_initial_sync_state_at(&mut core, directory.path(), None)); assert!(!bearer_path.exists()); assert!(!bearer_path.with_extension("tmp").exists()); assert_eq!(core.snapshot()?.sync_status.connection(), &SyncConnectionState::SignedOut); @@ -454,10 +442,56 @@ mod tests { std::fs::create_dir_all(bearer_path.parent().ok_or("missing bearer parent")?)?; std::fs::write(&bearer_path, "a".repeat(64))?; - assert!(probe_initial_sync_state_at(&mut core, directory.path())); - assert!(bearer_path.exists()); + assert!(probe_initial_sync_state_at(&mut core, directory.path(), Some(&profile_id),)); + assert!(!bearer_path.exists()); assert_eq!(core.snapshot()?.sync_status.connection(), &SyncConnectionState::SignedIn); + let store = ely_sync_client::BearerTokenStore::new(&profile_id, &profile_dir); + assert!(store.load()?.is_some()); + store.clear()?; + Ok(()) } + + #[cfg(unix)] + #[test] + fn credential_probe_failure_enters_unavailable_state() -> Result<(), Box> + { + use std::os::unix::fs::symlink; + + let directory = tempfile::tempdir()?; + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let profile_dir = sync_profile_data_dir(directory.path(), &profile_id); + let lock = profile_dir.join("sync/bearer.lock"); + let target = directory.path().join("untrusted.lock"); + std::fs::create_dir_all(lock.parent().ok_or("missing lock parent")?)?; + std::fs::write(&target, "target")?; + symlink(&target, &lock)?; + + assert!(!probe_initial_sync_state_at(&mut core, directory.path(), Some(&profile_id),)); + assert!(matches!( + core.snapshot()?.sync_status.connection(), + SyncConnectionState::CredentialUnavailable { .. } + )); + Ok(()) + } + + #[test] + fn credential_storage_failures_use_the_unavailable_update() { + let profile_id = ely_domain::ProfileId::new(); + let update = sync_failure_update( + profile_id.clone(), + ely_sync_client::SyncClientError::BearerCredentialStorage("locked".to_string()), + ); + + assert!(matches!( + update, + SyncStateUpdate::CredentialUnavailable { + profile_id: owner, + finishes_upload: true, + .. + } if owner == profile_id + )); + } } diff --git a/crates/ely_app/src/shell/sync_state/legacy_sync_migration.rs b/crates/ely_app/src/shell/sync_state/legacy_sync_migration.rs new file mode 100644 index 0000000..baef593 --- /dev/null +++ b/crates/ely_app/src/shell/sync_state/legacy_sync_migration.rs @@ -0,0 +1,351 @@ +use std::{ + io::{self, ErrorKind, Read, Write}, + path::{Component, Path}, +}; + +use cap_fs_ext::{ + DirExt, FollowSymlinks, MetadataExt as CrossPlatformMetadataExt, OpenOptionsFollowExt, +}; +use cap_std::{ + ambient_authority, + fs::{Dir, File, Metadata, OpenOptions, Permissions}, +}; +use ely_sync_client::DeviceIdentity; +use uuid::Uuid; + +const SOURCE_COMPONENTS: [&str; 3] = ["default", "servo", "sync"]; +const DEVICE_FILE: &str = "device.json"; +const COMPLETION_MARKER: &str = ".default-sync-device-migrated-v1"; +const LOCK_FILE: &str = ".default-sync-device-migration.lock"; +const TEMP_PREFIX: &str = ".ely-sync-migration-"; +const COMPLETION_BYTES: &[u8] = b"ely-default-sync-device-migration-v1\n"; +const MAX_DEVICE_BYTES: usize = 16 * 1024; + +#[cfg(unix)] +type ExpectedOwner = u32; +#[cfg(not(unix))] +type ExpectedOwner = (); + +pub(super) fn migrate_default_sync_device( + profile_root: &Path, + stable_profile_dir: &Path, +) -> io::Result<()> { + let expected_owner = effective_owner()?; + let root = Dir::open_ambient_dir(profile_root, ambient_authority())?; + validate_directory(&root, expected_owner)?; + let relative_destination = stable_profile_dir + .strip_prefix(profile_root) + .map_err(|_| invalid_source("stable profile directory escapes the profile root"))?; + let stable = open_or_create_path(&root, relative_destination, expected_owner)?; + let destination = open_or_create_child(&stable, "sync", expected_owner)?; + let _lock = acquire_lock(&destination, expected_owner)?; + cleanup_temporary_files(&destination)?; + + if completion_marker_exists(&destination, expected_owner)? { + validate_optional_device(&destination, expected_owner)?; + return Ok(()); + } + if validate_optional_device(&destination, expected_owner)? { + return write_completion_marker(&destination, expected_owner); + } + + let Some(source) = open_source_directory(&root, expected_owner)? else { + return Ok(()); + }; + let Some(bytes) = read_secure_file(&source, DEVICE_FILE, expected_owner, MAX_DEVICE_BYTES)? + else { + return Ok(()); + }; + validate_device_bytes(&bytes)?; + persist_bytes_if_absent( + &destination, + DEVICE_FILE, + &bytes, + expected_owner, + validate_device_bytes, + )?; + write_completion_marker(&destination, expected_owner) +} + +fn open_source_directory(root: &Dir, owner: ExpectedOwner) -> io::Result> { + let mut directory = root.try_clone()?; + for component in SOURCE_COMPONENTS { + directory = match directory.open_dir_nofollow(component) { + Ok(child) => child, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(error), + }; + validate_directory(&directory, owner)?; + } + Ok(Some(directory)) +} + +fn open_or_create_path(root: &Dir, path: &Path, owner: ExpectedOwner) -> io::Result { + let mut directory = root.try_clone()?; + for component in path.components() { + let Component::Normal(name) = component else { + return Err(invalid_source("stable profile directory contains an invalid component")); + }; + directory = open_or_create_child(&directory, name, owner)?; + } + Ok(directory) +} + +fn open_or_create_child( + parent: &Dir, + name: impl AsRef, + owner: ExpectedOwner, +) -> io::Result { + let name = name.as_ref(); + let directory = match parent.open_dir_nofollow(name) { + Ok(directory) => directory, + Err(error) if error.kind() == ErrorKind::NotFound => { + match parent.create_dir(name) { + Ok(()) => {} + Err(error) if error.kind() == ErrorKind::AlreadyExists => {} + Err(error) => return Err(error), + } + let directory = parent.open_dir_nofollow(name)?; + set_private_directory_permissions(&directory)?; + directory + } + Err(error) => return Err(error), + }; + validate_directory(&directory, owner)?; + Ok(directory) +} + +fn acquire_lock(directory: &Dir, owner: ExpectedOwner) -> io::Result { + let mut options = private_open_options(); + options.read(true).write(true).create(true); + let file = directory.open_with(LOCK_FILE, &options)?; + validate_file(&file, owner)?; + set_private_file_permissions(&file)?; + let file = file.into_std(); + fs2::FileExt::lock_exclusive(&file)?; + Ok(file) +} + +fn validate_optional_device(directory: &Dir, owner: ExpectedOwner) -> io::Result { + let Some(bytes) = read_secure_file(directory, DEVICE_FILE, owner, MAX_DEVICE_BYTES)? else { + return Ok(false); + }; + validate_device_bytes(&bytes)?; + Ok(true) +} + +fn completion_marker_exists(directory: &Dir, owner: ExpectedOwner) -> io::Result { + let Some(bytes) = + read_secure_file(directory, COMPLETION_MARKER, owner, COMPLETION_BYTES.len())? + else { + return Ok(false); + }; + validate_marker_bytes(&bytes)?; + Ok(true) +} + +fn write_completion_marker(directory: &Dir, owner: ExpectedOwner) -> io::Result<()> { + persist_bytes_if_absent( + directory, + COMPLETION_MARKER, + COMPLETION_BYTES, + owner, + validate_marker_bytes, + ) +} + +fn persist_bytes_if_absent( + directory: &Dir, + destination: &str, + bytes: &[u8], + owner: ExpectedOwner, + validate: fn(&[u8]) -> io::Result<()>, +) -> io::Result<()> { + if let Some(existing) = read_secure_file(directory, destination, owner, MAX_DEVICE_BYTES)? { + return validate(&existing); + } + + let temporary = format!("{TEMP_PREFIX}{}", Uuid::now_v7().simple()); + let mut options = private_open_options(); + options.write(true).create_new(true); + let mut file = directory.open_with(&temporary, &options)?; + validate_file(&file, owner)?; + set_private_file_permissions(&file)?; + if let Err(error) = file.write_all(bytes).and_then(|()| file.sync_all()) { + let _ = directory.remove_file(&temporary); + return Err(error); + } + + let linked = match directory.hard_link(&temporary, directory, destination) { + Ok(()) => true, + Err(error) if error.kind() == ErrorKind::AlreadyExists => false, + Err(error) => { + let _ = directory.remove_file(&temporary); + return Err(error); + } + }; + remove_file_if_present(directory, &temporary)?; + if linked { + sync_directory(directory)?; + } + let existing = read_secure_file(directory, destination, owner, MAX_DEVICE_BYTES)? + .ok_or_else(|| invalid_source("migration destination disappeared"))?; + validate(&existing) +} + +fn read_secure_file( + directory: &Dir, + name: &str, + owner: ExpectedOwner, + maximum_bytes: usize, +) -> io::Result>> { + let mut options = private_open_options(); + options.read(true); + let file = match directory.open_with(name, &options) { + Ok(file) => file, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(error), + }; + validate_file(&file, owner)?; + if file.metadata()?.len() > maximum_bytes as u64 { + return Err(invalid_source("migration file exceeds its size limit")); + } + let mut bytes = Vec::new(); + file.into_std().take((maximum_bytes + 1) as u64).read_to_end(&mut bytes)?; + if bytes.len() > maximum_bytes { + return Err(invalid_source("migration file exceeds its size limit")); + } + Ok(Some(bytes)) +} + +fn cleanup_temporary_files(directory: &Dir) -> io::Result<()> { + for entry in directory.entries()? { + let entry = entry?; + let name = entry.file_name(); + if !name.to_string_lossy().starts_with(TEMP_PREFIX) { + continue; + } + if entry.file_type()?.is_dir() { + return Err(invalid_source("migration temporary path is a directory")); + } + directory.remove_file_or_symlink(name)?; + } + Ok(()) +} + +fn remove_file_if_present(directory: &Dir, name: &str) -> io::Result<()> { + match directory.remove_file(name) { + Ok(()) => Ok(()), + Err(error) if error.kind() == ErrorKind::NotFound => Ok(()), + Err(error) => Err(error), + } +} + +fn private_open_options() -> OpenOptions { + let mut options = OpenOptions::new(); + options.follow(FollowSymlinks::No); + #[cfg(unix)] + { + use cap_std::fs::OpenOptionsExt; + options.mode(0o600); + } + options +} + +fn validate_directory(directory: &Dir, owner: ExpectedOwner) -> io::Result<()> { + let metadata = directory.dir_metadata()?; + if !metadata.is_dir() { + return Err(invalid_source("migration path is not a directory")); + } + validate_owner_and_mode(&metadata, owner) +} + +fn validate_file(file: &File, owner: ExpectedOwner) -> io::Result<()> { + let metadata = file.metadata()?; + if !metadata.is_file() || CrossPlatformMetadataExt::nlink(&metadata) != 1 { + return Err(invalid_source("migration file link state is invalid")); + } + validate_owner_and_mode(&metadata, owner) +} + +#[cfg(unix)] +fn validate_owner_and_mode(metadata: &Metadata, owner: ExpectedOwner) -> io::Result<()> { + use cap_std::fs::{MetadataExt, PermissionsExt}; + if metadata.uid() != owner || metadata.permissions().mode() & 0o022 != 0 { + return Err(io::Error::new( + ErrorKind::PermissionDenied, + "migration path ownership or permissions are invalid", + )); + } + Ok(()) +} + +#[cfg(not(unix))] +const fn validate_owner_and_mode(_metadata: &Metadata, _owner: ExpectedOwner) -> io::Result<()> { + Ok(()) +} + +#[cfg(unix)] +fn effective_owner() -> io::Result { + use std::os::unix::fs::MetadataExt; + let probe = tempfile::NamedTempFile::new()?; + Ok(probe.as_file().metadata()?.uid()) +} + +#[cfg(not(unix))] +const fn effective_owner() -> io::Result { + Ok(()) +} + +#[cfg(unix)] +fn set_private_directory_permissions(directory: &Dir) -> io::Result<()> { + use cap_std::fs::PermissionsExt; + directory.set_permissions(".", Permissions::from_mode(0o700)) +} + +#[cfg(not(unix))] +const fn set_private_directory_permissions(_directory: &Dir) -> io::Result<()> { + Ok(()) +} + +#[cfg(unix)] +fn set_private_file_permissions(file: &File) -> io::Result<()> { + use cap_std::fs::PermissionsExt; + file.set_permissions(Permissions::from_mode(0o600)) +} + +#[cfg(not(unix))] +const fn set_private_file_permissions(_file: &File) -> io::Result<()> { + Ok(()) +} + +#[cfg(unix)] +fn sync_directory(directory: &Dir) -> io::Result<()> { + directory.try_clone()?.into_std_file().sync_all() +} + +#[cfg(not(unix))] +const fn sync_directory(_directory: &Dir) -> io::Result<()> { + Ok(()) +} + +fn validate_device_bytes(bytes: &[u8]) -> io::Result<()> { + DeviceIdentity::validate_stored_bytes(bytes, Path::new(DEVICE_FILE)) + .map_err(|error| invalid_source(error.to_string())) +} + +fn validate_marker_bytes(bytes: &[u8]) -> io::Result<()> { + if bytes == COMPLETION_BYTES { + Ok(()) + } else { + Err(invalid_source("legacy sync migration marker is invalid")) + } +} + +fn invalid_source(message: impl Into) -> io::Error { + io::Error::new(ErrorKind::InvalidData, message.into()) +} + +#[cfg(test)] +#[path = "legacy_sync_migration_tests.rs"] +mod tests; diff --git a/crates/ely_app/src/shell/sync_state/legacy_sync_migration_tests.rs b/crates/ely_app/src/shell/sync_state/legacy_sync_migration_tests.rs new file mode 100644 index 0000000..6dd1130 --- /dev/null +++ b/crates/ely_app/src/shell/sync_state/legacy_sync_migration_tests.rs @@ -0,0 +1,159 @@ +use std::{fs, path::Path}; + +use super::*; + +const PUBLIC_KEY: &str = "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a"; + +#[test] +fn copies_only_valid_device_once() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let source = source_dir(directory.path()); + let stable = stable_dir(directory.path()); + fs::create_dir_all(source.join("nested"))?; + fs::write(source.join(DEVICE_FILE), device_bytes("Legacy"))?; + fs::write(source.join("bearer.token"), "secret")?; + fs::write(source.join("other.json"), "other")?; + migrate_default_sync_device(directory.path(), &stable)?; + let destination = stable.join("sync"); + assert_eq!(fs::read(destination.join(DEVICE_FILE))?, device_bytes("Legacy")); + assert!(!destination.join("bearer.token").exists()); + assert!(!destination.join("other.json").exists()); + assert!(!destination.join("nested").exists()); + fs::remove_file(destination.join(DEVICE_FILE))?; + fs::write(source.join(DEVICE_FILE), device_bytes("Changed"))?; + migrate_default_sync_device(directory.path(), &stable)?; + assert!(!destination.join(DEVICE_FILE).exists()); + Ok(()) +} + +#[test] +fn preserves_a_valid_existing_destination() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let source = source_dir(directory.path()); + let stable = stable_dir(directory.path()); + fs::create_dir_all(&source)?; + fs::create_dir_all(stable.join("sync"))?; + fs::write(source.join(DEVICE_FILE), device_bytes("Legacy"))?; + fs::write(stable.join("sync/device.json"), device_bytes("Current"))?; + migrate_default_sync_device(directory.path(), &stable)?; + assert_eq!(fs::read(stable.join("sync/device.json"))?, device_bytes("Current")); + assert!(stable.join("sync").join(COMPLETION_MARKER).exists()); + Ok(()) +} + +#[test] +fn missing_source_retries_later() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let stable = stable_dir(directory.path()); + migrate_default_sync_device(directory.path(), &stable)?; + assert!(!stable.join("sync").join(COMPLETION_MARKER).exists()); + let source = source_dir(directory.path()); + fs::create_dir_all(&source)?; + fs::write(source.join(DEVICE_FILE), device_bytes("Late"))?; + migrate_default_sync_device(directory.path(), &stable)?; + assert_eq!(fs::read(stable.join("sync/device.json"))?, device_bytes("Late")); + Ok(()) +} + +#[test] +fn rejects_invalid_and_oversized_sources() -> Result<(), Box> { + for bytes in [b"{}".to_vec(), vec![b'a'; MAX_DEVICE_BYTES + 1]] { + let directory = tempfile::tempdir()?; + let source = source_dir(directory.path()); + let stable = stable_dir(directory.path()); + fs::create_dir_all(&source)?; + fs::write(source.join(DEVICE_FILE), bytes)?; + assert!(migrate_default_sync_device(directory.path(), &stable).is_err()); + assert!(!stable.join("sync").join(COMPLETION_MARKER).exists()); + } + Ok(()) +} + +#[cfg(unix)] +#[test] +fn rejects_source_ancestor_and_destination_links() -> Result<(), Box> { + use std::os::unix::fs::symlink; + + for linked_component in 0..SOURCE_COMPONENTS.len() { + let directory = tempfile::tempdir()?; + let target = directory.path().join("source-target"); + let mut real = target.clone(); + for component in &SOURCE_COMPONENTS[linked_component + 1..] { + real.push(component); + } + fs::create_dir_all(&real)?; + fs::write(real.join(DEVICE_FILE), device_bytes("Linked"))?; + let mut link = directory.path().to_path_buf(); + for component in &SOURCE_COMPONENTS[..linked_component] { + link.push(component); + } + fs::create_dir_all(&link)?; + link.push(SOURCE_COMPONENTS[linked_component]); + symlink(&target, &link)?; + assert!( + migrate_default_sync_device(directory.path(), &stable_dir(directory.path())).is_err() + ); + } + let directory = tempfile::tempdir()?; + let stable = stable_dir(directory.path()); + fs::create_dir_all(&stable)?; + symlink(directory.path().join("elsewhere"), stable.join("sync"))?; + assert!(migrate_default_sync_device(directory.path(), &stable).is_err()); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn rejects_linked_destination_files() -> Result<(), Box> { + use std::os::unix::fs::symlink; + + for name in [DEVICE_FILE, COMPLETION_MARKER] { + let directory = tempfile::tempdir()?; + let stable = stable_dir(directory.path()); + let destination = stable.join("sync"); + let target = directory.path().join("target"); + fs::create_dir_all(&destination)?; + fs::write( + &target, + if name == DEVICE_FILE { device_bytes("Target") } else { COMPLETION_BYTES.to_vec() }, + )?; + symlink(&target, destination.join(name))?; + assert!(migrate_default_sync_device(directory.path(), &stable).is_err()); + } + Ok(()) +} + +#[test] +fn rejects_hardlinked_source_and_destination() -> Result<(), Box> { + for destination_link in [false, true] { + let directory = tempfile::tempdir()?; + let source = source_dir(directory.path()); + let stable = stable_dir(directory.path()); + let target = directory.path().join("target.json"); + fs::create_dir_all(&source)?; + fs::write(&target, device_bytes("Target"))?; + if destination_link { + fs::create_dir_all(stable.join("sync"))?; + fs::hard_link(&target, stable.join("sync/device.json"))?; + } else { + fs::hard_link(&target, source.join(DEVICE_FILE))?; + } + assert!(migrate_default_sync_device(directory.path(), &stable).is_err()); + } + Ok(()) +} + +fn source_dir(root: &Path) -> std::path::PathBuf { + root.join("default/servo/sync") +} + +fn stable_dir(root: &Path) -> std::path::PathBuf { + root.join("profile_stable/servo") +} + +fn device_bytes(name: &str) -> Vec { + format!( + r#"{{"device_id":"ely-legacy-device","public_key":"{PUBLIC_KEY}","device_name":"{name}","platform":"macos"}}"#, + ) + .into_bytes() +} diff --git a/crates/ely_browser_core/src/sync_engine.rs b/crates/ely_browser_core/src/sync_engine.rs index c4b10ca..5161d5e 100644 --- a/crates/ely_browser_core/src/sync_engine.rs +++ b/crates/ely_browser_core/src/sync_engine.rs @@ -3,6 +3,7 @@ use std::{ time::{SystemTime, UNIX_EPOCH}, }; +use ely_domain::ProfileId; use ely_sync_client::{ AccountKey, ApiClientConfig, AuthenticatedSnapshotHead, BearerToken, BearerTokenStore, DeviceIdentity, SNAPSHOT_ENCRYPTION_VERSION, SnapshotCryptoContext, SnapshotDownloadResult, @@ -34,6 +35,7 @@ impl SyncEngine { /// fresh device identity on first use, then keeps it stable across /// runs so the server's `user_devices` row stays bound. pub fn for_profile_dir( + profile_id: &ProfileId, profile_data_dir: &Path, device_name: impl Into, platform: impl Into, @@ -46,7 +48,7 @@ impl SyncEngine { .join(".sync-key-locks"); let identity = DeviceIdentity::load_or_create(&sync_dir.join("device.json"), device_name, platform)?; - let bearer_store = BearerTokenStore::new(sync_dir.join("bearer.token")); + let bearer_store = BearerTokenStore::new(profile_id, profile_data_dir); Ok(Self { api_config: ApiClientConfig::production(), bearer_store, @@ -60,10 +62,6 @@ impl SyncEngine { &self.identity } - pub fn bearer_path(&self) -> &Path { - self.bearer_store.path() - } - pub fn last_outcome(&self) -> Option<&SyncOutcome> { self.last_outcome.as_ref() } @@ -436,6 +434,7 @@ fn device_registration_idempotency_key(identity: &DeviceIdentity) -> String { #[derive(Debug)] pub struct SyncEngineBuilder { + pub profile_id: ProfileId, pub profile_data_dir: PathBuf, pub device_name: String, pub platform: String, @@ -443,7 +442,12 @@ pub struct SyncEngineBuilder { impl SyncEngineBuilder { pub fn build(self) -> Result { - SyncEngine::for_profile_dir(&self.profile_data_dir, self.device_name, self.platform) + SyncEngine::for_profile_dir( + &self.profile_id, + &self.profile_data_dir, + self.device_name, + self.platform, + ) } } diff --git a/crates/ely_browser_core/tests/sync.rs b/crates/ely_browser_core/tests/sync.rs index 867abb1..616033a 100644 --- a/crates/ely_browser_core/tests/sync.rs +++ b/crates/ely_browser_core/tests/sync.rs @@ -38,6 +38,17 @@ fn default_sync_status_reflects_local_browser_state() -> Result<(), Box Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + core.set_sync_connection_state(SyncConnectionState::CredentialUnavailable { + message: "credential unavailable".to_string(), + }); + + assert!(!core.cloud_sync_upload_enabled()); + Ok(()) +} + #[test] fn sync_object_policy_pauses_object_kind() -> Result<(), Box> { let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; diff --git a/crates/ely_domain/src/sync.rs b/crates/ely_domain/src/sync.rs index f632479..d2f73e7 100644 --- a/crates/ely_domain/src/sync.rs +++ b/crates/ely_domain/src/sync.rs @@ -13,6 +13,7 @@ #[derive(Clone, Debug, Eq, PartialEq)] pub enum SyncConnectionState { SignedOut, + CredentialUnavailable { message: String }, SignedIn, AwaitingDeviceApproval, SyncReady { last_synced_at_secs: u64 }, @@ -104,7 +105,8 @@ impl SyncStatus { #[must_use] pub fn new(connection: SyncConnectionState, objects: Vec) -> Self { let failed_objects = match &connection { - SyncConnectionState::SyncError { .. } => 1, + SyncConnectionState::CredentialUnavailable { .. } + | SyncConnectionState::SyncError { .. } => 1, _ => 0, }; Self { connection, pending_objects: 0, failed_objects, objects } diff --git a/crates/ely_sync_client/Cargo.toml b/crates/ely_sync_client/Cargo.toml index 5085829..ef3c128 100644 --- a/crates/ely_sync_client/Cargo.toml +++ b/crates/ely_sync_client/Cargo.toml @@ -7,6 +7,8 @@ rust-version.workspace = true [dependencies] base64.workspace = true +cap-fs-ext.workspace = true +cap-std.workspace = true chacha20poly1305.workspace = true ed25519-dalek.workspace = true ely_domain = { path = "../ely_domain" } @@ -15,7 +17,7 @@ getrandom.workspace = true hkdf.workspace = true hmac.workspace = true hpke.workspace = true -keyring.workspace = true +keyring-core.workspace = true serde = { workspace = true } serde_json = { workspace = true } sha2 = { workspace = true } @@ -24,5 +26,20 @@ ureq = { workspace = true, features = ["json"] } uuid = { workspace = true } zeroize.workspace = true +[dev-dependencies] +tempfile.workspace = true + +[target.'cfg(unix)'.dependencies] +rustix = { workspace = true, features = ["process"] } + +[target.'cfg(target_os = "macos")'.dependencies] +apple-native-keyring-store = { version = "1.0.0", features = ["keychain"] } + +[target.'cfg(target_os = "windows")'.dependencies] +windows-native-keyring-store = "1.1.0" + +[target.'cfg(all(unix, not(any(target_os = "macos", target_os = "ios", target_os = "android"))))'.dependencies] +zbus-secret-service-keyring-store = { version = "1.0.0", features = ["crypto-rust"] } + [lints] workspace = true diff --git a/crates/ely_sync_client/src/auth.rs b/crates/ely_sync_client/src/auth.rs index 90fb789..599b1ae 100644 --- a/crates/ely_sync_client/src/auth.rs +++ b/crates/ely_sync_client/src/auth.rs @@ -1,31 +1,56 @@ use std::{ - fs, - io::{self, ErrorKind}, + fmt, path::{Path, PathBuf}, }; -use crate::error::SyncClientError; +use ely_domain::ProfileId; +use fs2::FileExt; +use zeroize::Zeroizing; -/// Better Auth bearer token issued by `https:///api/auth/*`. The -/// token grants access to the per-user `withAuthenticatedApiControls` -/// routes and, once the device is bound to the session, to the -/// `withApprovedDeviceApiControls` routes used by sync. -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct BearerToken(String); +use crate::{ + auth_files::{ + ensure_migration_marker, migration_marker_exists, open_lock_file, read_legacy_bytes, + remove_file_if_present, + }, + credential_store::{clear_secret, load_secret, save_secret}, + error::SyncClientError, +}; + +const KEYCHAIN_SERVICE: &str = "com.elydora.ely-browser.auth.bearer.v1"; +const MIN_BEARER_TOKEN_BYTES: usize = 32; +const MAX_BEARER_TOKEN_BYTES: usize = 2560; + +/// Better Auth bearer token issued by `https:///api/auth/*`. +pub struct BearerToken(Zeroizing); + +impl Clone for BearerToken { + fn clone(&self) -> Self { + Self(Zeroizing::new(self.as_str().to_string())) + } +} + +impl fmt::Debug for BearerToken { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("BearerToken([REDACTED])") + } +} + +impl PartialEq for BearerToken { + fn eq(&self, other: &Self) -> bool { + self.as_str() == other.as_str() + } +} + +impl Eq for BearerToken {} impl BearerToken { - /// Construct a token from an existing string. Trims whitespace and - /// enforces the same character envelope the worker validates so - /// obviously-malformed tokens fail before we hit the network. pub fn new(value: impl Into) -> Result { - let value = value.into(); + let value = Zeroizing::new(value.into()); let trimmed = value.trim(); if trimmed.is_empty() || !is_better_auth_bearer(trimmed) { - return Err(SyncClientError::TokenStorage( - "bearer token is not a Better Auth session token".to_string(), - )); + return Err(storage_error("bearer token is not a Better Auth session token")); } - Ok(Self(trimmed.to_string())) + Ok(Self(Zeroizing::new(trimmed.to_string()))) } pub fn as_str(&self) -> &str { @@ -34,7 +59,7 @@ impl BearerToken { } fn is_better_auth_bearer(token: &str) -> bool { - let length_ok = (32..=4096).contains(&token.len()); + let length_ok = (MIN_BEARER_TOKEN_BYTES..=MAX_BEARER_TOKEN_BYTES).contains(&token.len()); let charset_ok = token .as_bytes() .iter() @@ -42,100 +67,238 @@ fn is_better_auth_bearer(token: &str) -> bool { length_ok && charset_ok } -/// File-backed bearer token store. Lives in the per-profile data -/// directory so a private window never inherits the standard -/// profile's session — same isolation the rest of the runtime -/// enforces. Writes go through a temp-rename so a partial write -/// can't corrupt the persisted token. +trait CredentialBackend { + fn load(&self, service: &str, account: &str) -> Result>>, String>; + fn save(&self, service: &str, account: &str, secret: &[u8]) -> Result<(), String>; + fn clear(&self, service: &str, account: &str) -> Result<(), String>; +} + +struct NativeCredentialBackend; + +impl CredentialBackend for NativeCredentialBackend { + fn load(&self, service: &str, account: &str) -> Result>>, String> { + load_secret(service, account) + } + + fn save(&self, service: &str, account: &str, secret: &[u8]) -> Result<(), String> { + save_secret(service, account, secret) + } + + fn clear(&self, service: &str, account: &str) -> Result<(), String> { + clear_secret(service, account) + } +} + +/// Profile-scoped native credential store with one-time plaintext-file migration. #[derive(Clone, Debug)] pub struct BearerTokenStore { - path: PathBuf, + account: String, + lock_path: PathBuf, + migration_marker_path: PathBuf, + legacy_paths: Vec, } impl BearerTokenStore { - pub fn new(path: PathBuf) -> Self { - Self { path } + pub fn new(profile_id: &ProfileId, profile_data_dir: &Path) -> Self { + let sync_dir = profile_data_dir.join("sync"); + Self { + account: profile_id.as_str().to_string(), + lock_path: sync_dir.join("bearer.lock"), + migration_marker_path: sync_dir.join("bearer.migrated"), + legacy_paths: vec![sync_dir.join("bearer.token")], + } } - pub fn path(&self) -> &Path { - &self.path + pub fn with_legacy_path(mut self, path: PathBuf) -> Self { + if !self.legacy_paths.contains(&path) { + self.legacy_paths.push(path); + } + self } pub fn load(&self) -> Result, SyncClientError> { - match fs::read_to_string(&self.path) { - Ok(contents) => { - if contents.trim().is_empty() { - return Ok(None); - } - BearerToken::new(contents).map(Some) - } - Err(error) if error.kind() == ErrorKind::NotFound => Ok(None), - Err(error) => Err(SyncClientError::TokenStorage(error.to_string())), - } + self.load_with(&NativeCredentialBackend) } pub fn save(&self, token: &BearerToken) -> Result<(), SyncClientError> { - if let Some(parent) = self.path.parent() { - fs::create_dir_all(parent).map_err(io_err)?; - } - let tmp = self.path.with_extension("tmp"); - fs::write(&tmp, token.as_str()).map_err(io_err)?; - fs::rename(&tmp, &self.path).map_err(io_err)?; - Ok(()) + self.save_with(&NativeCredentialBackend, token) } pub fn clear(&self) -> Result<(), SyncClientError> { - remove_file_if_present(&self.path)?; - remove_file_if_present(&self.path.with_extension("tmp")) + self.clear_with(&NativeCredentialBackend) + } + + pub fn clear_if_matches(&self, token: &BearerToken) -> Result { + self.clear_if_matches_with(&NativeCredentialBackend, token) + } + + fn clear_if_matches_with( + &self, + backend: &B, + token: &BearerToken, + ) -> Result { + self.with_lock(|| { + let current = load_backend_token(backend, &self.account)?; + ensure_migration_marker(&self.migration_marker_path)?; + self.cleanup_legacy_paths()?; + if current.as_ref().is_some_and(|current| current != token) { + return Ok(false); + } + backend.clear(KEYCHAIN_SERVICE, &self.account).map_err(storage_error)?; + Ok(true) + }) + } + + pub fn clear_legacy_files(&self) -> Result<(), SyncClientError> { + self.with_lock(|| self.cleanup_legacy_paths()) + } + + fn load_with( + &self, + backend: &B, + ) -> Result, SyncClientError> { + self.with_lock(|| self.load_locked(backend)) + } + + fn save_with( + &self, + backend: &B, + token: &BearerToken, + ) -> Result<(), SyncClientError> { + self.with_lock(|| { + ensure_migration_marker(&self.migration_marker_path)?; + self.cleanup_legacy_paths()?; + backend + .save(KEYCHAIN_SERVICE, &self.account, token.as_str().as_bytes()) + .map_err(storage_error) + }) + } + + fn clear_with(&self, backend: &B) -> Result<(), SyncClientError> { + self.with_lock(|| { + ensure_migration_marker(&self.migration_marker_path)?; + self.cleanup_legacy_paths()?; + backend.clear(KEYCHAIN_SERVICE, &self.account).map_err(storage_error) + }) + } + + fn load_locked( + &self, + backend: &B, + ) -> Result, SyncClientError> { + if let Some(token) = load_backend_token(backend, &self.account)? { + ensure_migration_marker(&self.migration_marker_path)?; + self.cleanup_legacy_paths()?; + return Ok(Some(token)); + } + + if migration_marker_exists(&self.migration_marker_path)? { + self.cleanup_legacy_paths()?; + return Ok(None); + } + + let mut candidate = None; + for path in &self.legacy_paths { + if let Some(token) = read_legacy_token(path)? { + candidate = Some(token); + break; + } + } + let Some(token) = candidate else { + self.cleanup_legacy_tmp_files()?; + return Ok(None); + }; + backend + .save(KEYCHAIN_SERVICE, &self.account, token.as_str().as_bytes()) + .map_err(storage_error)?; + if let Err(error) = verify_backend_token(backend, &self.account, &token) { + return Err(rollback_migration(backend, &self.account, error)); + } + if let Err(error) = ensure_migration_marker(&self.migration_marker_path) { + if matches!(migration_marker_exists(&self.migration_marker_path), Ok(false)) { + return Err(rollback_migration(backend, &self.account, error)); + } + return Err(error); + } + self.cleanup_legacy_paths()?; + Ok(Some(token)) + } + + fn cleanup_legacy_paths(&self) -> Result<(), SyncClientError> { + for path in &self.legacy_paths { + remove_file_if_present(path)?; + remove_file_if_present(&path.with_extension("tmp"))?; + } + Ok(()) + } + + fn cleanup_legacy_tmp_files(&self) -> Result<(), SyncClientError> { + for path in &self.legacy_paths { + remove_file_if_present(&path.with_extension("tmp"))?; + } + Ok(()) + } + + fn with_lock( + &self, + operation: impl FnOnce() -> Result, + ) -> Result { + let lock = open_lock_file(&self.lock_path)?; + lock.lock_exclusive().map_err(|error| storage_error(error.to_string()))?; + operation() } } -fn remove_file_if_present(path: &Path) -> Result<(), SyncClientError> { - match fs::remove_file(path) { - Ok(()) => Ok(()), - Err(error) if error.kind() == ErrorKind::NotFound => Ok(()), - Err(error) => Err(SyncClientError::TokenStorage(error.to_string())), +fn load_backend_token( + backend: &B, + account: &str, +) -> Result, SyncClientError> { + let Some(secret) = backend.load(KEYCHAIN_SERVICE, account).map_err(storage_error)? else { + return Ok(None); + }; + let value = + std::str::from_utf8(&secret).map_err(|error| storage_error(error.to_string()))?.to_string(); + BearerToken::new(value).map(Some) +} + +fn verify_backend_token( + backend: &B, + account: &str, + token: &BearerToken, +) -> Result<(), SyncClientError> { + let stored = load_backend_token(backend, account)?; + if stored.as_ref() != Some(token) { + return Err(storage_error("native credential read-back did not match")); + } + Ok(()) +} + +fn rollback_migration( + backend: &B, + account: &str, + cause: SyncClientError, +) -> SyncClientError { + match backend.clear(KEYCHAIN_SERVICE, account) { + Ok(()) => cause, + Err(rollback_error) => { + storage_error(format!("{cause}; native credential rollback failed: {rollback_error}")) + } } } -fn io_err(error: io::Error) -> SyncClientError { - SyncClientError::TokenStorage(error.to_string()) +fn read_legacy_token(path: &Path) -> Result, SyncClientError> { + let Some(bytes) = read_legacy_bytes(path, MAX_BEARER_TOKEN_BYTES + 1)? else { + return Ok(None); + }; + let value = + std::str::from_utf8(&bytes).map_err(|error| storage_error(error.to_string()))?.to_string(); + BearerToken::new(value).map(Some) +} + +fn storage_error(message: impl Into) -> SyncClientError { + SyncClientError::BearerCredentialStorage(message.into()) } #[cfg(test)] -mod tests { - use super::*; - use std::env::temp_dir; - - #[test] - fn rejects_obviously_broken_tokens() { - assert!(BearerToken::new("").is_err()); - assert!(BearerToken::new(" ").is_err()); - assert!(BearerToken::new("short").is_err()); - // Spaces are not in the Better Auth bearer charset. - assert!(BearerToken::new(format!("{}aaa bb", "a".repeat(40))).is_err()); - } - - #[test] - fn accepts_better_auth_shape() -> Result<(), SyncClientError> { - let token = format!("{}-{}_{}", "a".repeat(20), "b".repeat(20), "c".repeat(20)); - BearerToken::new(token).map(|_| ()) - } - - #[test] - fn token_store_round_trips() -> Result<(), SyncClientError> { - let dir = temp_dir().join(format!("ely-token-{}", uuid::Uuid::now_v7().simple())); - let store = BearerTokenStore::new(dir.join("token")); - let token = BearerToken::new("a".repeat(64))?; - assert_eq!(store.load()?, None); - - store.save(&token)?; - assert_eq!(store.load()?, Some(token.clone())); - fs::write(store.path().with_extension("tmp"), token.as_str()).map_err(io_err)?; - - store.clear()?; - assert_eq!(store.load()?, None); - assert!(!store.path().with_extension("tmp").exists()); - Ok(()) - } -} +#[path = "auth_tests.rs"] +mod tests; diff --git a/crates/ely_sync_client/src/auth_files.rs b/crates/ely_sync_client/src/auth_files.rs new file mode 100644 index 0000000..0793961 --- /dev/null +++ b/crates/ely_sync_client/src/auth_files.rs @@ -0,0 +1,296 @@ +use std::{ + fs, + io::{self, ErrorKind, Read, Write}, + path::{Component, Path, PathBuf}, +}; + +use cap_fs_ext::{ + DirExt, FollowSymlinks, MetadataExt as CrossPlatformMetadataExt, OpenOptionsFollowExt, +}; +use cap_std::{ + ambient_authority, + fs::{Dir, File, OpenOptions, Permissions}, +}; +use zeroize::Zeroizing; + +use crate::error::SyncClientError; + +const MARKER_CONTENT: &[u8] = b"ely-bearer-migration-v1\n"; + +pub(super) fn read_legacy_bytes( + path: &Path, + maximum_bytes: usize, +) -> Result>>, SyncClientError> { + let Some((directory, name)) = open_parent(path, false)? else { + return Ok(None); + }; + let Some(file) = open_existing(&directory, name)? else { + return Ok(None); + }; + validate_private_file(&file)?; + let mut bytes = Zeroizing::new(Vec::new()); + file.into_std().take((maximum_bytes + 1) as u64).read_to_end(&mut bytes).map_err(storage_io)?; + if bytes.len() > maximum_bytes { + return Err(storage_error("legacy bearer token is too large")); + } + Ok(Some(bytes)) +} + +pub(super) fn ensure_migration_marker(path: &Path) -> Result<(), SyncClientError> { + let (directory, name) = open_parent(path, true)? + .ok_or_else(|| storage_error("bearer migration marker parent is missing"))?; + if let Some(file) = open_existing(&directory, name)? { + return validate_migration_marker(file); + } + let temporary_path = marker_temporary_path(path)?; + remove_file_if_present(&temporary_path)?; + let temporary_name = temporary_path + .file_name() + .ok_or_else(|| storage_error("bearer migration marker temp name is missing"))?; + let mut options = private_open_options(); + options.write(true).create_new(true); + let mut file = directory.open_with(temporary_name, &options).map_err(storage_io)?; + validate_private_file(&file)?; + set_private_file_permissions(&file)?; + file.write_all(MARKER_CONTENT).map_err(storage_io)?; + file.sync_all().map_err(storage_io)?; + drop(file); + directory.rename(temporary_name, &directory, name).map_err(storage_io)?; + sync_directory(&directory) +} + +pub(super) fn migration_marker_exists(path: &Path) -> Result { + let Some((directory, name)) = open_parent(path, false)? else { + return Ok(false); + }; + let Some(file) = open_existing(&directory, name)? else { + return Ok(false); + }; + validate_migration_marker(file)?; + Ok(true) +} + +fn validate_migration_marker(file: File) -> Result<(), SyncClientError> { + validate_private_file(&file)?; + let mut bytes = Vec::new(); + file.into_std() + .take((MARKER_CONTENT.len() + 1) as u64) + .read_to_end(&mut bytes) + .map_err(storage_io)?; + if bytes != MARKER_CONTENT { + return Err(storage_error("bearer migration marker content is invalid")); + } + Ok(()) +} + +fn marker_temporary_path(path: &Path) -> Result { + let mut name = path + .file_name() + .ok_or_else(|| storage_error("bearer migration marker name is missing"))? + .to_os_string(); + name.push(".tmp"); + Ok(path.with_file_name(name)) +} + +pub(super) fn remove_file_if_present(path: &Path) -> Result<(), SyncClientError> { + let Some((directory, name)) = open_parent(path, false)? else { + return Ok(()); + }; + let metadata = match directory.symlink_metadata(name) { + Ok(metadata) => metadata, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(()), + Err(error) => return Err(storage_io(error)), + }; + if metadata.is_symlink() { + return directory.remove_file_or_symlink(name).map_err(storage_io); + } + let file = open_existing(&directory, name)?.ok_or_else(|| { + storage_error("legacy bearer token disappeared during cleanup validation") + })?; + validate_private_file(&file)?; + drop(file); + directory.remove_file_or_symlink(name).map_err(storage_io) +} + +pub(super) fn open_lock_file(path: &Path) -> Result { + let (directory, name) = + open_parent(path, true)?.ok_or_else(|| storage_error("bearer lock parent is missing"))?; + let mut options = private_open_options(); + options.read(true).write(true).create(true); + let file = directory.open_with(name, &options).map_err(storage_io)?; + validate_private_file(&file)?; + set_private_file_permissions(&file)?; + Ok(file.into_std()) +} + +fn open_parent( + path: &Path, + create: bool, +) -> Result, SyncClientError> { + let parent = path.parent().ok_or_else(|| storage_error("credential file parent is missing"))?; + let name = path.file_name().ok_or_else(|| storage_error("credential file name is missing"))?; + let Some(directory) = open_directory_nofollow(parent, create)? else { + return Ok(None); + }; + validate_private_directory(&directory)?; + set_private_directory_permissions(&directory)?; + Ok(Some((directory, name))) +} + +fn open_directory_nofollow(path: &Path, create: bool) -> Result, SyncClientError> { + let mut root = PathBuf::new(); + let mut names = Vec::new(); + for component in path.components() { + match component { + Component::Prefix(prefix) => root.push(prefix.as_os_str()), + Component::RootDir => root.push(std::path::MAIN_SEPARATOR_STR), + Component::Normal(name) => names.push(name.to_os_string()), + Component::CurDir => {} + Component::ParentDir => { + return Err(storage_error("credential path traversal is invalid")); + } + } + } + if root.as_os_str().is_empty() { + return Err(storage_error("credential paths must be absolute")); + } + let mut directory = Dir::open_ambient_dir(root, ambient_authority()).map_err(storage_io)?; + #[cfg(windows)] + let strict_component = names.len().saturating_sub(4); + #[cfg(windows)] + let mut require_nofollow = false; + #[cfg(windows)] + let mut component_index = 0; + #[cfg(not(windows))] + let mut require_nofollow = directory_requires_nofollow(&directory)?; + for name in names { + #[cfg(windows)] + { + require_nofollow |= component_index >= strict_component; + component_index += 1; + } + if create { + match directory.create_dir(&name) { + Ok(()) => {} + Err(error) if error.kind() == ErrorKind::AlreadyExists => {} + Err(error) => return Err(storage_io(error)), + } + } + let opened = match directory.open_dir_nofollow(&name) { + Ok(directory) => directory, + Err(_) if !require_nofollow => match directory.open_dir(&name) { + Ok(directory) => directory, + Err(error) if !create && error.kind() == ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(storage_io(error)), + }, + Err(error) if !create && error.kind() == ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(storage_io(error)), + }; + #[cfg(not(windows))] + { + require_nofollow |= directory_requires_nofollow(&opened)?; + } + directory = opened; + } + Ok(Some(directory)) +} + +#[cfg(unix)] +fn directory_requires_nofollow(directory: &Dir) -> Result { + use cap_std::fs::MetadataExt; + + let metadata = directory.dir_metadata().map_err(storage_io)?; + Ok(metadata.uid() == rustix::process::geteuid().as_raw() || metadata.mode() & 0o022 != 0) +} + +#[cfg(not(any(unix, windows)))] +fn directory_requires_nofollow(_directory: &Dir) -> Result { + Ok(true) +} + +fn open_existing(directory: &Dir, name: &std::ffi::OsStr) -> Result, SyncClientError> { + let mut options = private_open_options(); + options.read(true); + match directory.open_with(name, &options) { + Ok(file) => Ok(Some(file)), + Err(error) if error.kind() == ErrorKind::NotFound => Ok(None), + Err(error) => Err(storage_io(error)), + } +} + +fn private_open_options() -> OpenOptions { + let mut options = OpenOptions::new(); + options.follow(FollowSymlinks::No); + #[cfg(unix)] + { + use cap_std::fs::OpenOptionsExt; + options.mode(0o600); + } + options +} + +fn validate_private_directory(directory: &Dir) -> Result<(), SyncClientError> { + let metadata = directory.dir_metadata().map_err(storage_io)?; + if !metadata.is_dir() { + return Err(storage_error("credential file parent is not a directory")); + } + #[cfg(unix)] + { + use cap_std::fs::MetadataExt; + if metadata.uid() != rustix::process::geteuid().as_raw() { + return Err(storage_error("credential file parent ownership is invalid")); + } + } + Ok(()) +} + +fn validate_private_file(file: &File) -> Result<(), SyncClientError> { + let metadata = file.metadata().map_err(storage_io)?; + if !metadata.is_file() || CrossPlatformMetadataExt::nlink(&metadata) != 1 { + return Err(storage_error("credential file link state is invalid")); + } + #[cfg(unix)] + { + use cap_std::fs::MetadataExt; + if metadata.uid() != rustix::process::geteuid().as_raw() { + return Err(storage_error("credential file ownership is invalid")); + } + } + Ok(()) +} + +fn set_private_directory_permissions(directory: &Dir) -> Result<(), SyncClientError> { + #[cfg(unix)] + directory + .set_permissions(".", Permissions::from_std(fs::Permissions::from_mode(0o700))) + .map_err(storage_io)?; + Ok(()) +} + +fn set_private_file_permissions(file: &File) -> Result<(), SyncClientError> { + #[cfg(unix)] + file.set_permissions(Permissions::from_std(fs::Permissions::from_mode(0o600))) + .map_err(storage_io)?; + Ok(()) +} + +#[cfg(unix)] +fn sync_directory(directory: &Dir) -> Result<(), SyncClientError> { + directory.try_clone().map_err(storage_io)?.into_std_file().sync_all().map_err(storage_io) +} + +#[cfg(not(unix))] +fn sync_directory(_directory: &Dir) -> Result<(), SyncClientError> { + Ok(()) +} + +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; + +fn storage_io(error: io::Error) -> SyncClientError { + storage_error(error.to_string()) +} + +fn storage_error(message: impl Into) -> SyncClientError { + SyncClientError::BearerCredentialStorage(message.into()) +} diff --git a/crates/ely_sync_client/src/auth_tests.rs b/crates/ely_sync_client/src/auth_tests.rs new file mode 100644 index 0000000..4ef3d7f --- /dev/null +++ b/crates/ely_sync_client/src/auth_tests.rs @@ -0,0 +1,483 @@ +use std::{ + collections::BTreeMap, + fs, + sync::{Mutex, MutexGuard}, +}; + +use ely_domain::ProfileId; +use zeroize::Zeroizing; + +use super::*; + +#[derive(Default)] +struct MemoryCredentialBackend { + values: Mutex>>, +} + +impl CredentialBackend for MemoryCredentialBackend { + fn load(&self, service: &str, account: &str) -> Result>>, String> { + Ok(self + .values() + .get(&(service.to_string(), account.to_string())) + .cloned() + .map(Zeroizing::new)) + } + + fn save(&self, service: &str, account: &str, secret: &[u8]) -> Result<(), String> { + self.values().insert((service.to_string(), account.to_string()), secret.to_vec()); + Ok(()) + } + + fn clear(&self, service: &str, account: &str) -> Result<(), String> { + self.values().remove(&(service.to_string(), account.to_string())); + Ok(()) + } +} + +impl MemoryCredentialBackend { + fn values(&self) -> MutexGuard<'_, BTreeMap<(String, String), Vec>> { + self.values.lock().unwrap_or_else(std::sync::PoisonError::into_inner) + } +} + +struct DroppingCredentialBackend; + +impl CredentialBackend for DroppingCredentialBackend { + fn load(&self, _service: &str, _account: &str) -> Result>>, String> { + Ok(None) + } + + fn save(&self, _service: &str, _account: &str, _secret: &[u8]) -> Result<(), String> { + Ok(()) + } + + fn clear(&self, _service: &str, _account: &str) -> Result<(), String> { + Ok(()) + } +} + +struct SaveFailureBackend; + +impl CredentialBackend for SaveFailureBackend { + fn load(&self, _service: &str, _account: &str) -> Result>>, String> { + Ok(None) + } + + fn save(&self, _service: &str, _account: &str, _secret: &[u8]) -> Result<(), String> { + Err("credential backend is locked".to_string()) + } + + fn clear(&self, _service: &str, _account: &str) -> Result<(), String> { + Ok(()) + } +} + +#[derive(Default)] +struct ClearFailureBackend { + values: Mutex>>, +} + +impl CredentialBackend for ClearFailureBackend { + fn load(&self, service: &str, account: &str) -> Result>>, String> { + Ok(self + .values + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(&(service.to_string(), account.to_string())) + .cloned() + .map(Zeroizing::new)) + } + + fn save(&self, service: &str, account: &str, secret: &[u8]) -> Result<(), String> { + self.values + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert((service.to_string(), account.to_string()), secret.to_vec()); + Ok(()) + } + + fn clear(&self, _service: &str, _account: &str) -> Result<(), String> { + Err("credential backend is locked".to_string()) + } +} + +#[test] +fn validates_token_shape_boundaries() -> Result<(), SyncClientError> { + assert!(BearerToken::new("").is_err()); + assert!(BearerToken::new("short").is_err()); + assert!(BearerToken::new(format!("{}aaa bb", "a".repeat(40))).is_err()); + BearerToken::new("a".repeat(32))?; + BearerToken::new("a".repeat(MAX_BEARER_TOKEN_BYTES))?; + assert!(BearerToken::new("a".repeat(MAX_BEARER_TOKEN_BYTES + 1)).is_err()); + Ok(()) +} + +#[test] +fn debug_output_redacts_the_token() -> Result<(), SyncClientError> { + let token = BearerToken::new("secret-session-token".repeat(3))?; + let output = format!("{token:?}"); + + assert_eq!(output, "BearerToken([REDACTED])"); + assert!(!output.contains(token.as_str())); + Ok(()) +} + +#[test] +fn keychain_entry_wins_and_cleans_every_legacy_path() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + let old = directory.path().join("old/bearer.token"); + write_token(&stable, 'a')?; + write_token(&old, 'b')?; + write_token(&stable.with_extension("tmp"), 'c')?; + write_token(&old.with_extension("tmp"), 'd')?; + let backend = MemoryCredentialBackend::default(); + let expected = BearerToken::new("e".repeat(64))?; + backend.save(KEYCHAIN_SERVICE, profile_id.as_str(), expected.as_str().as_bytes())?; + let store = BearerTokenStore::new(&profile_id, directory.path()).with_legacy_path(old.clone()); + + assert_eq!(store.load_with(&backend)?, Some(expected)); + for path in [stable, old] { + assert!(!path.exists()); + assert!(!path.with_extension("tmp").exists()); + } + assert!(migration_marker_path(directory.path()).exists()); + Ok(()) +} + +#[test] +fn stable_legacy_token_precedes_old_default_source() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + let old = directory.path().join("old/bearer.token"); + write_token(&stable, 'a')?; + write_token(&old, 'b')?; + let backend = MemoryCredentialBackend::default(); + let store = BearerTokenStore::new(&profile_id, directory.path()).with_legacy_path(old.clone()); + + assert_eq!( + store.load_with(&backend)?.as_ref().map(BearerToken::as_str), + Some("a".repeat(64).as_str()) + ); + assert!(!stable.exists()); + assert!(!old.exists()); + Ok(()) +} + +#[test] +fn failed_keychain_readback_preserves_the_legacy_source() -> Result<(), Box> +{ + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + write_token(&stable, 'a')?; + let store = BearerTokenStore::new(&profile_id, directory.path()); + + assert!(store.load_with(&DroppingCredentialBackend).is_err()); + assert!(stable.exists()); + Ok(()) +} + +#[test] +fn existing_keychain_entry_finishes_a_crashed_migration() -> Result<(), Box> +{ + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + write_token(&stable, 'a')?; + let backend = MemoryCredentialBackend::default(); + backend.save(KEYCHAIN_SERVICE, profile_id.as_str(), "a".repeat(64).as_bytes())?; + let store = BearerTokenStore::new(&profile_id, directory.path()); + + assert!(store.load_with(&backend)?.is_some()); + assert!(!stable.exists()); + Ok(()) +} + +#[test] +fn profile_accounts_are_isolated_and_clear_is_idempotent() -> Result<(), Box> +{ + let directory = tempfile::tempdir()?; + let first_id = ProfileId::new(); + let second_id = ProfileId::new(); + let first = BearerTokenStore::new(&first_id, &directory.path().join("first")); + let second = BearerTokenStore::new(&second_id, &directory.path().join("second")); + let first_token = BearerToken::new("a".repeat(64))?; + let second_token = BearerToken::new("b".repeat(64))?; + let backend = MemoryCredentialBackend::default(); + + first.save_with(&backend, &first_token)?; + second.save_with(&backend, &second_token)?; + assert_eq!(first.load_with(&backend)?, Some(first_token)); + assert_eq!(second.load_with(&backend)?, Some(second_token.clone())); + first.clear_with(&backend)?; + first.clear_with(&backend)?; + assert_eq!(first.load_with(&backend)?, None); + assert_eq!(second.load_with(&backend)?, Some(second_token)); + Ok(()) +} + +#[test] +fn conditional_clear_preserves_a_newer_profile_token() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let store = BearerTokenStore::new(&profile_id, directory.path()); + let old = BearerToken::new("a".repeat(64))?; + let current = BearerToken::new("b".repeat(64))?; + let backend = MemoryCredentialBackend::default(); + + store.save_with(&backend, &old)?; + store.save_with(&backend, ¤t)?; + assert!(!store.clear_if_matches_with(&backend, &old)?); + assert_eq!(store.load_with(&backend)?, Some(current.clone())); + assert!(store.clear_if_matches_with(&backend, ¤t)?); + assert_eq!(store.load_with(&backend)?, None); + Ok(()) +} + +#[test] +fn clear_failure_preserves_the_credential_for_retry() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let store = BearerTokenStore::new(&profile_id, directory.path()); + let token = BearerToken::new("a".repeat(64))?; + let backend = ClearFailureBackend::default(); + + store.save_with(&backend, &token)?; + assert!(store.clear_with(&backend).is_err()); + assert_eq!(store.load_with(&backend)?, Some(token)); + Ok(()) +} + +#[test] +fn migration_marker_blocks_recreated_legacy_tokens() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let old = directory.path().join("old/bearer.token"); + let store = BearerTokenStore::new(&profile_id, directory.path()).with_legacy_path(old.clone()); + let backend = MemoryCredentialBackend::default(); + let token = BearerToken::new("a".repeat(64))?; + + store.save_with(&backend, &token)?; + store.clear_with(&backend)?; + write_token(&old, 'b')?; + + assert_eq!(store.load_with(&backend)?, None); + assert!(!old.exists()); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn cleanup_failure_prevents_a_direct_keychain_commit() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + let target = directory.path().join("target.tmp"); + write_token(&target, 'b')?; + fs::create_dir_all(stable.parent().ok_or("missing stable parent")?)?; + fs::hard_link(&target, stable.with_extension("tmp"))?; + let store = BearerTokenStore::new(&profile_id, directory.path()); + let backend = MemoryCredentialBackend::default(); + let token = BearerToken::new("a".repeat(64))?; + + assert!(store.save_with(&backend, &token).is_err()); + assert!(backend.load(KEYCHAIN_SERVICE, profile_id.as_str())?.is_none()); + Ok(()) +} + +#[test] +fn direct_save_failure_retires_stale_legacy_credentials() -> Result<(), Box> +{ + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + write_token(&stable, 'a')?; + let store = BearerTokenStore::new(&profile_id, directory.path()); + let token = BearerToken::new("b".repeat(64))?; + + assert!(store.save_with(&SaveFailureBackend, &token).is_err()); + assert!(!stable.exists()); + assert!(migration_marker_path(directory.path()).exists()); + assert_eq!(store.load_with(&MemoryCredentialBackend::default())?, None); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn migration_cleanup_failure_keeps_the_committed_keychain() -> Result<(), Box> +{ + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + let target = directory.path().join("target.tmp"); + write_token(&stable, 'a')?; + write_token(&target, 'b')?; + fs::hard_link(&target, stable.with_extension("tmp"))?; + let store = BearerTokenStore::new(&profile_id, directory.path()); + let backend = MemoryCredentialBackend::default(); + + assert!(store.load_with(&backend).is_err()); + assert!(backend.load(KEYCHAIN_SERVICE, profile_id.as_str())?.is_some()); + assert!(migration_marker_path(directory.path()).exists()); + fs::remove_file(stable.with_extension("tmp"))?; + assert_eq!( + store.load_with(&backend)?.as_ref().map(BearerToken::as_str), + Some("a".repeat(64).as_str()) + ); + Ok(()) +} + +#[test] +fn invalid_migration_marker_never_retires_a_legacy_token() -> Result<(), Box> +{ + for marker in [b"".as_slice(), b"partial".as_slice()] { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + let marker_path = migration_marker_path(directory.path()); + write_token(&stable, 'a')?; + fs::write(&marker_path, marker)?; + let store = BearerTokenStore::new(&profile_id, directory.path()); + let backend = MemoryCredentialBackend::default(); + + assert!(store.load_with(&backend).is_err()); + assert!(stable.exists()); + assert!(backend.load(KEYCHAIN_SERVICE, profile_id.as_str())?.is_none()); + } + Ok(()) +} + +#[test] +fn crashed_marker_temp_is_replaced_atomically() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let marker = migration_marker_path(directory.path()); + let marker_tmp = marker.with_file_name("bearer.migrated.tmp"); + fs::create_dir_all(marker.parent().ok_or("missing marker parent")?)?; + fs::write(&marker_tmp, b"partial")?; + let store = BearerTokenStore::new(&profile_id, directory.path()); + + store.clear_with(&MemoryCredentialBackend::default())?; + + assert_eq!(fs::read(marker)?, b"ely-bearer-migration-v1\n"); + assert!(!marker_tmp.exists()); + Ok(()) +} + +#[test] +fn stale_tmp_is_removed_without_becoming_a_token() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let stable = stable_legacy_path(directory.path()); + write_token(&stable.with_extension("tmp"), 'a')?; + let store = BearerTokenStore::new(&profile_id, directory.path()); + + assert_eq!(store.load_with(&MemoryCredentialBackend::default())?, None); + assert!(!stable.with_extension("tmp").exists()); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn symlink_and_hardlink_legacy_sources_are_rejected() -> Result<(), Box> { + use std::os::unix::fs::symlink; + + for hard_link in [false, true] { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let target = directory.path().join("target.token"); + let stable = stable_legacy_path(directory.path()); + write_token(&target, 'a')?; + fs::create_dir_all(stable.parent().ok_or("missing stable parent")?)?; + if hard_link { + fs::hard_link(&target, &stable)?; + } else { + symlink(&target, &stable)?; + } + let backend = MemoryCredentialBackend::default(); + let store = BearerTokenStore::new(&profile_id, directory.path()); + + assert!(store.load_with(&backend).is_err()); + assert!(backend.load(KEYCHAIN_SERVICE, profile_id.as_str())?.is_none()); + } + Ok(()) +} + +#[cfg(unix)] +#[test] +fn symlink_and_hardlink_lock_files_are_rejected() -> Result<(), Box> { + use std::os::unix::fs::{PermissionsExt, symlink}; + + for hard_link in [false, true] { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let target = directory.path().join("target.lock"); + let lock = directory.path().join("sync/bearer.lock"); + fs::write(&target, b"lock-target")?; + fs::set_permissions(&target, fs::Permissions::from_mode(0o644))?; + fs::create_dir_all(lock.parent().ok_or("missing lock parent")?)?; + if hard_link { + fs::hard_link(&target, &lock)?; + } else { + symlink(&target, &lock)?; + } + let store = BearerTokenStore::new(&profile_id, directory.path()); + + assert!(store.load_with(&MemoryCredentialBackend::default()).is_err()); + assert_eq!(fs::metadata(&target)?.permissions().mode() & 0o777, 0o644); + } + Ok(()) +} + +#[cfg(unix)] +#[test] +fn symlinked_profile_directories_are_rejected() -> Result<(), Box> { + use std::os::unix::fs::symlink; + + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let target = directory.path().join("target-profile"); + let linked_profile = directory.path().join("linked-profile"); + fs::create_dir_all(&target)?; + symlink(&target, &linked_profile)?; + let store = BearerTokenStore::new(&profile_id, &linked_profile); + + assert!(store.load_with(&MemoryCredentialBackend::default()).is_err()); + assert!(!target.join("sync").exists()); + Ok(()) +} + +#[cfg(target_os = "macos")] +#[test] +fn native_keychain_round_trip() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let profile_id = ProfileId::new(); + let store = BearerTokenStore::new(&profile_id, directory.path()); + let token = BearerToken::new("n".repeat(64))?; + let result = (|| { + store.save(&token)?; + assert_eq!(store.load()?, Some(token)); + Ok::<(), SyncClientError>(()) + })(); + store.clear()?; + result?; + Ok(()) +} + +fn stable_legacy_path(profile_dir: &std::path::Path) -> std::path::PathBuf { + profile_dir.join("sync/bearer.token") +} + +fn migration_marker_path(profile_dir: &std::path::Path) -> std::path::PathBuf { + profile_dir.join("sync/bearer.migrated") +} + +fn write_token(path: &std::path::Path, character: char) -> Result<(), std::io::Error> { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent)?; + } + fs::write(path, character.to_string().repeat(64)) +} diff --git a/crates/ely_sync_client/src/credential_store.rs b/crates/ely_sync_client/src/credential_store.rs index d79e485..c94c9e2 100644 --- a/crates/ely_sync_client/src/credential_store.rs +++ b/crates/ely_sync_client/src/credential_store.rs @@ -1,28 +1,174 @@ -use keyring::{Entry, Error as KeyringError}; +use keyring_core::{CredentialStore, Entry, Error as KeyringError}; +use std::sync::{Arc, Mutex}; use zeroize::Zeroizing; +static NATIVE_STORE: Mutex>> = Mutex::new(None); + pub(crate) fn load_secret( service: &str, account: &str, ) -> Result>>, String> { - match entry(service, account)?.get_secret() { + native_operation(service, account, |entry| match entry.get_secret() { Ok(secret) => Ok(Some(Zeroizing::new(secret))), Err(KeyringError::NoEntry) => Ok(None), - Err(error) => Err(error.to_string()), - } + Err(error) => Err(error), + }) } pub(crate) fn save_secret(service: &str, account: &str, secret: &[u8]) -> Result<(), String> { - entry(service, account)?.set_secret(secret).map_err(|error| error.to_string()) + native_operation(service, account, |entry| entry.set_secret(secret)) } pub(crate) fn clear_secret(service: &str, account: &str) -> Result<(), String> { - match entry(service, account)?.delete_credential() { + native_operation(service, account, |entry| match entry.delete_credential() { Ok(()) | Err(KeyringError::NoEntry) => Ok(()), - Err(error) => Err(error.to_string()), + Err(error) => Err(error), + }) +} + +fn native_operation( + service: &str, + account: &str, + operation: impl Fn(&Entry) -> keyring_core::Result, +) -> Result { + #[cfg(all(unix, not(any(target_os = "macos", target_os = "ios", target_os = "android"))))] + { + return retry_once( + || entry(service, account).and_then(|entry| operation(&entry)), + retryable_store_error, + clear_cached_store, + ) + .map_err(|error| error.to_string()); + } + #[cfg(not(all( + unix, + not(any(target_os = "macos", target_os = "ios", target_os = "android")) + )))] + { + entry(service, account) + .and_then(|entry| operation(&entry)) + .map_err(|error| error.to_string()) } } -fn entry(service: &str, account: &str) -> Result { - Entry::new(service, account).map_err(|error| error.to_string()) +#[cfg(any( + test, + all(unix, not(any(target_os = "macos", target_os = "ios", target_os = "android"))) +))] +fn retry_once( + mut attempt: impl FnMut() -> Result, + should_retry: impl Fn(&E) -> bool, + before_retry: impl FnOnce(), +) -> Result { + let first = attempt(); + if first.as_ref().is_err_and(should_retry) { + before_retry(); + attempt() + } else { + first + } +} + +fn entry(service: &str, account: &str) -> keyring_core::Result { + let store = { + let mut guard = NATIVE_STORE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if guard.is_none() { + *guard = Some(platform_store()?); + } + guard.as_ref().cloned().ok_or(KeyringError::NoDefaultStore)? + }; + build_entry(store.as_ref(), service, account) +} + +#[cfg(any( + test, + all(unix, not(any(target_os = "macos", target_os = "ios", target_os = "android"))) +))] +fn retryable_store_error(error: &KeyringError) -> bool { + matches!( + error, + KeyringError::PlatformFailure(_) + | KeyringError::NoStorageAccess(_) + | KeyringError::NoDefaultStore + ) +} + +#[cfg(all(unix, not(any(target_os = "macos", target_os = "ios", target_os = "android"))))] +fn clear_cached_store() { + let mut guard = NATIVE_STORE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + *guard = None; +} + +#[cfg(target_os = "windows")] +fn build_entry( + store: &CredentialStore, + service: &str, + account: &str, +) -> keyring_core::Result { + let modifiers = std::collections::HashMap::from([("persistence", "Local")]); + store.build(service, account, Some(&modifiers)) +} + +#[cfg(not(target_os = "windows"))] +fn build_entry( + store: &CredentialStore, + service: &str, + account: &str, +) -> keyring_core::Result { + store.build(service, account, None) +} + +#[cfg(target_os = "macos")] +fn platform_store() -> keyring_core::Result> { + apple_native_keyring_store::keychain::Store::new().map(|store| store as Arc) +} + +#[cfg(target_os = "windows")] +fn platform_store() -> keyring_core::Result> { + windows_native_keyring_store::Store::new().map(|store| store as Arc) +} + +#[cfg(all(unix, not(any(target_os = "macos", target_os = "ios", target_os = "android"))))] +fn platform_store() -> keyring_core::Result> { + zbus_secret_service_keyring_store::Store::new().map(|store| store as Arc) +} + +#[cfg(not(any( + target_os = "macos", + target_os = "windows", + all(unix, not(any(target_os = "ios", target_os = "android"))) +)))] +fn platform_store() -> keyring_core::Result> { + Err(KeyringError::NoDefaultStore) +} + +#[cfg(test)] +mod tests { + use super::{KeyringError, retry_once, retryable_store_error}; + use std::cell::Cell; + + #[test] + fn retry_policy_targets_store_connectivity_errors() { + assert!(retryable_store_error(&KeyringError::NoDefaultStore)); + assert!(!retryable_store_error(&KeyringError::NoEntry)); + } + + #[test] + fn retry_evicts_once_and_recreates_the_store_once() { + let attempts = Cell::new(0); + let evictions = Cell::new(0); + let result = retry_once( + || { + let attempt = attempts.get() + 1; + attempts.set(attempt); + Err::<(), _>(attempt) + }, + |_| true, + || evictions.set(evictions.get() + 1), + ); + + assert_eq!(result, Err(2)); + assert_eq!(attempts.get(), 2); + assert_eq!(evictions.get(), 1); + } } diff --git a/crates/ely_sync_client/src/device.rs b/crates/ely_sync_client/src/device.rs index 9b079ed..75fe023 100644 --- a/crates/ely_sync_client/src/device.rs +++ b/crates/ely_sync_client/src/device.rs @@ -17,6 +17,7 @@ use crate::{ const PUBLIC_KEY_BYTES: usize = 32; const MAX_DEVICE_TEXT_CHARS: usize = 128; +const MAX_STORED_IDENTITY_BYTES: usize = 16 * 1024; /// Public device identity persisted in the profile directory. Both private /// keys live in the macOS data-protection Keychain under `device_id`. @@ -33,6 +34,18 @@ pub struct DeviceIdentity { } impl DeviceIdentity { + pub fn validate_stored_bytes(bytes: &[u8], path: &Path) -> Result<(), SyncClientError> { + if bytes.len() > MAX_STORED_IDENTITY_BYTES { + return Err(key_error("stored device identity is too large")); + } + let contents = std::str::from_utf8(bytes) + .map_err(|error| SyncClientError::TokenStorage(error.to_string()))?; + match decode_stored_identity(contents, path)? { + StoredIdentity::V2(identity) => identity.validate(), + StoredIdentity::Legacy(identity) => identity.validate(), + } + } + /// Loads a v2 identity and its Keychain secrets. A legacy public-only /// identity is rotated to a fresh device ID because its private key was /// never persisted and cannot prove device continuity. @@ -413,6 +426,31 @@ mod tests { Ok(()) } + #[test] + fn stored_identity_validation_is_bounded_and_schema_strict() -> Result<(), SyncClientError> { + let (identity, _) = generate_key_material("Test".to_string(), "macos".to_string())?; + let bytes = serde_json::to_vec(&identity).map_err(|error| { + SyncClientError::TokenStorage(format!("device identity serialize: {error}")) + })?; + + DeviceIdentity::validate_stored_bytes(&bytes, Path::new("device.json"))?; + assert!( + DeviceIdentity::validate_stored_bytes( + &vec![b'a'; MAX_STORED_IDENTITY_BYTES + 1], + Path::new("device.json"), + ) + .is_err() + ); + assert!( + DeviceIdentity::validate_stored_bytes( + br#"{"device_id":"ely-test","unknown":true}"#, + Path::new("device.json"), + ) + .is_err() + ); + Ok(()) + } + #[cfg(target_os = "macos")] #[test] fn legacy_identity_rotates_to_new_device_id() -> Result<(), SyncClientError> { diff --git a/crates/ely_sync_client/src/email_otp.rs b/crates/ely_sync_client/src/email_otp.rs index 626391d..40b1af1 100644 --- a/crates/ely_sync_client/src/email_otp.rs +++ b/crates/ely_sync_client/src/email_otp.rs @@ -16,6 +16,7 @@ use serde::{Deserialize, Serialize}; use std::time::Duration; use ureq::{Agent, AgentBuilder}; +use zeroize::Zeroizing; use crate::{auth::BearerToken, client::ApiClientConfig, error::SyncClientError}; @@ -86,20 +87,21 @@ pub fn verify_email_otp( match response { Ok(ok) => { let cookie_token = better_auth_cookie_token(ok.header("set-cookie")); - let body = ok.into_string().map_err(|error| SyncClientError::HttpStatus { - endpoint: endpoint.clone(), - status: 200, - body: error.to_string(), - })?; + let body = + Zeroizing::new(ok.into_string().map_err(|error| SyncClientError::HttpStatus { + endpoint: endpoint.clone(), + status: 200, + body: error.to_string(), + })?); let json = serde_json::from_str::(&body).map_err(|error| { SyncClientError::Json { endpoint: endpoint.clone(), source: error } })?; - let token = json.token.or(cookie_token).ok_or_else(|| { + let token = json.token.map(Zeroizing::new).or(cookie_token).ok_or_else(|| { SyncClientError::TokenStorage( "sign-in response did not include a session token".to_string(), ) })?; - BearerToken::new(token) + BearerToken::new(token.as_str()) } Err(ureq::Error::Status(status, raw)) => { let body = raw.into_string().unwrap_or_default(); @@ -117,14 +119,14 @@ fn build_agent() -> Agent { /// better-auth.session_token=; …` header. Strip the cookie's /// attributes and return just the value. Multi-cookie responses are /// concatenated by `ureq` into a single header line per spec. -fn better_auth_cookie_token(set_cookie: Option<&str>) -> Option { +fn better_auth_cookie_token(set_cookie: Option<&str>) -> Option> { let header = set_cookie?; for cookie in header.split(',') { let trimmed = cookie.trim(); if let Some(rest) = trimmed.strip_prefix("better-auth.session_token=") { let token = rest.split(';').next()?.trim(); if !token.is_empty() { - return Some(token.to_string()); + return Some(Zeroizing::new(token.to_string())); } } } @@ -139,7 +141,10 @@ mod tests { fn picks_session_token_out_of_set_cookie_header() { let header = "better-auth.session_token=abc.def.ghi; Path=/; HttpOnly; Secure; SameSite=Lax"; - assert_eq!(better_auth_cookie_token(Some(header)), Some("abc.def.ghi".to_string()),); + assert_eq!( + better_auth_cookie_token(Some(header)).as_deref().map(String::as_str), + Some("abc.def.ghi"), + ); } #[test] diff --git a/crates/ely_sync_client/src/error.rs b/crates/ely_sync_client/src/error.rs index 01ac7fe..74c6cf1 100644 --- a/crates/ely_sync_client/src/error.rs +++ b/crates/ely_sync_client/src/error.rs @@ -8,6 +8,9 @@ pub enum SyncClientError { #[error("Bearer token storage is unavailable: {0}")] TokenStorage(String), + #[error("Bearer credential storage is unavailable: {0}")] + BearerCredentialStorage(String), + #[error("HTTP request failed for {endpoint}: {source}")] Http { endpoint: String, diff --git a/crates/ely_sync_client/src/lib.rs b/crates/ely_sync_client/src/lib.rs index 47613ce..4c7f103 100644 --- a/crates/ely_sync_client/src/lib.rs +++ b/crates/ely_sync_client/src/lib.rs @@ -17,6 +17,7 @@ //! the bearer token out-of-band and hand it to the client. pub mod auth; +mod auth_files; pub mod client; mod credential_store; pub mod device;