fix: harden setup against plain http, untrusted output and unsafe values
Require consent before sending the token over plain http to a non-loopback host, strip control characters from server-supplied text, restrict env values and tokens to printable ASCII, validate the URLs like other env values and fsync the directory after the atomic replace.
This commit is contained in:
1 parent
6c9b2b74ca
commit
517c675048
2 files changed
+198
-11
No files matched your search
@@ -711,3 +711,134 @@ def test_http_gitea_url_warns(httpx_mock: HTTPXMock, capsys: pytest.CaptureFixtu
|
||||
httpx_mock.add_response(url="http://git.test/api/v1/token", json={"scopes": ["all"]})
|
||||
assert main(["setup", "--yes", "--no-oauth", "--gitea-url", "http://git.test"]) == 0
|
||||
assert "plain http" in capsys.readouterr().out
|
||||
|
||||
|
||||
# --- hardening ------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_plain_http_non_loopback_needs_consent_before_any_request(
|
||||
httpx_mock: HTTPXMock, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
# No response is registered: any outgoing request would fail the test.
|
||||
assert main(["setup", "--no-oauth", "--gitea-url", "http://git.test"]) == 1
|
||||
captured = capsys.readouterr()
|
||||
assert "plain http" in captured.out
|
||||
assert "refusing to send the token over plain http" in captured.err
|
||||
assert httpx_mock.get_requests() == []
|
||||
assert_no_secrets(captured.out + captured.err)
|
||||
|
||||
|
||||
def test_plain_http_interactive_decline_and_accept(
|
||||
httpx_mock: HTTPXMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.setattr("sys.stdin.isatty", lambda: True, raising=False)
|
||||
monkeypatch.setattr("builtins.input", lambda _prompt="": "n")
|
||||
assert main(["setup", "--no-oauth", "--gitea-url", "http://git.test"]) == 1
|
||||
assert httpx_mock.get_requests() == []
|
||||
|
||||
monkeypatch.setattr("builtins.input", lambda _prompt="": "y")
|
||||
httpx_mock.add_response(url="http://git.test/api/v1/version", json={"version": "1"})
|
||||
httpx_mock.add_response(
|
||||
url="http://git.test/api/v1/user", json={"login": "root", "is_admin": True}
|
||||
)
|
||||
httpx_mock.add_response(url="http://git.test/api/v1/token", json={"scopes": ["all"]})
|
||||
assert main(["setup", "--no-oauth", "--gitea-url", "http://git.test"]) == 0
|
||||
|
||||
|
||||
def test_plain_http_loopback_needs_no_consent(
|
||||
httpx_mock: HTTPXMock, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
base = "http://127.0.0.1:3000/api/v1"
|
||||
httpx_mock.add_response(url=f"{base}/version", json={"version": "1"})
|
||||
httpx_mock.add_response(url=f"{base}/user", json={"login": "root", "is_admin": True})
|
||||
httpx_mock.add_response(url=f"{base}/token", json={"scopes": ["all"]})
|
||||
assert main(["setup", "--no-oauth", "--gitea-url", "http://127.0.0.1:3000"]) == 0
|
||||
assert "plain http" not in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_server_supplied_version_and_login_are_sanitised(
|
||||
httpx_mock: HTTPXMock, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
httpx_mock.add_response(url=f"{API}/version", json={"version": "1.99\x1b[31mRED\x07\n\x9bX"})
|
||||
httpx_mock.add_response(url=f"{API}/user", json={"login": "ro\x1b[2Jot", "is_admin": True})
|
||||
httpx_mock.add_response(url=f"{API}/token", json={"scopes": ["all"]})
|
||||
assert run_setup(oauth=False) == 0
|
||||
out = capsys.readouterr().out
|
||||
for bad in ("\x1b", "\x07", "", "\x9b"):
|
||||
assert bad not in out
|
||||
assert "1.99" in out
|
||||
|
||||
|
||||
def test_version_made_only_of_control_characters_is_rejected(
|
||||
httpx_mock: HTTPXMock, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
httpx_mock.add_response(url=f"{API}/version", json={"version": "\x1b\x07"})
|
||||
assert run_setup(oauth=False) == 1
|
||||
assert "printable" in capsys.readouterr().err
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"token",
|
||||
["tokén_0123456789abcdef", "tok\x1b[0m_0123456789", "tok_0123456789abcdef"],
|
||||
)
|
||||
def test_non_ascii_or_control_tokens_are_rejected_without_echo(
|
||||
token: str, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
monkeypatch.setenv("GITEA_TOKEN", token)
|
||||
assert run_setup(oauth=False) == 1
|
||||
captured = capsys.readouterr()
|
||||
assert "printable ASCII" in captured.err
|
||||
assert token not in captured.out + captured.err
|
||||
assert token.split("_")[0] not in captured.out + captured.err
|
||||
|
||||
|
||||
def test_unsafe_urls_are_rejected_before_any_request(
|
||||
httpx_mock: HTTPXMock, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
assert main(["setup", "--yes", "--no-oauth", "--gitea-url", "https://git.test/a'b"]) == 1
|
||||
assert "the Gitea URL" in capsys.readouterr().err
|
||||
assert (
|
||||
main(["setup", "--yes", "--gitea-url", GITEA, "--public-base-url", "https://mcp.x/a$b"])
|
||||
== 1
|
||||
)
|
||||
assert "the public base URL" in capsys.readouterr().err
|
||||
assert httpx_mock.get_requests() == []
|
||||
|
||||
|
||||
def test_env_file_directory_is_fsynced_after_replace(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
calls: list[tuple[str, bool]] = []
|
||||
real_fsync = os.fsync
|
||||
|
||||
def recording_fsync(fd: int) -> None:
|
||||
calls.append(("fsync", stat.S_ISDIR(os.fstat(fd).st_mode)))
|
||||
real_fsync(fd)
|
||||
|
||||
real_replace = os.replace
|
||||
|
||||
def recording_replace(src: Any, dst: Any) -> None:
|
||||
calls.append(("replace", False))
|
||||
real_replace(src, dst)
|
||||
|
||||
monkeypatch.setattr(os, "fsync", recording_fsync)
|
||||
monkeypatch.setattr(os, "replace", recording_replace)
|
||||
write_env_atomic(tmp_path / "x.env", "A=1\n")
|
||||
|
||||
assert calls == [("fsync", False), ("replace", False), ("fsync", True)]
|
||||
assert (tmp_path / "x.env").read_text() == "A=1\n"
|
||||
|
||||
|
||||
def test_directory_fsync_failure_is_not_fatal(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
real_open = os.open
|
||||
|
||||
def failing_open(path: Any, flags: int, *args: Any, **kwargs: Any) -> int:
|
||||
if Path(path) == tmp_path:
|
||||
raise OSError("no directory fds here")
|
||||
return real_open(path, flags, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(os, "open", failing_open)
|
||||
write_env_atomic(tmp_path / "x.env", "A=1\n")
|
||||
assert (tmp_path / "x.env").read_text() == "A=1\n"
|
||||
Reference in new issue
Block a user