Skip to content

feat: bbs2023 implementation#88

Merged
rongquan1 merged 4 commits into
alphafrom
feat/bbs-2023-implementation
Sep 24, 2025
Merged

feat: bbs2023 implementation#88
rongquan1 merged 4 commits into
alphafrom
feat/bbs-2023-implementation

Conversation

@rongquan1
Copy link
Copy Markdown
Contributor

@rongquan1 rongquan1 commented Sep 24, 2025

Summary by CodeRabbit

  • New Features

    • Added full support for BBS-2023 alongside ECDSA-SD-2023, including signing, selective disclosure, and verification.
    • Set ECDSA-SD-2023 as the default cryptosuite; legacy BBS2020 flows now return deprecation errors.
    • Improved detection and handling of base vs. derived credentials.
  • Documentation

    • Overhauled README with modern cryptosuite usage, derive-before-verify guidance, migration notes, and best practices.
  • Tests

    • Introduced scenario-driven tests covering modern cryptosuites and legacy compatibility.
  • Chores

    • Deprecated w3c-cli and removed apps from workspaces; added required cryptosuite dependencies.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 24, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Deprecates the w3c-cli app and excludes it from workspaces/Nx. Adds BBS-2023 support alongside ECDSA-SD-2023 in w3c-vc, updating sign/verify/derive flows, helper logic, and tests. Introduces new fixtures, dependencies, and ambient declarations. Updates documentation to reflect modern cryptosuites and legacy BBS2020 deprecation.

Changes

Cohort / File(s) Summary
Monorepo workspace & Nx ignore
package.json, .nxignore
Removes apps/* from Yarn workspaces; adds apps/w3c-cli to Nx ignore.
Deprecate w3c-cli app
apps/w3c-cli/package.json, apps/w3c-cli/project.json.deprecated
Marks package as deprecated via description and deprecated field; renames/stashes project config with deprecation banner to exclude from builds/tests.
Ambient declarations
packages/declaration.d.ts
Adds ambient module declaration for @digitalbazaar/bbs-2023-cryptosuite.
w3c-vc documentation
packages/w3c-vc/README.md
Overhauls README to document ECDSA-SD-2023 and BBS-2023 flows, selective disclosure, validation utilities, and legacy notes.
w3c-vc dependencies
packages/w3c-vc/package.json
Adds dependencies: @digitalbazaar/bls12-381-multikey and @digitalbazaar/bbs-2023-cryptosuite.
Fixtures: keys and credentials
packages/w3c-vc/src/lib/__fixtures__/key-pairs.ts, .../__fixtures__/modern-credentials.ts, .../__fixtures__/bbs2020-credentials.ts, .../__fixtures__/test-scenarios.ts
Introduces test key pairs, modern VC samples (v1.1/v2.0), legacy BBS2020 VC/derived samples, and scenario definitions for parameterized tests.
Helper updates
packages/w3c-vc/src/lib/helper/index.ts
Adjusts credentialSubject validation call; updates ID prefill logic to treat bbs-2023 like ecdsa-sd-2023.
Core VC logic
packages/w3c-vc/src/lib/w3c-vc.ts
Adds BBS-2023 support across sign/derive/verify; changes default cryptosuite to ecdsa-sd-2023; introduces base/derived proof detection for both; drops BBS2020 sign/derive support with explicit errors; expands mandatory pointer extraction.
Tests
packages/w3c-vc/src/lib/w3c-vc.test.ts
Rewrites tests using scenario-driven coverage for modern cryptosuites and legacy verification, with signing/derivation/tamper cases and error handling.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev
  participant VC as w3c-vc
  participant CS as Cryptosuite
  participant DL as DocumentLoader

  rect rgba(230,245,255,0.5)
  note right of Dev: Sign (default ecdsa-sd-2023)
  Dev->>VC: signCredential(credential, keyPair, cryptoSuite?)
  VC->>VC: select cryptosuite (ecdsa-sd-2023 | bbs-2023)
  alt cryptoSuite == ecdsa-sd-2023 or bbs-2023
    VC->>CS: createSignCryptosuite(...)
    VC->>DL: load contexts
    VC->>CS: sign with keyPair
    CS-->>VC: DataIntegrityProof (base)
    VC-->>Dev: Signed VC
  else cryptoSuite == BbsBlsSignature2020
    VC-->>Dev: Error("deprecated cryptosuite")
  else unsupported
    VC-->>Dev: Error("unsupported cryptosuite")
  end
  end

  rect rgba(235,255,235,0.5)
  note right of Dev: Derive (selective disclosure)
  Dev->>VC: deriveCredential(signedVC, pointers)
  VC->>VC: assert not already derived
  VC->>VC: extract mandatory pointers (by cryptosuite + version)
  VC->>CS: createDiscloseCryptosuite(...)
  VC->>DL: load contexts
  VC->>CS: derive(base VC, pointers+mandatory)
  CS-->>VC: DataIntegrityProof (derived)
  VC-->>Dev: Derived VC
  end

  rect rgba(255,245,230,0.5)
  note right of Dev: Verify (derived only for modern suites)
  Dev->>VC: verifyCredential(vc)
  alt vc has derived proof (ecdsa-sd-2023 | bbs-2023)
    VC->>CS: createVerifyCryptosuite(...)
    VC->>DL: load contexts
    VC->>CS: verify(vc)
    CS-->>VC: result
    VC-->>Dev: VerificationResult
  else base proof (modern)
    VC-->>Dev: Error("derive before verify")
  else legacy BBS2020
    VC->>CS: verify legacy (allowed)
    VC-->>Dev: VerificationResult
  end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested labels

released on @alpha

Suggested reviewers

  • nghaninn
  • Moiz47
  • sgtx-nghaninn

Poem

A nibble, a hop, I tweak the keys,
Old BBS burrows now marked “please.”
ECDSA sparks, BBS-23 sings,
Credentials revealed with pointered strings.
I thump in tests, I stamp and verify—
Carrots signed, disclosures fly. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning No pull request description was provided; the repository requires a template with sections "Summary", "Changes", "Issues", and "Releases", and this PR currently omits those sections, leaving the background, detailed changes, related issues, and release/ETA undocumented. Because of that omission the description check fails. Please add a PR description using the repository template: include a Summary describing the goal and background (adding BBS-2023 support), a Changes section listing the key file and behavioral changes (e.g., packages/w3c-vc additions/updates, new fixtures and tests, added dependencies, workspace change, w3c-cli deprecation), an Issues section linking related issue numbers, and a Releases section stating target release channel and ETA.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "feat: bbs2023 implementation" succinctly and accurately summarizes the primary change: adding BBS-2023 cryptosuite support across core code, fixtures, tests, and package dependencies (for example, packages/w3c-vc.ts, new fixtures and tests, and package.json dependency updates). It is concise, specific, and follows conventional commit style without extraneous detail.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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.

@rongquan1 rongquan1 force-pushed the feat/bbs-2023-implementation branch from 291ee9e to ed48f63 Compare September 24, 2025 07:21
@rongquan1
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 24, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/w3c-vc/src/lib/helper/index.ts (1)

429-441: URL-based URI validation rejects valid DID/URN values

Using the WHATWG URL parser will reject generic URIs like did: and many urn: forms. This can incorrectly fail common issuers (e.g., did:web, did:key).

Replace with a scheme check per RFC3986:

-export function _validateUriId({ id, propertyName }: { id: string; propertyName: string }): void {
-  let parsed: URL;
-  try {
-    parsed = new URL(id);
-  } catch (e) {
-    const error = new Error(`"${propertyName}" must be a URI: "${id}".`);
-    throw error;
-  }
-
-  if (!parsed.protocol) {
-    throw new Error(`"${propertyName}" must be a URI: "${id}".`);
-  }
-}
+export function _validateUriId({
+  id,
+  propertyName,
+}: {
+  id: string;
+  propertyName: string;
+}): void {
+  // Accept generic URIs (did:, urn:, http(s):, etc.)
+  if (typeof id !== 'string' || !/^[A-Za-z][A-Za-z0-9+.-]*:/.test(id)) {
+    throw new Error(`"${propertyName}" must be a URI: "${id}".`);
+  }
+}
🧹 Nitpick comments (13)
package.json (2)

4-4: Workspaces narrowed to packages only — gate/remove CLI scripts referencing apps/

Root still exposes cli:* scripts that point to apps/w3c-cli, which is no longer in workspaces and is ignored by Nx. Consider removing or hard‑failing these to avoid confusion.

Apply this diff to remove the deprecated CLI scripts:

   "scripts": {
     "affected": "nx affected",
     "affected:lint": "nx affected:lint",
     "test": "nx run-many --target=test",
     "test:skip-cache": "nx run-many --target=test --skip-nx-cache",
     "lint": "nx run-many --target=lint",
     "lint:fix": "nx run-many --target=lint --fix",
     "build": "nx run-many --target=build",
     "copy-files": "bash ./scripts/copy-files.sh",
     "clean": "nx clear-cache && nx run-many --target=clean",
     "dev": "nx run-many --target=dev",
     "precommit": "lint-staged",
-    "cli:install": "npm install --prefix apps/w3c-cli",
-    "cli:dev": "npm run dev --prefix apps/w3c-cli",
-    "cli:exec": "npm run cli --prefix apps/w3c-cli",
     "prepare": "husky",
     "release": "nx run-many --target=semantic-release --parallel=1 --verbose"
   },

151-152: Avoid publishing the repo root package

Root is currently "private": false and has dependencies; publishing the monorepo root is risky. Recommend setting "private": true unless you intentionally publish "@trustvc/w3c".

packages/w3c-vc/src/lib/helper/index.ts (3)

281-285: Fix outdated error message for proof type

Message still references BbsBlsSignature2020. Make it generic for all unsupported types.

-    if (!proofTypeMapping[proofType]) {
-      throw new Error('"proof" type is not of BbsBlsSignature2020.');
-    }
+    if (!proofTypeMapping[proofType]) {
+      throw new Error('Unsupported "proof" type.');
+    }

351-359: Use forEach instead of map for side‑effects

.map is unused; .forEach communicates intent and avoids creating an unused array.

-  if (Array.isArray(credential?.credentialSubject)) {
-    credential?.credentialSubject.map((subject: CredentialSubject) =>
-      _checkCredentialSubject(subject),
-    );
-    return;
-  }
-
-  _checkCredentialSubject(credential?.credentialSubject);
+  if (Array.isArray(credential?.credentialSubject)) {
+    credential.credentialSubject.forEach((subject: CredentialSubject) => {
+      _checkCredentialSubject(subject);
+    });
+    return;
+  }
+
+  _checkCredentialSubject(credential.credentialSubject);

474-477: Minor: consider renaming function to prefillCredentialId (spelling)

Public rename may be breaking; consider an alias now and migration later.

apps/w3c-cli/package.json (1)

19-24: Nx-based scripts will fail after deprecation — replace with direct commands or remove

Since Nx no longer recognizes this project, these scripts will error. Simplify or remove to avoid confusion.

   "scripts": {
     "test": "npx vitest --run",
     "lint": "npx eslint . --color --format=table --max-warnings=0",
     "start": "node dist/main.js",
     "dev": "tsx src/main.ts",
-    "build": "nx run @trustvc/w3c-cli:clean && tsup",
-    "build:tsc": "nx run @trustvc/w3c-cli:clean && nx run @trustvc/w3c-cli:build:cjs && nx run @trustvc/w3c-cli:build:type",
-    "build:cjs": "tsc --module commonjs --outDir dist/cjs --project ./tsconfig.build.json",
-    "build:type": "tsc -d --emitDeclarationOnly --outDir dist/types --project ./tsconfig.build.json",
-    "serve": "nx run @trustvc/w3c-cli:build && node dist/main.js",
-    "clean": "rm -rf dist/"
+    "build": "rm -rf dist/ && tsup",
+    "build:tsc": "rm -rf dist/ && npm run build:cjs && npm run build:type",
+    "build:cjs": "tsc --module commonjs --outDir dist/cjs --project ./tsconfig.build.json",
+    "build:type": "tsc -d --emitDeclarationOnly --outDir dist/types --project ./tsconfig.build.json",
+    "serve": "npm run build && node dist/main.js",
+    "clean": "rm -rf dist/"
   },
packages/w3c-vc/src/lib/w3c-vc.test.ts (1)

304-315: Consider adding more specific cryptosuite validation tests.

While the test validates rejection of unsupported cryptosuites, consider adding tests for:

  • Invalid cryptosuite format (not just unsupported string)
  • Null/undefined cryptosuite
  • Cryptosuite mismatch between signing and verification
it('should handle invalid cryptosuite formats gracefully', async () => {
  const testCredential = { ...credential, [dateField]: dateValue };
  
  // Test null cryptosuite
  const nullResult = await signCredential(testCredential, keyPair, null as any);
  expect(nullResult.signed).toBeUndefined();
  expect(nullResult.error).toBeDefined();
  
  // Test undefined cryptosuite  
  const undefinedResult = await signCredential(testCredential, keyPair, undefined as any);
  expect(undefinedResult.signed).toBeUndefined();
  expect(undefinedResult.error).toBeDefined();
});
packages/w3c-vc/src/lib/__fixtures__/bbs2020-credentials.ts (1)

14-20: Consider extracting test configuration values.

The fixture contains hardcoded URLs (https://localhost:3000) and blockchain addresses. Consider extracting these to a configuration object for easier maintenance across test environments.

+const TEST_CONFIG = {
+  qrCodeUri: process.env.TEST_QR_URI || 'https://localhost:3000/qrcode',
+  rendererUri: process.env.TEST_RENDERER_URI || 'https://localhost:3000/renderer',
+  tokenRegistry: '0xE0a94770B8e969B5D9179d6dA8730B01e19279e2',
+  tokenNetwork: { chain: 'MATIC', chainId: '80001' }
+};

 export const bbs2020CredentialV1_1: SignedVerifiableCredential = {
   // ... other properties
-  qrCode: { type: 'TrustVCQRCode', uri: 'https://localhost:3000/qrcode' },
+  qrCode: { type: 'TrustVCQRCode', uri: TEST_CONFIG.qrCodeUri },
   credentialStatus: {
     type: 'TransferableRecords',
-    tokenNetwork: { chain: 'MATIC', chainId: '80001' },
-    tokenRegistry: '0xE0a94770B8e969B5D9179d6dA8730B01e19279e2',
+    tokenNetwork: TEST_CONFIG.tokenNetwork,
+    tokenRegistry: TEST_CONFIG.tokenRegistry,
     // ... rest of properties
   },
packages/w3c-vc/src/lib/__fixtures__/modern-credentials.ts (1)

15-18: Consider parameterizing test URLs.

Similar to the BBS2020 fixtures, the hardcoded localhost:3000 URLs should be configurable.

+const TEST_ENDPOINTS = {
+  qrCode: process.env.TEST_QR_URI || 'https://localhost:3000/qrcode',
+  renderer: process.env.TEST_RENDERER_URI || 'https://localhost:3000/renderer',
+  links: process.env.TEST_LINKS_URI || 'https://localhost:3000/url'
+};

 // Modern credential for v1.1 testing
 export const modernCredentialV1_1: VerifiableCredential = {
   // ...
   qrCode: {
     type: 'TrustVCQRCode',
-    uri: 'https://localhost:3000/qrcode',
+    uri: TEST_ENDPOINTS.qrCode,
   },

Also applies to: 77-80

packages/w3c-vc/src/lib/__fixtures__/test-scenarios.ts (1)

36-36: Consider using const assertions for cryptosuite names.

The as CryptoSuiteName type assertions could be made safer with const assertions to catch typos at compile time.

 export const bbs2020TestScenarios: BBS2020TestScenario[] = [
   {
-    cryptosuite: 'BbsBlsSignature2020' as CryptoSuiteName,
+    cryptosuite: 'BbsBlsSignature2020' as const,
     // ...
   },

 export const modernCryptosuiteTestScenarios: ModernCryptosuiteTestScenario[] = [
   {
-    cryptosuite: 'ecdsa-sd-2023' as CryptoSuiteName,
+    cryptosuite: 'ecdsa-sd-2023' as const,
     // ...

Also applies to: 43-43, 54-54, 62-62, 70-70, 78-78

packages/w3c-vc/src/lib/w3c-vc.ts (3)

198-206: Consider adding JSDoc @throws annotation.

The function can throw errors during CBOR decoding or base64 decoding. Consider documenting these potential exceptions.

 /**
  * Extracts mandatory pointers from ECDSA-SD-2023 or BBS-2023 base proof values
  * @param {string} proofValue - The base proof value string
  * @param {string} cryptosuite - The cryptosuite type ('ecdsa-sd-2023' or 'bbs-2023')
  * @returns {Promise<string[]>} - Array of mandatory pointers, empty array if extraction fails
+ * @throws {Error} - May throw during base64 or CBOR decoding of malformed proof values
  */

225-236: Identical array structure handling needs clarification.

Both ECDSA-SD-2023 and BBS-2023 branches have identical logic for extracting mandatory pointers from position 4 of the components array. If this is intentional and both cryptosuites share the same CBOR structure, consider extracting this to reduce code duplication.

-    if (cryptosuite === 'ecdsa-sd-2023') {
-      // ECDSA-SD-2023 components: [baseSignature, publicKey, hmacKey, signatures, mandatoryPointers]
-      if (Array.isArray(components) && components.length === 5) {
-        return components[4] || [];
-      }
-    } else if (cryptosuite === 'bbs-2023') {
-      // BBS-2023 components: [baseSignature, publicKey, hmacKey, signatures, mandatoryPointers]
-      // Note: BBS-2023 has the same structure as ECDSA-SD-2023 for mandatory pointers
-      if (Array.isArray(components) && components.length === 5) {
-        return components[4] || [];
-      }
+    // Both ECDSA-SD-2023 and BBS-2023 share the same CBOR structure
+    // Components: [baseSignature, publicKey, hmacKey, signatures, mandatoryPointers]
+    if ((cryptosuite === 'ecdsa-sd-2023' || cryptosuite === 'bbs-2023') && 
+        Array.isArray(components) && components.length === 5) {
+      return components[4] || [];
     }

401-405: Consider including the list of supported cryptosuites in error message.

The error message could be more helpful by listing the supported cryptosuites.

         return {
           verified: false,
-          error: `DataIntegrityProof with cryptosuite "${cryptosuite}" is not supported for verification.`,
+          error: `DataIntegrityProof with cryptosuite "${cryptosuite}" is not supported for verification. Supported cryptosuites: ecdsa-sd-2023, bbs-2023.`,
         };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e54756f and ed48f63.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (14)
  • .nxignore (1 hunks)
  • apps/w3c-cli/package.json (1 hunks)
  • apps/w3c-cli/project.json.deprecated (1 hunks)
  • package.json (1 hunks)
  • packages/declaration.d.ts (1 hunks)
  • packages/w3c-vc/README.md (4 hunks)
  • packages/w3c-vc/package.json (1 hunks)
  • packages/w3c-vc/src/lib/__fixtures__/bbs2020-credentials.ts (1 hunks)
  • packages/w3c-vc/src/lib/__fixtures__/key-pairs.ts (1 hunks)
  • packages/w3c-vc/src/lib/__fixtures__/modern-credentials.ts (1 hunks)
  • packages/w3c-vc/src/lib/__fixtures__/test-scenarios.ts (1 hunks)
  • packages/w3c-vc/src/lib/helper/index.ts (2 hunks)
  • packages/w3c-vc/src/lib/w3c-vc.test.ts (1 hunks)
  • packages/w3c-vc/src/lib/w3c-vc.ts (16 hunks)
🧰 Additional context used
🪛 Gitleaks (8.28.0)
packages/w3c-vc/src/lib/__fixtures__/key-pairs.ts

[high] 14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 23-23: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 33-33: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/w3c-vc/src/lib/__fixtures__/bbs2020-credentials.ts

[high] 18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 19-19: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 99-99: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 101-101: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 166-166: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 167-167: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 245-245: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 247-247: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/w3c-vc/src/lib/__fixtures__/modern-credentials.ts

[high] 25-25: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 87-87: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (30)
.nxignore (1)

1-1: Nx ignore entry looks good

Excludes the deprecated CLI app from Nx as intended.

apps/w3c-cli/project.json.deprecated (1)

1-4: Deprecation banner is clear — verify Nx script references elsewhere

Given the file rename and .nxignore, Nx will not recognize this project. Ensure no CI or local scripts still invoke Nx targets for @trustvc/w3c-cli.

packages/w3c-vc/src/lib/helper/index.ts (1)

474-493: Credential ID prefill logic looks good

Correctly assigns URN UUIDs for ecdsa-sd-2023/bbs-2023 and blank-node style otherwise; maintains status list exceptions. Nice touch with TransferableRecords tokenId.

apps/w3c-cli/package.json (1)

4-6: Clear deprecation metadata

Description and deprecated field communicate status well.

packages/w3c-vc/README.md (1)

3-18: Docs update reads well and aligns with added BBS‑2023 support

Clear guidance on signing/deriving/verifying for modern cryptosuites and legacy notes.

packages/declaration.d.ts (1)

2-2: Ambient declaration added for bbs-2023 cryptosuite — good

Prevents TS resolution issues until upstream types exist.

packages/w3c-vc/package.json (1)

34-36: Dependency additions look correct

Adds bbs‑2023 cryptosuite and multikey support; engine >=18 is consistent.

If CI relies on npm 7/8 workspaces, confirm lockfile updates include these additions and no peer dep conflicts arise with jsonld@^6.

packages/w3c-vc/src/lib/w3c-vc.test.ts (4)

8-14: Well-structured test suite organization!

The test suite is well-organized with clear separation between legacy BBS2020 (deprecated) and modern cryptosuites (ECDSA-SD-2023 & BBS-2023). The documentation clearly explains the coverage scope.


127-159: Comprehensive test coverage for modern cryptosuites.

Excellent test coverage for the signing, derivation, and verification workflow. The test properly validates:

  • Successful signing with proof type verification
  • Selective disclosure with mandatory field preservation
  • Verification of derived credentials

233-238: Good security practice: Requiring derivation before verification.

The test correctly validates that base credentials with selective disclosure cryptosuites must be derived before verification. This prevents potential security issues from verifying non-disclosed base credentials.


250-258: Proper enforcement of single derivation constraint.

The test correctly validates that derived credentials cannot be further derived, which is an important security constraint for these cryptosuites.

packages/w3c-vc/src/lib/__fixtures__/modern-credentials.ts (2)

3-64: Good fixture structure for v1.1 credentials.

The credential structure properly follows W3C VC v1.1 specification with appropriate contexts and date fields (issuanceDate, expirationDate).


66-126: Correct v2.0 credential structure.

The v2.0 credential properly uses the updated context URLs and date field names (validFrom, validUntil) as per W3C VC v2.0 specification.

packages/w3c-vc/src/lib/__fixtures__/test-scenarios.ts (2)

16-31: Well-designed test scenario interfaces.

The interfaces clearly separate legacy BBS2020 scenarios from modern cryptosuite scenarios, with appropriate type constraints and clear field names.


52-85: Comprehensive test scenario coverage.

The test scenarios cover all combinations of:

  • Modern cryptosuites (ECDSA-SD-2023, BBS-2023)
  • VC versions (v1.1, v2.0)
  • Appropriate date field names per version

This provides excellent coverage for the test suite.

packages/w3c-vc/src/lib/__fixtures__/bbs2020-credentials.ts (1)

43-44: Placeholder attachment data in fixtures — no base64 decoding usage found

BASE64_ENCODED_FILE placeholders exist in packages/w3c-vc/src/lib/fixtures/modern-credentials.ts and packages/w3c-vc/src/lib/fixtures/bbs2020-credentials.ts; repository search found no base64-decode calls (atob, btoa, Buffer.from(...'base64'), fromBase64, decodeBase64), so tests do not rely on actual base64 decoding.

packages/w3c-vc/src/lib/w3c-vc.ts (14)

3-4: LGTM! BBS-2023 imports added correctly.

The new imports for BLS12-381 multikey and BBS-2023 cryptosuite are properly added to support the new BBS-2023 functionality.


23-32: LGTM! Well-structured cryptosuite function extraction.

The destructuring pattern for both ECDSA-SD-2023 and BBS-2023 cryptosuite functions is clean and consistent, making the code more readable and maintainable.


156-162: LGTM! Type-safe base proof detector mapping.

The baseProofDetectors object and SupportedCryptosuite type provide excellent type safety for the supported cryptosuites.


181-189: LGTM! BBS-2023 support added to isDerived function.

The extension to support BBS-2023 alongside ECDSA-SD-2023 is implemented correctly, using the appropriate base proof detector for each cryptosuite.


265-269: LGTM! Clear deprecation message for BbsBlsSignature2020.

The error message properly guides users to migrate from the deprecated BbsBlsSignature2020 to the latest cryptosuite versions.


270-279: LGTM! Proper key pair instantiation for each cryptosuite.

The conditional logic correctly instantiates either EcdsaMultikey or Bls12381Multikey based on the selected cryptosuite.


305-314: LGTM! Clean cryptosuite instance creation.

The DataIntegrityProof suite is properly configured with the appropriate sign cryptosuite and signer for each supported cryptosuite type.


374-385: LGTM! Excellent validation for base credentials.

The verification logic correctly prevents direct verification of base credentials and provides a clear error message directing users to derive them first.


388-397: LGTM! Proper verification cryptosuite selection.

The verification cryptosuite is correctly instantiated based on the cryptosuite type with clean conditional logic.


478-482: LGTM! Consistent deprecation message for derivation.

The deprecation message for BbsBlsSignature2020 in derivation is consistent with the signing function.


488-508: LGTM! Excellent validation to prevent double derivation.

The logic correctly prevents attempting to derive from an already-derived credential, with a clear error message explaining the limitation.


530-538: LGTM! Clean selective disclosure implementation.

The DataIntegrityProof suite is properly configured with the appropriate disclose cryptosuite for selective disclosure.


524-528: Document auto-inclusion of /credentialSubject

The code auto-adds '/credentialSubject' when not selected (packages/w3c-vc/src/lib/w3c-vc.ts:524–528). Add/update README.md or package docs to state when/why this happens and its impact on selective disclosure and W3C VC compliance — no existing documentation found.


132-154: Verify BBS‑2023 proof header bytes against W3C spec

Current detection checks for magic bytes 0xd9, 0x5d and an even third byte (0x02/0x04/0x06/0x08) to identify base proofs — confirm that exact header layout and the third‑byte→proof‑type mapping are specified in the W3C BBS‑2023 proofValue serialization / Base Proof Configuration. If the spec differs, replace the magic‑byte checks with parsing logic that follows the spec.

Location: packages/w3c-vc/src/lib/w3c-vc.ts Lines 132–154

Comment on lines +8 to +15
export const bbs2020KeyPair: BBSPrivateKeyPair = {
id: 'did:web:trustvc.github.io:did:1#keys-1',
controller: 'did:web:trustvc.github.io:did:1',
type: VerificationType.Bls12381G2Key2020,
publicKeyBase58:
'oRfEeWFresvhRtXCkihZbxyoi2JER7gHTJ5psXhHsdCoU1MttRMi3Yp9b9fpjmKh7bMgfWKLESiK2YovRd8KGzJsGuamoAXfqDDVhckxuc9nmsJ84skCSTijKeU4pfAcxeJ',
privateKeyBase58: '4LDU56PUhA9ZEutnR1qCWQnUhtLtpLu2EHSq4h1o7vtF',
};
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

Fix insecure test key exposure in version control.

The fixture contains hardcoded private keys (privateKeyBase58 and secretKeyMultibase) which should not be committed to version control, even for test fixtures. This poses security risks if developers accidentally use these keys in non-test environments.

Consider these alternatives:

  1. Generate keys dynamically in test setup
  2. Use environment variables for test keys
  3. Mock the key pairs in tests

Apply this pattern to protect sensitive test data:

-export const bbs2020KeyPair: BBSPrivateKeyPair = {
-  id: 'did:web:trustvc.github.io:did:1#keys-1',
-  controller: 'did:web:trustvc.github.io:did:1',
-  type: VerificationType.Bls12381G2Key2020,
-  publicKeyBase58:
-    'oRfEeWFresvhRtXCkihZbxyoi2JER7gHTJ5psXhHsdCoU1MttRMi3Yp9b9fpjmKh7bMgfWKLESiK2YovRd8KGzJsGuamoAXfqDDVhckxuc9nmsJ84skCSTijKeU4pfAcxeJ',
-  privateKeyBase58: '4LDU56PUhA9ZEutnR1qCWQnUhtLtpLu2EHSq4h1o7vtF',
-};
+// Use a factory function that can be mocked in tests
+export const createBbs2020KeyPair = (): BBSPrivateKeyPair => ({
+  id: 'did:web:trustvc.github.io:did:1#keys-1',
+  controller: 'did:web:trustvc.github.io:did:1',
+  type: VerificationType.Bls12381G2Key2020,
+  publicKeyBase58: process.env.TEST_BBS2020_PUBLIC_KEY || 'mock-public-key',
+  privateKeyBase58: process.env.TEST_BBS2020_PRIVATE_KEY || 'mock-private-key',
+});
+
+export const bbs2020KeyPair = createBbs2020KeyPair();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const bbs2020KeyPair: BBSPrivateKeyPair = {
id: 'did:web:trustvc.github.io:did:1#keys-1',
controller: 'did:web:trustvc.github.io:did:1',
type: VerificationType.Bls12381G2Key2020,
publicKeyBase58:
'oRfEeWFresvhRtXCkihZbxyoi2JER7gHTJ5psXhHsdCoU1MttRMi3Yp9b9fpjmKh7bMgfWKLESiK2YovRd8KGzJsGuamoAXfqDDVhckxuc9nmsJ84skCSTijKeU4pfAcxeJ',
privateKeyBase58: '4LDU56PUhA9ZEutnR1qCWQnUhtLtpLu2EHSq4h1o7vtF',
};
// Use a factory function that can be mocked in tests
export const createBbs2020KeyPair = (): BBSPrivateKeyPair => ({
id: 'did:web:trustvc.github.io:did:1#keys-1',
controller: 'did:web:trustvc.github.io:did:1',
type: VerificationType.Bls12381G2Key2020,
publicKeyBase58: process.env.TEST_BBS2020_PUBLIC_KEY || 'mock-public-key',
privateKeyBase58: process.env.TEST_BBS2020_PRIVATE_KEY || 'mock-private-key',
});
export const bbs2020KeyPair = createBbs2020KeyPair();
🧰 Tools
🪛 Gitleaks (8.28.0)

[high] 14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🤖 Prompt for AI Agents
packages/w3c-vc/src/lib/__fixtures__/key-pairs.ts lines 8-15: the fixture
currently contains committed private key material
(privateKeyBase58/secretKeyMultibase); remove hardcoded secrets from source and
instead provide a non-secret fixture plus a factory or helper that generates
keys at test runtime or reads them from environment variables/mocked providers.
Concretely, delete the private key fields from the exported constant, add/export
a function (used by tests) that either generates a new keypair programmatically
or loads a private key from a TEST_ONLY env var, and update tests to call that
factory so no secret material is stored in VCS.

Comment on lines +17 to +24
export const ecdsa2023KeyPair: EcdsaSd2023PrivateKeyPair = {
'@context': 'https://w3id.org/security/multikey/v1',
id: 'did:web:trustvc.github.io:did:1#multikey-1',
type: VerificationType.Multikey,
controller: 'did:web:trustvc.github.io:did:1',
publicKeyMultibase: 'zDnaemDNwi4G5eTzGfRooFFu5Kns3be6yfyVNtiaMhWkZbwtc',
secretKeyMultibase: 'z42tmUXTVn3n9BihE6NhdMpvVBTnFTgmb6fw18o5Ud6puhRW',
};
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

Private keys exposed in ECDSA-2023 key pair fixture.

Same security concern as above - the secretKeyMultibase should not be hardcoded.

Apply similar protection:

-export const ecdsa2023KeyPair: EcdsaSd2023PrivateKeyPair = {
-  '@context': 'https://w3id.org/security/multikey/v1',
-  id: 'did:web:trustvc.github.io:did:1#multikey-1',
-  type: VerificationType.Multikey,
-  controller: 'did:web:trustvc.github.io:did:1',
-  publicKeyMultibase: 'zDnaemDNwi4G5eTzGfRooFFu5Kns3be6yfyVNtiaMhWkZbwtc',
-  secretKeyMultibase: 'z42tmUXTVn3n9BihE6NhdMpvVBTnFTgmb6fw18o5Ud6puhRW',
-};
+export const createEcdsa2023KeyPair = (): EcdsaSd2023PrivateKeyPair => ({
+  '@context': 'https://w3id.org/security/multikey/v1',
+  id: 'did:web:trustvc.github.io:did:1#multikey-1',
+  type: VerificationType.Multikey,
+  controller: 'did:web:trustvc.github.io:did:1',
+  publicKeyMultibase: process.env.TEST_ECDSA2023_PUBLIC_KEY || 'mock-public-key',
+  secretKeyMultibase: process.env.TEST_ECDSA2023_SECRET_KEY || 'mock-secret-key',
+});
+
+export const ecdsa2023KeyPair = createEcdsa2023KeyPair();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const ecdsa2023KeyPair: EcdsaSd2023PrivateKeyPair = {
'@context': 'https://w3id.org/security/multikey/v1',
id: 'did:web:trustvc.github.io:did:1#multikey-1',
type: VerificationType.Multikey,
controller: 'did:web:trustvc.github.io:did:1',
publicKeyMultibase: 'zDnaemDNwi4G5eTzGfRooFFu5Kns3be6yfyVNtiaMhWkZbwtc',
secretKeyMultibase: 'z42tmUXTVn3n9BihE6NhdMpvVBTnFTgmb6fw18o5Ud6puhRW',
};
export const createEcdsa2023KeyPair = (): EcdsaSd2023PrivateKeyPair => ({
'@context': 'https://w3id.org/security/multikey/v1',
id: 'did:web:trustvc.github.io:did:1#multikey-1',
type: VerificationType.Multikey,
controller: 'did:web:trustvc.github.io:did:1',
publicKeyMultibase: process.env.TEST_ECDSA2023_PUBLIC_KEY || 'mock-public-key',
secretKeyMultibase: process.env.TEST_ECDSA2023_SECRET_KEY || 'mock-secret-key',
});
export const ecdsa2023KeyPair = createEcdsa2023KeyPair();
🧰 Tools
🪛 Gitleaks (8.28.0)

[high] 23-23: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🤖 Prompt for AI Agents
In packages/w3c-vc/src/lib/__fixtures__/key-pairs.ts around lines 17 to 24, the
fixture currently hardcodes a private key in secretKeyMultibase; remove the
embedded secret and instead populate secretKeyMultibase from a secure source
(e.g., process.env.SECRET_ECDSA2023_KEY or a test-only loader function) or
replace it with a non-sensitive placeholder/mocked value for tests; ensure any
code that consumes the fixture can handle the injected value and add a short
note/comment to prevent committing real private keys.

Comment on lines +26 to +34
export const bbs2023KeyPair: Bbs2023PrivateKeyPair = {
'@context': 'https://w3id.org/security/multikey/v1',
id: 'did:web:trustvc.github.io:did:1#multikey-2',
type: VerificationType.Multikey,
controller: 'did:web:trustvc.github.io:did:1',
publicKeyMultibase:
'zUC7HnpncVAkTjtL6B8prX6bQM2WA5sJ7rXFeCqyrvPnrzoFBjYsVUTNwzhhPUazja73tWwPeEBWCUgq5qBSrtrXiYhVvBCgZPTCiWANj7TSiZJ6SnyC3pkt94GiuChhAvmRRbt',
secretKeyMultibase: 'z488ur1KSFDd3Y1L6pXcPrZRjE18PNBhgzwJvMeoSxKPNysj',
};
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

Private keys exposed in BBS-2023 key pair fixture.

Same security concern - secretKeyMultibase should not be hardcoded.

Apply similar protection:

-export const bbs2023KeyPair: Bbs2023PrivateKeyPair = {
-  '@context': 'https://w3id.org/security/multikey/v1',
-  id: 'did:web:trustvc.github.io:did:1#multikey-2',
-  type: VerificationType.Multikey,
-  controller: 'did:web:trustvc.github.io:did:1',
-  publicKeyMultibase:
-    'zUC7HnpncVAkTjtL6B8prX6bQM2WA5sJ7rXFeCqyrvPnrzoFBjYsVUTNwzhhPUazja73tWwPeEBWCUgq5qBSrtrXiYhVvBCgZPTCiWANj7TSiZJ6SnyC3pkt94GiuChhAvmRRbt',
-  secretKeyMultibase: 'z488ur1KSFDd3Y1L6pXcPrZRjE18PNBhgzwJvMeoSxKPNysj',
-};
+export const createBbs2023KeyPair = (): Bbs2023PrivateKeyPair => ({
+  '@context': 'https://w3id.org/security/multikey/v1',
+  id: 'did:web:trustvc.github.io:did:1#multikey-2',
+  type: VerificationType.Multikey,
+  controller: 'did:web:trustvc.github.io:did:1',
+  publicKeyMultibase: process.env.TEST_BBS2023_PUBLIC_KEY || 'mock-public-key',
+  secretKeyMultibase: process.env.TEST_BBS2023_SECRET_KEY || 'mock-secret-key',
+});
+
+export const bbs2023KeyPair = createBbs2023KeyPair();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const bbs2023KeyPair: Bbs2023PrivateKeyPair = {
'@context': 'https://w3id.org/security/multikey/v1',
id: 'did:web:trustvc.github.io:did:1#multikey-2',
type: VerificationType.Multikey,
controller: 'did:web:trustvc.github.io:did:1',
publicKeyMultibase:
'zUC7HnpncVAkTjtL6B8prX6bQM2WA5sJ7rXFeCqyrvPnrzoFBjYsVUTNwzhhPUazja73tWwPeEBWCUgq5qBSrtrXiYhVvBCgZPTCiWANj7TSiZJ6SnyC3pkt94GiuChhAvmRRbt',
secretKeyMultibase: 'z488ur1KSFDd3Y1L6pXcPrZRjE18PNBhgzwJvMeoSxKPNysj',
};
export const createBbs2023KeyPair = (): Bbs2023PrivateKeyPair => ({
'@context': 'https://w3id.org/security/multikey/v1',
id: 'did:web:trustvc.github.io:did:1#multikey-2',
type: VerificationType.Multikey,
controller: 'did:web:trustvc.github.io:did:1',
publicKeyMultibase: process.env.TEST_BBS2023_PUBLIC_KEY || 'mock-public-key',
secretKeyMultibase: process.env.TEST_BBS2023_SECRET_KEY || 'mock-secret-key',
});
export const bbs2023KeyPair = createBbs2023KeyPair();
🧰 Tools
🪛 Gitleaks (8.28.0)

[high] 33-33: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

Comment on lines +140 to +150
// Check if it has the BBS-2023 base proof header (0xd9, 0x5d, and even third byte)
// Base proof headers: 0x02 (baseline), 0x04 (anonymous_holder_binding), 0x06 (pseudonym), 0x08 (holder_binding_pseudonym)
// Derived proof headers: 0x03, 0x05, 0x07 (odd numbers)
// Convert to numbers to handle both Buffer (Node.js) and Uint8Array (browser) environments
if (decoded.length < 3 || Number(decoded[0]) !== 0xd9 || Number(decoded[1]) !== 0x5d) {
return false;
}

const thirdByte = Number(decoded[2]);
// Base proofs have even third bytes: 0x02, 0x04, 0x06, 0x08
return thirdByte === 0x02 || thirdByte === 0x04 || thirdByte === 0x06 || thirdByte === 0x08;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add explicit error handling for malformed proof values.

Consider adding more robust validation for the decoded proof value before accessing array indices to prevent potential crashes with malformed inputs.

 const decoded = decode(proofValue.slice(1));
+// Validate minimum required bytes for header check
+if (!decoded || decoded.length < 3) {
+  return false;
+}
 // Check if it has the BBS-2023 base proof header (0xd9, 0x5d, and even third byte)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Check if it has the BBS-2023 base proof header (0xd9, 0x5d, and even third byte)
// Base proof headers: 0x02 (baseline), 0x04 (anonymous_holder_binding), 0x06 (pseudonym), 0x08 (holder_binding_pseudonym)
// Derived proof headers: 0x03, 0x05, 0x07 (odd numbers)
// Convert to numbers to handle both Buffer (Node.js) and Uint8Array (browser) environments
if (decoded.length < 3 || Number(decoded[0]) !== 0xd9 || Number(decoded[1]) !== 0x5d) {
return false;
}
const thirdByte = Number(decoded[2]);
// Base proofs have even third bytes: 0x02, 0x04, 0x06, 0x08
return thirdByte === 0x02 || thirdByte === 0x04 || thirdByte === 0x06 || thirdByte === 0x08;
// Validate minimum required bytes for header check
if (!decoded || decoded.length < 3) {
return false;
}
// Check if it has the BBS-2023 base proof header (0xd9, 0x5d, and even third byte)
// Base proof headers: 0x02 (baseline), 0x04 (anonymous_holder_binding), 0x06 (pseudonym), 0x08 (holder_binding_pseudonym)
// Derived proof headers: 0x03, 0x05, 0x07 (odd numbers)
// Convert to numbers to handle both Buffer (Node.js) and Uint8Array (browser) environments
if (decoded.length < 3 || Number(decoded[0]) !== 0xd9 || Number(decoded[1]) !== 0x5d) {
return false;
}
const thirdByte = Number(decoded[2]);
// Base proofs have even third bytes: 0x02, 0x04, 0x06, 0x08
return thirdByte === 0x02 || thirdByte === 0x04 || thirdByte === 0x06 || thirdByte === 0x08;
🤖 Prompt for AI Agents
packages/w3c-vc/src/lib/w3c-vc.ts lines 140-150: the code currently assumes
`decoded` is an indexable byte array and only checks length, which can throw on
malformed inputs (null/undefined/non-array); add robust validation to guard
before accessing indices by: ensure `decoded` is not null/undefined, is an
object with a numeric `length` and has numeric index properties (e.g., Array,
Uint8Array, Buffer) or coerce safely; if validation fails return false (or throw
a controlled error) and wrap the byte-inspection in a try/catch to return false
on any unexpected exception; keep existing numeric checks (Number(decoded[0])
etc.) intact once validated.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
33.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@rongquan1 rongquan1 requested a review from RishabhS7 September 24, 2025 08:10
@rongquan1 rongquan1 merged commit 082e6f0 into alpha Sep 24, 2025
17 of 19 checks passed
@rongquan1 rongquan1 deleted the feat/bbs-2023-implementation branch September 24, 2025 09:31
@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 1.3.0-alpha.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 1.3.0-alpha.12 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 1.3.0-alpha.12 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

🎉 This PR is included in version 1.3.0-alpha.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

🎉 This PR is included in version 1.3.0-alpha.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

@coderabbitai coderabbitai Bot mentioned this pull request Nov 10, 2025
@coderabbitai coderabbitai Bot mentioned this pull request May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants