From 767e653028029cfda41c9cb5362807f9ea5df335 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Mon, 13 Jul 2026 01:20:15 +0200 Subject: [PATCH] fix(cli): harden native service manager preflight --- .changeset/doctor-native-service-context.md | 2 +- README.md | 2 +- docs/install.html | 5 +- src/cli.test.ts | 14 + src/cli.ts | 22 +- src/nativeServices/serviceDoctor.test.ts | 145 +++++++++- src/nativeServices/serviceDoctor.ts | 265 ++++++++++++++---- src/nativeServices/serviceInstall.test.ts | 43 ++- src/nativeServices/serviceInstall.ts | 11 + src/nativeServices/servicePlan.test.ts | 9 +- src/nativeServices/servicePlan.ts | 10 +- src/nativeServices/serviceProbe.test.ts | 190 +++++++++---- src/nativeServices/serviceProbe.ts | 292 ++++++++++++-------- src/nativeServices/serviceRendering.test.ts | 25 +- src/nativeServices/serviceRendering.ts | 42 +-- 15 files changed, 829 insertions(+), 248 deletions(-) diff --git a/.changeset/doctor-native-service-context.md b/.changeset/doctor-native-service-context.md index 0bd0b13..8969167 100644 --- a/.changeset/doctor-native-service-context.md +++ b/.changeset/doctor-native-service-context.md @@ -2,4 +2,4 @@ "@jmfederico/pi-web": patch --- -Validate install and doctor service requirements in the real systemd or launchd manager context before changing native services, with plan-specific PATH guidance and safe probe cleanup. Thanks to @blain3white for the original report, reproduction, and diagnosis. +Validate install and doctor service requirements in the real systemd or launchd manager context before changing native services, with plan-specific PATH guidance and safe probe cleanup. Thanks to @blain3white for the original report, reproduction, and root-cause analysis. diff --git a/README.md b/README.md index a759c2d..5547cea 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ pi-web version pi-web uninstall ``` -`pi-web install` validates the exact production or development service plan inside the native user-service manager before changing config or replacing services. `pi-web doctor` repeats manager-context diagnostics, labels prospective production checks when an installed command strategy cannot be reconstructed, and keeps general shell/Pi/npm readiness separate from service-start requirements. +`pi-web install` validates the safely verifiable requirements of the exact production or development service plan inside the native user-service manager before changing config or replacing services; arbitrary configured command overrides are preserved but not executed by preflight. `pi-web doctor` repeats manager-context diagnostics, labels prospective production checks when an installed command strategy cannot be reconstructed, and keeps general shell/Pi/npm readiness separate from service-start requirements. For more install options, including one-line install, Pi package install, WSL/manual usage, and remote access, see the [installation guide](https://pi-web.dev/install). diff --git a/docs/install.html b/docs/install.html index 0c9bced..01e98e0 100644 --- a/docs/install.html +++ b/docs/install.html @@ -114,8 +114,9 @@ Important PATH detail: PI WEB services run through a non-interactive login shell with -lc. Setup that only lives in interactive shell files or prompt hooks may not be visible to the systemd or launchd manager. The installer - probes the exact candidate plan in that manager context before changing config or replacing services; run - pi-web doctor later to repeat plan-specific diagnostics. + probes the safely verifiable requirements of the exact candidate plan in that manager context before changing + config or replacing services. Arbitrary configured command overrides are preserved but not executed by + preflight; run pi-web doctor later to repeat plan-specific diagnostics. diff --git a/src/cli.test.ts b/src/cli.test.ts index 2131b10..1eb6aae 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -7,6 +7,7 @@ import { doctorExitCode, isCliEntrypoint, launchdRuntimeDetails, + regularFileExists, serviceBackendForPlatform, } from "./cli.js"; @@ -53,6 +54,19 @@ describe("native-service doctor CLI contracts", () => { expect(doctorExitCode(true, true, false)).toBe(1); }); + it("accepts only regular files as bundled entrypoints", () => { + const dir = mkdtempSync(join(tmpdir(), "pi-web-entrypoint-test-")); + try { + const file = join(dir, "entrypoint.js"); + writeFileSync(file, "export {};\n"); + expect(regularFileExists(file)).toBe(true); + expect(regularFileExists(dir)).toBe(false); + expect(regularFileExists(join(dir, "missing.js"))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it("surfaces launchd last exit code 127 in service status", () => { expect(launchdRuntimeDetails("state = exited\nlast exit code = 127\n")).toEqual({ state: "exited", diff --git a/src/cli.ts b/src/cli.ts index 5e395d5..035cd19 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,6 +1,6 @@ #!/usr/bin/env node import { spawnSync } from "node:child_process"; -import { existsSync, readFileSync, realpathSync } from "node:fs"; +import { existsSync, readFileSync, realpathSync, statSync } from "node:fs"; import { mkdir, rm, writeFile } from "node:fs/promises"; import { homedir, userInfo } from "node:os"; import { basename, dirname, join, resolve } from "node:path"; @@ -10,6 +10,7 @@ import { packageVersion, printPiWebVersionReport } from "./piWebVersionReport.js import { checkNodePtyDarwinSpawnHelper, formatNodePtyDarwinSpawnHelperCheck } from "./server/diagnostics/nodePtySpawnHelper.js"; import { installNativeServiceCandidate, + nativeServiceInstallFailureNeedsPathAdvice, type NativeServiceInstallCandidate, type NativeServiceInstallFailure, } from "./nativeServices/serviceInstall.js"; @@ -218,6 +219,10 @@ function packageEntrypointPath(name: "server" | "sessiond"): string { return join(packageRootPath(), "dist", "server", name === "server" ? "index.js" : "sessiond.js"); } +export function regularFileExists(path: string): boolean { + return existsSync(path) && statSync(path).isFile(); +} + function detectServiceShell(): NativeServiceShell { const userShell = userInfo().shell ?? undefined; const envShell = process.env["SHELL"]?.trim(); @@ -660,15 +665,18 @@ async function install(args: string[]): Promise { console.log(`Service shell: ${describeServiceShell()}`); const result = await installNativeServiceCandidate(candidate, { probe: createNativeServiceAuthoritativeProbe(), - fileExists: existsSync, + fileExists: regularFileExists, writeInitialConfig: () => writeInitialConfig(options, configPath), replaceServices: installNativeServices, }); if (!result.ok) { printNativeServiceInstallFailure(result.failure); - printPathSetupAdvice(); + if (nativeServiceInstallFailureNeedsPathAdvice(result.failure)) printPathSetupAdvice(); throw new Error("Install preflight checks failed without changing config or services. Fix the failure above, then run `pi-web doctor` for more detail."); } + for (const service of result.plan.services.filter((item) => item.strategy.kind === "configured-override")) { + console.log(`! ${service.description} uses a configured command override; preflight did not execute that arbitrary command.`); + } console.log(`\nPI WEB ${options.mode} services are installed and starting.`); console.log(`Config: ${configPath}`); @@ -893,7 +901,7 @@ function nativeServiceDoctorTarget(backend: ServiceBackend): NativeServiceDoctor async function printNativeServiceDoctorChecks(backend: ServiceBackend): Promise { const result = await runNativeServiceDoctor(nativeServiceDoctorTarget(backend), { probe: createNativeServiceAuthoritativeProbe(), - fileExists: existsSync, + fileExists: regularFileExists, }); const report = formatNativeServiceDoctorResult(result); for (const line of report.lines) console.log(line); @@ -994,11 +1002,11 @@ async function doctor(): Promise { } const nativeServicePlanOk = nativeServiceReport?.ok ?? true; - const pathFailure = !generalReadinessOk || nativeServiceReport?.failureKind === "requirements"; + const pathFailure = !generalReadinessOk || nativeServiceReport?.pathAdviceRecommended === true; if (pathFailure) { console.log("\nIf a command works in your terminal but fails in the service-manager check, compare the caller and manager contexts above."); - const adviceShell = nativeServiceReport?.failureKind === "requirements" && nativeServiceReport.plan !== null - ? nativeServiceReport.plan.shell + const adviceShell = nativeServiceReport?.pathAdviceRecommended === true && nativeServiceReport.adviceShell !== null + ? nativeServiceReport.adviceShell : detectServiceShell(); printPathSetupAdvice(adviceShell); } diff --git a/src/nativeServices/serviceDoctor.test.ts b/src/nativeServices/serviceDoctor.test.ts index 5f802bd..3da5e8d 100644 --- a/src/nativeServices/serviceDoctor.test.ts +++ b/src/nativeServices/serviceDoctor.test.ts @@ -98,13 +98,39 @@ describe("installed native-service mode and definition inspection", () => { }); }); + it("reconstructs escaped systemd paths, substitutions, and line controls exactly", () => { + const plan = createDevelopmentNativeServicePlan({ + backend: { kind: "systemd", label: "systemd" }, + shell: { + name: "zsh", + executable: "/shell $HOME/%h/zsh", + source: "detected", + detectedExecutable: "/shell $HOME/%h/zsh", + }, + environment: { PI_WEB_CONFIG: "/config/%h\nnext" }, + workingDirectory: "/checkout %h\nnext", + packageJsonPath: "/checkout %h\nnext/package.json", + }); + + expect(inspectInstalledDevelopmentServiceInput(plan.backend, renderedDefinitions(plan))).toEqual({ + ok: true, + value: { + backend: plan.backend, + shell: plan.shell, + environment: plan.services[0]?.environment, + workingDirectory: "/checkout %h\nnext", + packageJsonPath: "/checkout %h\nnext/package.json", + }, + }); + }); + it("inspects legacy systemd definitions without /usr/bin/env or quoted working directories", () => { const plan = developmentPlan("systemd"); const definitions = renderedDefinitions(plan).map((definition) => ({ ...definition, contents: definition.contents .replace("ExecStart=/usr/bin/env ", "ExecStart=") - .replace('WorkingDirectory="/checkout with space"', "WorkingDirectory=/checkout with space"), + .replace("WorkingDirectory=/checkout\\x20with\\x20space", "WorkingDirectory=/checkout with space"), })); expect(inspectInstalledDevelopmentServiceInput(plan.backend, definitions)).toMatchObject({ @@ -113,6 +139,80 @@ describe("installed native-service mode and definition inspection", () => { }); }); + it("rejects quoted systemd working directories that the manager treats as non-absolute", () => { + const plan = developmentPlan("systemd"); + const definitions = renderedDefinitions(plan).map((definition) => ({ + ...definition, + contents: definition.contents.replace( + "WorkingDirectory=/checkout\\x20with\\x20space", + 'WorkingDirectory="/checkout with space"', + ), + })); + + const inspection = inspectInstalledDevelopmentServiceInput(plan.backend, definitions); + expect(inspection.ok).toBe(false); + if (inspection.ok) throw new Error("Expected quoted working directory inspection to fail"); + expect(inspection.message).toContain("invalid quoted working directory"); + }); + + it("rejects unconsumed systemd environment syntax rather than checking a different context", () => { + const plan = developmentPlan("systemd"); + const definitions = renderedDefinitions(plan).map((definition) => ({ + ...definition, + contents: definition.contents.replace("[Service]\n", "[Service]\nEnvironment=PATH=/custom/bin\n"), + })); + + const inspection = inspectInstalledDevelopmentServiceInput(plan.backend, definitions); + expect(inspection.ok).toBe(false); + if (inspection.ok) throw new Error("Expected systemd environment inspection to fail"); + expect(inspection.message).toContain("environment entry"); + }); + + it.each([ + 'Environment="PI_WEB_CONFIG=/config" "PATH=/broken"', + "EnvironmentFile=/tmp/pi-web.env", + ])("rejects noncanonical systemd environment context: %s", (directive) => { + const plan = developmentPlan("systemd"); + const definitions = renderedDefinitions(plan).map((definition) => ({ + ...definition, + contents: definition.contents.replace("[Service]\n", `[Service]\n${directive}\n`), + })); + + expect(inspectInstalledDevelopmentServiceInput(plan.backend, definitions).ok).toBe(false); + }); + + it("rejects duplicate systemd ExecStart directives", () => { + const plan = developmentPlan("systemd"); + const definitions = renderedDefinitions(plan).map((definition) => ({ + ...definition, + contents: definition.contents.replace( + "Restart=no", + 'ExecStart=/usr/bin/env "/bin/zsh" -lc "exec true"\nRestart=no', + ), + })); + + const inspection = inspectInstalledDevelopmentServiceInput(plan.backend, definitions); + expect(inspection.ok).toBe(false); + if (inspection.ok) throw new Error("Expected duplicate ExecStart inspection to fail"); + expect(inspection.message).toContain("exactly one recognized ExecStart"); + }); + + it("rejects malformed launchd environment dictionaries rather than dropping entries", () => { + const plan = developmentPlan("launchd"); + const definitions = renderedDefinitions(plan).map((definition) => ({ + ...definition, + contents: definition.contents.replace( + " \n RunAtLoad", + " BROKEN\n 1\n \n RunAtLoad", + ), + })); + + const inspection = inspectInstalledDevelopmentServiceInput(plan.backend, definitions); + expect(inspection.ok).toBe(false); + if (inspection.ok) throw new Error("Expected launchd environment inspection to fail"); + expect(inspection.message).toContain("environment dictionary"); + }); + it("rejects a modified development command rather than claiming to check the installed plan", () => { const plan = developmentPlan("systemd"); const definitions = renderedDefinitions(plan); @@ -168,6 +268,31 @@ describe("native-service doctor planning and reporting", () => { ); }); + it("does not recommend PATH changes for checkout metadata failures", async () => { + const plan = developmentPlan("systemd"); + const inspected = inspectInstalledDevelopmentServiceInput(plan.backend, renderedDefinitions(plan)); + if (!inspected.ok) throw new Error(inspected.message); + const result = await runNativeServiceDoctor( + { kind: "installed-development", input: inspected.value }, + { + probe: { + run: (request) => Promise.resolve({ + kind: "completed", + outcomes: request.prerequisites.map((prerequisite) => ({ + prerequisiteId: prerequisite.id, + status: prerequisite.kind === "package-scripts" ? "unsatisfied" as const : "satisfied" as const, + detail: prerequisite.kind === "package-scripts" ? "scripts missing" : null, + })), + }), + }, + fileExists: () => true, + }, + ); + const report = formatNativeServiceDoctorResult(result); + + expect(report).toMatchObject({ ok: false, failureKind: "requirements", pathAdviceRecommended: false }); + }); + it("labels a production check as prospective and reports manager-context requirements", async () => { const target: NativeServiceDoctorTarget = { kind: "prospective-production", @@ -190,6 +315,22 @@ describe("native-service doctor planning and reporting", () => { ])); }); + it("retains the installed production shell when resolution fails before a plan exists", async () => { + const result = await runNativeServiceDoctor( + { kind: "prospective-production", input: productionInput(), reason: "installed strategy is unknown" }, + { probe: probeWithStatus("unsatisfied"), fileExists: () => false }, + ); + const report = formatNativeServiceDoctorResult(result); + + expect(report).toMatchObject({ + ok: false, + failureKind: "requirements", + plan: null, + adviceShell: shell, + pathAdviceRecommended: true, + }); + }); + it("preserves configured overrides as unverified and does not probe arbitrary commands", async () => { let calls = 0; const result = await runNativeServiceDoctor( @@ -201,7 +342,7 @@ describe("native-service doctor planning and reporting", () => { ); const report = formatNativeServiceDoctorResult(result); - expect(calls).toBe(0); + expect(calls).toBe(1); expect(report.ok).toBe(true); expect(report.lines.join("\n")).toContain("does not execute arbitrary configured commands"); }); diff --git a/src/nativeServices/serviceDoctor.ts b/src/nativeServices/serviceDoctor.ts index 9739dd6..9bc0a1e 100644 --- a/src/nativeServices/serviceDoctor.ts +++ b/src/nativeServices/serviceDoctor.ts @@ -1,6 +1,7 @@ import { basename, join } from "node:path"; import { createDevelopmentNativeServicePlan, + nativeServicePrerequisiteNeedsPathAdvice, resolveProductionNativeServicePlan, validateNativeServicePlan, type DevelopmentNativeServicePlanInput, @@ -49,6 +50,7 @@ export type NativeServiceDoctorTarget = interface NativeServiceDoctorScope { kind: "installed-development" | "prospective-production"; reason: string | null; + shell: NativeServiceShell; } export type NativeServiceDoctorResult = @@ -73,6 +75,8 @@ export interface NativeServiceDoctorReport { failureKind: "none" | "requirements" | "infrastructure" | "inspection"; lines: readonly string[]; plan: NativeServicePlan | null; + adviceShell: NativeServiceShell | null; + pathAdviceRecommended: boolean; failedPrerequisites: readonly NativeServicePrerequisite[]; } @@ -153,8 +157,8 @@ export async function runNativeServiceDoctor( if (target.kind === "inspection-failure") return target; const scope: NativeServiceDoctorScope = target.kind === "installed-development" - ? { kind: target.kind, reason: null } - : { kind: target.kind, reason: target.reason }; + ? { kind: target.kind, reason: null, shell: target.input.shell } + : { kind: target.kind, reason: target.reason, shell: target.input.shell }; let plan: NativeServicePlan; if (target.kind === "installed-development") { plan = createDevelopmentNativeServicePlan(target.input); @@ -180,6 +184,8 @@ export function formatNativeServiceDoctorResult(result: NativeServiceDoctorResul " Run `pi-web install` or `pi-web install --dev` to replace mixed, partial, or outdated service definitions.", ], plan: null, + adviceShell: null, + pathAdviceRecommended: false, failedPrerequisites: [], }; } @@ -205,6 +211,9 @@ export function formatNativeServiceDoctorResult(result: NativeServiceDoctorResul failureKind: infrastructure ? "infrastructure" : "requirements", lines, plan: null, + adviceShell: result.scope.shell, + pathAdviceRecommended: !infrastructure + && result.failures.some((failure) => failure.kind === "executable-unavailable"), failedPrerequisites: [], }; } @@ -215,7 +224,15 @@ export function formatNativeServiceDoctorResult(result: NativeServiceDoctorResul } if (result.validation.ok) { lines.push("✓ All verifiable native-service plan requirements are satisfied in the service-manager context."); - return { ok: true, failureKind: "none", lines, plan: result.plan, failedPrerequisites: [] }; + return { + ok: true, + failureKind: "none", + lines, + plan: result.plan, + adviceShell: result.plan.shell, + pathAdviceRecommended: false, + failedPrerequisites: [], + }; } const failedPrerequisites: NativeServicePrerequisite[] = []; @@ -236,6 +253,9 @@ export function formatNativeServiceDoctorResult(result: NativeServiceDoctorResul failureKind: infrastructure ? "infrastructure" : "requirements", lines, plan: result.plan, + adviceShell: result.plan.shell, + pathAdviceRecommended: !infrastructure + && failedPrerequisites.some(nativeServicePrerequisiteNeedsPathAdvice), failedPrerequisites, }; } @@ -275,33 +295,85 @@ function parseConsistentDefinitions( return { ok: true, value: parsed }; } +interface ParsedSystemdDirective { + name: string; + value: string; +} + +function systemdServiceDirectives(contents: string): ParsedSystemdDirective[] | undefined { + const allowed = new Set(["Type", "WorkingDirectory", "Environment", "ExecStart", "Restart", "RestartSec"]); + const directives: ParsedSystemdDirective[] = []; + let inServiceSection = false; + let foundServiceSection = false; + for (const line of contents.split(/\r?\n/u)) { + const trimmed = line.trim(); + if (/^\[[^\]]+\]$/u.test(trimmed)) { + inServiceSection = trimmed === "[Service]"; + foundServiceSection ||= inServiceSection; + continue; + } + if (!inServiceSection || trimmed === "" || trimmed.startsWith("#") || trimmed.startsWith(";")) continue; + const match = /^\s*([A-Za-z][A-Za-z0-9]*)=(.*)$/u.exec(line); + const name = match?.[1]; + const value = match?.[2]; + if (name === undefined || value === undefined || !allowed.has(name)) return undefined; + directives.push({ name, value }); + } + return foundServiceSection ? directives : undefined; +} + function parseSystemdDefinition( definition: InstalledNativeServiceDefinition, ): InstalledNativeServiceInspection { - const execStart = /^ExecStart=(?:\/usr\/bin\/env )?(.+?) -lc (.+)$/mu.exec(definition.contents); - if (execStart?.[1] === undefined || execStart[2] === undefined) { - return { ok: false, message: `Installed ${definition.id} systemd unit has an unrecognized ExecStart.` }; + const directives = systemdServiceDirectives(definition.contents); + if (directives === undefined) { + return { ok: false, message: `Installed ${definition.id} systemd unit has unrecognized service directives.` }; } - const shell = installedShell(execStart[1]); + const execStarts = directives.filter((directive) => directive.name === "ExecStart"); + const execStart = execStarts.length === 1 + ? /^(?:\/usr\/bin\/env )?(.+?) -lc (.+)$/u.exec(execStarts[0]?.value ?? "") + : null; + if (execStart?.[1] === undefined || execStart[2] === undefined) { + return { ok: false, message: `Installed ${definition.id} systemd unit must have exactly one recognized ExecStart.` }; + } + const shellExecutable = parseSystemdExecArgument(execStart[1]); + if (shellExecutable === undefined) { + return { ok: false, message: `Installed ${definition.id} systemd unit has an unrecognized login shell argument.` }; + } + const shell = installedShell(shellExecutable); if (!shell.ok) return shell; - const shellCommand = parseShellQuotedValue(shell.value.name, execStart[2]); + const shellCommand = parseSystemdShellCommand(shell.value.name, execStart[2]); if (shellCommand === undefined) { return { ok: false, message: `Installed ${definition.id} systemd unit has an unrecognized shell command.` }; } const environment: Record = {}; - for (const match of definition.contents.matchAll(/^Environment="((?:\\.|[^"])*)"$/gmu)) { - const assignment = systemdUnescape(match[1] ?? ""); - const separator = assignment.indexOf("="); - if (separator <= 0) return { ok: false, message: `Installed ${definition.id} systemd unit has a malformed environment entry.` }; - environment[assignment.slice(0, separator)] = assignment.slice(separator + 1); + for (const directive of directives.filter((item) => item.name === "Environment")) { + const rawValue = directive.value; + if (!/^"(?:\\.|[^"])*"$/u.test(rawValue)) { + return { ok: false, message: `Installed ${definition.id} systemd unit has an unrecognized environment entry.` }; + } + const assignment = parseSystemdDirectiveValue(rawValue); + const separator = assignment?.indexOf("=") ?? -1; + const key = assignment?.slice(0, separator) ?? ""; + if (separator <= 0 || Object.hasOwn(environment, key)) { + return { ok: false, message: `Installed ${definition.id} systemd unit has a malformed environment entry.` }; + } + environment[key] = assignment?.slice(separator + 1) ?? ""; } - const workingDirectoryMatch = /^WorkingDirectory=(.+)$/mu.exec(definition.contents); - const workingDirectory = workingDirectoryMatch?.[1] === undefined + const workingDirectories = directives.filter((directive) => directive.name === "WorkingDirectory"); + if (workingDirectories.length > 1) { + return { ok: false, message: `Installed ${definition.id} systemd unit has duplicate working directories.` }; + } + const rawWorkingDirectory = workingDirectories[0]?.value; + if (rawWorkingDirectory?.startsWith('"') === true || rawWorkingDirectory?.startsWith("'") === true) { + return { ok: false, message: `Installed ${definition.id} systemd unit has an invalid quoted working directory.` }; + } + const workingDirectory = rawWorkingDirectory === undefined ? null - : parseSystemdValue(workingDirectoryMatch[1]); - if (workingDirectoryMatch !== null && workingDirectory === undefined) { + : parseSystemdDirectiveValue(rawWorkingDirectory); + if (workingDirectories.length === 1 && workingDirectory === undefined) { return { ok: false, message: `Installed ${definition.id} systemd unit has a malformed working directory.` }; } @@ -314,25 +386,42 @@ function parseSystemdDefinition( function parseLaunchdDefinition( definition: InstalledNativeServiceDefinition, ): InstalledNativeServiceInspection { - const argumentsBlock = /ProgramArguments<\/key>\s*([\s\S]*?)<\/array>/u.exec(definition.contents)?.[1]; - if (argumentsBlock === undefined) { - return { ok: false, message: `Installed ${definition.id} LaunchAgent has no ProgramArguments array.` }; - } - const arguments_ = [...argumentsBlock.matchAll(/([\s\S]*?)<\/string>/gu)].map((match) => xmlUnescape(match[1] ?? "")); - if (arguments_.length !== 4 || arguments_[0] !== "/usr/bin/env" || arguments_[2] !== "-lc") { + const argumentsMatches = [...definition.contents.matchAll(/ProgramArguments<\/key>\s*([\s\S]*?)<\/array>/gu)]; + const arguments_ = argumentsMatches.length === 1 + ? parseXmlStringSequence(argumentsMatches[0]?.[1] ?? "") + : undefined; + if (arguments_?.length !== 4 || arguments_[0] !== "/usr/bin/env" || arguments_[2] !== "-lc") { return { ok: false, message: `Installed ${definition.id} LaunchAgent has unrecognized ProgramArguments.` }; } const shell = installedShell(arguments_[1] ?? ""); if (!shell.ok) return shell; - const environment: Record = {}; - const environmentBlock = /EnvironmentVariables<\/key>\s*([\s\S]*?)<\/dict>/u.exec(definition.contents)?.[1]; - if (environmentBlock !== undefined) { - for (const match of environmentBlock.matchAll(/([\s\S]*?)<\/key>\s*([\s\S]*?)<\/string>/gu)) { - environment[xmlUnescape(match[1] ?? "")] = xmlUnescape(match[2] ?? ""); - } + const environmentMatches = [...definition.contents.matchAll(/EnvironmentVariables<\/key>\s*([\s\S]*?)<\/dict>/gu)]; + const environmentKeyCount = [...definition.contents.matchAll(/EnvironmentVariables<\/key>/gu)].length; + if (environmentMatches.length > 1 || environmentKeyCount !== environmentMatches.length) { + return { ok: false, message: `Installed ${definition.id} LaunchAgent has a malformed environment dictionary.` }; + } + const environment = environmentMatches.length === 0 + ? {} + : parseXmlStringDictionary(environmentMatches[0]?.[1] ?? ""); + if (environment === undefined) { + return { ok: false, message: `Installed ${definition.id} LaunchAgent has a malformed environment dictionary.` }; + } + + const contentsWithoutEnvironment = environmentMatches[0]?.[0] === undefined + ? definition.contents + : definition.contents.replace(environmentMatches[0][0], ""); + const workingDirectoryMatches = [...contentsWithoutEnvironment.matchAll(/WorkingDirectory<\/key>\s*([\s\S]*?)<\/string>/gu)]; + const workingDirectoryKeyCount = [...contentsWithoutEnvironment.matchAll(/WorkingDirectory<\/key>/gu)].length; + if (workingDirectoryMatches.length > 1 || workingDirectoryKeyCount !== workingDirectoryMatches.length) { + return { ok: false, message: `Installed ${definition.id} LaunchAgent has a malformed working directory.` }; + } + const workingDirectory = workingDirectoryMatches[0]?.[1] === undefined + ? null + : xmlUnescapeStrict(workingDirectoryMatches[0][1]); + if (workingDirectoryMatches.length === 1 && workingDirectory === undefined) { + return { ok: false, message: `Installed ${definition.id} LaunchAgent has a malformed working directory.` }; } - const workingDirectory = launchdString(definition.contents, "WorkingDirectory"); return { ok: true, @@ -340,7 +429,7 @@ function parseLaunchdDefinition( id: definition.id, shell: shell.value, environment, - workingDirectory, + workingDirectory: workingDirectory ?? null, shellCommand: arguments_[3] ?? "", }, }; @@ -357,31 +446,87 @@ function installedShell(executable: string): InstalledNativeServiceInspection> = { + "\\": "\\", + '"': '"', + "'": "'", + a: "\u0007", + b: "\b", + e: "\u001b", + f: "\f", + n: "\n", + r: "\r", + s: " ", + t: "\t", + v: "\v", + }; + const simple = simpleEscapes[escape]; + if (simple !== undefined) { + result += simple; + index += 1; + continue; + } + + const length = escape === "x" ? 2 : escape === "u" ? 4 : escape === "U" ? 8 : 0; + if (length === 0) return undefined; + const encoded = value.slice(index + 2, index + 2 + length); + if (encoded.length !== length || !new RegExp(`^[0-9a-fA-F]{${String(length)}}$`, "u").test(encoded)) return undefined; + const codePoint = Number.parseInt(encoded, 16); + if (codePoint === 0 || codePoint > 0x10ffff) return undefined; + result += String.fromCodePoint(codePoint); + index += length + 1; } return result; } -function parseShellQuotedValue(shell: NativeServiceShell["name"], value: string): string | undefined { +function decodeSystemdSubstitutions(value: string, decodeDollars: boolean): string | undefined { + let result = ""; + for (let index = 0; index < value.length; index += 1) { + const character = value[index]; + if (character !== "%" && !(decodeDollars && character === "$")) { + result += character ?? ""; + continue; + } + if (value[index + 1] !== character) return undefined; + result += character; + index += 1; + } + return result; +} + +function parseSystemdShellCommand(shell: NativeServiceShell["name"], value: string): string | undefined { + if (value.startsWith('"') || value.endsWith('"')) return parseSystemdExecArgument(value); if (!value.startsWith("'") || !value.endsWith("'")) return undefined; const inner = value.slice(1, -1); - if (shell === "fish") return fishSingleQuoteUnescape(inner); - return inner.replaceAll("'\\''", "'").replaceAll("$$", "$").replaceAll("%%", "%"); + const unquoted = shell === "fish" ? fishSingleQuoteUnescape(inner) : inner.replaceAll("'\\''", "'"); + return decodeSystemdSubstitutions(unquoted, true); } function fishSingleQuoteUnescape(value: string): string { @@ -395,16 +540,38 @@ function fishSingleQuoteUnescape(value: string): string { result += character ?? ""; } } - return result.replaceAll("$$", "$").replaceAll("%%", "%"); + return result; } -function launchdString(contents: string, key: string): string | null { - const escapedKey = key.replaceAll(/[.*+?^${}()|[\]\\]/gu, "\\$&"); - const value = new RegExp(`${escapedKey}<\\/key>\\s*([\\s\\S]*?)<\\/string>`, "u").exec(contents)?.[1]; - return value === undefined ? null : xmlUnescape(value); +function parseXmlStringSequence(contents: string): string[] | undefined { + const values: string[] = []; + let cursor = 0; + for (const match of contents.matchAll(/([\s\S]*?)<\/string>/gu)) { + if (contents.slice(cursor, match.index).trim() !== "") return undefined; + const value = xmlUnescapeStrict(match[1] ?? ""); + if (value === undefined) return undefined; + values.push(value); + cursor = match.index + match[0].length; + } + return contents.slice(cursor).trim() === "" ? values : undefined; } -function xmlUnescape(value: string): string { +function parseXmlStringDictionary(contents: string): Record | undefined { + const values: Record = {}; + let cursor = 0; + for (const match of contents.matchAll(/([\s\S]*?)<\/key>\s*([\s\S]*?)<\/string>/gu)) { + if (contents.slice(cursor, match.index).trim() !== "") return undefined; + const key = xmlUnescapeStrict(match[1] ?? ""); + const value = xmlUnescapeStrict(match[2] ?? ""); + if (key === undefined || value === undefined || Object.hasOwn(values, key)) return undefined; + values[key] = value; + cursor = match.index + match[0].length; + } + return contents.slice(cursor).trim() === "" ? values : undefined; +} + +function xmlUnescapeStrict(value: string): string | undefined { + if (/[<>]/u.test(value) || /&(?!(?:apos|quot|gt|lt|amp);)/u.test(value)) return undefined; return value .replaceAll("'", "'") .replaceAll(""", '"') diff --git a/src/nativeServices/serviceInstall.test.ts b/src/nativeServices/serviceInstall.test.ts index 116a9fb..4a16033 100644 --- a/src/nativeServices/serviceInstall.test.ts +++ b/src/nativeServices/serviceInstall.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it, vi } from "vitest"; -import { installNativeServiceCandidate } from "./serviceInstall.js"; +import { + installNativeServiceCandidate, + nativeServiceInstallFailureNeedsPathAdvice, +} from "./serviceInstall.js"; import type { NativeServiceAuthoritativeProbe, NativeServicePlan, @@ -76,6 +79,41 @@ describe("native service install orchestration", () => { expect(replaceServices).toHaveBeenCalledWith(expect.objectContaining({ mode: "production" })); }); + it("validates manager and shell readiness without executing configured overrides", async () => { + const writeInitialConfig = vi.fn<() => Promise>(() => Promise.resolve()); + const replaceServices = vi.fn<() => Promise>(() => Promise.resolve()); + const requests: NativeServiceProbeRequest[] = []; + const configuredInput: ProductionNativeServicePlanInput = { + ...productionInput, + executables: { + sessiond: { ...productionInput.executables.sessiond, configuredCommand: "custom-sessiond --flag" }, + web: { ...productionInput.executables.web, configuredCommand: "custom-web --flag" }, + }, + }; + + const result = await installNativeServiceCandidate( + { mode: "production", input: configuredInput }, + { + probe: { + run: (request) => { + requests.push(request); + return Promise.resolve({ kind: "infrastructure-failure", reason: "manager", message: "manager unavailable" }); + }, + }, + fileExists: () => false, + writeInitialConfig, + replaceServices, + }, + ); + + expect(result).toMatchObject({ ok: false, failure: { kind: "plan-validation" } }); + if (result.ok) throw new Error("Expected manager validation failure"); + expect(nativeServiceInstallFailureNeedsPathAdvice(result.failure)).toBe(false); + expect(requests).toEqual([expect.objectContaining({ purpose: "plan-validation", prerequisites: [] })]); + expect(writeInitialConfig).not.toHaveBeenCalled(); + expect(replaceServices).not.toHaveBeenCalled(); + }); + it("does not make durable changes when exact plan requirements are unsatisfied", async () => { const writeInitialConfig = vi.fn<() => Promise>(() => Promise.resolve()); const replaceServices = vi.fn<() => Promise>(() => Promise.resolve()); @@ -99,6 +137,7 @@ describe("native service install orchestration", () => { if (result.ok || result.failure.kind !== "plan-validation") throw new Error("Expected validation failure"); expect(result.failure.failures).not.toHaveLength(0); expect(result.failure.failures.every((failure) => failure.kind === "prerequisite-unsatisfied")).toBe(true); + expect(nativeServiceInstallFailureNeedsPathAdvice(result.failure)).toBe(true); expect(writeInitialConfig).not.toHaveBeenCalled(); expect(replaceServices).not.toHaveBeenCalled(); }); @@ -135,6 +174,8 @@ describe("native service install orchestration", () => { }], }, }); + if (result.ok) throw new Error("Expected infrastructure failure"); + expect(nativeServiceInstallFailureNeedsPathAdvice(result.failure)).toBe(false); expect(writeInitialConfig).not.toHaveBeenCalled(); expect(replaceServices).not.toHaveBeenCalled(); }); diff --git a/src/nativeServices/serviceInstall.ts b/src/nativeServices/serviceInstall.ts index 108354e..aaa75d7 100644 --- a/src/nativeServices/serviceInstall.ts +++ b/src/nativeServices/serviceInstall.ts @@ -1,5 +1,6 @@ import { createDevelopmentNativeServicePlan, + nativeServicePrerequisiteNeedsPathAdvice, resolveProductionNativeServicePlan, validateNativeServicePlan, type DevelopmentNativeServicePlanInput, @@ -29,6 +30,16 @@ export type NativeServiceInstallResult = | { ok: true; plan: NativeServicePlan } | { ok: false; failure: NativeServiceInstallFailure }; +export function nativeServiceInstallFailureNeedsPathAdvice(failure: NativeServiceInstallFailure): boolean { + if (failure.kind === "plan-resolution") { + return failure.failures.every((item) => item.kind === "executable-unavailable") + && failure.failures.length > 0; + } + return failure.failures.some((item) => + item.kind === "prerequisite-unsatisfied" + && nativeServicePrerequisiteNeedsPathAdvice(item.prerequisite)); +} + /** * Keeps preflight effects ahead of durable install effects. The authoritative * probes may create bounded temporary artifacts, but they must clean those up diff --git a/src/nativeServices/servicePlan.test.ts b/src/nativeServices/servicePlan.test.ts index 4560bf5..c8b44eb 100644 --- a/src/nativeServices/servicePlan.test.ts +++ b/src/nativeServices/servicePlan.test.ts @@ -164,7 +164,14 @@ describe("production native service planning", () => { prerequisites: [], }, ]); - expect(planValidationProbeRequests(resolution.plan)).toEqual([]); + expect(planValidationProbeRequests(resolution.plan)).toEqual([{ + purpose: "plan-validation", + backend, + shell, + environment: { PI_WEB_CONFIG: "/home/user/.config/pi-web/config.json" }, + workingDirectory: null, + prerequisites: [], + }]); }); it("falls back per service to bundled entrypoints when named commands are unavailable", async () => { diff --git a/src/nativeServices/servicePlan.ts b/src/nativeServices/servicePlan.ts index 0a20a00..077fc5a 100644 --- a/src/nativeServices/servicePlan.ts +++ b/src/nativeServices/servicePlan.ts @@ -162,6 +162,7 @@ export interface DevelopmentNativeServicePlanInput { export interface NativeServicePlanDependencies { probe: NativeServiceAuthoritativeProbe; + /** Returns true only when the path exists and is a regular file. */ fileExists(path: string): boolean; } @@ -206,6 +207,10 @@ export type NativeServicePlanValidation = | { ok: true } | { ok: false; failures: readonly NativeServicePlanValidationFailure[] }; +export function nativeServicePrerequisiteNeedsPathAdvice(prerequisite: NativeServicePrerequisite): boolean { + return prerequisite.kind === "command-available" || prerequisite.kind === "node-version"; +} + export const nativeServiceManagerRefs: Readonly> = { sessiond: { systemdName: "pi-web-sessiond.service", @@ -386,7 +391,6 @@ export function createDevelopmentNativeServicePlan(input: DevelopmentNativeServi export function planValidationProbeRequests(plan: NativeServicePlan): readonly NativeServiceProbeRequest[] { const requests: (Omit & { prerequisites: NativeServicePrerequisite[] })[] = []; for (const service of plan.services) { - if (service.prerequisites.length === 0) continue; const existing = requests.find((request) => request.workingDirectory === service.workingDirectory && environmentsEqual(request.environment, service.environment)); @@ -558,7 +562,7 @@ function commandRequirement(serviceId: NativeServiceId, command: string): Native id: commandRequirementId(serviceId, command), kind: "command-available", command, - description: `${command} is available to the service shell`, + description: `${command} resolves to an external executable for the service shell`, }; } @@ -577,7 +581,7 @@ function readableFileRequirement(serviceId: NativeServiceId, path: string): Nati id: `${serviceId}.entrypoint`, kind: "readable-file", path, - description: `bundled entrypoint is readable: ${path}`, + description: `bundled entrypoint is a readable regular file: ${path}`, }; } diff --git a/src/nativeServices/serviceProbe.test.ts b/src/nativeServices/serviceProbe.test.ts index 657cb64..bd1b491 100644 --- a/src/nativeServices/serviceProbe.test.ts +++ b/src/nativeServices/serviceProbe.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it, vi } from "vitest"; import { LaunchdNativeServiceProbe, + SpawnProbeCommandRunner, SystemdNativeServiceProbe, launchdProbePlist, + nativeServicePrerequisiteShellCheck, systemdRunArguments, type LaunchdProbeFileSystem, type ProbeCommandResult, @@ -74,6 +76,8 @@ describe("systemd authoritative native-service probe", () => { "--pipe", "--quiet", "--unit=pi-web-authoritative-probe-fixed.service", + "--property=RuntimeMaxSec=15s", + "--property=TimeoutStopSec=5s", "--setenv=PI_WEB_CONFIG=/home/user/config with space.json", "--working-directory=/checkout with space", "/usr/bin/env", @@ -91,7 +95,7 @@ describe("systemd authoritative native-service probe", () => { outcomes: [{ prerequisiteId: "sessiond.command.npm", status: "unsatisfied", - detail: "npm was not found in the native service environment.", + detail: "npm did not resolve to an external executable in the native service environment.", }], }); @@ -106,7 +110,7 @@ describe("systemd authoritative native-service probe", () => { const runner = queuedRunner([ { kind: "timeout", stdout: "", stderr: "" }, completed(0), - completed(0), + completed(0, "not-found\n"), ]); const probe = new SystemdNativeServiceProbe({ commandRunner: runner, @@ -125,7 +129,7 @@ describe("systemd authoritative native-service probe", () => { const runner = queuedRunner([ { kind: "timeout", stdout: "", stderr: "" }, completed(0), - completed(1, "", "unit still loaded"), + completed(0, "loaded\n", "unit still loaded"), ]); const probe = new SystemdNativeServiceProbe({ commandRunner: runner, @@ -139,22 +143,43 @@ describe("systemd authoritative native-service probe", () => { expect(runner.calls.map(({ command, args }) => [command, ...args.slice(0, 3)])).toEqual([ ["systemd-run", "--user", "--wait", "--collect"], ["systemctl", "--user", "stop", "pi-web-authoritative-probe-fixed.service"], - ["systemctl", "--user", "reset-failed", "pi-web-authoritative-probe-fixed.service"], + ["systemctl", "--user", "show", "pi-web-authoritative-probe-fixed.service"], ]); }); }); +describe("spawn probe command runner", () => { + it("bounds captured command output", async () => { + const runner = new SpawnProbeCommandRunner(); + const result = await runner.run( + process.execPath, + ["-e", "process.stdout.write('x'.repeat(2 * 1024 * 1024))"], + 5_000, + ); + + expect(result).toMatchObject({ kind: "output-limit" }); + expect(result.stdout.length).toBeLessThanOrEqual(1024 * 1024); + }); + + it("settles a timeout without waiting for inherited pipes to close", async () => { + const runner = new SpawnProbeCommandRunner(); + const childScript = [ + "const { spawn } = require('node:child_process');", + "const child = spawn(process.execPath, ['-e', 'setTimeout(() => {}, 1000)'], { stdio: ['ignore', 'inherit', 'inherit'] });", + "child.unref();", + ].join(" "); + const startedAt = performance.now(); + + await expect(runner.run(process.execPath, ["-e", childScript], 20)).resolves.toMatchObject({ kind: "timeout" }); + expect(performance.now() - startedAt).toBeLessThan(500); + }); +}); + describe("launchd authoritative native-service probe", () => { it("bootstraps a uniquely labelled one-shot agent in gui/ and always cleans it up", async () => { - const runner = queuedRunner([ - completed(0), - completed(0, "state = running\n"), - completed(0, "state = not running\nlast exit code = 0\n"), - completed(0), - ]); + const runner = queuedRunner([completed(0), completed(0)]); const fileSystem = launchdFileSystem({ - "/tmp/probe/stdout.log": marker("sessiond.command.npm", "satisfied"), - "/tmp/probe/stderr.log": "", + "/tmp/probe/result.log": marker("sessiond.command.npm", "satisfied"), }); let now = 0; const probe = new LaunchdNativeServiceProbe({ @@ -172,28 +197,28 @@ describe("launchd authoritative native-service probe", () => { await expect(probe.run(request("launchd"))).resolves.toMatchObject({ kind: "completed" }); expect(runner.calls.map(({ command, args }) => [command, ...args])).toEqual([ ["launchctl", "bootstrap", "gui/501", "/tmp/probe/probe.plist"], - ["launchctl", "print", "gui/501/com.pi-web.authoritative-probe.501.fixed"], - ["launchctl", "print", "gui/501/com.pi-web.authoritative-probe.501.fixed"], ["launchctl", "bootout", "gui/501/com.pi-web.authoritative-probe.501.fixed"], ]); expect(fileSystem.writeFile).toHaveBeenCalledWith( "/tmp/probe/probe.plist", expect.stringContaining("/bin/zsh"), + 0o600, ); expect(fileSystem.writeFile).toHaveBeenCalledWith( "/tmp/probe/probe.plist", expect.stringContaining("WorkingDirectory\n /checkout with space"), + 0o600, + ); + expect(fileSystem.writeFile).toHaveBeenCalledWith( + "/tmp/probe/probe.plist", + expect.stringContaining("/bin/mv '/tmp/probe/result.pending' '/tmp/probe/result.log'"), + 0o600, ); expect(fileSystem.removeDirectory).toHaveBeenCalledWith("/tmp/probe"); }); it("times out deterministically, boots out the agent, and removes temporary files", async () => { - const runner = queuedRunner([ - completed(0), - completed(0, "state = running\n"), - completed(0, "state = running\n"), - completed(0), - ]); + const runner = queuedRunner([completed(0), completed(0)]); const fileSystem = launchdFileSystem({}); let now = 0; const probe = new LaunchdNativeServiceProbe({ @@ -219,15 +244,34 @@ describe("launchd authoritative native-service probe", () => { expect(fileSystem.removeDirectory).toHaveBeenCalledWith("/tmp/probe"); }); + it("bounds a stalled result-file read before cleaning up", async () => { + const runner = queuedRunner([completed(0), completed(0)]); + const fileSystem = launchdFileSystem({}); + fileSystem.readOptionalFile.mockReturnValueOnce(new Promise(() => undefined)); + const probe = new LaunchdNativeServiceProbe({ + commandRunner: runner, + fileSystem, + uid: 502, + createUniqueId: () => "fixed", + now: () => 0, + sleep: () => Promise.resolve(), + probeTimeoutMs: 10, + pollIntervalMs: 1, + commandTimeoutMs: 100, + }); + + await expect(probe.run(request("launchd"))).resolves.toMatchObject({ + kind: "infrastructure-failure", + reason: "timeout", + }); + expect(runner.calls.map(({ args }) => args[0])).toEqual(["bootstrap", "bootout"]); + expect(fileSystem.removeDirectory).toHaveBeenCalledWith("/tmp/probe"); + }); + it("surfaces cleanup failure instead of returning an otherwise successful probe", async () => { - const runner = queuedRunner([ - completed(0), - completed(0, "state = not running\nlast exit code = 0\n"), - completed(1, "", "bootout denied"), - ]); + const runner = queuedRunner([completed(0), completed(1, "", "bootout denied")]); const fileSystem = launchdFileSystem({ - "/tmp/probe/stdout.log": marker("sessiond.command.npm", "satisfied"), - "/tmp/probe/stderr.log": "", + "/tmp/probe/result.log": marker("sessiond.command.npm", "satisfied"), }); const probe = new LaunchdNativeServiceProbe({ commandRunner: runner, @@ -247,10 +291,9 @@ describe("launchd authoritative native-service probe", () => { expect(fileSystem.removeDirectory).toHaveBeenCalledWith("/tmp/probe"); }); - it("checks and boots out a label when bootstrap itself times out", async () => { + it("boots out a label when bootstrap itself times out", async () => { const runner = queuedRunner([ { kind: "timeout", stdout: "", stderr: "" }, - completed(0, "state = running\n"), completed(0), ]); const fileSystem = launchdFileSystem({}); @@ -270,12 +313,15 @@ describe("launchd authoritative native-service probe", () => { kind: "infrastructure-failure", reason: "timeout", }); - expect(runner.calls.map(({ args }) => args[0])).toEqual(["bootstrap", "print", "bootout"]); + expect(runner.calls.map(({ args }) => args[0])).toEqual(["bootstrap", "bootout"]); expect(fileSystem.removeDirectory).toHaveBeenCalledWith("/tmp/probe"); }); - it("removes temporary files when bootstrap fails without booting out an unloaded label", async () => { - const runner = queuedRunner([completed(1, "", "bootstrap denied")]); + it("treats an explicit not-loaded bootout response as successful cleanup after bootstrap fails", async () => { + const runner = queuedRunner([ + completed(1, "", "bootstrap denied"), + completed(3, "", "Could not find service in domain"), + ]); const fileSystem = launchdFileSystem({}); const probe = new LaunchdNativeServiceProbe({ commandRunner: runner, @@ -292,17 +338,13 @@ describe("launchd authoritative native-service probe", () => { const result = await probe.run(request("launchd")); expect(result).toMatchObject({ kind: "infrastructure-failure", reason: "manager" }); expect(result.kind === "infrastructure-failure" && result.message).toContain("bootstrap denied"); - expect(runner.calls).toHaveLength(1); + expect(runner.calls.map(({ args }) => args[0])).toEqual(["bootstrap", "bootout"]); expect(fileSystem.removeDirectory).toHaveBeenCalledWith("/tmp/probe"); }); - it("cleans a loaded label after malformed launchctl output", async () => { - const runner = queuedRunner([ - completed(0), - completed(0, "pid = 123\n"), - completed(0), - ]); - const fileSystem = launchdFileSystem({}); + it("cleans a loaded label after malformed private result output", async () => { + const runner = queuedRunner([completed(0), completed(0)]); + const fileSystem = launchdFileSystem({ "/tmp/probe/result.log": "malformed result" }); const probe = new LaunchdNativeServiceProbe({ commandRunner: runner, fileSystem, @@ -323,13 +365,10 @@ describe("launchd authoritative native-service probe", () => { expect(fileSystem.removeDirectory).toHaveBeenCalledWith("/tmp/probe"); }); - it("cleans a loaded label when probe output cannot be read", async () => { - const runner = queuedRunner([ - completed(0), - completed(0, "state = not running\nlast exit code = 0\n"), - completed(0), - ]); + it("cleans a loaded label when the private result cannot be read", async () => { + const runner = queuedRunner([completed(0), completed(0)]); const fileSystem = launchdFileSystem({}); + fileSystem.readOptionalFile.mockRejectedValueOnce(new Error("read denied")); const probe = new LaunchdNativeServiceProbe({ commandRunner: runner, fileSystem, @@ -344,13 +383,16 @@ describe("launchd authoritative native-service probe", () => { const result = await probe.run(request("launchd")); expect(result).toMatchObject({ kind: "infrastructure-failure", reason: "manager" }); - expect(result.kind === "infrastructure-failure" && result.message).toContain("Could not read launchd probe output"); + expect(result.kind === "infrastructure-failure" && result.message).toContain("Could not read launchd probe result"); expect(runner.calls.at(-1)?.args[0]).toBe("bootout"); expect(fileSystem.removeDirectory).toHaveBeenCalledWith("/tmp/probe"); }); it("reports temporary-file cleanup failures", async () => { - const runner = queuedRunner([completed(1, "", "bootstrap denied")]); + const runner = queuedRunner([ + completed(1, "", "bootstrap denied"), + completed(3, "", "Could not find service in domain"), + ]); const fileSystem = launchdFileSystem({}); fileSystem.removeDirectory.mockRejectedValueOnce(new Error("rm denied")); const probe = new LaunchdNativeServiceProbe({ @@ -372,28 +414,70 @@ describe("launchd authoritative native-service probe", () => { }); describe("probe service definitions", () => { + it("requires external executables instead of accepting shell functions or aliases", () => { + const commandRequirement = request().prerequisites[0]; + if (commandRequirement === undefined) throw new Error("Expected a command prerequisite"); + const bashCheck = nativeServicePrerequisiteShellCheck("bash", commandRequirement); + expect(bashCheck).toContain("case \"$pi_web_probe_executable\" in */*)"); + expect(bashCheck).toContain("test -f \"$pi_web_probe_executable\""); + expect(bashCheck).toContain("test -x \"$pi_web_probe_executable\""); + + const fishCheck = nativeServicePrerequisiteShellCheck("fish", commandRequirement); + expect(fishCheck).toContain("string match -q '*/*'"); + expect(fishCheck).toContain("test -f $pi_web_probe_executable[1]"); + expect(fishCheck).toContain("test -x $pi_web_probe_executable[1]"); + }); + + it("invokes the resolved external Node executable for version checks", () => { + const check = nativeServicePrerequisiteShellCheck("zsh", { + id: "sessiond.node", + kind: "node-version", + command: "node", + minimumMajor: 22, + description: "node >= 22", + }); + expect(check).toContain("\"$pi_web_probe_executable\" '-e'"); + expect(check).not.toContain("&& node -e"); + }); + + it("requires bundled entrypoints to be readable regular files", () => { + const check = nativeServicePrerequisiteShellCheck("bash", { + id: "sessiond.entrypoint", + kind: "readable-file", + path: "/package/server.js", + description: "entrypoint", + }); + expect(check).toBe("test -f '/package/server.js' && test -r '/package/server.js'"); + }); + it("renders backend inputs without inheriting the caller PATH", () => { const probeRequest = request(); - expect(systemdRunArguments(probeRequest, "probe.service", "echo ok")).not.toContain(expect.stringContaining("PATH=")); + const systemdArguments = systemdRunArguments(probeRequest, "probe.service", "echo ok"); + expect(systemdArguments.some((argument) => argument.includes("PATH="))).toBe(false); const plist = launchdProbePlist(probeRequest, "com.example.probe", "echo ok", "/tmp/out", "/tmp/err"); expect(plist).not.toContain("PATH"); expect(plist).toContain("PI_WEB_CONFIG"); + expect(plist).toContain("HardResourceLimits"); + }); + + it("escapes manager-side substitutions in the systemd probe payload", () => { + const args = systemdRunArguments(request(), "probe.service", "test -r '/tmp/$HOME/%h'"); + expect(args.at(-1)).toBe("test -r '/tmp/$$HOME/%h'"); }); }); function launchdFileSystem(contents: Record): LaunchdProbeFileSystem & { writeFile: ReturnType>; + readOptionalFile: ReturnType>; removeDirectory: ReturnType>; } { const writeFileMock = vi.fn(() => Promise.resolve()); + const readOptionalFileMock = vi.fn((path) => Promise.resolve(contents[path] ?? null)); const removeDirectoryMock = vi.fn(() => Promise.resolve()); return { createTemporaryDirectory: () => Promise.resolve("/tmp/probe"), writeFile: writeFileMock, - readFile: (path) => { - const content = contents[path]; - return content === undefined ? Promise.reject(new Error(`missing ${path}`)) : Promise.resolve(content); - }, + readOptionalFile: readOptionalFileMock, removeDirectory: removeDirectoryMock, }; } diff --git a/src/nativeServices/serviceProbe.ts b/src/nativeServices/serviceProbe.ts index 97cd1bc..3ff8b83 100644 --- a/src/nativeServices/serviceProbe.ts +++ b/src/nativeServices/serviceProbe.ts @@ -2,6 +2,7 @@ import { spawn } from "node:child_process"; import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir, userInfo } from "node:os"; import { join } from "node:path"; +import { performance } from "node:perf_hooks"; import { randomUUID } from "node:crypto"; import type { NativeServiceAuthoritativeProbe, @@ -15,7 +16,8 @@ import type { export type ProbeCommandResult = | { kind: "completed"; status: number; stdout: string; stderr: string } | { kind: "timeout"; stdout: string; stderr: string } - | { kind: "spawn-failure"; message: string; stdout: string; stderr: string }; + | { kind: "spawn-failure"; message: string; stdout: string; stderr: string } + | { kind: "output-limit"; stdout: string; stderr: string }; export interface ProbeCommandRunner { run(command: string, args: readonly string[], timeoutMs: number): Promise; @@ -23,8 +25,8 @@ export interface ProbeCommandRunner { export interface LaunchdProbeFileSystem { createTemporaryDirectory(prefix: string): Promise; - writeFile(path: string, contents: string): Promise; - readFile(path: string): Promise; + writeFile(path: string, contents: string, mode: number): Promise; + readOptionalFile(path: string): Promise; removeDirectory(path: string): Promise; } @@ -48,6 +50,8 @@ export interface LaunchdProbeDependencies extends CommonProbeDependencies { const defaultCommandTimeoutMs = 15_000; const defaultProbeTimeoutMs = 15_000; const defaultPollIntervalMs = 50; +const maxCapturedCommandOutputBytes = 1024 * 1024; +const maxLaunchdProbeFileBytes = 1024 * 1024; export class SystemdNativeServiceProbe implements NativeServiceAuthoritativeProbe { public constructor(private readonly dependencies: SystemdProbeDependencies) {} @@ -64,12 +68,12 @@ export class SystemdNativeServiceProbe implements NativeServiceAuthoritativeProb const args = systemdRunArguments(request, unitName, command); const result = await this.dependencies.commandRunner.run("systemd-run", args, this.dependencies.commandTimeoutMs); - if (result.kind === "timeout") { + if (result.kind === "timeout" || result.kind === "output-limit") { const cleanupFailure = await this.cleanupTimedOutUnit(unitName); - return cleanupFailure ?? infrastructureFailure( - "timeout", - `Timed out waiting for transient systemd unit ${unitName}.`, - ); + if (cleanupFailure !== null) return cleanupFailure; + return result.kind === "timeout" + ? infrastructureFailure("timeout", `Timed out waiting for transient systemd unit ${unitName}.`) + : infrastructureFailure("manager", `Transient systemd probe ${unitName} exceeded the output limit.`); } if (result.kind === "spawn-failure") { return infrastructureFailure("manager", `Could not start systemd-run: ${result.message}`); @@ -89,18 +93,19 @@ export class SystemdNativeServiceProbe implements NativeServiceAuthoritativeProb ["--user", "stop", unitName], this.dependencies.commandTimeoutMs, ); - if (stop.kind !== "completed" || stop.status !== 0) { - return infrastructureFailure("cleanup", `Could not stop timed-out transient systemd unit ${unitName}: ${commandFailureDetail(stop)}`); - } - const reset = await this.dependencies.commandRunner.run( + const inspected = await this.dependencies.commandRunner.run( "systemctl", - ["--user", "reset-failed", unitName], + ["--user", "show", unitName, "--property=LoadState", "--value"], this.dependencies.commandTimeoutMs, ); - if (reset.kind !== "completed" || reset.status !== 0) { - return infrastructureFailure("cleanup", `Could not collect timed-out transient systemd unit ${unitName}: ${commandFailureDetail(reset)}`); + if (inspected.kind === "completed" && inspected.status === 0 && inspected.stdout.trim() === "not-found") { + return null; } - return null; + const details = [ + `stop: ${commandFailureDetail(stop)}`, + `load state: ${commandFailureDetail(inspected)}`, + ].join("; "); + return infrastructureFailure("cleanup", `Could not confirm cleanup of timed-out transient systemd unit ${unitName}: ${details}`); } } @@ -128,10 +133,18 @@ export class LaunchdNativeServiceProbe implements NativeServiceAuthoritativeProb const plistPath = join(directory, "probe.plist"); const stdoutPath = join(directory, "stdout.log"); const stderrPath = join(directory, "stderr.log"); - const command = prerequisiteProbeCommand(request.shell.name, request.prerequisites, outputPrefix); + const pendingResultPath = join(directory, "result.pending"); + const resultPath = join(directory, "result.log"); + const command = prerequisiteProbeCommand( + request.shell.name, + request.prerequisites, + outputPrefix, + { pendingPath: pendingResultPath, completedPath: resultPath }, + ); await this.dependencies.fileSystem.writeFile( plistPath, launchdProbePlist(request, label, command, stdoutPath, stderrPath), + 0o600, ); const bootstrap = await this.dependencies.commandRunner.run( @@ -140,11 +153,11 @@ export class LaunchdNativeServiceProbe implements NativeServiceAuthoritativeProb this.dependencies.commandTimeoutMs, ); if (bootstrap.kind !== "completed" || bootstrap.status !== 0) { - bootstrapState = bootstrap.kind === "timeout" ? "uncertain" : "not-loaded"; + bootstrapState = bootstrap.kind === "spawn-failure" ? "not-loaded" : "uncertain"; result = commandInfrastructureFailure("bootstrap launchd probe", bootstrap); } else { bootstrapState = "loaded"; - result = await this.waitForResult(target, stdoutPath, stderrPath, request.prerequisites, outputPrefix); + result = await this.waitForResult(target, resultPath, request.prerequisites, outputPrefix); } } catch (error: unknown) { result = infrastructureFailure("manager", `Could not prepare launchd probe: ${errorMessage(error)}`); @@ -156,48 +169,25 @@ export class LaunchdNativeServiceProbe implements NativeServiceAuthoritativeProb private async waitForResult( target: string, - stdoutPath: string, - stderrPath: string, + resultPath: string, prerequisites: readonly NativeServicePrerequisite[], outputPrefix: string, ): Promise { const deadline = this.dependencies.now() + this.dependencies.probeTimeoutMs; while (this.dependencies.now() < deadline) { - const printed = await this.dependencies.commandRunner.run( - "launchctl", - ["print", target], - this.dependencies.commandTimeoutMs, + const remainingMs = Math.max(0, deadline - this.dependencies.now()); + const boundedRead = await readOptionalFileBounded( + this.dependencies.fileSystem, + resultPath, + remainingMs, ); - if (printed.kind !== "completed" || printed.status !== 0) { - return commandInfrastructureFailure("inspect launchd probe", printed); + if (boundedRead.kind === "deadline") break; + if (boundedRead.kind === "read-failure") { + return infrastructureFailure("manager", `Could not read launchd probe result: ${errorMessage(boundedRead.error)}`); } - - const state = launchdField(printed.stdout, "state"); - if (state === undefined) { - return infrastructureFailure("malformed-output", `launchctl returned no state for ${target}.`); - } - const lastExitCode = launchdIntegerField(printed.stdout, "last exit code"); - if (state === "not running" && lastExitCode !== undefined) { - let stdout: string; - let stderr: string; - try { - [stdout, stderr] = await Promise.all([ - this.dependencies.fileSystem.readFile(stdoutPath), - this.dependencies.fileSystem.readFile(stderrPath), - ]); - } catch (error: unknown) { - return infrastructureFailure("manager", `Could not read launchd probe output: ${errorMessage(error)}`); - } - if (lastExitCode !== 0) { - return infrastructureFailure( - "manager", - `Launchd probe service exited with status ${String(lastExitCode)}: ${firstOutput(stderr, stdout, "no output")}`, - ); - } - return parseProbeOutput(stdout, prerequisites, outputPrefix); - } - - await this.dependencies.sleep(this.dependencies.pollIntervalMs); + if (boundedRead.output !== null) return parseProbeOutput(boundedRead.output, prerequisites, outputPrefix); + const pollDelayMs = Math.min(this.dependencies.pollIntervalMs, Math.max(0, deadline - this.dependencies.now())); + await this.dependencies.sleep(pollDelayMs); } return infrastructureFailure("timeout", `Timed out waiting for launchd probe ${target}.`); } @@ -208,28 +198,21 @@ export class LaunchdNativeServiceProbe implements NativeServiceAuthoritativeProb bootstrapState: "not-loaded" | "loaded" | "uncertain", ): Promise { const failures: string[] = []; - let shouldBootout = bootstrapState === "loaded"; - if (bootstrapState === "uncertain") { - const inspection = await this.dependencies.commandRunner.run( - "launchctl", - ["print", target], - this.dependencies.commandTimeoutMs, - ); - if (inspection.kind === "completed") { - shouldBootout = inspection.status === 0; - } else { - // If launchctl cannot tell us whether a timed-out bootstrap loaded the - // label, bootout is the only operation that can make cleanup certain. - shouldBootout = true; - } - } + const shouldBootout = bootstrapState !== "not-loaded"; if (shouldBootout) { + // A failed or timed-out bootstrap may still have loaded the unique label. + // Bootout is the only race-free cleanup; an explicit not-loaded response + // is success when bootstrap completion was uncertain. + const absenceIsSuccess = bootstrapState === "uncertain"; const bootout = await this.dependencies.commandRunner.run( "launchctl", ["bootout", target], this.dependencies.commandTimeoutMs, ); - if (bootout.kind !== "completed" || bootout.status !== 0) { + if ( + (bootout.kind !== "completed" || bootout.status !== 0) + && !(absenceIsSuccess && launchdTargetNotLoaded(bootout)) + ) { failures.push(`bootout failed: ${commandFailureDetail(bootout)}`); } } @@ -258,7 +241,7 @@ export function createNativeServiceAuthoritativeProbe(): NativeServiceAuthoritat ...common, fileSystem: nodeLaunchdProbeFileSystem, uid: userInfo().uid, - now: Date.now, + now: performance.now.bind(performance), sleep: (milliseconds) => new Promise((resolve) => setTimeout(resolve, milliseconds)), probeTimeoutMs: defaultProbeTimeoutMs, pollIntervalMs: defaultPollIntervalMs, @@ -280,15 +263,21 @@ export function systemdRunArguments( "--pipe", "--quiet", `--unit=${unitName}`, + "--property=RuntimeMaxSec=15s", + "--property=TimeoutStopSec=5s", ...Object.entries(request.environment).map(([key, value]) => `--setenv=${key}=${value}`), ...(request.workingDirectory === null ? [] : [`--working-directory=${request.workingDirectory}`]), "/usr/bin/env", - request.shell.executable, + escapeSystemdCommandExpansion(request.shell.executable), "-lc", - shellCommand, + escapeSystemdCommandExpansion(shellCommand), ]; } +function escapeSystemdCommandExpansion(value: string): string { + return value.replaceAll("$", () => "$$"); +} + export function launchdProbePlist( request: NativeServiceProbeRequest, label: string, @@ -316,36 +305,63 @@ ${argumentsXml} ${workingDirectoryXml}${environmentXml} RunAtLoad + HardResourceLimits + + FileSize + ${String(maxLaunchdProbeFileBytes)} + ${plistString("StandardOutPath", stdoutPath)}${plistString("StandardErrorPath", stderrPath)} `; } -class SpawnProbeCommandRunner implements ProbeCommandRunner { +export class SpawnProbeCommandRunner implements ProbeCommandRunner { public run(command: string, args: readonly string[], timeoutMs: number): Promise { return new Promise((resolve) => { const child = spawn(command, args, { stdio: ["ignore", "pipe", "pipe"] }); let stdout = ""; let stderr = ""; + let capturedBytes = 0; let spawnFailure: string | null = null; - let timedOut = false; + let settled = false; + + const finish = (result: ProbeCommandResult, terminate: boolean): void => { + if (settled) return; + settled = true; + clearTimeout(timeout); + if (terminate) { + child.kill("SIGKILL"); + child.stdout.destroy(); + child.stderr.destroy(); + child.unref(); + } + resolve(result); + }; + const capture = (stream: "stdout" | "stderr", chunk: string): void => { + if (settled) return; + const bytes = Buffer.byteLength(chunk); + if (capturedBytes + bytes > maxCapturedCommandOutputBytes) { + finish({ kind: "output-limit", stdout, stderr }, true); + return; + } + capturedBytes += bytes; + if (stream === "stdout") stdout += chunk; + else stderr += chunk; + }; + child.stdout.setEncoding("utf8"); child.stderr.setEncoding("utf8"); - child.stdout.on("data", (chunk: string) => { stdout += chunk; }); - child.stderr.on("data", (chunk: string) => { stderr += chunk; }); + child.stdout.on("data", (chunk: string) => { capture("stdout", chunk); }); + child.stderr.on("data", (chunk: string) => { capture("stderr", chunk); }); child.on("error", (error) => { spawnFailure = error.message; }); const timeout = setTimeout(() => { - timedOut = true; - child.kill("SIGKILL"); + finish({ kind: "timeout", stdout, stderr }, true); }, timeoutMs); child.on("close", (status) => { - clearTimeout(timeout); - if (timedOut) { - resolve({ kind: "timeout", stdout, stderr }); - } else if (spawnFailure !== null) { - resolve({ kind: "spawn-failure", message: spawnFailure, stdout, stderr }); + if (spawnFailure !== null) { + finish({ kind: "spawn-failure", message: spawnFailure, stdout, stderr }, false); } else { - resolve({ kind: "completed", status: status ?? 1, stdout, stderr }); + finish({ kind: "completed", status: status ?? 1, stdout, stderr }, false); } }); }); @@ -354,49 +370,109 @@ class SpawnProbeCommandRunner implements ProbeCommandRunner { const nodeLaunchdProbeFileSystem: LaunchdProbeFileSystem = { createTemporaryDirectory: (prefix) => mkdtemp(prefix), - writeFile: (path, contents) => writeFile(path, contents, "utf8"), - readFile: (path) => readFile(path, "utf8"), + writeFile: (path, contents, mode) => writeFile(path, contents, { encoding: "utf8", mode }), + readOptionalFile: async (path) => { + try { + return await readFile(path, "utf8"); + } catch (error: unknown) { + if (isNodeErrorWithCode(error, "ENOENT")) return null; + throw error; + } + }, removeDirectory: (path) => rm(path, { recursive: true, force: true }), }; +function readOptionalFileBounded( + fileSystem: LaunchdProbeFileSystem, + path: string, + timeoutMs: number, +): Promise< + | { kind: "read"; output: string | null } + | { kind: "read-failure"; error: unknown } + | { kind: "deadline" } +> { + return new Promise((resolve) => { + let settled = false; + const finish = (result: + | { kind: "read"; output: string | null } + | { kind: "read-failure"; error: unknown } + | { kind: "deadline" }): void => { + if (settled) return; + settled = true; + clearTimeout(timeout); + resolve(result); + }; + const timeout = setTimeout(() => { finish({ kind: "deadline" }); }, timeoutMs); + void fileSystem.readOptionalFile(path).then( + (output) => { finish({ kind: "read", output }); }, + (error: unknown) => { finish({ kind: "read-failure", error }); }, + ); + }); +} + function prerequisiteProbeCommand( shell: NativeServiceShellName, prerequisites: readonly NativeServicePrerequisite[], outputPrefix: string, + resultFiles?: { pendingPath: string; completedPath: string }, ): string { - return prerequisites.map((prerequisite) => { + const markerPath = resultFiles?.pendingPath; + const checks = prerequisites.map((prerequisite) => { const check = nativeServicePrerequisiteShellCheck(shell, prerequisite); const encodedId = Buffer.from(prerequisite.id, "utf8").toString("base64"); - const satisfied = markerCommand(shell, outputPrefix, encodedId, "satisfied"); - const unsatisfied = markerCommand(shell, outputPrefix, encodedId, "unsatisfied"); + const satisfied = markerCommand(shell, outputPrefix, encodedId, "satisfied", markerPath); + const unsatisfied = markerCommand(shell, outputPrefix, encodedId, "unsatisfied", markerPath); return `${check} >/dev/null 2>&1 && ${satisfied} || ${unsatisfied}`; }).join("; ") || ":"; + if (resultFiles === undefined) return checks; + const pending = shellQuote(shell, resultFiles.pendingPath); + const completed = shellQuote(shell, resultFiles.completedPath); + return `printf '%s' '' > ${pending}; ${checks}; /bin/mv ${pending} ${completed}`; } export function nativeServicePrerequisiteShellCheck(shell: NativeServiceShellName, prerequisite: NativeServicePrerequisite): string { switch (prerequisite.kind) { case "command-available": - return `command -v ${shellQuote(shell, prerequisite.command)}`; + return externalExecutableShellCheck(shell, prerequisite.command); case "node-version": { const script = `const major=Number(process.versions.node.split('.')[0]);process.exit(major>=${String(prerequisite.minimumMajor)}?0:1)`; - return `node -e ${shellQuote(shell, script)}`; + return externalExecutableShellCheck(shell, "node", ["-e", script]); + } + case "readable-file": { + const path = shellQuote(shell, prerequisite.path); + return `test -f ${path} && test -r ${path}`; } - case "readable-file": - return `test -r ${shellQuote(shell, prerequisite.path)}`; case "package-scripts": { const script = "const p=require(process.argv[1]);const names=process.argv.slice(2);process.exit(names.every((name)=>typeof p.scripts?.[name]==='string')?0:1)"; - return ["node", "-e", shellQuote(shell, script), shellQuote(shell, prerequisite.packageJsonPath), ...prerequisite.scripts.map((name) => shellQuote(shell, name))].join(" "); + return externalExecutableShellCheck(shell, "node", ["-e", script, prerequisite.packageJsonPath, ...prerequisite.scripts]); } } } +function externalExecutableShellCheck( + shell: NativeServiceShellName, + command: string, + arguments_: readonly string[] = [], +): string { + const quotedCommand = shellQuote(shell, command); + const quotedArguments = arguments_.map((argument) => shellQuote(shell, argument)).join(" "); + if (shell === "fish") { + const invocation = quotedArguments === "" ? "" : `; and $pi_web_probe_executable[1] ${quotedArguments}`; + return `set -l pi_web_probe_executable (command -v ${quotedCommand}); and test (count $pi_web_probe_executable) -eq 1; and string match -q '*/*' -- $pi_web_probe_executable[1]; and test -f $pi_web_probe_executable[1]; and test -x $pi_web_probe_executable[1]${invocation}`; + } + const invocation = quotedArguments === "" ? "" : ` && "$pi_web_probe_executable" ${quotedArguments}`; + return `pi_web_probe_executable=$(command -v ${quotedCommand}) && case "$pi_web_probe_executable" in */*) test -f "$pi_web_probe_executable" && test -x "$pi_web_probe_executable"${invocation};; *) false;; esac`; +} + function markerCommand( shell: NativeServiceShellName, outputPrefix: string, encodedId: string, status: "satisfied" | "unsatisfied", + outputPath?: string, ): string { - return `printf '%s\\t%s\\t%s\\n' ${shellQuote(shell, outputPrefix)} ${shellQuote(shell, encodedId)} ${shellQuote(shell, status)}`; + const redirect = outputPath === undefined ? "" : ` >> ${shellQuote(shell, outputPath)}`; + return `printf '%s\\t%s\\t%s\\n' ${shellQuote(shell, outputPrefix)} ${shellQuote(shell, encodedId)} ${shellQuote(shell, status)}${redirect}`; } function parseProbeOutput( @@ -440,11 +516,11 @@ function parseProbeOutput( function unsatisfiedDetail(prerequisite: NativeServicePrerequisite): string { switch (prerequisite.kind) { case "command-available": - return `${prerequisite.command} was not found in the native service environment.`; + return `${prerequisite.command} did not resolve to an external executable in the native service environment.`; case "node-version": return `node >= ${String(prerequisite.minimumMajor)} was not available in the native service environment.`; case "readable-file": - return `${prerequisite.path} was not readable in the native service environment.`; + return `${prerequisite.path} was not a readable regular file in the native service environment.`; case "package-scripts": return `${prerequisite.packageJsonPath} did not provide scripts ${prerequisite.scripts.join(", ")} in the native service environment.`; } @@ -461,18 +537,9 @@ function safeUniqueId(value: string): string { return safe === "" ? "probe" : safe; } -function launchdField(output: string, field: string): string | undefined { - return new RegExp(`^\\s*${escapeRegExp(field)}\\s*=\\s*(.+)$`, "mu").exec(output)?.[1]?.trim(); -} - -function launchdIntegerField(output: string, field: string): number | undefined { - const value = launchdField(output, field); - if (value === undefined || !/^-?\d+$/u.test(value)) return undefined; - return Number(value); -} - -function escapeRegExp(value: string): string { - return value.replaceAll(/[.*+?^${}()|[\]\\]/gu, "\\$&"); +function launchdTargetNotLoaded(result: ProbeCommandResult): boolean { + if (result.kind !== "completed" || result.status === 0) return false; + return /(?:could not find (?:specified )?service|service not found|no such process)/iu.test(`${result.stderr}\n${result.stdout}`); } function plistString(key: string, value: string, indent = " "): string { @@ -496,6 +563,7 @@ function commandInfrastructureFailure(action: string, result: ProbeCommandResult function commandFailureDetail(result: ProbeCommandResult): string { if (result.kind === "timeout") return "command timed out"; if (result.kind === "spawn-failure") return result.message; + if (result.kind === "output-limit") return "command output exceeded the capture limit"; return firstOutput(result.stderr, result.stdout, `exit status ${String(result.status)}`); } @@ -517,3 +585,7 @@ function firstOutput(...values: string[]): string { function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } + +function isNodeErrorWithCode(error: unknown, code: string): error is NodeJS.ErrnoException { + return error instanceof Error && "code" in error && error.code === code; +} diff --git a/src/nativeServices/serviceRendering.test.ts b/src/nativeServices/serviceRendering.test.ts index eea8868..617d407 100644 --- a/src/nativeServices/serviceRendering.test.ts +++ b/src/nativeServices/serviceRendering.test.ts @@ -34,12 +34,33 @@ describe("native service rendering", () => { expect(unit).toContain("Description=PI WEB UI dev server"); expect(unit).toContain("After=pi-web-sessiond.service\nWants=pi-web-sessiond.service"); - expect(unit).toContain('WorkingDirectory="/checkout with space"'); + expect(unit).toContain("WorkingDirectory=/checkout\\x20with\\x20space"); expect(unit).toContain('Environment="PI_WEB_CONFIG=/home/user/config with \\"quote\\".json"'); - expect(unit).toContain("ExecStart=/usr/bin/env /bin/zsh -lc 'exec /usr/bin/env bash -c '\\''trap"); + expect(unit).toContain('ExecStart=/usr/bin/env "/bin/zsh" -lc "exec /usr/bin/env bash -c \'trap \\"kill 0\\" EXIT;'); expect(unit).toContain("Restart=no"); }); + it("escapes systemd specifiers and line controls without changing directives", () => { + const plan = createDevelopmentNativeServicePlan({ + backend: { kind: "systemd", label: "systemd" }, + shell: { + name: "bash", + executable: "/shell $HOME/%h/bash", + source: "detected", + detectedExecutable: "/shell $HOME/%h/bash", + }, + environment: { PI_WEB_CONFIG: "/config/%h\nEnvironment=INJECTED=yes" }, + workingDirectory: "/checkout %h\nwith newline", + packageJsonPath: "/checkout/package.json", + }); + const unit = renderSystemdUnit(plan, planService(plan, 0)); + + expect(unit).toContain("WorkingDirectory=/checkout\\x20%%h\\nwith\\x20newline"); + expect(unit).toContain('Environment="PI_WEB_CONFIG=/config/%%h\\nEnvironment=INJECTED=yes"'); + expect(unit).toContain('ExecStart=/usr/bin/env "/shell $$HOME/%%h/bash"'); + expect(unit.match(/^Environment=/gmu)).toHaveLength(1); + }); + it("renders launchd entirely from the canonical plan", () => { const plan = developmentPlan("launchd"); const plist = renderLaunchdPlist(plan, planService(plan, 0), "/logs"); diff --git a/src/nativeServices/serviceRendering.ts b/src/nativeServices/serviceRendering.ts index 39e1d4c..a421f9c 100644 --- a/src/nativeServices/serviceRendering.ts +++ b/src/nativeServices/serviceRendering.ts @@ -3,7 +3,6 @@ import type { NativeServiceId, NativeServicePlan, NativeServicePlanService, - NativeServiceShellName, } from "./servicePlan.js"; export function renderSystemdUnit( @@ -14,7 +13,7 @@ export function renderSystemdUnit( assertBackend(plan, "systemd"); const workingDirectory = service.workingDirectory === null ? "" - : `WorkingDirectory=${systemdQuotedValue(service.workingDirectory)}\n`; + : `WorkingDirectory=${systemdPathValue(service.workingDirectory)}\n`; const restart = service.restart === "on-failure" ? "Restart=on-failure\nRestartSec=2\n" : "Restart=no\n"; @@ -22,7 +21,7 @@ export function renderSystemdUnit( Description=${service.description} ${systemdDependencyLine(plan, "After", service.after)}${systemdDependencyLine(plan, "Wants", service.wants)}[Service] Type=simple -${workingDirectory}${systemdEnvironmentLines(service.environment)}ExecStart=/usr/bin/env ${plan.shell.executable} -lc ${systemdServiceShellQuote(plan.shell.name, service.shellCommand)} +${workingDirectory}${systemdEnvironmentLines(service.environment)}ExecStart=/usr/bin/env ${systemdExecArgument(plan.shell.executable)} -lc ${systemdExecArgument(service.shellCommand)} ${restart} [Install] WantedBy=default.target @@ -83,20 +82,37 @@ function systemdDependencyLine( function systemdEnvironmentLines(environment: Readonly>): string { return Object.entries(environment) - .map(([key, value]) => `Environment="${systemdEscape(key)}=${systemdEscape(value)}"\n`) + .map(([key, value]) => `Environment=${systemdQuotedDirectiveValue(`${key}=${value}`)}\n`) .join(""); } -function systemdServiceShellQuote(shell: NativeServiceShellName, value: string): string { - return shellQuote(shell, value.replaceAll("%", "%%").replaceAll("$", "$$")); +function systemdExecArgument(value: string): string { + return `"${systemdEscape(value.replaceAll("%", "%%").replaceAll("$", () => "$$"), false)}"`; } -function systemdQuotedValue(value: string): string { - return `"${systemdEscape(value)}"`; +function systemdQuotedDirectiveValue(value: string): string { + return `"${systemdEscape(value.replaceAll("%", "%%"), false)}"`; } -function systemdEscape(value: string): string { - return value.replaceAll("\\", "\\\\").replaceAll('"', '\\"'); +function systemdPathValue(value: string): string { + return systemdEscape(value.replaceAll("%", "%%"), true); +} + +function systemdEscape(value: string, escapeSpaces: boolean): string { + let escaped = ""; + for (const character of value) { + const code = character.codePointAt(0) ?? 0; + if (character === "\\") escaped += "\\\\"; + else if (character === '"') escaped += escapeSpaces ? "\\x22" : '\\"'; + else if (character === "'" && escapeSpaces) escaped += "\\x27"; + else if (character === " " && escapeSpaces) escaped += "\\x20"; + else if (character === "\n") escaped += "\\n"; + else if (character === "\r") escaped += "\\r"; + else if (character === "\t") escaped += "\\t"; + else if (code < 0x20 || code === 0x7f) escaped += `\\x${code.toString(16).padStart(2, "0")}`; + else escaped += character; + } + return escaped; } function plistProgramArguments(arguments_: readonly string[]): string { @@ -113,12 +129,6 @@ function plistString(key: string, value: string, indent = " "): string { return `${indent}${xmlEscape(key)}\n${indent}${xmlEscape(value)}\n`; } -function shellQuote(shell: NativeServiceShellName, value: string): string { - return shell === "fish" - ? `'${value.replaceAll("\\", "\\\\").replaceAll("'", "\\'")}'` - : `'${value.replaceAll("'", "'\\''")}'`; -} - function xmlEscape(value: string): string { return value .replaceAll("&", "&")