From 1772372553a1e3ce0984fadb307ef6b2ab062685 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 25 Jun 2024 23:45:14 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Ensure=20that=20no-token=20case?= =?UTF-8?q?=20is=20actually=20logged?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this patch, there was an unreachable code path that seems to have been intended for marking the case when no token is provided with the `"NOTOKEN"` string in the debug log output. However the logic behind choosing to show it contained two opposite checks. Said place in the logging utils, didn't have any coverage either. This change updates the code to make all of its branches reachable. Moreover, it adds test cases for all the branches that remain in the implementation. --- codecov_cli/helpers/logging_utils.py | 11 +-- tests/commands/test_upload_token_discovery.py | 83 +++++++++++++++++++ 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/codecov_cli/helpers/logging_utils.py b/codecov_cli/helpers/logging_utils.py index 658c8d439..0155dd410 100644 --- a/codecov_cli/helpers/logging_utils.py +++ b/codecov_cli/helpers/logging_utils.py @@ -42,11 +42,12 @@ def format(self, record): f"{prefix} - {asctime} -- {x}" for x in msg.splitlines() ) if hasattr(record, "extra_log_attributes"): - token = record.extra_log_attributes.get("token") - if token: - record.extra_log_attributes["token"] = ( - "NOTOKEN" if not token else (str(token)[:1] + 18 * "*") - ) + passed_token_attribute = record.extra_log_attributes.get("token") + record.extra_log_attributes["token"] = ( + "NOTOKEN" + if passed_token_attribute is None + else (str(passed_token_attribute)[:1] + 18 * "*") + ) msg += " --- " + json.dumps( record.extra_log_attributes, cls=JsonEncoder ) diff --git a/tests/commands/test_upload_token_discovery.py b/tests/commands/test_upload_token_discovery.py index 5080d567f..533a310c7 100644 --- a/tests/commands/test_upload_token_discovery.py +++ b/tests/commands/test_upload_token_discovery.py @@ -44,3 +44,86 @@ def test_no_cli_token_config_fallback( CliRunner().invoke(cli, ["do-upload", "--commit-sha=deadbeef"], obj={}) assert do_upload_cmd_spy.call_args[-1]["token"] == "sentinel-value" + + +def test_no_token_anywhere( + mocker: MockerFixture, + monkeypatch: MonkeyPatch, + tmp_path: Path, +) -> None: + """Test that a missing token produces `NOTOKEN` in logs.""" + # NOTE: The pytest's `caplog` fixture is not used in this test as it + # NOTE: doesn't play well with Click's testing CLI runner, and does + # NOTE: not capture any log entries for mysterious reasons. + # + # Refs: + # * https://github.com/pallets/click/issues/2573#issuecomment-1649773563 + # * https://github.com/pallets/click/issues/1763#issuecomment-767687608 + monkeypatch.chdir(tmp_path) + + mocker.patch.object(upload, "do_upload_logic") + do_upload_cmd_spy = mocker.spy(upload, "do_upload_logic") + debug_log_spy = mocker.spy(upload.logger, "debug") + + cov_upload_cmd_output = ( + CliRunner() + .invoke( + cli, + [ + "-v", # <- NOTE: this is the only way to turn on debug logger + "do-upload", + "--commit-sha=deadbeef", + ], + obj={}, + ) + .output + ) + + assert ( + debug_log_spy.call_args[1]["extra"]["extra_log_attributes"]["token"] + == "NOTOKEN" + ) + assert '"token": "NOTOKEN"' in cov_upload_cmd_output + assert do_upload_cmd_spy.call_args[-1]["token"] is None + + +def test_cli_token_masked_in_logs( + mocker: MockerFixture, + monkeypatch: MonkeyPatch, + tmp_path: Path, +) -> None: + """Test that a present token is masked in logs.""" + # NOTE: The pytest's `caplog` fixture is not used in this test as it + # NOTE: doesn't play well with Click's testing CLI runner, and does + # NOTE: not capture any log entries for mysterious reasons. + # + # Refs: + # * https://github.com/pallets/click/issues/2573#issuecomment-1649773563 + # * https://github.com/pallets/click/issues/1763#issuecomment-767687608 + monkeypatch.chdir(tmp_path) + + mocker.patch.object(upload, "do_upload_logic") + do_upload_cmd_spy = mocker.spy(upload, "do_upload_logic") + debug_log_spy = mocker.spy(upload.logger, "debug") + + cov_upload_cmd_output = ( + CliRunner() + .invoke( + cli, + [ + "-v", # <- NOTE: this is the only way to turn on debug logger + "do-upload", + "--commit-sha=deadbeef", + "--token=sentinel-value", + ], + obj={}, + ) + .output + ) + + assert ( + debug_log_spy.call_args[1]["extra"]["extra_log_attributes"]["token"] + == "s" + "*" * 18 + ) + assert f'"token": "s{"*" * 18}"' in cov_upload_cmd_output + assert do_upload_cmd_spy.call_args[-1]["token"] == "sentinel-value"