fix: cap network and memory blast radius on shared infra

[lfiaschi]

Summary

Three small reliability/security fixes surfaced during a principal-engineer review of the server. Each fix is independently verifiable.

• infra/github.py — every outbound GitHub call now has a finite timeout (15 s read / 5 s connect). Previously every helper used a bare httpx.AsyncClient(), which falls back to httpx's ~5-minute default. A slow github.com would pin the FastAPI event loop and starve other async traffic on the same Modal container.
• api/rate_limit.py — replaced an O(n) sum(len(v) for ...) purge trigger with a per-instance counter and added a hard _MAX_TRACKED_KEYS cap so a rotating-IP flood can no longer grow the dict without bound. Limiter rejects with 429 once saturated and nothing is purgeable.
• tests/test_infra/test_cache.py — replaced time.sleep with a fake-monotonic-clock fixture so the suite is deterministic and ~100 ms faster per run.

All 939 server tests pass. Lint, format, mypy clean on the touched files.

---

Review notes (for context)

This PR is one slice of a wider review. The full assessment (system map, top-15 leverage list, test plan, etc.) is below. Only the highest-confidence, smallest-blast-radius fixes are in this PR — bigger items are flagged for follow-up.

High-level summary

The codebase is a uv workspace monorepo with four components: a Typer-based CLI (client/), a FastAPI server on Modal (server/), a shared domain model package (shared/), and a React 19 + Vite frontend. The split is principled and the shared package keeps duplication low. The biggest risks are concentrated in a few large files (server/src/decision_hub/infra/database.py ~3.5k lines, client/src/dhub/cli/registry.py ~1.9k lines, server/src/decision_hub/api/registry_routes.py ~1.3k lines) and in implicit assumptions about external services — most notably the GitHub timeouts fixed here. Test coverage is broad but skewed toward route-level mocked tests; integration coverage of publish_pipeline and repo_utils is thin.

Top high-leverage changes (10 items, ranked)

#	Change	Category	Impact	Effort	Status
1	Add timeouts to all httpx.AsyncClient() calls in infra/github.py	security/reliability	H	S	Done in this PR
2	Bound rate-limiter memory and replace O(n) purge trigger	security/perf	H	S	Done in this PR
3	De-flake cache tests by mocking time.monotonic	testing	M	S	Done in this PR
4	Split client/src/dhub/cli/registry.py (1912 LOC) into per-command submodules; share a single httpx.Client factory with retry/backoff for 429/5xx	architecture	H	M	follow-up
5	Add an integration test for domain/publish_pipeline.execute_publish() covering the happy path and rollback on upload failure	testing	H	M	follow-up
6	Audit api/registry_routes.py for IDOR / enumeration via differing 403 vs 404 responses on org-scoped paths (delete_skill_version, audit-log, eval-run lookups)	security	H	M	follow-up
7	Make publish race-safe: replace the find_version-then-insert_version pattern with INSERT ... ON CONFLICT or SELECT ... FOR UPDATE	correctness	M	M	follow-up
8	Add an AbortController to frontend/src/api/client.ts so stale fetches don't update unmounted components	frontend	M	S	follow-up
9	Move from loguru import logger out of domain functions (e.g. gauntlet.py:1346); domain layer should return values, not log	code-health	M	S	follow-up
10	Add pytest-timeout and per-test 10 s budget to bound CI hang risk	testing/CI	M	S	follow-up

Why only items 1–3 here

Items 4–10 are higher effort or have broader review surface (UX changes, schema/transaction shifts, route-level error-shape changes). They deserve their own PRs with focused reviewers. Items 1–3 share a tight theme — bound the blast radius of network calls, memory growth, and test wall-clock — and are mechanical enough to land safely together.

Skipped findings (rejected after verification)

A few findings from the initial review were rejected on closer reading:

• Cache TOCTOU in set() — the len >= max_size check is already inside the lock at cache.py:60–62. Not a real race.
• SkillDetailPage.tsx:699 array index as key — the surrounding expandedIndex state is itself an index, so the index-as-key is intentional. Switching to check_name would actually break expand/collapse if names duplicate.
• DRY refactor of _find_credential_hits / _find_suspicious_lines / _find_prompt_injection_hits in gauntlet.py — the three functions return dicts with different schemas ({source,label,line} vs {file,label,line} vs {pattern,label,context}); collapsing them would force an artificial uniform shape and obscure intent.

Open questions / assumptions

• 15 s read timeout on GitHub calls assumes typical org-list pagination completes in well under that. If real users with very large org lists hit this we'll see it as 504s in logs and can raise the cap.
• _MAX_TRACKED_KEYS = 10_000 is sized for a single Modal container; revisit if container concurrency changes materially.

Test plan

• make test-server (excludes slow LLM tests) — 939 passed, 34 deselected
• uvx ruff check and uvx ruff format --check on the five changed files — clean
• uvx mypy server/src/decision_hub/infra/github.py server/src/decision_hub/api/rate_limit.py — no issues
• New tests cover periodic purge, saturation 429, deterministic cache TTL, and a regression scan that fails if any future httpx.AsyncClient() (no kwargs) creeps back into infra/github.py

https://claude.ai/code/session_012LMuzsUtWEUAfEBXW5yDyL

---

Generated by Claude Code

[lfiaschi]

Closing as part of the 2026-08-11 open-PR consolidation. This automated review-sweep PR overlaps heavily with the retained merge queue (#449, #448, #447, #446, #405, then #438, #443, #375). Unique fixes not covered by the retained set are catalogued in #451 for a follow-up best-of PR. The branch is preserved, so nothing is lost.