fix: address Codex review on PR #3282

This commit is contained in:
netcatty-bot committed 2026-09-05 10:51:45 +00:00
1 parent 1c4a220d9d
commit ebb4c4c6fa
2 files changed
+81 -10

No files matched your search

+26 -9
View File
@@ -136,8 +136,13 @@ function isWindowsAppExecutionAlias(candidate, env = process.env, platform) {
* write, hit the network, or block on input, which would stall the main
* process until the probe timeout and then kill the profile midway).
*
* An empty list means "do not probe": there is no argument set that
* guarantees the candidate exits immediately without side effects, so
* callers must never spawn it. See `probeAliasLaunchable`.
*
* @param {string} candidate Executable path.
* @returns {string[]} Extra arguments, or an empty list.
* @returns {string[]} Extra arguments, or an empty list when the candidate
* must not be probed.
*/
function probeArgsFor(candidate) {
const base = path.win32.basename(String(candidate || "")).toLowerCase();
@@ -154,18 +159,28 @@ function probeArgsFor(candidate) {
* while `lstatSync` succeeds matches both a live alias and a stale or broken
* reparse point (e.g. after an incomplete package removal). The only reliable
* pure-JS check is to let CreateProcess resolve it: a live alias spawns, a
* stale one fails to spawn. PowerShell-family candidates are probed with
* `-NoProfile -NonInteractive -Command exit` so the user's profile is never
* executed; other candidates receive empty stdin so they exit immediately
* instead of waiting for input. A timeout still proves the process started,
* which is all we care about.
* stale one fails to spawn.
*
* Only the PowerShell family is probed. `probeArgsFor()` returns
* `-NoProfile -NonInteractive -Command exit` for those, so the probe is
* guaranteed to exit immediately without running the user's profile.
* Any other WindowsApps alias (e.g. `python.exe`, `wt.exe`) is NOT probed:
* spawning an arbitrary alias with no arguments can open a GUI or the
* Microsoft Store, or block the Electron main process until the probe
* timeout. Unknown aliases are therefore treated as not launchable.
*
* @param {string} candidate WindowsApps alias path (AppExecLink reparse point).
* @param {{spawnSync?: typeof import("node:child_process").spawnSync}} [deps] Injectable for tests.
* @returns {boolean} True when the process could be started.
*/
function probeAliasLaunchable(candidate) {
function probeAliasLaunchable(candidate, deps = {}) {
const spawn = deps.spawnSync || spawnSync;
// Restrict the probe to candidates we know launch headlessly and exit
// immediately; anything else would be spawned with no arguments.
const args = probeArgsFor(candidate);
if (args.length === 0) return false;
try {
const result = spawnSync(candidate, probeArgsFor(candidate), {
const result = spawn(candidate, args, {
timeout: 5000,
windowsHide: true,
input: "",
@@ -192,7 +207,9 @@ function probeAliasLaunchable(candidate) {
* the full alias path. For those, fall back to `fs.lstatSync()`, which
* still sees the reparse point itself, and then confirm launchability with
* a spawn probe — `lstatSync` alone cannot tell a live alias from a stale
* reparse point. A zero-byte plain file in the same directory (e.g. a
* reparse point. Only PowerShell-family aliases are probed (see
* `probeAliasLaunchable`); other aliases are never spawned and are rejected.
* A zero-byte plain file in the same directory (e.g. a
* disabled alias) stats cleanly and is rejected.
*
* Non-alias candidates win over alias candidates so a real install (MSI,
+55 -1
View File
@@ -3,6 +3,7 @@ const assert = require("node:assert/strict");
const {
isWindowsAppExecutionAlias,
probeAliasLaunchable,
probeArgsFor,
selectExecutableCandidate,
windowsAppsAliasPrefix,
@@ -191,8 +192,61 @@ test("probeArgsFor suppresses the profile for PowerShell-family aliases", () =>
"-Command",
"exit",
]);
// Non-PowerShell candidates keep the plain empty-stdin probe.
// Non-PowerShell candidates must not be probed at all (empty args = "do
// not probe"): launching an arbitrary WindowsApps alias with no arguments
// can open a GUI or the Microsoft Store.
assert.deepEqual(probeArgsFor("C:\\Users\\u\\AppData\\Local\\Microsoft\\WindowsApps\\wt.exe"), []);
assert.deepEqual(probeArgsFor("C:\\Users\\u\\AppData\\Local\\Microsoft\\WindowsApps\\python.exe"), []);
});
test("probeAliasLaunchable never spawns non-PowerShell WindowsApps aliases", () => {
// Probing python.exe/wt.exe with no arguments can open a GUI or the
// Microsoft Store, so unknown aliases must be rejected without spawning.
const spawnCalls = [];
const spawnSync = (...args) => {
spawnCalls.push(args);
return { error: undefined };
};
assert.equal(
probeAliasLaunchable("C:\\Users\\u\\AppData\\Local\\Microsoft\\WindowsApps\\python.exe", { spawnSync }),
false,
);
assert.equal(spawnCalls.length, 0);
});
test("probeAliasLaunchable probes PowerShell-family aliases with -NoProfile", () => {
const spawnCalls = [];
const spawnSync = (...args) => {
spawnCalls.push(args);
return { error: undefined };
};
assert.equal(probeAliasLaunchable(ALIAS_PWSH, { spawnSync }), true);
assert.equal(spawnCalls.length, 1);
assert.deepEqual(spawnCalls[0][1], [
"-NoProfile",
"-NonInteractive",
"-Command",
"exit",
]);
});
test("probeAliasLaunchable treats a probe timeout as launchable", () => {
// ETIMEDOUT means the process ran until we killed it — it launched.
const spawnSync = () => {
const err = new Error("spawn timeout");
err.code = "ETIMEDOUT";
return { error: err };
};
assert.equal(probeAliasLaunchable(ALIAS_PWSH, { spawnSync }), true);
});
test("probeAliasLaunchable rejects aliases that fail to spawn", () => {
const spawnSync = () => {
const err = new Error("spawn failed");
err.code = "ENOENT";
return { error: err };
};
assert.equal(probeAliasLaunchable(ALIAS_PWSH, { spawnSync }), false);
});
test("selectExecutableCandidate ignores empty candidates", () => {