Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug where objects and arrays marked as secret weren't being masked #4236

Merged

Conversation

nmaludy
Copy link
Member

@nmaludy nmaludy commented Jul 10, 2018

Most likely due to the re-work of secret masking here #4140 objects and arrays that were marked with secret: true were not being masked. See #4235 for details on the symptoms.

This PR addresses this by looking for the secret property on object and array types. It then treats all data within the object or array as secret, returning a single masked value.

Example of the fixed version:

$ st2 run default.secret_objects secret_object='{"abc": "123"}' secret_array='[1, 2, 3]'
.
id: 5b44de76a814c004961c020e
status: succeeded
parameters: 
  secret_object: '********'
  secret_array: '********'
result: 
  failed: false
  return_code: 0
  stderr: ''
  stdout: ''
  succeeded: true

FYI, i create a unit test for this edge case so we can verify that it is not exposed in the future.

@nmaludy
Copy link
Member Author

nmaludy commented Jul 10, 2018

Closes #4235

@nmaludy
Copy link
Member Author

nmaludy commented Jul 10, 2018

@Kami i was able to find and fix the issue, please let me know if you have any comments

@Kami
Copy link
Member

Kami commented Jul 10, 2018

@nmaludy Thanks, will have a look.

@Kami Kami self-assigned this Jul 10, 2018
@@ -669,3 +701,39 @@ def test_mask_secret_parameters_nested_array_with_object(self):
]
}
self.assertEqual(expected, result)

def test_mask_secret_array(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also wouldn't hurt to add a test case for scenarios where we have an object which is marked "secret: true" at top level, but it also has a sub-schema with different items of which some have "secret: true" attribute. Same goes for array.

I assume in such scenarios, if top level "secret: true" attribute is present, this top level attribute should have a precedence (and we could even ignore / skip iterating over child object / array attributes).

@Kami Kami added this to the 2.8.1 milestone Jul 10, 2018
# is an `object` or `array`, and the user wants the full thing
# to be secret, that it is marked as secret.
if parameters.get('secret', False):
return parameters_type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 👍

That's also a more common scenario and it's much more performant :)

@Kami
Copy link
Member

Kami commented Jul 11, 2018

Thanks for good work on the tests 👍

@Kami Kami merged commit 1bf3300 into StackStorm:master Jul 11, 2018
@Kami Kami mentioned this pull request Jul 11, 2018
@nmaludy nmaludy deleted the hotfix/4235-mask-secret-objects-and-arrays branch July 11, 2018 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants