Skip to content

fix(vcr/verifier): handle unparseable issuer DID instead of panicking#4302

Open
reinkrul wants to merge 1 commit into
masterfrom
worktree-fix-4235-issuer-did-panic
Open

fix(vcr/verifier): handle unparseable issuer DID instead of panicking#4302
reinkrul wants to merge 1 commit into
masterfrom
worktree-fix-4235-issuer-did-panic

Conversation

@reinkrul
Copy link
Copy Markdown
Member

@reinkrul reinkrul commented Jun 2, 2026

Problem

vcr/verifier/verifier.go discarded the error from did.ParseDID(issuer) on the signature-check path, then dereferenced the resulting nil pointer. An unparseable issuer DID — e.g. a did:x509 with trailing unsupported fields, as emitted by the AET ZORG-ID SDK — made ParseDID return (nil, err), so the dereference panicked and crashed the entire node:

runtime error: invalid memory address or nil pointer dereference

Reproducible by posting such a credential to /internal/vcr/v2/holder/{subject}/vc.

Fix

Capture the parse error and return a clean validation error:

issuerDID, err := did.ParseDID(credentialToVerify.Issuer.String())
if err != nil {
    return fmt.Errorf("could not parse issuer DID: %w", err)
}

Why the validator didn't catch it first

Credentials with an unrecognized type fall back to defaultCredentialValidator, which only checks the issuer is non-empty — it never parses it as a DID. So a malformed issuer reaches the signature-check path untouched.

Test

Added a regression test to TestVerifier_Verify using did:x509:0:sha256:abc::san:otherName:1.2.3.4#extra (valid URI, invalid DID). It asserts a could not parse issuer DID error; without the fix it panics.

Fixes #4235

Assisted by AI

The signature-check path discarded the error from did.ParseDID(issuer)
and dereferenced the resulting nil pointer, crashing the node when a
credential carried an unparseable issuer DID (e.g. a did:x509 with
trailing unsupported fields, as emitted by the AET ZORG-ID SDK).

Capture the parse error and return a clean validation error instead.

Fixes #4235

Assisted by AI
@qltysh
Copy link
Copy Markdown
Contributor

qltysh Bot commented Jun 2, 2026

1 new issue

Tool Category Rule Count
qlty Structure Function with many returns (count = 10): Verify 1

@qltysh
Copy link
Copy Markdown
Contributor

qltysh Bot commented Jun 2, 2026

Qlty


Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (1)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
vcr/verifier/verifier.go100.0%
Total100.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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcr/verifier: nil-pointer panic on unparseable issuer DID

1 participant