promote(security): withhold the Kestrel key from the public host, redact the cache

[trentleslie]

Why now

Org main and dev both still carry KESTREL_API_URL = os.getenv("KESTREL_API_URL", "https://kestrel.nathanpricelab.com/api") with no hostname-based key withholding and no cache redaction. Every fix below is already merged and reviewed on trentleslie/biomapper2 dev (PRs #46–#53). This PR is path-scoped to the client and its config so the security fixes land now rather than waiting on the much larger studies/ promotion that follows.

The three defects

Each has a test that fails without its fix.

1 · The API key could be sent to a third-party host. KESTREL_API_URL now defaults to the public, keyless Kestrel (kestrel.krakenkg.com/api). Any environment that had set KESTREL_API_KEY for the internal endpoint and never set KESTREL_API_URL would have started leaking that key to a third party the moment the default changed — so kestrel_host_accepts_credentials() refuses to attach the credential to the public host at all. Compared by normalized hostname, so a trailing slash, a different path, an FQDN trailing dot, or an embedded user@ cannot smuggle it through; fails closed when the URL has no parseable host. _KestrelCachedSession.rebuild_auth additionally scrubs the header on a cross-origin redirect, which requests does only for Authorization — otherwise a 301 from the internal host to the public one (the obvious way to retire it) hands over the key.

2 · The key was persisted to disk in cleartext. requests_cache ships a default ignored-parameter list containing X-API-KEY, but it matches case-sensitively and we send X-API-Key. That one-character mismatch wrote the credential into every cached record. CACHE_IGNORED_PARAMETERS now enumerates the casings actually sent, CACHE_DIR is 0700, and session construction is centralized in get_session() so no call site can rebuild a session without the redaction arguments.

3 · No default request timeout. The timeout kwarg was forwarded to the transport but never defaulted, so the mapping-path callers that supply none had no timeout at all and a wedged request could hang a run indefinitely. The value is derived, not chosen: studies/analysis/request_timeout.py computes it from the observed successful-request duration distribution, and the test asserts the shipped constant against that derivation. A default under the server's own limit is worse than none — it converts a recoverable server error into a client-side abort — which is why the derivation and its committed artifact are in this PR rather than left behind.

Also included, and why

These live inside bulk_kestrel_request and cannot be separated from the above without inventing a version of that function that was never green as a unit:

• Retry ladder for transient 5xx / connection / timeout errors, with 4xx raised immediately since it will not self-heal.
• Per-endpoint request counters, including cache hit-vs-miss — a repeat run that replays a large cache would otherwise report near-perfect backend stability from an instrument that cannot observe its own failure mode.
• Bisect-on-5xx, shipping DORMANT (KESTREL_BISECT_ON_5XX_ENABLED = False). Its premise — that the server errors are payload-determined — is not yet confirmed by the gated diagnostic. If the cause is load, bisecting amplifies it, so budgets are counted in request volume rather than recursion depth and every cap fails loud.
• get_kestrel_api_key is no longer memoized. The old cache keyed on is None, so once an unset key legitimately returned None the memo only ever engaged when a key was present — freezing it for the process lifetime and making the result depend on call order.

config.py also carries constants for Tier B and the category validator. They are inert here (the modules that read them arrive with the studies/ promotion) and are included to keep this file byte-identical to the fork's, so the follow-up merge does not conflict.

Two deliberate deltas from the fork's copy

• pyproject.toml gains pythonpath = ["."] so studies.analysis resolves under pytest. The fork's line is identical; only the comment text differs.
• src/biomapper2/utils.py:631 drops a quoted annotation that trips ruff UP037. The fork is lint-red on this line and should take the same one-line fix.

Verification

• uv run ruff check — clean (this is what org CI runs).
• uv run pytest -m "not external" — 329 passed, 7 deselected.
• gitleaks clean. The internal hostname in comments is already present in README.md, .env.example, and config.py on dev.

Not in scope

Rotating the exposed Kestrel API key. That is an operator action and is still outstanding regardless of this merge.

🤖 Generated with Claude Code

[trentleslie]

Rebased onto the refreshed dev — 2026-08-23

Was CONFLICTING against the old dev. Rebased onto 2ecf5dd (post-#79, so dev now carries #74's CI + #80's Kraken 2.1.0 normalizer). Two conflicts, src/biomapper2/utils.py and tests/conftest.py.

The conflict that mattered

Everything in conftest.py and five of the six utils.py hunks were both-sides-added — kept both. The sixth was the same KESTREL_API_URL vs get_kestrel_api_url() conflict #79 adjudicated, and it reaches further than the URL line.

bulk_kestrel_request auto-merged into a state where the request URL came from the resolver but the key-withholding guard still read the import-time constant:

if kestrel_host_accepts_credentials(KESTREL_API_URL):   # import-time constant
    headers[_KESTREL_KEY_HEADER] = api_key
url = f"{get_kestrel_api_url()}/{endpoint}"             # resolved per call

Split those and the guard decides about one host while the request goes to another. Override the backend to the public endpoint after import — which is exactly what --kestrel-url does — and the constant still names the internal host, so kestrel_host_accepts_credentials returns True and the internal key is sent to the public host. The leak this branch exists to close, re-opened by a merge rather than an edit.

Fixed by resolving once per call and using that value for all three consumers: the credential decision, the request URL, and the 401/403 remediation hint. _warn_key_withheld_once now takes the URL it was withheld from, so it names the right host under an override.

Tests

test_kestrel_auth_header patched utils.KESTREL_API_URL — a value production no longer consults. Now patches the resolver.

Added test_key_follows_the_resolved_url_not_the_import_time_constant. It asserts structurally (guard-judged URL vs sent-to URL) rather than on a leaked header: my first version asserted on the header and passed against the injected pre-fix code, because on the default config the constant and the resolver agree. Re-verified — the structural version fails on pre-fix code with guard judged 'https://kestrel.krakenkg.com/api' but the request went to 'https://kestrel.nathanpricelab.com/api/hybrid-search'.

Dropped: .github/workflows/weekly-benchmarks.yml

Removed from this PR. It runs studies.external_benchmarks.run, which does not exist in this tree — that module lives on the fork only, so it would arrive broken. Its own header also notes GitHub honours schedule:/workflow_dispatch: only on the default branch, so on dev it is inert. It belongs in whatever PR actually brings the benchmark suite.

studies/analysis/request_timeout.py (+ its results JSON) is kept: tests/test_kestrel_client_hardening.py hard-imports RECOMMENDED_TIMEOUT_S from it to assert KESTREL_REQUEST_TIMEOUT_S = 180 stays under the server's own limit. That is provenance for a shipped constant, not benchmark output.

Pre-existing lint debt, now cleared

Invisible until #79 gave dev a gate: a duplicate import pytest from the merge, an incompatible log_message override, and 17 pyright argument-type errors on the duck-typed transport doubles (file-scoped suppression, rationale in the file — subclassing requests.Session would run real adapter/pool machinery inside fixtures built to avoid it).

Verification

ruff clean · black clean · pyright 0 errors · ./scripts/test-fast.sh 302 passed, 62 deselected