Skip to content

deps,test: fix undici globals on frozen globalThis#64622

Closed
mhayk wants to merge 1 commit into
nodejs:mainfrom
mhayk:fix/undici-frozen-globalthis-dispatcher
Closed

deps,test: fix undici globals on frozen globalThis#64622
mhayk wants to merge 1 commit into
nodejs:mainfrom
mhayk:fix/undici-frozen-globalthis-dispatcher

Conversation

@mhayk

@mhayk mhayk commented Jul 20, 2026

Copy link
Copy Markdown

Summary

When Object.freeze(globalThis) is called before any undici-backed global (fetch, WebSocket, Response, etc.) is accessed for the first time, the lazy initialisation inside setGlobalDispatcher throws:

TypeError: Cannot define property Symbol(undici.globalDispatcher.2),
           object is not extensible

This is particularly problematic because the Node.js security best practices guide explicitly recommends Object.freeze(globalThis) as a defence against monkey-patching (CWE-349).

Root cause

setGlobalDispatcher unconditionally calls Object.defineProperty(globalThis, ...). When globalThis is not extensible the call throws instead of degrading gracefully.

This became observable after globals were made lazy-loaded in #45659.

Fix

Wrap the Object.defineProperty calls in setGlobalDispatcher with a try/catch. When globalThis is not extensible the dispatcher is stored in a module-level fallback variable. getGlobalDispatcher is updated to return globalThis[symbol] ?? fallbackDispatcher so the normal (extensible) path is unchanged.

deps/undici/undici.js (the bundled file) is updated to match the source change in deps/undici/src/lib/global.js.

Test

A new test test/parallel/test-undici-frozen-globalthis.js is added that:

  1. Calls Object.freeze(globalThis) before any undici globals are touched
  2. Asserts that fetch, WebSocket, Response, Request, Headers, and FormData are all accessible without throwing

Checklist

  • make -j<N> build succeeds
  • New test passes: ./node test/parallel/test-undici-frozen-globalthis.js
  • Existing fetch/WebSocket tests pass
  • Normal setGlobalDispatcher / getGlobalDispatcher behaviour is unchanged

Fixes #46788

When Object.freeze(globalThis) is called before any undici-backed global
(fetch, WebSocket, Response, etc.) is accessed, the lazy initialisation
in setGlobalDispatcher throws:

  TypeError: Cannot define property Symbol(undici.globalDispatcher.2),
             object is not extensible

This is a regression introduced when globals were made lazy-loaded
(nodejs#45659). The Node.js security best practices guide explicitly
recommends Object.freeze(globalThis) as a monkey-patching defence
(CWE-349), so this failure is particularly unfortunate.

Fix: wrap the Object.defineProperty calls in setGlobalDispatcher with a
try/catch. When globalThis is not extensible the dispatcher is stored in
a module-level fallback variable. getGlobalDispatcher is updated to
return globalThis[symbol] ?? fallbackDispatcher so that the normal path
(extensible globalThis) is unchanged.

The bundled deps/undici/undici.js is updated to match.

A test is added in test/parallel/test-undici-frozen-globalthis.js that
freezes globalThis before accessing any of the affected globals.

Fixes: nodejs#46788
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Jul 20, 2026
@shrujalshah28

Copy link
Copy Markdown
Contributor

This change should go to - https://github.com/nodejs/undici, and once it has a new version with your changes merged, it will be included here.

@mhayk

mhayk commented Jul 20, 2026

Copy link
Copy Markdown
Author

Thanks for the feedback! You're absolutely right, the fix should go upstream first.

I'll close this PR and create a PR in the undici repository with the same fix. Once it's merged and a new version is released, it can be integrated into Node.js via the standard dependency update process.

Closing in favour of upstream fix.

@mhayk mhayk closed this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot redefine property when accessing properties of freezed global

3 participants