From a7e9ea53dd5ba541f95bee1474b12fc9ac14b5d1 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 9 Oct 2026 00:30:56 +0200 Subject: [PATCH] Don't let an http_proxy environment variable bypass the pinned address in link previews Opengraph::Fetch connects to the IP it checked, not to a fresh lookup of the host. With http_proxy set, Net::HTTP would instead send the hostname to the proxy, which looks it up again, so the connection could end up somewhere other than the checked address. Pass nil as the proxy address, as web push delivery already does. Co-Authored-By: Claude Opus 5.5 --- app/models/opengraph/fetch.rb | 5 ++++- test/models/opengraph/fetch_test.rb | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/models/opengraph/fetch.rb b/app/models/opengraph/fetch.rb index bd628127..2c3a40ff 100644 --- a/app/models/opengraph/fetch.rb +++ b/app/models/opengraph/fetch.rb @@ -23,11 +23,14 @@ class Opengraph::Fetch end private + # The nil proxy address ignores http_proxy: a proxy would look the host up again, and the connection + # would no longer go to the checked IP. + # # The timeouts bound each operation. A host can still send its headers or body a byte at a time, # in as many reads as it likes: UnfurlLinksController puts the whole unfurl under one deadline. def request(url, request_class, ip:) MAX_REDIRECTS.times do - Net::HTTP.start(url.host, url.port, ipaddr: ip, use_ssl: url.scheme == "https", **timeouts) do |http| + Net::HTTP.start(url.host, url.port, nil, ipaddr: ip, use_ssl: url.scheme == "https", **timeouts) do |http| http.request request_class.new(url) do |response| if response.is_a?(Net::HTTPRedirection) url, ip = resolve_redirect(response["location"]) diff --git a/test/models/opengraph/fetch_test.rb b/test/models/opengraph/fetch_test.rb index dc510cc5..561d2cf0 100644 --- a/test/models/opengraph/fetch_test.rb +++ b/test/models/opengraph/fetch_test.rb @@ -74,6 +74,23 @@ class Opengraph::FetchTest < ActiveSupport::TestCase end end + test "#fetch_document connects to the resolved IP even when a proxy is configured" do + url = URI.parse("http://www.example.com/") + saved = ENV.slice("http_proxy", "HTTP_PROXY") + %w[ http_proxy HTTP_PROXY ].each { |k| ENV[k] = "http://proxy.internal:3128" } + + WebMock.disable_net_connect! allow: [ url.host ] + TCPSocket.expects(:open).with { |*args, **| args.first == "proxy.internal" }.never + TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" && args[1] == 80 }.throws(:not_proxied) + + assert_throws :not_proxied do + @fetch.fetch_document(url, ip: "1.2.3.4") + end + ensure + %w[ http_proxy HTTP_PROXY ].each { |k| ENV.delete(k) } + saved.each { |k, v| ENV[k] = v } + end + test "#fetch_document is empty following redirects that never finish" do WebMock.stub_request(:get, "https://www.example.com/") .to_return(status: 302, headers: { location: "https://www.example.com/" })