gabotorresruiz commented on code in PR #43202:
URL: https://github.com/apache/superset/pull/43202#discussion_r3806930187


##########
superset/mcp_service/utils/sanitization.py:
##########
@@ -35,82 +35,16 @@
 
 import nh3
 
-LLM_CONTEXT_OPEN_DELIMITER = "<UNTRUSTED-CONTENT>"
-LLM_CONTEXT_CLOSE_DELIMITER = "</UNTRUSTED-CONTENT>"
-LLM_CONTEXT_ESCAPED_OPEN_DELIMITER = "[ESCAPED-UNTRUSTED-CONTENT-OPEN]"
-LLM_CONTEXT_ESCAPED_CLOSE_DELIMITER = "[ESCAPED-UNTRUSTED-CONTENT-CLOSE]"
-LLM_CONTEXT_EXCLUDED_FIELD_NAMES = frozenset(
-    {
-        "cache_key",
-        "database",
-        "database_name",
-        "schema",
-        "schema_name",
-        "slug",
-        "url",
-        "urls",
-        "uuid",
-    }
-)
-
-
-def _normalize_field_name(field_name: str) -> str:
-    """Normalize a field name for exclusion matching."""
-    return field_name.strip().lower().replace("-", "_")
-
-
-def _escape_llm_context_delimiters(value: str) -> str:
-    """Escape delimiter tokens without wrapping the value."""
-    return value.replace(
-        LLM_CONTEXT_OPEN_DELIMITER,
-        LLM_CONTEXT_ESCAPED_OPEN_DELIMITER,
-    ).replace(
-        LLM_CONTEXT_CLOSE_DELIMITER,
-        LLM_CONTEXT_ESCAPED_CLOSE_DELIMITER,
-    )
-
-
-def _escape_llm_context_dict_key(key: Any) -> Any:
-    """Escape delimiter tokens in string dict keys."""
-    if isinstance(key, str):
-        return _escape_llm_context_delimiters(key)
-    return key
-
 
 def escape_llm_context_delimiters(value: Any) -> Any:
-    """Escape delimiter tokens in operational values that should not be 
wrapped."""
-    if isinstance(value, str):
-        return _escape_llm_context_delimiters(value)
-    if isinstance(value, dict):
-        return {
-            _escape_llm_context_dict_key(key): escape_llm_context_delimiters(
-                nested_value
-            )
-            for key, nested_value in value.items()
-        }
-    if isinstance(value, list):
-        return [escape_llm_context_delimiters(item) for item in value]
-    if isinstance(value, tuple):
-        return tuple(escape_llm_context_delimiters(item) for item in value)
-    return value
-
-
-def _wrap_llm_context_string(value: str) -> str:
-    """Wrap an untrusted string with explicit LLM-context delimiters."""
-    wrapped_prefix = f"{LLM_CONTEXT_OPEN_DELIMITER}\n"
-    wrapped_suffix = f"\n{LLM_CONTEXT_CLOSE_DELIMITER}"
-    if value.startswith(wrapped_prefix) and value.endswith(wrapped_suffix):
-        inner_value = value[len(wrapped_prefix) : -len(wrapped_suffix)]
-        return (
-            f"{wrapped_prefix}"
-            f"{_escape_llm_context_delimiters(inner_value)}"
-            f"{wrapped_suffix}"
-        )
+    """Return an MCP result value without changing application data.
 
-    escaped_value = _escape_llm_context_delimiters(value)
-    return (
-        
f"{LLM_CONTEXT_OPEN_DELIMITER}\n{escaped_value}\n{LLM_CONTEXT_CLOSE_DELIMITER}"
-    )
+    This compatibility helper is retained while callers migrate away from the
+    former in-band delimiter convention. Trust classification belongs in
+    protocol metadata or in a client-owned presentation layer; changing a
+    domain value makes read-modify-write flows persist presentation markup.
+    """
+    return value

Review Comment:
   Not a blocker. With every in-repo caller removed in this PR, 
sanitize_for_llm_context and escape_llm_context_delimiters are identity 
functions whose names still promise sanitization, and the tool-side wrappers 
(wrap_sql_adhoc_metrics, _sanitize_chart_data_for_llm_context, 
_sanitize_chart_preview_for_llm_context, 
_sanitize_sql_lab_response_for_llm_context) are no-ops still invoked on the 
request path. A future contributor who imports one of these believing it adds 
protection gets silent identity behavior. Would you consider deleting them and 
their call sites, either here or in a fast follow-up, and letting 
test_production_code_defines_no_fixed_in_band_marker carry the invariant? If 
they are kept deliberately for out-of-tree importers, a DeprecationWarning in 
the two public helpers would make the contract explicit. Happy to help with the 
follow-up if you prefer to keep this diff mechanical.



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