diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a35343..be6c5c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,17 @@ exhaustive lists. ### Fixed +- With `network.tor: true`, personal credentials still went out with records + requests and told the server who was asking, even though the real IP was + hidden: `OPENFOIA_MUCKROCK_TOKEN`, `OPENFOIA_NARA_API_KEY` / + `CATALOG_API_KEY`, `OPENFOIA_GOVINFO_API_KEY`, and the crossref + `OPENFOIA_OPENSANCTIONS_API_KEY`. Over Tor, MuckRock and OpenSanctions now + send no key, GovInfo (and fec, regulations) use the shared `DEMO_KEY`, + and NARA, which needs a personal key, is skipped: `records search -s nara` + and `records fetch -s nara` exit with an explanation, and `--all-sources` + lists it as skipped. The route line reads "via Tor, real IP hidden, + credentials withheld" when a credential was held back. Tor still does not + hide what you search for (#131). - `network.tor: true` was ignored by `records search -s ` (every source except cia), `records search --all-sources` (all sources, cia included), `records fetch -s documentcloud`, `records fetch -s nara` without diff --git a/openfoia/cli.py b/openfoia/cli.py index 4689ea1..f7b44ee 100644 --- a/openfoia/cli.py +++ b/openfoia/cli.py @@ -5108,16 +5108,18 @@ def _records_search_all( sources = list_sources() policy = _records_policy_or_exit() data_gov_key = load_config().records.api_data_gov_key + adapters = {s: get_adapter(s, egress=policy, api_data_gov_key=data_gov_key) for s in sources} def say(msg: str) -> None: # Status goes to stderr in --json mode so stdout stays pure JSON lines. typer.echo(msg, err=json_lines) say( - f"Sending '{query}' to {len(sources)} services ({_route_note(policy)}): " + f"Sending '{query}' to {len(sources)} services " + f"({_route_note(policy, *adapters.values())}): " f"{', '.join(sources)}" ) - if "nara" in sources and getattr(get_adapter("nara", egress=policy), "has_key", False): + if "nara" in sources and getattr(adapters["nara"], "has_key", False): from .records.nara import ATTRIBUTION say( @@ -5130,8 +5132,10 @@ async def one(source: str): if kwargs.get(key) and source != owner: return source, None, f"skipped ({key} filter not supported)" try: - adapter = get_adapter(source, egress=policy, api_data_gov_key=data_gov_key) + adapter = adapters[source] if source == "nara" and not adapter.has_key: + if getattr(adapter, "credentials_withheld", False): + return source, None, "skipped (personal key not sent over Tor)" return source, None, "skipped (no OPENFOIA_NARA_API_KEY)" result = await _search_one(adapter, source, query, limit, kwargs) except Exception as e: @@ -5303,7 +5307,7 @@ def records_search( quiet = raw or json_lines if source != "cia": - typer.echo(f"Sending {query!r} to {source} ({_route_note(policy)}).", err=True) + typer.echo(f"Sending {query!r} to {source} ({_route_note(policy, adapter)}).", err=True) if source == "cia": notice = f"WARNING: This will send the query {query!r} to cia.gov ({_route_note(policy)})." if quiet: @@ -5311,11 +5315,9 @@ def records_search( else: rprint(f"\n[yellow]{notice}[/yellow]") elif source == "nara": - from .records.nara import ATTRIBUTION, NO_KEY_MESSAGE + from .records.nara import ATTRIBUTION - if not adapter.has_key: - rprint(f"[red]{NO_KEY_MESSAGE}[/red]") - raise typer.Exit(1) + _nara_key_or_exit(adapter) typer.echo( f"Note: sending {query or '*'!r} to catalog.archives.gov with your API key " f"(counted against the 10,000/month quota). {ATTRIBUTION}", @@ -5820,14 +5822,12 @@ def records_fetch( from .records import get_adapter policy = _records_policy_or_exit() - typer.echo(f"Fetching from {source} ({_route_note(policy)}).", err=True) adapter = get_adapter(source, egress=policy) + typer.echo(f"Fetching from {source} ({_route_note(policy, adapter)}).", err=True) if source == "nara": - from .records.nara import ATTRIBUTION, NO_KEY_MESSAGE + from .records.nara import ATTRIBUTION - if not adapter.has_key: - rprint(f"[red]{NO_KEY_MESSAGE}[/red]") - raise typer.Exit(1) + _nara_key_or_exit(adapter) typer.echo( "Note: fetching from catalog.archives.gov with your API key (one query per id, " f"counted against the 10,000/month quota). {ATTRIBUTION}", @@ -5998,6 +5998,14 @@ def _ingest_nara_objects(entity, naid: str, results) -> int: return done +def _nara_key_or_exit(adapter) -> None: + from .records.nara import NO_KEY_MESSAGE, TOR_KEY_MESSAGE + + if not adapter.has_key: + rprint(f"[red]{TOR_KEY_MESSAGE if adapter.credentials_withheld else NO_KEY_MESSAGE}[/red]") + raise typer.Exit(1) + + def _records_policy_or_exit() -> EgressPolicy: """Egress policy from config.network.tor, fail-closed when Tor is configured but down.""" from .config import load_config @@ -6007,8 +6015,11 @@ def _records_policy_or_exit() -> EgressPolicy: return policy -def _route_note(policy: EgressPolicy) -> str: - return "via Tor, real IP hidden" if policy.is_tor else "direct, real IP" +def _route_note(policy: EgressPolicy, *adapters: Any) -> str: + note = "via Tor, real IP hidden" if policy.is_tor else "direct, real IP" + if any(getattr(a, "credentials_withheld", False) for a in adapters): + note += ", credentials withheld" + return note def _records_fetch_cia(identifier: str) -> None: @@ -6099,8 +6110,8 @@ def records_download( from .records.muckrock import MuckRockAdapter policy = _records_policy_or_exit() - rprint(f"[dim]Contacting MuckRock ({_route_note(policy)}).[/dim]") adapter = MuckRockAdapter(egress=policy) + rprint(f"[dim]Contacting MuckRock ({_route_note(policy, adapter)}).[/dim]") # First fetch to show what we're downloading with Progress( diff --git a/openfoia/crossref.py b/openfoia/crossref.py index a96ad05..c6bd4bc 100644 --- a/openfoia/crossref.py +++ b/openfoia/crossref.py @@ -889,14 +889,16 @@ async def _check_opensanctions( """Search OpenSanctions for sanctions/PEP matches. Sends ``OPENFOIA_OPENSANCTIONS_API_KEY`` (environment only) in the - documented ``Authorization: ApiKey`` header when it is set. + documented ``Authorization: ApiKey`` header when it is set, except over + Tor: the key is tied to an account and would identify the user. """ import os hits = [] headers = {"Accept": "application/json"} api_key = os.environ.get(OPENSANCTIONS_KEY_ENV) - if api_key: + over_tor = egress is not None and egress.is_tor + if api_key and not over_tor: headers["Authorization"] = f"ApiKey {api_key}" try: @@ -911,9 +913,15 @@ async def _check_opensanctions( if resp.status_code in (401, 403): # Without a key every request fails the same way; stop sending # names for the rest of the run. + # Over Tor a personal key is withheld: it names the account. + error_type = ( + f"API key withheld over Tor ({OPENSANCTIONS_KEY_ENV} identifies you)" + if api_key and over_tor + else f"API key required: set {OPENSANCTIONS_KEY_ENV}" + ) raise _SourceCheckError( PermissionError(f"HTTP {resp.status_code}"), - error_type=f"API key required: set {OPENSANCTIONS_KEY_ENV}", + error_type=error_type, stop_source="skipped(api-key-required)", ) if resp.status_code != 200: diff --git a/openfoia/records/__init__.py b/openfoia/records/__init__.py index 9a9f1cf..9472a64 100644 --- a/openfoia/records/__init__.py +++ b/openfoia/records/__init__.py @@ -34,21 +34,19 @@ def get_adapter( the policy built from config.network or a configured Tor is bypassed. *api_data_gov_key* (config ``records.api_data_gov_key``) goes to the - api.data.gov sources; None leaves them on DEMO_KEY. Over Tor the literal - DEMO_KEY is always used: a personal key identifies the user regardless of - route, and None would let GovInfo fall back to its env key. + api.data.gov sources; None leaves them on DEMO_KEY. Over Tor the adapter + withholds any personal key and uses DEMO_KEY (RecordAdapter._personal_credential). Raises KeyError if the source is not registered. """ - from .base import DEMO_KEY, DataGovAdapter + from .base import DataGovAdapter if name not in _REGISTRY: available = ", ".join(sorted(_REGISTRY.keys())) raise KeyError(f"Unknown records source '{name}'. Available: {available}") cls = _REGISTRY[name] if issubclass(cls, DataGovAdapter): - key = DEMO_KEY if egress is not None and egress.is_tor else api_data_gov_key - return cls(api_key=key, egress=egress) + return cls(api_key=api_data_gov_key, egress=egress) return cls(egress=egress) diff --git a/openfoia/records/base.py b/openfoia/records/base.py index 4b46d1b..01379ae 100644 --- a/openfoia/records/base.py +++ b/openfoia/records/base.py @@ -185,10 +185,18 @@ class RecordAdapter(ABC): """ source_name: str = "" + credentials_withheld: bool = False def __init__(self, *, egress: EgressPolicy | None = None) -> None: self._egress = egress if egress is not None else EgressPolicy() + def _personal_credential(self, value: str | None) -> str | None: + """*value*, or None over Tor: a personal key or token names the user whatever the route.""" + if value and self._egress.is_tor: + self.credentials_withheld = True + return None + return value + async def _request( self, url: str, @@ -260,12 +268,14 @@ class DataGovAdapter(RecordAdapter): Without an explicit *api_key* these use the shared, rate-limited DEMO_KEY. Get a free key at https://api.data.gov/signup/ and set OPENFOIA_API_DATA_GOV_KEY, or ``"records": {"_api_data_gov_key": "..."}`` in config.json. The key is tied to the - email you signed up with, so it identifies you to api.data.gov even over Tor. + email you signed up with, so it identifies you to api.data.gov even over Tor, and + over Tor DEMO_KEY is used instead. """ def __init__(self, api_key: str | None = None, *, egress: EgressPolicy | None = None): super().__init__(egress=egress) - self._api_key = api_key or DEMO_KEY + personal = api_key if api_key != DEMO_KEY else None + self._api_key = self._personal_credential(personal) or DEMO_KEY def _redact(self, err: object) -> str: """Error text with the key removed, wherever httpx or a URL may have put it.""" diff --git a/openfoia/records/govinfo.py b/openfoia/records/govinfo.py index 113227c..a134887 100644 --- a/openfoia/records/govinfo.py +++ b/openfoia/records/govinfo.py @@ -63,7 +63,10 @@ class GovInfoAdapter(DataGovAdapter): source_name = "govinfo" def __init__(self, api_key: str | None = None, *, egress: EgressPolicy | None = None): - """Initialize. Without *api_key*, OPENFOIA_GOVINFO_API_KEY is used, else DEMO_KEY.""" + """Initialize. Without *api_key*, OPENFOIA_GOVINFO_API_KEY is used, else DEMO_KEY. + + Over Tor any personal key is withheld and DEMO_KEY is used (see DataGovAdapter). + """ import os super().__init__(api_key or os.environ.get("OPENFOIA_GOVINFO_API_KEY"), egress=egress) diff --git a/openfoia/records/muckrock.py b/openfoia/records/muckrock.py index 1a37247..d38478e 100644 --- a/openfoia/records/muckrock.py +++ b/openfoia/records/muckrock.py @@ -73,7 +73,7 @@ class MuckRockAdapter(RecordAdapter): def __init__(self, api_key: str | None = None, *, egress: EgressPolicy | None = None): """Initialize. Without *api_key*, OPENFOIA_MUCKROCK_TOKEN is used if set.""" super().__init__(egress=egress) - api_key = api_key or os.environ.get("OPENFOIA_MUCKROCK_TOKEN") + api_key = self._personal_credential(api_key or os.environ.get("OPENFOIA_MUCKROCK_TOKEN")) self.api_key = api_key self._headers: dict[str, str] = {"content-type": "application/json"} if api_key: diff --git a/openfoia/records/nara.py b/openfoia/records/nara.py index 6a9c17b..787438c 100644 --- a/openfoia/records/nara.py +++ b/openfoia/records/nara.py @@ -39,6 +39,10 @@ f"NARA Catalog API requires a key. Email Catalog_API@nara.gov for a free one " f"(there is no signup form) and set {KEY_ENV} (or {KEY_ENV_ALIAS})." ) +TOR_KEY_MESSAGE = ( + "NARA Catalog API needs your personal API key, which would identify you, so NARA is " + "not used over Tor. Turn off network.tor to query it directly." +) PAGE_MAX = 100 # Digital objects (scans, PDFs): the live API links them under the catalog host's # /medialive/ or /medialz/ paths (both seen 2026-10-20); older records point at @@ -149,7 +153,9 @@ class NARAAdapter(RecordAdapter): def __init__(self, api_key: str | None = None, *, egress=None): super().__init__(egress=egress) - self._api_key = api_key or os.environ.get(KEY_ENV) or os.environ.get(KEY_ENV_ALIAS) + self._api_key = self._personal_credential( + api_key or os.environ.get(KEY_ENV) or os.environ.get(KEY_ENV_ALIAS) + ) @property def has_key(self) -> bool: @@ -157,7 +163,9 @@ def has_key(self) -> bool: async def _get(self, params: dict[str, Any]) -> dict[str, Any]: if not self._api_key: - raise AdapterRequestError(NO_KEY_MESSAGE) + raise AdapterRequestError( + TOR_KEY_MESSAGE if self.credentials_withheld else NO_KEY_MESSAGE + ) base = api_base() # refuses to send the key anywhere but NARA or localhost _check_quota() await _throttle() diff --git a/tests/test_crossref_opensanctions.py b/tests/test_crossref_opensanctions.py index e9ca027..d68ece4 100644 --- a/tests/test_crossref_opensanctions.py +++ b/tests/test_crossref_opensanctions.py @@ -245,3 +245,20 @@ def test_keyless_401_stops_sending_names_for_the_rest_of_the_run(monkeypatch): "opensanctions": f"API key required: set {OPENSANCTIONS_KEY_ENV}" } assert report.results[1].source_statuses["opensanctions"] == "skipped(api-key-required)" + + +def test_api_key_is_withheld_over_tor(monkeypatch): + from openfoia.net import EgressMode, EgressPolicy + + monkeypatch.setenv(OPENSANCTIONS_KEY_ENV, FAKE_KEY) + tor = EgressPolicy(mode=EgressMode.TOR) + + seen = _patch_client(monkeypatch, _Resp(200, {"results": []})) + asyncio.run(_check_opensanctions("Ana Fixture", EntityType.PERSON, tor)) + assert "Authorization" not in seen[-1] + + _patch_client(monkeypatch, _Resp(401)) + with pytest.raises(crossref_mod._SourceCheckError) as exc: + asyncio.run(_check_opensanctions("Ana Fixture", EntityType.PERSON, tor)) + assert "withheld over Tor" in exc.value.error_type + assert FAKE_KEY not in str(exc.value) diff --git a/tests/test_records_nara.py b/tests/test_records_nara.py index 29cfb9a..b361635 100644 --- a/tests/test_records_nara.py +++ b/tests/test_records_nara.py @@ -470,12 +470,13 @@ async def must_not_download(*a, **k): assert [status for _, _, status in out] == ["present"] -def test_objects_use_the_configured_tor_policy(monkeypatch): +def test_objects_over_tor_exit_before_the_keyed_fetch(monkeypatch): from openfoia import cli from openfoia.net import EgressMode, EgressPolicy monkeypatch.setenv("OPENFOIA_NARA_API_KEY", "k") - _patch_request(monkeypatch, _response([_hit_with_files()]), []) + calls: list = [] + _patch_request(monkeypatch, _response([_hit_with_files()]), calls) tor = EgressPolicy(mode=EgressMode.TOR) monkeypatch.setattr(cli, "_records_policy_or_exit", lambda: tor) seen: list = [] @@ -486,8 +487,8 @@ async def fake_objects(self, entity, dest_dir): monkeypatch.setattr(NARAAdapter, "download_objects", fake_objects) r = runner.invoke(app, ["records", "fetch", "7283224", "-s", "nara", "--objects"]) - assert r.exit_code == 0, r.output - assert seen == [tor] and "via Tor" in r.output + assert r.exit_code == 1, r.output + assert calls == [] and seen == [] and "not used over Tor" in " ".join(r.output.split()) def test_ingest_persists_each_file_before_a_later_one_dies(monkeypatch, tmp_path): diff --git a/tests/test_security_egress_adapters.py b/tests/test_security_egress_adapters.py index 3085c26..6c039bb 100644 --- a/tests/test_security_egress_adapters.py +++ b/tests/test_security_egress_adapters.py @@ -303,3 +303,78 @@ def fake_egress_client(policy=None, *, timeout=15.0, isolation_token=None, heade assert seen["user_agent"] == SEC_DEFAULT_USER_AGENT assert seen["user_agent"] != DEFAULT_USER_AGENT + + +# --------------------------------------------------------------------------- +# #131: no personal credential leaves the machine over Tor +# --------------------------------------------------------------------------- + +TOR = EgressPolicy(mode=EgressMode.TOR) + + +def _capture_requests(monkeypatch, module, body: dict) -> list[httpx.Request]: + """Swap *module*.egress_client for a DIRECT client on a MockTransport. + + The adapter picks its credential before any client exists, so a direct + mock client shows exactly what the Tor client would have sent. + """ + sent: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + sent.append(request) + return httpx.Response(200, json=body) + + def fake_egress_client(policy=None, *, timeout=15.0, isolation_token=None, **kw): + return egress_client(None, timeout=timeout, transport=httpx.MockTransport(handler)) + + monkeypatch.setattr(module, "egress_client", fake_egress_client) + return sent + + +def _credentials(request: httpx.Request) -> set: + found = {request.headers.get(h) for h in ("authorization", "x-api-key")} + found.add(request.url.params.get("api_key")) + return found - {None, "DEMO_KEY"} + + +@pytest.mark.parametrize("policy, leaked", [(TOR, set()), (EgressPolicy(), {"Token tok-131"})]) +def test_muckrock_token_withheld_over_tor(monkeypatch, policy, leaked): + import openfoia.records.muckrock as muckrock_mod + + monkeypatch.setenv("OPENFOIA_MUCKROCK_TOKEN", "tok-131") + sent = _capture_requests(monkeypatch, muckrock_mod, {"count": 0, "results": []}) + adapter = MuckRockAdapter(egress=policy) + asyncio.run(adapter.fetch("1")) + assert sent and all(_credentials(r) == leaked for r in sent) + assert adapter.credentials_withheld is policy.is_tor + + +@pytest.mark.parametrize("policy, leaked", [(TOR, set()), (EgressPolicy(), {"gov-131"})]) +def test_govinfo_env_key_withheld_over_tor(monkeypatch, policy, leaked): + import openfoia.records.govinfo as govinfo_mod + + monkeypatch.setenv("OPENFOIA_GOVINFO_API_KEY", "gov-131") + sent = _capture_requests(monkeypatch, govinfo_mod, {"count": 0, "results": []}) + asyncio.run(GovInfoAdapter(api_key=None, egress=policy).search("Acme")) + assert sent and all(_credentials(r) == leaked for r in sent) + if policy.is_tor: + assert sent[0].headers["x-api-key"] == "DEMO_KEY" + + +@pytest.mark.parametrize("policy, leaked", [(TOR, set()), (EgressPolicy(), {"nara-131"})]) +def test_nara_key_withheld_over_tor(monkeypatch, tmp_path, policy, leaked): + import openfoia.records.base as base_mod + import openfoia.records.nara as nara_mod + + monkeypatch.setenv("OPENFOIA_DATA_DIR", str(tmp_path)) + monkeypatch.setenv("OPENFOIA_NARA_API_KEY", "nara-131") + monkeypatch.setattr(nara_mod, "_throttle", lambda: asyncio.sleep(0)) + sent = _capture_requests(monkeypatch, base_mod, {"body": {"hits": {"hits": []}}}) + adapter = NARAAdapter(egress=policy) + assert adapter.has_key is not policy.is_tor + result = asyncio.run(adapter.search("Acme")) + assert all(_credentials(r) == leaked for r in sent) + if policy.is_tor: + assert sent == [] and "not used over Tor" in result.error + else: + assert sent and result.error is None diff --git a/tests/test_security_egress_cli.py b/tests/test_security_egress_cli.py index 365781b..355d214 100644 --- a/tests/test_security_egress_cli.py +++ b/tests/test_security_egress_cli.py @@ -575,6 +575,23 @@ def test_all_sources_sends_cia_over_tor(monkeypatch, tor_records): assert cia and all(p.mode is EgressMode.TOR for p in cia) +@pytest.mark.parametrize("cmd", ["search", "fetch"]) +def test_nara_over_tor_exits_without_sending_key(monkeypatch, tor_records, cmd): + _tor_reachable(monkeypatch, True) + r = runner.invoke(app, ["records", cmd, "7283224", "-s", "nara"]) + assert r.exit_code == 1 + assert tor_records.requests == [] + assert "via Tor, real IP hidden, credentials withheld" in r.output + assert "would identify you" in " ".join(r.output.split()) + + +def test_all_sources_over_tor_skips_nara_and_says_credentials_withheld(monkeypatch, tor_records): + _tor_reachable(monkeypatch, True) + r = runner.invoke(app, ["records", "search", "acme", "--all-sources"]) + assert "credentials withheld" in r.output + assert "skipped (personal key not sent over Tor)" in r.output + + def test_get_adapter_uses_demo_key_over_tor_even_with_env_key(monkeypatch): from openfoia.net import EgressMode, EgressPolicy from openfoia.records import get_adapter