graham/towerops
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: 10 high-severity bugs from code audit (H1-H5, H14-H16, H27, H30)

Browse source 
- H1: batch count queries in MobileController (org sites/devices/alerts, site
  device counts) — reduces 1+3N+2M queries to 4 + 2
- H2: batch Oban cancel for stop_device_checks — single ANY(?) query instead
  of one per check
- H3: resolve_active_alerts_for_device uses update_all + deduped PubSub
  broadcasts (was N updates + N broadcasts)
- H4: atomic ON CONFLICT upsert for auto-discovery checks; restore partial
  unique index dropped in 20260404000002 (dedup migration included)
- H5: wrap apply_agent_to_all_equipment delete+insert in transaction so a
  failed insert_all rolls back the prior delete
- H14: replace String.to_integer/1 on untrusted SNMP OID components with
  Integer.parse/1 + validation (neighbor_discovery, discovery storage index,
  printer supply index)
- H15: ActivityFeedLive whitelists activity types against @all_types instead
  of String.to_existing_atom/1
- H16: defensive page param parsing in user_settings and admin dashboard
  LiveViews
- H27: search sanitize/1 delegates to QueryHelpers.sanitize_like/1 which
  escapes backslash first
- H30: JobCleanupTask no longer cancels jobs in "executing" state so
  in-flight polls complete naturally
This commit is contained in:
Graham McIntire 2026-05-12 10:55:01 -05:00
parent 810dafbf5e
commit 7e6ec7098c
15 changed files with 363 additions and 279 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

137
bugs.md
View file

@ -6,66 +6,6 @@
## HIGH
### H1. N+1 Queries in MobileController
**File:** `lib/towerops_web/controllers/api/mobile_controller.ex:38-46,78-84`
**Severity:** HIGH — 3N + 2M extra queries per request
**Description:** For each organization: 3 count queries (sites, devices, active alerts). For each site: 2 count queries (total devices, down devices). 1 + 3N + 2M DB round-trips.
**Fix:** Use batch count queries as already partially implemented in `devices.ex:98`.
---
### H2. N+1 Oban `cancel_all_jobs` per Check
**File:** `lib/towerops/monitoring.ex:463-473`
**Severity:** HIGH — One DB round-trip per check when stopping device checks
**Description:** Loops through check_ids calling `Oban.cancel_all_jobs` individually. A device with 50 checks = 50 queries. Should batch all check_ids into a single query using `ANY(?)`.
**Fix:** Use `fragment("args->>'check_id' = ANY(?)", ^check_ids)` in a single query.
---
### H3. N+1 Alert Resolution
**File:** `lib/towerops/alerts.ex:381-388`
**Severity:** HIGH — One DB write per alert when resolving
**Description:** Loads all alerts then `Enum.each` calls `resolve_alert_silent()` per alert. For 1000 alerts in a storm: 1000 individual `Repo.update` calls + 1000 PubSub broadcasts.
**Fix:** Use `Repo.update_all` with a single query.
---
### H4. Race Condition in `create_check` (Auto-Discovery)
**File:** `lib/towerops/monitoring.ex:152-184`
**Severity:** HIGH — Concurrent discoveries crash the job with Ecto.ConstraintError
**Description:** Two concurrent discoveries can both get `nil` from `get_by` and both attempt `insert`. The unique index prevents duplicates but one crashes with unhandled `Ecto.ConstraintError`.
**Fix:** Use `Repo.insert(..., on_conflict: :nothing)` or wrap in transaction with `FOR UPDATE`.
---
### H5. Missing Transaction in `apply_agent_to_all_equipment`
**File:** `lib/towerops/sites.ex:223-252`
**Severity:** HIGH — Partial data loss on failure
**Description:** `delete_all` runs before `insert_all`. If insert fails, assignments are lost without rollback.
**Fix:** Wrap in `Repo.transaction`.
---
### H6. N+1 Interface Capacity Queries
**File:** `lib/towerops/capacity.ex:53-83`
@ -115,18 +55,6 @@
---
### H10. Missing HSTS Header
**File:** `lib/towerops_web/plugs/security_headers.ex:22-29`
**Severity:** HIGH — SSL stripping attacks possible
**Description:** Security headers plug sets CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy — but NOT `Strict-Transport-Security`.
**Fix:** Add `strict-transport-security: max-age=63072000; includeSubDomains; preload`.
---
### H11. Hardcoded Session Salts (8 bytes)
**File:** `lib/towerops_web/endpoint.ex:11-12`
@ -165,47 +93,6 @@
---
### H14. `String.to_integer/1` on Unvalidated SNMP OID Components
**Files:**
- `lib/towerops/snmp/neighbor_discovery.ex:202,299,129`
- `lib/towerops/snmp/discovery.ex:1368`
- `lib/towerops/snmp/profiles/base.ex:1332`
**Severity:** HIGH — Malicious SNMP responses crash GenServers (DoS)
**Description:** OID suffixes extracted from SNMP responses are passed directly to `String.to_integer/1`. An attacker-controlled device can send non-numeric OID fragments, crashing the poller process.
**Fix:** Replace with `Integer.parse/1` + pattern match. Never trust OID suffixes from network data.
---
### H15. `String.to_existing_atom/1` on User Input in ActivityFeed
**File:** `lib/towerops/activity_feed_live.ex:55`
**Severity:** HIGH — Any user can crash the activity feed LiveView (DoS)
**Description:** `String.to_existing_atom(type_str)` where `type_str` comes from client event params. If the atom doesn't exist, `ArgumentError` kills the LiveView process.
**Fix:** Use a whitelist of allowed types.
---
### H16. `String.to_integer/1` on User-Controlled Page Param
**Files:**
- `lib/towerops_web/live/user_settings_live.ex:63`
- `lib/towerops_web/live/admin/dashboard_live.ex:19`
**Severity:** HIGH — Non-integer page param crashes LiveView (DoS)
**Description:** `params |> Map.get("page", "1") |> String.to_integer()` — passing `?page=abc` raises an error killing the process.
**Fix:** Use `Integer.parse` with fallback to 1.
---
### H17. Reports LiveView — Missing Org Scope on Individual Operations
**File:** `lib/towerops/reports_live.ex:73-101`
@ -331,18 +218,6 @@
---
### H27. Backslash-Wildcard Injection in Search
**File:** `lib/towerops/search.ex:90-94`
**Severity:** HIGH — Search bypass via backslash injection leaks wildcard matching
**Description:** `sanitize/1` escapes `%` and `_` but NOT `\` first. A search for `\%%` becomes `\\%\\%` — PG interprets `\\` as literal `\` followed by `%` wildcard, leaking wildcard matching.
**Fix:** Escape `\` first, or reuse `Towerops.QueryHelpers.sanitize_like/1`.
---
### H28. Nominatim API Response Injected via innerHTML
**File:** `assets/js/hooks/coverage_hooks.ts:635-645`
@ -367,18 +242,6 @@
---
### H30. JobCleanupTask Cancels Executing Jobs
**File:** `lib/towerops/workers/job_cleanup_task.ex:47-63`
**Severity:** HIGH — Rolling deployments kill currently executing poll jobs
**Description:** `cancel_all_jobs` filters on `["available", "scheduled", "executing", "retryable"]` states. In-progress polling/monitoring is discarded.
**Fix:** Exclude `"executing"` state so active jobs complete naturally.
---
## MEDIUM
### M1. Resource Scoping Reveals Org Membership

45
lib/towerops/alerts.ex
View file

@ -161,6 +161,25 @@ defmodule Towerops.Alerts do
)
end
@doc """
Batch count active (unresolved) device_down alerts for multiple organizations.
Returns a map of `%{organization_id => count}`. Organizations with zero
active alerts are omitted; callers should use `Map.get(map, org_id, 0)`.
"""
@spec batch_count_active_alerts([String.t()]) :: %{String.t() => non_neg_integer()}
def batch_count_active_alerts(organization_ids) when is_list(organization_ids) do
from(a in Alert,
where: a.organization_id in ^organization_ids,
where: a.alert_type == "device_down",
where: is_nil(a.resolved_at),
group_by: a.organization_id,
select: {a.organization_id, count(a.id)}
)
|> Repo.all()
|> Map.new()
end
@doc "Returns the count of active alerts for devices at a specific site."
def count_site_active_alerts(site_id) do
Repo.aggregate(
@ -379,12 +398,26 @@ defmodule Towerops.Alerts do
spurious "resolved" event.
"""
def resolve_active_alerts_for_device(device_id) do
from(a in Alert,
where: a.device_id == ^device_id,
where: is_nil(a.resolved_at)
)
|> Repo.all()
|> Enum.each(&resolve_alert_silent/1)
now = Towerops.Time.now()
{_count, org_ids} =
Repo.update_all(
from(a in Alert, where: a.device_id == ^device_id, where: is_nil(a.resolved_at), select: a.organization_id),
set: [resolved_at: now, updated_at: now]
)
org_ids
|> Enum.reject(&is_nil/1)
|> Enum.uniq()
|> Enum.each(fn org_id ->
Phoenix.PubSub.broadcast(
Towerops.PubSub,
"organization:#{org_id}:alerts",
{:alert_changed, org_id}
)
end)
:ok
end
# Performs the database update for resolving an alert

17
lib/towerops/devices.ex
View file

@ -80,6 +80,23 @@ defmodule Towerops.Devices do
|> Repo.aggregate(:count)
end
@doc """
Batch count devices for multiple organizations.
Returns a map of `%{organization_id => count}`. Organizations with zero
devices are omitted; callers should use `Map.get(map, org_id, 0)`.
"""
@spec batch_count_organization_devices([String.t()]) :: %{String.t() => non_neg_integer()}
def batch_count_organization_devices(organization_ids) when is_list(organization_ids) do
from(d in DeviceSchema,
where: d.organization_id in ^organization_ids,
group_by: d.organization_id,
select: {d.organization_id, count(d.id)}
)
|> Repo.all()
|> Map.new()
end
@doc """
Returns the count of devices for a site.
"""

60
lib/towerops/monitoring.ex
View file

@ -149,25 +149,9 @@ defmodule Towerops.Monitoring do
instead of creating duplicates.
"""
def create_check(attrs \\ %{}) do
# Use upsert for auto-discovery checks to avoid duplicates
result =
if Map.get(attrs, :source_type) == "auto_discovery" and Map.has_key?(attrs, :source_id) do
# For partial unique indexes, find existing record and update it
case Repo.get_by(Check,
device_id: attrs.device_id,
check_type: attrs.check_type,
source_id: attrs.source_id
) do
nil ->
%Check{}
|> Check.changeset(attrs)
|> Repo.insert()
existing_check ->
existing_check
|> Check.changeset(attrs)
|> Repo.update()
end
upsert_auto_discovery_check(attrs)
else
%Check{}
|> Check.changeset(attrs)
@ -183,6 +167,22 @@ defmodule Towerops.Monitoring do
result
end
# Atomic upsert against the partial unique index on
# (device_id, check_type, source_id) WHERE source_type = 'auto_discovery'
# AND source_id IS NOT NULL. Avoids the get-then-insert race where two
# concurrent discoveries would both see `nil` and one would crash with
# Ecto.ConstraintError.
defp upsert_auto_discovery_check(attrs) do
%Check{}
|> Check.changeset(attrs)
|> Repo.insert(
on_conflict: {:replace_all_except, [:id, :inserted_at]},
conflict_target:
{:unsafe_fragment,
"(device_id, check_type, source_id) WHERE source_type = 'auto_discovery' AND source_id IS NOT NULL"}
)
end
@doc """
Updates a check.
"""
@ -452,25 +452,27 @@ defmodule Towerops.Monitoring do
Used when deleting a device to clean up scheduled check executions.
"""
def stop_device_checks(device_id) do
# Get all check IDs for this device
check_ids =
device_id
|> CheckQuery.for_device()
|> select([c], c.id)
|> Repo.all()
# Cancel all scheduled jobs for these checks
Enum.each(check_ids, fn check_id ->
Oban.cancel_all_jobs(
from(j in Oban.Job,
where: j.queue == "check_executors",
where: j.state in ["available", "scheduled", "retryable"],
where: fragment("? @> ?", j.args, ^%{"check_id" => check_id})
)
)
end)
case check_ids do
[] ->
:ok
:ok
ids ->
Oban.cancel_all_jobs(
from(j in Oban.Job,
where: j.queue == "check_executors",
where: j.state in ["available", "scheduled", "retryable"],
where: fragment("?->>'check_id' = ANY(?)", j.args, ^ids)
)
)
:ok
end
end
@doc """

6
lib/towerops/search.ex
View file

@ -87,9 +87,5 @@ defmodule Towerops.Search do
end)
end
defp sanitize(query) do
query
|> String.replace("%", "\\%")
|> String.replace("_", "\\_")
end
defp sanitize(query), do: Towerops.QueryHelpers.sanitize_like(query)
end

68
lib/towerops/sites.ex
View file

@ -57,6 +57,23 @@ defmodule Towerops.Sites do
|> Repo.aggregate(:count)
end
@doc """
Batch count sites for multiple organizations.
Returns a map of `%{organization_id => count}`. Organizations with zero sites
are omitted from the map; callers should use `Map.get(map, org_id, 0)`.
"""
@spec batch_count_organization_sites([String.t()]) :: %{String.t() => non_neg_integer()}
def batch_count_organization_sites(organization_ids) when is_list(organization_ids) do
from(s in Site,
where: s.organization_id in ^organization_ids,
group_by: s.organization_id,
select: {s.organization_id, count(s.id)}
)
|> Repo.all()
|> Map.new()
end
@doc """
Returns the list of root sites (sites without a parent) for an organization.
Ordered by custom display_order (if set), then alphabetically by name.
@ -222,33 +239,40 @@ defmodule Towerops.Sites do
"""
def apply_agent_to_all_equipment(site_id) do
site = Repo.get!(Site, site_id)
# device IDs for this site
device_ids = Repo.all(from(e in Device, where: e.site_id == ^site_id, select: e.id))
if site.agent_token_id do
# Delete existing assignments
Repo.delete_all(from(a in AgentAssignment, where: a.device_id in ^device_ids))
# Wrap delete + insert in a transaction so an insert_all failure doesn't
# leave the site with empty assignments — either all old assignments are
# replaced atomically or the prior state is preserved.
{:ok, result} =
Repo.transaction(fn ->
Repo.delete_all(from(a in AgentAssignment, where: a.device_id in ^device_ids))
reassign_devices(device_ids, site.agent_token_id)
end)
# Create new assignments
now = Towerops.Time.now()
result
end
assignments =
Enum.map(device_ids, fn device_id ->
%{
device_id: device_id,
agent_token_id: site.agent_token_id,
inserted_at: now,
updated_at: now
}
end)
defp reassign_devices(device_ids, nil) do
# No agent set; assignments stay deleted (devices inherit from org or use cloud).
{length(device_ids), nil}
end
{count, _} = Repo.insert_all(AgentAssignment, assignments)
{count, nil}
else
# No agent set, delete all assignments (they will inherit from org or use cloud)
Repo.delete_all(from(a in AgentAssignment, where: a.device_id in ^device_ids))
end
defp reassign_devices(device_ids, agent_token_id) do
now = Towerops.Time.now()
assignments =
Enum.map(device_ids, fn device_id ->
%{
device_id: device_id,
agent_token_id: agent_token_id,
inserted_at: now,
updated_at: now
}
end)
{count, _} = Repo.insert_all(AgentAssignment, assignments)
{count, nil}
end
## Display Order Management

39
lib/towerops/snmp/discovery.ex
View file

@ -1318,12 +1318,21 @@ defmodule Towerops.Snmp.Discovery do
|> Repo.all()
|> Map.new(&{&1.storage_index, &1})
# Get the set of discovered storage_indices (convert string index to integer)
discovered_indices =
MapSet.new(discovered_storage, fn s ->
normalize_storage_index(s.storage_index)
# Drop entries with non-integer storage_index (malformed OIDs from network)
{valid_storage, invalid_count} =
Enum.reduce(discovered_storage, {[], 0}, fn entry, {acc, dropped} ->
case normalize_storage_index(entry.storage_index) do
nil -> {acc, dropped + 1}
idx -> {[Map.put(entry, :storage_index, idx) | acc], dropped}
end
end)
if invalid_count > 0 do
Logger.warning("Dropped #{invalid_count} storage entries with malformed storage_index")
end
discovered_indices = MapSet.new(valid_storage, & &1.storage_index)
# Delete storage that no longer exists on the device
existing_indices = MapSet.new(Map.keys(existing_storage))
removed_indices = MapSet.difference(existing_indices, discovered_indices)
@ -1340,11 +1349,8 @@ defmodule Towerops.Snmp.Discovery do
end
# Upsert each discovered storage
Enum.each(discovered_storage, fn storage_data ->
storage_index = normalize_storage_index(storage_data.storage_index)
storage_attrs = Map.put(storage_data, :storage_index, storage_index)
case Map.get(existing_storage, storage_index) do
Enum.each(valid_storage, fn storage_attrs ->
case Map.get(existing_storage, storage_attrs.storage_index) do
nil ->
# New storage - insert
%Storage{}
@ -1359,13 +1365,22 @@ defmodule Towerops.Snmp.Discovery do
end
end)
Logger.debug("Synced #{length(discovered_storage)} storage entries for device")
Logger.debug("Synced #{length(valid_storage)} storage entries for device")
:ok
end
# Normalize storage index to integer (Base.discover_storage may return string from OID parsing)
# Normalize storage index to integer (Base.discover_storage may return string from OID parsing).
# Returns nil for malformed values — OID suffixes come from SNMP responses and must not be trusted.
defp normalize_storage_index(index) when is_integer(index), do: index
defp normalize_storage_index(index) when is_binary(index), do: String.to_integer(index)
defp normalize_storage_index(index) when is_binary(index) do
case Integer.parse(index) do
{n, ""} -> n
_ -> nil
end
end
defp normalize_storage_index(_), do: nil
@spec sync_mempools(Device.t(), [map()]) :: :ok
@doc """

62
lib/towerops/snmp/neighbor_discovery.ex
View file

@ -109,32 +109,52 @@ defmodule Towerops.Snmp.NeighborDiscovery do
# Parse IPv4 management address (addr_subtype = 1)
defp parse_management_address(local_port, rem_index, "1", [_len | addr_bytes]) when length(addr_bytes) >= 4 do
# Take first 4 bytes for IPv4
ip =
addr_bytes
|> Enum.take(4)
|> Enum.join(".")
case parse_address_bytes(addr_bytes, 4) do
nil ->
nil
{"#{local_port}.#{rem_index}", ip}
bytes ->
{"#{local_port}.#{rem_index}", Enum.join(bytes, ".")}
end
end
# Parse IPv6 management address (addr_subtype = 2)
defp parse_management_address(local_port, rem_index, "2", [_len | addr_bytes]) when length(addr_bytes) >= 16 do
# Format IPv6 address
ip =
addr_bytes
|> Enum.take(16)
|> Enum.chunk_every(2)
|> Enum.map_join(":", fn [a, b] ->
Integer.to_string(String.to_integer(a) * 256 + String.to_integer(b), 16)
end)
case parse_address_bytes(addr_bytes, 16) do
nil ->
nil
{"#{local_port}.#{rem_index}", ip}
bytes ->
ip =
bytes
|> Enum.chunk_every(2)
|> Enum.map_join(":", fn [a, b] -> Integer.to_string(a * 256 + b, 16) end)
{"#{local_port}.#{rem_index}", ip}
end
end
# Unknown address type or malformed
defp parse_management_address(_local_port, _rem_index, _addr_subtype, _addr_parts), do: nil
# Take `count` byte values from a list of OID-component strings, returning
# a list of integers in 0..255 or nil if any component is malformed. SNMP
# responses come from the network and must never be trusted.
defp parse_address_bytes(parts, count) do
parts
|> Enum.take(count)
|> Enum.reduce_while([], fn part, acc ->
case Integer.parse(part) do
{n, ""} when n >= 0 and n <= 255 -> {:cont, [n | acc]}
_ -> {:halt, nil}
end
end)
|> case do
nil -> nil
bytes -> Enum.reverse(bytes)
end
end
defp parse_lldp_neighbors(entries, interfaces, mgmt_addresses) do
# Convert map from Client.walk to list of maps
entry_list = Enum.map(entries, fn {oid, value} -> %{oid: oid, value: value} end)
@ -199,7 +219,11 @@ defmodule Towerops.Snmp.NeighborDiscovery do
defp extract_local_port(acc, oid) do
parts = String.split(oid, ".")
[_rem_index, local_port | _] = Enum.reverse(parts)
Map.put(acc, :local_port, String.to_integer(local_port))
case Integer.parse(local_port) do
{n, ""} -> Map.put(acc, :local_port, n)
_ -> acc
end
end
defp build_neighbor_record(_protocol, _neighbor_map, _interfaces, nil), do: nil
@ -296,7 +320,11 @@ defmodule Towerops.Snmp.NeighborDiscovery do
defp extract_cdp_local_interface(acc, oid) do
parts = String.split(oid, ".")
[_device_idx, if_idx | _] = Enum.reverse(parts)
Map.put(acc, :local_if_index, String.to_integer(if_idx))
case Integer.parse(if_idx) do
{n, ""} -> Map.put(acc, :local_if_index, n)
_ -> acc
end
end
# Helper Functions

55
lib/towerops/snmp/profiles/base.ex
View file

@ -1291,19 +1291,25 @@ defmodule Towerops.Snmp.Profiles.Base do
supplies =
type_results
|> Enum.map(fn {oid, type_code} ->
index = extract_printer_supply_index(oid)
type_int = parse_integer(type_code)
case extract_printer_supply_index(oid) do
nil ->
nil
%{
supply_index: to_string(index),
supply_type: map_supply_type(type_int),
supply_description: to_string_or_nil(Map.get(description_map, index)),
supply_unit: map_supply_unit(parse_integer(Map.get(unit_map, index))),
max_capacity: parse_supply_capacity(Map.get(max_capacity_map, index)),
current_level: parse_supply_capacity(Map.get(level_map, index)),
color_name: to_string_or_nil(Map.get(color_map, index))
}
index ->
type_int = parse_integer(type_code)
%{
supply_index: to_string(index),
supply_type: map_supply_type(type_int),
supply_description: to_string_or_nil(Map.get(description_map, index)),
supply_unit: map_supply_unit(parse_integer(Map.get(unit_map, index))),
max_capacity: parse_supply_capacity(Map.get(max_capacity_map, index)),
current_level: parse_supply_capacity(Map.get(level_map, index)),
color_name: to_string_or_nil(Map.get(color_map, index))
}
end
end)
|> Enum.reject(&is_nil/1)
|> Enum.sort_by(& &1.supply_index)
Logger.debug("Discovered #{length(supplies)} printer supplies from Printer-MIB")
@ -1313,23 +1319,30 @@ defmodule Towerops.Snmp.Profiles.Base do
defp walk_printer_attribute(client_opts, oid) do
case Client.walk(client_opts, oid) do
{:ok, results} when is_map(results) ->
Map.new(results, fn {result_oid, value} ->
index = extract_printer_supply_index(result_oid)
{index, value}
end)
results
|> Enum.flat_map(&printer_attribute_entry/1)
|> Map.new()
_ ->
%{}
end
end
defp printer_attribute_entry({result_oid, value}) do
case extract_printer_supply_index(result_oid) do
nil -> []
index -> [{index, value}]
end
end
defp extract_printer_supply_index(oid) when is_binary(oid) do
# Extract the last two parts: table.entry.column.HR_INDEX.SUPPLY_INDEX
# e.g., "1.3.6.1.2.1.43.11.1.1.3.1.1" -> "1"
oid
|> String.split(".")
|> List.last()
|> String.to_integer()
# Extract the last OID component: table.entry.column.HR_INDEX.SUPPLY_INDEX
# e.g., "1.3.6.1.2.1.43.11.1.1.3.1.1" -> 1. Returns nil for malformed OIDs;
# SNMP responses come from the network and must never be trusted.
case oid |> String.split(".") |> List.last() |> Integer.parse() do
{n, ""} -> n
_ -> nil
end
end
defp map_supply_type(type_code) when is_integer(type_code) do

8
lib/towerops/workers/job_cleanup_task.ex
View file

@ -45,11 +45,15 @@ defmodule Towerops.Workers.JobCleanupTask do
end
defp cancel_all_polling_jobs do
# Exclude "executing" so in-flight polls/monitors complete naturally;
# cancelling them mid-run discards work and can leave partial state.
cancellable_states = ["available", "scheduled", "retryable"]
{:ok, poller_count} =
Oban.cancel_all_jobs(
from(j in Oban.Job,
where: j.worker == "Towerops.Workers.DevicePollerWorker",
where: j.state in ["available", "scheduled", "executing", "retryable"]
where: j.state in ^cancellable_states
)
)
@ -57,7 +61,7 @@ defmodule Towerops.Workers.JobCleanupTask do
Oban.cancel_all_jobs(
from(j in Oban.Job,
where: j.worker == "Towerops.Workers.DeviceMonitorWorker",
where: j.state in ["available", "scheduled", "executing", "retryable"]
where: j.state in ^cancellable_states
)
)

30
lib/towerops_web/controllers/api/mobile_controller.ex
View file

@ -31,17 +31,21 @@ defmodule ToweropsWeb.Api.MobileController do
"""
def list_organizations(conn, _params) do
user = conn.assigns.current_user
orgs = Organizations.list_user_organizations(user.id)
org_ids = Enum.map(orgs, & &1.id)
site_counts = Sites.batch_count_organization_sites(org_ids)
device_counts = Devices.batch_count_organization_devices(org_ids)
alert_counts = Alerts.batch_count_active_alerts(org_ids)
organizations =
user.id
|> Organizations.list_user_organizations()
|> Enum.map(fn org ->
Enum.map(orgs, fn org ->
%{
id: org.id,
name: org.name,
sites_count: Sites.count_organization_sites(org.id),
device_count: Devices.count_organization_devices(org.id),
active_alerts_count: Alerts.count_active_alerts(org.id)
sites_count: Map.get(site_counts, org.id, 0),
device_count: Map.get(device_counts, org.id, 0),
active_alerts_count: Map.get(alert_counts, org.id, 0)
}
end)
@ -71,16 +75,20 @@ defmodule ToweropsWeb.Api.MobileController do
case verify_organization_access(user, org_id) do
{:ok, _org} ->
site_list = Sites.list_organization_sites(org_id)
site_ids = Enum.map(site_list, & &1.id)
counts = Devices.batch_count_site_devices(site_ids)
sites =
org_id
|> Sites.list_organization_sites()
|> Enum.map(fn site ->
Enum.map(site_list, fn site ->
%{total: total, down: down} = Map.get(counts, site.id, %{total: 0, down: 0})
%{
id: site.id,
name: site.name,
location: site.location,
device_count: Devices.count_site_devices(site.id),
equipment_down_count: Devices.count_site_devices_down(site.id)
device_count: total,
equipment_down_count: down
}
end)

42
lib/towerops_web/live/activity_feed_live.ex
View file

@ -52,22 +52,27 @@ defmodule ToweropsWeb.ActivityFeedLive do
@impl true
def handle_event("toggle_type", %{"type" => type_str}, socket) do
type = String.to_existing_atom(type_str)
active = socket.assigns.active_types
case parse_activity_type(type_str) do
nil ->
{:noreply, socket}
new_active =
if type in active do
List.delete(active, type)
else
[type | active]
end
type ->
active = socket.assigns.active_types
{:noreply,
socket
|> assign(:active_types, new_active)
|> assign(:limit, @default_limit)
|> assign(:has_more, true)
|> load_activity()}
new_active =
if type in active do
List.delete(active, type)
else
[type | active]
end
{:noreply,
socket
|> assign(:active_types, new_active)
|> assign(:limit, @default_limit)
|> assign(:has_more, true)
|> load_activity()}
end
end
@impl true
@ -153,6 +158,15 @@ defmodule ToweropsWeb.ActivityFeedLive do
# --- Helpers ---
# Lookup against the static @all_types whitelist. Never use
# `String.to_existing_atom/1` on user-controlled input — an unknown atom
# raises ArgumentError and crashes the LiveView.
defp parse_activity_type(type_str) when is_binary(type_str) do
Enum.find(@all_types, fn t -> Atom.to_string(t) == type_str end)
end
defp parse_activity_type(_), do: nil
defp filter_options do
[
{:config_change, "Config", "hero-wrench-screwdriver"},

7
lib/towerops_web/live/admin/dashboard_live.ex
View file

@ -16,7 +16,12 @@ defmodule ToweropsWeb.Admin.DashboardLive do
@impl true
def handle_params(params, _url, socket) do
page = params |> Map.get("page", "1") |> String.to_integer()
page =
case Integer.parse(Map.get(params, "page", "1")) do
{n, _} when n >= 1 -> n
_ -> 1
end
per_page = 10
user_count = Admin.count_users()

8
lib/towerops_web/live/user_settings_live.ex
View file

@ -59,8 +59,12 @@ defmodule ToweropsWeb.UserSettingsLive do
# Default to "personal" tab if no tab param is provided
tab = Map.get(params, "tab", "personal")
# Get page from params, default to 1
page = params |> Map.get("page", "1") |> String.to_integer()
# Get page from params, default to 1 (defensive against non-integer input)
page =
case Integer.parse(Map.get(params, "page", "1")) do
{n, _} when n >= 1 -> n
_ -> 1
end
# Read modal state from URL
modal = params["modal"]

58
priv/repo/migrations/20260512105203_restore_auto_discovery_check_unique_index.exs Normal file
View file

@ -0,0 +1,58 @@
defmodule Towerops.Repo.Migrations.RestoreAutoDiscoveryCheckUniqueIndex do
use Ecto.Migration
# The partial unique index on (device_id, check_type, source_id) WHERE
# source_type = 'auto_discovery' was dropped in 20260404000002 because
# pg_stat reported zero scans. That removed DB-level uniqueness, leaving a
# race in Towerops.Monitoring.create_check/1 where two concurrent auto
# discoveries would each `get_by` returning nil and both insert a row,
# producing duplicate checks.
#
# Restore the index so we can use atomic ON CONFLICT upsert.
@disable_ddl_transaction true
@disable_migration_lock true
def up do
# Dedupe any duplicates that accumulated while the index was missing.
execute("SET statement_timeout = '10min'")
execute("""
DELETE FROM checks
WHERE id IN (
SELECT id FROM (
SELECT id,
ROW_NUMBER() OVER (
PARTITION BY device_id, check_type, source_id
ORDER BY inserted_at ASC
) AS rn
FROM checks
WHERE source_type = 'auto_discovery'
AND source_id IS NOT NULL
) ranked
WHERE rn > 1
)
""")
execute("RESET statement_timeout")
execute("DROP INDEX IF EXISTS checks_device_type_source_unique_index")
create(
unique_index(:checks, [:device_id, :check_type, :source_id],
where: "source_type = 'auto_discovery' AND source_id IS NOT NULL",
name: :checks_device_type_source_unique_index,
concurrently: true
)
)
end
def down do
drop_if_exists(
index(:checks, [:device_id, :check_type, :source_id],
name: :checks_device_type_source_unique_index,
concurrently: true
)
)
end
end