From 878ff8f55951b41049af61f21e4e509fefc36107 Mon Sep 17 00:00:00 2001 From: Hong Liu <88263959+Hylouis233@users.noreply.github.com> Date: Sun, 25 Oct 2026 06:38:23 +0800 Subject: [PATCH 1/4] fix: provision StepPage on Windows via the published bundle provisionBuiltinPlugin() skipped win32 entirely and ensureStepPageReady() refused to install without a healthy executable, so the built-in steppage plugin could never set up steppage-mcp on Windows and every launch reported "automatic installation is currently supported on macOS and Linux" (#219). Windows cannot run the POSIX installer script, so install the published bundle directly: download steppage-mcp.mjs, write a steppage-mcp.cmd wrapper next to it under ~/.local/bin, and verify the result. Executable probes run through `cmd /d /c` on Windows so both bare PATHEXT lookups and explicit .cmd files resolve on every Node runtime. The lock, Node.js >= 20 check, credential handling and failure reporting stay shared with the POSIX path; only the download-and-execute step is platform-specific. The Windows failure hint points at `step mcp prepare` instead of the POSIX-only `curl | sh` pipeline. --- docs/step-configuration.md | 6 +- packages/coding-agent/src/step/plugins.ts | 2 +- .../src/step/steppage-provision.ts | 144 +++++++++++---- .../test/steppage-provision-windows.test.ts | 172 ++++++++++++++++++ 4 files changed, 284 insertions(+), 40 deletions(-) create mode 100644 packages/coding-agent/test/steppage-provision-windows.test.ts diff --git a/docs/step-configuration.md b/docs/step-configuration.md index a99b4878..cd64fa97 100644 --- a/docs/step-configuration.md +++ b/docs/step-configuration.md @@ -226,7 +226,11 @@ markers do not suppress this repair. Healthy executables in `~/.local/bin` can be used even when the current shell's PATH lacks that directory. Failed setup is retried on the next session; `step mcp prepare` also retries without requiring sign-in. The server name combines the plugin ID and its server name with `__`. -Automatic StepPage installation is supported on macOS/Linux, including WSL. +Automatic StepPage installation is supported on macOS, Linux, Windows, and WSL. +On Windows the published bundle is downloaded directly and wrapped in a +`steppage-mcp.cmd` launcher under `~/.local/bin`, because the POSIX installer +script cannot run there; `step mcp prepare` probes that wrapper through +`cmd /c` so no shell PATH change is required. Preparation preserves credentials, custom server commands, global overrides, disabled servers, and intentionally uninstalled plugins. A global declaration diff --git a/packages/coding-agent/src/step/plugins.ts b/packages/coding-agent/src/step/plugins.ts index c4f5242c..b6415aa9 100644 --- a/packages/coding-agent/src/step/plugins.ts +++ b/packages/coding-agent/src/step/plugins.ts @@ -580,7 +580,7 @@ export function provisionInstallCommand( /** Install the executable declared by a built-in plugin, when it is missing. */ export async function provisionBuiltinPlugin(provision: StepPluginProvision): Promise { - if (provision.command !== "steppage-mcp" || process.platform === "win32") { + if (provision.command !== "steppage-mcp") { return undefined; } const install = provisionInstallCommand(provision); diff --git a/packages/coding-agent/src/step/steppage-provision.ts b/packages/coding-agent/src/step/steppage-provision.ts index ed27b015..8b04a87e 100644 --- a/packages/coding-agent/src/step/steppage-provision.ts +++ b/packages/coding-agent/src/step/steppage-provision.ts @@ -1,5 +1,5 @@ import { execFile, spawn } from "node:child_process"; -import { mkdir } from "node:fs/promises"; +import { mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { promisify } from "node:util"; @@ -20,6 +20,29 @@ export function stepPageInstallerUrl(env: NodeJS.ProcessEnv = process.env): stri return env.STEPCODE_STEPPAGE_INSTALLER_URL?.trim() || DEFAULT_STEPPAGE_INSTALLER_URL; } +/** + * The published bundle the installer writes next to its wrapper. Derived from the + * installer URL so a channel or mirror override covers both artifacts. + */ +export function stepPageBundleUrl(env: NodeJS.ProcessEnv = process.env): string { + return `${stepPageInstallerUrl(env).replace(/\/[^/]*$/u, "")}/latest/steppage-mcp.mjs`; +} + +/** + * Probe order for an existing executable: PATH first, then the managed install. + * Windows probes the `.cmd` wrapper because the bundle is a bare ESM file. + */ +export function stepPageCommandCandidates(platform: NodeJS.Platform, homeDir: string): string[] { + const managed = join(homeDir, ".local", "bin", "steppage-mcp"); + if (platform === "win32") return ["steppage-mcp", "steppage-mcp.cmd", `${managed}.cmd`]; + return ["steppage-mcp", managed]; +} + +/** The Windows wrapper installed next to the bundle; a bare ESM file must run via node. */ +export function renderStepPageWrapper(bundlePath: string): string { + return `@echo off\r\nnode "${bundlePath}" %*\r\n`; +} + /** Default executable ownership may survive in an old global declaration. */ export function isManagedStepPageCommand( declaration: { command?: unknown; args?: unknown }, @@ -76,37 +99,84 @@ async function runInstaller(script: string, env: NodeJS.ProcessEnv, signal: Abor }); } +/** `cmd /c` resolves both bare PATHEXT lookups and explicit `.cmd` files on every Node runtime. */ +async function probeStepPageVersion( + command: string, + env: NodeJS.ProcessEnv, + signal: AbortSignal | undefined, + platform: NodeJS.Platform, +): Promise { + try { + const [file, args] = + platform === "win32" + ? (["cmd", ["/d", "/c", command, "--version"]] as const) + : ([command, ["--version"]] as const); + const result = await execFileAsync(file, [...args], { env, signal, timeout: 5_000, maxBuffer: 8_192 }); + const version = result.stdout.trim(); + if (/^v?\d+\.\d+\.\d+(?:[-+][\w.-]+)?$/u.test(version)) return version; + } catch { + signal?.throwIfAborted(); + } + return undefined; +} + /** Only probes executables; never starts an MCP session or requires login. */ -async function findHealthyStepPage(env: NodeJS.ProcessEnv, signal?: AbortSignal): Promise { - const managed = join(resolveStepHomeDir(env), ".local", "bin", "steppage-mcp"); - for (const command of ["steppage-mcp", managed]) { +async function findHealthyStepPage( + env: NodeJS.ProcessEnv, + signal?: AbortSignal, + platform: NodeJS.Platform = process.platform, +): Promise { + for (const command of stepPageCommandCandidates(platform, resolveStepHomeDir(env))) { signal?.throwIfAborted(); - try { - const result = await execFileAsync(command, ["--version"], { env, signal, timeout: 5_000, maxBuffer: 8_192 }); - if (/^v?\d+\.\d+\.\d+(?:[-+][\w.-]+)?$/u.test(result.stdout.trim())) return command; - } catch { - signal?.throwIfAborted(); - } + if (await probeStepPageVersion(command, env, signal, platform)) return command; } return undefined; } +async function assertNodeRuntime(env: NodeJS.ProcessEnv, signal: AbortSignal): Promise { + let nodeVersion: string; + try { + nodeVersion = (await execFileAsync("node", ["--version"], { env, signal, timeout: 5_000 })).stdout.trim(); + } catch { + signal.throwIfAborted(); + throw new Error("Node.js >= 20 is required for StepPage; install Node.js and retry."); + } + const major = /^v(\d+)\./u.exec(nodeVersion)?.[1]; + if (!major || Number(major) < 20) throw new Error(`Node.js >= 20 is required for StepPage; found ${nodeVersion}.`); +} + +/** + * Windows cannot run the POSIX installer script, so install the published bundle + * directly: download it, then write the `cmd` wrapper the bundle needs to run. + */ +async function installStepPageOnWindows(env: NodeJS.ProcessEnv, signal: AbortSignal): Promise { + const home = resolveStepHomeDir(env); + const bundleDir = join(home, ".steppage-mcp", "bin"); + const bundlePath = join(bundleDir, "steppage-mcp.mjs"); + const wrapperPath = join(home, ".local", "bin", "steppage-mcp.cmd"); + await mkdir(bundleDir, { recursive: true, mode: 0o700 }); + await mkdir(join(home, ".local", "bin"), { recursive: true }); + // Download before writing the wrapper so a failed transfer cannot hide behind a stale one. + await execFileAsync("curl", ["-fsSL", "--max-time", "30", stepPageBundleUrl(env), "-o", bundlePath], { + env, + signal, + timeout: 35_000, + maxBuffer: 8_192, + }); + await writeFile(wrapperPath, renderStepPageWrapper(bundlePath)); +} + /** Repair the managed executable without changing plugin manifests or credentials. */ export async function ensureStepPageReady( input: { env?: NodeJS.ProcessEnv; signal?: AbortSignal; platform?: NodeJS.Platform } = {}, ): Promise { const env = input.env ?? process.env; + const platform = input.platform ?? process.platform; const compromised = new AbortController(); const signal = AbortSignal.any([compromised.signal, ...(input.signal ? [input.signal] : [])]); signal?.throwIfAborted(); - const healthy = await findHealthyStepPage(env, signal); + const healthy = await findHealthyStepPage(env, signal, platform); if (healthy) return { command: healthy, installed: false }; - if ((input.platform ?? process.platform) === "win32") { - return { - installed: false, - error: "StepPage is not ready: automatic installation is currently supported on macOS and Linux. Step remains usable.", - }; - } let release: (() => Promise) | undefined; try { const directory = join(resolveStepHomeDir(env), ".steppage-mcp"); @@ -128,28 +198,22 @@ export async function ensureStepPageReady( } } // Another Step process may have completed installation while we waited. - const available = await findHealthyStepPage(env, signal); + const available = await findHealthyStepPage(env, signal, platform); if (available) return { command: available, installed: false }; - let nodeVersion: string; - try { - nodeVersion = (await execFileAsync("node", ["--version"], { env, signal, timeout: 5_000 })).stdout.trim(); - } catch { - signal?.throwIfAborted(); - throw new Error("Node.js >= 20 is required for StepPage; install Node.js and retry."); + await assertNodeRuntime(env, signal); + if (platform === "win32") await installStepPageOnWindows(env, signal); + else { + // Download before executing so a failed curl cannot be hidden by `| sh`. + const script = await execFileAsync("curl", ["-fsSL", "--max-time", "30", stepPageInstallerUrl(env)], { + env, + signal, + timeout: 35_000, + maxBuffer: 1_000_000, + }); + if (!script.stdout.trim()) throw new Error("The StepPage installer download was empty."); + await runInstaller(script.stdout, env, signal); } - const major = /^v(\d+)\./u.exec(nodeVersion)?.[1]; - if (!major || Number(major) < 20) - throw new Error(`Node.js >= 20 is required for StepPage; found ${nodeVersion}.`); - // Download before executing so a failed curl cannot be hidden by `| sh`. - const script = await execFileAsync("curl", ["-fsSL", "--max-time", "30", stepPageInstallerUrl(env)], { - env, - signal, - timeout: 35_000, - maxBuffer: 1_000_000, - }); - if (!script.stdout.trim()) throw new Error("The StepPage installer download was empty."); - await runInstaller(script.stdout, env, signal); - const command = await findHealthyStepPage(env, signal); + const command = await findHealthyStepPage(env, signal, platform); if (!command) throw new Error("The StepPage installer completed, but steppage-mcp failed its version check."); return { command, installed: true }; } catch (error) { @@ -160,9 +224,13 @@ export async function ensureStepPageReady( detail = detail.replaceAll(value, "[redacted]"); } const installer = `curl -fsSL '${stepPageInstallerUrl(env).replace(/'/gu, "'\\''")}' | sh`; + const manual = + platform === "win32" + ? "You can also run: step mcp prepare, then restart Step." + : `Install it with: ${installer}, then restart Step. You can also run: step mcp prepare, then restart Step.`; return { installed: false, - error: `StepPage is not ready: ${detail.slice(-2_000)} Step remains usable; StepPage setup will retry on the next launch. Install it with: ${installer}, then restart Step. You can also run: step mcp prepare, then restart Step.`, + error: `StepPage is not ready: ${detail.slice(-2_000)} Step remains usable; StepPage setup will retry on the next launch. ${manual}`, }; } finally { await release?.().catch(() => undefined); diff --git a/packages/coding-agent/test/steppage-provision-windows.test.ts b/packages/coding-agent/test/steppage-provision-windows.test.ts new file mode 100644 index 00000000..b006ca66 --- /dev/null +++ b/packages/coding-agent/test/steppage-provision-windows.test.ts @@ -0,0 +1,172 @@ +import { existsSync } from "node:fs"; +import { copyFile, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { afterEach, expect, test, vi } from "vitest"; + +const handlers = vi.hoisted(() => ({ + nodeVersion: "v24.0.0", + onCurl: undefined as undefined | ((args: readonly string[]) => void), + commandExists: (_command: string): boolean => false, +})); + +// steppage-provision.ts promisifies execFile, so the mock must publish the same +// promisify.custom contract the real child_process.execFile exposes. +vi.mock("node:child_process", () => { + const promisifyCustom = Symbol.for("nodejs.util.promisify.custom"); + const execFile = vi.fn(); + const custom = vi.fn((file: string, args: readonly string[] = []) => { + return new Promise<{ stdout: string; stderr: string }>((resolve, reject) => { + const notFound = (command: string) => { + const error: NodeJS.ErrnoException = new Error(`spawn ${command} ENOENT`); + error.code = "ENOENT"; + return error; + }; + if (file === "node" && args[0] === "--version") { + resolve({ stdout: `${handlers.nodeVersion}\n`, stderr: "" }); + } else if (file === "cmd" && args[0] === "/d" && args[1] === "/c") { + const command = args[2] ?? ""; + if (!command.endsWith(".cmd") || !handlers.commandExists(command)) reject(notFound(command)); + else resolve({ stdout: "1.0.0\n", stderr: "" }); + } else if (file === "curl") { + Promise.resolve() + .then(() => handlers.onCurl?.(args)) + .then( + () => resolve({ stdout: "", stderr: "" }), + (error) => reject(error instanceof Error ? error : new Error(String(error))), + ); + } else { + reject(notFound(`${file} ${args.join(" ")}`)); + } + }); + }); + (execFile as unknown as Record)[promisifyCustom] = custom; + const spawn = vi.fn(() => { + throw new Error("spawn must not run on the Windows provisioning path"); + }); + return { execFile, spawn }; +}); + +vi.mock("proper-lockfile", () => ({ + default: { + lock: vi.fn(async () => async () => undefined), + }, +})); + +const { ensureStepPageReady, renderStepPageWrapper, stepPageBundleUrl, stepPageCommandCandidates } = await import( + "../src/step/steppage-provision.ts" +); + +const roots: string[] = []; + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); + handlers.nodeVersion = "v24.0.0"; + handlers.onCurl = undefined; + handlers.commandExists = () => false; +}); + +async function fixture() { + const root = await mkdtemp(join(tmpdir(), "steppage-windows-")); + roots.push(root); + await mkdir(join(root, "latest"), { recursive: true }); + await writeFile(join(root, "latest", "steppage-mcp.mjs"), "// fixture bundle\n"); + await mkdir(join(root, "bin"), { recursive: true }); + const env: NodeJS.ProcessEnv = { + HOME: root, + USERPROFILE: root, + PATH: `${join(root, "bin")}:/usr/bin:/bin`, + STEPCODE_STEPPAGE_INSTALLER_URL: pathToFileURL(join(root, "installer.sh")).href, + }; + return { root, env }; +} + +function managedWrapper(root: string): string { + return join(root, ".local", "bin", "steppage-mcp.cmd"); +} + +function managedBundle(root: string): string { + return join(root, ".steppage-mcp", "bin", "steppage-mcp.mjs"); +} + +test("bundle url derives from the installer url", () => { + expect(stepPageBundleUrl()).toBe("https://dl.stepfun.com/steppage-mcp/p/latest/steppage-mcp.mjs"); + expect(stepPageBundleUrl({ STEPCODE_STEPPAGE_INSTALLER_URL: "https://mirror.example/steppage/install.sh" })).toBe( + "https://mirror.example/steppage/latest/steppage-mcp.mjs", + ); +}); + +test("windows probes the cmd wrapper", () => { + const managed = join("/home/u", ".local", "bin", "steppage-mcp"); + expect(stepPageCommandCandidates("win32", "/home/u")).toEqual([ + "steppage-mcp", + "steppage-mcp.cmd", + `${managed}.cmd`, + ]); + expect(stepPageCommandCandidates("linux", "/home/u")).toEqual(["steppage-mcp", managed]); +}); + +test("wrapper runs the bundle through node", () => { + const bundle = join("C:", "Users", "u", ".steppage-mcp", "bin", "steppage-mcp.mjs"); + expect(renderStepPageWrapper(bundle)).toBe(`@echo off\r\nnode "${bundle}" %*\r\n`); +}); + +test("windows installs the published bundle and a cmd wrapper", async () => { + const f = await fixture(); + handlers.commandExists = (command) => command.endsWith(".cmd") && existsSync(command); + handlers.onCurl = (args) => { + const url = args.find((arg) => arg.startsWith("file://")); + const output = args[args.indexOf("-o") + 1]; + if (!url || !output) throw new Error(`unexpected curl args: ${args.join(" ")}`); + return copyFile(fileURLToPath(url), output); + }; + expect(await ensureStepPageReady({ env: f.env, platform: "win32" })).toEqual({ + command: managedWrapper(f.root), + installed: true, + }); + expect(await readFile(managedBundle(f.root), "utf8")).toBe("// fixture bundle\n"); + expect(await readFile(managedWrapper(f.root), "utf8")).toBe(renderStepPageWrapper(managedBundle(f.root))); +}); + +test("windows reuses a healthy wrapper without downloading", async () => { + const f = await fixture(); + await mkdir(dirname(managedWrapper(f.root)), { recursive: true }); + await writeFile(managedWrapper(f.root), renderStepPageWrapper(managedBundle(f.root))); + handlers.commandExists = (command) => command.endsWith(".cmd") && existsSync(command); + let curlCalls = 0; + handlers.onCurl = () => { + curlCalls += 1; + }; + expect(await ensureStepPageReady({ env: f.env, platform: "win32" })).toEqual({ + command: managedWrapper(f.root), + installed: false, + }); + expect(curlCalls).toBe(0); + expect(existsSync(managedBundle(f.root))).toBe(false); +}); + +test("windows requires Node.js >= 20", async () => { + const f = await fixture(); + handlers.nodeVersion = "v18.20.0"; + handlers.onCurl = () => { + throw new Error("curl must not run when the Node.js runtime is too old"); + }; + const result = await ensureStepPageReady({ env: f.env, platform: "win32" }); + expect(result.installed).toBe(false); + expect(result.error).toContain("Node.js >= 20"); + expect(result.error).toContain("v18.20.0"); + expect(existsSync(managedWrapper(f.root))).toBe(false); +}); + +test("windows failure keeps Step usable and points at step mcp prepare", async () => { + const f = await fixture(); + handlers.onCurl = () => { + throw new Error("curl: (22) The requested URL returned error: 404"); + }; + const result = await ensureStepPageReady({ env: f.env, platform: "win32" }); + expect(result.installed).toBe(false); + expect(result.error).toContain("StepPage is not ready"); + expect(result.error).toContain("step mcp prepare"); + expect(result.error).not.toContain("| sh"); +}); From 9ea890477462bc7c8e22fbad912de9f394d7961f Mon Sep 17 00:00:00 2001 From: Hong Liu <88263959+Hylouis233@users.noreply.github.com> Date: Sun, 25 Oct 2026 06:38:23 +0800 Subject: [PATCH 2/4] fix(steppage): harden Windows provisioning probes and mirror URLs On Windows, `cmd` searches the current working directory before PATH, so probing the bare `steppage-mcp` name could execute a project-controlled `steppage-mcp.cmd`. Windows candidates are now resolved to absolute files from absolute PATH entries only (`.cmd`, `.exe`, `.bat`), followed by the managed `~/.local/bin/steppage-mcp.cmd` wrapper; relative PATH entries, including `.`, are skipped. The bundle URL is now derived from STEPCODE_STEPPAGE_INSTALLER_URL with URL parsing, so a mirror's credentials, query parameters, percent-encoding and fragment are preserved when switching to `latest/steppage-mcp.mjs`. Tests cover absolute-only Windows probes, a PATH that contains the working directory, and mirror URL preservation. --- docs/step-configuration.md | 3 ++ .../src/step/steppage-provision.ts | 42 ++++++++++++++--- .../test/steppage-provision-windows.test.ts | 45 ++++++++++++++++--- 3 files changed, 76 insertions(+), 14 deletions(-) diff --git a/docs/step-configuration.md b/docs/step-configuration.md index cd64fa97..0f011b65 100644 --- a/docs/step-configuration.md +++ b/docs/step-configuration.md @@ -231,6 +231,9 @@ On Windows the published bundle is downloaded directly and wrapped in a `steppage-mcp.cmd` launcher under `~/.local/bin`, because the POSIX installer script cannot run there; `step mcp prepare` probes that wrapper through `cmd /c` so no shell PATH change is required. +`STEPCODE_STEPPAGE_INSTALLER_URL` can select a channel or mirror. Windows derives +the sibling `latest/steppage-mcp.mjs` path with URL parsing and preserves the +configured URL's authentication parameters and encoding. Preparation preserves credentials, custom server commands, global overrides, disabled servers, and intentionally uninstalled plugins. A global declaration diff --git a/packages/coding-agent/src/step/steppage-provision.ts b/packages/coding-agent/src/step/steppage-provision.ts index 8b04a87e..b3a12e20 100644 --- a/packages/coding-agent/src/step/steppage-provision.ts +++ b/packages/coding-agent/src/step/steppage-provision.ts @@ -1,6 +1,7 @@ import { execFile, spawn } from "node:child_process"; +import { existsSync } from "node:fs"; import { mkdir, writeFile } from "node:fs/promises"; -import { join } from "node:path"; +import { isAbsolute, join, win32 } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { promisify } from "node:util"; import lockfile from "proper-lockfile"; @@ -25,17 +26,44 @@ export function stepPageInstallerUrl(env: NodeJS.ProcessEnv = process.env): stri * installer URL so a channel or mirror override covers both artifacts. */ export function stepPageBundleUrl(env: NodeJS.ProcessEnv = process.env): string { - return `${stepPageInstallerUrl(env).replace(/\/[^/]*$/u, "")}/latest/steppage-mcp.mjs`; + const url = new URL(stepPageInstallerUrl(env)); + url.pathname = `${url.pathname.replace(/\/[^/]*$/u, "")}/latest/steppage-mcp.mjs`; + return url.href; } /** * Probe order for an existing executable: PATH first, then the managed install. * Windows probes the `.cmd` wrapper because the bundle is a bare ESM file. + * + * On Windows, `cmd` searches the current directory before PATH, so a bare name + * could execute a project-controlled `steppage-mcp.cmd`. Windows candidates are + * therefore resolved to absolute files from absolute PATH entries only. */ -export function stepPageCommandCandidates(platform: NodeJS.Platform, homeDir: string): string[] { +export function stepPageCommandCandidates( + platform: NodeJS.Platform, + homeDir: string, + pathValue?: string, + fileExists: (path: string) => boolean = existsSync, +): string[] { const managed = join(homeDir, ".local", "bin", "steppage-mcp"); - if (platform === "win32") return ["steppage-mcp", "steppage-mcp.cmd", `${managed}.cmd`]; - return ["steppage-mcp", managed]; + if (platform !== "win32") return ["steppage-mcp", managed]; + const candidates: string[] = []; + for (const rawEntry of (pathValue ?? "").split(";")) { + const entry = rawEntry.trim().replace(/^"(.*)"$/u, "$1"); + // Relative entries (including ".") resolve against the working directory. + if (!entry || !(win32.isAbsolute(entry) || isAbsolute(entry))) continue; + for (const extension of [".cmd", ".exe", ".bat"]) { + const candidate = join(entry, `steppage-mcp${extension}`); + if (!candidates.includes(candidate) && fileExists(candidate)) candidates.push(candidate); + } + } + if (!candidates.includes(`${managed}.cmd`)) candidates.push(`${managed}.cmd`); + return candidates; +} + +function pathValueOf(env: NodeJS.ProcessEnv): string | undefined { + const key = Object.keys(env).find((name) => name.toUpperCase() === "PATH"); + return key ? env[key] : undefined; } /** The Windows wrapper installed next to the bundle; a bare ESM file must run via node. */ @@ -99,7 +127,7 @@ async function runInstaller(script: string, env: NodeJS.ProcessEnv, signal: Abor }); } -/** `cmd /c` resolves both bare PATHEXT lookups and explicit `.cmd` files on every Node runtime. */ +/** On Windows every candidate is an absolute file; `cmd /c` runs `.cmd` wrappers on every Node runtime. */ async function probeStepPageVersion( command: string, env: NodeJS.ProcessEnv, @@ -126,7 +154,7 @@ async function findHealthyStepPage( signal?: AbortSignal, platform: NodeJS.Platform = process.platform, ): Promise { - for (const command of stepPageCommandCandidates(platform, resolveStepHomeDir(env))) { + for (const command of stepPageCommandCandidates(platform, resolveStepHomeDir(env), pathValueOf(env))) { signal?.throwIfAborted(); if (await probeStepPageVersion(command, env, signal, platform)) return command; } diff --git a/packages/coding-agent/test/steppage-provision-windows.test.ts b/packages/coding-agent/test/steppage-provision-windows.test.ts index b006ca66..1af09780 100644 --- a/packages/coding-agent/test/steppage-provision-windows.test.ts +++ b/packages/coding-agent/test/steppage-provision-windows.test.ts @@ -1,7 +1,7 @@ import { existsSync } from "node:fs"; import { copyFile, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { dirname, join } from "node:path"; +import { dirname, isAbsolute, join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { afterEach, expect, test, vi } from "vitest"; @@ -27,7 +27,7 @@ vi.mock("node:child_process", () => { resolve({ stdout: `${handlers.nodeVersion}\n`, stderr: "" }); } else if (file === "cmd" && args[0] === "/d" && args[1] === "/c") { const command = args[2] ?? ""; - if (!command.endsWith(".cmd") || !handlers.commandExists(command)) reject(notFound(command)); + if (!handlers.commandExists(command) || !command.endsWith(".cmd")) reject(notFound(command)); else resolve({ stdout: "1.0.0\n", stderr: "" }); } else if (file === "curl") { Promise.resolve() @@ -97,16 +97,47 @@ test("bundle url derives from the installer url", () => { ); }); +test("bundle URL preserves mirror authentication, encoding, and fragment", () => { + expect( + stepPageBundleUrl({ + STEPCODE_STEPPAGE_INSTALLER_URL: + "https://user:pass@mirror.example/a%20b/install.sh?token=abc%2F123&channel=preview#download", + }), + ).toBe("https://user:pass@mirror.example/a%20b/latest/steppage-mcp.mjs?token=abc%2F123&channel=preview#download"); +}); + test("windows probes the cmd wrapper", () => { const managed = join("/home/u", ".local", "bin", "steppage-mcp"); - expect(stepPageCommandCandidates("win32", "/home/u")).toEqual([ - "steppage-mcp", - "steppage-mcp.cmd", - `${managed}.cmd`, - ]); + expect(stepPageCommandCandidates("win32", "/home/u")).toEqual([`${managed}.cmd`]); expect(stepPageCommandCandidates("linux", "/home/u")).toEqual(["steppage-mcp", managed]); }); +test("windows resolves PATH candidates to absolute files and never the working directory", () => { + const managed = join("/home/u", ".local", "bin", "steppage-mcp"); + const tools = join("/opt", "tools"); + const present = new Set([join(tools, "steppage-mcp.cmd"), join("bin", "steppage-mcp.cmd")]); + const candidates = stepPageCommandCandidates("win32", "/home/u", `.;bin;;"${tools}";/missing`, (path) => + present.has(path), + ); + expect(candidates).toEqual([join(tools, "steppage-mcp.cmd"), `${managed}.cmd`]); + for (const candidate of candidates) expect(isAbsolute(candidate)).toBe(true); +}); + +test("windows never probes a bare command that cmd could find in the working directory", async () => { + const f = await fixture(); + const probed: string[] = []; + handlers.commandExists = (command) => { + probed.push(command); + return false; + }; + handlers.onCurl = () => { + throw new Error("offline"); + }; + await ensureStepPageReady({ env: { ...f.env, PATH: `.;${join(f.root, "bin")}` }, platform: "win32" }); + expect(probed.length).toBeGreaterThan(0); + for (const command of probed) expect(isAbsolute(command)).toBe(true); +}); + test("wrapper runs the bundle through node", () => { const bundle = join("C:", "Users", "u", ".steppage-mcp", "bin", "steppage-mcp.mjs"); expect(renderStepPageWrapper(bundle)).toBe(`@echo off\r\nnode "${bundle}" %*\r\n`); From d77b1efe74036a124beabed22e0c4224805c3792 Mon Sep 17 00:00:00 2001 From: Hong Liu <88263959+Hylouis233@users.noreply.github.com> Date: Mon, 26 Oct 2026 03:37:08 +0800 Subject: [PATCH 3/4] fix(steppage): pin node.exe and redact mirror credentials in errors Native Windows validation showed the generated `steppage-mcp.cmd` wrapper invoked a bare `node`, which `cmd` resolves from the working directory before PATH: a project-controlled `node.cmd` ran in its place. Windows now resolves `node.exe` from absolute PATH entries only, runs the Node.js >= 20 check with that absolute path, and writes the same path into the wrapper. Setup fails closed when no such `node.exe` exists. `process.execPath` is not used because in the standalone binary it is Step itself. Mirror URLs could also carry a password or token into the final error text on both the Windows and POSIX paths, either from a curl failure or from the manual install hint. The complete message is now redacted: URL userinfo, token-like query or fragment parameters, and secret-named environment values. Detail is redacted before truncation so a cut cannot strip the scheme that marks a URL. The missing-executable hint in the MCP startup message uses the same redaction. Tests cover node.exe resolution that skips relative entries and the working directory, the runtime check and wrapper sharing one absolute path, failing closed without node.exe, and credential-free failure text for Windows, Linux and the MCP startup hint. --- docs/step-configuration.md | 5 + packages/coding-agent/src/step/mcp.test.ts | 12 ++ packages/coding-agent/src/step/mcp.ts | 6 +- .../src/step/steppage-provision.ts | 107 ++++++++++++++---- .../test/steppage-provision-windows.test.ts | 99 ++++++++++++++-- 5 files changed, 198 insertions(+), 31 deletions(-) diff --git a/docs/step-configuration.md b/docs/step-configuration.md index 0f011b65..72aef05c 100644 --- a/docs/step-configuration.md +++ b/docs/step-configuration.md @@ -231,9 +231,14 @@ On Windows the published bundle is downloaded directly and wrapped in a `steppage-mcp.cmd` launcher under `~/.local/bin`, because the POSIX installer script cannot run there; `step mcp prepare` probes that wrapper through `cmd /c` so no shell PATH change is required. +The launcher names `node.exe` by the absolute path found on an absolute PATH +entry, the same executable that passed the Node.js >= 20 check, so a `node.cmd` +in the working directory cannot be run in its place. `STEPCODE_STEPPAGE_INSTALLER_URL` can select a channel or mirror. Windows derives the sibling `latest/steppage-mcp.mjs` path with URL parsing and preserves the configured URL's authentication parameters and encoding. +Setup errors and install hints redact URL credentials and token-like query +parameters before they are shown. Preparation preserves credentials, custom server commands, global overrides, disabled servers, and intentionally uninstalled plugins. A global declaration diff --git a/packages/coding-agent/src/step/mcp.test.ts b/packages/coding-agent/src/step/mcp.test.ts index 8c4e61c5..3067a510 100644 --- a/packages/coding-agent/src/step/mcp.test.ts +++ b/packages/coding-agent/src/step/mcp.test.ts @@ -42,6 +42,18 @@ test("login guidance quotes server names containing shell syntax", () => { expect(message).toContain("step mcp login 'team'\\''s mcp'"); }); +test("missing StepPage executable hint does not print mirror credentials", () => { + const message = describeMcpStartFailure({ + name: "steppage__steppage", + command: "steppage-mcp", + provision: { command: "steppage-mcp", installer: "steppageInstaller" }, + error: Object.assign(new Error("spawn steppage-mcp ENOENT"), { code: "ENOENT" }), + env: { STEPCODE_STEPPAGE_INSTALLER_URL: "https://alice:hunter2@mirror.example/install.sh?token=abc123" }, + }); + expect(message).toContain("curl -fsSL 'https://[redacted]@mirror.example/install.sh?token=[redacted]' | sh"); + for (const secret of ["alice", "hunter2", "abc123"]) expect(message).not.toContain(secret); +}); + afterEach(async () => { await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); }); diff --git a/packages/coding-agent/src/step/mcp.ts b/packages/coding-agent/src/step/mcp.ts index e6680b2d..c1fd1aa1 100644 --- a/packages/coding-agent/src/step/mcp.ts +++ b/packages/coding-agent/src/step/mcp.ts @@ -22,7 +22,7 @@ import { readStepPluginManifest, type StepPluginProvision, } from "./plugins.ts"; -import { ensureStepPageReady, isManagedStepPageCommand } from "./steppage-provision.ts"; +import { ensureStepPageReady, isManagedStepPageCommand, redactStepPageSecrets } from "./steppage-provision.ts"; import { STEPCODE_VERSION } from "./version.ts"; export { resolveStepMcpEnvironment } from "./mcp-environment.ts"; @@ -709,7 +709,9 @@ export function describeMcpStartFailure(input: { } if (!isMissingExecutable(input.error)) return `MCP server '${input.name}' could not start: ${detail}`; const install = input.provision ? provisionInstallCommand(input.provision, input.env ?? process.env) : undefined; - const remedy = install ? `Install it with: ${install}, then restart Step.` : "Install it, then restart Step."; + const remedy = install + ? `Install it with: ${redactStepPageSecrets(install, input.env ?? process.env)}, then restart Step.` + : "Install it, then restart Step."; return `MCP server '${input.name}' could not start: '${input.command}' is not installed or not on PATH. ${remedy}`; } diff --git a/packages/coding-agent/src/step/steppage-provision.ts b/packages/coding-agent/src/step/steppage-provision.ts index b3a12e20..c011ef12 100644 --- a/packages/coding-agent/src/step/steppage-provision.ts +++ b/packages/coding-agent/src/step/steppage-provision.ts @@ -48,10 +48,7 @@ export function stepPageCommandCandidates( const managed = join(homeDir, ".local", "bin", "steppage-mcp"); if (platform !== "win32") return ["steppage-mcp", managed]; const candidates: string[] = []; - for (const rawEntry of (pathValue ?? "").split(";")) { - const entry = rawEntry.trim().replace(/^"(.*)"$/u, "$1"); - // Relative entries (including ".") resolve against the working directory. - if (!entry || !(win32.isAbsolute(entry) || isAbsolute(entry))) continue; + for (const entry of absoluteWindowsPathEntries(pathValue)) { for (const extension of [".cmd", ".exe", ".bat"]) { const candidate = join(entry, `steppage-mcp${extension}`); if (!candidates.includes(candidate) && fileExists(candidate)) candidates.push(candidate); @@ -61,14 +58,78 @@ export function stepPageCommandCandidates( return candidates; } +/** Relative PATH entries (including ".") resolve against the working directory, so they are skipped. */ +function absoluteWindowsPathEntries(pathValue: string | undefined): string[] { + const entries: string[] = []; + for (const rawEntry of (pathValue ?? "").split(";")) { + const entry = rawEntry.trim().replace(/^"(.*)"$/u, "$1"); + if (entry && (win32.isAbsolute(entry) || isAbsolute(entry))) entries.push(entry); + } + return entries; +} + +/** + * The Node.js executable that checks the runtime and runs the bundle. On Windows + * `cmd` searches the working directory before PATH, so a bare `node` in the + * wrapper could run a project-controlled `node.cmd`; `node.exe` is resolved from + * absolute PATH entries instead. `process.execPath` is deliberately not used: in + * the standalone binary it is Step itself, not Node.js. + */ +export function resolveNodeExecutable( + platform: NodeJS.Platform, + pathValue?: string, + fileExists: (path: string) => boolean = existsSync, +): string | undefined { + if (platform !== "win32") return "node"; + for (const entry of absoluteWindowsPathEntries(pathValue)) { + const candidate = join(entry, "node.exe"); + if (fileExists(candidate)) return candidate; + } + return undefined; +} + function pathValueOf(env: NodeJS.ProcessEnv): string | undefined { const key = Object.keys(env).find((name) => name.toUpperCase() === "PATH"); return key ? env[key] : undefined; } -/** The Windows wrapper installed next to the bundle; a bare ESM file must run via node. */ -export function renderStepPageWrapper(bundlePath: string): string { - return `@echo off\r\nnode "${bundlePath}" %*\r\n`; +/** + * The Windows wrapper installed next to the bundle; a bare ESM file must run via + * node, named by the absolute path that passed the runtime check. `%` is doubled + * so a path cannot expand a variable inside the batch file. + */ +export function renderStepPageWrapper(nodePath: string, bundlePath: string): string { + const quote = (path: string) => `"${path.replaceAll("%", "%%")}"`; + return `@echo off\r\n${quote(nodePath)} ${quote(bundlePath)} %*\r\n`; +} + +const SECRET_PARAMETER = /token|key|secret|passw(?:or)?d|pwd|auth|credential|signature|sig$|session/iu; + +function redactUrl(url: string): string { + return url + .replace(/^([a-z][a-z\d+.-]*:\/\/)[^/?#\s]*@/iu, "$1[redacted]@") + .replace(/([?&;#])([^=&;#]+)=([^&;#]*)/gu, (match, separator: string, name: string, value: string) => { + let decoded = name; + try { + decoded = decodeURIComponent(name); + } catch {} + return value && SECRET_PARAMETER.test(decoded) ? `${separator}${name}=[redacted]` : match; + }); +} + +/** + * Remove credentials from text shown to the user: URL userinfo, secret-looking + * query or fragment parameters, and the values of secret-named environment + * variables. Applied to the complete message, so a mirror URL is covered whether + * it came from a curl error or from the manual install hint. + */ +export function redactStepPageSecrets(text: string, env: NodeJS.ProcessEnv = process.env): string { + let redacted = text.replace(/\b[a-z][a-z\d+.-]*:\/\/[^\s"'<>`]+/giu, redactUrl); + for (const [key, value] of Object.entries(env)) { + if (value && value.length >= 4 && /key|token|secret|password|authorization/iu.test(key)) + redacted = redacted.replaceAll(value, "[redacted]"); + } + return redacted; } /** Default executable ownership may survive in an old global declaration. */ @@ -161,23 +222,31 @@ async function findHealthyStepPage( return undefined; } -async function assertNodeRuntime(env: NodeJS.ProcessEnv, signal: AbortSignal): Promise { +/** Returns the Node.js executable that passed the check, for the wrapper to reuse. */ +async function assertNodeRuntime( + env: NodeJS.ProcessEnv, + signal: AbortSignal, + platform: NodeJS.Platform, +): Promise { + const node = resolveNodeExecutable(platform, pathValueOf(env)); + if (!node) throw new Error("Node.js >= 20 is required for StepPage; node.exe was not found on PATH."); let nodeVersion: string; try { - nodeVersion = (await execFileAsync("node", ["--version"], { env, signal, timeout: 5_000 })).stdout.trim(); + nodeVersion = (await execFileAsync(node, ["--version"], { env, signal, timeout: 5_000 })).stdout.trim(); } catch { signal.throwIfAborted(); throw new Error("Node.js >= 20 is required for StepPage; install Node.js and retry."); } const major = /^v(\d+)\./u.exec(nodeVersion)?.[1]; if (!major || Number(major) < 20) throw new Error(`Node.js >= 20 is required for StepPage; found ${nodeVersion}.`); + return node; } /** * Windows cannot run the POSIX installer script, so install the published bundle * directly: download it, then write the `cmd` wrapper the bundle needs to run. */ -async function installStepPageOnWindows(env: NodeJS.ProcessEnv, signal: AbortSignal): Promise { +async function installStepPageOnWindows(env: NodeJS.ProcessEnv, signal: AbortSignal, nodePath: string): Promise { const home = resolveStepHomeDir(env); const bundleDir = join(home, ".steppage-mcp", "bin"); const bundlePath = join(bundleDir, "steppage-mcp.mjs"); @@ -191,7 +260,7 @@ async function installStepPageOnWindows(env: NodeJS.ProcessEnv, signal: AbortSig timeout: 35_000, maxBuffer: 8_192, }); - await writeFile(wrapperPath, renderStepPageWrapper(bundlePath)); + await writeFile(wrapperPath, renderStepPageWrapper(nodePath, bundlePath)); } /** Repair the managed executable without changing plugin manifests or credentials. */ @@ -228,8 +297,8 @@ export async function ensureStepPageReady( // Another Step process may have completed installation while we waited. const available = await findHealthyStepPage(env, signal, platform); if (available) return { command: available, installed: false }; - await assertNodeRuntime(env, signal); - if (platform === "win32") await installStepPageOnWindows(env, signal); + const nodePath = await assertNodeRuntime(env, signal, platform); + if (platform === "win32") await installStepPageOnWindows(env, signal, nodePath); else { // Download before executing so a failed curl cannot be hidden by `| sh`. const script = await execFileAsync("curl", ["-fsSL", "--max-time", "30", stepPageInstallerUrl(env)], { @@ -246,11 +315,8 @@ export async function ensureStepPageReady( return { command, installed: true }; } catch (error) { input.signal?.throwIfAborted(); - let detail = error instanceof Error ? error.message : String(error); - for (const [key, value] of Object.entries(env)) { - if (value && value.length >= 4 && /key|token|secret|password|authorization/iu.test(key)) - detail = detail.replaceAll(value, "[redacted]"); - } + // Redact before truncating: a cut could drop the scheme that marks a URL. + const detail = redactStepPageSecrets(error instanceof Error ? error.message : String(error), env); const installer = `curl -fsSL '${stepPageInstallerUrl(env).replace(/'/gu, "'\\''")}' | sh`; const manual = platform === "win32" @@ -258,7 +324,10 @@ export async function ensureStepPageReady( : `Install it with: ${installer}, then restart Step. You can also run: step mcp prepare, then restart Step.`; return { installed: false, - error: `StepPage is not ready: ${detail.slice(-2_000)} Step remains usable; StepPage setup will retry on the next launch. ${manual}`, + error: redactStepPageSecrets( + `StepPage is not ready: ${detail.slice(-2_000)} Step remains usable; StepPage setup will retry on the next launch. ${manual}`, + env, + ), }; } finally { await release?.().catch(() => undefined); diff --git a/packages/coding-agent/test/steppage-provision-windows.test.ts b/packages/coding-agent/test/steppage-provision-windows.test.ts index 1af09780..26eb0d9e 100644 --- a/packages/coding-agent/test/steppage-provision-windows.test.ts +++ b/packages/coding-agent/test/steppage-provision-windows.test.ts @@ -7,6 +7,7 @@ import { afterEach, expect, test, vi } from "vitest"; const handlers = vi.hoisted(() => ({ nodeVersion: "v24.0.0", + nodeCalls: [] as string[], onCurl: undefined as undefined | ((args: readonly string[]) => void), commandExists: (_command: string): boolean => false, })); @@ -23,7 +24,8 @@ vi.mock("node:child_process", () => { error.code = "ENOENT"; return error; }; - if (file === "node" && args[0] === "--version") { + if ((file === "node" || file.endsWith("node.exe")) && args[0] === "--version") { + handlers.nodeCalls.push(file); resolve({ stdout: `${handlers.nodeVersion}\n`, stderr: "" }); } else if (file === "cmd" && args[0] === "/d" && args[1] === "/c") { const command = args[2] ?? ""; @@ -54,15 +56,21 @@ vi.mock("proper-lockfile", () => ({ }, })); -const { ensureStepPageReady, renderStepPageWrapper, stepPageBundleUrl, stepPageCommandCandidates } = await import( - "../src/step/steppage-provision.ts" -); +const { + ensureStepPageReady, + redactStepPageSecrets, + renderStepPageWrapper, + resolveNodeExecutable, + stepPageBundleUrl, + stepPageCommandCandidates, +} = await import("../src/step/steppage-provision.ts"); const roots: string[] = []; afterEach(async () => { await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); handlers.nodeVersion = "v24.0.0"; + handlers.nodeCalls = []; handlers.onCurl = undefined; handlers.commandExists = () => false; }); @@ -73,13 +81,14 @@ async function fixture() { await mkdir(join(root, "latest"), { recursive: true }); await writeFile(join(root, "latest", "steppage-mcp.mjs"), "// fixture bundle\n"); await mkdir(join(root, "bin"), { recursive: true }); + await writeFile(join(root, "bin", "node.exe"), ""); const env: NodeJS.ProcessEnv = { HOME: root, USERPROFILE: root, - PATH: `${join(root, "bin")}:/usr/bin:/bin`, + PATH: join(root, "bin"), STEPCODE_STEPPAGE_INSTALLER_URL: pathToFileURL(join(root, "installer.sh")).href, }; - return { root, env }; + return { root, env, node: join(root, "bin", "node.exe") }; } function managedWrapper(root: string): string { @@ -138,9 +147,37 @@ test("windows never probes a bare command that cmd could find in the working dir for (const command of probed) expect(isAbsolute(command)).toBe(true); }); -test("wrapper runs the bundle through node", () => { +test("wrapper runs the bundle through the absolute node path", () => { + const node = join("C:", "Program Files", "nodejs", "node.exe"); const bundle = join("C:", "Users", "u", ".steppage-mcp", "bin", "steppage-mcp.mjs"); - expect(renderStepPageWrapper(bundle)).toBe(`@echo off\r\nnode "${bundle}" %*\r\n`); + expect(renderStepPageWrapper(node, bundle)).toBe(`@echo off\r\n"${node}" "${bundle}" %*\r\n`); + expect(renderStepPageWrapper(join("C:", "100%", "node.exe"), bundle)).toContain( + `"${join("C:", "100%%", "node.exe")}"`, + ); +}); + +test("windows resolves node.exe from absolute PATH entries only", () => { + const nodejs = join("/opt", "nodejs"); + const present = new Set([join(".", "node.exe"), join("tools", "node.exe"), join(nodejs, "node.exe")]); + const exists = (path: string) => present.has(path); + expect(resolveNodeExecutable("win32", `.;tools;;"${nodejs}"`, exists)).toBe(join(nodejs, "node.exe")); + expect(resolveNodeExecutable("win32", ".;tools", exists)).toBeUndefined(); + expect(resolveNodeExecutable("win32", undefined, exists)).toBeUndefined(); + expect(resolveNodeExecutable("linux", "/usr/bin", exists)).toBe("node"); +}); + +test("windows fails closed when node.exe is not on an absolute PATH entry", async () => { + const f = await fixture(); + await mkdir(join(f.root, "project"), { recursive: true }); + await writeFile(join(f.root, "project", "node.cmd"), "@echo NODE_HIJACKED_FROM_CWD\r\n"); + handlers.onCurl = () => { + throw new Error("curl must not run without a resolved node.exe"); + }; + const result = await ensureStepPageReady({ env: { ...f.env, PATH: ".;project" }, platform: "win32" }); + expect(result.installed).toBe(false); + expect(result.error).toContain("node.exe was not found on PATH"); + expect(handlers.nodeCalls).toEqual([]); + expect(existsSync(managedWrapper(f.root))).toBe(false); }); test("windows installs the published bundle and a cmd wrapper", async () => { @@ -157,13 +194,16 @@ test("windows installs the published bundle and a cmd wrapper", async () => { installed: true, }); expect(await readFile(managedBundle(f.root), "utf8")).toBe("// fixture bundle\n"); - expect(await readFile(managedWrapper(f.root), "utf8")).toBe(renderStepPageWrapper(managedBundle(f.root))); + expect(await readFile(managedWrapper(f.root), "utf8")).toBe(renderStepPageWrapper(f.node, managedBundle(f.root))); + expect(await readFile(managedWrapper(f.root), "utf8")).toContain(`"${f.node}" "${managedBundle(f.root)}"`); + // The runtime check and the wrapper use the same absolute node.exe. + expect(handlers.nodeCalls).toEqual([f.node]); }); test("windows reuses a healthy wrapper without downloading", async () => { const f = await fixture(); await mkdir(dirname(managedWrapper(f.root)), { recursive: true }); - await writeFile(managedWrapper(f.root), renderStepPageWrapper(managedBundle(f.root))); + await writeFile(managedWrapper(f.root), renderStepPageWrapper(f.node, managedBundle(f.root))); handlers.commandExists = (command) => command.endsWith(".cmd") && existsSync(command); let curlCalls = 0; handlers.onCurl = () => { @@ -201,3 +241,42 @@ test("windows failure keeps Step usable and points at step mcp prepare", async ( expect(result.error).toContain("step mcp prepare"); expect(result.error).not.toContain("| sh"); }); + +test("redaction removes URL credentials and secret parameters but keeps the rest", () => { + expect( + redactStepPageSecrets( + "curl failed for https://alice:hunter2@mirror.example/a%20b/install.sh?channel=preview&token=abc123&X-Amz-Signature=deadbeef#access_token=xyz", + {}, + ), + ).toBe( + "curl failed for https://[redacted]@mirror.example/a%20b/install.sh?channel=preview&token=[redacted]&X-Amz-Signature=[redacted]#access_token=[redacted]", + ); + expect(redactStepPageSecrets("see 'https://ghp_secretvalue@mirror.example/install.sh' | sh", {})).toBe( + "see 'https://[redacted]@mirror.example/install.sh' | sh", + ); + expect(redactStepPageSecrets("https://mirror.example/install.sh?api%5Fkey=k1&v=2", {})).toBe( + "https://mirror.example/install.sh?api%5Fkey=[redacted]&v=2", + ); + expect(redactStepPageSecrets("plain https://dl.stepfun.com/steppage-mcp/p/install.sh text", {})).toBe( + "plain https://dl.stepfun.com/steppage-mcp/p/install.sh text", + ); + expect(redactStepPageSecrets("value s3cr3t-value leaked", { MIRROR_TOKEN: "s3cr3t-value" })).toBe( + "value [redacted] leaked", + ); +}); + +test.each(["win32", "linux"] as const)("%s failure text never contains mirror credentials", async (platform) => { + const f = await fixture(); + const installer = "https://alice:hunter2@mirror.example/steppage/install.sh?token=abc123&channel=preview"; + const env = { ...f.env, STEPCODE_STEPPAGE_INSTALLER_URL: installer }; + handlers.onCurl = (args) => { + const url = args.find((arg) => arg.startsWith("https://")); + throw new Error(`Command failed: curl -fsSL --max-time 30 ${url}\ncurl: (22) ${url}: 403 Forbidden`); + }; + const result = await ensureStepPageReady({ env, platform }); + expect(result.installed).toBe(false); + expect(result.error).toContain("403 Forbidden"); + expect(result.error).toContain("https://[redacted]@mirror.example/steppage/"); + expect(result.error).toContain("channel=preview"); + for (const secret of ["alice", "hunter2", "abc123"]) expect(result.error).not.toContain(secret); +}); From e37e8d44ac5cc1a290e0599466237968e781f6a5 Mon Sep 17 00:00:00 2001 From: li-xiu-qi Date: Mon, 26 Oct 2026 14:03:13 +0800 Subject: [PATCH 4/4] fix(steppage): harden Windows provisioning boundaries Resolve Windows provisioning executables from absolute PATH entries and probe wrappers by basename from their own directory. Redact quoted mirror URLs and use the native preparation hint on Windows. --- docs/step-configuration.md | 6 +- packages/coding-agent/src/step/mcp.test.ts | 17 ++++ packages/coding-agent/src/step/mcp.ts | 16 +++- .../src/step/steppage-provision.ts | 54 ++++++++---- .../test/steppage-provision-windows.test.ts | 87 ++++++++++++++++--- 5 files changed, 145 insertions(+), 35 deletions(-) diff --git a/docs/step-configuration.md b/docs/step-configuration.md index 72aef05c..e692cc9b 100644 --- a/docs/step-configuration.md +++ b/docs/step-configuration.md @@ -231,9 +231,9 @@ On Windows the published bundle is downloaded directly and wrapped in a `steppage-mcp.cmd` launcher under `~/.local/bin`, because the POSIX installer script cannot run there; `step mcp prepare` probes that wrapper through `cmd /c` so no shell PATH change is required. -The launcher names `node.exe` by the absolute path found on an absolute PATH -entry, the same executable that passed the Node.js >= 20 check, so a `node.cmd` -in the working directory cannot be run in its place. +Windows resolves `node.exe`, `cmd.exe`, and `curl.exe` only from absolute PATH +entries, so the working directory cannot supply those executables. The launcher +uses the same `node.exe` that passed the Node.js >= 20 check. `STEPCODE_STEPPAGE_INSTALLER_URL` can select a channel or mirror. Windows derives the sibling `latest/steppage-mcp.mjs` path with URL parsing and preserves the configured URL's authentication parameters and encoding. diff --git a/packages/coding-agent/src/step/mcp.test.ts b/packages/coding-agent/src/step/mcp.test.ts index 3067a510..61c9e662 100644 --- a/packages/coding-agent/src/step/mcp.test.ts +++ b/packages/coding-agent/src/step/mcp.test.ts @@ -48,12 +48,27 @@ test("missing StepPage executable hint does not print mirror credentials", () => command: "steppage-mcp", provision: { command: "steppage-mcp", installer: "steppageInstaller" }, error: Object.assign(new Error("spawn steppage-mcp ENOENT"), { code: "ENOENT" }), + platform: "linux", env: { STEPCODE_STEPPAGE_INSTALLER_URL: "https://alice:hunter2@mirror.example/install.sh?token=abc123" }, }); expect(message).toContain("curl -fsSL 'https://[redacted]@mirror.example/install.sh?token=[redacted]' | sh"); for (const secret of ["alice", "hunter2", "abc123"]) expect(message).not.toContain(secret); }); +test("Windows missing StepPage executable hint points to native preparation", () => { + const message = describeMcpStartFailure({ + name: "steppage__steppage", + command: "steppage-mcp", + provision: { command: "steppage-mcp", installer: "steppageInstaller" }, + error: Object.assign(new Error("spawn steppage-mcp ENOENT"), { code: "ENOENT" }), + platform: "win32", + env: { STEPCODE_STEPPAGE_INSTALLER_URL: "https://alice:hunter2@mirror.example/install.sh?token=abc123" }, + }); + expect(message).toContain("step mcp prepare"); + expect(message).not.toContain("| sh"); + for (const secret of ["alice", "hunter2", "abc123"]) expect(message).not.toContain(secret); +}); + afterEach(async () => { await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); }); @@ -149,6 +164,7 @@ test("a missing executable is reported with the installer command instead of a r error: Object.assign(new Error("spawn steppage-mcp ENOENT"), { code: "ENOENT", }), + platform: "linux", env: {}, }); @@ -166,6 +182,7 @@ test("the installer override is honoured in the guidance", () => { error: Object.assign(new Error("spawn steppage-mcp ENOENT"), { code: "ENOENT", }), + platform: "linux", env: { STEPCODE_STEPPAGE_INSTALLER_URL: "https://example.test/install.sh" }, }); diff --git a/packages/coding-agent/src/step/mcp.ts b/packages/coding-agent/src/step/mcp.ts index c1fd1aa1..b79b94b0 100644 --- a/packages/coding-agent/src/step/mcp.ts +++ b/packages/coding-agent/src/step/mcp.ts @@ -694,6 +694,7 @@ export function describeMcpStartFailure(input: { provision?: StepPluginProvision; error: unknown; env?: NodeJS.ProcessEnv; + platform?: NodeJS.Platform; }): string { const detail = input.error instanceof Error ? input.error.message : String(input.error); if ( @@ -708,10 +709,17 @@ export function describeMcpStartFailure(input: { return `MCP server '${input.name}' could not start: ${detail}\nAuthenticate with: step mcp login ${quoted}, then restart Step.`; } if (!isMissingExecutable(input.error)) return `MCP server '${input.name}' could not start: ${detail}`; - const install = input.provision ? provisionInstallCommand(input.provision, input.env ?? process.env) : undefined; - const remedy = install - ? `Install it with: ${redactStepPageSecrets(install, input.env ?? process.env)}, then restart Step.` - : "Install it, then restart Step."; + const nativeWindowsPreparation = + (input.platform ?? process.platform) === "win32" && input.provision?.installer === "steppageInstaller"; + const install = + input.provision && !nativeWindowsPreparation + ? provisionInstallCommand(input.provision, input.env ?? process.env) + : undefined; + const remedy = nativeWindowsPreparation + ? "Run: step mcp prepare, then restart Step." + : install + ? `Install it with: ${redactStepPageSecrets(install, input.env ?? process.env)}, then restart Step.` + : "Install it, then restart Step."; return `MCP server '${input.name}' could not start: '${input.command}' is not installed or not on PATH. ${remedy}`; } diff --git a/packages/coding-agent/src/step/steppage-provision.ts b/packages/coding-agent/src/step/steppage-provision.ts index c011ef12..5de136f8 100644 --- a/packages/coding-agent/src/step/steppage-provision.ts +++ b/packages/coding-agent/src/step/steppage-provision.ts @@ -1,7 +1,7 @@ import { execFile, spawn } from "node:child_process"; import { existsSync } from "node:fs"; import { mkdir, writeFile } from "node:fs/promises"; -import { isAbsolute, join, win32 } from "node:path"; +import { basename, dirname, isAbsolute, join, win32 } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { promisify } from "node:util"; import lockfile from "proper-lockfile"; @@ -68,6 +68,19 @@ function absoluteWindowsPathEntries(pathValue: string | undefined): string[] { return entries; } +/** Resolve a Windows executable without letting the process search the working directory. */ +function resolveWindowsExecutable( + executable: string, + pathValue: string | undefined, + fileExists: (path: string) => boolean = existsSync, +): string | undefined { + for (const entry of absoluteWindowsPathEntries(pathValue)) { + const candidate = join(entry, executable); + if (fileExists(candidate)) return candidate; + } + return undefined; +} + /** * The Node.js executable that checks the runtime and runs the bundle. On Windows * `cmd` searches the working directory before PATH, so a bare `node` in the @@ -81,11 +94,7 @@ export function resolveNodeExecutable( fileExists: (path: string) => boolean = existsSync, ): string | undefined { if (platform !== "win32") return "node"; - for (const entry of absoluteWindowsPathEntries(pathValue)) { - const candidate = join(entry, "node.exe"); - if (fileExists(candidate)) return candidate; - } - return undefined; + return resolveWindowsExecutable("node.exe", pathValue, fileExists); } function pathValueOf(env: NodeJS.ProcessEnv): string | undefined { @@ -106,7 +115,9 @@ export function renderStepPageWrapper(nodePath: string, bundlePath: string): str const SECRET_PARAMETER = /token|key|secret|passw(?:or)?d|pwd|auth|credential|signature|sig$|session/iu; function redactUrl(url: string): string { - return url + const trailingQuotes = /['"]*$/u.exec(url)?.[0] ?? ""; + const target = url.slice(0, url.length - trailingQuotes.length); + return `${target .replace(/^([a-z][a-z\d+.-]*:\/\/)[^/?#\s]*@/iu, "$1[redacted]@") .replace(/([?&;#])([^=&;#]+)=([^&;#]*)/gu, (match, separator: string, name: string, value: string) => { let decoded = name; @@ -114,7 +125,7 @@ function redactUrl(url: string): string { decoded = decodeURIComponent(name); } catch {} return value && SECRET_PARAMETER.test(decoded) ? `${separator}${name}=[redacted]` : match; - }); + })}${trailingQuotes}`; } /** @@ -124,7 +135,7 @@ function redactUrl(url: string): string { * it came from a curl error or from the manual install hint. */ export function redactStepPageSecrets(text: string, env: NodeJS.ProcessEnv = process.env): string { - let redacted = text.replace(/\b[a-z][a-z\d+.-]*:\/\/[^\s"'<>`]+/giu, redactUrl); + let redacted = text.replace(/\b[a-z][a-z\d+.-]*:\/\/[^\s<>`]+/giu, redactUrl); for (const [key, value] of Object.entries(env)) { if (value && value.length >= 4 && /key|token|secret|password|authorization/iu.test(key)) redacted = redacted.replaceAll(value, "[redacted]"); @@ -195,12 +206,18 @@ async function probeStepPageVersion( signal: AbortSignal | undefined, platform: NodeJS.Platform, ): Promise { + const file = platform === "win32" ? resolveWindowsExecutable("cmd.exe", pathValueOf(env)) : command; + if (!file) return undefined; + // Only the fixed wrapper basename enters cmd parsing; metacharacters in its directory stay in cwd. + const args = platform === "win32" ? ["/d", "/c", basename(command), "--version"] : ["--version"]; try { - const [file, args] = - platform === "win32" - ? (["cmd", ["/d", "/c", command, "--version"]] as const) - : ([command, ["--version"]] as const); - const result = await execFileAsync(file, [...args], { env, signal, timeout: 5_000, maxBuffer: 8_192 }); + const result = await execFileAsync(file, args, { + env, + signal, + timeout: 5_000, + maxBuffer: 8_192, + ...(platform === "win32" ? { cwd: dirname(command) } : {}), + }); const version = result.stdout.trim(); if (/^v?\d+\.\d+\.\d+(?:[-+][\w.-]+)?$/u.test(version)) return version; } catch { @@ -247,6 +264,13 @@ async function assertNodeRuntime( * directly: download it, then write the `cmd` wrapper the bundle needs to run. */ async function installStepPageOnWindows(env: NodeJS.ProcessEnv, signal: AbortSignal, nodePath: string): Promise { + const pathValue = pathValueOf(env); + // The post-install probe needs cmd.exe. Check it and curl before downloading anything. + if (!resolveWindowsExecutable("cmd.exe", pathValue)) { + throw new Error("StepPage requires cmd.exe on an absolute PATH entry."); + } + const curl = resolveWindowsExecutable("curl.exe", pathValue); + if (!curl) throw new Error("StepPage requires curl.exe on an absolute PATH entry."); const home = resolveStepHomeDir(env); const bundleDir = join(home, ".steppage-mcp", "bin"); const bundlePath = join(bundleDir, "steppage-mcp.mjs"); @@ -254,7 +278,7 @@ async function installStepPageOnWindows(env: NodeJS.ProcessEnv, signal: AbortSig await mkdir(bundleDir, { recursive: true, mode: 0o700 }); await mkdir(join(home, ".local", "bin"), { recursive: true }); // Download before writing the wrapper so a failed transfer cannot hide behind a stale one. - await execFileAsync("curl", ["-fsSL", "--max-time", "30", stepPageBundleUrl(env), "-o", bundlePath], { + await execFileAsync(curl, ["-fsSL", "--max-time", "30", stepPageBundleUrl(env), "-o", bundlePath], { env, signal, timeout: 35_000, diff --git a/packages/coding-agent/test/steppage-provision-windows.test.ts b/packages/coding-agent/test/steppage-provision-windows.test.ts index 26eb0d9e..29741d45 100644 --- a/packages/coding-agent/test/steppage-provision-windows.test.ts +++ b/packages/coding-agent/test/steppage-provision-windows.test.ts @@ -8,16 +8,21 @@ import { afterEach, expect, test, vi } from "vitest"; const handlers = vi.hoisted(() => ({ nodeVersion: "v24.0.0", nodeCalls: [] as string[], + commandInterpreterCalls: [] as string[], + commandInterpreterCommands: [] as string[], + commandInterpreterCwds: [] as Array, + curlCalls: [] as string[], onCurl: undefined as undefined | ((args: readonly string[]) => void), commandExists: (_command: string): boolean => false, })); // steppage-provision.ts promisifies execFile, so the mock must publish the same // promisify.custom contract the real child_process.execFile exposes. -vi.mock("node:child_process", () => { +vi.mock("node:child_process", async () => { + const path = await import("node:path"); const promisifyCustom = Symbol.for("nodejs.util.promisify.custom"); const execFile = vi.fn(); - const custom = vi.fn((file: string, args: readonly string[] = []) => { + const custom = vi.fn((file: string, args: readonly string[] = [], options: { cwd?: string } = {}) => { return new Promise<{ stdout: string; stderr: string }>((resolve, reject) => { const notFound = (command: string) => { const error: NodeJS.ErrnoException = new Error(`spawn ${command} ENOENT`); @@ -27,11 +32,18 @@ vi.mock("node:child_process", () => { if ((file === "node" || file.endsWith("node.exe")) && args[0] === "--version") { handlers.nodeCalls.push(file); resolve({ stdout: `${handlers.nodeVersion}\n`, stderr: "" }); - } else if (file === "cmd" && args[0] === "/d" && args[1] === "/c") { + } else if ((file === "cmd" || file.endsWith("cmd.exe")) && args[0] === "/d" && args[1] === "/c") { + handlers.commandInterpreterCalls.push(file); const command = args[2] ?? ""; - if (!handlers.commandExists(command) || !command.endsWith(".cmd")) reject(notFound(command)); + handlers.commandInterpreterCommands.push(command); + handlers.commandInterpreterCwds.push(options.cwd); + const resolvedCommand = + path.isAbsolute(command) || !options.cwd ? command : path.resolve(options.cwd, command); + if (!handlers.commandExists(resolvedCommand) || !resolvedCommand.endsWith(".cmd")) + reject(notFound(command)); else resolve({ stdout: "1.0.0\n", stderr: "" }); - } else if (file === "curl") { + } else if (file === "curl" || file.endsWith("curl.exe")) { + handlers.curlCalls.push(file); Promise.resolve() .then(() => handlers.onCurl?.(args)) .then( @@ -71,24 +83,37 @@ afterEach(async () => { await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); handlers.nodeVersion = "v24.0.0"; handlers.nodeCalls = []; + handlers.commandInterpreterCalls = []; + handlers.commandInterpreterCommands = []; + handlers.commandInterpreterCwds = []; + handlers.curlCalls = []; handlers.onCurl = undefined; handlers.commandExists = () => false; }); -async function fixture() { - const root = await mkdtemp(join(tmpdir(), "steppage-windows-")); - roots.push(root); +async function fixture(subdirectory = "") { + const container = await mkdtemp(join(tmpdir(), "steppage-windows-")); + const root = join(container, subdirectory); + roots.push(container); await mkdir(join(root, "latest"), { recursive: true }); await writeFile(join(root, "latest", "steppage-mcp.mjs"), "// fixture bundle\n"); await mkdir(join(root, "bin"), { recursive: true }); await writeFile(join(root, "bin", "node.exe"), ""); + await writeFile(join(root, "bin", "cmd.exe"), ""); + await writeFile(join(root, "bin", "curl.exe"), ""); const env: NodeJS.ProcessEnv = { HOME: root, USERPROFILE: root, PATH: join(root, "bin"), STEPCODE_STEPPAGE_INSTALLER_URL: pathToFileURL(join(root, "installer.sh")).href, }; - return { root, env, node: join(root, "bin", "node.exe") }; + return { + root, + env, + node: join(root, "bin", "node.exe"), + cmd: join(root, "bin", "cmd.exe"), + curl: join(root, "bin", "curl.exe"), + }; } function managedWrapper(root: string): string { @@ -100,7 +125,7 @@ function managedBundle(root: string): string { } test("bundle url derives from the installer url", () => { - expect(stepPageBundleUrl()).toBe("https://dl.stepfun.com/steppage-mcp/p/latest/steppage-mcp.mjs"); + expect(stepPageBundleUrl({})).toBe("https://dl.stepfun.com/steppage-mcp/p/latest/steppage-mcp.mjs"); expect(stepPageBundleUrl({ STEPCODE_STEPPAGE_INSTALLER_URL: "https://mirror.example/steppage/install.sh" })).toBe( "https://mirror.example/steppage/latest/steppage-mcp.mjs", ); @@ -181,7 +206,7 @@ test("windows fails closed when node.exe is not on an absolute PATH entry", asyn }); test("windows installs the published bundle and a cmd wrapper", async () => { - const f = await fixture(); + const f = await fixture("home&%PATH%"); handlers.commandExists = (command) => command.endsWith(".cmd") && existsSync(command); handlers.onCurl = (args) => { const url = args.find((arg) => arg.startsWith("file://")); @@ -195,9 +220,13 @@ test("windows installs the published bundle and a cmd wrapper", async () => { }); expect(await readFile(managedBundle(f.root), "utf8")).toBe("// fixture bundle\n"); expect(await readFile(managedWrapper(f.root), "utf8")).toBe(renderStepPageWrapper(f.node, managedBundle(f.root))); - expect(await readFile(managedWrapper(f.root), "utf8")).toContain(`"${f.node}" "${managedBundle(f.root)}"`); - // The runtime check and the wrapper use the same absolute node.exe. + // Every Windows executable comes from an absolute PATH entry, not the working directory. expect(handlers.nodeCalls).toEqual([f.node]); + expect(handlers.commandInterpreterCalls.length).toBeGreaterThan(0); + for (const interpreter of handlers.commandInterpreterCalls) expect(interpreter).toBe(f.cmd); + for (const command of handlers.commandInterpreterCommands) expect(command).toBe("steppage-mcp.cmd"); + for (const cwd of handlers.commandInterpreterCwds) expect(cwd).toBe(dirname(managedWrapper(f.root))); + expect(handlers.curlCalls).toEqual([f.curl]); }); test("windows reuses a healthy wrapper without downloading", async () => { @@ -242,6 +271,25 @@ test("windows failure keeps Step usable and points at step mcp prepare", async ( expect(result.error).not.toContain("| sh"); }); +test.each(["cmd.exe", "curl.exe"] as const)( + "windows fails closed when %s is not on an absolute PATH entry", + async (tool) => { + const f = await fixture(); + await rm(tool === "cmd.exe" ? f.cmd : f.curl); + handlers.commandExists = (command) => command.endsWith(".cmd") && existsSync(command); + handlers.onCurl = (args) => { + const url = args.find((arg) => arg.startsWith("file://")); + const output = args[args.indexOf("-o") + 1]; + if (!url || !output) throw new Error(`unexpected curl args: ${args.join(" ")}`); + return copyFile(fileURLToPath(url), output); + }; + const result = await ensureStepPageReady({ env: f.env, platform: "win32" }); + expect(result.installed).toBe(false); + expect(result.error).toContain(tool); + expect(existsSync(managedWrapper(f.root))).toBe(false); + }, +); + test("redaction removes URL credentials and secret parameters but keeps the rest", () => { expect( redactStepPageSecrets( @@ -265,6 +313,19 @@ test("redaction removes URL credentials and secret parameters but keeps the rest ); }); +test("redaction covers quotes in raw and shell-escaped URLs", () => { + const raw = + "curl failed for https://alice:pa'hunter2@mirror.example/install.sh?token=abc'123&channel=preview#access_token=xyz'456"; + const shell = + "curl -fsSL 'https://alice:pa'\\''hunter2@mirror.example/install.sh?token=abc'\\''123&channel=preview#access_token=xyz'\\''456' | sh"; + for (const text of [raw, shell]) { + const result = redactStepPageSecrets(text, {}); + for (const secret of ["alice", "hunter2", "abc", "xyz"]) expect(result).not.toContain(secret); + expect(result).toContain("[redacted]"); + expect(result).toContain("channel=preview"); + } +}); + test.each(["win32", "linux"] as const)("%s failure text never contains mirror credentials", async (platform) => { const f = await fixture(); const installer = "https://alice:hunter2@mirror.example/steppage/install.sh?token=abc123&channel=preview";