fix(security): RQ-2426 — sandbox rule code (QuickJS-WASM)#112
Conversation
WalkthroughThe PR replaces the previous host-realm Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.0)src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.jsFile contains syntax errors that prevent linting: Line 1: Illegal use of an import declaration outside of a module; Line 3: Illegal use of an import declaration outside of a module; Line 6: Illegal use of an import declaration outside of a module; Line 7: Illegal use of an import declaration outside of a module; Line 8: Illegal use of an import declaration outside of a module; Line 81: Illegal use of an export declaration outside of a module src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.jsFile contains syntax errors that prevent linting: Line 1: Illegal use of an import declaration outside of a module; Line 3: Illegal use of an import declaration outside of a module; Line 6: Illegal use of an import declaration outside of a module; Line 7: Illegal use of an import declaration outside of a module; Line 8: Illegal use of an import declaration outside of a module; Line 9: Illegal use of an import declaration outside of a module; Line 10: Illegal use of an import declaration outside of a module; Line 175: Illegal use of an export declaration outside of a module Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
"Code"-type Modify Request/Response rules ran rule-supplied JS via
`new Function(...)` directly in the proxy's Node process (full require/process/
fs/child_process). Code rules travel between users (shared lists, import/export,
team sync), so this was a supply-chain RCE primitive.
Rule code now runs inside QuickJS compiled to WebAssembly (quickjs-emscripten,
single-file embedded variant). QuickJS is a separate JS engine in the WASM
sandbox: no require/process/fs/global, and no prototype path back to the host
(constructor-escape blocked). Only injected primitives are reachable.
Why QuickJS-WASM (not isolated-vm or worker_threads + vm):
- isolated-vm is a native addon with no build for a supported Electron's V8
(6.x too old for V8 13, 7.x needs Node 26).
- worker_threads cannot create a Worker in an Electron renderer ("The V8
platform ... does not support creating Workers"), and the proxy runs in the
desktop app's background renderer.
QuickJS-WASM is pure WASM+JS — builds nowhere natively and runs in the renderer.
- src/utils/index.ts: executeUserFunction runs in QuickJS; 5s deadline interrupt,
128MB cap. isValidFunctionString compiles via `new Function` WITHOUT calling it
(parse-only, no execution). getFunctionFromString removed.
- both Modify Request/Response processors: validate -> pass the source string.
- contract preserved: returns a string (objects JSON-stringified), promises
awaited, console captured as {type,args}, $sharedState read + written back.
- intentional gap: no fetch/Buffer/timers (fetch needs the asyncify variant +
async host bridge — a follow-up; QuickJS can do it safely).
Verified: sandbox harness 13/13 (Node 24); instantiates + runs + blocks
host access inside the Electron 42 renderer; before/after exploit probe flips
from RCE/file/env/process access to fully blocked.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dinex-dev
force-pushed
the
fix/RQ-2426-sandbox-worker-vm
branch
from
77c31fd to
2f32940
Compare
dinex-dev
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@package.json`:
- Line 3: The version field in package.json has been downgraded from 1.5.0 to
1.4.0, which violates semantic versioning and will cause issues with release
management tools. Update the version value to a forward version number that is
higher than the previously published 1.5.0 (such as 1.5.1 or 1.6.0, depending on
whether this is a patch or minor release) to maintain proper version ordering
for releases.
In `@src/utils/index.ts`:
- Around line 167-168: The call to executePendingJobs() on the vm.runtime object
returns a result containing a QuickJSHandle that must be properly disposed to
prevent VM memory leaks. Capture the return value from executePendingJobs() and
check if it contains an error handle, then ensure the handle is disposed by
calling the appropriate disposal method on the returned result before
proceeding. This will prevent memory leaks when deadline interrupts or job
errors occur.
- Around line 114-120: The issue is a race condition where the snapshot read via
GlobalStateProvider.getInstance().getSharedStateCopy() occurs before an await
statement, allowing concurrent invocations to modify shared state via
setSharedState() while control is yielded, causing stale state to overwrite
concurrent updates when execution resumes. Move the entire try-catch block
containing the getSharedStateCopy() call to execute after the await
getQuickJSModule() statement completes, so that the snapshot is captured after
the async module resolution rather than before it. This closes the interleaving
window and ensures the snapshot reflects current state at the point it will be
used.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2509111-22a8-4f66-8744-b17a2a20643b
⛔ Files ignored due to path filters (5)
dist/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.jsis excluded by!**/dist/**dist/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.jsis excluded by!**/dist/**dist/utils/index.d.tsis excluded by!**/dist/**dist/utils/index.jsis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonsrc/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.jssrc/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.jssrc/utils/index.ts
| { | ||
| "name": "@requestly/requestly-proxy", | ||
| "version": "1.5.0", | ||
| "version": "1.4.0", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the current version and previous version in git
cd "$(git rev-parse --show-toplevel)"
# Show current version
echo "=== Current package.json version ==="
grep -A1 '"version"' package.json | head -2
# Show version changes in this PR/commit
echo -e "\n=== Git diff for version in package.json ==="
git diff HEAD -- package.json | grep -A2 -B2 '"version"' || echo "No changes in version"
# Show git log for package.json to see recent version history
echo -e "\n=== Recent version history (last 5 commits) ==="
git log --oneline -5 -- package.json
# Get the actual previous version
echo -e "\n=== Previous version (HEAD~1) ==="
git show HEAD~1:package.json 2>/dev/null | grep '"version"' || echo "Could not retrieve previous version"Repository: requestly/requestly-proxy
Length of output: 553
🏁 Script executed:
#!/bin/bash
# Check if there are uncommitted/staged changes
echo "=== Git status ==="
git status
echo -e "\n=== Check git diff --cached (staged changes) ==="
git diff --cached -- package.json | head -30
echo -e "\n=== Full git diff (all changes) ==="
git diff -- package.json | head -30
echo -e "\n=== Check all refs to find versions ==="
git log --all --oneline -- package.json | head -10
echo -e "\n=== Show version in last commit that modified package.json ==="
git log -1 -p -- package.json | grep -A2 -B2 '"version"' | head -15Repository: requestly/requestly-proxy
Length of output: 1062
🏁 Script executed:
#!/bin/bash
# Get the version at the current HEAD and the previous commit that modified package.json
echo "=== Version at current HEAD (2f32940) ==="
git show HEAD:package.json | grep '"version"'
echo -e "\n=== Version at previous release commit (381f6b9 - v1.5.0) ==="
git show 381f6b9:package.json | grep '"version"'
echo -e "\n=== Verify commit order and which one is parent ==="
git rev-list --ancestry-path 381f6b9..2f32940 | wc -l
git rev-list --ancestry-path 2f32940..381f6b9 | wc -l
echo -e "\n=== Show the full diff between 381f6b9 and 2f32940 for package.json ==="
git diff 381f6b9 HEAD -- package.json | head -50Repository: requestly/requestly-proxy
Length of output: 1258
Version was downgraded 1.5.0 → 1.4.0.
This is a backwards version bump that will break release-it/publish ordering and sit below the already-published 1.5.0. Please correct this to a forward version (e.g. 1.5.1) unless this downgrade is intentional.
🤖 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 `@package.json` at line 3, The version field in package.json has been
downgraded from 1.5.0 to 1.4.0, which violates semantic versioning and will
cause issues with release management tools. Update the version value to a
forward version number that is higher than the previously published 1.5.0 (such
as 1.5.1 or 1.6.0, depending on whether this is a patch or minor release) to
maintain proper version ordering for releases.
| try { | ||
| sharedStateJson = JSON.stringify( | ||
| GlobalStateProvider.getInstance().getSharedStateCopy() ?? {} | ||
| ); | ||
| } catch { | ||
| sharedStateJson = "{}"; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect GlobalStateProvider shared-state API and any concurrent invocation paths
fd -t f 'state' src/components/proxy-middleware/middlewares | head
rg -nP -C3 '(getSharedStateCopy|setSharedState|sharedState)\b' src/components/proxy-middleware/middlewares
rg -nP -C3 'executeUserFunction\s*\(' src/components/proxy-middlewareRepository: requestly/requestly-proxy
Length of output: 6261
🏁 Script executed:
# First, let's see the structure of src/utils/index.ts and find executeUserFunction
ast-grep outline src/utils/index.ts --view expandedRepository: requestly/requestly-proxy
Length of output: 547
🏁 Script executed:
# Read the specific section containing executeUserFunction (around lines 114-200)
wc -l src/utils/index.tsRepository: requestly/requestly-proxy
Length of output: 186
🏁 Script executed:
# Read the executeUserFunction from line 102 onwards
sed -n '102,206p' src/utils/index.tsRepository: requestly/requestly-proxy
Length of output: 3621
🏁 Script executed:
# Check how the proxy middleware processes requests - is there serialization or concurrency?
rg -nP 'executeUserFunction' src/components/proxy-middleware --context 5 -A 2 -B 2Repository: requestly/requestly-proxy
Length of output: 2979
🏁 Script executed:
# Look for middleware chaining and async handling
fd -t f '\.(js|ts)$' src/components/proxy-middleware | grep -E '(index|main|server|middleware)' | head -5Repository: requestly/requestly-proxy
Length of output: 451
🏁 Script executed:
# Find the main entry point and how requests are handled
find src -name 'index.ts' -o -name 'index.js' | head -10Repository: requestly/requestly-proxy
Length of output: 341
🏁 Script executed:
# Search for where executeUserFunction gets called in the request processing flow
rg -nP 'process_modify_request_action|process_modify_response_action' src/components/proxy-middleware --context 3 -B 3 -A 3Repository: requestly/requestly-proxy
Length of output: 5180
🏁 Script executed:
# Look for request handler setup - check if Express/HTTP middleware or similar
rg -nP '(app\.(use|get|post)|middleware|handler)' src/components/proxy-middleware/index --context 2Repository: requestly/requestly-proxy
Length of output: 241
🏁 Script executed:
# Look at the main proxy middleware index
cat src/components/proxy-middleware/index.js | head -50Repository: requestly/requestly-proxy
Length of output: 1831
🏁 Script executed:
# Check the main entry point
cat src/index.ts | head -50Repository: requestly/requestly-proxy
Length of output: 320
🏁 Script executed:
# Check the core HTTP proxy implementation
cat src/lib/proxy/index.js | head -100Repository: requestly/requestly-proxy
Length of output: 358
🏁 Script executed:
# Look for request handling - search for async request handlers
rg -nP 'onRequest|onResponse|intercept' src/lib/proxy --context 2 -B 2 -A 2 | head -40Repository: requestly/requestly-proxy
Length of output: 2316
🏁 Script executed:
# Check how the request handlers are actually invoked - are they awaited sequentially or in parallel?
rg -nP 'onRequest|onResponse|onRequestEnd|onResponseEnd' src/lib/proxy/lib/proxy.ts -A 5 -B 2 | head -80Repository: requestly/requestly-proxy
Length of output: 2413
🏁 Script executed:
# Search for how rules are actually executed in the middleware
rg -nP 'executeRulesProcessor|processRules|rulesMiddleware' src/components/proxy-middleware --context 3 -A 3Repository: requestly/requestly-proxy
Length of output: 163
🏁 Script executed:
# Search for handler invocation - look for forEach, Promise.all, or await patterns
rg -nP 'onRequest|onResponse|onRequestEnd|onResponseEnd' src/lib/proxy/lib/intercept.ts -A 10 -B 2 | head -100Repository: requestly/requestly-proxy
Length of output: 234
🏁 Script executed:
# Check if there's an intercept file that calls the handlers
ls -la src/lib/proxy/lib/Repository: requestly/requestly-proxy
Length of output: 453
🏁 Script executed:
# Search for where handlers are invoked in proxy.ts
rg -nP 'onRequestHandlers|onResponseHandlers|onRequestEndHandlers|onResponseEndHandlers' src/lib/proxy/lib/proxy.ts -A 5 -B 2Repository: requestly/requestly-proxy
Length of output: 4790
🏁 Script executed:
# Look for Promise.all, forEach, or async handler invocation patterns
rg -nP '(Promise\.all|forEach|for\s*\(|while|async)' src/lib/proxy/lib/proxy.ts | grep -E '(Handler|invoke|call)' -iRepository: requestly/requestly-proxy
Length of output: 363
🏁 Script executed:
# Verify the async library being used and how forEach works
head -30 src/lib/proxy/lib/proxy.ts | grep -E 'require|import'Repository: requestly/requestly-proxy
Length of output: 902
🏁 Script executed:
# Check if multiple HTTP connections are handled concurrently
rg -nP 'createServer|Server|listen' src/lib/proxy/lib/proxy.ts -B 2 -A 5Repository: requestly/requestly-proxy
Length of output: 18815
🏁 Script executed:
# Verify GlobalStateProvider is singleton and shared across requests
rg -nP 'GlobalStateProvider\.getInstance|GlobalStateProvider\.initInstance' src/components/proxy-middleware -B 2 -A 2Repository: requestly/requestly-proxy
Length of output: 163
🏁 Script executed:
# Search entire codebase for GlobalStateProvider initialization and usage
rg -nP 'GlobalStateProvider\.' src --context 2 -B 1 -A 1Repository: requestly/requestly-proxy
Length of output: 2027
Shared-state snapshot is read before an await, creating a last-writer-wins race condition in concurrent rule executions.
getSharedStateCopy() at line 116 reads a snapshot, then control yields at await getQuickJSModule() (line 122). If a concurrent executeUserFunction invocation runs during that yield and modifies shared state via setSharedState() at line 198, this invocation's now-stale snapshot will overwrite it wholesale when resumed, silently dropping the concurrent update.
Since GlobalStateProvider is a singleton instance shared across all requests and the proxy server handles multiple concurrent HTTP connections, this race is real. The rest of the function (context creation → eval → executePendingJobs → setSharedState) is synchronous, so moving the snapshot read to after the module resolves closes the interleaving window.
🔒️ Read the snapshot after the module await
- let argsJson = "{}";
- let sharedStateJson = "{}";
- try {
- argsJson = JSON.stringify(args ?? {});
- } catch {
- argsJson = "{}";
- }
- try {
- sharedStateJson = JSON.stringify(
- GlobalStateProvider.getInstance().getSharedStateCopy() ?? {}
- );
- } catch {
- sharedStateJson = "{}";
- }
-
- const QuickJS = await getQuickJSModule();
+ let argsJson = "{}";
+ try {
+ argsJson = JSON.stringify(args ?? {});
+ } catch {
+ argsJson = "{}";
+ }
+
+ const QuickJS = await getQuickJSModule();
+
+ let sharedStateJson = "{}";
+ try {
+ sharedStateJson = JSON.stringify(
+ GlobalStateProvider.getInstance().getSharedStateCopy() ?? {}
+ );
+ } catch {
+ sharedStateJson = "{}";
+ }
+
const vm = QuickJS.newContext();🤖 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 `@src/utils/index.ts` around lines 114 - 120, The issue is a race condition
where the snapshot read via
GlobalStateProvider.getInstance().getSharedStateCopy() occurs before an await
statement, allowing concurrent invocations to modify shared state via
setSharedState() while control is yielded, causing stale state to overwrite
concurrent updates when execution resumes. Move the entire try-catch block
containing the getSharedStateCopy() call to execute after the await
getQuickJSModule() statement completes, so that the snapshot is captured after
the async module resolution rather than before it. This closes the interleaving
window and ensures the snapshot reflects current state at the point it will be
used.
| // Resolve the user fn's (possibly async-but-IO-free) promise microtasks. | ||
| vm.runtime.executePendingJobs(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
quickjs-emscripten executePendingJobs return value error handle disposal 0.32
💡 Result:
In quickjs-emscripten v0.32.0 and later, executePendingJobs has been moved to the QuickJSRuntime class, accessed via context.runtime.executePendingJobs() [1][2]. Return Value and Error Handling: executePendingJobs returns an ExecutePendingJobsResult object [3][4]. This result indicates the number of jobs executed on success [3][5]. If the runtime encounters an exception, the result contains the exception handle and the context in which it occurred [3][4]. Important caveats regarding error handling: 1. It does not typically return errors thrown inside async functions or rejected promises [3][5]. 2. Those specific errors are handled by calling QuickJSContext#resolvePromise on the promise handle returned by the async function [3][6][5]. 3. For general evaluations (e.g., evalCode), results must be explicitly disposed of to prevent memory leaks in the VM [7][8]. Disposal: Since executePendingJobs operates on the runtime's internal job queue, it does not directly manage the disposal of handles created within the VM. You must continue to manually dispose of any handles (e.g., QuickJSHandle) returned by evaluation or promise resolution using their .dispose() method [7][9][8]. Failure to dispose of these handles—including those representing errors—will result in memory leaks [7][8]. To avoid deadlocks, it is often recommended to schedule executePendingJobs after any promise is settled [10][9].
Citations:
- 1: https://github.com/justjake/quickjs-emscripten/blob/HEAD/CHANGELOG.md
- 2: https://npmx.dev/package-changelog/@jitl/quickjs-wasmfile-debug-asyncify/v/0.32.0
- 3: https://github.com/justjake/quickjs-emscripten/blob/main/doc/quickjs-emscripten/classes/QuickJSRuntime.md
- 4: https://github.com/justjake/quickjs-emscripten/blob/main/doc/quickjs-emscripten-core/classes/QuickJSRuntime.md
- 5: https://github.com/justjake/quickjs-emscripten/blob/main/doc/quickjs-emscripten-core/classes/QuickJSAsyncRuntime.md
- 6: https://github.com/justjake/quickjs-emscripten/blob/main/doc/quickjs-emscripten/classes/QuickJSAsyncRuntime.md
- 7: https://github.com/justjake/quickjs-emscripten/blob/main/packages/quickjs-emscripten-core/src/context.ts
- 8: https://github.com/justjake/quickjs-emscripten/blob/main/doc/quickjs-emscripten/classes/QuickJSContext.md
- 9: https://github.com/justjake/quickjs-emscripten/
- 10: https://github.com/justjake/quickjs-emscripten/blob/main/doc/README.md
🏁 Script executed:
# First, check the file and lines mentioned in the review
head -180 src/utils/index.ts | tail -20Repository: requestly/requestly-proxy
Length of output: 796
🏁 Script executed:
# Get more context around the code
sed -n '150,180p' src/utils/index.tsRepository: requestly/requestly-proxy
Length of output: 1344
🏁 Script executed:
# Search for other usages of executePendingJobs in the codebase
rg "executePendingJobs" --type ts --type tsx -B 2 -A 2Repository: requestly/requestly-proxy
Length of output: 191
🏁 Script executed:
# Check imports to understand what vm.runtime is
head -50 src/utils/index.ts | grep -E "import|from|vm\."Repository: requestly/requestly-proxy
Length of output: 513
🏁 Script executed:
# Check package.json for quickjs-emscripten version
cat package.json | grep -A 5 -B 5 "quickjs"Repository: requestly/requestly-proxy
Length of output: 748
🏁 Script executed:
# Search for any type definitions or interfaces related to ExecutePendingJobs
rg "ExecutePendingJobs" --type json --type ts -A 5Repository: requestly/requestly-proxy
Length of output: 163
🏁 Script executed:
# Check if there's documentation or comments in the codebase about error disposal
rg "error.*dispose\|dispose.*error" --type ts -B 2 -A 2 | head -40Repository: requestly/requestly-proxy
Length of output: 163
🌐 Web query:
quickjs-emscripten ExecutePendingJobsResult type definition error handle dispose
💡 Result:
In quickjs-emscripten, ExecutePendingJobsResult is defined as a union type that represents the outcome of calling executePendingJobs [1]. It is structured as { value: number } | { error: QuickJSHandle } [1]. On success, the value field contains the number of async jobs executed by the runtime [1]. On error, the error field contains a handle to the exception (a QuickJSHandle) that stopped execution [1]. To handle and dispose of this result correctly: 1. Dispose of the Result: Because ExecutePendingJobsResult is a wrapper that may contain a QuickJSHandle (in the error case), it is critical to dispose of the result object or its contents to prevent memory leaks in the QuickJS VM [2][3]. If the result contains an error handle, you must dispose of that handle [4]. 2. Handling Errors: You can check if the result represents an error by inspecting the object properties. A common utility to handle such results is unwrapResult, which will either return the success value or convert the error handle to a native JavaScript object and throw it [1][2]. Example usage: const result = runtime.executePendingJobs; if ('error' in result) { // Handle or throw the error const error = context.dump(result.error); result.error.dispose; // Dispose the error handle throw new Error(String(error)); } else { // result.value is the number of jobs executed console.log(Executed ${result.value} jobs); } When iterating or polling, ensure that every call to executePendingJobs results in the disposal of its return object [4]. Failure to do so, especially when dealing with error handles, can lead to memory leaks or runtime assertion errors [4].
Citations:
- 1: https://cdn.jsdelivr.net/npm/esm-quickjs-emscripten@0.11.4/dist/quickjs.d.ts
- 2: https://cdn.jsdelivr.net/npm/quickjs-emscripten@0.24.0/dist/context.d.ts
- 3: https://github.com/justjake/quickjs-emscripten/blob/main/doc/quickjs-emscripten/classes/QuickJSContext.md
- 4: Reproducible list_empty assertion failure justjake/quickjs-emscripten#235
🏁 Script executed:
# Check if node_modules is available with type definitions
ls -la node_modules/quickjs-emscripten/ 2>/dev/null | head -20 || echo "node_modules not available"Repository: requestly/requestly-proxy
Length of output: 163
🏁 Script executed:
# Look for any comments in src/utils/index.ts that might explain the executePendingJobs behavior
sed -n '155,185p' src/utils/index.tsRepository: requestly/requestly-proxy
Length of output: 1130
Capture and dispose the executePendingJobs() error handle to prevent memory leaks.
When executePendingJobs() encounters a deadline interrupt or job error, it returns a result containing a QuickJSHandle that must be disposed to prevent VM memory leaks. The current code discards the result without checking.
🧹 Dispose the pending-jobs error handle
- // Resolve the user fn's (possibly async-but-IO-free) promise microtasks.
- vm.runtime.executePendingJobs();
+ // Resolve the user fn's (possibly async-but-IO-free) promise microtasks.
+ const jobs = vm.runtime.executePendingJobs();
+ if (jobs.error) {
+ jobs.error.dispose();
+ }📝 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.
| // Resolve the user fn's (possibly async-but-IO-free) promise microtasks. | |
| vm.runtime.executePendingJobs(); | |
| // Resolve the user fn's (possibly async-but-IO-free) promise microtasks. | |
| const jobs = vm.runtime.executePendingJobs(); | |
| if (jobs.error) { | |
| jobs.error.dispose(); | |
| } |
🤖 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 `@src/utils/index.ts` around lines 167 - 168, The call to executePendingJobs()
on the vm.runtime object returns a result containing a QuickJSHandle that must
be properly disposed to prevent VM memory leaks. Capture the return value from
executePendingJobs() and check if it contains an error handle, then ensure the
handle is disposed by calling the appropriate disposal method on the returned
result before proceeding. This will prevent memory leaks when deadline
interrupts or job errors occur.
1 participant
What
Sandboxes "Code"-type Modify Request/Response rules. They previously ran rule-supplied JS via
new Function(...)directly in the proxy's Node process (fullrequire/process/fs/child_process). Code rules travel between users (shared lists, import/export, team sync) → supply-chain RCE.Rule code now runs inside QuickJS compiled to WebAssembly (
quickjs-emscripten, single-file embedded variant).Why QuickJS-WASM
QuickJS is a separate JS engine in the WASM sandbox — no
require/process/fs/global, and no prototype path back to the host (constructor-escape blocked). Only explicitly-injected primitives are reachable. The two alternatives don't work here:worker_threadscannot create a Worker in an Electron renderer ("The V8 platform … does not support creating Workers"), and the proxy runs in the desktop app's background renderer.QuickJS-WASM is pure WASM+JS — builds nowhere natively and runs fine in the renderer (verified).
Changes
src/utils/index.ts:executeUserFunctionruns the rule in QuickJS — 5s deadline interrupt, 128 MB cap.isValidFunctionStringcompiles vianew Functionwithout calling it (parse-only, no execution; novm).getFunctionFromStringremoved.consolecaptured as{type,args},$sharedStateread + written back.Intentional gap
No
fetch/Buffer/timers in rule code.fetchwould need the asyncify QuickJS variant + an async host bridge — a follow-up (QuickJS can do it safely, unlike worker+vm).Verified
typeof process/require= undefined in-sandbox).Downstream
Desktop needs only a proxy dep bump (folded into #358) — no desktop code change. Ships alongside the Node-24
writeHeadfix (#113).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores