From 03202862f4117a33a6a6626b53dff0f2541a54a9 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Sat, 5 Sep 2026 23:19:33 +0900 Subject: [PATCH 1/4] Require Ruby 2.4 or later Match the gem requirement to the oldest Ruby version covered by the test matrix. --- pstore.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/pstore.gemspec b/pstore.gemspec index 86051d2..2831dc9 100644 --- a/pstore.gemspec +++ b/pstore.gemspec @@ -17,6 +17,7 @@ Gem::Specification.new do |spec| spec.description = spec.summary spec.homepage = "https://github.com/ruby/pstore" spec.licenses = ["Ruby", "BSD-2-Clause"] + spec.required_ruby_version = ">= 2.4" spec.metadata["homepage_uri"] = spec.homepage spec.metadata["source_code_uri"] = "https://github.com/ruby/pstore" From 04cb432b8d7cac7bde699c0cf70d36bb688d9de4 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Sun, 6 Sep 2026 10:36:09 +0900 Subject: [PATCH 2/4] Accept `thread_safe` as a keyword argument Preserve the positional argument while allowing callers to use the clearer keyword form. --- lib/pstore.rb | 5 +++-- test/test_pstore.rb | 7 +++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/pstore.rb b/lib/pstore.rb index 6dd771d..1c478b0 100644 --- a/lib/pstore.rb +++ b/lib/pstore.rb @@ -367,12 +367,13 @@ class Error < StandardError # # A \PStore object is # {reentrant}[https://en.wikipedia.org/wiki/Reentrancy_(computing)]. - # If argument +thread_safe+ is given as +true+, + # If argument or keyword argument +thread_safe+ is given as +true+, # the object is also thread-safe (at the cost of a small performance penalty): # # store = PStore.new(path, true) + # store = PStore.new(path, thread_safe: true) # - def initialize(file, thread_safe = false) + def initialize(file, _thread_safe = false, thread_safe: _thread_safe) dir = File::dirname(file) unless File::directory? dir raise PStore::Error, format("directory %s does not exist", dir) diff --git a/test/test_pstore.rb b/test/test_pstore.rb index b42917d..2f210e4 100644 --- a/test/test_pstore.rb +++ b/test/test_pstore.rb @@ -143,6 +143,13 @@ def test_thread_safe File.unlink(second_file) rescue nil end + def test_thread_safe_argument + assert_equal false, PStore.new(@pstore_file).instance_variable_get(:@thread_safe) + assert_equal true, PStore.new(@pstore_file, true).instance_variable_get(:@thread_safe) + assert_equal true, PStore.new(@pstore_file, thread_safe: true).instance_variable_get(:@thread_safe) + assert_equal false, PStore.new(@pstore_file, true, thread_safe: false).instance_variable_get(:@thread_safe) + end + def test_nested_transaction_raises_error assert_raise(PStore::Error) do @pstore.transaction { @pstore.transaction { } } From 9ced2a48028a2a9ad49e0ea68ca15fcb777a3a28 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Sun, 6 Sep 2026 11:40:31 +0900 Subject: [PATCH 3/4] Accept `ultra_safe` as an initializer option Allow stores to select atomic-save behavior when they are created while preserving the existing accessor. --- lib/pstore.rb | 10 ++++++++-- test/test_pstore.rb | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/pstore.rb b/lib/pstore.rb index 1c478b0..1f1ad00 100644 --- a/lib/pstore.rb +++ b/lib/pstore.rb @@ -373,7 +373,13 @@ class Error < StandardError # store = PStore.new(path, true) # store = PStore.new(path, thread_safe: true) # - def initialize(file, _thread_safe = false, thread_safe: _thread_safe) + # If keyword argument +ultra_safe+ is given as +true+, the object is + # set to +ultra_safe+ mode. + # + # store = PStore.new(path, ultra_safe: true) + # + def initialize(file, _thread_safe = false, thread_safe: _thread_safe, + ultra_safe: false) dir = File::dirname(file) unless File::directory? dir raise PStore::Error, format("directory %s does not exist", dir) @@ -383,7 +389,7 @@ def initialize(file, _thread_safe = false, thread_safe: _thread_safe) end @filename = File.path(file) @abort = false - @ultra_safe = false + @ultra_safe = ultra_safe @thread_safe = thread_safe @lock = Thread::Mutex.new end diff --git a/test/test_pstore.rb b/test/test_pstore.rb index 2f210e4..c9135bb 100644 --- a/test/test_pstore.rb +++ b/test/test_pstore.rb @@ -150,6 +150,12 @@ def test_thread_safe_argument assert_equal false, PStore.new(@pstore_file, true, thread_safe: false).instance_variable_get(:@thread_safe) end + def test_ultra_safe_argument + assert_equal false, PStore.new(@pstore_file).ultra_safe + assert_equal true, PStore.new(@pstore_file, ultra_safe: true).ultra_safe + assert_equal false, PStore.new(@pstore_file, ultra_safe: false).ultra_safe + end + def test_nested_transaction_raises_error assert_raise(PStore::Error) do @pstore.transaction { @pstore.transaction { } } From ffe587c36b7ad732c28e915658113a710a1e2a77 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Mon, 31 Aug 2026 16:36:58 +0900 Subject: [PATCH 4/4] Allow `PStore` to reject symbolic links Let security-sensitive callers prevent store files from resolving through symbolic links. --- lib/pstore.rb | 25 +++++++++++++++++-------- test/test_pstore.rb | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/lib/pstore.rb b/lib/pstore.rb index 1f1ad00..092ab2a 100644 --- a/lib/pstore.rb +++ b/lib/pstore.rb @@ -329,10 +329,11 @@ class PStore # :stopdoc: VERSION = "0.2.1" - RDWR_ACCESS = {mode: IO::RDWR | IO::CREAT | IO::BINARY, encoding: Encoding::ASCII_8BIT}.freeze - RD_ACCESS = {mode: IO::RDONLY | IO::BINARY, encoding: Encoding::ASCII_8BIT}.freeze - WR_ACCESS = {mode: IO::WRONLY | IO::CREAT | IO::TRUNC | IO::BINARY, encoding: Encoding::ASCII_8BIT}.freeze - private_constant :RDWR_ACCESS, :RD_ACCESS, :WR_ACCESS + RDWR_ACCESS = IO::RDWR | IO::CREAT | IO::BINARY + RD_ACCESS = IO::RDONLY | IO::BINARY + WR_ACCESS = IO::WRONLY | IO::CREAT | IO::TRUNC | IO::BINARY + NOFOLLOW = IO.const_defined?(:NOFOLLOW) ? IO::NOFOLLOW : 0 + private_constant :RDWR_ACCESS, :RD_ACCESS, :WR_ACCESS, :NOFOLLOW # :startdoc: # The error type thrown by all PStore methods. @@ -378,8 +379,15 @@ class Error < StandardError # # store = PStore.new(path, ultra_safe: true) # + # If keyword argument +follow_symlink+ is given as +false+, the store file + # itself must not be a symbolic link. This option has no effect on platforms + # that do not support IO::NOFOLLOW. Symbolic links in parent directories are + # not affected. + # + # store = PStore.new(path, follow_symlink: false) + # def initialize(file, _thread_safe = false, thread_safe: _thread_safe, - ultra_safe: false) + ultra_safe: false, follow_symlink: true) dir = File::dirname(file) unless File::directory? dir raise PStore::Error, format("directory %s does not exist", dir) @@ -391,6 +399,7 @@ def initialize(file, _thread_safe = false, thread_safe: _thread_safe, @abort = false @ultra_safe = ultra_safe @thread_safe = thread_safe + @nofollow = follow_symlink ? 0 : NOFOLLOW @lock = Thread::Mutex.new end @@ -632,12 +641,12 @@ def open_and_lock_file(filename, read_only) loop do if read_only begin - file = File.new(filename, **RD_ACCESS) + file = File.new(filename, mode: RD_ACCESS | @nofollow, encoding: Encoding::ASCII_8BIT) rescue Errno::ENOENT return nil end else - file = File.new(filename, **RDWR_ACCESS) + file = File.new(filename, mode: RDWR_ACCESS | @nofollow, encoding: Encoding::ASCII_8BIT) end current = false begin @@ -705,7 +714,7 @@ def save_data(original_checksum, original_file_size, file) def save_data_with_atomic_file_rename_strategy(data, file) temp_filename = "#{@filename}.tmp.#{Process.pid}.#{rand 1000000}" - temp_file = File.new(temp_filename, **WR_ACCESS, perm: 0o000) + temp_file = File.new(temp_filename, mode: WR_ACCESS | @nofollow, encoding: Encoding::ASCII_8BIT, perm: 0o000) begin temp_file.flock(File::LOCK_EX) temp_file.write(data) diff --git a/test/test_pstore.rb b/test/test_pstore.rb index c9135bb..2294496 100644 --- a/test/test_pstore.rb +++ b/test/test_pstore.rb @@ -156,6 +156,33 @@ def test_ultra_safe_argument assert_equal false, PStore.new(@pstore_file, ultra_safe: false).ultra_safe end + def test_follows_symlink_by_default + Dir.mktmpdir do |dir| + target = File.join(dir, "target") + link = File.join(dir, "link") + PStore.new(target).transaction { |store| store[:key] = "value" } + File.symlink(target, link) + + assert_equal "value", PStore.new(link).transaction(true) { |store| store[:key] } + end + end + + def test_does_not_follow_symlink_when_disabled + omit("O_NOFOLLOW is not supported") unless nofollow_supported? + + Dir.mktmpdir do |dir| + target = File.join(dir, "target") + link = File.join(dir, "link") + File.write(target, "") + File.symlink(target, link) + + assert_raise(Errno::ELOOP) do + PStore.new(link, follow_symlink: false).transaction {} + end + assert_empty File.read(target) + end + end + def test_nested_transaction_raises_error assert_raise(PStore::Error) do @pstore.transaction { @pstore.transaction { } } @@ -216,6 +243,23 @@ def clear_store end end + def nofollow_supported? + return false unless IO.const_defined?(:NOFOLLOW) + + Dir.mktmpdir do |dir| + target = File.join(dir, "target") + link = File.join(dir, "link") + File.write(target, "") + File.symlink(target, link) + File.open(link, IO::RDONLY|IO::NOFOLLOW).close + end + false + rescue Errno::ELOOP + true + rescue NotImplementedError, SystemCallError + false + end + def second_file File.join(Dir.tmpdir, "pstore.tmp2.#{Process.pid}") end