mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(browser): name the requesting frame and the permission in denial notices (#15542)
* fix(browser): name the requesting frame and the permission in denial notices Two defects in the same notice, both found by the review of #15481 and left out of it deliberately. The notice named the wrong site. setPermissionRequestHandler passed webContents.getURL(), which is the top-level document, so a permission request from a cross-origin sub-frame was attributed to the embedder. Every PermissionRequest variant carries requestingUrl, so all three call sites now use it and fall back to the top-level document only when it is absent. The notice also showed raw Chromium permission names. humanizePermission mapped two permissions and returned the raw token for the rest. That now matters more: #15481 granted ordinary storage-access and left top-level-storage-access denied, making it the storage denial a user can still hit - rendered as its raw token. The default still returns the raw token. Inventing prose for a permission nobody has seen is worse than showing its real name. Does not change any permission verdict, and does not fix Google sign-in (#15221). * 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)
|
||||
@@ -39,6 +54,8 @@ export function installBrowserSessionPartitionPolicies(profile: BrowserSessionPr
|
||||
sess.setPermissionRequestHandler((webContents, permission, callback, details) => {
|
||||
// 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(
|
||||
@@ -47,7 +64,7 @@ export function installBrowserSessionPartitionPolicies(profile: BrowserSessionPr
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl: webContents.getURL()
|
||||
rawUrl
|
||||
})
|
||||
}
|
||||
callback(granted)
|
||||
@@ -57,7 +74,7 @@ export function installBrowserSessionPartitionPolicies(profile: BrowserSessionPr
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl: webContents.getURL()
|
||||
rawUrl
|
||||
})
|
||||
callback(false)
|
||||
}
|
||||
@@ -66,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: webContents.getURL()
|
||||
rawUrl
|
||||
})
|
||||
}
|
||||
callback(allowed)
|
||||
|
||||
@@ -564,6 +564,58 @@ describe('BrowserSessionRegistry persistence', () => {
|
||||
permission: 'geolocation',
|
||||
rawUrl: 'https://example.com/account'
|
||||
})
|
||||
|
||||
// A subframe denial must name the requester, not its top-level embedder.
|
||||
browserManagerNotifyPermissionDeniedMock.mockClear()
|
||||
requestHandler(guestWc, 'geolocation', permissionCallback, {
|
||||
requestingUrl: 'https://widget.example.net/embed',
|
||||
isMainFrame: false
|
||||
})
|
||||
await vi.waitFor(() =>
|
||||
expect(browserManagerNotifyPermissionDeniedMock).toHaveBeenCalledWith({
|
||||
guestWebContentsId: 401,
|
||||
permission: 'geolocation',
|
||||
rawUrl: 'https://widget.example.net/embed'
|
||||
})
|
||||
)
|
||||
|
||||
// Missing or empty frame URLs fall back to the visible top-level page.
|
||||
browserManagerNotifyPermissionDeniedMock.mockClear()
|
||||
requestHandler(guestWc, 'geolocation', permissionCallback, { isMainFrame: true })
|
||||
await vi.waitFor(() =>
|
||||
expect(browserManagerNotifyPermissionDeniedMock).toHaveBeenCalledWith({
|
||||
guestWebContentsId: 401,
|
||||
permission: 'geolocation',
|
||||
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 +787,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,6 +19,61 @@ 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.')
|
||||
})
|
||||
|
||||
it('names the storage permission in words rather than its raw token', () => {
|
||||
const notice = formatPermissionNotice({
|
||||
browserPageId: 'browser-1',
|
||||
permission: 'top-level-storage-access',
|
||||
origin: 'https://example.com'
|
||||
})
|
||||
expect(notice).not.toContain('top-level-storage-access')
|
||||
expect(notice).toBe(
|
||||
'https://example.com asked for cookie access on behalf of an embedded site, and Orca denied 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({
|
||||
browserPageId: 'browser-1',
|
||||
permission: 'some-future-permission',
|
||||
origin: 'https://example.com'
|
||||
})
|
||||
).toBe('https://example.com asked for some-future-permission, and Orca denied it.')
|
||||
})
|
||||
|
||||
it('formats popup outcomes', () => {
|
||||
|
||||
@@ -15,12 +15,45 @@ export type LoadFailureMeta = {
|
||||
|
||||
type BrowserLoadErrorLike = BrowserLoadError | null
|
||||
|
||||
// Unknown Chromium permissions keep their raw name instead of disappearing behind invented copy.
|
||||
function humanizePermission(permission: string): string {
|
||||
switch (permission) {
|
||||
case 'media':
|
||||
return 'camera or microphone access'
|
||||
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 'cookie access on behalf of an embedded site'
|
||||
case 'geolocation':
|
||||
return 'your location'
|
||||
case 'idle-detection':
|
||||
return 'permission to detect when you are idle'
|
||||
case 'display-capture':
|
||||
return 'permission to capture your screen'
|
||||
case 'window-management':
|
||||
return 'screen information and multi-screen window placement'
|
||||
case 'keyboardLock':
|
||||
return 'permission to capture keyboard input'
|
||||
case 'openExternal':
|
||||
return 'permission to open a link outside Orca'
|
||||
case 'fileSystem':
|
||||
return 'access to your files or folders'
|
||||
case 'hid':
|
||||
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':
|
||||
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 'permission to choose an audio output device'
|
||||
default:
|
||||
return permission
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user