Skip to content

fix: enhance DID key handling and generation#99

Merged
RishabhS7 merged 1 commit into
mainfrom
fix/did-key-enhancement
Dec 30, 2025
Merged

fix: enhance DID key handling and generation#99
RishabhS7 merged 1 commit into
mainfrom
fix/did-key-enhancement

Conversation

@rongquan1
Copy link
Copy Markdown
Contributor

@rongquan1 rongquan1 commented Dec 30, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • DID documents now support using existing public keys instead of always generating new ones. Key type specification is required when providing existing keys.
  • Bug Fixes

    • Improved key ID parsing logic for more robust extraction and validation of numeric key identifiers.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 30, 2025

📝 Walkthrough

Walkthrough

These changes enhance key handling in the DID Web issuer by refining the nextKeyId function for more robust fragment parsing and enabling the use of existing user-provided public keys instead of always generating new ones.

Changes

Cohort / File(s) Summary
Key ID Parsing Refinement
packages/w3c-issuer/src/did-web/wellKnown/generate.ts
Improved nextKeyId function to safely parse key ID fragments; now validates fragments contain the 'keys-' pattern before processing, filters out nulls, and uses isNaN(Number(s)) for numeric validation instead of Number.isInteger. Overall behavior unchanged: returns 1 if no valid IDs, otherwise max(valid numbers) + 1.
Public Key Handling Logic
packages/w3c-issuer/src/did-web/wellKnown/index.ts
Modified to accept and use existing public keys (publicKeyMultibase or publicKeyBase58) from input instead of always generating new ones; added validation requiring a type to be provided when user supplies keys; branched control flow to conditionally generate or use provided key pair.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hops through fragments bright,
Parsing keys with safer might,
No more blind assumptions made—
Now users' keys are honored, weighed,
With validation clear and true! 🔑

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is missing entirely. The repository template requires Summary, Changes, Issues, and Releases sections, none of which are provided. Add a complete pull request description following the template: include background/summary, detailed list of changes, related issues, and release channel/ETA information.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: enhancing DID key handling and generation logic with safer parsing and validation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6aa9f54 and c648f13.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • packages/w3c-issuer/src/did-web/wellKnown/generate.ts
  • packages/w3c-issuer/src/did-web/wellKnown/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/w3c-issuer/src/did-web/wellKnown/index.ts (1)
packages/w3c-issuer/src/did-web/keyPair/index.ts (1)
  • generateKeyPair (49-103)
🔇 Additional comments (2)
packages/w3c-issuer/src/did-web/wellKnown/index.ts (1)

33-41: Good validation for user-provided keys.

The logic correctly detects both modern and legacy key formats and enforces that a type must be specified when keys are provided. This prevents downstream issues from ambiguous key types.

packages/w3c-issuer/src/did-web/wellKnown/generate.ts (1)

138-150: Improved robustness in key ID parsing.

The enhanced parsing logic is more defensive and correctly handles edge cases:

  • Only processes fragments matching the 'keys-' pattern, avoiding false matches
  • Filters out null/undefined values before numeric conversion
  • Uses parts[parts.length - 1] to correctly handle multiple # characters
  • Validates numeric values with isNaN(Number(s))

These changes make the function more resilient to malformed key IDs. The implementation correctly handles non-numeric key patterns like keys-existing by filtering them out, ensuring only properly formatted keys-{number} identifiers are counted when determining the next key ID.

}

// Only generate if user didn't provide complete key pair
const generatedKeyPair = userProvidedKeys ? { ...didInput } : await generateKeyPair(didInput);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check type definitions and usage of generatedKeyPair

echo "=== Finding IssuedDIDOption type ==="
ast-grep --pattern 'type IssuedDIDOption = {
  $$$
}'

ast-grep --pattern 'interface IssuedDIDOption {
  $$$
}'

echo -e "\n=== Finding GeneratedKeyPair type ==="
ast-grep --pattern 'type GeneratedKeyPair = {
  $$$
}'

ast-grep --pattern 'interface GeneratedKeyPair {
  $$$
}'

echo -e "\n=== Finding all property accesses on generatedKeyPair ==="
rg -n 'generatedKeyPair\.' packages/w3c-issuer/src/did-web/wellKnown/index.ts | head -20

Repository: TrustVC/w3c

Length of output: 445


🏁 Script executed:

#!/bin/bash
# Search for type definitions in related files

echo "=== Searching for IssuedDIDOption definition ==="
rg -n "IssuedDIDOption" packages/w3c-issuer/src/did-web/wellKnown/ | head -10

echo -e "\n=== Searching for GeneratedKeyPair definition ==="
rg -n "GeneratedKeyPair" packages/w3c-issuer/src/did-web/wellKnown/ | head -10

echo -e "\n=== Full context of line 44 and surrounding code ==="
sed -n '40,75p' packages/w3c-issuer/src/did-web/wellKnown/index.ts

echo -e "\n=== Check imports at top of file ==="
head -30 packages/w3c-issuer/src/did-web/wellKnown/index.ts

Repository: TrustVC/w3c

Length of output: 3715


🏁 Script executed:

#!/bin/bash
echo "=== Full types.ts file ==="
cat -n packages/w3c-issuer/src/did-web/wellKnown/types.ts

echo -e "\n=== Search for generateKeyPair function and its return type ==="
rg -n "generateKeyPair" packages/w3c-issuer/src/did-web/keyPair/ -A 10 | head -50

Repository: TrustVC/w3c

Length of output: 8302


🏁 Script executed:

#!/bin/bash
echo "=== Search for GeneratedKeyPair type definition ==="
rg -n "type GeneratedKeyPair" packages/w3c-issuer/src/

echo -e "\n=== Search for GenerateKeyPairOptions ==="
rg -n "type GenerateKeyPairOptions" packages/w3c-issuer/src/

echo -e "\n=== Full keyPair/index.ts to see function and types ==="
head -80 packages/w3c-issuer/src/did-web/keyPair/index.ts

Repository: TrustVC/w3c

Length of output: 3574


🏁 Script executed:

#!/bin/bash
echo "=== Checking lib/types.ts for type definitions ==="
rg -n "type BaseKeyPair|type GeneratedKeyPair|type GenerateKeyPairOptions|type DidWebGeneratedKeyPair|type DidWebGenerateKeyPairOptions" packages/w3c-issuer/src/lib/types.ts -A 5

Repository: TrustVC/w3c

Length of output: 387


🏁 Script executed:

#!/bin/bash
echo "=== Checking for DidWebGenerateKeyPairOptions and DidWebGeneratedKeyPair ==="
rg -n "type DidWebGenerateKeyPairOptions|type DidWebGeneratedKeyPair|interface DidWebGenerateKeyPairOptions|interface DidWebGeneratedKeyPair" packages/w3c-issuer/src/ -A 15

Repository: TrustVC/w3c

Length of output: 1775


🏁 Script executed:

#!/bin/bash
echo "=== Check what happens with keyPairs after creation ==="
sed -n '44,85p' packages/w3c-issuer/src/did-web/wellKnown/index.ts

echo -e "\n=== Check how generateWellKnownDid uses the newKeyPair ==="
rg -n "generateWellKnownDid" packages/w3c-issuer/src/did-web/wellKnown/generate.ts -A 30 | head -50

Repository: TrustVC/w3c

Length of output: 2525


🏁 Script executed:

#!/bin/bash
echo "=== Full generateKeyPair function to see validation logic ==="
sed -n '49,130p' packages/w3c-issuer/src/did-web/keyPair/index.ts

Repository: TrustVC/w3c

Length of output: 2023


Add validation for user-provided keys to match generateKeyPair behavior.

When userProvidedKeys is true, the code bypasses generateKeyPair entirely, skipping critical validations: type validation, seed parsing, and key matching. Users could provide invalid or malformed keys (invalid base58/multibase, mismatched key pairs, unsupported types) that would be inserted into the DID document without any checks. The optional chaining on key properties prevents crashes but doesn't prevent malformed data from being stored.

Consider adding validation to user-provided keys that mirrors the checks in generateKeyPair, particularly seed parsing for BLS/BBS-2023 types and key matching when both seed and key material are provided.

🤖 Prompt for AI Agents
In packages/w3c-issuer/src/did-web/wellKnown/index.ts around line 44, when
userProvidedKeys is true the code skips generateKeyPair and thus skips important
validations (key type checks, seed parsing for BLS/BBS-2023, base58/multibase
format checks, and ensuring provided seed matches provided public/private key
material). Mirror the generateKeyPair validation path for user-provided keys:
validate the key type is supported, parse/validate seed for BLS/BBS-2023
formats, verify public/private keys are valid multibase/base58 and that if both
seed and key material are provided they correspond (derive and compare), and
throw or return a clear error on any mismatch or malformed input so malformed
keys are rejected before being inserted into the DID document.

@rongquan1 rongquan1 requested a review from RishabhS7 December 30, 2025 03:47
@RishabhS7 RishabhS7 merged commit 7092c00 into main Dec 30, 2025
19 checks passed
@RishabhS7 RishabhS7 deleted the fix/did-key-enhancement branch December 30, 2025 05:37
nghaninn pushed a commit that referenced this pull request Dec 30, 2025
@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 2.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Dec 30, 2025
@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 2.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Dec 30, 2025
@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 2.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Dec 30, 2025
@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 2.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Dec 30, 2025
## [2.0.2](https://github.com/TrustVC/w3c/compare/@trustvc/w3c@2.0.1...@trustvc/w3c@2.0.2) (2025-12-30)

### Bug Fixes

* enhance DID key handling and generation ([#99](#99)) ([7092c00](7092c00))
@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 2.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants