Series URL repair: owner-typed, gated by the poller's own fetch gate (#151)

AGENTS.md

@@ -99,7 +99,7 @@ Go backend:
 
 - SQL always parameterized (`$N`). Only compile-time constants (`bookmarkColumns`) may be concatenated into query text — never a request value, not even a validated one.
 - `html/template` only for anything a browser parses, never `text/template`. Never wrap stored or fetched strings in `template.HTML`/`JS`/`URL`; that switches off the escaping every template depends on.
-- Any outbound fetch of a client-supplied URL passes `fetchableSeriesURL` (site + `https` + host check) first. `series_url` arrives in a PUT body, so without the gate the poller will probe arbitrary hosts from the server's own network position. New fetch path reuses the gate rather than re-deriving one.
+- Any outbound fetch of a client-supplied URL passes `FetchableSeriesURL` (site + `https` + host check) first. `series_url` arrives in a PUT body, so without the gate the poller will probe arbitrary hosts from the server's own network position. New fetch path reuses the gate rather than re-deriving one.
 - Cap every remote body with `io.LimitReader` (`maxBodyBytes`). An unbounded read is an OOM handed to whatever is on the other end.
 - Compare secrets with `hmac.Equal` / `subtle.ConstantTimeCompare`, never `==`. A credential is matched by the SHA-256 the `readers` table holds, which is already a fixed-width equality — a new secret comparison must not regress to `==`.
 - Errors: generic text to the client (`http.Error(w, "internal error", 500)`), detail to `log.Printf`. Never log `TOKEN_KEY`, a Reader's credential, `DISCORD_CLIENT_SECRET`, a session id, or a whole `Authorization` header.

backend/internal/latest/browser.go

@@ -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)

backend/internal/latest/cover.go

@@ -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 {

backend/internal/latest/poller.go

@@ -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

backend/internal/latest/poller_test.go

@@ -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)
 			}
 		})
 	}

backend/internal/latest/read.go

@@ -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)

backend/internal/latest/sites.go

@@ -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)

backend/internal/latest/smoke_lnw_test.go

@@ -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)
 	}
 

backend/internal/store/store.go

@@ -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

backend/internal/store/store_test.go

@@ -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)
+	}
+}

backend/internal/web/admin.go

@@ -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},

backend/internal/web/admin_series.go

@@ -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.

backend/internal/web/admin_series_detail.go

@@ -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 == "",

backend/internal/web/templates/series-detail.html

@@ -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">

backend/web_test.go

@@ -3371,3 +3371,124 @@ func TestAdminSeriesDetailCorrectionMarker(t *testing.T) {
 		t.Errorf("detail page does not show the machine-written number:\n%s", body)
 	}
 }
+
+// The series URL repair validates with the poller's own fetch gate and
+// answers 400 before anything reaches the store; a URL that passes the gate
+// is stored where an Upsert would have ignored it. The request performs no
+// outbound fetch — no fetcher is ever constructed on this path (the web
+// router has no fetcher seam at all, and the handler only calls the store),
+// so "storing is not verifying" is enforced by construction (#151).
+func TestSeriesURLRepairRoute(t *testing.T) {
+	st, dsn := newTestStoreURL(t)
+	db, err := sql.Open("pgx", dsn)
+	if err != nil {
+		t.Fatalf("open %s: %v", dsn, err)
+	}
+	defer db.Close()
+	seedSeriesRow(t, st, db, seriesRowSeed{
+		key: "asura:solo", url: "https://asurascans.com/comics/solo", checkedAt: 9000, bookmarks: 1,
+	})
+	router := newRouter(st, testConfig())
+	cookie := sessionCookie(t, st)
+
+	// A URL the gate refuses — foreign host, http scheme, host of another
+	// Site — answers 400 and never reaches the store.
+	for _, body := range []string{
+		"series_url=https://evil.example/solo",
+		"series_url=http://asurascans.com/stories/solo",
+		"series_url=https://kagane.to/series/solo",
+		"series_url=",
+	} {
+		req := httptest.NewRequest(http.MethodPost, "/admin/series/asura:solo/series-url", strings.NewReader(body))
+		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+		req.AddCookie(cookie)
+		rr := httptest.NewRecorder()
+		router.ServeHTTP(rr, req)
+		if rr.Code != http.StatusBadRequest {
+			t.Errorf("POST series-url with body %q: status = %d, want 400", body, rr.Code)
+		}
+	}
+
+	capReq := httptest.NewRequest(http.MethodPost, "/admin/series/asura:solo/series-url",
+		strings.NewReader("series_url=https://asurascans.com/stories/"+strings.Repeat("a", 1<<17)))
+	capReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	capReq.AddCookie(cookie)
+	capRR := httptest.NewRecorder()
+	router.ServeHTTP(capRR, capReq)
+	if capRR.Code != http.StatusBadRequest {
+		t.Errorf("POST series-url with an oversized body: status = %d, want 400", capRR.Code)
+	}
+	var stored string
+	if err := db.QueryRow(`SELECT series_url FROM series WHERE site = 'asura' AND series_id = 'solo'`).
+		Scan(&stored); err != nil {
+		t.Fatalf("read back: %v", err)
+	}
+	if stored != "https://asurascans.com/comics/solo" {
+		t.Fatalf("after 400s the stored URL = %q, want the seeded one untouched", stored)
+	}
+
+	// A URL that passes the gate lands, and the press answers with the meta
+	// fragment just like the other detail-page actions.
+	repair := "https://asurascans.com/stories/solo-renumbered"
+	req := httptest.NewRequest(http.MethodPost, "/admin/series/asura:solo/series-url",
+		strings.NewReader("series_url="+repair))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.AddCookie(cookie)
+	rr := httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("POST series-url status = %d, want 200 (body %s)", rr.Code, rr.Body.String())
+	}
+	if body := rr.Body.String(); !strings.Contains(body, `id="detail-meta"`) {
+		t.Errorf("repair answer is not the meta fragment:\n%s", body)
+	}
+	if err := db.QueryRow(`SELECT series_url FROM series WHERE site = 'asura' AND series_id = 'solo'`).
+		Scan(&stored); err != nil {
+		t.Fatalf("read back: %v", err)
+	}
+	if stored != repair {
+		t.Fatalf("stored URL = %q, want %q", stored, repair)
+	}
+}
+
+// The detail page offers the repair input prefilled with the stored address,
+// states the honest limit — a Site-wide host change is a SQL migration, not a
+// per-Series form — and a stored string renders back into the input escaped
+// (issue #151).
+func TestAdminSeriesDetailRepairForm(t *testing.T) {
+	st, dsn := newTestStoreURL(t)
+	db, err := sql.Open("pgx", dsn)
+	if err != nil {
+		t.Fatalf("open %s: %v", dsn, err)
+	}
+	defer db.Close()
+	seedSeriesRow(t, st, db, seriesRowSeed{
+		key: "asura:solo", url: "https://asurascans.com/stories/solo", bookmarks: 1,
+	})
+	seedSeriesRow(t, st, db, seriesRowSeed{
+		key: "asura:evil", url: `https://asurascans.com/x"><script>alert(1)</script>`, bookmarks: 1,
+	})
+	router := newRouter(st, testConfig())
+
+	body := seriesDetailPage(t, router, st, "asura:solo")
+	for _, want := range []string{
+		`name="series_url"`,
+		`hx-post="/admin/series/asura:solo/series-url"`,
+		`value="https://asurascans.com/stories/solo"`,
+		"A Site-wide host change", "SQL migration",
+	} {
+		if !strings.Contains(body, want) {
+			t.Errorf("detail page lacks %q:\n%s", want, body)
+		}
+	}
+
+	// The stored value that is markup stays markup in the input's value
+	// attribute, never executable HTML.
+	body = seriesDetailPage(t, router, st, "asura:evil")
+	if strings.Contains(body, `<script>alert(1)</script>`) {
+		t.Errorf("repair input renders stored URL unescaped:\n%s", body)
+	}
+	if !strings.Contains(body, `value="https://asurascans.com/x&#34;&gt;&lt;script&gt;alert(1)&lt;/script&gt;"`) {
+		t.Errorf("repair input does not carry the escaped stored URL:\n%s", body)
+	}
+}