feat: include local wheels in WASM exports#10071
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
2 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/core/wasm/bridge.ts">
<violation number="1" location="frontend/src/core/wasm/bridge.ts:162">
P1: `wasmWheelUrls` entries are only validated as plain strings in `mount.tsx`, so malformed or empty URL values can reach `bridge.ts`. In `startSession()`, `new URL(url, document.baseURI)` is called for each entry without any guard, and `new URL()` throws a `TypeError` on invalid input. Because `startSession()` is async and the `ready` listener calls it without `await` or `.catch()`, a single bad wheel URL produces an unhandled rejection that aborts the entire WASM session startup flow before the RPC call is even reached.
Consider wrapping the URL parsing so invalid entries are logged and filtered out rather than crashing initialization. For example, using `flatMap` with a `try/catch` block would let the session start with the valid URLs while surfacing the bad ones via `Logger.warn`.</violation>
</file>
<file name="frontend/src/core/wasm/worker/bootstrap.ts">
<violation number="1" location="frontend/src/core/wasm/worker/bootstrap.ts:135">
P1: Wheel URLs from `wasmWheelUrlsAtom` are installed automatically at session startup without validation of scheme or origin. Because the values come from notebook export options (only checked as a list of strings), a malicious or tampered export could force the browser to fetch and execute arbitrary remote wheels before the user interacts with the notebook. Since the PR is scoped to local wheels, consider restricting URLs to same-origin or blob origins, or adding an explicit allowlist, before passing them to `micropip.install`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const wheelUrls = store | ||
| .get(wasmWheelUrlsAtom) | ||
| .map((url) => new URL(url, document.baseURI).toString()); |
There was a problem hiding this comment.
P1: wasmWheelUrls entries are only validated as plain strings in mount.tsx, so malformed or empty URL values can reach bridge.ts. In startSession(), new URL(url, document.baseURI) is called for each entry without any guard, and new URL() throws a TypeError on invalid input. Because startSession() is async and the ready listener calls it without await or .catch(), a single bad wheel URL produces an unhandled rejection that aborts the entire WASM session startup flow before the RPC call is even reached.
Consider wrapping the URL parsing so invalid entries are logged and filtered out rather than crashing initialization. For example, using flatMap with a try/catch block would let the session start with the valid URLs while surfacing the bad ones via Logger.warn.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/core/wasm/bridge.ts, line 162:
<comment>`wasmWheelUrls` entries are only validated as plain strings in `mount.tsx`, so malformed or empty URL values can reach `bridge.ts`. In `startSession()`, `new URL(url, document.baseURI)` is called for each entry without any guard, and `new URL()` throws a `TypeError` on invalid input. Because `startSession()` is async and the `ready` listener calls it without `await` or `.catch()`, a single bad wheel URL produces an unhandled rejection that aborts the entire WASM session startup flow before the RPC call is even reached.
Consider wrapping the URL parsing so invalid entries are logged and filtered out rather than crashing initialization. For example, using `flatMap` with a `try/catch` block would let the session start with the valid URLs while surfacing the bad ones via `Logger.warn`.</comment>
<file context>
@@ -155,6 +159,9 @@ export class PyodideBridge implements RunRequests, EditRequests {
const fallbackCode = await fallbackFileStore.readFile();
const filename = store.get(filenameAtom) ?? PyodideRouter.getFilename();
const userConfig = store.get(userConfigAtom);
+ const wheelUrls = store
+ .get(wasmWheelUrlsAtom)
+ .map((url) => new URL(url, document.baseURI).toString());
</file context>
| const wheelUrls = store | |
| .get(wasmWheelUrlsAtom) | |
| .map((url) => new URL(url, document.baseURI).toString()); | |
| const wheelUrls = store | |
| .get(wasmWheelUrlsAtom) | |
| .flatMap((url) => { | |
| try { | |
| return new URL(url, document.baseURI).toString(); | |
| } catch { | |
| Logger.warn(`Invalid wheel URL: ${url}`); | |
| return []; | |
| } | |
| }); |
| self.query_params = queryParameters; | ||
| self.user_config = userConfig; | ||
|
|
||
| await this.installIncludedWheels(wheelUrls); |
There was a problem hiding this comment.
P1: Wheel URLs from wasmWheelUrlsAtom are installed automatically at session startup without validation of scheme or origin. Because the values come from notebook export options (only checked as a list of strings), a malicious or tampered export could force the browser to fetch and execute arbitrary remote wheels before the user interacts with the notebook. Since the PR is scoped to local wheels, consider restricting URLs to same-origin or blob origins, or adding an explicit allowlist, before passing them to micropip.install.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/core/wasm/worker/bootstrap.ts, line 135:
<comment>Wheel URLs from `wasmWheelUrlsAtom` are installed automatically at session startup without validation of scheme or origin. Because the values come from notebook export options (only checked as a list of strings), a malicious or tampered export could force the browser to fetch and execute arbitrary remote wheels before the user interacts with the notebook. Since the PR is scoped to local wheels, consider restricting URLs to same-origin or blob origins, or adding an explicit allowlist, before passing them to `micropip.install`.</comment>
<file context>
@@ -124,6 +132,8 @@ export class DefaultWasmController implements WasmController {
self.query_params = queryParameters;
self.user_config = userConfig;
+ await this.installIncludedWheels(wheelUrls);
+
const span = t.startSpan("startSession.runPython");
</file context>
wip
Related to #5488