fix(pairing): align mobile setup with secure endpoints#60128
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0821b85e15
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile SummaryThis PR closes the gap between Android's non-loopback TLS requirement and the pairing setup-code producer by failing closed on insecure remote Confidence Score: 5/5Safe to merge — security fix is correctly scoped, all call sites updated, and test coverage is solid. No P0/P1 issues found. The core validation logic in both TypeScript and Kotlin is correct, exception classification in GatewayTls.kt is appropriate, and the three Android input sources (QR, setup code, manual) all share the same enforcement path. The http→ws/https→wss conversion inside validateMobilePairingUrl is dead code in practice but harmless. The GatewayTlsProbeResult data class allows a both-null state, but the null arm in the when expression handles it gracefully. All remaining observations are P2 or lower. No files require special attention. Reviews (1): Last reviewed commit: "fix(pairing): align mobile setup with se..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05142fbdd9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8e0709d to
e67e0a2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 525f999c03
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| parsed = new URL(url); | ||
| } catch { | ||
| return "Resolved mobile pairing URL is invalid."; |
There was a problem hiding this comment.
Handle scoped IPv6 hosts before URL parsing
validateMobilePairingUrl now rejects any resolved setup URL that contains a scoped IPv6 literal (for example ws://[fe80::1%25wlan0]:18789) because new URL(...) throws before the host allowlist runs. This is a regression from the previous flow (which could emit these URLs), and it breaks private-LAN pairing on link-local IPv6 networks where a zone ID is required. Fresh evidence in this commit is the new parse-then-reject branch in src/pairing/setup-code.ts that returns "Resolved mobile pairing URL is invalid." on parse failure.
Useful? React with 👍 / 👎.
525f999 to
cd2a3f9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd2a3f98cc
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!tls && !isPrivateLanGatewayHost(host)) { | ||
| return GatewayEndpointParseResult(error = GatewayEndpointValidationError.INSECURE_REMOTE_URL) |
There was a problem hiding this comment.
Decode scoped IPv6 hosts before accepting private ws endpoints
The new private-LAN allowlist now accepts cleartext scoped IPv6 inputs like ws://[fe80::1%25wlan0]:18789, but the parser keeps the host as fe80::1%25wlan0 and does not normalize %25 back to %. That value is later used as the socket host (for example in TLS probing/connection setup), where Java treats it as interface 25wlan0 and fails hostname resolution, so these newly accepted setup/manual endpoints still cannot connect in practice. Normalize scoped IPv6 hostnames during parsing (decode %25 to % for the socket host) before returning a successful parse result.
Useful? React with 👍 / 👎.
Summary
ws://URLs and Android surfaced vague follow-on errors.ws://; Android setup/manual/QR validation shares one secure-endpoint rule and shows actionable remediation; TLS probe errors now explain whether no secure endpoint exists or the secure endpoint was unreachable; docs now point remote/mobile users to Tailscale Serve orwss://.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause / Regression History (if applicable)
ws://endpoints as valid mobile pairing targets.git blame, prior PR, issue, or refactor if known): Android remote TLS hardening landed separately from setup-code generation and docs, which still implied raw tailnet / remotews://could work.Regression Test Plan (if applicable)
src/pairing/setup-code.test.ts,src/cli/qr-cli.test.ts,src/cli/qr-dashboard.integration.test.ts,apps/android/app/src/test/java/ai/openclaw/app/ui/GatewayConfigResolverTest.kt,apps/android/app/src/test/java/ai/openclaw/app/GatewayBootstrapAuthTest.ktws://must fail before mobile pairing; loopback / emulator cleartext still works; Android shows secure-endpoint guidance instead of generic parse/fingerprint errors.apps/android/app/src/test/java/ai/openclaw/app/node/ConnectionManagerTest.ktalready covered the TLS-required policy for non-loopback endpoints.User-visible / Behavior Changes
openclaw qrnow fails instead of emitting remote insecure mobile pairing setup codes.wss://or Tailscale Serve.Diagram (if applicable)
Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) YesYes/No) NoYes, explain risk + mitigation:openclaw qrnow rejects an insecure remote pairing target that used to encode. This narrows behavior in the secure direction and prevents generating broken mobile setup codes.Repro + Verification
Environment
gateway.bind=custom|lan|tailnet,gateway.tailscale.mode=serve, optionalgateway.remote.url/plugins.entries.device-pair.config.publicUrlSteps
ws://URL.openclaw qror scan/paste that setup target into Android.Expected
wss://or Tailscale Serve.ws://is rejected early with actionable remediation.Actual
Evidence
Attach at least one:
Human Verification (required)
pnpm build; fullpnpm check.127.0.0.1cleartext remains allowed; Android emulator alias remains allowed; secure remotewss://remains allowed; Tailscale Serve path remains preferred.Review Conversations
Compatibility / Migration
Yes/No) NoYes/No) NoYes/No) Yesws://endpoint, switch that flow to Tailscale Serve / Funnel or anotherwss://gateway URL. Localhost and emulator cleartext flows do not change.Risks and Mitigations
gateway.tailscale.mode=serve,gateway.remote.url, orplugins.entries.device-pair.config.publicUrlwithwss://, and the docs now match the runtime policy.