fix(pairing): restore qr bootstrap onboarding handoff#58382
Conversation
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟠 Remote silent device pairing allowed via bootstrap token (auto-approval without locality/interactive checks)
DescriptionThe websocket handshake allows silent (auto-approved) device pairing when authenticating with a bootstrap token for a baseline node profile.
While bootstrap tokens are intended as onboarding secrets, this change expands their impact from “start pairing flow” to “complete pairing without operator approval” even for remote clients, increasing the blast radius of token leakage/guessing. RecommendationRequire an additional constraint before enabling silent/auto-approved pairing for bootstrap tokens. Options (choose one consistent with threat model):
const allowSilentBootstrapPairing =
authMethod === "bootstrap-token" &&
isLocalClient &&
reason === "not-paired" &&
role === "node" &&
scopes.length === 0 &&
!existingPairedDevice;
Also consider adding explicit audit logging and rate limiting specifically on bootstrap redemption/pairing attempts, and ensure the resulting Analyzed PR: #58382 at commit Last updated on: 2026-03-31T18:13:53Z |
Greptile SummaryThis PR fixes the QR bootstrap onboarding regression where the one-shot bootstrap token was consumed before iOS received a reusable device token. The fix spans three layers:
Test coverage is solid: the gateway seam test covers auto-approval, replay rejection, and device-token reconnect; unit tests cover Minor findings:
Confidence Score: 5/5Safe to merge — no P0 or P1 issues found; the auto-approval gate is tightly scoped and the bootstrap token is properly revoked after use. All findings are P2 style/cleanup suggestions. The security-relevant invariants (single-use binding, role/scope restrictions on auto-approval, explicit revocation, TTL expiry) are correctly preserved across the gateway, infra, and iOS layers. The test suite directly covers the regression scenarios described in the PR. No files require special attention.
|
| Filename | Overview |
|---|---|
| src/infra/device-bootstrap.ts | Bootstrap token verification now binds the token to the first presenting device identity; both binding branches share identical tails (minor duplication) and each write to disk unnecessarily on re-verify. |
| src/gateway/server/ws-connection/message-handler.ts | Adds tightly-gated silent auto-approval for fresh baseline node bootstrap connects and explicit token revocation after approval. |
| src/infra/device-pairing.ts | mergeScopes now preserves an explicit empty-scope list, enabling node device-token baseline issuance. |
| apps/ios/Sources/Model/NodeAppModel.swift | Operator loop suppressed during bootstrap-only auth; bootstrap token cleared from memory and Keychain after first successful node connect. |
| apps/ios/Sources/Gateway/GatewaySettingsStore.swift | Adds clear helpers for preferred/last-discovered gateway stable IDs and bootstrap token. |
| apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayTLSPinning.swift | Adds clearFingerprint and clearAllFingerprints using the separate 'ai.openclaw.tls-pinning' keychain service, safely scoped away from gateway credentials. |
| apps/ios/Sources/Settings/SettingsTab.swift | Onboarding reset now also clears preferred/last-discovered gateway stable IDs and TLS fingerprints. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/infra/device-bootstrap.ts
Line: 216-228
Comment:
**Duplicate state-update branches**
Both the "already-bound to same device" branch and the "first bind" fallthrough block execute exactly the same `state[tokenKey] = { …, lastUsedAtMs }` + `persistState` + `return { ok: true }`. The only behavioural distinction is the mismatch rejection in the early `return`. The shared tail can be de-duplicated:
```suggestion
if (boundDeviceId || boundPublicKey) {
if (boundDeviceId !== deviceId || boundPublicKey !== publicKey) {
return { ok: false, reason: "bootstrap_token_invalid" };
}
}
state[tokenKey] = {
...record,
profile: allowedProfile,
deviceId,
publicKey,
lastUsedAtMs: Date.now(),
};
await persistState(state, params.baseDir);
return { ok: true };
```
Not a bug, but the duplication obscures that the two paths are intentionally identical once the identity check passes.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/infra/device-bootstrap.ts
Line: 220-228
Comment:
**Unnecessary disk write on every repeated same-device verification**
When a token is already bound (`boundDeviceId && boundPublicKey` match), the code still writes the full state back to disk solely to update `lastUsedAtMs`. On the happy-path QR flow this fires at most once, but if iOS retries during a slow network it will trigger redundant I/O under the same lock that serialises all bootstrap operations. Consider skipping the persist when the only change is `lastUsedAtMs` (e.g., a tolerance window or a flag to mark it dirty only when the binding fields change).
How can I resolve this? If you propose a fix, please make it concise.Reviews (4): Last reviewed commit: "fix(pairing): clear ios tls trust on onb..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8c49d0ea8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39e1dd78e9
ℹ️ 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".
|
Addressed the follow-up reconnect/auth issues in 161846c:
I also resolved the now-fixed review threads for the stale in-memory bootstrap token and the stale cross-gateway operator-token path. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 161846c8fc
ℹ️ 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".
|
Rescoped the PR to stay tightly focused on the original What changed in c6172a7:
This should leave the PR focused on restoring the original onboarding flow without the broader cross-gateway token-migration scope. @codex review |
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🔵 Auth signature token precedence differs from auth decision, enabling handshake failures (DoS) when multiple tokens provided
Description
This mismatch means a client can present valid credentials yet be rejected at the signature-verification step if it signs the challenge with the token that the auth-decision code won't prioritize (or vice versa).
Impact:
Vulnerable code (signature token precedence): function resolveSignatureToken(connectParams: ConnectParams): string | null {
return (
connectParams.auth?.token ??
connectParams.auth?.deviceToken ??
connectParams.auth?.bootstrapToken ??
null
);
}RecommendationAlign signature-token selection with the server-side auth method/precedence, and ideally bind the signature to the exact credential actually used for authorization. Option A (simplest): match auth decision precedence for signature verification (shared token → bootstrap token → device token): function resolveSignatureToken(connectParams: ConnectParams): string | null {
return (
connectParams.auth?.token ??
connectParams.auth?.bootstrapToken ??
connectParams.auth?.deviceToken ??
null
);
}Option B (stronger): after Also add tests covering combinations where multiple credentials are provided to ensure signature verification and auth decision remain consistent. Analyzed PR: #58382 at commit Last updated on: 2026-03-31T15:59:35Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6172a7530
ℹ️ 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".
| bootstrapToken?: string; | ||
| deviceToken?: string; | ||
| }) { | ||
| return opts?.token ?? opts?.bootstrapToken ?? opts?.deviceToken; |
There was a problem hiding this comment.
Align signature token precedence with gateway connect logic
The helper now signs device payloads with bootstrapToken before deviceToken, but the gateway verifies signatures in resolveSignatureToken using token -> deviceToken -> bootstrapToken and the production client also signs with device-token precedence when both are present. In tests that pass both fields, this helper will generate signatures the server rejects, creating false failures and behavior drift from real clients; the precedence here should match gateway/client logic.
Useful? React with 👍 / 👎.
|
Follow-up pushed in 22f24a0 ( This narrows the remaining iOS reset issue by making Reset Onboarding also clear:
Validation:
@codex review |
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟠 Race condition in bootstrap token binding (in-memory lock only) allows multiple devices to bind same token
Description
This means:
Impact:
Vulnerable code (read-modify-write without cross-process lock): return await withLock(async () => {
const state = await loadState(params.baseDir);
...
const [tokenKey, record] = found;
...
const boundDeviceId = record.deviceId?.trim();
const boundPublicKey = record.publicKey?.trim();
if (boundDeviceId || boundPublicKey) {
...
state[tokenKey] = { ...record, deviceId, publicKey, lastUsedAtMs: Date.now() };
await persistState(state, params.baseDir);
return { ok: true };
}
state[tokenKey] = { ...record, deviceId, publicKey, lastUsedAtMs: Date.now() };
await persistState(state, params.baseDir);
return { ok: true };
});RecommendationEnsure the bootstrap token binding operation is atomic across processes. Options (pick one appropriate for your deployment model):
// Pseudocode: atomic bind-if-unbound
const updated = await db.bootstrapTokens.updateWhere(
{ tokenHash, deviceId: null, publicKey: null },
{ deviceId, publicKey, lastUsedAtMs: Date.now() },
);
if (!updated) return { ok: false, reason: "bootstrap_token_invalid" };
Analyzed PR: #58382 at commit Last updated on: 2026-03-31T17:11:11Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22f24a0f56
ℹ️ 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".
| bootstrapToken?: string; | ||
| deviceToken?: string; | ||
| }) { | ||
| return opts?.token ?? opts?.bootstrapToken ?? opts?.deviceToken; |
There was a problem hiding this comment.
Align signature token precedence with handshake auth
Update this helper to prefer deviceToken over bootstrapToken when token is absent; the gateway’s handshake verifier uses token ?? deviceToken ?? bootstrapToken in resolveSignatureToken, so connectReq currently signs with a different token whenever both bootstrap and device tokens are provided. In that input shape, the test client will fail with a device-signature error before auth fallback logic runs, which can make integration tests fail for the wrong reason.
Useful? React with 👍 / 👎.
22f24a0 to
7b74147
Compare
|
Landed via temp rebase onto main.
Thanks @ngutman! |
… (thanks @ngutman) * fix(pairing): restore qr bootstrap onboarding handoff * fix(pairing): tighten bootstrap handoff follow-ups * fix(pairing): migrate legacy gateway device auth * fix(pairing): narrow qr bootstrap handoff scope * fix(pairing): clear ios tls trust on onboarding reset * fix(pairing): restore qr bootstrap onboarding handoff (openclaw#58382) (thanks @ngutman)
… (thanks @ngutman) * fix(pairing): restore qr bootstrap onboarding handoff * fix(pairing): tighten bootstrap handoff follow-ups * fix(pairing): migrate legacy gateway device auth * fix(pairing): narrow qr bootstrap handoff scope * fix(pairing): clear ios tls trust on onboarding reset * fix(pairing): restore qr bootstrap onboarding handoff (openclaw#58382) (thanks @ngutman)
… (thanks @ngutman) * fix(pairing): restore qr bootstrap onboarding handoff * fix(pairing): tighten bootstrap handoff follow-ups * fix(pairing): migrate legacy gateway device auth * fix(pairing): narrow qr bootstrap handoff scope * fix(pairing): clear ios tls trust on onboarding reset * fix(pairing): restore qr bootstrap onboarding handoff (openclaw#58382) (thanks @ngutman)
… (thanks @ngutman) * fix(pairing): restore qr bootstrap onboarding handoff * fix(pairing): tighten bootstrap handoff follow-ups * fix(pairing): migrate legacy gateway device auth * fix(pairing): narrow qr bootstrap handoff scope * fix(pairing): clear ios tls trust on onboarding reset * fix(pairing): restore qr bootstrap onboarding handoff (openclaw#58382) (thanks @ngutman)
… (thanks @ngutman) * fix(pairing): restore qr bootstrap onboarding handoff * fix(pairing): tighten bootstrap handoff follow-ups * fix(pairing): migrate legacy gateway device auth * fix(pairing): narrow qr bootstrap handoff scope * fix(pairing): clear ios tls trust on onboarding reset * fix(pairing): restore qr bootstrap onboarding handoff (openclaw#58382) (thanks @ngutman)
Summary
/pair qrbootstrap onboarding consumed the one-shot bootstrap token, then left iOS retrying with dead bootstrap auth and no persisted node device-token handoff.bootstrap_token_invalid.hello-ok.auth.deviceTokenis issued, and iOS now clears bootstrap auth after node connect while preventing the operator loop from reusing bootstrap auth when a stored operator token exists.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause / Regression History (if applicable)
nodeand empty scopes, but the gateway still funneled that fresh bootstrap handshake through the normal pending-pairing flow, consuming the single-use bootstrap token before iOS ever received a reusable device token.git blame, prior PR, issue, or refactor if known): QR setup codes already switched to one-shot bootstrap tokens; the handshake and iOS reconnect behavior did not fully complete the handoff.Regression Test Plan (if applicable)
src/gateway/server.auth.control-ui.suite.tssrc/infra/device-pairing.test.tsapps/ios/Tests/NodeAppModelInvokeTests.swiftUser-visible / Behavior Changes
/pair qronboarding can complete in one node handshake again without a manual/pair approvestep.Diagram (if applicable)
Security Impact (required)
Yes/No) NoYes/No) YesYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation: token handling now clears consumed bootstrap auth sooner and preserves empty-scope node device-token baselines. Auto-approval is still limited to bootstrap-authenticated fresh node connects with role=node, empty scopes, and no existing paired device.Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
pnpm test -- src/gateway/server.auth.control-ui.test.ts -t bootstrappnpm test -- src/infra/device-pairing.test.tspnpm buildpnpm checkxcodebuild -project apps/ios/OpenClaw.xcodeproj -scheme OpenClaw -destination 'platform=iOS Simulator,name=iPhone 17' -configuration Debug buildsrc/gateway/server.auth.control-ui.test.tsfile: still failing in this checkout on unrelated pre-existing trusted-proxy/control-ui casesapps/ios/Tests/GatewayConnectionSecurityTests.swiftandapps/shared/OpenClawKit/Tests/OpenClawKitTests/TalkSystemSpeechSynthesizerTests.swiftReview Conversations
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoRisks and Mitigations
node, empty scopes, and no existing paired device, and negative tests lock in non-baseline cases.