Skip to content

Commit ce143f6

Browse files
committed
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
1 parent bedd65a commit ce143f6

3 files changed

Lines changed: 64 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
- Fix rejection of unknown algorithms from JWKs for RFC compliance and pquip [#728](https://github.com/jwt/ruby-jwt/pull/728)
1414
- Fix the `Style/DirectiveScope` RuboCop offense failing the build [#752](https://github.com/jwt/ruby-jwt/pull/752)
15+
- Fix `JWT::JWK::Set` sharing its key collection with the set it was copied from [#751](https://github.com/jwt/ruby-jwt/pull/751)
1516

1617
## [v3.2.0](https://github.com/jwt/ruby-jwt/tree/v3.2.0) (2026-05-13)
1718

lib/jwt/jwk/set.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def initialize(jwks = nil, options = {}) # rubocop:disable Metrics/CyclomaticCom
1717

1818
@keys = case jwks
1919
when JWT::JWK::Set # Simple duplication
20-
jwks.keys
20+
jwks.keys.dup
2121
when JWT::JWK::KeyBase # Singleton
2222
[jwks]
2323
when Hash
@@ -34,6 +34,13 @@ def initialize(jwks = nil, options = {}) # rubocop:disable Metrics/CyclomaticCom
3434
end
3535
end
3636

37+
# Ensures a duplicated set owns its key collection. The keys themselves are
38+
# intentionally shared; only the collection is copied.
39+
def initialize_copy(other)
40+
super
41+
@keys = other.keys.dup
42+
end
43+
3744
def export(options = {})
3845
{ keys: @keys.map { |k| k.export(options) } }
3946
end

spec/jwt/jwk/set_spec.rb

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,61 @@
3636
end
3737
end
3838

39+
context 'when created from an existing JWT::JWK::Set' do
40+
let(:jwk) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('testkey') }) }
41+
let(:other) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('otherkey') }) }
42+
let(:original) { described_class.new([jwk]) }
43+
let(:copy) { described_class.new(original) }
44+
45+
it 'does not share the key collection with the original' do
46+
expect(copy.keys).not_to be(original.keys)
47+
end
48+
49+
it 'keeps the original intact when keys are added to the copy' do
50+
copy.add(other)
51+
expect(original.keys).to eql([jwk])
52+
end
53+
54+
it 'keeps the original intact when keys are removed from the copy' do
55+
copy.delete(jwk)
56+
expect(original.keys).to eql([jwk])
57+
end
58+
59+
it 'keeps the original intact when the copy is filtered' do
60+
copy.select! { false }
61+
expect(original.keys).to eql([jwk])
62+
end
63+
64+
it 'shares the key objects with the original' do
65+
expect(copy.keys.first).to be(original.keys.first)
66+
end
67+
end
68+
69+
context 'when duplicated' do
70+
let(:jwk) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('testkey') }) }
71+
let(:other) { JWT::JWK.new({ kty: 'oct', k: Base64.strict_encode64('otherkey') }) }
72+
let(:original) { described_class.new([jwk]) }
73+
74+
it 'does not share the key collection with the original' do
75+
expect(original.dup.keys).not_to be(original.keys)
76+
end
77+
78+
it 'keeps the original intact when keys are added to the duplicate' do
79+
original.dup << other
80+
expect(original.keys).to eql([jwk])
81+
end
82+
83+
it 'keeps the original intact when the duplicate is filtered' do
84+
original.dup.reject! { true }
85+
expect(original.keys).to eql([jwk])
86+
end
87+
88+
it 'keeps the original intact when a union is built from it' do
89+
original.union([other])
90+
expect(original.keys).to eql([jwk])
91+
end
92+
end
93+
3994
it 'ignores keys with unsupported kty values (RFC 7517 §5), required for hybrid PQC' do
4095
jwks = {
4196
keys: [

0 commit comments

Comments
 (0)