From 8c188f4607378fbf6bdf796d43ff092649a8fcf9 Mon Sep 17 00:00:00 2001 From: BKF-Gitty Date: Fri, 17 Apr 2026 00:40:08 +0300 Subject: [PATCH] fix(onboard): preserve SecretRefs and prefer persisted token over ambient env Ambient OPENCLAW_GATEWAY_TOKEN no longer rotates a persisted token on re-onboard, and an existing SecretRef at gateway.auth.token is no longer silently overwritten with a plaintext literal. Explicit --gateway-token and --gateway-token-ref-env remain intentional overrides. Env-source SecretRefs are resolved inline for the health probe without persisting plaintext. --- .../local/gateway-config.test.ts | 159 +++++++++++++++++- .../local/gateway-config.ts | 42 ++++- 2 files changed, 191 insertions(+), 10 deletions(-) diff --git a/src/commands/onboard-non-interactive/local/gateway-config.test.ts b/src/commands/onboard-non-interactive/local/gateway-config.test.ts index 217ac7b3c60..e3be3215add 100644 --- a/src/commands/onboard-non-interactive/local/gateway-config.test.ts +++ b/src/commands/onboard-non-interactive/local/gateway-config.test.ts @@ -23,12 +23,20 @@ function createRuntime() { const baseOpts = {} as OnboardOptions; +const SAMPLE_SECRET_REF = { + source: "env" as const, + provider: "default", + id: "OPENCLAW_GATEWAY_TOKEN_REF", +}; + describe("applyNonInteractiveGatewayConfig token resolution chain", () => { const originalEnvToken = process.env.OPENCLAW_GATEWAY_TOKEN; + const originalRefValue = process.env[SAMPLE_SECRET_REF.id]; beforeEach(() => { vi.clearAllMocks(); delete process.env.OPENCLAW_GATEWAY_TOKEN; + delete process.env[SAMPLE_SECRET_REF.id]; }); afterEach(() => { @@ -37,9 +45,16 @@ describe("applyNonInteractiveGatewayConfig token resolution chain", () => { } else { process.env.OPENCLAW_GATEWAY_TOKEN = originalEnvToken; } + if (originalRefValue === undefined) { + delete process.env[SAMPLE_SECRET_REF.id]; + } else { + process.env[SAMPLE_SECRET_REF.id] = originalRefValue; + } }); - it("preserves existing gateway.auth.token when no flag or env override is provided", () => { + // --- Plaintext preservation (the original regression) --- + + it("preserves existing plaintext gateway.auth.token when no flag or env override is provided", () => { const nextConfig = { gateway: { auth: { mode: "token", token: "existing-user-token" } }, } as OpenClawConfig; @@ -56,7 +71,27 @@ describe("applyNonInteractiveGatewayConfig token resolution chain", () => { expect(randomToken).not.toHaveBeenCalled(); }); - it("prefers --gateway-token flag over existing config token", () => { + it("prefers existing plaintext token over ambient OPENCLAW_GATEWAY_TOKEN on re-onboard", () => { + // A stale shell/launchd OPENCLAW_GATEWAY_TOKEN must not rotate a + // persisted token — that would break already-paired clients. + process.env.OPENCLAW_GATEWAY_TOKEN = "stale-env-token"; + const nextConfig = { + gateway: { auth: { mode: "token", token: "existing-user-token" } }, + } as OpenClawConfig; + + const result = applyNonInteractiveGatewayConfig({ + nextConfig, + opts: baseOpts, + runtime: createRuntime() as never, + defaultPort: 18789, + }); + + expect(result?.gatewayToken).toBe("existing-user-token"); + expect(result?.nextConfig.gateway?.auth?.token).toBe("existing-user-token"); + expect(randomToken).not.toHaveBeenCalled(); + }); + + it("prefers --gateway-token flag over existing plaintext token", () => { const nextConfig = { gateway: { auth: { mode: "token", token: "existing-user-token" } }, } as OpenClawConfig; @@ -73,14 +108,11 @@ describe("applyNonInteractiveGatewayConfig token resolution chain", () => { expect(randomToken).not.toHaveBeenCalled(); }); - it("prefers OPENCLAW_GATEWAY_TOKEN env var over existing config token", () => { + it("uses OPENCLAW_GATEWAY_TOKEN to fill an empty config on first-run", () => { process.env.OPENCLAW_GATEWAY_TOKEN = "env-token"; - const nextConfig = { - gateway: { auth: { mode: "token", token: "existing-user-token" } }, - } as OpenClawConfig; const result = applyNonInteractiveGatewayConfig({ - nextConfig, + nextConfig: {} as OpenClawConfig, opts: baseOpts, runtime: createRuntime() as never, defaultPort: 18789, @@ -103,4 +135,117 @@ describe("applyNonInteractiveGatewayConfig token resolution chain", () => { expect(result?.gatewayToken).toBe("generated-random-token"); expect(result?.nextConfig.gateway?.auth?.token).toBe("generated-random-token"); }); + + // --- SecretRef preservation --- + + it("preserves an existing SecretRef when no flag or env override is provided", () => { + const nextConfig = { + gateway: { auth: { mode: "token", token: SAMPLE_SECRET_REF } }, + } as unknown as OpenClawConfig; + + const result = applyNonInteractiveGatewayConfig({ + nextConfig, + opts: baseOpts, + runtime: createRuntime() as never, + defaultPort: 18789, + }); + + expect(result?.nextConfig.gateway?.auth?.token).toEqual(SAMPLE_SECRET_REF); + expect(randomToken).not.toHaveBeenCalled(); + }); + + it("preserves an existing SecretRef even when ambient OPENCLAW_GATEWAY_TOKEN is set", () => { + // A stale ambient env must not declassify a configured SecretRef. + process.env.OPENCLAW_GATEWAY_TOKEN = "stale-env-token"; + const nextConfig = { + gateway: { auth: { mode: "token", token: SAMPLE_SECRET_REF } }, + } as unknown as OpenClawConfig; + + const result = applyNonInteractiveGatewayConfig({ + nextConfig, + opts: baseOpts, + runtime: createRuntime() as never, + defaultPort: 18789, + }); + + expect(result?.nextConfig.gateway?.auth?.token).toEqual(SAMPLE_SECRET_REF); + expect(randomToken).not.toHaveBeenCalled(); + }); + + it("resolves an env-source SecretRef for the health probe without persisting plaintext", () => { + // For probe/runtime use only: resolve process.env[ref.id] and return it + // as gatewayToken, while leaving the SecretRef intact in config. + process.env[SAMPLE_SECRET_REF.id] = "resolved-secret-value"; + const nextConfig = { + gateway: { auth: { mode: "token", token: SAMPLE_SECRET_REF } }, + } as unknown as OpenClawConfig; + + const result = applyNonInteractiveGatewayConfig({ + nextConfig, + opts: baseOpts, + runtime: createRuntime() as never, + defaultPort: 18789, + }); + + expect(result?.gatewayToken).toBe("resolved-secret-value"); + expect(result?.nextConfig.gateway?.auth?.token).toEqual(SAMPLE_SECRET_REF); + }); + + it("leaves gatewayToken undefined when an env-source SecretRef is unresolved", () => { + const nextConfig = { + gateway: { auth: { mode: "token", token: SAMPLE_SECRET_REF } }, + } as unknown as OpenClawConfig; + + const result = applyNonInteractiveGatewayConfig({ + nextConfig, + opts: baseOpts, + runtime: createRuntime() as never, + defaultPort: 18789, + }); + + expect(result?.gatewayToken).toBeUndefined(); + expect(result?.nextConfig.gateway?.auth?.token).toEqual(SAMPLE_SECRET_REF); + }); + + it("overrides an existing SecretRef when --gateway-token flag is provided", () => { + const nextConfig = { + gateway: { auth: { mode: "token", token: SAMPLE_SECRET_REF } }, + } as unknown as OpenClawConfig; + + const result = applyNonInteractiveGatewayConfig({ + nextConfig, + opts: { gatewayToken: "flag-token" } as OnboardOptions, + runtime: createRuntime() as never, + defaultPort: 18789, + }); + + expect(result?.gatewayToken).toBe("flag-token"); + expect(result?.nextConfig.gateway?.auth?.token).toBe("flag-token"); + expect(randomToken).not.toHaveBeenCalled(); + }); + + it("overrides an existing SecretRef when --gateway-token-ref-env is provided", () => { + const newRefId = "OPENCLAW_GATEWAY_TOKEN_NEW_REF"; + process.env[newRefId] = "resolved-new-ref-value"; + try { + const nextConfig = { + gateway: { auth: { mode: "token", token: SAMPLE_SECRET_REF } }, + } as unknown as OpenClawConfig; + + const result = applyNonInteractiveGatewayConfig({ + nextConfig, + opts: { gatewayTokenRefEnv: newRefId } as OnboardOptions, + runtime: createRuntime() as never, + defaultPort: 18789, + }); + + expect(result?.gatewayToken).toBe("resolved-new-ref-value"); + const newToken = result?.nextConfig.gateway?.auth?.token; + expect(newToken).toMatchObject({ source: "env", id: newRefId }); + expect(newToken).not.toEqual(SAMPLE_SECRET_REF); + expect(randomToken).not.toHaveBeenCalled(); + } finally { + delete process.env[newRefId]; + } + }); }); diff --git a/src/commands/onboard-non-interactive/local/gateway-config.ts b/src/commands/onboard-non-interactive/local/gateway-config.ts index c655ace9c7e..4c17c2bc77e 100644 --- a/src/commands/onboard-non-interactive/local/gateway-config.ts +++ b/src/commands/onboard-non-interactive/local/gateway-config.ts @@ -1,5 +1,5 @@ import type { OpenClawConfig } from "../../../config/types.openclaw.js"; -import { isValidEnvSecretRefId } from "../../../config/types.secrets.js"; +import { isValidEnvSecretRefId, resolveSecretInputRef } from "../../../config/types.secrets.js"; import type { RuntimeEnv } from "../../../runtime.js"; import { resolveDefaultSecretProviderAlias } from "../../../secrets/ref-contract.js"; import { normalizeOptionalString } from "../../../shared/string-coerce.js"; @@ -54,8 +54,18 @@ export function applyNonInteractiveGatewayConfig(params: { let nextConfig = params.nextConfig; const explicitGatewayToken = normalizeGatewayTokenInput(opts.gatewayToken); const envGatewayToken = normalizeGatewayTokenInput(process.env.OPENCLAW_GATEWAY_TOKEN); - const existingToken = normalizeGatewayTokenInput(nextConfig?.gateway?.auth?.token); - let gatewayToken = explicitGatewayToken || envGatewayToken || existingToken || undefined; + const existingTokenInput = nextConfig.gateway?.auth?.token; + const existingTokenRef = resolveSecretInputRef({ + value: existingTokenInput, + defaults: nextConfig.secrets?.defaults, + }).ref; + const existingPlaintextToken = normalizeGatewayTokenInput(existingTokenInput); + // Resolution order on re-onboard: explicit --gateway-token > persisted + // plaintext > ambient OPENCLAW_GATEWAY_TOKEN > randomToken(). Ambient env + // must not rotate a token already written to disk — a stale shell or + // launchd env var otherwise breaks already-paired clients. + let gatewayToken = + explicitGatewayToken || existingPlaintextToken || envGatewayToken || undefined; const gatewayTokenRefEnv = normalizeOptionalString(opts.gatewayTokenRefEnv ?? "") ?? ""; if (authMode === "token") { @@ -96,6 +106,32 @@ export function applyNonInteractiveGatewayConfig(params: { }, }, }; + } else if (!explicitGatewayToken && existingTokenRef) { + // Preserve an already-configured SecretRef on re-onboard. Without this + // branch, an ambient OPENCLAW_GATEWAY_TOKEN (or randomToken() fallback) + // would silently overwrite {source, provider, id} with a plaintext + // literal, de-secretref-ing the gateway. + nextConfig = { + ...nextConfig, + gateway: { + ...nextConfig.gateway, + auth: { + ...nextConfig.gateway?.auth, + mode: "token", + // token field intentionally preserved as the existing SecretRef. + }, + }, + }; + // Resolve env-source refs inline for the health probe only — do not + // persist any plaintext here. Other ref sources (file/exec) defer to + // the gateway's own resolver; the health probe may then fail unless + // --skip-health is set. + if (existingTokenRef.source === "env") { + const resolved = process.env[existingTokenRef.id]?.trim(); + gatewayToken = resolved || undefined; + } else { + gatewayToken = undefined; + } } else { if (!gatewayToken) { gatewayToken = randomToken();