From 49b1b084448be49708383e9bab8e32ea95b17442 Mon Sep 17 00:00:00 2001 From: leonaIee <63717587+leonaIee@users.noreply.github.com> Date: Fri, 1 May 2026 07:26:52 +0200 Subject: [PATCH] fix: drop stale service PATH entries --- src/commands/daemon-install-helpers.test.ts | 36 +++++++++++++++++ src/commands/daemon-install-helpers.ts | 8 ++++ src/daemon/service-audit.test.ts | 30 ++++++++++++++ src/daemon/service-audit.ts | 43 ++++++--------------- src/daemon/service-path-policy.ts | 39 +++++++++++++++++++ 5 files changed, 124 insertions(+), 32 deletions(-) create mode 100644 src/daemon/service-path-policy.ts diff --git a/src/commands/daemon-install-helpers.test.ts b/src/commands/daemon-install-helpers.test.ts index 104b32ddcbc..91fe277da96 100644 --- a/src/commands/daemon-install-helpers.test.ts +++ b/src/commands/daemon-install-helpers.test.ts @@ -583,6 +583,42 @@ describe("buildGatewayInstallPlan — dotenv merge", () => { expect(plan.environment.OPENCLAW_SERVICE_MARKER).toBeUndefined(); }); + it("drops stale non-minimal PATH entries from an existing service env", async () => { + mockNodeGatewayPlanFixture({ + serviceEnvironment: { + HOME: "/from-service", + OPENCLAW_PORT: "3000", + PATH: "/usr/local/bin:/usr/bin:/bin", + TMPDIR: "/tmp", + }, + }); + + const home = "/home/testuser"; + const plan = await buildGatewayInstallPlan({ + env: { HOME: tmpDir }, + port: 3000, + runtime: "node", + platform: "linux", + existingEnvironment: { + PATH: [ + `${home}/.volta/bin`, + `${home}/.asdf/shims`, + `${home}/.nvm/current/bin`, + `${home}/.local/share/fnm/aliases/default/bin`, + `${home}/.local/share/fnm/current/bin`, + `${home}/.fnm/aliases/default/bin`, + `${home}/.fnm/current/bin`, + `${home}/.local/share/pnpm`, + "/opt/pnpm/bin", + "/custom/go/bin", + "/usr/bin", + ].join(path.delimiter), + }, + }); + + expect(plan.environment.PATH).toBe("/usr/local/bin:/usr/bin:/bin:/custom/go/bin"); + }); + it("drops existing PATH entries that resolve through symlinks into temp dirs", async () => { mockNodeGatewayPlanFixture({ serviceEnvironment: { diff --git a/src/commands/daemon-install-helpers.ts b/src/commands/daemon-install-helpers.ts index 7e38938e73e..30436c5ab00 100644 --- a/src/commands/daemon-install-helpers.ts +++ b/src/commands/daemon-install-helpers.ts @@ -19,6 +19,7 @@ import { readManagedServiceEnvKeysFromEnvironment, writeManagedServiceEnvKeysToEnvironment, } from "../daemon/service-managed-env.js"; +import { isNonMinimalServicePathEntry } from "../daemon/service-path-policy.js"; import { isDangerousHostEnvOverrideVarName, isDangerousHostEnvVarName, @@ -173,6 +174,7 @@ function mergeServicePath( nextPath: string | undefined, existingPath: string | undefined, tmpDir: string | undefined, + platform: NodeJS.Platform, ): string | undefined { const segments: string[] = []; const seen = new Set(); @@ -233,6 +235,9 @@ function mergeServicePath( return normalized; }; const shouldPreserveNormalizedPathSegment = (segment: string) => { + if (isNonMinimalServicePathEntry(segment, platform)) { + return false; + } const resolved = path.resolve(segment); const realResolved = realpathExistingPath(resolved) ?? resolved; return ![...normalizedTmpDirs, ...realTmpDirs].some( @@ -319,6 +324,7 @@ async function buildGatewayInstallEnvironment(params: { warn?: DaemonInstallWarnFn; serviceEnvironment: Record; existingEnvironment?: Record; + platform: NodeJS.Platform; }): Promise> { const durableEnvironment = collectDurableServiceEnvVars({ env: params.env, @@ -353,6 +359,7 @@ async function buildGatewayInstallEnvironment(params: { params.serviceEnvironment.PATH, params.existingEnvironment?.PATH, params.serviceEnvironment.TMPDIR, + params.platform, ); if (mergedPath) { environment.PATH = mergedPath; @@ -427,6 +434,7 @@ export async function buildGatewayInstallPlan(params: { warn: params.warn, serviceEnvironment, existingEnvironment: params.existingEnvironment, + platform, }), }; } diff --git a/src/daemon/service-audit.test.ts b/src/daemon/service-audit.test.ts index a6c932c7ef6..871fbdf5209 100644 --- a/src/daemon/service-audit.test.ts +++ b/src/daemon/service-audit.test.ts @@ -167,6 +167,36 @@ describe("auditGatewayServiceConfig", () => { expect(issue?.message).toContain("/opt/pnpm"); }); + it("flags stale Linux version-manager and package-manager PATH entries", async () => { + const env = { HOME: "/tmp/openclaw-testuser-nonminimal" }; + const minimalPath = buildMinimalServicePath({ platform: "linux", env }); + const staleEntries = [ + `${env.HOME}/.volta/bin`, + `${env.HOME}/.asdf/shims`, + `${env.HOME}/.nvm/current/bin`, + `${env.HOME}/.local/share/fnm/current/bin`, + `${env.HOME}/.fnm/current/bin`, + `${env.HOME}/.local/share/pnpm`, + "/opt/pnpm/bin", + ]; + const audit = await auditGatewayServiceConfig({ + env, + platform: "linux", + command: { + programArguments: ["/usr/bin/node", "gateway"], + environment: { PATH: [minimalPath, ...staleEntries].join(":") }, + }, + }); + + const issue = audit.issues.find( + (entry) => entry.code === SERVICE_AUDIT_CODES.gatewayPathNonMinimal, + ); + expect(issue?.detail).toContain(`${env.HOME}/.volta/bin`); + expect(issue?.detail).toContain(`${env.HOME}/.local/share/fnm/current/bin`); + expect(issue?.detail).toContain(`${env.HOME}/.local/share/pnpm`); + expect(issue?.detail).toContain("/opt/pnpm/bin"); + }); + it("accepts Linux fnm aliases/default without requiring the legacy current symlink", async () => { const env = { HOME: "/tmp/openclaw-testuser", diff --git a/src/daemon/service-audit.ts b/src/daemon/service-audit.ts index a27086a8c85..6a24acec47a 100644 --- a/src/daemon/service-audit.ts +++ b/src/daemon/service-audit.ts @@ -1,10 +1,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { normalizeEnvVarKey } from "../infra/host-env-security.js"; -import { - normalizeLowercaseStringOrEmpty, - normalizeOptionalString, -} from "../shared/string-coerce.js"; +import { normalizeOptionalString } from "../shared/string-coerce.js"; import { resolveLaunchAgentPlistPath } from "./launchd.js"; import { isBunRuntime, isNodeRuntime } from "./runtime-binary.js"; import { @@ -19,6 +16,7 @@ import { hasInlineEnvironmentSource, isEnvironmentFileOnlySource, } from "./service-managed-env.js"; +import { isNonMinimalServicePathEntry, normalizeServicePathEntry } from "./service-path-policy.js"; import type { GatewayServiceEnvironmentValueSource } from "./service-types.js"; import { resolveSystemdUserUnitPath } from "./systemd.js"; @@ -385,15 +383,6 @@ function getPathModule(platform: NodeJS.Platform) { return platform === "win32" ? path.win32 : path.posix; } -function normalizePathEntry(entry: string, platform: NodeJS.Platform): string { - const pathModule = getPathModule(platform); - const normalized = pathModule.normalize(entry).replaceAll("\\", "/"); - if (platform === "win32") { - return normalizeLowercaseStringOrEmpty(normalized); - } - return normalized; -} - function getEquivalentMinimalPathEntries( entry: string, platform: NodeJS.Platform, @@ -410,7 +399,7 @@ function getEquivalentMinimalPathEntries( if (!equivalent) { return []; } - const normalizedEquivalent = normalizePathEntry(equivalent, platform); + const normalizedEquivalent = normalizeServicePathEntry(equivalent, platform); return normalizedExpected.has(normalizedEquivalent) ? [equivalent] : []; } @@ -442,15 +431,17 @@ function auditGatewayServicePath( .split(getPathModule(platform).delimiter) .map((entry) => entry.trim()) .filter(Boolean); - const normalizedParts = new Set(parts.map((entry) => normalizePathEntry(entry, platform))); - const normalizedExpected = new Set(expected.map((entry) => normalizePathEntry(entry, platform))); + const normalizedParts = new Set(parts.map((entry) => normalizeServicePathEntry(entry, platform))); + const normalizedExpected = new Set( + expected.map((entry) => normalizeServicePathEntry(entry, platform)), + ); const missing = expected.filter((entry) => { - const normalized = normalizePathEntry(entry, platform); + const normalized = normalizeServicePathEntry(entry, platform); if (normalizedParts.has(normalized)) { return false; } return !getEquivalentMinimalPathEntries(entry, platform, normalizedExpected).some( - (equivalent) => normalizedParts.has(normalizePathEntry(equivalent, platform)), + (equivalent) => normalizedParts.has(normalizeServicePathEntry(equivalent, platform)), ); }); if (missing.length > 0) { @@ -462,23 +453,11 @@ function auditGatewayServicePath( } const nonMinimal = parts.filter((entry) => { - const normalized = normalizePathEntry(entry, platform); + const normalized = normalizeServicePathEntry(entry, platform); if (normalizedExpected.has(normalized)) { return false; } - return ( - normalized.includes("/.nvm/") || - normalized.includes("/.fnm/") || - normalized.includes("/.volta/") || - normalized.includes("/.asdf/") || - normalized.includes("/.n/") || - normalized.includes("/.nodenv/") || - normalized.includes("/.nodebrew/") || - normalized.includes("/nvs/") || - normalized.includes("/.local/share/pnpm/") || - normalized.includes("/pnpm/") || - normalized.endsWith("/pnpm") - ); + return isNonMinimalServicePathEntry(normalized, platform); }); if (nonMinimal.length > 0) { issues.push({ diff --git a/src/daemon/service-path-policy.ts b/src/daemon/service-path-policy.ts new file mode 100644 index 00000000000..d808de78daa --- /dev/null +++ b/src/daemon/service-path-policy.ts @@ -0,0 +1,39 @@ +import path from "node:path"; + +function getPathModule(platform: NodeJS.Platform) { + return platform === "win32" ? path.win32 : path.posix; +} + +function normalizeLowercaseStringOrEmpty(value: string | undefined): string { + return value?.trim().toLowerCase() ?? ""; +} + +export function normalizeServicePathEntry(entry: string, platform: NodeJS.Platform): string { + const pathModule = getPathModule(platform); + const normalized = pathModule.normalize(entry).replaceAll("\\", "/"); + if (platform === "win32") { + return normalizeLowercaseStringOrEmpty(normalized); + } + return normalized; +} + +export function isNonMinimalServicePathEntry(entry: string, platform: NodeJS.Platform): boolean { + if (platform === "win32") { + return false; + } + const normalized = normalizeServicePathEntry(entry, platform); + return ( + normalized.includes("/.nvm/") || + normalized.includes("/.fnm/") || + normalized.includes("/.local/share/fnm/") || + normalized.includes("/.volta/") || + normalized.includes("/.asdf/") || + normalized.includes("/.n/") || + normalized.includes("/.nodenv/") || + normalized.includes("/.nodebrew/") || + normalized.includes("/nvs/") || + normalized.includes("/.local/share/pnpm/") || + normalized.includes("/pnpm/") || + normalized.endsWith("/pnpm") + ); +}