rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-26 19:01:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

repos: submodule links must stay in the repo; import checks before it moves

Browse source 
* A submodule's working-tree `.git` names the git directory git uses for
  it.  Before any submodule command (update, or deinit when submodules
  are turned off), and before an import, every `.git` below the top of
  the working tree must be a file pointing inside the repo's own
  .git/modules; a link, a directory, or a file pointing elsewhere could
  make git use a config that was never checked.
* Import validates and sanitizes the clone where it is, then moves it,
  and moves it back if marking it fails, so a refused or failed import
  never leaves a clone moved without a Repo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-10-06 19:03:07 -06:00
parent d0ea6ab313
commit 4b034ae3ee
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
4 changed files with 141 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

6
modules/repos/downloader.py
View file

@ -13,7 +13,7 @@ from wrolpi.downloader import Downloader, Download, DownloadContext, DownloadRes
from wrolpi.errors import DownloadError
from .lib import git_command, git_env, find_readme, get_repo_size, is_valid_branch, read_readme_text, \
submodule_fetch_config, resolve_repo_addresses, pin_config, HostAddresses, clone_ownership_error, mark_clone, \
sanitize_git_config, new_clone_token
sanitize_git_config, new_clone_token, working_tree_git_error
from .models import Repository
__all__ = ['GitDownloader', 'git_downloader', 'PreparedRepo', 'ExecutedRepo', 'parse_git_progress']
@ -185,6 +185,10 @@ class GitDownloader(Downloader):
git commands that git runs)."""
directory = prepared.directory
git = lambda *args, **kwargs: self._git(download, ctx, directory, *args, **kwargs) # noqa: E731
# Every submodule command uses the git directory a submodule's `.git` names; it must be one that was checked.
if (error := await asyncio.to_thread(working_tree_git_error, directory)) and (
prepared.submodules or (directory / '.git/modules').is_dir()):
return error
if not prepared.submodules:
if (directory / '.git/modules').is_dir():
# Submodules were turned off; the repo is a mirror of only its own files now.

71
modules/repos/lib.py
View file

@ -37,14 +37,14 @@ from .models import Repository, REPO_MODES
logger = logger.getChild(__name__)
__all__ = ['ParsedRepoURL', 'parse_repo_url', 'repo_url_key', 'git_command', 'git_env', 'REPO_MARKER',
'clone_ownership_error', 'mark_clone', 'sanitize_git_config', 'create_repository', 'remote_url_key',
'remote_https_url', 'resolve_import_directory', 'inspect_import', 'import_repository',
'update_repository', 'delete_repository', 'get_repositories', 'list_repo_tree', 'find_readme',
'repos_destination_root', 'validate_repos_destination', 'is_valid_branch', 'submodule_url_error',
'HostAddresses', 'resolve_repo_addresses', 'pin_config', 'is_public_address', 'submodule_fetch_config',
'read_readme_text', 'search_repos', 'count_repos', 'search_repos_by_name', 'get_repo_log',
'get_repo_root', 'repo_archive_name', 'stream_repo_archive', 'ReposConfig', 'repos_config',
'get_repos_config', 'save_repos_config', 'import_repos_config', 'DEFAULT_REPO_FREQUENCY']
'clone_ownership_error', 'mark_clone', 'sanitize_git_config', 'working_tree_git_error',
'create_repository', 'remote_url_key', 'remote_https_url', 'resolve_import_directory', 'inspect_import',
'import_repository', 'update_repository', 'delete_repository', 'get_repositories', 'list_repo_tree',
'find_readme', 'repos_destination_root', 'validate_repos_destination', 'is_valid_branch',
'submodule_url_error', 'HostAddresses', 'resolve_repo_addresses', 'pin_config', 'is_public_address',
'submodule_fetch_config', 'read_readme_text', 'search_repos', 'count_repos', 'search_repos_by_name',
'get_repo_log', 'get_repo_root', 'repo_archive_name', 'stream_repo_archive', 'ReposConfig',
'repos_config', 'get_repos_config', 'save_repos_config', 'import_repos_config', 'DEFAULT_REPO_FREQUENCY']
DEFAULT_REPO_FREQUENCY = DownloadFrequency.weekly.value
@ -472,6 +472,35 @@ def _sanitize_submodule_git_dirs(modules: pathlib.Path, root: pathlib.Path):
_sanitize_submodule_git_dirs(child / 'modules', root)
def working_tree_git_error(directory: pathlib.Path) -> Optional[str]:
"""Why a `.git` in the repo's working tree (below its top) must not be used, or None.
A submodule's `.git` file names the git directory git uses for it. It must point inside the repo's own
.git/modules, which is what `sanitize_git_config` checks; a link, a directory, or a file pointing elsewhere could
make git use a config which was never checked."""
root = directory.resolve()
modules = root / '.git' / 'modules'
for dir_path, dir_names, file_names in os.walk(root):
here = pathlib.Path(dir_path)
if '.git' in dir_names:
dir_names.remove('.git') # Never walk into a git directory.
if here != root:
return f'{here / ".git"} is a directory, not a submodule\'s link'
continue
if '.git' not in file_names:
continue
git_file = here / '.git'
if git_file.is_symlink():
return f'{git_file} is a link'
content = git_file.read_text(errors='replace')[:4096].strip()
if not content.startswith('gitdir: ') or '\n' in content:
return f'{git_file} is not a submodule\'s link'
target = (here / content[len('gitdir: '):]).resolve()
if modules not in target.parents:
return f'{git_file} points outside the repo\'s submodules'
return None
def find_readme(directory: pathlib.Path) -> Optional[str]:
"""Return the name of the preferred README file in `directory`, if any."""
try:
@ -869,22 +898,38 @@ async def import_repository(session: Session, directory: str, url: str = None, c
repo = _create_repository_records(session, parsed.url, tag_name, mode, branch, info['name'], description, None,
submodules)
destination = repo.directory
if destination.resolve() != source:
moving = destination.resolve() != source
if moving:
if source in destination.resolve().parents:
raise InvalidRepo('A repo cannot be moved into itself')
if destination.exists():
raise RepoConflict(f'{get_relative_to_media_directory(destination)} already exists')
# Everything which can refuse the import happens before the clone is moved.
if error := await asyncio.to_thread(working_tree_git_error, source):
raise InvalidRepo(f'{directory} cannot be imported: {error}')
try:
await asyncio.to_thread(sanitize_git_config, source)
except ValueError as e:
raise InvalidRepo(f'{directory} cannot be imported: {e}')
await asyncio.to_thread(_git_output_sync, source, 'config', '--replace-all', 'remote.origin.url', parsed.url)
if moving:
destination.parent.mkdir(parents=True, exist_ok=True)
await asyncio.to_thread(shutil.move, source, destination)
logger.info(f'Imported repo moved from {source} to {destination}')
try:
mark_clone(destination, repo.clone_token)
except BaseException:
if moving:
# Leave the clone where it was found.
await asyncio.to_thread(shutil.move, destination, source)
raise
if moving:
# Forget the files that were indexed in the old location.
from wrolpi.files.worker import file_worker
file_worker.queue_refresh([source])
await asyncio.to_thread(sanitize_git_config, destination)
await asyncio.to_thread(_git_output_sync, destination, 'config', '--replace-all', 'remote.origin.url', parsed.url)
mark_clone(destination, repo.clone_token)
for key, value in _read_head_commit(destination).items():
setattr(repo, key, value)
repo.readme_path = find_readme(destination)

30
modules/repos/test/test_import.py
View file

@ -207,3 +207,33 @@ async def test_import_refuses(test_session, test_directory, async_client, test_w
repo = await lib.import_repository(test_session, 'code/ssh-clone', url='https://git.example.com/owner/library',
confirm=True)
assert repo.url == 'https://git.example.com/owner/library'
@pytest.mark.asyncio
async def test_import_checks_before_moving(test_session, test_directory, async_client, test_wrolpi_config,
user_clone):
"""Everything which can refuse an import is checked before the clone is moved."""
directory, _ = user_clone()
(directory / 'other').mkdir()
(directory / 'other/.git').write_text(f'gitdir: {test_directory}\n')
with pytest.raises(InvalidRepo, match='outside'):
await lib.import_repository(test_session, 'code/kiwix-tools', confirm=True)
test_session.rollback()
assert (directory / '.git').is_dir()
assert not (test_directory / 'repos/kiwix-tools').exists()
assert test_session.query(Repository).count() == 0
@pytest.mark.asyncio
async def test_import_moves_back_on_failure(test_session, test_directory, async_client, test_wrolpi_config,
user_clone):
"""If the import fails after the move, the clone is moved back."""
from mock import mock
directory, _ = user_clone()
with mock.patch('modules.repos.lib.mark_clone', side_effect=OSError('disk full')):
with pytest.raises(OSError):
await lib.import_repository(test_session, 'code/kiwix-tools', confirm=True)
test_session.rollback()
assert (directory / '.git').is_dir()
assert not (test_directory / 'repos/kiwix-tools').exists()
assert test_session.query(Repository).count() == 0

48
modules/repos/test/test_ownership.py
View file

@ -398,3 +398,51 @@ async def test_clone_token_survives_the_config(test_session, async_client, git_r
download = await update(test_session, repos_download_manager)
assert download.status == DownloadStatus.complete, download.error
assert test_session.query(lib.Repository).one().head_sha == head
@pytest.mark.parametrize('plant', ['outside', 'directory', 'link', 'garbage'])
def test_working_tree_git_files(tmp_path, plant):
"""A submodule's `.git` (in the working tree) decides which git directory git uses for it; it must be a file
pointing inside the repo's own .git/modules."""
repo = tmp_path / 'repo'
(repo / '.git/modules/library').mkdir(parents=True)
(repo / 'vendor/library').mkdir(parents=True)
(repo / 'vendor/library/.git').write_text('gitdir: ../../.git/modules/library\n')
assert lib.working_tree_git_error(repo) is None
evil = tmp_path / 'evil'
evil.mkdir()
(repo / 'other').mkdir()
target = repo / 'other/.git'
if plant == 'outside':
target.write_text(f'gitdir: {evil}\n')
elif plant == 'directory':
target.mkdir()
elif plant == 'link':
target.symlink_to(evil)
elif plant == 'garbage':
target.write_text('not a gitdir line\n')
assert lib.working_tree_git_error(repo)
@pytest.mark.parametrize('submodules', [True, False])
@pytest.mark.asyncio
async def test_update_refuses_planted_submodule_git_file(test_session, test_directory, git_remote_factory,
repos_download_manager, submodules):
"""Submodule commands (update, and deinit when submodules are turned off) never use a git directory outside
the repo."""
library = git_remote_factory('library')
remote = git_remote_factory()
remote.add_submodule('vendor/library', library)
repo = lib.create_repository(test_session, remote.url, submodules=True)
await repos_download_manager.wait_for_all_downloads()
assert (repo.directory / 'vendor/library/lib.c').exists() or (repo.directory / 'vendor/library/README.md').exists()
evil = test_directory / 'evil-gitdir'
evil.mkdir()
(repo.directory / 'vendor/library/.git').write_text(f'gitdir: {evil}\n')
lib.update_repository(test_session, repo.id, submodules=submodules)
download = await update(test_session, repos_download_manager)
assert download.status == DownloadStatus.deferred
assert 'outside' in download.error
assert list(evil.iterdir()) == []