From 11fc41755673070a4d86124b31d77a17d158945b Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 22 Aug 2026 16:38:13 +0200 Subject: [PATCH] Cover tool/preflight_firebase.py at 100% branch Fourth module of the tool/ coverage task, and the first of the four large ones. Split into checks and loaders/main() up front rather than discovering the 250-line cap afterwards -- 245 and 225 lines. Every check is asserted on its *message*, not merely on having raised. The whole reason this tool exists is that the misconfigurations it catches are indistinguishable once data has started moving, so a check firing with the wrong explanation sends the reader to the wrong place. That includes the wrong-region 404, which answers with a correctUrl body rather than an error and would otherwise pass preflight, pass sign-in against a different host entirely, and only break mid-migration. main()'s stated contract is pinned too: cheapest-first ordering, and that it short-circuits -- a typo must be reported without a network round trip, so the later network checks must not run at all. The module's config paths are module-level constants pointing into the real ~/.config/crdt-sync, so the fixture redirecting them at tmp_path is autouse: a test that forgot it would read and report on the user's actual credentials. Confirmed afterwards that ~/.config/crdt-sync was not touched. crdt_sync 293 -> 333 passed, still 100%. 4 of 7 tool/ modules done. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CFSGeWxHPY5rHUGrPqhcmG --- .../tests/test_tool_preflight_firebase.py | 245 ++++++++++++++++++ .../test_tool_preflight_firebase_main.py | 225 ++++++++++++++++ 2 files changed, 470 insertions(+) create mode 100644 crdt-sync/crdt_sync/tests/test_tool_preflight_firebase.py create mode 100644 crdt-sync/crdt_sync/tests/test_tool_preflight_firebase_main.py diff --git a/crdt-sync/crdt_sync/tests/test_tool_preflight_firebase.py b/crdt-sync/crdt_sync/tests/test_tool_preflight_firebase.py new file mode 100644 index 0000000..1a2bb65 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_tool_preflight_firebase.py @@ -0,0 +1,245 @@ +"""Tests for the individual checks in ``tool/preflight_firebase``. + +``main()`` and the loading helpers are covered in +:mod:`test_tool_preflight_firebase_main`, split to stay under the 250-line cap. + +The point of this tool is that each check names one specific misconfiguration, +because the failures it catches are otherwise indistinguishable once data has +started moving. So every check is asserted on its *message*, not just on the +fact that it raised -- a check that fires with the wrong explanation sends the +reader to the wrong place, which is the cost this module exists to avoid. +""" + +from __future__ import annotations + +import base64 +import json +from unittest.mock import MagicMock, patch + +import pytest +import requests + +from tool import preflight_firebase as pf + +_UID = "the-uid-pinned-in-the-rules" +_GOOD = { + "apiKey": "AIzaSyAthisIsTheWebApiKeyFormat", + "databaseUrl": "https://project-default-rtdb.europe-west1.firebasedatabase.app", + "projectId": "the-project", + "uid": _UID, + "email": "sync@example.com", +} +_PASSWORD = "the-sync-account-password" + + +def _config(**overrides: str) -> dict[str, str]: + """A valid config with individual fields overridden.""" + return {**_GOOD, **overrides} + + +def test_keys_present_accepts_a_complete_config() -> None: + """The happy path raises nothing.""" + pf.check_keys_present(_config()) + + +@pytest.mark.parametrize("key", ["apiKey", "databaseUrl", "projectId", "uid", "email"]) +def test_every_required_key_is_actually_required(key: str) -> None: + """Each of the five keys is checked, not just the first.""" + config = _config() + del config[key] + + with pytest.raises(pf.PreflightError, match=key): + pf.check_keys_present(config) + + +def test_a_whitespace_only_value_counts_as_missing() -> None: + """A key present but blank is the same failure as an absent one.""" + with pytest.raises(pf.PreflightError, match="uid"): + pf.check_keys_present(_config(uid=" ")) + + +def test_all_missing_keys_are_reported_together() -> None: + """Reporting one at a time would need five runs to fix five typos.""" + config = {"apiKey": _GOOD["apiKey"]} + + with pytest.raises(pf.PreflightError) as raised: + pf.check_keys_present(config) + + assert "databaseUrl" in str(raised.value) + assert "email" in str(raised.value) + + +def test_no_placeholders_accepts_a_filled_in_config() -> None: + """Real values pass.""" + pf.check_no_placeholders(_config(), _PASSWORD) + + +def test_an_unreplaced_scaffold_placeholder_is_caught() -> None: + """The scaffold ships PASTE_ markers; one left behind is a typo.""" + with pytest.raises(pf.PreflightError, match="projectId"): + pf.check_no_placeholders(_config(projectId="PASTE_PROJECT_ID"), _PASSWORD) + + +def test_a_placeholder_in_the_password_file_is_caught() -> None: + """The password lives in its own file and is easy to forget.""" + with pytest.raises(pf.PreflightError, match="password"): + pf.check_no_placeholders(_config(), "PASTE_PASSWORD_HERE") + + +def test_shapes_accepts_well_formed_values() -> None: + """The happy path raises nothing.""" + pf.check_shapes(_config(), _PASSWORD) + + +def test_a_non_https_database_url_is_rejected() -> None: + """http:// would work locally and fail everywhere else.""" + with pytest.raises(pf.PreflightError, match="must start with https://"): + pf.check_shapes( + _config(databaseUrl="http://project.firebasedatabase.app"), _PASSWORD + ) + + +def test_a_trailing_slash_on_the_database_url_is_rejected() -> None: + """It yields a double slash in every request path built from it.""" + with pytest.raises(pf.PreflightError, match="must not end with"): + pf.check_shapes(_config(databaseUrl=f"{_GOOD['databaseUrl']}/"), _PASSWORD) + + +def test_an_api_key_in_the_wrong_format_is_rejected() -> None: + """Pasting a service-account key instead of the Web API key.""" + with pytest.raises(pf.PreflightError, match="AIza"): + pf.check_shapes(_config(apiKey="ya29.not-the-web-api-key"), _PASSWORD) + + +def test_a_password_with_a_trailing_newline_is_rejected() -> None: + """The single most likely hand-editing mistake, per the module docstring.""" + with pytest.raises(pf.PreflightError, match="whitespace"): + pf.check_shapes(_config(), f"{_PASSWORD}\n") + + +def test_an_email_without_an_at_sign_is_rejected() -> None: + """A uid pasted into the email field, typically.""" + with pytest.raises(pf.PreflightError, match="does not look like an address"): + pf.check_shapes(_config(email="not-an-address"), _PASSWORD) + + +def _get(status: int, payload: object = None) -> MagicMock: + """Fake a ``requests.get`` response with a status and JSON body.""" + response = MagicMock() + response.status_code = status + if isinstance(payload, ValueError): + response.json.side_effect = payload + else: + response.json.return_value = payload if payload is not None else {} + return response + + +def test_a_denied_anonymous_read_is_the_expected_outcome() -> None: + """401 means the rules are published and doing their job.""" + with patch.object(pf.requests, "get", return_value=_get(401)): + pf.check_rules_deny_anonymous(_config()) + + +def test_a_world_readable_database_is_reported_loudly() -> None: + """A 200 here means anyone on the internet can read the data.""" + with ( + patch.object(pf.requests, "get", return_value=_get(200)), + pytest.raises(pf.PreflightError, match="world-readable"), + ): + pf.check_rules_deny_anonymous(_config()) + + +def test_a_wrong_region_url_reports_the_correct_one() -> None: + """Firebase answers 404 with the real URL rather than an error. + + Without this branch the wrong-region case would pass preflight, pass + sign-in (a different host entirely) and only break mid-migration. + """ + body = { + "correctUrl": "https://project-default-rtdb.asia-southeast1.firebasedatabase.app" + } + + with ( + patch.object(pf.requests, "get", return_value=_get(404, body)), + pytest.raises(pf.PreflightError, match="asia-southeast1"), + ): + pf.check_rules_deny_anonymous(_config()) + + +def test_a_404_without_a_correct_url_reports_a_missing_database() -> None: + """No database at that URL at all.""" + with ( + patch.object(pf.requests, "get", return_value=_get(404)), + pytest.raises(pf.PreflightError, match="no database lives there"), + ): + pf.check_rules_deny_anonymous(_config()) + + +def test_a_404_with_an_unparseable_body_still_reports_a_missing_database() -> None: + """The body is not guaranteed to be JSON; that must not mask the 404.""" + response = _get(404, ValueError("not json")) + + with ( + patch.object(pf.requests, "get", return_value=response), + pytest.raises(pf.PreflightError, match="no database lives there"), + ): + pf.check_rules_deny_anonymous(_config()) + + +def _token_for(uid: str) -> str: + """Build a JWT whose payload carries ``uid`` as ``user_id``.""" + payload = base64.urlsafe_b64encode(json.dumps({"user_id": uid}).encode()) + return f"header.{payload.decode().rstrip('=')}.signature" + + +def _provider(uid: str) -> MagicMock: + """A FirebaseTokenProvider that signs in and yields ``uid``'s token.""" + provider = MagicMock() + provider.id_token.return_value = _token_for(uid) + return provider + + +def test_sign_in_with_the_pinned_uid_passes() -> None: + """The decisive check, in its passing form.""" + with patch.object(pf, "FirebaseTokenProvider", return_value=_provider(_UID)): + pf.check_sign_in_uid_matches(_config(), _PASSWORD) + + +def test_signing_in_as_a_different_uid_is_rejected() -> None: + """Authenticates fine, then denies every read and write.""" + with ( + patch.object( + pf, "FirebaseTokenProvider", return_value=_provider("another-uid") + ), + pytest.raises(pf.PreflightError, match="every read and write would be denied"), + ): + pf.check_sign_in_uid_matches(_config(), _PASSWORD) + + +def test_the_uid_is_read_from_sub_when_user_id_is_absent() -> None: + """Not every issuer sets user_id; sub is the standard claim.""" + payload = base64.urlsafe_b64encode(json.dumps({"sub": _UID}).encode()) + provider = MagicMock() + provider.id_token.return_value = f"h.{payload.decode().rstrip('=')}.s" + + with patch.object(pf, "FirebaseTokenProvider", return_value=provider): + pf.check_sign_in_uid_matches(_config(), _PASSWORD) + + +def test_the_password_is_passed_to_sign_in_verbatim() -> None: + """A stripped or re-encoded password would fail for the wrong reason.""" + provider = _provider(_UID) + + with patch.object(pf, "FirebaseTokenProvider", return_value=provider): + pf.check_sign_in_uid_matches(_config(), _PASSWORD) + + provider.sign_in.assert_called_once_with(_GOOD["email"], _PASSWORD) + + +def test_requests_exceptions_are_not_swallowed_by_the_checks() -> None: + """Transport errors belong to main(), which reports them differently.""" + with ( + patch.object(pf.requests, "get", side_effect=requests.ConnectionError("down")), + pytest.raises(requests.ConnectionError), + ): + pf.check_rules_deny_anonymous(_config()) diff --git a/crdt-sync/crdt_sync/tests/test_tool_preflight_firebase_main.py b/crdt-sync/crdt_sync/tests/test_tool_preflight_firebase_main.py new file mode 100644 index 0000000..1315dd1 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_tool_preflight_firebase_main.py @@ -0,0 +1,225 @@ +"""Tests for ``tool/preflight_firebase``'s loaders and ``main()``. + +Split from :mod:`test_tool_preflight_firebase` to stay under the 250-line file +cap; that module covers the individual checks. + +The config paths are module-level constants pointing into the real +``~/.config/crdt-sync``, so every test here redirects them at ``tmp_path`` +first. Nothing may read or write the user's actual credentials. +""" + +from __future__ import annotations + +import json +import logging +from typing import TYPE_CHECKING +from unittest.mock import patch + +import pytest +import requests + +from tool import preflight_firebase as pf + +if TYPE_CHECKING: + from collections.abc import Iterator + from pathlib import Path + +_GOOD = { + "apiKey": "AIzaSyAthisIsTheWebApiKeyFormat", + "databaseUrl": "https://project-default-rtdb.europe-west1.firebasedatabase.app", + "projectId": "the-project", + "uid": "the-uid-pinned-in-the-rules", + "email": "sync@example.com", +} +_PASSWORD = "the-sync-account-password" + + +@pytest.fixture(autouse=True) +def config_dir(tmp_path: Path) -> Iterator[Path]: + """Point the module's config constants at a throwaway directory. + + Autouse, because a test that forgot this would read -- and report on -- + the real ~/.config/crdt-sync. + """ + with ( + patch.object(pf, "_CONFIG_FILE", tmp_path / "firebase.json"), + patch.object(pf, "_PASSWORD_FILE", tmp_path / "password"), + ): + yield tmp_path + + +def _write(directory: Path, config: object = None, password: str | None = None) -> None: + """Write a config and/or password file into the redirected directory.""" + if config is not None: + text = config if isinstance(config, str) else json.dumps(config) + (directory / "firebase.json").write_text(text, encoding="utf-8") + if password is not None: + (directory / "password").write_text(password, encoding="utf-8") + + +def test_a_missing_config_file_is_reported_by_path(config_dir: Path) -> None: + """The message must name the file, since creating it is the fix.""" + with pytest.raises(pf.PreflightError, match="does not exist"): + pf._load_config() + + +def test_invalid_json_is_reported_as_such(config_dir: Path) -> None: + """A trailing comma is the usual cause and needs naming as a syntax error.""" + _write(config_dir, "{not json,}") + + with pytest.raises(pf.PreflightError, match="not valid JSON"): + pf._load_config() + + +def test_a_json_scalar_is_rejected(config_dir: Path) -> None: + """Valid JSON, wrong shape: the file must hold an object.""" + _write(config_dir, "[]") + + with pytest.raises(pf.PreflightError, match="must contain a JSON object"): + pf._load_config() + + +def test_scaffold_comment_keys_are_stripped(config_dir: Path) -> None: + """They are documentation for whoever fills the file in, not config.""" + _write(config_dir, {**_GOOD, "_comment_apiKey": "where to find this"}) + + config = pf._load_config() + + assert config == _GOOD + + +def test_a_missing_password_file_is_reported_by_path(config_dir: Path) -> None: + """Absent and empty are the same failure with the same fix.""" + with pytest.raises(pf.PreflightError, match="missing or empty"): + pf._load_password() + + +def test_an_empty_password_file_is_reported(config_dir: Path) -> None: + """A touched-but-unfilled file must not read as a valid empty password.""" + _write(config_dir, password="") + + with pytest.raises(pf.PreflightError, match="missing or empty"): + pf._load_password() + + +def test_the_password_is_returned_unstripped(config_dir: Path) -> None: + """check_shapes is what rejects the newline, so the loader must keep it. + + Stripping here would hide the single most likely hand-editing mistake + rather than report it. + """ + _write(config_dir, password=f"{_PASSWORD}\n") + + assert pf._load_password() == f"{_PASSWORD}\n" + + +def test_main_passes_when_every_check_passes( + config_dir: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """The happy path: exit 0, and every check reported by name.""" + _write(config_dir, _GOOD, _PASSWORD) + + with ( + patch.object(pf, "check_rules_deny_anonymous"), + patch.object(pf, "check_sign_in_uid_matches"), + caplog.at_level(logging.INFO, logger="preflight"), + ): + code = pf.main() + + assert code == 0 + messages = "\n".join(record.getMessage() for record in caplog.records) + assert "PASS config keys present" in messages + assert "PASS sign-in uid matches rules" in messages + assert "safe to run the migration" in messages + + +def test_main_returns_1_and_names_the_failing_check( + config_dir: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """A PreflightError is an exit code plus an explanation, not a traceback.""" + _write(config_dir, {**_GOOD, "email": "not-an-address"}, _PASSWORD) + + with caplog.at_level(logging.INFO, logger="preflight"): + code = pf.main() + + assert code == 1 + assert "FAIL" in caplog.text + assert "does not look like an address" in caplog.text + + +def test_main_stops_at_the_first_failure(config_dir: Path) -> None: + """Cheapest-first ordering only pays off if it short-circuits. + + A typo must be reported without a network round trip, so the later + network checks must not run at all. + """ + _write(config_dir, {**_GOOD, "email": "not-an-address"}, _PASSWORD) + + with ( + patch.object(pf, "check_rules_deny_anonymous") as rules, + patch.object(pf, "check_sign_in_uid_matches") as sign_in, + ): + code = pf.main() + + assert code == 1 + rules.assert_not_called() + sign_in.assert_not_called() + + +def test_main_reports_a_network_failure_distinctly( + config_dir: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """An unreachable Firebase is not a misconfiguration; say so.""" + _write(config_dir, _GOOD, _PASSWORD) + + with ( + patch.object( + pf, + "check_rules_deny_anonymous", + side_effect=requests.ConnectionError("no route to host"), + ), + caplog.at_level(logging.INFO, logger="preflight"), + ): + code = pf.main() + + assert code == 1 + assert "could not reach Firebase" in caplog.text + + +def test_main_reports_an_unreadable_config_as_a_failure( + config_dir: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """An OSError reading the files is caught rather than crashing.""" + _write(config_dir, _GOOD, _PASSWORD) + + with ( + patch.object(pf, "_load_password", side_effect=OSError("permission denied")), + caplog.at_level(logging.INFO, logger="preflight"), + ): + code = pf.main() + + assert code == 1 + assert "could not reach Firebase" in caplog.text + + +def test_main_runs_the_checks_in_cheapest_first_order(config_dir: Path) -> None: + """The ordering is the module's stated contract, so pin it.""" + _write(config_dir, _GOOD, _PASSWORD) + called: list[str] = [] + + with ( + patch.object(pf, "check_keys_present", lambda *_: called.append("keys")), + patch.object(pf, "check_no_placeholders", lambda *_: called.append("holders")), + patch.object(pf, "check_shapes", lambda *_: called.append("shapes")), + patch.object( + pf, "check_rules_deny_anonymous", lambda *_: called.append("rules") + ), + patch.object(pf, "check_sign_in_uid_matches", lambda *_: called.append("uid")), + ): + assert pf.main() == 0 + + assert called == ["keys", "holders", "shapes", "rules", "uid"]