From 36883ae460473b6be014f89335acd03e915f8691 Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Tue, 20 Oct 2026 12:07:51 +0200 Subject: [PATCH] fix(nara): allow /medialz/ objects, atomic downloads, honest sizes and help Found by an agent gathering test PDFs with the openfoia CLI: 19 of 27 NARA records serve objects under /medialz/, which --objects refused. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wdqj51cPquyTRKmoJLQV6y --- CHANGELOG.md | 15 ++++++++++++ openfoia/cli.py | 23 ++++++++++++++---- openfoia/records/base.py | 10 +++++--- openfoia/records/nara.py | 17 +++++++++---- tests/test_records_nara.py | 43 +++++++++++++++++++++++++++++++++ tests/test_security_gateways.py | 38 +++++++++++++++++++++++++++++ 6 files changed, 133 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1923244..8627ce4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,21 @@ exhaustive lists. ## [Unreleased] +### Fixed + +- `records fetch -s nara --objects` still skipped most records: + besides `/medialive/`, the live Catalog API serves scans under + `https://catalog.archives.gov/medialz/` (19 of 27 records sampled), which + the download allowlist refused. Both paths are allowed now (https only, no + `..`). +- A rerun of `--objects` downloaded files again when NARA's catalog lists a + placeholder size (1,234 bytes) for them. Downloads now write to a `.part` + file and rename it when complete, so a file on disk is always whole and a + rerun skips it. This applies to every download openfoia makes. + `--list-objects` shows such sizes as "size unknown". +- `records search --help` listed 2 of 11 sources and said no API keys are + needed. It now lists every source and the key each one reads. + ## [4.6.1] - 2026-10-19 ### Fixed diff --git a/openfoia/cli.py b/openfoia/cli.py index a0beb25..9e9ab57 100644 --- a/openfoia/cli.py +++ b/openfoia/cli.py @@ -5140,12 +5140,21 @@ def records_search( """Search public records databases. Searches external public records APIs and returns normalized entities. - No API keys needed for basic searches. When a case is active the query - is logged to it (see 'openfoia case'). - - Sources: + Each search sends the query to that source. When a case is active the + query is logged to it (see 'openfoia case'). + + Sources (key = environment variable it needs): + documentcloud - Public documents with extracted text + muckrock - Completed FOIA requests (key: OPENFOIA_MUCKROCK_TOKEN) + nara - National Archives Catalog (key: OPENFOIA_NARA_API_KEY) + cia - CIA FOIA Reading Room (often blocked to scripts) opencorporates - Company registrations worldwide sec - SEC EDGAR filings (US public companies) + usaspending - Federal contracts and grants + nonprofits - ProPublica Nonprofit Explorer (IRS Form 990) + govinfo - US government publications (key: OPENFOIA_GOVINFO_API_KEY or demo) + fec - Campaign finance (shared demo key, rate-limited) + regulations - Rulemaking dockets (shared demo key, rate-limited) Examples: openfoia records search "Acme Corp" --source opencorporates @@ -5789,6 +5798,7 @@ def _records_nara_objects( from .cases import active_case_id_in_db from .db import ensure_db, get_data_dir + from .records.nara import PLACEHOLDER_SIZE as NARA_PLACEHOLDER_SIZE listing, failed, ok, notified = [], 0, 0, False for raw in ids: @@ -5811,7 +5821,10 @@ def _records_nara_objects( continue rprint(f"\n[bold]{naid}[/bold] {entity.name}: {len(objs)} digital objects") for i, o in enumerate(objs, 1): - size = f"{o['size']:,} bytes" if o.get("size") else "-" + # ponytail: NARA lists 1234 as a placeholder size on many objects + # (seen 2026-10-20); a real 1,234-byte scan is implausible. + real = o.get("size") and o["size"] != NARA_PLACEHOLDER_SIZE + size = f"{o['size']:,} bytes" if real else "size unknown" typer.echo(f" {i:>4} {o.get('type') or '-'} {size} {o.get('url') or '-'}") continue diff --git a/openfoia/records/base.py b/openfoia/records/base.py index 6adb381..6c316c2 100644 --- a/openfoia/records/base.py +++ b/openfoia/records/base.py @@ -81,12 +81,16 @@ async def download_to_file(client: Any, url: str, dest: Any, max_bytes: int) -> from pathlib import Path dest = Path(dest) + # Stream into a sibling .part file and rename when complete, so *dest* only + # ever exists whole: even a killed process can't leave a truncated file + # that a resume would mistake for a finished download. + part = dest.with_name(dest.name + ".part") written = 0 try: async with client.stream("GET", url) as response: response.raise_for_status() - with open(dest, "wb") as f: + with open(part, "wb") as f: async for chunk in response.aiter_bytes(): written += len(chunk) if written > max_bytes: @@ -94,9 +98,9 @@ async def download_to_file(client: Any, url: str, dest: Any, max_bytes: int) -> f"Refusing download over {max_bytes} bytes (cap exceeded): {url!r}" ) f.write(chunk) + part.replace(dest) except BaseException: - if dest.exists(): - dest.unlink() + part.unlink(missing_ok=True) raise return written diff --git a/openfoia/records/nara.py b/openfoia/records/nara.py index cd86392..6a9c17b 100644 --- a/openfoia/records/nara.py +++ b/openfoia/records/nara.py @@ -41,11 +41,16 @@ ) PAGE_MAX = 100 # Digital objects (scans, PDFs): the live API links them under the catalog host's -# /medialive/ path (seen 2026-10-19); older records point at NARA's S3 buckets. +# /medialive/ or /medialz/ paths (both seen 2026-10-20); older records point at +# NARA's S3 buckets. OBJECT_HOST = "nara-media-001.s3.amazonaws.com" OBJECT_PATH_STYLE = ("s3.amazonaws.com", "/NARAprodstorage/") -OBJECT_CATALOG_PATH = ("catalog.archives.gov", "/medialive/") +OBJECT_CATALOG_PATHS = ( + ("catalog.archives.gov", "/medialive/"), + ("catalog.archives.gov", "/medialz/"), +) MAX_OBJECT_BYTES = 500 * 1024 * 1024 +PLACEHOLDER_SIZE = 1234 # objectFileSize NARA reports when it does not know the size _last_call = 0.0 _quota_warned = False @@ -83,7 +88,7 @@ def object_url_allowed(url: str) -> bool: return True return any( parts.hostname == host and parts.path.startswith(prefix) - for host, prefix in (OBJECT_PATH_STYLE, OBJECT_CATALOG_PATH) + for host, prefix in (OBJECT_PATH_STYLE, *OBJECT_CATALOG_PATHS) ) @@ -250,8 +255,10 @@ async def download_objects( _object_filename(obj.get("filename")) or safe_download_filename(url, default=f"object-{i}") ) - size = obj.get("size") - if dest.exists() and (not size or dest.stat().st_size == size): + # Downloads are atomic (download_to_file renames when complete), so a + # file on disk is whole. Don't compare with objectFileSize: NARA lists + # 1234 as a placeholder on many objects. + if dest.exists(): out.append((obj, dest, "present")) continue if not object_url_allowed(url): diff --git a/tests/test_records_nara.py b/tests/test_records_nara.py index bf0fa53..77109ff 100644 --- a/tests/test_records_nara.py +++ b/tests/test_records_nara.py @@ -445,6 +445,29 @@ def test_object_host_allowlist(): assert not nara_mod.object_url_allowed("https://catalog.archives.gov/api/v2/records") assert not nara_mod.object_url_allowed("https://catalog.archives.gov/medialive/../api/v2") assert not nara_mod.object_url_allowed("https://catalog.archives.gov.evil.example/medialive/a") + # Many records (e.g. NAID 122681436, State Dept RG 59) serve objects under /medialz/. + medialz = "https://catalog.archives.gov/medialz/dc-metro/rg-059/81119169/122673999/x.jpg" + assert nara_mod.object_url_allowed(medialz) + assert not nara_mod.object_url_allowed(medialz.replace("https", "http", 1)) + assert not nara_mod.object_url_allowed("https://catalog.archives.gov/medialz/../api/v2") + + +def test_existing_file_counts_as_present_despite_placeholder_size(tmp_path, monkeypatch): + """NARA reports objectFileSize 1234 as a placeholder on many objects (seen live, + NAID 122681436: listed 1,234 bytes, real 1,573,115). Downloads are atomic, so a + file on disk is complete: a rerun must not download it again.""" + import asyncio + from types import SimpleNamespace + + async def must_not_download(*a, **k): + raise AssertionError("a complete file on disk was downloaded again") + + monkeypatch.setattr("openfoia.records.base.download_to_file", must_not_download) + (tmp_path / "1.jpg").write_bytes(b"x" * 10) + obj = {"url": "https://catalog.archives.gov/medialz/a/1.jpg", "filename": "1.jpg", "size": 1234} + entity = SimpleNamespace(extra_data={"objects": [obj]}) + out = asyncio.run(NARAAdapter(api_key="k").download_objects(entity, tmp_path)) + assert [status for _, _, status in out] == ["present"] def test_objects_use_the_configured_tor_policy(monkeypatch): @@ -543,3 +566,23 @@ def test_object_filenames_keep_url_special_characters(): assert nara_mod._object_filename("a?b;c.pdf") == "a_b_c.pdf" assert nara_mod._object_filename("../../etc/passwd") == "passwd" assert nara_mod._object_filename("..") == "" and nara_mod._object_filename(None) == "" + + +def test_list_objects_shows_placeholder_size_as_unknown(monkeypatch): + monkeypatch.setenv("OPENFOIA_NARA_API_KEY", "k") + hit = _hit_with_files() + hit["_source"]["record"]["digitalObjects"][0]["objectFileSize"] = 1234 + _patch_request(monkeypatch, _response([hit]), []) + r = runner.invoke(app, ["records", "fetch", "7283224", "-s", "nara", "--list-objects"]) + assert r.exit_code == 0, r.output + assert "1,234 bytes" not in r.output and "size unknown" in r.output + + +def test_search_help_lists_every_source_and_keys(): + from openfoia.records import list_sources + + r = runner.invoke(app, ["records", "search", "--help"]) + assert "No API keys needed" not in r.output + for name in list_sources(): + assert name in r.output, name + assert "OPENFOIA_NARA_API_KEY" in r.output diff --git a/tests/test_security_gateways.py b/tests/test_security_gateways.py index 02742d5..bb92843 100644 --- a/tests/test_security_gateways.py +++ b/tests/test_security_gateways.py @@ -327,3 +327,41 @@ def test_download_filename_is_sanitized(raw, expected): assert "/" not in got and "\\" not in got assert not got.startswith(".") assert got == expected + + +def test_download_to_file_never_exposes_a_partial_file(tmp_path): + """A download killed mid-stream must not leave a file at *dest* that looks complete: + callers (NARA --objects resume) treat an existing file as done.""" + import asyncio + + from openfoia.records.base import download_to_file + + dest = tmp_path / "out.bin" + seen_mid_stream = [] + + class _Resp: + status_code = 200 + + def raise_for_status(self): + return None + + async def aiter_bytes(self, chunk_size=65536): + yield b"hello " + seen_mid_stream.append(dest.exists()) + yield b"world" + + class _Stream: + async def __aenter__(self): + return _Resp() + + async def __aexit__(self, *a): + return False + + class _Client: + def stream(self, method, url): + return _Stream() + + asyncio.run(download_to_file(_Client(), "https://x.test/a.pdf", dest, max_bytes=1024)) + assert seen_mid_stream == [False] + assert dest.read_bytes() == b"hello world" + assert sorted(p.name for p in tmp_path.iterdir()) == ["out.bin"]