Address review findings on the cover-path deletion (#63)
- Move the kagane cover URL shape into the extraction module (sites.go):
browserOnlyCoverURL + kaganeImageURLRe now own the claim; the byte-fetch
router and BrowserFetcher.Image reference it. One shape gate for producer
and fetcher (the id regex is folded into the full-URL match), so no Site
name appears in a cover path outside the extraction module and the
producer cannot emit an address the fetch would refuse.
- Restore the serving-boundary guarantee: GET /covers/{addr} re-checks the
stored media type via store.CoverContentType and 404s a poisoned row;
TestPublicCoverNeverEchoesNonImage now seeds one directly behind the
write gate and pins the refusal where bytes leave.
- Restore the SSRF rationale (client-supplied stored URL, headless browser
as a strong primitive) on the URL regex.
This commit is contained in:
+5
-5
@@ -168,11 +168,11 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN
|
||||
served by the one public `GET /covers/{addr}` route from content-addressed
|
||||
bytes. There is no proxy, no per-Site rewrite, no second place that decides
|
||||
a Cover's renderable address: the wire `cover` is it. The only place a Site
|
||||
name still appears in cover code is the acquisition module, where kagane's
|
||||
image URLs are claimed by `latest.BrowserFetcher` (`browserCoverURL`) because
|
||||
they answer a plain fetch with a challenge and
|
||||
`cross-origin-resource-policy: same-origin`; every other Site's CDN answers
|
||||
plain TLS. Templates render `.Cover` — the wire value — never anything else.
|
||||
name still appears in cover code is the extraction module (`latest`), where
|
||||
kagane's image URLs are claimed by `browserOnlyCoverURL` — they answer a
|
||||
plain fetch with a challenge and `cross-origin-resource-policy: same-origin`;
|
||||
every other Site's CDN answers plain TLS. Templates render `.Cover` — the
|
||||
wire value — never anything else.
|
||||
- **Web UI also owns:** session-gated `GET /install/{manga,novel}-bookmark.user.js`
|
||||
(renders the bindmounted script with the acting Reader's derived credential
|
||||
substituted in — the credential never appears in page markup, the address
|
||||
|
||||
+23
-6
@@ -1,6 +1,7 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
@@ -72,18 +73,34 @@ func TestPublicCoverRejectsUnknownAddress(t *testing.T) {
|
||||
|
||||
// A content type outside the image set is never echoed back. The old kagane
|
||||
// proxy could fetch text/html from a challenged fetch and had to refuse it;
|
||||
// the general route's only input is what the store accepted, and the store
|
||||
// refuses to record anything that is not an image, so the address that would
|
||||
// name one is a miss, not a served body.
|
||||
// the general route's only input is the store, and the store refuses to
|
||||
// record anything that is not an image — but the guarantee is pinned at the
|
||||
// serving boundary, not the write gate, so a poisoned row (migrated data, a
|
||||
// writer that skips the gate) is also never served.
|
||||
func TestPublicCoverNeverEchoesNonImage(t *testing.T) {
|
||||
const sourceURL = "https://cdn.example/cover"
|
||||
srv, st := newWebTestServer(t, testConfig())
|
||||
st, dsn := newTestStoreURL(t)
|
||||
// The write gate refuses non-image content types outright.
|
||||
if err := st.PutCover(sourceURL, []byte("<script>"), "text/html"); err == nil {
|
||||
t.Fatal("PutCover accepted a non-image content type")
|
||||
}
|
||||
rr := getCover(t, srv, "/covers/"+store.CoverAddress(sourceURL), nil)
|
||||
// A legitimate row, then the content type flipped behind the store's back:
|
||||
// the bytes exist at the address, so only the type is hostile.
|
||||
address := store.CoverAddress(sourceURL)
|
||||
if err := st.SetSeriesCover("asura", "solo", sourceURL, []byte("<script>"), "image/png"); err != nil {
|
||||
t.Fatalf("seed row: %v", err)
|
||||
}
|
||||
db, err := sql.Open("pgx", dsn)
|
||||
if err != nil {
|
||||
t.Fatalf("open %s: %v", dsn, err)
|
||||
}
|
||||
defer db.Close()
|
||||
if _, err := db.Exec(`UPDATE covers SET content_type = 'text/html' WHERE address = $1`, address); err != nil {
|
||||
t.Fatalf("poison row: %v", err)
|
||||
}
|
||||
rr := getCover(t, newRouter(st, testConfig()), "/covers/"+address, nil)
|
||||
if rr.Code == http.StatusOK {
|
||||
t.Fatalf("status = 200, want nothing served for a cover the store refused")
|
||||
t.Fatalf("status = 200, want a refusal for a non-image row (body %q)", rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -150,6 +150,14 @@ func (h *Handler) Cover(w http.ResponseWriter, r *http.Request) {
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
// Refuse anything the write gate would not have recorded: a poisoned row
|
||||
// (migrated data, a writer that skips the gate) must never be echoed back
|
||||
// as bytes of a type no Cover may have.
|
||||
if _, ok := store.CoverContentType(contentType); !ok {
|
||||
log.Printf("cover %s: refusing non-image content type %q", r.PathValue("address"), contentType)
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", contentType)
|
||||
// Content-addressed, so the bytes at this URL can never change. Public
|
||||
// rather than private: no credential gates the route.
|
||||
|
||||
@@ -24,26 +24,6 @@ const challengeTimeout = 45 * time.Second
|
||||
|
||||
var kaganeSeriesRe = regexp.MustCompile(`^/series/([0-9a-f-]{36})/?$`)
|
||||
|
||||
// kaganeImageIDRe pins the only image id the kagane extractor accepts. It
|
||||
// arrives from the browser-fetched API body, so it is matched rather than
|
||||
// trusted.
|
||||
var kaganeImageIDRe = regexp.MustCompile(`^[0-9a-f-]{36}$`)
|
||||
|
||||
// kaganeImageURLRe matches the only cover URL Image fetches: the canonical
|
||||
// compressed image route kagane's API publishes. It doubles as the byte-fetch
|
||||
// router's claim check (fetchCoverBytes) — an address of this shape answers a
|
||||
// plain fetch with a challenge, so the browser is the only route for it.
|
||||
var kaganeImageURLRe = regexp.MustCompile(`^https://kagane\.to/api/v2/image/([0-9a-f-]{36})/compressed$`)
|
||||
|
||||
// browserCoverURL reports whether the browser sidecar is the only fetcher for
|
||||
// cover bytes at imageURL. kagane serves them behind the same challenge as its
|
||||
// pages, so a plain TLS fetch would only ever retrieve a challenge page and
|
||||
// must not be attempted (ADR-0007). Per-Site knowledge, kept in the browser
|
||||
// module with the rest of it.
|
||||
func browserCoverURL(imageURL string) bool {
|
||||
return kaganeImageURLRe.MatchString(imageURL)
|
||||
}
|
||||
|
||||
// BrowserFetcher retrieves pages through a remote headless Chrome over the
|
||||
// DevTools Protocol.
|
||||
//
|
||||
|
||||
@@ -30,7 +30,7 @@ type CoverBytesFetcher interface {
|
||||
// fallback onto a path that cannot succeed. One routing rule for the poll and
|
||||
// the acquirer, so the two cannot drift apart.
|
||||
func fetchCoverBytes(ctx context.Context, cover string, browser BrowserCoverFetcher, tls CoverBytesFetcher) ([]byte, string, error) {
|
||||
if browserCoverURL(cover) {
|
||||
if browserOnlyCoverURL(cover) {
|
||||
if browser == nil {
|
||||
return nil, "", errors.New("no cover fetcher")
|
||||
}
|
||||
|
||||
@@ -165,6 +165,23 @@ var singleQuotedMetaAttrRe = regexp.MustCompile(`(?is)([a-z][a-z0-9:_-]*)\s*=\s*
|
||||
// the target detail entry avoids matching posters from recommended results.
|
||||
var comixInitialDataRe = regexp.MustCompile(`(?is)<script\b[^>]*\bid\s*=\s*["']initial-data["'][^>]*>(.*?)</script>`)
|
||||
|
||||
// kaganeImageURLRe matches the canonical compressed image route kagane's API
|
||||
// publishes — the only cover URL form the extractor emits and the browser
|
||||
// fetcher accepts. The URL is matched in full (scheme, host, id shape) rather
|
||||
// than trusted: the value a fetcher is pointed at may have been client-
|
||||
// supplied, and a headless browser is a strong SSRF primitive.
|
||||
var kaganeImageURLRe = regexp.MustCompile(`^https://kagane\.to/api/v2/image/([0-9a-f-]{36})/compressed$`)
|
||||
|
||||
// browserOnlyCoverURL reports whether the browser sidecar is the only fetcher
|
||||
// for cover bytes at imageURL. kagane's image route answers a plain fetch with
|
||||
// a challenge and `cross-origin-resource-policy: same-origin`, so a TLS fetch
|
||||
// would only ever retrieve a challenge page and must not be attempted
|
||||
// (ADR-0007). This is the byte-fetch router's per-Site knowledge; it lives in
|
||||
// the extraction module, which owns kagane's URL shapes.
|
||||
func browserOnlyCoverURL(imageURL string) bool {
|
||||
return kaganeImageURLRe.MatchString(imageURL)
|
||||
}
|
||||
|
||||
// kagane's browser-fetched series response publishes cover image IDs under
|
||||
// series_covers. The API's canonical compressed image route is the only URL
|
||||
// form accepted by the store and browser fetcher; no rendition is guessed.
|
||||
@@ -178,8 +195,12 @@ func kaganeCoverURL(body string) string {
|
||||
return ""
|
||||
}
|
||||
for _, cover := range response.SeriesCovers {
|
||||
if kaganeImageIDRe.MatchString(cover.ImageID) {
|
||||
return "https://kagane.to/api/v2/image/" + cover.ImageID + "/compressed"
|
||||
// Validate the assembled URL against the same regex the browser
|
||||
// fetcher enforces, so the extractor can never emit an address the
|
||||
// fetch would refuse.
|
||||
imageURL := "https://kagane.to/api/v2/image/" + cover.ImageID + "/compressed"
|
||||
if kaganeImageURLRe.MatchString(imageURL) {
|
||||
return imageURL
|
||||
}
|
||||
}
|
||||
return ""
|
||||
|
||||
Reference in New Issue
Block a user