Repository navigation
fix: Check caveat in invokeKeyring - #4128
Merged
Merged
Conversation
Mrtenz
previously approved these changes
Sep 22, 2026
Mrtenz
left a comment
Member
There was a problem hiding this comment.
Should be noted that this is technically a breaking change.
FrederikBolding
marked this pull request as ready for review
September 22, 2026 13:32
FrederikBolding
enabled auto-merge
September 22, 2026 13:32
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4128 +/- ##
=======================================
Coverage 98.60% 98.60%
=======================================
Files 429 429
Lines 12511 12513 +2
Branches 1978 1977 -1
=======================================
+ Hits 12336 12338 +2
Misses 175 175 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Mrtenz
approved these changes
Sep 22, 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.
Inspect permission and check caveat in
invokeKeyring.Note
High Risk
Tightens authorization for keyring Snap invocation; incorrect caveat handling could block legitimate calls or leave overly broad access if misimplemented.
Overview
wallet_invokeKeyringnow verifies the dapp is connected to the requested Snap, not just that it holds the broadwallet_snappermission.The handler switches from
PermissionController:hasPermissiontogetPermission, reads theSnapIdscaveat, and only proceeds when the targetsnapIdis listed for the origin. Otherwise it returns the existing “not connected” error. Tests and handler metadata were updated to mockgetPermissionwith caveat data (ornullwhen denied). Jest line coverage threshold was bumped slightly (99.22 → 99.23).Reviewed by Cursor Bugbot for commit 87f29a9. Bugbot is set up for automated code reviews on this repo. Configure here.