Verification improvements next batch (#4203)

* Upgrade req

* Fetch body even on non-200 statuses

Turns out sites, will happily go with 307 and no location
header for example. In which case saying we can't reach it
is an overstatement. After all we should let the headless
check determine the rest.

* Update callback status interpretation cases

* Update tests

* Fix flaky test

* Update expectation

lib/plausible/verification/checks/fetch_body.ex

@@ -27,8 +27,8 @@ defmodule Plausible.Verification.Checks.FetchBody do
     {req, resp} = opts |> Req.new() |> Req.Request.run_request()
 
     case resp do
-      %Req.Response{status: status, body: body}
+      %Req.Response{body: body}
-      when is_binary(body) and status in 200..299 ->
+      when is_binary(body) ->
         state
         |> assign(final_domain: req.url.host)
         |> extract_document(resp)

lib/plausible/verification/diagnostics.ex

@@ -135,8 +135,6 @@ defmodule Plausible.Verification.Diagnostics do
 
   def interpret(
         %__MODULE__{
-          snippets_found_in_body: 0,
-          snippets_found_in_head: 1,
           plausible_installed?: true,
           wordpress_likely?: false,
           callback_status: -1
@@ -148,8 +146,6 @@ defmodule Plausible.Verification.Diagnostics do
 
   def interpret(
         %__MODULE__{
-          snippets_found_in_body: 0,
-          snippets_found_in_head: 1,
           plausible_installed?: true,
           wordpress_likely?: true,
           wordpress_plugin?: false,
@@ -162,8 +158,6 @@ defmodule Plausible.Verification.Diagnostics do
 
   def interpret(
         %__MODULE__{
-          snippets_found_in_body: 0,
-          snippets_found_in_head: 1,
           plausible_installed?: true,
           wordpress_likely?: true,
           wordpress_plugin?: true,
@@ -336,10 +330,12 @@ defmodule Plausible.Verification.Diagnostics do
     %Result{ok?: true}
   end
 
-  def interpret(rating, url) do
+  def interpret(diagnostics, url) do
-    Sentry.capture_message("Unhandled case for site verification: #{url}",
+    Sentry.capture_message("Unhandled case for site verification",
       extra: %{
-        message: inspect(rating)
+        message: inspect(diagnostics),
+        url: url,
+        hash: :erlang.phash2(diagnostics)
       }
     )
 

mix.exs

@@ -143,7 +143,7 @@ defmodule Plausible.MixProject do
       {:sweet_xml, "~> 0.7.4"},
       {:zstream, "~> 0.6.4"},
       {:con_cache, "~> 1.1.0"},
-      {:req, "~> 0.4.14"}
+      {:req, "~> 0.5.0"}
     ]
   end
 

mix.lock

@@ -88,7 +88,7 @@
   "mua": {:hex, :mua, "0.2.0", "ca7e7a00848719c95c6f72094b41e7b9af1ad85048b948d696d6a8f9e31dcdd3", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}], "hexpm", "b8dc60683ff03ca85359ecd83db7f7da66cf2eada1289e0bb531b0724702c09a"},
   "nanoid": {:hex, :nanoid, "2.1.0", "d192a5bf1d774258bc49762b480fca0e3128178fa6d35a464af2a738526607fd", [:mix], [], "hexpm", "ebc7a342d02d213534a7f93a091d569b9fea7f26fcd3a638dc655060fc1f76ac"},
   "nimble_csv": {:hex, :nimble_csv, "1.2.0", "4e26385d260c61eba9d4412c71cea34421f296d5353f914afe3f2e71cce97722", [:mix], [], "hexpm", "d0628117fcc2148178b034044c55359b26966c6eaa8e2ce15777be3bbc91b12a"},
-  "nimble_options": {:hex, :nimble_options, "1.1.0", "3b31a57ede9cb1502071fade751ab0c7b8dbe75a9a4c2b5bbb0943a690b63172", [:mix], [], "hexpm", "8bbbb3941af3ca9acc7835f5655ea062111c9c27bcac53e004460dfd19008a99"},
+  "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},
   "nimble_ownership": {:hex, :nimble_ownership, "0.3.1", "99d5244672fafdfac89bfad3d3ab8f0d367603ce1dc4855f86a1c75008bce56f", [:mix], [], "hexpm", "4bf510adedff0449a1d6e200e43e57a814794c8b5b6439071274d248d272a549"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
   "nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"},
@@ -118,7 +118,7 @@
   "phoenix_template": {:hex, :phoenix_template, "1.0.4", "e2092c132f3b5e5b2d49c96695342eb36d0ed514c5b252a77048d5969330d639", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}], "hexpm", "2c0c81f0e5c6753faf5cca2f229c9709919aba34fab866d3bc05060c9c444206"},
   "phoenix_view": {:hex, :phoenix_view, "2.0.3", "4d32c4817fce933693741deeb99ef1392619f942633dde834a5163124813aad3", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}], "hexpm", "cd34049af41be2c627df99cd4eaa71fc52a328c0c3d8e7d4aa28f880c30e7f64"},
   "php_serializer": {:hex, :php_serializer, "2.0.0", "b43f31aca22ed7321f32da2b94fe2ddf9b6739a965cb51541969119e572e821d", [:mix], [], "hexpm", "61e402e99d9062c0225a3f4fcf7e43b4cba1b8654944c0e7c139c3ca9de481da"},
-  "plug": {:hex, :plug, "1.15.3", "712976f504418f6dff0a3e554c40d705a9bcf89a7ccef92fc6a5ef8f16a30a97", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "cc4365a3c010a56af402e0809208873d113e9c38c401cabd88027ef4f5c01fd2"},
+  "plug": {:hex, :plug, "1.16.0", "1d07d50cb9bb05097fdf187b31cf087c7297aafc3fed8299aac79c128a707e47", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "cbf53aa1f5c4d758a7559c0bd6d59e286c2be0c6a1fac8cc3eee2f638243b93e"},
   "plug_cowboy": {:hex, :plug_cowboy, "2.7.0", "3ae9369c60641084363b08fe90267cbdd316df57e3557ea522114b30b63256ea", [:mix], [{:cowboy, "~> 2.7.0 or ~> 2.8.0 or ~> 2.9.0 or ~> 2.10.0", [hex: :cowboy, repo: "hexpm", optional: false]}, {:cowboy_telemetry, "~> 0.3", [hex: :cowboy_telemetry, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "d85444fb8aa1f2fc62eabe83bbe387d81510d773886774ebdcb429b3da3c1a4a"},
   "plug_crypto": {:hex, :plug_crypto, "1.2.5", "918772575e48e81e455818229bf719d4ab4181fcbf7f85b68a35620f78d89ced", [:mix], [], "hexpm", "26549a1d6345e2172eb1c233866756ae44a9609bd33ee6f99147ab3fd87fd842"},
   "postgrex": {:hex, :postgrex, "0.17.5", "0483d054938a8dc069b21bdd636bf56c487404c241ce6c319c1f43588246b281", [:mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "50b8b11afbb2c4095a3ba675b4f055c416d0f3d7de6633a595fc131a828a67eb"},
@@ -128,7 +128,7 @@
   "recon": {:hex, :recon, "2.5.4", "05dd52a119ee4059fa9daa1ab7ce81bc7a8161a2f12e9d42e9d551ffd2ba901c", [:mix, :rebar3], [], "hexpm", "e9ab01ac7fc8572e41eb59385efeb3fb0ff5bf02103816535bacaedf327d0263"},
   "ref_inspector": {:hex, :ref_inspector, "2.0.0", "f3e97e51d9782de4c792f56eed26c80903bc39174c878285392ce76d5e67fe98", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}, {:yamerl, "~> 0.7", [hex: :yamerl, repo: "hexpm", optional: false]}], "hexpm", "bf62f3f1a87d6b8b30f457a480668f965373e64f184611282b5e89d8dd81fd33"},
   "referrer_blocklist": {:git, "https://github.com/plausible/referrer-blocklist.git", "d6f52c225cccb4f04b80e3a5d588868ec234139d", []},
-  "req": {:hex, :req, "0.4.14", "103de133a076a31044e5458e0f850d5681eef23dfabf3ea34af63212e3b902e2", [:mix], [{:aws_signature, "~> 0.3.2", [hex: :aws_signature, repo: "hexpm", optional: true]}, {:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.17", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 1.6 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:nimble_ownership, "~> 0.2.0 or ~> 0.3.0", [hex: :nimble_ownership, repo: "hexpm", optional: false]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "2ddd3d33f9ab714ced8d3c15fd03db40c14dbf129003c4a3eb80fac2cc0b1b08"},
+  "req": {:hex, :req, "0.5.0", "6d8a77c25cfc03e06a439fb12ffb51beade53e3fe0e2c5e362899a18b50298b3", [:mix], [{:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.17", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 1.6 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "dda04878c1396eebbfdec6db6f3d4ca609e5c8846b7ee88cc56eb9891406f7a3"},
   "rustler_precompiled": {:hex, :rustler_precompiled, "0.6.2", "d2218ba08a43fa331957f30481d00b666664d7e3861431b02bd3f4f30eec8e5b", [:mix], [{:castore, "~> 0.1 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: false]}, {:rustler, "~> 0.23", [hex: :rustler, repo: "hexpm", optional: true]}], "hexpm", "b9048eaed8d7d14a53f758c91865cc616608a438d2595f621f6a4b32a5511709"},
   "scrivener": {:hex, :scrivener, "2.7.2", "1d913c965ec352650a7f864ad7fd8d80462f76a32f33d57d1e48bc5e9d40aba2", [:mix], [], "hexpm", "7866a0ec4d40274efbee1db8bead13a995ea4926ecd8203345af8f90d2b620d9"},
   "scrivener_ecto": {:hex, :scrivener_ecto, "2.7.0", "cf64b8cb8a96cd131cdbcecf64e7fd395e21aaa1cb0236c42a7c2e34b0dca580", [:mix], [{:ecto, "~> 3.3", [hex: :ecto, repo: "hexpm", optional: false]}, {:scrivener, "~> 2.4", [hex: :scrivener, repo: "hexpm", optional: false]}], "hexpm", "e809f171687806b0031129034352f5ae44849720c48dd839200adeaf0ac3e260"},

test/plausible/shield/ip_rule_cache_test.exs

@@ -33,7 +33,7 @@ defmodule Plausible.Shield.IPRuleCacheTest do
       refute IPRuleCache.get({site.domain, "3.3.3.3"}, cache_name: test, force?: true)
     end
 
-    test "cache allows lookups for sites with changed domain", %{test: test} do
+    test "cache allows IP lookups for sites with changed domain", %{test: test} do
       {:ok, _} = start_test_cache(test)
       site = insert(:site, domain: "new.example.com", domain_changed_from: "old.example.com")
 

test/plausible/site/verification/checks/fetch_body_test.exs

@@ -32,13 +32,10 @@ defmodule Plausible.Verification.Checks.FetchBodyTest do
     assert state.diagnostics.body_fetched?
   end
 
-  test "doesn't extract on non-2xx", %{state: state} do
+  test "does extract on non-2xx", %{state: state} do
     stub(400)
     state = @check.perform(state)
-
+    assert state.diagnostics.body_fetched?
-    assert map_size(state.assigns) == 0
-
-    refute state.diagnostics.body_fetched?
   end
 
   test "doesn't extract non-HTML", %{state: state} do

test/plausible/site/verification/checks_test.exs

@@ -2,6 +2,7 @@ defmodule Plausible.Verification.ChecksTest do
   use Plausible.DataCase, async: true
 
   alias Plausible.Verification.Checks
+  alias Plausible.Verification.Diagnostics
   alias Plausible.Verification.State
 
   import ExUnit.CaptureLog
@@ -167,7 +168,7 @@ defmodule Plausible.Verification.ChecksTest do
         with_log(fn ->
           run_checks()
           |> Checks.interpret_diagnostics()
-          |> assert_error(@errors.unreachable, url: "https://example.com")
+          |> assert_ok()
         end)
 
       assert log =~ "3 attempts left"
@@ -200,15 +201,6 @@ defmodule Plausible.Verification.ChecksTest do
       refute_receive _
     end
 
-    test "fetching body fails at non-2xx status, but installation is ok" do
-      stub_fetch_body(599, "boo")
-      stub_installation()
-
-      run_checks()
-      |> Checks.interpret_diagnostics()
-      |> assert_error(@errors.unreachable, url: "https://example.com")
-    end
-
     @snippet_in_body """
     <html>
     <head>
@@ -758,6 +750,37 @@ defmodule Plausible.Verification.ChecksTest do
     end
   end
 
+  describe "unhhandled cases from sentry" do
+    test "APP-58: 4b1435e3f8a048eb949cc78fa578d1e4" do
+      %Plausible.Verification.Diagnostics{
+        plausible_installed?: true,
+        snippets_found_in_head: 0,
+        snippets_found_in_body: 0,
+        snippet_found_after_busting_cache?: false,
+        snippet_unknown_attributes?: false,
+        disallowed_via_csp?: false,
+        service_error: nil,
+        body_fetched?: true,
+        wordpress_likely?: true,
+        cookie_banner_likely?: false,
+        gtm_likely?: false,
+        callback_status: -1,
+        proxy_likely?: false,
+        manual_script_extension?: false,
+        data_domain_mismatch?: false,
+        wordpress_plugin?: false
+      }
+      |> interpret_sentry_case()
+      |> assert_error(@errors.old_script_wp_no_plugin)
+    end
+  end
+
+  defp interpret_sentry_case(diagnostics) do
+    diagnostics
+    |> Diagnostics.interpret("example.com")
+    |> refute_unhandled()
+  end
+
   defp run_checks(extra_opts \\ []) do
     Checks.run(
       "https://example.com",
@@ -794,6 +817,18 @@ defmodule Plausible.Verification.ChecksTest do
     %{"data" => %{"plausibleInstalled" => bool, "callbackStatus" => callback_status}}
   end
 
+  defp refute_unhandled(interpretation) do
+    refute interpretation.errors == [
+             @errors.unknown.message
+           ]
+
+    refute interpretation.recommendations == [
+             @errors.unknown.recommendation
+           ]
+
+    interpretation
+  end
+
   defp assert_error(interpretation, error) do
     refute interpretation.ok?