Standardize UUID validation across all REST API controllers
Nobody has claimed this yet.
- Dominant language
- Elixir
- Stars
- 296
- Forks
- 86
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 50
Description
User story
As an API consumer, when I accidentally pass a malformed identifier (e.g. a typo, a slug, or a non-UUID string) to any API endpoint, I expect a clear 400 Bad Request response telling me the parameter is invalid. Instead, most endpoints return a 500 Internal Server Error with no useful message, which makes me think the server is broken rather than that my request was wrong. This is inconsistent with the behavior on /api/workflows endpoints, which do return proper errors.
Details
UUID validation before database queries is only implemented in 2 of the 9 API controllers:
WorkflowsControllervalidates UUIDs across all actions via a privatevalidate_uuid/1functionCredentialControllervalidates UUIDs only in itsdeleteaction via its own duplicate privatevalidate_uuid/1function
The remaining 7 controllers pass user-supplied ID parameters directly into Ecto/Repo queries with no format validation:
| Controller | Unvalidated UUID parameters |
|---|---|
JobController |
id (path), project_id (query) |
ProjectController |
id (path) |
RunController |
id (path), project_id (path), workflow_id (query), work_order_id (query) |
WorkOrdersController |
id (path in show, query filter in index), project_id (path), workflow_id (query) |
LogLinesController |
project_id, workflow_id, job_id, work_order_id, run_id (all query) |
ProvisioningController |
id (path and body) |
AiAssistantController |
job_id, project_id, workflow_id (all query) |
CredentialController (gaps) |
project_id in index and create actions |
When a malformed UUID (e.g. "notauuid") reaches Ecto, it raises either Ecto.Query.CastError or Postgrex.Error. The FallbackController has no clause to handle either exception, so Phoenix returns a generic 500 response.
Test coverage is nearly absent. Only a single test across all API controller test files (credential_controller_test.exs:842) exercises a malformed UUID input. Even WorkflowsController, which has correct production validation, has no corresponding test. All other "not found" tests use Ecto.UUID.generate() (valid UUID format, just nonexistent) — these verify 404 behavior but do not test the malformed-input path.
Implementation notes
- Shared helper: Extract a single
validate_uuid/1function intolib/lightning_web/controllers/api/helpers.ex, which already exists as a home for shared API utilities. The existing implementation pattern usingEcto.UUID.dump/1is sound and can be reused. An alternative approach would be a Plug that validates UUID-shaped path/query params before the action is called, keeping validation logic out of individual action functions. - Replace duplicates: Remove the two private
validate_uuid/1functions inWorkflowsControllerandCredentialControllerand replace their call sites with the shared version. - Apply consistently: Audit every controller action that accepts a UUID parameter (path or query) and add validation at the entry point of each action. The table above can serve as a checklist.
- Error response:
FallbackControlleralready handles{:error, :bad_request}→ 400, so the shared helper just needs to return that tuple on validation failure. - Test helper: Consider a shared ExUnit helper (e.g.
assert_returns_400_for_invalid_uuid(conn, method, path)) to reduce boilerplate when adding malformed-UUID tests across all controller test files. Each controller should have at least one test per action confirming that malformed UUIDs return 400 with a meaningful error body.
Release notes
Fixed a bug where passing a malformed (non-UUID) identifier to most API endpoints returned a 500 Internal Server Error instead of a 400 Bad Request. UUID parameter validation is now applied consistently across all API controllers.
User acceptance criteria
- Passing a malformed UUID as a path parameter (e.g.
GET /api/jobs/notauuid) returns400 Bad Requestwith a JSON error body — not 500 - Passing a malformed UUID as a query parameter (e.g.
GET /api/runs?project_id=notauuid) returns400 Bad Requestwith a JSON error body — not 500 - All 9 API controllers validate all UUID parameters consistently
- The two existing private
validate_uuid/1implementations are replaced by a single shared helper - Every controller action that accepts a UUID parameter has at least one test asserting that a malformed UUID returns 400
- Existing behavior preserved: valid-format UUIDs referencing nonexistent resources still return 404
- No unhandled
Ecto.Query.CastErrororPostgrex.Errorexceptions from UUID parameter handling in any API controller
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with lib/lightning_web/controllers/api/helpers.ex, the existing validate_uuid/1 implementations in WorkflowsController and CredentialController, and the FallbackController handling of {:error, :bad_request}. Audit the listed API controllers and their ExUnit files, beginning with credential_controller_test.exs:842. Done means malformed path and query UUIDs return 400 with a JSON error, valid nonexistent UUIDs still return 404, and every UUID-accepting action has coverage.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- elixir
- Domain
- api, backend, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100