Skip to content

fix(askar): use NativeAskar.instance instead of the deprecated askar export#2843

Open
tarunvadde wants to merge 1 commit into
openwallet-foundation:mainfrom
tarunvadde:fix/askar-native-instance
Open

fix(askar): use NativeAskar.instance instead of the deprecated askar export#2843
tarunvadde wants to merge 1 commit into
openwallet-foundation:mainfrom
tarunvadde:fix/askar-native-instance

Conversation

@tarunvadde

Copy link
Copy Markdown
Contributor

The KMS reads the askar binding through the deprecated askar export from askar-shared. That binding is only set as a side effect of importing askar-nodejs, so if anything loads the KMS first (ESM, bundlers, test runners) it's still undefined and key ops throw Cannot read properties of undefined (reading 'keyGetJwkSecret').

This switches AskarKeyManagementService and the ECDH derive helpers (plus the two tests that used it) to NativeAskar.instance, the getter added in askar 0.6.0 (openwallet-foundation/askar-wrapper-javascript#77 / #78), which resolves the binding on each call so import order stops mattering.

NativeAskar only exists from 0.6.0, so the askar-shared peer is narrowed to ^0.6.0 and the changeset is minor. Flagging that this drops pre-0.6.0 support, in case anyone still pins an older askar.

Relates to #2597, #2607.

…export

Signed-off-by: Tarun Vadde <vaddeofficial@gmail.com>
@tarunvadde tarunvadde requested a review from a team as a code owner June 23, 2026 21:08
@changeset-bot

changeset-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: fc49aa2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages
Name Type
@credo-ts/askar Minor
@credo-ts/askar-to-drizzle-storage-migration Minor
@credo-ts/action-menu Minor
@credo-ts/anoncreds Minor
@credo-ts/cheqd Minor
@credo-ts/core Minor
@credo-ts/didcomm Minor
@credo-ts/drizzle-storage Minor
@credo-ts/drpc Minor
@credo-ts/hedera Minor
@credo-ts/indy-vdr Minor
@credo-ts/node Minor
@credo-ts/openid4vc Minor
@credo-ts/question-answer Minor
@credo-ts/react-native Minor
@credo-ts/redis-cache Minor
@credo-ts/tenants Minor
@credo-ts/webvh Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -1,5 +1,5 @@
import { Kms, TypedArrayEncoder } from '@credo-ts/core'
import { askar, Key } from '@openwallet-foundation/askar-shared'
import { Key, NativeAskar } from '@openwallet-foundation/askar-shared'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should try to read Askar from the module config: https://github.com/openwallet-foundation/credo-ts/blob/main/packages/askar/src/AskarModuleConfig.ts#L130

  • inject Moduleconfig on agent context
  • extract askar instance from it
  • We should probably update the askar getter in the module config, to see if Askar is passed or the NativeAskar?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, reading the injected instance from AskarModuleConfig is cleaner than the global getter,.

Two things I want to confirm before I push it:

  1. Scope. deriveKey, fetchAskarKey, and encrypt/decrypt all have agentContext, so those are straightforward. But keyFromJwk, privateJwkFromKey, and publicJwkFromKey don't receive agentContext, and they're called from ~13 spots across getPublicKey/createKey/sign/verify/encrypt/decrypt. Doing this consistently means threading the resolved instance through those helpers too (doing it only in deriveKey would leave the JWK paths on the global, which defeats the purpose). Good with that?

  2. The getter. askar is currently a required option, so config.askar always resolves and a fallback isn't strictly needed. Do you want me to make it optional and fall back to NativeAskar.instance when it isn't passed? Heads up that this decides the peer range: with the fallback we keep referencing NativeAskar, so askar-shared stays at ^0.6.0 (changeset minor); if askar stays required, nothing references NativeAskar anymore and the peer can go back to the broad range (patch).

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.

2 participants