Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Feb 11, 2026

This PR adds a new endpoint, that works exactly like the issuer's "search" API, to find credentials in a wallet that are revoked/expired. The existing List() function on the wallet can't be used for that, since it filters out those unusable credentials (which is used for building VPs).

So, as backwards compatible solution which mimics the existing issuer.SearchCredentials() functionality, I added an API that does the same for wallets:

/internal/vcr/v2/holder/{subjectID}/vc/search

It currently supports no search parameters yet, which can be added later if needed.

This API is required, for being able to inspect and clean up expired/revoked credentials (there's currently no way to list them).

Copilot AI changed the title [WIP] Add wallet credential search to mirror SearchIssuedVCs API call VCR: Add wallet credential search endpoint for expired/revoked credential management Feb 11, 2026
Copilot AI requested a review from reinkrul February 11, 2026 05:13
@qltysh
Copy link

qltysh bot commented Feb 11, 2026

Qlty

Coverage Impact

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

Modified Files with Diff Coverage (6)

RatingFile% DiffUncovered Line #s
Coverage rating: B Coverage rating: B
vcr/holder/sql_wallet.go100.0%
Coverage rating: A Coverage rating: A
vcr/api/vcr/v2/api.go80.0%480-481, 487-488
Coverage rating: A Coverage rating: A
vcr/verifier/verifier.go100.0%
Coverage rating: F Coverage rating: F
vcr/holder/memory_wallet.go0.0%81-83
Coverage rating: C Coverage rating: C
vcr/revocation/statuslist2021_issuer.go100.0%
Coverage rating: A Coverage rating: A
vcr/revocation/statuslist2021_verifier.go100.0%
Total87.5%
🤖 Increase coverage with AI coding...

In the `copilot/add-wallet-credential-search` branch, add test coverage for this new code:

- `vcr/api/vcr/v2/api.go` -- Lines 480-481 and 487-488
- `vcr/holder/memory_wallet.go` -- Line 81-83

🚦 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.

@reinkrul
Copy link
Member

@copilot use VerifyVC() instead of GetRevocation().

Copy link
Contributor Author

Copilot AI commented Feb 11, 2026

@copilot use VerifyVC() instead of GetRevocation().

Changed to use Verifier.Verify() to check credential status before retrieving revocation details. The implementation now calls Verify() first (allowing untrusted and skipping signature check to focus on revocation status), and only calls GetRevocation() if the credential is revoked to get the details. See commit 71ba955.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • REDACTED
    • Triggering command: /tmp/go-build3719477838/b883/vcr.test /tmp/go-build3719477838/b883/vcr.test -test.testlogfile=/tmp/go-build3719477838/b883/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.short=true g_.a -I ux-amd64/pkg/tool/linux_amd64/compile --gdwarf-5 node/crypto/stor-atomic l/linux_amd64/co-bool ux-amd64/pkg/too-buildtags -o trust.go om/prometheus/cl-ifaceassert ux-amd64/pkg/tool/linux_amd64/vet -p github.com/knadh-atomic mpile ux-amd64/pkg/too-buildtags (dns block)
  • cdn.jsdelivr.net
    • Triggering command: /tmp/go-build3719477838/b926/test.test /tmp/go-build3719477838/b926/test.test -test.testlogfile=/tmp/go-build3719477838/b926/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.short=true om/microsoft/go-mssqldb@v1.9.3/i-errorsas .cfg ux-amd64/pkg/tool/linux_amd64/compile -p /azure-sdk-for-g-o mpile ux-amd64/pkg/too-trimpath abis�� om/twmb/murmur3@-p .cfg ux-amd64/pkg/tool/linux_amd64/compile . soft/go-mssqldb/-atomic mpile ux-amd64/pkg/too-buildtags (dns block)
  • nuts.nl
    • Triggering command: /tmp/go-build3719477838/b907/openid4vci.test /tmp/go-build3719477838/b907/openid4vci.test -test.testlogfile=/tmp/go-build3719477838/b907/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.short=true v5.7.5/pgconn/au-errorsas v5.7.5/pgconn/co-ifaceassert ux-amd64/pkg/tool/linux_amd64/vet -p anner -lang=go1.24 ux-amd64/pkg/too-buildtags 6936�� g_.a /tmp/go-build175-ifaceassert ux-amd64/pkg/tool/linux_amd64/vet . g/protobuf/encod-atomic mpile ux-amd64/pkg/too-buildtags (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@reinkrul reinkrul force-pushed the copilot/add-wallet-credential-search branch from ea24c5f to 773071f Compare February 11, 2026 11:32
@reinkrul reinkrul marked this pull request as ready for review February 11, 2026 11:38
Copy link
Member

Choose a reason for hiding this comment

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

Improve the docs of the list:
Lists all VALID VCs in the holders wallet.

Perhaps adding a reference to the search operation for when you want to list ALL VCs.

I personally prefer adding some filter params to the list rather than creating an almost duplicate endpoint.

Something like:
GET /internal/vcr/v2/holder/{subjectID}/vc?include_invalid=true

Copy link
Member

Choose a reason for hiding this comment

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

I'd like an endpoint that includes the revocation status, but the existing list endpoint only returns the credential. We can't simply change that due to backwards compatibility, that's why I decided to introduce a new endpoint.

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, I could alter the List() call to include revoked/expired credentials, but UIs will still have to perform an API call for each credential to check revocation status.

@qltysh
Copy link

qltysh bot commented Feb 11, 2026

1 new issue

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

@reinkrul reinkrul merged commit 17348d9 into master Feb 11, 2026
9 checks passed
@reinkrul reinkrul deleted the copilot/add-wallet-credential-search branch February 11, 2026 15:16
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.

3 participants