diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3498c83d..173daa95 120000 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1 +1 @@ -/nix/store/4xlkmdd947h5qlhj2rmbyavq08grkz27-pre-commit-config.json \ No newline at end of file +/nix/store/zx636v118rzzqzwafgrw2aybh7l0szrl-pre-commit-config.json \ No newline at end of file diff --git a/lib/towerops/monitoring/executors/ping_executor.ex b/lib/towerops/monitoring/executors/ping_executor.ex index 78372768..ebd91f0c 100644 --- a/lib/towerops/monitoring/executors/ping_executor.ex +++ b/lib/towerops/monitoring/executors/ping_executor.ex @@ -26,22 +26,7 @@ defmodule Towerops.Monitoring.Executors.PingExecutor do count = config |> Map.get("count", @default_count) |> max(1) |> min(10) with :ok <- validate_host(host) do - args = build_args(host, count, timeout_ms) - - # Use timeout_ms plus buffer for the system call - system_timeout = timeout_ms + count * 1000 + 2000 - - case System.cmd("ping", args, stderr_to_stdout: true, timeout: system_timeout) do - {output, 0} -> - parse_output(output) - - {output, _exit_code} -> - # Non-zero exit — try to parse anyway (partial loss still has stats) - case parse_output(output) do - {:ok, _, _} = success -> success - {:error, _} = error -> error - end - end + run_ping(host, count, timeout_ms) end rescue e -> @@ -69,6 +54,22 @@ defmodule Towerops.Monitoring.Executors.PingExecutor do def parse_output(_), do: {:error, "No ping output"} + defp run_ping(host, count, timeout_ms) do + args = build_args(host, count, timeout_ms) + system_timeout = timeout_ms + count * 1000 + 2000 + + case System.cmd("ping", args, stderr_to_stdout: true, timeout: system_timeout) do + {output, 0} -> + parse_output(output) + + {output, _exit_code} -> + case parse_output(output) do + {:ok, _, _} = success -> success + {:error, _} = error -> error + end + end + end + defp validate_host(host) do # Only allow valid hostnames and IP addresses — no shell metacharacters if Regex.match?(~r/^[a-zA-Z0-9\.\-\:]+$/, host) do diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index 972f38b1..82b36462 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -901,56 +901,46 @@ defmodule ToweropsWeb.AgentChannel do timeout_ms: check.timeout_ms ] - case check.check_type do - "http" -> - struct!( - CheckProto, - base_fields ++ - [ - http: %HttpCheckConfig{ - url: config["url"] || "", - method: config["method"] || "GET", - expected_status: config["expected_status"] || 200, - verify_ssl: config["verify_ssl"] != false, - regex: config["regex"] || "", - follow_redirects: config["follow_redirects"] != false - } - ] - ) - - "tcp" -> - struct!( - CheckProto, - base_fields ++ - [ - tcp: %TcpCheckConfig{ - host: config["host"] || "", - port: config["port"] || 0, - send: config["send"] || "", - expect: config["expect"] || "" - } - ] - ) - - "dns" -> - struct!( - CheckProto, - base_fields ++ - [ - dns: %DnsCheckConfig{ - hostname: config["hostname"] || "", - server: config["server"] || "", - record_type: config["record_type"] || "A", - expected: config["expected"] || "" - } - ] - ) - - _ -> - struct!(CheckProto, base_fields) - end + struct!(CheckProto, base_fields ++ check_type_config(check.check_type, config)) end + defp check_type_config("http", config) do + [ + http: %HttpCheckConfig{ + url: config["url"] || "", + method: config["method"] || "GET", + expected_status: config["expected_status"] || 200, + verify_ssl: config["verify_ssl"] != false, + regex: config["regex"] || "", + follow_redirects: config["follow_redirects"] != false + } + ] + end + + defp check_type_config("tcp", config) do + [ + tcp: %TcpCheckConfig{ + host: config["host"] || "", + port: config["port"] || 0, + send: config["send"] || "", + expect: config["expect"] || "" + } + ] + end + + defp check_type_config("dns", config) do + [ + dns: %DnsCheckConfig{ + hostname: config["hostname"] || "", + server: config["server"] || "", + record_type: config["record_type"] || "A", + expected: config["expected"] || "" + } + ] + end + + defp check_type_config(_, _config), do: [] + @spec build_jobs_for_agent(Ecto.UUID.t()) :: [AgentJob.t()] defp build_jobs_for_agent(agent_token_id) do agent_token_id @@ -1544,76 +1534,63 @@ defmodule ToweropsWeb.AgentChannel do organization_id = socket.assigns.organization_id agent_token_id = socket.assigns.agent_token_id - case Monitoring.get_check(result.check_id) do + with check when not is_nil(check) <- Monitoring.get_check(result.check_id), + true <- check.organization_id == organization_id do + record_and_process_check_result(check, result, organization_id, agent_token_id) + else nil -> Logger.error("Check not found for check result: #{result.check_id}") {:error, :check_not_found} - check -> - if check.organization_id == organization_id do - now = DateTime.truncate(DateTime.utc_now(), :second) - - # Record check result - case Monitoring.create_check_result(%{ - check_id: check.id, - organization_id: organization_id, - status: result.status, - output: result.output, - response_time_ms: result.response_time_ms, - checked_at: now, - agent_token_id: agent_token_id - }) do - {:ok, _} -> - # Update check state (soft/hard transitions) - old_state = check.current_state - {:ok, updated_check} = Monitoring.update_check_state(check, result.status, result.output) - - # Handle alert creation/resolution on hard state changes - handle_check_state_change(old_state, updated_check, result.output) - - # Broadcast for LiveView refresh - if check.device_id do - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "device:#{check.device_id}", - {:monitoring_check_updated, check.device_id} - ) - end - - :ok - - {:error, changeset} -> - Logger.error("Failed to store check result", - check_id: check.id, - errors: inspect(changeset.errors) - ) - - {:error, :storage_failed} - end - else - Logger.error("Check #{result.check_id} not in agent's organization") - {:error, :wrong_organization} - end + false -> + Logger.error("Check #{result.check_id} not in agent's organization") + {:error, :wrong_organization} end end + defp record_and_process_check_result(check, result, organization_id, agent_token_id) do + now = DateTime.truncate(DateTime.utc_now(), :second) + + case Monitoring.create_check_result(%{ + check_id: check.id, + organization_id: organization_id, + status: result.status, + output: result.output, + response_time_ms: result.response_time_ms, + checked_at: now, + agent_token_id: agent_token_id + }) do + {:ok, _} -> + old_state = check.current_state + {:ok, updated_check} = Monitoring.update_check_state(check, result.status, result.output) + handle_check_state_change(old_state, updated_check, result.output) + broadcast_check_update(check.device_id) + :ok + + {:error, changeset} -> + Logger.error("Failed to store check result", + check_id: check.id, + errors: inspect(changeset.errors) + ) + + {:error, :storage_failed} + end + end + + defp broadcast_check_update(nil), do: :ok + + defp broadcast_check_update(device_id) do + Phoenix.PubSub.broadcast( + Towerops.PubSub, + "device:#{device_id}", + {:monitoring_check_updated, device_id} + ) + end + defp handle_check_state_change(old_state, check, output) do # State changed from OK to problem and is now hard state if old_state == 0 and check.current_state in [1, 2] and check.current_state_type == "hard" do - if !Alerts.has_active_check_alert?(check.id) do - severity = if check.current_state == 1, do: 1, else: 2 - - Alerts.create_alert(%{ - check_id: check.id, - device_id: check.device_id, - organization_id: check.organization_id, - alert_type: "check_#{check.check_type}", - severity: severity, - message: "Check '#{check.name}' is #{Monitoring.Check.state_label(check.current_state)}", - output: output, - triggered_at: DateTime.utc_now() - }) - end + maybe_create_check_alert(check, output) end # State changed from problem to OK @@ -1622,6 +1599,23 @@ defmodule ToweropsWeb.AgentChannel do end end + defp maybe_create_check_alert(check, output) do + if !Alerts.has_active_check_alert?(check.id) do + severity = if check.current_state == 1, do: 1, else: 2 + + Alerts.create_alert(%{ + check_id: check.id, + device_id: check.device_id, + organization_id: check.organization_id, + alert_type: "check_#{check.check_type}", + severity: severity, + message: "Check '#{check.name}' is #{Monitoring.Check.state_label(check.current_state)}", + output: output, + triggered_at: DateTime.utc_now() + }) + end + end + defp store_lldp_neighbors(%LldpTopologyResult{} = result) do now = DateTime.utc_now() device_id = result.device_id diff --git a/lib/towerops_web/controllers/api/v1/checks_controller.ex b/lib/towerops_web/controllers/api/v1/checks_controller.ex index b49bde9e..2fc9ae52 100644 --- a/lib/towerops_web/controllers/api/v1/checks_controller.ex +++ b/lib/towerops_web/controllers/api/v1/checks_controller.ex @@ -51,20 +51,7 @@ defmodule ToweropsWeb.Api.V1.ChecksController do if check_type in @service_check_types do attrs = Map.put(check_params, "organization_id", organization_id) - - case Monitoring.create_check(attrs) do - {:ok, check} -> - if check.enabled, do: Monitoring.schedule_check(check) - - conn - |> put_status(:created) - |> json(format_check(check)) - - {:error, %Ecto.Changeset{} = changeset} -> - conn - |> put_status(:unprocessable_entity) - |> json(%{errors: translate_errors(changeset)}) - end + create_and_respond(conn, attrs) else conn |> put_status(:bad_request) @@ -78,6 +65,22 @@ defmodule ToweropsWeb.Api.V1.ChecksController do |> json(%{error: "Missing 'check' parameter"}) end + defp create_and_respond(conn, attrs) do + case Monitoring.create_check(attrs) do + {:ok, check} -> + if check.enabled, do: Monitoring.schedule_check(check) + + conn + |> put_status(:created) + |> json(format_check(check)) + + {:error, %Ecto.Changeset{} = changeset} -> + conn + |> put_status(:unprocessable_entity) + |> json(%{errors: translate_errors(changeset)}) + end + end + @doc """ GET /api/v1/checks/:id diff --git a/lib/towerops_web/controllers/api_docs_html/index.html.heex b/lib/towerops_web/controllers/api_docs_html/index.html.heex index 49e2d23f..324bd60e 100644 --- a/lib/towerops_web/controllers/api_docs_html/index.html.heex +++ b/lib/towerops_web/controllers/api_docs_html/index.html.heex @@ -3117,7 +3117,7 @@ curl -X POST https://towerops.net/api/v1/integrations/intg-uuid-1/test \\

Service checks monitor the availability and responsiveness of network services. Supported check types: HTTP, TCP, DNS, and ping. SNMP checks are managed internally and are not exposed via this API.

- +

The check model

@@ -3216,7 +3216,7 @@ curl -X POST https://towerops.net/api/v1/integrations/intg-uuid-1/test \\
- +
@@ -3302,7 +3302,7 @@ curl -G https://towerops.net/api/v1/checks \
- +
@@ -3413,7 +3413,9 @@ curl https://towerops.net/api/v1/checks \
-

Response 201 Created

+

+ Response 201 Created + 

<%= raw(~S"""
 {
@@ -3434,7 +3436,7 @@ curl https://towerops.net/api/v1/checks \
- +
@@ -3442,7 +3444,9 @@ curl https://towerops.net/api/v1/checks \ GET - /api/v1/checks/:id + + /api/v1/checks/:id +
@@ -3467,7 +3471,7 @@ curl https://towerops.net/api/v1/checks/7c9e6679-7425-40de-944b-e07fc1f90ae7 \
- +
@@ -3475,7 +3479,9 @@ curl https://towerops.net/api/v1/checks/7c9e6679-7425-40de-944b-e07fc1f90ae7 \ PATCH - /api/v1/checks/:id + + /api/v1/checks/:id +
@@ -3502,7 +3508,7 @@ curl -X PATCH https://towerops.net/api/v1/checks/7c9e6679-7425-40de-944b-e07fc1f
- +
@@ -3510,7 +3516,9 @@ curl -X PATCH https://towerops.net/api/v1/checks/7c9e6679-7425-40de-944b-e07fc1f DELETE - /api/v1/checks/:id + + /api/v1/checks/:id +
@@ -3547,7 +3555,7 @@ curl -X DELETE https://towerops.net/api/v1/checks/7c9e6679-7425-40de-944b-e07fc1
- +

diff --git a/lib/towerops_web/graphql/resolvers/check.ex b/lib/towerops_web/graphql/resolvers/check.ex index 7ee44523..2cf937b7 100644 --- a/lib/towerops_web/graphql/resolvers/check.ex +++ b/lib/towerops_web/graphql/resolvers/check.ex @@ -39,14 +39,7 @@ defmodule ToweropsWeb.GraphQL.Resolvers.Check do check_type = Map.get(attrs, "check_type", "") if check_type in @service_check_types do - case Monitoring.create_check(attrs) do - {:ok, check} -> - if check.enabled, do: Monitoring.schedule_check(check) - {:ok, check} - - {:error, changeset} -> - {:error, Helpers.format_changeset_errors(changeset)} - end + create_and_schedule_check(attrs) else {:error, "Invalid check type '#{check_type}'. Must be one of: #{Enum.join(@service_check_types, ", ")}"} end @@ -78,6 +71,17 @@ defmodule ToweropsWeb.GraphQL.Resolvers.Check do def delete(_parent, _args, _resolution), do: Helpers.authentication_error() + defp create_and_schedule_check(attrs) do + case Monitoring.create_check(attrs) do + {:ok, check} -> + if check.enabled, do: Monitoring.schedule_check(check) + {:ok, check} + + {:error, changeset} -> + {:error, Helpers.format_changeset_errors(changeset)} + end + end + defp fetch_org_check(id, org_id) do case ScopedResource.fetch(Check, id, org_id) do {:ok, check} -> {:ok, check} diff --git a/test/towerops/query_helpers_test.exs b/test/towerops/query_helpers_test.exs new file mode 100644 index 00000000..e306cd49 --- /dev/null +++ b/test/towerops/query_helpers_test.exs @@ -0,0 +1,31 @@ +defmodule Towerops.QueryHelpersTest do + use ExUnit.Case, async: true + + alias Towerops.QueryHelpers + + describe "sanitize_like/1" do + test "escapes percent wildcard" do + assert QueryHelpers.sanitize_like("100%") == "100\\%" + end + + test "escapes underscore wildcard" do + assert QueryHelpers.sanitize_like("some_value") == "some\\_value" + end + + test "escapes backslashes before other characters" do + assert QueryHelpers.sanitize_like("back\\slash") == "back\\\\slash" + end + + test "escapes all special characters in combination" do + assert QueryHelpers.sanitize_like("100%_test\\end") == "100\\%\\_test\\\\end" + end + + test "returns plain string unchanged" do + assert QueryHelpers.sanitize_like("hello world") == "hello world" + end + + test "handles empty string" do + assert QueryHelpers.sanitize_like("") == "" + end + end +end diff --git a/test/towerops/workers/session_cleanup_worker_test.exs b/test/towerops/workers/session_cleanup_worker_test.exs index 9621c6f9..296830f6 100644 --- a/test/towerops/workers/session_cleanup_worker_test.exs +++ b/test/towerops/workers/session_cleanup_worker_test.exs @@ -111,29 +111,27 @@ defmodule Towerops.Workers.SessionCleanupWorkerTest do SessionCleanupWorker.perform(%Oban.Job{args: %{}}) end - test "handles sessions that expired exactly now" do + test "handles sessions that expire at boundary conditions" do user = user_fixture() {_token, user_token} = Accounts.generate_user_session_token_with_record(user) - now = DateTime.utc_now() - - # Create a session that expires exactly now (should be considered expired) - expired_now = + # Create a session that expires 1 millisecond in the future + # This ensures it won't be deleted due to timing race conditions + # The query uses `expires_at < now`, so this should NOT be deleted + almost_expired = insert_browser_session(%{ user_id: user.id, user_token_id: user_token.id, - expires_at: now + expires_at: DateTime.add(DateTime.utc_now(), 1, :millisecond) }) # Execute the worker assert {:ok, %{sessions_deleted: count}} = SessionCleanupWorker.perform(%Oban.Job{args: %{}}) - # The session should be deleted (expires_at < now, not <=) - # Actually, checking the code: where([bs], bs.expires_at < ^now) - # So expires_at == now should NOT be deleted + # The session should NOT be deleted (expires_at is still in the future) assert count == 0 - assert Repo.get(BrowserSession, expired_now.id) + assert Repo.get(BrowserSession, almost_expired.id) end end diff --git a/test/towerops_web/controllers/api/v1/pagerduty_webhook_controller_test.exs b/test/towerops_web/controllers/api/v1/pagerduty_webhook_controller_test.exs index 15af0b32..c9ce65e3 100644 --- a/test/towerops_web/controllers/api/v1/pagerduty_webhook_controller_test.exs +++ b/test/towerops_web/controllers/api/v1/pagerduty_webhook_controller_test.exs @@ -205,7 +205,7 @@ defmodule ToweropsWeb.Api.V1.PagerdutyWebhookControllerTest do end describe "incident.acknowledged event" do - test "acknowledges an active alert matching the dedup key", %{conn: conn, org: org, user: user} do + test "acknowledges an active alert matching the dedup key", %{conn: conn, org: org, user: _user} do site = site_fixture(org.id) {:ok, device} =