From 6a7743b274ef7c65b425fa583f4aa0b60cabdb5f Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Fri, 17 Mar 2017 16:55:34 -0400 Subject: [PATCH] fix a callback bug that causes a task_args leak between job events see: #5802 --- awx/lib/tests/test_display_callback.py | 31 ++++++++++++++++ awx/lib/tower_display_callback/module.py | 46 ++++++------------------ 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/awx/lib/tests/test_display_callback.py b/awx/lib/tests/test_display_callback.py index 1cd2ec613c..d0c53f4fc0 100644 --- a/awx/lib/tests/test_display_callback.py +++ b/awx/lib/tests/test_display_callback.py @@ -166,6 +166,37 @@ def test_callback_plugin_no_log_filters(executor, cache, playbook): assert 'SENSITIVE' not in json.dumps(cache.items()) +@pytest.mark.parametrize('playbook', [ +{'no_log_on_ok.yml': ''' +- name: args should not be logged when task-level no_log is set + connection: local + hosts: all + gather_facts: no + tasks: + - shell: echo "SENSITIVE" + - shell: echo "PRIVATE" + no_log: true +'''}, # noqa +]) +def test_callback_plugin_task_args_leak(executor, cache, playbook): + executor.run() + events = cache.values() + assert events[0]['event'] == 'playbook_on_start' + assert events[1]['event'] == 'playbook_on_play_start' + + # task 1 + assert events[2]['event'] == 'playbook_on_task_start' + assert 'SENSITIVE' in events[2]['event_data']['task_args'] + assert events[3]['event'] == 'runner_on_ok' + assert 'SENSITIVE' in events[3]['event_data']['task_args'] + + # task 2 no_log=True + assert events[4]['event'] == 'playbook_on_task_start' + assert events[4]['event_data']['task_args'] == "the output has been hidden due to the fact that 'no_log: true' was specified for this result" # noqa + assert events[5]['event'] == 'runner_on_ok' + assert events[5]['event_data']['task_args'] == "the output has been hidden due to the fact that 'no_log: true' was specified for this result" # noqa + + @pytest.mark.parametrize('playbook', [ {'strip_env_vars.yml': ''' - name: sensitive environment variables should be stripped from events diff --git a/awx/lib/tower_display_callback/module.py b/awx/lib/tower_display_callback/module.py index 9d8cf24fb9..59575d7989 100644 --- a/awx/lib/tower_display_callback/module.py +++ b/awx/lib/tower_display_callback/module.py @@ -55,31 +55,10 @@ class BaseCallbackModule(CallbackBase): 'playbook_on_no_hosts_remaining', ] - CENSOR_FIELD_WHITELIST = [ - 'msg', - 'failed', - 'changed', - 'results', - 'start', - 'end', - 'delta', - 'cmd', - '_ansible_no_log', - 'rc', - 'failed_when_result', - 'skipped', - 'skip_reason', - ] - def __init__(self): super(BaseCallbackModule, self).__init__() self.task_uuids = set() - def censor_result(self, res): - if res.get('_ansible_no_log', False): - return {'censored': "the output has been hidden due to the fact that 'no_log: true' was specified for this result"} # noqa - return res - @contextlib.contextmanager def capture_event_data(self, event, **event_data): @@ -90,6 +69,9 @@ class BaseCallbackModule(CallbackBase): else: task = None + if event_data.get('res') and event_data['res'].get('_ansible_no_log', False): + event_data['res'] = {'censored': "the output has been hidden due to the fact that 'no_log: true' was specified for this result"} # noqa + with event_context.display_lock: try: event_context.add_local(event=event, **event_data) @@ -137,7 +119,9 @@ class BaseCallbackModule(CallbackBase): task_ctx['task_path'] = task.get_path() except AttributeError: pass - if not task.no_log: + if task.no_log: + task_ctx['task_args'] = "the output has been hidden due to the fact that 'no_log: true' was specified for this result" + else: task_args = ', '.join(('%s=%s' % a for a in task.args.items())) task_ctx['task_args'] = task_args if getattr(task, '_role', None): @@ -315,13 +299,11 @@ class BaseCallbackModule(CallbackBase): if result._task.get_name() == 'setup': result._result.get('ansible_facts', {}).pop('ansible_env', None) - res = self.censor_result(result._result) - event_data = dict( host=result._host.get_name(), remote_addr=result._host.address, task=result._task, - res=res, + res=result._result, event_loop=result._task.loop if hasattr(result._task, 'loop') else None, ) with self.capture_event_data('runner_on_ok', **event_data): @@ -329,13 +311,10 @@ class BaseCallbackModule(CallbackBase): def v2_runner_on_failed(self, result, ignore_errors=False): # FIXME: Add verbosity for exception/results output. - - res = self.censor_result(result._result) - event_data = dict( host=result._host.get_name(), remote_addr=result._host.address, - res=res, + res=result._result, task=result._task, ignore_errors=ignore_errors, event_loop=result._task.loop if hasattr(result._task, 'loop') else None, @@ -425,31 +404,28 @@ class BaseCallbackModule(CallbackBase): super(BaseCallbackModule, self).v2_on_file_diff(result) def v2_runner_item_on_ok(self, result): - res = self.censor_result(result._result) event_data = dict( host=result._host.get_name(), task=result._task, - res=res, + res=result._result, ) with self.capture_event_data('runner_item_on_ok', **event_data): super(BaseCallbackModule, self).v2_runner_item_on_ok(result) def v2_runner_item_on_failed(self, result): - res = self.censor_result(result._result) event_data = dict( host=result._host.get_name(), task=result._task, - res=res, + res=result._result, ) with self.capture_event_data('runner_item_on_failed', **event_data): super(BaseCallbackModule, self).v2_runner_item_on_failed(result) def v2_runner_item_on_skipped(self, result): - res = self.censor_result(result._result) event_data = dict( host=result._host.get_name(), task=result._task, - res=res, + res=result._result, ) with self.capture_event_data('runner_item_on_skipped', **event_data): super(BaseCallbackModule, self).v2_runner_item_on_skipped(result)