mangaBookmark/backend/AGENTS.md at 741b23322be5377c1c672b76df384b009b2858be

Files

T

sulthan 741b23322b Fix comix titles and covers, kagane volume chapters, and kagane cover rendering (#37 )

Fixes five reported symptoms across comix.to and kagane.to. Diagnosing them turned up two latent bugs underneath, both of which had to be fixed for the kagane cover work to function at all.

## Reported symptoms and their causes

| # | Symptom | Cause |
|---|---------|-------|
| 1 | comix bookmark titled `Comix - Read Comics online for free` | comix is an SPA that rewrites `document.title` on client routing but never touches the server-rendered `og:title`. The adapter read `og:title`, so a cold load stored the homepage's title. |
| 2 | next comix bookmark gets the *previous* series' title | Same cause. After an in-page hop, `og:title` still holds whatever page loaded first. |
| 3 | comix cover shows the placeholder | comix serves no `og:image` at all, so `coverFromPage()` had nothing to read. |
| 4 | kagane chapter never appears in the bookmark list | Reader URLs carry no chapter number, so it is parsed out of `og:title`. Volume-numbered series render `"<Series> - Volume <v> Chapter <n>"`, which the suffix regex did not match, so `chapterNum` came back null and nothing was recorded. |
| 5 | kagane title includes the chapter, e.g. `SP Baby - Volume 1 Chapter 1` | Same unmatched regex — the tail was never stripped. One fix covers 4 and 5. |
| 6 | kagane cover blocked in the web UI | kagane serves covers behind its Cloudflare challenge **and** with `cross-origin-resource-policy: same-origin`. No `<img>` on the UI's origin can load one even from a browser holding the clearance cookie. Hot-linking cannot be made to work. |

## What changed

**Userscript.** comix titles now come from `document.title` with the chapter page's `" - Ch.<n>"` tail stripped, and the cover is the `img` whose `alt` matches the cleaned title. comix fills `document.title` a beat *after* the URL changes — later than the nav watcher's 300 ms snapshot — so the watcher also re-detects when the `detect()` signature changes, not only when the URL does. The kagane suffix regex takes an optional `Volume <v> ` segment. All three page shapes were captured live on 2026-08-08 and pinned as regression tests.

**Cover proxy.** `Bookmark.CoverURL()` rewrites a stored kagane `og:image` to `/img/kagane/{id}`; templates render `.CoverURL` instead of `.Cover`. The endpoint is session-gated like every other UI route and fetches through the shared headless browser, which is same-origin with kagane and so satisfies both the challenge and the CORP header. Results are memoised in-process, so a cover costs one navigation per deployment lifetime. With `BROWSER_WS_URL` unset the endpoint answers 404 rather than reaching for a nil fetcher — the same degrade-to-userscript behaviour the poller already has.

The image id is matched against a UUID regex before it reaches the browser. That gate is load-bearing rather than tidiness: the cover is a stored client-supplied string, so an unvalidated one turns this endpoint into an SSRF primitive aimed at the deployment's own network. `ServeMux` path-cleans a traversal into a redirect before the handler runs, but the handler does not depend on that, and a test pins it.

## Two latent bugs found underneath

**`BrowserFetcher.run` never let a challenge solve.** It navigated, waited for `body`, read once, and closed the tab — roughly half a second end to end. The Cloudflare interstitial has a `body` too, so `WaitReady` was satisfied by the challenge page itself. This made the challenge *unclearable* rather than merely slow: an interstitial needs several seconds of a live page to solve itself and write clearance into the browser's shared cookie jar, so tearing the tab down first means every subsequent call is challenged exactly like the one before it. `run` now holds one tab and re-reads until the caller's predicate reports an answer, bounded by `challengeTimeout` and the caller's own deadline. Exhausting the budget maps back to the 403 the poller already expects, keeping a challenged site distinct from a broken transport.

**`chromedp/headless-shell` cannot clear kagane's challenge at all.** It is a stripped Chrome build and the tells are structural rather than a header: `navigator.webdriver` is true, the plugin list is empty, and the client hints are Chromium- rather than Chrome-branded. Overriding `webdriver` through CDP was tried on its own and changed nothing.

All measured 2026-08-08 from one IP against the same cover, so the comparisons are like for like:

| Browser | Result |
|---------|--------|
| `chromedp/headless-shell:stable` | never cleared (90 s) |
| `zenika/alpine-chrome` | never cleared — ships Chrome 124, old enough that Cloudflare refuses it and old enough to break chromedp's CDP structs |
| `google-chrome`, default UA | never cleared (60 s) — `--headless=new` advertises `HeadlessChrome` |
| `google-chrome`, stock UA, `TZ=UTC` | never cleared (90 s) |
| `google-chrome`, stock UA, any non-UTC `TZ` | **cleared in ~4 s** |

Both remaining tells are load-bearing, and each was tested in isolation. `chrome/` is a Debian image with `google-chrome-stable`, a UA whose version is read back out of the binary at startup (a hardcoded one would drift out of step with the `Sec-CH-UA` hints on the next Chrome update and become a fresh tell), and no `--enable-automation`.

### The timezone tell: UTC, not a country mismatch

The first pass concluded the zone had to match the egress IP's country. Re-measuring against the actual deployment case shows that was wrong, and the correction is in `1552dd1`.

The original inference read the host's `/etc/timezone` (`Asia/Bangkok`) and assumed a Thai egress. It isn't — this host egresses from an Indonesian IP. `Asia/Bangkok` cleared not because it matched a country but because it simply isn't UTC, and the two share +07, which hid the distinction. Same container, same Indonesian IP:

| `TZ` | Result |
|------|--------|
| `UTC` | never cleared (60 s, **twice**) |
| `Asia/Jakarta` | cleared in 4 s |
| `America/New_York` | cleared in 4 s |

`America/New_York` matches neither the country nor the offset nor the hemisphere and clears just as fast. A UTC clock is itself the bot signal — Cloudflare scores it as the datacenter default — and any real zone satisfies the check. `BROWSER_TZ` therefore needs a plausible zone, not a geolocated one, and a deployment that changes region need not keep it in sync.

One sharp edge remains: the usual `-v /etc/localtime:/etc/localtime:ro` does **not** work. Chrome resolves the zone through ICU, which takes the name from that path's symlink target and ignores the file's contents, so glibc reports the host zone while Chrome still reports UTC. `/etc/timezone` carries the name and is mounted instead.

Chrome also binds its DevTools port to loopback and silently ignores `--remote-debugging-address`, which is why headless-shell fronted it with socat. This image does the same, so it stays a drop-in: the compose service keeps the `headless-shell` name and its pinned address, and `BROWSER_WS_URL` is unchanged.

## Verification

```
go test ./... all packages ok
node --test 37 + 12 pass, 0 fail

SMOKE_BROWSER_WS_URL=... go test -run TestSmokeKagane ./internal/latest
TestSmokeKaganeImage PASS (5.29s) fetched 56710 bytes of image/webp
TestSmokeKaganeGet PASS (1.17s) status=200, real chapter-list JSON
```

The smoke test ran against the exact compose configuration — built image, empty `BROWSER_TZ`, `/etc/timezone` mounted, cold profile — hitting real kagane.to. It skips unless `SMOKE_BROWSER_WS_URL` names a sidecar, so `go test ./...` stays hermetic and Docker-only.

A red smoke run means the challenge is not clearing from that IP, which is a live, time-varying fact to re-check rather than necessarily a defect.

## Security invariants

- Auth unchanged. `/img/kagane/{id}` is session-gated by `requireSession`, the same guard as every other UI route.
- Outbound fetch gated: the id is UUID-validated before it reaches the browser, keeping the existing rule that a client-supplied string never selects a fetch target unchecked.
- No new secrets, no new logging of credentials, no change to CORS, sessions, or crypto.
- Templates still escape everything; `.CoverURL` returns a plain string and is not wrapped in `template.HTML`/`URL`.
- One new dependency-free image (`chrome/`) built from Debian plus Google's own apt repo; no new Go modules.

## Deploying

Needs `docker compose build headless-shell`.

**A UTC host must set `BROWSER_TZ`, or kagane silently stops working.** With it unset the sidecar falls back to the host's `/etc/timezone`; on a UTC server that yields UTC, which is the one value that never clears. Any real zone works — `BROWSER_TZ=Asia/Jakarta` for the current deployment. `.env.example` now documents this; it previously did not mention the knob at all.

Only the browser sidecar reads `BROWSER_TZ`. The backend keeps its UTC clock, and stored timestamps are unix ms, so nothing else shifts.

## Deliberately not done

Retry/backoff around the cover proxy, and a panel-side cover fix. The panel renders no covers, and covers cache in-process after the first fetch. Worth adding if kagane starts rate-limiting.

## Correction after review of the deployment case

`1552dd1` was added after the branch was first pushed: the deployment host runs UTC with an Indonesian egress IP, which prompted re-measuring the timezone claim and falsifying it. The earlier commits' reasoning is left intact rather than rebased away, so the diagnostic trail — including the wrong turn and what disproved it — stays readable.

Reviewed-on: #37
Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com>
Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>

2026-08-08 23:27:32 +07:00

12 KiB

Raw Blame History

Guidance for OpenCode (and Claude Code) working under backend/. See root AGENTS.md for the project-wide architecture diagram, hard constraints, and design system.

Backend (backend/): stdlib net/http (handful routes, no framework) + Postgres over jackc/pgx/v5 (pure Go, CGO_ENABLED=0 -> static binary -> distroless/scratch image). Reverse proxy terminates TLS; Go service listens plain :8080. Single binary, split into packages under backend/internal/: store (Bookmark type, Postgres persistence, migration runner), latest (background poller, site parsers, TLS fetcher), session (cookie signing, login rate limiter), httpmw (Auth/Gzip/CORS middleware), api (JSON bookmark handlers), userscript (userscript-serving handler), web (browser UI handler + templates/ + static/, go:embed-ed). backend/main.go is the composition root — the only place that wires packages together into newRouter. Root-level *_test.go hold integration tests that exercise the full router; unit tests for a package live beside it under internal/.
Schema is migration-owned. internal/store/migrations/*.sql is go:embed-ed and applied on every start by store.migrate: one numbered file per change, one transaction each, versions recorded in schema_migrations. Files are append-only — editing an applied one changes nothing on a database that already ran it. No column probing, no data-fixup migrations: both were SQLite-era machinery and are gone.
Tests need Docker. internal/pgtest starts one postgres:17-alpine container per test binary (TestMain -> pgtest.Main) and hands each test its own database (pgtest.URL(t)). A package whose tests touch the store must have that TestMain.
Reader-owned store, four tables. readers is keyed by Discord user ID and carries the SHA-256 of the Reader's userscript credential plus a token_epoch (issue #24). Credentials are derived, never stored: token.Token(TOKEN_KEY, discord_id, epoch) (HMAC, internal/token), and only its SHA-256 sits in readers.token_sha256, so install URLs can be rebuilt after any restart while a database leak yields nothing but hashes. The seed creates the owner row at startup; its epoch-0 hash is refreshed on every start only while the row has never been rotated, so a restart can never resurrect a rotated-away credential. Every other row is created by that Reader's own first login (Store.EnsureReader, idempotent on discord_id, and it never rewrites an existing row's hash). Rotation is Store.RotateToken (epoch bump + hash rewrite in one transaction), driven by the web UI. series keyed (site, series_id) (asura|demonic|comix|kagane|novelfull|lightnovelworld) owns the shared facts — title, cover, canonical URL, kind (manga|novel), Latest Chapter, latest_checked_at — and bookmarks holds only what differs between readers: progress, favourite, lifecycle bucket, updated_at. A bookmark is keyed (reader_id, site, series_id) — no surrogate id; the wire key is derived as site:series_id on read — and every store read/write is scoped to the reader it names. Auth resolves the acting Reader from the presented credential (httpmw.Auth) and nothing else — there is no unauthenticated-by-Reader route and no global token; the reader id travels in the request context. Sync last-write-wins; the wire format stays flat (ADR-0004). Store.Upsert decomposes one flat body across two tables and enforces the ownership rule: client title/series_url/cover are written only when the series row is new (ADR-0003).
Endpoints: GET /bookmarks, PUT /bookmarks/{key} (upsert; see updated_at rule below), DELETE /bookmarks/{key}, GET /healthz (no auth).
Web UI: same binary serve the browser UI on a second hostname — GET / (list, or login page when no session), GET /auth/discord + GET /auth/discord/callback (Discord OAuth, ADR-0002), POST /logout, GET /static/*, htmx fragment endpoints under /ui/*. Templates + assets go:embed-ed under backend/internal/web/, so backend/Dockerfile must copy the whole internal/ tree, not just *.go. Sessions are rows in the sessions table: the cookie carries only an opaque id, looked up (and expiry- checked) on every request, and deleting the row revokes the session. Guild membership is registration (issue #27): discordCallback gates on membership (and DISCORD_REQUIRED_ROLE when set) and then calls Store.EnsureReader, so a refusal creates nothing and a returning Reader reuses their row. The owner is the only Reader with administrative reach: POST /readers/{id}/revoke (404 for anyone else) drops that Reader's sessions, and the readers panel renders only on the owner's page. A Reader with no bookmarks at all sees listView.Fresh, whose empty state offers both install links instead of describing a filter. UI mutations read-modify-write through Store.Get + Store.Upsert so updated_at rule stays one place. See docs/superpowers/specs/2026-07-25-web-ui-design.md. Design-tool caveat: templates link /static/style.css root-absolutely (correct — served from /), but impeccable detector resolves stylesheet href with path.resolve(fileDir, href), drops directory on leading / and silently skip file. Relative href don't help either: template's directory isn't its served path. So detect.mjs backend/internal/web/templates reports false clean — always pass backend/internal/web/static too. One finding there, overused-font on "Instrument Serif", deliberate identity choice, not debt.
Every action that moves series out of list is confirm-gated. Archive, finish, remove each open own .confirm-row disclosure (toggleConfirmRow(key, kind) in filter.js, kind ∈ archive|finish|remove); restore fire instantly since it's the reversal. Remove's row wear ember wash, two reversible ones wear .calm grey. --ember stay reserved for new-chapter signal: busy bar and inline error use --mute.
Latest-chapter poller: ticker goroutine in same binary re-check each bookmarked series' newest published chapter from backend's own network access, so latest_chapter stay fresh when user not browsing. Second, parallel signal — userscript keep own maybeCaptureLatestOnSeriesPage/backgroundRefreshLatest logic unchanged. Two independent clocks: per-series cooldown (series.latest_checked_at, enforced by Store.DueForLatestCheck's WHERE clause) and wake interval. The poller walks Series, not Bookmarks — a series referenced by several bookmarks is fetched once per cycle, and the due queue orders reader_count DESC, latest_checked_at ASC (ADR-0003). Series row stamped before fetch so broken series wait out full cooldown instead of retrying every tick; found chapter written straight to the series row via Store.SetLatestChapter, so a bookmark's updated_at — and the list order — is never touched. Fetches use bogdanfinn/tls-client with Chrome profile as defence in depth against fingerprint-based blocking; any failure log and skip. kagane and novelfull sit behind Cloudflare JavaScript challenges the TLS client can't clear, so they are browser-only: fetched over CDP via BROWSER_WS_URL, and simply not polled when that's unset. See docs/superpowers/specs/2026-07-26-server-latest-chapter-polling-design.md. The poller's series write is a single-column UPDATE (Store.SetLatestChapter), not a read-modify-write of the whole bookmark: it cannot revert read progress or move updated_at, so the old stale-re-read race is gone with the Get+Upsert flow.
updated_at drives list order, so moves only on real reading progress: server apply its timestamp when row new or last_chapter_num changes, else keep stored value — favouriting series or recording newly published chapter must not reorder list. PUT therefore returns row as stored, clients must adopt that response rather than own payload. See plans/2026-07-25-bookmark-list-favorites-design.md §4.
Lifecycle buckets: status on each bookmark is reading | archived | finished, orthogonal to favorite. Archived and finished appear only in own tab — not in All, Updated, Favourites, or recent strip. Poller keeps checking archived series and skip finished ones. finished settable only from web UI; PUT /bookmarks/{key} reject it with 400. Empty incoming status means "keep stored one" — resolved on the VALUES side of Store.Upsert, not conflict clause, since excluded.* is post-evaluation row and default applied there would wipe bucket on every PUT from client that predates column. See docs/superpowers/specs/2026-07-27-status-buckets-design.md.
Config via env: TOKEN_KEY (derives every Reader's userscript credential; required), OWNER_DISCORD_ID (seeds the owner Reader — the administrator and the owner of every pre-registration bookmark; required), ALLOWED_ORIGINS (comma list), DATABASE_URL (Postgres connection URL, required — no default), PORT (default 8080), DISCORD_CLIENT_ID/_CLIENT_SECRET/_GUILD_ID/ _REDIRECT_URI (required; Discord OAuth for the browser UI), DISCORD_REQUIRED_ROLE (optional role gate, empty by default), DISCORD_API_BASE (default https://discord.com/api/v10), LATEST_CHAPTER_POLL_ENABLED/_COOLDOWN/_INTERVAL/_BATCH/_STAGGER (background latest-chapter poller; defaults on, 1h/10m/14/20s). USERSCRIPT_PATH and NOVEL_USERSCRIPT_PATH (files served at /u/{token}/manga-bookmark.user.js and /u/{token}/novel-bookmark.user.js, defaults /userscript/manga-bookmark.user.js and /userscript/novel-bookmark.user.js, both supplied by bindmount; the __API_TOKEN__ placeholder inside them is substituted with the requesting Reader's credential at serve time). BROWSER_WS_URL (CDP endpoint of the chrome/ sidecar, used by the poller for kagane and novelfull and by the web UI's kagane cover proxy; unset disables browser polling and serves 404 from the proxy, leaving those sites to the userscript alone).
kagane covers are proxied, not hot-linked: kagane serves cover images behind the same challenge as its pages and with cross-origin-resource-policy: same-origin, so no <img> on the web UI's origin can load one — not even from a browser holding the clearance cookie (verified 2026-08-08). Bookmark.CoverURL rewrites a stored kagane og:image to /img/kagane/{id}, served by internal/web/cover.go through latest.BrowserFetcher.Image and memoised in-process. The templates render .CoverURL, never .Cover. The id is matched against a UUID regex before it reaches the browser: the stored value is client-supplied, so an unchecked one is an SSRF primitive pointed at the deployment's own network.
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 bar, or a redirect; ?download=1 adds Content-Disposition: attachment for mobile Violentmonkey, which ignores a .user.js navigation) and POST /rotate-token (atomic epoch bump + hash rewrite; invalidates every installed copy, so the panel warns to reinstall on all devices). Owner-only POST /readers/{id}/revoke (drops one Reader's session rows and re-renders the readers panel; 404 for any non-owner) is the only route that reaches across Readers.

12 KiB Raw Blame History

12 KiB

Raw Blame History