Repository navigation
fix: avoid cmd.exe spawn on Windows by using os.hostname() before execSync fallback #7841
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,13 @@ | ||
| import { execFileSync, execSync } from "child_process"; | ||
| import { existsSync, readFileSync } from "fs"; | ||
| import os from "os"; | ||
|
|
||
| /** | ||
| * Module-level cache: on Windows, execSync("hostname") spawns | ||
| * cmd.exe which is expensive. Cache the result after first call | ||
| * since the machine ID never changes at runtime. | ||
| */ | ||
| let cachedRawId: string | null = null; | ||
|
|
||
| /** | ||
| * Get raw machine ID using OS-specific methods. | ||
|
|
@@ -11,6 +19,9 @@ import { existsSync, readFileSync } from "fs"; | |
| * On Linux: skips Windows (REG.exe) and macOS (ioreg) strategies entirely. | ||
| */ | ||
| function getMachineIdRaw(): string { | ||
| // Return cached result immediately (machine identity is stable at runtime) | ||
| if (cachedRawId !== null) return cachedRawId; | ||
|
Comment on lines
29
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the Repository Style Guide (Rule 9 under Hard Rules), you must always include tests when changing production code under References
|
||
|
|
||
| // Strategy 1: Windows — REG.exe query for MachineGuid | ||
| try { | ||
| if (process.platform !== "win32") { | ||
|
|
@@ -28,7 +39,10 @@ function getMachineIdRaw(): string { | |
| .split("REG_SZ")[1] | ||
| ?.replace(/\r+|\n+|\s+/gi, "") | ||
| ?.toLowerCase(); | ||
| if (id && id.length > 8) return id; | ||
| if (id && id.length > 8) { | ||
| cachedRawId = id; | ||
| return id; | ||
| } | ||
| } | ||
| } catch { | ||
| // Not Windows or REG.exe failed — continue | ||
|
|
@@ -49,7 +63,10 @@ function getMachineIdRaw(): string { | |
| ?.split("\n")[0] | ||
| ?.replace(/=|\s+|"/gi, "") | ||
| ?.toLowerCase(); | ||
| if (id && id.length > 8) return id; | ||
| if (id && id.length > 8) { | ||
| cachedRawId = id; | ||
| return id; | ||
| } | ||
| } | ||
| } catch { | ||
| // Not macOS or ioreg not available — continue | ||
|
|
@@ -62,7 +79,10 @@ function getMachineIdRaw(): string { | |
| const content = readFileSync(/* turbopackIgnore: true */ filePath, "utf8") | ||
| .trim() | ||
| .toLowerCase(); | ||
| if (content.length > 8) return content; | ||
| if (content.length > 8) { | ||
| cachedRawId = content; | ||
| return content; | ||
| } | ||
| } catch { | ||
| // Try the next candidate file | ||
| } | ||
|
|
@@ -71,23 +91,30 @@ function getMachineIdRaw(): string { | |
| // Files not readable — continue | ||
| } | ||
|
|
||
| // Strategy 4: Hostname fallback (works on all platforms) | ||
| // Strategy 4: Node.js os.hostname() — no child process, works everywhere | ||
| try { | ||
| const hostname = execSync("hostname", { encoding: "utf8", timeout: 5000 }); | ||
| const id = hostname.trim().toLowerCase(); | ||
| if (id) return id; | ||
| const hostname = os.hostname().toLowerCase(); | ||
| if (hostname) { | ||
| cachedRawId = hostname; | ||
| return hostname; | ||
| } | ||
| } catch { | ||
| // hostname failed — continue | ||
| // os.hostname() not available — continue | ||
| } | ||
|
|
||
| // Strategy 5: Node.js os.hostname() (no exec needed) | ||
| // Strategy 5: execSync("hostname") shell fallback (for constrained environments) | ||
| try { | ||
| const os = require("os"); | ||
| return os.hostname().toLowerCase(); | ||
| const hostname = execSync("hostname", { encoding: "utf8", timeout: 5000 }); | ||
| const id = hostname.trim().toLowerCase(); | ||
| if (id) { | ||
| cachedRawId = id; | ||
| return id; | ||
| } | ||
| } catch { | ||
| // Final fallback | ||
| // hostname failed — continue | ||
| } | ||
|
|
||
| cachedRawId = "unknown-machine"; | ||
| return "unknown-machine"; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The introduction of the module-level
cachedRawIdvariable makes it difficult to unit test different strategies or mock the environment across multiple test cases, as the cache cannot be cleared. To improve testability, consider exporting a helper function to reset the cache.