1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

add/improve comments and logging

This commit is contained in:
John Harrington
2025-11-20 09:51:49 -07:00
parent 61b26241ad
commit 18064efc4f
10 changed files with 35 additions and 31 deletions

View File

@@ -59,7 +59,7 @@ impl InstalledBrowserRetriever for DefaultInstalledBrowserRetriever {
fn get_installed_browsers() -> Result<Vec<String>> {
let mut browsers = Vec::with_capacity(SUPPORTED_BROWSER_MAP.len());
#[allow(unused_variables)] // config only used in non-sandbox mode
#[allow(unused_variables)] // config only used outside of sandbox
for (browser, config) in SUPPORTED_BROWSER_MAP.iter() {
#[cfg(all(target_os = "macos", feature = "sandbox"))]
{
@@ -69,7 +69,7 @@ impl InstalledBrowserRetriever for DefaultInstalledBrowserRetriever {
#[cfg(not(all(target_os = "macos", feature = "sandbox")))]
{
// All other platforms OR macOS without sandbox: check file system directly
// When not in sandbox check file system directly
let data_dir = get_browser_data_dir(config)?;
if data_dir.exists() {
browsers.push((*browser).to_string());
@@ -88,12 +88,14 @@ pub fn get_available_profiles(browser_name: &String) -> Result<Vec<ProfileInfo>>
/// Request access to browser directory (sandbox mode only)
/// This shows the permission dialog and creates a security-scoped bookmark,
/// but does NOT start accessing the resource (that happens in resume()).
#[cfg(all(target_os = "macos", feature = "sandbox"))]
pub fn request_browser_access(browser_name: &String) -> Result<()> {
eprintln!("[SANDBOX] request_browser_access called for: {}", browser_name);
println!("request_browser_access() called for: {}", browser_name);
platform::ScopedBrowserAccess::request_only(browser_name)?;
eprintln!("[SANDBOX] request_browser_access completed successfully");
println!("request_browser_access() completed successfully");
Ok(())
}
@@ -101,7 +103,7 @@ pub async fn import_logins(
browser_name: &String,
profile_id: &String,
) -> Result<Vec<LoginImportResult>> {
// In sandbox mode, resume access to browser directory
// In sandbox mode, resume access to browser directory (use the formerly created bookmark)
#[cfg(all(target_os = "macos", feature = "sandbox"))]
let _access = platform::ScopedBrowserAccess::resume(browser_name)?;

View File

@@ -11,7 +11,7 @@ use crate::chromium::{BrowserConfig, CryptoService, LocalState};
use crate::util;
//
// Sandbox
// Sandbox specific (for Mac App Store Builds)
//
#[cfg(feature = "sandbox")]
@@ -29,34 +29,34 @@ pub struct ScopedBrowserAccess {
#[cfg(feature = "sandbox")]
impl ScopedBrowserAccess {
/// Request access to browser directory and create a security bookmark if access is approved
pub fn request_only(browser_name: &str) -> Result<()> {
println!("request_only() called for {}", browser_name);
let c_name = CString::new(browser_name)?;
let bookmark_ptr = unsafe { requestBrowserAccess(c_name.as_ptr()) };
if bookmark_ptr.is_null() {
return Err(anyhow!("User declined access or browser not found"));
return Err(anyhow!("User declined access"));
}
unsafe { libc::free(bookmark_ptr as *mut libc::c_void) };
Ok(())
}
pub fn request_and_start(browser_name: &str) -> Result<Self> {
Self::request_only(browser_name)?;
Self::resume(browser_name)
}
/// Resume access using previously stored bookmark
/// Resume browser directory access using previously created security bookmark
pub fn resume(browser_name: &str) -> Result<Self> {
println!("resume() called for {}", browser_name);
let c_name = CString::new(browser_name)?;
if !unsafe { hasStoredBrowserAccess(c_name.as_ptr()) } {
return Err(anyhow!("No stored access for this browser"));
return Err(anyhow!("Access has not been granted for this browser"));
}
let path_ptr = unsafe { startBrowserAccess(c_name.as_ptr()) };
if path_ptr.is_null() {
return Err(anyhow!("Failed to resume access (bookmark may be stale)"));
return Err(anyhow!("Failed to use browser existing security access, it may be stale"));
}
unsafe { libc::free(path_ptr as *mut libc::c_void) };
@@ -65,7 +65,17 @@ impl ScopedBrowserAccess {
})
}
/// First requests access to browser directory and then ensures access is still usable
pub fn request_and_start(browser_name: &str) -> Result<Self> {
println!("request_and_start() called for {}", browser_name);
Self::request_only(browser_name)?;
Self::resume(browser_name)
}
pub fn has_stored_access(browser_name: &str) -> bool {
println!("has_stored_access() called for {}", browser_name);
let Ok(c_name) = CString::new(browser_name) else {
return false;
};
@@ -76,6 +86,8 @@ impl ScopedBrowserAccess {
#[cfg(feature = "sandbox")]
impl Drop for ScopedBrowserAccess {
fn drop(&mut self) {
println!("drop ScopedBrowserAccess has been called");
let Ok(c_name) = CString::new(self.browser_name.as_str()) else {
return;
};

View File

@@ -249,6 +249,7 @@ export declare namespace chromium_importer {
export function getMetadata(): Record<string, NativeImporterMetadata>
export function getAvailableProfiles(browser: string): Array<ProfileInfo>
export function importLogins(browser: string, profileId: string): Promise<Array<LoginImportResult>>
// used only on Mac OS App Store builds, no-op on other platforms
export function requestBrowserAccess(browser: string): void
}
export declare namespace autotype {

View File

@@ -17,12 +17,3 @@ if (featuresArg) {
}
execSync(`napi build --platform --js false ${featuresArg}`, { stdio: 'inherit', env: process.env });
/* Mac App Store build with sandboxing - Does this belong here?
const target = process.env.npm_config_target || '';
const featuresArg = target.includes('mas') ? '--features sandbox' : '';
execSync(`napi build --platform --js false ${featuresArg}`, { stdio: 'inherit', env: process.env });
*/

View File

@@ -1184,6 +1184,8 @@ pub mod chromium_importer {
#[napi]
pub fn request_browser_access(browser: String) -> napi::Result<()> {
println!("request_browser_access() was called from napi");
#[cfg(all(target_os = "macos", feature = "sandbox"))]
{
chromium_importer::chromium::request_browser_access(&browser)

View File

@@ -2,8 +2,6 @@
#import "browser_access.h"
#import "../utils.h"
// Import the Swift-generated header
// The name matches the module-name in build.rs: "Bitwarden"
#import "Bitwarden-Swift.h"
static BrowserAccessManager* sharedManager = nil;

View File

@@ -9,6 +9,7 @@ export class ChromiumImporterService {
return await chromium_importer.getMetadata();
});
// 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) => {
console.log("[IPC] requestBrowserAccess handler called for:", browser);
console.log("[IPC] chromium_importer keys:", Object.keys(chromium_importer));
@@ -21,7 +22,6 @@ export class ChromiumImporterService {
console.log("[IPC] Calling native requestBrowserAccess");
return await chromium_importer.requestBrowserAccess(browser);
}
// No-op if not compiled with sandbox support
console.log("[IPC] requestBrowserAccess not found, returning no-op");
return;
});

View File

@@ -40,7 +40,6 @@ export class ImportDesktopComponent {
protected disabled = false;
protected loading = false;
// Bind callbacks in constructor to maintain reference equality
protected readonly onLoadProfilesFromBrowser = this._onLoadProfilesFromBrowser.bind(this);
protected readonly onImportFromBrowser = this._onImportFromBrowser.bind(this);

View File

@@ -5,7 +5,7 @@ import type { chromium_importer } from "@bitwarden/desktop-napi";
const chromiumImporter = {
getMetadata: (): Promise<Record<string, chromium_importer.NativeImporterMetadata>> =>
ipcRenderer.invoke("chromium_importer.getMetadata"),
// Request browser access for sandboxed builds (no-op in non-sandboxed builds)
// Request browser access for Mac OS App Store (sandboxed) builds (no-op in non-sandboxed builds)
requestBrowserAccess: (browser: string): Promise<void> =>
ipcRenderer.invoke("chromium_importer.requestBrowserAccess", browser),
getAvailableProfiles: (browser: string): Promise<chromium_importer.ProfileInfo[]> =>

View File

@@ -120,7 +120,6 @@ export class ImportChromeComponent implements OnInit, OnDestroy {
);
} catch (error) {
this.logService.error("Error loading profiles from browser:", error);
// FIXME: Add error handling and display when profiles could not be loaded/retrieved
}
}
});