From 40c5edb5b1ca4ffe01583bc29dc79d3dc9ebf2dc Mon Sep 17 00:00:00 2001 From: WarrenJones Date: Thu, 9 Apr 2026 16:59:54 +0800 Subject: [PATCH] fix: preserve safe gateway env vars on reinstall (#63136) (thanks @WarrenJones) * fix(daemon): preserve safe env vars on gateway reinstall Pass the existing service environment into gateway reinstall planning so safe custom variables survive LaunchAgent rewrites and existing PATH entries are merged instead of being silently dropped. Made-with: Cursor * fix(daemon): track managed env keys on reinstall * fix: preserve safe gateway env vars on reinstall (#63136) (thanks @WarrenJones) * fix: validate preserved PATH entries on reinstall (#63136) (thanks @WarrenJones) --------- Co-authored-by: WarrenJones <8704779+WarrenJones@users.noreply.github.com> Co-authored-by: Ayaan Zaidi --- CHANGELOG.md | 1 + src/cli/daemon-cli.coverage.test.ts | 41 +++++- src/cli/daemon-cli/install.ts | 1 + src/commands/daemon-install-helpers.test.ts | 83 +++++++++++ src/commands/daemon-install-helpers.ts | 147 ++++++++++++++++++-- 5 files changed, 262 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 978c18fa710..26615086923 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai - Config/plugins: let config writes keep disabled plugin entries without forcing required plugin config schemas or crashing raw plugin validation, so slot switches and similar plugin-state updates persist cleanly. (#63296) Thanks @fuller-stack-dev. - WhatsApp/outbound queue: drain queued WhatsApp deliveries when the listener reconnects without dropping reconnect-delayed sends after a special TTL or rewriting retry history, so disconnect-window outbound messages can recover once the channel is ready again. (#46299) Thanks @manuel-claw. - Tools/web_fetch: add an opt-in `tools.web.fetch.ssrfPolicy.allowRfc2544BenchmarkRange` config so fake-IP proxy environments that resolve public sites into `198.18.0.0/15` can use `web_fetch` without weakening the default SSRF block. (#61830) Thanks @xing-xing-coder. +- Daemon/gateway install: preserve safe custom service env vars on forced reinstall, merge prior custom PATH segments behind the managed service PATH, and stop removed managed env keys from persisting as custom carryover. (#63136) Thanks @WarrenJones. - Config validation: surface the actual offending field for strict-schema union failures in bindings, including top-level unexpected keys on the matching ACP branch. (#40841) Thanks @Hollychou924. - QQBot/security: replace raw `fetch()` in the image-size probe with SSRF-guarded `fetchRemoteMedia`, fix `resolveRepoRoot()` to walk up to `.git` instead of hardcoding two parent levels, and refresh the raw-fetch allowlist to match the corrected scan. (#63495) Thanks @dims. diff --git a/src/cli/daemon-cli.coverage.test.ts b/src/cli/daemon-cli.coverage.test.ts index 8b8c18c1f1b..cdfbb1377ab 100644 --- a/src/cli/daemon-cli.coverage.test.ts +++ b/src/cli/daemon-cli.coverage.test.ts @@ -26,7 +26,12 @@ const inspectPortUsage = vi.fn(async (port: number) => ({ hints: [], })); const buildGatewayInstallPlan = vi.fn( - async (params: { port: number; token?: string; env?: NodeJS.ProcessEnv }) => ({ + async (params: { + port: number; + token?: string; + env?: NodeJS.ProcessEnv; + existingEnvironment?: Record; + }) => ({ programArguments: ["/bin/node", "cli", "gateway", "--port", String(params.port)], workingDirectory: process.cwd(), environment: { @@ -113,8 +118,12 @@ vi.mock("../runtime.js", async () => ({ })); vi.mock("../commands/daemon-install-helpers.js", () => ({ - buildGatewayInstallPlan: (params: { port: number; token?: string; env?: NodeJS.ProcessEnv }) => - buildGatewayInstallPlan(params), + buildGatewayInstallPlan: (params: { + port: number; + token?: string; + env?: NodeJS.ProcessEnv; + existingEnvironment?: Record; + }) => buildGatewayInstallPlan(params), })); vi.mock("./deps.js", () => ({ @@ -257,6 +266,32 @@ describe("daemon-cli coverage", () => { expect(parsed.result).toBe("installed"); }); + it("passes the existing service environment into the install plan on forced reinstall", async () => { + runtimeLogs.length = 0; + serviceIsLoaded.mockResolvedValueOnce(true); + serviceReadCommand.mockResolvedValueOnce({ + programArguments: ["/bin/node", "cli", "gateway", "--port", "18789"], + environment: { + PATH: "/custom/go/bin:/usr/bin", + GOPATH: "/Users/test/.local/gopath", + GOBIN: "/Users/test/.local/gopath/bin", + }, + sourcePath: "/tmp/ai.openclaw.gateway.plist", + }); + + await runDaemonCommand(["daemon", "install", "--force", "--json"]); + + expect(buildGatewayInstallPlan).toHaveBeenCalledWith( + expect.objectContaining({ + existingEnvironment: { + PATH: "/custom/go/bin:/usr/bin", + GOPATH: "/Users/test/.local/gopath", + GOBIN: "/Users/test/.local/gopath/bin", + }, + }), + ); + }); + it("starts and stops daemon (json output)", async () => { runtimeLogs.length = 0; serviceRestart.mockClear(); diff --git a/src/cli/daemon-cli/install.ts b/src/cli/daemon-cli/install.ts index e494315fe7f..756df7a26ec 100644 --- a/src/cli/daemon-cli/install.ts +++ b/src/cli/daemon-cli/install.ts @@ -124,6 +124,7 @@ export async function runDaemonInstall(opts: DaemonInstallOptions) { env: installEnv, port, runtime: runtimeRaw, + existingEnvironment: existingServiceEnv, warn: (message) => { if (json) { warnings.push(message); diff --git a/src/commands/daemon-install-helpers.test.ts b/src/commands/daemon-install-helpers.test.ts index 61c5e2f6e50..e4dac3042bd 100644 --- a/src/commands/daemon-install-helpers.test.ts +++ b/src/commands/daemon-install-helpers.test.ts @@ -182,6 +182,7 @@ describe("buildGatewayInstallPlan", () => { // Config env vars should be present expect(plan.environment.GOOGLE_API_KEY).toBe("test-key"); expect(plan.environment.CUSTOM_VAR).toBe("custom-value"); + expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBe("CUSTOM_VAR,GOOGLE_API_KEY"); // Service environment vars should take precedence expect(plan.environment.OPENCLAW_PORT).toBe("3000"); expect(plan.environment.HOME).toBe("/Users/me"); @@ -483,6 +484,88 @@ describe("buildGatewayInstallPlan — dotenv merge", () => { expect(plan.environment.HOME).toBe("/from-service"); }); + it("preserves safe custom vars from an existing service env and merges PATH", async () => { + mockNodeGatewayPlanFixture({ + serviceEnvironment: { + HOME: "/from-service", + OPENCLAW_PORT: "3000", + PATH: "/managed/bin:/usr/bin", + }, + }); + + const plan = await buildGatewayInstallPlan({ + env: { HOME: tmpDir }, + port: 3000, + runtime: "node", + existingEnvironment: { + PATH: "/custom/go/bin:/usr/bin", + GOBIN: "/Users/test/.local/gopath/bin", + BLOGWATCHER_HOME: "/Users/test/.blogwatcher", + NODE_OPTIONS: "--require /tmp/evil.js", + GOPATH: "/Users/test/.local/gopath", + OPENCLAW_SERVICE_MARKER: "openclaw", + }, + }); + + expect(plan.environment.PATH).toBe("/managed/bin:/usr/bin:/custom/go/bin"); + expect(plan.environment.GOBIN).toBe("/Users/test/.local/gopath/bin"); + expect(plan.environment.BLOGWATCHER_HOME).toBe("/Users/test/.blogwatcher"); + expect(plan.environment.NODE_OPTIONS).toBeUndefined(); + expect(plan.environment.GOPATH).toBeUndefined(); + expect(plan.environment.OPENCLAW_SERVICE_MARKER).toBeUndefined(); + }); + + it("drops non-absolute and temp PATH entries from an existing service env", async () => { + mockNodeGatewayPlanFixture({ + serviceEnvironment: { + HOME: "/from-service", + OPENCLAW_PORT: "3000", + PATH: "/managed/bin:/usr/bin", + TMPDIR: "/tmp", + }, + }); + + const plan = await buildGatewayInstallPlan({ + env: { HOME: tmpDir }, + port: 3000, + runtime: "node", + existingEnvironment: { + PATH: ".:/tmp/evil:/custom/go/bin:/usr/bin", + }, + }); + + expect(plan.environment.PATH).toBe("/managed/bin:/usr/bin:/custom/go/bin"); + }); + + it("drops keys that were previously tracked as managed service env", async () => { + mockNodeGatewayPlanFixture({ + serviceEnvironment: { + HOME: "/from-service", + OPENCLAW_PORT: "3000", + PATH: "/managed/bin:/usr/bin", + }, + }); + + const plan = await buildGatewayInstallPlan({ + env: { HOME: tmpDir }, + port: 3000, + runtime: "node", + existingEnvironment: { + PATH: "/custom/go/bin:/usr/bin", + GOBIN: "/Users/test/.local/gopath/bin", + BLOGWATCHER_HOME: "/Users/test/.blogwatcher", + GOPATH: "/Users/test/.local/gopath", + OPENCLAW_SERVICE_MANAGED_ENV_KEYS: "GOBIN,GOPATH", + }, + }); + + expect(plan.environment.PATH).toBe("/managed/bin:/usr/bin:/custom/go/bin"); + expect(plan.environment.GOBIN).toBeUndefined(); + expect(plan.environment.BLOGWATCHER_HOME).toBe("/Users/test/.blogwatcher"); + expect(plan.environment.GOPATH).toBeUndefined(); + expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBeUndefined(); + }); + it("works when .env file does not exist", async () => { mockNodeGatewayPlanFixture({ serviceEnvironment: { OPENCLAW_PORT: "3000" } }); diff --git a/src/commands/daemon-install-helpers.ts b/src/commands/daemon-install-helpers.ts index ac20612d8eb..01a4a8bb4dc 100644 --- a/src/commands/daemon-install-helpers.ts +++ b/src/commands/daemon-install-helpers.ts @@ -1,3 +1,5 @@ +import os from "node:os"; +import path from "node:path"; import { loadAuthProfileStoreForSecretsRuntime, type AuthProfileStore, @@ -29,6 +31,8 @@ export type GatewayInstallPlan = { environment: Record; }; +const MANAGED_SERVICE_ENV_KEYS_VAR = "OPENCLAW_SERVICE_MANAGED_ENV_KEYS"; + function collectAuthProfileServiceEnvVars(params: { env: Record; authStore?: AuthProfileStore; @@ -68,14 +72,126 @@ function collectAuthProfileServiceEnvVars(params: { return entries; } +function mergeServicePath( + nextPath: string | undefined, + existingPath: string | undefined, + tmpDir: string | undefined, +): string | undefined { + const segments: string[] = []; + const seen = new Set(); + const normalizedTmpDirs = [tmpDir, os.tmpdir()] + .map((value) => value?.trim()) + .filter((value): value is string => Boolean(value)) + .map((value) => path.resolve(value)); + const shouldPreservePathSegment = (segment: string) => { + if (!path.isAbsolute(segment)) { + return false; + } + const resolved = path.resolve(segment); + return !normalizedTmpDirs.some( + (tmpRoot) => resolved === tmpRoot || resolved.startsWith(`${tmpRoot}${path.sep}`), + ); + }; + const addPath = (value: string | undefined, options?: { preserve?: boolean }) => { + if (typeof value !== "string" || value.trim().length === 0) { + return; + } + for (const segment of value.split(path.delimiter)) { + const trimmed = segment.trim(); + if (options?.preserve && !shouldPreservePathSegment(trimmed)) { + continue; + } + if (!trimmed || seen.has(trimmed)) { + continue; + } + seen.add(trimmed); + segments.push(trimmed); + } + }; + addPath(nextPath); + addPath(existingPath, { preserve: true }); + return segments.length > 0 ? segments.join(path.delimiter) : undefined; +} + +function readManagedServiceEnvKeys( + existingEnvironment: Record | undefined, +): Set { + if (!existingEnvironment) { + return new Set(); + } + for (const [rawKey, rawValue] of Object.entries(existingEnvironment)) { + const key = normalizeEnvVarKey(rawKey, { portable: true }); + if (!key || key.toUpperCase() !== MANAGED_SERVICE_ENV_KEYS_VAR) { + continue; + } + return new Set( + rawValue?.split(",").flatMap((value) => { + const normalized = normalizeEnvVarKey(value, { portable: true }); + return normalized ? [normalized.toUpperCase()] : []; + }) ?? [], + ); + } + return new Set(); +} + +function formatManagedServiceEnvKeys( + managedEnvironment: Record, +): string | undefined { + const keys = Object.keys(managedEnvironment) + .flatMap((key) => { + const normalized = normalizeEnvVarKey(key, { portable: true }); + return normalized ? [normalized.toUpperCase()] : []; + }) + .toSorted(); + return keys.length > 0 ? keys.join(",") : undefined; +} + +function collectPreservedExistingServiceEnvVars( + existingEnvironment: Record | undefined, + managedServiceEnvKeys: Set, +): Record { + if (!existingEnvironment) { + return {}; + } + const preserved: Record = {}; + for (const [rawKey, rawValue] of Object.entries(existingEnvironment)) { + const key = normalizeEnvVarKey(rawKey, { portable: true }); + if (!key) { + continue; + } + const upper = key.toUpperCase(); + if ( + upper === "HOME" || + upper === "PATH" || + upper === "TMPDIR" || + upper.startsWith("OPENCLAW_") + ) { + continue; + } + if (managedServiceEnvKeys.has(upper)) { + continue; + } + if (isDangerousHostEnvVarName(key) || isDangerousHostEnvOverrideVarName(key)) { + continue; + } + const value = rawValue?.trim(); + if (!value) { + continue; + } + preserved[key] = value; + } + return preserved; +} + function buildGatewayInstallEnvironment(params: { env: Record; config?: OpenClawConfig; authStore?: AuthProfileStore; warn?: DaemonInstallWarnFn; serviceEnvironment: Record; + existingEnvironment?: Record; }): Record { - const environment: Record = { + const managedEnvironment: Record = { ...collectDurableServiceEnvVars({ env: params.env, config: params.config, @@ -86,7 +202,26 @@ function buildGatewayInstallEnvironment(params: { warn: params.warn, }), }; + const environment: Record = { + ...collectPreservedExistingServiceEnvVars( + params.existingEnvironment, + readManagedServiceEnvKeys(params.existingEnvironment), + ), + ...managedEnvironment, + }; Object.assign(environment, params.serviceEnvironment); + const mergedPath = mergeServicePath( + params.serviceEnvironment.PATH, + params.existingEnvironment?.PATH, + params.serviceEnvironment.TMPDIR, + ); + if (mergedPath) { + environment.PATH = mergedPath; + } + const managedServiceEnvKeys = formatManagedServiceEnvKeys(managedEnvironment); + if (managedServiceEnvKeys) { + environment[MANAGED_SERVICE_ENV_KEYS_VAR] = managedServiceEnvKeys; + } return environment; } @@ -94,6 +229,7 @@ export async function buildGatewayInstallPlan(params: { env: Record; port: number; runtime: GatewayDaemonRuntime; + existingEnvironment?: Record; devMode?: boolean; nodePath?: string; warn?: DaemonInstallWarnFn; @@ -127,16 +263,10 @@ export async function buildGatewayInstallPlan(params: { process.platform === "darwin" ? resolveGatewayLaunchAgentLabel(params.env.OPENCLAW_PROFILE) : undefined, - // Keep npm/pnpm available to the service when the selected daemon node comes from - // a version-manager bin directory that isn't covered by static PATH guesses. extraPathDirs: resolveDaemonNodeBinDir(nodePath), }); - // Merge env sources into the service environment in ascending priority: - // 1. ~/.openclaw/.env file vars (lowest — user secrets / fallback keys) - // 2. Config env vars (openclaw.json env.vars + inline keys) - // 3. Auth-profile env refs (credential store → env var lookups) - // 4. Service environment (HOME, PATH, OPENCLAW_* — highest) + // Lowest to highest: preserved custom vars, durable config, auth env refs, generated service env. return { programArguments, workingDirectory, @@ -146,6 +276,7 @@ export async function buildGatewayInstallPlan(params: { authStore: params.authStore, warn: params.warn, serviceEnvironment, + existingEnvironment: params.existingEnvironment, }), }; }