amorfo77/parsedmarc
1
0
Fork 0
mirror of https://github.com/domainaware/parsedmarc.git synced 2026-08-02 13:42:19 +00:00
Code Issues Projects Releases Wiki Activity

IPinfo API: keep only documented behavior (#721)

Browse Source 
* Strip invented IPinfo API behavior; keep documented-only

The IPinfo Lite API docs (https://ipinfo.io/developers/lite-api) state:
"The API has no daily or monthly limit and provides unlimited access."
Auth is documented as a ?token= query param only. The /me shown in the
docs returns geolocation for the caller's IP — it is not a documented
account/quota endpoint for Lite.

Removed everything that was speculating beyond the docs:

- The /me probe that pretended to return plan/limit/remaining fields.
- 429 rate-limit handling, 402 quota-exhausted handling, Retry-After
  parsing, cooldown state, and the rate-limit warning / recovery-info
  logging around them.
- The Authorization: Bearer header (not documented for Lite).

Kept:

- Lookups against the documented /lite/<ip>?token=<token> endpoint.
- 401/403 treated as a fatal invalid-token (reasonable defensive check).
- Network-error and non-2xx fallback to the bundled/cached MMDB.
- A simple startup probe that validates the token with a single lookup
  and logs "IPinfo API configured" at info level.

Test consolidated to cover only documented paths: success, 401 fatal,
non-2xx fallback, and that auth goes in ?token= (not Authorization).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* AGENTS.md: warn against speculating past external-service docs

New subsection under Configuration spelling out that third-party API
integrations must start with a direct WebFetch of the canonical docs
page, not a subagent query. Calls out the two traps that produced the
IPinfo speculation: (1) asking subagents question shapes that
presuppose the answer exists, and (2) treating feature asks as "build
this" without first checking "does this apply to this service?".

Uses the now-reverted IPinfo speculation as the cautionary tale so the
next session has a concrete example to recognize the shape of the
mistake.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Bump to 9.10.1; put removal under a new CHANGELOG section

Restored the 9.10.0 entry to its as-shipped wording and moved the
speculation-removal note into its own 9.10.1 Fixed section.
Editing the 9.10.0 entry would have misrepresented what was
actually released — the shipped tag does contain the /me probe,
429/402 cooldown, Retry-After parsing, and Bearer auth, and the
changelog should say so.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Sean Whalen <seanthegeek@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Sean Whalen
2026-04-23 11:51:44 -04:00
committed by GitHub
co-authored by Claude Opus 4.7 Sean Whalen
parent 9d1152d4f8
commit f0781c6191
5 changed files with 68 additions and 282 deletions
Download Patch File Download Diff File Expand all files Collapse all files
AGENTS.md
+11
View File
@@ -67,6 +67,17 @@ Every new option becomes documented surface area the project has to support fore
When you do add an option: surface it in the INI schema, the `_parse_config` branch, the `Namespace` defaults, the CLI docs (`docs/source/usage.md`), and SIGHUP-reload wiring together in one PR. Half-wired options (parsed but not consulted, or consulted but not documented) are worse than none.
#### Read the primary source before coding against an external service
For any third-party REST API, SDK, on-disk format, or protocol, fetch the actual docs page with `WebFetch` as the first step — before writing code, and before spawning a research subagent. Only after confirming what the docs actually say should you ask "how do I handle this?".
Two traps to avoid:
- **Don't outsource primary-source reading to subagents.** Asking a subagent "what are service X's rate-limit codes?" presupposes those codes exist; the agent will synthesize a plausible-sounding answer from adjacent APIs, community posts, and HTTP conventions even when the service documents none of it. Subagents are good for cross-source synthesis, bad for "what does this one page say" — use `WebFetch` yourself for the latter.
- **Don't treat a feature ask as "build this" without first checking "does this apply?".** If the user asks for rate-limit fallback, verify rate limits exist for this service. If they ask to log quota, verify a quota endpoint exists. When the docs are silent on an edge case, silence means "not specified", not "use HTTP conventions" — default to not implementing it, or flag the assumption in the PR body.
Canonical cautionary tale: the IPinfo Lite integration initially shipped ~230 lines of speculative 429/402 cooldown, `Retry-After` parsing, a fabricated `/me` plan/quota endpoint, and `Authorization: Bearer` auth — none of which the Lite docs support. The docs open with "The API has no daily or monthly limit" and document `?token=` query-param auth only. All of it was removed in a follow-up PR. Don't reintroduce any of it here, and apply the same rule to other external integrations.
### Caching
IP address info cached for 4 hours, seen aggregate report IDs cached for 1 hour (via `ExpiringDict`).
CHANGELOG.md
+6
View File
@@ -1,5 +1,11 @@
# Changelog
## 9.10.1
### Fixed
- Stripped speculative behavior from the IPinfo Lite REST API integration shipped in 9.10.0 after auditing the code against the [Lite API docs](https://ipinfo.io/developers/lite-api). The docs state the Lite API has "no daily or monthly limit and provides unlimited access" and document `?token=` query-parameter auth only; nothing else removed here is documented for Lite. Removed: the 429 rate-limit and 402 quota-exhausted handling, `Retry-After` parsing, cooldown state, and the associated warning/recovery logging; the `https://ipinfo.io/me` account-info probe that expected plan/limit/remaining fields (that endpoint isn't a Lite account endpoint); and the `Authorization: Bearer` header. Auth is now the documented `?token=` query param; the startup probe is a single `/lite/1.1.1.1` lookup that logs `IPinfo API configured` at info level. Retained behavior: 401/403 remains a fatal `InvalidIPinfoAPIKey`, and any other non-2xx or network error falls back to the bundled/cached MMDB per request.
## 9.10.0
### Changes
parsedmarc/constants.py
+1 -1
View File
@@ -1,4 +1,4 @@
__version__ = "9.10.0"
__version__ = "9.10.1"
USER_AGENT = f"parsedmarc/{__version__}"
parsedmarc/utils.py
+32 -200
View File
@@ -16,7 +16,6 @@ import re
import shutil
import subprocess
import tempfile
import time
from datetime import datetime, timedelta, timezone
from typing import Optional, TypedDict, Union, cast
@@ -472,31 +471,18 @@ class InvalidIPinfoAPIKey(Exception):
"""Raised when the IPinfo API rejects the configured token."""
# IPinfo Lite REST API. When ``_IPINFO_API_TOKEN`` is set, ``get_ip_address_db_record()``
# queries the API first and falls through to the bundled/cached MMDB only on
# rate-limit/quota/network errors. A 401/403 on any lookup propagates as
# ``InvalidIPinfoAPIKey`` so the CLI exits fatally; callers of the library
# should catch it.
# IPinfo Lite REST API. When ``_IPINFO_API_TOKEN`` is set,
# ``get_ip_address_db_record()`` queries the API first and falls back to the
# bundled/cached MMDB on any non-2xx response or network error. A 401/403
# propagates as ``InvalidIPinfoAPIKey`` so the CLI exits fatally.
#
# The IPinfo Lite API is documented as having no daily or monthly request
# limit ("unlimited access"), so there is no rate-limit or quota handling
# here — adding it would be inventing behavior the service doesn't document.
# Authentication uses the documented ``?token=`` query parameter.
_IPINFO_API_URL = "https://api.ipinfo.io/lite"
# Account-info / quota endpoint. Separate from the lookup URL because ``/me``
# lives at the ipinfo.io root, not under ``/lite``. Hitting it at startup
# both validates the token and surfaces plan/usage details; IPinfo documents
# it as a quota-free meta endpoint.
_IPINFO_ACCOUNT_URL = "https://ipinfo.io/me"
_IPINFO_API_TOKEN: Optional[str] = None
_IPINFO_API_TIMEOUT: float = 5.0
# Default cooldowns when the API returns 429/402 without a ``Retry-After``
# header. Rate limits are usually short; quota resets (402) are typically at a
# day/month boundary, so we pick a longer default there.
_IPINFO_API_RATE_LIMIT_COOLDOWN_SECONDS: float = 300.0
_IPINFO_API_QUOTA_COOLDOWN_SECONDS: float = 3600.0
# Unix timestamp before which lookups skip the API and go straight to the
# MMDB. ``0`` means the API is currently available.
_IPINFO_API_COOLDOWN_UNTIL: float = 0.0
# Latch for recovery logging: True while the API is in a rate-limited or
# quota-exhausted state, so the next successful lookup can log "recovered"
# exactly once per event.
_IPINFO_API_RATE_LIMITED: bool = False
def configure_ipinfo_api(
@@ -507,170 +493,50 @@ def configure_ipinfo_api(
"""Configure the IPinfo Lite REST API as the primary source for IP lookups.
When a token is configured, ``get_ip_address_db_record()`` hits the API
first for every lookup and falls back to the MMDB on rate-limit, quota, or
network errors. An invalid token raises ``InvalidIPinfoAPIKey`` — the CLI
catches that and exits fatally.
first for every lookup and falls back to the MMDB on network errors. An
invalid token raises ``InvalidIPinfoAPIKey`` — the CLI catches that and
exits fatally.
Args:
token: IPinfo API token. ``None`` or empty disables the API.
probe: If ``True``, verify the token by hitting ``/me`` (and, if that
is unreachable, by looking up ``1.1.1.1``). A 401/403 raises
``InvalidIPinfoAPIKey``; other errors are logged and the token is
still accepted so per-request fallback can take over.
probe: If ``True``, verify the token by looking up ``1.1.1.1``. A
401/403 raises ``InvalidIPinfoAPIKey``; other errors are logged
and the token is still accepted so per-request fallback can take
over.
"""
global _IPINFO_API_TOKEN
global _IPINFO_API_COOLDOWN_UNTIL, _IPINFO_API_RATE_LIMITED
_IPINFO_API_TOKEN = token or None
_IPINFO_API_COOLDOWN_UNTIL = 0.0
_IPINFO_API_RATE_LIMITED = False
if not _IPINFO_API_TOKEN:
if not _IPINFO_API_TOKEN or not probe:
return
if probe:
# Verify the token. Any network/quota failure here is non-fatal — we
# still accept the token and let per-request fallback handle it — but
# an invalid-key response must fail fast so operators notice
# immediately instead of seeing silent MMDB-only lookups all day.
#
# The /me meta endpoint doubles as a free-of-quota token check and a
# plan/usage lookup, so we try it first. If /me is unreachable, fall
# back to a lookup of 1.1.1.1 to validate the token.
account: Optional[dict] = None
try:
account = _ipinfo_api_account_info()
except InvalidIPinfoAPIKey:
raise
except Exception as e:
logger.debug(f"IPinfo account info fetch failed: {e}")
if account is not None:
summary = _format_ipinfo_account_summary(account)
if summary:
logger.info(f"IPinfo API configured — {summary}")
else:
logger.info("IPinfo API configured")
return
try:
_ipinfo_api_lookup("1.1.1.1")
except InvalidIPinfoAPIKey:
raise
except Exception as e:
logger.warning(f"IPinfo API probe failed (will fall back per-request): {e}")
else:
logger.info("IPinfo API configured")
def _ipinfo_api_account_info() -> Optional[dict]:
"""Fetch the IPinfo ``/me`` account endpoint.
Returns the parsed JSON dict on success, or ``None`` when the endpoint is
unreachable (network error, non-JSON body, non-2xx other than 401/403).
A 401/403 raises ``InvalidIPinfoAPIKey`` — this endpoint is the best way
to validate a token since it doesn't consume a lookup-quota unit.
"""
if not _IPINFO_API_TOKEN:
return None
headers = {
"User-Agent": USER_AGENT,
"Authorization": f"Bearer {_IPINFO_API_TOKEN}",
"Accept": "application/json",
}
response = requests.get(
_IPINFO_ACCOUNT_URL, headers=headers, timeout=_IPINFO_API_TIMEOUT
)
if response.status_code in (401, 403):
raise InvalidIPinfoAPIKey(
f"IPinfo API rejected the configured token (HTTP {response.status_code})"
)
if not response.ok:
logger.debug(f"IPinfo /me returned HTTP {response.status_code}")
return None
try:
payload = response.json()
except ValueError:
return None
return payload if isinstance(payload, dict) else None
def _format_ipinfo_account_summary(account: dict) -> Optional[str]:
"""Render a short, log-friendly summary of the IPinfo /me response.
Field names in /me have varied across IPinfo plan generations, so we
probe a few aliases rather than commit to one schema. If nothing
useful is present we return ``None`` and the caller falls back to a
generic "configured" message.
"""
plan = (
account.get("plan")
or account.get("tier")
or account.get("token_type")
or account.get("type")
)
limit = account.get("limit") or account.get("monthly_limit")
remaining = account.get("remaining") or account.get("requests_remaining")
used = account.get("month") or account.get("month_requests") or account.get("used")
parts = []
if plan:
parts.append(f"plan: {plan}")
if used is not None and limit:
parts.append(f"usage: {used}/{limit} this month")
elif limit:
parts.append(f"monthly limit: {limit}")
if remaining is not None:
parts.append(f"{remaining} remaining")
return ", ".join(parts) if parts else None
def _parse_retry_after(response, default_seconds: float) -> float:
"""Parse an HTTP ``Retry-After`` header as seconds.
Supports the delta-seconds form. HTTP-date form is rare enough for an API
client to ignore; we just fall back to the default.
"""
raw = response.headers.get("Retry-After")
if raw:
try:
return max(float(raw.strip()), 1.0)
except ValueError:
pass
return default_seconds
_ipinfo_api_lookup("1.1.1.1")
except InvalidIPinfoAPIKey:
raise
except Exception as e:
logger.warning(f"IPinfo API probe failed (will fall back per-request): {e}")
else:
logger.info("IPinfo API configured")
def _ipinfo_api_lookup(ip_address: str) -> Optional[_IPDatabaseRecord]:
"""Look up an IP via the IPinfo Lite REST API.
Returns the normalized record on success, or ``None`` when the API is
unavailable for any reason the caller should fall back from (network
error, 429 rate limit, 402 quota exhausted, malformed response).
On 429/402 the API is put in a cooldown (using ``Retry-After`` when
present) so we stop hammering it, and we log once per event at warning
level. After the cooldown expires the next lookup retries transparently;
a successful retry logs "API recovered" once at info level so operators
can see service came back.
Raises:
InvalidIPinfoAPIKey: on 401/403. Propagates to abort the run.
Returns the normalized record on success, or ``None`` on network error or
any non-2xx response (other than 401/403). 401/403 raises
``InvalidIPinfoAPIKey``.
"""
global _IPINFO_API_COOLDOWN_UNTIL, _IPINFO_API_RATE_LIMITED
if not _IPINFO_API_TOKEN:
return None
if _IPINFO_API_COOLDOWN_UNTIL and time.time() < _IPINFO_API_COOLDOWN_UNTIL:
return None
url = f"{_IPINFO_API_URL}/{ip_address}"
headers = {
"User-Agent": USER_AGENT,
"Authorization": f"Bearer {_IPINFO_API_TOKEN}",
"Accept": "application/json",
}
params = {"token": _IPINFO_API_TOKEN}
headers = {"User-Agent": USER_AGENT, "Accept": "application/json"}
try:
response = requests.get(url, headers=headers, timeout=_IPINFO_API_TIMEOUT)
response = requests.get(
url, headers=headers, params=params, timeout=_IPINFO_API_TIMEOUT
)
except requests.exceptions.RequestException as e:
logger.debug(f"IPinfo API request for {ip_address} failed: {e}")
return None
@@ -679,35 +545,6 @@ def _ipinfo_api_lookup(ip_address: str) -> Optional[_IPDatabaseRecord]:
raise InvalidIPinfoAPIKey(
f"IPinfo API rejected the configured token (HTTP {response.status_code})"
)
if response.status_code == 429:
cooldown = _parse_retry_after(response, _IPINFO_API_RATE_LIMIT_COOLDOWN_SECONDS)
_IPINFO_API_COOLDOWN_UNTIL = time.time() + cooldown
# First hit of a rate-limit event is visible at warning; subsequent
# 429s after cooldown-and-retry cycles stay at debug so we don't spam
# the log when a run spans a long quota reset.
if not _IPINFO_API_RATE_LIMITED:
logger.warning(
"IPinfo API rate limit hit; falling back to the local MMDB "
f"for {cooldown:.0f}s before retrying"
)
_IPINFO_API_RATE_LIMITED = True
else:
logger.debug(f"IPinfo API still rate-limited; retry after {cooldown:.0f}s")
return None
if response.status_code == 402:
cooldown = _parse_retry_after(response, _IPINFO_API_QUOTA_COOLDOWN_SECONDS)
_IPINFO_API_COOLDOWN_UNTIL = time.time() + cooldown
if not _IPINFO_API_RATE_LIMITED:
logger.warning(
"IPinfo API quota exhausted; falling back to the local MMDB "
f"for {cooldown:.0f}s before retrying"
)
_IPINFO_API_RATE_LIMITED = True
else:
logger.debug(
f"IPinfo API quota still exhausted; retry after {cooldown:.0f}s"
)
return None
if not response.ok:
logger.debug(
f"IPinfo API returned HTTP {response.status_code} for {ip_address}"
@@ -722,11 +559,6 @@ def _ipinfo_api_lookup(ip_address: str) -> Optional[_IPDatabaseRecord]:
if not isinstance(payload, dict):
return None
if _IPINFO_API_RATE_LIMITED:
logger.info("IPinfo API recovered; resuming API lookups")
_IPINFO_API_RATE_LIMITED = False
_IPINFO_API_COOLDOWN_UNTIL = 0.0
return _normalize_ip_record(payload)
tests.py
+18 -81
View File
@@ -275,25 +275,27 @@ class Test(unittest.TestCase):
self.assertEqual(info["as_domain"], "unmapped-for-this-test.example")
def testIPinfoAPIPrimarySourceAndInvalidKeyIsFatal(self):
"""With an API token configured, lookups hit the API first. A 401/403
response propagates as ``InvalidIPinfoAPIKey`` so the CLI can exit.
A 429 puts the API in a cooldown (falling back to the MMDB) and a
successful retry after the cooldown logs recovery."""
"""With an API token configured, lookups hit the API first via the
documented ?token= query param. A 401/403 response propagates as
``InvalidIPinfoAPIKey`` so the CLI can exit fatally. Any other
non-2xx or network error falls through to the MMDB silently.
The IPinfo Lite API is documented as having no request limit, so
there is no rate-limit/quota handling to test — only the fatal path
on invalid tokens and the success path."""
from unittest.mock import patch, MagicMock
import parsedmarc.utils as utils_module
from parsedmarc.utils import (
InvalidIPinfoAPIKey,
configure_ipinfo_api,
get_ip_address_db_record,
)
def _mock_response(status_code, json_body=None, headers=None):
def _mock_response(status_code, json_body=None):
resp = MagicMock()
resp.status_code = status_code
resp.ok = 200 <= status_code < 300
resp.json.return_value = json_body or {}
resp.headers = headers or {}
return resp
try:
@@ -308,12 +310,16 @@ class Test(unittest.TestCase):
with patch(
"parsedmarc.utils.requests.get",
return_value=_mock_response(200, api_json),
):
) as mock_get:
configure_ipinfo_api("fake-token", probe=False)
record = get_ip_address_db_record("8.8.8.8")
self.assertEqual(record["country"], "US")
self.assertEqual(record["asn"], 15169)
self.assertEqual(record["as_domain"], "google.com")
# Auth must use the documented query param, not a Bearer header.
_, kwargs = mock_get.call_args
self.assertEqual(kwargs["params"], {"token": "fake-token"})
self.assertNotIn("Authorization", kwargs["headers"])
# Invalid key: 401 raises a fatal exception even on a random lookup.
with patch(
@@ -324,84 +330,15 @@ class Test(unittest.TestCase):
with self.assertRaises(InvalidIPinfoAPIKey):
get_ip_address_db_record("8.8.8.8")
# Rate limited: 429 sets a cooldown and falls back to the MMDB.
# The first rate-limit event is logged at WARNING; during the
# cooldown no further API requests are made.
configure_ipinfo_api("rate-limited", probe=False)
# Any other non-2xx (e.g. 500, 503) falls back to the MMDB silently.
configure_ipinfo_api("fake-token", probe=False)
with patch(
"parsedmarc.utils.requests.get",
return_value=_mock_response(429, headers={"Retry-After": "120"}),
return_value=_mock_response(500),
):
with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
record = get_ip_address_db_record("8.8.8.8")
record = get_ip_address_db_record("8.8.8.8")
# MMDB fallback fills in Google's ASN from the bundled MMDB.
self.assertEqual(record["asn"], 15169)
self.assertTrue(
any("rate limit" in line.lower() for line in cm.output),
f"expected a rate-limit warning, got: {cm.output}",
)
self.assertTrue(utils_module._IPINFO_API_RATE_LIMITED)
self.assertGreater(utils_module._IPINFO_API_COOLDOWN_UNTIL, 0.0)
# During cooldown: no API call; fall straight through to MMDB.
poisoned = {"asn": "AS1", "country_code": "ZZ"}
with patch(
"parsedmarc.utils.requests.get",
return_value=_mock_response(200, poisoned),
) as mock_get:
record = get_ip_address_db_record("8.8.8.8")
mock_get.assert_not_called()
# Simulate the cooldown expiring, then a successful retry: the
# recovery is logged at INFO and API lookups resume.
utils_module._IPINFO_API_COOLDOWN_UNTIL = 0.0
with patch(
"parsedmarc.utils.requests.get",
return_value=_mock_response(200, api_json),
):
with self.assertLogs("parsedmarc.log", level="INFO") as cm:
record = get_ip_address_db_record("8.8.8.8")
self.assertEqual(record["as_domain"], "google.com")
self.assertTrue(
any("recovered" in line.lower() for line in cm.output),
f"expected a recovery info log, got: {cm.output}",
)
self.assertFalse(utils_module._IPINFO_API_RATE_LIMITED)
finally:
configure_ipinfo_api(None)
def testIPinfoAPIStartupLogsAccountQuota(self):
"""``configure_ipinfo_api(..., probe=True)`` should hit the /me
endpoint and log plan/usage info at INFO level when available."""
from unittest.mock import patch, MagicMock
from parsedmarc.utils import configure_ipinfo_api
me_body = {
"plan": "Lite",
"month": 12345,
"limit": 50000,
"remaining": 37655,
}
mock_resp = MagicMock()
mock_resp.status_code = 200
mock_resp.ok = True
mock_resp.json.return_value = me_body
mock_resp.headers = {}
try:
with patch(
"parsedmarc.utils.requests.get", return_value=mock_resp
) as mock_get:
with self.assertLogs("parsedmarc.log", level="INFO") as cm:
configure_ipinfo_api("good-token", probe=True)
# /me is the first (and only) probe request when it succeeds.
called_urls = [args[0] for args, _ in mock_get.call_args_list]
self.assertIn("https://ipinfo.io/me", called_urls)
output = " ".join(cm.output)
self.assertIn("Lite", output)
self.assertIn("12345/50000", output)
self.assertIn("37655", output)
finally:
configure_ipinfo_api(None)