refactor(keys): one WIF-to-identity-key matcher and one home for key enums - #373
Merged
Merged
Conversation
…enums
Five services each carried their own copy of 'decode the wasm key's hex data, build IdentityPublicKeyInfo, call findMatchingKeyIndex, then filter by purpose and security level' (state-transition, dpns, token, tip, identity). They filtered in different orders: token-service filtered by level BEFORE matching so a weaker duplicate of the same key could not mask the right one, the others matched first. lib/crypto/keys.ts now has matchIdentityKey(wif, keys, { purpose, allowedSecurityLevels, keyId? }) which always filters first and returns the original key object for the signer, plus toKeyInfo() that reads both the wasm getter shape (keyId/purposeNumber/hex data) and the JSON shape (id/purpose/base64 data). Ten specs cover both shapes, the level-first ordering, disabled keys, and the pinned-key-id path.
KeyPurpose/SecurityLevel/KeyType lived in signer-service (as consts), key-validation (KEY_PURPOSE), identity-service (IDENTITY_KEY_*) and as bare 0/1/2 literals in eight more files. They now live once in lib/crypto/identity-keys.ts with the name helpers, and every comparison against a raw purpose/type/level number goes through them.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Deploying yappr-v2 with
|
| Latest commit: |
a6fec25
|
| Status: | ⚡️ Build in progress... |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Collaborator
|
🕓 Ready for review — 33 ahead in queue (commit a6fec25) |
… uses the shared matcher Review findings on the matcher: toKeyInfo now resolves purpose/level/type through the shared name-aware resolvers and returns null when any cannot be resolved, instead of defaulting an unknown purpose to AUTHENTICATION. matchIdentityKey distinguishes 'rejected' (the WIF matches an enabled key that fails the purpose/level filter, returned so the log can name it) from 'no-match', so the state-transition and DPNS logs are as specific as before. allowedSecurityLevels is optional (any level) so tip-service stops enumerating all four. The DPNS registration wizard uses matchIdentityKey too, so its ordering matches dpnsService. identity-service's private enum normalizer is the shared one. Tests pin the returned key by identity and use realistic fixtures (wasm shape with both name and number fields, JSON shape with base64 data). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Fourth PR of the anti-slop series (after #370, #371, #372). One matcher for "which identity key does this WIF correspond to", and one home for the key enums.
The five copies
disabledAt< CRITICALEach also inlined a hex decoder for the wasm key's
datagetter. The ordering matters: matching first means a MEDIUM key registered from the same private key as a HIGH key (e.g. after a rotation) can win the byte comparison and then fail the level check, masking the valid key. Only token-service filtered first; its own comment explains the bug.What replaces them
lib/crypto/keys.ts:matchIdentityKey(wif, keys, { network, purpose, allowedSecurityLevels, keyId? }). Filters enabled keys by purpose and level first, then matches, then enforces a pinned key id. Returns{ ok: true, key, match }with the original key object so callers hand it straight tocreateSignerFromWasmKey, or{ ok: false, reason }with'no-candidates' | 'no-match' | 'wrong-key-id'.toKeyInfo(key): reads both shapes an identity key arrives in (the wasm getter object withkeyId/purposeNumber/hexdata, and the JSON/app shape withid/purpose/base64data) into the numericIdentityPublicKeyInfothe matcher compares.lib/crypto/identity-keys.ts:KeyPurpose,SecurityLevel,KeyTypeand the name helpers, replacing the copies insigner-service.ts,key-validation.ts(KEY_PURPOSE) andidentity-service.ts(IDENTITY_KEY_*), and the bare0/1/2comparisons in eight more files (encryption-key-lookup,private-feed-service,direct-message-service,key-validation-service,registration-wizard,identity-update-builder, …).lib/crypto/keys.test.tsadds ten specs: both key shapes, the level-first ordering with a weaker duplicate present, disabled keys, purpose filtering, the no-candidates/no-match distinction, and the pinned-id path.Behaviour changes
identity-service.validateKeySecurityLevelkeeps matching against every key first so its error can name the wrong level the user supplied, then requires MASTER. Same outward behaviour as before.Validation
npm run lint,npx tsc --noEmit,npm run test(70 specs),npx knip(empty),npm run buildall clean locally. Independent review is running; findings land as a follow-up commit here.🤖 Generated with Claude Code