codeant-ai-for-open-source[bot] commented on code in PR #33924:
URL: https://github.com/apache/superset/pull/33924#discussion_r3599746494
##########
docs/admin_docs/configuration/sql-templating.mdx:
##########
@@ -372,6 +372,41 @@ Here's a concrete example:
WHERE country_code = 'US'
```
+**Guest User Attributes**
+
+The `{{ get_guest_user_attribute('attribute_name') }}` macro returns a
specific attribute value from the guest user context.
+This is useful when working with embedded Superset where guest tokens can
contain custom attributes that need to be
+accessed in SQL queries.
+
+This macro only works when the current user is a guest user (authenticated via
guest token). If the current user is
+not a guest user, or if the specified attribute doesn't exist, the macro will
return `None` or the provided default value.
+
+If you have caching enabled in your Superset configuration, then by default
the attribute value will be used
+by Superset when calculating the cache key. A cache key is a unique identifier
that determines if there's a
+cache hit in the future and Superset can retrieve cached data.
+
+You can disable the inclusion of the attribute value in the calculation of the
+cache key by adding the following parameter to your Jinja code:
+
+```
+{{ get_guest_user_attribute('department', add_to_cache_keys=False) }}
+```
+
+You can also provide a default value if the attribute is not found:
+
+```
+{{ get_guest_user_attribute('region', default='US') }}
+```
+
+Here's a concrete example of using guest user attributes in a query:
+
+```sql
+SELECT *
+FROM sales_data
+WHERE region = '{{ get_guest_user_attribute("user_region", default="global")
}}'
+ AND department = '{{ get_guest_user_attribute("department") }}'
Review Comment:
**Suggestion:** This example directly interpolates
`get_guest_user_attribute()` into SQL string literals, but guest token
attributes can contain quote characters or attacker-controlled text and the
macro returns raw values. That can break SQL syntax or enable SQL injection in
templated queries. Add a security warning (similar to `url_param()`), and
document escaping/allowlisting before interpolation. [sql injection]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Embedded guest dashboard queries can become SQL injectable.
- ❌ Queries break if guest attributes contain quotes.
- ⚠️ Admin docs example encourages unsafe direct interpolation pattern.
- ⚠️ Multi-tenant attribute-based filters risk privilege escalation.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a guest token via the documented endpoint
`/api/v1/security/guest_token/` (see
docs at `docs/developer_docs/api.mdx:51` and OpenAPI schema at
`docs/static/resources/openapi.json:31214`), passing a `user` object with an
`attributes`
dict such as:
- `{"user": {"attributes": {"user_region": "US' OR 1=1 --", "department":
"sales"}}}`.
The SecurityManager stores this user payload verbatim in the JWT claims
(no escaping)
when minting the token in `superset/security/manager.py:19-49`, where
`create_guest_access_token()` sets `"user": user` in `claims`.
2. Use the admin docs example query in SQL Lab or in a virtual dataset:
- File: `docs/admin_docs/configuration/sql-templating.mdx`
- Lines: `404-407`
- Query:
```sql
SELECT *
FROM sales_data
WHERE region = '{{ get_guest_user_attribute("user_region",
default="global") }}'
AND department = '{{ get_guest_user_attribute("department") }}'
```
This matches the `existing_code` snippet at lines `406-407`.
3. When the query is executed, Superset’s Jinja processing creates a
`JinjaTemplateProcessor` (`superset/jinja_context.py:946-970`) and injects
the
`get_guest_user_attribute` macro into the template context in
`set_context()` at
`superset/jinja_context.py:962-1010`, where:
- `"get_guest_user_attribute": partial(safe_proxy,
extra_cache.get_guest_user_attribute)` (lines `1008-1010`).
- The `ExtraCache.get_guest_user_attribute()` implementation at
`superset/jinja_context.py:314-363`:
- Reads the guest token from `g.user.guest_token` (line `349`), which
was created
from the JWT claims.
- Pulls `user_attributes = token.get("user", {}).get("attributes") or
{}` (lines
`349-351`).
- Returns `result = user_attributes[attribute_name]` unchanged (lines
`356-363`)
after optional cache-key bookkeeping.
- `safe_proxy()` at `superset/jinja_context.py:669-691` only enforces
allowed JSON
types and re-serializes collections; it does not escape or sanitize
strings.
4. Because the template wraps the macro in single quotes (`'{{
get_guest_user_attribute("user_region", ...) }}'` at
`docs/admin_docs/configuration/sql-templating.mdx:406`), the raw attribute
value is
interpolated directly into a SQL string literal. With the malicious example
value `"US' OR
1=1 --"`:
- The rendered SQL sent to the database becomes:
```sql
WHERE region = 'US' OR 1=1 --'
AND department = 'sales'
```
- This is a classic SQL injection pattern (or at minimum malformed SQL)
caused by
unescaped, attacker-controlled text from the guest token being embedded
directly into
SQL literals.
- The existing admin docs already warn that other templating helpers like
`url_param()`
return untrusted values that must be validated/allowlisted before use in
SQL (see
`docs/admin_docs/configuration/sql-templating.mdx:331-343`), but no
analogous warning
is given for `get_guest_user_attribute`, despite it returning equally
raw, unescaped
data from the guest token.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5c68d9b1ba544ea9b87abac1a25f287c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5c68d9b1ba544ea9b87abac1a25f287c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/admin_docs/configuration/sql-templating.mdx
**Line:** 406:407
**Comment:**
*Sql Injection: This example directly interpolates
`get_guest_user_attribute()` into SQL string literals, but guest token
attributes can contain quote characters or attacker-controlled text and the
macro returns raw values. That can break SQL syntax or enable SQL injection in
templated queries. Add a security warning (similar to `url_param()`), and
document escaping/allowlisting before interpolation.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F33924&comment_hash=e8849c40ffbf4ad8fbc40333b64410af8e693f0c47ea4a36f9273fae4cc9cb5a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F33924&comment_hash=e8849c40ffbf4ad8fbc40333b64410af8e693f0c47ea4a36f9273fae4cc9cb5a&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
