Fix for #4139 - Secrets not being masked in pack config

CHANGELOG.rst

@@ -42,6 +42,18 @@ Changed
 Fixed
 ~~~~~
 
+* Fixed a bug where secrets in pack configs weren't being masked. Recently we
+  introduced support for nested objects and arrays. Secret parameters within these
+  nested objects and arrays were not being masked. The fix involves us fully
+  traversing deeply nested objects and arrays and masking out any variables
+  marked as secret. This means we now support pack config JSON schemas with
+  ``type: object`` and its corresponding ``parameters: {}`` stanza, along with
+  ``type: array`` and its corresponding ``items: {}`` stanza. We still do NOT
+  support JSON schema combinations that includes the ``anyOf``, ``allOf``,
+  ``oneOf``, and ``not`` keywords. (bug fix) #4139
+
+  Contributed by Nick Maludy (Encore Technologies).
+
 2.7.2 - May 16, 2018
 --------------------
 

st2client/tests/unit/test_inquiry.py

@@ -32,6 +32,11 @@
 def _randomize_inquiry_id(inquiry):
     newinquiry = copy.deepcopy(inquiry)
     newinquiry['id'] = str(uuid.uuid4())
+    # ID can't have '1440' in it, otherwise our `count()` fails
+    # when inspecting the inquiry list output for test:
+    # test_list_inquiries_limit()
+    while '1440' in newinquiry['id']:
+        newinquiry['id'] = str(uuid.uuid4())
     return newinquiry
 
 

st2common/st2common/models/db/execution.py

@@ -120,9 +120,14 @@ def mask_secrets(self, value):
                 # schema.
                 #
                 # To prevent leakage, we can just mask all response fields.
+                #
+                # Note: The 'string' type in secret_parameters doesn't matter,
+                #       it's just a placeholder to tell mask_secret_parameters()
+                #       that this parameter is indeed a secret parameter and to
+                #       mask it.
                 result['parameters']['response'] = mask_secret_parameters(
                     parameters=liveaction['parameters']['response'],
-                    secret_parameters=[p for p in liveaction['parameters']['response']]
+                    secret_parameters={p: 'string' for p in liveaction['parameters']['response']}
                 )
 
         # TODO(mierdin): This logic should be moved to the dedicated Inquiry

st2common/st2common/util/secrets.py

@@ -27,36 +27,146 @@
 
 def get_secret_parameters(parameters):
     """
-    Filter the provided parameters dict and return a list of parameter names which are marked as
-    secret.
+    Filter the provided parameters dict and return a dict of parameters which are marked as
+    secret. Every key in the dict is the parameter name and values are the parameter type:
+
+    >>> d = get_secret_parameters(params)
+    >>> d
+    {
+        "param_a": "string",
+        "param_b": "boolean",
+        "param_c": "integer"
+    }
+
+    If a paramter is a dictionary or a list, then the value will be a nested dictionary
+    containing information about that sub-object:
+
+    >>> d = get_secret_parameters(params)
+    >>> d
+    {
+        "param_dict": {
+            "nested_a": "boolean",
+            "nested_b": "string",
+        },
+        "param_list": {
+            "nested_dict: {
+              "param_c": "integer"
+            }
+        }
+    }
+
+    Note: in JSON Schema, we're assuming lists contain the same data type for every element
+
 
     :param parameters: Dictionary with runner or action parameters schema specification.
     :type parameters: ``dict``
 
     :rtype ``list``
     """
-    secret_parameters = [parameter for parameter, options in
-                         six.iteritems(parameters) if options.get('secret', False)]
+
+    secret_parameters = {}
+    parameters_type = parameters.get('type')
+    iterator = None
+    if parameters_type == 'object':
+        # if this is an object, then iterate over the properties within
+        # the object
+        # result = dict
+        iterator = six.iteritems(parameters.get('properties', {}))
+    elif parameters_type == 'array':
+        # if this is an array, then iterate over the items definition as a single
+        # property
+        # result = list
+        iterator = enumerate([parameters.get('items', {})])
+        secret_parameters = []
+    elif parameters_type in ['integer', 'number', 'boolean', 'null', 'string']:
+        # if this a "plain old datatype", then iterate over the properties set
+        # of the data type
+        # result = string (property type)
+        iterator = enumerate([parameters])
+    else:
+        # otherwise, assume we're in an object's properties definition
+        # this is the default case for the "root" level for schema specs.
+        # result = dict
+        iterator = six.iteritems(parameters)
+
+    # iterate over all of the parameters recursively
+    for parameter, options in iterator:
+        if not isinstance(options, dict):
+            continue
+
+        parameter_type = options.get('type')
+        if parameter_type in ['object', 'array']:
+            sub_params = get_secret_parameters(options)
+            if sub_params:
+                if isinstance(secret_parameters, list):
+                    secret_parameters.append(sub_params)
+                elif isinstance(secret_parameters, dict):
+                    secret_parameters[parameter] = sub_params
+                else:
+                    return sub_params
+        elif options.get('secret', False):
+            # if this parameter is secret, then add it our secret parameters
+            if isinstance(secret_parameters, list):
+                secret_parameters.append(parameter_type)
+            elif isinstance(secret_parameters, dict):
+                secret_parameters[parameter] = parameter_type
+            else:
+                return parameter_type
 
     return secret_parameters
 
 
-def mask_secret_parameters(parameters, secret_parameters):
+def mask_secret_parameters(parameters, secret_parameters, result=None):
     """
     Introspect the parameters dict and return a new dict with masked secret
     parameters.
-
     :param parameters: Parameters to process.
-    :type parameters: ``dict``
-
-    :param secret_parameters: List of parameter names which are secret.
-    :type secret_parameters: ``list``
+    :type parameters: ``dict`` or ``list`` or ``string``
+
+    :param secret_parameters: Dict of parameter names which are secret.
+                              The type must be the same type as ``parameters``
+                              (or at least behave in the same way),
+                              so that they can be traversed in the same way as
+                              recurse down into the structure.
+    :type secret_parameters: ``dict``
+
+    :param result: Deep copy of parameters so that parameters is not modified
+                   in place. Default = None, meaning this function will make a
+                   deep copy before starting.
+    :type result: ``dict`` or ``list`` or ``string``
     """
-    result = copy.deepcopy(parameters)
-
-    for parameter in secret_parameters:
-        if parameter in result:
-            result[parameter] = MASKED_ATTRIBUTE_VALUE
+    # how we iterate depends on what data type was passed in
+    iterator = None
+    is_dict = isinstance(secret_parameters, dict)
+    is_list = isinstance(secret_parameters, list)
+    if is_dict:
+        iterator = six.iteritems(secret_parameters)
+    elif is_list:
+        iterator = enumerate(secret_parameters)
+    else:
+        return MASKED_ATTRIBUTE_VALUE
+
+    # only create a deep copy of parameters on the first call
+    # all other recursive calls pass back referneces to this result object
+    # so we can reuse it, saving memory and CPU cycles
+    if result is None:
+        result = copy.deepcopy(parameters)
+
+    # iterate over the secret parameters
+    for secret_param, secret_sub_params in iterator:
+        if is_dict:
+            if secret_param in result:
+                result[secret_param] = mask_secret_parameters(parameters[secret_param],
+                                                              secret_sub_params,
+                                                              result=result[secret_param])
+        elif is_list:
+            # we're assuming lists contain the same data type for every element
+            for idx, value in enumerate(result):
+                result[idx] = mask_secret_parameters(parameters[idx],
+                                                     secret_sub_params,
+                                                     result=result[idx])
+        else:
+            result[secret_param] = MASKED_ATTRIBUTE_VALUE
 
     return result