From 21fa0d9a12828b47b7e3fdbe1263b73a1d42bd53 Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 16:31:05 +1000 Subject: [PATCH] fix(advice): make the id helper unusable rather than merely rejected _as_doc_id returned None for a hostile id. That was guarded at the one call site, but the helper could still be misused into "$in needs an array" - and a caller who forgot the guard would have got an error rather than a safe no-op. It now returns an EMPTY LIST for anything that is not a reading id. An empty list is a valid $in argument that matches nothing, so the helper cannot produce a query error, cannot match every document, and cannot be interpreted as a query operator. Verified against the live database: a hostile id matches 0 documents and updates 0 documents. 164 tests. --- src/flask/mongo/repository.py | 8 ++++++-- tests/test_brew_advice.py | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/flask/mongo/repository.py b/src/flask/mongo/repository.py index 0044244..5aceb89 100644 --- a/src/flask/mongo/repository.py +++ b/src/flask/mongo/repository.py @@ -230,7 +230,7 @@ class BeerRepository: generated or is a note explaining that it could not be. """ doc_id = _as_doc_id(reading_id) - if doc_id is None: + if not doc_id: return 0 # `$in` matches either form. A plain list would be an exact # ARRAY match in Mongo, which is not what is wanted here. @@ -422,7 +422,11 @@ def _as_doc_id(value: object): except Exception: # Not hex - it can only be the original app's string id. return [value] - return None + # Anything else - a dict, a list, an int, None - is not a reading + # id. An EMPTY LIST is returned rather than None: it is a valid + # `$in` argument that matches nothing, so the helper cannot be + # misused into a query error or into matching every document. + return [] def _as_date(value: object) -> str: diff --git a/tests/test_brew_advice.py b/tests/test_brew_advice.py index f67deae..7098057 100644 --- a/tests/test_brew_advice.py +++ b/tests/test_brew_advice.py @@ -414,6 +414,14 @@ def test_the_reading_id_cannot_be_used_to_inject_a_query(repo): modified = repo.set_reading_advice(hostile, "INJECTED", True) assert modified == 0, f"{hostile!r} matched something" + # The helper itself must return something that is safe to put in a + # query - an empty list matches nothing. Returning None would raise + # "$in needs an array" if it were ever used without a guard. + from mongo.repository import _as_doc_id + + for hostile in [{"$ne": None}, {"$gt": ""}, ["a"], 123, None, ""]: + assert _as_doc_id(hostile) == [], f"{hostile!r} produced a usable id" + # And no document was touched. for doc in repo.readings.find({}): assert doc.get("advice") is None