Skip to content

FeaturesAPI PATCH returns HTTP 500 (KeyError) when the request body omits feature_changes or stages #6464

Description

@lucasccordeiro

Describe the bug

PATCH /api/v0/features/<feature_id> returns HTTP 500 Internal Server Error
instead of HTTP 400 Bad Request when the JSON request body does not contain
the expected top-level keys.

FeaturesAPI.do_patch (api/features_api.py:549) reads the body and then indexes
it with no presence guard:

body = self.get_json_param_dict()          # returns {} for a missing/invalid body
if 'id' not in body['feature_changes']:    # line 552 — KeyError if 'feature_changes' is absent
    self.abort(400, msg='Missing feature ID in feature updates')
...
stage_ids = [s['id'] for s in body['stages'] if 'id' in s]   # line 567 — KeyError if 'stages' is absent

When feature_changes is missing, body['feature_changes'] raises KeyError.
APIHandler.patch (framework/basehandlers.py:285) calls do_patch with no
surrounding except, so the exception propagates to Flask and becomes HTTP 500.

This follows the same convention as PR #6451 ("Give 400 for bad channel range."):
code in api/ should self.abort(400, …) on bad user input rather than letting
an exception escape as a 500.

To Reproduce

Steps to reproduce the behavior:

  1. As a signed-in user (the endpoint requires sign-in + XSRF), send a PATCH with a
    body that omits feature_changes:
    PATCH /api/v0/features/123
    Content-Type: application/json
    
    {}
    
  2. Observe HTTP 500 Internal Server Error (a KeyError: 'feature_changes'
    traceback in the logs) instead of HTTP 400. This path is reached before the
    feature lookup and before permission validation, so any signed-in user can
    trigger it.
  3. The body['stages'] access (line 567) is the same defect, but it is gated: it
    is reached only for a body such as
    {"feature_changes": {"id": <existing_feature_id>}} where the feature exists
    and the caller has edit permission
    on it. In that case, omitting stages
    yields the same KeyError → HTTP 500.

Expected behavior

A body missing a required top-level key should be rejected with HTTP 400 Bad
Request
and a clear message, not produce an HTTP 500.

Additional context

  • Affected code (commit 4294104b):
    • api/features_api.py:552body['feature_changes'] (also :554, :580)
    • api/features_api.py:567 / :570body['stages']
    • framework/basehandlers.py:285APIHandler.patch has no except around
      do_patch; get_json_param_dict (basehandlers.py:112-114) returns {} for a
      missing/invalid body.
  • Suggested fix:
    if 'feature_changes' not in body:
        self.abort(400, msg='Missing feature_changes')
    # ...and for the stages access:
    for s in body.get('stages', []):
        ...
  • Same class as the ChannelsAPI ?start > ?end → HTTP 500 issue fixed in
    PR Give 400 for bad channel range. #6451; found by sweeping api/ handlers for user input that reaches an uncaught exception. Confirmed with a minimal standalone reproducer and an ESBMC bounded-model-checking harness by the Veribee Team (https://www.veribee.co/).

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions