Bug description
There is a regression in the PR fix(Jinja): Extra cache keys to consider vars with set #30549 . I mentioned it as a comment within the PR but I guess since it's merged I'm filing a proper bug report.
This PR breaks are existing usage of RLS (thank heavens one for our campus users notified us 😅 ). This is our input string:
campus_id IN UNNEST( {{ cache_key_wrapper(current_user_campus_ids()) }} )
which doesn't match using the pattern : cache_key_wrapper\([^()]*\),
but does using the old one (v4.0.2): cache_key_wrapper\(.*\).
The function we use looks as follows:
def current_user_campus_ids() -> list[int]:
"""
Get the list of campus ids associated with the current user
"""
user: FortyTwoUser = g.user
return user.campus_ids()
and is populated using the oauth2 jwt from keycloak.
Is the only workaround to monkey patch the jinja ExtraCache processor to add our macro within? How is the cache_key_wrapper() to be used if it can't invoke another function?
Screenshots/recordings
No response
Superset version
4.1.2
Python version
3.10
Node version
I don't know
Browser
Chrome
Additional context
No response
Checklist
- [x] I have searched Superset docs and Slack and didn't find a solution to my problem.
- [x] I have searched the GitHub issue tracker and didn't find a similar bug report.
- [x] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
Comment From: dosubot[bot]
This regression is due to a change in the regex used by Superset to detect cache key wrappers in Jinja templates—it now only allows simple, non-nested expressions like cache_key_wrapper(foo) and blocks nested calls like cache_key_wrapper(current_user_campus_ids()) that worked in previous versions. This breaks use cases like yours for RLS and dynamic user-based cache keys. There’s currently no official workaround or extensibility for this in the code or docs, and no open issues or discussions about it in the repo.
Monkey patching the Jinja ExtraCache processor is the only available workaround if you need to restore the previous behavior, since the regex restriction is hardcoded and there’s no config or plugin hook for extending cache key computation. If you go this route, be aware it’s an unsupported hack and could break with future updates. Otherwise, you’d need to refactor your templates to avoid nested calls, but that may not be possible for your use case.
For reference, here’s the relevant implementation and restriction in the code:
cache_key_wrapper and regex in ExtraCache
And the intended usage in the docs:
SQL templating and cache_key_wrapper
If you need to keep your current pattern, monkey patching is your only option until the project provides a more flexible or documented solution.
To reply, just mention @dosu.
How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other 
Comment From: neuromantik33
@dosu Ok that is great but according to the docs you mentioned :
Explicitly Including Values in Cache Key The {{ cache_key_wrapper() }} function explicitly instructs Superset to add a value to the accumulated list of values used in the calculation of the cache key. This function is only needed when you want to wrap your own custom function return values in the cache key.
If I can't include a function in the cache_key_wrapper what exactly is it wrapping? I don't know how to call a function without including parentheses unless it's a property accessor. Can you at least agree that the regexp for cache_key_wrapper should be a little more lenient?
Comment From: dosubot[bot]
You're right—there's a disconnect between the docs and the current implementation. The docs say cache_key_wrapper() is for wrapping custom function return values, but the regex in the code now only allows simple, non-nested expressions (like a single variable or property accessor) inside cache_key_wrapper, not function calls with parentheses or any nested expressions. So, in practice, you can't use it to wrap the result of a function call like current_user_campus_ids() anymore—only something like cache_key_wrapper(variable) will work, which is pretty limiting for dynamic use cases like yours.
I agree that the regex should be more lenient to match the documented intent and real-world needs. Right now, the only way around this is to monkey patch the ExtraCache processor or revert the regex change, since there's no config or extension point for this part of the logic. No relevant discussions or plans to relax the regex have come up in the repo or docs so far.
If you need this flexibility, advocating for a change to the regex or opening a discussion about restoring support for nested calls would be reasonable next steps. For now, the restriction is hardcoded and not aligned with the documentation or common Jinja usage patterns.
Reference: implementation and regex restriction
Reference: docs on intended usage
To reply, just mention @dosu.
How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other
Comment From: rusackas
Pinging @geido (the author) and @villebro (the reviewer) of the PR in question, for good measure.
Comment From: neuromantik33
Any news on this issue? This is a pretty big security regression IMHO, I guess I'll have to resort to monkeypatching the cache_key_wrapper..