From 5bff2b5a08c74513c3d2ff227ffd8f676abaa40d Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Mon, 19 Feb 2024 16:53:52 +0100 Subject: [PATCH 1/3] gh-112997: Don't log arguments in asyncio unless debugging Nothing else in Python generally logs the contents of variables, so this can be very unexpected for developers and could leak sensitive information in to terminals and log files. --- Lib/asyncio/events.py | 6 +++-- Lib/asyncio/format_helpers.py | 19 ++++++++++------ Lib/test/test_asyncio/test_events.py | 22 ++++++++++++++++--- ...-02-19-16-53-48.gh-issue-112997.sYBXRZ.rst | 2 ++ 4 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-02-19-16-53-48.gh-issue-112997.sYBXRZ.rst diff --git a/Lib/asyncio/events.py b/Lib/asyncio/events.py index 072a99fee123c3..680749325025db 100644 --- a/Lib/asyncio/events.py +++ b/Lib/asyncio/events.py @@ -54,7 +54,8 @@ def _repr_info(self): info.append('cancelled') if self._callback is not None: info.append(format_helpers._format_callback_source( - self._callback, self._args)) + self._callback, self._args, + debug=self._loop.get_debug())) if self._source_traceback: frame = self._source_traceback[-1] info.append(f'created at {frame[0]}:{frame[1]}') @@ -90,7 +91,8 @@ def _run(self): raise except BaseException as exc: cb = format_helpers._format_callback_source( - self._callback, self._args) + self._callback, self._args, + debug=self._loop.get_debug()) msg = f'Exception in callback {cb}' context = { 'message': msg, diff --git a/Lib/asyncio/format_helpers.py b/Lib/asyncio/format_helpers.py index 27d11fd4fa9553..acd544d5dc1eac 100644 --- a/Lib/asyncio/format_helpers.py +++ b/Lib/asyncio/format_helpers.py @@ -19,19 +19,23 @@ def _get_function_source(func): return None -def _format_callback_source(func, args): - func_repr = _format_callback(func, args, None) +def _format_callback_source(func, args, *, debug=False): + func_repr = _format_callback(func, args, None, debug=debug) source = _get_function_source(func) if source: func_repr += f' at {source[0]}:{source[1]}' return func_repr -def _format_args_and_kwargs(args, kwargs): +def _format_args_and_kwargs(args, kwargs, *, debug=False): """Format function arguments and keyword arguments. Special case for a single parameter: ('hello',) is formatted as ('hello'). """ + # Avoid printing sensitive argument contents unless debugging + if not debug: + return '()' + # use reprlib to limit the length of the output items = [] if args: @@ -41,10 +45,11 @@ def _format_args_and_kwargs(args, kwargs): return '({})'.format(', '.join(items)) -def _format_callback(func, args, kwargs, suffix=''): +def _format_callback(func, args, kwargs, *, debug=False, suffix=''): if isinstance(func, functools.partial): - suffix = _format_args_and_kwargs(args, kwargs) + suffix - return _format_callback(func.func, func.args, func.keywords, suffix) + suffix = _format_args_and_kwargs(args, kwargs, debug=debug) + suffix + return _format_callback(func.func, func.args, func.keywords, + debug=debug, suffix=suffix) if hasattr(func, '__qualname__') and func.__qualname__: func_repr = func.__qualname__ @@ -53,7 +58,7 @@ def _format_callback(func, args, kwargs, suffix=''): else: func_repr = repr(func) - func_repr += _format_args_and_kwargs(args, kwargs) + func_repr += _format_args_and_kwargs(args, kwargs, debug=debug) if suffix: func_repr += suffix return func_repr diff --git a/Lib/test/test_asyncio/test_events.py b/Lib/test/test_asyncio/test_events.py index b25c0975736e20..4bc7ea4571cb0d 100644 --- a/Lib/test/test_asyncio/test_events.py +++ b/Lib/test/test_asyncio/test_events.py @@ -2236,7 +2236,7 @@ def test_handle_repr(self): h = asyncio.Handle(noop, (1, 2), self.loop) filename, lineno = test_utils.get_function_source(noop) self.assertEqual(repr(h), - '' + '' % (filename, lineno)) # cancelled handle @@ -2254,14 +2254,14 @@ def test_handle_repr(self): # partial function cb = functools.partial(noop, 1, 2) h = asyncio.Handle(cb, (3,), self.loop) - regex = (r'^$' + regex = (r'^$' % (re.escape(filename), lineno)) self.assertRegex(repr(h), regex) # partial function with keyword args cb = functools.partial(noop, x=1) h = asyncio.Handle(cb, (2, 3), self.loop) - regex = (r'^$' + regex = (r'^$' % (re.escape(filename), lineno)) self.assertRegex(repr(h), regex) @@ -2302,6 +2302,22 @@ def test_handle_repr_debug(self): '' % (filename, lineno, create_filename, create_lineno)) + # partial function + cb = functools.partial(noop, 1, 2) + create_lineno = sys._getframe().f_lineno + 1 + h = asyncio.Handle(cb, (3,), self.loop) + regex = (r'^$' + % (re.escape(filename), lineno, create_filename, create_lineno)) + self.assertRegex(repr(h), regex) + + # partial function with keyword args + cb = functools.partial(noop, x=1) + create_lineno = sys._getframe().f_lineno + 1 + h = asyncio.Handle(cb, (2, 3), self.loop) + regex = (r'^$' + % (re.escape(filename), lineno, create_filename, create_lineno)) + self.assertRegex(repr(h), regex) + def test_handle_source_traceback(self): loop = asyncio.get_event_loop_policy().new_event_loop() loop.set_debug(True) diff --git a/Misc/NEWS.d/next/Library/2024-02-19-16-53-48.gh-issue-112997.sYBXRZ.rst b/Misc/NEWS.d/next/Library/2024-02-19-16-53-48.gh-issue-112997.sYBXRZ.rst new file mode 100644 index 00000000000000..4f97b2d6085ba0 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-02-19-16-53-48.gh-issue-112997.sYBXRZ.rst @@ -0,0 +1,2 @@ +Stop logging potentially sensitive callback arguments in :mod:`asyncio` +unless debug mode is active. From 1b083c4b8aaa25cc67e77bbd76aab1e0bd392913 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 27 Feb 2024 11:04:31 +0100 Subject: [PATCH 2/3] Move debug behaviour note to docstring So that this information is more readily available. --- Lib/asyncio/format_helpers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/asyncio/format_helpers.py b/Lib/asyncio/format_helpers.py index acd544d5dc1eac..93737b7708a67b 100644 --- a/Lib/asyncio/format_helpers.py +++ b/Lib/asyncio/format_helpers.py @@ -31,8 +31,11 @@ def _format_args_and_kwargs(args, kwargs, *, debug=False): """Format function arguments and keyword arguments. Special case for a single parameter: ('hello',) is formatted as ('hello'). + + Note that this function only returns argument details when + debug=True is specified, as arguments may contain sensitive + information. """ - # Avoid printing sensitive argument contents unless debugging if not debug: return '()' From 988267521afadc5b71edcd682a625e5ac187769b Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 27 Feb 2024 16:00:45 +0100 Subject: [PATCH 3/3] Fix escaping of file names --- Lib/test/test_asyncio/test_events.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_asyncio/test_events.py b/Lib/test/test_asyncio/test_events.py index 4bc7ea4571cb0d..7462d44864b10d 100644 --- a/Lib/test/test_asyncio/test_events.py +++ b/Lib/test/test_asyncio/test_events.py @@ -2307,7 +2307,8 @@ def test_handle_repr_debug(self): create_lineno = sys._getframe().f_lineno + 1 h = asyncio.Handle(cb, (3,), self.loop) regex = (r'^$' - % (re.escape(filename), lineno, create_filename, create_lineno)) + % (re.escape(filename), lineno, + re.escape(create_filename), create_lineno)) self.assertRegex(repr(h), regex) # partial function with keyword args @@ -2315,7 +2316,8 @@ def test_handle_repr_debug(self): create_lineno = sys._getframe().f_lineno + 1 h = asyncio.Handle(cb, (2, 3), self.loop) regex = (r'^$' - % (re.escape(filename), lineno, create_filename, create_lineno)) + % (re.escape(filename), lineno, + re.escape(create_filename), create_lineno)) self.assertRegex(repr(h), regex) def test_handle_source_traceback(self):