aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAman Kumar <145339995+amankumarsingh77@users.noreply.github.com>2026-08-06 00:47:15 +0530
committerGitHub <noreply@github.com>2026-08-06 00:47:15 +0530
commitd49068d9ae55ca15f3b58c000fcbb27a59bdb895 (patch)
treec848dacf3d4e79995c9012a40e43a7d064a0fd07 /src
parentd9bc46b508f8c6f7e0f0ff5d54d804340f3e6c80 (diff)
fix: accept Windows script paths and start the daemon from a real install (#23)
Two independent Windows/production defects reported in #22. browser_run_script rejected every absolute path on Windows. The allowed-root check compared with a hardcoded `${root}/`, but path.resolve returns backslash-separated paths there, so a file inside os.tmpdir() was refused even though the rejection message listed tmpdir as allowed. Decide containment with path.relative instead: it is separator-agnostic and carries each platform's own case rules, so it also accepts a differing drive-letter case that a path.sep patch would still reject. Escapes via `..` and sibling directories sharing the root's name prefix stay rejected, as before. The daemon could not start under `npm omit=dev`: it is spawned through tsx, which was declared only as a dev dependency. Moving tsx to dependencies is not enough on its own — pi installs extensions into one flat hoisted tree, so tsx lands beside the package and the fixed <pkg>/node_modules/.bin/tsx lookup still misses, falling through to an `npx tsx` that resolves against the wrong cwd and dies against stdio: "ignore". Resolve the CLI entry with Node's own resolver, which walks up to the hoisted copy. That removes the shell entirely, and with it the Windows .cmd shim, windowsVerbatimArguments, and the manual quoting that paths containing spaces needed. Both fixes live in platform-branching code, so their tests are registered in the three-OS CI matrix that already exists for that purpose — a Linux-only run cannot exercise either one.
Diffstat (limited to 'src')
-rw-r--r--src/daemon/spawn.ts34
-rw-r--r--src/domains/js.ts6
-rw-r--r--src/util/paths.ts12
3 files changed, 32 insertions, 20 deletions
diff --git a/src/daemon/spawn.ts b/src/daemon/spawn.ts
index a88855f..ffba92d 100644
--- a/src/daemon/spawn.ts
+++ b/src/daemon/spawn.ts
@@ -1,6 +1,6 @@
import { spawn, type ChildProcess } from "node:child_process";
import { access, unlink } from "node:fs/promises";
-import { existsSync } from "node:fs";
+import { createRequire } from "node:module";
import { fileURLToPath } from "node:url";
import { dirname, join } from "node:path";
import { createConnection } from "node:net";
@@ -8,6 +8,8 @@ import { DAEMON_SOCKET_PATH, serialize, type WireControl } from "./protocol";
const moduleDir = dirname(fileURLToPath(import.meta.url));
+const requireFromHere = createRequire(import.meta.url);
+
const isWindows = process.platform === "win32";
// No-op on Windows: a named pipe is not a filesystem entry and unlinking a pipe path throws.
@@ -78,26 +80,28 @@ export const isDaemonRunning = async (timeoutMs = 2_000): Promise<boolean> => {
});
};
-export const spawnDaemon = (): ChildProcess | null => {
+// Node's own resolver, not a guessed `node_modules/.bin` path: pi installs every extension into
+// one flat, hoisted tree, so tsx lands beside the package rather than inside it and a fixed
+// `<pkg>/node_modules/.bin/tsx` lookup always misses. Resolving the CLI entry also means no shell,
+// so the Windows `.cmd` shim and its argument quoting are gone with it.
+export const daemonSpawnCommand = (): { readonly command: string; readonly args: ReadonlyArray<string> } | null => {
const daemonScript = join(moduleDir, "index.ts");
+ try {
+ return { command: process.execPath, args: [requireFromHere.resolve("tsx/cli"), daemonScript] };
+ } catch {
+ return null;
+ }
+};
- // Windows npm binaries are `.cmd` shims, which only launch through a shell — spawning one directly yields EINVAL.
- const tsxBinBase = join(moduleDir, "..", "..", "node_modules", ".bin", "tsx");
- const tsxBin = isWindows ? `${tsxBinBase}.cmd` : tsxBinBase;
- const hasLocalTsx = existsSync(tsxBin);
-
- const cmd = hasLocalTsx ? tsxBin : isWindows ? "npx.cmd" : "npx";
- const args = hasLocalTsx ? [daemonScript] : ["tsx", daemonScript];
-
- // With shell: true Node passes the command line verbatim, so a path containing spaces must be quoted or it splits into tokens.
- const quote = (s: string): string => (isWindows ? `"${s}"` : s);
+export const spawnDaemon = (): ChildProcess | null => {
+ const spec = daemonSpawnCommand();
+ if (!spec) return null;
- const child = spawn(quote(cmd), args.map(quote), {
+ const child = spawn(spec.command, [...spec.args], {
detached: true,
stdio: "ignore",
env: { ...process.env },
- shell: isWindows,
- windowsVerbatimArguments: isWindows,
+ windowsHide: true,
});
child.unref();
diff --git a/src/domains/js.ts b/src/domains/js.ts
index 6ba21cd..19c8913 100644
--- a/src/domains/js.ts
+++ b/src/domains/js.ts
@@ -1,5 +1,6 @@
import { readFile } from "node:fs/promises";
import { resolve, isAbsolute } from "node:path";
+import { isPathWithin } from "../util/paths";
import { tmpdir } from "node:os";
import { createRequire } from "node:module";
import { Type } from "typebox";
@@ -121,10 +122,7 @@ const allowedRoots = (): ReadonlyArray<string> => {
return [tmpdir(), process.cwd(), ...(env !== undefined ? [env] : [])].map((d) => resolve(d));
};
-const isPathAllowed = (p: string): boolean => {
- const abs = resolve(p);
- return allowedRoots().some((root) => abs === root || abs.startsWith(`${root}/`));
-};
+const isPathAllowed = (p: string): boolean => allowedRoots().some((root) => isPathWithin(root, p));
const MAX_SOURCE_BYTES = 1_000_000;
diff --git a/src/util/paths.ts b/src/util/paths.ts
index 30a1bbd..3aa1237 100644
--- a/src/util/paths.ts
+++ b/src/util/paths.ts
@@ -1,6 +1,16 @@
import { randomUUID } from "node:crypto";
import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { isAbsolute, join, relative, resolve } from "node:path";
+
+// `relative` rather than a string prefix: it is separator-agnostic and carries each platform's
+// own case rules, so a Windows path is not rejected for its backslashes or its drive-letter case.
+export const isPathWithin = (root: string, target: string): boolean => {
+ const absRoot = resolve(root);
+ const absTarget = resolve(target);
+ if (absTarget === absRoot) return true;
+ const rel = relative(absRoot, absTarget);
+ return rel !== "" && !rel.startsWith("..") && !isAbsolute(rel);
+};
const NAMESPACE_RE = /^[a-zA-Z0-9_-]+$/;