From 67f1266fe8a2f25e9701954992dd8ff12074b498 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Apr 2026 10:12:57 +0100 Subject: [PATCH] fix: repair managed service env install migration --- docs/gateway/doctor.md | 1 + src/commands/daemon-install-helpers.test.ts | 83 ++++++++-- src/commands/daemon-install-helpers.ts | 73 +++------ src/commands/doctor-gateway-services.test.ts | 39 +++++ src/commands/doctor-gateway-services.ts | 79 +++++++-- .../doctor-service-audit.test-helpers.ts | 7 +- src/daemon/service-audit.test.ts | 75 ++++++++- src/daemon/service-audit.ts | 29 +++- src/daemon/service-managed-env.ts | 153 ++++++++++++++++++ src/daemon/service-types.ts | 4 +- src/daemon/systemd.test.ts | 2 +- src/daemon/systemd.ts | 22 ++- 12 files changed, 478 insertions(+), 89 deletions(-) create mode 100644 src/daemon/service-managed-env.ts diff --git a/docs/gateway/doctor.md b/docs/gateway/doctor.md index 24b68e495ee..5459f5558a8 100644 --- a/docs/gateway/doctor.md +++ b/docs/gateway/doctor.md @@ -433,6 +433,7 @@ That stages grounded durable candidates into the short-term dreaming store while - `openclaw doctor --repair --force` overwrites custom supervisor configs. - `OPENCLAW_SERVICE_REPAIR_POLICY=external` keeps doctor read-only for gateway service lifecycle. It still reports service health and runs non-service repairs, but skips service install/start/restart/bootstrap, supervisor config rewrites, and legacy service cleanup because an external supervisor owns that lifecycle. - If token auth requires a token and `gateway.auth.token` is SecretRef-managed, doctor service install/repair validates the SecretRef but does not persist resolved plaintext token values into supervisor service environment metadata. + - Doctor detects managed `.env`/SecretRef-backed service environment values that older LaunchAgent/systemd installs embedded inline and rewrites the service metadata so those values load from the runtime source instead of the supervisor definition. - If token auth requires a token and the configured token SecretRef is unresolved, doctor blocks the install/repair path with actionable guidance. - If both `gateway.auth.token` and `gateway.auth.password` are configured and `gateway.auth.mode` is unset, doctor blocks install/repair until mode is set explicitly. - For Linux user-systemd units, doctor token drift checks now include both `Environment=` and `EnvironmentFile=` sources when comparing service auth metadata. diff --git a/src/commands/daemon-install-helpers.test.ts b/src/commands/daemon-install-helpers.test.ts index 5561d069915..7927f2e0c54 100644 --- a/src/commands/daemon-install-helpers.test.ts +++ b/src/commands/daemon-install-helpers.test.ts @@ -243,7 +243,7 @@ describe("buildGatewayInstallPlan", () => { expect(plan.environment.OPENCLAW_WRAPPER).toBe(wrapperPath); }); - it("merges safe config env while dropping unsafe values and keeping service precedence", async () => { + it("tracks safe config env keys without embedding literal values", async () => { mockNodeGatewayPlanFixture({ serviceEnvironment: { HOME: "/Users/service", @@ -271,16 +271,16 @@ describe("buildGatewayInstallPlan", () => { }, }); - expect(plan.environment.GOOGLE_API_KEY).toBe("test-key"); - expect(plan.environment.CUSTOM_VAR).toBe("custom-value"); - expect(plan.environment.SAFE_KEY).toBe("safe-value"); + expect(plan.environment.GOOGLE_API_KEY).toBeUndefined(); + expect(plan.environment.CUSTOM_VAR).toBeUndefined(); + expect(plan.environment.SAFE_KEY).toBeUndefined(); expect(plan.environment.NODE_OPTIONS).toBeUndefined(); expect(plan.environment.EMPTY_KEY).toBeUndefined(); expect(plan.environment.TRIMMED_KEY).toBeUndefined(); expect(plan.environment.HOME).toBe("/Users/service"); expect(plan.environment.OPENCLAW_PORT).toBe("3000"); expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBe( - "CUSTOM_VAR,GOOGLE_API_KEY,OPENCLAW_PORT,SAFE_KEY", + "CUSTOM_VAR,GOOGLE_API_KEY,SAFE_KEY", ); }); @@ -328,6 +328,7 @@ describe("buildGatewayInstallPlan", () => { }); expect(plan.environment.OPENAI_API_KEY).toBe("sk-openai-test"); + expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBeUndefined(); expect(mocks.hasAnyAuthProfileStoreSource).not.toHaveBeenCalled(); expect(mocks.loadAuthProfileStoreForSecretsRuntime).not.toHaveBeenCalled(); }); @@ -393,6 +394,7 @@ describe("buildGatewayInstallPlan", () => { expect(plan.environment.MISSING_TOKEN).toBeUndefined(); expect(plan.environment.OPENAI_API_KEY).toBe("sk-openai-test"); expect(plan.environment.ANTHROPIC_TOKEN).toBe("ant-test-token"); + expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBeUndefined(); expect(warn).toHaveBeenCalledWith(expect.stringContaining("NODE_OPTIONS"), "Auth profile"); expect(warn).toHaveBeenCalledWith(expect.stringContaining("GIT_ASKPASS"), "Auth profile"); }); @@ -409,7 +411,7 @@ describe("buildGatewayInstallPlan — dotenv merge", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); - it("merges .env vars with config and service precedence", async () => { + it("tracks .env vars with config while preserving service precedence", async () => { await writeStateDirDotEnv( "BRAVE_API_KEY=BSA-from-env\nOPENROUTER_API_KEY=or-key\nMY_KEY=from-dotenv\nHOME=/from-dotenv\n", { @@ -436,11 +438,14 @@ describe("buildGatewayInstallPlan — dotenv merge", () => { }, }); - expect(plan.environment.BRAVE_API_KEY).toBe("BSA-from-env"); - expect(plan.environment.OPENROUTER_API_KEY).toBe("or-key"); - expect(plan.environment.MY_KEY).toBe("from-config"); + expect(plan.environment.BRAVE_API_KEY).toBeUndefined(); + expect(plan.environment.OPENROUTER_API_KEY).toBeUndefined(); + expect(plan.environment.MY_KEY).toBeUndefined(); expect(plan.environment.HOME).toBe("/from-service"); expect(plan.environment.OPENCLAW_PORT).toBe("3000"); + expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBe( + "BRAVE_API_KEY,MY_KEY,OPENROUTER_API_KEY", + ); }); it("works when .env file does not exist", async () => { @@ -515,6 +520,66 @@ describe("buildGatewayInstallPlan — dotenv merge", () => { expect(plan.environment.GOPATH).toBeUndefined(); expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBeUndefined(); }); + + it("drops legacy inline env values when the key is now managed by .env", async () => { + await writeStateDirDotEnv("TAVILY_API_KEY=fresh-dotenv-value\n", { + stateDir: path.join(tmpDir, ".openclaw"), + }); + mockNodeGatewayPlanFixture({ + serviceEnvironment: { + HOME: "/from-service", + OPENCLAW_PORT: "3000", + }, + }); + + const plan = await buildGatewayInstallPlan({ + env: { HOME: tmpDir }, + port: 3000, + runtime: "node", + existingEnvironment: { + TAVILY_API_KEY: "old-inline-value", + CUSTOM_TOOL_HOME: "/Users/test/.custom-tool", + }, + }); + + expect(plan.environment.TAVILY_API_KEY).toBeUndefined(); + expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBe("TAVILY_API_KEY"); + expect(plan.environment.CUSTOM_TOOL_HOME).toBe("/Users/test/.custom-tool"); + }); + + it("does not embed auth-profile env refs when the key is already durable", async () => { + await writeStateDirDotEnv("OPENAI_API_KEY=dotenv-openai\n", { + stateDir: path.join(tmpDir, ".openclaw"), + }); + mockNodeGatewayPlanFixture({ + serviceEnvironment: { + HOME: "/from-service", + OPENCLAW_PORT: "3000", + }, + }); + + const plan = await buildGatewayInstallPlan({ + env: { + HOME: tmpDir, + OPENAI_API_KEY: "shell-openai", + }, + port: 3000, + runtime: "node", + authStore: { + version: 1, + profiles: { + "openai:default": { + type: "api_key", + provider: "openai", + keyRef: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + }, + }, + }, + }); + + expect(plan.environment.OPENAI_API_KEY).toBeUndefined(); + expect(plan.environment.OPENCLAW_SERVICE_MANAGED_ENV_KEYS).toBe("OPENAI_API_KEY"); + }); }); describe("gatewayInstallErrorHint", () => { diff --git a/src/commands/daemon-install-helpers.ts b/src/commands/daemon-install-helpers.ts index 18e0ebe8d86..202406d1d3d 100644 --- a/src/commands/daemon-install-helpers.ts +++ b/src/commands/daemon-install-helpers.ts @@ -12,6 +12,11 @@ import { resolveOpenClawWrapperPath, } from "../daemon/program-args.js"; import { buildServiceEnvironment } from "../daemon/service-env.js"; +import { + formatManagedServiceEnvKeys, + readManagedServiceEnvKeysFromEnvironment, + writeManagedServiceEnvKeysToEnvironment, +} from "../daemon/service-managed-env.js"; import { isDangerousHostEnvOverrideVarName, isDangerousHostEnvVarName, @@ -33,8 +38,6 @@ export type GatewayInstallPlan = { environment: Record; }; -const MANAGED_SERVICE_ENV_KEYS_VAR = "OPENCLAW_SERVICE_MANAGED_ENV_KEYS"; - let daemonInstallAuthProfileSourceRuntimePromise: | Promise | undefined; @@ -147,39 +150,6 @@ function mergeServicePath( 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, @@ -239,24 +209,27 @@ async function buildGatewayInstallEnvironment(params: { serviceEnvironment: Record; existingEnvironment?: Record; }): Promise> { - const managedEnvironment: Record = { - ...collectDurableServiceEnvVars({ - env: params.env, - config: params.config, - }), - ...(await collectAuthProfileServiceEnvVars({ - env: params.env, - authStore: params.authStore, - warn: params.warn, - })), - }; + const durableEnvironment = collectDurableServiceEnvVars({ + env: params.env, + config: params.config, + }); + const authProfileEnvironment = await collectAuthProfileServiceEnvVars({ + env: params.env, + authStore: params.authStore, + warn: params.warn, + }); const environment: Record = { ...collectPreservedExistingServiceEnvVars( params.existingEnvironment, - readManagedServiceEnvKeys(params.existingEnvironment), + readManagedServiceEnvKeysFromEnvironment(params.existingEnvironment), ), - ...managedEnvironment, + ...durableEnvironment, + ...authProfileEnvironment, }; + const managedServiceEnvKeys = formatManagedServiceEnvKeys(durableEnvironment, { + omitKeys: Object.keys(params.serviceEnvironment), + }); + writeManagedServiceEnvKeysToEnvironment(environment, managedServiceEnvKeys); Object.assign(environment, params.serviceEnvironment); const mergedPath = mergeServicePath( params.serviceEnvironment.PATH, @@ -266,10 +239,6 @@ async function buildGatewayInstallEnvironment(params: { if (mergedPath) { environment.PATH = mergedPath; } - const managedServiceEnvKeys = formatManagedServiceEnvKeys(managedEnvironment); - if (managedServiceEnvKeys) { - environment[MANAGED_SERVICE_ENV_KEYS_VAR] = managedServiceEnvKeys; - } return environment; } diff --git a/src/commands/doctor-gateway-services.test.ts b/src/commands/doctor-gateway-services.test.ts index bc69586e9e5..fd925eb4182 100644 --- a/src/commands/doctor-gateway-services.test.ts +++ b/src/commands/doctor-gateway-services.test.ts @@ -68,6 +68,8 @@ vi.mock("../daemon/service-audit.js", () => ({ readEmbeddedGatewayToken: readEmbeddedGatewayTokenForTest, SERVICE_AUDIT_CODES: { gatewayEntrypointMismatch: testServiceAuditCodes.gatewayEntrypointMismatch, + gatewayManagedEnvEmbedded: testServiceAuditCodes.gatewayManagedEnvEmbedded, + gatewayTokenMismatch: testServiceAuditCodes.gatewayTokenMismatch, }, })); @@ -282,6 +284,43 @@ describe("maybeRepairGatewayServiceConfig", () => { expect(mocks.install).toHaveBeenCalledTimes(1); }); + it("passes planned managed env keys into service audit for legacy inline secret detection", async () => { + mocks.readCommand.mockResolvedValue({ + programArguments: gatewayProgramArguments, + environment: { + TAVILY_API_KEY: "old-inline-value", + }, + }); + mocks.buildGatewayInstallPlan.mockResolvedValue({ + programArguments: gatewayProgramArguments, + workingDirectory: "/tmp", + environment: { + OPENCLAW_SERVICE_MANAGED_ENV_KEYS: "TAVILY_API_KEY", + }, + }); + mocks.auditGatewayServiceConfig.mockResolvedValue({ + ok: false, + issues: [ + { + code: "gateway-managed-env-embedded", + message: "Gateway service embeds managed environment values that should load at runtime.", + detail: "inline keys: TAVILY_API_KEY", + level: "recommended", + }, + ], + }); + mocks.install.mockResolvedValue(undefined); + + await runRepair({ gateway: {} }); + + expect(mocks.auditGatewayServiceConfig).toHaveBeenCalledWith( + expect.objectContaining({ + expectedManagedServiceEnvKeys: new Set(["TAVILY_API_KEY"]), + }), + ); + expect(mocks.install).toHaveBeenCalledTimes(1); + }); + 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-gateway-services.ts b/src/commands/doctor-gateway-services.ts index a059d9e4547..1597cf9e5ba 100644 --- a/src/commands/doctor-gateway-services.ts +++ b/src/commands/doctor-gateway-services.ts @@ -19,6 +19,7 @@ import { readEmbeddedGatewayToken, SERVICE_AUDIT_CODES, } from "../daemon/service-audit.js"; +import { readManagedServiceEnvKeysFromEnvironment } from "../daemon/service-managed-env.js"; import { resolveGatewayService, type GatewayServiceCommandConfig } from "../daemon/service.js"; import { uninstallLegacySystemdUnits } from "../daemon/systemd.js"; import type { RuntimeEnv } from "../runtime.js"; @@ -85,6 +86,45 @@ function resolveGatewayServiceWrapperPath( return normalizeOptionalString(command?.environment?.[OPENCLAW_WRAPPER_ENV_KEY]) ?? null; } +async function buildExpectedGatewayServicePlan(params: { + cfg: OpenClawConfig; + command: GatewayServiceCommandConfig; + serviceInstallEnv: NodeJS.ProcessEnv; + port: number; + runtime: GatewayDaemonRuntime; + nodePath?: string; +}) { + return buildGatewayInstallPlan({ + env: params.serviceInstallEnv, + port: params.port, + runtime: params.runtime, + nodePath: params.nodePath, + existingEnvironment: params.command.environment, + warn: (message, title) => note(message, title), + config: params.cfg, + }); +} + +async function buildGatewayServiceAuditInputs(params: { + cfg: OpenClawConfig; + command: GatewayServiceCommandConfig; + serviceInstallEnv: NodeJS.ProcessEnv; +}) { + const port = resolveGatewayPort(params.cfg, process.env); + const runtimeChoice = detectGatewayRuntime(params.command.programArguments); + const expectedPlan = await buildExpectedGatewayServicePlan({ + cfg: params.cfg, + command: params.command, + serviceInstallEnv: params.serviceInstallEnv, + port, + runtime: runtimeChoice, + }); + const expectedManagedServiceEnvKeys = readManagedServiceEnvKeysFromEnvironment( + expectedPlan.environment, + ); + return { expectedManagedServiceEnvKeys, expectedPlan, port, runtimeChoice }; +} + async function normalizeExecutablePath(value: string): Promise { const resolvedPath = path.resolve(value); try { @@ -267,10 +307,17 @@ export async function maybeRepairGatewayServiceConfig( ); } const expectedGatewayToken = tokenRefConfigured ? undefined : gatewayTokenResolution.token; + const { expectedManagedServiceEnvKeys, expectedPlan, port, runtimeChoice } = + await buildGatewayServiceAuditInputs({ + cfg, + command, + serviceInstallEnv, + }); const audit = await auditGatewayServiceConfig({ env: process.env, command, expectedGatewayToken, + expectedManagedServiceEnvKeys, }); const serviceToken = readEmbeddedGatewayToken(command); if (tokenRefConfigured && serviceToken) { @@ -298,17 +345,18 @@ export async function maybeRepairGatewayServiceConfig( ); } - const port = resolveGatewayPort(cfg, process.env); - const runtimeChoice = detectGatewayRuntime(command.programArguments); - const { programArguments } = await buildGatewayInstallPlan({ - env: serviceInstallEnv, - port, - runtime: needsNodeRuntime && systemNodePath ? "node" : runtimeChoice, - nodePath: systemNodePath ?? undefined, - existingEnvironment: command.environment, - warn: (message, title) => note(message, title), - config: cfg, - }); + const expectedRuntimePlan = + needsNodeRuntime && systemNodePath + ? await buildExpectedGatewayServicePlan({ + cfg, + command, + serviceInstallEnv, + port, + runtime: "node", + nodePath: systemNodePath, + }) + : expectedPlan; + const { programArguments } = expectedRuntimePlan; const expectedEntrypoint = findGatewayEntrypoint(programArguments); const currentEntrypoint = findGatewayEntrypoint(command.programArguments); const normalizedExpectedEntrypoint = expectedEntrypoint @@ -414,14 +462,13 @@ export async function maybeRepairGatewayServiceConfig( } const updatedPort = resolveGatewayPort(cfgForServiceInstall, process.env); - const updatedPlan = await buildGatewayInstallPlan({ - env: serviceInstallEnv, + const updatedPlan = await buildExpectedGatewayServicePlan({ + cfg: cfgForServiceInstall, + command, + serviceInstallEnv, port: updatedPort, runtime: needsNodeRuntime && systemNodePath ? "node" : runtimeChoice, nodePath: systemNodePath ?? undefined, - existingEnvironment: command.environment, - warn: (message, title) => note(message, title), - config: cfgForServiceInstall, }); try { await (updateRepairMode ? service.stage : service.install)({ diff --git a/src/commands/doctor-service-audit.test-helpers.ts b/src/commands/doctor-service-audit.test-helpers.ts index d27003bc503..92275777d57 100644 --- a/src/commands/doctor-service-audit.test-helpers.ts +++ b/src/commands/doctor-service-audit.test-helpers.ts @@ -1,17 +1,20 @@ +import { isEnvironmentFileOnlySource } from "../daemon/service-managed-env.js"; +import type { GatewayServiceEnvironmentValueSource } from "../daemon/service-types.js"; import { normalizeOptionalString } from "../shared/string-coerce.js"; export const testServiceAuditCodes = { gatewayEntrypointMismatch: "gateway-entrypoint-mismatch", + gatewayManagedEnvEmbedded: "gateway-managed-env-embedded", gatewayTokenMismatch: "gateway-token-mismatch", } as const; export function readEmbeddedGatewayTokenForTest( command: { environment?: Record; - environmentValueSources?: Record; + environmentValueSources?: Record; } | null, ) { - return command?.environmentValueSources?.OPENCLAW_GATEWAY_TOKEN === "file" + return isEnvironmentFileOnlySource(command?.environmentValueSources?.OPENCLAW_GATEWAY_TOKEN) ? undefined : normalizeOptionalString(command?.environment?.OPENCLAW_GATEWAY_TOKEN); } diff --git a/src/daemon/service-audit.test.ts b/src/daemon/service-audit.test.ts index 449ea36d3a0..8978101d534 100644 --- a/src/daemon/service-audit.test.ts +++ b/src/daemon/service-audit.test.ts @@ -5,6 +5,7 @@ import { SERVICE_AUDIT_CODES, } from "./service-audit.js"; import { buildMinimalServicePath } from "./service-env.js"; +import type { GatewayServiceEnvironmentValueSource } from "./service-types.js"; function hasIssue( audit: Awaited>, @@ -15,24 +16,30 @@ function hasIssue( function createGatewayAudit({ expectedGatewayToken, + expectedManagedServiceEnvKeys, path = "/usr/local/bin:/usr/bin:/bin", serviceToken, + extraEnvironment, environmentValueSources, }: { expectedGatewayToken?: string; + expectedManagedServiceEnvKeys?: Iterable; path?: string; serviceToken?: string; - environmentValueSources?: Record; + extraEnvironment?: Record; + environmentValueSources?: Record; } = {}) { return auditGatewayServiceConfig({ env: { HOME: "/tmp" }, platform: "linux", expectedGatewayToken, + expectedManagedServiceEnvKeys, command: { programArguments: ["/usr/bin/node", "gateway"], environment: { PATH: path, ...(serviceToken ? { OPENCLAW_GATEWAY_TOKEN: serviceToken } : {}), + ...extraEnvironment, }, ...(environmentValueSources ? { environmentValueSources } : {}), }, @@ -183,6 +190,72 @@ describe("auditGatewayServiceConfig", () => { }); expectTokenAudit(audit, { embedded: false, mismatch: false }); }); + + it("treats tokens present inline and in EnvironmentFile as embedded", async () => { + const audit = await createGatewayAudit({ + expectedGatewayToken: "new-token", + serviceToken: "old-token", + environmentValueSources: { + OPENCLAW_GATEWAY_TOKEN: "inline-and-file", + }, + }); + expectTokenAudit(audit, { embedded: true, mismatch: true }); + }); + + it("flags inline managed service env values from the service key list", async () => { + const audit = await createGatewayAudit({ + extraEnvironment: { + OPENCLAW_SERVICE_MANAGED_ENV_KEYS: "TAVILY_API_KEY,OPENROUTER_API_KEY", + TAVILY_API_KEY: "tvly-test", + OPENROUTER_API_KEY: "or-test", + }, + }); + + const issue = audit.issues.find( + (entry) => entry.code === SERVICE_AUDIT_CODES.gatewayManagedEnvEmbedded, + ); + expect(issue?.detail).toContain("OPENROUTER_API_KEY"); + expect(issue?.detail).toContain("TAVILY_API_KEY"); + }); + + it("flags inline managed values expected by the current install plan for old services", async () => { + const audit = await createGatewayAudit({ + expectedManagedServiceEnvKeys: ["TAVILY_API_KEY"], + extraEnvironment: { + TAVILY_API_KEY: "tvly-test", + }, + }); + + expect(hasIssue(audit, SERVICE_AUDIT_CODES.gatewayManagedEnvEmbedded)).toBe(true); + }); + + it("does not flag managed env values loaded from EnvironmentFile", async () => { + const audit = await createGatewayAudit({ + expectedManagedServiceEnvKeys: ["TAVILY_API_KEY"], + extraEnvironment: { + TAVILY_API_KEY: "tvly-test", + }, + environmentValueSources: { + TAVILY_API_KEY: "file", + }, + }); + + expect(hasIssue(audit, SERVICE_AUDIT_CODES.gatewayManagedEnvEmbedded)).toBe(false); + }); + + it("flags managed env values present inline even when an EnvironmentFile overrides them", async () => { + const audit = await createGatewayAudit({ + expectedManagedServiceEnvKeys: ["TAVILY_API_KEY"], + extraEnvironment: { + TAVILY_API_KEY: "tvly-test", + }, + environmentValueSources: { + TAVILY_API_KEY: "inline-and-file", + }, + }); + + expect(hasIssue(audit, SERVICE_AUDIT_CODES.gatewayManagedEnvEmbedded)).toBe(true); + }); }); describe("checkTokenDrift", () => { diff --git a/src/daemon/service-audit.ts b/src/daemon/service-audit.ts index ee174b2f700..d4987a432c3 100644 --- a/src/daemon/service-audit.ts +++ b/src/daemon/service-audit.ts @@ -12,13 +12,18 @@ import { resolveSystemNodePath, } from "./runtime-paths.js"; import { getMinimalServicePathPartsFromEnv } from "./service-env.js"; +import { + collectInlineManagedServiceEnvKeys, + isEnvironmentFileOnlySource, +} from "./service-managed-env.js"; +import type { GatewayServiceEnvironmentValueSource } from "./service-types.js"; import { resolveSystemdUserUnitPath } from "./systemd.js"; export type GatewayServiceCommand = { programArguments: string[]; workingDirectory?: string; environment?: Record; - environmentValueSources?: Record; + environmentValueSources?: Record; sourcePath?: string; } | null; @@ -41,6 +46,7 @@ export const SERVICE_AUDIT_CODES = { gatewayPathMissingDirs: "gateway-path-missing-dirs", gatewayPathNonMinimal: "gateway-path-nonminimal", gatewayTokenEmbedded: "gateway-token-embedded", + gatewayManagedEnvEmbedded: "gateway-managed-env-embedded", gatewayTokenMismatch: "gateway-token-mismatch", gatewayRuntimeBun: "gateway-runtime-bun", gatewayRuntimeNodeVersionManager: "gateway-runtime-node-version-manager", @@ -237,11 +243,28 @@ function auditGatewayToken( }); } +function auditManagedServiceEnvironment( + command: GatewayServiceCommand, + issues: ServiceConfigIssue[], + expectedManagedServiceEnvKeys?: Iterable, +) { + const inlineKeys = collectInlineManagedServiceEnvKeys(command, expectedManagedServiceEnvKeys); + if (inlineKeys.length === 0) { + return; + } + issues.push({ + code: SERVICE_AUDIT_CODES.gatewayManagedEnvEmbedded, + message: "Gateway service embeds managed environment values that should load at runtime.", + detail: `inline keys: ${inlineKeys.join(", ")}`, + level: "recommended", + }); +} + export function readEmbeddedGatewayToken(command: GatewayServiceCommand): string | undefined { if (!command) { return undefined; } - if (command.environmentValueSources?.OPENCLAW_GATEWAY_TOKEN === "file") { + if (isEnvironmentFileOnlySource(command.environmentValueSources?.OPENCLAW_GATEWAY_TOKEN)) { return undefined; } return normalizeOptionalString(command.environment?.OPENCLAW_GATEWAY_TOKEN); @@ -433,11 +456,13 @@ export async function auditGatewayServiceConfig(params: { command: GatewayServiceCommand; platform?: NodeJS.Platform; expectedGatewayToken?: string; + expectedManagedServiceEnvKeys?: Iterable; }): Promise { const issues: ServiceConfigIssue[] = []; const platform = params.platform ?? process.platform; auditGatewayCommand(params.command?.programArguments, issues); + auditManagedServiceEnvironment(params.command, issues, params.expectedManagedServiceEnvKeys); 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-managed-env.ts b/src/daemon/service-managed-env.ts new file mode 100644 index 00000000000..ea0ad68fa0e --- /dev/null +++ b/src/daemon/service-managed-env.ts @@ -0,0 +1,153 @@ +import { normalizeEnvVarKey } from "../infra/host-env-security.js"; +import type { GatewayServiceEnvironmentValueSource } from "./service-types.js"; + +export const MANAGED_SERVICE_ENV_KEYS_VAR = "OPENCLAW_SERVICE_MANAGED_ENV_KEYS"; + +type ServiceEnvCommand = { + environment?: Record; + environmentValueSources?: Record; +} | null; + +function normalizeServiceEnvKey(key: string): string | null { + return normalizeEnvVarKey(key, { portable: true })?.toUpperCase() ?? null; +} + +export function hasInlineEnvironmentSource( + source: GatewayServiceEnvironmentValueSource | undefined, +): boolean { + return source === undefined || source === "inline" || source === "inline-and-file"; +} + +export function isEnvironmentFileOnlySource( + source: GatewayServiceEnvironmentValueSource | undefined, +): boolean { + return source === "file"; +} + +export function parseManagedServiceEnvKeys(value: string | undefined): Set { + const keys = new Set(); + for (const entry of value?.split(",") ?? []) { + const key = normalizeServiceEnvKey(entry.trim()); + if (key) { + keys.add(key); + } + } + return keys; +} + +export function formatManagedServiceEnvKeys( + managedEnvironment: Record, + options?: { omitKeys?: Iterable }, +): string | undefined { + const omitKeys = new Set( + [...(options?.omitKeys ?? [])].flatMap((key) => { + const normalized = normalizeServiceEnvKey(key); + return normalized ? [normalized] : []; + }), + ); + const keys = Object.keys(managedEnvironment) + .flatMap((key) => { + const normalized = normalizeServiceEnvKey(key); + if (!normalized || omitKeys.has(normalized)) { + return []; + } + return [normalized]; + }) + .toSorted(); + return keys.length > 0 ? keys.join(",") : undefined; +} + +export function readManagedServiceEnvKeysFromEnvironment( + environment: Record | undefined, +): Set { + if (!environment) { + return new Set(); + } + for (const [rawKey, rawValue] of Object.entries(environment)) { + if (normalizeServiceEnvKey(rawKey) === MANAGED_SERVICE_ENV_KEYS_VAR) { + return parseManagedServiceEnvKeys(rawValue); + } + } + return new Set(); +} + +export function deleteManagedServiceEnvKeys( + environment: Record, + keys: Iterable, +): void { + const normalizedKeys = new Set( + [...keys].flatMap((key) => { + const normalized = normalizeServiceEnvKey(key); + return normalized ? [normalized] : []; + }), + ); + if (normalizedKeys.size === 0) { + return; + } + for (const rawKey of Object.keys(environment)) { + const key = normalizeServiceEnvKey(rawKey); + if (key && normalizedKeys.has(key)) { + delete environment[rawKey]; + } + } +} + +export function writeManagedServiceEnvKeysToEnvironment( + environment: Record, + value: string | undefined, +): void { + if (!value) { + return; + } + deleteManagedServiceEnvKeys(environment, parseManagedServiceEnvKeys(value)); + environment[MANAGED_SERVICE_ENV_KEYS_VAR] = value; +} + +function readEnvironmentValueSource( + command: ServiceEnvCommand, + normalizedKey: string, +): GatewayServiceEnvironmentValueSource | undefined { + for (const [rawKey, source] of Object.entries(command?.environmentValueSources ?? {})) { + if (normalizeServiceEnvKey(rawKey) === normalizedKey) { + return source; + } + } + return undefined; +} + +export function collectInlineManagedServiceEnvKeys( + command: ServiceEnvCommand, + expectedManagedKeys?: Iterable, +): string[] { + if (!command?.environment) { + return []; + } + const managedKeys = parseManagedServiceEnvKeys(command.environment[MANAGED_SERVICE_ENV_KEYS_VAR]); + for (const key of expectedManagedKeys ?? []) { + const normalized = normalizeServiceEnvKey(key); + if (normalized) { + managedKeys.add(normalized); + } + } + if (managedKeys.size === 0) { + 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 || !managedKeys.has(normalized)) { + continue; + } + if (normalized === MANAGED_SERVICE_ENV_KEYS_VAR) { + continue; + } + if (!hasInlineEnvironmentSource(readEnvironmentValueSource(command, normalized))) { + continue; + } + inlineKeys.push(normalized); + } + return [...new Set(inlineKeys)].toSorted(); +} diff --git a/src/daemon/service-types.ts b/src/daemon/service-types.ts index 22ced3954d9..2fccd0efc5b 100644 --- a/src/daemon/service-types.ts +++ b/src/daemon/service-types.ts @@ -29,11 +29,13 @@ export type GatewayServiceEnvArgs = { env?: GatewayServiceEnv; }; +export type GatewayServiceEnvironmentValueSource = "inline" | "file" | "inline-and-file"; + export type GatewayServiceCommandConfig = { programArguments: string[]; workingDirectory?: string; environment?: Record; - environmentValueSources?: Record; + environmentValueSources?: Record; sourcePath?: string; }; diff --git a/src/daemon/systemd.test.ts b/src/daemon/systemd.test.ts index 0bace19ca6e..036f0adf34d 100644 --- a/src/daemon/systemd.test.ts +++ b/src/daemon/systemd.test.ts @@ -578,7 +578,7 @@ describe("readSystemdServiceExecStart", () => { const command = await readSystemdServiceExecStart({ HOME: TEST_SERVICE_HOME }); expect(command?.environment?.OPENCLAW_GATEWAY_TOKEN).toBe("env-file-token"); - expect(command?.environmentValueSources?.OPENCLAW_GATEWAY_TOKEN).toBe("file"); + expect(command?.environmentValueSources?.OPENCLAW_GATEWAY_TOKEN).toBe("inline-and-file"); }); it("ignores missing optional EnvironmentFile entries", async () => { diff --git a/src/daemon/systemd.ts b/src/daemon/systemd.ts index f68abcba5fe..634bfedbe4b 100644 --- a/src/daemon/systemd.ts +++ b/src/daemon/systemd.ts @@ -22,6 +22,7 @@ import type { GatewayServiceControlArgs, GatewayServiceEnv, GatewayServiceEnvArgs, + GatewayServiceEnvironmentValueSource, GatewayServiceInstallArgs, GatewayServiceManageArgs, GatewayServiceRestartResult, @@ -114,10 +115,10 @@ export async function readSystemdServiceExecStart( ...inlineEnvironment, ...environmentFromFiles.environment, }; - const mergedEnvironmentSources = { - ...buildEnvironmentValueSources(inlineEnvironment, "inline"), - ...buildEnvironmentValueSources(environmentFromFiles.environment, "file"), - }; + const mergedEnvironmentSources = mergeEnvironmentValueSources( + inlineEnvironment, + environmentFromFiles.environment, + ); const programArguments = parseSystemdExecStart(execStart); return { programArguments, @@ -136,10 +137,21 @@ export async function readSystemdServiceExecStart( function buildEnvironmentValueSources( environment: Record, source: "inline" | "file", -): Record { +): Record { return Object.fromEntries(Object.keys(environment).map((key) => [key, source])); } +function mergeEnvironmentValueSources( + inlineEnvironment: Record, + fileEnvironment: Record, +): Record { + const sources = buildEnvironmentValueSources(inlineEnvironment, "inline"); + for (const key of Object.keys(fileEnvironment)) { + sources[key] = Object.hasOwn(inlineEnvironment, key) ? "inline-and-file" : "file"; + } + return sources; +} + function expandSystemdSpecifier(input: string, env: GatewayServiceEnv): string { // Support the common unit-specifier used in user services. return input.replaceAll("%h", toPosixPath(resolveHomeDir(env)));