mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 00:03:52 +00:00
fix(browser): keep permission denial attribution accurate
Capture fallback URLs before asynchronous media permission handling and treat opaque requesters as unknown rather than blaming the top-level page. Clarify permission descriptions and cover origin normalization, navigation races, and mapped copy.
This commit is contained in:
@@ -123,6 +123,16 @@ describe('browserManager', () => {
|
||||
permission: 'media',
|
||||
origin: 'https://example.com'
|
||||
})
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: guest.id,
|
||||
permission: 'geolocation',
|
||||
rawUrl: ''
|
||||
})
|
||||
expect(rendererSendMock).toHaveBeenCalledWith('browser:permission-denied', {
|
||||
browserPageId: 'browser-1',
|
||||
permission: 'geolocation',
|
||||
origin: 'unknown'
|
||||
})
|
||||
expect(rendererSendMock).toHaveBeenCalledWith(
|
||||
'browser:download-requested',
|
||||
expect.objectContaining({
|
||||
|
||||
@@ -22,6 +22,21 @@ const handleWillDownload = (
|
||||
browserManager.handleGuestWillDownload({ guestWebContentsId: webContents.id, item })
|
||||
}
|
||||
|
||||
function resolvePermissionNoticeUrl(
|
||||
webContents: Electron.WebContents,
|
||||
details: Electron.PermissionRequest | undefined
|
||||
): string {
|
||||
const requestingUrl = details?.requestingUrl
|
||||
if (!requestingUrl) {
|
||||
return webContents.getURL()
|
||||
}
|
||||
try {
|
||||
return new URL(requestingUrl).origin === 'null' ? '' : requestingUrl
|
||||
} catch {
|
||||
return ''
|
||||
}
|
||||
}
|
||||
|
||||
export function installBrowserSessionPartitionPolicies(profile: BrowserSessionProfile): void {
|
||||
const { partition } = profile
|
||||
const sess = session.fromPartition(partition)
|
||||
@@ -37,13 +52,10 @@ export function installBrowserSessionPartitionPolicies(profile: BrowserSessionPr
|
||||
setupClientHintsOverride(sess, cleanUA)
|
||||
}
|
||||
sess.setPermissionRequestHandler((webContents, permission, callback, details) => {
|
||||
// Why: a permission request can come from a cross-origin sub-frame, and getURL() reports the
|
||||
// TOP-LEVEL document. Attributing a sub-frame's denial to the embedder tells the user the wrong
|
||||
// site asked. Every PermissionRequest variant carries requestingUrl; fall back to the top-level
|
||||
// document only when it is missing.
|
||||
const requestingUrl = (details as Electron.PermissionRequest | undefined)?.requestingUrl
|
||||
// Why: defer media to macOS TCC; denying at the session layer throws NotAllowedError even after the user granted Camera/Mic to the OS.
|
||||
if (permission === 'media') {
|
||||
// Capture before async handling; opaque frames cannot be attributed to a named site.
|
||||
const rawUrl = resolvePermissionNoticeUrl(webContents, details)
|
||||
void requestSystemMediaAccess(
|
||||
details as Electron.MediaAccessPermissionRequest | undefined
|
||||
).then(
|
||||
@@ -52,7 +64,7 @@ export function installBrowserSessionPartitionPolicies(profile: BrowserSessionPr
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl: requestingUrl || webContents.getURL()
|
||||
rawUrl
|
||||
})
|
||||
}
|
||||
callback(granted)
|
||||
@@ -62,7 +74,7 @@ export function installBrowserSessionPartitionPolicies(profile: BrowserSessionPr
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl: requestingUrl || webContents.getURL()
|
||||
rawUrl
|
||||
})
|
||||
callback(false)
|
||||
}
|
||||
@@ -71,10 +83,11 @@ export function installBrowserSessionPartitionPolicies(profile: BrowserSessionPr
|
||||
}
|
||||
const allowed = isAutoGrantedBrowserSessionPermission(permission)
|
||||
if (!allowed) {
|
||||
const rawUrl = resolvePermissionNoticeUrl(webContents, details)
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl: requestingUrl || webContents.getURL()
|
||||
rawUrl
|
||||
})
|
||||
}
|
||||
callback(allowed)
|
||||
|
||||
@@ -538,8 +538,7 @@ describe('BrowserSessionRegistry persistence', () => {
|
||||
rawUrl: 'https://example.com/account'
|
||||
})
|
||||
|
||||
// Why: a cross-origin sub-frame can request a permission, and getURL() reports the TOP-LEVEL
|
||||
// document. Attributing the denial to the embedder tells the user the wrong site asked.
|
||||
// A subframe denial must name the requester, not its top-level embedder.
|
||||
browserManagerNotifyPermissionDeniedMock.mockClear()
|
||||
requestHandler(guestWc, 'geolocation', permissionCallback, {
|
||||
requestingUrl: 'https://widget.example.net/embed',
|
||||
@@ -553,8 +552,7 @@ describe('BrowserSessionRegistry persistence', () => {
|
||||
})
|
||||
)
|
||||
|
||||
// Why: requestingUrl is absent on some request shapes; the top-level document is the fallback,
|
||||
// not a crash or an empty origin.
|
||||
// Missing or empty frame URLs fall back to the visible top-level page.
|
||||
browserManagerNotifyPermissionDeniedMock.mockClear()
|
||||
requestHandler(guestWc, 'geolocation', permissionCallback, { isMainFrame: true })
|
||||
await vi.waitFor(() =>
|
||||
@@ -564,6 +562,33 @@ describe('BrowserSessionRegistry persistence', () => {
|
||||
rawUrl: 'https://example.com/account'
|
||||
})
|
||||
)
|
||||
|
||||
browserManagerNotifyPermissionDeniedMock.mockClear()
|
||||
requestHandler(guestWc, 'geolocation', permissionCallback, {
|
||||
requestingUrl: '',
|
||||
isMainFrame: false
|
||||
})
|
||||
await vi.waitFor(() =>
|
||||
expect(browserManagerNotifyPermissionDeniedMock).toHaveBeenCalledWith({
|
||||
guestWebContentsId: 401,
|
||||
permission: 'geolocation',
|
||||
rawUrl: 'https://example.com/account'
|
||||
})
|
||||
)
|
||||
|
||||
// Opaque frame URLs have no site Orca can name accurately.
|
||||
browserManagerNotifyPermissionDeniedMock.mockClear()
|
||||
requestHandler(guestWc, 'geolocation', permissionCallback, {
|
||||
requestingUrl: 'about:blank',
|
||||
isMainFrame: false
|
||||
})
|
||||
await vi.waitFor(() =>
|
||||
expect(browserManagerNotifyPermissionDeniedMock).toHaveBeenCalledWith({
|
||||
guestWebContentsId: 401,
|
||||
permission: 'geolocation',
|
||||
rawUrl: ''
|
||||
})
|
||||
)
|
||||
expect(
|
||||
browserManagerNotifyPermissionDeniedMock.mock.calls.map(([args]) => args.permission)
|
||||
).toEqual(['geolocation'])
|
||||
@@ -735,6 +760,7 @@ describe('BrowserSessionRegistry persistence', () => {
|
||||
const callback = vi.fn()
|
||||
|
||||
requestHandler(guestWc, 'media', callback, { mediaTypes: ['video'] })
|
||||
guestWc.getURL.mockReturnValue('https://example.com/after-navigation')
|
||||
|
||||
await vi.waitFor(() => expect(callback).toHaveBeenCalledWith(false))
|
||||
expect(browserManagerNotifyPermissionDeniedMock).toHaveBeenCalledWith({
|
||||
|
||||
@@ -19,10 +19,15 @@ describe('browser notice formatting', () => {
|
||||
origin: 'https://example.com'
|
||||
})
|
||||
).toBe('https://example.com asked for camera or microphone access, and Orca denied it.')
|
||||
expect(
|
||||
formatPermissionNotice({
|
||||
browserPageId: 'browser-1',
|
||||
permission: 'geolocation',
|
||||
origin: 'unknown'
|
||||
})
|
||||
).toBe('this page asked for your location, and Orca denied it.')
|
||||
})
|
||||
|
||||
// Why: with ordinary storage-access now granted, top-level-storage-access is the storage denial a
|
||||
// user can still hit, and it was rendering as its raw Chromium token.
|
||||
it('names the storage permission in words rather than its raw token', () => {
|
||||
const notice = formatPermissionNotice({
|
||||
browserPageId: 'browser-1',
|
||||
@@ -31,12 +36,36 @@ describe('browser notice formatting', () => {
|
||||
})
|
||||
expect(notice).not.toContain('top-level-storage-access')
|
||||
expect(notice).toBe(
|
||||
'https://example.com asked for access to its own cookies and storage on this site, and Orca denied it.'
|
||||
'https://example.com asked for cookie access on behalf of an embedded site, and Orca denied it.'
|
||||
)
|
||||
})
|
||||
|
||||
// Why: an unrecognised permission must still name itself rather than render as empty or as prose
|
||||
// invented for it.
|
||||
it.each([
|
||||
['storage-access', 'access to its own cookies and storage while embedded on this page'],
|
||||
['idle-detection', 'permission to detect when you are idle'],
|
||||
['display-capture', 'permission to capture your screen'],
|
||||
['window-management', 'screen information and multi-screen window placement'],
|
||||
['keyboardLock', 'permission to capture keyboard input'],
|
||||
['openExternal', 'permission to open a link outside Orca'],
|
||||
['fileSystem', 'access to your files or folders'],
|
||||
['hid', 'access to a connected human interface device'],
|
||||
['usb', 'access to a USB device'],
|
||||
['serial', 'access to a serial device'],
|
||||
['midi', 'access to your MIDI devices'],
|
||||
['midiSysex', 'access to system-exclusive MIDI messages'],
|
||||
['mediaKeySystem', 'access to protected media playback'],
|
||||
['speaker-selection', 'permission to choose an audio output device']
|
||||
])('formats the %s permission as readable copy', (permission, description) => {
|
||||
expect(
|
||||
formatPermissionNotice({
|
||||
browserPageId: 'browser-1',
|
||||
permission,
|
||||
origin: 'https://example.com'
|
||||
})
|
||||
).toBe(`https://example.com asked for ${description}, and Orca denied it.`)
|
||||
})
|
||||
|
||||
// Pin the raw-token fallback for permissions Chromium adds later.
|
||||
it('falls back to the raw permission name for anything unmapped', () => {
|
||||
expect(
|
||||
formatPermissionNotice({
|
||||
|
||||
@@ -15,10 +15,7 @@ export type LoadFailureMeta = {
|
||||
|
||||
type BrowserLoadErrorLike = BrowserLoadError | null
|
||||
|
||||
// Why: the raw Chromium permission name is what the user sees in the denial notice, and most of
|
||||
// them do not read as English - "top-level-storage-access" is the one users hit most now that
|
||||
// ordinary storage-access is granted. The default still returns the raw token, because inventing
|
||||
// prose for a permission nobody has seen yet is worse than showing its real name.
|
||||
// Unknown Chromium permissions keep their raw name instead of disappearing behind invented copy.
|
||||
function humanizePermission(permission: string): string {
|
||||
switch (permission) {
|
||||
case 'media':
|
||||
@@ -26,33 +23,37 @@ function humanizePermission(permission: string): string {
|
||||
case 'pointerLock':
|
||||
return 'pointer lock'
|
||||
case 'storage-access':
|
||||
return 'access to its own cookies and storage while embedded on this page'
|
||||
case 'top-level-storage-access':
|
||||
return 'access to its own cookies and storage on this site'
|
||||
return 'cookie access on behalf of an embedded site'
|
||||
case 'geolocation':
|
||||
return 'your location'
|
||||
case 'idle-detection':
|
||||
return 'to detect when you are idle'
|
||||
return 'permission to detect when you are idle'
|
||||
case 'display-capture':
|
||||
return 'to capture your screen'
|
||||
return 'permission to capture your screen'
|
||||
case 'window-management':
|
||||
return 'to manage windows on your screen'
|
||||
return 'screen information and multi-screen window placement'
|
||||
case 'keyboardLock':
|
||||
return 'to capture all keyboard input'
|
||||
return 'permission to capture keyboard input'
|
||||
case 'openExternal':
|
||||
return 'to open an external app'
|
||||
return 'permission to open a link outside Orca'
|
||||
case 'fileSystem':
|
||||
return 'access to your files'
|
||||
return 'access to your files or folders'
|
||||
case 'hid':
|
||||
return 'access to a USB input device'
|
||||
return 'access to a connected human interface device'
|
||||
case 'usb':
|
||||
return 'access to a USB device'
|
||||
case 'serial':
|
||||
return 'access to a serial device'
|
||||
case 'midi':
|
||||
case 'midiSysex':
|
||||
return 'access to your MIDI devices'
|
||||
case 'midiSysex':
|
||||
return 'access to system-exclusive MIDI messages'
|
||||
case 'mediaKeySystem':
|
||||
return 'access to protected media playback'
|
||||
case 'speaker-selection':
|
||||
return 'to choose an audio output device'
|
||||
return 'permission to choose an audio output device'
|
||||
default:
|
||||
return permission
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user