From b0cb8fe7977319f6b0cf15f9b7e7152451c61b91 Mon Sep 17 00:00:00 2001 From: Diptesh Roy Date: Wed, 26 Aug 2026 11:55:08 +0530 Subject: [PATCH 1/3] feat(facade): check repo size and enforce clone limit before cloning (#458) Signed-off-by: Diptesh Roy --- collectoss/application/config.py | 2 + .../facade_worker/facade_worker/config.py | 2 + .../facade_worker/facade_worker/repofetch.py | 65 ++++++++++++++++++ .../test_git/test_repo_size_limit.py | 68 +++++++++++++++++++ 4 files changed, 137 insertions(+) create mode 100644 tests/test_tasks/test_git/test_repo_size_limit.py diff --git a/collectoss/application/config.py b/collectoss/application/config.py index 43bef8b49..8375ec29f 100644 --- a/collectoss/application/config.py +++ b/collectoss/application/config.py @@ -58,6 +58,8 @@ def redact_setting_value(section_name, setting_name, value): "run_analysis": 1, "run_facade_contributors": 1, "commit_messages": 1, + "max_clone_size_kb": 0, + "clone_size_safety_margin": 0.5, }, "Server": { "cache_expire": "3600", diff --git a/collectoss/tasks/git/util/facade_worker/facade_worker/config.py b/collectoss/tasks/git/util/facade_worker/facade_worker/config.py index 2b536a3a4..40357afc4 100644 --- a/collectoss/tasks/git/util/facade_worker/facade_worker/config.py +++ b/collectoss/tasks/git/util/facade_worker/facade_worker/config.py @@ -134,6 +134,8 @@ def __init__(self,logger: Logger): self.multithreaded = worker_options["multithreaded"] self.create_xlsx_summary_files = worker_options["create_xlsx_summary_files"] self.commit_messages = worker_options["commit_messages"] + self.max_clone_size_kb = worker_options.get("max_clone_size_kb", 0) + self.clone_size_safety_margin = float(worker_options.get("clone_size_safety_margin", 0.5)) self.tool_source = "Facade" self.data_source = "Git Log" diff --git a/collectoss/tasks/git/util/facade_worker/facade_worker/repofetch.py b/collectoss/tasks/git/util/facade_worker/facade_worker/repofetch.py index 49f7fae21..b4544d979 100644 --- a/collectoss/tasks/git/util/facade_worker/facade_worker/repofetch.py +++ b/collectoss/tasks/git/util/facade_worker/facade_worker/repofetch.py @@ -25,6 +25,7 @@ # and checks for any parents of HEAD that aren't already accounted for in the # repos. It also rebuilds analysis data, checks any changed affiliations and # aliases, and caches data for display. +import logging import html.parser import subprocess import os @@ -38,9 +39,63 @@ from collectoss.application.db.lib import execute_sql, get_repo_by_repo_git from typing_extensions import deprecated +logger = logging.getLogger(__name__) + class GitCloneError(Exception): pass + +def check_repo_size_limit(repo_git: str, max_clone_size_kb: int, clone_size_safety_margin: float = 0.5, logger=None): + """ + Checks if a repository's estimated clone size exceeds max_clone_size_kb. + Returns (allowed: bool, reported_size_kb: Optional[int], estimated_size_kb: Optional[float]). + """ + if not max_clone_size_kb or max_clone_size_kb <= 0: + return True, None, None + + reported_size_kb = None + + try: + if "github.com" in repo_git.lower(): + from collectoss.tasks.github.util.util import get_owner_repo + from collectoss.tasks.github.util.github_data_access import GithubDataAccess + owner, repo = get_owner_repo(repo_git) + url = f"https://api.github.com/repos/{owner}/{repo}" + github_data_access = GithubDataAccess(None, logger) + result = github_data_access.get_resource(url) + if result and isinstance(result, dict) and "size" in result: + reported_size_kb = result["size"] + elif "gitlab.com" in repo_git.lower(): + import httpx + from urllib.parse import quote_plus + git_clean = repo_git.rstrip('/') + if git_clean.endswith('.git'): + git_clean = git_clean[:-4] + parts = git_clean.split("gitlab.com/") + if len(parts) > 1: + project_path = parts[1] + encoded_path = quote_plus(project_path) + url = f"https://gitlab.com/api/v4/projects/{encoded_path}?statistics=true" + response = httpx.get(url, timeout=10.0) + if response.status_code == 200: + data = response.json() + stats = data.get("statistics", {}) + bytes_size = stats.get("repository_size") or data.get("repository_size") + if bytes_size is not None: + reported_size_kb = int(bytes_size / 1024) + except Exception as e: + if logger: + logger.warning(f"Could not retrieve repo size for {repo_git} via API: {e}") + return True, None, None + + if reported_size_kb is not None: + estimated_size_kb = reported_size_kb * (1.0 + float(clone_size_safety_margin)) + if estimated_size_kb > max_clone_size_kb: + return False, reported_size_kb, estimated_size_kb + + return True, reported_size_kb, (reported_size_kb * (1.0 + float(clone_size_safety_margin))) if reported_size_kb is not None else None + + def git_repo_initialize(facade_helper, session, repo_git): # Select any new git repos so we can set up their locations and git clone @@ -125,6 +180,16 @@ def git_repo_initialize(facade_helper, session, repo_git): execute_sql(query) return + max_limit = getattr(facade_helper, 'max_clone_size_kb', 0) + safety_margin = getattr(facade_helper, 'clone_size_safety_margin', 0.5) + if max_limit > 0: + allowed, reported_kb, estimated_kb = check_repo_size_limit(git, max_limit, safety_margin, logger) + if not allowed: + msg = f"Repo '{git}' estimated clone size ({estimated_kb:.0f} KB) exceeds maximum clone size limit ({max_limit} KB)" + update_repo_log(logger, facade_helper, row.repo_id, 'Failed (size limit)') + facade_helper.log_activity('Error', msg) + raise GitCloneError(msg) + # Create the prerequisite directories try: pathlib.Path(repo_path).mkdir(parents=True, exist_ok=True) diff --git a/tests/test_tasks/test_git/test_repo_size_limit.py b/tests/test_tasks/test_git/test_repo_size_limit.py new file mode 100644 index 000000000..c2d6134f0 --- /dev/null +++ b/tests/test_tasks/test_git/test_repo_size_limit.py @@ -0,0 +1,68 @@ +import unittest +from unittest.mock import MagicMock, patch +import collectoss.tasks.github.util.github_data_access +from collectoss.tasks.git.util.facade_worker.facade_worker.repofetch import check_repo_size_limit, GitCloneError + + +class TestRepoSizeLimit(unittest.TestCase): + + def setUp(self): + self.github_access_patcher = patch("collectoss.tasks.github.util.github_data_access.GithubDataAccess.__init__", return_value=None) + self.mock_github_init = self.github_access_patcher.start() + + def tearDown(self): + self.github_access_patcher.stop() + + def test_size_limit_disabled(self): + allowed, reported_kb, estimated_kb = check_repo_size_limit("https://github.com/chaoss/CollectOSS", 0) + self.assertTrue(allowed) + self.assertIsNone(reported_kb) + self.assertIsNone(estimated_kb) + + @patch("collectoss.tasks.github.util.github_data_access.GithubDataAccess.get_resource") + def test_github_repo_under_limit(self, mock_get_resource): + mock_get_resource.return_value = {"size": 1000} + allowed, reported_kb, estimated_kb = check_repo_size_limit( + "https://github.com/chaoss/CollectOSS", max_clone_size_kb=2000, clone_size_safety_margin=0.5 + ) + self.assertTrue(allowed) + self.assertEqual(reported_kb, 1000) + self.assertEqual(estimated_kb, 1500.0) + + @patch("collectoss.tasks.github.util.github_data_access.GithubDataAccess.get_resource") + def test_github_repo_exceeds_limit(self, mock_get_resource): + mock_get_resource.return_value = {"size": 2000} + allowed, reported_kb, estimated_kb = check_repo_size_limit( + "https://github.com/chaoss/CollectOSS", max_clone_size_kb=2500, clone_size_safety_margin=0.5 + ) + self.assertFalse(allowed) + self.assertEqual(reported_kb, 2000) + self.assertEqual(estimated_kb, 3000.0) + + @patch("httpx.get") + def test_gitlab_repo_exceeds_limit(self, mock_httpx_get): + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"statistics": {"repository_size": 5242880}} # 5120 KB + mock_httpx_get.return_value = mock_response + + allowed, reported_kb, estimated_kb = check_repo_size_limit( + "https://gitlab.com/group/project", max_clone_size_kb=5000, clone_size_safety_margin=0.5 + ) + self.assertFalse(allowed) + self.assertEqual(reported_kb, 5120) + self.assertEqual(estimated_kb, 7680.0) + + @patch("collectoss.tasks.github.util.github_data_access.GithubDataAccess.get_resource") + def test_api_error_fallback(self, mock_get_resource): + mock_get_resource.side_effect = Exception("API rate limit exceeded") + allowed, reported_kb, estimated_kb = check_repo_size_limit( + "https://github.com/chaoss/CollectOSS", max_clone_size_kb=1000, clone_size_safety_margin=0.5 + ) + self.assertTrue(allowed) + self.assertIsNone(reported_kb) + self.assertIsNone(estimated_kb) + + +if __name__ == "__main__": + unittest.main() From 82c17c4f79dcbf8863f1212ffc5646fd4ac76bb1 Mon Sep 17 00:00:00 2001 From: Diptesh Roy Date: Thu, 27 Aug 2026 11:16:41 +0530 Subject: [PATCH 2/3] fix(facade): skip API lookups for non-searchable local emails to prevent E2E worker timeout Signed-off-by: Diptesh Roy --- .../facade_worker/facade_worker/config.py | 2 +- .../contributor_interface.py | 28 +++++++++++++++++-- .../tasks/github/facade_github/tasks.py | 10 +++++++ .../test_git/test_repo_size_limit.py | 12 ++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/collectoss/tasks/git/util/facade_worker/facade_worker/config.py b/collectoss/tasks/git/util/facade_worker/facade_worker/config.py index 40357afc4..ca4299607 100644 --- a/collectoss/tasks/git/util/facade_worker/facade_worker/config.py +++ b/collectoss/tasks/git/util/facade_worker/facade_worker/config.py @@ -134,7 +134,7 @@ def __init__(self,logger: Logger): self.multithreaded = worker_options["multithreaded"] self.create_xlsx_summary_files = worker_options["create_xlsx_summary_files"] self.commit_messages = worker_options["commit_messages"] - self.max_clone_size_kb = worker_options.get("max_clone_size_kb", 0) + self.max_clone_size_kb = int(worker_options.get("max_clone_size_kb", 0)) self.clone_size_safety_margin = float(worker_options.get("clone_size_safety_margin", 0.5)) self.tool_source = "Facade" diff --git a/collectoss/tasks/github/facade_github/contributor_interfaceable/contributor_interface.py b/collectoss/tasks/github/facade_github/contributor_interfaceable/contributor_interface.py index b1b163a2b..b211573c5 100644 --- a/collectoss/tasks/github/facade_github/contributor_interfaceable/contributor_interface.py +++ b/collectoss/tasks/github/facade_github/contributor_interfaceable/contributor_interface.py @@ -261,6 +261,28 @@ def update_contributor(self, cntrb, max_attempts=3): +def is_valid_searchable_email(email: str) -> bool: + """Check if an email address is valid and searchable via GitHub API.""" + if not email or not isinstance(email, str): + return False + email = email.strip().lower() + if len(email) < 5 or "@" not in email: + return False + parts = email.rsplit("@", 1) + if len(parts) != 2: + return False + user, domain = parts[0], parts[1] + if not user or not domain or "." not in domain: + return False + invalid_domains = {"localhost", "augur", "none", "local", "internal", "test", "example", "invalid"} + if domain in invalid_domains: + return False + for suffix in [".local", ".internal", ".lan", ".dhcp.missouri.edu"]: + if domain.endswith(suffix): + return False + return True + + def fetch_username_from_email(logger, auth, commit) -> dict | None: """Try every distinct email found within a commit for possible username resolution. Add email to garbage table if can't be resolved. @@ -283,9 +305,9 @@ def fetch_username_from_email(logger, auth, commit) -> dict | None: logger.info(f"Here is the commit: {commit}") email_raw = commit.get('email_raw') - if not email_raw or not isinstance(email_raw, str) or len(email_raw.strip()) <= 2: - logger.warning("Commit does not contain a valid 'email_raw' value.") - return login_json # Don't bother with emails that are blank or less than 2 characters + if not is_valid_searchable_email(email_raw): + logger.warning(f"Commit contains non-searchable or local email format '{email_raw}'. Skipping API lookup.") + return login_json try: url = create_endpoint_from_email(email_raw) diff --git a/collectoss/tasks/github/facade_github/tasks.py b/collectoss/tasks/github/facade_github/tasks.py index cc380d497..4a1149653 100644 --- a/collectoss/tasks/github/facade_github/tasks.py +++ b/collectoss/tasks/github/facade_github/tasks.py @@ -11,6 +11,7 @@ from collectoss.tasks.git.util.facade_worker.facade_worker.facade00mainprogram import * from collectoss.application.db.lib import bulk_insert_dicts from collectoss.application.db.data_parse import extract_needed_contributor_data as extract_github_contributor +from collectoss.tasks.github.facade_github.contributor_interfaceable.contributor_interface import is_valid_searchable_email @@ -46,6 +47,15 @@ def process_commit_metadata(logger, auth, contributorQueue, repo_id, platform_id logger.debug(f"Commit data with email {email} has been unresolved in the past, skipping...") continue + if not is_valid_searchable_email(email): + logger.debug(f"Email '{email}' is non-searchable or local format. Marking as unresolved and skipping...") + unresolved = {"email": email, "name": name} + try: + bulk_insert_dicts(logger, unresolved, UnresolvedCommitEmail, ['email']) + except Exception as e: + logger.error(f"Could not insert non-searchable email {email} into unresolved_commit_emails: {e}") + continue + login = None #Check the contributors table for a login for the given name diff --git a/tests/test_tasks/test_git/test_repo_size_limit.py b/tests/test_tasks/test_git/test_repo_size_limit.py index c2d6134f0..afd85db8d 100644 --- a/tests/test_tasks/test_git/test_repo_size_limit.py +++ b/tests/test_tasks/test_git/test_repo_size_limit.py @@ -63,6 +63,18 @@ def test_api_error_fallback(self, mock_get_resource): self.assertIsNone(reported_kb) self.assertIsNone(estimated_kb) + def test_is_valid_searchable_email(self): + from collectoss.tasks.github.facade_github.contributor_interfaceable.contributor_interface import is_valid_searchable_email + self.assertTrue(is_valid_searchable_email("user@example.org")) + self.assertTrue(is_valid_searchable_email("john.doe@company.co.uk")) + + self.assertFalse(is_valid_searchable_email("root@augur")) + self.assertFalse(is_valid_searchable_email("michaelwoodruff@mwc-021001.dhcp.missouri.edu")) + self.assertFalse(is_valid_searchable_email("user@localhost")) + self.assertFalse(is_valid_searchable_email("invalid_email")) + self.assertFalse(is_valid_searchable_email("")) + self.assertFalse(is_valid_searchable_email(None)) + if __name__ == "__main__": unittest.main() From 4af28298e8c88dd97520f1b64d551a1faa80ffc8 Mon Sep 17 00:00:00 2001 From: Diptesh Roy Date: Sun, 6 Sep 2026 02:52:38 +0530 Subject: [PATCH 3/3] refactor: remove safety margin, use raw forge-reported size directly Per MoralCode's review feedback, the safety margin concept was confusing because the GitHub/GitLab API size field is already an approximation (measured from a bare repo, not an actual clone). Adding a multiplier on top of an already-inaccurate value made the limit unpredictable. Simplified to compare the raw reported size directly against max_clone_size_kb. Users set the limit knowing it's an approximation of the bare repo size, which is the most honest and predictable behavior. Also removes clone_size_safety_margin from config and FacadeHelper. Signed-off-by: Diptesh Roy --- collectoss/application/config.py | 1 - .../facade_worker/facade_worker/config.py | 1 - .../facade_worker/facade_worker/repofetch.py | 32 +++++++++++-------- .../test_git/test_repo_size_limit.py | 29 +++++++---------- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/collectoss/application/config.py b/collectoss/application/config.py index 8375ec29f..66668ebff 100644 --- a/collectoss/application/config.py +++ b/collectoss/application/config.py @@ -59,7 +59,6 @@ def redact_setting_value(section_name, setting_name, value): "run_facade_contributors": 1, "commit_messages": 1, "max_clone_size_kb": 0, - "clone_size_safety_margin": 0.5, }, "Server": { "cache_expire": "3600", diff --git a/collectoss/tasks/git/util/facade_worker/facade_worker/config.py b/collectoss/tasks/git/util/facade_worker/facade_worker/config.py index ca4299607..32cf6ee15 100644 --- a/collectoss/tasks/git/util/facade_worker/facade_worker/config.py +++ b/collectoss/tasks/git/util/facade_worker/facade_worker/config.py @@ -135,7 +135,6 @@ def __init__(self,logger: Logger): self.create_xlsx_summary_files = worker_options["create_xlsx_summary_files"] self.commit_messages = worker_options["commit_messages"] self.max_clone_size_kb = int(worker_options.get("max_clone_size_kb", 0)) - self.clone_size_safety_margin = float(worker_options.get("clone_size_safety_margin", 0.5)) self.tool_source = "Facade" self.data_source = "Git Log" diff --git a/collectoss/tasks/git/util/facade_worker/facade_worker/repofetch.py b/collectoss/tasks/git/util/facade_worker/facade_worker/repofetch.py index b4544d979..474b67ccf 100644 --- a/collectoss/tasks/git/util/facade_worker/facade_worker/repofetch.py +++ b/collectoss/tasks/git/util/facade_worker/facade_worker/repofetch.py @@ -45,13 +45,22 @@ class GitCloneError(Exception): pass -def check_repo_size_limit(repo_git: str, max_clone_size_kb: int, clone_size_safety_margin: float = 0.5, logger=None): +def check_repo_size_limit(repo_git: str, max_clone_size_kb: int, logger=None): """ - Checks if a repository's estimated clone size exceeds max_clone_size_kb. - Returns (allowed: bool, reported_size_kb: Optional[int], estimated_size_kb: Optional[float]). + Checks if a repository's reported size exceeds max_clone_size_kb. + + Uses the forge API's reported size (GitHub: size field in KB, GitLab: + repository_size in bytes) as a best-effort estimate. Note that forge-reported + sizes are measured from a bare repo and may differ from the actual on-disk + size after a full clone. Users should set max_clone_size_kb with this in mind. + + If the size cannot be determined (API error, unsupported forge), cloning is + allowed to proceed. + + Returns (allowed: bool, reported_size_kb: Optional[int]). """ if not max_clone_size_kb or max_clone_size_kb <= 0: - return True, None, None + return True, None reported_size_kb = None @@ -86,14 +95,12 @@ def check_repo_size_limit(repo_git: str, max_clone_size_kb: int, clone_size_safe except Exception as e: if logger: logger.warning(f"Could not retrieve repo size for {repo_git} via API: {e}") - return True, None, None + return True, None - if reported_size_kb is not None: - estimated_size_kb = reported_size_kb * (1.0 + float(clone_size_safety_margin)) - if estimated_size_kb > max_clone_size_kb: - return False, reported_size_kb, estimated_size_kb + if reported_size_kb is not None and reported_size_kb > max_clone_size_kb: + return False, reported_size_kb - return True, reported_size_kb, (reported_size_kb * (1.0 + float(clone_size_safety_margin))) if reported_size_kb is not None else None + return True, reported_size_kb def git_repo_initialize(facade_helper, session, repo_git): @@ -181,11 +188,10 @@ def git_repo_initialize(facade_helper, session, repo_git): return max_limit = getattr(facade_helper, 'max_clone_size_kb', 0) - safety_margin = getattr(facade_helper, 'clone_size_safety_margin', 0.5) if max_limit > 0: - allowed, reported_kb, estimated_kb = check_repo_size_limit(git, max_limit, safety_margin, logger) + allowed, reported_kb = check_repo_size_limit(git, max_limit, logger) if not allowed: - msg = f"Repo '{git}' estimated clone size ({estimated_kb:.0f} KB) exceeds maximum clone size limit ({max_limit} KB)" + msg = f"Repo '{git}' reported size ({reported_kb} KB) exceeds maximum clone size limit ({max_limit} KB)" update_repo_log(logger, facade_helper, row.repo_id, 'Failed (size limit)') facade_helper.log_activity('Error', msg) raise GitCloneError(msg) diff --git a/tests/test_tasks/test_git/test_repo_size_limit.py b/tests/test_tasks/test_git/test_repo_size_limit.py index afd85db8d..60dd813c9 100644 --- a/tests/test_tasks/test_git/test_repo_size_limit.py +++ b/tests/test_tasks/test_git/test_repo_size_limit.py @@ -14,54 +14,49 @@ def tearDown(self): self.github_access_patcher.stop() def test_size_limit_disabled(self): - allowed, reported_kb, estimated_kb = check_repo_size_limit("https://github.com/chaoss/CollectOSS", 0) + allowed, reported_kb = check_repo_size_limit("https://github.com/chaoss/CollectOSS", 0) self.assertTrue(allowed) self.assertIsNone(reported_kb) - self.assertIsNone(estimated_kb) @patch("collectoss.tasks.github.util.github_data_access.GithubDataAccess.get_resource") def test_github_repo_under_limit(self, mock_get_resource): mock_get_resource.return_value = {"size": 1000} - allowed, reported_kb, estimated_kb = check_repo_size_limit( - "https://github.com/chaoss/CollectOSS", max_clone_size_kb=2000, clone_size_safety_margin=0.5 + allowed, reported_kb = check_repo_size_limit( + "https://github.com/chaoss/CollectOSS", max_clone_size_kb=2000 ) self.assertTrue(allowed) self.assertEqual(reported_kb, 1000) - self.assertEqual(estimated_kb, 1500.0) @patch("collectoss.tasks.github.util.github_data_access.GithubDataAccess.get_resource") def test_github_repo_exceeds_limit(self, mock_get_resource): - mock_get_resource.return_value = {"size": 2000} - allowed, reported_kb, estimated_kb = check_repo_size_limit( - "https://github.com/chaoss/CollectOSS", max_clone_size_kb=2500, clone_size_safety_margin=0.5 + mock_get_resource.return_value = {"size": 2001} + allowed, reported_kb = check_repo_size_limit( + "https://github.com/chaoss/CollectOSS", max_clone_size_kb=2000 ) self.assertFalse(allowed) - self.assertEqual(reported_kb, 2000) - self.assertEqual(estimated_kb, 3000.0) + self.assertEqual(reported_kb, 2001) @patch("httpx.get") def test_gitlab_repo_exceeds_limit(self, mock_httpx_get): mock_response = MagicMock() mock_response.status_code = 200 - mock_response.json.return_value = {"statistics": {"repository_size": 5242880}} # 5120 KB + mock_response.json.return_value = {"statistics": {"repository_size": 5242880}} # 5120 KB mock_httpx_get.return_value = mock_response - allowed, reported_kb, estimated_kb = check_repo_size_limit( - "https://gitlab.com/group/project", max_clone_size_kb=5000, clone_size_safety_margin=0.5 + allowed, reported_kb = check_repo_size_limit( + "https://gitlab.com/group/project", max_clone_size_kb=5000 ) self.assertFalse(allowed) self.assertEqual(reported_kb, 5120) - self.assertEqual(estimated_kb, 7680.0) @patch("collectoss.tasks.github.util.github_data_access.GithubDataAccess.get_resource") def test_api_error_fallback(self, mock_get_resource): mock_get_resource.side_effect = Exception("API rate limit exceeded") - allowed, reported_kb, estimated_kb = check_repo_size_limit( - "https://github.com/chaoss/CollectOSS", max_clone_size_kb=1000, clone_size_safety_margin=0.5 + allowed, reported_kb = check_repo_size_limit( + "https://github.com/chaoss/CollectOSS", max_clone_size_kb=1000 ) self.assertTrue(allowed) self.assertIsNone(reported_kb) - self.assertIsNone(estimated_kb) def test_is_valid_searchable_email(self): from collectoss.tasks.github.facade_github.contributor_interfaceable.contributor_interface import is_valid_searchable_email