Skip to content

fix: PATCH threat_models owner with an empty email returns 500 with a deserialization message instead of 400 #815

Description

@ericfitz

Summary

Patching a threat model's owner with an email of "" returns 500 server_error carrying an internal deserialization message, rather than a 400 rejecting the invalid input. The empty string fails the User.email regex in the schema, so this is an input-validation failure being surfaced as a server error.

Steps to Reproduce

TOK=$(uv run scripts/cats-token.py --user test-user --server http://localhost:30080)
API=http://localhost:30080
ID=<a threat model owned by test-user>

curl -s -w "\nHTTP %{http_code}\n" -X PATCH "$API/threat_models/$ID" \
  -H "Authorization: Bearer $TOK" \
  -H 'Content-Type: application/json-patch+json' \
  -d '[{"op":"replace","path":"/owner","value":{
        "principal_type":"user","provider":"tmi","provider_id":"test-reviewer",
        "display_name":"test-reviewer","email":""}}]'

Expected Behavior

400 with a message naming the offending field — the same shape any other schema violation gets. User.email carries a pattern, and "" does not match it, so this is a client error.

Actual Behavior

HTTP 500
{"error":"server_error",
 "error_description":"Failed to deserialize patched entity: email: failed to pass regex validation"}

Two problems:

  1. Wrong status class. A malformed request body is a 4xx. A 500 tells the client to retry, and clients do — tmi-ux's PATCH retry logic re-sent this three times before surfacing it.
  2. Internal detail leaks. "Failed to deserialize patched entity" describes the server's internal unmarshalling step, not anything the caller can act on.

Evidence

For contrast, the same request with email omitted succeeds and the server correctly derives both email and display_name from provider + provider_id:

HTTP 200
owner -> {"display_name":"Test Reviewer (TMI User)","email":"test-reviewer@tmi.local",
          "principal_type":"user","provider":"tmi","provider_id":"test-reviewer"}

And an owner naming a principal that cannot be resolved returns a clean 404 not_found with the owner unchanged — which is the correct handling this case should resemble.

Possible Cause

api/patch_utils.go:88-95 returns http.StatusInternalServerError for any json.Unmarshal failure on the patched entity:

if err := json.Unmarshal(modifiedBytes, &modified); err != nil {
    return zero, &RequestError{
        Status:  http.StatusInternalServerError,
        Code:    "server_error",
        Message: "Failed to deserialize patched entity: " + err.Error(),
    }
}

That path conflates two different failures: the server producing malformed JSON (genuinely a 500) and the caller's patch producing an entity that violates the schema (a 400). Generated-type validation errors — the regex/minLength family — arrive here from the second case and should map to 400 with the field name preserved and the deserialization framing dropped.

Impact

Low frequency, but misleading whenever it fires: a 500 sends clients into retry loops over input that can never succeed. Found while fixing ericfitz/tmi-ux#895, where the client was sending email: "" for an owner promoted from an unsaved permission row. The tmi-ux client no longer sends it (it now omits empty server-derived fields), so this is not blocking anything — but any other client hitting the same input gets the same misleading answer.

Environment

  • Server: dev stack on docker-desktop, tmi-platform namespace, NodePort http://localhost:30080
  • Content-Type: application/json-patch+json
  • Observed: 2026-08-23

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    • Status
      This milestone

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions