Skip to content

src: prepare for v8 sandboxing #58376

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

Closed

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented May 18, 2025

When the v8 sandbox is enabled (possibly at some point in the future) all ArrayBuffer allocations must be done within the isolate. Externally allocated backing stores are not permitted. This starts to prepare for that by adding some compile time branches to copy some external buffers rather than wrapping them. We also disable the --secure-heap feature since that fundamentally doesn't make sense and can't be supported with v8 sandbox enabled.

This PR cannot yet include additional tests for the new branches largely because we cannot yet build node with v8 sandbox enabled.

@jasnell jasnell requested review from anonrig, targos and joyeecheung May 18, 2025 17:52
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. 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 May 18, 2025
@jasnell jasnell force-pushed the jasnell/some-v8-sandbox-preparations branch from d21804d to 474cba4 Compare May 18, 2025 17:54
@nodejs-github-bot

This comment was marked as outdated.

@panva panva requested a review from tniessen May 18, 2025 18:33

This comment was marked as outdated.

@jasnell jasnell force-pushed the jasnell/some-v8-sandbox-preparations branch from 474cba4 to fd4f0e7 Compare May 18, 2025 18:56
@nodejs-github-bot

This comment was marked as outdated.

@jasnell jasnell force-pushed the jasnell/some-v8-sandbox-preparations branch from fd4f0e7 to d9af9b2 Compare May 19, 2025 00:09
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 19, 2025
@nodejs-github-bot

This comment was marked as outdated.

@jasnell jasnell force-pushed the jasnell/some-v8-sandbox-preparations branch from d9af9b2 to 48346dc Compare May 24, 2025 21:45
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@jasnell jasnell force-pushed the jasnell/some-v8-sandbox-preparations branch from 48346dc to 6514931 Compare May 26, 2025 13:54
@nodejs-github-bot
Copy link
Collaborator

jasnell added a commit that referenced this pull request May 26, 2025
PR-URL: #58376
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented May 26, 2025

Landed in a430ef5

@jasnell jasnell closed this May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants