From b58207e409ab17d2dd2a4b826a18619dc7ac4250 Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Tue, 2 Dec 2025 10:03:03 -0700 Subject: [PATCH] remove sandbox feature flag and refactor to use bool --- .github/workflows/build-desktop.yml | 3 -- .../chromium_importer/Cargo.toml | 3 -- .../chromium_importer/src/chromium/mod.rs | 33 ++++++++++++------- .../src/chromium/platform/macos.rs | 1 - apps/desktop/desktop_native/napi/Cargo.toml | 1 - apps/desktop/desktop_native/napi/index.d.ts | 4 +-- .../desktop_native/napi/scripts/build.js | 7 +--- apps/desktop/desktop_native/napi/src/lib.rs | 14 ++++---- apps/desktop/package.json | 1 - apps/desktop/scripts/start.js | 2 +- .../tools/import/chromium-importer.service.ts | 6 ++-- 11 files changed, 36 insertions(+), 39 deletions(-) diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 8280f433ec..949263b34b 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -1688,9 +1688,6 @@ jobs: - name: Build Native Module if: steps.cache.outputs.cache-hit != 'true' working-directory: apps/desktop/desktop_native - env: - # required to bypass sandbox (MAS only) - SANDBOX_BUILD: 1 run: node build.js cross-platform - name: Build diff --git a/apps/desktop/desktop_native/chromium_importer/Cargo.toml b/apps/desktop/desktop_native/chromium_importer/Cargo.toml index ec545ad6e3..a7c8754a9d 100644 --- a/apps/desktop/desktop_native/chromium_importer/Cargo.toml +++ b/apps/desktop/desktop_native/chromium_importer/Cargo.toml @@ -43,6 +43,3 @@ sha1 = "=0.10.6" [lints] workspace = true - -[features] -sandbox = [] diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs index 2f022946ec..6e87485b33 100644 --- a/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs +++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs @@ -59,8 +59,6 @@ pub struct DefaultInstalledBrowserRetriever {} impl InstalledBrowserRetriever for DefaultInstalledBrowserRetriever { fn get_installed_browsers(mas_build: bool) -> Result> { let mut browsers = Vec::with_capacity(SUPPORTED_BROWSER_MAP.len()); - - #[allow(unused_variables)] // config only used outside of sandbox for (browser, config) in SUPPORTED_BROWSER_MAP.iter() { if mas_build { // show all browsers for MAS builds, user will grant access when selected @@ -83,19 +81,30 @@ pub fn get_available_profiles(browser_name: &str) -> Result> { Ok(get_profile_info(&local_state)) } -/// Request access to browser directory (sandbox mode only) -/// This shows the permission dialog and creates a security-scoped bookmark, -#[cfg(all(target_os = "macos", feature = "sandbox"))] -pub fn request_browser_access(browser_name: &str) -> Result<()> { - platform::sandbox::ScopedBrowserAccess::request_only(browser_name)?; - +/// Request access to browser directory (MAS builds only) +/// This shows the permission dialog and creates a security-scoped bookmark +#[cfg(target_os = "macos")] +pub fn request_browser_access(browser_name: &str, mas_build: bool) -> Result<()> { + if mas_build { + platform::sandbox::ScopedBrowserAccess::request_only(browser_name)?; + } Ok(()) } -pub async fn import_logins(browser_name: &str, profile_id: &str) -> Result> { - // In sandbox mode, resume access to browser directory (use the formerly created bookmark) - #[cfg(all(target_os = "macos", feature = "sandbox"))] - let _access = platform::sandbox::ScopedBrowserAccess::resume(browser_name)?; +pub async fn import_logins( + browser_name: &str, + profile_id: &str, + mas_build: bool, +) -> Result> { + // MAS builds will use the formerly created security bookmark + #[cfg(target_os = "macos")] + let _access = if mas_build { + Some(platform::sandbox::ScopedBrowserAccess::resume( + browser_name, + )?) + } else { + None + }; let (data_dir, local_state) = load_local_state_for_browser(browser_name)?; diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/macos.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/macos.rs index c929ab2159..3ddd64a71f 100644 --- a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/macos.rs +++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/macos.rs @@ -11,7 +11,6 @@ use crate::{ // Sandbox specific (for Mac App Store Builds) // -#[cfg(feature = "sandbox")] pub mod sandbox { use std::{ffi::CString, os::raw::c_char}; diff --git a/apps/desktop/desktop_native/napi/Cargo.toml b/apps/desktop/desktop_native/napi/Cargo.toml index 77659a93c0..b5847a602d 100644 --- a/apps/desktop/desktop_native/napi/Cargo.toml +++ b/apps/desktop/desktop_native/napi/Cargo.toml @@ -12,7 +12,6 @@ crate-type = ["cdylib"] [features] default = [] manual_test = [] -sandbox = ["chromium_importer/sandbox"] [dependencies] anyhow = { workspace = true } diff --git a/apps/desktop/desktop_native/napi/index.d.ts b/apps/desktop/desktop_native/napi/index.d.ts index 3432e19dfa..ea3eac8e62 100644 --- a/apps/desktop/desktop_native/napi/index.d.ts +++ b/apps/desktop/desktop_native/napi/index.d.ts @@ -255,8 +255,8 @@ export declare namespace chromium_importer { /** Returns OS aware metadata describing supported Chromium based importers as a JSON string. */ export function getMetadata(masBuild: boolean): Record export function getAvailableProfiles(browser: string): Array - export function importLogins(browser: string, profileId: string): Promise> - export function requestBrowserAccess(browser: string): void + export function importLogins(browser: string, profileId: string, masBuild: boolean): Promise> + export function requestBrowserAccess(browser: string, masBuild: boolean): void } export declare namespace autotype { export function getForegroundWindowTitle(): string diff --git a/apps/desktop/desktop_native/napi/scripts/build.js b/apps/desktop/desktop_native/napi/scripts/build.js index 313ece87ec..a6680f5d31 100644 --- a/apps/desktop/desktop_native/napi/scripts/build.js +++ b/apps/desktop/desktop_native/napi/scripts/build.js @@ -11,9 +11,4 @@ if (isRelease) { process.env.RUST_LOG = 'debug'; } -const featuresArg = process.env.SANDBOX_BUILD === '1' ? '--features sandbox' : ''; -if (featuresArg) { - console.log('Building with sandbox feature enabled.'); -} - -execSync(`napi build --platform --js false ${featuresArg}`, { stdio: 'inherit', env: process.env }); +execSync(`napi build --platform --js false`, { stdio: 'inherit', env: process.env }); diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index 4afaf666fe..bfbabe205d 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -1186,24 +1186,24 @@ pub mod chromium_importer { pub async fn import_logins( browser: String, profile_id: String, + mas_build: bool, ) -> napi::Result> { - chromium_importer::chromium::import_logins(&browser, &profile_id) + chromium_importer::chromium::import_logins(&browser, &profile_id, mas_build) .await .map(|logins| logins.into_iter().map(LoginImportResult::from).collect()) .map_err(|e| napi::Error::from_reason(e.to_string())) } #[napi] - #[allow(unused_variables)] - pub fn request_browser_access(browser: String) -> napi::Result<()> { - #[cfg(all(target_os = "macos", feature = "sandbox"))] + pub fn request_browser_access(browser: String, mas_build: bool) -> napi::Result<()> { + #[cfg(target_os = "macos")] { - chromium_importer::chromium::request_browser_access(&browser) + chromium_importer::chromium::request_browser_access(&browser, mas_build) .map_err(|e| napi::Error::from_reason(e.to_string())) } - #[cfg(not(all(target_os = "macos", feature = "sandbox")))] + #[cfg(not(target_os = "macos"))] { - // No-op when built without sandbox feature + // No-op outside of Mac OS Ok(()) } } diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 5ffd9556c2..bb8118cb7e 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -35,7 +35,6 @@ "build:renderer:watch": "cross-env NODE_ENV=development webpack --config webpack.config.js --config-name renderer --watch", "electron": "node ./scripts/start.js", "electron:ignore": "node ./scripts/start.js --ignore-certificate-errors", - "electron:sandbox": "SANDBOX_BUILD=1 node ./scripts/start.js", "flatpak:dev": "npm run clean:dist && electron-builder --dir -p never && flatpak-builder --force-clean --install --user ../../.flatpak/ ./resources/com.bitwarden.desktop.devel.yaml && flatpak run com.bitwarden.desktop", "clean:dist": "rimraf ./dist", "pack:dir": "npm run clean:dist && electron-builder --dir -p never", diff --git a/apps/desktop/scripts/start.js b/apps/desktop/scripts/start.js index da58b199a2..0e11ebd908 100644 --- a/apps/desktop/scripts/start.js +++ b/apps/desktop/scripts/start.js @@ -25,7 +25,7 @@ concurrently( }, { name: "Elec", - command: `npx wait-on ./build/main.js && npx electron ${process.env.SANDBOX_BUILD ? "" : "--no-sandbox "}--inspect=5858 ${args.join( + command: `npx wait-on ./build/main.js && npx electron --no-sandbox --inspect=5858 ${args.join( " ", )} ./build --watch`, prefixColor: "green", diff --git a/apps/desktop/src/app/tools/import/chromium-importer.service.ts b/apps/desktop/src/app/tools/import/chromium-importer.service.ts index b56a5b2be9..0e255c1879 100644 --- a/apps/desktop/src/app/tools/import/chromium-importer.service.ts +++ b/apps/desktop/src/app/tools/import/chromium-importer.service.ts @@ -2,6 +2,8 @@ import { ipcMain } from "electron"; import { chromium_importer } from "@bitwarden/desktop-napi"; +import { isMacAppStore } from "../../../utils"; + export class ChromiumImporterService { constructor() { ipcMain.handle("chromium_importer.getMetadata", async (event, isMas: boolean) => { @@ -11,7 +13,7 @@ export class ChromiumImporterService { // Used on Mac OS App Store builds to request permissions to browser entries outside the sandbox ipcMain.handle("chromium_importer.requestBrowserAccess", async (event, browser: string) => { if (chromium_importer.requestBrowserAccess) { - return await chromium_importer.requestBrowserAccess(browser); + return await chromium_importer.requestBrowserAccess(browser, isMacAppStore()); } // requestBrowserAccess not found, returning with no-op return; @@ -24,7 +26,7 @@ export class ChromiumImporterService { ipcMain.handle( "chromium_importer.importLogins", async (event, browser: string, profileId: string) => { - return await chromium_importer.importLogins(browser, profileId); + return await chromium_importer.importLogins(browser, profileId, isMacAppStore()); }, ); }