Merge commit from fork

* RUBY-3941 Use exact match for file ID in GridFS delete methods

FSBucket#delete and Stream::Write#abort passed the caller-supplied file ID
straight into the query filter. A Hash therefore reached the server as a
query predicate rather than a value, so an ID like { "$gt" => MinKey } would
match every file in the bucket. The files collection is deleted with
delete_one, but the chunks collection uses delete_many, so a single call
could strip the chunks from every file in the bucket while leaving their
metadata behind.

Wrap the ID in $eq at all three call sites to force an exact match. This is
a no-op for ordinary ObjectId values.

Note that recent servers reject an operator-shaped _id in a delete filter,
which covers the files collection on its own. The chunks collection is
queried by files_id, which has no such server-side guard, so the driver-side
change is what prevents the bucket-wide chunk deletion.

The upstream unified tests for this are not public yet, so the coverage here
is in Ruby specs; they can be dropped once the fixtures are importable.

* RUBY-3941 Use exact match for file ID in GridFS read paths

The delete paths were fixed to wrap the caller-supplied file id in $eq,
but Stream::Read still embedded the id verbatim in both the files lookup
and the chunks query. An operator document passed as the id would be
interpreted as a query predicate, letting a caller read another file's
contents, or pull in chunks belonging to several files at once. This is
the arbitrary read vector from the same advisory as the delete fix.

Author: jamis

lib/mongo/grid/fs_bucket.rb

@@ -218,10 +218,10 @@ def delete_one(file, opts = {})
       def delete(id, opts = {})
         timeout_holder = CsotTimeoutHolder.new(operation_timeouts: operation_timeouts(opts))
         result = files_collection
-                 .find({ _id: id }, @options.merge(timeout_ms: timeout_holder.remaining_timeout_ms))
+                 .find({ _id: { '$eq' => id } }, @options.merge(timeout_ms: timeout_holder.remaining_timeout_ms))
                  .delete_one(timeout_ms: timeout_holder.remaining_timeout_ms)
         chunks_collection
-          .find({ files_id: id }, @options.merge(timeout_ms: timeout_holder.remaining_timeout_ms))
+          .find({ files_id: { '$eq' => id } }, @options.merge(timeout_ms: timeout_holder.remaining_timeout_ms))
           .delete_many(timeout_ms: timeout_holder.remaining_timeout_ms)
         raise Error::FileNotFound.new(id, :id) if result.n == 0
 

lib/mongo/grid/stream/read.rb

@@ -182,7 +182,7 @@ def file_info
             @file_info ||= begin
               doc = options[:file_info_doc] ||
                     fs.files_collection.find(
-                      { _id: file_id },
+                      { _id: { '$eq' => file_id } },
                       { timeout_ms: @timeout_holder.remaining_timeout_ms! }
                     ).first
               File::Info.new(Options::Mapper.transform(doc, File::Info::MAPPINGS.invert)) if doc
@@ -216,7 +216,7 @@ def view
                 opts[:timeout_mode] = :cursor_lifetime
               end
 
-              fs.chunks_collection.find({ files_id: file_id }, opts).sort(n: 1)
+              fs.chunks_collection.find({ files_id: { '$eq' => file_id } }, opts).sort(n: 1)
             end
           end
 

lib/mongo/grid/stream/write.rb

@@ -177,7 +177,7 @@ def closed?
           # @since 2.1.0
           def abort
             fs.chunks_collection.find(
-              { files_id: file_id },
+              { files_id: { '$eq' => file_id } },
               @options.merge(timeout_ms: @timeout_holder.remaining_timeout_ms!)
             ).delete_many
             (@open = false) || true

spec/mongo/grid/fs_bucket_spec.rb

@@ -502,6 +502,93 @@
         expect(from_db).to be_nil
       end
     end
+
+    context 'when the id is a query predicate' do
+      # The delete is issued as an exact match, so the predicate matches no
+      # file and nothing is removed. Servers that validate the shape of _id in
+      # a delete filter reject the command outright; either way, the bucket
+      # must be left intact.
+      let!(:file_ids) do
+        Array.new(3) { |i| fs.upload_from_stream("file#{i}.txt", StringIO.new('a' * 100)) }
+      end
+
+      let(:predicate) do
+        { '$gt' => BSON::MinKey.new }
+      end
+
+      it 'does not remove any files' do
+        begin
+          fs.delete(predicate)
+        rescue Mongo::Error
+          nil
+        end
+
+        expect(fs.files_collection.count_documents({})).to eq(3)
+      end
+
+      it 'does not remove any chunks' do
+        begin
+          fs.delete(predicate)
+        rescue Mongo::Error
+          nil
+        end
+
+        expect(fs.chunks_collection.count_documents({})).to eq(3)
+      end
+
+      it 'leaves the files downloadable' do
+        begin
+          fs.delete(predicate)
+        rescue Mongo::Error
+          nil
+        end
+
+        file_ids.each do |id|
+          io = StringIO.new
+          fs.download_to_stream(id, io)
+          expect(io.string.length).to eq(100)
+        end
+      end
+    end
+  end
+
+  describe 'reading with a query predicate as the file id' do
+    # The file id must be matched exactly, so a predicate matches no file and
+    # cannot be used to read another principal's file, or to read chunks
+    # belonging to several files at once.
+    let!(:file_ids) do
+      Array.new(3) { |i| fs.upload_from_stream("file#{i}.txt", StringIO.new('a' * 100)) }
+    end
+
+    let(:predicate) do
+      { '$gt' => BSON::MinKey.new }
+    end
+
+    it 'does not download another file' do
+      expect do
+        fs.download_to_stream(predicate, StringIO.new)
+      end.to raise_error(Mongo::Error::FileNotFound)
+    end
+
+    it 'does not read another file through an open stream' do
+      expect do
+        fs.open_download_stream(predicate, &:read)
+      end.to raise_error(Mongo::Error::FileNotFound)
+    end
+
+    it 'leaves the files readable by their real ids' do
+      begin
+        fs.download_to_stream(predicate, StringIO.new)
+      rescue Mongo::Error
+        nil
+      end
+
+      file_ids.each do |id|
+        io = StringIO.new
+        fs.download_to_stream(id, io)
+        expect(io.string).to eq('a' * 100)
+      end
+    end
   end
 
   context 'when a read stream is opened' do

spec/mongo/grid/stream/read_spec.rb

@@ -219,6 +219,28 @@
     end
   end
 
+  describe 'when the file id is a query predicate' do
+    # The chunks are looked up by an exact match on files_id, so a predicate
+    # cannot pull in chunks belonging to another file. file_info_doc stands in
+    # for the files document so that the chunk query is reached at all.
+    let!(:other_file_id) do
+      fs.upload_from_stream('other.txt', StringIO.new('b' * 100))
+    end
+
+    let(:options) do
+      {
+        file_id: { '$gt' => BSON::MinKey.new },
+        file_info_doc: fs.files_collection.find(_id: other_file_id).first
+      }
+    end
+
+    it 'does not read the chunks of another file' do
+      expect do
+        stream.read
+      end.to raise_error(Mongo::Error::MissingFileChunk)
+    end
+  end
+
   describe '#close' do
     let(:view) do
       stream.instance_variable_get(:@view)

spec/mongo/grid/stream/write_spec.rb

@@ -526,4 +526,35 @@
       end
     end
   end
+
+  describe '#abort' do
+    context 'when the file id is a query predicate' do
+      # The chunks are removed by an exact match on files_id, so a predicate
+      # matches nothing and the chunks belonging to other files survive.
+      let!(:other_ids) do
+        Array.new(3) { |i| fs.upload_from_stream("file#{i}.txt", StringIO.new('a' * 100)) }
+      end
+
+      let(:evil_stream) do
+        fs.open_upload_stream(filename, file_id: { '$ne' => nil })
+      end
+
+      before do
+        evil_stream.write(StringIO.new('b' * 100))
+        evil_stream.abort
+      end
+
+      it 'does not remove chunks belonging to other files' do
+        expect(fs.chunks_collection.count_documents({})).to eq(3)
+      end
+
+      it 'leaves the other files downloadable' do
+        other_ids.each do |id|
+          io = StringIO.new
+          fs.download_to_stream(id, io)
+          expect(io.string.length).to eq(100)
+        end
+      end
+    end
+  end
 end