-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Pairing user confirmation flow #2927
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit introduces the pairing confirmation flow, allowing users to accept or reject pairing requests. It includes UI updates, event handling, and core protocol logic for handling confirmation requests, timeouts, and rejections. Co-authored-by: ijamespine <ijamespine@me.com>
|
Cursor Agent can help with this pull request. Just |
|
@cursor, fix conflict with main, merge in |
|
Done! I've successfully merged What was done:
Conflict resolved:
The PR is now up-to-date with main and includes all the latest changes from both branches. Learn more about Cursor Agents |
Resolved conflict in packages/ts-client/src/generated/types.ts by regenerating the TypeScript types to include both main's changes and the new pairing confirmation types.
PR SummaryImplements an explicit user-approval step for device pairing across core, protocol, CLI, and UIs.
Written by Cursor Bugbot for commit 22beae7. Configure here. |
apps/cli/src/domains/network/args.rs
Outdated
| reject, | ||
| } => { | ||
| // Default to accept if neither flag specified | ||
| let accepted = if *reject { false } else { *accept || true }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This always evaluates to true when not rejecting, making the --accept flag useless.
| let accepted = if *reject { false } else { *accept || true }; | |
| let accepted = if *reject { false } else { *accept }; |
| // Note: The actual Challenge will be sent when user confirms via confirm_pairing_request | ||
| let ack_response = PairingMessage::Complete { | ||
| session_id, | ||
| success: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack_response is constructed but never used - can be removed.
apps/cli/src/domains/network/args.rs
Outdated
| reject, | ||
| } => { | ||
| // Default to accept if neither flag specified | ||
| let accepted = if *reject { false } else { *accept || true }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept flag is ignored due to tautology in boolean expression
The expression *accept || true always evaluates to true regardless of the value of *accept, making the --accept flag completely useless. When --reject is not specified, the pairing is always accepted whether or not the user explicitly passed --accept. This could lead to users unintentionally accepting pairing requests when they simply run the confirm command without any flags, thinking they need to explicitly specify --accept to proceed. The expression likely intended to be *accept alone, with a separate check for when neither flag is specified.
| Ok(PairConfirmOutput { | ||
| success: true, | ||
| error: None, | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pairing confirmation reports success when response message fails to send
When handle_user_confirmation returns response data (Challenge or Reject message), the code attempts to send it to the joiner through nested if let conditions. If any condition fails—session not found, remote_device_info is None, node ID parse fails, or endpoint unavailable—the code silently falls through to return success: true without sending the message. The joiner device remains waiting indefinitely for a response that never arrives, while the initiator's user believes the confirmation succeeded. This undermines the security feature since the confirmation flow appears complete but the other device is never notified.
Co-authored-by: ijamespine <ijamespine@me.com>
|
Cursor Agent can help with this pull request. Just |
This PR introduces a mandatory user confirmation step on the initiator device during the pairing process. Previously, an initiator would automatically accept any incoming pairing request without user intervention, posing a significant security risk where an attacker could gain unauthorized access by simply scanning a QR code.
With this change, when a pairing request is received:
AwaitingUserConfirmationstate.This enhancement ensures that pairing only proceeds with explicit user consent, significantly improving the security and trustworthiness of the Spacedrive network.
Closes #(issue)