Skip to content

vm: enable interception on global restricted properties#64202

Open
legendecas wants to merge 2 commits into
nodejs:mainfrom
legendecas:v8-vm-restricted-global
Open

vm: enable interception on global restricted properties#64202
legendecas wants to merge 2 commits into
nodejs:mainfrom
legendecas:v8-vm-restricted-global

Conversation

@legendecas

Copy link
Copy Markdown
Member

deps: V8: backport a05321ebd98e

Original commit message:

[execution] Lookup interceptor for RestrictedGlobalProperty

When the script context looks up if a global property is
restricted, it should also query the global interceptor.

To avoid calling the interceptor for every declaration
unconditionally, an interceptor has to be defined with
`PropertyHandlerFlags::kHasDontDeleteProperty`
to intercept restricted global property queries.

Refs: https://github.com/nodejs/node/issues/63715
Refs: https://github.com/nodejs/node/issues/52634

Change-Id: I623ff285c4e8773d8ee7f681cbad68ba24bd3f40
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7898818
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#108280}

Refs: v8/v8@a05321e

vm: enable interception on global restricted properties

Fixes: #63715

Original commit message:

    [execution] Lookup interceptor for RestrictedGlobalProperty

    When the script context looks up if a global property is
    restricted, it should also query the global interceptor.

    To avoid calling the interceptor for every declaration
    unconditionally, an interceptor has to be defined with
    `PropertyHandlerFlags::kHasDontDeleteProperty`
    to intercept restricted global property queries.

    Refs: nodejs#63715
    Refs: nodejs#52634

    Change-Id: I623ff285c4e8773d8ee7f681cbad68ba24bd3f40
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7898818
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Igor Sheludko <ishell@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#108280}

Refs: v8/v8@a05321e
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
@nodejs-github-bot

nodejs-github-bot commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

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

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jun 29, 2026
@legendecas legendecas added the v8 engine Issues and PRs related to the V8 dependency. label Jun 29, 2026
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
@legendecas legendecas force-pushed the v8-vm-restricted-global branch from 93e842e to e5fe063 Compare June 30, 2026 18:21
@legendecas legendecas added the vm Issues and PRs related to the vm subsystem. label Jul 1, 2026

@joyeecheung joyeecheung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I think this counts as semver-major?

@joyeecheung joyeecheung added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jul 2, 2026
@joyeecheung

Copy link
Copy Markdown
Member

(Technically the V8 commit isn't semver-major)

@legendecas

legendecas commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

I'd count this as a fix to #63549 and the V8 API bit change is not ABI breaking (it's addition only).

I marked #63549 as v26.x only, so marking this as do-not-backport to earlier than v26.x, unless otherwise disagree?

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

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.

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