Skip to content

feat(vc): JSON-LD credential issuance signing compatibility helper - #592

Open
rmlearney-digicatapult wants to merge 7 commits into
mainfrom
feat/jsonld-credential-helper
Open

rmlearney-digicatapult wants to merge 7 commits into
mainfrom
feat/jsonld-credential-helper

Conversation

@rmlearney-digicatapult

@rmlearney-digicatapult rmlearney-digicatapult commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Checklist

  • Have you read Digital Catapult's Code of Conduct?
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

PR Type

Please delete options that are irrelevant.

  • Feature

Linked tickets

High level description

Ensure the key type selected to sign a JSON-LD credential is one that we advertise in our DID document

Detailed description

Interesting error on testing. Key type selected to sign JSON-LD credential must match the key type we're advertising in our DID Document.

Created new helper to read our did document and available signature suites to confirm compatibility before proceeding to offer or issue the credential, with fail-fast 400 error.

Also removed credentialFormats from the AcceptCredentialRequestOptions interface because this is a security risk, there is no reason to permit override at this late stage of the credential exchange process to contradict the agreed state.

Describe alternatives you've considered

Operational impact

Additional context

Signed-off-by: rmlearney-digicatapult <robert.learney@digicatapult.org.uk>
Signed-off-by: rmlearney-digicatapult <robert.learney@digicatapult.org.uk>
Copilot AI balanced review requested due to automatic review settings September 18, 2026 12:52
@rmlearney-digicatapult
rmlearney-digicatapult requested a review from a team as a code owner September 18, 2026 12:52
@rmlearney-digicatapult rmlearney-digicatapult added the v:patch Change requires a semver patch version change label Sep 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Proposal-derived offers and issuance-time verification-method overrides still bypass the new validation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds fail-fast JSON-LD credential signing compatibility checks by comparing the requested proof type with issuer DID verification methods.

Changes:

  • Adds reusable proof-type/DID validation.
  • Applies validation to direct credential-offer endpoints.
  • Adds mismatch coverage and bumps the patch version.

Blockers (Must-Fix)

  • CredentialController.ts:296: JSON-LD offers created through acceptProposal bypass validation.
  • CredentialController.ts:103-108: acceptRequest verification-method overrides are not validated.

Targeted Suggestions

  • Validate proposal-derived offers before accepting proposals.
  • Validate the actual verification method selected during issuance.

Test Gap Analysis

  • Add coverage for /create-offer, proposal acceptance, and issuance-time verification-method overrides.

Integration Risks

Invalid JSON-LD exchanges can progress until issuance and then fail instead of returning the documented 400 response.

Scores

Category Score
Correctness 6/10
Security 8/10
Testing 6/10
Maintainability 7/10
Performance 9/10
Integration/Infra 6/10
Overall 7/10

Release Notes Draft

  • Validate JSON-LD proof types against issuer DID verification methods.
  • Return HTTP 400 for incompatible direct credential offers.
  • Add JSON-LD signing compatibility tests.
File summaries
File Description
src/utils/credentials.ts Adds proof-type compatibility validation.
src/controllers/v1/credentials/CredentialController.ts Applies validation to direct offer endpoints.
tests/unit/credential.test.ts Tests compatible and incompatible proof types.
package.json Bumps package version.
package-lock.json Synchronizes package version.
Review details

Suppressed comments (1)

src/controllers/v1/credentials/CredentialController.ts:296

  • This validation is bypassed when an issuer accepts a JSON-LD proposal: acceptProposal creates the offer by copying the proposal attachment, but that controller path never calls this helper. A proposer can therefore supply an incompatible proofType, receive an offer, and defer the same signing failure until issuance instead of getting the intended fail-fast 400. Validate the effective JSON-LD offer in acceptProposal as well.
      await this.assertProofTypeMatchesIssuerDid(options.credentialFormats.jsonld)
  • Files reviewed: 4/5 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/controllers/v1/credentials/CredentialController.ts
Comment thread tests/unit/credential.test.ts
Signed-off-by: rmlearney-digicatapult <robert.learney@digicatapult.org.uk>
Signed-off-by: rmlearney-digicatapult <robert.learney@digicatapult.org.uk>
Signed-off-by: rmlearney-digicatapult <robert.learney@digicatapult.org.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v:patch Change requires a semver patch version change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants