OpenFn / OpenFn/lightning

Standardize UUID validation across all REST API controllers

Open
#4,588 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
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:

  • WorkflowsController validates UUIDs across all actions via a private validate_uuid/1 function
  • CredentialController validates UUIDs only in its delete action via its own duplicate private validate_uuid/1 function

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/1 function into lib/lightning_web/controllers/api/helpers.ex, which already exists as a home for shared API utilities. The existing implementation pattern using Ecto.UUID.dump/1 is 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/1 functions in WorkflowsController and CredentialController and 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: FallbackController already 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) returns 400 Bad Request with a JSON error body — not 500
  • Passing a malformed UUID as a query parameter (e.g. GET /api/runs?project_id=notauuid) returns 400 Bad Request with a JSON error body — not 500
  • All 9 API controllers validate all UUID parameters consistently
  • The two existing private validate_uuid/1 implementations 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.CastError or Postgrex.Error exceptions from UUID parameter handling in any API controller

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.