Skip to content

Standardize UUID validation across all REST API controllers#4591

Open
m4cd4r4 wants to merge 1 commit into
OpenFn:mainfrom
m4cd4r4:fix/uuid-validation-api-controllers
Open

Standardize UUID validation across all REST API controllers#4591
m4cd4r4 wants to merge 1 commit into
OpenFn:mainfrom
m4cd4r4:fix/uuid-validation-api-controllers

Conversation

@m4cd4r4
Copy link
Copy Markdown

@m4cd4r4 m4cd4r4 commented Apr 1, 2026

Summary

Closes #4588.

Malformed UUID path/query params currently reach Repo.get and raise Ecto.Query.CastError, which Phoenix turns into a 500 response. This extracts a shared validate_uuid/1 helper and applies it consistently across all REST API controllers.

Changes:

  • Add validate_uuid/1 to LightningWeb.API.Helpers - returns :ok or {:error, :bad_request}
  • WorkflowsController - private validate_uuid/1 now delegates to the shared helper (preserves existing 422 message behaviour via maybe_handle_error)
  • CredentialController - replaces private validate_uuid/1 with shared helper; malformed credential id now returns 400 instead of 404
  • JobController, ProjectController, RunController, WorkOrdersController, ProvisioningController, AiAssistantController - add validate_uuid/1 calls before any DB lookup that would otherwise raise Ecto.Query.CastError

Tests: UUID validation test cases added to all affected controller test files.

AI disclosure

  • I used Claude Code to assist with this contribution

Test plan

  • New tests in each controller test file assert 400 for not-a-uuid inputs
  • Existing tests continue to pass
  • mix test test/lightning_web/controllers/api/ passes

@github-project-automation github-project-automation Bot moved this to New Issues in Core Apr 1, 2026
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
@m4cd4r4 m4cd4r4 force-pushed the fix/uuid-validation-api-controllers branch from e6abefc to 516e7a6 Compare May 12, 2026 06:17
@m4cd4r4
Copy link
Copy Markdown
Author

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 new Jobs.get_job(id, include: [...]) API (no bang, single preload) that landed upstream.
  • test/.../api/run_controller_test.exs: kept both the new upstream describe "show" block and this PR's describe "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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New Issues

Development

Successfully merging this pull request may close these issues.

Standardize UUID validation across all REST API controllers

1 participant