From c0cade51a1f252db89fd9e8dfbed7dddf2dfd018 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Tue, 21 Jul 2026 11:53:37 +0100 Subject: [PATCH] Address Copilot review: require ipaddr and re-assert the socket port The guard uses IPAddr but relied on something else loading it first; require it explicitly. And the rebinding tests lost their port assertion when the matchers were loosened for newer Net::HTTP keyword args, so check the port alongside the IP again. --- lib/restricted_http/private_network_guard.rb | 1 + test/models/opengraph/fetch_test.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/restricted_http/private_network_guard.rb b/lib/restricted_http/private_network_guard.rb index 5dd8f98..11901ba 100644 --- a/lib/restricted_http/private_network_guard.rb +++ b/lib/restricted_http/private_network_guard.rb @@ -1,3 +1,4 @@ +require "ipaddr" require "resolv" module RestrictedHTTP diff --git a/test/models/opengraph/fetch_test.rb b/test/models/opengraph/fetch_test.rb index e0bf2b0..c110a44 100644 --- a/test/models/opengraph/fetch_test.rb +++ b/test/models/opengraph/fetch_test.rb @@ -50,7 +50,7 @@ class Opengraph::FetchTest < ActiveSupport::TestCase WebMock.disable_net_connect! allow: [ @url.host ] Resolv.stubs(:getaddress).with(@url.host).returns("1.2.3.4", "127.0.0.1") TCPSocket.expects(:open).with { |*args, **| args.first == @url.host }.never - TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" }.throws(:dns_not_rebound) + TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" && args[1] == 443 }.throws(:dns_not_rebound) assert_throws :dns_not_rebound do @fetch.fetch_document(@url) @@ -67,7 +67,7 @@ class Opengraph::FetchTest < ActiveSupport::TestCase WebMock.disable_net_connect! allow: [ @url.host ] Resolv.stubs(:getaddress).with(@url.host).returns("1.2.3.4", "127.0.0.1") TCPSocket.expects(:open).with { |*args, **| args.first == @url.host }.never - TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" }.throws(:dns_not_rebound) + TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" && args[1] == 443 }.throws(:dns_not_rebound) assert_throws :dns_not_rebound do @fetch.fetch_document(URI.parse("https://www.other.com/"), ip: "1.2.3.4")