From b709ae9fbeb2d205cac49d2c127c878f2a9db05f Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sat, 31 Jan 2026 08:25:07 -0600 Subject: [PATCH] fix prod crash with yaml parsing --- k8s/traefik-security-headers-middleware.yaml | 42 +++++ lib/towerops/profiles/yaml_profiles.ex | 32 +++- .../live/account_live/activity.ex | 158 ++++++++++-------- .../live/admin/audit_live/index.ex | 34 ++-- test/towerops/profiles/yaml_profiles_test.exs | 51 ++++++ 5 files changed, 234 insertions(+), 83 deletions(-) create mode 100644 k8s/traefik-security-headers-middleware.yaml diff --git a/k8s/traefik-security-headers-middleware.yaml b/k8s/traefik-security-headers-middleware.yaml new file mode 100644 index 00000000..17c3163a --- /dev/null +++ b/k8s/traefik-security-headers-middleware.yaml @@ -0,0 +1,42 @@ +--- +apiVersion: traefik.io/v1alpha1 +kind: Middleware +metadata: + name: security-headers + namespace: towerops +spec: + headers: + # HSTS - Force HTTPS for 1 year, include subdomains, allow preloading + # WARNING: Only enable after confirming HTTPS works correctly! + # Once set, browsers will REFUSE to connect via HTTP for 1 year + customResponseHeaders: + Strict-Transport-Security: "max-age=31536000; includeSubDomains; preload" + + # Permissions-Policy - Disable unnecessary browser features + # Adjust based on your needs (e.g., if you use geolocation, remove it from the deny list) + Permissions-Policy: "geolocation=(), microphone=(), camera=(), payment=(), usb=(), magnetometer=(), gyroscope=(), accelerometer=()" + + # X-Permitted-Cross-Domain-Policies - Prevent Adobe Flash/PDF from loading data + X-Permitted-Cross-Domain-Policies: "none" +--- +# Example: How to apply this middleware to your IngressRoute +# You would add this to your existing IngressRoute in k8s/ingressroute.yaml +# +# apiVersion: traefik.io/v1alpha1 +# kind: IngressRoute +# metadata: +# name: towerops-web +# namespace: towerops +# spec: +# entryPoints: +# - websecure +# routes: +# - kind: Rule +# match: Host(`towerops.net`) || Host(`www.towerops.net`) +# middlewares: +# - name: security-headers # <-- Add this line +# services: +# - name: towerops-web +# port: 4000 +# tls: +# certResolver: letsencrypt diff --git a/lib/towerops/profiles/yaml_profiles.ex b/lib/towerops/profiles/yaml_profiles.ex index 792778d5..cae73b2e 100644 --- a/lib/towerops/profiles/yaml_profiles.ex +++ b/lib/towerops/profiles/yaml_profiles.ex @@ -108,16 +108,34 @@ defmodule Towerops.Profiles.YamlProfiles do |> Path.wildcard() # Load profiles and collect raw detection YAMLs - raw_profiles = - yaml_files - |> Enum.map(fn file -> + results = + Enum.map(yaml_files, fn file -> name = Path.basename(file, ".yaml") - load_profile(profiles_path, name) + {name, file, load_profile(profiles_path, name)} end) - |> Enum.reject(&is_nil/1) - count = length(yaml_files) - Logger.info("Loaded #{count} device profiles from YAML files") + # Filter successful profiles and log errors + raw_profiles = + results + |> Enum.filter(fn + {name, file, {:error, error}} -> + Logger.warning("Failed to load profile #{name} from #{file}: #{inspect(error)}") + + false + + {_name, _file, nil} -> + false + + {_name, _file, _profile} -> + true + end) + |> Enum.map(fn {_name, _file, profile} -> profile end) + + success_count = length(raw_profiles) + total_count = length(yaml_files) + + Logger.info("Loaded #{success_count}/#{total_count} device profiles from YAML files") + raw_profiles else Logger.debug("No profiles directory found at #{detection_path}") diff --git a/lib/towerops_web/live/account_live/activity.ex b/lib/towerops_web/live/account_live/activity.ex index e9b76783..987535c6 100644 --- a/lib/towerops_web/live/account_live/activity.ex +++ b/lib/towerops_web/live/account_live/activity.ex @@ -149,38 +149,44 @@ defmodule ToweropsWeb.AccountLive.Activity do # Helper functions defp action_badge_class(action) do - case action do - "user_data_viewed" -> - "bg-blue-50 text-blue-700 ring-blue-600/20 dark:bg-blue-500/10 dark:text-blue-400 dark:ring-blue-500/20" + action + |> action_category() + |> badge_class_for_category() + end - "user_data_exported" -> + defp action_category(action) do + cond do + action in ["user_data_exported", "device_created"] -> :success + action in ["user_profile_updated", "device_updated"] -> :warning + action in ["device_deleted", "failed_access_attempt"] -> :danger + action in ["impersonate_start", "impersonate_end"] -> :impersonation + action == "privilege_escalation" -> :privilege + action == "user_data_viewed" -> :info + true -> :default + end + end + + defp badge_class_for_category(category) do + case category do + :success -> "bg-green-50 text-green-700 ring-green-600/20 dark:bg-green-500/10 dark:text-green-400 dark:ring-green-500/20" - "user_profile_updated" -> + :warning -> "bg-yellow-50 text-yellow-700 ring-yellow-600/20 dark:bg-yellow-500/10 dark:text-yellow-400 dark:ring-yellow-500/20" - "device_created" -> - "bg-green-50 text-green-700 ring-green-600/20 dark:bg-green-500/10 dark:text-green-400 dark:ring-green-500/20" - - "device_updated" -> - "bg-yellow-50 text-yellow-700 ring-yellow-600/20 dark:bg-yellow-500/10 dark:text-yellow-400 dark:ring-yellow-500/20" - - "device_deleted" -> + :danger -> "bg-red-50 text-red-700 ring-red-600/20 dark:bg-red-500/10 dark:text-red-400 dark:ring-red-500/20" - "failed_access_attempt" -> - "bg-red-50 text-red-700 ring-red-600/20 dark:bg-red-500/10 dark:text-red-400 dark:ring-red-500/20" + :impersonation -> + "bg-orange-50 text-orange-700 ring-orange-600/20 dark:bg-orange-500/10 dark:text-orange-400 dark:ring-orange-500/20" - "privilege_escalation" -> + :privilege -> "bg-purple-50 text-purple-700 ring-purple-600/20 dark:bg-purple-500/10 dark:text-purple-400 dark:ring-purple-500/20" - "impersonate_start" -> - "bg-orange-50 text-orange-700 ring-orange-600/20 dark:bg-orange-500/10 dark:text-orange-400 dark:ring-orange-500/20" + :info -> + "bg-blue-50 text-blue-700 ring-blue-600/20 dark:bg-blue-500/10 dark:text-blue-400 dark:ring-blue-500/20" - "impersonate_end" -> - "bg-orange-50 text-orange-700 ring-orange-600/20 dark:bg-orange-500/10 dark:text-orange-400 dark:ring-orange-500/20" - - _ -> + :default -> "bg-gray-50 text-gray-700 ring-gray-600/20 dark:bg-gray-500/10 dark:text-gray-400 dark:ring-gray-500/20" end end @@ -192,62 +198,82 @@ defmodule ToweropsWeb.AccountLive.Activity do end defp action_description(log) do - case log.action do - "user_data_viewed" -> - if log.superuser do - "Admin viewed your account data" - else - "You viewed your account data" - end + description_handlers = %{ + "user_data_viewed" => &describe_user_data_viewed/1, + "user_data_exported" => fn _log -> "You exported your account data" end, + "user_profile_updated" => fn _log -> "You updated your profile" end, + "device_created" => &describe_device_created/1, + "device_updated" => &describe_device_updated/1, + "device_deleted" => &describe_device_deleted/1, + "org_data_accessed" => &describe_org_data_accessed/1, + "failed_access_attempt" => &describe_failed_access/1, + "privilege_escalation" => &describe_privilege_escalation/1, + "impersonate_start" => &describe_impersonate_start/1, + "impersonate_end" => &describe_impersonate_end/1 + } - "user_data_exported" -> - "You exported your account data" + handler = Map.get(description_handlers, log.action) - "user_profile_updated" -> - "You updated your profile" + if handler do + handler.(log) + else + format_action(log.action) + end + end - "device_created" -> - device_name = get_in(log.metadata, ["device_name"]) || "device" - "You created device: #{device_name}" + defp describe_user_data_viewed(log) do + if log.superuser do + "Admin viewed your account data" + else + "You viewed your account data" + end + end - "device_updated" -> - device_id = get_in(log.metadata, ["device_id"]) - "You updated device (ID: #{device_id})" + defp describe_device_created(log) do + device_name = get_in(log.metadata, ["device_name"]) || "device" + "You created device: #{device_name}" + end - "device_deleted" -> - device_name = get_in(log.metadata, ["device_name"]) || "device" - "You deleted device: #{device_name}" + defp describe_device_updated(log) do + device_id = get_in(log.metadata, ["device_id"]) + "You updated device (ID: #{device_id})" + end - "org_data_accessed" -> - org_id = get_in(log.metadata, ["organization_id"]) - action_detail = get_in(log.metadata, ["action"]) || "accessed" - "You #{action_detail} organization data (ID: #{org_id})" + defp describe_device_deleted(log) do + device_name = get_in(log.metadata, ["device_name"]) || "device" + "You deleted device: #{device_name}" + end - "failed_access_attempt" -> - resource = get_in(log.metadata, ["resource"]) || "resource" - "Failed access attempt to: #{resource}" + defp describe_org_data_accessed(log) do + org_id = get_in(log.metadata, ["organization_id"]) + action_detail = get_in(log.metadata, ["action"]) || "accessed" + "You #{action_detail} organization data (ID: #{org_id})" + end - "privilege_escalation" -> - from_role = get_in(log.metadata, ["from_role"]) || "member" - to_role = get_in(log.metadata, ["to_role"]) || "admin" - "Privilege changed from #{from_role} to #{to_role}" + defp describe_failed_access(log) do + resource = get_in(log.metadata, ["resource"]) || "resource" + "Failed access attempt to: #{resource}" + end - "impersonate_start" -> - if log.superuser do - "Admin #{log.superuser.email} started impersonating you" - else - "Impersonation started" - end + defp describe_privilege_escalation(log) do + from_role = get_in(log.metadata, ["from_role"]) || "member" + to_role = get_in(log.metadata, ["to_role"]) || "admin" + "Privilege changed from #{from_role} to #{to_role}" + end - "impersonate_end" -> - if log.superuser do - "Admin #{log.superuser.email} stopped impersonating you" - else - "Impersonation ended" - end + defp describe_impersonate_start(log) do + if log.superuser do + "Admin #{log.superuser.email} started impersonating you" + else + "Impersonation started" + end + end - _ -> - format_action(log.action) + defp describe_impersonate_end(log) do + if log.superuser do + "Admin #{log.superuser.email} stopped impersonating you" + else + "Impersonation ended" end end diff --git a/lib/towerops_web/live/admin/audit_live/index.ex b/lib/towerops_web/live/admin/audit_live/index.ex index c333494f..e0933410 100644 --- a/lib/towerops_web/live/admin/audit_live/index.ex +++ b/lib/towerops_web/live/admin/audit_live/index.ex @@ -427,16 +427,30 @@ defmodule ToweropsWeb.Admin.AuditLive.Index do end defp action_badge_class(action) do - case action do - "user_data_viewed" -> "bg-blue-50 text-blue-700 ring-blue-600/20" - "user_data_exported" -> "bg-green-50 text-green-700 ring-green-600/20" - "device_created" -> "bg-green-50 text-green-700 ring-green-600/20" - "device_updated" -> "bg-yellow-50 text-yellow-700 ring-yellow-600/20" - "device_deleted" -> "bg-red-50 text-red-700 ring-red-600/20" - "failed_access_attempt" -> "bg-red-50 text-red-700 ring-red-600/20" - "impersonate_start" -> "bg-orange-50 text-orange-700 ring-orange-600/20" - "impersonate_end" -> "bg-orange-50 text-orange-700 ring-orange-600/20" - _ -> "bg-gray-50 text-gray-700 ring-gray-600/20" + action + |> audit_action_category() + |> audit_badge_class_for_category() + end + + defp audit_action_category(action) do + cond do + action in ["user_data_exported", "device_created"] -> :success + action == "device_updated" -> :warning + action in ["device_deleted", "failed_access_attempt"] -> :danger + action in ["impersonate_start", "impersonate_end"] -> :impersonation + action == "user_data_viewed" -> :info + true -> :default + end + end + + defp audit_badge_class_for_category(category) do + case category do + :success -> "bg-green-50 text-green-700 ring-green-600/20" + :warning -> "bg-yellow-50 text-yellow-700 ring-yellow-600/20" + :danger -> "bg-red-50 text-red-700 ring-red-600/20" + :impersonation -> "bg-orange-50 text-orange-700 ring-orange-600/20" + :info -> "bg-blue-50 text-blue-700 ring-blue-600/20" + :default -> "bg-gray-50 text-gray-700 ring-gray-600/20" end end diff --git a/test/towerops/profiles/yaml_profiles_test.exs b/test/towerops/profiles/yaml_profiles_test.exs index 0543abf8..21657289 100644 --- a/test/towerops/profiles/yaml_profiles_test.exs +++ b/test/towerops/profiles/yaml_profiles_test.exs @@ -244,6 +244,57 @@ defmodule Towerops.Profiles.YamlProfilesTest do end end + describe "error handling" do + test "application survives malformed YAML files in profiles directory" do + # This test ensures that malformed YAML files don't crash the GenServer during startup. + # The bug: load_profile/2 returns {:error, ...} tuples which end up in the profiles list, + # then extract_mib_names_from_profiles/1 tries to do profile["discovery"] on error tuples, + # causing FunctionClauseError in Access.get/3. + + # Create a temporary malformed YAML file in the profiles directory + profiles_path = Path.join(:code.priv_dir(:towerops), "profiles/os_detection") + malformed_file = Path.join(profiles_path, "test_malformed.yaml") + + # Write truly malformed YAML (unclosed bracket) + File.write!(malformed_file, "sysObjectID: [\n - 1.2.3.4.5\ntext: bad") + + try do + # Verify that YamlProfiles GenServer is still running and functional + # despite the malformed YAML file existing + assert Process.whereis(YamlProfiles) + + # Verify we can still match profiles (the GenServer didn't crash) + system_info = %{ + sys_object_id: "1.3.6.1.4.1.10002.1", + sys_descr: "Linux" + } + + client_opts = [ + ip: "192.168.1.1", + community: "public", + version: "2c", + port: 161 + ] + + stub(SnmpMock, :get, fn _target, _oid, _opts -> + {:error, :no_such_object} + end) + + stub(SnmpMock, :walk, fn _target, _oid, _opts -> + {:error, :no_such_object} + end) + + # Should be able to match profiles without crashing + _profile = YamlProfiles.match_profile(system_info, client_opts) + + # The test passes if we get here without crashing + after + # Cleanup: remove the malformed file + File.rm(malformed_file) + end + end + end + describe "MIB name caching" do test "creates :mib_cache ETS table on startup" do # Verify table exists (created by MibCache GenServer)