Standardize UUID validation across all REST API controllers

m4cd4r4 · m4cd4r4 · commit 516e7a63af41 · 2026-05-12T14:17:24.000+08:00
Extract a shared validate_uuid/1 function into LightningWeb.API.Helpers using the existing Ecto.UUID.dump/1 pattern. Replace private duplicate implementations in WorkflowsController and CredentialController. Apply validation to all controllers that accept UUID path or query params, so malformed UUIDs return 400 instead of raising Ecto.Query.CastError (500). Closes #4588
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -281,6 +281,9 @@ and this project adheres to
   [PR#4551](https://github.com/OpenFn/lightning/pull/4551)
 - Fix AI assistant authorization for support users on projects with support
   access enabled [#4571](https://github.com/OpenFn/lightning/issues/4571)
+- REST API controllers now return 400 instead of 500 for malformed UUID
+  parameters. Extracts a shared `validate_uuid/1` helper used across all API
+  controllers. [#4588](https://github.com/OpenFn/lightning/issues/4588)
 
 ## [2.16.0] - 2026-03-24
 
diff --git a/lib/lightning_web/controllers/api/ai_assistant_controller.ex b/lib/lightning_web/controllers/api/ai_assistant_controller.ex
@@ -9,6 +9,7 @@ defmodule LightningWeb.API.AiAssistantController do
   alias Lightning.Policies.Permissions
   alias Lightning.Projects
   alias Lightning.Workflows
+  alias LightningWeb.API.Helpers
   alias LightningWeb.Channels.AiAssistantJSON
 
   action_fallback LightningWeb.FallbackController
@@ -75,17 +76,21 @@ defmodule LightningWeb.API.AiAssistantController do
   end
 
   defp get_resource("job_code", %{"job_id" => job_id}) do
-    {:ok, job_id}
+    with :ok <- Helpers.validate_uuid(job_id) do
+      {:ok, job_id}
+    end
   end
 
   defp get_resource("job_code", _params) do
     {:error, :bad_request}
   end
 
   defp get_resource("workflow_template", %{"project_id" => project_id}) do
-    case Projects.get_project(project_id) do
-      nil -> {:error, :not_found}
-      project -> {:ok, project}
+    with :ok <- Helpers.validate_uuid(project_id) do
+      case Projects.get_project(project_id) do
+        nil -> {:error, :not_found}
+        project -> {:ok, project}
+      end
     end
   end
 
diff --git a/lib/lightning_web/controllers/api/credential_controller.ex b/lib/lightning_web/controllers/api/credential_controller.ex
@@ -25,6 +25,7 @@ defmodule LightningWeb.API.CredentialController do
   alias Lightning.Policies.Permissions
   alias Lightning.Policies.ProjectUsers
   alias Lightning.Projects
+  alias LightningWeb.API.Helpers
 
   action_fallback LightningWeb.FallbackController
 
@@ -61,7 +62,8 @@ defmodule LightningWeb.API.CredentialController do
   def index(conn, %{"project_id" => project_id}) do
     current_user = conn.assigns.current_resource
 
-    with project when not is_nil(project) <- Projects.get_project(project_id),
+    with :ok <- Helpers.validate_uuid(project_id),
+         project when not is_nil(project) <- Projects.get_project(project_id),
          :ok <-
            ProjectUsers
            |> Permissions.can(
@@ -72,6 +74,9 @@ defmodule LightningWeb.API.CredentialController do
       credentials = Credentials.list_credentials(project)
       render(conn, "index.json", credentials: credentials)
     else
+      {:error, :bad_request} ->
+        {:error, :bad_request}
+
       nil ->
         {:error, :not_found}
 
@@ -166,15 +171,15 @@ defmodule LightningWeb.API.CredentialController do
   def delete(conn, %{"id" => id}) do
     current_user = conn.assigns.current_resource
 
-    with :ok <- validate_uuid(id),
+    with :ok <- Helpers.validate_uuid(id),
          credential when not is_nil(credential) <-
            Credentials.get_credential(id),
          :ok <- validate_credential_ownership(credential, current_user),
          {:ok, _} <- Credentials.delete_credential(credential) do
       send_resp(conn, :no_content, "")
     else
-      {:error, :invalid_uuid} ->
-        {:error, :not_found}
+      {:error, :bad_request} ->
+        {:error, :bad_request}
 
       nil ->
         {:error, :not_found}
@@ -187,13 +192,6 @@ defmodule LightningWeb.API.CredentialController do
     end
   end
 
-  defp validate_uuid(id) do
-    case Ecto.UUID.dump(to_string(id)) do
-      {:ok, _bin} -> :ok
-      :error -> {:error, :invalid_uuid}
-    end
-  end
-
   defp validate_credential_ownership(credential, current_user) do
     if credential.user_id == current_user.id do
       :ok
diff --git a/lib/lightning_web/controllers/api/helpers.ex b/lib/lightning_web/controllers/api/helpers.ex
@@ -51,4 +51,20 @@ defmodule LightningWeb.API.Helpers do
     }
     |> URI.to_string()
   end
+
+  @doc """
+  Validates that the given value is a well-formed UUID.
+
+  Returns `:ok` on success or `{:error, :bad_request}` when the value
+  cannot be parsed as a UUID. Use this in API controllers before passing
+  an ID to the database layer, which would raise `Ecto.Query.CastError`
+  for invalid values.
+  """
+  @spec validate_uuid(any()) :: :ok | {:error, :bad_request}
+  def validate_uuid(id) do
+    case Ecto.UUID.dump(to_string(id)) do
+      {:ok, _bin} -> :ok
+      :error -> {:error, :bad_request}
+    end
+  end
 end
diff --git a/lib/lightning_web/controllers/api/job_controller.ex b/lib/lightning_web/controllers/api/job_controller.ex
@@ -54,6 +54,7 @@ defmodule LightningWeb.API.JobController do
   alias Lightning.Policies.Permissions
   alias Lightning.Policies.ProjectUsers
   alias Lightning.Workflows
+  alias LightningWeb.API.Helpers
 
   action_fallback LightningWeb.FallbackController
 
@@ -94,7 +95,8 @@ defmodule LightningWeb.API.JobController do
   def index(conn, %{"project_id" => project_id} = params) do
     pagination_attrs = Map.take(params, ["page_size", "page"])
 
-    with project <- Lightning.Projects.get_project(project_id),
+    with :ok <- Helpers.validate_uuid(project_id),
+         project <- Lightning.Projects.get_project(project_id),
          :ok <-
            ProjectUsers
            |> Permissions.can(
@@ -145,7 +147,8 @@ defmodule LightningWeb.API.JobController do
   """
   @spec show(Plug.Conn.t(), map()) :: Plug.Conn.t()
   def show(conn, %{"id" => id}) do
-    with {:ok, job} <- Jobs.get_job(id, include: [workflow: :project]),
+    with :ok <- Helpers.validate_uuid(id),
+         {:ok, job} <- Jobs.get_job(id, include: [workflow: :project]),
          :ok <-
            ProjectUsers
            |> Permissions.can(
diff --git a/lib/lightning_web/controllers/api/project_controller.ex b/lib/lightning_web/controllers/api/project_controller.ex
@@ -36,6 +36,7 @@ defmodule LightningWeb.API.ProjectController do
   alias Lightning.Policies.Permissions
   alias Lightning.Policies.ProjectUsers
   alias Lightning.Projects
+  alias LightningWeb.API.Helpers
 
   action_fallback LightningWeb.FallbackController
 
@@ -94,7 +95,8 @@ defmodule LightningWeb.API.ProjectController do
   """
   @spec show(Plug.Conn.t(), map()) :: Plug.Conn.t()
   def show(conn, %{"id" => id}) do
-    with %Lightning.Projects.Project{} = project <-
+    with :ok <- Helpers.validate_uuid(id),
+         %Lightning.Projects.Project{} = project <-
            Projects.get_project(id),
          :ok <-
            ProjectUsers
diff --git a/lib/lightning_web/controllers/api/provisioning_controller.ex b/lib/lightning_web/controllers/api/provisioning_controller.ex
@@ -20,6 +20,7 @@ defmodule LightningWeb.API.ProvisioningController do
   alias Lightning.Projects.Provisioner
   alias Lightning.Workflows
   alias Lightning.WorkflowVersions
+  alias LightningWeb.API.Helpers
 
   action_fallback(LightningWeb.FallbackController)
 
@@ -129,7 +130,8 @@ defmodule LightningWeb.API.ProvisioningController do
   """
   @spec show(Plug.Conn.t(), map()) :: Plug.Conn.t()
   def show(conn, params) do
-    with project = %Project{} <-
+    with :ok <- Helpers.validate_uuid(params["id"]),
+         project = %Project{} <-
            Projects.get_project(params["id"]) || {:error, :not_found},
          :ok <-
            Permissions.can(
@@ -186,7 +188,8 @@ defmodule LightningWeb.API.ProvisioningController do
   """
   @spec show_yaml(Plug.Conn.t(), map()) :: Plug.Conn.t()
   def show_yaml(conn, %{"id" => id} = params) do
-    with %Projects.Project{} = project <-
+    with :ok <- Helpers.validate_uuid(id),
+         %Projects.Project{} = project <-
            Projects.get_project(id) || {:error, :not_found},
          :ok <-
            Permissions.can(
diff --git a/lib/lightning_web/controllers/api/run_controller.ex b/lib/lightning_web/controllers/api/run_controller.ex
@@ -56,6 +56,7 @@ defmodule LightningWeb.API.RunController do
   alias Lightning.Policies.Permissions
   alias Lightning.Policies.ProjectUsers
   alias Lightning.Runs
+  alias LightningWeb.API.Helpers
 
   action_fallback LightningWeb.FallbackController
 
@@ -104,7 +105,8 @@ defmodule LightningWeb.API.RunController do
   def index(conn, %{"project_id" => project_id} = params) do
     pagination_attrs = Map.take(params, ["page_size", "page"])
 
-    with :ok <-
+    with :ok <- Helpers.validate_uuid(project_id),
+         :ok <-
            Invocation.Query.validate_datetime_params(params, [
              "inserted_after",
              "inserted_before",
@@ -174,7 +176,8 @@ defmodule LightningWeb.API.RunController do
   """
   @spec show(Plug.Conn.t(), map()) :: Plug.Conn.t()
   def show(conn, %{"id" => id}) do
-    with %Lightning.Run{} = run <-
+    with :ok <- Helpers.validate_uuid(id),
+         %Lightning.Run{} = run <-
            Runs.get(id, include: [work_order: [workflow: :project]]),
          :ok <-
            ProjectUsers
diff --git a/lib/lightning_web/controllers/api/work_orders_controller.ex b/lib/lightning_web/controllers/api/work_orders_controller.ex
@@ -63,6 +63,7 @@ defmodule LightningWeb.API.WorkOrdersController do
   alias Lightning.Policies.Permissions
   alias Lightning.Policies.ProjectUsers
   alias Lightning.WorkOrders
+  alias LightningWeb.API.Helpers
 
   action_fallback LightningWeb.FallbackController
 
@@ -111,7 +112,8 @@ defmodule LightningWeb.API.WorkOrdersController do
   def index(conn, %{"project_id" => project_id} = params) do
     pagination_attrs = Map.take(params, ["page_size", "page"])
 
-    with :ok <-
+    with :ok <- Helpers.validate_uuid(project_id),
+         :ok <-
            Invocation.Query.validate_datetime_params(params, [
              "inserted_after",
              "inserted_before",
@@ -181,7 +183,8 @@ defmodule LightningWeb.API.WorkOrdersController do
   """
   @spec show(Plug.Conn.t(), map()) :: Plug.Conn.t()
   def show(conn, %{"id" => id}) do
-    with %Lightning.WorkOrder{} = work_order <-
+    with :ok <- Helpers.validate_uuid(id),
+         %Lightning.WorkOrder{} = work_order <-
            WorkOrders.get(id, include: [workflow: :project, runs: []]),
          :ok <-
            ProjectUsers
diff --git a/lib/lightning_web/controllers/api/workflows_controller.ex b/lib/lightning_web/controllers/api/workflows_controller.ex
@@ -92,6 +92,7 @@ defmodule LightningWeb.API.WorkflowsController do
   alias Lightning.Workflows.Edge
   alias Lightning.Workflows.Presence
   alias Lightning.Workflows.Workflow
+  alias LightningWeb.API.Helpers
   alias LightningWeb.ChangesetJSON
 
   action_fallback LightningWeb.FallbackController
@@ -499,10 +500,10 @@ defmodule LightningWeb.API.WorkflowsController do
     end
   end
 
-  defp validate_uuid(project_id) do
-    case Ecto.UUID.dump(to_string(project_id)) do
-      {:ok, _bin} -> :ok
-      :error -> {:error, :invalid_id, project_id}
+  defp validate_uuid(id) do
+    case Helpers.validate_uuid(id) do
+      :ok -> :ok
+      {:error, :bad_request} -> {:error, :invalid_id, id}
     end
   end
 
diff --git a/test/lightning_web/controllers/api/credential_controller_test.exs b/test/lightning_web/controllers/api/credential_controller_test.exs
@@ -261,6 +261,11 @@ defmodule LightningWeb.API.CredentialControllerTest do
       assert json_response(conn, 404) == %{"error" => "Not Found"}
     end
 
+    test "returns 400 for malformed project_id", %{conn: conn, user: _user} do
+      conn = get(conn, ~p"/api/projects/not-a-uuid/credentials")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+
     test "returns empty list when project has no credentials", %{
       conn: conn,
       user: user
@@ -841,7 +846,7 @@ defmodule LightningWeb.API.CredentialControllerTest do
 
     test "handles invalid UUID format", %{conn: conn, user: _user} do
       conn = delete(conn, ~p"/api/credentials/invalid-uuid")
-      assert json_response(conn, 404) == %{"error" => "Not Found"}
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
     end
   end
 end
diff --git a/test/lightning_web/controllers/api/job_controller_test.exs b/test/lightning_web/controllers/api/job_controller_test.exs
@@ -94,6 +94,20 @@ defmodule LightningWeb.API.JobControllerTest do
                "type" => "jobs"
              }
     end
+
+    test "returns 400 for malformed id", %{conn: conn} do
+      conn = get(conn, ~p"/api/jobs/not-a-uuid")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+  end
+
+  describe "index with invalid project_id" do
+    setup [:assign_bearer_for_api]
+
+    test "returns 400 for malformed project_id", %{conn: conn} do
+      conn = get(conn, ~p"/api/projects/not-a-uuid/jobs")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
   end
 
   defp create_job(%{project: project}) do
diff --git a/test/lightning_web/controllers/api/project_controller_test.exs b/test/lightning_web/controllers/api/project_controller_test.exs
@@ -91,6 +91,11 @@ defmodule LightningWeb.API.ProjectControllerTest do
       assert json_response(conn, 401) == %{"error" => "Unauthorized"}
     end
 
+    test "returns 400 for malformed id", %{conn: conn} do
+      conn = get(conn, ~p"/api/projects/not-a-uuid")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+
     test "shows the project", %{conn: conn, project: project} do
       conn = get(conn, Routes.api_project_path(conn, :show, project))
       response = json_response(conn, 200)
diff --git a/test/lightning_web/controllers/api/provisioning_controller_test.exs b/test/lightning_web/controllers/api/provisioning_controller_test.exs
@@ -1441,4 +1441,18 @@ defmodule LightningWeb.API.ProvisioningControllerTest do
       end)
     end)
   end
+
+  describe "malformed UUID params" do
+    setup [:assign_bearer_for_api]
+
+    test "returns 400 for malformed id in show", %{conn: conn} do
+      conn = get(conn, ~p"/api/provision/not-a-uuid")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+
+    test "returns 400 for malformed id in show_yaml", %{conn: conn} do
+      conn = get(conn, ~p"/api/provision/yaml?id=not-a-uuid")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+  end
 end
diff --git a/test/lightning_web/controllers/api/run_controller_test.exs b/test/lightning_web/controllers/api/run_controller_test.exs
@@ -893,4 +893,18 @@ defmodule LightningWeb.API.RunControllerTest do
       assert json_response(conn, 401)
     end
   end
+
+  describe "malformed UUID params" do
+    setup [:assign_bearer_for_api]
+
+    test "returns 400 for malformed project_id in index", %{conn: conn} do
+      conn = get(conn, ~p"/api/projects/not-a-uuid/runs")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+
+    test "returns 400 for malformed id in show", %{conn: conn} do
+      conn = get(conn, ~p"/api/runs/not-a-uuid")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+  end
 end
diff --git a/test/lightning_web/controllers/api/work_orders_controller_test.exs b/test/lightning_web/controllers/api/work_orders_controller_test.exs
@@ -708,4 +708,18 @@ defmodule LightningWeb.API.WorkOrdersControllerTest do
       assert json_response(conn, 401)
     end
   end
+
+  describe "malformed UUID params" do
+    setup [:assign_bearer_for_api]
+
+    test "returns 400 for malformed project_id in index", %{conn: conn} do
+      conn = get(conn, ~p"/api/projects/not-a-uuid/work_orders")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+
+    test "returns 400 for malformed id in show", %{conn: conn} do
+      conn = get(conn, ~p"/api/work_orders/not-a-uuid")
+      assert json_response(conn, 400) == %{"error" => "Bad Request"}
+    end
+  end
 end
