feat!: unify secret function signature across all code paths

[paolochiodi]

BREAKING CHANGE: The secret function signature is now called with (context, callback) where context is { operation, payload, header?, signature?, request? } instead of the previous inconsistent signatures (request, token, callback) or (request, payload).

This PR fixes #388, by changing how secret is handled - especially when it's a function. This happens in three ways:

• resolve secret before calling fast-jwt (specific fix for Secret function is called with different signature depending on context #388)
• uniform signature for the secret function across request and instance level functions: I propose to just pass a context object as first parameter, which will include the payload (always present) and an optional request (when called from request level methods and a request context is present)
• uniform signature for the secret function when called from either sign or verify - making the difference explicit by adding an operation attribute to the context. The other difference is that when called from a sign function, the context will be missing header and signature

Tests have been added to cover function being called on instances, and checks for the various context options.

_Disclaimer: Claude Opus was used to assist development

Checklist

• run npm run test && npm run benchmark --if-present
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[Copilot]

Pull request overview

This PR implements the breaking change described in #388 by standardizing how secret functions are invoked across all signing/verifying entry points. It introduces a single (context, callback) secret-function signature (with a context.operation discriminator) and resolves function-based secrets before calling fast-jwt, plus updates docs, typings, and tests accordingly.

Changes:

• Introduce a SecretContext object (operation, payload, and for verify also header/signature, plus optional request) and update TypeScript types to match.
• Update runtime logic to resolve function-based secrets before creating/verifying tokens (including adding a complete decoder for verify context).
• Expand tests and README documentation to cover the new signature and behavior (including decode error cases).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
jwt.js	Adds resolveSecret, builds SecretContext, and changes sign/verify flows to resolve secrets per call when needed.
types/jwt.d.ts	Defines SecretContext* types and updates Secret function signatures to (context, cb) / (context) => Promise.
types/jwt.tst.ts	Updates type tests to use the new secret function signatures.
test/jwt.test.js	Adds coverage for server-method sign/verify with function secrets and context shape; adds decode error tests.
README.md	Documents the new secret function context/callback signature and callback requirement for server methods when using function secrets.

Comments suppressed due to low confidence (3)

jwt.js:336

• In sign(), when a callback is provided you always resolve secretOrPrivateKey and then overwrite signerConfig.options.key via withResolvedKey(...). This means a per-call options.key override is ignored in callback mode, even though the README documents sign.key overriding the plugin secret. The callback code path should prefer an explicitly provided signerConfig.options.key and only call the plugin secret function when no key override is present.

    const cb = signerConfig.callback
    const context = { operation: 'sign', payload }
    resolveSecret(secretOrPrivateKey, context, function (err, secret) {
      if (err) return cb(err)
      const resolvedOptions = withResolvedKey(signerConfig.options, secret)
      const localSigner = createSigner(resolvedOptions)
      cb(null, localSigner(payload))
    })

jwt.js:363

• In verify(), the callback code path always decodes the token and resolves secretOrPublicKey, then overwrites verifierConfig.options.key with that value. This ignores a per-call options.key override in callback mode (contrary to the documented verify.key behavior). It should only call the plugin-level secret function when verifierConfig.options.key is not already provided.

    const cb = verifierConfig.callback
    const decoded = completeDecoder(token)
    const context = { operation: 'verify', header: decoded.header, payload: decoded.payload, signature: decoded.signature }
    resolveSecret(secretOrPublicKey, context, function (err, secret) {
      if (err) return cb(err)
      const resolvedOptions = withResolvedKey(verifierConfig.options, secret)
      const localVerifier = getVerifier(resolvedOptions)
      cb(null, localVerifier(token))
    })

jwt.js:352

• The assert(hasStaticPublicKey, 'callback is required when secret is a function') check blocks synchronous fastify.jwt.verify(token, { key: ... }) when the plugin secret is a function, even though a static per-call options.key should make verification possible without resolving the secret function. Consider basing this assertion on whether verifierConfig.options.key is set (and static) rather than the plugin secret type.

    if (typeof verifierConfig.callback !== 'function') {
      assert(hasStaticPublicKey, 'callback is required when secret is a function')
      let localVerifier = verifier
      if (options && typeof options !== 'function') {
        localVerifier = getVerifier(verifierConfig.options)
      }
      return localVerifier(token)

---

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

[paolochiodi]

Implemented a mitigation similar to the suggested change.

[paolochiodi]

After investigation, the issue here seems more fundamental: sign options object allow the caller to override the key attribute, and the code doesn't account for that - later also using secretOrPrivateKey instead of options.key - potentially losing the override.

I probably missed this because of the naming discrepancy: what is called secret in the plugin setup options, is now called key in the sign function.

I'll consider a change.

[paolochiodi]

I fixed the issue as in my previous comment. Now the code for the instance-level sign and verify properly use the options input to override the plugin config.
As part of that I also expanded options.key type to also optionally be a function (it was either a string or buffer beforehand).
This was necessary because even if the type only allowed string or buffer, not runtime check actually controlled that - the options were just passed to fast-jwt which supported functions (although with a different signature). This made plain javascript and typescript code behavior different.
Instead of adding runtime checks I expanded the type to match the actual code.

However, I wanted to highlight another discrepancy I decided to not address here. The same logical concept is called key in the sign and verify options, but secret on the plugin config.

[paolochiodi]

I guess this is a fair comment. I'll experiment with keeping a fast-path here, and see how much complexity is added to the code

[paolochiodi]

Added back a fast-path when key is static and no custom option is provided to the methods.

[mcollina]

are you adressing those feedback?

[paolochiodi]

are you adressing those feedback?

Yes, working on it - it would take me a few days.

[paolochiodi]

I've addressed copilot's feedback, rebased after and resolved conflicts that came from #408 being merged.
I've responded to the threads with my take and explanations.

I'm going to trigger another review from copilot.

[Copilot]

Pull request overview

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

[paolochiodi]

Fixed the callback delivery issue: when complete decoding is needed, its errors are now caught and passed to the callback before secret resolution. Regression tests cover malformed tokens and invalid types with static secrets, function secrets, and per-call keys.

I kept the original fast-jwt error rather than mapping it to AuthorizationTokenInvalidError. The other instance verify paths expose raw fast-jwt errors, whereas request verification applies the HTTP-oriented plugin errors. This preserves that distinction instead of introducing different error types depending on which instance verification path runs.

[paolochiodi]

Added the try/catch: synchronous provider failures before completion now go through once(error), preserving the original error.

There is one deliberate guard: synchronous exceptions after completion are rethrown, so exceptions from downstream callbacks are not silently swallowed. I also moved instance sign/verify success callbacks outside the operation try/catch blocks to prevent a throwing consumer callback from being invoked a second time.

Tests cover throwing providers, plugin and per-call keys, request/reply callback and Promise APIs, and providers that both invoke a callback and return a Promise.

[paolochiodi]

The current requestVerify implementation merges plugin-level and per-call decode options, then passes them to decode() with complete: true. This enforces the effective checkTyp before secret resolution while retaining the full token context needed by the provider.

Added regression tests for stricter per-call checks, overriding global checks, retaining defaults, and complete: false, using both callback and Promise APIs.

For the instance API, nested options.decode is not part of the supported contract. fastify.jwt.verify accepts flat verification options, including checkTyp; I have left that API unchanged.

[paolochiodi]

Addressed by reusing the existing decode() function in requestVerify, rather than introducing another mapping helper. The duplicated malformed/invalid-type mapping is gone, and regression tests cover the request error code, HTTP status, and custom messages.

Instance verify deliberately retains raw fast-jwt errors to match its other execution paths; its callback error delivery is fixed separately without applying the request-level mapping.

[paolochiodi]

@mcollina I finally came back to this PR. I brought it up to date with latest changes, then addressed the comments from the last review round.

I've also updated the repro repo and migrated to typescript so types are properly checked too: https://github.com/paolochiodi/fastify-jwt-secret-as-function-repro