Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions webhook_server/libs/github_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
OTHER_MAIN_BRANCH,
PRE_COMMIT_STR,
PYTHON_MODULE_INSTALL_STR,
SECURITY_COMMITTER_IDENTITY_STR,
SECURITY_SUSPICIOUS_PATHS_STR,
SUCCESS_STR,
TOX_STR,
)
Expand Down Expand Up @@ -1413,6 +1415,13 @@ def _current_pull_request_supported_retest(self) -> list[str]:
check_name = custom_check["name"]
current_pull_request_supported_retest.append(check_name)

# Add security checks when enabled
if self.security_committer_identity_check:
current_pull_request_supported_retest.append(SECURITY_COMMITTER_IDENTITY_STR)

if self.security_suspicious_paths:
current_pull_request_supported_retest.append(SECURITY_SUSPICIOUS_PATHS_STR)
Comment thread
myakove marked this conversation as resolved.

return current_pull_request_supported_retest

async def cleanup(self) -> None:
Expand Down
5 changes: 5 additions & 0 deletions webhook_server/libs/handlers/runner_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1971,6 +1971,11 @@ async def run_retests(self, supported_retests: list[str], pull_request: PullRequ
check_key = custom_check["name"]
_retests_to_func_map[check_key] = partial(self.run_custom_check, check_config=custom_check)

# Add security checks to the retest map
# Security methods don't take pull_request param, so wrap with lambda
_retests_to_func_map[SECURITY_COMMITTER_IDENTITY_STR] = lambda **_kwargs: self.run_security_committer_identity()
_retests_to_func_map[SECURITY_SUSPICIOUS_PATHS_STR] = lambda **_kwargs: self.run_security_suspicious_paths()

tasks: list[Coroutine[Any, Any, Any] | Task[Any]] = []
scheduled_tests: list[str] = []
for _test in supported_retests:
Expand Down
71 changes: 71 additions & 0 deletions webhook_server/tests/test_github_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from webhook_server.libs.github_api import GithubWebhook
from webhook_server.libs.handlers.owners_files_handler import OwnersFileHandler
from webhook_server.tests.conftest import TEST_GITHUB_TOKEN
from webhook_server.utils.constants import SECURITY_COMMITTER_IDENTITY_STR, SECURITY_SUSPICIOUS_PATHS_STR


class TestGithubWebhook:
Expand Down Expand Up @@ -1319,6 +1320,76 @@ def test_current_pull_request_supported_retest_property(
assert "pre-commit" in result
assert "conventional-title" in result

def test_supported_retest_includes_security_checks(
self, minimal_hook_data: dict[str, Any], minimal_headers: Headers, logger: Mock
) -> None:
"""Test that security checks are included in supported retests when enabled."""
with patch("webhook_server.libs.github_api.Config") as mock_config:
mock_config.return_value.repository = True
mock_config.return_value.repository_local_data.return_value = {}

with patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") as mock_get_api:
mock_get_api.return_value = (Mock(), "token", "apiuser")

with patch("webhook_server.libs.github_api.get_github_repo_api") as mock_get_repo_api:
mock_get_repo_api.return_value = Mock()

with patch("webhook_server.libs.github_api.get_repository_github_app_api") as mock_get_app_api:
mock_get_app_api.return_value = Mock()

with patch("webhook_server.utils.helpers.get_repository_color_for_log_prefix") as mock_color:
mock_color.return_value = "test-repo"

gh = GithubWebhook(minimal_hook_data, minimal_headers, logger)

gh.security_committer_identity_check = True
gh.security_suspicious_paths = ["Dockerfile", ".github/"]
gh.tox = False
gh.build_and_push_container = False
gh.pypi = False
gh.pre_commit = False
gh.conventional_title = False
gh.custom_check_runs = []

result = gh._current_pull_request_supported_retest
assert SECURITY_COMMITTER_IDENTITY_STR in result
assert SECURITY_SUSPICIOUS_PATHS_STR in result

def test_supported_retest_excludes_disabled_security_checks(
self, minimal_hook_data: dict[str, Any], minimal_headers: Headers, logger: Mock
) -> None:
Comment thread
myakove marked this conversation as resolved.
"""Test that disabled security checks are excluded from supported retests."""
with patch("webhook_server.libs.github_api.Config") as mock_config:
mock_config.return_value.repository = True
mock_config.return_value.repository_local_data.return_value = {}

with patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") as mock_get_api:
mock_get_api.return_value = (Mock(), "token", "apiuser")

with patch("webhook_server.libs.github_api.get_github_repo_api") as mock_get_repo_api:
mock_get_repo_api.return_value = Mock()

with patch("webhook_server.libs.github_api.get_repository_github_app_api") as mock_get_app_api:
mock_get_app_api.return_value = Mock()

with patch("webhook_server.utils.helpers.get_repository_color_for_log_prefix") as mock_color:
mock_color.return_value = "test-repo"

gh = GithubWebhook(minimal_hook_data, minimal_headers, logger)

gh.security_committer_identity_check = False
gh.security_suspicious_paths = []
gh.tox = False
gh.build_and_push_container = False
gh.pypi = False
gh.pre_commit = False
gh.conventional_title = False
gh.custom_check_runs = []

result = gh._current_pull_request_supported_retest
assert SECURITY_COMMITTER_IDENTITY_STR not in result
assert SECURITY_SUSPICIOUS_PATHS_STR not in result

@pytest.mark.asyncio
async def test_get_last_commit(self, minimal_hook_data: dict, minimal_headers: Headers, logger: Mock) -> None:
"""Test _get_last_commit method."""
Expand Down