Skip to content

Disable node integration in worker #8822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 30 commits into from
Jul 14, 2025

Conversation

jackkav
Copy link
Contributor

@jackkav jackkav commented Jun 30, 2025

motivation: turn web worker into a sandbox capable of limiting what can be done from a nunjucks crafted tag. eg {{ require('fs').rmdir('/Users') }}

approach: build fewest bridges as possible to limit exploitable surface, therefore maximise use of available browser apis.

consequences of this approach:

  • deprecating md5 support because its no longer supporting in browsers (see wikipedia md5 page)
  • moving spectral to utility process slows it down a bit but also solves bundling ambiguity
  • I chose to keep iconv-lite for decoding since i cant be sure how its being used and it doesn't seem risky to expose
  • jsonpath runs in a safer place now

todo

  • use polyfills where possible (url,uuid,crypto)
  • create bridges for required node/npm modules
  • tidy up require interceptor and vite config
  • fix spectral lint
  • fix elevated extensions
  • test bridges
  • test polyfills
  • abstract fetch bridge to ease typing and avoid typos
  • review bridge granularity in light of plugin api goals
  • move jsonpath-plus to devDeps in order to use browser versions
  • scope out spectral changes and find a simple way to degrade
  • avoid out the nothing true workaround

For later

  • still offers readFile, we could limit paths?
  • still offers email from os.userInfo()

issues

  • spectral doesn't play nice with our settings so they don't resolve to browser versions when we try to use them in the worker directly, complaining of missing require
  • playwright timeouts, previously we were timing out the action which discarded any progress, i moved the timeout to playwright and made the action timeout longer so it didn't discard time outs.

@cwangsmv
Copy link
Contributor

cwangsmv commented Jul 7, 2025

Request => URL & Request => Cookie tag will fail.
It seemed that with nodeIntegrationInWorker: false, there is no document API in Webwork which leads to the failure of smartEncodeUrl used for these two tags

@jackkav jackkav force-pushed the disable-node-integration branch from 28eeb45 to 49ad3cf Compare July 7, 2025 11:41
@jackkav jackkav marked this pull request as ready for review July 7, 2025 12:56
@jackkav jackkav force-pushed the disable-node-integration branch from 1c7d0c3 to 3eaa758 Compare July 11, 2025 12:25
@jackkav jackkav force-pushed the disable-node-integration branch from 7140033 to 86ba77f Compare July 14, 2025 10:54
@jackkav jackkav enabled auto-merge (squash) July 14, 2025 16:01
@jackkav jackkav merged commit 5f60b01 into Kong:develop Jul 14, 2025
10 checks passed
@jackkav jackkav deleted the disable-node-integration branch July 14, 2025 16:10
RoamingLost pushed a commit to RoamingLost/insomnia that referenced this pull request Aug 6, 2025
* simplify out tough-cookie

* remove fs from vite config

* bridge fs os and decode

* polyfill crypto and uuid

* replace node:url

* remove require interceptor

* bridge jsonpath

* disable node in worker

* fix elevated extension

* remove spectral optimzation

* abstract and type db router

* complete abstraction

* add info about dev deps

* revert encode url

* fix and extend tests

* use jsonpath-plus import esm

* fix type check

* hide the openapi spam

* rename readFile

* optimise import

* fix md5 test

* speed up grpc test

* fix grpc test

* use global timeout

* fix lint

* fix tests

* fix types

* complete os support

* fix test

* update nodeOS
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.

3 participants