Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • updated rce to use isolate pkg for RCE

Type of Change

  • Other: Security

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Dec 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
docs Ready Ready Preview, Comment Dec 15, 2025 9:27pm

@waleedlatif1 waleedlatif1 merged commit 17b2e58 into staging Dec 15, 2025
5 of 6 checks passed
@waleedlatif1 waleedlatif1 deleted the improvement/rce branch December 15, 2025 21:26
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 15, 2025

Greptile Overview

Greptile Summary

This PR replaces the Node.js vm module with the isolated-vm package for remote code execution (RCE), providing a significant security improvement.

Key Changes:

  • Introduces executeInIsolatedVM() function using isolated-vm v6.0.2 which creates true V8 isolates with 128MB memory limit
  • Provides proper sandboxing that prevents prototype chain escape attacks (e.g., this.constructor.constructor("return process")())
  • Implements secure fetch bridge using ivm.Reference for async callbacks with existing SSRF validation
  • Sets up console.log/error/warn/info via ivm.Callback for stdout capture
  • Blocks dangerous globals (process, require, module, etc.) using non-writable/non-configurable property definitions
  • Maintains E2B integration for Python code and JavaScript with imports

Security Improvements:

  • True V8 isolate separation prevents Node.js API access from sandboxed code
  • Memory limits prevent resource exhaustion attacks
  • Timeout enforcement for execution time limits
  • SSRF protection maintained in the new secureFetch wrapper

Confidence Score: 4/5

  • This PR is a well-structured security improvement that replaces a potentially vulnerable sandbox with a more secure isolation mechanism.
  • Score of 4 reflects solid implementation with proper security controls (SSRF validation, memory limits, timeout handling, global blocking). Minor considerations around error handling edge cases but overall low risk.
  • Pay attention to apps/sim/lib/execution/isolated-vm.ts for the core sandboxing logic.

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/lib/execution/isolated-vm.ts 4/5 New file implementing secure JavaScript execution using isolated-vm package. Provides true V8 isolate sandboxing with SSRF protection via secureFetch, console/fetch APIs, and proper error handling.
apps/sim/app/api/function/execute/route.ts 4/5 Refactored to use executeInIsolatedVM instead of Node.js vm module. Removes createSecureFetch (moved to isolated-vm.ts). Handles isolated-vm error responses properly with enhanced error formatting.
apps/sim/app/api/function/execute/route.test.ts 4/5 Updated tests to mock executeInIsolatedVM instead of Node.js vm module. Added new security tests for VM escape prevention, process/require exposure. Test assertions adjusted for new execution model.
apps/sim/next.config.ts 5/5 Added 'isolated-vm' to serverExternalPackages array to prevent bundling of native module.
package.json 5/5 Added isolated-vm 6.0.2 dependency at workspace root level.
bun.lock 5/5 Lock file updated with isolated-vm 6.0.2 and its dependencies. Added isolated-vm to trustedDependencies for native module installation.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Route as POST /api/function/execute
    participant IsolatedVM as executeInIsolatedVM
    participant Isolate as V8 Isolate
    participant SecureFetch as secureFetch

    Client->>Route: POST { code, params, envVars, ... }
    Route->>Route: Resolve variables in code
    Route->>IsolatedVM: Execute code request
    
    IsolatedVM->>Isolate: Create V8 Isolate (128MB limit)
    IsolatedVM->>Isolate: Create context
    IsolatedVM->>Isolate: Set up globals (params, envVars, console)
    IsolatedVM->>Isolate: Set up __fetchRef callback
    IsolatedVM->>Isolate: Run bootstrap (console, fetch, block globals)
    IsolatedVM->>Isolate: Compile and run user code

    alt User code calls fetch
        Isolate->>SecureFetch: __fetchRef.apply(url, options)
        SecureFetch->>SecureFetch: validateProxyUrl (SSRF check)
        SecureFetch-->>Isolate: JSON response
    end

    alt Success
        Isolate-->>IsolatedVM: JSON result
        IsolatedVM-->>Route: { result, stdout }
        Route-->>Client: 200 { success: true, output }
    else Error
        Isolate-->>IsolatedVM: Error info
        IsolatedVM-->>Route: { error, stdout }
        Route->>Route: Format enhanced error
        Route-->>Client: 500 { success: false, error }
    end

    IsolatedVM->>Isolate: dispose()
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants