Closes #63 Deletes the second way to reach a Cover. Since #62, every Site's cover bytes land in the content-addressed store at creation or on the poll, and the one public route serves them all — nothing needs the kagane proxy anymore. ## What went - **Template-level rewrite:** `Bookmark.CoverURL()` and both templates' use of it. Cards and chrome now render `.Cover` — the wire value — and nothing else. `Bookmark.CoverSource` was dead once `CoverURL` went, so it and its `bookmarkColumns` entry are gone too. - **Kagane-only cover route and its identifier validation:** `GET /img/kagane/{id}`, `web.CoverFetcher`, `coverIDRe`, and the whole `internal/web/cover.go`. - **The proxy's persistence:** `store.KaganeImageID`, `GetKaganeCover`, `PutKaganeCover`, `kaganeCoverSourceURL`, `kaganeCoverRe`. - **The kagane-shaped branch in the byte-fetch routing:** `fetchCoverBytes` no longer takes a `site` argument and no longer names a Site. The URL shape kagane's API publishes is claimed by the browser module itself — `kaganeImageURLRe` + `browserCoverURL` live in `latest/browser.go` with the rest of the per-Site knowledge — and `BrowserFetcher.Image` is now URL-driven (it validates the URL it will navigate to, same SSRF discipline as before). The no-plain-TLS-fallback rule for a claimed URL is preserved: a claimed address with no browser is an error, never a challenge-page fetch. ## What stayed (deliberately) - `BrowserFetcher.Image` and the browser-backed acquisition path: kagane genuinely serves cover bytes behind the challenge + `cross-origin-resource-policy: same-origin`, so the sidecar remains the only fetcher for them — it just routes by URL claim now instead of by Site name. - `fetcherFor`'s per-Site page routing (kagane/novelfull page fetches) — that is the page path, not a cover path. ## Acceptance criteria - [x] Template-level kagane cover rewrite gone - [x] Kagane-only cover route and its identifier validation gone - [x] Tests removed/rewritten against the general route, guarantees kept: unstored + traversal-shaped addresses serve nothing (`TestPublicCoverRejectsUnknownAddress`), non-image content types never echoed (`TestPublicCoverNeverEchoesNonImage` — new; the store-side gate was already pinned by `TestCoverStoreAcceptsAnySourceURL`). Store reopen-persistence and filesystem content-addressing tests rewritten against `PutCover`/`GetCover`, no guarantee lost. - [x] No Site name in a cover code path outside the acquisition module (`grep kagane backend`: store/web/templates/api are clean; remaining hits are `latest/browser.go` + `latest/sites.go`, tests, docs) - [x] Web UI and panel render Covers for all six Sites (templates render the wire address; panel renders `b.cover` — untouched, it never had a kagane path) - [x] `go test ./...` green ## Verification - `go vet ./...` clean - `go test ./...` — all packages pass (root 16.9s, latest 12.7s, store 12.7s, web 0.004s) - `CGO_ENABLED=0 go build` produces the static binary - Cover-path tests run verbosely: `TestPublicCoverServesStoredBytesUnauthenticated`, `TestPublicCoverRejectsUnknownAddress` (unknown/malformed/traversal/empty), `TestPublicCoverNeverEchoesNonImage`, `TestListRendersAcquiredCover`, `TestAcquireKaganeCoverThroughBrowser`, `TestRunOncePrefetchesKaganeCover`, `TestRunOnceRoutesNonKaganeCoverToPublicFetcher` all pass; the three `SMOKE_*` tests skip without the browser sidecar, as designed Live browser verification of the "web UI and panel render Covers for all six Sites" criterion is being run separately with Playwright against real Site pages and a locally mocked backend. Reviewed-on: #73 Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com> Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
This commit was merged in pull request #73.
This commit is contained in:
@@ -42,8 +42,8 @@ type Acquirer struct {
|
||||
// Covers retrieves the cover bytes. Nil leaves the Cover blank and the
|
||||
// chapter half working.
|
||||
Covers CoverBytesFetcher
|
||||
// BrowserCoverFetch retrieves kagane cover bytes through the browser
|
||||
// sidecar. Nil leaves kagane Covers blank; nothing falls back to a plain
|
||||
// BrowserCoverFetch retrieves browser-claimed cover bytes through the
|
||||
// sidecar. Nil leaves those Covers blank; nothing falls back to a plain
|
||||
// fetch, which would only ever retrieve a challenge page.
|
||||
BrowserCoverFetch BrowserCoverFetcher
|
||||
// Ctx cancels in-flight acquisitions at shutdown. A hook signature has
|
||||
@@ -136,7 +136,7 @@ func (a *Acquirer) acquire(ctx context.Context, sr store.Series) {
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
bytes, contentType, err := fetchCoverBytes(ctx, sr.Site, cover, a.BrowserCoverFetch, a.Covers)
|
||||
bytes, contentType, err := fetchCoverBytes(ctx, cover, a.BrowserCoverFetch, a.Covers)
|
||||
if err != nil {
|
||||
log.Printf("acquire %q: fetch cover %s: %v", sr.Key(), cover, err)
|
||||
return
|
||||
|
||||
@@ -312,8 +312,8 @@ func TestAcquireKaganeCoverThroughBrowser(t *testing.T) {
|
||||
if got := covers.callCount(); got != 1 {
|
||||
t.Fatalf("browser cover fetches = %d, want 1", got)
|
||||
}
|
||||
if got := covers.calls[0]; got != kaganeImageID {
|
||||
t.Fatalf("browser cover fetched image id %q, want %q", got, kaganeImageID)
|
||||
if got := covers.calls[0]; got != kaganeCoverSrc {
|
||||
t.Fatalf("browser cover fetched URL %q, want %q", got, kaganeCoverSrc)
|
||||
}
|
||||
got := readBookmark(t, s, kaganeKey)
|
||||
if want := testCoverBaseURL + "/covers/" + store.CoverAddress(kaganeCoverSrc); got.Cover != want {
|
||||
|
||||
@@ -24,12 +24,6 @@ const challengeTimeout = 45 * time.Second
|
||||
|
||||
var kaganeSeriesRe = regexp.MustCompile(`^/series/([0-9a-f-]{36})/?$`)
|
||||
|
||||
// kaganeImageIDRe pins the only path segment Image interpolates into an
|
||||
// outbound URL. The id arrives from a stored cover URL, which a client
|
||||
// supplied, so it is matched rather than trusted: a headless browser is a
|
||||
// strong SSRF primitive.
|
||||
var kaganeImageIDRe = regexp.MustCompile(`^[0-9a-f-]{36}$`)
|
||||
|
||||
// BrowserFetcher retrieves pages through a remote headless Chrome over the
|
||||
// DevTools Protocol.
|
||||
//
|
||||
@@ -134,12 +128,14 @@ func (f *BrowserFetcher) Get(ctx context.Context, seriesURL string) (string, int
|
||||
return body, 200, nil
|
||||
}
|
||||
|
||||
// Image retrieves one kagane cover as raw bytes and its content type.
|
||||
// Image retrieves one cover's bytes through the browser sidecar, and its
|
||||
// content type.
|
||||
//
|
||||
// It exists because kagane serves covers behind the same challenge as its
|
||||
// pages *and* with `cross-origin-resource-policy: same-origin`, so an <img> on
|
||||
// the web UI's origin cannot load one even from a browser that already holds
|
||||
// the clearance cookie (verified 2026-08-08). Proxying is the only route.
|
||||
// pages *and* with `cross-origin-resource-policy: same-origin`, so the bytes
|
||||
// are only reachable from inside a browser that already holds the clearance
|
||||
// cookie (verified 2026-08-08). Acquisition through the sidecar is the only
|
||||
// route.
|
||||
//
|
||||
// The image URL is navigated to rather than fetched from some other kagane
|
||||
// page: the challenge only runs on a top-level navigation, and once it clears
|
||||
@@ -149,12 +145,14 @@ func (f *BrowserFetcher) Get(ctx context.Context, seriesURL string) (string, int
|
||||
// The challenge is not solved by the first read: WaitReady("body") is satisfied
|
||||
// by the interstitial too. run holds the tab open until the in-page fetch
|
||||
// succeeds, which is what gives the challenge script the seconds it needs.
|
||||
func (f *BrowserFetcher) Image(ctx context.Context, imageID string) ([]byte, string, error) {
|
||||
if !kaganeImageIDRe.MatchString(imageID) {
|
||||
return nil, "", fmt.Errorf("not a kagane image id: %q", imageID)
|
||||
func (f *BrowserFetcher) Image(ctx context.Context, imageURL string) ([]byte, string, error) {
|
||||
m := kaganeImageURLRe.FindStringSubmatch(imageURL)
|
||||
if m == nil {
|
||||
return nil, "", fmt.Errorf("not a browser-fetchable cover url: %q", imageURL)
|
||||
}
|
||||
imageID := m[1]
|
||||
var dataURL string
|
||||
err := f.run(ctx, "https://kagane.to/api/v2/image/"+imageID+"/compressed",
|
||||
err := f.run(ctx, imageURL,
|
||||
chromedp.Evaluate(`fetch(location.href).then(r => r.ok
|
||||
? r.blob().then(b => new Promise(res => {
|
||||
const fr = new FileReader();
|
||||
|
||||
@@ -22,22 +22,19 @@ type CoverBytesFetcher interface {
|
||||
Fetch(ctx context.Context, sourceURL string) (body []byte, contentType string, err error)
|
||||
}
|
||||
|
||||
// fetchCoverBytes routes a cover's byte retrieval by Site: only kagane needs
|
||||
// the browser for image bytes — its covers answer a plain fetch with a
|
||||
// challenge and `cross-origin-resource-policy: same-origin` — while every
|
||||
// other Site's CDN answers plain TLS. Missing fetchers degrade to an error the
|
||||
// caller logs, never a 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, site, cover string, browser BrowserCoverFetcher, tls CoverBytesFetcher) ([]byte, string, error) {
|
||||
if site == "kagane" {
|
||||
// fetchCoverBytes routes a cover's byte retrieval by URL shape, not by Site
|
||||
// name: the browser fetcher's module claims the addresses only it can fetch
|
||||
// (kagane's image route answers a plain fetch with a challenge and
|
||||
// `cross-origin-resource-policy: same-origin`), and everything else goes over
|
||||
// plain TLS. Missing fetchers degrade to an error the caller logs, never a
|
||||
// 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 browserOnlyCoverURL(cover) {
|
||||
if browser == nil {
|
||||
return nil, "", errors.New("no cover fetcher")
|
||||
}
|
||||
imageID, ok := store.KaganeImageID(cover)
|
||||
if !ok {
|
||||
return nil, "", errors.New("invalid kagane cover URL")
|
||||
}
|
||||
return browser.Image(ctx, imageID)
|
||||
return browser.Image(ctx, cover)
|
||||
}
|
||||
if tls == nil {
|
||||
return nil, "", errors.New("no cover fetcher")
|
||||
|
||||
@@ -16,9 +16,11 @@ type Fetcher interface {
|
||||
Get(ctx context.Context, url string) (body string, status int, err error)
|
||||
}
|
||||
|
||||
// BrowserCoverFetcher retrieves one kagane cover through the browser-backed path.
|
||||
// BrowserCoverFetcher retrieves one cover's bytes through the browser-backed
|
||||
// path — the only route that clears the challenge kagane's image URLs answer
|
||||
// a plain fetch with. Satisfied by BrowserFetcher.
|
||||
type BrowserCoverFetcher interface {
|
||||
Image(ctx context.Context, imageID string) (body []byte, contentType string, err error)
|
||||
Image(ctx context.Context, imageURL string) (body []byte, contentType string, err error)
|
||||
}
|
||||
|
||||
// Poller re-checks each bookmarked series' newest published chapter on a
|
||||
@@ -44,8 +46,8 @@ type Poller struct {
|
||||
BrowserFetch Fetcher
|
||||
// CoverFetch is optional; failures are logged and never affect the chapter poll.
|
||||
CoverFetch BrowserCoverFetcher
|
||||
// CoverBytesFetch is optional; it handles non-kagane sources through the same
|
||||
// failure-isolated prefetch path.
|
||||
// CoverBytesFetch is optional; it handles plain-TLS sources through the
|
||||
// same failure-isolated prefetch path.
|
||||
CoverBytesFetch CoverBytesFetcher
|
||||
Now func() time.Time // injected so tests can freeze it
|
||||
Cooldown time.Duration
|
||||
@@ -81,9 +83,10 @@ func (p *Poller) fillBlankCover(ctx context.Context, sr store.Series, body strin
|
||||
|
||||
// prefetchCover heals Series that already carry a third-party source URL but
|
||||
// no stored address — the state left by client-supplied covers before
|
||||
// acquisition moved server-side. Every Site takes the same path; only the
|
||||
// byte fetcher differs (kagane needs the browser). New blanks have no source
|
||||
// URL and go through fillBlankCover from the series page instead.
|
||||
// acquisition moved server-side. Every Site takes the same path; fetchCoverBytes
|
||||
// routes by URL shape, so browser-claimed URLs still need the sidecar. New
|
||||
// blanks have no source URL and go through fillBlankCover from the series page
|
||||
// instead.
|
||||
func (p *Poller) prefetchCover(ctx context.Context, sr store.Series) {
|
||||
if sr.Cover == "" || sr.CoverAddress != "" {
|
||||
return
|
||||
@@ -106,7 +109,7 @@ func (p *Poller) prefetchCover(ctx context.Context, sr store.Series) {
|
||||
// failure is logged against the Series and swallowed so the chapter poll
|
||||
// cannot see it.
|
||||
func (p *Poller) storeCover(ctx context.Context, sr store.Series, sourceURL string) {
|
||||
bytes, contentType, err := fetchCoverBytes(ctx, sr.Site, sourceURL, p.CoverFetch, p.CoverBytesFetch)
|
||||
bytes, contentType, err := fetchCoverBytes(ctx, sourceURL, p.CoverFetch, p.CoverBytesFetch)
|
||||
if err != nil {
|
||||
log.Printf("latest poll %q: fetch cover %s: %v", sr.Key(), sourceURL, err)
|
||||
return
|
||||
|
||||
@@ -116,9 +116,9 @@ type fakeCoverFetcher struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (f *fakeCoverFetcher) Image(_ context.Context, imageID string) ([]byte, string, error) {
|
||||
func (f *fakeCoverFetcher) Image(_ context.Context, imageURL string) ([]byte, string, error) {
|
||||
f.mu.Lock()
|
||||
f.calls = append(f.calls, imageID)
|
||||
f.calls = append(f.calls, imageURL)
|
||||
f.mu.Unlock()
|
||||
if f.err != nil {
|
||||
return nil, "", f.err
|
||||
@@ -778,9 +778,9 @@ func TestRunOncePrefetchesKaganeCover(t *testing.T) {
|
||||
}
|
||||
p.runOnce(context.Background())
|
||||
|
||||
body, contentType, ok, err := s.GetKaganeCover("019f84bc-9ba0-7ed9-86f5-8b905ec7c28b")
|
||||
body, contentType, ok, err := s.CoverByAddress(store.CoverAddress(coverURL))
|
||||
if err != nil || !ok {
|
||||
t.Fatalf("GetKaganeCover: %v found=%v", err, ok)
|
||||
t.Fatalf("CoverByAddress: %v found=%v", err, ok)
|
||||
}
|
||||
if string(body) != "cover-bytes" || contentType != "image/webp" {
|
||||
t.Fatalf("stored cover = (%q, %q), want (cover-bytes, image/webp)", body, contentType)
|
||||
@@ -877,7 +877,7 @@ func TestRunOnceRejectsInvalidKaganeCover(t *testing.T) {
|
||||
}
|
||||
p.runOnce(context.Background())
|
||||
|
||||
if _, _, found, err := s.GetKaganeCover("019f84bc-9ba0-7ed9-86f5-8b905ec7c28b"); err != nil || found {
|
||||
if _, _, found, err := s.CoverByAddress(store.CoverAddress(coverURL)); err != nil || found {
|
||||
t.Fatalf("invalid cover persisted = %v, err %v; want missing", found, err)
|
||||
}
|
||||
}
|
||||
@@ -902,7 +902,7 @@ func TestRunOnceWithoutCoverFetcherStillPollsKagane(t *testing.T) {
|
||||
}
|
||||
p.runOnce(context.Background())
|
||||
|
||||
if _, _, found, err := s.GetKaganeCover("019f84bc-9ba0-7ed9-86f5-8b905ec7c28b"); err != nil || found {
|
||||
if _, _, found, err := s.CoverByAddress(store.CoverAddress(coverURL)); err != nil || found {
|
||||
t.Fatalf("cover after nil CoverFetch = found %v, err %v; want missing", found, err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 ""
|
||||
|
||||
@@ -10,9 +10,10 @@ import (
|
||||
"bookmarkmanager/backend/internal/store"
|
||||
)
|
||||
|
||||
// TestSmokeKaganeImage is the live proof that the cover proxy's fetch actually
|
||||
// clears Cloudflare and returns image bytes. It needs the real browser unit
|
||||
// with outbound network, so it runs only when SMOKE_BROWSER_WS_URL is set:
|
||||
// TestSmokeKaganeImage is the live proof that the acquisition path's browser
|
||||
// fetch actually clears Cloudflare and returns image bytes. It needs the real
|
||||
// browser unit with outbound network, so it runs only when SMOKE_BROWSER_WS_URL
|
||||
// is set:
|
||||
//
|
||||
// cd chrome && BROWSER_BIND_ADDR=127.0.0.1 docker compose up -d --build
|
||||
// SMOKE_BROWSER_WS_URL=ws://127.0.0.1:9222 go test -run TestSmokeKaganeImage ./internal/latest
|
||||
@@ -24,12 +25,11 @@ func TestSmokeKaganeImage(t *testing.T) {
|
||||
if ws == "" {
|
||||
t.Skip("SMOKE_BROWSER_WS_URL unset")
|
||||
}
|
||||
const imageID = "019fe11a-84c3-7fc3-a84b-88787374b617" // SP Baby's cover
|
||||
const imageURL = "https://kagane.to/api/v2/image/019fe11a-84c3-7fc3-a84b-88787374b617/compressed" // SP Baby's cover
|
||||
|
||||
// The same URL through a plain client is what the web UI's <img> gets.
|
||||
// The same URL through a plain client is what any other fetcher would get.
|
||||
// Asserting on it keeps the test honest about why the browser is needed.
|
||||
req, err := http.NewRequest(http.MethodGet,
|
||||
"https://kagane.to/api/v2/image/"+imageID+"/compressed", nil)
|
||||
req, err := http.NewRequest(http.MethodGet, imageURL, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -48,7 +48,7 @@ func TestSmokeKaganeImage(t *testing.T) {
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second)
|
||||
defer cancel()
|
||||
body, contentType, err := f.Image(ctx, imageID)
|
||||
body, contentType, err := f.Image(ctx, imageURL)
|
||||
if err != nil {
|
||||
t.Fatalf("Image: %v", err)
|
||||
}
|
||||
@@ -64,8 +64,10 @@ func TestSmokeKaganeImage(t *testing.T) {
|
||||
}
|
||||
t.Logf("fetched %d bytes of %s", len(body), contentType)
|
||||
|
||||
if _, _, err := f.Image(ctx, "not-a-uuid"); err == nil {
|
||||
t.Fatal("Image accepted a non-uuid id")
|
||||
// The browser module claims only the cover URL shape it can clear a
|
||||
// challenge for; anything else must be refused before any navigation.
|
||||
if _, _, err := f.Image(ctx, "https://cdn.example/cover.jpg"); err == nil {
|
||||
t.Fatal("Image accepted a cover URL the browser module does not claim")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user