pass -- as an argument from CLI to Electron application to avoi… (#5853)

* if cypress gets -- as leading argument, remove it

* start Eletron app args with --

* remove only

* update CLI to pass --no-sandbox before -- separator

* remove -- from args in cypress server

* do not use --no-sandbox in CLI when running in dev mode

cli/lib/exec/spawn.js

@@ -73,10 +73,15 @@ module.exports = {
 
     debug('needs to start own Xvfb?', needsXvfb)
 
-    // always push cwd into the args
+    // 1. Start arguments with "--" so Electron knows these are OUR
+    // arguments and does not try to sanitize them. Otherwise on Windows
+    // an url in one of the arguments crashes it :(
+    // https://github.com/cypress-io/cypress/issues/5466
+
+    // 2. Always push cwd into the args
     // which additionally acts as a signal to the
     // binary that it was invoked through the NPM module
-    args = [].concat(args, '--cwd', process.cwd())
+    args = ['--'].concat(args, '--cwd', process.cwd())
 
     _.defaults(options, {
       dev: false,
@@ -99,6 +104,8 @@ module.exports = {
           args.unshift(
             path.resolve(__dirname, '..', '..', '..', 'scripts', 'start.js')
           )
+
+          debug('in dev mode the args became %o', args)
         }
 
         const { onStderrData, electronLogging } = overrides
@@ -106,8 +113,10 @@ module.exports = {
         const electronArgs = _.clone(args)
         const node11WindowsFix = isPlatform('win32')
 
-        if (verify.needsSandbox()) {
-          electronArgs.push('--no-sandbox')
+        if (!options.dev && verify.needsSandbox()) {
+          // this is one of the Electron's command line switches
+          // thus it needs to be before "--" separator
+          electronArgs.unshift('--no-sandbox')
         }
 
         // strip dev out of child process options

cli/test/lib/exec/spawn_spec.js

@@ -94,6 +94,7 @@ describe('lib/exec/spawn', function () {
       return spawn.start('--foo', { foo: 'bar' })
       .then(() => {
         expect(cp.spawn).to.be.calledWithMatch('/path/to/cypress', [
+          '--',
           '--foo',
           '--cwd',
           cwd,
@@ -114,11 +115,13 @@ describe('lib/exec/spawn', function () {
         // and also less risk that a failed assertion would dump the
         // entire ENV object with possible sensitive variables
         const args = cp.spawn.firstCall.args.slice(0, 2)
+        // it is important for "--no-sandbox" to appear before "--" separator
         const expectedCliArgs = [
+          '--no-sandbox',
+          '--',
           '--foo',
           '--cwd',
           cwd,
-          '--no-sandbox',
         ]
 
         expect(args).to.deep.equal(['/path/to/cypress', expectedCliArgs])
@@ -135,6 +138,28 @@ describe('lib/exec/spawn', function () {
       .then(() => {
         expect(cp.spawn).to.be.calledWithMatch('node', [
           p,
+          '--',
+          '--foo',
+          '--cwd',
+          cwd,
+        ], {
+          detached: false,
+          stdio: ['inherit', 'inherit', 'pipe'],
+        })
+      })
+    })
+
+    it('does not pass --no-sandbox when running in dev mode', function () {
+      this.spawnedProcess.on.withArgs('close').yieldsAsync(0)
+      sinon.stub(verify, 'needsSandbox').returns(true)
+
+      const p = path.resolve('..', 'scripts', 'start.js')
+
+      return spawn.start('--foo', { dev: true, foo: 'bar' })
+      .then(() => {
+        expect(cp.spawn).to.be.calledWithMatch('node', [
+          p,
+          '--',
           '--foo',
           '--cwd',
           cwd,