Skip to content

deps: V8: respect interceptors in lexical restricted-global check#63718

Draft
3zrv wants to merge 2 commits into
nodejs:mainfrom
3zrv:regression/deps/vm
Draft

deps: V8: respect interceptors in lexical restricted-global check#63718
3zrv wants to merge 2 commits into
nodejs:mainfrom
3zrv:regression/deps/vm

Conversation

@3zrv
Copy link
Copy Markdown
Contributor

@3zrv 3zrv commented Jun 2, 2026

fixes: #63715

A top-level lexical declaration (let/const) that collides with a non-configurable ("restricted") own property on a vm context's global object is no longer rejected with a SyntaxError, violating ES#sec-globaldeclarationinstantiation step 5.d.

This is a regression between v24.x and v26.x, found via the test262 case language/global-code/script-decl-lex-restricted-global.js.

const vm = require('vm');
const context = vm.createContext({});
vm.runInContext(
  "Object.defineProperty(this, 'foo', { value: 1, configurable: false });",
  context);
vm.runInContext('let foo;', context);
// v24.12.0: SyntaxError: Identifier 'foo' has already been declared
// v26.3.0:  silently accepted  <-- bug

Root cause

V8's Execution::NewScriptContext (deps/v8/src/execution/execution.cc) checks for a restricted global with LookupIterator::OWN_SKIP_INTERCEPTOR. vm globals are interceptor-backed and keep script-defined properties on a sandbox object, so skipping interceptors hides them and the collision goes undetected. The sibling var/function path in runtime-scopes.cc already keeps interceptors for the non-var case; the lexical path skipping them is the asymmetry that regressed.

Fix

Switch that lookup to LookupIterator::OWN (interceptor-respecting) and propagate a possible pending exception instead of CHECK-ing. No-op for ordinary globals; restores correct behavior for vm contexts.

Upstreaming

This is an interim floating patch; upstreaming to V8 is tracked/forthcoming

Testing

  • Added a regression test to test/parallel/test-vm-global-property-interceptors.js.
  • The repro now throws SyntaxError: Identifier 'foo' has already been declared.
  • All test/parallel/test-vm-* tests pass against a local release build.

Mohamed Sayed added 2 commits June 2, 2026 22:28
Signed-off-by: Mohamed Sayed <k@3zrv.com>
Assert that a top-level `let`/`const` declaration colliding with a
non-configurable property on a vm context's global object throws a
SyntaxError, and that a configurable property does not block the
declaration.

Refs: test262 language/global-code/script-decl-lex-restricted-global.js

Signed-off-by: Mohamed Sayed <k@3zrv.com>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Jun 2, 2026
@Renegade334
Copy link
Copy Markdown
Member

This is an interim floating patch; upstreaming to V8 is tracked/forthcoming

That's not how it works; see https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-V8.md#unfixed-upstream-bugs

@3zrv 3zrv marked this pull request as draft June 2, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vm: let declaration no longer throws SyntaxError for non-configurable global property in vm context (regression in v26)

3 participants