towerops/docs/SECURITY_AUDIT_API.md

Security Audit: API & External Integrations

Date: 2026-03-14 Scope: REST API, GraphQL API, webhook handlers, integration clients, background workers Auditor: Automated code review

---

Summary

Severity	Count
High	2
Medium	5
Low	3

Overall the codebase demonstrates strong security practices: webhook signature verification uses Plug.Crypto.secure_compare, API tokens are hashed with SHA-256, credentials are encrypted at rest (Cloak AES-256-GCM), rate limiting is applied to auth and API endpoints, and GraphQL has complexity/depth limits. The findings below are areas for improvement.

---

HIGH Severity

H1: SSRF via User-Controlled Integration URLs (Sonar, Splynx)

Files:

• lib/towerops/sonar/client.ex — lines 141-142
• lib/towerops/splynx/client.ex — lines 13-15 (via authenticate/3), lines 95-96 (via get/4)
• lib/towerops/sonar/sync.ex — line 21
• lib/towerops/splynx/sync.ex — line 20

Description: Sonar and Splynx integrations accept a user-supplied instance_url stored in integration.credentials["instance_url"]. This URL is directly concatenated into HTTP requests without any validation:

# sonar/client.ex:142
url = String.trim_trailing(instance_url, "/") <> "/api/graphql"

# splynx/client.ex — authenticate/3 and get/4
url = String.trim_trailing(instance_url, "/") <> "/api/2.0/admin/auth/tokens"

An attacker with org-admin access could set instance_url to http://169.254.169.254 (cloud metadata), http://localhost:5432, or any internal service, causing the server to make requests to internal network resources. The integration sync workers then execute these requests periodically in the background.

Impact: Server-Side Request Forgery (SSRF). Could access cloud metadata endpoints (AWS/GCP instance credentials), internal services, or scan internal networks.

Suggested Fix: Add URL validation in Towerops.Integrations when creating/updating integrations:

defp validate_instance_url(changeset) do
  validate_change(changeset, :credentials, fn :credentials, creds ->
    case creds["instance_url"] do
      nil -> []
      url ->
        uri = URI.parse(url)
        cond do
          uri.scheme not in ["https", "http"] ->
            [credentials: "instance_url must use http or https"]
          is_private_ip?(uri.host) ->
            [credentials: "instance_url cannot point to a private IP address"]
          uri.host in ["localhost", "127.0.0.1", "::1", "0.0.0.0"] ->
            [credentials: "instance_url cannot point to localhost"]
          true -> []
        end
    end
  end)
end

Also consider resolving the hostname and checking the IP before making requests.

---

H2: Admin API Routes Missing Rate Limiting

File: lib/towerops_web/router.ex — lines 172-180

Description: The admin API scope (/admin/api) only pipes through :api_v1 but does not include :rate_limit_api or any rate limiting pipeline:

scope "/admin/api", V1 do
  pipe_through :api_v1          # <-- no rate_limit_api

  get "/mibs", MibController, :index
  post "/mibs", MibController, :upload
  delete "/mibs/:vendor", MibController, :delete
  post "/geoip/import", GeoipController, :import_database
end

While these endpoints do check for superuser status in the controller, a compromised superuser API token could be used to flood the server with MIB uploads or GeoIP imports without any rate limiting, potentially causing DoS.

Impact: Denial of service via resource exhaustion (disk space from MIB uploads, database from GeoIP imports).

Suggested Fix: Add rate limiting to the admin API pipeline:

scope "/admin/api", V1 do
  pipe_through [:api_v1, :rate_limit_api]  # or a stricter :admin_rate_limit
  ...
end

---

MEDIUM Severity

M1: GraphQL Complexity/Depth Limits Disabled in Non-Production Environments

File: lib/towerops_web/graphql/context.ex — lines 15-20

Description: The GraphQL context plug conditionally applies complexity analysis only in production:

opts =
  if Application.get_env(:towerops, :env) == :prod do
    [context: context, analyze_complexity: true, max_complexity: 500]
  else
    [context: context]
  end

Note: The router (router.ex:139-141) does set analyze_complexity: true, max_complexity: 500, max_depth: 10 at the Absinthe.Plug level. However, the Context plug overwrites these options via Absinthe.Plug.put_options/2, which replaces the entire options map. In non-production environments, this effectively removes the complexity and depth limits set in the router.

Impact: In staging/dev environments (which may be publicly accessible), deeply nested or highly complex queries could cause CPU/memory exhaustion.

Suggested Fix: Apply complexity limits in all environments:

def call(conn, _opts) do
  context = build_context(conn)
  Absinthe.Plug.put_options(conn,
    context: context,
    analyze_complexity: true,
    max_complexity: 500
  )
end

---

M2: GraphQL Introspection Block is Bypassable

File: lib/towerops_web/plugs/graphql_introspection.ex — lines 18-24

Description: The introspection check uses simple string matching:

String.contains?(query, "__schema") or String.contains?(query, "__type")

This can be bypassed using:

1. GraphQL aliases: query { myAlias: __schema { types { name } } } — Actually still contains __schema, so this specific bypass won't work.
2. Batch requests: If the query parameter is sent as part of a JSON array (batch queries), conn.body_params won't match the %{"query" => query} pattern since it would be a list, not a map.
3. The operationName + variables pattern: The check only examines the query field, but if the GraphQL request uses persisted queries or extensions, the query text may not be in body_params["query"].

More critically, the check only runs on body_params["query"]. If the query is sent via GET request with query parameters (which Absinthe supports by default), the body params would be empty and the check is bypassed entirely.

Impact: Schema exposure in production, revealing the full API surface to attackers.

Suggested Fix: Also check query params for GET requests, and consider using Absinthe's built-in introspection control:

# In schema.ex - more reliable approach
def context(ctx) do
  if Application.get_env(:towerops, :env) == :prod do
    Map.put(ctx, :introspection, false)
  else
    ctx
  end
end

Or disable introspection at the Absinthe level by adding a phase or middleware.

---

M3: Webhook Endpoints (Gaiia, PagerDuty) Lack Dedicated Rate Limiting

File: lib/towerops_web/router.ex — lines 163-169

Description: The Gaiia and PagerDuty webhook routes share the general :rate_limit_api (1000 req/min per IP):

scope "/api/v1/webhooks", V1 do
  pipe_through [:api, :rate_limit_api]

  post "/gaiia/:organization_id", GaiiaWebhookController, :create
  post "/pagerduty/:organization_id", PagerdutyWebhookController, :create
end

These endpoints are unauthenticated (signature verification happens in the controller after the request is processed). At 1000 requests/minute, an attacker can:

• Force the server to perform 1000 database lookups per minute (to find the integration and its webhook secret)
• Force 1000 HMAC computations per minute
• The organization_id in the URL can be enumerated to discover valid organizations

Impact: Organization enumeration and moderate DoS potential through resource consumption.

Suggested Fix: Apply stricter rate limiting (e.g., 60 req/min) to webhook endpoints:

pipeline :rate_limit_webhook do
  plug RateLimit, type: :webhook  # Add a :webhook type with lower limits
end

---

M4: Stripe Webhook Raw Body May Be Empty

File: lib/towerops_web/controllers/api/v1/stripe_webhook_controller.ex — line 14

Description: The Stripe webhook controller reads the raw body with a fallback to empty string:

raw_body = conn.private[:raw_body] || ""

If raw_body is empty (e.g., due to a middleware issue or the body being consumed), verify_signature/2 would still attempt verification against an empty payload. The parse_signature_header/1 would crash on nil input if the stripe-signature header is also missing (since List.first([]) returns nil, and String.split(nil, ",") raises).

Impact: Unhandled exception could leak stack traces in non-production error responses, and the empty body fallback could mask misconfiguration issues.

Suggested Fix: Add explicit nil/empty checks:

def create(conn, _params) do
  with signature when is_binary(signature) <- get_req_header(conn, "stripe-signature") |> List.first(),
       raw_body when is_binary(raw_body) and raw_body != "" <- conn.private[:raw_body] do
    # ... process
  else
    nil -> conn |> put_status(400) |> json(%{error: "Missing signature header"})
    "" -> conn |> put_status(400) |> json(%{error: "Empty request body"})
  end
end

---

M5: to_atom_keys in IntegrationsController Could Crash on Invalid Keys

File: lib/towerops_web/controllers/api/v1/integrations_controller.ex — lines 89-93

Description: The to_atom_keys/1 helper converts user-supplied string keys to atoms using String.to_existing_atom/1:

defp to_atom_keys(map) when is_map(map) do
  Map.new(map, fn {k, v} -> {String.to_existing_atom(k), v} end)
rescue
  ArgumentError -> map
end

While String.to_existing_atom/1 is safe (won't create new atoms), the rescue clause catches ArgumentError and returns the original map with string keys. This means:

1. If ANY key is not an existing atom, ALL keys remain as strings (including valid ones)
2. The changeset then receives a mix of expected atom keys and unexpected string keys, potentially silently dropping valid fields

This is a correctness bug more than a security issue, but it could lead to integration updates silently failing to apply credential changes.

Suggested Fix: Convert keys individually, falling back per-key:

defp to_atom_keys(map) when is_map(map) do
  Map.new(map, fn {k, v} ->
    key = try do
      String.to_existing_atom(k)
    rescue
      ArgumentError -> k
    end
    {key, v}
  end)
end

---

LOW Severity

L1: API Token Verification Uses Database Lookup Instead of Constant-Time Comparison

File: lib/towerops/api_tokens.ex — lines 79-97

Description: API token verification hashes the input token and does a database lookup:

def verify_token(raw_token) do
  token_hash = hash_token(raw_token)
  case Repo.get_by(ApiToken, token_hash: token_hash) do
    nil -> {:error, :invalid_token}
    token -> ...
  end
end

This is the standard and correct approach for hashed tokens — database lookup timing is dominated by query execution time, making timing attacks impractical. The hash comparison happens in PostgreSQL which uses constant-time comparison for indexed lookups.

However, the SHA-256 hash is not salted. While this doesn't enable practical attacks (tokens are 32 random bytes with ~256 bits of entropy), salted hashing (e.g., bcrypt) would provide defense-in-depth if the database were compromised.

Impact: Theoretical — if the database is breached, unsalted SHA-256 hashes of tokens are exposed. Given the high entropy of the tokens, brute-forcing is infeasible.

No immediate action required. Note for future consideration.

---

L2: Mobile Session Token Lookup Timing

File: lib/towerops/mobile_sessions.ex — lines 38-47

Description: Mobile session tokens are hashed and looked up via database query:

def get_session_by_token(token) when is_binary(token) do
  hashed = MobileSession.hash_token(token)
  MobileSession
  |> where([s], s.token == ^hashed)
  |> where([s], s.expires_at > ^now)
  |> Repo.one()
end

Same pattern as API tokens — hash-then-lookup. This is secure for the same reasons as L1.

No action required.

---

L3: Gaiia Webhook Signature Mismatch Logging Leaks Partial Signatures

File: lib/towerops_web/controllers/api/v1/gaiia_webhook_controller.ex — lines 96-103

Description: On signature mismatch, the controller logs partial signature values:

Logger.warning(
  "Gaiia webhook signature mismatch — " <>
    "ts=#{timestamp} body_len=#{byte_size(raw_body)} " <>
    "secret_len=#{byte_size(secret)} " <>
    "expected=#{String.slice(expected, 0, 12)}… " <>
    "received=#{String.slice(v1_signature, 0, 12)}…"
)

This logs:

• The length of the webhook secret (secret_len)
• The first 12 characters of the expected HMAC signature

While the HMAC output is not the secret itself, logging the secret length reveals information that could help an attacker. The first 12 hex chars of the expected signature provide limited information (6 bytes of a 32-byte HMAC).

Impact: Minor information leakage in logs. Not directly exploitable but violates principle of least information.

Suggested Fix: Remove secret_len and expected from the log message:

Logger.warning(
  "Gaiia webhook signature mismatch — " <>
    "ts=#{timestamp} body_len=#{byte_size(raw_body)}"
)

---

Positive Findings (No Issues)

These areas were reviewed and found to be properly secured:

1. Webhook signature verification: All three webhook handlers (Stripe, Gaiia, PagerDuty) use Plug.Crypto.secure_compare/2 for constant-time signature comparison. ✅

2. Agent webhook auth: Uses Plug.Crypto.secure_compare/2 against a shared secret. ✅

3. Rate limiting: Applied to auth endpoints (10/min), API endpoints (1000/min), and admin browser routes (100/min). ✅

4. GraphQL depth/complexity limits: Router configures max_depth: 10 and max_complexity: 500 with analyze_complexity: true. ✅ (But see M1 about context plug overwriting in non-prod.)

5. Credentials at rest: Integration credentials use Encrypted.Map (Cloak AES-256-GCM). ✅

6. CSRF protection: Browser pipeline includes :protect_from_forgery. ✅

7. Security headers: CSP, frame-ancestors, and secure browser headers configured. ✅

8. MIB upload path traversal protection: Vendor names validated with regex, extracted archive paths validated against extraction directory. ✅

9. Oban worker error handling: Workers use max_attempts configuration and return :ok on non-retryable errors, {:error, reason} on retryable errors. No infinite retry loops found. ✅

10. Organization scoping: API controllers consistently scope queries to conn.assigns.current_organization_id. ✅

11. Integration client error handling: All HTTP clients (VISP, Sonar, Splynx, Gaiia, Preseem) handle error responses gracefully with pattern matching and don't leak credentials in error messages. ✅

12. Token generation: Uses :crypto.strong_rand_bytes(32) for API tokens — cryptographically secure. ✅