repos: pin the repo's host once, and only exempt its own host and port

A submodule on the repo's host was allowed unpinned, on any port.  The
owner of the repo's domain could serve the clone from a public address,
then answer the submodule's lookup with an internal address (DNS
rebinding), or point it at another port on that name.

The repo's host is now resolved once per download, and every fetch
(ls-remote, clone, fetch, submodule update) is pinned to that answer.  A
submodule on the repo's host *and port* is fetched from those same
addresses, still allowing a LAN forge the user chose; another port on
that name is checked like any other host.  Tests no longer use real DNS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-10-06 17:38:45 -06:00
parent 420bd8295b
commit 00d7816cfd
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
4 changed files with 161 additions and 40 deletions

View file

@ -86,8 +86,15 @@ def git_remote_factory(tmp_path_factory) -> Callable[..., GitRemote]:
remotes[url] = path
return remote
def resolve_host_addresses(host):
# The remotes' host has a fixed public address; no test uses real DNS.
if host == 'git.example.com':
return ['93.184.215.14']
raise OSError(f'Unknown host {host}')
with mock.patch('modules.repos.lib.ALLOWED_PROTOCOLS', ('https', 'file')), \
mock.patch('modules.repos.downloader.git_env', git_env):
mock.patch('modules.repos.downloader.git_env', git_env), \
mock.patch('modules.repos.lib.resolve_host_addresses', resolve_host_addresses):
yield factory

View file

@ -4,7 +4,6 @@ import re
from dataclasses import dataclass
from datetime import datetime
from typing import Optional
from urllib.parse import urlparse
from sqlalchemy.orm import Session
@ -13,7 +12,7 @@ from wrolpi.dates import now
from wrolpi.downloader import Downloader, Download, DownloadContext, DownloadResult, make_progress_callback
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
submodule_fetch_config, resolve_repo_addresses, pin_config, HostAddresses
from .models import Repository
__all__ = ['GitDownloader', 'git_downloader', 'PreparedRepo', 'ExecutedRepo', 'parse_git_progress']
@ -44,6 +43,8 @@ class PreparedRepo:
mode: str
branch: Optional[str]
submodules: bool = False
# The repo's host, resolved once by execute_download; every fetch from it connects to these addresses.
addresses: Optional[HostAddresses] = None
@dataclass
@ -87,8 +88,13 @@ class GitDownloader(Downloader):
branch=repo.branch, submodules=repo.submodules)
async def _git(self, download: Download, ctx: DownloadContext, cwd: pathlib.Path, *args,
config: tuple = (), progress: bool = False, check: bool = True) -> str:
"""Run a git command, return its output (stdout and stderr)."""
config: tuple = (), progress: bool = False, check: bool = True,
prepared: PreparedRepo = None) -> str:
"""Run a git command, return its output (stdout and stderr).
Pass `prepared` for a command which connects to the repo's host, so it connects to the resolved addresses."""
if prepared and prepared.addresses and (pin := pin_config(prepared.addresses)):
config = (pin, *config)
cmd = git_command(*args, config=config)
stdout_callback = make_progress_callback(ctx.report_progress, parse_git_progress) if progress else None
result = await self.process_runner(download, cmd, cwd, env=git_env(), merge_stderr=True,
@ -103,7 +109,7 @@ class GitDownloader(Downloader):
async def _default_branch(self, download: Download, ctx: DownloadContext, prepared: PreparedRepo) -> str:
"""Ask the remote for its default branch. This fails quickly when the upstream is gone."""
output = await self._git(download, ctx, prepared.directory.parent, 'ls-remote', '--symref', prepared.url,
'HEAD')
'HEAD', prepared=prepared)
for line in output.splitlines():
if line.startswith('ref: refs/heads/') and line.endswith('\tHEAD'):
return line[len('ref: refs/heads/'):-len('\tHEAD')]
@ -119,11 +125,12 @@ class GitDownloader(Downloader):
if prepared.mode == 'snapshot':
args.extend(['--depth', '1', '--single-branch', '--no-tags'])
args.extend(['--', prepared.url, str(directory)])
await self._git(download, ctx, directory.parent, *args, progress=True)
await self._git(download, ctx, directory.parent, *args, progress=True, prepared=prepared)
async def _update(self, download: Download, ctx: DownloadContext, prepared: PreparedRepo, branch: str):
directory = prepared.directory
git = lambda *args, **kwargs: self._git(download, ctx, directory, *args, **kwargs) # noqa: E731
git = lambda *args, **kwargs: self._git(download, ctx, directory, *args, prepared=prepared, # noqa: E731
**kwargs)
old_head = await git('rev-parse', '--verify', '--quiet', 'HEAD', check=False)
if prepared.mode == 'snapshot':
@ -187,7 +194,8 @@ class GitDownloader(Downloader):
return
if depth >= MAXIMUM_SUBMODULE_DEPTH:
raise DownloadError(f'Submodules are nested more than {MAXIMUM_SUBMODULE_DEPTH} levels deep')
git = lambda *args, **kwargs: self._git(download, ctx, directory, *args, **kwargs) # noqa: E731
git = lambda *args, **kwargs: self._git(download, ctx, directory, *args, prepared=prepared, # noqa: E731
**kwargs)
paths = await self._git_config_values(download, ctx, directory, '.path', '--file', '.gitmodules')
@ -196,9 +204,8 @@ class GitDownloader(Downloader):
await git('submodule', 'init')
await git('submodule', 'sync')
urls = await self._git_config_values(download, ctx, directory, '.url', '--local')
repo_host = (urlparse(prepared.url).hostname or '').lower()
try:
fetch_config = await asyncio.to_thread(submodule_fetch_config, urls, repo_host)
fetch_config = await asyncio.to_thread(submodule_fetch_config, urls, prepared.addresses)
except ValueError as e:
raise DownloadError(str(e))
@ -242,6 +249,9 @@ class GitDownloader(Downloader):
directory = prepared.directory
directory.parent.mkdir(parents=True, exist_ok=True)
# Resolved once; the repo and its same-host submodules are all fetched from these addresses, so a DNS
# answer cannot change in between.
prepared.addresses = await asyncio.to_thread(resolve_repo_addresses, prepared.url)
try:
default_branch = await self._default_branch(download, ctx, prepared)
branch = prepared.branch or default_branch

View file

@ -8,7 +8,7 @@ import shutil
import socket
import stat
from dataclasses import dataclass, field
from typing import List, Optional, Tuple
from typing import List, NamedTuple, Optional, Tuple
from urllib.parse import urlparse
from sqlalchemy import func, text
@ -33,10 +33,11 @@ logger = logger.getChild(__name__)
__all__ = ['ParsedRepoURL', 'parse_repo_url', 'repo_url_key', 'git_command', 'git_env', 'create_repository',
'update_repository', 'delete_repository', 'get_repositories', 'list_repo_tree', 'find_readme',
'repo_destination_variables', 'is_valid_branch', 'submodule_url_error', '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']
'repo_destination_variables', '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
@ -162,8 +163,44 @@ ABSOLUTE_SUBMODULE_URL_RE = re.compile(
r'(?:/[A-Za-z0-9._~%!$&\'()*+,;=:/-]*)?$')
def _check_submodule_url(url: str, repo_host: str) -> Tuple[Optional[str], Optional[Tuple[str, int, List[str]]]]:
"""Returns why the URL must not be fetched (or None), and the (host, port, addresses) to pin it to (or None).
class HostAddresses(NamedTuple):
"""A host and port, and the addresses its name resolved to (None for an IP address, or a failed lookup)."""
host: str
port: int
addresses: Optional[List[str]]
def resolve_repo_addresses(url: str) -> HostAddresses:
"""Resolve the host of a Repo's URL, once per download. Every fetch from the host is pinned to the result."""
parsed = urlparse(url)
host = (parsed.hostname or '').lower()
port = parsed.port or 443
if _is_ip_address(host):
return HostAddresses(host, port, None)
try:
return HostAddresses(host, port, sorted(resolve_host_addresses(host)))
except OSError:
return HostAddresses(host, port, None)
def pin_config(pin: HostAddresses) -> Optional[str]:
"""git config which makes git connect to `pin.host` at exactly `pin.addresses`."""
if not pin.addresses:
return None
addresses = ','.join(f'[{i}]' if ':' in i else i for i in pin.addresses)
return f'http.curloptResolve={pin.host}:{pin.port}:{addresses}'
def _is_ip_address(host: str) -> bool:
try:
ipaddress.ip_address(host.strip('[]'))
return True
except ValueError:
return False
def _check_submodule_url(url: str, repo: HostAddresses) -> Tuple[Optional[str], Optional[HostAddresses]]:
"""Returns why the URL must not be fetched (or None), and the host to pin it to (or None).
`url` must be absolute: git resolves relative submodule URLs (which can climb to another host, e.g.
`../../../10.0.0.1/x`), and the result is checked."""
@ -172,47 +209,50 @@ def _check_submodule_url(url: str, repo_host: str) -> Tuple[Optional[str], Optio
return 'only plain https:// submodule URLs can be downloaded', None
host = match.group('host').lower()
port = int(match.group('port') or 443)
if host == repo_host:
# The user chose this host.
return None, None
try:
if (host, port) == (repo.host, repo.port):
# The host the user chose (it may be on their network). It is fetched from the addresses the repo was
# fetched from (`pin_config(repo)` is part of every repo fetch), so it is not resolved again.
if repo.addresses or _is_ip_address(host):
return None, None
return f'could not resolve {host}', None
if _is_ip_address(host):
addresses = [ipaddress.ip_address(host.strip('[]'))]
pin = None # An address needs no DNS.
except ValueError:
else:
try:
addresses = [ipaddress.ip_address(i.split('%')[0]) for i in resolve_host_addresses(host)]
except (OSError, ValueError):
return f'could not resolve {host}', None
pin = (host, port, [str(i) for i in addresses])
pin = HostAddresses(host, port, [str(i) for i in addresses])
if not addresses or not all(is_public_address(i) for i in addresses):
return f'{host} is not a public address', None
return None, pin
def submodule_url_error(url: str, repo_host: str) -> Optional[str]:
def submodule_url_error(url: str, repo: HostAddresses) -> Optional[str]:
"""Why a submodule URL must not be fetched, or None when it may be.
A submodule's URL is chosen by whoever controls the repo, so it may not make WROLPi fetch from its own network
(e.g. `https://localhost:8443/...` or `https://10.0.0.1/...`). Allowed: the repo's own host (which the user
chose), and hosts with only public addresses. A relative URL must first be resolved by git."""
return _check_submodule_url(url, repo_host)[0]
(e.g. `https://localhost:8443/...` or `https://10.0.0.1/...`). Allowed: the repo's own host and port (which
the user chose), and hosts with only public addresses. A relative URL must first be resolved by git."""
return _check_submodule_url(url, repo)[0]
def submodule_fetch_config(urls: List[str], repo_host: str) -> tuple:
def submodule_fetch_config(urls: List[str], repo: HostAddresses) -> tuple:
"""git config for fetching submodules with these URLs. Raises ValueError if any URL must not be fetched.
Each checked host is pinned to the addresses that were checked, so its DNS cannot change before git fetches it,
and redirects are not followed, so a checked host cannot send git elsewhere."""
and redirects are not followed, so a checked host cannot send git elsewhere. (The repo's own host is pinned by
`pin_config(repo)`.)"""
config = ['http.followRedirects=false']
pinned = set()
for url in urls:
error, pin = _check_submodule_url(url, repo_host)
error, pin = _check_submodule_url(url, repo)
if error:
raise ValueError(f'Refusing to download submodule {url}: {error}')
if pin and (pin[0], pin[1]) not in pinned:
pinned.add((pin[0], pin[1]))
addresses = ','.join(f'[{i}]' if ':' in i else i for i in pin[2])
config.append(f'http.curloptResolve={pin[0]}:{pin[1]}:{addresses}')
if pin and (pin.host, pin.port) not in pinned:
pinned.add((pin.host, pin.port))
config.append(pin_config(pin))
return tuple(config)

View file

@ -10,6 +10,9 @@ from modules.repos import lib
from modules.repos.models import Repository
from wrolpi.downloader import Download, DownloadStatus
# The repo the submodules belong to, and the (public) address its host resolved to.
REPO = lib.HostAddresses('git.example.com', 443, ['93.184.215.14'])
def git(directory, *args) -> str:
return subprocess.run(['git', '-c', 'safe.directory=*', *args], cwd=directory, check=True,
@ -219,7 +222,7 @@ def test_submodule_url_error(url, resolved, allowed):
side_effect = resolved if isinstance(resolved, Exception) else None
with mock.patch('modules.repos.lib.resolve_host_addresses', side_effect=side_effect,
return_value=resolved or []):
error = lib.submodule_url_error(url, 'git.example.com')
error = lib.submodule_url_error(url, REPO)
assert (error is None) is allowed, error
@ -231,7 +234,7 @@ async def test_submodule_to_private_address(test_session, git_remote_factory, re
remote.add_submodule('vendor/library', library, url='https://10.0.0.5/admin/library')
repo = lib.create_repository(test_session, remote.url, submodules=True)
with mock.patch('modules.repos.lib.resolve_host_addresses') as resolve:
with mock.patch('modules.repos.lib.resolve_host_addresses', return_value=['93.184.215.14']) as resolve:
await repos_download_manager.wait_for_all_downloads()
test_session.expire_all()
@ -240,7 +243,8 @@ async def test_submodule_to_private_address(test_session, git_remote_factory, re
assert 'not a public address' in download.error
assert repo.head_sha == remote.head
assert not (repo.directory / 'vendor/library/README.md').exists()
resolve.assert_not_called() # An IP address is checked without DNS.
# Only the repo's own host was resolved; an IP address is checked without DNS.
assert [i.args for i in resolve.call_args_list] == [('git.example.com',)]
@pytest.mark.asyncio
@ -296,7 +300,7 @@ def test_submodule_fetch_config():
fetch."""
with mock.patch('modules.repos.lib.resolve_host_addresses', return_value=['217.197.91.145', '2606:4700::1111']):
config = lib.submodule_fetch_config(['https://codeberg.org/a/b', 'https://codeberg.org:8443/c/d',
'https://git.example.com/own/host'], 'git.example.com')
'https://git.example.com/own/host'], REPO)
assert config == (
'http.followRedirects=false',
'http.curloptResolve=codeberg.org:443:217.197.91.145,[2606:4700::1111]',
@ -368,4 +372,64 @@ def test_is_public_address(address, public):
'https://224.0.0.1/',
])
def test_submodule_url_error_rejects_non_public_literals(url):
assert lib.submodule_url_error(url, 'git.example.com')
assert lib.submodule_url_error(url, REPO)
def test_submodule_on_repo_host():
"""The repo's own host is fetched from the addresses the repo was fetched from, never resolved again (so its
DNS cannot change in between). The same name on another port is checked like any other host."""
lan_forge = lib.HostAddresses('git.lan', 443, ['192.168.1.20'])
with mock.patch('modules.repos.lib.resolve_host_addresses', return_value=['127.0.0.1']) as resolve:
# A LAN forge the user chose: its own submodules work.
assert lib.submodule_url_error('https://git.lan/owner/library', lan_forge) is None
assert lib.submodule_url_error('https://git.lan:443/owner/library', lan_forge) is None
resolve.assert_not_called()
# Another port on that name (e.g. a Docker API) is not the repo.
assert 'not a public address' in lib.submodule_url_error('https://git.lan:2375/x', lan_forge)
# A repo host which could not be resolved cannot vouch for anything.
unresolved = lib.HostAddresses('git.lan', 443, None)
assert lib.submodule_url_error('https://git.lan/owner/library', unresolved)
# An address needs no DNS.
literal = lib.HostAddresses('192.168.1.20', 443, None)
assert lib.submodule_url_error('https://192.168.1.20/owner/library', literal) is None
@pytest.mark.parametrize('url,expected', [
('https://git.example.com/owner/repo', lib.HostAddresses('git.example.com', 443, ['93.184.215.14'])),
('https://git.example.com:8443/repo', lib.HostAddresses('git.example.com', 8443, ['93.184.215.14'])),
('https://192.168.1.20/repo', lib.HostAddresses('192.168.1.20', 443, None)),
])
def test_resolve_repo_addresses(url, expected):
with mock.patch('modules.repos.lib.resolve_host_addresses', return_value=['93.184.215.14']):
assert lib.resolve_repo_addresses(url) == expected
assert lib.pin_config(lib.HostAddresses('h', 443, ['1.2.3.4', '2606:4700::1111'])) == \
'http.curloptResolve=h:443:1.2.3.4,[2606:4700::1111]'
assert lib.pin_config(lib.HostAddresses('h', 443, None)) is None
@pytest.mark.asyncio
async def test_repo_fetches_are_pinned(test_session, git_remote_factory, repos_download_manager):
"""Every git command which connects to the repo's host connects to the address resolved at the start."""
from modules.repos.downloader import GitDownloader
remote = git_remote_factory()
remote.commit({'a.txt': 'a'}, 'Second')
lib.create_repository(test_session, remote.url)
commands = []
original = GitDownloader.process_runner
async def record(self, download, cmd, *args, **kwargs):
commands.append([str(i) for i in cmd])
return await original(self, download, cmd, *args, **kwargs)
with mock.patch('modules.repos.lib.resolve_host_addresses', return_value=['93.184.215.14']), \
mock.patch.object(GitDownloader, 'process_runner', record):
await repos_download_manager.wait_for_all_downloads()
await update(test_session, repos_download_manager)
pin = 'http.curloptResolve=git.example.com:443:93.184.215.14'
network = [i for i in commands if {'ls-remote', 'clone', 'fetch'} & set(i)]
assert {'ls-remote', 'clone', 'fetch'} <= {j for i in network for j in i}
assert all(pin in i for i in network), network