From 8e3f20a589450d4327e2a27e911e8cf6ee32aa19 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 13 May 2026 12:50:53 -0500 Subject: [PATCH] cleanup --- bugs.md | 127 -------------------------------------------------------- 1 file changed, 127 deletions(-) delete mode 100644 bugs.md diff --git a/bugs.md b/bugs.md deleted file mode 100644 index 52b81271..00000000 --- a/bugs.md +++ /dev/null @@ -1,127 +0,0 @@ -# Code Review Findings - -> Full codebase audit conducted 2026-05-12. Covers OWASP Top 10, performance, reliability, and correctness issues across the entire TowerOps Web application. - ---- - -## HIGH - -### 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. - ---- - -### 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. - ---- - - - - -### 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. - ---- - -## MEDIUM - - - -### 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}"`. - ---- - - - - - - - - - - - - -## 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. - ---- - - -### 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. - ---- - - - - - -### 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. - ---- -