Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cypress >= 10 GUI crashes on Windows as non-admin user when registry editing is disabled #22110

Closed
Swaana opened this issue Jun 5, 2022 · 5 comments · Fixed by #22119
Closed

Comments

@Swaana
Copy link
Contributor

Swaana commented Jun 5, 2022

Current behavior

When opening the GUI for Cypress and then clicking on type of test (either e2e or component, doesn't matter which), it displays "Initializing Config... Please what while we load your project and find browsers installed on your system" and shortly after crashes with the console error below:

Error: Error while obtaining machine id: Error: Command failed: %windir%\System32\REG.exe QUERY HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Cryptography /v MachineGuid
ERROR: Registry editing has been disabled by your administrator.

at ChildProcess.exithandler (node:child_process:406:12)
at ChildProcess.emit (node:events:390:28)
at ChildProcess.emit (node:domain:475:12)
at maybeClose (node:internal/child_process:1064:16)
at Socket.<anonymous> (node:internal/child_process:450:11)
at Socket.emit (node:events:390:28)
at Socket.emit (node:domain:475:12)
at Pipe.<anonymous> (node:net:687:12)

at C:\Users\swaana\AppData\Local\Cypress\Cache\10.0.1\Cypress\resources\app\node_modules\node-machine-id\dist\index.js:1:7964
at ChildProcess.exithandler (node:child_process:413:5)
at ChildProcess.emit (node:events:390:28)
at ChildProcess.emit (node:domain:475:12)
at maybeClose (node:internal/child_process:1064:16)
at Socket.<anonymous> (node:internal/child_process:450:11)
at Socket.emit (node:events:390:28)
at Socket.emit (node:domain:475:12)
at Pipe.<anonymous> (node:net:687:12)

Desired behavior

Getting the unique machine id should not crash for non admin users. The issue is caused by the underlying library being used (node-machine-id) which is calling %windir%\System32\REG.exe to fetch a unique id from the registry. Even though it is only reading the key the command itself is sensitive on Windows machines and therefore closed for non-admin users. This library seems to be badly maintained as the last publish was 3 years ago and for a year there has already been an open PR to fix this but without any response.

For my situation I am able to get this ID as a non-admin by using Powershell ("Powershell.exe -Command (Get-ItemProperty -Path HKLM:\SOFTWARE\Microsoft\Cryptography).MachineGuid") and there are probably other ways as well. So the issue is with the library itself and not with there being limitations due to user privileges.

Test code to reproduce

Due to the system setup required for this I cannot reproduce this with a repo or a test. It requires a Windows setup with a user that is not an Administrator and perhaps also extra security preventing a user from running REG.exe as is the case for the company where I work.

Cypress Version

10.0.1

Other

No response

@lmiller1990
Copy link
Contributor

Hi! Thanks for the bug report.

Just to clarify: did you only encounter this bug in Cypress 10.0.1, or are you also able to reproduce it in Cypress 9? Know when it was introduced will greatly help us track down and solve the issue.

@Swaana
Copy link
Contributor Author

Swaana commented Jun 6, 2022

We have been running Cypress 9 for months now on our machines, so it is definitely something introduced in Cypress 10.

I think I have found the issue. The error occurs in packages/data-context/src/sources/VersionsDataSource.ts:

// at line nr 9
const nmi = require('node-machine-id')

  // at line nr 183
  private static async machineId (): Promise<string | undefined> {
    try {
      return nmi.machineId()
    } catch (error) {
      return undefined
    }
  }

The code seems to intend to account for a failure, but because it returns the entire Promise it will not catch the promise.reject but only failure in the function or module itself. I think adding an await (return await nmi.machineId())will result in the desired effect. That way the catch block will also catch the promise reject and prevent the application from crashing if for some reason the machine id couldn't be retrieved.

At line nr 153 the resulting id is used and is optional.

    if (id) {
      manifestHeaders['x-machine-id'] = id
    }

Adding the await will not fix the underlying problem that the module node-machine-id is not able to fetch our machine-id, but it is a fix within Cypress itself and not an external module and will enable us to start using Cypress 10 (we're very eager to try out the Component testing!). The actual fix to also retrieve the machine-id for non admin users can be addressed separately.

@cypress-bot cypress-bot bot added the stage: needs review The PR code is done & tested, needs review label Jun 6, 2022
@Swaana
Copy link
Contributor Author

Swaana commented Jun 6, 2022

I have added a PR with the proposed fix. In this PR one of the existing unit tests is modified to account for the promise.reject. With this change I was able to see the unit test fail before the change and succeed after the change.

@mjhenkes mjhenkes added the PATCH label Jun 6, 2022
@flotwig flotwig changed the title Cypress >= 10 GUI crashes on Windows as non-admin user Cypress >= 10 GUI crashes on Windows as non-admin user when registry editing is disabled Jun 6, 2022
@cypress-bot cypress-bot bot added stage: contributor pr in progress and removed stage: needs review The PR code is done & tested, needs review labels Jun 6, 2022
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 6, 2022

The code for this is done in cypress-io/cypress#22119, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 6, 2022

Released in 10.0.3.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v10.0.3, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 6, 2022
@mjhenkes mjhenkes removed the PATCH label Jun 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants