Skip to content

feat: SSRF-hardened fetch-from-URL input module (remote_input.py)#196

Open
seonghobae wants to merge 1 commit into
mainfrom
feat/url-input
Open

feat: SSRF-hardened fetch-from-URL input module (remote_input.py)#196
seonghobae wants to merge 1 commit into
mainfrom
feat/url-input

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Why

Users often have recordings sitting at an https URL — cloud recording links, podcast episodes — and today codec-carver can only shrink files that are already local. This PR adds a composable, stdlib-only ingestion module that safely downloads a remote media file to a local path so the existing engine can take over. No CLI wiring yet by design; the module (plus a tiny python -m remote_input <url> <dest_dir> entry point) is the unit.

Threat model

The URL is attacker-influenced input, so the module is designed SSRF-first:

  • Scheme allowlisthttps only. http, file, ftp, data are rejected with explicit errors (local-read / SSRF vectors).
  • Address validation before connecting — the hostname is resolved via socket.getaddrinfo and every resolved address must be globally routable. Private (RFC 1918 / ULA), loopback, link-local (incl. 169.254.169.254 metadata), multicast, reserved, and unspecified ranges are rejected — for hostnames, numeric-IP URLs, and IPv4-mapped IPv6 literals alike. localhost and *.localhost are rejected by name.
  • Redirects disabled entirely — a custom opener installs a redirect handler that raises on every 3xx. Otherwise a validated public URL could 302-bounce the client to an internal address after checks passed.
  • Size capContent-Length pre-check when present, plus a hard byte counter while streaming in 1 MiB chunks (default cap 5 GiB). Overruns abort and delete the partial file.
  • Filename hygiene — the local name derives only from the sanitized URL path basename (empty/./../separator/NUL/overlong all fall back to download.bin). The server-controlled Content-Disposition header is deliberately ignored. Existing files are never overwritten (open(..., "xb")).
  • Also: embedded user:pass@ credentials rejected; socket timeout on connect and each read; every rejection raises RemoteInputError with an actionable message.
  • Residual risk documented honestly in the module docstring: a DNS-rebinding TOCTOU window remains (validation resolves, urllib re-resolves to connect); slow-loris trickle is bounded per-read, not per-download.

What

  • remote_input.pyfetch_media(url, dest_dir, *, max_bytes=5*1024**3, timeout=60) -> Path, plus RemoteInputError and a minimal __main__ CLI. Stdlib only (urllib.request, ipaddress, socket). New files only; no existing file touched.

Tests

tests/test_remote_input.py — 33 cases, fully offline (mocked getaddrinfo + fake opener/response doubles):

  • scheme rejections (http/file/ftp/data/schemeless) and embedded-credential rejection
  • private/loopback/link-local/metadata-IP rejection, IPv6 [::1], IPv4-mapped [::ffff:127.0.0.1], mixed public+private DNS answers, unresolvable hosts
  • localhost and *.localhost rejection
  • redirect refusal (handler raises; fetch leaves no file; real opener carries the handler)
  • Content-Length pre-check; mid-stream overrun aborts and deletes the partial; exact-limit body accepted
  • filename sanitization (empty, trailing slash, .., %2e%2e, %2F traversal, overlong, query strings)
  • happy path streaming to temp dir; existing destination never overwritten

Gates: python3.14 -m unittest discover -s tests → 146 tests, no new failures (baseline exactly 5 macOS os.listxattr errors). interrogate → 100% docstring coverage.

🤖 Generated with Claude Code

Add remote_input.fetch_media(url, dest_dir, *, max_bytes, timeout) so
recordings hosted at an https URL (cloud recording links, podcast
episodes) can be downloaded to a local temp path for the existing
shrinking engine. Stdlib-only (urllib.request + ipaddress + socket).

Defensive design:
- https-only scheme allowlist (http/file/ftp/data rejected)
- hostname resolved and ALL addresses validated before connecting:
  private/loopback/link-local/multicast/reserved rejected, including
  numeric-IP URLs, IPv4-mapped IPv6 literals, and localhost/*.localhost
- redirects disabled entirely via a raising redirect handler (a 3xx
  could bounce the request to an internal address post-validation)
- Content-Length pre-check plus streaming 1 MiB byte cap; overruns
  abort and delete the partial file
- filename derived only from the sanitized URL path basename
  (Content-Disposition deliberately ignored), fallback download.bin
- embedded URL credentials rejected; never overwrites existing files
- module docstring documents the threat model, including the residual
  DNS-rebinding TOCTOU window

Tests (tests/test_remote_input.py, 33 cases, no network): scheme and
credential rejection, private/loopback/link-local/mixed-answer DNS
rejection via mocked getaddrinfo, redirect refusal, size-cap aborts
deleting partials, filename sanitization, and mocked happy path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJRVbDrp1vGYkJgNHMGPpG
@seonghobae seonghobae enabled auto-merge (squash) July 6, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant