From f6974fb3550fe089663bc56ce89d8e1ec79170de Mon Sep 17 00:00:00 2001 From: Kevin Date: Sat, 26 Sep 2026 20:39:25 -0700 Subject: [PATCH] fix(node): reconnect after fallback pairing code expires (#159360) --- docs/cli/node.md | 8 ++- src/cli/node-cli/gateway-options.ts | 9 ++- src/cli/node-cli/register.test.ts | 59 +++++++++++++++++ src/cli/node-cli/register.ts | 7 +- src/node-host/runner.test-support.ts | 5 ++ src/node-host/runner.test.ts | 96 ++++++++++++++++++++++++++++ src/node-host/runner.ts | 90 +++++++++++++++++++------- src/pairing/setup-code.ts | 4 +- 8 files changed, 249 insertions(+), 29 deletions(-) diff --git a/docs/cli/node.md b/docs/cli/node.md index e0e3883f6a32..7c032a67ed90 100644 --- a/docs/cli/node.md +++ b/docs/cli/node.md @@ -122,8 +122,12 @@ persisted in service arguments. For a managed foreground process, `--pair-if-needed` reuses native device-token storage across restarts; it does not keep a separate enrollment marker. Preserve -the node state directory. An expired setup code cannot enroll a new state -directory or replace a revoked device token; provision a fresh code when needed. +the node state directory. After the setup code expires, the node can still reconnect +when its saved identity and node token exist and every selected Gateway endpoint +matches the saved Gateway scope. The expired bootstrap token is never sent. +An expired setup code cannot enroll a new state directory or replace a revoked +device token; provision a fresh code when needed. Explicit `--pair` still rejects +expired setup codes. `openclaw node run` and `openclaw node install` resolve gateway auth from config/env (no `--token`/`--password` flags on node commands): diff --git a/src/cli/node-cli/gateway-options.ts b/src/cli/node-cli/gateway-options.ts index c301038a4ef7..b5d6fc7964f7 100644 --- a/src/cli/node-cli/gateway-options.ts +++ b/src/cli/node-cli/gateway-options.ts @@ -23,6 +23,7 @@ type NodePairGatewayOptions = { tls: boolean; tlsFingerprint?: string; bootstrapToken: string; + expiresAtMs?: number; candidates: NodeHostGatewayConfig[]; }; @@ -40,8 +41,11 @@ function gatewayConfigFromUrl(url: string, tlsFingerprint?: string): NodeHostGat }; } -export function resolveNodePairGatewayOptions(input: string): NodePairGatewayOptions { - return resolveNodePairGatewayPayload(decodePairingSetupCode(input)); +export function resolveNodePairGatewayOptions( + input: string, + options: { allowExpired?: boolean } = {}, +): NodePairGatewayOptions { + return resolveNodePairGatewayPayload(decodePairingSetupCode(input, options)); } /** Project a validated pairing payload into the canonical node-host candidate list. */ @@ -59,6 +63,7 @@ export function resolveNodePairGatewayPayload( tls: primary.tls ?? false, ...(primary.tlsFingerprint ? { tlsFingerprint: primary.tlsFingerprint } : {}), bootstrapToken: payload.bootstrapToken, + ...(payload.expiresAtMs !== undefined ? { expiresAtMs: payload.expiresAtMs } : {}), candidates, }; } diff --git a/src/cli/node-cli/register.test.ts b/src/cli/node-cli/register.test.ts index 16f3dfa77c7b..509ea72284a1 100644 --- a/src/cli/node-cli/register.test.ts +++ b/src/cli/node-cli/register.test.ts @@ -338,6 +338,65 @@ describe("registerNodeCli", () => { ); }); + it("defers an expired fallback code to the node host credential owner", async () => { + const setupCode = encodePairingSetupCode({ + url: "wss://paired.example/node", + bootstrapToken: "expired-test-bootstrap", + expiresAtMs: 1, + }); + + await createProgram().parseAsync(["node", "run", "--pair-if-needed", setupCode], { + from: "user", + }); + + expect(daemonMocks.defaultRuntime.error).not.toHaveBeenCalled(); + expect(daemonMocks.runNodeHost).toHaveBeenCalledWith( + expect.objectContaining({ + gatewayHost: "paired.example", + gatewayBootstrapToken: "expired-test-bootstrap", + gatewayBootstrapExpiresAtMs: 1, + preferGatewayBootstrapToken: false, + }), + ); + }); + + it.each([ + { urls: ["wss://paired.example/node", 42] }, + { tlsFingerprint: "invalid-pin" }, + { expiresAtMs: -1 }, + ])("rejects malformed fallback payload fields: %j", async (invalidFields) => { + const setupCode = Buffer.from( + JSON.stringify({ + url: "wss://paired.example/node", + bootstrapToken: "expired-test-bootstrap", + expiresAtMs: 1, + ...invalidFields, + }), + ).toString("base64url"); + + await createProgram().parseAsync(["node", "run", "--pair-if-needed", setupCode], { + from: "user", + }); + + expect(daemonMocks.defaultRuntime.error).toHaveBeenCalledWith("Invalid pairing setup payload."); + expect(daemonMocks.runNodeHost).not.toHaveBeenCalled(); + }); + + it("rejects an expired explicit pairing code before starting the node host", async () => { + const setupCode = encodePairingSetupCode({ + url: "wss://paired.example/node", + bootstrapToken: "expired-test-bootstrap", + expiresAtMs: 1, + }); + + await createProgram().parseAsync(["node", "run", "--pair", setupCode], { from: "user" }); + + expect(daemonMocks.defaultRuntime.error).toHaveBeenCalledWith( + "Pairing setup code has expired.", + ); + expect(daemonMocks.runNodeHost).not.toHaveBeenCalled(); + }); + it("rejects simultaneous forced and resumable pairing", async () => { await expect( createProgram().parseAsync(["node", "run", "--pair", "first", "--pair-if-needed", "second"], { diff --git a/src/cli/node-cli/register.ts b/src/cli/node-cli/register.ts index 8326e45e0aa1..296c103ff5b7 100644 --- a/src/cli/node-cli/register.ts +++ b/src/cli/node-cli/register.ts @@ -74,7 +74,11 @@ export function registerNodeCli(program: Command) { let gatewayOptions; try { const setupCode = opts.pair ?? opts.pairIfNeeded; - pair = setupCode ? resolveNodePairGatewayOptions(setupCode) : undefined; + pair = setupCode + ? resolveNodePairGatewayOptions(setupCode, { + allowExpired: opts.pairIfNeeded !== undefined, + }) + : undefined; const existing = await loadNodeHostConfig(); gatewayOptions = resolveNodeGatewayOptions(opts, existing, pair); } catch (error) { @@ -104,6 +108,7 @@ export function registerNodeCli(program: Command) { gatewayCloudflareAccess: cloudflareAccess, gatewayCandidates, gatewayBootstrapToken: pair?.bootstrapToken, + gatewayBootstrapExpiresAtMs: pair?.expiresAtMs, preferGatewayBootstrapToken: opts.pair !== undefined, ...(opts.ephemeral === true || opts.sessionHost === true ? { forceWorkerRuns: true } : {}), ...(opts.ephemeral === true ? { ephemeral: true } : {}), diff --git a/src/node-host/runner.test-support.ts b/src/node-host/runner.test-support.ts index a833d6911e8e..2d8dedd158b0 100644 --- a/src/node-host/runner.test-support.ts +++ b/src/node-host/runner.test-support.ts @@ -38,6 +38,9 @@ const mocks = vi.hoisted(() => ({ closeMcpManager: vi.fn(async () => undefined), loadNodeHostConfig: vi.fn<() => Promise>(async () => null), loadDeviceAuthTokenReadOnly: vi.fn(async () => null), + loadDeviceIdentityIfPresent: vi.fn( + () => null as { deviceId: string; publicKeyPem: string; privateKeyPem: string } | null, + ), configureNodeHost: vi.fn(async (params: Parameters[0]) => { mocks.capturedConfiguredGatewayConfigs.push(params.gateway); return { @@ -111,6 +114,7 @@ vi.mock("../infra/device-auth-store.js", async (importOriginal) => ({ vi.mock("../infra/device-identity.js", async (importOriginal) => ({ ...(await importOriginal()), + loadDeviceIdentityIfPresent: mocks.loadDeviceIdentityIfPresent, loadOrCreateDeviceIdentity: vi.fn(() => ({ deviceId: "device-test", publicKeyPem: "public-key-test", @@ -263,6 +267,7 @@ export function resetRunnerTestState() { vi.clearAllMocks(); mocks.loadNodeHostConfig.mockReset().mockResolvedValue(null); mocks.loadDeviceAuthTokenReadOnly.mockReset().mockResolvedValue(null); + mocks.loadDeviceIdentityIfPresent.mockReset().mockReturnValue(null); mocks.getRuntimeConfig.mockReturnValue({ gateway: { handshakeTimeoutMs: 1_000 }, }); diff --git a/src/node-host/runner.test.ts b/src/node-host/runner.test.ts index a6129982a70d..be11f5dbd4f9 100644 --- a/src/node-host/runner.test.ts +++ b/src/node-host/runner.test.ts @@ -410,6 +410,102 @@ describe("runNodeHost", () => { mocks.resolveGatewayCredentialsWithSecretInputs.mockResolvedValue({}); }); + describe("expired fallback setup codes", () => { + const expiredOptions = { + ...runOptions, + gatewayBootstrapToken: "expired-test-bootstrap", + gatewayBootstrapExpiresAtMs: 1, + preferGatewayBootstrapToken: false, + }; + + beforeEach(() => { + mocks.loadDeviceIdentityIfPresent.mockReturnValue({ + deviceId: "device-test", + publicKeyPem: "public-key-test", + privateKeyPem: "private-key-test", + }); + }); + + it("reconnects with the saved token without submitting the expired bootstrap", async () => { + await expect(runNodeHost(expiredOptions)).rejects.toThrow("event loop readiness timeout"); + + const options = lastCapturedOptions(); + expect(options?.bootstrapToken).toBeUndefined(); + const auth = buildGatewayConnectAuth( + selectGatewayConnectAuth({ ...options, storedToken: "paired-node-token" }), + ); + expect(auth).toMatchObject({ deviceToken: "paired-node-token", bootstrapToken: undefined }); + expect(mocks.resolveGatewayCredentialsWithSecretInputs).not.toHaveBeenCalled(); + }); + + it("never falls back to shared auth if the saved token disappears before connect", async () => { + mocks.loadDeviceAuthTokenReadOnly + .mockResolvedValueOnce({ + role: "node", + token: "paired-node-token", + scopes: [], + updatedAtMs: 1, + }) + .mockResolvedValue(null); + vi.stubEnv("OPENCLAW_GATEWAY_TOKEN", "shared-test-token"); + + await expect(runNodeHost(expiredOptions)).rejects.toThrow("event loop readiness timeout"); + + // GatewayClient rereads native auth when connecting; the token can be gone by then. + const stored = await mocks.loadDeviceAuthTokenReadOnly({ + deviceId: "device-test", + role: "node", + }); + const options = lastCapturedOptions(); + expect(options?.bootstrapToken).toBeUndefined(); + const auth = buildGatewayConnectAuth( + selectGatewayConnectAuth({ ...options, storedToken: stored?.token }), + ); + expect(auth).toBeUndefined(); + expect(mocks.resolveGatewayCredentialsWithSecretInputs).not.toHaveBeenCalled(); + }); + + it.each(["identity", "token", "saved gateway"])( + "rejects before changing node state when the saved %s is missing", + async (missing) => { + if (missing === "identity") { + mocks.loadDeviceIdentityIfPresent.mockReturnValue(null); + } else if (missing === "token") { + mocks.loadDeviceAuthTokenReadOnly.mockResolvedValue(null); + } else { + mocks.loadNodeHostConfig.mockResolvedValue(null); + } + + await expect(runNodeHost(expiredOptions)).rejects.toThrow( + "Pairing setup code has expired.", + ); + expect(mocks.configureNodeHost).not.toHaveBeenCalled(); + expect(mocks.capturedGatewayClients).toHaveLength(0); + }, + ); + + it.each([ + { candidates: [{ ...gateway, host: "other.example" }] }, + { candidates: [gateway, { ...gateway, contextPath: "/other-node" }] }, + ])( + "rejects a changed or mixed gateway candidate scope: $candidates", + async ({ candidates }) => { + await expect( + runNodeHost({ ...expiredOptions, gatewayCandidates: candidates }), + ).rejects.toThrow("Pairing setup code has expired."); + expect(mocks.configureNodeHost).not.toHaveBeenCalled(); + expect(mocks.capturedGatewayClients).toHaveLength(0); + }, + ); + + it("does not relax forced pairing even when a saved token exists", async () => { + await expect( + runNodeHost({ ...expiredOptions, preferGatewayBootstrapToken: true }), + ).rejects.toThrow("Pairing setup code has expired."); + expect(mocks.configureNodeHost).not.toHaveBeenCalled(); + }); + }); + it("restarts a paired service without sending the source Gateway password", async () => { await expect(runNodeHost(runOptions)).rejects.toThrow("event loop readiness timeout"); diff --git a/src/node-host/runner.ts b/src/node-host/runner.ts index 6b52cff9c0f6..9a5e5bb2a348 100644 --- a/src/node-host/runner.ts +++ b/src/node-host/runner.ts @@ -16,7 +16,10 @@ import { GatewayClientRequestError } from "../gateway/client.js"; import { resolveGatewayCredentialsWithSecretInputs } from "../gateway/credentials-secret-inputs.js"; import { resolveExplicitGatewayAuth } from "../gateway/credentials.js"; import { loadDeviceAuthTokenReadOnly } from "../infra/device-auth-store.js"; -import { loadOrCreateDeviceIdentity } from "../infra/device-identity.js"; +import { + loadDeviceIdentityIfPresent, + loadOrCreateDeviceIdentity, +} from "../infra/device-identity.js"; import { formatErrorMessage } from "../infra/errors.js"; import { getMachineDisplayName } from "../infra/machine-name.js"; import { logInfo } from "../logger.js"; @@ -54,6 +57,7 @@ type NodeHostRunOptions = { gatewayCloudflareAccess?: NodeHostCloudflareAccessConfig; gatewayCandidates?: NodeHostGatewayConfig[]; gatewayBootstrapToken?: string; + gatewayBootstrapExpiresAtMs?: number; preferGatewayBootstrapToken?: boolean; /** Stop cleanly after the first authenticated hello (used before service install). */ stopAfterFirstConnect?: boolean; @@ -87,6 +91,30 @@ const NODE_HOST_EXIT_ON_RECONNECT_PAUSE_CODES: ReadonlySet = new Set([ ConnectErrorDetailCodes.CLIENT_VERSION_MISMATCH, ]); +async function canReuseNodeHostDeviceToken(params: { + savedGateway?: NodeHostGatewayConfig; + gatewayCandidates: readonly NodeHostGatewayConfig[]; + deviceId: string; + env?: NodeJS.ProcessEnv; +}): Promise { + const savedGatewayScope = params.savedGateway + ? gatewayOriginScope(formatGatewayCandidateUrl(params.savedGateway)) + : undefined; + return Boolean( + savedGatewayScope && + params.gatewayCandidates.every( + (candidate) => gatewayOriginScope(formatGatewayCandidateUrl(candidate)) === savedGatewayScope, + ) && + ( + await loadDeviceAuthTokenReadOnly({ + deviceId: params.deviceId, + role: "node", + env: params.env ?? process.env, + }) + )?.token, + ); +} + async function resolveNodeHostGatewayCredentials(params: { config: OpenClawConfig; savedGateway?: NodeHostGatewayConfig; @@ -102,16 +130,7 @@ async function resolveNodeHostGatewayCredentials(params: { password: env.OPENCLAW_GATEWAY_PASSWORD, }); } - const savedGatewayScope = params.savedGateway - ? gatewayOriginScope(formatGatewayCandidateUrl(params.savedGateway)) - : undefined; - if ( - savedGatewayScope && - params.gatewayCandidates.every( - (candidate) => gatewayOriginScope(formatGatewayCandidateUrl(candidate)) === savedGatewayScope, - ) && - (await loadDeviceAuthTokenReadOnly({ deviceId: params.deviceId, role: "node", env }))?.token - ) { + if (await canReuseNodeHostDeviceToken(params)) { // A co-located Gateway's shared password must not displace the paired node // credential. GatewayClient rereads the current token when connecting. return resolveExplicitGatewayAuth({ @@ -161,6 +180,32 @@ export async function runNodeHost(opts: NodeHostRunOptions): Promise { contextPath: opts.gatewayContextPath, cloudflareAccess: opts.gatewayCloudflareAccess, }; + let gatewayBootstrapToken = opts.gatewayBootstrapToken; + let reuseDeviceTokenOnly = false; + if ( + gatewayBootstrapToken && + opts.gatewayBootstrapExpiresAtMs !== undefined && + opts.gatewayBootstrapExpiresAtMs <= Date.now() + ) { + const identity = loadDeviceIdentityIfPresent(); + if ( + opts.preferGatewayBootstrapToken || + opts.gatewayAuthFromEnv || + !identity || + !(await canReuseNodeHostDeviceToken({ + savedGateway: savedConfig?.gateway, + gatewayCandidates: opts.gatewayCandidates?.length + ? opts.gatewayCandidates + : [plannedGateway], + deviceId: identity.deviceId, + })) + ) { + throw new Error("Pairing setup code has expired."); + } + // The existing pairing can reconnect; never submit its expired fallback bearer. + gatewayBootstrapToken = undefined; + reuseDeviceTokenOnly = true; + } const fallbackDisplayName = await getMachineDisplayName(); const config = await configureNodeHost({ nodeId: opts.nodeId, @@ -219,16 +264,17 @@ export async function runNodeHost(opts: NodeHostRunOptions): Promise { }); logInfo(`node-host: advertised commands: ${preparedRuntime.manifest.commands.join(", ")}`); const deviceIdentity = loadOrCreateDeviceIdentity(); - const { token, password } = opts.gatewayBootstrapToken - ? {} - : await resolveNodeHostGatewayCredentials({ - config: cfg, - envOnly: opts.gatewayAuthFromEnv, - savedGateway: savedConfig?.gateway, - gatewayCandidates, - deviceId: deviceIdentity.deviceId, - env: process.env, - }); + const { token, password } = + gatewayBootstrapToken || reuseDeviceTokenOnly + ? {} + : await resolveNodeHostGatewayCredentials({ + config: cfg, + envOnly: opts.gatewayAuthFromEnv, + savedGateway: savedConfig?.gateway, + gatewayCandidates, + deviceId: deviceIdentity.deviceId, + env: process.env, + }); let consecutivePermanentGatewayRejections = 0; const autoUpdateAbort = new AbortController(); @@ -251,7 +297,7 @@ export async function runNodeHost(opts: NodeHostRunOptions): Promise { cloudflareAccessByCandidate, clientOptions: { token: token || undefined, - bootstrapToken: opts.gatewayBootstrapToken, + bootstrapToken: gatewayBootstrapToken, preferBootstrapToken: opts.preferGatewayBootstrapToken, password: password || undefined, instanceId: nodeId, diff --git a/src/pairing/setup-code.ts b/src/pairing/setup-code.ts index 5a89eefc8e7b..9158f7283aef 100644 --- a/src/pairing/setup-code.ts +++ b/src/pairing/setup-code.ts @@ -384,7 +384,7 @@ const PAIRING_SETUP_CODE_RE = /^[A-Za-z0-9_-]+$/u; /** Decode the current setup payload plus additive fields emitted by older pairing surfaces. */ export function decodePairingSetupCode( input: string, - options: { nowMs?: number } = {}, + options: { nowMs?: number; allowExpired?: boolean } = {}, ): PairingSetupPayload { const trimmed = input.trim(); const setupCode = trimmed.toLowerCase().startsWith(PAIRING_SETUP_URL_PREFIX) @@ -432,7 +432,7 @@ export function decodePairingSetupCode( throw new Error("Invalid pairing setup payload."); } expiresAtMs = candidate; - if (candidate <= (options.nowMs ?? Date.now())) { + if (!options.allowExpired && candidate <= (options.nowMs ?? Date.now())) { throw new Error("Pairing setup code has expired."); } }