[PR #3973] [MERGED] chore: migrate `Node.js` implementation for `js-sandbox` to `isolated-vm` #4620

New issue

Closed

opened 2026-03-17 02:08:18 +03:00 by kerem · 0 comments

kerem commented

2026-03-17 02:08:18 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hoppscotch/hoppscotch/pull/3973
Author: @jamesgeorge007
Created: 4/12/2024
Status: ✅ Merged
Merged: 4/19/2024
Merged by: @AndrewBastin

Base: release/2024.3.1 ← Head: sandbox-vulnerability-remediation

📝 Commits (10+)

5cc8716 chore: migrate JS sandbox to isolated-vm
23e959a fix: pre-request script execution
bac9743 refactor: serialize test script API methods before sending to the isolate
c2bd74a chore: migrate test framework to vitest
d75a76d chore: resolve build errors wrt wrt CJS interop with ESM
318402d refactor: leverage Proxy to intercept scripting API calls
023072e refactor: spawn child process to add no-node-snapshot flag when required
0e2d2d0 chore: bump jest timeout for test.spec.ts
27d58c0 fix: condition to spawn child process
7eca5ba chore: make isolated-vm a peer-dependency

📊 Changes

52 files changed (+1029 additions, -286 deletions)

View changed files

📝 .github/workflows/tests.yml (+7 -8)
📝 packages/hoppscotch-cli/bin/hopp.js (+26 -1)
📝 packages/hoppscotch-cli/package.json (+2 -1)
📝 packages/hoppscotch-cli/src/__tests__/commands/test.spec.ts (+1 -1)
📝 packages/hoppscotch-cli/src/__tests__/samples/collections/coll-v1-req-v0.json (+53 -25)
📝 packages/hoppscotch-cli/src/__tests__/samples/collections/coll-v1-req-v1.json (+52 -3)
📝 packages/hoppscotch-cli/src/__tests__/samples/collections/coll-v2-req-v2.json (+58 -3)
📝 packages/hoppscotch-cli/src/__tests__/samples/collections/coll-v2-req-v3.json (+58 -3)
📝 packages/hoppscotch-cli/src/__tests__/samples/collections/secret-envs-coll.json (+51 -21)
📝 packages/hoppscotch-cli/src/__tests__/samples/collections/secret-envs-persistence-coll.json (+13 -13)
📝 packages/hoppscotch-cli/src/__tests__/samples/collections/secret-envs-persistence-scripting-coll.json (+2 -2)
📝 packages/hoppscotch-cli/src/__tests__/samples/environments/secret-envs.json (+6 -1)
📝 packages/hoppscotch-cli/src/__tests__/samples/environments/secret-supplied-values-envs.json (+6 -1)
📝 packages/hoppscotch-cli/src/__tests__/utils.ts (+18 -11)
📝 packages/hoppscotch-cli/src/index.ts (+2 -1)
📝 packages/hoppscotch-cli/src/utils/mutators.ts (+36 -18)
➖ packages/hoppscotch-js-sandbox/jest.config.js (+0 -10)
➖ packages/hoppscotch-js-sandbox/jest.setup.ts (+0 -1)
📝 packages/hoppscotch-js-sandbox/node.d.ts (+2 -2)
📝 packages/hoppscotch-js-sandbox/package.json (+11 -3)

...and 32 more files

📄 Description

Description

This PR aims at migrating the sandbox implementation for Node.js from node:vm to isolated-vm.

Ref: https://github.com/hoppscotch/hoppscotch/security/advisories/GHSA-qmmm-73r2-f8xr

Closes HFE-475.

Changes

Add isolated-vm as a peer dependency (there were issues with not being able to require isolated-vm when it was made a direct dependency) to js-sandbox and mark it optional since it's specific to the Node.js implementation. Also, it is added as a direct dependency on the CLI.
Migrate the testing framework from Jest (support for ESM is experimental) to Vitest for js-sandbox.
Directory hierarchy updates for js-sandbox.
- Arrange the source directories based on the platform (node, web) instead of the type of scripts (pre-request & test-runner) easily accommodating platform-specific utility functions. ~/src/utils.ts at the root compiling utility functions reused across platform implementations is renamed to ~/src/shared-utils to differentiate between platform-specific utils ~/src/node/utils.ts.
- Remove the nested testing directory from the test suite and enforce a naming convention for the directories based on the scripting API namespace objects (env, expect, etc).
The value supplied to pw.expect(expectVal) is stringified with an additional property isStringifiedWithinIsolate if it's a non-primitive while transferring to the isolate context as required by isolated-vm. The value is parsed at the pw.expect method definition after checking for the above additional property to differentiate between stringification done while transferring to the isolate context and an original value. The newly introduced property is removed before consumption.
Spawn a new process if the Node.js version is 20 and above from the CLI supplying the --no-node-snapshot flag to node as required by isolated-vm, Ref and later proceed with the argument parsing phase.
Account for requests nested under folders within collections while validating against the versioned entity schema and translating to the latest version. Follow up of #3912.
Fix flaky tests in the CLI (update tests to refer to the echo.hoppscotch.io endpoint and prevent test failures if the auth endpoint resulted in a non 200 status code).
GH Action config updates
- Bump dependent actions.
- Fix a typo while referring to the Node.js version and ensure run tests on LTS.
- Move the Setup node step above Setup pnpm.

Checks

My pull request adheres to the code style of this project
All the tests have passed

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/hoppscotch/hoppscotch/pull/3973 **Author:** [@jamesgeorge007](https://github.com/jamesgeorge007) **Created:** 4/12/2024 **Status:** ✅ Merged **Merged:** 4/19/2024 **Merged by:** [@AndrewBastin](https://github.com/AndrewBastin) **Base:** `release/2024.3.1` ← **Head:** `sandbox-vulnerability-remediation` --- ### 📝 Commits (10+) - [`5cc8716`](https://github.com/hoppscotch/hoppscotch/commit/5cc8716457feb6e2127d51364aaa0cb2518bff97) chore: migrate JS sandbox to isolated-vm - [`23e959a`](https://github.com/hoppscotch/hoppscotch/commit/23e959a390fee12e904195f49e94174f114ecfe4) fix: pre-request script execution - [`bac9743`](https://github.com/hoppscotch/hoppscotch/commit/bac974378f5095dd0a0e4e389016d0617960a574) refactor: serialize test script API methods before sending to the isolate - [`c2bd74a`](https://github.com/hoppscotch/hoppscotch/commit/c2bd74a331b5e3c6fb063c7b07a55f461b3af960) chore: migrate test framework to vitest - [`d75a76d`](https://github.com/hoppscotch/hoppscotch/commit/d75a76d2a94f0ff3019fb5f1eaed8a4b145bc29b) chore: resolve build errors wrt wrt CJS interop with ESM - [`318402d`](https://github.com/hoppscotch/hoppscotch/commit/318402d55960cdc52b542de87c8e52a88b9845de) refactor: leverage Proxy to intercept scripting API calls - [`023072e`](https://github.com/hoppscotch/hoppscotch/commit/023072e5075202464a6fcf7338754c5ab142cc76) refactor: spawn child process to add no-node-snapshot flag when required - [`0e2d2d0`](https://github.com/hoppscotch/hoppscotch/commit/0e2d2d0f5ed4a3ef408c5c90d643358d2868eaca) chore: bump jest timeout for test.spec.ts - [`27d58c0`](https://github.com/hoppscotch/hoppscotch/commit/27d58c0f48de738797d07588727b8dae19287b56) fix: condition to spawn child process - [`7eca5ba`](https://github.com/hoppscotch/hoppscotch/commit/7eca5bac84369784fbe7c9195e35193b53f40bd8) chore: make `isolated-vm` a peer-dependency ### 📊 Changes **52 files changed** (+1029 additions, -286 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/tests.yml` (+7 -8) 📝 `packages/hoppscotch-cli/bin/hopp.js` (+26 -1) 📝 `packages/hoppscotch-cli/package.json` (+2 -1) 📝 `packages/hoppscotch-cli/src/__tests__/commands/test.spec.ts` (+1 -1) 📝 `packages/hoppscotch-cli/src/__tests__/samples/collections/coll-v1-req-v0.json` (+53 -25) 📝 `packages/hoppscotch-cli/src/__tests__/samples/collections/coll-v1-req-v1.json` (+52 -3) 📝 `packages/hoppscotch-cli/src/__tests__/samples/collections/coll-v2-req-v2.json` (+58 -3) 📝 `packages/hoppscotch-cli/src/__tests__/samples/collections/coll-v2-req-v3.json` (+58 -3) 📝 `packages/hoppscotch-cli/src/__tests__/samples/collections/secret-envs-coll.json` (+51 -21) 📝 `packages/hoppscotch-cli/src/__tests__/samples/collections/secret-envs-persistence-coll.json` (+13 -13) 📝 `packages/hoppscotch-cli/src/__tests__/samples/collections/secret-envs-persistence-scripting-coll.json` (+2 -2) 📝 `packages/hoppscotch-cli/src/__tests__/samples/environments/secret-envs.json` (+6 -1) 📝 `packages/hoppscotch-cli/src/__tests__/samples/environments/secret-supplied-values-envs.json` (+6 -1) 📝 `packages/hoppscotch-cli/src/__tests__/utils.ts` (+18 -11) 📝 `packages/hoppscotch-cli/src/index.ts` (+2 -1) 📝 `packages/hoppscotch-cli/src/utils/mutators.ts` (+36 -18) ➖ `packages/hoppscotch-js-sandbox/jest.config.js` (+0 -10) ➖ `packages/hoppscotch-js-sandbox/jest.setup.ts` (+0 -1) 📝 `packages/hoppscotch-js-sandbox/node.d.ts` (+2 -2) 📝 `packages/hoppscotch-js-sandbox/package.json` (+11 -3) _...and 32 more files_ </details> ### 📄 Description ### Description This PR aims at migrating the sandbox implementation for `Node.js` from [node:vm](https://nodejs.org/api/vm.html#vm-executing-javascript) to [isolated-vm](https://github.com/laverdet/isolated-vm). Ref: https://github.com/hoppscotch/hoppscotch/security/advisories/GHSA-qmmm-73r2-f8xr Closes HFE-475. ### Changes - Add `isolated-vm` as a peer dependency (there were issues with not being able to require `isolated-vm` when it was made a direct dependency) to `js-sandbox` and mark it optional since it's specific to the `Node.js` implementation. Also, it is added as a direct dependency on the CLI. - Migrate the testing framework from `Jest` (support for ESM is experimental) to `Vitest` for `js-sandbox`. - Directory hierarchy updates for `js-sandbox`. - Arrange the source directories based on the platform (`node`, `web`) instead of the type of scripts (`pre-request` & `test-runner`) easily accommodating platform-specific utility functions. `~/src/utils.ts` at the root compiling utility functions reused across platform implementations is renamed to `~/src/shared-utils` to differentiate between platform-specific utils `~/src/node/utils.ts`. - Remove the nested `testing` directory from the test suite and enforce a naming convention for the directories based on the scripting API namespace objects (`env`, `expect`, etc). - The value supplied to `pw.expect(expectVal)` is stringified with an additional property `isStringifiedWithinIsolate` if it's a non-primitive while transferring to the isolate context as required by `isolated-vm`. The value is parsed at the `pw.expect` method definition after checking for the above additional property to differentiate between stringification done while transferring to the isolate context and an original value. The newly introduced property is removed before consumption. - Spawn a new process if the `Node.js` version is `20` and above from the CLI supplying the `--no-node-snapshot` flag to `node` as required by `isolated-vm`, [Ref](https://github.com/laverdet/isolated-vm?tab=readme-ov-file#requirements) and later proceed with the argument parsing phase. - Account for requests nested under `folders` within collections while validating against the versioned entity schema and translating to the latest version. Follow up of #3912. - Fix flaky tests in the CLI (update tests to refer to the [echo.hoppscotch.io](https://echo.hoppscotch.io/) endpoint and prevent test failures if the auth endpoint resulted in a non `200` status code). - GH Action config updates - Bump dependent actions. - Fix a typo while referring to the Node.js version and ensure run tests on LTS. - Move the `Setup node` step above `Setup pnpm`. ### Checks - [x] My pull request adheres to the code style of this project - [x] All the tests have passed --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>