fix: disable browser downloads and reject binary file URLs

[Oaklight]

Summary

• Set accept_downloads=False on all browser contexts to prevent file downloads inside the container
• Reject known binary file extensions (.parquet, .zip, .tar.gz, etc.) early in URL validation, returning 400 before allocating a browser slot
• Catch Playwright's "Download is starting" error as a 400 instead of a generic 502, for URLs that slip past extension-based checks (e.g. Content-Disposition: attachment on normal-looking URLs)

Closes #56

Test plan

• ruff check and ruff format pass
• ty check passes
• Dev server starts, /health returns 200
• Binary URLs (.parquet, .zip, .tar.gz) return 400 with clear error message
• Normal URLs (https://example.com) render successfully (200)
• .html URLs are not rejected

[clementine-oaklight]

Clean, well-scoped PR — defense-in-depth approach is right: extension check saves browser slots, accept_downloads=False is the primary guard, and the string catch is a pragmatic safety net.

One non-blocking observation:

render.py — download-triggered 400s inflate failure stats

The "Download is starting" catch sits after stats.render.record_failure(elapsed), so these client errors get counted as render failures. The logger.error is correctly skipped, but the metric isn't. Could move the download check before the stats call, or record it separately (e.g. stats.render.record_rejected(elapsed)), so dashboards don't mix client errors with actual render failures.

Everything else reads well:

• _has_binary_extension sorting by length descending handles .tar.gz vs .gz correctly
• Using parsed.path avoids query-string false positives
• Extension list is well-chosen — all clearly non-renderable, no false positives like .pdf

[milo-oaklight]

Clean three-layer defense: extension guard → accept_downloads=False → download-error catch. Approved.

Two non-blocking notes:

1. Extension check runs after DNS resolution — _has_binary_extension is pure string matching but sits after _check_resolved_ips(hostname). Moving it before the DNS call would skip a network round-trip for obviously binary URLs. Trivial reorder.

2. stats.render.record_failure on download 400 — the download catch still records a render failure in stats. A download-triggered 400 is a client error, not a server failure. Might want to skip the stat or use a separate counter so it doesn't inflate the failure rate. Not blocking since it's conservative.

[Oaklight]

Thanks for the reviews!

@clementine-oaklight — good catch on the stats inflation. Fixed in 9821f94: moved the download check before record_failure().

@milo-oaklight — both addressed:

1. Extension check moved before DNS resolution in 8e867aa.
2. Stats issue was already fixed in 9821f94 (same as clementine's note).

[elena-oaklight]

Approved ✅

Three-layer defense (extension check → accept_downloads=False → catch download error) is the right approach. Clean, well-scoped fix.

Minor nits (non-blocking):

1. _has_binary_extension sorts on every call — _BINARY_EXTENSIONS is a frozenset, so sorted(..., key=len, reverse=True) runs per request. Pre-compute as a tuple at module level:

   _BINARY_EXTENSIONS_SORTED = tuple(sorted(_BINARY_EXTENSIONS, key=len, reverse=True))

   then iterate over that. Marginal perf, but it's free to fix.

2. Download errors are swallowed silently — the "Download is starting" branch returns 400 without hitting the logger.error line below. Consider a logger.info for observability:

   if "Download is starting" in exc_str:
       logger.info("Download triggered by %s", req.url)
       return JSONResponse(...)

3. String-matching on Playwright exception text ("Download is starting") is inherently fragile — if Playwright changes the wording, this silently falls through to the 502 path. Fine for now since there's no typed exception for this, just worth a comment noting why.