feat: add minimal device pairing with desktop approval#191
Closed
SrinjoyeeDey wants to merge 1 commit into
Closed
Conversation
Contributor
WalkthroughThis PR implements a complete device pairing workflow for WebSocket authentication. Clients can request pairing, which creates a pending request tracked by the server. Administrators can approve or reject pairings via the settings UI, issuing tokens or denying access. Clients display status overlays while awaiting approval. Changes
Sequence Diagram(s)sequenceDiagram
participant Mobile as Mobile Client
participant Server as Server/WebSocket
participant Admin as Admin (Settings)
Mobile->>Mobile: Connect, no token
Mobile->>Server: WebSocket upgrade + request-pairing
Server->>Server: Create pairing request (5min expiry)
Server->>Mobile: pairing-requested (requestId)
Mobile->>Mobile: Update to "waiting-approval" status
Admin->>Server: Connect to /ws from localhost
Server->>Admin: pending-pairings (list of requests)
Admin->>Admin: Display pairing requests
Admin->>Server: approve-pairing (requestId)
Server->>Server: Generate token, store it
Server->>Mobile: pairing-approved + token
Mobile->>Mobile: Save token, reconnect as "connected"
alt Rejection Flow
Admin->>Server: reject-pairing (requestId)
Server->>Mobile: pairing-rejected
Mobile->>Mobile: Transition to "disconnected"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useRemoteConnection.ts (1)
15-20:⚠️ Potential issue | 🔴 CriticalPairing flow uses stale
tokenandpairingRequestIdsnapshots.Line 19, Line 53, and Line 103 read values captured at mount. After approval, reconnect logic can still behave as unauthenticated and re-enter pairing flow incorrectly.
Suggested fix
export const useRemoteConnection = () => { const wsRef = useRef<WebSocket | null>(null) + const pairingRequestIdRef = useRef<string | null>(null) const [status, setStatus] = useState< "connecting" | "connected" | "disconnected" | "waiting-approval" >("disconnected") const [pairingRequestId, setPairingRequestId] = useState<string | null>(null) @@ - const token = urlToken || storedToken + const getCurrentToken = () => + urlParams.get("token") || localStorage.getItem("rein_auth_token") + const setPairingState = (id: string | null) => { + pairingRequestIdRef.current = id + setPairingRequestId(id) + } @@ - let wsUrl = `${protocol}//${host}/ws` - if (token) { - wsUrl += `?token=${encodeURIComponent(token)}` - } + const buildWsUrl = () => { + const currentToken = getCurrentToken() + return currentToken + ? `${protocol}//${host}/ws?token=${encodeURIComponent(currentToken)}` + : `${protocol}//${host}/ws` + } @@ - const socket = new WebSocket(wsUrl) + const socket = new WebSocket(buildWsUrl()) @@ - if (!token && !pairingRequestId) { + if (!getCurrentToken() && !pairingRequestIdRef.current) { @@ - setPairingRequestId(data.requestId) + setPairingState(data.requestId) } else if (data.type === "pairing-approved" && data.token && isMounted) { localStorage.setItem("rein_auth_token", data.token) - setPairingRequestId(null) + setPairingState(null) socket.close() setTimeout(() => connect(), 500) } else if (data.type === "pairing-rejected" && isMounted) { setStatus("disconnected") - setPairingRequestId(null) + setPairingState(null) socket.close() } @@ - if (!pairingRequestId) { + if (!pairingRequestIdRef.current) { reconnectTimer = setTimeout(connect, 3000) }Also applies to: 50-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useRemoteConnection.ts` around lines 15 - 20, The pairing flow captures stale values for token (urlToken/storedToken) and pairingRequestId at mount, causing reconnect/approval handlers (e.g., the reconnect logic and anything referencing pairingRequestId) to behave as unauthenticated; fix by reading the current token and pairingRequestId at the time of use instead of relying on mount-time snapshots—either by reading localStorage/URLSearchParams inside the reconnect/approval handlers, or store token and pairingRequestId in refs (e.g., tokenRef/pairingRequestIdRef) and update them when values change so functions like the reconnect handler always access tokenRef.current and pairingRequestIdRef.current; update any useEffect or callback closures that reference token or pairingRequestId (including the reconnect flow and pairing entry points) to use the fresh values or add proper dependencies to avoid stale closures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/settings.tsx`:
- Around line 436-504: This JSX block renders hardcoded user strings; extract
all user-facing text ("Device Pairing", "No pending device pairing requests",
"{pendingPairings.length} device(s) waiting for approval", "Requested",
"Approve", "Reject") into i18n resource keys and replace them with the project's
translation hook/func (e.g., t('...') or useTranslation) where used in this
component; update the JSX around pendingPairings, pairing.deviceName/userAgent
display and the Approve/Reject buttons (which call handleApprovePairing and
handleRejectPairing) to use the localized strings and add any new keys to the
locale resource files.
- Around line 213-217: The cleanup currently only closes the socket when
socket.readyState === WebSocket.OPEN, which can leave sockets in CONNECTING
state dangling; update the cleanup in the settings.tsx effect (the
isMounted/socket block) to close the socket when its readyState is either
WebSocket.OPEN or WebSocket.CONNECTING (or alternatively close when readyState
is not WebSocket.CLOSING and not WebSocket.CLOSED) so the socket is always
closed on unmount and avoid leaked connections.
- Around line 231-250: The code in handleRejectPairing (and the similar approve
handler) currently removes items from pendingPairings optimistically; instead,
stop removing the item immediately and wait for the server acknowledgement
before updating state. Change handleRejectPairing to send the JSON payload
(type: "reject-pairing", requestId) and mark the request as "processing" (e.g.,
add a processing set or flag keyed by requestId) so the UI can disable actions,
then only call setPendingPairings(prev => prev.filter(...)) when you receive the
server confirmation event (e.g., "pairing-rejected"/"pairing-updated" handler on
wsConnection); on error from the server clear the processing flag and surface an
error (alert/toast) without removing the pending item so UI stays consistent.
Ensure to apply the same pattern to the approve handler and reference
setPendingPairings, handleRejectPairing, the WebSocket send JSON payload, and
the wsConnection message handler for the server ack.
In `@src/routes/trackpad.tsx`:
- Around line 196-200: Replace the hardcoded "Waiting for desktop approval..."
string inside the status === "waiting-approval" JSX branch with a localized
lookup (e.g. call the project's translation function such as
t('trackpad.waitingApproval') or useTranslation hook) and add the corresponding
key ("trackpad.waitingApproval") with the message to all locale resource files;
update imports in src/routes/trackpad.tsx if necessary to use the existing i18n
helper and verify the component renders the translated string.
In `@src/server/tokenStore.ts`:
- Around line 166-176: approvePairingRequest currently approves by matching
requestId without checking expiration, so stale entries can be accepted; update
approvePairingRequest to locate the request in pairingRequests, verify its
expiresAt is > Date.now() before removing and issuing a token (use
pairingRequests.findIndex or find to check expiresAt on the found object), and
only then generateToken, push to tokens and call save(true); if the request is
expired return null and do not mutate pairingRequests or create a token.
In `@src/server/websocket.ts`:
- Around line 422-426: The cleanup loop in websocket.ts only removes the first
pendingPairingSockets entry matching ws because of the break; change the logic
in the block that iterates pendingPairingSockets (the for (const [reqId, sock]
of pendingPairingSockets.entries()) loop) to remove all mappings for that socket
instead of breaking after the first match — e.g., collect matching reqId values
(or remove in-place without breaking) and delete each via
pendingPairingSockets.delete(reqId) so no stale entries remain for the ws.
- Around line 233-307: The approve flow and reject flow are accidentally
combined so approve logic falls through into reject handling; split the branches
by checking msg.type === "approve-pairing" and msg.type === "reject-pairing"
separately (or add an explicit return after successful approve handling) around
the calls to approvePairingRequest and rejectPairingRequest, and ensure each
branch notifies and deletes from pendingPairingSockets independently (avoid
re-declaring pendingSocket twice in the same scope and use WebSocket.OPEN checks
as before).
- Around line 223-231: The handler currently sends pending pairing metadata
unconditionally and returns, which can short‑circuit other message handling and
leak data; update the message handling around getPendingPairingRequests() so it
first checks that the incoming message's type equals "get-pending-pairings" AND
the connection is authenticated as local (use the existing isLocal check/flag),
only then call getPendingPairingRequests() and ws.send(...); do not return for
unrelated messages—limit the early return to the guarded branch so other
msg.type cases continue to be processed.
---
Outside diff comments:
In `@src/hooks/useRemoteConnection.ts`:
- Around line 15-20: The pairing flow captures stale values for token
(urlToken/storedToken) and pairingRequestId at mount, causing reconnect/approval
handlers (e.g., the reconnect logic and anything referencing pairingRequestId)
to behave as unauthenticated; fix by reading the current token and
pairingRequestId at the time of use instead of relying on mount-time
snapshots—either by reading localStorage/URLSearchParams inside the
reconnect/approval handlers, or store token and pairingRequestId in refs (e.g.,
tokenRef/pairingRequestIdRef) and update them when values change so functions
like the reconnect handler always access tokenRef.current and
pairingRequestIdRef.current; update any useEffect or callback closures that
reference token or pairingRequestId (including the reconnect flow and pairing
entry points) to use the fresh values or add proper dependencies to avoid stale
closures.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/hooks/useRemoteConnection.tssrc/routes/settings.tsxsrc/routes/trackpad.tsxsrc/server/tokenStore.tssrc/server/websocket.ts
Comment on lines
+213
to
+217
| return () => { | ||
| isMounted = false | ||
| if (socket.readyState === WebSocket.OPEN) { | ||
| socket.close() | ||
| } |
Contributor
There was a problem hiding this comment.
Close CONNECTING sockets in effect cleanup as well.
Line 215 only closes when OPEN. Unmount during CONNECTING can leave a dangling socket.
Suggested fix
return () => {
isMounted = false
- if (socket.readyState === WebSocket.OPEN) {
+ if (
+ socket.readyState === WebSocket.OPEN ||
+ socket.readyState === WebSocket.CONNECTING
+ ) {
socket.close()
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return () => { | |
| isMounted = false | |
| if (socket.readyState === WebSocket.OPEN) { | |
| socket.close() | |
| } | |
| return () => { | |
| isMounted = false | |
| if ( | |
| socket.readyState === WebSocket.OPEN || | |
| socket.readyState === WebSocket.CONNECTING | |
| ) { | |
| socket.close() | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/settings.tsx` around lines 213 - 217, The cleanup currently only
closes the socket when socket.readyState === WebSocket.OPEN, which can leave
sockets in CONNECTING state dangling; update the cleanup in the settings.tsx
effect (the isMounted/socket block) to close the socket when its readyState is
either WebSocket.OPEN or WebSocket.CONNECTING (or alternatively close when
readyState is not WebSocket.CLOSING and not WebSocket.CLOSED) so the socket is
always closed on unmount and avoid leaked connections.
Comment on lines
+231
to
+250
| // Remove from pending list locally | ||
| setPendingPairings((prev) => | ||
| prev.filter((p) => p.requestId !== requestId), | ||
| ) | ||
| } | ||
|
|
||
| const handleRejectPairing = (requestId: string) => { | ||
| if (!wsConnection || wsConnection.readyState !== WebSocket.OPEN) { | ||
| alert("Not connected to server") | ||
| return | ||
| } | ||
|
|
||
| wsConnection.send( | ||
| JSON.stringify({ type: "reject-pairing", requestId }), | ||
| ) | ||
|
|
||
| // Remove from pending list locally | ||
| setPendingPairings((prev) => | ||
| prev.filter((p) => p.requestId !== requestId), | ||
| ) |
Contributor
There was a problem hiding this comment.
Don’t remove pending items before server acknowledgment.
Line 231 and Line 247 optimistically mutate pendingPairings. If approval/rejection fails server-side, the UI becomes inconsistent.
Suggested fix
const handleApprovePairing = (requestId: string) => {
@@
wsConnection.send(
JSON.stringify({ type: "approve-pairing", requestId }),
)
-
- // Remove from pending list locally
- setPendingPairings((prev) =>
- prev.filter((p) => p.requestId !== requestId),
- )
+ // Refresh from server source of truth
+ wsConnection.send(JSON.stringify({ type: "get-pending-pairings" }))
}
@@
const handleRejectPairing = (requestId: string) => {
@@
wsConnection.send(
JSON.stringify({ type: "reject-pairing", requestId }),
)
-
- // Remove from pending list locally
- setPendingPairings((prev) =>
- prev.filter((p) => p.requestId !== requestId),
- )
+ // Refresh from server source of truth
+ wsConnection.send(JSON.stringify({ type: "get-pending-pairings" }))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/settings.tsx` around lines 231 - 250, The code in
handleRejectPairing (and the similar approve handler) currently removes items
from pendingPairings optimistically; instead, stop removing the item immediately
and wait for the server acknowledgement before updating state. Change
handleRejectPairing to send the JSON payload (type: "reject-pairing", requestId)
and mark the request as "processing" (e.g., add a processing set or flag keyed
by requestId) so the UI can disable actions, then only call
setPendingPairings(prev => prev.filter(...)) when you receive the server
confirmation event (e.g., "pairing-rejected"/"pairing-updated" handler on
wsConnection); on error from the server clear the processing flag and surface an
error (alert/toast) without removing the pending item so UI stays consistent.
Ensure to apply the same pattern to the approve handler and reference
setPendingPairings, handleRejectPairing, the WebSocket send JSON payload, and
the wsConnection message handler for the server ack.
Comment on lines
+436
to
+504
| <h2 className="text-xl font-semibold">Device Pairing</h2> | ||
|
|
||
| {pendingPairings.length === 0 ? ( | ||
| <div className="alert alert-info text-xs"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| fill="none" | ||
| viewBox="0 0 24 24" | ||
| className="stroke-current shrink-0 w-6 h-6" | ||
| > | ||
| <path | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| strokeWidth="2" | ||
| d="M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z" | ||
| /> | ||
| </svg> | ||
| <span>No pending device pairing requests</span> | ||
| </div> | ||
| ) : ( | ||
| <div className="space-y-3"> | ||
| <p className="text-sm opacity-70"> | ||
| {pendingPairings.length} device(s) waiting for approval | ||
| </p> | ||
| {pendingPairings.map((pairing) => ( | ||
| <div | ||
| key={pairing.requestId} | ||
| className="card bg-base-300 border border-base-400" | ||
| > | ||
| <div className="card-body p-4"> | ||
| <div className="flex items-start justify-between gap-3"> | ||
| <div className="flex-1 min-w-0"> | ||
| <h3 className="font-semibold text-sm truncate"> | ||
| {pairing.deviceName} | ||
| </h3> | ||
| <p className="text-xs opacity-60 truncate"> | ||
| {pairing.userAgent} | ||
| </p> | ||
| <p className="text-xs opacity-50 mt-1"> | ||
| Requested{" "} | ||
| {new Date(pairing.createdAt).toLocaleTimeString()} | ||
| </p> | ||
| </div> | ||
| <div className="flex gap-2"> | ||
| <button | ||
| type="button" | ||
| className="btn btn-success btn-xs rounded-md" | ||
| onClick={() => | ||
| handleApprovePairing(pairing.requestId) | ||
| } | ||
| > | ||
| Approve | ||
| </button> | ||
| <button | ||
| type="button" | ||
| className="btn btn-error btn-xs rounded-md" | ||
| onClick={() => | ||
| handleRejectPairing(pairing.requestId) | ||
| } | ||
| > | ||
| Reject | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
Externalize new Device Pairing copy to i18n resources.
The new section introduces hardcoded user-facing strings (Device Pairing, Approve, Reject, etc.). Please move them to localization keys.
As per coding guidelines, "Internationalization: User-visible strings should be externalized to resource files (i18n)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/settings.tsx` around lines 436 - 504, This JSX block renders
hardcoded user strings; extract all user-facing text ("Device Pairing", "No
pending device pairing requests", "{pendingPairings.length} device(s) waiting
for approval", "Requested", "Approve", "Reject") into i18n resource keys and
replace them with the project's translation hook/func (e.g., t('...') or
useTranslation) where used in this component; update the JSX around
pendingPairings, pairing.deviceName/userAgent display and the Approve/Reject
buttons (which call handleApprovePairing and handleRejectPairing) to use the
localized strings and add any new keys to the locale resource files.
Comment on lines
+196
to
+200
| {status === "waiting-approval" && ( | ||
| <div className="absolute inset-0 flex items-center justify-center bg-yellow-200 bg-opacity-75"> | ||
| <p className="text-center text-sm font-semibold"> | ||
| Waiting for desktop approval... | ||
| </p> |
Contributor
There was a problem hiding this comment.
Externalize the new waiting-state message to i18n resources.
The new UI copy is hardcoded in JSX. Please move it to the project’s localization resource keys.
As per coding guidelines, "Internationalization: User-visible strings should be externalized to resource files (i18n)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/trackpad.tsx` around lines 196 - 200, Replace the hardcoded
"Waiting for desktop approval..." string inside the status ===
"waiting-approval" JSX branch with a localized lookup (e.g. call the project's
translation function such as t('trackpad.waitingApproval') or useTranslation
hook) and add the corresponding key ("trackpad.waitingApproval") with the
message to all locale resource files; update imports in src/routes/trackpad.tsx
if necessary to use the existing i18n helper and verify the component renders
the translated string.
Comment on lines
+166
to
+176
| export function approvePairingRequest(requestId: string): string | null { | ||
| const index = pairingRequests.findIndex((r) => r.requestId === requestId) | ||
| if (index === -1) return null | ||
|
|
||
| const request = pairingRequests[index] | ||
| pairingRequests.splice(index, 1) | ||
|
|
||
| const token = generateToken() | ||
| const now = Date.now() | ||
| tokens.push({ token, createdAt: now, lastUsed: now }) | ||
| save(true) |
Contributor
There was a problem hiding this comment.
Enforce request expiry in the approval path.
Line 166 currently approves by requestId without validating expiresAt. A stale request can still be approved if getPendingPairingRequests() hasn’t run first.
Suggested fix
export function approvePairingRequest(requestId: string): string | null {
+ const now = Date.now()
+ pairingRequests = pairingRequests.filter((r) => r.expiresAt > now)
+
const index = pairingRequests.findIndex((r) => r.requestId === requestId)
if (index === -1) return null
const request = pairingRequests[index]
pairingRequests.splice(index, 1)
const token = generateToken()
- const now = Date.now()
tokens.push({ token, createdAt: now, lastUsed: now })
save(true)
return token
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function approvePairingRequest(requestId: string): string | null { | |
| const index = pairingRequests.findIndex((r) => r.requestId === requestId) | |
| if (index === -1) return null | |
| const request = pairingRequests[index] | |
| pairingRequests.splice(index, 1) | |
| const token = generateToken() | |
| const now = Date.now() | |
| tokens.push({ token, createdAt: now, lastUsed: now }) | |
| save(true) | |
| export function approvePairingRequest(requestId: string): string | null { | |
| const now = Date.now() | |
| pairingRequests = pairingRequests.filter((r) => r.expiresAt > now) | |
| const index = pairingRequests.findIndex((r) => r.requestId === requestId) | |
| if (index === -1) return null | |
| const request = pairingRequests[index] | |
| pairingRequests.splice(index, 1) | |
| const token = generateToken() | |
| tokens.push({ token, createdAt: now, lastUsed: now }) | |
| save(true) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/tokenStore.ts` around lines 166 - 176, approvePairingRequest
currently approves by matching requestId without checking expiration, so stale
entries can be accepted; update approvePairingRequest to locate the request in
pairingRequests, verify its expiresAt is > Date.now() before removing and
issuing a token (use pairingRequests.findIndex or find to check expiresAt on the
found object), and only then generateToken, push to tokens and call save(true);
if the request is expired return null and do not mutate pairingRequests or
create a token.
Comment on lines
+223
to
+231
| const pendingRequests = getPendingPairingRequests() | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "pending-pairings", | ||
| requests: pendingRequests, | ||
| }), | ||
| ) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
pending-pairings must be explicitly gated by message type and localhost auth.
As written around Line 223, this block responds with pending requests and returns without an explicit msg.type === "get-pending-pairings" + isLocal guard, which can short-circuit unrelated messages and expose pairing metadata to remote clients.
Suggested fix
- const pendingRequests = getPendingPairingRequests()
- ws.send(
- JSON.stringify({
- type: "pending-pairings",
- requests: pendingRequests,
- }),
- )
- return
- }
+ if (msg.type === "get-pending-pairings") {
+ if (!isLocal) {
+ ws.send(
+ JSON.stringify({
+ type: "auth-error",
+ error: "Only localhost can view pending pairings",
+ }),
+ )
+ return
+ }
+ const pendingRequests = getPendingPairingRequests()
+ ws.send(
+ JSON.stringify({
+ type: "pending-pairings",
+ requests: pendingRequests,
+ }),
+ )
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const pendingRequests = getPendingPairingRequests() | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pending-pairings", | |
| requests: pendingRequests, | |
| }), | |
| ) | |
| return | |
| } | |
| if (msg.type === "get-pending-pairings") { | |
| if (!isLocal) { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "auth-error", | |
| error: "Only localhost can view pending pairings", | |
| }), | |
| ) | |
| return | |
| } | |
| const pendingRequests = getPendingPairingRequests() | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pending-pairings", | |
| requests: pendingRequests, | |
| }), | |
| ) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/websocket.ts` around lines 223 - 231, The handler currently sends
pending pairing metadata unconditionally and returns, which can short‑circuit
other message handling and leak data; update the message handling around
getPendingPairingRequests() so it first checks that the incoming message's type
equals "get-pending-pairings" AND the connection is authenticated as local (use
the existing isLocal check/flag), only then call getPendingPairingRequests() and
ws.send(...); do not return for unrelated messages—limit the early return to the
guarded branch so other msg.type cases continue to be processed.
Comment on lines
+233
to
+307
| if (msg.type === "approve-pairing") { | ||
| // Only localhost can approve pairings | ||
| if (!isLocal) { | ||
| logger.warn("Non-localhost attempted to approve pairing") | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "auth-error", | ||
| error: "Only localhost can approve pairings", | ||
| }), | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| const requestId = msg.requestId | ||
| if (!requestId || typeof requestId !== "string") { | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "pairing-error", | ||
| error: "Invalid requestId", | ||
| }), | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| const approvedToken = approvePairingRequest(requestId) | ||
| if (!approvedToken) { | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "pairing-error", | ||
| error: "Pairing request not found or expired", | ||
| }), | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| logger.info(`Pairing approved: ${requestId}`) | ||
| // notify original device if still connected | ||
| const pendingSocket = pendingPairingSockets.get(requestId) | ||
| if (pendingSocket && pendingSocket.readyState === WebSocket.OPEN) { | ||
| pendingSocket.send( | ||
| JSON.stringify({ | ||
| type: "pairing-approved", | ||
| requestId, | ||
| token: approvedToken, | ||
| message: "Device pairing approved", | ||
| }), | ||
| ) | ||
| } | ||
| pendingPairingSockets.delete(requestId) | ||
|
|
||
|
|
||
| const rejected = rejectPairingRequest(requestId) | ||
| if (!rejected) { | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "pairing-error", | ||
| error: "Pairing request not found or already expired", | ||
| }), | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| logger.info(`Pairing rejected: ${requestId}`) | ||
| // notify original device as well | ||
| const pendingSocket = pendingPairingSockets.get(requestId) | ||
| if (pendingSocket && pendingSocket.readyState === WebSocket.OPEN) { | ||
| pendingSocket.send( | ||
| JSON.stringify({ | ||
| type: "pairing-rejected", | ||
| requestId, | ||
| message: "Device pairing rejected", | ||
| }), | ||
| ) | ||
| } | ||
| pendingPairingSockets.delete(requestId) |
Contributor
There was a problem hiding this comment.
Reject flow is currently coupled into the approve flow.
Line 284 onward executes immediately after approval logic, so an approve can fall through into reject handling. reject-pairing must be a separate message branch.
Suggested fix
if (msg.type === "approve-pairing") {
// Only localhost can approve pairings
if (!isLocal) {
@@
logger.info(`Pairing approved: ${requestId}`)
const pendingSocket = pendingPairingSockets.get(requestId)
if (pendingSocket && pendingSocket.readyState === WebSocket.OPEN) {
pendingSocket.send(
JSON.stringify({
type: "pairing-approved",
requestId,
token: approvedToken,
message: "Device pairing approved",
}),
)
}
pendingPairingSockets.delete(requestId)
+ return
+ }
+ if (msg.type === "reject-pairing") {
const rejected = rejectPairingRequest(requestId)
if (!rejected) {
ws.send(
@@
}
pendingPairingSockets.delete(requestId)
+ return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (msg.type === "approve-pairing") { | |
| // Only localhost can approve pairings | |
| if (!isLocal) { | |
| logger.warn("Non-localhost attempted to approve pairing") | |
| ws.send( | |
| JSON.stringify({ | |
| type: "auth-error", | |
| error: "Only localhost can approve pairings", | |
| }), | |
| ) | |
| return | |
| } | |
| const requestId = msg.requestId | |
| if (!requestId || typeof requestId !== "string") { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pairing-error", | |
| error: "Invalid requestId", | |
| }), | |
| ) | |
| return | |
| } | |
| const approvedToken = approvePairingRequest(requestId) | |
| if (!approvedToken) { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pairing-error", | |
| error: "Pairing request not found or expired", | |
| }), | |
| ) | |
| return | |
| } | |
| logger.info(`Pairing approved: ${requestId}`) | |
| // notify original device if still connected | |
| const pendingSocket = pendingPairingSockets.get(requestId) | |
| if (pendingSocket && pendingSocket.readyState === WebSocket.OPEN) { | |
| pendingSocket.send( | |
| JSON.stringify({ | |
| type: "pairing-approved", | |
| requestId, | |
| token: approvedToken, | |
| message: "Device pairing approved", | |
| }), | |
| ) | |
| } | |
| pendingPairingSockets.delete(requestId) | |
| const rejected = rejectPairingRequest(requestId) | |
| if (!rejected) { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pairing-error", | |
| error: "Pairing request not found or already expired", | |
| }), | |
| ) | |
| return | |
| } | |
| logger.info(`Pairing rejected: ${requestId}`) | |
| // notify original device as well | |
| const pendingSocket = pendingPairingSockets.get(requestId) | |
| if (pendingSocket && pendingSocket.readyState === WebSocket.OPEN) { | |
| pendingSocket.send( | |
| JSON.stringify({ | |
| type: "pairing-rejected", | |
| requestId, | |
| message: "Device pairing rejected", | |
| }), | |
| ) | |
| } | |
| pendingPairingSockets.delete(requestId) | |
| if (msg.type === "approve-pairing") { | |
| // Only localhost can approve pairings | |
| if (!isLocal) { | |
| logger.warn("Non-localhost attempted to approve pairing") | |
| ws.send( | |
| JSON.stringify({ | |
| type: "auth-error", | |
| error: "Only localhost can approve pairings", | |
| }), | |
| ) | |
| return | |
| } | |
| const requestId = msg.requestId | |
| if (!requestId || typeof requestId !== "string") { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pairing-error", | |
| error: "Invalid requestId", | |
| }), | |
| ) | |
| return | |
| } | |
| const approvedToken = approvePairingRequest(requestId) | |
| if (!approvedToken) { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pairing-error", | |
| error: "Pairing request not found or expired", | |
| }), | |
| ) | |
| return | |
| } | |
| logger.info(`Pairing approved: ${requestId}`) | |
| // notify original device if still connected | |
| const pendingSocket = pendingPairingSockets.get(requestId) | |
| if (pendingSocket && pendingSocket.readyState === WebSocket.OPEN) { | |
| pendingSocket.send( | |
| JSON.stringify({ | |
| type: "pairing-approved", | |
| requestId, | |
| token: approvedToken, | |
| message: "Device pairing approved", | |
| }), | |
| ) | |
| } | |
| pendingPairingSockets.delete(requestId) | |
| return | |
| } | |
| if (msg.type === "reject-pairing") { | |
| const requestId = msg.requestId | |
| if (!requestId || typeof requestId !== "string") { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pairing-error", | |
| error: "Invalid requestId", | |
| }), | |
| ) | |
| return | |
| } | |
| const rejected = rejectPairingRequest(requestId) | |
| if (!rejected) { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "pairing-error", | |
| error: "Pairing request not found or already expired", | |
| }), | |
| ) | |
| return | |
| } | |
| logger.info(`Pairing rejected: ${requestId}`) | |
| // notify original device as well | |
| const pendingSocket = pendingPairingSockets.get(requestId) | |
| if (pendingSocket && pendingSocket.readyState === WebSocket.OPEN) { | |
| pendingSocket.send( | |
| JSON.stringify({ | |
| type: "pairing-rejected", | |
| requestId, | |
| message: "Device pairing rejected", | |
| }), | |
| ) | |
| } | |
| pendingPairingSockets.delete(requestId) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/websocket.ts` around lines 233 - 307, The approve flow and reject
flow are accidentally combined so approve logic falls through into reject
handling; split the branches by checking msg.type === "approve-pairing" and
msg.type === "reject-pairing" separately (or add an explicit return after
successful approve handling) around the calls to approvePairingRequest and
rejectPairingRequest, and ensure each branch notifies and deletes from
pendingPairingSockets independently (avoid re-declaring pendingSocket twice in
the same scope and use WebSocket.OPEN checks as before).
Author
|
Hii @imxade , I’ve kept the changes scoped strictly to the pairing logic so existing functionality remains untouched. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues
Fixes #190
Description
This PR introduces a minimal device-pairing workflow with desktop approval to enhance LAN security without modifying existing functionality.
Previously, any device on the same LAN that knew the server IP and port could connect and begin sending input events. This PR adds a lightweight authorization layer to ensure only approved devices can control the desktop.
The implementation is intentionally minimal and scoped strictly to pairing-related logic.
🔒 What This PR Adds
Secure First-Time Pairing Flow
request-pairingmessage.🛠 Implementation Details
Server Changes (
src/server)tokenStore.tsPairingRequesttype.Implemented:
createPairingRequestgetPendingPairingRequestsapprovePairingRequestrejectPairingRequestwebsocket.tsrequest-pairing.request-pairingget-pending-pairingsapprove-pairingreject-pairingClient Changes
useRemoteConnection.tsrequest-pairingwhen connecting without a token.pairing-requestedpairing-approvedpairing-rejectedwaiting-approval.settings.tsxpendingPairingswsConnectiontrackpad.tsx✅ Functional Verification
Please check off the behaviors verified with this change.
Authentication
Basic Gestures
Modes & Settings
Advanced Input
Any Other Gesture or Input Behavior Introduced
waiting-approvalstate until approved.Additional Notes
Future Enhancements (Not Included in This PR)
Checklist
This PR was implemented carefully with attention to maintaining existing behavior. All changes were manually reviewed and verified locally. The feature builds successfully and preserves current functionality outside of the new pairing flow.
Feedback and architectural suggestions are welcome. 🚀