Cover byte fetcher gated by destination class #57

Closed
opened 2026-08-09 23:12:11 +07:00 by sulthan · 1 comment
Owner

Parent

Spec: #55. Originating bug: #47. Architecture and rejected alternatives: docs/adr/0007-backend-hosts-cover-bytes.md. Domain vocabulary: CONTEXT.md.

Do not close #47 or #55 from this ticket.

What to build

The backend gains the ability to fetch cover bytes from a third-party host over plain TLS and store them through the content-addressed store, with a gate that decides where the request is allowed to go.

This gate is the security control of the whole feature and deserves care. Every cover URL originates in a page controlled by someone else, so fetching one points the deployment's server at an address an attacker may choose. The server sits somewhere no internet client can reach — its own network — so a naive fetcher is a probe of internal services handed to whoever can edit a manga page's metadata tag. Validating the response does not address this: by the time the bytes are inspected the request has already happened, and the timing of the failure alone tells an attacker what is listening.

The gate is therefore destination-class, applied before any connection:

  • https only.
  • Resolve the host, then classify the resolved address, refusing loopback, private, link-local, unique-local and CGNAT ranges. Resolve-then-classify is the load-bearing part — refusing literal IP addresses alone is defeated by a hostname that resolves to 127.0.0.1, which any hostile page can publish.
  • Re-apply the identical check on every redirect hop. The first hop's address says nothing about the second's, and a public host redirecting inward is the obvious bypass.
  • Cap the response body and accept only image content types, reusing the caps that already exist rather than writing parallel ones.

Note that this deliberately differs from the host allowlist that gates series URLs, and the ADR records why: cover hosts are CDNs that move independently of their Site — demonicscans serves its covers from an unrelated host — so an allowlist would stop producing Covers the day a Site switched CDN, and that failure would look exactly like the bug this whole effort is fixing.

For testability, name resolution is injected rather than called directly, defaulting to the real resolver. This mirrors how the poller already injects its fetcher, and it exists because the most important case — a hostname resolving into private space — cannot otherwise be produced in a test.

Acceptance criteria

  • A cover URL on a public host is fetched over plain TLS and stored through the content-addressed store
  • A non-https URL is refused without a connection being made
  • A literal loopback, private, link-local, unique-local or CGNAT address is refused
  • A hostname that resolves into any of those ranges is refused
  • A redirect from a public host into a refused range is stopped mid-chain
  • A response larger than the cap is refused rather than buffered
  • A response whose content type is not an image is refused and nothing is stored
  • Name resolution is injected, defaulting to the real resolver
  • Tests assert observable outcomes — no connection made, no bytes stored — rather than which predicate returned what
  • go test ./... is green, with no test touching the live network

Blocked by

  • #56 — it stores through that store.
## Parent Spec: #55. Originating bug: #47. Architecture and rejected alternatives: `docs/adr/0007-backend-hosts-cover-bytes.md`. Domain vocabulary: `CONTEXT.md`. Do not close #47 or #55 from this ticket. ## What to build The backend gains the ability to fetch cover bytes from a third-party host over plain TLS and store them through the content-addressed store, with a gate that decides **where the request is allowed to go**. This gate is the security control of the whole feature and deserves care. Every cover URL originates in a page controlled by someone else, so fetching one points the deployment's server at an address an attacker may choose. The server sits somewhere no internet client can reach — its own network — so a naive fetcher is a probe of internal services handed to whoever can edit a manga page's metadata tag. Validating the *response* does not address this: by the time the bytes are inspected the request has already happened, and the timing of the failure alone tells an attacker what is listening. The gate is therefore destination-class, applied before any connection: - `https` only. - Resolve the host, then classify the resolved address, refusing loopback, private, link-local, unique-local and CGNAT ranges. **Resolve-then-classify is the load-bearing part** — refusing literal IP addresses alone is defeated by a hostname that resolves to `127.0.0.1`, which any hostile page can publish. - Re-apply the identical check on every redirect hop. The first hop's address says nothing about the second's, and a public host redirecting inward is the obvious bypass. - Cap the response body and accept only image content types, reusing the caps that already exist rather than writing parallel ones. Note that this deliberately differs from the host allowlist that gates series URLs, and the ADR records why: cover hosts are CDNs that move independently of their Site — demonicscans serves its covers from an unrelated host — so an allowlist would stop producing Covers the day a Site switched CDN, and that failure would look exactly like the bug this whole effort is fixing. For testability, name resolution is injected rather than called directly, defaulting to the real resolver. This mirrors how the poller already injects its fetcher, and it exists because the most important case — a hostname resolving into private space — cannot otherwise be produced in a test. ## Acceptance criteria - [x] A cover URL on a public host is fetched over plain TLS and stored through the content-addressed store - [x] A non-https URL is refused without a connection being made - [x] A literal loopback, private, link-local, unique-local or CGNAT address is refused - [x] A hostname that resolves into any of those ranges is refused - [x] A redirect from a public host into a refused range is stopped mid-chain - [x] A response larger than the cap is refused rather than buffered - [x] A response whose content type is not an image is refused and nothing is stored - [x] Name resolution is injected, defaulting to the real resolver - [x] Tests assert observable outcomes — no connection made, no bytes stored — rather than which predicate returned what - [x] go test ./... is green, with no test touching the live network ## Blocked by - #56 — it stores through that store.
sulthan added the ready-for-agent label 2026-08-09 23:12:11 +07:00
Author
Owner

Implemented on branch issue-57-cover-fetch-gate. Added plain-net/http TLS cover fetcher with injected DNS resolution, resolve-then-classify SSRF gate, redirect revalidation, direct dialing, shared body cap, and safe image content-type allowlist. Wired non-kagane covers through generic content-addressed Store.PutCover/GetCover while preserving kagane browser path and failure isolation. Added observable no-connection, DNS-range, redirect, streaming-cap, non-image, storage, and end-to-end poller tests. Verification: go test -count=1 ./...; go vet ./...; both pass. Closing PR will reference #57.

Implemented on branch issue-57-cover-fetch-gate. Added plain-net/http TLS cover fetcher with injected DNS resolution, resolve-then-classify SSRF gate, redirect revalidation, direct dialing, shared body cap, and safe image content-type allowlist. Wired non-kagane covers through generic content-addressed Store.PutCover/GetCover while preserving kagane browser path and failure isolation. Added observable no-connection, DNS-range, redirect, streaming-cap, non-image, storage, and end-to-end poller tests. Verification: go test -count=1 ./...; go vet ./...; both pass. Closing PR will reference #57.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sulthan/mangaBookmark#57