address review comments

.github/CODEOWNERS

@@ -33,6 +33,7 @@ apps/browser/src/tools @bitwarden/team-tools-dev
 apps/cli/src/tools @bitwarden/team-tools-dev
 apps/desktop/desktop_native/bitwarden_chromium_import_helper @bitwarden/team-tools-dev
 apps/desktop/desktop_native/chromium_importer @bitwarden/team-tools-dev
+apps/desktop/desktop_native/objc/src/native/chromium_importer @bitwarden/team-tools-dev
 apps/desktop/src/app/tools @bitwarden/team-tools-dev
 apps/web/src/app/tools @bitwarden/team-tools-dev
 libs/angular/src/tools @bitwarden/team-tools-dev

apps/desktop/desktop_native/build.js

@@ -28,10 +28,7 @@ let crossPlatform = process.argv.length > 2 && process.argv[2] === "cross-platfo
 function buildNapiModule(target, release = true) {
     const targetArg = target ? `--target ${target}` : "";
     const releaseArg = release ? "--release" : "";
-    child_process.execSync(`npm run build -- ${releaseArg} ${targetArg}`, {
+    child_process.execSync(`npm run build -- ${releaseArg} ${targetArg}`, { stdio: 'inherit', cwd: path.join(__dirname, "napi") });
-        stdio: 'inherit',
-        cwd: path.join(__dirname, "napi")    
-    });
 }
 
 function buildProxyBin(target, release = true) {

apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs

@@ -51,28 +51,30 @@ pub enum LoginImportResult {
 }
 
 pub trait InstalledBrowserRetriever {
-    fn get_installed_browsers(mas_build: bool) -> Result<Vec<String>>;
+    fn get_installed_browsers(mas_build: bool) -> Vec<String>;
 }
 
 pub struct DefaultInstalledBrowserRetriever {}
 
 impl InstalledBrowserRetriever for DefaultInstalledBrowserRetriever {
-    fn get_installed_browsers(mas_build: bool) -> Result<Vec<String>> {
+    fn get_installed_browsers(mas_build: bool) -> Vec<String> {
-        let mut browsers = Vec::with_capacity(SUPPORTED_BROWSER_MAP.len());
+        // Show all browsers for MAS builds, user will grant access when selected
-        for (browser, config) in SUPPORTED_BROWSER_MAP.iter() {
+        if mas_build {
-            if mas_build {
+            return SUPPORTED_BROWSER_MAP
-                // show all browsers for MAS builds, user will grant access when selected
+                .keys()
-                browsers.push((*browser).to_string());
+                .map(|browser| (*browser).to_string())
-            } else {
+                .collect();
-                // When not in sandbox check file system directly
-                let data_dir = get_and_validate_data_dir(config)?;
-                if data_dir.exists() {
-                    browsers.push((*browser).to_string());
-                }
-            }
         }
-
+        // When not in sandbox, check file system directly
-        Ok(browsers)
+        SUPPORTED_BROWSER_MAP
+            .iter()
+            .filter_map(|(browser, config)| {
+                get_and_validate_data_dir(config)
+                    .ok()
+                    .filter(|data_dir| data_dir.exists())
+                    .map(|_| (*browser).to_string())
+            })
+            .collect()
     }
 }
 
@@ -83,11 +85,9 @@ pub async fn get_available_profiles(
 ) -> Result<Vec<ProfileInfo>> {
     // MAS builds need to resume security-scoped access before reading browser files
     #[cfg(target_os = "macos")]
-    let _access = if mas_build {
+    if mas_build {
-        Some(platform::sandbox::ScopedBrowserAccess::resume(browser_name).await?)
+        platform::sandbox::ScopedBrowserAccess::resume(browser_name).await?;
-    } else {
+    }
-        None
-    };
 
     let (_, local_state) = load_local_state_for_browser(browser_name)?;
     Ok(get_profile_info(&local_state))
@@ -103,18 +103,17 @@ pub async fn request_browser_access(browser_name: &str, mas_build: bool) -> Resu
     Ok(())
 }
 
+#[allow(unused_variables)]
 pub async fn import_logins(
     browser_name: &str,
     profile_id: &str,
-    _mas_build: bool,
+    mas_build: bool,
 ) -> Result<Vec<LoginImportResult>> {
     // MAS builds will use the formerly created security bookmark
     #[cfg(target_os = "macos")]
-    let _access = if _mas_build {
+    if mas_build {
-        Some(platform::sandbox::ScopedBrowserAccess::resume(browser_name).await?)
+        platform::sandbox::ScopedBrowserAccess::resume(browser_name).await?;
-    } else {
+    }
-        None
-    };
 
     let (data_dir, local_state) = load_local_state_for_browser(browser_name)?;
 

apps/desktop/desktop_native/chromium_importer/src/chromium/platform/macos.rs

@@ -7,220 +7,6 @@ use crate::{
     util,
 };
 
-//
-// Sandbox specific (for Mac App Store Builds)
-//
-
-pub mod sandbox {
-    use anyhow::{anyhow, Result};
-    use serde::{Deserialize, Serialize};
-
-    // Before requesting access outside of sandbox, determine if browser is actually installed
-    const BROWSER_BUNDLE_IDS: &[(&str, &str)] = &[
-        ("Chrome", "com.google.Chrome"),
-        ("Chromium", "org.chromium.Chromium"),
-        ("Microsoft Edge", "com.microsoft.edgemac"),
-        ("Brave", "com.brave.Browser"),
-        ("Arc", "company.thebrowser.Browser"),
-        ("Opera", "com.operasoftware.Opera"),
-        ("Vivaldi", "com.vivaldi.Vivaldi"),
-    ];
-
-    #[derive(Debug, Deserialize)]
-    #[serde(tag = "type")]
-    enum CommandResult<T> {
-        #[serde(rename = "success")]
-        Success { value: T },
-        #[serde(rename = "error")]
-        Error { error: String },
-    }
-
-    #[derive(Debug, Deserialize)]
-    struct RequestAccessResponse {
-        #[allow(dead_code)]
-        bookmark: String,
-    }
-
-    #[derive(Debug, Deserialize)]
-    struct HasStoredAccessResponse {
-        #[serde(rename = "hasAccess")]
-        has_access: bool,
-    }
-
-    #[derive(Debug, Deserialize)]
-    struct StartAccessResponse {
-        #[allow(dead_code)]
-        path: String,
-    }
-
-    #[derive(Debug, Serialize)]
-    struct CommandInput {
-        namespace: String,
-        command: String,
-        params: serde_json::Value,
-    }
-
-    pub struct ScopedBrowserAccess {
-        browser_name: String,
-    }
-
-    impl ScopedBrowserAccess {
-        /// Request access to browser directory and create a security bookmark if access is approved
-        pub async fn request_only(browser_name: &str) -> Result<()> {
-            let config = crate::chromium::platform::SUPPORTED_BROWSERS
-                .iter()
-                .find(|b| b.name == browser_name)
-                .ok_or_else(|| anyhow!("Unsupported browser: {}", browser_name))?;
-
-            if !is_browser_installed(browser_name).await? {
-                return Err(anyhow!(
-                    "chromiumImporterBrowserNotInstalled:{}",
-                    browser_name
-                ));
-            }
-
-            // For macOS, data_dir is always a single-element array
-            let relative_path = config
-                .data_dir
-                .first()
-                .ok_or_else(|| anyhow!("No data directory configured for browser"))?;
-
-            let input = CommandInput {
-                namespace: "chromium_importer".to_string(),
-                command: "request_access".to_string(),
-                params: serde_json::json!({
-                    "browserName": browser_name,
-                    "relativePath": relative_path,
-                }),
-            };
-
-            let output = desktop_objc::run_command(serde_json::to_string(&input)?)
-                .await
-                .map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
-
-            let result: CommandResult<RequestAccessResponse> = serde_json::from_str(&output)
-                .map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
-
-            match result {
-                CommandResult::Success { .. } => Ok(()),
-                CommandResult::Error { error } => Err(anyhow!("{}", error)),
-            }
-        }
-
-        /// Resume browser directory access using previously created security bookmark
-        pub async fn resume(browser_name: &str) -> Result<Self> {
-            // First check if we have stored access
-            let has_access_input = CommandInput {
-                namespace: "chromium_importer".to_string(),
-                command: "has_stored_access".to_string(),
-                params: serde_json::json!({
-                    "browserName": browser_name,
-                }),
-            };
-
-            let has_access_output =
-                desktop_objc::run_command(serde_json::to_string(&has_access_input)?)
-                    .await
-                    .map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
-
-            let has_access_result: CommandResult<HasStoredAccessResponse> =
-                serde_json::from_str(&has_access_output)
-                    .map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
-
-            let has_access = match has_access_result {
-                CommandResult::Success { value } => value.has_access,
-                CommandResult::Error { error } => return Err(anyhow!("{}", error)),
-            };
-
-            if !has_access {
-                return Err(anyhow!("Access has not been granted for this browser"));
-            }
-
-            // Start accessing the browser
-            let start_input = CommandInput {
-                namespace: "chromium_importer".to_string(),
-                command: "start_access".to_string(),
-                params: serde_json::json!({
-                    "browserName": browser_name,
-                }),
-            };
-
-            let start_output = desktop_objc::run_command(serde_json::to_string(&start_input)?)
-                .await
-                .map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
-
-            let start_result: CommandResult<StartAccessResponse> =
-                serde_json::from_str(&start_output)
-                    .map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
-
-            match start_result {
-                CommandResult::Success { .. } => Ok(Self {
-                    browser_name: browser_name.to_string(),
-                }),
-                CommandResult::Error { error } => Err(anyhow!("{}", error)),
-            }
-        }
-    }
-
-    impl Drop for ScopedBrowserAccess {
-        fn drop(&mut self) {
-            let browser_name = self.browser_name.clone();
-
-            tokio::task::spawn(async move {
-                let input = CommandInput {
-                    namespace: "chromium_importer".to_string(),
-                    command: "stop_access".to_string(),
-                    params: serde_json::json!({
-                        "browserName": browser_name,
-                    }),
-                };
-
-                if let Ok(input_json) = serde_json::to_string(&input) {
-                    let _ = desktop_objc::run_command(input_json).await;
-                }
-            });
-        }
-    }
-
-    async fn is_browser_installed(browser_name: &str) -> Result<bool> {
-        let bundle_id = BROWSER_BUNDLE_IDS
-            .iter()
-            .find(|(name, _)| *name == browser_name)
-            .map(|(_, id)| *id);
-
-        let bundle_id = match bundle_id {
-            Some(id) => id,
-            None => return Ok(true),
-        };
-
-        let input = CommandInput {
-            namespace: "chromium_importer".to_string(),
-            command: "check_browser_installed".to_string(),
-            params: serde_json::json!({
-                "bundleId": bundle_id,
-            }),
-        };
-
-        let output = desktop_objc::run_command(serde_json::to_string(&input)?)
-            .await
-            .map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
-
-        let result: CommandResult<CheckBrowserInstalledResponse> = serde_json::from_str(&output)
-            .map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
-
-        match result {
-            CommandResult::Success { value } => Ok(value.is_installed),
-            CommandResult::Error { error } => Err(anyhow!("{}", error)),
-        }
-    }
-
-    #[derive(Debug, Deserialize)]
-    struct CheckBrowserInstalledResponse {
-        #[serde(rename = "isInstalled")]
-        is_installed: bool,
-    }
-}
-
 //
 // Public API
 //

apps/desktop/desktop_native/chromium_importer/src/chromium/platform/mod.rs

@@ -4,6 +4,10 @@
 #[cfg_attr(target_os = "macos", path = "macos.rs")]
 mod native;
 
+// Sandbox support (macOS only for MAS builds)
+#[cfg(target_os = "macos")]
+pub mod sandbox;
+
 // Windows exposes public const
 #[allow(unused_imports)]
 pub use native::*;

apps/desktop/desktop_native/chromium_importer/src/chromium/platform/sandbox.rs

@@ -0,0 +1,206 @@
+//
+// Sandbox specific (for Mac App Store Builds)
+//
+
+use anyhow::{anyhow, Result};
+use serde::{Deserialize, Serialize};
+
+// Before requesting access outside of sandbox, determine if browser is actually installed
+const BROWSER_BUNDLE_IDS: &[(&str, &str)] = &[
+    ("Chrome", "com.google.Chrome"),
+    ("Chromium", "org.chromium.Chromium"),
+    ("Microsoft Edge", "com.microsoft.edgemac"),
+    ("Brave", "com.brave.Browser"),
+    ("Arc", "company.thebrowser.Browser"),
+    ("Opera", "com.operasoftware.Opera"),
+    ("Vivaldi", "com.vivaldi.Vivaldi"),
+];
+
+#[derive(Debug, Deserialize)]
+#[serde(tag = "type")]
+enum CommandResult<T> {
+    #[serde(rename_all = "camelCase")]
+    Success { value: T },
+    #[serde(rename_all = "camelCase")]
+    Error { error: String },
+}
+
+#[derive(Debug, Deserialize)]
+struct RequestAccessResponse {
+    #[allow(dead_code)]
+    bookmark: String,
+}
+
+#[derive(Debug, Deserialize)]
+struct HasStoredAccessResponse {
+    #[serde(rename = "hasAccess")]
+    has_access: bool,
+}
+
+#[derive(Debug, Deserialize)]
+struct StartAccessResponse {
+    #[allow(dead_code)]
+    path: String,
+}
+
+#[derive(Debug, Serialize)]
+struct CommandInput {
+    namespace: String,
+    command: String,
+    params: serde_json::Value,
+}
+
+pub struct ScopedBrowserAccess {
+    browser_name: String,
+}
+
+impl ScopedBrowserAccess {
+    /// Request access to browser directory and create a security bookmark if access is approved
+    pub async fn request_only(browser_name: &str) -> Result<()> {
+        let config = crate::chromium::platform::SUPPORTED_BROWSERS
+            .iter()
+            .find(|b| b.name == browser_name)
+            .ok_or_else(|| anyhow!("Unsupported browser: {}", browser_name))?;
+
+        if !is_browser_installed(browser_name).await? {
+            return Err(anyhow!(
+                "chromiumImporterBrowserNotInstalled:{}",
+                browser_name
+            ));
+        }
+
+        // For macOS, data_dir is always a single-element array
+        let relative_path = config
+            .data_dir
+            .first()
+            .ok_or_else(|| anyhow!("No data directory configured for browser"))?;
+
+        let input = CommandInput {
+            namespace: "chromium_importer".to_string(),
+            command: "request_access".to_string(),
+            params: serde_json::json!({
+                "browserName": browser_name,
+                "relativePath": relative_path,
+            }),
+        };
+
+        let output = desktop_objc::run_command(serde_json::to_string(&input)?)
+            .await
+            .map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
+
+        let result: CommandResult<RequestAccessResponse> = serde_json::from_str(&output)
+            .map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
+
+        match result {
+            CommandResult::Success { .. } => Ok(()),
+            CommandResult::Error { error } => Err(anyhow!("{}", error)),
+        }
+    }
+
+    /// Resume browser directory access using previously created security bookmark
+    pub async fn resume(browser_name: &str) -> Result<Self> {
+        // First check if we have stored access
+        let has_access_input = CommandInput {
+            namespace: "chromium_importer".to_string(),
+            command: "has_stored_access".to_string(),
+            params: serde_json::json!({
+                "browserName": browser_name,
+            }),
+        };
+
+        let has_access_output =
+            desktop_objc::run_command(serde_json::to_string(&has_access_input)?)
+                .await
+                .map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
+
+        let has_access_result: CommandResult<HasStoredAccessResponse> =
+            serde_json::from_str(&has_access_output)
+                .map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
+
+        let has_access = match has_access_result {
+            CommandResult::Success { value } => value.has_access,
+            CommandResult::Error { error } => return Err(anyhow!("{}", error)),
+        };
+
+        if !has_access {
+            return Err(anyhow!("Access has not been granted for this browser"));
+        }
+
+        // Start accessing the browser
+        let start_input = CommandInput {
+            namespace: "chromium_importer".to_string(),
+            command: "start_access".to_string(),
+            params: serde_json::json!({
+                "browserName": browser_name,
+            }),
+        };
+
+        let start_output = desktop_objc::run_command(serde_json::to_string(&start_input)?)
+            .await
+            .map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
+
+        let start_result: CommandResult<StartAccessResponse> = serde_json::from_str(&start_output)
+            .map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
+
+        match start_result {
+            CommandResult::Success { .. } => Ok(Self {
+                browser_name: browser_name.to_string(),
+            }),
+            CommandResult::Error { error } => Err(anyhow!("{}", error)),
+        }
+    }
+}
+
+impl Drop for ScopedBrowserAccess {
+    fn drop(&mut self) {
+        let browser_name = self.browser_name.clone();
+
+        tokio::task::spawn(async move {
+            let input = CommandInput {
+                namespace: "chromium_importer".to_string(),
+                command: "stop_access".to_string(),
+                params: serde_json::json!({
+                    "browserName": browser_name,
+                }),
+            };
+
+            if let Ok(input_json) = serde_json::to_string(&input) {
+                let _ = desktop_objc::run_command(input_json).await;
+            }
+        });
+    }
+}
+
+async fn is_browser_installed(browser_name: &str) -> Result<bool> {
+    let bundle_id = BROWSER_BUNDLE_IDS
+        .iter()
+        .find(|(name, _)| *name == browser_name)
+        .map(|(_, id)| *id)
+        .ok_or(true);
+
+    let input = CommandInput {
+        namespace: "chromium_importer".to_string(),
+        command: "check_browser_installed".to_string(),
+        params: serde_json::json!({
+            "bundleId": bundle_id,
+        }),
+    };
+
+    let output = desktop_objc::run_command(serde_json::to_string(&input)?)
+        .await
+        .map_err(|e| anyhow!("Failed to call ObjC command: {}", e))?;
+
+    let result: CommandResult<CheckBrowserInstalledResponse> = serde_json::from_str(&output)
+        .map_err(|e| anyhow!("Failed to parse ObjC response: {}", e))?;
+
+    match result {
+        CommandResult::Success { value } => Ok(value.is_installed),
+        CommandResult::Error { error } => Err(anyhow!("{}", error)),
+    }
+}
+
+#[derive(Debug, Deserialize)]
+struct CheckBrowserInstalledResponse {
+    #[serde(rename = "isInstalled")]
+    is_installed: bool,
+}

apps/desktop/desktop_native/chromium_importer/src/metadata.rs

@@ -22,7 +22,7 @@ pub fn get_supported_importers<T: InstalledBrowserRetriever>(
     let mut map = HashMap::new();
 
     // Check for installed browsers
-    let installed_browsers = T::get_installed_browsers(mas_build).unwrap_or_default();
+    let installed_browsers = T::get_installed_browsers(mas_build);
 
     const IMPORTERS: &[(&str, &str)] = &[
         ("chromecsv", "Chrome"),
@@ -68,11 +68,11 @@ mod tests {
     pub struct MockInstalledBrowserRetriever {}
 
     impl InstalledBrowserRetriever for MockInstalledBrowserRetriever {
-        fn get_installed_browsers(_mas_build: bool) -> Result<Vec<String>, anyhow::Error> {
+        fn get_installed_browsers(_mas_build: bool) -> Vec<String> {
-            Ok(SUPPORTED_BROWSER_MAP
+            SUPPORTED_BROWSER_MAP
                 .keys()
                 .map(|browser| browser.to_string())
-                .collect())
+                .collect()
         }
     }
 

apps/desktop/desktop_native/objc/src/native/bridging-header.h

@@ -1,3 +0,0 @@
-#import <Foundation/Foundation.h>
-#import "utils.h"
-#import "interop.h"