feat(electron): Electron-compatible app shell — run existing Electron apps natively#5547
Open
proggeramlug wants to merge 1 commit into
Open
feat(electron): Electron-compatible app shell — run existing Electron apps natively#5547proggeramlug wants to merge 1 commit into
proggeramlug wants to merge 1 commit into
Conversation
… apps natively
Adds `packages/electron`, a drop-in Electron-compat toolkit, plus the native
support it needs (macOS first). Existing Electron app code (`app`,
`BrowserWindow`, `ipcMain`, `contextBridge`, …) runs unmodified: app logic
compiles native, the view runs in the OS-native webview (WKWebView) — Tauri-model
internals, Electron API surface, no bundled Chromium, no Rust to write.
Native (perry-ui-macos / perry-runtime / perry-codegen / perry-dispatch):
- webview.rs: bidirectional IPC bridge — WKScriptMessageHandler (renderer→main),
document-start user-script injection (add_user_script), set_on_message; file://
loads via loadFileURL:allowingReadAccessToURL: + allowFileAccessFromFileURLs so a
file:// page can load its own subresources.
- app.rs: app_run_loop (body-less event loop resolving whenReady), app_quit,
full-window Auto-Layout pinning in window_set_body, window_show activates +
orderFrontRegardless.
- ui_loop.rs + codegen entry.rs hook: js_ui_loop_take_over runs the UI loop at the
TOP LEVEL of generated main (not from a microtask) — required or windows created
from app.whenReady().then(...) never composite on macOS.
- FFI exports, perry-dispatch table rows, and types/perry/ui/index.d.ts decls.
TS toolkit (packages/electron, a nativeModule package):
- app, BrowserWindow, ipcMain, webContents, Menu, dialog, shell, contextBridge/
ipcRenderer implemented on perry/ui + node:*.
- Renderer-side bridge runtime (bridge/preload-runtime.js): require('electron'),
a CommonJS loader for local renderer requires, ipcRenderer/contextBridge,
console forwarding to the main process.
- examples/system-explorer: a full IPC demo (invoke/handle, send/on, main→renderer
push, fs/os, persisted userData).
Verified end-to-end on macOS: czonios/pomodoro-electron (vanilla) and
electron-vite-react (React + Vite + Tailwind, contextBridge) run unmodified with
rendering windows and working IPC.
Known limits: macOS only so far (Windows/Linux bridge is the same pattern);
dialog/Menu are v1 stubs; JSON-only IPC; renderer-side Node usage (old
nodeIntegration + `remote` apps) is unsupported by design (system webview has no
Node — same boundary as Tauri).
📝 WalkthroughWalkthroughAdds a full Electron-compatibility layer to Perry. A native UI event-loop takeover mechanism is introduced in ChangesElectron Compat Layer
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
Note over GeneratedMain,AsyncEventLoop: Startup
participant GeneratedMain
participant js_ui_loop_take_over
participant perry_runtime_register_ui_loop
participant app_run_loop as app_run_loop (macOS)
participant AsyncEventLoop
end
perry_runtime_register_ui_loop->>js_ui_loop_take_over: stores ui_loop_entry fn ptr
GeneratedMain->>js_ui_loop_take_over: call after microtask/timer flush
js_ui_loop_take_over->>app_run_loop: invoke registered fn ptr
app_run_loop->>app_run_loop: init NSApplication, delegate, menu, timers
app_run_loop->>app_run_loop: resolve on_ready TS closure
app_run_loop->>AsyncEventLoop: NSApplication.run() (blocks)
sequenceDiagram
rect rgba(60, 179, 113, 0.5)
Note over RendererPage,TSHandler: IPC invoke flow
end
participant RendererPage
participant preload_runtime as preload-runtime (renderer)
participant WKScriptMessageHandler
participant ipcMain
participant TSHandler
RendererPage->>preload_runtime: ipcRenderer.invoke(channel, args)
preload_runtime->>WKScriptMessageHandler: postMessage({kind:"invoke", id, channel, args})
WKScriptMessageHandler->>ipcMain: dispatch message
ipcMain->>TSHandler: registered handle(channel, event, args)
TSHandler-->>ipcMain: result
ipcMain->>WKScriptMessageHandler: evaluateJavaScript(__perryResolve(id, true, json))
WKScriptMessageHandler-->>preload_runtime: window.__perryResolve(id, true, json)
preload_runtime-->>RendererPage: Promise resolves with result
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/electron/bridge/preload-runtime.js (1)
170-193: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winModule cache checked after file load; wastes network round-trip for already-cached modules.
loadSync(url)is called before checkingmoduleCache[url]. If the module is already cached, the XHR is unnecessary.Move the cache check before the load:
♻️ Proposed fix
function makeRequire(baseUrl) { return function (name) { if (name === "electron") return electron; if (name.charAt(0) === "." || name.charAt(0) === "/") { var url = resolveUrl(name, baseUrl); + if (moduleCache[url]) return moduleCache[url].exports; var src = loadSync(url); - if (src === null && url.slice(-3) !== ".js") { url = url + ".js"; src = loadSync(url); } + if (src === null && url.slice(-3) !== ".js") { + url = url + ".js"; + if (moduleCache[url]) return moduleCache[url].exports; + src = loadSync(url); + } if (src === null) throw new Error("[perry-electron] require: cannot load '" + name + "'"); - if (moduleCache[url]) return moduleCache[url].exports; var mod = { exports: {} };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/electron/bridge/preload-runtime.js` around lines 170 - 193, In the makeRequire function, the moduleCache check is performed after calling loadSync(url), which wastes a network round-trip for already-cached modules. Move the cache lookup check (if (moduleCache[url]) return moduleCache[url].exports;) to occur before the loadSync(url) call, so that cached modules are returned immediately without making unnecessary network requests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-ui-macos/src/widgets/webview.rs`:
- Around line 619-621: The `allowUniversalAccessFromFileURLs` setting is
unconditionally enabled and should instead be made opt-in behind an explicit
security configuration flag. Additionally, locate the `forMainFrameOnly` setting
(referenced around line 951) and change its default from false to true to
restrict script bridge injection to the main frame only. Both settings should be
guarded by explicit insecure configuration options that require deliberate
opt-in rather than being enabled by default. Ensure the
`did_receive_script_message` message handler includes origin and frame context
validation before forwarding IPC calls to the renderer-main bridge.
- Around line 597-603: Several Objective-C objects created with `new`, `alloc`,
or `init` methods are stored as raw pointers instead of using the `Retained`
wrapper type, causing reference count leaks when ownership transfers to receiver
methods. Convert the declarations for `ucc` at line 599, `cfg` at line 580, `wv`
at lines 624-625, and `script` in the loop at lines 944-952 to use
`Retained<AnyObject>` type annotations instead of raw pointers. Update each
`msg_send!` invocation that creates these objects to directly assign to the
`Retained`-typed variable, which will properly track and balance the +1
reference count from Cocoa's ownership conventions.
In `@packages/electron/examples/system-explorer/main.ts`:
- Around line 95-106: The notes:load and notes:save IPC handlers lack schema
validation for the string[] contract between main and renderer processes. In the
notes:load handler, after parsing the JSON from notesPath(), add validation to
ensure the result is actually an array of strings before returning it; if
validation fails, return an empty array instead. In the notes:save handler,
validate that the incoming notes parameter is a string[] before calling
writeFileSync; reject the operation or return an error response if validation
fails. This prevents malformed notes.json from breaking the renderer's UI logic.
In `@packages/electron/examples/system-explorer/renderer/renderer.js`:
- Around line 23-25: The code at lines 23-25 and 35-56 uses innerHTML with
dynamically concatenated HTML strings containing untrusted values (file names),
which creates an XSS vulnerability. Replace the innerHTML assignment with safe
DOM node construction by creating elements using document.createElement for the
container div and span elements, setting the untrusted values (r[0] and r[1])
using textContent instead of concatenating them into HTML strings, and then
appending the constructed nodes to the parent element. This ensures that
untrusted data is treated as text content rather than executable markup.
In `@packages/electron/src/index.ts`:
- Around line 293-298: The loadFile method constructs a file:// URL by directly
concatenating the absolute path without proper URL encoding, which will break
for paths containing special characters like spaces, hash marks, or question
marks. Fix this by encoding the absolute path before constructing the URL. In
the webviewLoadUrl call, replace the string concatenation of "file://" + abs
with either the URL constructor approach (new URL("file://" + abs).href) or by
wrapping the abs variable with encodeURI to ensure special characters are
properly percent-encoded in the resulting URL.
---
Nitpick comments:
In `@packages/electron/bridge/preload-runtime.js`:
- Around line 170-193: In the makeRequire function, the moduleCache check is
performed after calling loadSync(url), which wastes a network round-trip for
already-cached modules. Move the cache lookup check (if (moduleCache[url])
return moduleCache[url].exports;) to occur before the loadSync(url) call, so
that cached modules are returned immediately without making unnecessary network
requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 616057d5-eb3d-47b7-a23d-ac01c5b98913
📒 Files selected for processing (22)
crates/perry-codegen/src/codegen/entry.rscrates/perry-codegen/src/runtime_decls/strings_part2.rscrates/perry-dispatch/src/ui_table.rscrates/perry-runtime/src/lib.rscrates/perry-runtime/src/ui_loop.rscrates/perry-ui-macos/src/app.rscrates/perry-ui-macos/src/lib_ffi/core_widgets.rscrates/perry-ui-macos/src/lib_ffi/interactivity.rscrates/perry-ui-macos/src/widgets/webview.rspackages/electron/.gitignorepackages/electron/DESIGN.mdpackages/electron/README.mdpackages/electron/bridge/preload-runtime.jspackages/electron/examples/system-explorer/main.tspackages/electron/examples/system-explorer/package.jsonpackages/electron/examples/system-explorer/preload.jspackages/electron/examples/system-explorer/renderer/index.htmlpackages/electron/examples/system-explorer/renderer/renderer.jspackages/electron/package.jsonpackages/electron/src/index.tspackages/electron/src/preload-runtime.tstypes/perry/ui/index.d.ts
Comment on lines
+597
to
+603
| let ucc_cls = AnyClass::get(c"WKUserContentController") | ||
| .expect("WKUserContentController not found — link WebKit.framework"); | ||
| let ucc: *mut AnyObject = msg_send![ucc_cls, new]; | ||
| let handler_name = NSString::from_str("perry"); | ||
| let _: () = msg_send![ucc, addScriptMessageHandler: &*delegate, name: &*handler_name]; | ||
| let _: () = msg_send![cfg, setUserContentController: ucc]; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect +1 object creation patterns in changed webview paths
rg -n -C2 'msg_send!\[.*\b(new|alloc|initWithFrame|initWithSource)\b' crates/perry-ui-macos/src/widgets/webview.rsRepository: PerryTS/perry
Length of output: 1772
🏁 Script executed:
#!/bin/bash
# Get broader context around the identified lines
echo "=== Lines 575-610 (around line 599 ucc) ==="
sed -n '575,610p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n
echo "=== Lines 620-650 (around lines 624-625 wv) ==="
sed -n '620,650p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n
echo "=== Lines 940-960 (around line 944 script) ==="
sed -n '940,960p' crates/perry-ui-macos/src/widgets/webview.rs | cat -nRepository: PerryTS/perry
Length of output: 5411
🏁 Script executed:
#!/bin/bash
# Check the function context and object usage patterns
echo "=== Check line 580 cfg creation and usage ==="
sed -n '575,630p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n | head -60
echo "=== Verify line numbers in create_webview function ==="
rg -n 'fn (create_webview|new_webview)' crates/perry-ui-macos/src/widgets/webview.rs
echo "=== Check if Retained is used elsewhere for similar patterns ==="
rg -n 'Retained<.*>' crates/perry-ui-macos/src/widgets/webview.rs | head -20Repository: PerryTS/perry
Length of output: 4336
🏁 Script executed:
#!/bin/bash
# Check how cfg and ucc are managed after creation
echo "=== Lines 650-655 showing wv conversion to Retained ==="
sed -n '650,655p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n
# Check the full add_user_script function context
echo "=== Lines 920-960 showing script creation in loop ==="
sed -n '920,960p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n
# Check if cfg and ucc are passed to methods that transfer ownership
echo "=== Check setUserContentController method call ==="
sed -n '595,610p' crates/perry-ui-macos/src/widgets/webview.rs | cat -nRepository: PerryTS/perry
Length of output: 3752
🏁 Script executed:
#!/bin/bash
# Check if cfg is retained by WKWebView
echo "=== Check Cocoa documentation patterns in comments ==="
sed -n '570,610p' crates/perry-ui-macos/src/widgets/webview.rs | grep -A5 -B5 'retain'
# Check the actual function definition to see return type
echo "=== Check the create_webview function signature ==="
rg -B5 'let cfg: \*mut AnyObject = msg_send!' crates/perry-ui-macos/src/widgets/webview.rs | head -20
# Compare with line 149 pattern
echo "=== Verify line 149 uses Retained consistently ==="
sed -n '145,165p' crates/perry-ui-macos/src/widgets/webview.rs | cat -nRepository: PerryTS/perry
Length of output: 3214
🏁 Script executed:
#!/bin/bash
# Check if msg_send returns Retained or raw pointer for these methods
echo "=== Search for other setUserContentController patterns ==="
rg -i 'setUserContentController' crates/perry-ui-macos/src/widgets/webview.rs
echo "=== Check if cfg/ucc are truly stored or just passed through ==="
sed -n '597,605p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n
echo "=== Check the whole cfg lifecycle ==="
rg -n 'cfg:' crates/perry-ui-macos/src/widgets/webview.rs | head -20
echo "=== Check what happens to script after addUserScript ==="
sed -n '940,960p' crates/perry-ui-macos/src/widgets/webview.rs | cat -nRepository: PerryTS/perry
Length of output: 2315
🏁 Script executed:
#!/bin/bash
# Check the function return type and how cfg is ultimately used
echo "=== Check create_webview full context around cfg ==="
sed -n '560,630p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n | grep -A2 -B2 'initWithFrame'
# Verify that cfg is passed to WKWebView initialization
echo "=== Check initWithFrame signature expectation ==="
rg -A3 'initWithFrame:.*configuration:' crates/perry-ui-macos/src/widgets/webview.rs
# Check if WKWebViewConfiguration retains objects passed to it
echo "=== Understand ownership transfer semantics in comments ==="
sed -n '577,608p' crates/perry-ui-macos/src/widgets/webview.rs | cat -nRepository: PerryTS/perry
Length of output: 3003
Use Retained ownership for Objective-C objects created with new/alloc/init.
Line 599 (ucc), line 580 (cfg), lines 624–625 (wv), and lines 944–952 (script) create +1 Cocoa objects via raw pointers without balanced ownership tracking. When cfg and ucc are passed to methods like setUserContentController: and initWithFrame:configuration:, ownership transfers to the receiver (which retains them), leaving the raw pointer pointing to objects with leaked +1 refcounts. Line 944's script in the add_user_script loop leaks on every call, causing cumulative memory pressure.
Convert these to Retained on creation:
let cfg: Retained<AnyObject> = msg_send![cfg_cls, new];let ucc: Retained<AnyObject> = msg_send![ucc_cls, new];let script: Retained<AnyObject> = msg_send![script, initWithSource: ...];
Per coding guidelines, "use msg_send! which returns Retained directly" for all +1 methods.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-ui-macos/src/widgets/webview.rs` around lines 597 - 603, Several
Objective-C objects created with `new`, `alloc`, or `init` methods are stored as
raw pointers instead of using the `Retained` wrapper type, causing reference
count leaks when ownership transfers to receiver methods. Convert the
declarations for `ucc` at line 599, `cfg` at line 580, `wv` at lines 624-625,
and `script` in the loop at lines 944-952 to use `Retained<AnyObject>` type
annotations instead of raw pointers. Update each `msg_send!` invocation that
creates these objects to directly assign to the `Retained`-typed variable, which
will properly track and balance the +1 reference count from Cocoa's ownership
conventions.
Source: Coding guidelines
Comment on lines
+619
to
+621
| let k2 = NSString::from_str("allowUniversalAccessFromFileURLs"); | ||
| let _: () = msg_send![cfg, setValue: yes_num, forKey: &*k2]; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify broad bridge exposure and file-origin relaxations
rg -n -C2 'allowUniversalAccessFromFileURLs|forMainFrameOnly:\s*false|addScriptMessageHandler:\s*&\*delegate' crates/perry-ui-macos/src/widgets/webview.rs
rg -n -C2 'messageHandlers\.perry\.postMessage|webviewSetOnMessage|webviewAddUserScript' packages/electron/src/preload-runtime.ts packages/electron/src/index.tsRepository: PerryTS/perry
Length of output: 4119
🏁 Script executed:
#!/bin/bash
# Get broader context around the sensitive lines
echo "=== Context around line 619-620 (allowUniversalAccessFromFileURLs) ==="
sed -n '605,630p' crates/perry-ui-macos/src/widgets/webview.rs
echo ""
echo "=== Context around line 946-951 (forMainFrameOnly: false) ==="
sed -n '935,960p' crates/perry-ui-macos/src/widgets/webview.rs
echo ""
echo "=== Check for any validation in the message handler (around line 601) ==="
sed -n '590,620p' crates/perry-ui-macos/src/widgets/webview.rsRepository: PerryTS/perry
Length of output: 4928
🏁 Script executed:
#!/bin/bash
# Find the did_receive_script_message implementation
rg -n 'did_receive_script_message|WKScriptMessageHandler' crates/perry-ui-macos/src/widgets/webview.rs | head -20
echo ""
echo "=== Look for the handler implementation ==="
rg -n -A15 'fn did_receive_script_message' crates/perry-ui-macos/src/widgets/webview.rsRepository: PerryTS/perry
Length of output: 1087
🏁 Script executed:
#!/bin/bash
# Get full implementation of did_receive_script_message
sed -n '461,530p' crates/perry-ui-macos/src/widgets/webview.rsRepository: PerryTS/perry
Length of output: 2836
Restrict script bridge injection to main frame by default and require explicit opt-in for file-origin access.
The forMainFrameOnly: false setting (line 951) injects the perry IPC bridge into all frames including iframes, and allowUniversalAccessFromFileURLs (line 620) allows any file-origin to access cross-origin resources. The message handler (did_receive_script_message, lines 461–497) does not validate the origin or frame context before forwarding IPC calls, allowing untrusted iframe or file-origin content to reach the renderer→main bridge without restriction.
While both settings appear designed for Electron-style loadFile() compatibility, they should be opt-in and guarded behind explicit insecure flags rather than unconditionally enabled. Default to forMainFrameOnly: true and protect universal file-access behind a documented security caveat.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-ui-macos/src/widgets/webview.rs` around lines 619 - 621, The
`allowUniversalAccessFromFileURLs` setting is unconditionally enabled and should
instead be made opt-in behind an explicit security configuration flag.
Additionally, locate the `forMainFrameOnly` setting (referenced around line 951)
and change its default from false to true to restrict script bridge injection to
the main frame only. Both settings should be guarded by explicit insecure
configuration options that require deliberate opt-in rather than being enabled
by default. Ensure the `did_receive_script_message` message handler includes
origin and frame context validation before forwarding IPC calls to the
renderer-main bridge.
Comment on lines
+95
to
+106
| ipcMain.handle("notes:load", async () => { | ||
| try { | ||
| const text = fs.readFileSync(notesPath(), "utf8"); | ||
| return JSON.parse(text); | ||
| } catch (e) { | ||
| return []; | ||
| } | ||
| }); | ||
|
|
||
| ipcMain.handle("notes:save", async (_event, notes: string[]) => { | ||
| fs.writeFileSync(notesPath(), JSON.stringify(notes, null, 2), "utf8"); | ||
| return { ok: true, count: notes.length }; |
There was a problem hiding this comment.
Validate notes schema at IPC boundary before returning/saving.
notes:load returns arbitrary parsed JSON, but the renderer assumes an array and immediately calls .map(...). A malformed notes.json can break the notes UI path. Validate both load and save payloads as string[] to keep the main↔renderer contract stable.
Proposed fix
ipcMain.handle("notes:load", async () => {
try {
const text = fs.readFileSync(notesPath(), "utf8");
- return JSON.parse(text);
+ const parsed = JSON.parse(text);
+ if (!Array.isArray(parsed)) return [];
+ return parsed.filter((n): n is string => typeof n === "string");
} catch (e) {
return [];
}
});
ipcMain.handle("notes:save", async (_event, notes: string[]) => {
- fs.writeFileSync(notesPath(), JSON.stringify(notes, null, 2), "utf8");
- return { ok: true, count: notes.length };
+ const safeNotes = Array.isArray(notes) ? notes.filter((n): n is string => typeof n === "string") : [];
+ fs.writeFileSync(notesPath(), JSON.stringify(safeNotes, null, 2), "utf8");
+ return { ok: true, count: safeNotes.length };
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ipcMain.handle("notes:load", async () => { | |
| try { | |
| const text = fs.readFileSync(notesPath(), "utf8"); | |
| return JSON.parse(text); | |
| } catch (e) { | |
| return []; | |
| } | |
| }); | |
| ipcMain.handle("notes:save", async (_event, notes: string[]) => { | |
| fs.writeFileSync(notesPath(), JSON.stringify(notes, null, 2), "utf8"); | |
| return { ok: true, count: notes.length }; | |
| ipcMain.handle("notes:load", async () => { | |
| try { | |
| const text = fs.readFileSync(notesPath(), "utf8"); | |
| const parsed = JSON.parse(text); | |
| if (!Array.isArray(parsed)) return []; | |
| return parsed.filter((n): n is string => typeof n === "string"); | |
| } catch (e) { | |
| return []; | |
| } | |
| }); | |
| ipcMain.handle("notes:save", async (_event, notes: string[]) => { | |
| const safeNotes = Array.isArray(notes) ? notes.filter((n): n is string => typeof n === "string") : []; | |
| fs.writeFileSync(notesPath(), JSON.stringify(safeNotes, null, 2), "utf8"); | |
| return { ok: true, count: safeNotes.length }; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/electron/examples/system-explorer/main.ts` around lines 95 - 106,
The notes:load and notes:save IPC handlers lack schema validation for the
string[] contract between main and renderer processes. In the notes:load
handler, after parsing the JSON from notesPath(), add validation to ensure the
result is actually an array of strings before returning it; if validation fails,
return an empty array instead. In the notes:save handler, validate that the
incoming notes parameter is a string[] before calling writeFileSync; reject the
operation or return an error response if validation fails. This prevents
malformed notes.json from breaking the renderer's UI logic.
Comment on lines
+23
to
+25
| document.getElementById("sysinfo").innerHTML = rows | ||
| .map((r) => '<div class="kv"><span>' + r[0] + "</span><span>" + r[1] + "</span></div>") | ||
| .join(""); |
There was a problem hiding this comment.
Replace dynamic innerHTML rendering with DOM node construction to prevent XSS.
The current string-based HTML rendering injects untrusted values (notably file names) directly into markup/attributes. A crafted filename can execute script in the renderer and then invoke privileged preload APIs. Build nodes with textContent/dataset instead of concatenated HTML strings.
Proposed fix
- document.getElementById("sysinfo").innerHTML = rows
- .map((r) => '<div class="kv"><span>' + r[0] + "</span><span>" + r[1] + "</span></div>")
- .join("");
+ const sysEl = document.getElementById("sysinfo");
+ sysEl.textContent = "";
+ rows.forEach((r) => {
+ const row = document.createElement("div");
+ row.className = "kv";
+ const k = document.createElement("span");
+ const v = document.createElement("span");
+ k.textContent = String(r[0]);
+ v.textContent = String(r[1]);
+ row.appendChild(k);
+ row.appendChild(v);
+ sysEl.appendChild(row);
+ });
@@
- const html = res.entries
- .map(function (e) {
- const ic = e.isDir ? "📁" : "📄";
- const sz = e.isDir ? "" : '<span class="sz">' + fmtBytes(e.sizeBytes) + "</span>";
- return (
- '<div class="file" data-name="' +
- e.name +
- '" data-dir="' +
- (e.isDir ? "1" : "0") +
- '"><span class="ic">' +
- ic +
- '</span><span>' +
- e.name +
- "</span>" +
- sz +
- "</div>"
- );
- })
- .join("");
const filesEl = document.getElementById("files");
- filesEl.innerHTML = html;
- filesEl.querySelectorAll(".file").forEach(function (el) {
- el.addEventListener("click", async function () {
- if (el.getAttribute("data-dir") === "1") return;
- const name = el.getAttribute("data-name");
+ filesEl.textContent = "";
+ const frag = document.createDocumentFragment();
+ res.entries.forEach(function (e) {
+ const el = document.createElement("div");
+ el.className = "file";
+ el.dataset.name = e.name;
+ el.dataset.dir = e.isDir ? "1" : "0";
+
+ const ic = document.createElement("span");
+ ic.className = "ic";
+ ic.textContent = e.isDir ? "📁" : "📄";
+ const name = document.createElement("span");
+ name.textContent = e.name;
+ el.appendChild(ic);
+ el.appendChild(name);
+ if (!e.isDir) {
+ const sz = document.createElement("span");
+ sz.className = "sz";
+ sz.textContent = fmtBytes(e.sizeBytes);
+ el.appendChild(sz);
+ }
+
+ el.addEventListener("click", async function () {
+ if (el.dataset.dir === "1") return;
+ const name = el.dataset.name || "";
const fileRes = await api.readFile(lastDir + "/" + name);
const pre = document.getElementById("preview");
pre.textContent = fileRes.ok ? fileRes.text : "⚠️ " + fileRes.error;
});
+ frag.appendChild(el);
});
+ filesEl.appendChild(frag);Also applies to: 35-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/electron/examples/system-explorer/renderer/renderer.js` around lines
23 - 25, The code at lines 23-25 and 35-56 uses innerHTML with dynamically
concatenated HTML strings containing untrusted values (file names), which
creates an XSS vulnerability. Replace the innerHTML assignment with safe DOM
node construction by creating elements using document.createElement for the
container div and span elements, setting the untrusted values (r[0] and r[1])
using textContent instead of concatenating them into HTML strings, and then
appending the constructed nodes to the parent element. This ensures that
untrusted data is treated as text content rather than executable markup.
Source: Linters/SAST tools
Comment on lines
+293
to
+298
| loadFile(filePath: string, _options?: any): Promise<void> { | ||
| // Resolve relative to cwd; Electron resolves relative to the app dir. | ||
| const abs = path.isAbsolute(filePath) ? filePath : path.join(process.cwd(), filePath); | ||
| webviewLoadUrl(this.webContents._wv, "file://" + abs); | ||
| return Promise.resolve(); | ||
| } |
There was a problem hiding this comment.
file:// URL may break on paths with special characters.
filePath is concatenated directly into the URL without encoding. Paths containing spaces, #, ?, or other URL-significant characters will produce malformed URLs or truncated loads.
Consider using encodeURI or constructing via URL:
const fileUrl = new URL("file://" + abs).href;Or at minimum:
webviewLoadUrl(this.webContents._wv, "file://" + encodeURI(abs));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/electron/src/index.ts` around lines 293 - 298, The loadFile method
constructs a file:// URL by directly concatenating the absolute path without
proper URL encoding, which will break for paths containing special characters
like spaces, hash marks, or question marks. Fix this by encoding the absolute
path before constructing the URL. In the webviewLoadUrl call, replace the string
concatenation of "file://" + abs with either the URL constructor approach (new
URL("file://" + abs).href) or by wrapping the abs variable with encodeURI to
ensure special characters are properly percent-encoded in the resulting URL.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A drop-in Electron-compat toolkit for Perry: existing Electron app code (
app,BrowserWindow,ipcMain,contextBridge,Menu,dialog, …) runs unmodified. App logic compiles to a native binary; the view runs in the OS-native webview (WKWebView). Internally it's the Tauri model (system webview, single native process, no bundled Chromium); the public surface is Electron's — so it's a drop-in, with no Rust to write.macOS first. Windows (WebView2) / Linux (WebKitGTK) are the same pattern, not yet ported.
How it works
Electron splits across a main process (Node) and renderers (Chromium); this mirrors that:
app,BrowserWindow,ipcMain,Menu,dialog,webContentspackages/electron/src/index.tsonperry/ui+node:*ipcRenderer,contextBridge,require('electron')in preloadbridge/preload-runtime.jsIPC: a
WKScriptMessageHandlercarries renderer→main;evaluateJavaScriptcarries main→renderer;ipcRenderer.invoke↔ipcMain.handlecorrelate by id.Native changes (perry-ui-macos / perry-runtime / perry-codegen / perry-dispatch)
WKScriptMessageHandler+ document-start user-script injection +set_on_message);file://loads vialoadFileURL:allowingReadAccessToURL:+allowFileAccessFromFileURLsso afile://page can load its own subresources.app_run_loop(body-less loop resolvingwhenReady),app_quit, full-window Auto-Layout pinning, window activation.entry.rshook —js_ui_loop_take_overruns the UI loop at the top level of generatedmain(not from a microtask). Required: entering[NSApp run]from a microtask leaves windows non-composited on macOS.types/perry/ui/index.d.tsdecls.TS toolkit (
packages/electron)app,BrowserWindow,ipcMain,webContents,Menu,dialog,shell,contextBridge/ipcRenderer.require('electron'), a CommonJS loader for local renderer requires, console forwarding to the main process.examples/system-explorer— full IPC demo (invoke/handle, send/on, main→renderer push, fs/os, persisted userData).Verified end-to-end (macOS)
contextBridge) — unmodified, window renders, IPC works. (npm install+vite buildfor the renderer; built ESM main + CJS preload compiled by Perry;electron-updaterswapped for a small shim.)Known limits (honest)
dialog/Menuare v1 stubs (template accepted; native NSOpenPanel/menubar TODO).contextIsolationsecurity boundary; a webview hang takes the app down.nodeIntegration+remoteapps that callfs/npm directly in the renderer hit the system-webview boundary (same as Tauri).webContentsdid-finish-loaddoesn't fire yet (webviewonLoadeddelegate gap forfile://).See
packages/electron/{README,DESIGN}.mdfor details.Notes for review
perry-ui-macos); no behavior change for non-UI programs (the codegenjs_ui_loop_take_overcall is a no-op when no UI loop is registered).Summary by CodeRabbit
Release Notes
New Features
Documentation