mirror of
https://github.com/moltbot/moltbot.git
synced 2026-05-13 15:47:28 +00:00
fix: avoid persisting proxy env in gateway services
This commit is contained in:
@@ -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`).
|
||||
</Accordion>
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -40,12 +40,11 @@ type SharedServiceEnvironmentFields = {
|
||||
configPath: string | undefined;
|
||||
tmpDir: string;
|
||||
minimalPath: string | undefined;
|
||||
proxyEnv: Record<string, string | undefined>;
|
||||
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<string, string | undefined>,
|
||||
): Record<string, string | undefined> {
|
||||
const out: Record<string, string | undefined> = {};
|
||||
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<string, string | undefined> = {
|
||||
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,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user