diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ac20163a9e..cdecd36345 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -78,6 +78,11 @@ Added Contributed by @Kami. +* Mask secrets in output of an action execution in the API if the action has an output schema + defined and one or more output parameters are marked as secret. #5250 + + Contributed by @mahesh-orch. + Changed ~~~~~~~ diff --git a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py index d1c0c249ab..dcf06c8923 100644 --- a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py +++ b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py @@ -29,11 +29,14 @@ tests_config.parse_args() +from python_runner import python_runner from tests.unit import base from st2common.bootstrap import actionsregistrar from st2common.bootstrap import runnersregistrar from st2common.constants import action as ac_const +from st2common.constants import secrets as secrets_const +from st2common.models.api import execution as ex_api_models from st2common.models.db import liveaction as lv_db_models from st2common.persistence import execution as ex_db_access from st2common.persistence import liveaction as lv_db_access @@ -58,6 +61,23 @@ st2tests.fixturesloader.get_fixtures_packs_base_path() + "/core", ] +TEST_1 = "xyz" +TEST_2 = "床前明月光 疑是地上霜 舉頭望明月 低頭思故鄉" +MOCK_PY_RESULT_1 = { + "stderr": "", + "stdout": "", + "result": {"k2": TEST_1}, + "exit_code": 0, +} +MOCK_PY_RESULT_2 = { + "stderr": "", + "stdout": "", + "result": {"k2": TEST_2}, + "exit_code": 0, +} +MOCK_PY_OUTPUT_1 = (ac_const.LIVEACTION_STATUS_SUCCEEDED, MOCK_PY_RESULT_1, None) +MOCK_PY_OUTPUT_2 = (ac_const.LIVEACTION_STATUS_SUCCEEDED, MOCK_PY_RESULT_2, None) + @mock.patch.object( publishers.CUDPublisher, "publish_update", mock.MagicMock(return_value=None) @@ -172,10 +192,28 @@ def assert_data_flow(self, data): # Manually handle action execution completion. wf_svc.handle_action_execution_completion(tk3_ac_ex_db) - # Assert task3 succeeded and workflow is completed. + # Assert task3 succeeded and workflow is still running. tk3_ex_db = wf_db_access.TaskExecution.get_by_id(tk3_ex_db.id) self.assertEqual(tk3_ex_db.status, wf_statuses.SUCCEEDED) wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + self.assertEqual(wf_ex_db.status, wf_statuses.RUNNING) + + # Assert task4 is already completed. + query_filters = {"workflow_execution": str(wf_ex_db.id), "task_id": "task4"} + tk4_ex_db = wf_db_access.TaskExecution.query(**query_filters)[0] + tk4_ac_ex_db = ex_db_access.ActionExecution.query( + task_execution=str(tk4_ex_db.id) + )[0] + tk4_lv_ac_db = lv_db_access.LiveAction.get_by_id(tk4_ac_ex_db.liveaction["id"]) + self.assertEqual(tk4_lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + # Manually handle action execution completion. + wf_svc.handle_action_execution_completion(tk4_ac_ex_db) + + # Assert task4 succeeded and workflow is completed. + tk4_ex_db = wf_db_access.TaskExecution.get_by_id(tk4_ex_db.id) + self.assertEqual(tk4_ex_db.status, wf_statuses.SUCCEEDED) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) self.assertEqual(wf_ex_db.status, wf_statuses.SUCCEEDED) lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) self.assertEqual(lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) @@ -183,9 +221,13 @@ def assert_data_flow(self, data): self.assertEqual(ac_ex_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) # Check workflow output. + expected_value = wf_input["a1"] if six.PY3 else wf_input["a1"].decode("utf-8") + expected_output = { - "a5": wf_input["a1"] if six.PY3 else wf_input["a1"].decode("utf-8"), - "b5": wf_input["a1"] if six.PY3 else wf_input["a1"].decode("utf-8"), + "a6": expected_value, + "b6": expected_value, + "a7": expected_value, + "b7": expected_value, } self.assertDictEqual(wf_ex_db.output, expected_output) @@ -196,8 +238,34 @@ def assert_data_flow(self, data): self.assertDictEqual(lv_ac_db.result, expected_result) self.assertDictEqual(ac_ex_db.result, expected_result) + # Assert expected output on conversion to API model + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model( + ac_ex_db, mask_secrets=True + ) + + expected_masked_output = { + "a6": expected_value, + "b6": expected_value, + "a7": expected_value, + "b7": secrets_const.MASKED_ATTRIBUTE_VALUE, + } + + expected_masked_result = {"output": expected_masked_output} + + self.assertDictEqual(ac_ex_api.result, expected_masked_result) + + @mock.patch.object( + python_runner.PythonRunner, + "run", + mock.MagicMock(return_value=MOCK_PY_OUTPUT_1), + ) def test_string(self): - self.assert_data_flow("xyz") + self.assert_data_flow(TEST_1) + @mock.patch.object( + python_runner.PythonRunner, + "run", + mock.MagicMock(return_value=MOCK_PY_OUTPUT_2), + ) def test_unicode_string(self): - self.assert_data_flow("床前明月光 疑是地上霜 舉頭望明月 低頭思故鄉") + self.assert_data_flow(TEST_2) diff --git a/st2actions/tests/unit/test_output_schema.py b/st2actions/tests/unit/test_output_schema.py new file mode 100644 index 0000000000..330aa46860 --- /dev/null +++ b/st2actions/tests/unit/test_output_schema.py @@ -0,0 +1,201 @@ +# Copyright 2021 The StackStorm Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import + +import mock + +from http_runner import http_runner +from python_runner import python_runner +from orquesta_runner import orquesta_runner + +import st2tests + +import st2tests.config as tests_config + +tests_config.parse_args() + +from st2common.bootstrap import actionsregistrar +from st2common.bootstrap import runnersregistrar +from st2common.constants import action as ac_const +from st2common.constants import secrets as secrets_const +from st2common.models.api import execution as ex_api_models +from st2common.models.db import liveaction as lv_db_models +from st2common.services import action as action_service +from st2common.transport import liveaction as lv_ac_xport +from st2common.transport import publishers +from st2tests.mocks import liveaction as mock_lv_ac_xport + + +PACKS = [ + st2tests.fixturesloader.get_fixtures_packs_base_path() + "/dummy_pack_1", + st2tests.fixturesloader.get_fixtures_packs_base_path() + "/orquesta_tests", +] + +MOCK_PYTHON_ACTION_RESULT = { + "stderr": "", + "stdout": "", + "result": {"k1": "foobar", "k2": "shhhh!"}, + "exit_code": 0, +} + +MOCK_PYTHON_RUNNER_OUTPUT = ( + ac_const.LIVEACTION_STATUS_SUCCEEDED, + MOCK_PYTHON_ACTION_RESULT, + None, +) + +MOCK_HTTP_ACTION_RESULT = { + "status_code": 200, + "body": {"k1": "foobar", "k2": "shhhh!"}, +} + +MOCK_HTTP_RUNNER_OUTPUT = ( + ac_const.LIVEACTION_STATUS_SUCCEEDED, + MOCK_HTTP_ACTION_RESULT, + None, +) + +MOCK_ORQUESTA_ACTION_RESULT = { + "errors": [], + "output": {"a6": "foobar", "b6": "foobar", "a7": "foobar", "b7": "shhhh!"}, +} + +MOCK_ORQUESTA_RUNNER_OUTPUT = ( + ac_const.LIVEACTION_STATUS_SUCCEEDED, + MOCK_ORQUESTA_ACTION_RESULT, + None, +) + + +@mock.patch.object( + publishers.CUDPublisher, "publish_update", mock.MagicMock(return_value=None) +) +@mock.patch.object( + publishers.CUDPublisher, + "publish_create", + mock.MagicMock(side_effect=mock_lv_ac_xport.MockLiveActionPublisher.publish_create), +) +@mock.patch.object( + lv_ac_xport.LiveActionPublisher, + "publish_state", + mock.MagicMock(side_effect=mock_lv_ac_xport.MockLiveActionPublisher.publish_state), +) +class ActionExecutionOutputSchemaTest(st2tests.ExecutionDbTestCase): + @classmethod + def setUpClass(cls): + super(ActionExecutionOutputSchemaTest, cls).setUpClass() + + # Register runners. + runnersregistrar.register_runners() + + # Register test pack(s). + actions_registrar = actionsregistrar.ActionsRegistrar( + use_pack_cache=False, fail_on_failure=True + ) + + for pack in PACKS: + actions_registrar.register_from_pack(pack) + + @mock.patch.object( + python_runner.PythonRunner, + "run", + mock.MagicMock(return_value=MOCK_PYTHON_RUNNER_OUTPUT), + ) + def test_python_action(self): + # Execute a python action with output schema and secret + lv_ac_db = lv_db_models.LiveActionDB(action="dummy_pack_1.my_py_action") + lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) + ac_ex_db = self._wait_on_ac_ex_status( + ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED + ) + + # Assert expected output written to the database + expected_output = {"k1": "foobar", "k2": "shhhh!"} + self.assertDictEqual(ac_ex_db.result["result"], expected_output) + + # Assert expected output on conversion to API model + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model( + ac_ex_db, mask_secrets=True + ) + expected_masked_output = { + "k1": "foobar", + "k2": secrets_const.MASKED_ATTRIBUTE_VALUE, + } + self.assertDictEqual(ac_ex_api.result["result"], expected_masked_output) + + @mock.patch.object( + http_runner.HttpRunner, + "run", + mock.MagicMock(return_value=MOCK_HTTP_RUNNER_OUTPUT), + ) + def test_http_action(self): + # Execute a http action with output schema and secret + lv_ac_db = lv_db_models.LiveActionDB(action="dummy_pack_1.my_http_action") + lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) + ac_ex_db = self._wait_on_ac_ex_status( + ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED + ) + + # Assert expected output written to the database + expected_output = {"k1": "foobar", "k2": "shhhh!"} + self.assertDictEqual(ac_ex_db.result["body"], expected_output) + + # Assert expected output on conversion to API model + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model( + ac_ex_db, mask_secrets=True + ) + expected_masked_output = { + "k1": "foobar", + "k2": secrets_const.MASKED_ATTRIBUTE_VALUE, + } + self.assertDictEqual(ac_ex_api.result["body"], expected_masked_output) + + @mock.patch.object( + orquesta_runner.OrquestaRunner, + "run", + mock.MagicMock(return_value=MOCK_ORQUESTA_RUNNER_OUTPUT), + ) + def test_orquesta_action(self): + wf_input = "foobar" + + # Execute an orquesta action with output schema and secret + lv_ac_db = lv_db_models.LiveActionDB( + action="orquesta_tests.data-flow", parameters={"a1": wf_input} + ) + lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) + ac_ex_db = self._wait_on_ac_ex_status( + ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED + ) + + # Assert expected output written to the database + expected_output = { + "a6": wf_input, + "b6": wf_input, + "a7": wf_input, + "b7": "shhhh!", + } + self.assertDictEqual(ac_ex_db.result["output"], expected_output) + + # Assert expected output on conversion to API model + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model( + ac_ex_db, mask_secrets=True + ) + expected_masked_output = { + "a6": wf_input, + "b6": wf_input, + "a7": wf_input, + "b7": secrets_const.MASKED_ATTRIBUTE_VALUE, + } + self.assertDictEqual(ac_ex_api.result["output"], expected_masked_output) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index f471889fb7..cc20c61f52 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -86,7 +86,9 @@ class ActionExecutionsControllerMixin(BaseRestControllerMixin): # parameters mandatory_include_fields_retrieve = [ "action.parameters", + "action.output_schema", "runner.runner_parameters", + "runner.output_key", "parameters", # Attributes below are mandatory for RBAC installations "action.pack", @@ -276,6 +278,7 @@ def _get_children( ) mask_secrets = self._get_mask_secrets(requester_user, show_secrets=show_secrets) + return [ self.model.from_model(descendant, mask_secrets=mask_secrets) for descendant in descendants @@ -309,9 +312,19 @@ def get_one( :rtype: ``list`` """ + if not requester_user: + requester_user = UserDB(name=cfg.CONF.system_user.user) + + from_model_kwargs = { + "mask_secrets": self._get_mask_secrets( + requester_user, show_secrets=show_secrets + ) + } + execution_db = self._get_one_by_id( id=id, requester_user=requester_user, + from_model_kwargs=from_model_kwargs, permission_type=PermissionType.EXECUTION_VIEW, ) id = str(execution_db.id) @@ -468,6 +481,7 @@ def get_one( output_format="raw", existing_only=False, requester_user=None, + show_secrets=False, ): # Special case for id == "last" if id == "last": @@ -478,9 +492,19 @@ def get_one( id = str(execution_db.id) + if not requester_user: + requester_user = UserDB(name=cfg.CONF.system_user.user) + + from_model_kwargs = { + "mask_secrets": self._get_mask_secrets( + requester_user, show_secrets=show_secrets + ) + } + execution_db = self._get_one_by_id( id=id, requester_user=requester_user, + from_model_kwargs=from_model_kwargs, permission_type=PermissionType.EXECUTION_VIEW, ) execution_id = str(execution_db.id) diff --git a/st2api/tests/unit/controllers/v1/test_executions.py b/st2api/tests/unit/controllers/v1/test_executions.py index 00648c5b5c..5afcdbaeda 100644 --- a/st2api/tests/unit/controllers/v1/test_executions.py +++ b/st2api/tests/unit/controllers/v1/test_executions.py @@ -171,6 +171,34 @@ }, } +ACTION_WITH_OUTPUT_SCHEMA_WITH_SECRET_PARAMS = { + "name": "st2.dummy.action_with_output_schema_secret_param", + "description": "An action that contains output_schema with secret parameters", + "enabled": True, + "entry_point": "/tmp/test/action_with_output_schema_secret_param.py", + "pack": "starterpack", + "runner_type": "python-script", + "parameters": {}, + "output_schema": { + "secret_param_1": {"type": "string", "required": True, "secret": True}, + "secret_param_2": {"type": "string", "required": True, "secret": True}, + }, +} + +ACTION_WITH_OUTPUT_SCHEMA_WITHOUT_SECRET_PARAMS = { + "name": "st2.dummy.action_with_output_schema_without_secret_params", + "description": "An action that contains output_schema without secret parameters", + "enabled": True, + "entry_point": "/tmp/test/action_with_output_schema_without_secret_params.py", + "pack": "starterpack", + "runner_type": "python-script", + "parameters": {}, + "output_schema": { + "non_secret_param_1": {"type": "string", "required": True}, + "non_secret_param_2": {"type": "string", "required": True}, + }, +} + LIVE_ACTION_1 = { "action": "sixpack.st2.dummy.action1", "parameters": { @@ -252,6 +280,16 @@ "action": "sixpack.st2.dummy.action1", } +LIVE_ACTION_WITH_OUTPUT_SCHEMA_SECRET_PARAM = { + "parameters": {}, + "action": "starterpack.st2.dummy.action_with_output_schema_secret_param", +} + +LIVE_ACTION_WITH_OUTPUT_SCHEMA_WITHOUT_SECRET_PARAM = { + "parameters": {}, + "action": "starterpack.st2.dummy.action_with_output_schema_without_secret_params", +} + # Do not add parameters to this. There are tests that will test first without params, # then make a copy with params. LIVE_ACTION_DEFAULT_TEMPLATE = { @@ -327,6 +365,22 @@ def setUpClass(cls): post_resp = cls.app.post_json("/v1/actions", cls.action_decrypt_secret_param) cls.action_decrypt_secret_param["id"] = post_resp.json["id"] + cls.action_with_output_schema_secret_param = copy.deepcopy( + ACTION_WITH_OUTPUT_SCHEMA_WITH_SECRET_PARAMS + ) + post_resp = cls.app.post_json( + "/v1/actions", cls.action_with_output_schema_secret_param + ) + cls.action_with_output_schema_secret_param["id"] = post_resp.json["id"] + + cls.action_with_output_schema_without_secret_params = copy.deepcopy( + ACTION_WITH_OUTPUT_SCHEMA_WITHOUT_SECRET_PARAMS + ) + post_resp = cls.app.post_json( + "/v1/actions", cls.action_with_output_schema_without_secret_params + ) + cls.action_with_output_schema_without_secret_params["id"] = post_resp.json["id"] + @classmethod def tearDownClass(cls): cls.app.delete("/v1/actions/%s" % cls.action1["id"]) @@ -336,6 +390,12 @@ def tearDownClass(cls): cls.app.delete("/v1/actions/%s" % cls.action_inquiry["id"]) cls.app.delete("/v1/actions/%s" % cls.action_template["id"]) cls.app.delete("/v1/actions/%s" % cls.action_decrypt["id"]) + cls.app.delete( + "/v1/actions/%s" % cls.action_with_output_schema_secret_param["id"] + ) + cls.app.delete( + "/v1/actions/%s" % cls.action_with_output_schema_without_secret_params["id"] + ) super(BaseActionExecutionControllerTestCase, cls).tearDownClass() def test_get_one(self): @@ -1786,6 +1846,80 @@ def test_get_single_include_attributes_and_secret_parameters(self): in resp.json["faultstring"] ) + def test_get_one_with_masked_secrets_in_output_schema(self): + """ + Test that the parameters marked secret as true in output schema are masked in + GET API of action execution. + """ + + post_resp = self._do_post(LIVE_ACTION_WITH_OUTPUT_SCHEMA_SECRET_PARAM) + actionexecution_id = self._get_actionexecution_id(post_resp) + + updates = { + "status": "succeeded", + "result": { + "exit_code": 0, + "stderr": "", + "stdout": "", + "result": { + "secret_param_1": "foo", + "secret_param_2": "bar", + }, + }, + } + + put_resp = self._do_put(actionexecution_id, updates) + self.assertEqual(put_resp.status_int, 200) + get_resp = self._do_get_one(actionexecution_id) + self.assertEqual(get_resp.status_int, 200) + self.assertEqual(self._get_actionexecution_id(get_resp), actionexecution_id) + + expected_result_in_get_resp = { + "secret_param_1": MASKED_ATTRIBUTE_VALUE, + "secret_param_2": MASKED_ATTRIBUTE_VALUE, + } + + self.assertDictEqual( + get_resp.json["result"]["result"], expected_result_in_get_resp + ) + + def test_get_one_without_masked_secrets_in_output_schema(self): + """ + Test that the parameters not marked secret as true in output schema are not masked + in GET API of action execution. + """ + + post_resp = self._do_post(LIVE_ACTION_WITH_OUTPUT_SCHEMA_WITHOUT_SECRET_PARAM) + actionexecution_id = self._get_actionexecution_id(post_resp) + + updates = { + "status": "succeeded", + "result": { + "exit_code": 0, + "stderr": "", + "stdout": "", + "result": { + "non_secret_param_1": "abc", + "non_secret_param_2": "xyz", + }, + }, + } + + put_resp = self._do_put(actionexecution_id, updates) + self.assertEqual(put_resp.status_int, 200) + get_resp = self._do_get_one(actionexecution_id) + self.assertEqual(get_resp.status_int, 200) + self.assertEqual(self._get_actionexecution_id(get_resp), actionexecution_id) + + expected_result_in_get_resp = { + "non_secret_param_1": "abc", + "non_secret_param_2": "xyz", + } + + self.assertDictEqual( + get_resp.json["result"]["result"], expected_result_in_get_resp + ) + def _insert_mock_models(self): execution_1_id = self._get_actionexecution_id(self._do_post(LIVE_ACTION_1)) execution_2_id = self._get_actionexecution_id(self._do_post(LIVE_ACTION_2)) diff --git a/st2api/tests/unit/controllers/v1/test_packs.py b/st2api/tests/unit/controllers/v1/test_packs.py index 111101eb65..0af29407cd 100644 --- a/st2api/tests/unit/controllers/v1/test_packs.py +++ b/st2api/tests/unit/controllers/v1/test_packs.py @@ -616,7 +616,7 @@ def test_packs_register_endpoint(self, mock_get_packs): self.assertEqual(resp.status_int, 200) # 13 real plus 1 mock runner - self.assertEqual(resp.json, {"actions": 1, "runners": 14}) + self.assertEqual(resp.json, {"actions": 3, "runners": 14}) # Verify that plural name form also works resp = self.app.post_json( @@ -625,7 +625,7 @@ def test_packs_register_endpoint(self, mock_get_packs): self.assertEqual(resp.status_int, 200) # 13 real plus 1 mock runner - self.assertEqual(resp.json, {"actions": 1, "runners": 14}) + self.assertEqual(resp.json, {"actions": 3, "runners": 14}) # Register single resource from a single pack specified multiple times - verify that # resources from the same pack are only registered once @@ -640,7 +640,7 @@ def test_packs_register_endpoint(self, mock_get_packs): self.assertEqual(resp.status_int, 200) # 13 real plus 1 mock runner - self.assertEqual(resp.json, {"actions": 1, "runners": 14}) + self.assertEqual(resp.json, {"actions": 3, "runners": 14}) # Register resources from a single (non-existent pack) resp = self.app.post_json( diff --git a/st2common/st2common/models/db/execution.py b/st2common/st2common/models/db/execution.py index 4950965a7e..1f74661302 100644 --- a/st2common/st2common/models/db/execution.py +++ b/st2common/st2common/models/db/execution.py @@ -24,6 +24,7 @@ from st2common.fields import JSONDictEscapedFieldCompatibilityField from st2common.fields import ComplexDateTimeField from st2common.util import date as date_utils +from st2common.util import output_schema from st2common.util.secrets import get_secret_parameters from st2common.util.secrets import mask_inquiry_response from st2common.util.secrets import mask_secret_parameters @@ -105,6 +106,16 @@ def get_uid(self): return ":".join(uid) def mask_secrets(self, value): + """ + Masks the secret parameters in input and output schema for action execution output. + + :param value: action execution object. + :type value: ``dict`` + + :return: result: action execution object with masked secret paramters in input and output schema. + :rtype: result: ``dict`` + """ + result = copy.deepcopy(value) liveaction = result["liveaction"] @@ -142,10 +153,12 @@ def mask_secrets(self, value): }, ) + output_value = ActionExecutionDB.result.parse_field_value(result["result"]) + masked_output_value = output_schema.mask_secret_output(result, output_value) + result["result"] = masked_output_value + # TODO(mierdin): This logic should be moved to the dedicated Inquiry # data model once it exists. - result["result"] = ActionExecutionDB.result.parse_field_value(result["result"]) - if self.runner.get("name") == "inquirer": schema = result["result"].get("schema", {}) response = result["result"].get("response", {}) diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 2bde19c3c0..6070fe08a8 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -12,14 +12,15 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import sys -import logging +import logging +import sys import traceback import jsonschema from st2common.util import schema from st2common.constants import action as action_constants +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE LOG = logging.getLogger(__name__) @@ -51,6 +52,29 @@ def _validate_action(action_schema, result, output_key): schema.validate(final_result, action_schema, cls=schema.get_validator("custom")) +def mask_secret_output(ac_ex, output_value): + if not output_value: + return output_value + + if not isinstance(output_value, dict) and not isinstance(output_value, list): + return output_value + + output_key = ac_ex["runner"].get("output_key") + output_schema = ac_ex["action"].get("output_schema") + + if not output_key or not output_schema: + return output_value + + if not output_value.get(output_key): + return output_value + + for key, spec in output_schema.items(): + if key in output_value[output_key] and spec.get("secret", False): + output_value[output_key][key] = MASKED_ATTRIBUTE_VALUE + + return output_value + + def validate_output(runner_schema, action_schema, result, status, output_key): """Validate output of action with runner and action schema.""" try: diff --git a/st2common/tests/integration/test_register_content_script.py b/st2common/tests/integration/test_register_content_script.py index 24f1712083..1e06ebc796 100644 --- a/st2common/tests/integration/test_register_content_script.py +++ b/st2common/tests/integration/test_register_content_script.py @@ -52,7 +52,7 @@ def test_register_from_pack_success(self): ] cmd = BASE_REGISTER_ACTIONS_CMD_ARGS + opts exit_code, _, stderr = run_command(cmd=cmd) - self.assertIn("Registered 1 actions.", stderr) + self.assertIn("Registered 3 actions.", stderr) self.assertEqual(exit_code, 0) def test_register_from_pack_fail_on_failure_pack_dir_doesnt_exist(self): diff --git a/st2common/tests/unit/test_db_execution.py b/st2common/tests/unit/test_db_execution.py index e94ccb3d94..84f40ce813 100644 --- a/st2common/tests/unit/test_db_execution.py +++ b/st2common/tests/unit/test_db_execution.py @@ -72,21 +72,48 @@ "action": "st2.inquiry.respond", } +OUTPUT_SCHEMA_RESULT = { + "stdout": "", + "stderr": "", + "result": { + "os_secret_param": "to_be_masked", + "os_non_secret_param": "not_to_be_masked", + }, +} + +OUTPUT_SCHEMA_LIVEACTION = { + "action": "core.ask", + "parameters": {}, +} + ACTIONEXECUTIONS = { "execution_1": { - "action": {"uid": "action:core:ask"}, + "action": {"uid": "action:core:ask", "output_schema": {}}, "status": "succeeded", "runner": {"name": "inquirer"}, "liveaction": INQUIRY_LIVEACTION, "result": INQUIRY_RESULT, }, "execution_2": { - "action": {"uid": "action:st2:inquiry.respond"}, + "action": {"uid": "action:st2:inquiry.respond", "output_schema": {}}, "status": "succeeded", "runner": {"name": "python-script"}, "liveaction": RESPOND_LIVEACTION, "result": {"exit_code": 0, "result": None, "stderr": "", "stdout": ""}, }, + "execution_3": { + "action": { + "uid": "action:core:ask", + "output_schema": { + "os_secret_param": {"type": "string", "required": True, "secret": True}, + "os_non_secret_param": {"type": "string", "required": True}, + }, + }, + "status": "succeeded", + "runner": {"name": "inquirer", "output_key": "result"}, + "liveaction": OUTPUT_SCHEMA_LIVEACTION, + "result": OUTPUT_SCHEMA_RESULT, + }, } @@ -166,6 +193,24 @@ def test_execution_inquiry_response_action(self): for value in masked["parameters"]["response"].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) + def test_output_schema_secret_param_masking(self): + """Test that the output marked as secret in the output schema is masked in the output result + + In this test case, one of the output parameters is marked as secret in the output schema + while the other output parameter is not marked as secret. The value of the first output + parameter should be masked in the output result. + """ + + masked = self.executions["execution_3"].mask_secrets( + self.executions["execution_3"].to_serializable_dict() + ) + self.assertEqual( + masked["result"]["result"]["os_secret_param"], MASKED_ATTRIBUTE_VALUE + ) + self.assertEqual( + masked["result"]["result"]["os_non_secret_param"], "not_to_be_masked" + ) + @staticmethod def _save_execution(execution): return ActionExecution.add_or_update(execution) diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index af9570d4fa..64ed13237e 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import copy import unittest2 @@ -22,24 +23,28 @@ LIVEACTION_STATUS_FAILED, ) +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE + ACTION_RESULT = { "output": { "output_1": "Bobby", "output_2": 5, + "output_3": "shhh!", "deep_output": { "deep_item_1": "Jindal", }, } } -RUNNER_SCHEMA = { +RUNNER_OUTPUT_SCHEMA = { "output": {"type": "object"}, "error": {"type": "array"}, } -ACTION_SCHEMA = { +ACTION_OUTPUT_SCHEMA = { "output_1": {"type": "string"}, "output_2": {"type": "integer"}, + "output_3": {"type": "string"}, "deep_output": { "type": "object", "parameters": { @@ -50,22 +55,36 @@ }, } -RUNNER_SCHEMA_FAIL = { +RUNNER_OUTPUT_SCHEMA_FAIL = { "not_a_key_you_have": {"type": "string"}, } -ACTION_SCHEMA_FAIL = { +ACTION_OUTPUT_SCHEMA_FAIL = { "not_a_key_you_have": {"type": "string"}, } OUTPUT_KEY = "output" +ACTION_OUTPUT_SCHEMA_WITH_SECRET = { + "output_1": {"type": "string"}, + "output_2": {"type": "integer"}, + "output_3": {"type": "string", "secret": True}, + "deep_output": { + "type": "object", + "parameters": { + "deep_item_1": { + "type": "string", + }, + }, + }, +} + class OutputSchemaTestCase(unittest2.TestCase): def test_valid_schema(self): result, status = output_schema.validate_output( - copy.deepcopy(RUNNER_SCHEMA), - copy.deepcopy(ACTION_SCHEMA), + copy.deepcopy(RUNNER_OUTPUT_SCHEMA), + copy.deepcopy(ACTION_OUTPUT_SCHEMA), copy.deepcopy(ACTION_RESULT), LIVEACTION_STATUS_SUCCEEDED, OUTPUT_KEY, @@ -76,8 +95,8 @@ def test_valid_schema(self): def test_invalid_runner_schema(self): result, status = output_schema.validate_output( - copy.deepcopy(RUNNER_SCHEMA_FAIL), - copy.deepcopy(ACTION_SCHEMA), + copy.deepcopy(RUNNER_OUTPUT_SCHEMA_FAIL), + copy.deepcopy(ACTION_OUTPUT_SCHEMA), copy.deepcopy(ACTION_RESULT), LIVEACTION_STATUS_SUCCEEDED, OUTPUT_KEY, @@ -85,12 +104,12 @@ def test_invalid_runner_schema(self): expected_result = { "error": ( - "Additional properties are not allowed ('output' was unexpected)" - "\n\nFailed validating 'additionalProperties' in schema:\n {'addi" - "tionalProperties': False,\n 'properties': {'not_a_key_you_have': " - "{'type': 'string'}},\n 'type': 'object'}\n\nOn instance:\n {'" - "output': {'deep_output': {'deep_item_1': 'Jindal'},\n " - "'output_1': 'Bobby',\n 'output_2': 5}}" + "Additional properties are not allowed ('output' was unexpected)\n\n" + "Failed validating 'additionalProperties' in schema:\n " + "{'additionalProperties': False,\n 'properties': {'not_a_key_you_have': " + "{'type': 'string'}},\n 'type': 'object'}\n\nOn instance:\n {'output': " + "{'deep_output': {'deep_item_1': 'Jindal'},\n 'output_1': 'Bobby'," + "\n 'output_2': 5,\n 'output_3': 'shhh!'}}" ), "message": "Error validating output. See error output for more details.", } @@ -100,8 +119,8 @@ def test_invalid_runner_schema(self): def test_invalid_action_schema(self): result, status = output_schema.validate_output( - copy.deepcopy(RUNNER_SCHEMA), - copy.deepcopy(ACTION_SCHEMA_FAIL), + copy.deepcopy(RUNNER_OUTPUT_SCHEMA), + copy.deepcopy(ACTION_OUTPUT_SCHEMA_FAIL), copy.deepcopy(ACTION_RESULT), LIVEACTION_STATUS_SUCCEEDED, OUTPUT_KEY, @@ -117,3 +136,98 @@ def test_invalid_action_schema(self): self.assertIn(expected_result["error"], result["error"]) self.assertEqual(result["message"], expected_result["message"]) self.assertEqual(status, LIVEACTION_STATUS_FAILED) + + def test_mask_secret_output(self): + ac_ex = { + "action": { + "output_schema": ACTION_OUTPUT_SCHEMA_WITH_SECRET, + }, + "runner": { + "output_key": OUTPUT_KEY, + "output_schema": RUNNER_OUTPUT_SCHEMA, + }, + } + + expected_masked_output = { + "output": { + "output_1": "Bobby", + "output_2": 5, + "output_3": MASKED_ATTRIBUTE_VALUE, + "deep_output": { + "deep_item_1": "Jindal", + }, + } + } + + masked_output = output_schema.mask_secret_output( + ac_ex, copy.deepcopy(ACTION_RESULT) + ) + self.assertDictEqual(masked_output, expected_masked_output) + + def test_mask_secret_output_no_secret(self): + ac_ex = { + "action": { + "output_schema": ACTION_OUTPUT_SCHEMA, + }, + "runner": { + "output_key": OUTPUT_KEY, + "output_schema": RUNNER_OUTPUT_SCHEMA, + }, + } + + expected_masked_output = { + "output": { + "output_1": "Bobby", + "output_2": 5, + "output_3": "shhh!", + "deep_output": { + "deep_item_1": "Jindal", + }, + } + } + + masked_output = output_schema.mask_secret_output( + ac_ex, copy.deepcopy(ACTION_RESULT) + ) + self.assertDictEqual(masked_output, expected_masked_output) + + def test_mask_secret_output_noop(self): + ac_ex = { + "action": { + "output_schema": ACTION_OUTPUT_SCHEMA_WITH_SECRET, + }, + "runner": { + "output_key": OUTPUT_KEY, + "output_schema": RUNNER_OUTPUT_SCHEMA, + }, + } + + # The result is type of None. + ac_ex_result = None + expected_masked_output = None + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertEqual(masked_output, expected_masked_output) + + # The result is empty. + ac_ex_result = {} + expected_masked_output = {} + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertDictEqual(masked_output, expected_masked_output) + + # The output is type of None. + ac_ex_result = {"output": None} + expected_masked_output = {"output": None} + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertDictEqual(masked_output, expected_masked_output) + + # The output is not type of dict or list. + ac_ex_result = {"output": "foobar"} + expected_masked_output = {"output": "foobar"} + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertDictEqual(masked_output, expected_masked_output) + + # The output key is missing. + ac_ex_result = {"output1": None} + expected_masked_output = {"output1": None} + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertDictEqual(masked_output, expected_masked_output) diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_http_action.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_http_action.yaml new file mode 100644 index 0000000000..dadeb4b59c --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_http_action.yaml @@ -0,0 +1,22 @@ +--- +name: my_http_action +runner_type: http-request +description: A simple mock http action for testing purposes. +enabled: true +entry_point: '' +parameters: + url: + type: string + required: true + immutable: true + default: https://api.stackstorm.com +output_schema: + k1: + type: string + required: true + k2: + type: string + required: true + secret: true + k3: + type: boolean diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.py b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.py new file mode 100644 index 0000000000..8e3e15e463 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.py @@ -0,0 +1,25 @@ +# Copyright 2020 The StackStorm Authors. +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import +from st2common.runners.base_action import Action + + +class MyAction(Action): + def run(self): + k1 = "xyz" + k2 = "abc" + k3 = True + return {"k1": k1, "k2": k2, "k3": k3} diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.yaml new file mode 100644 index 0000000000..87553c9a76 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.yaml @@ -0,0 +1,16 @@ +--- +name: my_py_action +runner_type: "python-script" +description: A simple python action for testing purposes. +enabled: true +entry_point: my_py_action.py +output_schema: + k1: + type: string + required: true + k2: + type: string + required: true + secret: true + k3: + type: boolean diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml index 589e966e94..85105f435e 100644 --- a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml @@ -9,3 +9,17 @@ parameters: a1: required: true type: string +output_schema: + a6: + type: string + required: true + b6: + type: string + required: true + a7: + type: string + required: true + b7: + type: string + required: true + secret: true diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.py b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.py new file mode 100644 index 0000000000..3007056b91 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.py @@ -0,0 +1,22 @@ +# Copyright 2021 The StackStorm Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import + +from st2common.runners.base_action import Action + + +class PyAction(Action): + def run(self, k1): + return {"k2": k1} diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.yaml new file mode 100644 index 0000000000..b6126e8ce1 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.yaml @@ -0,0 +1,15 @@ +--- +name: py_secret_output +runner_type: "python-script" +description: A simple python action with an output schema for testing purposes. +enabled: true +entry_point: py-secret-output.py +parameters: + k1: + type: string + required: true +output_schema: + k2: + type: string + required: true + secret: true diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/data-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/data-flow.yaml index 7660d02cb5..2aa03c0598 100644 --- a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/data-flow.yaml +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/data-flow.yaml @@ -11,8 +11,10 @@ vars: - b2: <% ctx().a2 %> output: - - a5: <% ctx().b4 %> - - b5: <% ctx().a5 %> + - a6: <% ctx().a4 %> + - b6: <% ctx().a6 %> + - a7: <% ctx().a5 %> + - b7: <% ctx().a7 %> tasks: task1: @@ -32,4 +34,10 @@ tasks: publish: a4=<% result().stdout %> b4=<% ctx().a4 %> do: task3 task3: + action: orquesta_tests.py_secret_output k1=<% ctx().b4 %> + next: + - when: <% succeeded() %> + publish: a5=<% result().result.k2 %> + do: task4 + task4: action: core.noop