You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by sb...@apache.org on 2026/08/11 17:37:31 UTC
(tooling-trusted-releases) branch sbp updated: Ensure that SVN credentials are only sent to trusted servers
This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch sbp
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
The following commit(s) were added to refs/heads/sbp by this push:
new 6fa4972c Ensure that SVN credentials are only sent to trusted servers
6fa4972c is described below
commit 6fa4972c2e748e09f0004e9f8124cd314a0b74b7
Author: Sean B. Palmer <se...@miscoranda.com>
AuthorDate: Tue Aug 11 18:36:56 2026 +0100
Ensure that SVN credentials are only sent to trusted servers
---
atr/svn/__init__.py | 64 +++++++++++++++++++++-------------
tests/unit/test_svn_errors.py | 9 ++---
tests/unit/test_svn_info.py | 51 +++++++++++++++++++++++++--
tests/unit/test_svn_publish_release.py | 6 +++-
4 files changed, 98 insertions(+), 32 deletions(-)
diff --git a/atr/svn/__init__.py b/atr/svn/__init__.py
index 7bb77c15..34fcec64 100644
--- a/atr/svn/__init__.py
+++ b/atr/svn/__init__.py
@@ -19,6 +19,7 @@ import asyncio
import datetime
import pathlib
import re
+import urllib.parse
from typing import Final, Self
import defusedxml.ElementTree as ElementTree
@@ -26,6 +27,7 @@ import pydantic
import pydantic_xml
import atr.config as config
+import atr.constants as constants
import atr.log as log
ASF_TOOL: Final[str] = "atr"
@@ -140,9 +142,7 @@ class SvnLog(pydantic_xml.BaseXmlModel, tag="log"):
async def commit(path: pathlib.Path, url: str, username: str, revision: str, message: str) -> str:
log.debug(f"running svn commit for user '{username}'")
# The username here is the ASF UID of the committer
- svn_token = config.get().SVN_TOKEN
- if svn_token is None:
- raise ValueError("SVN_TOKEN must be set")
+ stdin_bytes = _authentication(url)
return await _run_svnmucc_command(
"put",
str(path),
@@ -157,7 +157,7 @@ async def commit(path: pathlib.Path, url: str, username: str, revision: str, mes
revision,
"-m",
message,
- stdin_bytes=svn_token.encode(),
+ stdin_bytes=stdin_bytes,
)
@@ -231,9 +231,6 @@ async def get_log(path: pathlib.Path) -> SvnLog:
async def info_authenticated(url: str) -> str:
- svn_token = config.get().SVN_TOKEN
- if svn_token is None:
- raise ValueError("SVN_TOKEN must be set")
return await _run_svn_command(
"info",
url,
@@ -242,7 +239,7 @@ async def info_authenticated(url: str) -> str:
"--password-from-stdin",
"--non-interactive",
timeout_seconds=INFO_TIMEOUT_SECONDS,
- stdin_bytes=svn_token.encode(),
+ stdin_bytes=_authentication(url),
)
@@ -260,9 +257,7 @@ def parse_committed_revision(output: str) -> int | None:
async def publish_file(local_path: pathlib.Path, target_url: str, username: str, message: str) -> None:
log.debug(f"running svnmucc put for user '{username}'")
- svn_token = config.get().SVN_TOKEN
- if svn_token is None:
- raise ValueError("SVN_TOKEN must be set")
+ stdin_bytes = _authentication(target_url)
await _run_svnmucc_command(
"put",
str(local_path),
@@ -276,15 +271,13 @@ async def publish_file(local_path: pathlib.Path, target_url: str, username: str,
"-m",
message,
timeout_seconds=KEYS_TIMEOUT_SECONDS,
- stdin_bytes=svn_token.encode(),
+ stdin_bytes=stdin_bytes,
)
async def publish_release(source_dir: pathlib.Path, target_url: str, username: str, message: str) -> int | None:
log.debug(f"running svn import for user '{username}'")
- svn_token = config.get().SVN_TOKEN
- if svn_token is None:
- raise ValueError("SVN_TOKEN must be set")
+ stdin_bytes = _authentication(target_url)
output = await run_command(
"svn",
"import",
@@ -303,7 +296,7 @@ async def publish_release(source_dir: pathlib.Path, target_url: str, username: s
"-m",
message,
timeout_seconds=PUBLISH_TIMEOUT_SECONDS,
- stdin_bytes=svn_token.encode(),
+ stdin_bytes=stdin_bytes,
)
revision = parse_committed_revision(output)
if revision is None:
@@ -312,9 +305,7 @@ async def publish_release(source_dir: pathlib.Path, target_url: str, username: s
async def publish_revision_matches(info: SvnInfo, author: str, message: str) -> bool:
- svn_token = config.get().SVN_TOKEN
- if svn_token is None:
- raise ValueError("SVN_TOKEN must be set")
+ stdin_bytes = _authentication(info.url)
revision = info.last_changed_rev_number
log_output = await _run_svn_command(
"log",
@@ -328,7 +319,7 @@ async def publish_revision_matches(info: SvnInfo, author: str, message: str) ->
"--password-from-stdin",
"--non-interactive",
timeout_seconds=INFO_TIMEOUT_SECONDS,
- stdin_bytes=svn_token.encode(),
+ stdin_bytes=stdin_bytes,
)
root = ElementTree.fromstring(log_output)
entries = root.findall("logentry")
@@ -358,16 +349,14 @@ async def publish_revision_matches(info: SvnInfo, author: str, message: str) ->
"--password-from-stdin",
"--non-interactive",
timeout_seconds=INFO_TIMEOUT_SECONDS,
- stdin_bytes=svn_token.encode(),
+ stdin_bytes=stdin_bytes,
)
return tool.strip() == ASF_TOOL
async def remove_files(base_url: str, rel_paths: list[str], username: str, message: str) -> None:
log.debug(f"running svnmucc rm for user '{username}'")
- svn_token = config.get().SVN_TOKEN
- if svn_token is None:
- raise ValueError("SVN_TOKEN must be set")
+ stdin_bytes = _authentication(base_url)
actions: list[str] = []
for rel_path in rel_paths:
actions.extend(["rm", f"{base_url}/{rel_path}"])
@@ -382,7 +371,7 @@ async def remove_files(base_url: str, rel_paths: list[str], username: str, messa
"-m",
message,
timeout_seconds=PUBLISH_TIMEOUT_SECONDS,
- stdin_bytes=svn_token.encode(),
+ stdin_bytes=stdin_bytes,
)
@@ -428,6 +417,31 @@ async def update(path: pathlib.Path) -> str:
return await _run_svn_command("update", str(path), "--parents")
+def _authentication(url: str) -> bytes:
+ svn_token = config.get().SVN_TOKEN
+ if svn_token is None:
+ raise ValueError("SVN_TOKEN must be set")
+ if not _is_trusted_url(url):
+ raise ValueError(f"Refusing to send SVN credentials to {url}")
+ return svn_token.encode()
+
+
+def _is_trusted_url(url: str) -> bool:
+ target = urllib.parse.urlsplit(url)
+ if target.query or target.fragment:
+ return False
+ for base in (config.get().SVN_PUBLISH_URL, constants.SVN_DIST_ROOT_URL):
+ if not base:
+ continue
+ trusted = urllib.parse.urlsplit(base)
+ if (target.scheme.lower(), target.netloc.lower()) != (trusted.scheme.lower(), trusted.netloc.lower()):
+ continue
+ trusted_path = trusted.path.rstrip("/")
+ if (target.path == trusted_path) or target.path.startswith(trusted_path + "/"):
+ return True
+ return False
+
+
async def _run_svn_command(
sub_cmd: str, path: str, *args: str, timeout_seconds: float | None = None, stdin_bytes: bytes | None = None
) -> str:
diff --git a/tests/unit/test_svn_errors.py b/tests/unit/test_svn_errors.py
index 96635a39..1d3506f1 100644
--- a/tests/unit/test_svn_errors.py
+++ b/tests/unit/test_svn_errors.py
@@ -127,7 +127,8 @@ async def test_publish_revision_matches_without_author_on_local_repository(monke
run = mock.AsyncMock(side_effect=[log_output, "atr"])
monkeypatch.setattr(svn, "_run_svn_command", run)
- assert await svn.publish_revision_matches(_svn_info(), "alice", "Publish project-1.0.0")
+ info = _svn_info("svn://127.0.0.1:3690/atr-dev-publish")
+ assert await svn.publish_revision_matches(info, "alice", "Publish project-1.0.0")
async def test_publish_revision_rejects_provenance_mismatch(monkeypatch: pytest.MonkeyPatch) -> None:
@@ -259,13 +260,13 @@ def _wait_for_file(path: pathlib.Path) -> bool:
return False
-def _svn_info() -> svn.SvnInfo:
+def _svn_info(root: str = "https://dist.apache.org/repos/dist/atr") -> svn.SvnInfo:
return svn.SvnInfo(
path="project",
name="project",
- url="https://example.invalid/project",
+ url=f"{root}/project",
relative_url="^/project",
- repository_root="https://example.invalid",
+ repository_root=root,
revision="43",
last_changed_author="alice",
last_changed_rev="42",
diff --git a/tests/unit/test_svn_info.py b/tests/unit/test_svn_info.py
index a3d6f37a..e524c933 100644
--- a/tests/unit/test_svn_info.py
+++ b/tests/unit/test_svn_info.py
@@ -30,12 +30,30 @@ requires_svn = pytest.mark.skipif(
)
+async def test_info_authenticated_allows_dist_root_urls(monkeypatch: pytest.MonkeyPatch) -> None:
+ monkeypatch.setattr(
+ svn.config,
+ "get",
+ lambda: types.SimpleNamespace(SVN_TOKEN="dummy", SVN_PUBLISH_URL="file:///repo"),
+ )
+ run_command = mock.AsyncMock(return_value="")
+ monkeypatch.setattr(svn, "run_command", run_command)
+
+ await svn.info_authenticated("https://dist.apache.org/repos/dist/release/tooling")
+
+ run_command.assert_awaited_once()
+
+
@requires_svn
async def test_info_authenticated_reads_local_repository(
tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
) -> None:
- monkeypatch.setattr(svn.config, "get", lambda: types.SimpleNamespace(SVN_TOKEN="dummy"))
repository = tmp_path / "repository"
+ monkeypatch.setattr(
+ svn.config,
+ "get",
+ lambda: types.SimpleNamespace(SVN_TOKEN="dummy", SVN_PUBLISH_URL=f"file://{repository}"),
+ )
await svn.run_command("svnadmin", "create", str(repository))
output = await svn.info_authenticated(f"file://{repository}")
@@ -43,8 +61,37 @@ async def test_info_authenticated_reads_local_repository(
assert f"file://{repository}" in output
+async def test_info_authenticated_rejects_untrusted_urls(monkeypatch: pytest.MonkeyPatch) -> None:
+ monkeypatch.setattr(
+ svn.config,
+ "get",
+ lambda: types.SimpleNamespace(SVN_TOKEN="dummy", SVN_PUBLISH_URL="https://dist.apache.org/repos/dist/atr"),
+ )
+ run_command = mock.AsyncMock(return_value="")
+ monkeypatch.setattr(svn, "run_command", run_command)
+ untrusted = [
+ "https://dist.apache.org.example.net/repos/dist/atr/project",
+ "https://dist.apache.org/other/atr/project",
+ "https://user@dist.apache.org/repos/dist/atr/project",
+ "http://dist.apache.org/repos/dist/atr/project",
+ "https://dist.apache.org/repos/distant",
+ "https://dist.apache.org/repos/dist/atr/project?query=1",
+ "https://dist.apache.org/repos/dist/atr/project#fragment",
+ ]
+
+ for url in untrusted:
+ with pytest.raises(ValueError, match="Refusing to send SVN credentials"):
+ await svn.info_authenticated(url)
+
+ run_command.assert_not_awaited()
+
+
async def test_info_authenticated_sends_token_on_stdin(monkeypatch: pytest.MonkeyPatch) -> None:
- monkeypatch.setattr(svn.config, "get", lambda: types.SimpleNamespace(SVN_TOKEN="dummy"))
+ monkeypatch.setattr(
+ svn.config,
+ "get",
+ lambda: types.SimpleNamespace(SVN_TOKEN="dummy", SVN_PUBLISH_URL="https://svn.example.invalid/repos"),
+ )
run_command = mock.AsyncMock(return_value="")
monkeypatch.setattr(svn, "run_command", run_command)
diff --git a/tests/unit/test_svn_publish_release.py b/tests/unit/test_svn_publish_release.py
index 0400c4cd..34cbb58a 100644
--- a/tests/unit/test_svn_publish_release.py
+++ b/tests/unit/test_svn_publish_release.py
@@ -39,7 +39,11 @@ async def test_publish_release_ignores_auto_props(tmp_path: pathlib.Path, monkey
"[miscellany]\nenable-auto-props = yes\n[auto-props]\n*.txt = svn:eol-style=CRLF\n"
)
monkeypatch.setenv("HOME", str(home))
- monkeypatch.setattr(svn.config, "get", lambda: types.SimpleNamespace(SVN_TOKEN="dummy"))
+ monkeypatch.setattr(
+ svn.config,
+ "get",
+ lambda: types.SimpleNamespace(SVN_TOKEN="dummy", SVN_PUBLISH_URL=f"file://{repository}"),
+ )
source = tmp_path / "source"
source.mkdir()
(source / "a.txt").write_bytes(b"one\ntwo\n")
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org