after audit

tests/unit/test_logging_policy.py

@@ -0,0 +1,40 @@
+from __future__ import annotations
+
+import ast
+from pathlib import Path
+
+RAW_LOG_METHODS = {
+    "debug",
+    "info",
+    "warning",
+    "error",
+    "exception",
+    "critical",
+}
+APP_DIR = Path(__file__).resolve().parents[2] / "app"
+
+
+def _raw_logger_calls(path: Path) -> list[tuple[int, str]]:
+    tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
+    violations: list[tuple[int, str]] = []
+    for node in ast.walk(tree):
+        if not isinstance(node, ast.Call):
+            continue
+        func = node.func
+        if not isinstance(func, ast.Attribute):
+            continue
+        if not isinstance(func.value, ast.Name) or func.value.id != "logger":
+            continue
+        if func.attr not in RAW_LOG_METHODS:
+            continue
+        violations.append((int(node.lineno), func.attr))
+    return violations
+
+
+def test_app_runtime_uses_structured_log_only() -> None:
+    violations: list[str] = []
+    for path in sorted(APP_DIR.rglob("*.py")):
+        for line_number, method in _raw_logger_calls(path):
+            relative_path = path.relative_to(APP_DIR.parent)
+            violations.append(f"{relative_path}:{line_number} logger.{method}()")
+    assert not violations, "raw logger calls found:\n" + "\n".join(violations)

tests/unit/test_portability_import.py

@@ -0,0 +1,121 @@
+from __future__ import annotations
+
+from unittest.mock import MagicMock
+
+from app.services.portability.publication_import import (
+    _build_imported_publication_input,
+    _initialize_import_counters,
+    _update_link_counters,
+)
+
+
+def _mock_profile(scholar_id: str = "ABC123DEF456") -> MagicMock:
+    profile = MagicMock()
+    profile.id = 1
+    profile.scholar_id = scholar_id
+    return profile
+
+
+def _scholar_map(scholar_id: str = "ABC123DEF456") -> dict[str, MagicMock]:
+    return {scholar_id: _mock_profile(scholar_id)}
+
+
+class TestBuildImportedPublicationInput:
+    def test_returns_parsed_input_for_valid_item(self) -> None:
+        item = {
+            "scholar_id": "ABC123DEF456",
+            "title": "Deep Learning",
+            "year": 2024,
+            "author_text": "Smith, J",
+            "venue_text": "ICML",
+            "citation_count": 10,
+        }
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is not None
+        assert result.title == "Deep Learning"
+        assert result.year == 2024
+        assert result.citation_count == 10
+        assert result.author_text == "Smith, J"
+
+    def test_returns_none_when_title_missing(self) -> None:
+        item = {"scholar_id": "ABC123DEF456"}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is None
+
+    def test_returns_none_when_scholar_id_missing(self) -> None:
+        item = {"title": "Test Title"}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is None
+
+    def test_returns_none_when_scholar_not_in_map(self) -> None:
+        item = {"scholar_id": "UNKNOWN12345", "title": "Test Title"}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is None
+
+    def test_defaults_is_read_to_false(self) -> None:
+        item = {"scholar_id": "ABC123DEF456", "title": "Test Title"}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is not None
+        assert result.is_read is False
+
+    def test_respects_is_read_true(self) -> None:
+        item = {"scholar_id": "ABC123DEF456", "title": "Test Title", "is_read": True}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is not None
+        assert result.is_read is True
+
+    def test_normalizes_year(self) -> None:
+        item = {"scholar_id": "ABC123DEF456", "title": "Test Title", "year": "invalid"}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is not None
+        assert result.year is None
+
+    def test_normalizes_citation_count(self) -> None:
+        item = {"scholar_id": "ABC123DEF456", "title": "Test Title", "citation_count": "not_a_number"}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is not None
+        assert result.citation_count == 0
+
+    def test_computes_fingerprint_when_not_provided(self) -> None:
+        item = {"scholar_id": "ABC123DEF456", "title": "Test Title"}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is not None
+        assert len(result.fingerprint) == 64
+
+    def test_uses_provided_valid_fingerprint(self) -> None:
+        valid_sha = "b" * 64
+        item = {"scholar_id": "ABC123DEF456", "title": "Test Title", "fingerprint_sha256": valid_sha}
+        result = _build_imported_publication_input(item=item, scholar_map=_scholar_map())
+        assert result is not None
+        assert result.fingerprint == valid_sha
+
+
+class TestInitializeImportCounters:
+    def test_adds_publication_and_link_keys(self) -> None:
+        counters: dict[str, int] = {"existing_key": 5}
+        _initialize_import_counters(counters)
+        assert counters["publications_created"] == 0
+        assert counters["publications_updated"] == 0
+        assert counters["links_created"] == 0
+        assert counters["links_updated"] == 0
+        assert counters["existing_key"] == 5
+
+
+class TestUpdateLinkCounters:
+    def test_increments_created(self) -> None:
+        counters = {"links_created": 0, "links_updated": 0}
+        _update_link_counters(counters=counters, link_created=True, link_updated=False)
+        assert counters["links_created"] == 1
+        assert counters["links_updated"] == 0
+
+    def test_increments_updated(self) -> None:
+        counters = {"links_created": 0, "links_updated": 0}
+        _update_link_counters(counters=counters, link_created=False, link_updated=True)
+        assert counters["links_created"] == 0
+        assert counters["links_updated"] == 1
+
+    def test_no_change_when_both_false(self) -> None:
+        counters = {"links_created": 0, "links_updated": 0}
+        _update_link_counters(counters=counters, link_created=False, link_updated=False)
+        assert counters["links_created"] == 0
+        assert counters["links_updated"] == 0

tests/unit/test_portability_normalize.py

@@ -0,0 +1,193 @@
+from __future__ import annotations
+
+import pytest
+
+from app.services.portability.constants import MAX_IMPORT_PUBLICATIONS, MAX_IMPORT_SCHOLARS
+from app.services.portability.normalize import (
+    _build_fingerprint,
+    _first_author_last_name,
+    _first_venue_word,
+    _normalize_citation_count,
+    _normalize_optional_text,
+    _normalize_optional_year,
+    _resolve_fingerprint,
+    _validate_import_sizes,
+)
+from app.services.portability.types import ImportExportError
+
+
+class TestNormalizeOptionalText:
+    def test_returns_stripped_string(self) -> None:
+        assert _normalize_optional_text("  hello  ") == "hello"
+
+    def test_returns_none_for_none(self) -> None:
+        assert _normalize_optional_text(None) is None
+
+    def test_returns_none_for_empty_string(self) -> None:
+        assert _normalize_optional_text("") is None
+
+    def test_returns_none_for_whitespace_only(self) -> None:
+        assert _normalize_optional_text("   ") is None
+
+    def test_coerces_non_string_to_string(self) -> None:
+        assert _normalize_optional_text(42) == "42"
+
+
+class TestNormalizeOptionalYear:
+    def test_valid_year(self) -> None:
+        assert _normalize_optional_year(2024) == 2024
+
+    def test_string_year(self) -> None:
+        assert _normalize_optional_year("1999") == 1999
+
+    def test_returns_none_for_none(self) -> None:
+        assert _normalize_optional_year(None) is None
+
+    def test_returns_none_for_non_numeric(self) -> None:
+        assert _normalize_optional_year("abc") is None
+
+    def test_returns_none_for_year_below_1500(self) -> None:
+        assert _normalize_optional_year(1499) is None
+
+    def test_returns_none_for_year_above_3000(self) -> None:
+        assert _normalize_optional_year(3001) is None
+
+    def test_boundary_1500_accepted(self) -> None:
+        assert _normalize_optional_year(1500) == 1500
+
+    def test_boundary_3000_accepted(self) -> None:
+        assert _normalize_optional_year(3000) == 3000
+
+
+class TestNormalizeCitationCount:
+    def test_valid_positive(self) -> None:
+        assert _normalize_citation_count(10) == 10
+
+    def test_zero(self) -> None:
+        assert _normalize_citation_count(0) == 0
+
+    def test_negative_clamped_to_zero(self) -> None:
+        assert _normalize_citation_count(-5) == 0
+
+    def test_string_number(self) -> None:
+        assert _normalize_citation_count("42") == 42
+
+    def test_none_returns_zero(self) -> None:
+        assert _normalize_citation_count(None) == 0
+
+    def test_invalid_string_returns_zero(self) -> None:
+        assert _normalize_citation_count("abc") == 0
+
+
+class TestFirstAuthorLastName:
+    def test_single_author(self) -> None:
+        assert _first_author_last_name("John Smith") == "smith"
+
+    def test_multiple_authors(self) -> None:
+        assert _first_author_last_name("Jane Doe, John Smith") == "doe"
+
+    def test_empty_string(self) -> None:
+        assert _first_author_last_name("") == ""
+
+    def test_none(self) -> None:
+        assert _first_author_last_name(None) == ""
+
+
+class TestFirstVenueWord:
+    def test_returns_first_word(self) -> None:
+        assert _first_venue_word("Nature Communications") == "nature"
+
+    def test_empty_string(self) -> None:
+        assert _first_venue_word("") == ""
+
+    def test_none(self) -> None:
+        assert _first_venue_word(None) == ""
+
+
+class TestBuildFingerprint:
+    def test_produces_64_hex_digest(self) -> None:
+        fp = _build_fingerprint(title="Test Title", year=2024, author_text="Smith", venue_text="ICML")
+        assert len(fp) == 64
+        assert all(c in "0123456789abcdef" for c in fp)
+
+    def test_deterministic(self) -> None:
+        kwargs = {"title": "Test Title", "year": 2024, "author_text": "Smith", "venue_text": "ICML"}
+        assert _build_fingerprint(**kwargs) == _build_fingerprint(**kwargs)
+
+    def test_different_titles_produce_different_fingerprints(self) -> None:
+        fp1 = _build_fingerprint(title="Title A", year=2024, author_text=None, venue_text=None)
+        fp2 = _build_fingerprint(title="Title B", year=2024, author_text=None, venue_text=None)
+        assert fp1 != fp2
+
+    def test_none_year_handled(self) -> None:
+        fp = _build_fingerprint(title="Test", year=None, author_text=None, venue_text=None)
+        assert len(fp) == 64
+
+
+class TestResolveFingerprint:
+    def test_uses_provided_valid_sha256(self) -> None:
+        valid_sha = "a" * 64
+        result = _resolve_fingerprint(
+            title="Test",
+            year=2024,
+            author_text=None,
+            venue_text=None,
+            provided_fingerprint=valid_sha,
+        )
+        assert result == valid_sha
+
+    def test_computes_fingerprint_when_provided_is_none(self) -> None:
+        result = _resolve_fingerprint(
+            title="Test",
+            year=2024,
+            author_text=None,
+            venue_text=None,
+            provided_fingerprint=None,
+        )
+        assert len(result) == 64
+
+    def test_computes_fingerprint_when_provided_is_invalid(self) -> None:
+        result = _resolve_fingerprint(
+            title="Test",
+            year=2024,
+            author_text=None,
+            venue_text=None,
+            provided_fingerprint="not-a-sha",
+        )
+        assert len(result) == 64
+
+    def test_normalizes_provided_to_lowercase(self) -> None:
+        valid_sha = "A" * 64
+        result = _resolve_fingerprint(
+            title="Test",
+            year=2024,
+            author_text=None,
+            venue_text=None,
+            provided_fingerprint=valid_sha,
+        )
+        assert result == "a" * 64
+
+
+class TestValidateImportSizes:
+    def test_accepts_within_limits(self) -> None:
+        _validate_import_sizes(scholars=[{}], publications=[{}])
+
+    def test_rejects_too_many_scholars(self) -> None:
+        with pytest.raises(ImportExportError, match="max scholars"):
+            _validate_import_sizes(
+                scholars=[{}] * (MAX_IMPORT_SCHOLARS + 1),
+                publications=[],
+            )
+
+    def test_rejects_too_many_publications(self) -> None:
+        with pytest.raises(ImportExportError, match="max publications"):
+            _validate_import_sizes(
+                scholars=[],
+                publications=[{}] * (MAX_IMPORT_PUBLICATIONS + 1),
+            )
+
+    def test_accepts_exact_limit(self) -> None:
+        _validate_import_sizes(
+            scholars=[{}] * MAX_IMPORT_SCHOLARS,
+            publications=[{}] * MAX_IMPORT_PUBLICATIONS,
+        )

tests/unit/test_scholar_validators.py

@@ -0,0 +1,142 @@
+from __future__ import annotations
+
+import os
+import tempfile
+
+import pytest
+
+from app.services.scholars.constants import MAX_IMAGE_URL_LENGTH
+from app.services.scholars.exceptions import ScholarServiceError
+from app.services.scholars.uploads import (
+    _ensure_upload_root,
+    _resolve_upload_path,
+    _safe_remove_upload,
+    resolve_upload_file_path,
+)
+from app.services.scholars.validators import (
+    normalize_display_name,
+    normalize_profile_image_url,
+    validate_scholar_id,
+)
+
+
+class TestValidateScholarId:
+    def test_valid_12_char_id(self) -> None:
+        assert validate_scholar_id("ABCDEF123456") == "ABCDEF123456"
+
+    def test_valid_with_hyphens_and_underscores(self) -> None:
+        assert validate_scholar_id("AB-CD_EF1234") == "AB-CD_EF1234"
+
+    def test_strips_whitespace(self) -> None:
+        assert validate_scholar_id("  ABCDEF123456  ") == "ABCDEF123456"
+
+    def test_rejects_too_short(self) -> None:
+        with pytest.raises(ScholarServiceError, match="12"):
+            validate_scholar_id("ABC123")
+
+    def test_rejects_too_long(self) -> None:
+        with pytest.raises(ScholarServiceError, match="12"):
+            validate_scholar_id("ABCDEF1234567")
+
+    def test_rejects_special_characters(self) -> None:
+        with pytest.raises(ScholarServiceError, match="12"):
+            validate_scholar_id("ABCDEF12345!")
+
+
+class TestNormalizeDisplayName:
+    def test_returns_stripped_name(self) -> None:
+        assert normalize_display_name("  John Doe  ") == "John Doe"
+
+    def test_returns_none_for_empty(self) -> None:
+        assert normalize_display_name("") is None
+
+    def test_returns_none_for_whitespace_only(self) -> None:
+        assert normalize_display_name("   ") is None
+
+    def test_preserves_non_empty(self) -> None:
+        assert normalize_display_name("A") == "A"
+
+
+class TestNormalizeProfileImageUrl:
+    def test_valid_https_url(self) -> None:
+        assert normalize_profile_image_url("https://example.com/img.png") == "https://example.com/img.png"
+
+    def test_valid_http_url(self) -> None:
+        assert normalize_profile_image_url("http://example.com/img.png") == "http://example.com/img.png"
+
+    def test_returns_none_for_none(self) -> None:
+        assert normalize_profile_image_url(None) is None
+
+    def test_returns_none_for_empty(self) -> None:
+        assert normalize_profile_image_url("") is None
+
+    def test_returns_none_for_whitespace(self) -> None:
+        assert normalize_profile_image_url("   ") is None
+
+    def test_rejects_ftp_scheme(self) -> None:
+        with pytest.raises(ScholarServiceError, match="http"):
+            normalize_profile_image_url("ftp://example.com/img.png")
+
+    def test_rejects_no_scheme(self) -> None:
+        with pytest.raises(ScholarServiceError, match="http"):
+            normalize_profile_image_url("example.com/img.png")
+
+    def test_rejects_url_too_long(self) -> None:
+        long_url = "https://example.com/" + "a" * MAX_IMAGE_URL_LENGTH
+        with pytest.raises(ScholarServiceError, match="characters or fewer"):
+            normalize_profile_image_url(long_url)
+
+    def test_accepts_url_at_max_length(self) -> None:
+        url = "https://example.com/" + "a" * (MAX_IMAGE_URL_LENGTH - len("https://example.com/"))
+        assert len(url) == MAX_IMAGE_URL_LENGTH
+        assert normalize_profile_image_url(url) == url
+
+
+class TestUploadPathSafety:
+    def test_resolve_upload_path_rejects_traversal(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            root = _ensure_upload_root(tmpdir, create=False)
+            with pytest.raises(ScholarServiceError, match="Invalid"):
+                _resolve_upload_path(root, "../../../etc/passwd")
+
+    def test_resolve_upload_path_accepts_valid_relative(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            root = _ensure_upload_root(tmpdir, create=False)
+            result = _resolve_upload_path(root, "scholar_img.png")
+            assert result.name == "scholar_img.png"
+            assert root in result.parents or root == result.parent
+
+    def test_ensure_upload_root_creates_directory(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            new_dir = os.path.join(tmpdir, "uploads", "images")
+            root = _ensure_upload_root(new_dir, create=True)
+            assert root.exists()
+
+    def test_safe_remove_upload_removes_existing_file(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            root = _ensure_upload_root(tmpdir, create=False)
+            test_file = root / "test.png"
+            test_file.write_bytes(b"fake image")
+            assert test_file.exists()
+            _safe_remove_upload(root, "test.png")
+            assert not test_file.exists()
+
+    def test_safe_remove_upload_ignores_missing_file(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            root = _ensure_upload_root(tmpdir, create=False)
+            _safe_remove_upload(root, "nonexistent.png")
+
+    def test_safe_remove_upload_ignores_none(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            root = _ensure_upload_root(tmpdir, create=False)
+            _safe_remove_upload(root, None)
+
+    def test_safe_remove_upload_ignores_traversal_path(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            root = _ensure_upload_root(tmpdir, create=False)
+            _safe_remove_upload(root, "../../../etc/passwd")
+
+    def test_resolve_upload_file_path(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            result = resolve_upload_file_path(upload_dir=tmpdir, relative_path="img.png")
+            assert result.name == "img.png"

tests/unit/test_user_validation.py

@@ -0,0 +1,79 @@
+from __future__ import annotations
+
+import pytest
+
+from app.services.users.application import (
+    UserServiceError,
+    normalize_email,
+    validate_email,
+    validate_password,
+)
+
+
+class TestNormalizeEmail:
+    def test_lowercases_and_strips(self) -> None:
+        assert normalize_email("  Alice@Example.COM  ") == "alice@example.com"
+
+    def test_already_normalized(self) -> None:
+        assert normalize_email("user@example.com") == "user@example.com"
+
+
+class TestValidateEmail:
+    def test_valid_email(self) -> None:
+        assert validate_email("user@example.com") == "user@example.com"
+
+    def test_strips_and_lowercases(self) -> None:
+        assert validate_email("  User@Example.COM  ") == "user@example.com"
+
+    def test_rejects_missing_at(self) -> None:
+        with pytest.raises(UserServiceError, match="valid email"):
+            validate_email("userexample.com")
+
+    def test_rejects_missing_domain(self) -> None:
+        with pytest.raises(UserServiceError, match="valid email"):
+            validate_email("user@")
+
+    def test_rejects_missing_tld(self) -> None:
+        with pytest.raises(UserServiceError, match="valid email"):
+            validate_email("user@example")
+
+    def test_rejects_empty_string(self) -> None:
+        with pytest.raises(UserServiceError, match="valid email"):
+            validate_email("")
+
+    def test_rejects_whitespace_only(self) -> None:
+        with pytest.raises(UserServiceError, match="valid email"):
+            validate_email("   ")
+
+    def test_rejects_spaces_in_local_part(self) -> None:
+        with pytest.raises(UserServiceError, match="valid email"):
+            validate_email("us er@example.com")
+
+    def test_accepts_plus_addressing(self) -> None:
+        assert validate_email("user+tag@example.com") == "user+tag@example.com"
+
+    def test_accepts_dots_in_local(self) -> None:
+        assert validate_email("first.last@example.com") == "first.last@example.com"
+
+
+class TestValidatePassword:
+    def test_valid_password(self) -> None:
+        assert validate_password("securepass") == "securepass"
+
+    def test_exactly_8_characters(self) -> None:
+        assert validate_password("12345678") == "12345678"
+
+    def test_rejects_7_characters(self) -> None:
+        with pytest.raises(UserServiceError, match="at least 8"):
+            validate_password("1234567")
+
+    def test_rejects_empty(self) -> None:
+        with pytest.raises(UserServiceError, match="at least 8"):
+            validate_password("")
+
+    def test_strips_whitespace(self) -> None:
+        assert validate_password("  securepass  ") == "securepass"
+
+    def test_whitespace_only_too_short(self) -> None:
+        with pytest.raises(UserServiceError, match="at least 8"):
+            validate_password("       ")