Skip to content

Conversation

@ilya-korotya
Copy link
Contributor

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the jwx library from v3.0.10 to v3.0.12 and implements proper header merging for JWE decryption to handle the library's deprecation of legacy header merging behavior.

Key Changes:

  • Upgrades jwx library to v3.0.12 with related dependency updates
  • Implements explicit header merging logic in the Decrypt function to combine protected, unprotected, and per-recipient headers
  • Adds comprehensive test coverage for the new mergeHeaders function

Reviewed Changes

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

File Description
go.mod Updates jwx library to v3.0.12 and related dependencies
packers/providers/jwe/jwe.go Disables legacy header merging and implements custom mergeHeaders function with duplicate key detection
packers/providers/jwe/jwe_test.go Adds comprehensive test coverage for header merging scenarios including error cases
Makefile Adds jwx_es256k build tag to test command

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

expectedErr: "duplicate header key found: alg",
},
{
name: "deuplicate between all three headers",
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'deuplicate' to 'duplicate'.

Suggested change
name: "deuplicate between all three headers",
name: "duplicate between all three headers",

Copilot uses AI. Check for mistakes.
// Merge headers (no duplicates, so safe to merge)
result := jwe.NewHeaders()
if protected != nil {
result = protected
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

Direct assignment of protected headers creates a reference to the original headers object. If the original headers are modified later, the result will be affected. Use result.Merge(protected) instead to create a proper copy.

Suggested change
result = protected
var err error
result, err = result.Merge(protected)
if err != nil {
return nil, errors.Wrap(err, "failed to merge protected headers")
}

Copilot uses AI. Check for mistakes.
@ilya-korotya
Copy link
Contributor Author

I don't like that we now have to build the library with -tags jwx_es256k. This means that all services that will use jws from the library will have to add the -tags jwx_es256k build tag. I didn't find a solution to enable the tag by default.

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