fix: Fix issue with undefined or extra args passed to privileged comm…

…ands (#27157)

cli/CHANGELOG.md

@@ -9,6 +9,7 @@ _Released 07/05/2023 (PENDING)_
 
 **Bugfixes:**
 
+- Fixed issues where commands would fail with the error `must only be invoked from the spec file or support file`. Fixes [#27149](https://github.com/cypress-io/cypress/issues/27149).
 - Fixed an issue where chrome was not recovering from browser crashes properly. Fixes [#24650](https://github.com/cypress-io/cypress/issues/24650).
 - Fixed a race condition that was causing a GraphQL error to appear on the [Debug page](https://docs.cypress.io/guides/cloud/runs#Debug) when viewing a running Cypress Cloud build. Fixed in [#27134](https://github.com/cypress-io/cypress/pull/27134).
 

packages/driver/cypress/e2e/commands/task.cy.js

@@ -216,7 +216,7 @@ describe('src/cy/commands/task', () => {
           expect(lastLog.get('error')).to.eq(err)
           expect(lastLog.get('state')).to.eq('failed')
 
-          expect(err.message).to.eq(`\`cy.task('bar')\` failed with the following error:\n\nThe task 'bar' was not handled in the setupNodeEvents method. The following tasks are registered: return:arg, cypress:env, arg:is:undefined, wait, create:long:file, check:screenshot:size\n\nFix this in your setupNodeEvents method here:\n${Cypress.config('configFile')}`)
+          expect(err.message).to.eq(`\`cy.task('bar')\` failed with the following error:\n\nThe task 'bar' was not handled in the setupNodeEvents method. The following tasks are registered: return:arg, return:foo, return:bar, return:baz, cypress:env, arg:is:undefined, wait, create:long:file, check:screenshot:size\n\nFix this in your setupNodeEvents method here:\n${Cypress.config('configFile')}`)
 
           done()
         })

packages/driver/cypress/e2e/e2e/privileged_commands.cy.ts

@@ -70,17 +70,36 @@ describe('privileged commands', () => {
     })
 
     it('handles undefined argument(s)', () => {
-      cy.task('arg:is:undefined')
-      cy.task('arg:is:undefined', undefined)
-      cy.task('arg:is:undefined', undefined, undefined)
-      cy.task('arg:is:undefined', undefined, { timeout: 9999 })
+      // these intentionally use different tasks because otherwise there can
+      // be false positives due to them equating to the same call
+      cy.task('arg:is:undefined').should('equal', 'arg was undefined')
+      cy.task('return:foo', undefined).should('equal', 'foo')
+      cy.task('return:bar', undefined, undefined).should('equal', 'bar')
+      cy.task('return:baz', undefined, { timeout: 9999 }).should('equal', 'baz')
     })
 
     it('handles null argument(s)', () => {
-      cy.task('return:arg', null)
-      // @ts-ignore
-      cy.task('return:arg', null, null)
-      cy.task('return:arg', null, { timeout: 9999 })
+      cy.task('return:arg', null).should('be.null')
+      // @ts-expect-error
+      cy.task('return:arg', null, null).should('be.null')
+      cy.task('return:arg', null, { timeout: 9999 }).should('be.null')
+    })
+
+    it('handles extra, unexpected arguments', () => {
+      // @ts-expect-error
+      cy.exec('echo "hey-o"', { log: true }, { should: 'be ignored' })
+      // @ts-expect-error
+      cy.readFile('cypress/fixtures/app.json', 'utf-8', { log: true }, { should: 'be ignored' })
+      // @ts-expect-error
+      cy.writeFile('cypress/_test-output/written.json', 'contents', 'utf-8', { log: true }, { should: 'be ignored' })
+      // @ts-expect-error
+      cy.task('return:arg', 'arg2', { log: true }, { should: 'be ignored' })
+      // @ts-expect-error
+      cy.get('#basic').selectFile('cypress/fixtures/valid.json', { log: true }, { should: 'be ignored' })
+      if (!isWebkit) {
+        // @ts-expect-error
+        cy.origin('http://foobar.com:3500', {}, () => {}, { should: 'be ignored' })
+      }
     })
 
     it('passes in test body .then() callback', () => {

packages/driver/cypress/plugins/index.js

@@ -47,6 +47,15 @@ module.exports = async (on, config) => {
     'return:arg' (arg) {
       return arg
     },
+    'return:foo' () {
+      return 'foo'
+    },
+    'return:bar' () {
+      return 'bar'
+    },
+    'return:baz' () {
+      return 'baz'
+    },
     'cypress:env' () {
       return process.env['CYPRESS']
     },

packages/driver/src/cy/commands/actions/selectFile.ts

@@ -283,8 +283,11 @@ export default (Commands, Cypress, cy, state, config) => {
   }
 
   Commands.addAll({ prevSubject: 'element' }, {
-    async selectFile (subject: JQuery<any>, files: Cypress.FileReference | Cypress.FileReference[], options: Partial<InternalSelectFileOptions>): Promise<JQuery> {
-      const userArgs = trimUserArgs([files, _.isObject(options) ? { ...options } : undefined])
+    async selectFile (subject: JQuery<any>, files: Cypress.FileReference | Cypress.FileReference[], options: Partial<InternalSelectFileOptions>, ...extras: never[]): Promise<JQuery> {
+      // privileged commands need to send any and all args, even if not part
+      // of their API, so they can be compared to the args collected when the
+      // command is invoked
+      const userArgs = trimUserArgs([files, _.isObject(options) ? { ...options } : undefined, ...extras])
 
       options = _.defaults({}, options, {
         action: 'select',

packages/driver/src/cy/commands/exec.ts

@@ -13,8 +13,11 @@ interface InternalExecOptions extends Partial<Cypress.ExecOptions> {
 
 export default (Commands, Cypress, cy) => {
   Commands.addAll({
-    exec (cmd: string, userOptions: Partial<Cypress.ExecOptions>) {
-      const userArgs = trimUserArgs([cmd, userOptions])
+    exec (cmd: string, userOptions: Partial<Cypress.ExecOptions>, ...extras: never[]) {
+      // privileged commands need to send any and all args, even if not part
+      // of their API, so they can be compared to the args collected when the
+      // command is invoked
+      const userArgs = trimUserArgs([cmd, userOptions, ...extras])
 
       userOptions = userOptions || {}
 

packages/driver/src/cy/commands/files.ts

@@ -21,8 +21,11 @@ type WriteFileOptions = Partial<Cypress.WriteFileOptions & Cypress.Timeoutable>
 
 export default (Commands, Cypress, cy, state) => {
   Commands.addAll({
-    readFile (file: string, encoding: Cypress.Encodings | ReadFileOptions | undefined, userOptions?: ReadFileOptions) {
-      const userArgs = trimUserArgs([file, encoding, _.isObject(userOptions) ? { ...userOptions } : undefined])
+    readFile (file: string, encoding: Cypress.Encodings | ReadFileOptions | undefined, userOptions?: ReadFileOptions, ...extras: never[]) {
+      // privileged commands need to send any and all args, even if not part
+      // of their API, so they can be compared to the args collected when the
+      // command is invoked
+      const userArgs = trimUserArgs([file, encoding, _.isObject(userOptions) ? { ...userOptions } : undefined, ...extras])
 
       if (_.isObject(encoding)) {
         userOptions = encoding
@@ -142,8 +145,11 @@ export default (Commands, Cypress, cy, state) => {
       return verifyAssertions()
     },
 
-    writeFile (fileName: string, contents: string, encoding: Cypress.Encodings | WriteFileOptions | undefined, userOptions: WriteFileOptions) {
-      const userArgs = trimUserArgs([fileName, contents, encoding, _.isObject(userOptions) ? { ...userOptions } : undefined])
+    writeFile (fileName: string, contents: string, encoding: Cypress.Encodings | WriteFileOptions | undefined, userOptions: WriteFileOptions, ...extras: never[]) {
+      // privileged commands need to send any and all args, even if not part
+      // of their API, so they can be compared to the args collected when the
+      // command is invoked
+      const userArgs = trimUserArgs([fileName, contents, encoding, _.isObject(userOptions) ? { ...userOptions } : undefined, ...extras])
 
       if (_.isObject(encoding)) {
         userOptions = encoding

packages/driver/src/cy/commands/origin/index.ts

@@ -31,24 +31,28 @@ function stringifyFn (fn?: any) {
   return _.isFunction(fn) ? fn.toString() : undefined
 }
 
-function getUserArgs<T> (urlOrDomain: string, optionsOrFn: OptionsOrFn<T>, fn?: Fn<T>) {
+function getUserArgs<T> (urlOrDomain: string, optionsOrFn: OptionsOrFn<T>, extras: never[], fn?: Fn<T>) {
   return trimUserArgs([
     urlOrDomain,
     fn && _.isObject(optionsOrFn) ? { ...optionsOrFn } : stringifyFn(optionsOrFn),
     fn ? stringifyFn(fn) : undefined,
+    ...extras,
   ])
 }
 
 export default (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy, state: StateFunc, config: Cypress.InternalConfig) => {
   const communicator = Cypress.primaryOriginCommunicator
 
   Commands.addAll({
-    origin<T> (urlOrDomain: string, optionsOrFn: OptionsOrFn<T>, fn?: Fn<T>) {
+    origin<T> (urlOrDomain: string, optionsOrFn: OptionsOrFn<T>, fn?: Fn<T>, ...extras: never[]) {
       if (Cypress.isBrowser('webkit')) {
         return $errUtils.throwErrByPath('webkit.origin')
       }
 
-      const userArgs = getUserArgs<T>(urlOrDomain, optionsOrFn, fn)
+      // privileged commands need to send any and all args, even if not part
+      // of their API, so they can be compared to the args collected when the
+      // command is invoked
+      const userArgs = getUserArgs<T>(urlOrDomain, optionsOrFn, extras, fn)
 
       const userInvocationStack = state('current').get('userInvocationStack')
 

packages/driver/src/cy/commands/task.ts

@@ -14,8 +14,11 @@ interface InternalTaskOptions extends Partial<Cypress.Loggable & Cypress.Timeout
 
 export default (Commands, Cypress, cy) => {
   Commands.addAll({
-    task (task, arg, userOptions: Partial<Cypress.Loggable & Cypress.Timeoutable>) {
-      const userArgs = trimUserArgs([task, arg, _.isObject(userOptions) ? { ...userOptions } : undefined])
+    task (task, arg, userOptions: Partial<Cypress.Loggable & Cypress.Timeoutable>, ...extras: never[]) {
+      // privileged commands need to send any and all args, even if not part
+      // of their API, so they can be compared to the args collected when the
+      // command is invoked
+      const userArgs = trimUserArgs([task, arg, _.isObject(userOptions) ? { ...userOptions } : undefined, ...extras])
 
       userOptions = userOptions || {}
 

packages/driver/src/util/privileged_channel.ts

@@ -35,6 +35,7 @@ export function runPrivilegedCommand ({ commandName, cy, Cypress, options, userA
   })
 }
 
+// removes trailing undefined args
 export function trimUserArgs (args: any[]) {
   return _.dropRightWhile(args, _.isUndefined)
 }

packages/server/lib/privileged-commands/privileged-channel.js

@@ -127,6 +127,7 @@
     return `${4294967296 * (2097151 & h2) + (h1 >>> 0)}`
   }
 
+  // removes trailing undefined args
   function dropRightUndefined (array) {
     if (!isArray(array)) return []
 
@@ -158,8 +159,7 @@
     // hash the args to avoid `413 Request Entity Too Large` error from express.
     // see https://github.com/cypress-io/cypress/issues/27099 and
     // https://github.com/cypress-io/cypress/issues/27097
-    const args = dropRightUndefined(map.call([...command.args], (arg) => {
-      // undefined can't be JSON-stringified
+    const args = map.call(dropRightUndefined([...(command.args || [])]), (arg) => {
       if (arg === undefined) {
         arg = null
       }
@@ -169,7 +169,7 @@
       }
 
       return hash(stringify(arg))
-    }))
+    })
 
     // if we verify a privileged command was invoked from the spec frame, we
     // send it to the server, where it's stored in state. when the command is