sha174n commented on code in PR #45452:
URL: https://github.com/apache/superset/pull/45452#discussion_r4329171455


##########
superset/security/guest_token.py:
##########
@@ -138,6 +139,44 @@ class GuestTokenUser(TypedDict, total=False):
     attributes: Optional[dict[str, Any]]
 
 
+# Upper bound for the identity strings carried in a guest token's ``user``
+# claim, a generous bound for a display identity.
+GUEST_TOKEN_IDENTITY_MAX_LENGTH = 255
+GUEST_TOKEN_IDENTITY_FIELDS = ("username", "first_name", "last_name")
+
+
+def validate_guest_token_identity(user: Optional[GuestTokenUser]) -> None:
+    """Validate the identity strings carried in a guest token's ``user`` claim.
+
+    ``username``/``first_name``/``last_name`` are supplied by the host
+    application that mints the token and later surface in logging, request
+    context, and templated query contexts. Constrain them to bounded,
+    single-line strings where they enter the system, consistent with how other
+    request-supplied strings are validated: a display identity has no reason to
+    carry control characters (e.g. an embedded newline) or an unbounded value.
+
+    A missing ``user`` claim is not rejected here; that check belongs to the
+    point where the token is consumed.
+
+    :param user: the ``user`` claim from a guest token
+    :raises ValueError: if a provided identity field is not a valid string
+    """
+    if user is None:
+        return
+    if not isinstance(user, dict):
+        raise ValueError("Guest token user must be an object")
+    for field in GUEST_TOKEN_IDENTITY_FIELDS:
+        value = user.get(field)
+        if value is None:
+            continue
+        if not isinstance(value, str):
+            raise ValueError(f"Guest token {field} must be a string")
+        if len(value) > GUEST_TOKEN_IDENTITY_MAX_LENGTH:
+            raise ValueError(f"Guest token {field} exceeds the maximum length")
+        if CONTROL_CHARS_RE.search(value):

Review Comment:
   Now rejects U+2028/U+2029 too (single regex covering C0/C1 plus both 
separators), with tests, in b17ea27c16.



##########
superset/utils/file.py:
##########
@@ -22,7 +22,7 @@
 # All C0 (U+0000–U+001F) and C1 (U+007F–U+009F) control characters.
 # Stripping every control char (including tab, LF, CR) keeps titles safe for
 # SMTP headers, Content-Disposition filenames, and headless-browser 
document.title.
-_CONTROL_CHARS_RE = re.compile(r"[\x00-\x1f\x7f-\x9f]")
+CONTROL_CHARS_RE = re.compile(r"[\x00-\x1f\x7f-\x9f]")

Review Comment:
   In b17ea27c16 the guest token check uses its own regex, so the file.py one 
is back to private and filename-only, with no auth caller to loosen.



-- 
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]

Reply via email to