mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
repos: a clone is owned by a random token, not its URL
The ownership marker held the Repo's URL key, which anyone can write, so a copied clone, an extracted archive or a hand-made `.git` passed the check and was reset and cleaned. Each Repo now has a random clone_token (repository column and repos.yaml, so a Repo restored from the config still owns its clone); the marker holds it, and only a clone holding the Repo's token is updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
a1d4c42eee
commit
d0ea6ab313
6 changed files with 116 additions and 20 deletions
22
alembic/versions/2026_10_06_1500_repository_clone_token.py
Normal file
22
alembic/versions/2026_10_06_1500_repository_clone_token.py
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
"""A git Repo's clone holds a random token: repository.clone_token.
|
||||
|
||||
Revision ID: 2026_10_06_1500
|
||||
Revises: 2026_10_06_1400
|
||||
"""
|
||||
import sqlalchemy as sa
|
||||
from alembic import op
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision = '2026_10_06_1500'
|
||||
down_revision = '2026_10_06_1400'
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade():
|
||||
op.add_column('repository', sa.Column('clone_token', sa.String(), nullable=True))
|
||||
|
||||
|
||||
def downgrade():
|
||||
with op.batch_alter_table('repository') as batch_op:
|
||||
batch_op.drop_column('clone_token')
|
||||
|
|
@ -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
|
||||
sanitize_git_config, new_clone_token
|
||||
from .models import Repository
|
||||
|
||||
__all__ = ['GitDownloader', 'git_downloader', 'PreparedRepo', 'ExecutedRepo', 'parse_git_progress']
|
||||
|
|
@ -44,6 +44,7 @@ class PreparedRepo:
|
|||
mode: str
|
||||
branch: Optional[str]
|
||||
submodules: bool = False
|
||||
clone_token: Optional[str] = None
|
||||
# The repo's host, resolved once by execute_download; every fetch from it connects to these addresses.
|
||||
addresses: Optional[HostAddresses] = None
|
||||
|
||||
|
|
@ -85,8 +86,10 @@ class GitDownloader(Downloader):
|
|||
if not repo:
|
||||
raise DownloadError(f'No Repo for this download, add it on the Repos page: {download.url}')
|
||||
directory = repo.collection.get_or_set_directory(session)
|
||||
if not repo.clone_token:
|
||||
repo.clone_token = new_clone_token()
|
||||
return PreparedRepo(repository_id=repo.id, url=repo.url, directory=directory, mode=repo.mode,
|
||||
branch=repo.branch, submodules=repo.submodules)
|
||||
branch=repo.branch, submodules=repo.submodules, clone_token=repo.clone_token)
|
||||
|
||||
async def _git(self, download: Download, ctx: DownloadContext, cwd: pathlib.Path, *args,
|
||||
config: tuple = (), progress: bool = False, check: bool = True,
|
||||
|
|
@ -127,7 +130,7 @@ class GitDownloader(Downloader):
|
|||
args.extend(['--depth', '1', '--single-branch', '--no-tags'])
|
||||
args.extend(['--', prepared.url, str(directory)])
|
||||
await self._git(download, ctx, directory.parent, *args, progress=True, prepared=prepared)
|
||||
mark_clone(directory, prepared.url)
|
||||
mark_clone(directory, prepared.clone_token)
|
||||
|
||||
async def _update(self, download: Download, ctx: DownloadContext, prepared: PreparedRepo, branch: str):
|
||||
directory = prepared.directory
|
||||
|
|
@ -136,7 +139,7 @@ class GitDownloader(Downloader):
|
|||
|
||||
# Only a clone WROLPi made (or imported) for this Repo is reset and cleaned; its config cannot change what
|
||||
# git runs or where it connects, and it is fetched from the Repo's URL, whatever its origin says.
|
||||
if error := clone_ownership_error(directory, prepared.url):
|
||||
if error := clone_ownership_error(directory, prepared.clone_token):
|
||||
raise DownloadError(error)
|
||||
try:
|
||||
await asyncio.to_thread(sanitize_git_config, directory)
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ import ipaddress
|
|||
import os
|
||||
import pathlib
|
||||
import re
|
||||
import hmac
|
||||
import secrets
|
||||
import shutil
|
||||
import socket
|
||||
import stat
|
||||
|
|
@ -316,9 +318,13 @@ def git_env() -> dict:
|
|||
return env
|
||||
|
||||
|
||||
# WROLPi marks the clones it makes (and imports) with this file in `.git`; it holds the Repo's `repo_url_key`.
|
||||
# WROLPi marks the clones it makes (and imports) with this file in `.git`; it holds the Repo's `clone_token`.
|
||||
REPO_MARKER = 'wrolpi-repo'
|
||||
|
||||
|
||||
def new_clone_token() -> str:
|
||||
return secrets.token_hex(16)
|
||||
|
||||
# The only settings kept in a clone's config: anything else (include.path, url.*.insteadOf, http.proxy,
|
||||
# core.sshCommand, ...) could change what git runs or where it connects.
|
||||
ALLOWED_GIT_CONFIG_RE = re.compile(
|
||||
|
|
@ -339,8 +345,8 @@ def _borrowing_git_file(git_dir: pathlib.Path) -> Optional[str]:
|
|||
return None
|
||||
|
||||
|
||||
def clone_ownership_error(directory: pathlib.Path, url: str) -> Optional[str]:
|
||||
"""Why the clone in `directory` must not be updated as the Repo with `url`, or None.
|
||||
def clone_ownership_error(directory: pathlib.Path, clone_token: Optional[str]) -> Optional[str]:
|
||||
"""Why the clone in `directory` must not be updated as the Repo with `clone_token`, or None.
|
||||
|
||||
WROLPi resets and cleans its clones, so it only updates the ones it made (or imported) for this Repo."""
|
||||
git_dir = directory / '.git'
|
||||
|
|
@ -349,19 +355,20 @@ def clone_ownership_error(directory: pathlib.Path, url: str) -> Optional[str]:
|
|||
if borrowed := _borrowing_git_file(git_dir):
|
||||
return f'{directory} uses another git directory (.git/{borrowed})'
|
||||
marker = git_dir / REPO_MARKER
|
||||
if marker.is_symlink() or not marker.is_file() or marker.read_text().strip() != repo_url_key(url):
|
||||
if not clone_token or marker.is_symlink() or not marker.is_file() \
|
||||
or not hmac.compare_digest(marker.read_text()[:100].strip(), clone_token):
|
||||
return f'{directory} was not created by WROLPi. Import it from the Repos page'
|
||||
return None
|
||||
|
||||
|
||||
def mark_clone(directory: pathlib.Path, url: str):
|
||||
def mark_clone(directory: pathlib.Path, clone_token: str):
|
||||
marker = directory / '.git' / REPO_MARKER
|
||||
# The clone may hold a link here (e.g. one being imported); never write through it.
|
||||
if marker.is_symlink():
|
||||
marker.unlink()
|
||||
fd = os.open(marker, os.O_WRONLY | os.O_CREAT | os.O_TRUNC | os.O_NOFOLLOW, 0o644)
|
||||
with os.fdopen(fd, 'w') as fh:
|
||||
fh.write(f'{repo_url_key(url)}\n')
|
||||
fh.write(f'{clone_token}\n')
|
||||
|
||||
|
||||
def _git_config_entries(config_file: pathlib.Path) -> List[Tuple[str, str]]:
|
||||
|
|
@ -653,10 +660,12 @@ def _validate_frequency(frequency: int):
|
|||
|
||||
def create_repository(session: Session, url: str, tag_name: str = None, frequency: int = DEFAULT_REPO_FREQUENCY,
|
||||
mode: str = 'full', branch: str = None, name: str = None, description: str = None,
|
||||
directory: pathlib.Path = None, submodules: bool = False) -> Repository:
|
||||
directory: pathlib.Path = None, submodules: bool = False,
|
||||
clone_token: str = None) -> Repository:
|
||||
"""Create a Repo (Collection + Repository + recurring Download). The download clones it."""
|
||||
_validate_frequency(frequency)
|
||||
repo = _create_repository_records(session, url, tag_name, mode, branch, name, description, directory, submodules)
|
||||
repo = _create_repository_records(session, url, tag_name, mode, branch, name, description, directory, submodules,
|
||||
clone_token)
|
||||
_create_repository_download(session, repo, frequency)
|
||||
return repo
|
||||
|
||||
|
|
@ -674,7 +683,7 @@ def _create_repository_download(session: Session, repo: Repository, frequency: i
|
|||
|
||||
def _create_repository_records(session: Session, url: str, tag_name: str, mode: str, branch: Optional[str],
|
||||
name: Optional[str], description: Optional[str], directory: Optional[pathlib.Path],
|
||||
submodules: bool) -> Repository:
|
||||
submodules: bool, clone_token: str = None) -> Repository:
|
||||
"""Create (but do not commit) a Repo's Collection and Repository, in their directory."""
|
||||
parsed = parse_repo_url(url)
|
||||
_validate_mode(mode)
|
||||
|
|
@ -695,7 +704,8 @@ def _create_repository_records(session: Session, url: str, tag_name: str, mode:
|
|||
session.add(collection)
|
||||
session.flush([collection])
|
||||
repo = Repository(url=parsed.url, host=parsed.host, owner=parsed.owner, mode=mode, branch=branch,
|
||||
submodules=bool(submodules), collection=collection)
|
||||
submodules=bool(submodules), clone_token=clone_token or new_clone_token(),
|
||||
collection=collection)
|
||||
repo.update_search_name()
|
||||
session.add(repo)
|
||||
session.flush([repo])
|
||||
|
|
@ -873,7 +883,7 @@ async def import_repository(session: Session, directory: str, url: str = None, c
|
|||
|
||||
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, parsed.url)
|
||||
mark_clone(destination, repo.clone_token)
|
||||
|
||||
for key, value in _read_head_commit(destination).items():
|
||||
setattr(repo, key, value)
|
||||
|
|
@ -1251,7 +1261,8 @@ class ReposConfig(ConfigFile):
|
|||
if not repo:
|
||||
return create_repository(session, parsed.url, tag_name=tag_name, frequency=frequency, mode=mode,
|
||||
branch=data.get('branch'), name=name, description=data.get('description'),
|
||||
directory=directory, submodules=bool(data.get('submodules')))
|
||||
directory=directory, submodules=bool(data.get('submodules')),
|
||||
clone_token=data.get('clone_token'))
|
||||
|
||||
collection = repo.collection
|
||||
collection.name = name
|
||||
|
|
@ -1262,6 +1273,8 @@ class ReposConfig(ConfigFile):
|
|||
collection.directory = directory
|
||||
repo.mode = mode
|
||||
repo.submodules = bool(data.get('submodules'))
|
||||
if data.get('clone_token'):
|
||||
repo.clone_token = data['clone_token']
|
||||
branch = data.get('branch') or None
|
||||
_validate_branch(branch)
|
||||
repo.branch = branch
|
||||
|
|
@ -1291,6 +1304,8 @@ class ReposConfig(ConfigFile):
|
|||
branch=repo.branch,
|
||||
submodules=repo.submodules,
|
||||
frequency=download.frequency if download else None,
|
||||
# Restoring a Repo from the config keeps its clone.
|
||||
clone_token=repo.clone_token,
|
||||
)
|
||||
data.append(entry)
|
||||
|
||||
|
|
|
|||
|
|
@ -31,6 +31,9 @@ class Repository(ModelHelper, Base):
|
|||
mode = Column(String, nullable=False, default='full')
|
||||
# The branch to check out; None follows the remote's default branch.
|
||||
branch = Column(String)
|
||||
# Random; written in the clone's .git (see lib.mark_clone). Only a clone holding it is updated, so a clone which
|
||||
# WROLPi did not make for this Repo is never reset or cleaned.
|
||||
clone_token = Column(String)
|
||||
# Also download the repo's submodules.
|
||||
submodules = Column(Boolean, nullable=False, default=False, server_default=false())
|
||||
default_branch = Column(String)
|
||||
|
|
|
|||
|
|
@ -16,7 +16,10 @@ async def test_dump_and_import(test_session, test_directory, async_client, test_
|
|||
|
||||
config = lib.get_repos_config()
|
||||
assert config.get_file().is_file()
|
||||
assert config.repos == [
|
||||
# Each Repo's random clone token is kept, so a restored Repo still owns its clone.
|
||||
dumped = [dict(i) for i in config.repos]
|
||||
assert all(len(i.pop('clone_token')) == 32 for i in dumped)
|
||||
assert dumped == [
|
||||
dict(name='b', url='https://github.com/a/b', directory='repos/b', tag_name=None, description=None,
|
||||
mode='snapshot', branch='dev', submodules=False, frequency=DownloadFrequency.weekly),
|
||||
dict(name='kiwix-tools', url='https://github.com/kiwix/kiwix-tools', directory='repos/software/kiwix-tools',
|
||||
|
|
|
|||
|
|
@ -26,7 +26,10 @@ async def test_clone_is_marked(test_session, git_remote_factory, repos_download_
|
|||
remote = git_remote_factory()
|
||||
repo = lib.create_repository(test_session, remote.url)
|
||||
await repos_download_manager.wait_for_all_downloads()
|
||||
assert (repo.directory / '.git' / lib.REPO_MARKER).read_text().strip() == lib.repo_url_key(remote.url)
|
||||
test_session.expire_all()
|
||||
# The marker holds the Repo's own random token, which nothing else can know (unlike its URL).
|
||||
assert len(repo.clone_token) == 32
|
||||
assert (repo.directory / '.git' / lib.REPO_MARKER).read_text().strip() == repo.clone_token
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -178,10 +181,10 @@ def test_marker_and_config_do_not_follow_links(tmp_path):
|
|||
victim.write_text('precious')
|
||||
|
||||
(git_dir / lib.REPO_MARKER).symlink_to(victim)
|
||||
lib.mark_clone(repo, 'https://github.com/a/b')
|
||||
lib.mark_clone(repo, 'a1b2c3')
|
||||
assert victim.read_text() == 'precious'
|
||||
assert not (git_dir / lib.REPO_MARKER).is_symlink()
|
||||
assert (git_dir / lib.REPO_MARKER).read_text().strip() == 'github.com/a/b'
|
||||
assert (git_dir / lib.REPO_MARKER).read_text().strip() == 'a1b2c3'
|
||||
|
||||
(git_dir / 'config').write_text('[http]\n\tproxy = evil\n[core]\n\tbare = false\n')
|
||||
(git_dir / 'config.wrolpi').symlink_to(victim)
|
||||
|
|
@ -348,3 +351,50 @@ def test_nested_submodule_git_directories(tmp_path):
|
|||
(inner / 'commondir').write_text(f'{tmp_path}\n')
|
||||
with pytest.raises(ValueError):
|
||||
lib.sanitize_git_config(repo)
|
||||
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_copied_clone_is_not_owned(test_session, test_directory, git_remote_factory, repos_download_manager):
|
||||
"""A clone with a marker WROLPi did not write (e.g. copied from another WROLPi, or made by hand) is not owned:
|
||||
the marker must hold this Repo's own token."""
|
||||
remote = git_remote_factory()
|
||||
directory = test_directory / 'repos/example'
|
||||
directory.parent.mkdir(parents=True)
|
||||
git(directory.parent, 'clone', '--quiet', str(remote.path), str(directory))
|
||||
# What a marker used to hold, which anyone could write.
|
||||
(directory / '.git' / lib.REPO_MARKER).write_text(lib.repo_url_key(remote.url))
|
||||
(directory / 'my-work.txt').write_text('uncommitted')
|
||||
|
||||
lib.create_repository(test_session, remote.url)
|
||||
await repos_download_manager.wait_for_all_downloads()
|
||||
download = test_session.query(Download).one()
|
||||
assert download.status == DownloadStatus.deferred
|
||||
assert 'was not created by WROLPi' in download.error
|
||||
assert (directory / 'my-work.txt').exists()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_clone_token_survives_the_config(test_session, async_client, git_remote_factory,
|
||||
repos_download_manager, await_switches):
|
||||
"""The token is in repos.yaml, so a Repo restored from the config still owns its clone."""
|
||||
remote = git_remote_factory()
|
||||
repo = lib.create_repository(test_session, remote.url)
|
||||
await repos_download_manager.wait_for_all_downloads()
|
||||
await await_switches()
|
||||
token = repo.clone_token
|
||||
config = lib.get_repos_config()
|
||||
assert config.repos[0]['clone_token'] == token
|
||||
|
||||
# Lose the database, restore it from the config.
|
||||
lib._delete_repository(test_session, repo)
|
||||
test_session.commit()
|
||||
config.import_config()
|
||||
test_session.expire_all()
|
||||
restored = test_session.query(lib.Repository).one()
|
||||
assert restored.clone_token == token
|
||||
|
||||
head = remote.commit({'new.txt': 'new'}, 'New')
|
||||
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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue