From b3bcb6d63cafb4c2913301d3f681afd8f7f5604b Mon Sep 17 00:00:00 2001 From: Patodo Date: Thu, 17 Sep 2026 23:07:06 +0800 Subject: [PATCH 1/3] fix(windows): spawn .cmd shims through the interpreter and report owned exit codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Windows defects blocked the local project workflow: `localapp init` installed dependencies with spawn("npm", …). Node refuses to spawn a .cmd shim without a shell, so it failed with ENOENT and then rolled the project back, leaving the "run npm install yourself" hint impossible to follow. Route the bare package-manager name through the command interpreter, which is absolute and resolves the shim through PATHEXT. `--job-owner` waited for its root and returned Ok(()) without reading its exit code, so every owned command looked successful. Read it with GetExitCodeProcess and make it the wrapper's own exit status; a failing project script is no longer reported as a passing one. Verified on Windows 11 26200: `localapp init` now installs 570 packages instead of failing, and the wrapper reports 7, 0 and 3 for children that exit 7, 0 and 3. `localapp dev` is still blocked and is not touched here: its script runner hands a bare `npm.cmd` to the owned-process wrapper, which only accepts an absolute executable. Routing it through cmd.exe from this layer does not work — the wrapper re-quotes each argv element and the interpreter then starts an interactive session instead of running the command — so that fix belongs in `job_owner`, which should wrap a .cmd/.bat target itself. --- packages/localapp/native/windows/src/main.rs | 34 ++++++++----- .../scripts/native-adapter.node-test.mjs | 15 ++++++ packages/localapp/src/commands/init.ts | 4 +- .../src/process/command-invocation.ts | 48 +++++++++++++++++++ .../localapp/tests/command-invocation.test.ts | 39 +++++++++++++++ 5 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 packages/localapp/src/process/command-invocation.ts create mode 100644 packages/localapp/tests/command-invocation.test.ts diff --git a/packages/localapp/native/windows/src/main.rs b/packages/localapp/native/windows/src/main.rs index db58624..a4ab06b 100644 --- a/packages/localapp/native/windows/src/main.rs +++ b/packages/localapp/native/windows/src/main.rs @@ -49,7 +49,7 @@ mod platform { use windows_sys::Win32::Foundation::{CloseHandle, HANDLE}; use windows_sys::Win32::System::JobObjects::{AssignProcessToJobObject, CreateJobObjectW, SetInformationJobObject, JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, JobObjectExtendedLimitInformation}; use windows_sys::Win32::System::Registry::{RegCloseKey, RegCreateKeyExW, RegSetValueExW, HKEY, HKEY_CURRENT_USER, KEY_WRITE, REG_OPTION_NON_VOLATILE, REG_SZ}; - use windows_sys::Win32::System::Threading::{CreateProcessW, ResumeThread, TerminateProcess, WaitForSingleObject, PROCESS_INFORMATION, STARTUPINFOW, CREATE_SUSPENDED}; + use windows_sys::Win32::System::Threading::{CreateProcessW, GetExitCodeProcess, ResumeThread, TerminateProcess, WaitForSingleObject, PROCESS_INFORMATION, STARTUPINFOW, CREATE_SUSPENDED}; use windows_sys::Win32::UI::Shell::ShellExecuteW; use windows::core::{Error, HSTRING, Interface, PCWSTR, PWSTR}; use windows::Data::Xml::Dom::XmlDocument; @@ -125,7 +125,9 @@ mod platform { /// Suspended create -> kill-on-close Job assignment -> resume. Every /// partial failure terminates and closes the root before it can escape. - pub unsafe fn job_owner(executable: &str, arguments: &[String]) -> Result<(), String> { + /// The owned root's exit code is returned so callers see a failing child + /// as a failure instead of a successful wrapper. + pub unsafe fn job_owner(executable: &str, arguments: &[String]) -> Result { if !safe_absolute_path(executable) { return Err("invalid executable".into()); } let application_name = wide(executable); let mut command_line = wide(&create_process_command_line(executable, arguments)); @@ -154,9 +156,12 @@ mod platform { CloseHandle(job); return Err("owned process wait failed".into()); } + let mut exit_code: u32 = 1; + let observed = GetExitCodeProcess(process.hProcess, &mut exit_code) != 0; CloseHandle(process.hProcess); CloseHandle(job); - Ok(()) + if !observed { return Err("owned process exit code is unavailable".into()); } + Ok(exit_code) } pub fn forward_scheme(config_path: &str, url: &str) -> Result<(), String> { @@ -295,19 +300,24 @@ mod platform { #[cfg(windows)] fn main() { let arguments = std::env::args().skip(1).collect::>(); - let result = match arguments.first().map(String::as_str) { + // Every arm reports an exit code so a failed owned process is not reported + // as a successful wrapper; commands without an owned process use 0. + let result: Result = match arguments.first().map(String::as_str) { Some("--job-owner") if arguments.len() >= 3 && arguments[1] == "--" => unsafe { platform::job_owner(&arguments[2], &arguments[3..]) }, - Some("--register") if arguments.len() == 3 && arguments[1] == "--config" => unsafe { platform::register_scheme(&arguments[2]) }, - Some("--scheme") if arguments.len() == 4 && arguments[1] == "--config" => platform::forward_scheme(&arguments[2], &arguments[3]), - Some("--open-url") if arguments.len() == 2 => unsafe { platform::open_external_url(&arguments[1]) }, - Some("--permission-state") if arguments.len() == 1 => { println!("{}", platform::notification_permission_state()); Ok(()) }, - Some("--request-permission") if arguments.len() == 1 => { println!("{}", platform::notification_permission_state()); Ok(()) }, - Some("--show-notification") if arguments.len() == 2 => platform::show_notification(&arguments[1]), + Some("--register") if arguments.len() == 3 && arguments[1] == "--config" => unsafe { platform::register_scheme(&arguments[2]).map(|()| 0) }, + Some("--scheme") if arguments.len() == 4 && arguments[1] == "--config" => platform::forward_scheme(&arguments[2], &arguments[3]).map(|()| 0), + Some("--open-url") if arguments.len() == 2 => unsafe { platform::open_external_url(&arguments[1]).map(|()| 0) }, + Some("--permission-state") if arguments.len() == 1 => { println!("{}", platform::notification_permission_state()); Ok(0) }, + Some("--request-permission") if arguments.len() == 1 => { println!("{}", platform::notification_permission_state()); Ok(0) }, + Some("--show-notification") if arguments.len() == 2 => platform::show_notification(&arguments[1]).map(|()| 0), Some("--validate-notification") if arguments.len() == 2 => localapp_native_contract::NotificationEnvelope::parse(&arguments[1], localapp_native_contract::Platform::Windows) - .and_then(|envelope| envelope.verify_icon()).map_err(String::from), + .and_then(|envelope| envelope.verify_icon()).map(|()| 0).map_err(String::from), _ => Err("unsupported native command".into()), }; - if result.is_err() { std::process::exit(1); } + match result { + Ok(code) => std::process::exit(code as i32), + Err(_) => std::process::exit(1), + } } #[cfg(not(windows))] diff --git a/packages/localapp/scripts/native-adapter.node-test.mjs b/packages/localapp/scripts/native-adapter.node-test.mjs index 82c12c3..635d9c0 100644 --- a/packages/localapp/scripts/native-adapter.node-test.mjs +++ b/packages/localapp/scripts/native-adapter.node-test.mjs @@ -183,6 +183,21 @@ test("Windows helper preserves argv, has an explicit application path, and keeps assert.match(source, /ShellExecuteW/); }); +test("Windows helper reports the owned process exit code instead of always succeeding", async () => { + // Break caught: --job-owner waited for the root and returned Ok(()) without + // reading its exit code, so every owned command looked successful — a failing + // project script under `localapp dev` was reported as a passing one. + const source = await fs.readFile(path.join(repositoryRoot, "packages/localapp/native/windows/src/main.rs"), "utf8"); + assert.match(source, /GetExitCodeProcess\(process\.hProcess, &mut exit_code\)/); + assert.match(source, /Ok\(exit_code\)/); + assert.match(source, /Some\("--job-owner"\)[^;]*platform::job_owner/); + // The exit code must reach the process exit status, not be dropped in main. + assert.match(source, /Ok\(code\) => std::process::exit\(code as i32\)/); + // A relative executable stays rejected: PATH resolution belongs to the + // interpreter the caller passes, never to the owned-process wrapper. + assert.match(source, /if !safe_absolute_path\(executable\)/); +}); + test("Windows helper hands the shortcut property store a task-allocator buffer, never a Rust allocation", async () => { // Break caught: `--register` wrote the Scheme registry key and then died with // STATUS_HEAP_CORRUPTION before the CLI could install the daemon, because the diff --git a/packages/localapp/src/commands/init.ts b/packages/localapp/src/commands/init.ts index 52cca3e..55a98f9 100644 --- a/packages/localapp/src/commands/init.ts +++ b/packages/localapp/src/commands/init.ts @@ -5,6 +5,7 @@ import { fileURLToPath } from "node:url"; import type { CliIo } from "../cli/output.js"; import { LocalAppLifecycleError, lifecycleError } from "../errors.js"; import { isValidProjectName, writeProjectManifest } from "../project/manifest.js"; +import { resolveCommandInvocation } from "../process/command-invocation.js"; import { isManagedSkillName, verifyInitParent } from "../project/safety.js"; import { copyDirectory, isDirectory, type CopyDestinationMutations } from "../template/copy.js"; @@ -101,7 +102,8 @@ export function isManagedSkill(name: string): boolean { async function installDependencies(projectDirectory: string, io: CliIo): Promise { await new Promise((resolve, reject) => { - const child = spawn("npm", ["install"], { cwd: projectDirectory, stdio: ["ignore", "pipe", "pipe"] }); + const invocation = resolveCommandInvocation("npm", ["install"]); + const child = spawn(invocation.command, invocation.args, { cwd: projectDirectory, stdio: ["ignore", "pipe", "pipe"] }); child.stdout.setEncoding("utf8"); child.stderr.setEncoding("utf8"); child.stdout.on("data", (chunk) => io.stdout(chunk)); diff --git a/packages/localapp/src/process/command-invocation.ts b/packages/localapp/src/process/command-invocation.ts new file mode 100644 index 0000000..ad11caf --- /dev/null +++ b/packages/localapp/src/process/command-invocation.ts @@ -0,0 +1,48 @@ +import path from "node:path"; + +export interface ResolvedCommandInvocation { + command: string; + args: string[]; +} + +export interface ResolveCommandInvocationOptions { + platform?: NodeJS.Platform; + env?: NodeJS.ProcessEnv; + commandInterpreter?: string; +} + +/** + * Turns a logical command into something this platform can actually spawn. + * + * Node refuses to spawn a `.cmd`/`.bat` shim without a shell since the + * CVE-2024-27980 fix, so `spawn("npm", …)` fails with ENOENT on Windows even + * though `npm.cmd` is on PATH. Routing a bare name through the command + * interpreter fixes that: the interpreter itself is absolute, and it resolves + * the shim through PATHEXT. + * + * Only bare names are rewritten; a caller that already holds a path keeps it. + * The arguments here are fixed internal values, never user input. + */ +export function resolveCommandInvocation( + command: string, + args: readonly string[], + options: ResolveCommandInvocationOptions = {}, +): ResolvedCommandInvocation { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + if (platform !== "win32" || !isBareCommandName(command)) return { command, args: [...args] }; + const interpreter = options.commandInterpreter ?? env.ComSpec ?? env.COMSPEC ?? defaultCommandInterpreter(env); + return { command: interpreter, args: ["/d", "/s", "/c", command, ...args] }; +} + +function isBareCommandName(command: string): boolean { + if (command.includes("/") || command.includes("\\") || command.includes("\0")) return false; + // A name with an extension (npm.cmd, node.exe) is still bare: cmd resolves it + // through PATHEXT exactly like the extensionless form. + return command.length > 0; +} + +function defaultCommandInterpreter(env: NodeJS.ProcessEnv): string { + const systemRoot = env.SystemRoot ?? env.SYSTEMROOT ?? "C:\\Windows"; + return path.win32.join(systemRoot, "System32", "cmd.exe"); +} diff --git a/packages/localapp/tests/command-invocation.test.ts b/packages/localapp/tests/command-invocation.test.ts new file mode 100644 index 0000000..3207a68 --- /dev/null +++ b/packages/localapp/tests/command-invocation.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { resolveCommandInvocation } from "../src/process/command-invocation.js"; + +const WINDOWS_ENV = { SystemRoot: "C:\\Windows" } as NodeJS.ProcessEnv; + +describe("command invocation resolution", () => { + it("routes a bare Windows package-manager name through the command interpreter", () => { + // Break caught: Node cannot spawn a .cmd shim without a shell, and the + // owned-process wrapper rejects a relative executable, so `npm`/`npm.cmd` + // reached neither CreateProcessW nor cmd — `localapp init` failed to install + // dependencies and `localapp dev` never started the project scripts. + for (const name of ["npm", "npm.cmd", "pnpm", "yarn", "bun"]) { + expect(resolveCommandInvocation(name, ["run", "test"], { platform: "win32", env: WINDOWS_ENV })).toEqual({ + command: "C:\\Windows\\System32\\cmd.exe", + args: ["/d", "/s", "/c", name, "run", "test"], + }); + } + }); + + it("honours an explicit interpreter and ComSpec", () => { + expect(resolveCommandInvocation("npm", ["install"], { platform: "win32", commandInterpreter: "D:\\cmd.exe" }).command).toBe("D:\\cmd.exe"); + expect(resolveCommandInvocation("npm", ["install"], { platform: "win32", env: { ComSpec: "D:\\Windows\\cmd.exe" } as NodeJS.ProcessEnv }).command).toBe("D:\\Windows\\cmd.exe"); + expect(resolveCommandInvocation("npm", ["install"], { platform: "win32", env: {} as NodeJS.ProcessEnv }).command).toBe("C:\\Windows\\System32\\cmd.exe"); + }); + + it("leaves an explicit path untouched so the owned-process wrapper keeps validating it", () => { + const absolute = "C:\\Program Files\\nodejs\\node.exe"; + expect(resolveCommandInvocation(absolute, ["-e", "1"], { platform: "win32", env: WINDOWS_ENV })).toEqual({ + command: absolute, + args: ["-e", "1"], + }); + expect(resolveCommandInvocation("C:\\tools\\npm.cmd", [], { platform: "win32", env: WINDOWS_ENV }).command).toBe("C:\\tools\\npm.cmd"); + }); + + it("keeps other platforms unchanged", () => { + expect(resolveCommandInvocation("npm", ["run", "test"], { platform: "linux" })).toEqual({ command: "npm", args: ["run", "test"] }); + expect(resolveCommandInvocation("npm", ["run", "test"], { platform: "darwin" })).toEqual({ command: "npm", args: ["run", "test"] }); + }); +}); From 273bab2a5a94852e2bf6ecaba5412e24285cfa1b Mon Sep 17 00:00:00 2001 From: Patodo Date: Thu, 17 Sep 2026 23:41:13 +0800 Subject: [PATCH 2/3] fix(windows): make the local project flow work end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, verified on one machine by running the whole flow: `localapp init` wrote a project whose own test suite could never pass on Windows. The template's failures were the two assertions that compare multi-line source text: a Windows checkout and the Windows template sync carry CRLF, so `toContain("...\n...")` never matched and `localapp check` failed at the tests phase — which also blocked `localapp dev`, since it runs the same check. Normalize line endings in the template test's reader. `localapp dev` handed the owned-process wrapper a bare `npm.cmd`. The wrapper only accepts an absolute executable path and cannot load a script image, so the project scripts never started. Resolve the shim through PATH and PATHEXT on the CLI side — an extensionless file of the same name must not win: npm ships one, and returning it made the interpreter fail with exit 1 — and wrap a `.cmd`/ `.bat` target in the command interpreter inside `job_owner`, the only layer that controls the quoting cmd.exe parses. Installing an application into the local Server failed on Windows with `APP_INSTALL_FAILED: EPERM fsync`. The retained-package step opened the copy read-only and called `handle.sync()`; FlushFileBuffers needs a write-capable handle there. Use the file's own `syncFile` helper, which already owns that rule, instead of the inline open/sync/close. Verified with the packaged build on Windows 11 26200: localapp init app1 -> 570 packages installed localapp check -> success, all 7 phases passed localapp dev -> App URL served (HTTP 200, Vite dev shell) and the embedded Server answered /health, with no shim --- init-repo/tests/dev-shell-template.test.ts | 5 +- packages/localapp/native/windows/src/main.rs | 30 +++++++++++- .../scripts/native-adapter.node-test.mjs | 8 +++- packages/localapp/src/commands/dev.ts | 8 ++-- .../src/process/command-invocation.ts | 48 +++++++++++++++++++ .../localapp/tests/command-invocation.test.ts | 45 ++++++++++++++++- packages/server/src/lib/app-installer.ts | 6 ++- 7 files changed, 139 insertions(+), 11 deletions(-) diff --git a/init-repo/tests/dev-shell-template.test.ts b/init-repo/tests/dev-shell-template.test.ts index 12fb1a1..4901e8f 100644 --- a/init-repo/tests/dev-shell-template.test.ts +++ b/init-repo/tests/dev-shell-template.test.ts @@ -9,7 +9,10 @@ const TRANSPARENT = "rgba(0, 0, 0, 0)"; function readTemplateFile(relativePath: string) { const parts = relativePath.split(/[\\/]/); const resolved = parts[0] === "runtime" ? path.join(runtimeRoot, ...parts.slice(1)) : relativePath; - return fs.readFileSync(path.join(root, resolved), "utf-8"); + // Several assertions compare multi-line source text. A Windows checkout, and + // the template sync that writes these files on Windows, carry CRLF, so + // normalize before matching or those assertions can never pass there. + return fs.readFileSync(path.join(root, resolved), "utf-8").replace(/\r\n/g, "\n"); } function cssForRuntimeTokens() { diff --git a/packages/localapp/native/windows/src/main.rs b/packages/localapp/native/windows/src/main.rs index a4ab06b..97b4213 100644 --- a/packages/localapp/native/windows/src/main.rs +++ b/packages/localapp/native/windows/src/main.rs @@ -123,14 +123,40 @@ mod platform { CloseHandle(process.hProcess); } + /// A `.cmd`/`.bat` target is not an executable image, so CreateProcessW + /// cannot load it directly; the interpreter has to run it instead. + fn is_command_script(value: &str) -> bool { + let lower = value.to_ascii_lowercase(); + lower.ends_with(".cmd") || lower.ends_with(".bat") + } + + fn command_interpreter() -> Result { + let from_environment = std::env::var("ComSpec").ok().filter(|value| safe_absolute_path(value)); + if let Some(value) = from_environment { return Ok(value); } + let system_root = std::env::var("SystemRoot").unwrap_or_else(|_| "C:\\Windows".to_string()); + let candidate = format!("{system_root}\\System32\\cmd.exe"); + if safe_absolute_path(&candidate) { Ok(candidate) } else { Err("command interpreter is unavailable".into()) } + } + /// Suspended create -> kill-on-close Job assignment -> resume. Every /// partial failure terminates and closes the root before it can escape. /// The owned root's exit code is returned so callers see a failing child /// as a failure instead of a successful wrapper. pub unsafe fn job_owner(executable: &str, arguments: &[String]) -> Result { if !safe_absolute_path(executable) { return Err("invalid executable".into()); } - let application_name = wide(executable); - let mut command_line = wide(&create_process_command_line(executable, arguments)); + // The interpreter supplies its own command line, so a script target is + // wrapped here rather than by the caller: this is the only layer that + // controls quoting for the single string cmd.exe parses. + let (application, command_line_text) = if is_command_script(executable) { + let interpreter = command_interpreter()?; + if !safe_absolute_path(&interpreter) { return Err("invalid command interpreter".into()); } + let inner = create_process_command_line(executable, arguments); + (interpreter, format!("/d /s /c \"{inner}\"")) + } else { + (executable.to_string(), create_process_command_line(executable, arguments)) + }; + let application_name = wide(&application); + let mut command_line = wide(&command_line_text); let mut startup: STARTUPINFOW = std::mem::zeroed(); startup.cb = std::mem::size_of::() as u32; let mut process: PROCESS_INFORMATION = std::mem::zeroed(); diff --git a/packages/localapp/scripts/native-adapter.node-test.mjs b/packages/localapp/scripts/native-adapter.node-test.mjs index 635d9c0..26b41ff 100644 --- a/packages/localapp/scripts/native-adapter.node-test.mjs +++ b/packages/localapp/scripts/native-adapter.node-test.mjs @@ -193,8 +193,14 @@ test("Windows helper reports the owned process exit code instead of always succe assert.match(source, /Some\("--job-owner"\)[^;]*platform::job_owner/); // The exit code must reach the process exit status, not be dropped in main. assert.match(source, /Ok\(code\) => std::process::exit\(code as i32\)/); + // A .cmd/.bat target is not an executable image, so the wrapper has to run it + // through the interpreter itself; a caller cannot express cmd's quoting + // through the per-argument command line the wrapper rebuilds. + assert.match(source, /fn is_command_script\(value: &str\) -> bool/); + assert.match(source, /\/d \/s \/c /); + assert.match(source, /let \(application, command_line_text\) = if is_command_script\(executable\)/); // A relative executable stays rejected: PATH resolution belongs to the - // interpreter the caller passes, never to the owned-process wrapper. + // caller, never to the owned-process wrapper. assert.match(source, /if !safe_absolute_path\(executable\)/); }); diff --git a/packages/localapp/src/commands/dev.ts b/packages/localapp/src/commands/dev.ts index d783355..5c51d0d 100644 --- a/packages/localapp/src/commands/dev.ts +++ b/packages/localapp/src/commands/dev.ts @@ -8,6 +8,7 @@ import type { CliIo } from "../cli/output.js"; import { readOrCreateDevCredentials } from "../dev/credentials.js"; import { lifecycleError } from "../errors.js"; import { LocalAppClient } from "../http/localapp-client.js"; +import { resolveOwnedCommand } from "../process/command-invocation.js"; import { spawnOwnedProcess, type OwnedProcess, type WindowsProcessTreeAdapter } from "../process/process-tree.js"; import { waitForServerReady } from "../process/readiness.js"; import type { ProjectCommandRunner } from "../project/check.js"; @@ -136,7 +137,7 @@ export async function runDev(options: RunDevOptions, dependencies: RunDevDepende "--strictPort", ]; lifecycle.assertActive(); - const vite = lifecycle.spawn(() => spawnProcess(configuredVite.command, viteArgs, { + const vite = lifecycle.spawn(() => spawnProcess(resolveOwnedCommand(configuredVite.command), viteArgs, { cwd: projectDir, env: { ...process.env, LOCALAPP_DEV_API_KEY: credentials.apiKey }, stdio: "ignore", @@ -332,8 +333,9 @@ function createOwnedProjectCommandRunner( ): ProjectCommandRunner { return async (invocation) => { lifecycle.assertActive(); - const command = process.platform === "win32" ? `${invocation.command}.cmd` : invocation.command; - const child = lifecycle.spawn(() => spawnProcess(command, invocation.args, { + // The wrapper validates an absolute executable path and handles a .cmd/.bat + // target itself, so resolve the shim instead of handing it a bare name. + const child = lifecycle.spawn(() => spawnProcess(resolveOwnedCommand(invocation.command), invocation.args, { cwd: invocation.cwd, stdio: "ignore", windowsAdapter, diff --git a/packages/localapp/src/process/command-invocation.ts b/packages/localapp/src/process/command-invocation.ts index ad11caf..acbeb92 100644 --- a/packages/localapp/src/process/command-invocation.ts +++ b/packages/localapp/src/process/command-invocation.ts @@ -1,3 +1,4 @@ +import fs from "node:fs"; import path from "node:path"; export interface ResolvedCommandInvocation { @@ -46,3 +47,50 @@ function defaultCommandInterpreter(env: NodeJS.ProcessEnv): string { const systemRoot = env.SystemRoot ?? env.SYSTEMROOT ?? "C:\\Windows"; return path.win32.join(systemRoot, "System32", "cmd.exe"); } + +/** + * Resolves a bare command name to the absolute file PATH would run, following + * PATHEXT on Windows. Returns undefined for a path (already resolved) or when + * nothing matches. + */ +export function resolveExecutablePath(command: string, options: ResolveCommandInvocationOptions = {}): string | undefined { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + if (!isBareCommandName(command)) return undefined; + const pathApi = platform === "win32" ? path.win32 : path.posix; + const searchPath = env.PATH ?? env.Path ?? env.path ?? ""; + // Windows resolves a bare name through PATHEXT, so an extensionless file of + // the same name (npm ships one: the POSIX shell script) must not win. + const extensions = platform === "win32" + ? (env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD").split(";").filter(Boolean).map((entry) => entry.toLowerCase()) + : [""]; + const names = platform === "win32" + ? (pathApi.extname(command) === "" ? extensions.map((extension) => `${command}${extension}`) : [command]) + : [command]; + for (const directory of searchPath.split(pathApi.delimiter)) { + const trimmed = directory.trim(); + if (trimmed === "") continue; + const unquoted = trimmed.length >= 2 && trimmed.startsWith("\"") && trimmed.endsWith("\"") ? trimmed.slice(1, -1) : trimmed; + for (const name of names) { + const candidate = pathApi.join(unquoted, name); + try { + if (fs.statSync(candidate).isFile()) return candidate; + } catch { + // Keep searching: a missing or unreadable candidate is not a match. + } + } + } + return undefined; +} + +/** + * The command to hand the owned-process wrapper, which validates an absolute + * executable path and delegates a `.cmd`/`.bat` target to the command + * interpreter itself. Falls back to the original name so the wrapper reports a + * missing executable rather than this helper silently doing nothing. + */ +export function resolveOwnedCommand(command: string, options: ResolveCommandInvocationOptions = {}): string { + const platform = options.platform ?? process.platform; + if (platform !== "win32") return command; + return resolveExecutablePath(command, options) ?? command; +} diff --git a/packages/localapp/tests/command-invocation.test.ts b/packages/localapp/tests/command-invocation.test.ts index 3207a68..165ac76 100644 --- a/packages/localapp/tests/command-invocation.test.ts +++ b/packages/localapp/tests/command-invocation.test.ts @@ -1,7 +1,22 @@ -import { describe, expect, it } from "vitest"; -import { resolveCommandInvocation } from "../src/process/command-invocation.js"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { resolveCommandInvocation, resolveExecutablePath, resolveOwnedCommand } from "../src/process/command-invocation.js"; const WINDOWS_ENV = { SystemRoot: "C:\\Windows" } as NodeJS.ProcessEnv; +const fixtures: string[] = []; + +afterEach(() => { + for (const directory of fixtures.splice(0)) fs.rmSync(directory, { recursive: true, force: true }); +}); + +function toolDirectory(names: string[]): string { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "localapp-tools-")); + fixtures.push(directory); + for (const name of names) fs.writeFileSync(path.join(directory, name), ""); + return directory; +} describe("command invocation resolution", () => { it("routes a bare Windows package-manager name through the command interpreter", () => { @@ -36,4 +51,30 @@ describe("command invocation resolution", () => { expect(resolveCommandInvocation("npm", ["run", "test"], { platform: "linux" })).toEqual({ command: "npm", args: ["run", "test"] }); expect(resolveCommandInvocation("npm", ["run", "test"], { platform: "darwin" })).toEqual({ command: "npm", args: ["run", "test"] }); }); + + it("resolves a bare name to the file PATH would run, following PATHEXT", () => { + const first = toolDirectory(["other.exe"]); + // npm ships an extensionless POSIX script next to its .cmd shim; Windows + // must skip it and keep walking PATHEXT. + const second = toolDirectory(["npm", "npm.cmd"]); + const env = { PATH: `${first}${path.delimiter}${second}`, PATHEXT: ".COM;.EXE;.BAT;.CMD" } as NodeJS.ProcessEnv; + expect(resolveExecutablePath("npm", { platform: "win32", env })).toBe(path.win32.join(second, "npm.cmd")); + expect(resolveExecutablePath("missing", { platform: "win32", env })).toBeUndefined(); + // A name that already carries an extension is used as given. + expect(resolveExecutablePath("other.exe", { platform: "win32", env })).toBe(path.win32.join(first, "other.exe")); + // A path is already resolved and is never rewritten. + expect(resolveExecutablePath("C:\\tools\\npm.cmd", { platform: "win32", env })).toBeUndefined(); + }); + + it("hands the owned-process wrapper an absolute shim path so it can wrap .cmd itself", () => { + // Break caught: the wrapper rejects a relative executable, so `localapp dev` + // could never start `npm.cmd`; it now receives the absolute shim and wraps + // it in the command interpreter on its own side. + const tools = toolDirectory(["npm.cmd"]); + const env = { PATH: tools, PATHEXT: ".CMD" } as NodeJS.ProcessEnv; + expect(resolveOwnedCommand("npm", { platform: "win32", env })).toBe(path.win32.join(tools, "npm.cmd")); + expect(resolveOwnedCommand("npm", { platform: "linux", env })).toBe("npm"); + // Unresolvable names stay untouched so the wrapper reports the real failure. + expect(resolveOwnedCommand("absent", { platform: "win32", env })).toBe("absent"); + }); }); diff --git a/packages/server/src/lib/app-installer.ts b/packages/server/src/lib/app-installer.ts index 341d5dc..38a7784 100644 --- a/packages/server/src/lib/app-installer.ts +++ b/packages/server/src/lib/app-installer.ts @@ -973,8 +973,10 @@ async function retainExactPackage( await fs.promises.chmod(tempPath, 0o600); const copied = await inspectAppPackage(tempPath); if (copied.digest !== inspected.digest) throw new AppInstallError("APP_PACKAGE_STORAGE_CORRUPT", "Retained package digest mismatch", 500); - const handle = await fs.promises.open(tempPath, "r"); - try { await handle.sync(); } finally { await handle.close(); } + // syncFile owns the Windows rule that FlushFileBuffers needs a + // write-capable handle: a read-only handle fails with EPERM there, which + // aborted every install that had to retain its package. + syncFile(tempPath); fs.renameSync(tempPath, finalPath); syncDirectory(directory); syncDirectory(pageDir); From dfcbd902a86c4b5b1fc4e44d5f2c9cc9dd9d7114 Mon Sep 17 00:00:00 2001 From: Patodo Date: Thu, 17 Sep 2026 23:56:03 +0800 Subject: [PATCH 3/3] test: keep the PATH resolution tests platform-independent The new cases built a fake PATH with the host path delimiter and a native temp directory, so they passed on Windows and failed on the Linux runner: the resolver splits with the win32 delimiter and the injected win32 paths do not exist there. Take the existence check as an injected seam instead, so the PATHEXT ordering rule (npm's extensionless POSIX script must not win over its .cmd shim) is asserted the same way on every platform. --- .../src/process/command-invocation.ts | 16 ++++--- .../localapp/tests/command-invocation.test.ts | 43 ++++++------------- 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/packages/localapp/src/process/command-invocation.ts b/packages/localapp/src/process/command-invocation.ts index acbeb92..026cdbd 100644 --- a/packages/localapp/src/process/command-invocation.ts +++ b/packages/localapp/src/process/command-invocation.ts @@ -10,6 +10,8 @@ export interface ResolveCommandInvocationOptions { platform?: NodeJS.Platform; env?: NodeJS.ProcessEnv; commandInterpreter?: string; + /** Path-existence seam so the resolution rules stay testable off-Windows. */ + isFile?: (candidate: string) => boolean; } /** @@ -67,17 +69,21 @@ export function resolveExecutablePath(command: string, options: ResolveCommandIn const names = platform === "win32" ? (pathApi.extname(command) === "" ? extensions.map((extension) => `${command}${extension}`) : [command]) : [command]; + const isFile = options.isFile ?? ((candidate: string) => { + try { + return fs.statSync(candidate).isFile(); + } catch { + // A missing or unreadable candidate is not a match. + return false; + } + }); for (const directory of searchPath.split(pathApi.delimiter)) { const trimmed = directory.trim(); if (trimmed === "") continue; const unquoted = trimmed.length >= 2 && trimmed.startsWith("\"") && trimmed.endsWith("\"") ? trimmed.slice(1, -1) : trimmed; for (const name of names) { const candidate = pathApi.join(unquoted, name); - try { - if (fs.statSync(candidate).isFile()) return candidate; - } catch { - // Keep searching: a missing or unreadable candidate is not a match. - } + if (isFile(candidate)) return candidate; } } return undefined; diff --git a/packages/localapp/tests/command-invocation.test.ts b/packages/localapp/tests/command-invocation.test.ts index 165ac76..ef39595 100644 --- a/packages/localapp/tests/command-invocation.test.ts +++ b/packages/localapp/tests/command-invocation.test.ts @@ -1,22 +1,7 @@ -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { resolveCommandInvocation, resolveExecutablePath, resolveOwnedCommand } from "../src/process/command-invocation.js"; const WINDOWS_ENV = { SystemRoot: "C:\\Windows" } as NodeJS.ProcessEnv; -const fixtures: string[] = []; - -afterEach(() => { - for (const directory of fixtures.splice(0)) fs.rmSync(directory, { recursive: true, force: true }); -}); - -function toolDirectory(names: string[]): string { - const directory = fs.mkdtempSync(path.join(os.tmpdir(), "localapp-tools-")); - fixtures.push(directory); - for (const name of names) fs.writeFileSync(path.join(directory, name), ""); - return directory; -} describe("command invocation resolution", () => { it("routes a bare Windows package-manager name through the command interpreter", () => { @@ -53,28 +38,28 @@ describe("command invocation resolution", () => { }); it("resolves a bare name to the file PATH would run, following PATHEXT", () => { - const first = toolDirectory(["other.exe"]); - // npm ships an extensionless POSIX script next to its .cmd shim; Windows - // must skip it and keep walking PATHEXT. - const second = toolDirectory(["npm", "npm.cmd"]); - const env = { PATH: `${first}${path.delimiter}${second}`, PATHEXT: ".COM;.EXE;.BAT;.CMD" } as NodeJS.ProcessEnv; - expect(resolveExecutablePath("npm", { platform: "win32", env })).toBe(path.win32.join(second, "npm.cmd")); - expect(resolveExecutablePath("missing", { platform: "win32", env })).toBeUndefined(); + const env = { PATH: "C:\\tools\\first;C:\\tools\\second", PATHEXT: ".COM;.EXE;.BAT;.CMD" } as NodeJS.ProcessEnv; + // Windows resolves a bare name through PATHEXT, so npm's extensionless + // POSIX script must not win over the .cmd shim sitting next to it. + const present = new Set(["C:\\tools\\second\\npm", "C:\\tools\\second\\npm.cmd", "C:\\tools\\first\\other.exe"]); + const isFile = (candidate: string) => present.has(candidate); + expect(resolveExecutablePath("npm", { platform: "win32", env, isFile })).toBe("C:\\tools\\second\\npm.cmd"); + expect(resolveExecutablePath("missing", { platform: "win32", env, isFile })).toBeUndefined(); // A name that already carries an extension is used as given. - expect(resolveExecutablePath("other.exe", { platform: "win32", env })).toBe(path.win32.join(first, "other.exe")); + expect(resolveExecutablePath("other.exe", { platform: "win32", env, isFile })).toBe("C:\\tools\\first\\other.exe"); // A path is already resolved and is never rewritten. - expect(resolveExecutablePath("C:\\tools\\npm.cmd", { platform: "win32", env })).toBeUndefined(); + expect(resolveExecutablePath("C:\\tools\\npm.cmd", { platform: "win32", env, isFile })).toBeUndefined(); }); it("hands the owned-process wrapper an absolute shim path so it can wrap .cmd itself", () => { // Break caught: the wrapper rejects a relative executable, so `localapp dev` // could never start `npm.cmd`; it now receives the absolute shim and wraps // it in the command interpreter on its own side. - const tools = toolDirectory(["npm.cmd"]); - const env = { PATH: tools, PATHEXT: ".CMD" } as NodeJS.ProcessEnv; - expect(resolveOwnedCommand("npm", { platform: "win32", env })).toBe(path.win32.join(tools, "npm.cmd")); + const env = { PATH: "C:\\tools", PATHEXT: ".CMD" } as NodeJS.ProcessEnv; + const isFile = (candidate: string) => candidate === "C:\\tools\\npm.cmd"; + expect(resolveOwnedCommand("npm", { platform: "win32", env, isFile })).toBe("C:\\tools\\npm.cmd"); expect(resolveOwnedCommand("npm", { platform: "linux", env })).toBe("npm"); // Unresolvable names stay untouched so the wrapper reports the real failure. - expect(resolveOwnedCommand("absent", { platform: "win32", env })).toBe("absent"); + expect(resolveOwnedCommand("absent", { platform: "win32", env, isFile })).toBe("absent"); }); });