From ce143f6d6f4847ad4ec37e8aa28e63a10e6c93fa Mon Sep 17 00:00:00 2001 From: Joakim Antman Date: Wed, 2 Sep 2026 14:17:12 +0300 Subject: [PATCH 1/2] Fix JWT::JWK::Set sharing its key collection when copied JWT::JWK::Set.new(other_set) returned the source set's `keys` array by reference, despite the "Simple duplication" comment on that branch. The class also defined no `initialize_copy`, so `dup` and `clone` shared the array as well. Mutating a copy through `add`, `<<`, `delete`, `select!`, `reject!` or `uniq!` therefore mutated the original too, which matters because key sets are commonly cached and shared between consumers. `union` was unaffected, as `merge` rebinds `@keys` with `+=` instead of mutating. Both paths now copy the array. The JWK objects themselves stay shared. Fixes #750 --- CHANGELOG.md | 1 + lib/jwt/jwk/set.rb | 9 ++++++- spec/jwt/jwk/set_spec.rb | 55 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca5c1d89..c738af18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Fix rejection of unknown algorithms from JWKs for RFC compliance and pquip [#728](https://github.com/jwt/ruby-jwt/pull/728) - Fix the `Style/DirectiveScope` RuboCop offense failing the build [#752](https://github.com/jwt/ruby-jwt/pull/752) +- Fix `JWT::JWK::Set` sharing its key collection with the set it was copied from [#751](https://github.com/jwt/ruby-jwt/pull/751) ## [v3.2.0](https://github.com/jwt/ruby-jwt/tree/v3.2.0) (2026-05-13) diff --git a/lib/jwt/jwk/set.rb b/lib/jwt/jwk/set.rb index 2a11aee5..b48330bb 100644 --- a/lib/jwt/jwk/set.rb +++ b/lib/jwt/jwk/set.rb @@ -17,7 +17,7 @@ def initialize(jwks = nil, options = {}) # rubocop:disable Metrics/CyclomaticCom @keys = case jwks when JWT::JWK::Set # Simple duplication - jwks.keys + jwks.keys.dup when JWT::JWK::KeyBase # Singleton [jwks] when Hash @@ -34,6 +34,13 @@ def initialize(jwks = nil, options = {}) # rubocop:disable Metrics/CyclomaticCom end end + # Ensures a duplicated set owns its key collection. The keys themselves are + # intentionally shared; only the collection is copied. + def initialize_copy(other) + super + @keys = other.keys.dup + end + def export(options = {}) { keys: @keys.map { |k| k.export(options) } } end diff --git a/spec/jwt/jwk/set_spec.rb b/spec/jwt/jwk/set_spec.rb index 4678db08..480a8218 100644 --- a/spec/jwt/jwk/set_spec.rb +++ b/spec/jwt/jwk/set_spec.rb @@ -36,6 +36,61 @@ end end + context 'when created from an existing JWT::JWK::Set' do + let(:jwk) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('testkey') }) } + let(:other) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('otherkey') }) } + let(:original) { described_class.new([jwk]) } + let(:copy) { described_class.new(original) } + + it 'does not share the key collection with the original' do + expect(copy.keys).not_to be(original.keys) + end + + it 'keeps the original intact when keys are added to the copy' do + copy.add(other) + expect(original.keys).to eql([jwk]) + end + + it 'keeps the original intact when keys are removed from the copy' do + copy.delete(jwk) + expect(original.keys).to eql([jwk]) + end + + it 'keeps the original intact when the copy is filtered' do + copy.select! { false } + expect(original.keys).to eql([jwk]) + end + + it 'shares the key objects with the original' do + expect(copy.keys.first).to be(original.keys.first) + end + end + + context 'when duplicated' do + let(:jwk) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('testkey') }) } + let(:other) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('otherkey') }) } + let(:original) { described_class.new([jwk]) } + + it 'does not share the key collection with the original' do + expect(original.dup.keys).not_to be(original.keys) + end + + it 'keeps the original intact when keys are added to the duplicate' do + original.dup << other + expect(original.keys).to eql([jwk]) + end + + it 'keeps the original intact when the duplicate is filtered' do + original.dup.reject! { true } + expect(original.keys).to eql([jwk]) + end + + it 'keeps the original intact when a union is built from it' do + original.union([other]) + expect(original.keys).to eql([jwk]) + end + end + it 'ignores keys with unsupported kty values (RFC 7517 ยง5), required for hybrid PQC' do jwks = { keys: [ From 650851a8018b5c934e2612fa9bb49ef264b69ee7 Mon Sep 17 00:00:00 2001 From: Joakim Antman Date: Thu, 3 Sep 2026 15:57:20 +0300 Subject: [PATCH 2/2] Duplicate the copied key collection through the ivar After `super`, `initialize_copy` already holds a reference to the source set's array in `@keys`, so duplicate that directly instead of going through the `keys` reader. A subclass overriding `keys` with a filtered or computed view then keeps its own state on copy. Also move the union regression test out of the duplication context, since it does not duplicate anything. --- lib/jwt/jwk/set.rb | 2 +- spec/jwt/jwk/set_spec.rb | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/jwt/jwk/set.rb b/lib/jwt/jwk/set.rb index b48330bb..c1dff7b5 100644 --- a/lib/jwt/jwk/set.rb +++ b/lib/jwt/jwk/set.rb @@ -38,7 +38,7 @@ def initialize(jwks = nil, options = {}) # rubocop:disable Metrics/CyclomaticCom # intentionally shared; only the collection is copied. def initialize_copy(other) super - @keys = other.keys.dup + @keys = @keys.dup end def export(options = {}) diff --git a/spec/jwt/jwk/set_spec.rb b/spec/jwt/jwk/set_spec.rb index 480a8218..9c0adf11 100644 --- a/spec/jwt/jwk/set_spec.rb +++ b/spec/jwt/jwk/set_spec.rb @@ -84,8 +84,14 @@ original.dup.reject! { true } expect(original.keys).to eql([jwk]) end + end + + context 'when a union is built from it' do + let(:jwk) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('testkey') }) } + let(:other) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('otherkey') }) } + let(:original) { described_class.new([jwk]) } - it 'keeps the original intact when a union is built from it' do + it 'keeps the original intact' do original.union([other]) expect(original.keys).to eql([jwk]) end