Harden automation content ingestion: SSRF, import hijack, governance, observability#47
Harden automation content ingestion: SSRF, import hijack, governance, observability#47oratis wants to merge 4 commits into
Conversation
import-hosted upserted by slugify(card.name) with no guards on the update path, so a card in the community registry whose name collided with an existing entry silently overwrote that entry's endpoint/cardUrl while keeping its APPROVED status — a takeover vector via a single registry PR. Now guard: skip rows owned by another publisher or of kind PROJECT, and drop cross-origin endpoint changes back to PENDING for admin re-review. scrape-agents forced status=APPROVED on every refresh (and re-created deleted rows as APPROVED), so a scheduled run undid admin REJECT/DELETE — the root cause behind junk repos resurfacing at the top of /registry. Now pre-load REJECTED/DISABLED slugs and skip them, and no longer touch status on update; only newly discovered repos land APPROVED. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fetchOne used redirect: "follow" and only checked the initial URL string, so a public URL could 302 into 169.254.169.254 (cloud metadata) or a name could resolve to an internal IP unchecked. This path is reachable from the public /api/agents/submit endpoint, not just cron. Follow redirects manually (max 3), revalidating every hop, and add assertPublicHost() which resolves A/AAAA and rejects private / loopback / link-local / CGNAT / benchmarking (198.18/15) targets. Prod-gated to match the existing host guard so localhost cards still work in dev. Residual connect-level TOCTOU noted for socket-level follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All three cron responses now include durationMs so the scheduler log can
spot runs approaching the timeout (a truncated run leaves partial,
non-transactional writes). health gains maxDuration=120 and counts
writeErrors instead of swallowing them via .catch(()=>{}). Clarified that
maxDuration is not the effective ceiling on Cloud Run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
05: full review of every auto-ingestion path with confirmed evidence, pro/con debate, and verdict per finding. 06: authoritative cron job list (frequency, params, auth) with idempotent gcloud setup — the schedules were previously out-of-band knowledge with no config in the repo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review整体方向和实现质量都很好:per-hop 重定向复验 + DNS 解析校验是 SSRF 的标准修法,墓碑集合用单查询避免了 N 次往返, 1. import-hosted 的跨域降级会复活被 REJECTED/DISABLED 的条目(与本 PR 自己修的 #2 同类问题)
同理,同源刷新分支虽然不改 status,但仍会刷新 REJECTED 条目的 name/description/endpoint 等字段——无害但没必要,一并 skip 更干净。 2. skip 被计入
|
1 participant
What & why
A full review of every path that auto-adds content to the registry (GitHub project scrape, hosted A2A import, user submit, health probe), plus fixes for the confirmed issues. Each finding was verified to a reproducible level and argued for/against before fixing — see
docs/05-automation-ingestion-review.md.Two first-pass claims were retracted after code-level verification (honesty over confirmation): the
classifyScenarios"recompiles regex every call" claim was false (MATCHERSis module-level), and "cron scrape almost certainly times out" was overstated (~50s at defaults) — reframed to the real defects.Confirmed issues fixed
import-hostedupserts byslugify(card.name)with no guards on the update path → a community-registry PR whose name collides with an existing entry silently overwrites itsendpoint/cardUrland keepsAPPROVED. Can also flip aPROJECTentry.PENDINGfor re-review.scrape-agentsforcesstatus:APPROVEDon refresh and re-creates deleted rows asAPPROVED→ a scheduled run undoes admin REJECT/DELETE (root cause of junk repos resurfacing atop/registry).REJECTED/DISABLEDslugs and skip them; stop touchingstatuson update.redirect:"follow"+ string-only host check → SSRF to169.254.169.254via redirect, or DNS pointing at an internal IP. Reachable from the public/api/agents/submit.assertPublicHostDNS/IP check (private/loopback/link-local/CGNAT/benchmarking).healthhas no time limit and swallows write errors via.catch(()=>{}).durationMson all three cron responses;healthgetsmaxDuration=120+ awriteErrorscount.docs/06-cron-schedule.md: authoritative job list + idempotentgcloudsetup.Verification
npx tsc --noEmitclean;eslintclean on changed files. (The stale-Prisma-client errors on first run vanish afterprisma generateand are unrelated to this diff.)127.0.0.1→block,8.8.8.8→allow,169.254.169.254→block).Notes
🤖 Generated with Claude Code