rusackas commented on code in PR #43474:
URL: https://github.com/apache/superset/pull/43474#discussion_r3856892010
##########
superset/semantic_layers/api.py:
##########
@@ -81,10 +81,51 @@
logger = logging.getLogger(__name__)
+def _mask_configuration(layer: SemanticLayer, config: dict[str, Any]) ->
dict[str, Any]:
+ """
+ Redact configuration values the connector's schema marks as write-only.
+
+ A connector publishes its configuration shape via
``get_configuration_schema``;
+ a property with ``"writeOnly": true`` (the standard JSON Schema way of
+ marking a field that's set but never echoed back, e.g. a password or API
+ key) is replaced with ``PASSWORD_MASK`` here rather than returned in the
+ clear.
+ """
+ cls = registry.get(layer.type)
+ if not cls:
Review Comment:
Good catch, missing connector fell through to the raw config.
`_mask_configuration` now fails closed there too, masking everything when the
type isn't registered or its schema can't load.
##########
superset/semantic_layers/api.py:
##########
@@ -81,10 +81,51 @@
logger = logging.getLogger(__name__)
+def _mask_configuration(layer: SemanticLayer, config: dict[str, Any]) ->
dict[str, Any]:
+ """
+ Redact configuration values the connector's schema marks as write-only.
+
+ A connector publishes its configuration shape via
``get_configuration_schema``;
+ a property with ``"writeOnly": true`` (the standard JSON Schema way of
+ marking a field that's set but never echoed back, e.g. a password or API
+ key) is replaced with ``PASSWORD_MASK`` here rather than returned in the
+ clear.
+ """
+ cls = registry.get(layer.type)
+ if not cls:
+ return config
+
+ try:
+ schema = cls.get_configuration_schema()
+ except Exception: # pylint: disable=broad-except
+ # We can't tell which fields are secret, so fail closed: mask every
+ # truthy value rather than risk echoing a credential back in the clear.
+ logger.warning(
+ "Failed to load configuration schema for semantic layer type %s; "
+ "masking all configuration values.",
+ layer.type,
+ )
+ return {key: PASSWORD_MASK if value else value for key, value in
config.items()}
+
+ secret_keys = {
+ key
+ for key, prop in schema.get("properties", {}).items()
+ if isinstance(prop, dict) and prop.get("writeOnly")
+ }
+ if not secret_keys:
+ return config
+
+ return {
+ key: PASSWORD_MASK if key in secret_keys and value else value
Review Comment:
You're right, a name-only PUT would round-trip the mask straight into
storage. UpdateSemanticLayerCommand now swaps masked write-only fields back to
the stored value before validating and saving.
--
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]
