From 7928a1b33931778ace10a3fdaf2eaeecd89da764 Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Mon, 18 Jan 2021 11:22:22 -0500 Subject: [PATCH] Fix buggy partial startup on macOS due to flag added by Gatekeeper/XProtect (#21861) macOS Gatekeeper adds a flag ("-psn_0_[six or seven digits here]") when it intercepts Atom launches. This happens for fresh downloads, new installs, or first launches after upgrading). We don't need this flag, and yargs interprets it as many short flags. So, we filter it out. --- spec/main-process/atom-application.test.js | 4 +- spec/main-process/parse-command-line.test.js | 88 ++++++++++++++++++++ src/main-process/parse-command-line.js | 12 ++- 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/spec/main-process/atom-application.test.js b/spec/main-process/atom-application.test.js index 179d486a3..9568b2b88 100644 --- a/spec/main-process/atom-application.test.js +++ b/spec/main-process/atom-application.test.js @@ -724,7 +724,7 @@ describe('AtomApplication', function() { }); it('opens a file to a specific line number and column', async function() { - await scenario.open(parseCommandLine('b/2.md:12:5')); + await scenario.open(parseCommandLine(['b/2.md:12:5'])); await scenario.assert('[_ 2.md]'); const w = scenario.getWindow(0); @@ -750,7 +750,7 @@ describe('AtomApplication', function() { }); it('truncates trailing whitespace and colons', async function() { - await scenario.open(parseCommandLine('b/2.md:: ')); + await scenario.open(parseCommandLine(['b/2.md:: '])); await scenario.assert('[_ 2.md]'); const w = scenario.getWindow(0); diff --git a/spec/main-process/parse-command-line.test.js b/spec/main-process/parse-command-line.test.js index e41feacf1..d46b80bc7 100644 --- a/spec/main-process/parse-command-line.test.js +++ b/spec/main-process/parse-command-line.test.js @@ -21,6 +21,65 @@ describe('parseCommandLine', () => { ]); assert.deepEqual(args.pathsToOpen, ['/some/path']); }); + + // The "underscore flag" with no "non-flag argument" after it + // is the minimal reproducer for the macOS Gatekeeper startup bug. + // By default, it causes the addition of boolean "true"s into yargs' "non-flag argument" array: `argv._` + // Whereas we do string-only operations on these arguments, expecting them to be paths or URIs. + describe('and --_ or -_ are passed', () => { + it('does not attempt to parse booleans as paths or URIs', () => { + const args = parseCommandLine([ + '--_', + '/some/path', + '-_', + '-_', + 'some/other/path', + 'atom://test/url', + '--_', + 'atom://other/url', + '-_', + './another-path.file', + '-_', + '-_', + '-_' + ]); + assert.deepEqual(args.urlsToOpen, [ + 'atom://test/url', + 'atom://other/url' + ]); + assert.deepEqual(args.pathsToOpen, [ + '/some/path', + 'some/other/path', + './another-path.file' + ]); + }); + }); + + describe('and a non-flag number is passed as an argument', () => { + it('does not attempt to parse numbers as paths or URIs', () => { + const args = parseCommandLine([ + '43', + '/some/path', + '22', + '97', + 'some/other/path', + 'atom://test/url', + '885', + 'atom://other/url', + '42', + './another-path.file' + ]); + assert.deepEqual(args.urlsToOpen, [ + 'atom://test/url', + 'atom://other/url' + ]); + assert.deepEqual(args.pathsToOpen, [ + '/some/path', + 'some/other/path', + './another-path.file' + ]); + }); + }); }); describe('when --uri-handler is passed', () => { @@ -41,4 +100,33 @@ describe('parseCommandLine', () => { assert.deepEqual(args.pathsToOpen, []); }); }); + + describe('when evil macOS Gatekeeper flag "-psn_0_[six or seven digits here]" is passed', () => { + it('ignores any arguments starting with "-psn_"', () => { + const getPsnFlag = () => { + return `-psn_0_${Math.floor(Math.random() * 10_000_000)}`; + }; + const args = parseCommandLine([ + getPsnFlag(), + '/some/path', + getPsnFlag(), + getPsnFlag(), + 'some/other/path', + 'atom://test/url', + getPsnFlag(), + 'atom://other/url', + '-psn_ Any argument starting with "-psn_" should be ignored, even this one.', + './another-path.file' + ]); + assert.deepEqual(args.urlsToOpen, [ + 'atom://test/url', + 'atom://other/url' + ]); + assert.deepEqual(args.pathsToOpen, [ + '/some/path', + 'some/other/path', + './another-path.file' + ]); + }); + }); }); diff --git a/src/main-process/parse-command-line.js b/src/main-process/parse-command-line.js index d00464d43..61614e04a 100644 --- a/src/main-process/parse-command-line.js +++ b/src/main-process/parse-command-line.js @@ -5,7 +5,12 @@ const yargs = require('yargs'); const { app } = require('electron'); module.exports = function parseCommandLine(processArgs) { - const options = yargs(processArgs).wrap(yargs.terminalWidth()); + // macOS Gatekeeper adds a flag ("-psn_0_[six or seven digits here]") when it intercepts Atom launches. + // (This happens for fresh downloads, new installs, or first launches after upgrading). + // We don't need this flag, and yargs interprets it as many short flags. So, we filter it out. + const filteredArgs = processArgs.filter(arg => !arg.startsWith('-psn_')); + + const options = yargs(filteredArgs).wrap(yargs.terminalWidth()); const version = app.getVersion(); options.usage( dedent`Atom Editor v${version} @@ -187,6 +192,11 @@ module.exports = function parseCommandLine(processArgs) { let devMode = args['dev']; for (const path of args._) { + if (typeof path !== 'string') { + // Sometimes non-strings (such as numbers or boolean true) get into args._ + // In the next block, .startsWith() only works on strings. So, skip non-string arguments. + continue; + } if (path.startsWith('atom://')) { urlsToOpen.push(path); } else {