Skip to content

Commit

Permalink
src: tolerate EPERM returned from tcsetattr
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
branchseer authored and codebytere committed Jul 14, 2020
1 parent 25023cf commit d16e51b
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/node.cc
Expand Up @@ -634,7 +634,10 @@ void ResetStdio() {
err = tcsetattr(fd, TCSANOW, &s.termios);
while (err == -1 && errno == EINTR); // NOLINT
CHECK_EQ(0, pthread_sigmask(SIG_UNBLOCK, &sa, nullptr));
CHECK_EQ(0, err);

// Normally we expect err == 0. But if macOS App Sandbox is enabled,
// tcsetattr will fail with err == -1 and errno == EPERM.
CHECK_IMPLIES(err != 0, err == -1 && errno == EPERM);
}
}
#endif // __POSIX__
Expand Down
24 changes: 24 additions & 0 deletions test/fixtures/macos-app-sandbox/Info.plist
@@ -0,0 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>CFBundleExecutable</key>
<string>node</string>
<key>CFBundleIdentifier</key>
<string>org.nodejs.test.node_sandboxed</string>
<key>CFBundleInfoDictionaryVersion</key>
<string>6.0</string>
<key>CFBundleName</key>
<string>node_sandboxed</string>
<key>CFBundlePackageType</key>
<string>APPL</string>
<key>CFBundleShortVersionString</key>
<string>1.0</string>
<key>CFBundleSupportedPlatforms</key>
<array>
<string>MacOSX</string>
</array>
<key>CFBundleVersion</key>
<string>1</string>
</dict>
</plist>
8 changes: 8 additions & 0 deletions test/fixtures/macos-app-sandbox/node_sandboxed.entitlements
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>com.apple.security.app-sandbox</key>
<true/>
</dict>
</plist>
65 changes: 65 additions & 0 deletions test/parallel/test-macos-app-sandbox.js
@@ -0,0 +1,65 @@
'use strict';
const common = require('../common');
if (process.platform !== 'darwin')
common.skip('App Sandbox is only avaliable on Darwin');

const fixtures = require('../common/fixtures');
const tmpdir = require('../common/tmpdir');
const assert = require('assert');
const child_process = require('child_process');
const path = require('path');
const fs = require('fs');
const os = require('os');

const nodeBinary = process.execPath;

tmpdir.refresh();

const appBundlePath = path.join(tmpdir.path, 'node_sandboxed.app');
const appBundleContentPath = path.join(appBundlePath, 'Contents');
const appExecutablePath = path.join(
appBundleContentPath, 'MacOS', 'node');

// Construct the app bundle and put the node executable in it:
// node_sandboxed.app/
// └── Contents
// β”œβ”€β”€ Info.plist
// β”œβ”€β”€ MacOS
// β”‚ └── node
fs.mkdirSync(appBundlePath);
fs.mkdirSync(appBundleContentPath);
fs.mkdirSync(path.join(appBundleContentPath, 'MacOS'));
fs.copyFileSync(
fixtures.path('macos-app-sandbox', 'Info.plist'),
path.join(appBundleContentPath, 'Info.plist'));
fs.copyFileSync(
nodeBinary,
appExecutablePath);


// Sign the app bundle with sandbox entitlements:
assert.strictEqual(
child_process.spawnSync('/usr/bin/codesign', [
'--entitlements', fixtures.path(
'macos-app-sandbox', 'node_sandboxed.entitlements'),
'-s', '-',
appBundlePath
]).status,
0);

// Sandboxed app shouldn't be able to read the home dir
assert.notStrictEqual(
child_process.spawnSync(appExecutablePath, [
'-e', 'fs.readdirSync(process.argv[1])', os.homedir()
]).status,
0);

if (process.stdin.isTTY) {
// Run the sandboxed node instance with inherited tty stdin
const spawnResult = child_process.spawnSync(
appExecutablePath, ['-e', ''],
{ stdio: 'inherit' }
);

assert.strictEqual(spawnResult.signal, null);
}

0 comments on commit d16e51b

Please sign in to comment.