bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00
Code Issues Releases Wiki Activity

eval Claude's comments and address legitimate findings

Browse Source
This commit is contained in:
John Harrington
2025-12-02 15:19:22 -07:00
parent 084a291929
commit dc9b05b620
5 changed files with 16 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
apps/desktop/desktop_native/chromium_importer/src/chromium/platform/macos.rs
View File

@@ -63,7 +63,7 @@ pub mod sandbox {
let path_ptr = unsafe { startBrowserAccess(c_name.as_ptr()) };
if path_ptr.is_null() {
return Err(anyhow!(
"Failed to use browser existing security access, it may be stale"
"Existing security scoped access has become stale"
));
}
unsafe { libc::free(path_ptr as *mut libc::c_void) };

30
apps/desktop/desktop_native/objc/README.md
View File

@@ -1,30 +0,0 @@
# File Scoped Security Access on Mac App Store Builds
## Accessing filesystem entries outside of the app sandbox
### TLDR
#### Build & run locally using `npm run electron:sandbox`
#### `build-desktop.yml` section `macos-package-mas` was modified to support this in CI builds:
```
- 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
```
### The Rust code uses a feature named `sandbox` that is used to conditionally compile almost all of the logic introduced in this PR: `#[cfg(all(target_os = "macos", feature = "sandbox"))]`
### How it works
- `apps/desktop/src/app/tools/import/chromium-importer.service.ts` serves as the entrypoint to this logic. __Regardless of the `SANDBOX_BUILD` env var__, this service will call `request_browser_access()`
in `apps/desktop/desktop_native/napi/src/lib.rs` which returns an `Ok` unit variant (no-op) when the `sandbox` feature is not enabled. Outside of sandbox builds, `ChromiumImporterService` will avoid any of the logic added in this PR and continue the chromium importer workflow exactly as before.
- When the `sandbox` feature is enabled the call chain advances to `apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs` where another conditionally compiled (cfg gated) `request_browser_access()` function is defined. This function is used to advance the call chain into the platform specific (Mac OS) Rust code inside `apps/desktop/desktop_native/chromium_importer/src/chromium/platform/macos.rs`.
- When the `sandbox` feature is enabled, a series of cfg gated functions are used in this file to interface with native Objective-C bridges and Swift code responsible for requesting access to browser specific directories.
- `request_only()` is responsible for requesting access to a given browser's directory AND creating the security scoped bookmark, the mechanism Mac uses to persist these kinds of permissions. It corresponds to `requestAccessToBroswerDir()` in the file `apps/desktop/desktop_native/objc/src/native/chromium_importer/browser_access.swift`. This Swift code will determine the appropriate directory to request access to, based upon the browser name passed as argument, and will then display an `NSOpenPanel` with a message instructing the user to select the browser's directory where profile information is stored.
- `resume()` is responsible for determining if a security scoped bookmark exists and is valid, if so, it will be used to access the browser directory.
- See directory `apps/desktop/desktop_native/objc/src/native/chromium_importer` for the native Objective-C and Swift code. Instead of removing debug output I've commented out `NSLog` statements since they may be useful in the future.

22
apps/desktop/desktop_native/objc/src/native/chromium_importer/browser_access_manager.m
View File

@@ -14,18 +14,14 @@
}
- (NSString *)requestAccessToBrowserDir:(NSString *)browserName relativePath:(NSString *)relativePath {
// NSLog(@"[OBJC] requestAccessToBrowserDir called for: %@", browserName);
if (!relativePath) {
// NSLog(@"[OBJC] Unknown browser: %@", browserName);
return nil;
}
NSURL *homeDir = [[NSFileManager defaultManager] homeDirectoryForCurrentUser];
NSURL *browserPath = [homeDir URLByAppendingPathComponent:relativePath];
// NSLog(@"[OBJC] Browser path: %@", browserPath.path);
// NSOpenPanel must be run on the main thread
__block NSURL *selectedURL = nil;
__block NSModalResponse panelResult = NSModalResponseCancel;
@@ -41,31 +37,22 @@
openPanel.canChooseFiles = NO;
openPanel.directoryURL = browserPath;
// NSLog(@"[OBJC] About to call runModal");
panelResult = [openPanel runModal];
selectedURL = openPanel.URL;
// NSLog(@"[OBJC] runModal returned: %ld", (long)panelResult);
};
if ([NSThread isMainThread]) {
// NSLog(@"[OBJC] Already on main thread");
showPanel();
} else {
// NSLog(@"[OBJC] Dispatching to main queue...");
dispatch_sync(dispatch_get_main_queue(), showPanel);
}
if (panelResult != NSModalResponseOK || !selectedURL) {
// NSLog(@"[OBJC] User cancelled access request or panel failed");
return nil;
}
// NSLog(@"[OBJC] User selected URL: %@", selectedURL.path);
NSURL *localStatePath = [selectedURL URLByAppendingPathComponent:@"Local State"];
if (![[NSFileManager defaultManager] fileExistsAtPath:localStatePath.path]) {
// NSLog(@"[OBJC] Selected folder doesn't appear to be a valid %@ directory", browserName);
NSAlert *alert = [[NSAlert alloc] init];
alert.messageText = @"Invalid Folder";
alert.informativeText = [NSString stringWithFormat:
@@ -85,12 +72,10 @@
error:&error];
if (!bookmarkData) {
// NSLog(@"[OBJC] Failed to create bookmark: %@", error);
return nil;
}
[self saveBookmark:bookmarkData forBrowser:browserName];
// NSLog(@"[OBJC] Successfully created and saved bookmark");
return [bookmarkData base64EncodedStringWithOptions:0];
}
@@ -113,19 +98,16 @@
error:&error];
if (!url) {
// NSLog(@"Failed to resolve bookmark: %@", error);
return nil;
}
if (isStale) {
// NSLog(@"Security bookmark for %@ is stale, attempting to re-create it", browserName);
NSData *newBookmarkData = [url bookmarkDataWithOptions:NSURLBookmarkCreationWithSecurityScope
includingResourceValuesForKeys:nil
relativeToURL:nil
error:&error];
if (!newBookmarkData) {
// NSLog(@"Failed to create bookmark: %@", error);
return nil;
}
@@ -133,7 +115,6 @@
}
if (![url startAccessingSecurityScopedResource]) {
// NSLog(@"Failed to start accessing security-scoped resource");
return nil;
}
@@ -155,7 +136,6 @@
error:&error];
if (!url) {
// NSLog(@"Failed to resolve bookmark for stop: %@", error);
return;
}

14
apps/desktop/src/app/tools/import/import-desktop.component.ts
View File

@@ -2,6 +2,7 @@ import { CommonModule } from "@angular/common";
import { Component } from "@angular/core";
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { DialogRef, AsyncActionsModule, ButtonModule, DialogModule } from "@bitwarden/components";
import type { chromium_importer } from "@bitwarden/desktop-napi";
import { ImportMetadataServiceAbstraction } from "@bitwarden/importer-core";
@@ -42,7 +43,10 @@ export class ImportDesktopComponent {
protected readonly onLoadProfilesFromBrowser = this._onLoadProfilesFromBrowser.bind(this);
protected readonly onImportFromBrowser = this._onImportFromBrowser.bind(this);
constructor(public dialogRef: DialogRef) {}
constructor(
public dialogRef: DialogRef,
private i18nService: I18nService,
) {}
/**
* Callback that is called after a successful import.
@@ -54,8 +58,12 @@ export class ImportDesktopComponent {
private async _onLoadProfilesFromBrowser(
browser: string,
): Promise<chromium_importer.ProfileInfo[]> {
// Request browser access (required for sandboxed builds, no-op otherwise)
await ipc.tools.chromiumImporter.requestBrowserAccess(browser);
try {
// Request browser access (required for sandboxed builds, no-op otherwise)
await ipc.tools.chromiumImporter.requestBrowserAccess(browser);
} catch {
throw new Error(this.i18nService.t("browserAccessDenied"));
}
return ipc.tools.chromiumImporter.getAvailableProfiles(browser);
}

3
apps/desktop/src/locales/en/messages.json
View File

@@ -4237,5 +4237,8 @@
},
"sessionTimeoutHeader": {
"message": "Session timeout"
},
"browserAccessDenied": {
"message": "Either an invalid browser directory was selected, or access was denied"
}
}