Machine id fix #35
Machine id fix #35
Conversation
source/helpers/machineId.ts
Outdated
| export function getPlatform() { | ||
| const platform: SupportedPlatforms = process.platform as SupportedPlatforms; | ||
| if (supportedPlatforms.indexOf(platform) === -1) { | ||
| throw new Error(`Unsupported platform: ${process.platform}`); |
There was a problem hiding this comment.
We should avoid throwing an exception on the application startup. I would recommend returning undefined here and generate a random GUID that will be cached in the application memory. What do you think about it?
There was a problem hiding this comment.
We can also read mac address if os is not supported. Example of the unsupported system: Custom android box
source/helpers/machineId.ts
Outdated
| } | ||
|
|
||
| function windowsMachineId(): string { | ||
| const regVal = reg.getValue(reg.HKEY.LOCAL_MACHINE, 'SOFTWARE\\Microsoft\\Cryptography', 'MachineGuid') || 'null'; |
There was a problem hiding this comment.
should we also try to catch situations when a HKEY/LOCAL_MACHINE object is not available?
There was a problem hiding this comment.
Wouldn't it be btter if we will return undefined instead of string: "null". "null" is harder to handle and "null" won't be interpreted correctly by coronerd. In current situation when we return null we will return it to backtrace-node or we will hash it. I'm not sure if this is good idea.
|
@konraddysput have another look when you get a chance. I've made sure that it does not throw, and I've made it fall back to generating a random GUID when a machine ID is not available. I like the idea of adding the mac address as a fallback, but will have to research some methods or libraries for getting a reliable and stable mac address. |
|
Closing in favor of #37 |
This pull request is to address issue #34
To summarize that issue: one of the libraries that backtrace-node depends on, node-machine-id, expects access to binary reg.exe in order to get the machine ID on windows platforms. This is an unsafe assumption, for example when windows group policy disables access to that executable.
There is an open PR to node-machine-id to address this issue [1]. It leverages another node.js library, native-reg, to read the machine ID from the windows registry in an unprivileged way that does not rely on reg.exe. That PR comes from a fork which is linked below at reference [2]. Thanks and appreciation to https://github.com/Nokel81
This pull request vendors and modifies the contents of [2]. The modification is to address a few lingering issues on the windows platform that were discovered while testing this fix on a windows test machine. License information is included as well as a credit to the source repository.
[1] "Switch windows to using native-reg to avoid permissions"
automation-stack/node-machine-id#57
[2] https://github.com/Nokel81/node-machine-id/tree/no-regexe