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: check submodule URLs where git resolves them

Browse source 
A relative submodule URL is resolved by git against the repo's remote,
and can climb to another host: with the remote
https://git.example.com/owner/repo, `../../../10.0.0.5/x` is
https://10.0.0.5/x.  Relative URLs were allowed unchecked.

git now resolves the URLs first (`submodule init` + `sync` write them to
the repo's config), and those resolved URLs, the ones git fetches, are
checked.  A relative URL is never allowed by the check itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-10-06 16:47:44 -06:00
parent 4810b8bc50
commit 2dd8e908c2
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
3 changed files with 61 additions and 28 deletions
Download patch file Download diff file Expand all files Collapse all files

42
modules/repos/downloader.py
View file

@ -189,30 +189,20 @@ class GitDownloader(Downloader):
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
# NUL-delimited: a submodule's name and URL can contain spaces and newlines. Each record is "key\nvalue".
output = await git('config', '--file', '.gitmodules', '-z', '--get-regexp', r'^submodule\..*\.(url|path)$',
check=False)
urls, paths = [], []
for record in output.split('\0'):
if not record:
continue
key, newline, value = record.partition('\n')
if not newline or not key.startswith('submodule.'):
raise DownloadError(f'Refusing to download submodules, cannot read .gitmodules: {record!r}')
if key.endswith('.url'):
urls.append(value)
elif key.endswith('.path'):
paths.append(value)
else:
raise DownloadError(f'Refusing to download submodules, cannot read .gitmodules: {record!r}')
paths = await self._git_config_values(download, ctx, directory, '.path', '--file', '.gitmodules')
# git resolves each submodule's URL (a relative URL is resolved against this repo's remote, and may be
# another host) into this repo's config. Those are the URLs git fetches, so those are checked.
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)
except ValueError as e:
raise DownloadError(str(e))
await git('submodule', 'sync')
args = ['submodule', 'update', '--init', '--force', '--progress']
args = ['submodule', 'update', '--force', '--progress']
if prepared.mode == 'snapshot':
args.extend(['--depth', '1'])
await git(*args, progress=True, config=fetch_config)
@ -222,6 +212,22 @@ class GitDownloader(Downloader):
if submodule.is_dir() and directory.resolve() in submodule.parents:
await self._check_out_submodules(download, ctx, submodule, prepared, depth + 1)
async def _git_config_values(self, download: Download, ctx: DownloadContext, directory: pathlib.Path,
suffix: str, *source: str) -> list:
"""The values of the `submodule.<name><suffix>` keys in a git config. Refuses anything it cannot read."""
# NUL-delimited: a submodule's name and URL can contain spaces and newlines. Each record is "key\nvalue".
output = await self._git(download, ctx, directory, 'config', *source, '-z', '--get-regexp',
rf'^submodule\..*\{suffix}$', check=False)
values = []
for record in output.split('\0'):
if not record:
continue
key, newline, value = record.partition('\n')
if not newline or not key.startswith('submodule.') or not key.endswith(suffix):
raise DownloadError(f'Refusing to download submodules, cannot read their config: {record!r}')
values.append(value)
return values
async def _is_ancestor(self, download: Download, ctx: DownloadContext, directory: pathlib.Path,
ancestor: str, descendant: str) -> bool:
cmd = git_command('merge-base', '--is-ancestor', ancestor, descendant)

13
modules/repos/lib.py
View file

@ -134,14 +134,13 @@ ABSOLUTE_SUBMODULE_URL_RE = re.compile(
r'^https://(?P<host>[A-Za-z0-9.-]+|\[[0-9A-Fa-f:.]+\])'
r'(?::(?P<port>[0-9]{1,5}))?'
r'(?:/[A-Za-z0-9._~%!$&\'()*+,;=:/-]*)?$')
RELATIVE_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)."""
if RELATIVE_SUBMODULE_URL_RE.match(url):
# Relative to the repo's own remote.
return None, None
"""Returns why the URL must not be fetched (or None), and the (host, port, addresses) 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."""
match = ABSOLUTE_SUBMODULE_URL_RE.match(url)
if not match:
return 'only plain https:// submodule URLs can be downloaded', None
@ -168,8 +167,8 @@ def submodule_url_error(url: str, repo_host: str) -> 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: URLs relative to the repo's remote,
the repo's own host (which the user chose), and hosts with only public addresses."""
(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]

34
modules/repos/test/test_extras.py
View file

@ -187,8 +187,9 @@ async def test_archive_after_session_closes(test_session, async_client, git_remo
@pytest.mark.parametrize('url,resolved,allowed', [
('../library.git', None, True), # Relative to the repo's own remote.
('./library', None, True),
# Relative URLs are resolved by git (against the repo's remote) before they are checked.
('../library.git', None, False),
('./library', None, False),
('https://git.example.com/owner/library', None, True), # The repo's own host.
('https://codeberg.org/owner/library', ['217.197.91.145'], True),
('https://localhost/library', ['127.0.0.1'], False),
@ -295,9 +296,36 @@ 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',
'../relative', 'https://git.example.com/own/host'], 'git.example.com')
'https://git.example.com/own/host'], 'git.example.com')
assert config == (
'http.followRedirects=false',
'http.curloptResolve=codeberg.org:443:217.197.91.145,[2606:4700::1111]',
'http.curloptResolve=codeberg.org:8443:217.197.91.145,[2606:4700::1111]',
)
@pytest.mark.parametrize('relative_url,allowed', [
('../library', True), # https://git.example.com/owner/library
('../../../10.0.0.5/library', False), # git resolves this to https://10.0.0.5/library
])
@pytest.mark.asyncio
async def test_relative_submodule_url(test_session, git_remote_factory, repos_download_manager, relative_url,
allowed):
"""A relative submodule URL is checked where git resolves it, which may be another host."""
library = git_remote_factory('library')
remote = git_remote_factory()
remote.add_submodule('vendor/library', library, url=relative_url)
repo = lib.create_repository(test_session, remote.url, submodules=True)
await repos_download_manager.wait_for_all_downloads()
test_session.expire_all()
download = test_session.query(Download).one()
if allowed:
assert download.status == DownloadStatus.complete, download.error
assert (repo.directory / 'vendor/library/README.md').is_file()
else:
assert download.status == DownloadStatus.deferred
assert 'Refusing to download submodule https://10.0.0.5/library' in download.error
assert not (repo.directory / 'vendor/library/README.md').exists()