FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix an assertion failure at exit in the macOS app sandbox by wan9chi · Pull Request #33944 · nodejs/node · GitHub

/ node Public

Fix an assertion failure at exit in the macOS app sandbox - #33944

Closed
wan9chi wants to merge 5 commits into
nodejs:masterfrom
wan9chi:mac_app_sandbox_pr
ConversationCommits (5)ChecksFiles changed
Closed

Fix an assertion failure at exit in the macOS app sandbox#33944
wan9chi wants to merge 5 commits into
nodejs:masterfrom
wan9chi:mac_app_sandbox_pr

Conversation

wan9chi commented Jun 18, 2020

Copy link
Copy Markdown
Contributor

The node process raises at exit an assertion failure at

node/src/node.cc

Line 740 in a85ce88

CHECK_EQ(0, err);
when it runs in macOS app sandbox.

This PR includes

  • A basic test that verifies Node.js is able to run in the macOS app sandbox,
  • A fix of the problem described above,
  • A test of the fix.

Checklist

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

wan9chi added 2 commits June 18, 2020 22:51
src: tolerate EPERM returned from tcsetattr
317621b 
macOS app sandbox makes tcsetattr return EPERM. The CHECK_EQ(0, err) here would fail when a sandboxed Node.js process is exiting. This commit fixes this issue.
test: add test for running in macOS app sandbox
5f4f969 
Bare-bone command-line executables cannot run directly in the app sandbox. To test that Node.js is able to run in the sandbox (and to test the fix in 317621b), this commit creates a typical Cocoa app bundle, puts the node executable in it and calles Apple's codesign command to enable sandbox.
nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 18, 2020
richardlau reviewed Jun 18, 2020
View reviewed changes
Comment thread test/parallel/test-macos-app-sandbox.js Outdated

richardlau commented Jun 18, 2020

Copy link
Copy Markdown
Member

This looks okay to me, but I'm not really a macOS person.

cc @nodejs/platform-macos

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jun 25, 2020

nodejs-github-bot commented Jun 25, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32076/

jasnell pushed a commit that referenced this pull request Jun 25, 2020
src: tolerate EPERM returned from tcsetattr
cb2c810 
macOS app sandbox makes tcsetattr return EPERM. The CHECK_EQ(0, err)
here would fail when a sandboxed Node.js process is exiting. This commit
fixes this issue.

* test: add test for running in macOS app sandbox

Bare-bone command-line executables cannot run directly in the app sandbox.
To test that Node.js is able to run in the sandbox (and to test the fix in
317621b), this commit creates a typical
Cocoa app bundle, puts the node executable in it and calles Apple's codesign
command to enable sandbox.

* test: use process.execPath to get path of testing node

PR-URL: #33944
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell commented Jun 25, 2020

Copy link
Copy Markdown
Member

Landed in cb2c810

jasnell closed this Jun 25, 2020
codebytere pushed a commit that referenced this pull request Jun 27, 2020
src: tolerate EPERM returned from tcsetattr
da81930 
macOS app sandbox makes tcsetattr return EPERM. The CHECK_EQ(0, err)
here would fail when a sandboxed Node.js process is exiting. This commit
fixes this issue.

* test: add test for running in macOS app sandbox

Bare-bone command-line executables cannot run directly in the app sandbox.
To test that Node.js is able to run in the sandbox (and to test the fix in
317621b), this commit creates a typical
Cocoa app bundle, puts the node executable in it and calles Apple's codesign
command to enable sandbox.

* test: use process.execPath to get path of testing node

PR-URL: #33944
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere mentioned this pull request Jun 28, 2020
Merged
codebytere pushed a commit that referenced this pull request Jun 30, 2020
src: tolerate EPERM returned from tcsetattr
2510ffc 
macOS app sandbox makes tcsetattr return EPERM. The CHECK_EQ(0, err)
here would fail when a sandboxed Node.js process is exiting. This commit
fixes this issue.

* test: add test for running in macOS app sandbox

Bare-bone command-line executables cannot run directly in the app sandbox.
To test that Node.js is able to run in the sandbox (and to test the fix in
317621b), this commit creates a typical
Cocoa app bundle, puts the node executable in it and calles Apple's codesign
command to enable sandbox.

* test: use process.execPath to get path of testing node

PR-URL: #33944
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 10, 2020
src: tolerate EPERM returned from tcsetattr
46da871 
macOS app sandbox makes tcsetattr return EPERM. The CHECK_EQ(0, err)
here would fail when a sandboxed Node.js process is exiting. This commit
fixes this issue.

* test: add test for running in macOS app sandbox

Bare-bone command-line executables cannot run directly in the app sandbox.
To test that Node.js is able to run in the sandbox (and to test the fix in
317621b), this commit creates a typical
Cocoa app bundle, puts the node executable in it and calles Apple's codesign
command to enable sandbox.

* test: use process.execPath to get path of testing node

PR-URL: #33944
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 12, 2020
src: tolerate EPERM returned from tcsetattr
b57778f 
macOS app sandbox makes tcsetattr return EPERM. The CHECK_EQ(0, err)
here would fail when a sandboxed Node.js process is exiting. This commit
fixes this issue.

* test: add test for running in macOS app sandbox

Bare-bone command-line executables cannot run directly in the app sandbox.
To test that Node.js is able to run in the sandbox (and to test the fix in
317621b), this commit creates a typical
Cocoa app bundle, puts the node executable in it and calles Apple's codesign
command to enable sandbox.

* test: use process.execPath to get path of testing node

PR-URL: #33944
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere mentioned this pull request Jul 13, 2020
Merged
codebytere pushed a commit that referenced this pull request Jul 14, 2020
src: tolerate EPERM returned from tcsetattr
d16e51b 
macOS app sandbox makes tcsetattr return EPERM. The CHECK_EQ(0, err)
here would fail when a sandboxed Node.js process is exiting. This commit
fixes this issue.

* test: add test for running in macOS app sandbox

Bare-bone command-line executables cannot run directly in the app sandbox.
To test that Node.js is able to run in the sandbox (and to test the fix in
317621b), this commit creates a typical
Cocoa app bundle, puts the node executable in it and calles Apple's codesign
command to enable sandbox.

* test: use process.execPath to get path of testing node

PR-URL: #33944
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

This test seems to be reliably failing in GitHub actions

richardlau commented Jul 21, 2020

Copy link
Copy Markdown
Member

This test seems to be reliably failing in GitHub actions

There's #34331.

cjihrig added a commit to cjihrig/node that referenced this pull request Jul 22, 2020
test: force resigning of app
e7ed066 
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: nodejs#33944
PR-URL: nodejs#34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
cjihrig added a commit that referenced this pull request Jul 23, 2020
test: force resigning of app
eba2ae4 
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: #33944
PR-URL: #34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Jul 27, 2020
test: force resigning of app
cd35d00 
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: #33944
PR-URL: #34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
test: force resigning of app
771c3e4 
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: #33944
PR-URL: #34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
test: force resigning of app
26c5f9f 
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: #33944
PR-URL: #34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
robobun mentioned this pull request Feb 14, 2026
Closed
4 tasks
robobun pushed a commit to oven-sh/bun that referenced this pull request Feb 14, 2026
fix(process): support running Bun inside macOS App Sandbox
1af1095 
Fix two issues in `bun_initialize_process()` that prevent Bun from
running inside a macOS App Sandbox (`com.apple.security.app-sandbox`):

1. Move `bun_stdio_tty[fd] = 1` inside the `tcgetattr` success check.
   In the macOS App Sandbox, `tcgetattr` fails with EPERM even though
   `isatty()` returns true. Previously, the TTY flag was set
   unconditionally, causing `bun_restore_stdio()` to call `tcsetattr`
   with uninitialized termios state at exit. This is the same class of
   bug Node.js fixed in nodejs/node#33944.

2. Fix `dup2` return value check: `dup2(oldfd, newfd)` returns `newfd`
   on success (not 0), so `err != 0` incorrectly triggered `abort()`
   for stdout/stderr. Changed to `err < 0` and replaced `abort()` with
   a graceful fallback. Also handle `open("/dev/null")` failure
   gracefully for restricted environments.

Closes #15661

Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

richardlau richardlau left review comments
jasnell jasnell approved these changes
addaleax addaleax approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL