fix: backup keys are on the secp256k1 curve#193
Merged
Merged
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
There was a problem hiding this comment.
Pull request overview
Fixes the reset/backup recovery flow to verify backup_account_id signatures on the correct curve (secp256k1), aligning backend behavior with how backup keys are generated and represented.
Changes:
- Switch reset signature verification from
p256tok256(secp256k1) when parsingbackup_account_idcompressed public keys. - Update reset unit + integration tests to generate and sign challenges with
secp256k1backup keys. - Update workspace dependencies to include
k256(and adjustp256features).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/routes/reset.rs |
Uses k256 to parse compressed backup_account_id public keys and verify ECDSA signatures on secp256k1. |
tests/reset_backup_integration.rs |
Updates reset integration tests to derive backup_account_id and sign challenges using secp256k1 backup keys. |
Cargo.toml |
Adds k256 dependency and adjusts p256 features in the workspace. |
Cargo.lock |
Locks k256 and its transitive dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
aurel-fr
approved these changes
Mar 13, 2026
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
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.

The backend was incorrectly verifying backup account keys on the P256 curve. See https://github.com/worldcoin/bedrock/blob/d15b20a65d452cc974b45d17c0f1f753921bfb4b/bedrock/src/root_key/mod.rs#L210
Note
Medium Risk
Changes cryptographic signature verification for the reset flow from
p256tok256(secp256k1), which can affect account recovery behavior if key parsing/signature validation differs.Overview
Fixes reset signature verification to match backup key curve. The reset route now parses
backup_account_idpublic keys and verifies signatures usingk256/secp256k1rather thanp256.Workspace dependencies were updated to add
k256(and adjustp256features), and unit/integration tests for reset were updated to generate/sign with secp256k1 keys (including a dedicated helper for backup-key challenge signing).Written by Cursor Bugbot for commit 407ff6b. This will update automatically on new commits. Configure here.