Series URL repair: owner-typed, gated by the poller's own fetch gate (#151)
This commit is contained in:
@@ -124,7 +124,7 @@ func (f *BrowserFetcher) Get(ctx context.Context, seriesURL string) (string, int
|
||||
// request must be made from inside the page so it carries the clearance
|
||||
// cookie, and the API is the only place the list exists. Refusing any other
|
||||
// address is the per-Site half of the SSRF gate, kept deliberately behind
|
||||
// fetchableSeriesURL (see browserRead.Read).
|
||||
// FetchableSeriesURL (see browserRead.Read).
|
||||
func kaganeRead(seriesURL string, out *string) (chromedp.Action, bool) {
|
||||
apiURL, ok := kaganeAPIURL(seriesURL)
|
||||
if !ok {
|
||||
@@ -326,7 +326,7 @@ func (f *BrowserFetcher) run(ctx context.Context, target string, read chromedp.A
|
||||
|
||||
// kaganeAPIURL maps a stored series_url to the JSON endpoint carrying its
|
||||
// chapter list. Returning false for anything else is a second line of defence
|
||||
// behind fetchableSeriesURL: a headless browser is a strong SSRF primitive and
|
||||
// behind FetchableSeriesURL: a headless browser is a strong SSRF primitive and
|
||||
// series_url is client-supplied, so the host is pinned here too.
|
||||
func kaganeAPIURL(seriesURL string) (string, bool) {
|
||||
u, err := url.Parse(seriesURL)
|
||||
|
||||
@@ -174,7 +174,7 @@ func (f *TLSCoverFetcher) Fetch(ctx context.Context, sourceURL string) ([]byte,
|
||||
return body, contentType, nil
|
||||
}
|
||||
|
||||
// This gate deliberately differs from fetchableSeriesURL: cover hosts are
|
||||
// This gate deliberately differs from FetchableSeriesURL: cover hosts are
|
||||
// site-independent CDNs, so a Site host allowlist would reject valid covers.
|
||||
func (f *TLSCoverFetcher) validateURL(ctx context.Context, u *url.URL) error {
|
||||
if u == nil || u.Scheme != "https" || u.Host == "" || u.User != nil {
|
||||
|
||||
@@ -710,7 +710,7 @@ func (p *Poller) waitCovers() {
|
||||
p.coverWG.Wait()
|
||||
}
|
||||
|
||||
// fetchableSeriesURL reports whether site is a Site the registry knows and
|
||||
// FetchableSeriesURL reports whether site is a Site the registry knows and
|
||||
// seriesURL is safe to hand to a fetcher: an https URL whose host matches the
|
||||
// Site's pinned hostname exactly. series_url comes from client-supplied PUT
|
||||
// bodies, so this is a defence against the poller being used to probe
|
||||
@@ -719,7 +719,11 @@ func (p *Poller) waitCovers() {
|
||||
// browser Site guards a control that executes JavaScript and carries cookies,
|
||||
// a parser Site guards a wasted request — but the rule is one rule, from the
|
||||
// registry.
|
||||
func fetchableSeriesURL(site, seriesURL string) bool {
|
||||
//
|
||||
// The owner's series URL repair (issue #151) is a second caller: the web
|
||||
// layer validates with this same gate before storing a repair, so there is
|
||||
// never a second copy of it.
|
||||
func FetchableSeriesURL(site, seriesURL string) bool {
|
||||
s, known := sites[site]
|
||||
if !known {
|
||||
return false
|
||||
|
||||
@@ -588,8 +588,8 @@ func TestFetchableSeriesURL(t *testing.T) {
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := fetchableSeriesURL(tt.site, tt.seriesURL); got != tt.want {
|
||||
t.Errorf("fetchableSeriesURL(%q, %q) = %v, want %v",
|
||||
if got := FetchableSeriesURL(tt.site, tt.seriesURL); got != tt.want {
|
||||
t.Errorf("FetchableSeriesURL(%q, %q) = %v, want %v",
|
||||
tt.site, tt.seriesURL, got, tt.want)
|
||||
}
|
||||
})
|
||||
@@ -1019,8 +1019,8 @@ func TestFetchableSeriesURLPinsNovelHosts(t *testing.T) {
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := fetchableSeriesURL(tc.site, tc.url); got != tc.want {
|
||||
t.Fatalf("fetchableSeriesURL(%q, %q) = %v, want %v", tc.site, tc.url, got, tc.want)
|
||||
if got := FetchableSeriesURL(tc.site, tc.url); got != tc.want {
|
||||
t.Fatalf("FetchableSeriesURL(%q, %q) = %v, want %v", tc.site, tc.url, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -41,7 +41,7 @@ var (
|
||||
// its own network position to whatever URL a token-holder writes, including
|
||||
// link-local/internal addresses or non-https schemes.
|
||||
func readSeriesPage(ctx context.Context, site, seriesURL string, browser, tls Fetcher) (seriesRead, error) {
|
||||
if !fetchableSeriesURL(site, seriesURL) {
|
||||
if !FetchableSeriesURL(site, seriesURL) {
|
||||
return seriesRead{}, fmt.Errorf("%w: site=%q url=%q", errNotFetchable, site, seriesURL)
|
||||
}
|
||||
f := fetcherFor(site, browser, tls)
|
||||
|
||||
@@ -46,7 +46,7 @@ type site struct {
|
||||
type browserRead struct {
|
||||
// Read builds the tab read for seriesURL, refusing (false) an address
|
||||
// this Site will not open in a browser — the per-Site half of the SSRF
|
||||
// gate, kept deliberately behind fetchableSeriesURL: a headless browser
|
||||
// gate, kept deliberately behind FetchableSeriesURL: a headless browser
|
||||
// executes JavaScript and carries cookies, and series_url is
|
||||
// client-supplied.
|
||||
Read func(seriesURL string, out *string) (chromedp.Action, bool)
|
||||
|
||||
@@ -40,7 +40,7 @@ func TestSmokeLnwCommentBoundary(t *testing.T) {
|
||||
if seriesURL == "" {
|
||||
t.Skip("SMOKE_LNW_SERIES_URL unset")
|
||||
}
|
||||
if !fetchableSeriesURL("lightnovelworld", seriesURL) {
|
||||
if !FetchableSeriesURL("lightnovelworld", seriesURL) {
|
||||
t.Fatalf("%q is not a fetchable lightnovelworld series URL", seriesURL)
|
||||
}
|
||||
|
||||
|
||||
@@ -60,10 +60,11 @@ type Bookmark struct {
|
||||
// exists once no matter how many bookmarks point at it (ADR-0003).
|
||||
//
|
||||
// Title, SeriesURL and Cover are written once, at creation: a PUT naming an
|
||||
// existing Series has them ignored, and only the backend's own Poll may change
|
||||
// them. Kind and the latest-chapter fields are last-write-wins like the
|
||||
// bookmark's own fields. Never serialized: the wire format is the flat
|
||||
// Bookmark (ADR-0004).
|
||||
// existing Series has them ignored, and only the backend's own Poll may
|
||||
// change them. The one exception is SeriesURL, which the owner's
|
||||
// SetSeriesURL may repair (issue #151). Kind and the latest-chapter fields
|
||||
// are last-write-wins like the bookmark's own fields. Never serialized: the
|
||||
// wire format is the flat Bookmark (ADR-0004).
|
||||
type Series struct {
|
||||
Site string
|
||||
SeriesID string
|
||||
@@ -1394,6 +1395,21 @@ func (s *Store) SetLatestChapter(site, seriesID, label string, num float64) erro
|
||||
return nil
|
||||
}
|
||||
|
||||
// SetSeriesURL stores the owner's repair for a Series' source address
|
||||
// (issue #151): the one write that lifts the write-once rule documented on
|
||||
// Series.SeriesURL. It is a store, not a verification — the caller has
|
||||
// already passed the poller's fetch gate. The handler 404s on an unknown row
|
||||
// before calling; the write itself is a plain single-column UPDATE like
|
||||
// MarkLatestChecked.
|
||||
func (s *Store) SetSeriesURL(site, seriesID, seriesURL string) error {
|
||||
if _, err := s.db.Exec(
|
||||
`UPDATE series SET series_url = $3 WHERE site = $1 AND series_id = $2`,
|
||||
site, seriesID, seriesURL); err != nil {
|
||||
return fmt.Errorf("set series url %s:%s: %w", site, seriesID, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// CorrectLatestChapter makes the Latest Chapter the owner's: one UPDATE
|
||||
// carrying the number, the derived label and the correction stamp. The label
|
||||
// shape is the poller's and the userscript's ("Chapter " + the number as
|
||||
|
||||
@@ -2144,3 +2144,38 @@ func TestGetCoverResolvesLegacyURLDerivedAddress(t *testing.T) {
|
||||
t.Fatalf("legacy cover = (%q, %q), want the planted bytes", body, contentType)
|
||||
}
|
||||
}
|
||||
|
||||
// SetSeriesURL is the one write that lifts the write-once rule of
|
||||
// Series.SeriesURL (issue #151): a client PUT naming an existing Series still
|
||||
// has its new URL dropped, yet the owner's repair lands where the Upsert
|
||||
// would have ignored it.
|
||||
func TestSetSeriesURLWritesWhereUpsertIgnores(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
base := Bookmark{
|
||||
Key: "asura:solo", Site: "asura", SeriesID: "solo",
|
||||
Title: "Solo Leveling", SeriesURL: "https://asurascans.com/comics/solo",
|
||||
UpdatedAt: 1000,
|
||||
}
|
||||
if _, err := store.Upsert(store.OwnerID(), base); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
|
||||
// A client PUT naming the existing Series is refused: the row is shared,
|
||||
// so the stored URL stands.
|
||||
base.SeriesURL = "https://evil.example/solo"
|
||||
if got, err := store.Upsert(store.OwnerID(), base); err != nil {
|
||||
t.Fatalf("Upsert: %v", err)
|
||||
} else if got.SeriesURL != "https://asurascans.com/comics/solo" {
|
||||
t.Fatalf("Upsert stored %q, want the original URL untouched", got.SeriesURL)
|
||||
}
|
||||
|
||||
// The owner's repair writes where the Upsert would have ignored it.
|
||||
repair := "https://asurascans.com/comics/solo-renumbered"
|
||||
if err := store.SetSeriesURL("asura", "solo", repair); err != nil {
|
||||
t.Fatalf("SetSeriesURL: %v", err)
|
||||
}
|
||||
sr := readSeries(t, store, "asura", "solo")
|
||||
if sr.SeriesURL != repair {
|
||||
t.Fatalf("stored URL = %q, want %q", sr.SeriesURL, repair)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -48,6 +48,7 @@ func (h *Handler) adminRoutes() []adminRoute {
|
||||
{"GET /admin/series/{key}", h.adminSeriesDetail},
|
||||
{"POST /admin/series/{key}/poll", h.adminSeriesPoll},
|
||||
{"POST /admin/series/{key}/latest", h.adminSeriesCorrectLatest},
|
||||
{"POST /admin/series/{key}/series-url", h.adminSeriesSetURL},
|
||||
{"POST /admin/lanes/{site}/pause", h.adminLanePause},
|
||||
{"POST /admin/lanes/{site}/resume", h.adminLaneResume},
|
||||
{"GET /ui/admin/lanes", h.uiLanes},
|
||||
|
||||
@@ -225,6 +225,60 @@ func (h *Handler) adminSeriesCorrectLatest(w http.ResponseWriter, r *http.Reques
|
||||
h.render(w, http.StatusOK, "series-detail-meta", h.seriesDetailView(a))
|
||||
}
|
||||
|
||||
// adminSeriesSetURL is the series URL repair: the owner types one address
|
||||
// and the Series' Poll fetches it from then on, verified by the same gate
|
||||
// the poller uses before it fetches anything — a URL failing
|
||||
// latest.FetchableSeriesURL answers 400 and never reaches the store. The
|
||||
// repair is a store, not a verification: it performs no outbound fetch, and
|
||||
// the owner presses Check now afterwards. This lifts the write-once rule of
|
||||
// Series.SeriesURL for the owner only — a Reader's PUT is still ignored. The
|
||||
// owner gate is the route's, not this handler's; the body is capped like the
|
||||
// API path caps its bodies; the key is validated here — a malformed key is a
|
||||
// 400 and an unknown one a 404.
|
||||
func (h *Handler) adminSeriesSetURL(w http.ResponseWriter, r *http.Request) {
|
||||
site, seriesID, ok := strings.Cut(r.PathValue("key"), ":")
|
||||
if !ok || site == "" || seriesID == "" {
|
||||
http.Error(w, "bad series key", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<16)
|
||||
if err := r.ParseForm(); err != nil {
|
||||
http.Error(w, "invalid form", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
seriesURL := r.PostFormValue("series_url")
|
||||
if !latest.FetchableSeriesURL(site, seriesURL) {
|
||||
http.Error(w, "series URL must be an https address on this site's host", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
if _, found, err := h.adminSeriesByKey(site, seriesID); err != nil {
|
||||
log.Printf("series url %s: %v", site+":"+seriesID, err)
|
||||
http.Error(w, "internal error", http.StatusInternalServerError)
|
||||
return
|
||||
} else if !found {
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
if err := h.store.SetSeriesURL(site, seriesID, seriesURL); err != nil {
|
||||
log.Printf("series url %s: %v", site+":"+seriesID, err)
|
||||
http.Error(w, "internal error", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
// Re-read after the write: the answer must describe the state after the
|
||||
// press, like the correction's answer does.
|
||||
a, found, err := h.adminSeriesByKey(site, seriesID)
|
||||
if err != nil {
|
||||
log.Printf("series url %s: %v", site+":"+seriesID, err)
|
||||
http.Error(w, "internal error", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
if !found {
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
h.render(w, http.StatusOK, "series-detail-meta", h.seriesDetailView(a))
|
||||
}
|
||||
|
||||
// seriesListView assembles one Series list view from the request's query
|
||||
// string. An unknown filter value is the absent All case, never an error: the
|
||||
// select's options are not the only way this URL can be reached.
|
||||
|
||||
@@ -24,11 +24,14 @@ type seriesDetailView struct {
|
||||
Cover string
|
||||
Chapter string // Latest Chapter number, or "—" before the first capture
|
||||
Checked string // how long ago the poller last checked, or "never"
|
||||
// URL is the stored source address, prefilled into the repair input —
|
||||
// the one stored string this page renders back into a form (issue #151).
|
||||
URL string
|
||||
Readers int
|
||||
// Corrected is the correction marker's text, "" while no Correction
|
||||
// stands: "corrected <age> ago" — the copy that says the value is the
|
||||
// owner's, and it dies with the stamp (a machine write of the number).
|
||||
Corrected string
|
||||
Readers int
|
||||
|
||||
// Marks, one per hygiene fact, rendered only while it holds.
|
||||
Unpollable bool // no SeriesURL to fetch
|
||||
@@ -106,6 +109,7 @@ func (h *Handler) seriesDetailView(a store.AdminSeries) seriesDetailView {
|
||||
Kind: a.Kind,
|
||||
Title: a.Title,
|
||||
Cover: h.store.CoverWireURL(a.CoverAddress),
|
||||
URL: a.SeriesURL,
|
||||
Readers: a.ReaderCount,
|
||||
Unpollable: a.SeriesURL == "",
|
||||
NoCover: a.CoverAddress == "",
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
{{/* Per-Series page: one address per Series, keyed "<site>:<series_id>" so the
|
||||
list row is one hop from it. Everything here is a Series-level fact plus
|
||||
the anonymous Reader count. The Check now control lands in its own .dform
|
||||
below the .detail-grid; the correction form is the grid's first column,
|
||||
and the grid's second waits for the series-URL repair (#151). The pending
|
||||
and corrected markers ride the meta line with the other marks. */}}
|
||||
the anonymous Reader count. Check now lands in its own .dform below the
|
||||
.detail-grid; the correction form is the grid's first column and the URL
|
||||
repair the second (issue #151). The pending and corrected markers ride the
|
||||
meta line with the other marks. */}}
|
||||
{{define "series-detail"}}
|
||||
<a class="ghost detail-back" href="/admin/series">← Series</a>
|
||||
<h1 class="detail-title">{{.Title}}</h1>
|
||||
@@ -20,6 +20,14 @@
|
||||
<button type="submit" class="ghost">Set</button>
|
||||
</div>
|
||||
</form>
|
||||
<form class="dform" hx-post="/admin/series/{{.Key}}/series-url" hx-target="#detail-meta" hx-swap="outerHTML">
|
||||
<h3>Repair series URL</h3>
|
||||
<p class="hint">The Poll fetches this address — storing is not verifying it. A Site-wide host change is a SQL migration, not two hundred forms.</p>
|
||||
<div class="field">
|
||||
<input type="url" name="series_url" value="{{.URL}}" required>
|
||||
<button type="submit" class="ghost">Set</button>
|
||||
</div>
|
||||
</form>
|
||||
</div>
|
||||
{{if .CanPoll}}
|
||||
<div class="dform">
|
||||
|
||||
Reference in New Issue
Block a user