diff --git a/docs/gateway/doctor.md b/docs/gateway/doctor.md index beb4fb95cc9..1256159d657 100644 --- a/docs/gateway/doctor.md +++ b/docs/gateway/doctor.md @@ -102,6 +102,7 @@ cat ~/.openclaw/openclaw.json - Gateway runtime checks (service installed but not running; cached launchd label). - Channel status warnings (probed from the running gateway). - Supervisor config audit (launchd/systemd/schtasks) with optional repair. + - Embedded proxy environment cleanup for gateway services that captured shell `HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY` values during install or update. - Gateway runtime best-practice checks (Node vs Bun, version-manager paths). - Gateway port collision diagnostics (default `18789`). diff --git a/scripts/e2e/doctor-install-switch-docker.sh b/scripts/e2e/doctor-install-switch-docker.sh index 7f6fa2b1795..933101f30ce 100755 --- a/scripts/e2e/doctor-install-switch-docker.sh +++ b/scripts/e2e/doctor-install-switch-docker.sh @@ -251,6 +251,44 @@ NODE "$npm_bin doctor --repair --force --yes" \ "$npm_entry" + run_proxy_env_flow() { + local name="proxy-env-cleanup" + local install_log="/tmp/openclaw-doctor-switch-${name}-install.log" + local doctor_log="/tmp/openclaw-doctor-switch-${name}-doctor.log" + local command_timeout="${OPENCLAW_DOCKER_DOCTOR_SWITCH_COMMAND_TIMEOUT:-300s}" + + echo "== Flow: $name ==" + home_dir=$(mktemp -d "/tmp/openclaw-switch-${name}.XXXXXX") + export HOME="$home_dir" + export USER="testuser" + + unit_path="$HOME/.config/systemd/user/openclaw-gateway.service" + if ! timeout "$command_timeout" env \ + HTTP_PROXY="http://proxy.local:7890" \ + HTTPS_PROXY="https://proxy.local:7890" \ + NO_PROXY="localhost,127.0.0.1" \ + "$npm_bin" gateway install --force >"$install_log" 2>&1; then + cat "$install_log" + exit 1 + fi + assert_no_env_key "$unit_path" "HTTP_PROXY" + assert_no_env_key "$unit_path" "HTTPS_PROXY" + assert_no_env_key "$unit_path" "NO_PROXY" + + { + printf "%s\n" "Environment=HTTP_PROXY=http://stale-proxy.local:7890" + printf "%s\n" "Environment=HTTPS_PROXY=https://stale-proxy.local:7890" + } >>"$unit_path" + if ! timeout "$command_timeout" node "$git_cli" doctor --repair --yes >"$doctor_log" 2>&1; then + cat "$doctor_log" + exit 1 + fi + assert_no_env_key "$unit_path" "HTTP_PROXY" + assert_no_env_key "$unit_path" "HTTPS_PROXY" + } + + run_proxy_env_flow + run_wrapper_flow() { local name="wrapper-persistence" local install_log="/tmp/openclaw-doctor-switch-${name}-install.log" diff --git a/src/commands/doctor-gateway-services.test.ts b/src/commands/doctor-gateway-services.test.ts index fd925eb4182..75d0c658215 100644 --- a/src/commands/doctor-gateway-services.test.ts +++ b/src/commands/doctor-gateway-services.test.ts @@ -69,6 +69,7 @@ vi.mock("../daemon/service-audit.js", () => ({ SERVICE_AUDIT_CODES: { gatewayEntrypointMismatch: testServiceAuditCodes.gatewayEntrypointMismatch, gatewayManagedEnvEmbedded: testServiceAuditCodes.gatewayManagedEnvEmbedded, + gatewayProxyEnvEmbedded: testServiceAuditCodes.gatewayProxyEnvEmbedded, gatewayTokenMismatch: testServiceAuditCodes.gatewayTokenMismatch, }, })); @@ -321,6 +322,44 @@ describe("maybeRepairGatewayServiceConfig", () => { expect(mocks.install).toHaveBeenCalledTimes(1); }); + it("repairs gateway services with embedded proxy environment values", async () => { + mocks.readCommand.mockResolvedValue({ + programArguments: gatewayProgramArguments, + environment: { + HTTP_PROXY: "http://proxy.local:7890", + HTTPS_PROXY: "https://proxy.local:7890", + }, + }); + mocks.buildGatewayInstallPlan.mockResolvedValue({ + programArguments: gatewayProgramArguments, + workingDirectory: "/tmp", + environment: {}, + }); + mocks.auditGatewayServiceConfig.mockResolvedValue({ + ok: false, + issues: [ + { + code: "gateway-proxy-env-embedded", + message: "Gateway service embeds proxy environment values that should not be persisted.", + detail: "inline keys: HTTP_PROXY, HTTPS_PROXY", + level: "recommended", + }, + ], + }); + mocks.install.mockResolvedValue(undefined); + + await runRepair({ gateway: {} }); + + expect(mocks.install).toHaveBeenCalledWith( + expect.objectContaining({ + environment: expect.not.objectContaining({ + HTTP_PROXY: expect.any(String), + HTTPS_PROXY: expect.any(String), + }), + }), + ); + }); + it("uses OPENCLAW_GATEWAY_TOKEN when config token is missing", async () => { await withEnvAsync({ OPENCLAW_GATEWAY_TOKEN: "env-token" }, async () => { setupGatewayTokenRepairScenario(); diff --git a/src/commands/doctor-service-audit.test-helpers.ts b/src/commands/doctor-service-audit.test-helpers.ts index 92275777d57..7a017348b48 100644 --- a/src/commands/doctor-service-audit.test-helpers.ts +++ b/src/commands/doctor-service-audit.test-helpers.ts @@ -5,6 +5,7 @@ import { normalizeOptionalString } from "../shared/string-coerce.js"; export const testServiceAuditCodes = { gatewayEntrypointMismatch: "gateway-entrypoint-mismatch", gatewayManagedEnvEmbedded: "gateway-managed-env-embedded", + gatewayProxyEnvEmbedded: "gateway-proxy-env-embedded", gatewayTokenMismatch: "gateway-token-mismatch", } as const; diff --git a/src/daemon/service-audit.test.ts b/src/daemon/service-audit.test.ts index 8978101d534..55741e3aad3 100644 --- a/src/daemon/service-audit.test.ts +++ b/src/daemon/service-audit.test.ts @@ -256,6 +256,62 @@ describe("auditGatewayServiceConfig", () => { expect(hasIssue(audit, SERVICE_AUDIT_CODES.gatewayManagedEnvEmbedded)).toBe(true); }); + + it("flags inline proxy environment values embedded in the service", async () => { + const audit = await createGatewayAudit({ + extraEnvironment: { + HTTP_PROXY: "http://proxy.local:7890", + HTTPS_PROXY: "https://proxy.local:7890", + NO_PROXY: "localhost,127.0.0.1", + }, + }); + + const issue = audit.issues.find( + (entry) => entry.code === SERVICE_AUDIT_CODES.gatewayProxyEnvEmbedded, + ); + expect(issue?.detail).toContain("HTTP_PROXY"); + expect(issue?.detail).toContain("HTTPS_PROXY"); + expect(issue?.detail).toContain("NO_PROXY"); + }); + + it("flags lowercase inline proxy environment values using portable key names", async () => { + const audit = await createGatewayAudit({ + extraEnvironment: { + https_proxy: "https://proxy.local:7890", + }, + }); + + const issue = audit.issues.find( + (entry) => entry.code === SERVICE_AUDIT_CODES.gatewayProxyEnvEmbedded, + ); + expect(issue?.detail).toContain("HTTPS_PROXY"); + }); + + it("does not flag proxy values loaded only from EnvironmentFile", async () => { + const audit = await createGatewayAudit({ + extraEnvironment: { + HTTP_PROXY: "http://proxy.local:7890", + }, + environmentValueSources: { + HTTP_PROXY: "file", + }, + }); + + expect(hasIssue(audit, SERVICE_AUDIT_CODES.gatewayProxyEnvEmbedded)).toBe(false); + }); + + it("flags proxy values present inline even when an EnvironmentFile overrides them", async () => { + const audit = await createGatewayAudit({ + extraEnvironment: { + HTTP_PROXY: "http://proxy.local:7890", + }, + environmentValueSources: { + HTTP_PROXY: "inline-and-file", + }, + }); + + expect(hasIssue(audit, SERVICE_AUDIT_CODES.gatewayProxyEnvEmbedded)).toBe(true); + }); }); describe("checkTokenDrift", () => { diff --git a/src/daemon/service-audit.ts b/src/daemon/service-audit.ts index d4987a432c3..80ec6ba09ad 100644 --- a/src/daemon/service-audit.ts +++ b/src/daemon/service-audit.ts @@ -1,5 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { normalizeEnvVarKey } from "../infra/host-env-security.js"; import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, @@ -12,8 +13,10 @@ import { resolveSystemNodePath, } from "./runtime-paths.js"; import { getMinimalServicePathPartsFromEnv } from "./service-env.js"; +import { SERVICE_PROXY_ENV_KEYS } from "./service-env.js"; import { collectInlineManagedServiceEnvKeys, + hasInlineEnvironmentSource, isEnvironmentFileOnlySource, } from "./service-managed-env.js"; import type { GatewayServiceEnvironmentValueSource } from "./service-types.js"; @@ -47,6 +50,7 @@ export const SERVICE_AUDIT_CODES = { gatewayPathNonMinimal: "gateway-path-nonminimal", gatewayTokenEmbedded: "gateway-token-embedded", gatewayManagedEnvEmbedded: "gateway-managed-env-embedded", + gatewayProxyEnvEmbedded: "gateway-proxy-env-embedded", gatewayTokenMismatch: "gateway-token-mismatch", gatewayRuntimeBun: "gateway-runtime-bun", gatewayRuntimeNodeVersionManager: "gateway-runtime-node-version-manager", @@ -260,6 +264,66 @@ function auditManagedServiceEnvironment( }); } +function normalizeServiceEnvKey(key: string): string | null { + return normalizeEnvVarKey(key, { portable: true })?.toUpperCase() ?? null; +} + +function readEnvironmentValueSource( + command: GatewayServiceCommand, + normalizedKey: string, +): GatewayServiceEnvironmentValueSource | undefined { + for (const [rawKey, source] of Object.entries(command?.environmentValueSources ?? {})) { + if (normalizeServiceEnvKey(rawKey) === normalizedKey) { + return source; + } + } + return undefined; +} + +const SERVICE_PROXY_ENV_KEY_SET = new Set( + SERVICE_PROXY_ENV_KEYS.flatMap((key) => { + const normalized = normalizeServiceEnvKey(key); + return normalized ? [normalized] : []; + }), +); + +function collectInlineProxyEnvKeys(command: GatewayServiceCommand): string[] { + if (!command?.environment) { + return []; + } + const inlineKeys: string[] = []; + for (const [rawKey, value] of Object.entries(command.environment)) { + if (typeof value !== "string" || !value.trim()) { + continue; + } + const normalized = normalizeServiceEnvKey(rawKey); + if (!normalized || !SERVICE_PROXY_ENV_KEY_SET.has(normalized)) { + continue; + } + if (!hasInlineEnvironmentSource(readEnvironmentValueSource(command, normalized))) { + continue; + } + inlineKeys.push(normalized); + } + return [...new Set(inlineKeys)].toSorted(); +} + +function auditProxyServiceEnvironment( + command: GatewayServiceCommand, + issues: ServiceConfigIssue[], +) { + const inlineKeys = collectInlineProxyEnvKeys(command); + if (inlineKeys.length === 0) { + return; + } + issues.push({ + code: SERVICE_AUDIT_CODES.gatewayProxyEnvEmbedded, + message: "Gateway service embeds proxy environment values that should not be persisted.", + detail: `inline keys: ${inlineKeys.join(", ")}`, + level: "recommended", + }); +} + export function readEmbeddedGatewayToken(command: GatewayServiceCommand): string | undefined { if (!command) { return undefined; @@ -463,6 +527,7 @@ export async function auditGatewayServiceConfig(params: { auditGatewayCommand(params.command?.programArguments, issues); auditManagedServiceEnvironment(params.command, issues, params.expectedManagedServiceEnvKeys); + auditProxyServiceEnvironment(params.command, issues); auditGatewayToken(params.command, issues, params.expectedGatewayToken); auditGatewayServicePath(params.command, issues, params.env, platform); await auditGatewayRuntime(params.env, params.command, issues, platform); diff --git a/src/daemon/service-env.test.ts b/src/daemon/service-env.test.ts index fbd57862f38..ffeaa73b09e 100644 --- a/src/daemon/service-env.test.ts +++ b/src/daemon/service-env.test.ts @@ -449,7 +449,7 @@ describe("buildServiceEnvironment", () => { } }); - it("forwards proxy environment variables for launchd/systemd runtime", () => { + it("does not persist ambient proxy environment variables for launchd/systemd runtime", () => { const env = buildServiceEnvironment({ env: { HOME: "/home/user", @@ -462,11 +462,11 @@ describe("buildServiceEnvironment", () => { port: 18789, }); - expect(env.HTTP_PROXY).toBe("http://proxy.local:7890"); - expect(env.HTTPS_PROXY).toBe("https://proxy.local:7890"); - expect(env.NO_PROXY).toBe("localhost,127.0.0.1"); - expect(env.http_proxy).toBe("http://proxy.local:7890"); - expect(env.all_proxy).toBe("socks5://proxy.local:1080"); + expect(env.HTTP_PROXY).toBeUndefined(); + expect(env.HTTPS_PROXY).toBeUndefined(); + expect(env.NO_PROXY).toBeUndefined(); + expect(env.http_proxy).toBeUndefined(); + expect(env.all_proxy).toBeUndefined(); }); it("omits PATH on Windows so Scheduled Tasks can inherit the current shell path", () => { @@ -529,7 +529,7 @@ describe("buildNodeServiceEnvironment", () => { expect(env.OPENCLAW_GATEWAY_TOKEN).toBeUndefined(); }); - it("forwards proxy environment variables for node services", () => { + it("does not persist ambient proxy environment variables for node services", () => { const env = buildNodeServiceEnvironment({ env: { HOME: "/home/user", @@ -538,8 +538,8 @@ describe("buildNodeServiceEnvironment", () => { }, }); - expect(env.HTTPS_PROXY).toBe("https://proxy.local:7890"); - expect(env.no_proxy).toBe("localhost,127.0.0.1"); + expect(env.HTTPS_PROXY).toBeUndefined(); + expect(env.no_proxy).toBeUndefined(); }); it("forwards TMPDIR for node services on Linux", () => { diff --git a/src/daemon/service-env.ts b/src/daemon/service-env.ts index 4233bdda3ae..29c86eff35b 100644 --- a/src/daemon/service-env.ts +++ b/src/daemon/service-env.ts @@ -40,12 +40,11 @@ type SharedServiceEnvironmentFields = { configPath: string | undefined; tmpDir: string; minimalPath: string | undefined; - proxyEnv: Record; nodeCaCerts: string | undefined; nodeUseSystemCa: string | undefined; }; -const SERVICE_PROXY_ENV_KEYS = [ +export const SERVICE_PROXY_ENV_KEYS = [ "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", @@ -56,24 +55,6 @@ const SERVICE_PROXY_ENV_KEYS = [ "all_proxy", ] as const; -function readServiceProxyEnvironment( - env: Record, -): Record { - const out: Record = {}; - for (const key of SERVICE_PROXY_ENV_KEYS) { - const value = env[key]; - if (typeof value !== "string") { - continue; - } - const trimmed = value.trim(); - if (!trimmed) { - continue; - } - out[key] = trimmed; - } - return out; -} - function addNonEmptyDir(dirs: string[], dir: string | undefined): void { if (dir) { dirs.push(dir); @@ -351,7 +332,6 @@ function buildCommonServiceEnvironment( const serviceEnv: Record = { HOME: env.HOME, TMPDIR: sharedEnv.tmpDir, - ...sharedEnv.proxyEnv, NODE_EXTRA_CA_CERTS: sharedEnv.nodeCaCerts, NODE_USE_SYSTEM_CA: sharedEnv.nodeUseSystemCa, OPENCLAW_STATE_DIR: sharedEnv.stateDir, @@ -386,7 +366,6 @@ function resolveSharedServiceEnvironmentFields( const stateDir = env.OPENCLAW_STATE_DIR; const configPath = env.OPENCLAW_CONFIG_PATH; const tmpDir = resolveServiceTmpDir(env, platform); - const proxyEnv = readServiceProxyEnvironment(env); // On macOS, launchd services don't inherit the shell environment, so Node's undici/fetch // cannot locate the system CA bundle. Default to /etc/ssl/cert.pem so TLS verification // works correctly when running as a LaunchAgent without extra user configuration. @@ -406,7 +385,6 @@ function resolveSharedServiceEnvironmentFields( platform === "win32" ? undefined : buildMinimalServicePath({ env, platform, extraDirs: extraPathDirs }), - proxyEnv, nodeCaCerts: startupTlsEnv.NODE_EXTRA_CA_CERTS, nodeUseSystemCa: startupTlsEnv.NODE_USE_SYSTEM_CA, };