Merge pull request #10 from currentslab/agent/0.1.2-hardening

fix(client): hardening round from external audit (0.1.2)

Author: Autumn-Atlas

CHANGELOG.md

@@ -1,5 +1,25 @@
 # Changelog
 
+## 0.1.2
+
+### Fixed
+- Restored compatibility with the declared `python-dateutil>=2.8.0` floor:
+  the date-parse error handler no longer references `parser.ParserError`
+  (introduced in dateutil 2.8.1).
+- Non-JSON error responses (e.g. HTML gateway 502s) and non-object JSON
+  error payloads now raise `CurrentsAPIError` instead of leaking
+  `JSONDecodeError`/`AttributeError`. The exception now carries the real
+  HTTP status code via its `status` property; the payload's `status` value
+  is only used when no HTTP status is available.
+- `search()` now rejects falsey invalid arguments (`keywords=0`, empty/
+  whitespace keywords) instead of silently omitting them.
+- Naive `datetime` inputs are rejected with a clear `ValueError` (they were
+  silently labeled UTC before); `date` objects are now explicitly formatted
+  as UTC midnight.
+- `examples/source_linked_briefing` now escapes publisher-controlled
+  Markdown text and validates URL schemes, matching the protection the
+  company-monitor example already had (blocks `javascript:` link injection).
+
 ## 0.1.1
 
 ### Fixed

currentsapi/__init__.py

@@ -1,7 +1,7 @@
 import sys
 
 __project__ = "currentsapi"
-__version__ = "0.1.1"
+__version__ = "0.1.2"
 __repo__ = "https://github.com/currentslab/currentsapi-python"
 
 from .client import CurrentsAPI

currentsapi/client.py

@@ -9,19 +9,25 @@
 class CurrentsAPIError(Exception):
     """Raised when the Currents API returns an error response."""
 
-    def __init__(self, response):
+    def __init__(self, response, http_status=None):
+        if not isinstance(response, dict):
+            response = {}
         self.response = response
-        self._status = response.get("status")
+        self._http_status = http_status
         self._code = response.get("code")
         self._message = response.get("message") or response.get("msg")
-        super().__init__(self._message or str(response))
+        super().__init__(self._message or (str(response) if response else "Unknown API error"))
 
     @property
     def status(self):
+        """HTTP status code as int; payload status used only as last resort."""
+        if self._http_status is not None:
+            return self._http_status
+        payload_status = self.response.get("status")
         try:
-            return int(self._status)
+            return int(payload_status)
         except (TypeError, ValueError):
-            return self._status
+            return payload_status
 
     @property
     def code(self):
@@ -58,17 +64,36 @@ def _get(self, endpoint, params=None):
             params=params or {},
         )
         if r.status_code != requests.codes.ok:
-            raise CurrentsAPIError(r.json())
-        return r.json()
+            raise self._error_from_response(r)
+        try:
+            payload = r.json()
+        except ValueError:
+            raise CurrentsAPIError(
+                {"message": "Response body is not valid JSON"},
+                http_status=r.status_code,
+            )
+        return payload
+
+    @staticmethod
+    def _error_from_response(r):
+        try:
+            payload = r.json()
+        except ValueError:
+            payload = {}
+        if not isinstance(payload, dict):
+            payload = {
+                "message": "API returned a non-object error payload",
+                "details": payload,
+            }
+        return CurrentsAPIError(payload, http_status=r.status_code)
 
     def latest_news(self, language=None):
         params = {}
-        if language:
+        if language is not None:
             if not isinstance(language, str):
                 raise ValueError("language must be a string")
             params["language"] = language
         return self._get(self.latest_endpoint, params)
-
     def search(
         self,
         language=None,
@@ -80,32 +105,38 @@ def search(
     ):
         params = {}
 
-        if keywords:
+        if keywords is not None:
             if not isinstance(keywords, str):
                 raise ValueError("keywords must be a string")
+            if not keywords.strip():
+                raise ValueError("keywords must not be empty")
             params["keywords"] = keywords
 
-        if country:
+        if country is not None:
             if not isinstance(country, str):
                 raise ValueError("country must be a string")
             params["country"] = country
 
-        if language:
+        if language is not None:
             if not isinstance(language, str):
                 raise ValueError("language must be a string")
             params["language"] = language
 
-        if category:
+        if category is not None:
             if not isinstance(category, str):
                 raise ValueError("category must be a string")
             params["category"] = category
 
-        if start_date:
-            date = self._normalize_date(self._parse_date(start_date, "start_date"))
+        if start_date is not None:
+            date = self._normalize_date(
+                self._parse_date(start_date, "start_date"), "start_date"
+            )
             params["start_date"] = date.strftime("%Y-%m-%dT%H:%M:%SZ")
 
-        if end_date:
-            date = self._normalize_date(self._parse_date(end_date, "end_date"))
+        if end_date is not None:
+            date = self._normalize_date(
+                self._parse_date(end_date, "end_date"), "end_date"
+            )
             params["end_date"] = date.strftime("%Y-%m-%dT%H:%M:%SZ")
 
         return self._get(self.search_endpoint, params)
@@ -124,7 +155,7 @@ def _parse_date(date_value, param_name):
         if isinstance(date_value, str):
             try:
                 return parser.parse(date_value)
-            except (parser.ParserError, OverflowError, ValueError) as exc:
+            except (OverflowError, ValueError) as exc:
                 raise ValueError(
                     "{} is not a parsable date: {}".format(param_name, exc)
                 ) from exc
@@ -138,7 +169,18 @@ def _parse_date(date_value, param_name):
             )
 
     @staticmethod
-    def _normalize_date(date_value):
-        if isinstance(date_value, datetime.datetime) and date_value.tzinfo is not None:
+    def _normalize_date(date_value, param_name):
+        if isinstance(date_value, datetime.datetime):
+            if date_value.tzinfo is None:
+                raise ValueError(
+                    "{} datetime must be timezone-aware; attach a tzinfo "
+                    "(naive datetimes are ambiguous and are NOT assumed to be "
+                    "UTC)".format(param_name)
+                )
             return date_value.astimezone(datetime.timezone.utc)
+        if isinstance(date_value, datetime.date):
+            return datetime.datetime(
+                date_value.year, date_value.month, date_value.day,
+                tzinfo=datetime.timezone.utc,
+            )
         return date_value

examples/_example_utils.py

@@ -0,0 +1,32 @@
+"""Shared helpers for the shipped Currents API examples."""
+
+import re
+
+
+_MARKDOWN_ESCAPES = re.compile(r"([\\`*_\[\]()<>#!|{}])")
+
+
+def escape_markdown_text(value):
+    """Escape Markdown-significant characters in publisher-controlled text."""
+    return _MARKDOWN_ESCAPES.sub(r"\\\1", str(value))
+
+
+def safe_markdown_url(url, allowed_schemes=("http", "https")):
+    """Return a URL safe to embed in Markdown, or ``None`` if unsafe.
+
+    Rejects empty values, non-string values, unknown/unsafe schemes
+    (e.g. ``javascript:``), and whitespace/control characters that could
+    break out of a Markdown link target.
+    """
+    if not isinstance(url, str):
+        return None
+    candidate = url.strip()
+    if not candidate:
+        return None
+    if any(ch.isspace() or ord(ch) < 0x20 for ch in candidate):
+        return None
+    match = re.match(r"^([A-Za-z][A-Za-z0-9+.-]*):", candidate)
+    if not match or match.group(1).lower() not in allowed_schemes:
+        return None
+    # Escape parentheses so the URL cannot break out of [..](..) syntax.
+    return candidate.replace("(", "%28").replace(")", "%29")

examples/source_linked_briefing/briefing.py

@@ -4,9 +4,15 @@
 import argparse
 import json
 import os
+import sys
 from datetime import datetime, timezone
 from pathlib import Path
 
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+from _example_utils import escape_markdown_text, safe_markdown_url
+
+sys.path.pop(0)
+
 
 def parse_args():
     parser = argparse.ArgumentParser(description=__doc__)
@@ -100,17 +106,17 @@ def resolve_generated_at(args, response):
 def build_output(response, generated_at):
     validate_response(response)
     articles = [normalize_article(article) for article in response["news"]]
-    articles.sort(key=lambda article: article["title"].casefold())
+    articles.sort(key=lambda article: escape_markdown_text(article["title"]).casefold())
     articles.sort(key=lambda article: article["published"], reverse=True)
-    lines = ["# Source-Linked News Briefing", "", "Generated at: {}".format(generated_at), ""]
+    lines = ["# Source-Linked News Briefing", "", "Generated at: {}".format(escape_markdown_text(generated_at)), ""]
     for article in articles:
-        title = article["title"]
-        url = article["url"]
+        title = escape_markdown_text(article["title"])
+        url = safe_markdown_url(article["url"])
         lines.append("- {} - <{}>".format(title, url) if url else "- {}".format(title))
         if article["published"]:
-            lines.append("  - Published: {}".format(article["published"]))
+            lines.append("  - Published: {}".format(escape_markdown_text(article["published"])))
         if article["description"]:
-            lines.append("  - {}".format(article["description"]))
+            lines.append("  - {}".format(escape_markdown_text(article["description"])))
     return "\n".join(lines) + "\n", {
         "generated_at": generated_at,
         "articles": articles,

tests/test_client.py

@@ -71,8 +71,8 @@ def test_search_all_params(self, mock_get):
             language="en",
             country="US",
             category="technology",
-            start_date="2024-01-15",
-            end_date="2024-06-30",
+            start_date="2024-01-15T00:00:00Z",
+            end_date="2024-06-30T00:00:00Z",
         )
         args, kwargs = mock_get.call_args
         self.assertEqual(
@@ -146,9 +146,10 @@ def test_api_error_raises_exception(self, mock_get):
         api = CurrentsAPI("key")
         with self.assertRaises(CurrentsAPIError) as ctx:
             api.latest_news()
-        self.assertEqual(ctx.exception.status, "error")
+        self.assertEqual(ctx.exception.status, 401)
         self.assertEqual(ctx.exception.code, "Unauthorized")
         self.assertEqual(ctx.exception.message, "Invalid key")
+        self.assertEqual(ctx.exception.response["status"], "error")
 
     def test_invalid_keywords_type(self):
         api = CurrentsAPI("key")
@@ -209,5 +210,57 @@ def test_impossible_date_string_raises_valueerror(self):
         with self.assertRaises(ValueError):
             api.search(start_date="2026-13-45")
 
+    @patch("currentsapi.client.requests.get")
+    def test_non_json_502_raises_currents_api_error(self, mock_get):
+        mock_get.return_value = Mock(status_code=502)
+        mock_get.return_value.json.side_effect = ValueError("Expecting value")
+        api = CurrentsAPI("key")
+        with self.assertRaises(CurrentsAPIError) as ctx:
+            api.latest_news()
+        self.assertEqual(ctx.exception.status, 502)
+
+    @patch("currentsapi.client.requests.get")
+    def test_non_object_error_payload_raises_currents_api_error(self, mock_get):
+        mock_get.return_value = Mock(status_code=401)
+        mock_get.return_value.json.return_value = []
+        api = CurrentsAPI("key")
+        with self.assertRaises(CurrentsAPIError) as ctx:
+            api.latest_news()
+        self.assertEqual(ctx.exception.status, 401)
+        self.assertIn("non-object", ctx.exception.message)
+
+    def test_falsey_keywords_rejected(self):
+        api = CurrentsAPI("key")
+        with self.assertRaises(ValueError):
+            api.search(keywords=0)
+        with self.assertRaises(ValueError):
+            api.search(keywords="   ")
+
+    def test_naive_datetime_rejected(self):
+        api = CurrentsAPI("key")
+        with self.assertRaises(ValueError):
+            api.search(start_date=datetime.datetime(2024, 6, 1, 12, 0))
+
+    def test_date_object_becomes_utc_midnight(self):
+        mock_response = Mock(status_code=200, json=Mock(return_value={"status": "ok"}))
+        with patch("currentsapi.client.requests.get", return_value=mock_response) as mock_get:
+            api = CurrentsAPI("key")
+            api.search(start_date=datetime.date(2024, 6, 1))
+        kwargs = mock_get.call_args.kwargs
+        self.assertEqual(kwargs["params"]["start_date"], "2024-06-01T00:00:00Z")
+
+    def test_status_prefers_http_code_over_payload(self):
+        exc = CurrentsAPIError({"status": "error"}, http_status=401)
+        self.assertEqual(exc.status, 401)
+        exc2 = CurrentsAPIError({"status": "404"})
+        self.assertEqual(exc2.status, 404)
+        exc3 = CurrentsAPIError({"status": "error"})
+        self.assertEqual(exc3.status, "error")
+
+    def test_exception_accepts_non_dict_payload(self):
+        exc = CurrentsAPIError(["unexpected"])
+        self.assertIsNone(exc.code)
+        self.assertEqual(str(exc), "Unknown API error")
+
 if __name__ == "__main__":
     unittest.main()