`
+ const button = popupEl.appendChild(document.createElement('button'))
+ button.setAttribute('data-site-id', site.id)
+ button.setAttribute('data-action', 'view-site')
+ button.className = 'text-xs bg-blue-600 text-white px-2 py-1 rounded hover:bg-blue-700'
+ button.textContent = 'View Site →'
- marker.bindPopup(popupContent)
+ marker.bindPopup(popupEl)
marker.on('popupopen', () => {
const button = marker.getPopup()?.getElement()?.querySelector('[data-action="view-site"]') as HTMLElement
if (button) {
diff --git a/bugs.md b/bugs.md
index 79979a65..d86543fd 100644
--- a/bugs.md
+++ b/bugs.md
@@ -1,36 +1,1084 @@
-# Bugs And Risk Findings
+# Code Review Findings
-Review date: 2026-05-12 (all issues fixed 2026-05-12)
+> Full codebase audit conducted 2026-05-12. Covers OWASP Top 10, performance, reliability, and correctness issues across the entire TowerOps Web application.
-Scope: application-owned Elixir/Phoenix code, templates, config, deployment manifests, scripts, and dependency manifests. Vendored code and bundled third-party MIB data were not reviewed as first-party code.
+---
-Validation run:
+## CRITICAL
-- `mix test`: passed, 12,668 tests, 0 failures, 59 skipped, 235 excluded.
+### C1. ✓ FIXED — Stored XSS via Sites Map Popup (innerHTML)
-## Fixed
+**Files:**
+- `assets/js/hooks/sites_map.ts:75-101` — popup construction with template literals
+- `map_live/index.html.heex:51` — `data-sites={Jason.encode!(@sites)}`
-### 1. Users can mint API tokens for organizations they do not belong to (HIGH)
+**Severity:** CRITICAL — Stored XSS affecting all users viewing the map
-- Fix: Added `validate_membership/2` in `ApiTokens.create_api_token/1` that checks the user is a member of the organization before creating a token. Added test for unauthorized token creation rejection.
-- Files: `lib/towerops/api_tokens.ex`, `test/towerops/api_tokens_test.exs`, `test/towerops_web/live/user_settings_live/api_token_manager_test.exs`
+**Description:** Site name, description, and address from the database are interpolated into template literals and injected via `marker.bindPopup()` (which uses `innerHTML`). HEEx escapes the JSON attribute in the template, but `JSON.parse` in the hook restores the original unescaped strings, then template literals inject them directly into HTML without sanitization. Any user who can create/edit a site can execute arbitrary JS for every map viewer.
-### 2. Admin security allowlist form crashes because it reads `current_user` (MEDIUM)
+```typescript
+// vulnerable code
+let popupContent = `
+
${site.name}
`
+```
-- Fix: Changed `socket.assigns.current_user` to `socket.assigns.current_scope.user` in the `add_whitelist` event handler.
-- File: `lib/towerops_web/live/admin/security_live/index.ex`
+**Fix:** Use `textContent`-based popup rendering or sanitize via DOMPurify. Create a DOM element and set `textContent` for user data:
-### 3. Hidden archive entries bypass MIB upload validation (MEDIUM)
+```typescript
+const popupEl = document.createElement('div')
+popupEl.className = 'p-2'
+const h3 = popupEl.appendChild(document.createElement('h3'))
+h3.className = 'font-semibold text-lg mb-1'
+h3.textContent = site.name
+marker.bindPopup(popupEl)
+```
-- Fix: Replaced `Path.wildcard` (which excludes dotfiles) with recursive `File.ls/1` via `list_all_entries/1`. Updated `check_no_symlinks/1` to use `File.lstat/1` instead of `File.read_link/1`.
-- File: `lib/towerops_web/controllers/api/v1/mib_controller.ex`
+---
-### 4. ZIP MIB uploads are extracted before checking upload size (MEDIUM)
+### C2. ✓ FIXED — IDOR in Policy Consent Handler — Client-Controlled User ID
-- Fix: Added `check_upload_size/1` call before extraction in `extract_zip/3`, matching the tarball path.
-- File: `lib/towerops_web/controllers/api/v1/mib_controller.ex`
+**File:** `lib/towerops_web/user_auth.ex:693`
-### 5. CSP is duplicated and weakened by inline scripts (MEDIUM)
+**Severity:** CRITICAL — Any authenticated user can grant/revoke consent for any other user
-- Fix: Removed CSP from router's `put_secure_browser_headers` (centralized in `SecurityHeaders` plug). Replaced `'unsafe-inline'` with per-request nonces on all inline scripts. Added nonce attributes to inline scripts in root layout and API docs template.
-- Files: `lib/towerops_web/router.ex`, `lib/towerops_web/plugs/security_headers.ex`, `lib/towerops_web/components/layouts/root.html.heex`, `lib/towerops_web/controllers/api_docs_html/index.html.heex`, `test/towerops_web/plugs/security_headers_test.exs`
+**Description:** The `accept_updated_policies` on_mount handler reads `user-id` directly from LiveView event params with zero validation:
+
+```elixir
+"accept_updated_policies", %{"user-id" => user_id}, socket ->
+ ...
+ Accounts.grant_consent(user_id, policy_type)
+```
+
+Attached to ALL authenticated `live_session` scopes. An attacker can send arbitrary `user_id` values.
+
+**Fix:** Derive user ID from `socket.assigns.current_scope.user.id` instead of client params.
+
+---
+
+### C3. ✓ FIXED — Broken `halt()` in GDPR Data Export
+
+**File:** `lib/towerops_web/controllers/api/account_data_controller.ex:20-26`
+
+**Severity:** CRITICAL — Unconfirmed users can download full GDPR data export
+
+**Description:** The halting check for unconfirmed users discards the result:
+
+```elixir
+_ = if !user.confirmed_at do
+ conn |> put_status(:forbidden) |> json(...) |> halt()
+end
+```
+
+`halt()` returns a modified conn, but it's bound to `_`. The original `conn` is used, so the function proceeds to gather ALL user data.
+
+**Fix:** Restructure to early-return pattern, not orphan `halt()`.
+
+---
+
+### C4. ✓ FIXED — Missing Authorization on Member Role Changes
+
+**File:** `lib/towerops_web/controllers/api/v1/members_controller.ex:20-51`
+
+**Severity:** CRITICAL — Any API token holder can change roles or remove any member
+
+**Description:** `update` and `delete` actions call into `Organizations` context functions that do NOT check whether the requesting user has permission. The context functions look up membership by organization_id + user_id and perform the action — no role/authority check for the caller.
+
+**Fix:** Add authorization at the controller level — verify `current_user` is owner/admin before allowing role changes or removals.
+
+---
+
+### C5. ✓ FIXED — Agent Release Webhook — Optional Signature Means No Auth
+
+**File:** `lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex:37-42`
+
+**Severity:** CRITICAL — Anyone who discovers the endpoint can trigger mass agent updates
+
+**Description:** If signature header is absent, the webhook accepts the request unconditionally:
+
+```elixir
+defp verify_optional_signature(conn) do
+ sig_headers = Plug.Conn.get_req_header(conn, "x-agent-webhook-signature")
+ case sig_headers do
+ [] -> :ok # passes without any auth
+ ...
+```
+
+**Fix:** Remove the "optional" behavior. Require HMAC signature on every request.
+
+---
+
+### C6. ✓ FIXED — Repo.all_by/2 Does Not Exist — Runtime Crash
+
+**File:** `lib/towerops/accounts.ex:386-387`
+
+**Severity:** CRITICAL — Runtime FunctionClauseError when updating passwords/changing email
+
+**Description:** `Repo.all_by/2` is NOT a standard Ecto function. It will raise `FunctionClauseError` at runtime when `update_user_and_delete_all_tokens/1` is called (password change, email change). The DB transaction crashes with a generic error.
+
+```elixir
+tokens_to_expire = Repo.all_by(UserToken, user_id: user.id)
+```
+
+**Fix:** Replace with `Repo.all(from t in UserToken, where: t.user_id == ^user.id)`.
+
+---
+
+### C7. ✓ FIXED — Brute Force Protection Runs Before Authentication
+
+**File:** `lib/towerops_web/endpoint.ex:68` → plug runs here
+**File:** `lib/towerops_web/plugs/brute_force_protection.ex:47` — dead auth exemption check
+
+**Severity:** CRITICAL — Authenticated user exemption path is unreachable; every authenticated 404 gets tracked
+
+**Description:** `BruteForceProtection` runs in the endpoint pipeline LONG BEFORE auth happens. The check `conn.assigns[:current_user] != nil` will ALWAYS be `nil`. The authenticated-user exemption path is completely dead code.
+
+**Fix:** Move auth exemption check to `register_before_send` callback, or restructure pipeline ordering.
+
+---
+
+### C8. ✓ FIXED — Status Page `custom_css` Injection
+
+**File:** `lib/towerops_web/live/status_page_live.html.heex:26-30`
+
+**Severity:** CRITICAL — Any org admin with status page access can inject arbitrary HTML/JS
+
+**Description:** `custom_css` is inserted raw into a `` injection:
+
+```heex
+<%= if @config.custom_css do %>
+
+<% end %>
+```
+
+**Fix:** Sanitize to remove ``, ``, `>'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`
+
+**Severity:** HIGH — 20+ individual queries per site capacity view
+
+**Description:** `get_site_capacity_summary` loops over interfaces calling `get_utilization` which does `InterfaceStat |> where(interface_id: ^id) |> Repo.all()` per interface.
+
+**Fix:** Batch-load all interface stats in a single query.
+
+---
+
+### H7. Unauthenticated Webhook Endpoints
+
+**File:** `lib/towerops/router.ex:232-239`
+
+**Severity:** HIGH — Webhook endpoints protected only by per-org secret, no router-level auth
+
+**Description:** Gaiia, PagerDuty, and MikroTik webhooks use the `:api` pipeline which only has `plug :accepts, ["json"]` — no authentication. Auth is delegated entirely to each controller.
+
+**Fix:** Add webhook auth plug at pipeline level.
+
+---
+
+### H8. Missing Session Expiry Check in Mobile/GraphQL Auth
+
+**File:** `lib/towerops_web/plugs/mobile_auth.ex:48-53`
+**File:** `lib/towerops_web/plugs/graphql_auth.ex:58-63`
+
+**Severity:** HIGH — Expired/revoked sessions remain usable indefinitely
+
+**Description:** `validate_session` checks the record exists but does NOT check `expires_at`. If the query doesn't filter expired rows, revoked sessions stay active.
+
+**Fix:** Add `DateTime.compare(expires_at, DateTime.utc_now())` check.
+
+---
+
+### H9. CSP `img-src https:` Wildcard
+
+**File:** `lib/towerops_web/plugs/security_headers.ex:38`
+
+**Severity:** HIGH — Broad data exfiltration vector via any XSS
+
+**Description:** `https:` for img-src allows images from any HTTPS origin. Combined with any XSS, an attacker can exfiltrate data via image request URLs.
+
+**Fix:** Restrict to specific domains.
+
+---
+
+### 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`
+**File:** `config/config.exs:135`
+
+**Severity:** HIGH — Weak entropy for cookie signing; identical across all environments
+
+**Description:** `signing_salt: "hrDZxLhd"` and `encryption_salt: "vK3p8mNx"` are hardcoded 8-byte salts. Phoenix recommends 16+ bytes. Also baked into source code — can't rotate without deploy.
+
+**Fix:** Load from env vars, increase to 16+ bytes.
+
+---
+
+### H12. Missing `Secure` and `HttpOnly` on Session Cookie
+
+**File:** `lib/towerops_web/endpoint.ex:8-14`
+**File:** `lib/towerops/user_auth.ex:29-33` (remember-me cookie)
+
+**Severity:** HIGH — Session cookie sent over HTTP; accessible via JS
+
+**Description:** `@session_options` has `same_site: "Lax"` but no `secure: true` or `http_only: true`. Session cookie transmitted in cleartext over HTTP and accessible via `document.cookie`.
+
+**Fix:** Add `secure: true, http_only: true` to both session and remember-me cookie configs.
+
+---
+
+### H13. PromEx Metrics Server Unauthenticated
+
+**File:** `config/config.exs:106-117`
+
+**Severity:** HIGH — Internal metrics accessible without any auth
+
+**Description:** `auth_strategy: :none` on metrics server. Exposes DB query performance, memory info, business logic execution times.
+
+**Fix:** Add at minimum basic token auth, or document network isolation explicitly.
+
+---
+
+### 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`
+
+**Severity:** HIGH — IDOR on report toggle, delete, run_now
+
+**Description:** `toggle`, `delete`, `run_now` fetch reports by ID without verifying org membership. A user who guesses another org's report ID can manipulate it.
+
+**Fix:** Add org scope verification to all individual operations.
+
+---
+
+### H18. Mass Assignment Risk Across All CRUD Endpoints
+
+**Files:** Multiple v1 controllers
+
+**Severity:** HIGH — Unchecked params passed to context functions
+
+**Description:** All create/update actions pass raw user params to context functions. Only protection is changeset `cast/3`. If any changeset doesn't restrict fields properly, arbitrary columns can be set.
+
+**Fix:** Add explicit field allowlist filtering at controller level (like `IntegrationsController.to_atom_keys/1`).
+
+---
+
+### H19. QR Token Verify Leaks User Email
+
+**File:** `lib/towerops_web/controllers/api/mobile_auth_controller.ex:43-45`
+
+**Severity:** HIGH — Token validity + user email disclosed to anyone with a QR token
+
+**Description:** `json(conn, %{valid: true, user_email: qr_token.user.email})` — reveals the associated email on token verify.
+
+**Fix:** Only return `valid: true/false`. Don't disclose email until token is consumed.
+
+---
+
+### H20. CoverageWorker Contradictory Retry Settings
+
+**File:** `lib/towerops/workers/coverage_worker.ex:23-25, 485-486`
+
+**Severity:** HIGH — `max_attempts: 3` but `fail/2` returns `:ok`
+
+**Description:** Configured for retries, but the `fail` path returns `:ok`, preventing Oban from ever retrying. Misleading configuration.
+
+**Fix:** Remove `max_attempts` (use default 1) or return `{:error, reason}` for transient failures.
+
+---
+
+### H21. Unbounded Sequential Processing in Sync Workers
+
+**Files:**
+- `gaiia_sync_worker.ex:20`
+- `sonar_sync_worker.ex:16`
+- `splynx_sync_worker.ex:16`
+- `visp_sync_worker.ex:16`
+- `netbox_sync_worker.ex:20`
+
+**Severity:** HIGH — Single job processes ALL integrations sequentially; 10+ minute blocks
+
+**Description:** `Enum.map(integrations, &sync_integration/1)` — sequentially syncs every enabled integration in a single Oban job. Blocks the queue slot for extended periods.
+
+**Fix:** Follow `UispSyncWorker` pattern — staggered per-integration jobs with unique constraints.
+
+---
+
+### H22. DataRetentionWorker Unbounded DELETEs
+
+**File:** `lib/towerops/workers/data_retention_worker.ex:61-173`
+
+**Severity:** HIGH — Massive DELETE statements with no batching
+
+**Description:** Each table purged with a single DELETE. On millions of rows: WAL amplification, autovacuum storms, long row locks, blocked concurrent reads.
+
+**Fix:** Use batched deletes (`LIMIT 10000` in a loop) or partition-based truncation.
+
+---
+
+### H23. Sync Workers Silently Swallow Errors
+
+**Files:** All sync workers (uisp, preseem, cn_maestro, sonar, splynx, visp, netbox, gaiia)
+
+**Severity:** HIGH — Transient failures NEVER retried; data silently stale
+
+**Description:** All sync workers log errors but return `:ok`. Network timeouts, API rate limits, DNS failures are all silently accepted. Data goes stale until next cron cycle.
+
+**Fix:** Distinguish permanent (return `:ok`/`:discard`) from transient (return `{:error, reason}` for Oban retry).
+
+---
+
+### H24. WeatherSyncWorker Never Starts
+
+**File:** `lib/towerops/workers/weather_sync_worker.ex`
+
+**Severity:** HIGH — Weather data never fetched (dead code path)
+
+**Description:** Worker self-schedules at end of each run via `schedule_next/0`, but no initial trigger exists in any cron config. First run never occurs.
+
+**Fix:** Add to Oban cron schedule or trigger at application startup.
+
+---
+
+### H25. No Device/Site Organization Validation on Check Creation
+
+**File:** `lib/towerops_web/controllers/api/v1/checks_controller.ex:47-60`
+
+**Severity:** HIGH — Check can reference devices from other organizations
+
+**Description:** Only `check_type` is validated. The `device_id` is not verified to belong to the authenticated org.
+
+**Fix:** Add device access verification using `ScopedResource.fetch/3`.
+
+---
+
+### H26. N+1 Latency Queries per Site Device
+
+**File:** `lib/towerops_web/live/site_live/show.ex:101-128`
+
+**Severity:** HIGH — 50+ queries for large sites
+
+**Description:** Loops through devices calling `Monitoring.get_latency_data` per device. For 50+ devices at a site, 50+ individual queries.
+
+**Fix:** Batch the latency query or add pagination/limit.
+
+---
+
+### 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`
+
+**Severity:** HIGH — Third-party API response flows into innerHTML
+
+**Description:** Nominatim search API `display_name` is passed directly to `marker.bindPopup()` (innerHTML). If Nominatim is compromised or DNS poisoned, arbitrary JS executes.
+
+**Fix:** Sanitize `display_name` with `escapeHtml()` before `bindPopup()`.
+
+---
+
+### H29. OID Injection via Substring Match in Profiles
+
+**File:** `lib/towerops/profiles/yaml_profiles.ex:788`
+
+**Severity:** HIGH — Incorrect profile matching due to partial OID match
+
+**Description:** `String.contains?(sys_object_id, block.oid)` matches partial octets. OID `1.3.6.1.4.1.9` would incorrectly match `1.3.6.1.4.1.99` because "9" is a substring of "99".
+
+**Fix:** Use `String.starts_with?(sys_object_id <> ".", block.oid <> ".")` for dot-boundary matching.
+
+---
+
+### 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
+
+**File:** `lib/towerops_web/scoped_resource.ex:15-20`
+
+**Severity:** MEDIUM — Cross-org resource existence probing
+
+**Description:** Returns `:forbidden` for wrong-org resources vs `:not_found` for nonexistent. Organization A can probe if a specific resource ID belongs to Organization B.
+
+**Fix:** Return `:not_found` in both cases — use `Repo.get_by(schema, id: id, organization_id: organization_id)`.
+
+---
+
+### M2. Email Lookups Are Case-Sensitive
+
+**File:** `lib/towerops/accounts.ex:39-41`
+
+**Severity:** MEDIUM — Lookalike account registration possible
+
+**Description:** `Repo.get_by(User, email: email)` — `User@Example.com` and `user@example.com` are separate accounts.
+
+**Fix:** Add `citext` column or use `fragment("LOWER(email) = LOWER(?)", ^email)`.
+
+---
+
+### M3. Captcha/Verification Token Pattern Matches
+
+**File:** `lib/towerops_web/user_auth.ex:70-86` (valid_return_path?)
+
+**Severity:** MEDIUM — Open redirect via URL encoding bypasses
+
+**Description:** Blacklist-based `valid_return_path?` can be bypassed with URL encoding: `/%2f%2fevil.com`, tab injection, double encoding.
+
+**Fix:** Use whitelist approach matching known route prefixes.
+
+---
+
+### M4. Stripe Webhook Signature Not Enforced at Plug Level
+
+**File:** `lib/towerops_web/router.ex:144-148`
+
+**Severity:** MEDIUM — Any future controller could forget to verify
+
+**Description:** Stripe webhook verification is deferred entirely to the controller with no router-level enforcement.
+
+**Fix:** Create a dedicated Stripe webhook auth plug.
+
+---
+
+### M5. Rate Limiting Keyed Only by IP
+
+**File:** `lib/towerops_web/plugs/rate_limit.ex:57`
+
+**Severity:** MEDIUM — NAT'd users share budget; IP-rotation bypasses limits
+
+**Description:** Rate limit key is `"#{type}:#{remote_ip}"` with no user/token component. Users behind NAT share a single budget. Attackers with large IP pools bypass limits entirely.
+
+**Fix:** Include `current_user.id` or token ID in the key: `"#{type}:#{user_id}:#{remote_ip}"`.
+
+---
+
+### M6. `get_node_detail` Loads Entire Organization
+
+**File:** `lib/towerops/topology.ex:612-663`
+
+**Severity:** MEDIUM — Massive unnecessary data transfer for single node lookup
+
+**Description:** Loads ALL device IDs and ALL unlinked DeviceLink records for the entire organization just to find one node.
+
+**Fix:** Find the link directly with a join query based on the discovered node ID.
+
+---
+
+### M7. No Unique Constraint on `(organization_id, email)` for Invitations
+
+**File:** Migration `20251221192513_create_organization_invitations.exs`
+
+**Severity:** MEDIUM — Duplicate pending invitations possible; XSS-like confusion
+
+**Description:** Individual indexes on `token`, `organization_id`, `email` but no composite unique for pending invitations.
+
+**Fix:** Add `create unique_index(:organization_invitations, [:organization_id, :email], where: "accepted_at IS NULL")`.
+
+---
+
+### M8. Webhook Secret Misconfiguration Disclosed
+
+**File:** `lib/towerops_web/plugs/webhook_auth.ex:36-43`
+
+**Severity:** MEDIUM — Attacker can determine webhook endpoint has zero auth
+
+**Description:** Returns `"Webhook authentication not configured"` in the HTTP response body.
+
+**Fix:** Return generic `%{error: "Internal server error"}`.
+
+---
+
+### M9. Information Disclosure via KMZ Error
+
+**File:** `lib/towerops_web/controllers/api/v1/coverages_controller.ex:203-204`
+
+**Severity:** MEDIUM — Internal paths and system errors leaked to client
+
+**Description:** `inspect(reason)` is sent in error response. Could include system paths from `File.read`, `:zip.create`, or file system errors.
+
+**Fix:** Log error server-side, return generic message.
+
+---
+
+### M10. `[membership]` Pattern Match Crash
+
+**File:** `lib/towerops_web/controllers/api/account_data_controller.ex:75`
+
+**Severity:** MEDIUM — 500 error for edge-case membership counts
+
+**Description:** `[membership] = org.memberships` crashes with MatchError if 0 or 2+ memberships.
+
+**Fix:** Use `case org.memberships do [membership] -> ... end`.
+
+---
+
+### M11. ActivityController `String.to_existing_atom` on User Input
+
+**File:** `lib/towerops_web/controllers/api/v1/activity_controller.ex:37-41`
+
+**Severity:** MEDIUM — Unpredictable atom matching from user-supplied strings
+
+**Description:** User-supplied filter types are converted to atoms via `to_existing_atom`. If a string happens to match an internal atom, query behavior could be altered.
+
+**Fix:** Whitelist allowed atom values.
+
+---
+
+### M12. No Superuser Verification in Admin LiveViews
+
+**Files:** All `admin/*.ex` LiveViews
+
+**Severity:** MEDIUM — No defense-in-depth if route misconfiguration exposes admin views
+
+**Description:** Router applies `:require_superuser` but admin LiveViews don't perform their own `current_scope.superuser` check.
+
+**Fix:** Add `on_mount` or `mount` superuser verification to admin LiveViews.
+
+---
+
+### M13. CSV/Download Content-Type Validation Missing
+
+**File:** `assets/js/app.ts:374-393`
+
+**Severity:** MEDIUM — Downloaded HTML file could execute scripts
+
+**Description:** The `phx:download` handler accepts any `mime_type` from server. If server ever allows user content to influence mime_type, an HTML download with scripts could be created.
+
+**Fix:** Add client-side `mime_type` allowlist.
+
+---
+
+### M14. WeatherSyncWorker Blocks Queue with `Process.sleep`
+
+**File:** `lib/towerops/workers/weather_sync_worker.ex:46-53`
+
+**Severity:** MEDIUM — Worker holds queue slot for 110s+ for 100 sites
+
+**Description:** `Process.sleep(1_100)` between each site fetch blocks the Oban queue slot.
+
+**Fix:** Use `Task.async_stream` with `max_concurrency: 1` and `timeout`, or dispatch per-site jobs.
+
+---
+
+### M15. CloudLatencyProbeWorker Unbounded PubSub Message
+
+**File:** `lib/towerops/workers/cloud_latency_probe_worker.ex:39-43`
+
+**Severity:** MEDIUM — Megabyte-sized PubSub messages with many devices
+
+**Description:** Full device list broadcast as single PubSub message. With thousands of devices, this can overwhelm PubSub.
+
+**Fix:** Chunk the device list or send IDs that agents resolve independently.
+
+---
+
+### M16. MutationObserver Not Stored for Cleanup
+
+**File:** `assets/js/app.ts:284-310`
+
+**Severity:** MEDIUM — Memory leak on every page navigation mounting this hook
+
+**Description:** `const observer = new MutationObserver(...)` — stored locally, not as `this.observer`. The `destroyed()` method checks `this.observer` which is never set.
+
+**Fix:** Change `const observer` to `this.observer = ...`.
+
+---
+
+### M17. SNMPv3 Fallback to MD5 Auth Protocol
+
+**Files:** Multiple executor files
+
+**Severity:** MEDIUM — Cryptographically broken auth protocol
+
+**Description:** Default SNMPv3 auth protocol is `"MD5"`. MD5 is cryptographically broken for authentication.
+
+**Fix:** Change default to `"SHA-256"` for new devices.
+
+---
+
+### M18. No String Length Validation for SNMP Credentials in Protobuf
+
+**File:** `lib/towerops/proto/decode.ex:1509-1593`
+
+**Severity:** MEDIUM — Memory exhaustion via oversized credential strings
+
+**Description:** `decode_snmp_device` does NOT call `validate_string` on `community`, `v3_auth_password`, `v3_priv_password`. An attacker could send multi-megabyte credential strings.
+
+**Fix:** Add `validate_string` with reasonable max lengths (255 bytes).
+
+---
+
+### M19. Missing Validation on Multiple Protobuf Decode Paths
+
+**File:** `lib/towerops/proto/decode.ex` (multiple decode functions)
+
+**Severity:** MEDIUM — No validation on `decode_mikrotik_device`, `decode_mikrotik_command`, `decode_check`, `decode_snmp_query`, `decode_sensor`, `decode_interface`
+
+**Description:** Several decode functions have zero validation (no `validate_string` calls, no field-length checks).
+
+**Fix:** Add validation consistent with existing validated decode paths.
+
+---
+
+### M20. MIB Upload — Hard Links Not Checked in Archive Extraction
+
+**File:** `lib/towerops_web/controllers/api/v1/mib_controller.ex:453-458`
+
+**Severity:** MEDIUM — Hard-linked files in archive could overwrite system files
+
+**Description:** `check_no_symlinks` only checks symlinks. Hard links, device nodes, and fifos are not inspected.
+
+**Fix:** Add `check_no_hardlinks(paths)` and `check_no_special_files(paths)` using `File.stat`.
+
+---
+
+### M21. ETS Rate Limit Table Is Public
+
+**File:** `lib/towerops/rate_limit.ex:156-164`
+
+**Severity:** MEDIUM — Any Erlang/Elixir process can manipulate rate limit counters
+
+**Description:** `:ets.new(table, [:named_table, :set, :public, ...])` — `:public` allows any process to read/write counters.
+
+**Fix:** Use `:protected` and have GenServer expose `hit/3` via `call`.
+
+---
+
+### M22. Impersonation Session Has No Expiry
+
+**File:** `lib/towerops_web/user_auth.ex:879-910`
+
+**Severity:** MEDIUM — Superuser impersonation persists indefinitely
+
+**Description:** If a superuser starts impersonating and walks away, the session continues impersonating until manually stopped.
+
+**Fix:** Tie impersonation to sudo mode or add dedicated `expires_at`.
+
+---
+
+### M23. Permission Checks Silent Failure on Unpreloaded Associations
+
+**File:** `lib/towerops_web/permissions.ex:140-146`
+
+**Severity:** MEDIUM — Hard-to-debug silent denial of access
+
+**Description:** When `org.memberships` is not preloaded, `Ecto.assoc_loaded?` returns false, the `if` block evaluates to `nil`, and `do_can?` returns `false` silently.
+
+**Fix:** Log a warning when permissions are checked without preloaded data.
+
+---
+
+## LOW
+
+### L1. Cloudflare API Token Potentially Leaked in Logs
+
+**File:** `lib/towerops/security/cloudflare_client.ex:47,74-76`
+
+**Severity:** LOW — Debug logging could expose bearer token
+
+**Description:** Cloudflare API token is passed as Bearer token. If HTTP client logs headers in debug mode, token leaks to logs.
+
+**Fix:** Ensure HTTP client strips authorization headers from logs.
+
+---
+
+### L2. 404 Tracker EXPIRE Without NX Flag
+
+**File:** `lib/towerops/security/four_oh_four_tracker.ex:32`
+
+**Severity:** LOW — Attacker can extend tracking window indefinitely
+
+**Description:** `EXPIRE` called on every 404 regardless of whether key already exists. NO `NX` option means each hit refreshes the TTL.
+
+**Fix:** Use `EXPIRE key 60 NX` to only set expiry on first creation.
+
+---
+
+### L3. IPv4-Mapped IPv6 Not Handled in Whitelist
+
+**File:** `lib/towerops/security/brute_force.ex:36-49`
+
+**Severity:** LOW — `::ffff:192.168.1.1` doesn't match whitelist entry `192.168.1.1`
+
+**Description:** Exact string matching means IPv4-mapped IPv6 addresses bypass whitelist matching.
+
+**Fix:** Normalize IPs via `:inet.parse_address/1` before comparing.
+
+---
+
+### L4. No Auth on PromEx Metrics Port (Documentation Gap)
+
+**File:** `config/config.exs:106-117`
+
+**Severity:** LOW — Document that network isolation is the only protection
+
+**Description:** `auth_strategy: :none` is acceptable if the port is firewalled, but this should be explicitly documented.
+
+**Fix:** Add comment documenting expected network security.
+
+---
+
+### L5. Vault Initialization Reads Prod Key in Dev
+
+**File:** `lib/towerops/vault.ex:29`
+
+**Severity:** LOW — If `CLOAK_KEY` env var is set, dev uses production encryption key
+
+**Description:** `if System.get_env("CLOAK_KEY") do` — no environment guard. A developer with prod env set will use the production key in dev.
+
+**Fix:** Only read `CLOAK_KEY` in production, or use a separate `CLOAK_KEY_DEV`.
+
+---
+
+### L6. Health Endpoint Leaks App Version
+
+**File:** `lib/towerops_web/controllers/health_controller.ex:29`
+
+**Severity:** LOW — Helps attackers identify vulnerable versions
+
+**Description:** `Application.spec(:towerops, :vsn)` returned publicly.
+
+**Fix:** Remove version from public health endpoint.
+
+---
+
+### L7. Device Capacity Operations Missing Org Scoping
+
+**File:** `lib/towerops_web/live/device_live/show.ex:728-757`
+
+**Severity:** LOW — `interface_id` from params not verified to belong to user's org
+
+**Description:** `set_capacity` handler uses the `interface_id` from client params with no org membership verification.
+
+**Fix:** Verify interface belongs to a device in the user's org.
+
+---
+
+### L8. Insights Dismiss Missing Org Scope
+
+**File:** `lib/towerops_web/live/insights_live/index.ex:71-82`
+
+**Severity:** LOW — IDOR on dismiss, but low impact (read-only dismissal)
+
+**Description:** `Preseem.dismiss_insight(id)` with no org scope check.
+
+**Fix:** Verify org ownership before dismissing.
+
+---
+
+### L9. Duplicate Pattern in Sync Workers
+
+**Files:** `gaiia_sync_worker.ex`, `sonar_sync_worker.ex`, `splynx_sync_worker.ex`, `visp_sync_worker.ex`, `netbox_sync_worker.ex`
+
+**Severity:** LOW — Code duplication invites inconsistency
+
+**Description:** Five workers have identical `should_sync?/1` logic duplicated verbatim.
+
+**Fix:** Extract to `Integrations` context module.
+
+---
+
+### L10. SidebarCollapse Event Listener Not Cleaned Up
+
+**File:** `assets/js/app.ts:316-324`
+
+**Severity:** LOW — Anonymous arrow function can't be removed; listener accumulates
+
+**Description:** `mounted()` adds event listener but no `destroyed()` method to remove it.
+
+**Fix:** Add `destroyed()` with stored handler reference.
+
+---
+
+### L11. WebMCP Navigate Path Not Validated
+
+**File:** `assets/js/app.ts:478-480`
+
+**Severity:** LOW — `window.location.href = path` could accept `javascript:` URLs
+
+**Description:** If a compromised WebMCP agent sends a malicious path, user could be navigated to phishing URL.
+
+**Fix:** Validate path starts with `/`.
+
+---
+
+### L12. `window.liveSocket` Exposed Globally
+
+**File:** `assets/js/app.ts:423`
+
+**Severity:** LOW — CSRF token accessible via `liveSocket.socket.params._csrf_token`
+
+**Description:** Exposing LiveSocket globally makes CSRF token trivially accessible to any XSS.
+
+**Fix:** Guard behind `NODE_ENV !== "production"`.
+
+---
+
+### L13. Overly Broad Changeset in Alert Updates
+
+**File:** `lib/towerops/alerts.ex:391-405`
+
+**Severity:** LOW — `Alert.changeset/2` casts 17 fields for a partial update
+
+**Description:** Using the full changeset for simple field updates (`resolved_at`) is fragile. Extra keys could overwrite system fields.
+
+**Fix:** Use a dedicated changeset or `Ecto.Changeset.change/2`.
+
+---
+
+### L14. `list_checks` No Pagination
+
+**File:** `lib/towerops/monitoring.ex:24-32`
+
+**Severity:** LOW — No limit on check listing for orgs with thousands of checks
+
+**Fix:** Add default limit and pagination support.
+
+---
+
+### L15. OpenWeatherMap API Key in URL Query Param
+
+**File:** `lib/towerops/weather/client.ex:13,65`
+
+**Severity:** LOW — API key in URL may be logged by Req in debug/trace
+
+**Fix:** Use header-based auth or ensure Req strips query params from logs.
+
+---
+
+### L16. COW/Vault ETS Table Name Collision Risk
+
+**File:** `lib/towerops/vault.ex` (ETS table naming)
+
+**Severity:** LOW — If two nodes share a node name, ETS table creation could conflict
+
+**Fix:** Use unique ETS names incorporating node identifier.
+
+---
+
+## SUMMARY
+
+### By Severity
+
+| Severity | Count | Key Areas |
+|----------|-------|-----------|
+| CRITICAL | 13 | XSS (maps, status page), IDOR (consent, members, reports), auth bypass (GDPR halt, webhook), credential exposure (k8s/secrets.yaml, Stripe in git, MikroTik over PubSub), runtime crash (Repo.all_by), dead code (brute force), SSL (no force_ssl), race condition (device quota) |
+| HIGH | 30 | N+1 queries (mobile, Oban, alerts, capacity, latency), race conditions (create_check, apply_agent), injection (Search wildcard, OID, user input → atom/integer), IDOR (reports, checks), auth gaps (webhooks, sessions), CSP/HSTS, hardcoded salts, cookie flags, missing validation (protobuf, check creation), worker reliability (sync errors swallowed, weather never starts, unbounded processing) |
+| MEDIUM | 23 | IDOR (resource scoping), email case-sensitivity, open redirect, Stripe webhook not enforced, rate limiting key, overfetching, missing unique constraint, error disclosure, pattern match crashes, admin defense-in-depth, download content-type, queue blocking, PubSub unbounded, memory leaks (MutationObserver), MD5 default, incomplete archive validation, public ETS, impersonation expiry, silent permission failures |
+| LOW | 16 | Log leaks, TTL extension, IP normalization, config documentation, vault dev/prod bleed, version disclosure, device capacity scoping, insights IDOR, code duplication, listener cleanup, path validation, liveSocket global, broad changesets, pagination, API key in URL, ETS naming |
+
+**Total: 82 issues** (13 Critical, 30 High, 23 Medium, 16 Low)
+
+### Top 10 Priority Fixes
+
+1. **C1** — Stored XSS in Sites Map popup
+2. **C2** — IDOR in policy consent handler (client-controlled user_id)
+3. **C3** — Broken `halt()` in GDPR data export
+4. **C6** — `Repo.all_by/2` runtime crash on password/email changes
+5. **C9** — Remove `k8s/secrets.yaml` with live production credentials
+6. **C10** — Remove hardcoded Stripe test key from source
+7. **C8** — Sanitize `custom_css` in status page to prevent HTML injection
+8. **C11** — Enable `force_ssl` in production
+9. **H7** — Add auth plug to webhook pipeline
+10. **H17** — Fix missing org scope on individual report operations
diff --git a/config/dev.exs b/config/dev.exs
index f5188f0b..817a84a1 100644
--- a/config/dev.exs
+++ b/config/dev.exs
@@ -239,8 +239,7 @@ config :towerops, dev_routes: true
# Stripe configuration for development
config :towerops,
- stripe_secret_key:
- "sk_test_51T7zcQS77kvnTfgyu0DCQU2xVKzeaneVCueHaXV3jsIw6HAwGWWllEL3J8jGZOybHAtPyu6oYIiuvJHhReFtTycE00VrJYJ8Ao",
+ stripe_secret_key: System.get_env("STRIPE_SECRET_KEY", "sk_test_use_your_own_key"),
stripe_webhook_secret: "whsec_dev_fake_secret",
stripe_price_id: "price_1T81XBS77kvnTfgyPlw1jF8N",
stripe_meter_id: "mtr_test_61UHPU067veEZ7bhl41S77kvnTfgyODY"
diff --git a/k8s/secrets.example.yaml b/k8s/secrets.example.yaml
index 60cbe8bd..f067281e 100644
--- a/k8s/secrets.example.yaml
+++ b/k8s/secrets.example.yaml
@@ -1,5 +1,5 @@
# Unified template for every Kubernetes Secret the towerops Deployment
-# references. Bootstrap your local copy:
+# references. Copy to k8s/secrets.yaml and fill in real values:
#
# cp k8s/secrets.example.yaml k8s/secrets.yaml
# # edit k8s/secrets.yaml — fill in real values
@@ -26,12 +26,12 @@ metadata:
type: Opaque
stringData:
# Generate with: openssl rand -base64 32
- RELEASE_COOKIE: ""
+ RELEASE_COOKIE: "CHANGE_ME_openssl_rand_-base64_32"
# Generate with: mix phx.gen.secret (or openssl rand -base64 64)
- SECRET_KEY_BASE: ""
+ SECRET_KEY_BASE: "CHANGE_ME_mix_phx_gen_secret"
# Generate with: openssl rand -base64 32
# CRITICAL: losing this makes encrypted columns unrecoverable.
- CLOAK_KEY: ""
+ CLOAK_KEY: "CHANGE_ME_openssl_rand_-base64_32"
---
apiVersion: v1
kind: Secret
@@ -41,14 +41,12 @@ metadata:
type: Opaque
stringData:
# Full ecto:// URL — what runtime.exs reads.
- DATABASE_URL: "ecto://USER:PASSWORD@HOST:5432/DATABASE"
- # The individual fields are convenience for ops — not required by the
- # app but kept here for parity with the existing prod secret.
- POSTGRES_HOST: ""
+ DATABASE_URL: "postgresql://towerops:CHANGE_ME@postgres-host:5432/towerops"
+ POSTGRES_HOST: "CHANGE_ME_postgres_host"
POSTGRES_PORT: "5432"
- POSTGRES_DB: ""
- POSTGRES_USER: ""
- POSTGRES_PASSWORD: ""
+ POSTGRES_DB: "towerops"
+ POSTGRES_USER: "towerops"
+ POSTGRES_PASSWORD: "CHANGE_ME"
---
apiVersion: v1
kind: Secret
@@ -57,9 +55,9 @@ metadata:
namespace: towerops
type: Opaque
stringData:
- AWS_ACCESS_KEY_ID: ""
- AWS_SECRET_ACCESS_KEY: ""
- AWS_REGION: ""
+ AWS_ACCESS_KEY_ID: "CHANGE_ME"
+ AWS_SECRET_ACCESS_KEY: "CHANGE_ME"
+ AWS_REGION: "us-east-1"
---
apiVersion: v1
kind: Secret
@@ -68,11 +66,9 @@ metadata:
namespace: towerops
type: Opaque
stringData:
- # K8s pattern — runtime.exs builds the connection from these
- # individual fields. Dokku alternative is REDIS_URL.
- REDIS_HOST: ""
+ REDIS_HOST: "CHANGE_ME_redis_host"
REDIS_PORT: "6379"
- REDIS_PASSWORD: ""
+ REDIS_PASSWORD: "CHANGE_ME"
---
apiVersion: v1
kind: Secret
@@ -81,11 +77,10 @@ metadata:
namespace: towerops
type: Opaque
stringData:
- # Stripe — every key is optional; the deployment marks them optional: true.
- STRIPE_SECRET_KEY: ""
- STRIPE_WEBHOOK_SECRET: ""
- STRIPE_PRICE_ID: ""
- STRIPE_METER_ID: ""
+ STRIPE_SECRET_KEY: "CHANGE_ME_sk_live_..."
+ STRIPE_WEBHOOK_SECRET: "CHANGE_ME_whsec_..."
+ STRIPE_PRICE_ID: "CHANGE_ME_price_..."
+ STRIPE_METER_ID: "CHANGE_ME_mtr_..."
---
apiVersion: v1
kind: Secret
@@ -94,8 +89,5 @@ metadata:
namespace: towerops
type: Opaque
stringData:
- # DeepSeek API key — get one at https://platform.deepseek.com/
- # Without this, insight LLM enrichment stays no-op (insights still render).
- DEEPSEEK_API_KEY: ""
- # Optional: override the default model (default: deepseek-v4-pro).
+ DEEPSEEK_API_KEY: "CHANGE_ME_sk_..."
DEEPSEEK_MODEL: ""
diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex
index fa2fe826..45405f6e 100644
--- a/lib/towerops/accounts.ex
+++ b/lib/towerops/accounts.ex
@@ -383,7 +383,7 @@ defmodule Towerops.Accounts do
fn ->
case Repo.update(changeset) do
{:ok, user} ->
- tokens_to_expire = Repo.all_by(UserToken, user_id: user.id)
+ tokens_to_expire = Repo.all(from t in UserToken, where: t.user_id == ^user.id)
Repo.delete_all(from(t in UserToken, where: t.id in ^Enum.map(tokens_to_expire, & &1.id)))
{:ok, {user, tokens_to_expire}}
diff --git a/lib/towerops/status_pages/status_page_config.ex b/lib/towerops/status_pages/status_page_config.ex
index 961733ab..fd952f28 100644
--- a/lib/towerops/status_pages/status_page_config.ex
+++ b/lib/towerops/status_pages/status_page_config.ex
@@ -53,10 +53,23 @@ defmodule Towerops.StatusPages.StatusPageConfig do
changeset
css when is_binary(css) ->
- if String.contains?(String.downcase(css), "url(") do
- add_error(changeset, :custom_css, "url() references are not allowed in custom CSS")
- else
- changeset
+ downcased = String.downcase(css)
+
+ cond do
+ String.contains?(downcased, "url(") ->
+ add_error(changeset, :custom_css, "url() references are not allowed in custom CSS")
+
+ String.contains?(downcased, "
+ add_error(changeset, :custom_css, "HTML tags are not allowed in custom CSS")
+
+ String.contains?(downcased, "
+ add_error(changeset, :custom_css, "HTML tags are not allowed in custom CSS")
+
+ String.contains?(downcased, "
+ add_error(changeset, :custom_css, "HTML tags are not allowed in custom CSS")
+
+ true ->
+ changeset
end
_ ->
diff --git a/lib/towerops/workers/mikrotik_backup_worker.ex b/lib/towerops/workers/mikrotik_backup_worker.ex
index 8f0c8aaa..b516e5a5 100644
--- a/lib/towerops/workers/mikrotik_backup_worker.ex
+++ b/lib/towerops/workers/mikrotik_backup_worker.ex
@@ -14,9 +14,6 @@ defmodule Towerops.Workers.MikrotikBackupWorker do
use Oban.Worker, queue: :maintenance, max_attempts: 3
- alias Towerops.Agent.AgentJob
- alias Towerops.Agent.MikrotikCommand
- alias Towerops.Agent.MikrotikDevice
alias Towerops.Devices
alias Towerops.Devices.BackupRequests
@@ -65,68 +62,16 @@ defmodule Towerops.Workers.MikrotikBackupWorker do
# Build backup job
job_id = "backup:#{device.id}:#{DateTime.to_unix(DateTime.utc_now())}"
- mikrotik_config = Devices.get_mikrotik_config(device)
-
- # Generate unique filename for this backup (without .rsc extension, RouterOS adds it)
- backup_filename = "towerops-backup-#{DateTime.to_unix(DateTime.utc_now())}"
-
- # Build chunked read commands
- # Max chunk size is 32KB (32768 bytes), we'll read 10 chunks to cover 320KB
- # Most MikroTik configs are under this size
- chunk_size = 32_768
- num_chunks = 10
-
- read_commands =
- for i <- 0..(num_chunks - 1) do
- %MikrotikCommand{
- command: "/file/read",
- args: %{
- "file" => "#{backup_filename}.rsc",
- "offset" => to_string(i * chunk_size),
- "chunk-size" => to_string(chunk_size)
- }
- }
- end
-
- job = %AgentJob{
- job_id: job_id,
- job_type: :MIKROTIK,
- device_id: device.id,
- mikrotik_device: %MikrotikDevice{
- ip: device.ip_address,
- username: mikrotik_config.username,
- password: mikrotik_config.password,
- port: mikrotik_config.port,
- ssh_port: mikrotik_config.ssh_port,
- use_ssl: mikrotik_config.use_ssl
- },
- mikrotik_commands:
- [
- # Step 1: Export config to file on router (compact format, no defaults)
- %MikrotikCommand{
- command: "/export",
- args: %{"file" => backup_filename, "compact" => ""}
- }
- ] ++
- read_commands ++
- [
- # Final step: Delete the temporary file
- %MikrotikCommand{
- command: "/file/remove",
- args: %{"numbers" => "#{backup_filename}.rsc"}
- }
- ]
- }
-
# Create tracking request
case BackupRequests.create_request(device.id, job_id) do
{:ok, _request} ->
- # Broadcast to agent via PubSub
+ # Broadcast device id to agent channel via PubSub — the channel
+ # resolves credentials itself to avoid exposing them on PubSub.
_ =
Phoenix.PubSub.broadcast(
Towerops.PubSub,
"agent:#{agent_token_id}:backup",
- {:backup_requested, job}
+ {:backup_requested, device.id, job_id}
)
Logger.info("Backup job sent to agent",
diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex
index c72c9dad..ef74f609 100644
--- a/lib/towerops_web/channels/agent_channel.ex
+++ b/lib/towerops_web/channels/agent_channel.ex
@@ -316,17 +316,27 @@ defmodule ToweropsWeb.AgentChannel do
end
# Handle PubSub broadcast when backup is requested for a device
- def handle_info({:backup_requested, job}, socket) do
- Logger.info("Backup requested for device, sending backup job to agent",
+ def handle_info({:backup_requested, device_id, job_id}, socket) do
+ Logger.info("Backup requested for device, assembling and sending backup job to agent",
agent_token_id: socket.assigns.agent_token_id,
- device_id: job.device_id,
- job_id: job.job_id
+ device_id: device_id,
+ job_id: job_id
)
- job_list = %AgentJobList{jobs: [job]}
- binary = AgentJobList.encode(job_list)
+ case Devices.get_device_with_details(device_id) do
+ nil ->
+ Logger.warning("Backup requested but device not found",
+ device_id: device_id,
+ job_id: job_id
+ )
+
+ device ->
+ job = build_backup_job(device, job_id)
+ job_list = %AgentJobList{jobs: [job]}
+ binary = AgentJobList.encode(job_list)
+ push(socket, "backup_job", %{binary: Base.encode64(binary)})
+ end
- push(socket, "backup_job", %{binary: Base.encode64(binary)})
{:noreply, socket}
end
@@ -452,6 +462,53 @@ defmodule ToweropsWeb.AgentChannel do
{:noreply, socket}
end
+ defp build_backup_job(device, job_id) do
+ mikrotik_config = Devices.get_mikrotik_config(device)
+ backup_filename = "towerops-backup-#{DateTime.to_unix(DateTime.utc_now())}"
+ chunk_size = 32_768
+ num_chunks = 10
+
+ read_commands =
+ for i <- 0..(num_chunks - 1) do
+ %MikrotikCommand{
+ command: "/file/read",
+ args: %{
+ "file" => "#{backup_filename}.rsc",
+ "offset" => to_string(i * chunk_size),
+ "chunk-size" => to_string(chunk_size)
+ }
+ }
+ end
+
+ %AgentJob{
+ job_id: job_id,
+ job_type: :MIKROTIK,
+ device_id: device.id,
+ mikrotik_device: %MikrotikDevice{
+ ip: device.ip_address || "",
+ username: mikrotik_config.username || "",
+ password: mikrotik_config.password || "",
+ port: mikrotik_config.port || 8729,
+ ssh_port: mikrotik_config.ssh_port || 22,
+ use_ssl: mikrotik_config.use_ssl || false
+ },
+ mikrotik_commands:
+ [
+ %MikrotikCommand{
+ command: "/export",
+ args: %{"file" => backup_filename, "compact" => "true"}
+ }
+ ] ++
+ read_commands ++
+ [
+ %MikrotikCommand{
+ command: "/file/remove",
+ args: %{"numbers" => "#{backup_filename}.rsc"}
+ }
+ ]
+ }
+ end
+
@impl true
@spec handle_in(String.t(), map(), socket()) :: {:noreply, socket()}
def handle_in("result", %{"binary" => binary_b64}, socket) when is_binary(binary_b64) do
diff --git a/lib/towerops_web/controllers/api/account_data_controller.ex b/lib/towerops_web/controllers/api/account_data_controller.ex
index d08bffe6..d94661f4 100644
--- a/lib/towerops_web/controllers/api/account_data_controller.ex
+++ b/lib/towerops_web/controllers/api/account_data_controller.ex
@@ -17,38 +17,37 @@ defmodule ToweropsWeb.Api.AccountDataController do
user = conn.assigns.current_scope.user
# Verify user account is confirmed before allowing data export
- _ =
- if !user.confirmed_at do
- conn
- |> put_status(:forbidden)
- |> json(%{error: "Email address must be confirmed before exporting account data"})
- |> halt()
- end
+ if user.confirmed_at do
+ # Log the data export for audit trail
+ AuditLogger.log_user_data_exported(conn, user.id)
- # Log the data export for audit trail
- AuditLogger.log_user_data_exported(conn, user.id)
-
- # Gather all user data
- data = %{
- profile: build_profile_data(user),
- organizations: build_organizations_data(user),
- devices: build_devices_data(user),
- alerts: build_alerts_data(user),
- audit_logs: build_audit_logs_data(user),
- export_info: %{
- exported_at: DateTime.utc_now(),
- format: "JSON",
- gdpr_article: "Article 15 - Right to Access"
+ # Gather all user data
+ data = %{
+ profile: build_profile_data(user),
+ organizations: build_organizations_data(user),
+ devices: build_devices_data(user),
+ alerts: build_alerts_data(user),
+ audit_logs: build_audit_logs_data(user),
+ export_info: %{
+ exported_at: DateTime.utc_now(),
+ format: "JSON",
+ gdpr_article: "Article 15 - Right to Access"
+ }
}
- }
- conn
- |> put_resp_content_type("application/json")
- |> put_resp_header(
- "content-disposition",
- "attachment; filename=\"towerops-data-#{user.id}-#{DateTime.to_unix(DateTime.utc_now())}.json\""
- )
- |> json(data)
+ conn
+ |> put_resp_content_type("application/json")
+ |> put_resp_header(
+ "content-disposition",
+ "attachment; filename=\"towerops-data-#{user.id}-#{DateTime.to_unix(DateTime.utc_now())}.json\""
+ )
+ |> json(data)
+ else
+ conn
+ |> put_status(:forbidden)
+ |> json(%{error: "Email address must be confirmed before exporting account data"})
+ |> halt()
+ end
end
# Build profile data
diff --git a/lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex b/lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex
index 8120b595..baca54d8 100644
--- a/lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex
+++ b/lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex
@@ -14,7 +14,7 @@ defmodule ToweropsWeb.Api.V1.AgentReleaseWebhookController do
@max_age_seconds 300
def create(conn, _params) do
- case verify_optional_signature(conn) do
+ case verify_signature(conn) do
:ok ->
case Oban.insert(AgentReleaseWebhookWorker.new(%{})) do
{:ok, _job} ->
@@ -34,17 +34,17 @@ defmodule ToweropsWeb.Api.V1.AgentReleaseWebhookController do
end
end
- defp verify_optional_signature(conn) do
+ defp verify_signature(conn) do
sig_headers = Plug.Conn.get_req_header(conn, "x-agent-webhook-signature")
case sig_headers do
- [] ->
- :ok
-
[header] ->
secret = Application.get_env(:towerops, :agent_webhook_secret)
raw_body = conn.private[:raw_body] || ""
verify_hmac_signature(header, raw_body, secret)
+
+ [] ->
+ {:error, :missing_signature}
end
end
diff --git a/lib/towerops_web/controllers/api/v1/members_controller.ex b/lib/towerops_web/controllers/api/v1/members_controller.ex
index efadf32a..c2994924 100644
--- a/lib/towerops_web/controllers/api/v1/members_controller.ex
+++ b/lib/towerops_web/controllers/api/v1/members_controller.ex
@@ -20,19 +20,25 @@ defmodule ToweropsWeb.Api.V1.MembersController do
def update(conn, %{"id" => user_id, "role" => role}) do
organization_id = conn.assigns.current_organization_id
- case Organizations.update_member_role(organization_id, user_id, role) do
- {:ok, membership} ->
- membership = Towerops.Repo.preload(membership, :user)
- json(conn, %{data: format_member(membership)})
+ case authorize_member_management(conn, organization_id) do
+ {:ok, conn} ->
+ case Organizations.update_member_role(organization_id, user_id, role) do
+ {:ok, membership} ->
+ membership = Towerops.Repo.preload(membership, :user)
+ json(conn, %{data: format_member(membership)})
- {:error, :cannot_change_owner_role} ->
- conn |> put_status(:forbidden) |> json(%{error: "Cannot change owner role"})
+ {:error, :cannot_change_owner_role} ->
+ conn |> put_status(:forbidden) |> json(%{error: "Cannot change owner role"})
- {:error, :not_found} ->
- conn |> put_status(:not_found) |> json(%{error: "Member not found"})
+ {:error, :not_found} ->
+ conn |> put_status(:not_found) |> json(%{error: "Member not found"})
- {:error, changeset} ->
- conn |> put_status(:unprocessable_entity) |> json(%{errors: translate_errors(changeset)})
+ {:error, changeset} ->
+ conn |> put_status(:unprocessable_entity) |> json(%{errors: translate_errors(changeset)})
+ end
+
+ {:halt, conn} ->
+ conn
end
end
@@ -43,10 +49,44 @@ defmodule ToweropsWeb.Api.V1.MembersController do
def delete(conn, %{"id" => user_id}) do
organization_id = conn.assigns.current_organization_id
- case Organizations.remove_member(organization_id, user_id) do
- {:ok, _} -> conn |> put_status(:no_content) |> send_resp(204, "")
- {:error, :cannot_remove_owner} -> conn |> put_status(:forbidden) |> json(%{error: "Cannot remove owner"})
- {:error, :not_found} -> conn |> put_status(:not_found) |> json(%{error: "Member not found"})
+ case authorize_member_management(conn, organization_id) do
+ {:ok, conn} ->
+ case Organizations.remove_member(organization_id, user_id) do
+ {:ok, _} -> conn |> put_status(:no_content) |> send_resp(204, "")
+ {:error, :cannot_remove_owner} -> conn |> put_status(:forbidden) |> json(%{error: "Cannot remove owner"})
+ {:error, :not_found} -> conn |> put_status(:not_found) |> json(%{error: "Member not found"})
+ end
+
+ {:halt, conn} ->
+ conn
+ end
+ end
+
+ defp authorize_member_management(conn, organization_id) do
+ user = conn.assigns[:current_user]
+
+ if user do
+ membership = Organizations.get_membership(organization_id, user.id)
+
+ if membership && membership.role in [:owner, :admin] do
+ {:ok, conn}
+ else
+ conn =
+ conn
+ |> put_status(:forbidden)
+ |> json(%{error: "Only owners and admins can manage members"})
+ |> halt()
+
+ {:halt, conn}
+ end
+ else
+ conn =
+ conn
+ |> put_status(:unauthorized)
+ |> json(%{error: "Authentication required"})
+ |> halt()
+
+ {:halt, conn}
end
end
diff --git a/lib/towerops_web/live/device_live/show.ex b/lib/towerops_web/live/device_live/show.ex
index ea27363b..854359fc 100644
--- a/lib/towerops_web/live/device_live/show.ex
+++ b/lib/towerops_web/live/device_live/show.ex
@@ -932,71 +932,19 @@ defmodule ToweropsWeb.DeviceLive.Show do
end
defp trigger_manual_backup(socket, device, agent_token_id) do
- alias Towerops.Agent.AgentJob
- alias Towerops.Agent.MikrotikCommand
- alias Towerops.Agent.MikrotikDevice
alias Towerops.Devices.BackupRequests
job_id = "backup:#{device.id}:#{DateTime.to_unix(DateTime.utc_now())}"
- mikrotik_config = Devices.get_mikrotik_config(device)
-
- # Generate unique filename for this backup (without .rsc extension, RouterOS adds it)
- backup_filename = "towerops-backup-#{DateTime.to_unix(DateTime.utc_now())}"
-
- # Build chunked read commands
- # Max chunk size is 32KB (32768 bytes), we'll read 10 chunks to cover 320KB
- chunk_size = 32_768
- num_chunks = 10
-
- read_commands =
- for i <- 0..(num_chunks - 1) do
- %MikrotikCommand{
- command: "/file/read",
- args: %{
- "file" => "#{backup_filename}.rsc",
- "offset" => to_string(i * chunk_size),
- "chunk-size" => to_string(chunk_size)
- }
- }
- end
-
- job = %AgentJob{
- job_id: job_id,
- job_type: :MIKROTIK,
- device_id: device.id,
- mikrotik_device: %MikrotikDevice{
- ip: device.ip_address,
- username: mikrotik_config.username,
- password: mikrotik_config.password,
- port: mikrotik_config.port,
- ssh_port: mikrotik_config.ssh_port,
- use_ssl: mikrotik_config.use_ssl
- },
- mikrotik_commands:
- [
- # Step 1: Export config to file on router (compact format, no defaults)
- %MikrotikCommand{
- command: "/export",
- args: %{"file" => backup_filename, "compact" => ""}
- }
- ] ++
- read_commands ++
- [
- # Final step: Delete the temporary file
- %MikrotikCommand{
- command: "/file/remove",
- args: %{"numbers" => "#{backup_filename}.rsc"}
- }
- ]
- }
case BackupRequests.create_request(device.id, job_id, "manual") do
{:ok, _request} ->
+ # Send device id + job id over PubSub — agent channel resolves
+ # credentials itself to avoid exposing them on PubSub.
_ =
Phoenix.PubSub.broadcast(
Towerops.PubSub,
"agent:#{agent_token_id}:backup",
- {:backup_requested, job}
+ {:backup_requested, device.id, job_id}
)
{:noreply,
diff --git a/lib/towerops_web/plugs/brute_force_protection.ex b/lib/towerops_web/plugs/brute_force_protection.ex
index 932187b0..8a8389ff 100644
--- a/lib/towerops_web/plugs/brute_force_protection.ex
+++ b/lib/towerops_web/plugs/brute_force_protection.ex
@@ -15,7 +15,7 @@ defmodule ToweropsWeb.Plugs.BruteForceProtection do
## Exemptions
- - Authenticated users (conn.assigns[:current_user] present)
+ - Authenticated users (checked in before_send callback, since auth runs after this plug)
- Agent WebSocket connections (/socket/agent - authenticated at channel join)
- Whitelisted IPs (checked via BruteForce context)
@@ -43,10 +43,6 @@ defmodule ToweropsWeb.Plugs.BruteForceProtection do
ip_address = RemoteIp.from_conn(conn)
cond do
- # Skip for authenticated users
- conn.assigns[:current_user] != nil ->
- conn
-
# Skip for agent WebSocket connections (authenticated at channel join)
String.starts_with?(conn.request_path, "/socket/agent") ->
conn
@@ -69,7 +65,12 @@ defmodule ToweropsWeb.Plugs.BruteForceProtection do
defp register_404_tracker(conn, ip_address) do
register_before_send(conn, fn response_conn ->
- track_404_if_needed(response_conn, ip_address, conn.request_path)
+ # Skip tracking for authenticated users (auth runs after this plug,
+ # but before_send runs after the full pipeline including auth).
+ if !response_conn.assigns[:current_user] do
+ track_404_if_needed(response_conn, ip_address, conn.request_path)
+ end
+
response_conn
end)
end
diff --git a/lib/towerops_web/user_auth.ex b/lib/towerops_web/user_auth.ex
index c79abda0..db606f53 100644
--- a/lib/towerops_web/user_auth.ex
+++ b/lib/towerops_web/user_auth.ex
@@ -690,9 +690,10 @@ defmodule ToweropsWeb.UserAuth do
# Attach LiveView event handler for accepting updated policies
socket =
LiveView.attach_hook(socket, :policy_consent_handler, :handle_event, fn
- "accept_updated_policies", %{"user-id" => user_id}, socket ->
+ "accept_updated_policies", _params, socket ->
# Grant consent for all policies that need re-consent
policies_needing_consent = Map.get(socket.assigns, :policies_needing_consent, [])
+ user_id = socket.assigns.current_scope.user.id
Enum.each(policies_needing_consent, fn policy_type ->
Accounts.grant_consent(user_id, policy_type)
diff --git a/setup_stripe_meter.exs b/setup_stripe_meter.exs
index 5a753903..0d147750 100644
--- a/setup_stripe_meter.exs
+++ b/setup_stripe_meter.exs
@@ -2,7 +2,7 @@
# Setup script for Stripe billing meter
-stripe_key = "sk_test_51T7zcQS77kvnTfgyu0DCQU2xVKzeaneVCueHaXV3jsIw6HAwGWWllEL3J8jGZOybHAtPyu6oYIiuvJHhReFtTycE00VrJYJ8Ao"
+stripe_key = System.get_env("STRIPE_SECRET_KEY", "sk_test_use_your_own_key")
product_id = "prod_U6DpFdl21ftiDz"
IO.puts("Creating Stripe billing meter...")
diff --git a/test/towerops/workers/mikrotik_backup_worker_test.exs b/test/towerops/workers/mikrotik_backup_worker_test.exs
index 8c1c3e60..f339c8b1 100644
--- a/test/towerops/workers/mikrotik_backup_worker_test.exs
+++ b/test/towerops/workers/mikrotik_backup_worker_test.exs
@@ -93,22 +93,13 @@ defmodule Towerops.Workers.MikrotikBackupWorkerTest do
assert :ok = perform_job(MikrotikBackupWorker, %{})
end)
- assert_receive {:backup_requested, job}, 1_000
- assert job.device_id == device.id
- assert job.mikrotik_device.username == "admin"
- assert job.mikrotik_device.port == 8729
- assert job.mikrotik_device.ssh_port == 22
- assert job.mikrotik_device.use_ssl == true
- assert job.job_type == :MIKROTIK
- # 1 export + 10 read chunks + 1 remove = 12 commands
- assert length(job.mikrotik_commands) == 12
- [first | _] = job.mikrotik_commands
- assert first.command == "/export"
- last = List.last(job.mikrotik_commands)
- assert last.command == "/file/remove"
+ assert_receive {:backup_requested, received_device_id, job_id}, 1_000
+ assert received_device_id == device.id
+ assert is_binary(job_id)
+ assert String.starts_with?(job_id, "backup:#{device.id}:")
# Backup request was created in DB
- assert BackupRequests.get_request_by_job_id(job.job_id)
+ assert BackupRequests.get_request_by_job_id(job_id)
end
test "returns {:error, _} when at least one backup request fails to create" do
diff --git a/test/towerops_web/channels/agent_channel_processing_test.exs b/test/towerops_web/channels/agent_channel_processing_test.exs
index 69896445..fce6164d 100644
--- a/test/towerops_web/channels/agent_channel_processing_test.exs
+++ b/test/towerops_web/channels/agent_channel_processing_test.exs
@@ -10,7 +10,6 @@ defmodule ToweropsWeb.AgentChannelProcessingTest do
alias Towerops.AccountsFixtures
alias Towerops.Agent.AgentHeartbeat
- alias Towerops.Agent.AgentJob
alias Towerops.Agent.AgentJobList
alias Towerops.Agent.MikrotikResult
alias Towerops.Agent.MikrotikSentence
@@ -288,23 +287,19 @@ defmodule ToweropsWeb.AgentChannelProcessingTest do
agent_token: agent_token,
device: device
} do
- job = %AgentJob{
- job_id: "backup:#{device.id}",
- job_type: :MIKROTIK,
- device_id: device.id
- }
+ job_id = "backup:#{device.id}"
Phoenix.PubSub.broadcast(
Towerops.PubSub,
"agent:#{agent_token.id}:backup",
- {:backup_requested, job}
+ {:backup_requested, device.id, job_id}
)
assert_push "backup_job", %{binary: jobs_binary}, 200
{:ok, decoded} = Base.decode64(jobs_binary)
{:ok, job_list} = AgentJobList.decode(decoded)
- assert hd(job_list.jobs).job_id == "backup:#{device.id}"
+ assert hd(job_list.jobs).job_id == job_id
end
end
diff --git a/test/towerops_web/channels/agent_channel_test.exs b/test/towerops_web/channels/agent_channel_test.exs
index 6f1e1aa6..ca63eb4f 100644
--- a/test/towerops_web/channels/agent_channel_test.exs
+++ b/test/towerops_web/channels/agent_channel_test.exs
@@ -6,7 +6,6 @@ defmodule ToweropsWeb.AgentChannelTest do
alias Towerops.AccountsFixtures
alias Towerops.Agent.AgentError
alias Towerops.Agent.AgentHeartbeat
- alias Towerops.Agent.AgentJob
alias Towerops.Agent.AgentJobList
alias Towerops.Agent.CheckResult, as: CheckResultProto
alias Towerops.Agent.CredentialTestResult
@@ -771,17 +770,11 @@ defmodule ToweropsWeb.AgentChannelTest do
end
end
- describe "handle_info {:backup_requested, job}" do
+ describe "handle_info {:backup_requested, device_id, job_id}" do
test "pushes backup job to agent", %{socket: socket, device: device} do
- job = %AgentJob{
- job_id: "backup:#{device.id}",
- job_type: :MIKROTIK,
- device_id: device.id,
- mikrotik_device: nil,
- mikrotik_commands: []
- }
+ job_id = "backup:#{device.id}"
- send(socket.channel_pid, {:backup_requested, job})
+ send(socket.channel_pid, {:backup_requested, device.id, job_id})
assert_push "backup_job", %{binary: jobs_binary}
@@ -789,7 +782,7 @@ defmodule ToweropsWeb.AgentChannelTest do
{:ok, job_list} = AgentJobList.decode(decoded_binary)
backup_job = hd(job_list.jobs)
- assert backup_job.job_id == "backup:#{device.id}"
+ assert backup_job.job_id == job_id
assert backup_job.device_id == device.id
end
end
@@ -2170,17 +2163,12 @@ defmodule ToweropsWeb.AgentChannelTest do
agent_token: agent_token,
device: device
} do
- # Channel should be subscribed to "agent:#{agent_token.id}:backup"
- job = %AgentJob{
- job_id: "backup:#{device.id}",
- job_type: :MIKROTIK,
- device_id: device.id
- }
+ job_id = "backup:#{device.id}"
Phoenix.PubSub.broadcast(
Towerops.PubSub,
"agent:#{agent_token.id}:backup",
- {:backup_requested, job}
+ {:backup_requested, device.id, job_id}
)
assert_push "backup_job", %{binary: jobs_binary}, 200
diff --git a/test/towerops_web/controllers/api/v1/agent_release_webhook_controller_test.exs b/test/towerops_web/controllers/api/v1/agent_release_webhook_controller_test.exs
index 5951799f..f3a06888 100644
--- a/test/towerops_web/controllers/api/v1/agent_release_webhook_controller_test.exs
+++ b/test/towerops_web/controllers/api/v1/agent_release_webhook_controller_test.exs
@@ -36,11 +36,29 @@ defmodule ToweropsWeb.Api.V1.AgentReleaseWebhookControllerTest do
describe "create/2 response" do
test "enqueues job and returns 200", %{conn: conn} do
- conn = post(conn, ~p"/api/v1/webhooks/agent-release")
+ ts = :second |> System.system_time() |> to_string()
+ secret = Application.get_env(:towerops, :agent_webhook_secret)
+ raw_body = ""
+
+ sig =
+ :hmac
+ |> :crypto.mac(:sha256, secret, ts <> "." <> raw_body)
+ |> Base.encode16(case: :lower)
+
+ conn =
+ conn
+ |> put_req_header("x-agent-webhook-signature", "t=#{ts},v1=#{sig}")
+ |> post(~p"/api/v1/webhooks/agent-release")
assert json_response(conn, 200) == %{"status" => "ok"}
assert_enqueued(worker: AgentReleaseWebhookWorker)
end
+
+ test "returns 401 when signature header is missing", %{conn: conn} do
+ conn = post(conn, ~p"/api/v1/webhooks/agent-release")
+
+ assert json_response(conn, 401)["error"] == "Signature verification failed"
+ end
end
describe "check_timestamp/1" do
diff --git a/test/towerops_web/plugs/brute_force_protection_test.exs b/test/towerops_web/plugs/brute_force_protection_test.exs
index 4a3c5232..a0b9b9b8 100644
--- a/test/towerops_web/plugs/brute_force_protection_test.exs
+++ b/test/towerops_web/plugs/brute_force_protection_test.exs
@@ -132,9 +132,10 @@ defmodule ToweropsWeb.Plugs.BruteForceProtectionTest do
|> BruteForceProtection.call([])
refute conn.halted
- # No before_send callback registered for authenticated users — they
- # bypass the 404 tracker entirely.
- assert conn.private[:before_send] in [nil, []]
+ # before_send callback is registered but will skip tracking for
+ # authenticated users (auth check moved into the callback since
+ # this plug runs before authentication).
+ assert is_list(conn.private[:before_send])
end
test "passes through immediately for agent socket paths" do