mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-12 09:41:11 +00:00
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 <hi@obviy.us>
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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<string, string>;
|
||||
}) => ({
|
||||
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<string, string>;
|
||||
}) => 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();
|
||||
|
||||
@@ -124,6 +124,7 @@ export async function runDaemonInstall(opts: DaemonInstallOptions) {
|
||||
env: installEnv,
|
||||
port,
|
||||
runtime: runtimeRaw,
|
||||
existingEnvironment: existingServiceEnv,
|
||||
warn: (message) => {
|
||||
if (json) {
|
||||
warnings.push(message);
|
||||
|
||||
@@ -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" } });
|
||||
|
||||
|
||||
@@ -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<string, string | undefined>;
|
||||
};
|
||||
|
||||
const MANAGED_SERVICE_ENV_KEYS_VAR = "OPENCLAW_SERVICE_MANAGED_ENV_KEYS";
|
||||
|
||||
function collectAuthProfileServiceEnvVars(params: {
|
||||
env: Record<string, string | undefined>;
|
||||
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<string>();
|
||||
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<string, string | undefined> | undefined,
|
||||
): Set<string> {
|
||||
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, string | undefined>,
|
||||
): 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<string, string | undefined> | undefined,
|
||||
managedServiceEnvKeys: Set<string>,
|
||||
): Record<string, string | undefined> {
|
||||
if (!existingEnvironment) {
|
||||
return {};
|
||||
}
|
||||
const preserved: Record<string, string | undefined> = {};
|
||||
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<string, string | undefined>;
|
||||
config?: OpenClawConfig;
|
||||
authStore?: AuthProfileStore;
|
||||
warn?: DaemonInstallWarnFn;
|
||||
serviceEnvironment: Record<string, string | undefined>;
|
||||
existingEnvironment?: Record<string, string | undefined>;
|
||||
}): Record<string, string | undefined> {
|
||||
const environment: Record<string, string | undefined> = {
|
||||
const managedEnvironment: Record<string, string | undefined> = {
|
||||
...collectDurableServiceEnvVars({
|
||||
env: params.env,
|
||||
config: params.config,
|
||||
@@ -86,7 +202,26 @@ function buildGatewayInstallEnvironment(params: {
|
||||
warn: params.warn,
|
||||
}),
|
||||
};
|
||||
const environment: Record<string, string | undefined> = {
|
||||
...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<string, string | undefined>;
|
||||
port: number;
|
||||
runtime: GatewayDaemonRuntime;
|
||||
existingEnvironment?: Record<string, string | undefined>;
|
||||
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,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user