[Issue #11658] Relax CommonGrants URL pre-validation from strict to lax pydantic HttpUrl

api/src/services/common_grants/url_utils.py

@@ -13,16 +13,17 @@
 
 from marshmallow import ValidationError as MarshmallowValidationError
 from marshmallow import fields as marshmallow_fields
-from pydantic import BaseModel, Field, HttpUrl, ValidationError
+from pydantic import BaseModel, HttpUrl, ValidationError
 
 
 class _UrlValidator(BaseModel):
-    """Pydantic strict HttpUrl validator.
+    """Pydantic HttpUrl validator.
 
     Mirrors the HttpUrl field in OpportunityBase and other CommonGrants models.
+    Not strict: strict drops URLs (e.g. raw ``{}`` in a query) the pipeline serves fine.
     """
 
-    url: HttpUrl = Field(strict=True)
+    url: HttpUrl
 
 
 _marshmallow_url_field = marshmallow_fields.URL()

api/tests/src/services/common_grants/test_transformation.py

@@ -1,7 +1,7 @@
 """Tests for the transformation utility."""
 
 from datetime import date, datetime, timezone
-from urllib.parse import urlparse
+from http import HTTPStatus
 from uuid import uuid4
 
 from common_grants_sdk.schemas.pydantic import (
@@ -12,15 +12,21 @@
     MoneyRange,
     MoneyRangeFilter,
     OppFilters,
+    OpportunitiesSearchResponse,
     OppSortBy,
     OppSorting,
     OppStatusOptions,
     PaginatedBodyParams,
+    PaginatedResultsInfo,
     RangeOperator,
+    SortedResultsInfo,
     StringArrayFilter,
 )
 from freezegun import freeze_time
 
+from src.api.common_grants.schemas.marshmallow.schemas import (
+    OpportunitiesSearchResponse as OpportunitiesSearchResponseSchema,
+)
 from src.api.common_grants.schemas.marshmallow.schemas import OpportunityCustomFields
 from src.api.common_grants.schemas.pydantic.custom_fields import (
     AgencyField,
@@ -47,28 +53,42 @@
 )
 
 
-def _legacy_validate_url(value: str | None) -> str | None:
-    """
-    Validate a URL string.
-
-    Args:
-        value: The string to validate
-
-    Returns:
-        A valid URL string or None
-    """
-    # Parse the string
-    parsed = urlparse(value)
-
-    # Check for scheme and netloc (i.e. it's a complete url)
-    if parsed.scheme and parsed.netloc:
-        return value
+def _opp_data_with_info_url(url: str, description: str) -> dict:
+    """Minimal search-result payload whose summary.additional_info_url is `url`."""
+    return {
+        "opportunity_id": uuid4(),
+        "opportunity_title": "Test Opportunity",
+        "opportunity_status": OpportunityStatus.POSTED,
+        "created_at": datetime(2024, 1, 1, 12, 0, 0),
+        "updated_at": datetime(2024, 1, 2, 12, 0, 0),
+        "summary": {
+            "summary_description": "Test description",
+            "post_date": date(2024, 1, 1),
+            "close_date": date(2024, 12, 31),
+            "additional_info_url": url,
+            "additional_info_url_description": description,
+            "created_at": datetime(2024, 1, 1, 12, 0, 0),
+            "updated_at": datetime(2024, 1, 2, 12, 0, 0),
+        },
+    }
 
-    # Check for netloc only (i.e. it's a domain name)
-    if not parsed.scheme and parsed.netloc:
-        return f"https://{value}"
 
-    return None
+def _load_through_response_schema(cg_opportunity) -> dict:
+    """Run one CG opportunity down the route's response path: pydantic response model
+    -> model_dump -> marshmallow load. The load is what 500s on a divergent URL."""
+    response = OpportunitiesSearchResponse(
+        status=HTTPStatus.OK,
+        message="ok",
+        items=[cg_opportunity],
+        pagination_info=PaginatedResultsInfo(page=1, page_size=1, totalItems=1, totalPages=1),
+        sort_info=SortedResultsInfo(
+            sort_by=OppSortBy.LAST_MODIFIED_AT.value,
+            sort_order="desc",
+            errors=[],
+        ),
+        filter_info=build_filter_info(None),
+    )
+    return OpportunitiesSearchResponseSchema().load(response.model_dump(by_alias=True, mode="json"))
 
 
 DEFAULT_MOCK_OPP_FIELDS = {
@@ -698,25 +718,23 @@ def test_transform_search_result_to_cg_with_invalid_data(self):
         # Should return None for invalid data
         assert result is None
 
-    def test_validate_url_with_nasa_url_bug(self):
-        """Test that validate_url() properly rejects URLs that Pydantic HttpUrl rejects.
-
-        This test reproduces a bug where validate_url() lets through URLs
-        that Pydantic's HttpUrl validation rejects, causing transformations to fail.
-
-        The NASA URL has curly braces or other characters that urlparse accepts but
-        Pydantic's strict HttpUrl validation rejects.
+    def test_validate_url_nasa_url_round_trips_through_response_pipeline(self):
+        """A URL that survives validate_url() must survive the whole response path.
+        Fails if a future SDK re-tightens URL validation, which would otherwise drop
+        the field (or 500) in prod instead of here.
         """
-        # This URL has characters that urlparse accepts but Pydantic HttpUrl rejects
-        # Based on error: "non-URL code point" - likely curly braces or other invalid chars
         nasa_url = "https://nspires.nasaprs.com/external/solicitations/summary!init.do?solId={D8604BE7-CAB6-C1C0-B668-423042C43AA6}&path=&method=init"
 
-        # Old implementation used urlparse, new implementation uses Pydantic HttpUrl
-        old_result = _legacy_validate_url(nasa_url)
-        new_result = validate_url(nasa_url)
+        assert validate_url(nasa_url) == nasa_url
 
-        assert old_result is not None, "_legacy_validate_url() should accept NASA URL"
-        assert new_result is None, "validate_url() should reject NASA URL"
+        cg_opportunity = transform_search_result_to_cg(
+            _opp_data_with_info_url(nasa_url, "NSPIRES solicitation")
+        )
+        assert cg_opportunity is not None
+        assert str(cg_opportunity.source) == nasa_url
+
+        validated = _load_through_response_schema(cg_opportunity)
+        assert validated["items"][0]["source"] == nasa_url
 
     def test_validate_url_rejects_urls_that_marshmallow_url_field_rejects(self):
         """validate_url() must also reject URLs that pass Pydantic's HttpUrl but fail
@@ -754,55 +772,15 @@ def test_search_response_marshmallow_load_does_not_500_on_problematic_url(self):
         items[0].customFields.additionalInfo.value.url. After the fix the record loads
         cleanly with additionalInfo absent (because validate_url returned None).
         """
-        from http import HTTPStatus
-
-        from common_grants_sdk.schemas.pydantic import (
-            OpportunitiesSearchResponse,
-            PaginatedResultsInfo,
-            SortedResultsInfo,
-        )
-
-        from src.api.common_grants.schemas.marshmallow.schemas import (
-            OpportunitiesSearchResponse as OpportunitiesSearchResponseSchema,
-        )
-
         problematic_url = "https://www.grants.gov,https://other.example"
-        opp_data = {
-            "opportunity_id": uuid4(),
-            "opportunity_title": "Test Opportunity",
-            "opportunity_status": OpportunityStatus.POSTED,
-            "created_at": datetime(2024, 1, 1, 12, 0, 0),
-            "updated_at": datetime(2024, 1, 2, 12, 0, 0),
-            "summary": {
-                "summary_description": "Test description",
-                "post_date": date(2024, 1, 1),
-                "close_date": date(2024, 12, 31),
-                "additional_info_url": problematic_url,
-                "additional_info_url_description": "Multiple links",
-                "created_at": datetime(2024, 1, 1, 12, 0, 0),
-                "updated_at": datetime(2024, 1, 2, 12, 0, 0),
-            },
-        }
-
-        cg_opportunity = transform_search_result_to_cg(opp_data)
-        assert cg_opportunity is not None, "transform itself must not drop the record"
 
-        response = OpportunitiesSearchResponse(
-            status=HTTPStatus.OK,
-            message="ok",
-            items=[cg_opportunity],
-            pagination_info=PaginatedResultsInfo(page=1, page_size=1, totalItems=1, totalPages=1),
-            sort_info=SortedResultsInfo(
-                sort_by=OppSortBy.LAST_MODIFIED_AT.value,
-                sort_order="desc",
-                errors=[],
-            ),
-            filter_info=build_filter_info(None),
+        cg_opportunity = transform_search_result_to_cg(
+            _opp_data_with_info_url(problematic_url, "Multiple links")
         )
-        response_json = response.model_dump(by_alias=True, mode="json")
+        assert cg_opportunity is not None, "transform itself must not drop the record"
 
-        # This is the line that raises marshmallow.ValidationError in prod.
-        validated = OpportunitiesSearchResponseSchema().load(response_json)
+        # The load inside is the line that raises marshmallow.ValidationError in prod.
+        validated = _load_through_response_schema(cg_opportunity)
 
         assert len(validated["items"]) == 1
         # Once validate_url filters the bad URL, additionalInfo is absent; that's the
@@ -896,43 +874,6 @@ def test_dropped_url_log_includes_field_path_and_opportunity_id(self, caplog):
             assert getattr(r, "opportunity_id", None) == opp_id
             assert r.levelname == "WARNING"
 
-    def test_transform_search_result_to_cg_with_nasa_url_bug(self):
-        """Test that transformation works correctly with NASA URL that Pydantic rejects.
-
-        This test ensures that when validate_url() properly rejects invalid URLs,
-        the transformation still succeeds (with source=None) rather than failing.
-        """
-        # This URL has characters that urlparse accepts but Pydantic HttpUrl rejects
-        nasa_url = "https://nspires.nasaprs.com/external/solicitations/summary!init.do?solId={D8604BE7-CAB6-C1C0-B668-423042C43AA6}&path=&method=init"
-        assert (
-            _legacy_validate_url(nasa_url) is not None
-        ), "_legacy_validate_url() should accept NASA URL"
-
-        opp_data = {
-            "opportunity_id": uuid4(),
-            "opportunity_title": "Test Opportunity",
-            "opportunity_status": OpportunityStatus.POSTED,
-            "created_at": datetime(2024, 1, 1, 12, 0, 0),
-            "updated_at": datetime(2024, 1, 2, 12, 0, 0),
-            "summary": {
-                "summary_description": "Test description",
-                "post_date": date(2024, 1, 1),
-                "close_date": date(2024, 12, 31),
-                "estimated_total_program_funding": 1000000,
-                "award_ceiling": 500000,
-                "award_floor": 10000,
-                "additional_info_url": nasa_url,
-            },
-        }
-
-        # The transformation should succeed without raising a Pydantic validation error
-        result = transform_search_result_to_cg(opp_data)
-
-        # After fix: validate_url() should reject the URL, so source should be None
-        # but transformation should still succeed
-        assert result is not None, "Transformation should succeed even with invalid URL"
-        assert result.source is None, "Invalid URL should result in source=None"
-
     def test_build_money_range_filter(self):
         """Test building money range filters."""
         # Test with min and max amounts
@@ -1087,8 +1028,9 @@ def test_url_validation_error_logging(self, caplog):
         # Set up logging to capture info level logs
         caplog.set_level(logging.INFO)
 
-        # Test with an invalid URL that should trigger the logging
-        invalid_url = "https://example.com/path/{invalid-chars}"
+        # Comma-separated URL: pydantic accepts it, marshmallow rejects it, so the
+        # dual-check drops it and logs.
+        invalid_url = "https://example.com,https://other.example"
 
         result = validate_url(invalid_url)
 
@@ -1162,8 +1104,7 @@ def test_transformation_with_invalid_url_logs_but_succeeds(self, caplog):
         # a data-suppression event, not a routine validation observation.
         caplog.set_level(logging.WARNING)
 
-        # Create opportunity data with an invalid URL
-        invalid_url = "https://example.com/path/{invalid}"
+        invalid_url = "https://example.com,https://other.example"
         opp_data = {
             "opportunity_id": uuid4(),
             "opportunity_title": "Test Opportunity",

api/tests/src/services/common_grants/test_url_utils.py

@@ -3,15 +3,15 @@
 import pytest
 from marshmallow import ValidationError as MarshmallowValidationError
 from marshmallow import fields as marshmallow_fields
-from pydantic import BaseModel, Field, HttpUrl
+from pydantic import BaseModel, HttpUrl
 
 from src.services.common_grants.url_utils import validate_url_compatible
 
 
 class _PydanticHttpUrl(BaseModel):
     """Minimal pydantic HttpUrl validator used to demonstrate the divergence."""
 
-    url: HttpUrl = Field(strict=True)
+    url: HttpUrl
 
 
 class TestValidateUrlCompatible:
@@ -36,6 +36,16 @@ def test_rejects_garbage(self):
         assert validate_url_compatible("not-a-url") is None
         assert validate_url_compatible("sam.gov") is None  # missing scheme
 
+    def test_accepts_unencoded_braces_in_query(self):
+        """Raw {} in a query is RFC-invalid but the whole pipeline accepts it."""
+        url = "https://nspires.nasaprs.com/x/summary.do?solId={9455D565-3411-0574}&method=init"
+        assert validate_url_compatible(url) == url
+
+    def test_rejects_non_http_schemes(self):
+        # HttpUrl restricts scheme to http/https regardless of strictness.
+        assert validate_url_compatible("javascript:alert(1)") is None
+        assert validate_url_compatible("ftp://example.gov/file") is None
+
     @pytest.mark.parametrize(
         "bad_url",
         [