-
Notifications
You must be signed in to change notification settings - Fork 82
Standardize UUID validation across all REST API controllers#4591
Standardize UUID validation across all REST API controllers #4591m4cd4r4 wants to merge 1 commit into
Conversation
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 OpenFn#4588
e6abefc to
516e7a6
Compare
m4cd4r4
commented
May 12, 2026
Rebased onto current main (was 61 commits behind, CONFLICTING) and addressed the failing test_elixir job.
Conflicts resolved:
lib/.../api/run_controller.ex(show): merged with the new%Lightning.Run{}struct match so we get UUID validation plus the stricter return-shape check.lib/.../api/work_orders_controller.ex(show): same pattern for%Lightning.WorkOrder{}.lib/.../api/project_controller.ex(show): kept the%Lightning.Projects.Project{}struct match introduced upstream.lib/.../api/job_controller.ex(show): switched to the newJobs.get_job(id, include: [...])API (no bang, single preload) that landed upstream.test/.../api/run_controller_test.exs: kept both the new upstreamdescribe "show"block and this PR'sdescribe "malformed UUID params"block.
Test failure root cause:
The index action in credential_controller.ex had an else clause that handled nil and {:error, :unauthorized} but not {:error, :bad_request}, so Helpers.validate_uuid/1 returning :bad_request for a malformed project_id blew up the with (WithClauseError). Added the missing {:error, :bad_request} -> {:error, :bad_request} arm so it falls through to FallbackController, which already returns 400. Matches the pattern this PR already uses in delete.
The second failure in the prior CI (test list_dataclips_for_job/3 doesn't return a dataclip if the wrong text is entered in Lightning.InvocationTest) is unrelated to this PR (touches no invocation code) - flagging in case it's a known flake.
CI is re-running now.
Summary
Closes #4588.
Malformed UUID path/query params currently reach
Repo.getand raiseEcto.Query.CastError, which Phoenix turns into a 500 response. This extracts a sharedvalidate_uuid/1helper and applies it consistently across all REST API controllers.Changes:
validate_uuid/1toLightningWeb.API.Helpers- returns:okor{:error, :bad_request}WorkflowsController- privatevalidate_uuid/1now delegates to the shared helper (preserves existing 422 message behaviour viamaybe_handle_error)CredentialController- replaces privatevalidate_uuid/1with shared helper; malformed credentialidnow returns 400 instead of 404JobController,ProjectController,RunController,WorkOrdersController,ProvisioningController,AiAssistantController- addvalidate_uuid/1calls before any DB lookup that would otherwise raiseEcto.Query.CastErrorTests: UUID validation test cases added to all affected controller test files.
AI disclosure
Test plan
not-a-uuidinputsmix test test/lightning_web/controllers/api/passes