Skip to content

src: ensure Socket(fd) cannot bypass allow-net permission - #66117

Open
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/permissions-socket-fd
Open

jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/permissions-socket-fd

Conversation

@jasnell

@jasnell jasnell commented Sep 18, 2026

Copy link
Copy Markdown
Member

An alternative to #63848, which appears to have stalled.

When using --permission, new Socket(fd) would not assert the --allow-net permission. It needs a bit of special handling to ensure stdio and IPC were handled appropriately.

Note: this was reported a while ago as a public PR but we've received multiple, duplicative hackerone reports on this that have been closed as informative. Since there's already a public PR there's no point in not opening this publicly.

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
@jasnell
jasnell requested a review from mcollina September 18, 2026 20:36
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net
  • @nodejs/security-wg

@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 involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 18, 2026

@mcollina mcollina 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

@jasnell jasnell added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 18, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 18, 2026
@nodejs-github-bot

This comment was marked as outdated.

@jasnell jasnell added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 18, 2026
@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.28%. Comparing base (dd5dfb5) to head (ad4447f).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/tcp_wrap.cc 50.00% 0 Missing and 2 partials ⚠️
src/env.cc 87.50% 0 Missing and 1 partial ⚠️
src/pipe_wrap.cc 66.66% 0 Missing and 1 partial ⚠️
src/stream_wrap.cc 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66117      +/-   ##
==========================================
+ Coverage   90.27%   90.28%   +0.01%     
==========================================
  Files         790      790              
  Lines      271651   271670      +19     
  Branches    51842    51850       +8     
==========================================
+ Hits       245228   245289      +61     
+ Misses      16928    16884      -44     
- Partials     9495     9497       +2     
Files with missing lines Coverage Δ
src/env-inl.h 93.95% <100.00%> (+0.02%) ⬆️
src/env.h 97.26% <ø> (ø)
src/stream_wrap.h 100.00% <ø> (ø)
src/env.cc 81.91% <87.50%> (-0.18%) ⬇️
src/pipe_wrap.cc 79.41% <66.66%> (-0.29%) ⬇️
src/stream_wrap.cc 88.28% <50.00%> (-0.33%) ⬇️
src/tcp_wrap.cc 68.24% <50.00%> (-0.21%) ⬇️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants