From 2929848f514081b2fce770f1b7b2b3c0f3f7896a Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 17 Feb 2026 10:49:51 -0600 Subject: [PATCH] fix: validate NetBox URL scheme before making HTTP requests The NetBox client crashed with "scheme is required for url" when integration credentials had a nil or blank URL. normalize_url/1 now validates the scheme and returns a clear error instead of letting Finch blow up. --- lib/towerops/netbox/client.ex | 94 ++++++++++++++++------------ test/towerops/netbox/client_test.exs | 65 +++++++++++++++++++ 2 files changed, 119 insertions(+), 40 deletions(-) create mode 100644 test/towerops/netbox/client_test.exs diff --git a/lib/towerops/netbox/client.ex b/lib/towerops/netbox/client.ex index 5df8adf5..04dbeb64 100644 --- a/lib/towerops/netbox/client.ex +++ b/lib/towerops/netbox/client.ex @@ -79,57 +79,61 @@ defmodule Towerops.NetBox.Client do # -- HTTP helpers -- defp get(base_url, path, token, params \\ %{}) do - url = normalize_url(base_url) <> path + with {:ok, url} <- normalize_url(base_url) do + full_url = url <> path - case Req.get(url, - headers: auth_headers(token), - params: params, - connect_options: [timeout: 10_000], - receive_timeout: 15_000 - ) do - {:ok, %Req.Response{status: 200, body: body}} -> - {:ok, body} + case Req.get(full_url, + headers: auth_headers(token), + params: params, + connect_options: [timeout: 10_000], + receive_timeout: 15_000 + ) do + {:ok, %Req.Response{status: 200, body: body}} -> + {:ok, body} - {:ok, %Req.Response{status: 401}} -> - {:error, :unauthorized} + {:ok, %Req.Response{status: 401}} -> + {:error, :unauthorized} - {:ok, %Req.Response{status: 403}} -> - {:error, :unauthorized} + {:ok, %Req.Response{status: 403}} -> + {:error, :unauthorized} - {:ok, %Req.Response{status: 404}} -> - {:error, :not_found} + {:ok, %Req.Response{status: 404}} -> + {:error, :not_found} - {:ok, %Req.Response{status: status}} -> - {:error, "HTTP #{status}"} + {:ok, %Req.Response{status: status}} -> + {:error, "HTTP #{status}"} - {:error, %Req.TransportError{reason: reason}} -> - {:error, "#{reason}"} + {:error, %Req.TransportError{reason: reason}} -> + {:error, "#{reason}"} - {:error, reason} -> - {:error, inspect(reason)} + {:error, reason} -> + {:error, inspect(reason)} + end end end defp post(base_url, path, token, body) do - url = normalize_url(base_url) <> path + with {:ok, url} <- normalize_url(base_url) do + full_url = url <> path - case Req.post(url, - headers: auth_headers(token), - json: body, - connect_options: [timeout: 10_000], - receive_timeout: 15_000 - ) do - {:ok, %Req.Response{status: status, body: body}} when status in [200, 201] -> - {:ok, body} + case Req.post(full_url, + headers: auth_headers(token), + json: body, + connect_options: [timeout: 10_000], + receive_timeout: 15_000 + ) do + {:ok, %Req.Response{status: status, body: body}} when status in [200, 201] -> + {:ok, body} - {:ok, %Req.Response{status: 401}} -> - {:error, :unauthorized} + {:ok, %Req.Response{status: 401}} -> + {:error, :unauthorized} - {:ok, %Req.Response{status: status, body: body}} -> - {:error, "HTTP #{status}: #{inspect(body)}"} + {:ok, %Req.Response{status: status, body: body}} -> + {:error, "HTTP #{status}: #{inspect(body)}"} - {:error, reason} -> - {:error, inspect(reason)} + {:error, reason} -> + {:error, inspect(reason)} + end end end @@ -137,9 +141,19 @@ defmodule Towerops.NetBox.Client do [{"authorization", "Token #{token}"}, {"accept", "application/json"}] end - defp normalize_url(url) do - url - |> String.trim() - |> String.trim_trailing("/") + @invalid_url_error "NetBox URL is missing or invalid — a full URL with https:// is required" + + defp normalize_url(nil), do: {:error, @invalid_url_error} + + defp normalize_url(url) when is_binary(url) do + trimmed = url |> String.trim() |> String.trim_trailing("/") + + case URI.parse(trimmed) do + %URI{scheme: scheme} when scheme in ["http", "https"] -> + {:ok, trimmed} + + _ -> + {:error, @invalid_url_error} + end end end diff --git a/test/towerops/netbox/client_test.exs b/test/towerops/netbox/client_test.exs new file mode 100644 index 00000000..4240cf0f --- /dev/null +++ b/test/towerops/netbox/client_test.exs @@ -0,0 +1,65 @@ +defmodule Towerops.NetBox.ClientTest do + use ExUnit.Case, async: true + + alias Towerops.NetBox.Client + + describe "test_connection/2 with invalid URLs" do + test "returns error when base_url is nil" do + assert {:error, message} = Client.test_connection(nil, "some-token") + assert message =~ "missing or invalid" + end + + test "returns error when base_url is empty string" do + assert {:error, message} = Client.test_connection("", "some-token") + assert message =~ "missing or invalid" + end + + test "returns error when base_url is whitespace only" do + assert {:error, message} = Client.test_connection(" ", "some-token") + assert message =~ "missing or invalid" + end + + test "returns error when base_url has no scheme" do + assert {:error, message} = Client.test_connection("netbox.example.com", "some-token") + assert message =~ "missing or invalid" + end + + test "returns error when base_url has unsupported scheme" do + assert {:error, message} = Client.test_connection("ftp://netbox.example.com", "some-token") + assert message =~ "missing or invalid" + end + end + + describe "list_sites/3 with invalid URLs" do + test "returns error when base_url is nil" do + assert {:error, message} = Client.list_sites(nil, "some-token") + assert message =~ "missing or invalid" + end + + test "returns error when base_url is empty string" do + assert {:error, message} = Client.list_sites("", "some-token") + assert message =~ "missing or invalid" + end + end + + describe "list_devices/3 with invalid URLs" do + test "returns error when base_url is nil" do + assert {:error, message} = Client.list_devices(nil, "some-token") + assert message =~ "missing or invalid" + end + end + + describe "upsert_site/3 with invalid URLs" do + test "returns error when base_url is nil" do + assert {:error, message} = Client.upsert_site(nil, "some-token", %{name: "Test"}) + assert message =~ "missing or invalid" + end + end + + describe "upsert_device/3 with invalid URLs" do + test "returns error when base_url is nil" do + assert {:error, message} = Client.upsert_device(nil, "some-token", %{name: "Test"}) + assert message =~ "missing or invalid" + end + end +end