INTPYTHON-827 Prevent Value strings and model update values from being interpreted as expressions

[WaVEV]

INTPYTHON-827

Summary

This PR fixes unsafe handling of $-prefixed values during save() and update() operations in the Django MongoDB backend.
Values like {"$concat": [...]} or ("$name", "-", "$name") were previously interpreted as MongoDB operators instead of being stored literally.
The patch ensures that all such values are wrapped with $literal when appropriate, preventing unintended expression or operator injection.

Changes in this PR

• Updated value compilation to wrap dict, tuple, string, and numeric values in $literal where needed.
• Update SQLUpdateCompiler to wrap any direct value (i.e. string, dict, tuple, etc).

Test Plan

• Added unit tests verifying that $-prefixed values are safely stored and not interpreted as expressions.
• Tested scenarios for dicts, tuples, and Value() wrappers.
• Verified that regular (non-$-prefixed) values are unaffected (basic unit test from Django)

Checklist

Checklist for Author

• Did you update the changelog (if necessary)?
• Is the intention of the code captured in relevant tests?
• If there are new TODOs, has a related JIRA ticket been created?

Checklist for Reviewer @Jibola @aclark4life @timgraham

• Does the title of the PR reference a JIRA Ticket?
• Do you fully understand the implementation?
• Have you checked for spelling & grammar errors?
• Is all relevant documentation (README or docstring) updated?

Focus Areas for Reviewer

Pay attention to the logic handling $literal wrapping in value compilation.

[timgraham]

Is it necessary to wrap all values? Are there problems with other types? Our logic in Value() only wraps list/int/str.

[WaVEV]

A unit test (test_update_dollar_prefix_in_value_expression) shows the issue. Because we could update passing an expression, and the expression could be Value("$update")

[timgraham]

What I meant: Is it necessary to wrap all direct values?

[WaVEV]

understood, 🤔 . Will try to make a test and try to inject something.

[WaVEV]

well, 🤔. maybe only string should be wrapped, not any kind of value.

EDIT
it is possible:

def test_update_dict(self):
        obj = Author.objects.create(name="foobar")
        obj.name = Value({"$concat": ["$name", "-", "$name"]})
        obj.save()
        obj.refresh_from_db()
        self.assertEqual(obj.name, {"$concat": ["$name", "-", "$name"]})

this tails fails and the name was update as foobar-foobar
I think we should scape any kind of literal

[timgraham]

That seems sufficient to me.

[WaVEV]

Another case:

def test_update_dictvalue(self):
        obj = Author.objects.create(name="foobar", data={})
        obj.data = {"$concat": ["$name", "-", "$name"]}
        obj.save()
        obj.refresh_from_db()
        self.assertEqual(obj.data, {"$concat": ["$name", "-", "$name"]})

If the field is a JSONField. Any kind of type that could be a MongoDB query, could be exploitable.
I think wrapping al values will bring us a safe net.

[timgraham]

Out of curiosity, you replaced $project with aggregate. Is there another place in aggregate besides $project where this could be an issue?

[WaVEV]

Yes, in place like $group or $match (for having or filter). Do we need a test for that?

[timgraham]

I don't think so, just wondering.

[timgraham]

I still question whether the $literal escaping is overly broad. Better safe than sorry, sure, but it decreases readability a bit and makes queries larger. But I'm not sure how to compare the tradeoffs between some more complicated logic (isinstance() CPU time) and leaving it as is. It seems if we did wrapping of only the types that Value() wraps, it should be safe, unless the logic in Value() is deficient. And are there any string values besides those that start with $ that could be problematic? Perhaps to be discussed in chat tomorrow.

[WaVEV]

🤔 I agree. Seeing $literal everywhere is annoying, but Value's resolver is probably covering the most common types used in queries. On the other hand, implementing the same escaping logic that Value uses is not a big deal.

[timgraham]

Comments to explain a few of my edits.