Skip to content

Commit 3624e4e

Browse files
koicandreibondarev
andauthored
Replace insecure URI.open with URI.parse#open (#982)
`URI.open` is not safe because it can execute commands like those shown below: ```console $ ruby -ropen-uri -e 'p URI.open(%q{| echo "hi"}).read' "hi\n" ``` Replacing it with `URI.parse#open`, as already used in `Langchain::Loader`, makes it more secure: https://github.com/patterns-ai-core/langchainrb/blob/0.19.5/lib/langchain/loader.rb#L95 ```console $ ruby -ropen-uri -e 'p URI.parse(%q{| echo "hi"}).open.read' /Users/koic/.rbenv/versions/3.4.3/lib/ruby/gems/3.4.0/gems/uri-1.0.3/lib/uri/rfc3986_parser.rb:130: in 'URI::RFC3986_Parser#split': bad URI (is not URI?): "| echo \"hi\"" (URI::InvalidURIError) from /Users/koic/.rbenv/versions/3.4.3/lib/ruby/gems/3.4.0/gems/uri-1.0.3/lib/uri/rfc3986_parser.rb:135: in 'URI::RFC3986_Parser#parse' from /Users/koic/.rbenv/versions/3.4.3/lib/ruby/gems/3.4.0/gems/uri-1.0.3/lib/uri/common.rb:212:in 'URI.parse' from -e:1:in '<main>' ``` It likely makes sense also in terms of reusing the parsed `uri` object. Co-authored-by: Andrei Bondarev <[email protected]>
1 parent dd29cdc commit 3624e4e

File tree

2 files changed

+6
-2
lines changed

2 files changed

+6
-2
lines changed

lib/langchain/utils/image_wrapper.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ def open_image
3030
# TODO: Make it work with local files
3131
uri = URI.parse(image_url)
3232
raise URI::InvalidURIError, "Invalid URL scheme" unless %w[http https].include?(uri.scheme)
33-
@open_image ||= URI.open(image_url) # rubocop:disable Security/Open
33+
34+
@open_image ||= uri.open
3435
end
3536
end
3637
end

spec/langchain/utils/image_wrapper_spec.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22

33
RSpec.describe Langchain::Utils::ImageWrapper do
44
let(:image_url) { "https://example.com/sf-cable-car.jpeg" }
5+
let(:uri_https) { instance_double(URI::HTTPS) }
56

67
before do
7-
allow(URI).to receive(:open).with(image_url).and_return(File.open("./spec/fixtures/loaders/sf-cable-car.jpeg"))
8+
allow(URI).to receive(:parse).with(image_url).and_return(uri_https)
9+
allow(uri_https).to receive(:scheme).and_return("https")
10+
allow(uri_https).to receive(:open).and_return(File.open("./spec/fixtures/loaders/sf-cable-car.jpeg"))
811
end
912

1013
subject { described_class.new(image_url) }

0 commit comments

Comments
 (0)