Skip to content

Commit a8d6620

Browse files
Make blocked navigation the default for views nobody claimed (#1093)
Only the lab view guards its navigations today. The welcome view, the title bar, the dialogs and the session window itself load a data: document and nothing stops one of them being navigated away from it, which matters most on the welcome view, since that is the surface that renders the news feed. Deny navigation and window creation for any webContents that has not claimed a policy of its own, and refuse webview attachment everywhere. The check runs when a navigation happens rather than when the webContents is created, because creation fires before its owner has had a chance to claim it. The views that render a bundled document claim the app-owned policy, which keeps them on their document and hands a link the user follows to the system browser. The lab view and the server connection window claim themselves and keep deciding as they do now, one per origin and the other by following a login wherever it goes. Doyensec's rule for this is named LIMIT_NAVIGATION_GLOBAL_CHECK, that is, limits at application level rather than per window, and Electron's checklist items 13 and 14 read the same way. Signal denies window creation globally and blocks webview attachment; VS Code prevents will-navigate for every webContents with an exception for its browser views. Closes #1088. Audited every anchor in the bundled views first: ten are href="#" and two are javascript:void(0), none navigates, and none of those views calls window.open, so nothing legitimate is being taken away. Unit tests cover both sides of the claim check and the scheme allowlist, verified by mutation: making the fallback ignore claims fails the claimed-view test, dropping the allowlist fails three. Build, lint, prettier and the full suite pass, and the app starts with no blocked navigation logged. Done with Claude Code. Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>
1 parent 1026da5 commit a8d6620

11 files changed

Lines changed: 275 additions & 0 deletions

File tree

src/main/app.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
setupJlabCLICommandWithElevatedRights,
3030
waitForDuration
3131
} from './utils';
32+
import { installGlobalNavigationGuard } from './navigationguard';
3233
import { IServerFactory, JupyterServerFactory } from './server';
3334
import { connectAndGetServerInfo, IJupyterServerInfo } from './connect';
3435
import { UpdateDialog } from './updatedialog/updatedialog';
@@ -279,6 +280,7 @@ export class JupyterApplication implements IApplication, IDisposable {
279280
this._serverFactory.createFreeServer().catch(error => {
280281
console.error('Failed to create free server', error);
281282
});
283+
installGlobalNavigationGuard();
282284
this._registerListeners();
283285

284286
if (

src/main/connect.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { BrowserWindow, Cookie } from 'electron';
55
import { clearSession } from './utils';
6+
import { markGuarded } from './navigationguard';
67

78
export let connectWindow: BrowserWindow;
89

@@ -55,6 +56,8 @@ export async function connectAndGetServerInfo(
5556
}
5657

5758
const window = new BrowserWindow(browserOptions);
59+
// this window exists to follow a login wherever the server sends it
60+
markGuarded(window.webContents);
5861

5962
const timeout = options?.timeout || 30000;
6063

src/main/dialog/themedview.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { WebContentsView } from 'electron';
55
import * as fs from 'fs';
66
import * as path from 'path';
77
import { DarkThemeBGColor, LightThemeBGColor } from '../utils';
8+
import { guardAppOwnedView } from '../navigationguard';
89

910
export class ThemedView {
1011
constructor(options: ThemedView.IOptions) {
@@ -14,6 +15,7 @@ export class ThemedView {
1415
preload: options.preload || path.join(__dirname, './preload.js')
1516
}
1617
});
18+
guardAppOwnedView(this._view.webContents);
1719
this._view.setBackgroundColor(
1820
this._isDarkTheme ? DarkThemeBGColor : LightThemeBGColor
1921
);

src/main/dialog/themedwindow.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { BrowserWindow } from 'electron';
55
import * as fs from 'fs';
66
import * as path from 'path';
77
import { DarkThemeBGColor, LightThemeBGColor } from '../utils';
8+
import { guardAppOwnedView } from '../navigationguard';
89

910
export class ThemedWindow {
1011
constructor(options: ThemedWindow.IOptions) {
@@ -27,6 +28,8 @@ export class ThemedWindow {
2728
}
2829
});
2930

31+
guardAppOwnedView(this._window.webContents);
32+
3033
// hide the traffic lights
3134
if (process.platform === 'darwin') {
3235
this._window.setWindowButtonVisibility(false);

src/main/labview/labview.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
isSameServerOrigin,
2020
LightThemeBGColor
2121
} from '../utils';
22+
import { markGuarded } from '../navigationguard';
2223
import { SessionWindow } from '../sessionwindow/sessionwindow';
2324
import {
2425
CtrlWBehavior,
@@ -64,6 +65,8 @@ export class LabView implements IDisposable {
6465
partition
6566
}
6667
});
68+
// this view decides per origin in _registerNavigationGuard
69+
markGuarded(this._view.webContents);
6770

6871
this._view.setBackgroundColor(
6972
options.isDarkTheme ? DarkThemeBGColor : LightThemeBGColor

src/main/navigationguard.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright (c) Jupyter Development Team.
2+
// Distributed under the terms of the Modified BSD License.
3+
4+
import { app, shell, WebContents } from 'electron';
5+
import log from 'electron-log';
6+
7+
// http/https for ordinary links, mailto for contact links
8+
const EXTERNAL_SCHEMES = ['https:', 'http:', 'mailto:'];
9+
10+
const guarded = new WeakSet<WebContents>();
11+
12+
/**
13+
* Declare that this webContents carries its own navigation policy, so the
14+
* application-wide guard leaves its navigations alone. The lab view decides per
15+
* origin, and the server connection window has to follow a login wherever it
16+
* goes.
17+
*/
18+
export function markGuarded(contents: WebContents): void {
19+
guarded.add(contents);
20+
}
21+
22+
export function openUrlInSystemBrowser(url: string): void {
23+
try {
24+
const { protocol, href } = new URL(url);
25+
if (EXTERNAL_SCHEMES.includes(protocol)) {
26+
shell.openExternal(href);
27+
}
28+
} catch {
29+
// unparseable target, nothing safe to open
30+
}
31+
}
32+
33+
/**
34+
* Pin a view that renders a bundled document. Those views are built from a
35+
* data: URL and are never meant to navigate: keeping them on their own document
36+
* means a link in content they render, the news feed on the welcome page for
37+
* instance, cannot replace app chrome with a page from the network.
38+
*/
39+
export function guardAppOwnedView(contents: WebContents): void {
40+
markGuarded(contents);
41+
42+
const sendToBrowser = (event: Electron.Event, url: string) => {
43+
event.preventDefault();
44+
openUrlInSystemBrowser(url);
45+
};
46+
47+
contents.on('will-navigate', sendToBrowser);
48+
contents.on('will-redirect', sendToBrowser);
49+
contents.setWindowOpenHandler(({ url }) => {
50+
openUrlInSystemBrowser(url);
51+
return { action: 'deny' };
52+
});
53+
}
54+
55+
/**
56+
* Deny navigation for any webContents nobody claimed. Views are added over
57+
* time and the safe default is that a new one cannot be navigated away from its
58+
* document until someone decides what its policy should be. The check runs when
59+
* a navigation happens rather than when the webContents is created, because
60+
* creation fires before the owner has had a chance to claim it.
61+
*/
62+
export function installGlobalNavigationGuard(): void {
63+
app.on('web-contents-created', (_event, contents) => {
64+
contents.on('will-attach-webview', event => {
65+
event.preventDefault();
66+
});
67+
68+
const denyUnclaimed = (event: Electron.Event, url: string) => {
69+
if (guarded.has(contents)) {
70+
return;
71+
}
72+
event.preventDefault();
73+
log.warn(`Blocked navigation to ${url} in an unguarded view`);
74+
};
75+
76+
contents.on('will-navigate', denyUnclaimed);
77+
contents.on('will-redirect', denyUnclaimed);
78+
79+
// an owner that sets its own handler replaces this one, which is what
80+
// claiming a view looks like for window creation
81+
contents.setWindowOpenHandler(({ url }) => {
82+
log.warn(`Blocked window opening ${url} from an unguarded view`);
83+
return { action: 'deny' };
84+
});
85+
});
86+
}

src/main/sessionwindow/sessionwindow.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
WorkspaceSettings
2323
} from '../config/settings';
2424
import { TitleBarView } from '../titlebarview/titlebarview';
25+
import { guardAppOwnedView } from '../navigationguard';
2526
import {
2627
DarkThemeBGColor,
2728
envPathForPythonPath,
@@ -134,6 +135,8 @@ export class SessionWindow implements IDisposable {
134135
}
135136
});
136137

138+
guardAppOwnedView(this._window.webContents);
139+
137140
this._window.setMenuBarVisibility(false);
138141
this._window.show();
139142

src/main/titlebarview/titlebarview.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as path from 'path';
66
import * as fs from 'fs';
77
import * as ejs from 'ejs';
88
import { DarkThemeBGColor, LightThemeBGColor } from '../utils';
9+
import { guardAppOwnedView } from '../navigationguard';
910
import { EventTypeRenderer } from '../eventtypes';
1011

1112
export class TitleBarView {
@@ -18,6 +19,8 @@ export class TitleBarView {
1819
}
1920
});
2021

22+
guardAppOwnedView(this._view.webContents);
23+
2124
this._view.setBackgroundColor(
2225
this._isDarkTheme ? DarkThemeBGColor : LightThemeBGColor
2326
);

src/main/welcomeview/welcomeview.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { net, WebContentsView } from 'electron';
55
import { DarkThemeBGColor, getUserHomeDir, LightThemeBGColor } from '../utils';
6+
import { guardAppOwnedView } from '../navigationguard';
67
import * as path from 'path';
78
import * as fs from 'fs';
89
import { parseNewsFeed } from './newsfeed';
@@ -31,6 +32,8 @@ export class WelcomeView {
3132
}
3233
});
3334

35+
guardAppOwnedView(this._view.webContents);
36+
3437
this._view.setBackgroundColor(
3538
this._isDarkTheme ? DarkThemeBGColor : LightThemeBGColor
3639
);

test/setup/electron-stub.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { join } from 'path';
1010
const userDataPath = join(tmpdir(), 'jlab-test-userdata');
1111

1212
export const app = {
13+
on: vi.fn(),
1314
getPath: vi.fn((name: string) => join(userDataPath, name)),
1415
getVersion: vi.fn(() => '4.4.7'),
1516
getName: vi.fn(() => 'JupyterLab'),

0 commit comments

Comments
 (0)