Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion init-repo/tests/dev-shell-template.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
64 changes: 50 additions & 14 deletions packages/localapp/native/windows/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -123,12 +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<String, String> {
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.
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<u32, String> {
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::<STARTUPINFOW>() as u32;
let mut process: PROCESS_INFORMATION = std::mem::zeroed();
Expand All @@ -154,9 +182,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> {
Expand Down Expand Up @@ -295,19 +326,24 @@ mod platform {
#[cfg(windows)]
fn main() {
let arguments = std::env::args().skip(1).collect::<Vec<_>>();
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<u32, String> = 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))]
Expand Down
21 changes: 21 additions & 0 deletions packages/localapp/scripts/native-adapter.node-test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,27 @@ 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 .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
// caller, 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
Expand Down
8 changes: 5 additions & 3 deletions packages/localapp/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion packages/localapp/src/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -101,7 +102,8 @@ export function isManagedSkill(name: string): boolean {

async function installDependencies(projectDirectory: string, io: CliIo): Promise<void> {
await new Promise<void>((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));
Expand Down
102 changes: 102 additions & 0 deletions packages/localapp/src/process/command-invocation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import fs from "node:fs";
import path from "node:path";

export interface ResolvedCommandInvocation {
command: string;
args: string[];
}

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;
}

/**
* 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");
}

/**
* 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];
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);
if (isFile(candidate)) return candidate;
}
}
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;
}
Loading
Loading