fix(gateway): harden token fallback/reconnect behavior and docs by joshavant · Pull Request #42507 · openclaw/openclaw

joshavant · 2026-03-10T21:26:15Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: Gateway auth drift around shared token vs paired device token caused token_mismatch, reconnect churn, and inconsistent Control UI/CLI/macOS behavior.
Why it matters: auth regressions here break core operator connectivity and can create noisy reconnect loops during token churn/restart windows.
What changed: implemented bounded trusted device-token fallback retries, added structured server recovery hints, tightened reconnect gating, and added compatibility/regression tests plus docs runbook updates.
What did NOT change (scope boundary): no rollout flag/gradual enablement; no broad auth model redesign; no pairing-store format migration.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Shared-token clients now keep explicit shared-token precedence on first connect attempt.
On AUTH_TOKEN_MISMATCH, trusted clients can do one bounded retry with cached device token when server hints permit it.
After retry budget is exhausted or nonrecoverable auth errors occur, clients stop reconnect loops and surface actionable auth failures.
Cached device tokens are no longer cleared on generic shared-auth mismatch paths; clearing is limited to explicit device-token mismatch outcomes.
Server connect auth errors now include recovery hints in error.details (canRetryWithDeviceToken, recommendedNextStep).
Added compatibility baseline auth suite and docs clarifications/runbooks for token drift recovery.

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (Yes)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation:
- Risk: fallback retry logic could accidentally over-send device tokens or create retry loops.
- Mitigation: retry is bounded (single budget), gated to trusted endpoints, and reconnect is paused for nonrecoverable cases / exhausted mismatch retries.

Repro + Verification

Environment

OS: macOS (dev host)
Runtime/container: Node 22 + pnpm, Vitest, Swift lint/format checks
Model/provider: N/A
Integration/channel (if any): Gateway WS auth path (CLI, Control UI, macOS shared kit)
Relevant config (redacted): gateway.auth.mode=token, shared token configured, paired device token present

Steps

Configure shared gateway token and pair device/client.
Attempt connect with mismatched shared token but valid cached paired device token.
Observe bounded retry and final reconnect gating behavior.

Expected

First attempt uses shared token only.
If server indicates AUTH_TOKEN_MISMATCH with retry hint, one trusted retry adds auth.deviceToken.
On continued auth failure, loop is suppressed and actionable error remains.

Actual

Matches expected behavior across server, JS client, Control UI browser client, and Swift channel path.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
- pnpm test:auth:compat
- pnpm vitest run --config vitest.gateway.config.ts src/gateway/server.auth.control-ui.test.ts src/gateway/server.auth.compat-baseline.test.ts src/gateway/client.test.ts src/gateway/reconnect-gating.test.ts src/gateway/protocol/connect-error-details.test.ts
- pnpm --dir ui test gateway.node.test.ts app-gateway.node.test.ts views/overview.node.test.ts
- pnpm tsgo
- pnpm check
- swiftlint lint --config .swiftlint.yml apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift
- swiftformat --lint --config .swiftformat apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift
Edge cases checked:
- no unbounded reconnect on nonrecoverable auth errors
- no pairing clear side-effect on shared token mismatch close reason
- retry trusted-endpoint gating
- docs anchor/link integrity for new runbook additions
What you did not verify:
- full repository test matrix beyond touched auth/UI paths

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly:
- Revert this PR commit range.
Files/config to restore:
- src/gateway/client.ts
- src/gateway/server/ws-connection/message-handler.ts
- ui/src/ui/gateway.ts
- apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift
Known bad symptoms reviewers should watch for:
- repeated auth reconnect loops after mismatch
- valid paired device cannot recover after shared token mismatch
- Control UI retries on untrusted endpoints

Risks and Mitigations

Risk: retry heuristics become too permissive in future clients.
- Mitigation: server emits structured recovery hints and tests lock expected behavior for trusted-only one-shot retry.
Risk: docs drift from implementation around insecure-auth and token-drift handling.
- Mitigation: updated gateway/web/help/cli docs with code-aligned semantics and linked checklists.

aisle-research-bot · 2026-03-10T21:26:22Z

🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

#	Severity	Title
1	🟠 High	Device token exfiltration via naive loopback hostname check (`host.hasPrefix("127.")`)
2	🟠 High	Device token exfiltration via naive loopback hostname check in Control UI gateway retry logic

1. 🟠 Device token exfiltration via naive loopback hostname check (`host.hasPrefix("127.")`)

Property	Value
Severity	High
CWE	CWE-807
Location	`apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift:771-783`

Description

The Swift gateway client introduced a trust gate (isTrustedDeviceRetryEndpoint()) to decide whether it is safe to attach a cached per-device credential (auth.deviceToken) on an authentication retry.

However, the loopback check is implemented using a string prefix test:

URL.host returns the hostname label for a URL such as ws://127.evil.com:18789 (e.g., "127.evil.com"), not an IP literal.
The check host.hasPrefix("127.") will therefore treat attacker-controlled hostnames beginning with 127. as “loopback trusted”.
If a user is tricked into using such a URL, a malicious gateway can respond with AUTH_TOKEN_MISMATCH / canRetryWithDeviceToken to trigger the retry path, causing the client to send the cached deviceToken (and also the shared token) to the attacker-controlled server.

Vulnerable code (trust decision):

if host == "localhost" || host == "::1" || host == "127.0.0.1" || host.hasPrefix("127.") {
    return true
}

Security impact:

Leakage of a stored per-device authentication token to a remote attacker (credential exfiltration via trust-check bypass).
The decision is security-critical because it gates whether auth["deviceToken"] is added to the connect request.

Recommendation

Do not use string prefix checks for loopback detection. Only treat the endpoint as trusted when the host is an IP literal that parses as loopback (or when a hostname is resolved and all resolved addresses are loopback).

Safer example using Network’s IP address parsers (rejects 127.evil.com because it is not an IP literal):

import Network

private func isTrustedDeviceRetryEndpoint() -> Bool {
    guard let host = url.host?.trimmingCharacters(in: .whitespacesAndNewlines).lowercased(),
          !host.isEmpty else { return false }

    // If you want to allow localhost, consider resolving and verifying loopback.
    if host == "localhost" { return true }

    if let ipv4 = IPv4Address(host) {
        // 127.0.0.0/8
        return ipv4.rawValue.first == 127
    }

    if let ipv6 = IPv6Address(host) {
        return ipv6 == IPv6Address("::1")
    }

    return false
}

If hostname support is required beyond localhost, resolve via getaddrinfo and only trust when all returned addresses are loopback. Also ensure IPv6 bracket forms are handled correctly by relying on url.host (already unbracketed) and IP parsing rather than string patterns.

2. 🟠 Device token exfiltration via naive loopback hostname check in Control UI gateway retry logic

Property	Value
Severity	High
CWE	CWE-807
Location	`ui/src/ui/gateway.ts:81-93`

Description

The Control UI’s bounded device-token retry logic can misclassify attacker-controlled hostnames as “trusted loopback” and send a cached per-device token to them.

isTrustedRetryEndpoint() treats any hostname starting with "127." as loopback (host.startsWith("127.")).
For a URL like wss://127.evil.com:18789, new URL(...).hostname is "127.evil.com", so this check returns true even though it is a normal DNS name.
When a connect attempt fails with AUTH_TOKEN_MISMATCH (or server-provided canRetryWithDeviceToken / recommendedNextStep), the client may perform a one-time retry that includes auth.deviceToken loaded from localStorage (openclaw.device.auth.v1).
This causes the cached device token to be transmitted to the attacker-controlled WebSocket endpoint, enabling token theft.

Notes:

host === "[::1]" is dead code because URL.hostname for bracketed IPv6 literals (e.g. ws://[::1]:18789) is typically "::1" (no brackets).

Vulnerable code:

const host = gatewayUrl.hostname.trim().toLowerCase();
const isLoopbackHost =
  host === "localhost" || host === "::1" || host === "[::1]" || host === "127.0.0.1";
const isLoopbackIPv4 = host.startsWith("127.");
if (isLoopbackHost || isLoopbackIPv4) {
  return true;
}

Impact:

Secret exposure of auth.deviceToken (stored in localStorage for the Control UI origin) to an attacker-selected endpoint, if the user is tricked into confirming a malicious gatewayUrl such as wss://127.evil.com/....

Recommendation

Harden loopback/trust detection to only treat IP literals in the loopback ranges as loopback, and avoid prefix matching on arbitrary hostnames.

Suggested approach:

function isLoopbackHostname(hostname: string): boolean {
  const h = hostname.trim().toLowerCase();
  if (h === "localhost" || h === "::1") return true;

  // Only treat numeric IPv4 literals as candidates for 127.0.0.0/8.
  if (!/^\d{1,3}(?:\.\d{1,3}){3}$/.test(h)) return false;
  const parts = h.split(".").map((p) => Number(p));
  if (parts.some((p) => !Number.isInteger(p) || p < 0 || p > 255)) return false;
  return parts[0] === 127;
}

function isTrustedRetryEndpoint(url: string): boolean {
  const gatewayUrl = new URL(url, window.location.href);
  const pageUrl = new URL(window.location.href);

  if (isLoopbackHostname(gatewayUrl.hostname)) {
    return true;
  }

  // Same host+port as the Control UI (optionally also enforce wss when page is https).
  if (gatewayUrl.hostname === pageUrl.hostname && gatewayUrl.port === pageUrl.port) {
    if (pageUrl.protocol === "https:" && gatewayUrl.protocol !== "wss:") return false;
    return true;
  }

  return false;
}

Additionally:

Remove the unused "[::1]" branch (or switch to comparing against gatewayUrl.host if brackets are intended).
Consider requiring wss: for non-loopback endpoints before ever sending auth.deviceToken.

Analyzed PR: #42507 at commit 4bcbb5a

_{Last updated on: 2026-03-10T22:49:59Z}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c3a5363f3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift

ui/src/ui/gateway.ts

greptile-apps · 2026-03-10T21:40:36Z

Greptile Summary

This PR hardens gateway authentication by implementing a bounded one-shot device-token fallback retry when a shared-token AUTH_TOKEN_MISMATCH occurs, adding structured server recovery hints (canRetryWithDeviceToken, recommendedNextStep) to connect error responses, tightening reconnect gating to suppress loops after budget exhaustion or non-recoverable errors, and updating docs/runbooks across CLI, gateway, web, and help pages.

The implementation is well-structured across all three client platforms (Node.js, browser, Swift) with proper gating on trusted endpoints and stored token availability. The server-side recovery hints and retry state machine logic are sound. However, there is a behavioral difference in how the browser client handles AUTH_TOKEN_MISMATCH when no device-token retry is possible: it suppresses reconnect attempts immediately, whereas the Swift client continues retrying until rate-limited. For unpaired browser clients encountering transient mismatches (server restart, token rotation), this prevents auto-recovery without manual intervention.

Confidence Score: 4/5

The auth retry logic is sound and well-gated, with proper server-side hints and client-side guards on stored tokens. One identified concern about browser client behavior on transient mismatches does not present a functional regression but indicates a design choice that differs from the Swift platform.
The core auth/retry implementation across all three client platforms is correct and properly bounded with budget tracking and endpoint trust checks. The server-side recovery hints are well-implemented. Mocked tests in the Node.js client work correctly due to automatic device identity creation in the constructor. One notable design difference exists in how the browser client handles AUTH_TOKEN_MISMATCH when no retry is possible (immediate suppression vs. Swift's continued retry), but this appears intentional based on code comments and does not create a functional regression — it is a platform-specific behavior choice.
ui/src/ui/gateway.ts — The early-return guard for AUTH_TOKEN_MISMATCH suppresses reconnects more aggressively than documented, with no test coverage for the unpaired-client path.

_{Last reviewed commit: 2c3a536}

ui/src/ui/gateway.ts

joshavant · 2026-03-10T21:45:47Z

Addressed the follow-up feedback in ec588e7:

Added Node client reconnect gating for terminal auth failures:
- tracks connect error detail codes
- pauses reconnect on nonrecoverable auth failures
- pauses AUTH_TOKEN_MISMATCH loops when retry is untrusted or retry budget is exhausted
- tests added in src/gateway/client.test.ts
Fixed browser trusted endpoint parsing for IPv6 loopback ([::1]) and added coverage in ui/src/ui/gateway.node.test.ts.
Adjusted browser mismatch suppression to only pause after retry-budget exhaustion; added test coverage for first mismatch with no stored token (reconnect continues).
Fixed Swift reconnect churn by introducing reconnectPausedForAuthFailure and gating both scheduleReconnect() and watchdogLoop().
Added a clarifying Swift comment that retry trust is currently loopback-only by design (no pinned TLS-fingerprint trust path in this client yet).

Validation run:

pnpm vitest run --config vitest.gateway.config.ts src/gateway/client.test.ts src/gateway/reconnect-gating.test.ts src/gateway/protocol/connect-error-details.test.ts
pnpm --dir ui test gateway.node.test.ts app-gateway.node.test.ts
swiftlint lint --config .swiftlint.yml apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift
swiftformat --lint --config .swiftformat apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift
pnpm tsgo
pnpm check

joshavant · 2026-03-10T22:06:28Z

Landed via temp rebase onto main.

Gate: pnpm lint && pnpm build && pnpm test
Land commit: 4bcbb5a
Merge commit: a76e810

Thanks @joshavant!

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4bcbb5af17

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-10T22:16:36Z

src/gateway/client.ts

+      (shouldUseDeviceRetryToken || !(explicitGatewayToken || this.opts.password?.trim())
        ? (storedToken ?? undefined)
        : undefined);


Clear stale token after explicit retry mismatch

This new retry path attaches a cached deviceToken even when an explicit shared token is set, but the stale-token cleanup path still requires !this.opts.token, so an AUTH_DEVICE_TOKEN_MISMATCH on that retry leaves the bad cached token in place. In practice, clients with a wrong shared token plus stale cached device token will repeat the doomed fallback on every process restart (burning the one-shot retry and adding extra reconnect churn) instead of invalidating the stale device token after the explicit retry fails.

Useful? React with 👍 / 👎.

* main: (42 commits) test: share runtime group policy fallback cases refactor: share windows command shim resolution refactor: share approval gateway client setup refactor: share telegram payload send flow refactor: share passive account lifecycle helpers refactor: share channel config schema fragments refactor: share channel config security scaffolding refactor: share onboarding secret prompt flows refactor: share scoped account config patching feat(discord): add autoArchiveDuration config option (openclaw#35065) fix(gateway): harden token fallback/reconnect behavior and docs (openclaw#42507) fix(acp): strip provider auth env for child ACP processes (openclaw#42250) fix(browser): surface 429 rate limit errors with actionable hints (openclaw#40491) fix(acp): scope cancellation and event routing by runId (openclaw#41331) docs: require codex review in contributing guide (openclaw#42503) Fix stale runtime model reuse on session reset (openclaw#41173) docs: document r: spam auto-close label fix(ci): auto-close and lock r: spam items fix(acp): implicit streamToParent for mode=run without thread (openclaw#42404) test: extract sendpayload outbound contract suite ...

@joshavant