Bug: fix VP JWT marshalling flattening single-entry arrays incorrectly by reinkrul · Pull Request #3958 · nuts-foundation/nuts-node

reinkrul · 2025-12-18T11:55:51Z

We used JSON-LD marshalling semantics for JWT VPs, which is incorrect; single-entry arrays shouldn't be flattened to singular values.

Found during LSPxNuts hackaton.

This applied to the following fields:

type
verifiableCredential
@context

Fixes #3957

TODO:

Switch to release version of go-did after Add support for creating JWT VPs go-did#141 is merged
Fix Presentation Definition: be lenient about single-entry arrays v.s. singular values #4126 as well for improved Presentation Definition compatibility (current PDs might make assumptions about credentialSubject being an object or array)

qltysh · 2025-12-18T12:03:39Z

Coverage Impact

⬇️ Merging this pull request will decrease total coverage on master by 0.03%.

Modified Files with Diff Coverage (2)

Rating	File	% Diff	Uncovered Line #s
	vcr/holder/presenter.go	100.0%
	vcr/pe/presentation_submission.go	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.
Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.
File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)
- Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

…-marshalling

…ngraded dependencies - Update go-did to v0.18.1 which makes ExpiresAt optional (*time.Time), restoring the old behaviour of omitting the exp claim when no expiry is set - Restore avast/retry-go/v4 v4.7.0, gorm.io/gorm v1.31.1, and eko/gocache/lib/v4 v4.2.3 which were accidentally downgraded in a merge conflict Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

reinkrul · 2026-03-26T13:06:14Z

Backwards Compatibility Report

1. JWT VP `type` field: string → array ✅ Compatible

Before (master): "type": "VerifiablePresentation" — a string, because vc.VerifiablePresentation.MarshalJSON() applies Unplural on the type field.

After: "type": ["VerifiablePresentation"] — an array, because CreateJWTVerifiablePresentation (go-did) builds the vp claim as a raw map[string]interface{} instead of going through MarshalJSON.

Why compatible: go-did's ParseVerifiablePresentation applies marshal.Plural(typeKey) during parsing, normalising both forms to []URI. Old nodes receiving new JWTs handle the array form transparently.

2. JWT VP `verifiableCredential`: single string → array ✅ Compatible

Before (master): "verifiableCredential": "jwt_vc_string" — MarshalJSON applies Unplural for a single VC.

After: "verifiableCredential": ["jwt_vc_string"] — go-did stores credentials as a slice in the raw map, so it's always an array.

The presentation submission path is always generated by the same node that creates the VP, so old/new nodes stay self-consistent:

Holder	Verifier	Submission path	VP `verifiableCredential`	Resolution
new	old	`$.verifiableCredential[0]`	array	index extracts the string ✅
old	new	`$.verifiableCredential`	string	JSONPath returns the string directly ✅
new	new	`$.verifiableCredential[0]`	array	✅
old	old	`$.verifiableCredential`	string	✅ (unchanged)

Additionally, ParseVerifiablePresentation applies Plural(verifiableCredentialKey) during parsing, so both wire forms are normalised correctly.

3. JWT VP expiry ✅ Compatible

ExpiresAt in go-did v0.18.1 is *time.Time (optional). options.ProofOptions.Expires is passed directly — when nil, no exp claim is emitted, identical to master's behaviour.

4. Dependencies ✅ Up to date

All three previously-downgraded dependencies have been restored:

Dependency	Was (bad merge)	Now
`avast/retry-go/v4`	v4.6.1	v4.7.0
`gorm.io/gorm`	v1.30.2	v1.31.1
`eko/gocache/lib/v4`	v4.2.0	v4.2.3

Additionally, go-did was upgraded from v0.18.0 → v0.18.1 to fix the mandatory ExpiresAt constraint.

Overall verdict: the PR is backwards compatible.

Copilot

Pull request overview

Fixes incorrect JWT Verifiable Presentation (VP) JSON marshalling semantics (single-entry arrays being flattened) by upgrading go-did and aligning Nuts-node VP creation/submission logic with JWT VP expectations.

Changes:

Bump github.com/nuts-foundation/go-did from v0.18.0 to v0.18.1 (fixing JWT VP marshalling semantics).
Switch JWT VP creation in presenter to go-did’s JWT VP creation helper and adjust PEX submission mapping path for JWT single-VC cases.
Add/extend tests for issue #3957 to validate JWT VP type is encoded as an array.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`vcr/pe/presentation_submission.go`	Adjusts single-VC mapping path depending on VP format (JWT uses array indexing).
`vcr/pe/presentation_submission_test.go`	Updates expected JWT descriptor map path to `$.verifiableCredential[0]`.
`vcr/holder/presenter.go`	Replaces manual JWT VP claim construction/signing with `vc.CreateJWTVerifiablePresentation`.
`vcr/holder/presenter_test.go`	Adds regression test for JWT VP `type` array marshalling; updates JWT test options.
`auth/client/iam/openid4vp.go`	Import reordering only.
`go.mod`	Updates `github.com/nuts-foundation/go-did` to `v0.18.1`.
`go.sum`	Adds checksums for updated/affected dependencies.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T13:10:24Z

vcr/holder/presenter.go

+	return vc.CreateJWTVerifiablePresentation(ctx, subjectDID.URI(), credentials, vc.PresentationOptions{
+		AdditionalContexts:        options.AdditionalContexts,
+		AdditionalTypes:           options.AdditionalTypes,
+		AdditionalProofProperties: options.ProofOptions.AdditionalProperties,
+		Holder:                    options.Holder,
+		Nonce:                     options.ProofOptions.Nonce,
+		Audience:                  options.ProofOptions.Domain,
+		IssuedAt:                  &options.ProofOptions.Created,
+		ExpiresAt:                 options.ProofOptions.Expires,


In JWT mode this always passes a non-nil IssuedAt pointer, even when options.ProofOptions.Created is the zero time. Previously, when Created was zero, the code defaulted the JWT time claim(s) to time.Now(); now a zero Created could produce an invalid/negative Unix timestamp (or an unexpectedly old value) in the JWT. Consider only setting IssuedAt when Created is non-zero (otherwise pass nil or set it to time.Now() first) to preserve the prior behavior and avoid generating invalid tokens.

vcr/holder/presenter_test.go

Copilot · 2026-03-26T13:10:25Z

vcr/holder/presenter_test.go

+				vpAsMap := result.JWT().PrivateClaims()["vp"].(map[string]any)
+				t.Run("make sure type now marshalls as array", func(t *testing.T) {
+					typeProp := vpAsMap["type"].([]any)


These direct type assertions (to map[string]any and []any) will panic if the JWT claims shape changes or if the regression reappears (e.g., type becomes a string again). Prefer using the comma-ok form or require.IsType/require.Contains to fail the test with a clear assertion message instead of a panic.

Suggested change

vpAsMap := result.JWT().PrivateClaims()["vp"].(map[string]any)

t.Run("make sure type now marshalls as array", func(t *testing.T) {

typeProp := vpAsMap["type"].([]any)

vpClaim, ok := result.JWT().PrivateClaims()["vp"]

require.True(t, ok, "vp claim must be present in JWT private claims")

vpAsMap, ok := vpClaim.(map[string]any)

require.True(t, ok, "vp claim must be a map[string]any, got %T", vpClaim)

t.Run("make sure type now marshalls as array", func(t *testing.T) {

typeVal, ok := vpAsMap["type"]

require.True(t, ok, "vp.type must be present")

typeProp, ok := typeVal.([]any)

require.True(t, ok, "vp.type must be a []any, got %T", typeVal)

Copilot · 2026-03-26T13:10:25Z

vcr/holder/presenter_test.go

+				t.Run("make sure type now marshalls as array", func(t *testing.T) {
+					typeProp := vpAsMap["type"].([]any)
+					assert.Equal(t, []any{"VerifiablePresentation"}, typeProp)
+				})


The PR description mentions the flattening bug also affected @context and verifiableCredential in JWT VPs, but this new regression test only asserts the type field. Consider extending this test to also assert that vp.@context and vp.verifiableCredential are encoded as arrays (even for a single entry), so the full intended fix is covered by automated tests.

Suggested change

})

})

t.Run("make sure @context now marshalls as array", func(t *testing.T) {

contextProp, ok := vpAsMap["@context"].([]any)

require.True(t, ok, "@context must be encoded as an array")

assert.GreaterOrEqual(t, len(contextProp), 1, "@context array must have at least one entry")

})

t.Run("make sure verifiableCredential now marshalls as array", func(t *testing.T) {

vcProp, ok := vpAsMap["verifiableCredential"].([]any)

require.True(t, ok, "verifiableCredential must be encoded as an array")

assert.Len(t, vcProp, 1, "single VC presentation must still use an array")

})

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix VP JWT type marshalling being a []string

c3ab3dc

reinkrul requested review from confiks, gerardsn, stevenvegt and woutslakhorst as code owners December 18, 2025 11:55

Claude AI tried to fix the e2e test

7a27d5b

reinkrul marked this pull request as draft December 18, 2025 13:29

reinkrul added 10 commits December 18, 2025 14:56

Claude AI tried to fix the e2e test

a05b175

Claude AI tried to fix the e2e test (revert 1)

6cdd44d

fix test?

7aa3568

debug level for e2e test

3647757

info level logging

7ec04b0

Attempt

021ad6c

fixzz

630345d

fixzz

f37f7c8

Merge branch 'master' into iss3957-vp-jwt-type-marshalling

ba4dbb3

reverts

ba228ea

reinkrul marked this pull request as ready for review December 24, 2025 13:18

reinkrul changed the title ~~Fix VP JWT type marshalling being a []string~~ Fix VP JWT marshalling flattening single-entry arrays incorrectly Dec 24, 2025

reinkrul changed the title ~~Fix VP JWT marshalling flattening single-entry arrays incorrectly~~ Bug: fix VP JWT marshalling flattening single-entry arrays incorrectly Dec 24, 2025

reinkrul and others added 9 commits February 12, 2026 14:42

Merge branch 'master' into iss3957-vp-jwt-type-marshalling

7dd4275

Merge remote-tracking branch 'origin/master' into iss3957-vp-jwt-type…

22a27a5

…-marshalling

Switch to go-did for creating JWT VPs

25af536

update dependency

f8e2092

Merge branch 'master' into iss3957-vp-jwt-type-marshalling

912f62a

Merge branch 'master' into iss3957-vp-jwt-type-marshalling

56fdbe3

merge

780c91e

fix conflict

41ad809

reinkrul requested review from Copilot and removed request for confiks March 26, 2026 13:05

Copilot started reviewing on behalf of reinkrul March 26, 2026 13:06 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Update vcr/holder/presenter_test.go

dc51558

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug: fix VP JWT marshalling flattening single-entry arrays incorrectly#3958

Bug: fix VP JWT marshalling flattening single-entry arrays incorrectly#3958
reinkrul wants to merge 22 commits intomasterfrom
iss3957-vp-jwt-type-marshalling

reinkrul commented Dec 18, 2025 •

edited

Loading

Uh oh!

qltysh bot commented Dec 18, 2025 •

edited

Loading

Uh oh!

reinkrul commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-				vpAsMap := result.JWT().PrivateClaims()["vp"].(map[string]any)
-				t.Run("make sure type now marshalls as array", func(t *testing.T) {
-					typeProp := vpAsMap["type"].([]any)
+				vpClaim, ok := result.JWT().PrivateClaims()["vp"]
+				require.True(t, ok, "vp claim must be present in JWT private claims")
+				vpAsMap, ok := vpClaim.(map[string]any)
+				require.True(t, ok, "vp claim must be a map[string]any, got %T", vpClaim)
+				t.Run("make sure type now marshalls as array", func(t *testing.T) {
+					typeVal, ok := vpAsMap["type"]
+					require.True(t, ok, "vp.type must be present")
+					typeProp, ok := typeVal.([]any)
+					require.True(t, ok, "vp.type must be a []any, got %T", typeVal)

-				})
+				})
+				t.Run("make sure @context now marshalls as array", func(t *testing.T) {
+					contextProp, ok := vpAsMap["@context"].([]any)
+					require.True(t, ok, "@context must be encoded as an array")
+					assert.GreaterOrEqual(t, len(contextProp), 1, "@context array must have at least one entry")
+				})
+				t.Run("make sure verifiableCredential now marshalls as array", func(t *testing.T) {
+					vcProp, ok := vpAsMap["verifiableCredential"].([]any)
+					require.True(t, ok, "verifiableCredential must be encoded as an array")
+					assert.Len(t, vcProp, 1, "single VC presentation must still use an array")
+				})

Conversation

reinkrul commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qltysh bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reinkrul commented Mar 26, 2026

Backwards Compatibility Report

1. JWT VP type field: string → array ✅ Compatible

2. JWT VP verifiableCredential: single string → array ✅ Compatible

3. JWT VP expiry ✅ Compatible

4. Dependencies ✅ Up to date

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

reinkrul commented Dec 18, 2025 •

edited

Loading

qltysh bot commented Dec 18, 2025 •

edited

Loading

1. JWT VP `type` field: string → array ✅ Compatible

2. JWT VP `verifiableCredential`: single string → array ✅ Compatible