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, }), }; }