Skip to content

kw_api_custom_endpoint: POST answers 200 for a write that never happened (endpoint domain ignored on by-id branches) #2

Description

@brmk

What breaks

A POST to a custom endpoint answers 200 for a write that never happened, whenever
the target record is outside what a bare search can see — in practice, whenever it is
archived.

{"content": [], "code": "200"}

The record is unchanged. The caller is told it succeeded. Nothing is logged as an error.

Why

The configured domain is applied on the LIST branch
(custom_endpoint.py:250-259) and ignored by every by-id branch. change() resolves
its target with a bare search:

# kw_api_custom_endpoint/models/custom_endpoint.py:365
obj_id = m.search([(self.model_id_field, '=', obj_id)], limit=1)
obj_id.write(data)

Odoo only skips the implicit active = True when a domain mentions active. This one
mentions nothing, so an archived record is never found, write() on the empty recordset is
a silent no-op, and data_response serialises the empty recordset as the 200 above.

So an endpoint explicitly configured with domain = [("active","in",[True,False])] — which
says archived records are in scope — still cannot write to them.

Note the asymmetry: response() (GET by id, line 268) has the same bare search but answers
400: Wrong ID when nothing resolves. Only the write path reports success. delete()
(line 392) shares the pattern.

Reproduce

Endpoint on product.template, domain = [("active","in",[True,False])], update enabled,
field name changeable.

POST /kw_api/custom/<api_name>/<id>   {"name": "new name"}

active template   -> 200, record echoed back, name changed
archived template -> 200, {"content": []}, name NOT changed

What it costs, concretely

Our storefront keeps ~127 of 1044 Odoo-linked products on archived templates: the
products are discontinued in Odoo but still rank in Google, so the pages stay up. A nightly
job pushes product names and translations from the storefront into Odoo.

Every night, ~130 name slots were reported written and were not. For months. The job's own
summary said failed: 0, because a 200 is a 200.

The general shape of the damage:

  • Silent divergence. Two systems drift apart while both believe they agree. Nobody
    investigates, because there is no error anywhere to investigate.
  • No way to detect it from the API. A client cannot distinguish "written" from "quietly
    discarded" — both are code: "200". We ended up having to verify writes by checking
    whether Odoo echoed the record back, and treat a bare 2xx as meaningless.
  • Long-lived data, permanently stale. Archived records are exactly the ones nobody looks
    at manually, so nothing corrects the drift later.
  • A scoping surprise in the other direction. Because the by-id branches ignore the
    domain entirely, a POST can also update a record the endpoint's domain was configured
    to exclude. The domain reads as a scope, but only filters reads.

Suggested fix

Apply the endpoint's own domain when resolving by id — the same thing the list branch does:

domain = [(self.model_id_field, '=', obj_id)]
try:
    endpoint_domain = safe_eval(self.domain) if self.domain else []
except Exception as e:
    _logger.debug(e)
    endpoint_domain = []
if endpoint_domain:
    domain = AND([domain, endpoint_domain])
obj_id = m.search(domain, limit=1)

This needs no context manipulation: a domain that mentions active makes Odoo skip the
implicit active_test on its own. It also makes the domain mean the same thing on reads
and writes.

Answering 400: Wrong ID when the id resolves to nothing — matching what response()
already does — would close the "success for nothing" hole even where a domain is absent.

A pull request for change() is attached. response() and delete() have the same bare
search; I left those alone since widening a delete's reach deserves the maintainer's call.

Warning for whoever implements this

Do not fix it by widening active_test on the endpoint recordset. We tried

super(CustomEndpoint, self.with_context(active_test=False)).change(...)

and it put active_test=False into the whole request environment. kw_api's translation
helper does:

# kw_api/models/mixin.py:119
for lang in self.env['res.lang'].sudo().search([]):

res.lang is archivable, so that started returning inactive languages, and reading a
translation for a language Odoo has not loaded raises KeyError: '<model>.<field>' from the
ORM field cache — after a successful write, inside the response serialiser. The result
is that every write breaks, not just archived ones. It reached our production and was
reverted 23 minutes later. Applying the domain avoids this entirely.

Affected

kw_api_custom_endpoint 19.0.1.7.4 (19.0 branch); the same code is on 18.0.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions