From 68a9fc55db05b362c277d87b355d0c89aa0870f5 Mon Sep 17 00:00:00 2001 From: Juan Pasutti Date: Tue, 15 Sep 2026 23:54:07 -0300 Subject: [PATCH 1/4] Ignore empty GitHub API keys Signed-off-by: Juan Pasutti --- .../github/util/github_api_key_handler.py | 7 ++- tests/test_classes/test_github_api_keys.py | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 tests/test_classes/test_github_api_keys.py diff --git a/collectoss/tasks/github/util/github_api_key_handler.py b/collectoss/tasks/github/util/github_api_key_handler.py index 5cabe1fab..93570ed79 100644 --- a/collectoss/tasks/github/util/github_api_key_handler.py +++ b/collectoss/tasks/github/util/github_api_key_handler.py @@ -102,8 +102,11 @@ def get_api_keys(self) -> List[str]: time.sleep(5) attempts += 1 - if self.config_key is not None: - keys += [self.config_key] + if self.config_key is not None: # Leave out None values + if self.config_key.strip(): # Leave out empty strings + keys += [self.config_key] + else: + self.logger.warning("GitHub API key is an empty string. Please, add a valid one.") if len(keys) == 0: return [] diff --git a/tests/test_classes/test_github_api_keys.py b/tests/test_classes/test_github_api_keys.py new file mode 100644 index 000000000..92eac261d --- /dev/null +++ b/tests/test_classes/test_github_api_keys.py @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: MIT +import pytest +from unittest.mock import Mock, patch + +from collectoss.tasks.github.util.github_api_key_handler import GithubApiKeyHandler + + +github_whitespace_api_keys_list = ["", " "] +github_none_api_key = None +github_valid_api_key = "ghp_1234567890abcdef1234567890abcdef12345678" + +def build_handler(config_key, db_keys): + logger = Mock() + + with patch("collectoss.tasks.github.util.github_api_key_handler.RedisList"), \ + patch.object(GithubApiKeyHandler, "get_config_key", return_value=config_key), \ + patch.object(GithubApiKeyHandler, "get_api_keys_from_database", return_value=db_keys), \ + patch.object(GithubApiKeyHandler, "is_bad_api_key", return_value=False) as probe: + handler = GithubApiKeyHandler(logger) + + return handler, probe, logger + +@pytest.mark.unit +class TestConfigKeys: + + @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) + def test_whitespace_config_key_with_no_db_keys(self, github_whitespace_api_key): + db_keys = [] + handler, probe, logger = build_handler(github_whitespace_api_key, db_keys) + + assert handler.keys == [] + assert probe.call_count == 0 + logger.warning.assert_called_once() + + def test_none_config_key_with_no_db_keys(self): + db_keys = [] + handler, probe, logger = build_handler(github_none_api_key, db_keys) + + assert handler.keys == [] + assert probe.call_count == 0 + logger.warning.assert_not_called() + + def test_valid_config_key_with_no_db_keys(self): + db_keys = [] + handler, probe, logger = build_handler(github_valid_api_key, db_keys) + + assert handler.keys == [github_valid_api_key] + assert probe.call_count == 1 + logger.warning.assert_not_called() + + @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) + def test_whitespace_config_key_with_db_keys(self, github_whitespace_api_key): + db_keys = ["ghp_abcdef1234567890abcdef1234567890abcdef12"] + handler, probe, logger = build_handler(github_whitespace_api_key, db_keys) + + assert handler.keys == db_keys + assert probe.call_count == 1 + logger.warning.assert_called_once() From 27a4b7a1647d5d7185771377f110261c6df92a30 Mon Sep 17 00:00:00 2001 From: Juan Pasutti Date: Tue, 22 Sep 2026 12:00:56 -0300 Subject: [PATCH 2/4] test: assert on the redis key list, not just the warning Signed-off-by: Juan Pasutti --- tests/test_classes/test_github_api_keys.py | 33 ++++++++++++++-------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/tests/test_classes/test_github_api_keys.py b/tests/test_classes/test_github_api_keys.py index 92eac261d..a1212a1f5 100644 --- a/tests/test_classes/test_github_api_keys.py +++ b/tests/test_classes/test_github_api_keys.py @@ -8,6 +8,7 @@ github_whitespace_api_keys_list = ["", " "] github_none_api_key = None github_valid_api_key = "ghp_1234567890abcdef1234567890abcdef12345678" +github_valid_db_api_key = "ghp_abcdef1234567890abcdef1234567890abcdef12" def build_handler(config_key, db_keys): logger = Mock() @@ -15,10 +16,10 @@ def build_handler(config_key, db_keys): with patch("collectoss.tasks.github.util.github_api_key_handler.RedisList"), \ patch.object(GithubApiKeyHandler, "get_config_key", return_value=config_key), \ patch.object(GithubApiKeyHandler, "get_api_keys_from_database", return_value=db_keys), \ - patch.object(GithubApiKeyHandler, "is_bad_api_key", return_value=False) as probe: + patch.object(GithubApiKeyHandler, "is_bad_api_key", return_value=False) as mock_is_bad_api_key: handler = GithubApiKeyHandler(logger) - return handler, probe, logger + return handler, mock_is_bad_api_key, logger @pytest.mark.unit class TestConfigKeys: @@ -26,33 +27,41 @@ class TestConfigKeys: @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) def test_whitespace_config_key_with_no_db_keys(self, github_whitespace_api_key): db_keys = [] - handler, probe, logger = build_handler(github_whitespace_api_key, db_keys) + handler, mock_is_bad_api_key, logger = build_handler(github_whitespace_api_key, db_keys) assert handler.keys == [] - assert probe.call_count == 0 + assert mock_is_bad_api_key.call_count == 0 + # with no keys left, get_api_keys returns before it reaches redis + handler.redis_key_list.clear.assert_not_called() + handler.redis_key_list.extend.assert_not_called() logger.warning.assert_called_once() def test_none_config_key_with_no_db_keys(self): db_keys = [] - handler, probe, logger = build_handler(github_none_api_key, db_keys) + handler, mock_is_bad_api_key, logger = build_handler(github_none_api_key, db_keys) assert handler.keys == [] - assert probe.call_count == 0 + assert mock_is_bad_api_key.call_count == 0 + handler.redis_key_list.clear.assert_not_called() + handler.redis_key_list.extend.assert_not_called() logger.warning.assert_not_called() def test_valid_config_key_with_no_db_keys(self): db_keys = [] - handler, probe, logger = build_handler(github_valid_api_key, db_keys) + handler, mock_is_bad_api_key, logger = build_handler(github_valid_api_key, db_keys) assert handler.keys == [github_valid_api_key] - assert probe.call_count == 1 + assert mock_is_bad_api_key.call_count == 1 + handler.redis_key_list.extend.assert_called_once_with([github_valid_api_key]) logger.warning.assert_not_called() @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) def test_whitespace_config_key_with_db_keys(self, github_whitespace_api_key): - db_keys = ["ghp_abcdef1234567890abcdef1234567890abcdef12"] - handler, probe, logger = build_handler(github_whitespace_api_key, db_keys) + expected_keys = [github_valid_db_api_key] + # get_api_keys appends to the list it gets back, so hand it a copy + handler, mock_is_bad_api_key, logger = build_handler(github_whitespace_api_key, list(expected_keys)) - assert handler.keys == db_keys - assert probe.call_count == 1 + assert handler.keys == expected_keys + assert mock_is_bad_api_key.call_count == 1 + handler.redis_key_list.extend.assert_called_once_with(expected_keys) logger.warning.assert_called_once() From 5e74418309ec70ed468097ec9140f44e22aca7d7 Mon Sep 17 00:00:00 2001 From: Juan Pasutti Date: Thu, 24 Sep 2026 12:28:58 -0300 Subject: [PATCH 3/4] Strip the config key when it is read Signed-off-by: Juan Pasutti --- .../tasks/github/util/github_api_key_handler.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/collectoss/tasks/github/util/github_api_key_handler.py b/collectoss/tasks/github/util/github_api_key_handler.py index 93570ed79..d0c9d8d48 100644 --- a/collectoss/tasks/github/util/github_api_key_handler.py +++ b/collectoss/tasks/github/util/github_api_key_handler.py @@ -36,6 +36,8 @@ def __init__(self, logger): self.redis_key_list = RedisList(self.oauth_redis_key) self.config_key = self.get_config_key() + if self.config_key: + self.config_key = self.config_key.strip() self.keys = self.get_api_keys() @@ -102,11 +104,10 @@ def get_api_keys(self) -> List[str]: time.sleep(5) attempts += 1 - if self.config_key is not None: # Leave out None values - if self.config_key.strip(): # Leave out empty strings - keys += [self.config_key] - else: - self.logger.warning("GitHub API key is an empty string. Please, add a valid one.") + if self.config_key: + keys += [self.config_key] + elif self.config_key is not None: # None means it was never set + self.logger.warning("GitHub API key is an empty string. Please, add a valid one.") if len(keys) == 0: return [] From 700b8ffbd304abe0e193a2e6611515583a96a514 Mon Sep 17 00:00:00 2001 From: Juan Pasutti Date: Mon, 28 Sep 2026 19:51:14 -0300 Subject: [PATCH 4/4] Warn when whitespace is stripped from the config key Signed-off-by: Juan Pasutti --- collectoss/tasks/github/util/github_api_key_handler.py | 5 ++++- tests/test_classes/test_github_api_keys.py | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/collectoss/tasks/github/util/github_api_key_handler.py b/collectoss/tasks/github/util/github_api_key_handler.py index d0c9d8d48..ed809b834 100644 --- a/collectoss/tasks/github/util/github_api_key_handler.py +++ b/collectoss/tasks/github/util/github_api_key_handler.py @@ -37,7 +37,10 @@ def __init__(self, logger): self.config_key = self.get_config_key() if self.config_key: - self.config_key = self.config_key.strip() + stripped_config_key = self.config_key.strip() + if stripped_config_key and stripped_config_key != self.config_key: + self.logger.warning("GitHub API key in config had leading or trailing whitespace. It was stripped.") + self.config_key = stripped_config_key self.keys = self.get_api_keys() diff --git a/tests/test_classes/test_github_api_keys.py b/tests/test_classes/test_github_api_keys.py index a1212a1f5..e7821a624 100644 --- a/tests/test_classes/test_github_api_keys.py +++ b/tests/test_classes/test_github_api_keys.py @@ -55,6 +55,15 @@ def test_valid_config_key_with_no_db_keys(self): handler.redis_key_list.extend.assert_called_once_with([github_valid_api_key]) logger.warning.assert_not_called() + def test_padded_config_key_is_stripped(self): + db_keys = [] + handler, mock_is_bad_api_key, logger = build_handler(f" {github_valid_api_key}\n", db_keys) + + assert handler.keys == [github_valid_api_key] + assert mock_is_bad_api_key.call_count == 1 + handler.redis_key_list.extend.assert_called_once_with([github_valid_api_key]) + logger.warning.assert_called_once() + @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) def test_whitespace_config_key_with_db_keys(self, github_whitespace_api_key): expected_keys = [github_valid_db_api_key]