From aba048a3e983074b2a0fba0d3e304c00cd090d79 Mon Sep 17 00:00:00 2001 From: Rodos Date: Sun, 7 Dec 2025 21:20:54 +1100 Subject: [PATCH 1/5] fix: warn when --private used without authentication --- bin/github-backup | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bin/github-backup b/bin/github-backup index d685bc9..dcac622 100755 --- a/bin/github-backup +++ b/bin/github-backup @@ -9,6 +9,7 @@ from github_backup.github_backup import ( backup_repositories, check_git_lfs_install, filter_repositories, + get_auth, get_authenticated_user, logger, mkdir_p, @@ -37,6 +38,12 @@ logging.basicConfig(level=logging.INFO, handlers=[stdout_handler, stderr_handler def main(): args = parse_args() + if args.private and not get_auth(args): + logger.warning( + "The --private flag has no effect without authentication. " + "Use -t/--token, -f/--token-fine, or -u/--username to authenticate." + ) + if args.quiet: logger.setLevel(logging.WARNING) From 6e2a7e521ca1e9b8aae58bbe4eaebbb107d828bb Mon Sep 17 00:00:00 2001 From: Rodos Date: Sun, 7 Dec 2025 21:21:14 +1100 Subject: [PATCH 2/5] fix: --all-starred now clones repos without --repositories --- github_backup/github_backup.py | 14 ++- tests/test_all_starred.py | 161 +++++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 tests/test_all_starred.py diff --git a/github_backup/github_backup.py b/github_backup/github_backup.py index cdb536d..bbacdae 100644 --- a/github_backup/github_backup.py +++ b/github_backup/github_backup.py @@ -561,7 +561,7 @@ def get_github_host(args): def read_file_contents(file_uri): - return open(file_uri[len(FILE_URI_PREFIX) :], "rt").readline().strip() + return open(file_uri[len(FILE_URI_PREFIX):], "rt").readline().strip() def get_github_repo_url(args, repository): @@ -1672,9 +1672,10 @@ def backup_repositories(args, output_directory, repositories): repo_url = get_github_repo_url(args, repository) include_gists = args.include_gists or args.include_starred_gists + include_starred = args.all_starred and repository.get("is_starred") if (args.include_repository or args.include_everything) or ( include_gists and repository.get("is_gist") - ): + ) or include_starred: repo_name = ( repository.get("name") if not repository.get("is_gist") @@ -2023,12 +2024,9 @@ def fetch_repository( ): if bare_clone: if os.path.exists(local_dir): - clone_exists = ( - subprocess.check_output( - ["git", "rev-parse", "--is-bare-repository"], cwd=local_dir - ) - == b"true\n" - ) + clone_exists = subprocess.check_output( + ["git", "rev-parse", "--is-bare-repository"], cwd=local_dir + ) == b"true\n" else: clone_exists = False else: diff --git a/tests/test_all_starred.py b/tests/test_all_starred.py new file mode 100644 index 0000000..f59a67e --- /dev/null +++ b/tests/test_all_starred.py @@ -0,0 +1,161 @@ +"""Tests for --all-starred flag behavior (issue #225).""" + +import pytest +from unittest.mock import Mock, patch + +from github_backup import github_backup + + +class TestAllStarredCloning: + """Test suite for --all-starred repository cloning behavior. + + Issue #225: --all-starred should clone starred repos without requiring --repositories. + """ + + def _create_mock_args(self, **overrides): + """Create a mock args object with sensible defaults.""" + args = Mock() + args.user = "testuser" + args.output_directory = "/tmp/backup" + args.include_repository = False + args.include_everything = False + args.include_gists = False + args.include_starred_gists = False + args.all_starred = False + args.skip_existing = False + args.bare_clone = False + args.lfs_clone = False + args.no_prune = False + args.include_wiki = False + args.include_issues = False + args.include_issue_comments = False + args.include_issue_events = False + args.include_pulls = False + args.include_pull_comments = False + args.include_pull_commits = False + args.include_pull_details = False + args.include_labels = False + args.include_hooks = False + args.include_milestones = False + args.include_releases = False + args.include_assets = False + args.include_attachments = False + args.incremental = False + args.incremental_by_files = False + args.github_host = None + args.prefer_ssh = False + args.token_classic = None + args.token_fine = None + args.username = None + args.password = None + args.as_app = False + args.osx_keychain_item_name = None + args.osx_keychain_item_account = None + + for key, value in overrides.items(): + setattr(args, key, value) + + return args + + @patch('github_backup.github_backup.fetch_repository') + @patch('github_backup.github_backup.get_github_repo_url') + def test_all_starred_clones_without_repositories_flag(self, mock_get_url, mock_fetch): + """--all-starred should clone starred repos without --repositories flag. + + This is the core fix for issue #225. + """ + args = self._create_mock_args(all_starred=True) + mock_get_url.return_value = "https://github.com/otheruser/awesome-project.git" + + # A starred repository (is_starred flag set by retrieve_repositories) + starred_repo = { + "name": "awesome-project", + "full_name": "otheruser/awesome-project", + "owner": {"login": "otheruser"}, + "private": False, + "fork": False, + "has_wiki": False, + "is_starred": True, # This flag is set for starred repos + } + + with patch('github_backup.github_backup.mkdir_p'): + github_backup.backup_repositories(args, "/tmp/backup", [starred_repo]) + + # fetch_repository should be called for the starred repo + assert mock_fetch.called, "--all-starred should trigger repository cloning" + mock_fetch.assert_called_once() + call_args = mock_fetch.call_args + assert call_args[0][0] == "awesome-project" # repo name + + @patch('github_backup.github_backup.fetch_repository') + @patch('github_backup.github_backup.get_github_repo_url') + def test_starred_repo_not_cloned_without_all_starred_flag(self, mock_get_url, mock_fetch): + """Starred repos should NOT be cloned if --all-starred is not set.""" + args = self._create_mock_args(all_starred=False) + mock_get_url.return_value = "https://github.com/otheruser/awesome-project.git" + + starred_repo = { + "name": "awesome-project", + "full_name": "otheruser/awesome-project", + "owner": {"login": "otheruser"}, + "private": False, + "fork": False, + "has_wiki": False, + "is_starred": True, + } + + with patch('github_backup.github_backup.mkdir_p'): + github_backup.backup_repositories(args, "/tmp/backup", [starred_repo]) + + # fetch_repository should NOT be called + assert not mock_fetch.called, "Starred repos should not be cloned without --all-starred" + + @patch('github_backup.github_backup.fetch_repository') + @patch('github_backup.github_backup.get_github_repo_url') + def test_non_starred_repo_not_cloned_with_only_all_starred(self, mock_get_url, mock_fetch): + """Non-starred repos should NOT be cloned when only --all-starred is set.""" + args = self._create_mock_args(all_starred=True) + mock_get_url.return_value = "https://github.com/testuser/my-project.git" + + # A regular (non-starred) repository + regular_repo = { + "name": "my-project", + "full_name": "testuser/my-project", + "owner": {"login": "testuser"}, + "private": False, + "fork": False, + "has_wiki": False, + # No is_starred flag + } + + with patch('github_backup.github_backup.mkdir_p'): + github_backup.backup_repositories(args, "/tmp/backup", [regular_repo]) + + # fetch_repository should NOT be called for non-starred repos + assert not mock_fetch.called, "Non-starred repos should not be cloned with only --all-starred" + + @patch('github_backup.github_backup.fetch_repository') + @patch('github_backup.github_backup.get_github_repo_url') + def test_repositories_flag_still_works(self, mock_get_url, mock_fetch): + """--repositories flag should still clone repos as before.""" + args = self._create_mock_args(include_repository=True) + mock_get_url.return_value = "https://github.com/testuser/my-project.git" + + regular_repo = { + "name": "my-project", + "full_name": "testuser/my-project", + "owner": {"login": "testuser"}, + "private": False, + "fork": False, + "has_wiki": False, + } + + with patch('github_backup.github_backup.mkdir_p'): + github_backup.backup_repositories(args, "/tmp/backup", [regular_repo]) + + # fetch_repository should be called + assert mock_fetch.called, "--repositories should trigger repository cloning" + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) From 58ad1c2378691802dbdf9e23d2137ea73bcc4690 Mon Sep 17 00:00:00 2001 From: Rodos Date: Sun, 7 Dec 2025 21:21:26 +1100 Subject: [PATCH 3/5] docs: fix RST formatting in Known blocking errors section --- README.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.rst b/README.rst index a33db61..9fd35fd 100644 --- a/README.rst +++ b/README.rst @@ -281,11 +281,11 @@ If the incremental argument is used, this will result in the next backup only re It's therefore recommended to only use the incremental argument if the output/result is being actively monitored, or complimented with periodic full non-incremental runs, to avoid unexpected missing data in a regular backup runs. -1. **Starred public repo hooks blocking** +**Starred public repo hooks blocking** - Since the ``--all`` argument includes ``--hooks``, if you use ``--all`` and ``--all-starred`` together to clone a users starred public repositories, the backup will likely error and block the backup continuing. +Since the ``--all`` argument includes ``--hooks``, if you use ``--all`` and ``--all-starred`` together to clone a users starred public repositories, the backup will likely error and block the backup continuing. - This is due to needing the correct permission for ``--hooks`` on public repos. +This is due to needing the correct permission for ``--hooks`` on public repos. "bare" is actually "mirror" From b80049e96e5d57e869203e09dc9db1e39329c68c Mon Sep 17 00:00:00 2001 From: Rodos Date: Sun, 7 Dec 2025 21:21:37 +1100 Subject: [PATCH 4/5] test: add missing test coverage for case sensitivity fix --- tests/test_case_sensitivity.py | 112 +++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 tests/test_case_sensitivity.py diff --git a/tests/test_case_sensitivity.py b/tests/test_case_sensitivity.py new file mode 100644 index 0000000..1398d0d --- /dev/null +++ b/tests/test_case_sensitivity.py @@ -0,0 +1,112 @@ +"""Tests for case-insensitive username/organization filtering.""" + +import pytest +from unittest.mock import Mock + +from github_backup import github_backup + + +class TestCaseSensitivity: + """Test suite for case-insensitive username matching in filter_repositories.""" + + def test_filter_repositories_case_insensitive_user(self): + """Should filter repositories case-insensitively for usernames. + + Reproduces issue #198 where typing 'iamrodos' fails to match + repositories with owner.login='Iamrodos' (the canonical case from GitHub API). + """ + # Simulate user typing lowercase username + args = Mock() + args.user = "iamrodos" # lowercase (what user typed) + args.repository = None + args.name_regex = None + args.languages = None + args.exclude = None + args.fork = False + args.private = False + args.public = False + args.all = True + + # Simulate GitHub API returning canonical case + repos = [ + { + "name": "repo1", + "owner": {"login": "Iamrodos"}, # Capital I (canonical from API) + "private": False, + "fork": False, + }, + { + "name": "repo2", + "owner": {"login": "Iamrodos"}, + "private": False, + "fork": False, + }, + ] + + filtered = github_backup.filter_repositories(args, repos) + + # Should match despite case difference + assert len(filtered) == 2 + assert filtered[0]["name"] == "repo1" + assert filtered[1]["name"] == "repo2" + + def test_filter_repositories_case_insensitive_org(self): + """Should filter repositories case-insensitively for organizations. + + Tests the example from issue #198 where 'prai-org' doesn't match 'PRAI-Org'. + """ + args = Mock() + args.user = "prai-org" # lowercase (what user typed) + args.repository = None + args.name_regex = None + args.languages = None + args.exclude = None + args.fork = False + args.private = False + args.public = False + args.all = True + + repos = [ + { + "name": "repo1", + "owner": {"login": "PRAI-Org"}, # Different case (canonical from API) + "private": False, + "fork": False, + }, + ] + + filtered = github_backup.filter_repositories(args, repos) + + # Should match despite case difference + assert len(filtered) == 1 + assert filtered[0]["name"] == "repo1" + + def test_filter_repositories_case_variations(self): + """Should handle various case combinations correctly.""" + args = Mock() + args.user = "TeSt-UsEr" # Mixed case + args.repository = None + args.name_regex = None + args.languages = None + args.exclude = None + args.fork = False + args.private = False + args.public = False + args.all = True + + repos = [ + {"name": "repo1", "owner": {"login": "test-user"}, "private": False, "fork": False}, + {"name": "repo2", "owner": {"login": "TEST-USER"}, "private": False, "fork": False}, + {"name": "repo3", "owner": {"login": "TeSt-UsEr"}, "private": False, "fork": False}, + {"name": "repo4", "owner": {"login": "other-user"}, "private": False, "fork": False}, + ] + + filtered = github_backup.filter_repositories(args, repos) + + # Should match first 3 (all case variations of same user) + assert len(filtered) == 3 + assert set(r["name"] for r in filtered) == {"repo1", "repo2", "repo3"} + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) From 1d6d474408968f728b11aa50c55ec9bb7ddf068e Mon Sep 17 00:00:00 2001 From: Rodos Date: Sun, 7 Dec 2025 21:50:49 +1100 Subject: [PATCH 5/5] fix: improve error messages for inaccessible repos and empty wikis --- github_backup/github_backup.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/github_backup/github_backup.py b/github_backup/github_backup.py index bbacdae..0282809 100644 --- a/github_backup/github_backup.py +++ b/github_backup/github_backup.py @@ -2041,11 +2041,14 @@ def fetch_repository( "git ls-remote " + remote_url, stdout=FNULL, stderr=FNULL, shell=True ) if initialized == 128: - logger.info( - "Skipping {0} ({1}) since it's not initialized".format( - name, masked_remote_url + if ".wiki.git" in remote_url: + logger.info( + "Skipping {0} wiki (wiki is enabled but has no content)".format(name) + ) + else: + logger.info( + "Skipping {0} (repository not accessible - may be empty, private, or credentials invalid)".format(name) ) - ) return if clone_exists: