Compare commits

..

19 Commits

Author SHA1 Message Date
MechaCat02
35c02066fe refactor(clippy): fix the never-loop error and clear all lint warnings
All checks were successful
deploy / test-backend (push) Successful in 30m17s
deploy / test-frontend (push) Successful in 10m33s
deploy / build-and-push (push) Successful in 11m11s
deploy / deploy (push) Successful in 13s
`cargo clippy --all-targets` was failing on a deny-by-default
`never_loop` in the analysis SSE handler (the `loop` always returned on
the first iteration — the stream `unfold` already re-enters per event),
plus ~28 warnings. All resolved with no behaviour change:

- admin/analysis SSE: drop the dead `loop` wrapper.
- app: match port literals directly instead of `if p == 80` guards.
- repo/user: separate doc list from the following paragraphs.
- repo/upload_history: `sort_by_key(Reverse(..))` over `sort_by`.
- crawler/nav test: construct the error directly (no `unwrap_err` on a
  literal `Err`).
- test helpers: build configs via struct-update syntax instead of
  `Default::default()` + field reassignment; add a type alias for a
  complex audit-row tuple.
- plus the mechanical `deref`/etc. fixes from `cargo clippy --fix`.

Note: not running `cargo fmt` — the backend uses a consistent
hand-formatted style that differs from rustfmt-default across ~110
files, so a blanket reformat would be pure churn.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-02 20:45:37 +02:00
MechaCat02
ded77fe4ba test(e2e): kill cold-start flakiness with an /api/v1 fallback + one retry
A full parallel run intermittently failed the first few tests: unmocked
/api/v1 calls fell through to the dev proxy, whose backend isn't running
under E2E, so each incurred a slow ECONNREFUSED round-trip + Vite error
logging that piled up across 8 workers at cold start.

- Add e2e/fixtures.ts: a shared `test` whose auto-use fixture installs an
  `**/api/v1/**` fallback (fast 503, logged) for any endpoint a spec
  didn't mock. Registered before the test body, so per-test routes still
  win; only genuine gaps land here. Turns silent mock-gap hangs into
  instant, attributable failures — and cuts the full run ~96s → ~25s.
- Point all 22 specs at ./fixtures instead of @playwright/test.
- retries: 1 in the config as a safety net for the residual Vite
  cold-compile timing (re-runs on the now-warm server).

Two consecutive full runs: 110 passed, 0 flaky, no retries consumed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-02 20:06:20 +02:00
MechaCat02
4d02b56b77 test(e2e): realign mocks + assertions with the current API and OCR-era UI
The route-mocked E2E specs had drifted from the app they exercise, so
each rendered an empty/500 page and timed out on missing elements:

- Manga detail + reader loads now also fetch read-progress/{id} and
  /similar; several specs never mocked them, so the load's Promise.all
  rejected. Add the mocks and fill the manga fixtures out to a real
  MangaDetail (status/alt_titles/authors/genres/tags/... ) so the
  components stop throwing mid-render.
- back-nav + library: widen `read-progress*`/`page-tags` globs — `*`
  doesn't cross `/`, so `/me/read-progress/{id}` fell through to the
  proxy and 500'd; mock the library page-tags pair too.
- manga-list search: wait for the initial load to settle before
  searching so the search fetch can't race the mount-time load.
- reader-chapter-select: assert the shared `chapterLabel` output.
- admin-analysis: the active OCR backend extracts text only, so the
  detail modal shows OCR (no tags/NSFW) and metrics shows tiles +
  trend charts (no by-model) — assert what the OCR-era UI renders.
- profile: assert the wrong-password 401 stays inline (guards the
  fix in the preceding commit) and mock the guest bookmark/collection
  fetches the /profile load maps to the sign-in prompt.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 20:11:23 +02:00
MechaCat02
0865659ab3 fix(auth): keep session on a wrong-current-password 401
The change-password endpoint returns 401 for a wrong *current* password,
the same status the global on401 hook uses to detect an expired session.
The hook fired first and cleared session.user, so the account form's
"still signed in?" guard always saw null and bounced the user to /login
instead of surfacing the error inline.

Add a per-call `suppressOn401` option to `request`; changePassword opts
in, so a 401 there stays inline and the session is left intact.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 20:11:07 +02:00
MechaCat02
5596e1920d fix(compose): wire ANALYSIS_OCR_MAX_DECODE_PIXELS into the backend service
The OCR decompression-bomb guard added the env var to .env.example and
config.rs but not docker-compose.yml, so the container never received it
and the compose-env-coverage test failed once the branches were integrated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:25:59 +02:00
MechaCat02
3cba9ecf95 feat(auth): support optional expiry for bot API tokens
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:22:57 +02:00
MechaCat02
ed18d95bb0 feat(crawler): guard Chromium chapter navigation against internal targets
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:22:10 +02:00
MechaCat02
2ed42f7b9e feat(crawler): re-validate redirect hops to close SSRF via 3xx
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:21:15 +02:00
MechaCat02
cbbc626768 docs: correct ?text= OCR-search drift (no longer 501)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:19:38 +02:00
MechaCat02
a3e53f303b test(chapters): lock open-contribution upload contract + document intent
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:19:04 +02:00
MechaCat02
2af421893f fix(crawler): don't hold browser mutex across Chromium close()
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:18:49 +02:00
MechaCat02
48f0439273 fix(crawler): evict idle hosts from the per-host rate-limiter map
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:18:35 +02:00
MechaCat02
de7aefef69 fix(auth): guard the login + change-password verify paths against oversized passwords
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:18:19 +02:00
MechaCat02
3ba30f4ba9 fix(login): close ?next= control-char open-redirect + cover register
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:18:01 +02:00
MechaCat02
4306d1c96a fix(frontend): strip hop-by-hop headers from proxied responses too
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:17:32 +02:00
MechaCat02
886caaecfa fix(settings): bound manga_limit + the timeout knobs the finding named
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:17:14 +02:00
MechaCat02
de510cc19a fix(admin): make force-reanalyze audit transactional; document storage exception
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:16:08 +02:00
MechaCat02
5b76e0cc37 fix(analysis): bound concurrent OCR inferences with a shared semaphore
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:14:19 +02:00
MechaCat02
66ae4b221b fix(analysis): cap OCR decoded pixels to stop decompression-bomb OOM
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 07:12:40 +02:00
71 changed files with 1692 additions and 226 deletions

View File

@@ -328,6 +328,10 @@ ANALYSIS_API_KEY=
# beyond cap). Default 16.
# ANALYSIS_MAX_IMAGE_BYTES Per-image byte cap before downscale.
# Default 8388608 (8 MiB).
# ANALYSIS_OCR_MAX_DECODE_PIXELS Decompression-bomb guard for the OCR
# backend: hard cap on a page's *decoded* pixel
# count (encoded size is bounded separately).
# Default 100000000 (100 MP).
# ANALYSIS_RESPONSE_FORMAT `json_schema` | `json_object` | `none`.
# Default `json_schema`.
# ANALYSIS_FREQUENCY_PENALTY Discourages repetition loops. Default 0.3.
@@ -340,6 +344,7 @@ ANALYSIS_SLICE_OVERLAP=0.12
ANALYSIS_TALL_ASPECT=1.6
ANALYSIS_MAX_SLICES=16
ANALYSIS_MAX_IMAGE_BYTES=8388608
ANALYSIS_OCR_MAX_DECODE_PIXELS=100000000
ANALYSIS_RESPONSE_FORMAT=json_schema
ANALYSIS_FREQUENCY_PENALTY=0.3
ANALYSIS_TEMPERATURE=0.0

View File

@@ -137,7 +137,7 @@ These are first-class slots in the architecture. When adding any of them, plug i
- **Tags / lists**: new tables joined to `mangas`. New `domain`, `repo`, and `api` modules; the existing manga endpoints do not need to change.
- **Per-page collections / tags**: `collections` is heterogeneous — `collection_mangas` holds whole mangas, `collection_pages` holds individual pages (FK to `pages.id`). Per-user page tags live in `page_tags`, which references the **shared** `tags` table by `tag_id` (migration 0024) — the same lookup table `manga_tags` uses, so manga tags and page tags share one global vocabulary. The HTTP contract still speaks tag *names*; `repo::page_tag` resolves name↔id via `repo::tag::upsert_by_name` and applies the stricter page-tag `normalize_tag` (lowercase, collapse whitespace, reject wildcards/control/invisible chars) at the API layer. Both `collection_pages` and `page_tags` cascade-delete with `pages` and `chapters`, so re-uploading a chapter drops saved-page references by design.
- **Tag-based content search (`/search`)**: the user-facing search surface lives at [frontend/src/routes/search/+page.svelte](frontend/src/routes/search/+page.svelte). Three result views (Pages / Chapters / Mangas) consume the matching `/v1/me/page-tags`, `/v1/me/page-tags/chapters`, and `/v1/me/page-tags/mangas` endpoints. Note the two distinct query-param spaces: `?q=` on `/v1/me/page-tags` is a tag-name prefix (for autocomplete in the "Add tag" sheet); `?text=` on the aggregation endpoints is **reserved** for the planned OCR text-search input. Both aggregation handlers accept `text=` on the wire but reject non-empty values with 501 `text_search_not_yet_supported` so adding OCR later doesn't break the API shape. Adding OCR is then: a background worker writes `page_ocr_text` rows, a JOIN on the existing aggregation queries adds the new filter, the `text=` param starts validating instead of rejecting.
- **Tag-based content search (`/search`)**: the user-facing search surface lives at [frontend/src/routes/search/+page.svelte](frontend/src/routes/search/+page.svelte). Three result views (Pages / Chapters / Mangas) consume the matching `/v1/me/page-tags`, `/v1/me/page-tags/chapters`, and `/v1/me/page-tags/mangas` endpoints. Note the two distinct query-param spaces: `?q=` on `/v1/me/page-tags` is a tag-name prefix (for autocomplete in the "Add tag" sheet); `?text=` on the aggregation endpoints performs **OCR full-text search** the active OCR backend writes `page_ocr_text` rows and a weighted `search_doc` tsvector, and the aggregation queries JOIN on a `plainto_tsquery` filter ranked by `ts_rank` (see [backend/src/repo/page_analysis.rs](backend/src/repo/page_analysis.rs)). (`text=` was previously reserved and returned 501 `text_search_not_yet_supported`; that placeholder is gone now that the OCR backend is active. The generic `AppError::NotImplemented` 501 mechanism remains for future feature reservations.)
- **Full-text / fuzzy search**: enable `pg_trgm` in a migration and add a GIN index on `mangas.title`; swap the `WHERE` in `repo::manga::list` to use `%` operator or `tsvector`. The API shape (`?search=...`) does not change.
- **OCR / autotagging**: a background worker (a separate binary or a tokio task spawned in `app::build`) that reads pages from `storage::Storage` and writes tag rows. Do not couple OCR to upload handlers — it runs asynchronously.
- **S3 storage**: add `storage::S3Storage` implementing `Storage`. Branch in `app::build` based on a config field (e.g., `STORAGE_BACKEND=s3`). Handlers do not change.

2
backend/Cargo.lock generated
View File

@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
[[package]]
name = "mangalord"
version = "0.90.0"
version = "0.93.2"
dependencies = [
"anyhow",
"argon2",

View File

@@ -1,6 +1,6 @@
[package]
name = "mangalord"
version = "0.90.0"
version = "0.93.2"
edition = "2021"
default-run = "mangalord"

View File

@@ -0,0 +1,9 @@
-- Optional expiry for bot API tokens. NULL = never expires (the prior
-- behaviour, preserved for all existing rows). When set, `find_active`
-- rejects the token past this instant, mirroring the sessions table's
-- `expires_at > now()` gate.
ALTER TABLE api_tokens ADD COLUMN expires_at TIMESTAMPTZ;
-- Partial index to keep the active-token lookup cheap once expiries exist.
CREATE INDEX api_tokens_expires_at_idx ON api_tokens (expires_at)
WHERE expires_at IS NOT NULL;

View File

@@ -60,13 +60,23 @@ pub fn lines_to_analysis(lines: Vec<String>) -> VisionAnalysis {
/// recognize path borrows `&self`.
pub struct OcrsEngine {
engine: ocrs::OcrEngine,
/// Hard cap on decoded pixel count (decompression-bomb guard). See
/// [`decode_rgb8_within`].
max_decode_pixels: u64,
}
impl OcrsEngine {
/// Load the detection + recognition models from disk and build the engine.
/// Fails (at startup) if either model file is missing or unreadable, so a
/// misconfigured path is a loud boot error rather than a per-page failure.
pub fn from_model_paths(detection: &str, recognition: &str) -> anyhow::Result<Self> {
///
/// `max_decode_pixels` bounds the decoded image size (see
/// [`decode_rgb8_within`]) — wired from `AnalysisConfig::ocr_max_decode_pixels`.
pub fn from_model_paths(
detection: &str,
recognition: &str,
max_decode_pixels: u64,
) -> anyhow::Result<Self> {
use anyhow::Context;
let detection_model = rten::Model::load_file(detection)
.with_context(|| format!("load ocrs detection model {detection}"))?;
@@ -78,17 +88,41 @@ impl OcrsEngine {
..Default::default()
})
.context("construct ocrs engine")?;
Ok(Self { engine })
Ok(Self { engine, max_decode_pixels })
}
}
/// Decode encoded image bytes to RGB8 while refusing decompression bombs.
///
/// `image::load_from_memory` allocates the full decoded buffer up front, so a
/// tiny file declaring enormous dimensions (e.g. a 50000×50000 PNG) inflates
/// to billions of bytes and OOM-kills the process. We cap the decoder's
/// allocation at `max_decode_pixels` worth of RGBA (4 bytes/px headroom over
/// the RGB8 result), which the `image` crate checks against the header
/// *before* allocating — so an over-size image fails fast instead of dying.
fn decode_rgb8_within(image: &[u8], max_decode_pixels: u64) -> anyhow::Result<image::RgbImage> {
use anyhow::Context;
use std::io::Cursor;
let mut reader = image::ImageReader::new(Cursor::new(image))
.with_guessed_format()
.context("guess page image format for OCR")?;
let mut limits = image::Limits::default();
// 4 bytes/px (RGBA) gives headroom over the eventual RGB8 buffer and any
// single intermediate the decoder allocates per pixel.
limits.max_alloc = Some(max_decode_pixels.saturating_mul(4));
reader.limits(limits);
Ok(reader
.decode()
.context("decode page image for OCR")?
.into_rgb8())
}
impl OcrEngine for OcrsEngine {
fn recognize(&self, image: &[u8]) -> anyhow::Result<Vec<String>> {
use anyhow::Context;
// Decode to RGB8 so `ImageSource` gets a known channel layout.
let rgb = image::load_from_memory(image)
.context("decode page image for OCR")?
.into_rgb8();
// Decode to RGB8 so `ImageSource` gets a known channel layout,
// bounding the decoded size against the decompression-bomb cap.
let rgb = decode_rgb8_within(image, self.max_decode_pixels)?;
let source = ocrs::ImageSource::from_bytes(rgb.as_raw(), rgb.dimensions())
.map_err(|e| anyhow::anyhow!("build OCR image source: {e}"))?;
let input = self.engine.prepare_input(source)?;
@@ -109,6 +143,18 @@ impl OcrEngine for OcrsEngine {
}
}
/// Bound on concurrent CPU-bound OCR inferences, given the number of analysis
/// workers and the host's available parallelism.
///
/// Each worker's `dispatch` fires one `spawn_blocking` OCR run, so without a
/// bound `ANALYSIS_WORKERS` blocking tasks can run at once. OCR is fully
/// CPU-bound, so running more than there are cores just thrashes the scheduler
/// (and balloons the blocking pool). Cap at the core count, but never below 1
/// and never above the worker count (more permits than workers is pointless).
pub fn ocr_concurrency_limit(workers: usize, cores: usize) -> usize {
workers.min(cores.max(1)).max(1)
}
/// Production dispatcher for the OCR backend: load the page, read its image
/// from storage, run OCR on the blocking pool, and persist the lines. Mirrors
/// [`crate::analysis::daemon::RealAnalyzeDispatcher`] but with no network I/O.
@@ -117,6 +163,10 @@ pub struct OcrAnalyzeDispatcher {
pub storage: Arc<dyn Storage>,
pub engine: Arc<dyn OcrEngine>,
pub max_image_bytes: usize,
/// Caps concurrent CPU-bound OCR inferences across all workers (see
/// [`ocr_concurrency_limit`]). Shared via the `Arc<dyn AnalyzeDispatcher>`,
/// so one permit pool covers every worker.
pub ocr_permits: Arc<tokio::sync::Semaphore>,
}
#[async_trait]
@@ -140,7 +190,13 @@ impl AnalyzeDispatcher for OcrAnalyzeDispatcher {
);
}
// OCR inference is CPU-bound and synchronous — keep it off the async
// worker's runtime thread.
// worker's runtime thread, and gate it behind the shared permit pool so
// ANALYSIS_WORKERS > cores can't oversubscribe the blocking pool.
let _permit = self
.ocr_permits
.acquire()
.await
.map_err(|e| anyhow::anyhow!("OCR semaphore closed: {e}"))?;
let engine = Arc::clone(&self.engine);
let lines = tokio::task::spawn_blocking(move || engine.recognize(&bytes))
.await
@@ -198,4 +254,58 @@ mod tests {
let v = lines_to_analysis(Vec::new());
assert!(v.ocr_results.is_empty());
}
/// A minimal valid PNG of `w`×`h` (single black pixel scaled via IHDR is
/// not valid; instead encode a real tiny image, then we patch the IHDR
/// dimensions for the bomb case). For the happy path we just encode a real
/// small image.
fn encode_png(w: u32, h: u32) -> Vec<u8> {
let img = image::RgbImage::new(w, h);
let mut buf = std::io::Cursor::new(Vec::new());
image::DynamicImage::ImageRgb8(img)
.write_to(&mut buf, image::ImageFormat::Png)
.unwrap();
buf.into_inner()
}
#[test]
fn decode_rgb8_within_accepts_normal_page() {
// A perfectly ordinary page decodes fine under a generous cap.
let png = encode_png(64, 96);
let rgb = decode_rgb8_within(&png, 100_000_000).unwrap();
assert_eq!(rgb.dimensions(), (64, 96));
}
#[test]
fn decode_rgb8_within_rejects_oversize_image() {
// The same image, but the cap is set below its pixel count: the
// allocation limit must trip rather than the decode succeeding. This
// is the decompression-bomb guard in miniature — a real bomb declares
// a huge size in a few bytes; here we shrink the budget instead.
let png = encode_png(2000, 2000); // 4 MP
let err = decode_rgb8_within(&png, 1_000).unwrap_err();
assert!(
err.to_string().contains("decode page image"),
"expected a decode error, got: {err}"
);
}
#[test]
fn ocr_concurrency_limit_caps_at_cores() {
// More workers than cores → clamp to cores (don't oversubscribe).
assert_eq!(ocr_concurrency_limit(8, 4), 4);
}
#[test]
fn ocr_concurrency_limit_caps_at_workers() {
// Fewer workers than cores → only `workers` ever run anyway.
assert_eq!(ocr_concurrency_limit(2, 16), 2);
}
#[test]
fn ocr_concurrency_limit_never_zero() {
// Degenerate inputs still yield at least one permit.
assert_eq!(ocr_concurrency_limit(0, 0), 1);
assert_eq!(ocr_concurrency_limit(1, 0), 1);
}
}

View File

@@ -1212,14 +1212,16 @@ mod tests {
// VisionClient pointed at a closed local port. Reqwest fails the
// POST immediately (connection refused), but the prep work runs
// before the POST is even built.
let mut cfg = AnalysisConfig::default();
cfg.endpoint = "http://127.0.0.1:1/v1/chat/completions".into();
cfg.model = "stub".into();
cfg.request_timeout = Duration::from_secs(1);
// Tune slice geometry to match the test image's shape.
cfg.max_pixels = 1_000_000;
cfg.min_slice_height = 100;
cfg.tall_aspect_threshold = 1.8;
let cfg = AnalysisConfig {
endpoint: "http://127.0.0.1:1/v1/chat/completions".into(),
model: "stub".into(),
request_timeout: Duration::from_secs(1),
// Tune slice geometry to match the test image's shape.
max_pixels: 1_000_000,
min_slice_height: 100,
tall_aspect_threshold: 1.8,
..Default::default()
};
let http = reqwest::Client::builder()
.timeout(cfg.request_timeout)
.no_proxy()

View File

@@ -58,20 +58,20 @@ async fn stream_status(
) -> Sse<impl Stream<Item = Result<Event, Infallible>>> {
let rx = state.analysis_events.subscribe();
let stream = futures_util::stream::unfold(rx, |mut rx| async move {
loop {
match rx.recv().await {
Ok(ev) => {
let event = Event::default()
.event("analysis")
.json_data(&ev)
.unwrap_or_else(|_| Event::default().comment("serialize error"));
return Some((Ok(event), rx));
}
Err(RecvError::Lagged(_)) => {
return Some((Ok(Event::default().event("lagged").data("")), rx));
}
Err(RecvError::Closed) => return None,
// One recv per unfold step; the stream driver re-enters for the next
// event, so no explicit loop is needed here.
match rx.recv().await {
Ok(ev) => {
let event = Event::default()
.event("analysis")
.json_data(&ev)
.unwrap_or_else(|_| Event::default().comment("serialize error"));
Some((Ok(event), rx))
}
Err(RecvError::Lagged(_)) => {
Some((Ok(Event::default().event("lagged").data("")), rx))
}
Err(RecvError::Closed) => None,
}
});
Sse::new(stream).keep_alive(KeepAlive::default())
@@ -368,21 +368,15 @@ async fn reenqueue(
(repo::page_analysis::ReenqueueScope::All, "analysis", None)
};
// Enqueue + audit in one transaction so a failed audit insert rolls the
// enqueue back too — the audit trail can't silently miss a re-enqueue that
// actually landed. Both are plain DB writes, so they share the tx cleanly
// (the live event + response are emitted only after commit).
let mut tx = state.db.begin().await?;
let enqueued =
repo::page_analysis::enqueue_pages(&state.db, scope, body.only_unanalyzed).await?;
// Push a live event so connected dashboards mark the in-scope pages as
// queued. Skip the no-op (nothing actually enqueued).
if enqueued > 0 {
state.analysis_events.publish(AnalysisEvent::Enqueued {
count: enqueued,
manga_id: body.manga_id,
chapter_id: body.chapter_id,
});
}
repo::page_analysis::enqueue_pages(&mut *tx, scope, body.only_unanalyzed).await?;
repo::admin_audit::insert(
&state.db,
&mut *tx,
admin.0.id,
"analysis_reenqueue",
target_type,
@@ -395,6 +389,18 @@ async fn reenqueue(
}),
)
.await?;
tx.commit().await?;
// Push a live event so connected dashboards mark the in-scope pages as
// queued. Skip the no-op (nothing actually enqueued). After commit so a
// rolled-back enqueue never emits a phantom event.
if enqueued > 0 {
state.analysis_events.publish(AnalysisEvent::Enqueued {
count: enqueued,
manga_id: body.manga_id,
chapter_id: body.chapter_id,
});
}
Ok(Json(ReenqueueResponse { enqueued }))
}
@@ -421,8 +427,13 @@ async fn analyze_page(
// when a `force=false` job was already pending — the worker would
// then pick up the non-force row, hit skip-if-done, ack done, and
// the admin saw "queued for re-analysis" with no re-analysis.
//
// Enqueue + audit in one transaction (via the `_conn` form) so a failed
// audit insert rolls the enqueue/upgrade back too — the audit trail can't
// miss a force-reanalyze that landed.
let mut tx = state.db.begin().await?;
let outcome =
repo::page_analysis::enqueue_for_page(&state.db, page_id, true).await?;
repo::page_analysis::enqueue_for_page_conn(&mut tx, page_id, true).await?;
let outcome_label = match outcome {
repo::page_analysis::EnqueueForPageOutcome::Inserted => "inserted",
@@ -430,7 +441,7 @@ async fn analyze_page(
repo::page_analysis::EnqueueForPageOutcome::AlreadyEnqueued => "already_force_enqueued",
};
repo::admin_audit::insert(
&state.db,
&mut *tx,
admin.0.id,
"analysis_force_page",
"page",
@@ -438,6 +449,7 @@ async fn analyze_page(
json!({ "force": true, "outcome": outcome_label }),
)
.await?;
tx.commit().await?;
Ok(Json(AnalyzePageResponse { enqueued: true }))
}

View File

@@ -205,6 +205,13 @@ async fn backfill(
resp.more_remaining = more_pages || more_covers;
}
// Audit is written after the action here *by necessity*, not oversight
// (unlike reenqueue/analyze_page, which wrap their single DB mutation +
// audit in one tx). The backfill is a budgeted scan that interleaves
// filesystem `storage.size()` reads with per-batch DB writes across many
// iterations; wrapping it in a transaction would hold one open across all
// that I/O (a long-running tx). The batch size-writes are also idempotent
// recomputations, so a lost audit row has negligible impact.
repo::admin_audit::insert(
&state.db,
admin.0.id,

View File

@@ -75,8 +75,16 @@ pub struct AuthResponse {
#[derive(Debug, Deserialize)]
pub struct CreateTokenInput {
pub name: String,
/// Optional lifetime in days. Omit (or `null`) for a non-expiring
/// token (the historical behaviour). When set, must be 1..=3650.
#[serde(default)]
pub expires_in_days: Option<i64>,
}
/// Upper bound on a requested token lifetime (~10 years). A token that needs
/// to outlive this should be rotated, not minted once forever.
const MAX_TOKEN_EXPIRY_DAYS: i64 = 3650;
#[derive(Debug, Deserialize)]
pub struct ChangePassword {
pub current_password: String,
@@ -135,6 +143,10 @@ async fn login(
"username and password are required".into(),
));
}
// Bound the password before argon2 runs — guards BOTH the real-verify and
// the dummy-hash timing-equaliser branch below, so a giant password can't
// make every login attempt a CPU-DoS.
reject_oversized_password(&input.password)?;
let user = repo::user::find_by_username(&state.db, username).await?;
let Some(user) = user else {
@@ -205,6 +217,9 @@ async fn change_password(
Json(input): Json<ChangePassword>,
) -> AppResult<impl IntoResponse> {
check_auth_rate_limit(&state, "change_password")?;
// Cap current_password before verify_password runs argon2 (same DoS
// vector as login). new_password is bounded by validate_password below.
reject_oversized_password(&input.current_password)?;
if !verify_password(&input.current_password, &user.password_hash) {
return Err(AppError::Unauthenticated);
}
@@ -305,8 +320,22 @@ async fn create_token(
details: serde_json::json!({ "name": "max 64 characters" }),
});
}
let expires_at = match input.expires_in_days {
None => None,
Some(days) if (1..=MAX_TOKEN_EXPIRY_DAYS).contains(&days) => {
Some(Utc::now() + Duration::days(days))
}
Some(_) => {
return Err(AppError::ValidationFailed {
message: "token expiry out of range".into(),
details: serde_json::json!({
"expires_in_days": format!("must be between 1 and {MAX_TOKEN_EXPIRY_DAYS}")
}),
});
}
};
let (raw, hash) = generate_token();
let token = repo::api_token::create(&state.db, user.id, name, &hash).await?;
let token = repo::api_token::create(&state.db, user.id, name, &hash, expires_at).await?;
Ok((
StatusCode::CREATED,
Json(CreatedTokenResponse { token, bearer: raw }),
@@ -425,11 +454,67 @@ pub(crate) fn validate_username(u: &str) -> AppResult<()> {
Ok(())
}
/// Upper bound on password length (bytes). argon2 has no inherent length
/// limit, so without a cap an attacker could submit a multi-megabyte password
/// and make every hash a CPU-heavy DoS. 1024 bytes is far longer than any real
/// passphrase. Measured in bytes (like the min check) since that's what argon2
/// actually processes.
pub(crate) const MAX_PASSWORD_BYTES: usize = 1024;
/// Reject an over-cap password *before* any argon2 work runs. The
/// verification paths (login, change-password) don't go through
/// [`validate_password`] — they hash/verify the raw input — so without this
/// an attacker could submit a multi-megabyte password and turn every
/// login/verify into a CPU-DoS. Length-only and independent of whether the
/// account exists, so it leaks nothing (no username enumeration).
pub(crate) fn reject_oversized_password(p: &str) -> AppResult<()> {
if p.len() > MAX_PASSWORD_BYTES {
return Err(AppError::InvalidInput(format!(
"password must be at most {MAX_PASSWORD_BYTES} bytes"
)));
}
Ok(())
}
pub(crate) fn validate_password(p: &str) -> AppResult<()> {
if p.len() < 8 {
return Err(AppError::InvalidInput(
"password must be at least 8 characters".into(),
));
}
if p.len() > MAX_PASSWORD_BYTES {
return Err(AppError::InvalidInput(
format!("password must be at most {MAX_PASSWORD_BYTES} bytes"),
));
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn validate_password_rejects_too_short() {
assert!(validate_password("short").is_err());
assert!(validate_password("1234567").is_err()); // 7 chars
}
#[test]
fn validate_password_accepts_in_range() {
assert!(validate_password("hunter2hunter2").is_ok());
// Exactly at the cap is allowed.
assert!(validate_password(&"a".repeat(MAX_PASSWORD_BYTES)).is_ok());
// Minimum length boundary.
assert!(validate_password("12345678").is_ok());
}
#[test]
fn validate_password_rejects_over_cap() {
// One byte past the cap is refused so a giant password can't turn each
// login/register into an argon2 CPU-DoS.
let too_long = "a".repeat(MAX_PASSWORD_BYTES + 1);
let err = validate_password(&too_long).unwrap_err();
assert!(matches!(err, AppError::InvalidInput(_)));
}
}

View File

@@ -69,6 +69,21 @@ async fn get_one(
Ok(Json(chapter))
}
/// Add a chapter to a manga.
///
/// **Authorization is intentionally open**: any authenticated principal —
/// a browser session *or* a bot API token — may add a chapter to *any*
/// manga, including crawler-imported rows. This is by design: chapters are
/// treated as community contributions, unlike the manga record itself
/// (title/cover/metadata), whose edits gate through `require_can_edit`
/// (see [`crate::api::mangas`]). The `CurrentUser` binding still requires a
/// valid identity, so contributions are attributable, just not owner-scoped.
///
/// This contract is locked by `tests/api_chapters.rs`
/// (`non_owner_can_upload_chapter`); changing it to owner-only is a
/// deliberate decision, not a drive-by tightening. Until a richer
/// contributor/moderation model lands this is acknowledged, intended
/// behaviour — see the auth note in CLAUDE.md.
async fn create(
State(state): State<AppState>,
CurrentUser(user): CurrentUser,

View File

@@ -255,8 +255,8 @@ async fn create(
)
.await?;
let author_refs = repo::author::set_for_manga(&mut *tx, manga.id, &authors).await?;
repo::genre::set_for_manga(&mut *tx, manga.id, &metadata.genre_ids).await?;
let author_refs = repo::author::set_for_manga(&mut tx, manga.id, &authors).await?;
repo::genre::set_for_manga(&mut tx, manga.id, &metadata.genre_ids).await?;
if let Some(img) = cover {
let key = format!("mangas/{}/cover.{}", manga.id, img.ext);
@@ -321,7 +321,7 @@ async fn update(
let mut tx = state.db.begin().await?;
let _updated = repo::manga::update_basics(
&mut *tx,
&mut tx,
id,
patch.title.as_deref().map(str::trim),
patch.status.as_deref().map(str::trim),
@@ -331,10 +331,10 @@ async fn update(
)
.await?;
if let Some(ref names) = authors_owned {
repo::author::set_for_manga(&mut *tx, id, names).await?;
repo::author::set_for_manga(&mut tx, id, names).await?;
}
if let Some(ref ids) = patch.genre_ids {
repo::genre::set_for_manga(&mut *tx, id, ids).await?;
repo::genre::set_for_manga(&mut tx, id, ids).await?;
}
tx.commit().await?;

View File

@@ -440,13 +440,22 @@ async fn spawn_analysis_daemon(
let engine = crate::analysis::ocr::OcrsEngine::from_model_paths(
&cfg.ocr_detection_model,
&cfg.ocr_recognition_model,
cfg.ocr_max_decode_pixels,
)
.context("build ocrs engine")?;
// Cap concurrent CPU-bound OCR runs across all workers so a high
// ANALYSIS_WORKERS can't oversubscribe the blocking pool.
let cores = std::thread::available_parallelism()
.map(|n| n.get())
.unwrap_or(1);
let permits =
crate::analysis::ocr::ocr_concurrency_limit(cfg.workers, cores);
let dispatcher = Arc::new(crate::analysis::ocr::OcrAnalyzeDispatcher {
db: db.clone(),
storage,
engine: Arc::new(engine),
max_image_bytes: cfg.max_image_bytes,
ocr_permits: Arc::new(tokio::sync::Semaphore::new(permits)),
});
// In-process engine is always ready — no gate.
(dispatcher, None)
@@ -460,6 +469,12 @@ async fn spawn_analysis_daemon(
// both. Mirrors the crawler client's `.no_proxy()` (see
// `spawn_crawler_daemon`).
.no_proxy()
// Re-validate redirect hops so a hostile/compromised vision
// endpoint can't 302 the bearer token + page bytes into the
// deployment's internal network. No allowlist here (the
// endpoint is a single admin-configured URL), so the policy
// enforces scheme + private-IP only.
.redirect(crate::crawler::safety::public_redirect_policy())
.build()
.context("build analysis http client")?;
let vision = crate::analysis::vision::VisionClient::new(http, cfg);
@@ -485,6 +500,7 @@ async fn spawn_analysis_daemon(
// still gets a useful side-channel on backend
// uptime + vision health.
.no_proxy()
.redirect(crate::crawler::safety::public_redirect_policy())
.build()
.context("build vision readiness http client")?;
Some(Arc::new(crate::analysis::daemon::HttpVisionReadiness {
@@ -562,6 +578,13 @@ async fn spawn_crawler_daemon(
let mut http_builder = reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(30))
.no_proxy()
// Re-validate every redirect hop against the download allowlist:
// reqwest's default policy follows up to 10 redirects, and
// `is_safe_url` only guards the initial URL, so an allowlisted CDN
// 302ing to a private IP would otherwise be followed (SSRF).
.redirect(crate::crawler::safety::safe_redirect_policy(
cfg.download_allowlist.clone(),
))
.cookie_provider(Arc::clone(&cookie_jar));
if let Some(ua) = &cfg.user_agent {
http_builder = http_builder.user_agent(ua);
@@ -1243,8 +1266,8 @@ fn parse_origin(raw: &str) -> Option<String> {
let host = url.host_str()?;
let scheme = url.scheme();
let port_str = match (url.port(), scheme) {
(Some(p), "http") if p == 80 => String::new(),
(Some(p), "https") if p == 443 => String::new(),
(Some(80), "http") => String::new(),
(Some(443), "https") => String::new(),
(Some(p), _) => format!(":{p}"),
(None, _) => String::new(),
};
@@ -1486,7 +1509,6 @@ mod tests {
let storage_dir = tempfile::tempdir().unwrap();
let storage: Arc<dyn Storage> =
Arc::new(LocalStorage::new(storage_dir.path()));
let mut cfg = crate::config::AnalysisConfig::default();
// The worker always runs OCR now (vision is dormant — see
// `effective_backend`), and the `.rten` models aren't shipped to unit
// CI. Point the engine at a path that can't exist so the *engine build*
@@ -1495,10 +1517,13 @@ mod tests {
// readiness, so the row must still be reclaimed even though spawn
// returns Err. That's exactly the regression this test guards (an
// analysis-only deploy must reclaim orphaned leases at startup).
cfg.ocr_detection_model = "/nonexistent/text-detection.rten".to_string();
cfg.ocr_recognition_model = "/nonexistent/text-recognition.rten".to_string();
cfg.workers = 1;
cfg.job_timeout = Duration::from_secs(1);
let cfg = crate::config::AnalysisConfig {
ocr_detection_model: "/nonexistent/text-detection.rten".to_string(),
ocr_recognition_model: "/nonexistent/text-recognition.rten".to_string(),
workers: 1,
job_timeout: Duration::from_secs(1),
..Default::default()
};
let events = Arc::new(crate::analysis::events::AnalysisEvents::new());
let spawned = spawn_analysis_daemon(pool.clone(), storage, &cfg, events).await;

View File

@@ -112,9 +112,20 @@ async fn main() -> anyhow::Result<()> {
cookie_jar.add_cookie_str(&cookie_str, &seed_url);
tracing::info!(domain, "seeded PHPSESSID into reqwest cookie jar");
}
// SSRF defence: only download from the catalog host + CDN host (plus
// optional CRAWLER_DOWNLOAD_ALLOWLIST extras). Built here so the same
// allowlist guards both the redirect policy and the per-image check.
let allowlist = Arc::new(build_download_allowlist(&start_url, cdn_host.as_deref()));
let mut http_builder = reqwest::Client::builder()
.timeout(Duration::from_secs(30))
.no_proxy()
// Re-validate every redirect hop against the allowlist — reqwest's
// default follows up to 10 redirects and `is_safe_url` only guards
// the initial URL, so a 302 to a private IP would otherwise pivot
// inside the deployment (SSRF).
.redirect(mangalord::crawler::safety::safe_redirect_policy(
(*allowlist).clone(),
))
.cookie_provider(cookie_jar);
if let Some(ua) = &user_agent {
http_builder = http_builder.user_agent(ua);
@@ -216,6 +227,7 @@ async fn main() -> anyhow::Result<()> {
rate_ms,
cdn_host.as_deref(),
cdn_rate_ms,
Arc::clone(&allowlist),
limit,
skip_chapters,
skip_chapter_content || !session_ready,
@@ -246,6 +258,7 @@ async fn run(
rate_ms: u64,
cdn_host: Option<&str>,
cdn_rate_ms: u64,
allowlist: Arc<mangalord::crawler::safety::DownloadAllowlist>,
limit: usize,
skip_chapters: bool,
skip_chapter_content: bool,
@@ -259,38 +272,12 @@ async fn run(
}
let rate = Arc::new(rate);
// SSRF defence: only download from the catalog host + CDN host
// (plus optional CRAWLER_DOWNLOAD_ALLOWLIST extras), and cap
// single-image downloads at CRAWLER_MAX_IMAGE_BYTES bytes.
// CRAWLER_ALLOW_ANY_HOST=true short-circuits the host check for
// sharded-CDN sources; private-IP and scheme guards still apply.
let allowlist = if env_bool("CRAWLER_ALLOW_ANY_HOST", false) {
mangalord::crawler::safety::DownloadAllowlist::allow_any()
} else {
let mut allow = mangalord::crawler::safety::DownloadAllowlist::new();
if let Ok(parsed) = reqwest::Url::parse(start_url) {
if let Some(h) = parsed.host_str() {
allow = allow.allow(h);
}
}
if let Some(host) = cdn_host {
allow = allow.allow(host);
}
if let Ok(extras) = std::env::var("CRAWLER_DOWNLOAD_ALLOWLIST") {
for piece in extras.split(',') {
let trimmed = piece.trim();
if !trimmed.is_empty() {
allow = allow.allow(trimmed);
}
}
}
allow
};
// Per-image download cap (the allowlist is built in `main` and passed in
// so the HTTP client's redirect policy and this check share one source).
let max_image_bytes: usize = std::env::var("CRAWLER_MAX_IMAGE_BYTES")
.ok()
.and_then(|s| s.parse().ok())
.unwrap_or(mangalord::crawler::safety::DEFAULT_MAX_IMAGE_BYTES);
let allowlist = Arc::new(allowlist);
let stats = pipeline::run_metadata_pass(
manager.as_ref(),
@@ -497,3 +484,37 @@ fn env_bool(name: &str, default: bool) -> bool {
}
}
/// Build the crawler download allowlist from env + the catalog/CDN hosts.
/// Shared by the HTTP client's redirect policy and the per-image safety
/// check so both agree on which hosts are reachable.
///
/// `CRAWLER_ALLOW_ANY_HOST=true` short-circuits the host check for
/// sharded-CDN sources; private-IP and scheme guards still apply.
fn build_download_allowlist(
start_url: &str,
cdn_host: Option<&str>,
) -> mangalord::crawler::safety::DownloadAllowlist {
use mangalord::crawler::safety::DownloadAllowlist;
if env_bool("CRAWLER_ALLOW_ANY_HOST", false) {
return DownloadAllowlist::allow_any();
}
let mut allow = DownloadAllowlist::new();
if let Ok(parsed) = reqwest::Url::parse(start_url) {
if let Some(h) = parsed.host_str() {
allow = allow.allow(h);
}
}
if let Some(host) = cdn_host {
allow = allow.allow(host);
}
if let Ok(extras) = std::env::var("CRAWLER_DOWNLOAD_ALLOWLIST") {
for piece in extras.split(',') {
let trimmed = piece.trim();
if !trimmed.is_empty() {
allow = allow.allow(trimmed);
}
}
}
allow
}

View File

@@ -202,6 +202,13 @@ pub struct AnalysisConfig {
/// Hard cap on a page image's stored size; larger pages are skipped
/// (`ANALYSIS_MAX_IMAGE_BYTES`).
pub max_image_bytes: usize,
/// Hard cap on a page image's **decoded** pixel count for the OCR backend
/// (`ANALYSIS_OCR_MAX_DECODE_PIXELS`). `max_image_bytes` only bounds the
/// *encoded* size; without a decode bound a tiny image declaring
/// 50000×50000 inflates to billions of bytes and OOM-kills the worker
/// (decompression bomb). Generous by default (100 MP) so legitimately
/// tall, un-sliced manga pages still decode.
pub ocr_max_decode_pixels: u64,
/// Output-constraint mode (`ANALYSIS_RESPONSE_FORMAT`):
/// `json_schema` (default) | `json_object` | `none`.
pub response_format: ResponseFormat,
@@ -251,6 +258,7 @@ impl Default for AnalysisConfig {
tall_aspect_threshold: 1.6,
max_slices: 16,
max_image_bytes: 8 * 1024 * 1024,
ocr_max_decode_pixels: 100_000_000,
response_format: ResponseFormat::JsonSchema,
frequency_penalty: 0.3,
temperature: 0.0,
@@ -327,6 +335,10 @@ impl AnalysisConfig {
.max(1.0),
max_slices: env_usize("ANALYSIS_MAX_SLICES", d.max_slices).max(1),
max_image_bytes: env_usize("ANALYSIS_MAX_IMAGE_BYTES", d.max_image_bytes),
ocr_max_decode_pixels: env_u64(
"ANALYSIS_OCR_MAX_DECODE_PIXELS",
d.ocr_max_decode_pixels,
),
response_format: std::env::var("ANALYSIS_RESPONSE_FORMAT")
.map(|s| ResponseFormat::from_str(&s))
.unwrap_or(d.response_format),
@@ -864,8 +876,10 @@ mod tests {
// what `ANALYSIS_BACKEND` parsed to. `backend` still reflects the raw
// request (so the override is visible/loggable), but `effective_backend`
// is the value the daemon actually dispatches through.
let mut cfg = AnalysisConfig::default();
cfg.backend = AnalysisBackend::Vision;
let mut cfg = AnalysisConfig {
backend: AnalysisBackend::Vision,
..Default::default()
};
assert_eq!(cfg.effective_backend(), AnalysisBackend::Ocr);
cfg.backend = AnalysisBackend::Ocr;
assert_eq!(cfg.effective_backend(), AnalysisBackend::Ocr);

View File

@@ -234,12 +234,20 @@ impl BrowserManager {
await_drain(&self.active, drain_deadline).await;
self.set_phase(RestartPhase::Restarting);
let relaunch = {
// Take the dead handle out under the lock, release the lock, THEN run
// the (slow) Chromium teardown — so a worker that raced past the drain
// can't block on `acquire()` behind one dead browser's close(). Mirrors
// the idle reaper's take-drop-close ordering. Re-acquire to relaunch.
let dead = {
let mut guard = self.inner.lock().await;
guard.shared = None;
if let Some(handle) = guard.handle.take() {
let _ = handle.close().await;
}
guard.handle.take()
};
if let Some(handle) = dead {
let _ = handle.close().await;
}
let relaunch = {
let mut guard = self.inner.lock().await;
self.launch_into(&mut guard).await
};
@@ -257,9 +265,14 @@ impl BrowserManager {
/// Used on daemon shutdown. After this returns the next acquire will
/// re-launch from scratch.
pub async fn shutdown(&self) {
let mut guard = self.inner.lock().await;
guard.shared = None;
if let Some(handle) = guard.handle.take() {
// Take-then-drop-then-close: don't hold the lock across Chromium
// teardown (see `invalidate` / the idle reaper).
let handle = {
let mut guard = self.inner.lock().await;
guard.shared = None;
guard.handle.take()
};
if let Some(handle) = handle {
let _ = handle.close().await;
}
}
@@ -278,9 +291,16 @@ impl BrowserManager {
/// Idempotent: calling on an already-invalidated manager is a
/// no-op.
pub async fn invalidate(&self) {
let mut guard = self.inner.lock().await;
guard.shared = None;
if let Some(handle) = guard.handle.take() {
// Take the handle out under the lock, then release the lock BEFORE the
// slow Chromium close() — otherwise every other worker's `acquire()`
// serializes behind one dead browser's teardown. Matches the idle
// reaper's ordering at the bottom of this file.
let handle = {
let mut guard = self.inner.lock().await;
guard.shared = None;
guard.handle.take()
};
if let Some(handle) = handle {
let _ = handle.close().await;
tracing::warn!("BrowserManager: handle invalidated — next acquire will relaunch");
}

View File

@@ -18,7 +18,7 @@ use uuid::Uuid;
use crate::crawler::detect::PageError;
use crate::crawler::rate_limit::HostRateLimiters;
use crate::crawler::safety::{fetch_stream, looks_like_image, DownloadAllowlist};
use crate::crawler::safety::{ensure_public_target, fetch_stream, looks_like_image, DownloadAllowlist};
use crate::crawler::session::{self, ChapterProbe};
use crate::storage::{Storage, StorageError};
@@ -95,6 +95,19 @@ enum ChapterFetchOutcome {
PersistentTransient,
}
/// Refuse to navigate Chromium at a chapter URL that points inside the
/// deployment. The image-download path goes through `is_safe_url`, but
/// `new_page` is a second network surface: a `chapter_sources` row whose
/// host resolves to (or was crafted to be) a private IP would otherwise let
/// the headless browser probe `postgres:5432`, the cloud metadata service,
/// etc. Reuses the allowlist-free `ensure_public_target` (scheme +
/// private-IP literal check) since there is no per-host allowlist for the
/// scraped catalog itself.
fn guard_nav_url(source_url: &str) -> anyhow::Result<()> {
ensure_public_target(source_url)
.map_err(|e| anyhow::anyhow!("refuse to navigate unsafe chapter URL {source_url}: {e}"))
}
/// Single rate-limited Chromium navigation to the chapter URL,
/// returning the page HTML. Extracted from `sync_chapter_content` so
/// the recircuit loop can call it once per attempt.
@@ -103,6 +116,7 @@ async fn fetch_chapter_html_once(
rate: &HostRateLimiters,
source_url: &str,
) -> anyhow::Result<String> {
guard_nav_url(source_url)?;
rate.wait_for(source_url).await?;
let page = browser
.new_page(source_url)
@@ -609,6 +623,27 @@ mod tests {
use super::*;
use crate::storage::LocalStorage;
#[test]
fn guard_nav_url_rejects_private_and_loopback_targets() {
// A chapter_sources row that resolves to / was crafted as an
// internal target must be refused before Chromium navigates.
for url in [
"http://127.0.0.1:5432/",
"http://169.254.169.254/latest/meta-data/",
"http://10.0.0.1/chapter/1",
"http://localhost:8080/",
"file:///etc/passwd",
] {
assert!(guard_nav_url(url).is_err(), "must reject {url}");
}
}
#[test]
fn guard_nav_url_allows_public_chapter_urls() {
assert!(guard_nav_url("https://reader.example.com/chapter/42").is_ok());
assert!(guard_nav_url("http://manga-host.test/c/1/p/2").is_ok());
}
#[tokio::test]
async fn cleanup_orphans_deletes_written_keys() {
let dir = tempfile::tempdir().unwrap();

View File

@@ -132,14 +132,17 @@ fn backoff_for(attempts: i32) -> Duration {
/// `Skipped`. The slot frees again once the previous job leaves the
/// in-flight states (done/failed/dead), so a re-enqueue after a force
/// refetch succeeds.
pub async fn enqueue(pool: &PgPool, payload: &JobPayload) -> sqlx::Result<EnqueueResult> {
pub async fn enqueue<'e, E>(executor: E, payload: &JobPayload) -> sqlx::Result<EnqueueResult>
where
E: sqlx::PgExecutor<'e>,
{
let json = serde_json::to_value(payload).expect("JobPayload is always serializable");
let id: Option<Uuid> = sqlx::query_scalar(
"INSERT INTO crawler_jobs (payload) VALUES ($1) \
ON CONFLICT DO NOTHING RETURNING id",
)
.bind(json)
.fetch_optional(pool)
.fetch_optional(executor)
.await?;
Ok(match id {
Some(id) => EnqueueResult::Inserted(id),
@@ -422,7 +425,10 @@ pub async fn release(
/// the original worker becomes a no-op. Returns the attempt to
/// `pending` and refunds the retry attempt (the operator click
/// isn't a job-level failure).
pub async fn release_unowned(pool: &PgPool, lease_id: Uuid) -> sqlx::Result<()> {
pub async fn release_unowned<'e, E>(executor: E, lease_id: Uuid) -> sqlx::Result<()>
where
E: sqlx::PgExecutor<'e>,
{
let res = sqlx::query(
"UPDATE crawler_jobs \
SET state = 'pending', leased_until = NULL, \
@@ -432,7 +438,7 @@ pub async fn release_unowned(pool: &PgPool, lease_id: Uuid) -> sqlx::Result<()>
WHERE id = $1 AND state = 'running'",
)
.bind(lease_id)
.execute(pool)
.execute(executor)
.await?;
if res.rows_affected() == 0 {
tracing::warn!(

View File

@@ -164,8 +164,7 @@ mod tests {
#[test]
fn anyhow_with_nav_timeout_in_chain_is_flagged() {
let inner: Result<(), NavError> = Err(NavError::Timeout(NAV_TIMEOUT));
let outer = inner.unwrap_err();
let outer = NavError::Timeout(NAV_TIMEOUT);
let wrapped: anyhow::Error =
anyhow::Error::new(outer).context("wait for chapter nav");
assert!(anyhow_looks_browser_dead(&wrapped));

View File

@@ -44,6 +44,25 @@ impl RateLimiter {
}
}
/// Once the per-host map grows past this many entries, the next `wait_for`
/// sweeps out hosts idle longer than [`IDLE_EVICT_AFTER`]. A long crawl that
/// touches thousands of distinct CDN shards would otherwise retain one bucket
/// per host for the daemon's whole lifetime. Far above any single source's
/// real host count, so the sweep is rare.
const MAX_TRACKED_HOSTS: usize = 1024;
/// A host bucket untouched for at least this long is evicted on the next
/// over-cap sweep. Generous: a host still being crawled is touched every
/// `interval`, so only genuinely-finished hosts age out.
const IDLE_EVICT_AFTER: Duration = Duration::from_secs(3600);
/// A host's bucket plus when it was last used, so idle entries can be evicted.
#[derive(Debug)]
struct HostEntry {
limiter: Arc<Mutex<RateLimiter>>,
last_used: Instant,
}
/// Per-host rate limiter map. The outer `Mutex<HashMap>` is held only
/// during the entry-or-insert + Arc clone; the per-host `Mutex<RateLimiter>`
/// is held during the actual `wait().await`. So N workers calling
@@ -54,7 +73,7 @@ impl RateLimiter {
pub struct HostRateLimiters {
default_interval: Duration,
overrides: HashMap<String, Duration>,
map: Mutex<HashMap<String, Arc<Mutex<RateLimiter>>>>,
map: Mutex<HashMap<String, HostEntry>>,
}
impl HostRateLimiters {
@@ -82,20 +101,37 @@ impl HostRateLimiters {
.ok_or_else(|| anyhow::anyhow!("no host in url: {url}"))?;
let limiter = {
let mut map = self.map.lock().await;
map.entry(host.clone())
.or_insert_with(|| {
let interval = self
.overrides
.get(&host)
.copied()
.unwrap_or(self.default_interval);
Arc::new(Mutex::new(RateLimiter::new(interval)))
})
.clone()
let now = Instant::now();
// Bound the map: when it grows past the soft cap, drop hosts that
// have been idle past the TTL. Only fires over-cap, so the common
// path is a plain lookup. A host still being crawled is touched
// every `interval` and so never ages out.
if map.len() >= MAX_TRACKED_HOSTS {
map.retain(|_, e| now.duration_since(e.last_used) < IDLE_EVICT_AFTER);
}
let entry = map.entry(host.clone()).or_insert_with(|| {
let interval = self
.overrides
.get(&host)
.copied()
.unwrap_or(self.default_interval);
HostEntry {
limiter: Arc::new(Mutex::new(RateLimiter::new(interval))),
last_used: now,
}
});
entry.last_used = now;
entry.limiter.clone()
};
limiter.lock().await.wait().await;
Ok(())
}
/// Number of host buckets currently tracked. Test/observability hook.
#[cfg(test)]
pub(crate) async fn tracked_hosts(&self) -> usize {
self.map.lock().await.len()
}
}
// `host_of` was duplicated across session/rate_limit/pipeline; the
@@ -165,6 +201,46 @@ mod tests {
);
}
#[tokio::test(start_paused = true)]
async fn host_rate_limiters_evict_idle_hosts_over_cap() {
// Fill the map to the soft cap with distinct hosts (first call to a
// fresh host never sleeps, so this is fast even under real time).
let rl = HostRateLimiters::new(Duration::from_millis(1));
for i in 0..MAX_TRACKED_HOSTS {
rl.wait_for(&format!("https://host{i}.example/x")).await.unwrap();
}
assert_eq!(rl.tracked_hosts().await, MAX_TRACKED_HOSTS);
// Let every tracked host age past the idle TTL, then touch one new
// host: the over-cap sweep should evict all the idle ones, leaving
// only the freshly-inserted entry.
tokio::time::sleep(IDLE_EVICT_AFTER + Duration::from_secs(1)).await;
rl.wait_for("https://newcomer.example/y").await.unwrap();
assert_eq!(
rl.tracked_hosts().await,
1,
"idle hosts should be swept once the map is over the cap"
);
}
#[tokio::test(start_paused = true)]
async fn host_rate_limiters_keep_recently_used_hosts() {
// A host touched within the TTL must survive a sweep so active crawls
// aren't reset. Fill to the cap, then re-touch one host right before
// adding a newcomer that triggers the sweep.
let rl = HostRateLimiters::new(Duration::from_millis(1));
for i in 0..MAX_TRACKED_HOSTS {
rl.wait_for(&format!("https://host{i}.example/x")).await.unwrap();
}
tokio::time::sleep(IDLE_EVICT_AFTER + Duration::from_secs(1)).await;
// Re-touch host0 so it's recent again.
rl.wait_for("https://host0.example/x").await.unwrap();
// Newcomer triggers the sweep (map is at cap+1 conceptually).
rl.wait_for("https://newcomer.example/y").await.unwrap();
// host0 (recent) + newcomer survive; the rest aged out.
assert_eq!(rl.tracked_hosts().await, 2);
}
#[tokio::test(start_paused = true)]
async fn host_rate_limiters_honor_overrides() {
let rl = HostRateLimiters::new(Duration::from_millis(1000))

View File

@@ -226,6 +226,73 @@ pub enum UrlSafetyError {
HostNotAllowed(String),
}
/// Maximum number of redirects the crawler will follow before giving up.
/// Matches reqwest's historical default; every hop is re-validated by
/// [`check_redirect_hop`], so the cap is a belt-and-braces stop against a
/// redirect loop rather than the primary SSRF defence.
pub const MAX_REDIRECTS: usize = 10;
/// Why a redirect hop was refused.
#[derive(Debug, thiserror::Error)]
pub enum RedirectError {
#[error("redirect chain exceeded {0} hops")]
TooManyHops(usize),
#[error("redirect target rejected: {0}")]
Unsafe(#[from] UrlSafetyError),
}
/// Decide whether a single redirect hop is safe to follow.
///
/// `is_safe_url` only inspects the *initial* URL a caller hands to reqwest;
/// without re-validation an allowlisted CDN that answers `302 ->
/// http://169.254.169.254/...` or `-> http://127.0.0.1:5432/` would be
/// followed transparently (reqwest's default policy follows up to 10
/// redirects). This re-runs the full allowlist + private-IP + scheme check on
/// each hop and enforces [`MAX_REDIRECTS`]. `completed_hops` is the number of
/// URLs already visited in the chain (reqwest's `attempt.previous().len()`).
pub fn check_redirect_hop(
next_url: &str,
completed_hops: usize,
allow: &DownloadAllowlist,
) -> Result<(), RedirectError> {
if completed_hops >= MAX_REDIRECTS {
return Err(RedirectError::TooManyHops(completed_hops));
}
is_safe_url(next_url, allow)?;
Ok(())
}
/// Build a reqwest redirect policy that re-validates every hop against the
/// download allowlist (see [`check_redirect_hop`]). Use for the crawler image
/// clients, which fetch attacker-influenced URLs.
pub fn safe_redirect_policy(allow: DownloadAllowlist) -> reqwest::redirect::Policy {
reqwest::redirect::Policy::custom(move |attempt| {
let hops = attempt.previous().len();
match check_redirect_hop(attempt.url().as_str(), hops, &allow) {
Ok(()) => attempt.follow(),
Err(e) => attempt.error(e),
}
})
}
/// Build a reqwest redirect policy that re-validates every hop with
/// [`ensure_public_target`] (scheme + private-IP, no allowlist). Use for
/// single-endpoint clients (the analysis vision endpoint / its probe) where
/// there is no per-deployment allowlist but a redirect into the deployment's
/// internal network must still be refused.
pub fn public_redirect_policy() -> reqwest::redirect::Policy {
reqwest::redirect::Policy::custom(move |attempt| {
let hops = attempt.previous().len();
if hops >= MAX_REDIRECTS {
return attempt.error(RedirectError::TooManyHops(hops));
}
match ensure_public_target(attempt.url().as_str()) {
Ok(()) => attempt.follow(),
Err(e) => attempt.error(RedirectError::Unsafe(e)),
}
})
}
/// Drain a byte stream into a single buffer, bailing out as soon as
/// the running total exceeds `max_bytes`. Generic over the stream so
/// it's testable without a live HTTP response.
@@ -578,6 +645,54 @@ mod tests {
assert!(is_safe_url("https://CDN.EXAMPLE.com/x.jpg", &allow).is_ok());
}
// --- redirect-hop re-validation (SSRF via 3xx) ---
#[test]
fn redirect_hop_allows_listed_public_target() {
let allow = allow_just("cdn.example.com");
assert!(check_redirect_hop("https://cdn.example.com/next.jpg", 1, &allow).is_ok());
}
#[test]
fn redirect_hop_blocks_private_ip_target() {
// The core SSRF case: an allowlisted CDN 302s to the cloud metadata
// service / an intra-compose port. Must be refused mid-chain.
let allow = allow_just("cdn.example.com");
for url in ["http://169.254.169.254/", "http://127.0.0.1:5432/", "http://10.0.0.1/"] {
let err = check_redirect_hop(url, 1, &allow).unwrap_err();
assert!(
matches!(err, RedirectError::Unsafe(UrlSafetyError::PrivateIp(_))),
"expected PrivateIp for {url}, got {err:?}"
);
}
}
#[test]
fn redirect_hop_blocks_off_allowlist_public_host() {
// Per the strict policy: a redirect to an unlisted *public* host is
// also refused (allow_any covers the numbered-CDN case instead).
let allow = allow_just("cdn.example.com");
let err = check_redirect_hop("https://evil.example.org/x", 1, &allow).unwrap_err();
assert!(matches!(err, RedirectError::Unsafe(UrlSafetyError::HostNotAllowed(_))));
}
#[test]
fn redirect_hop_blocks_bad_scheme_target() {
let allow = DownloadAllowlist::allow_any();
let err = check_redirect_hop("file:///etc/passwd", 1, &allow).unwrap_err();
assert!(matches!(err, RedirectError::Unsafe(UrlSafetyError::BadScheme(_))));
}
#[test]
fn redirect_hop_caps_chain_length() {
let allow = allow_just("cdn.example.com");
// A safe target is still refused once the hop cap is reached, so a
// redirect loop can't spin forever.
let err = check_redirect_hop("https://cdn.example.com/x", MAX_REDIRECTS, &allow)
.unwrap_err();
assert!(matches!(err, RedirectError::TooManyHops(n) if n == MAX_REDIRECTS));
}
#[tokio::test]
async fn accumulate_capped_returns_full_body_under_cap() {
let chunks: Vec<Result<bytes::Bytes, std::io::Error>> = vec![

View File

@@ -12,4 +12,6 @@ pub struct ApiToken {
pub token_hash: Vec<u8>,
pub created_at: DateTime<Utc>,
pub last_used_at: Option<DateTime<Utc>>,
/// When the token stops authenticating. `None` = never expires.
pub expires_at: Option<DateTime<Utc>>,
}

View File

@@ -2,6 +2,7 @@
//! token; the raw value is shown to the user once at creation and never
//! stored.
use chrono::{DateTime, Utc};
use sqlx::PgPool;
use uuid::Uuid;
@@ -13,17 +14,19 @@ pub async fn create(
user_id: Uuid,
name: &str,
token_hash: &[u8],
expires_at: Option<DateTime<Utc>>,
) -> AppResult<ApiToken> {
let row = sqlx::query_as::<_, ApiToken>(
r#"
INSERT INTO api_tokens (user_id, name, token_hash)
VALUES ($1, $2, $3)
RETURNING id, user_id, name, token_hash, created_at, last_used_at
INSERT INTO api_tokens (user_id, name, token_hash, expires_at)
VALUES ($1, $2, $3, $4)
RETURNING id, user_id, name, token_hash, created_at, last_used_at, expires_at
"#,
)
.bind(user_id)
.bind(name)
.bind(token_hash)
.bind(expires_at)
.fetch_one(pool)
.await?;
Ok(row)
@@ -32,9 +35,10 @@ pub async fn create(
pub async fn find_active(pool: &PgPool, token_hash: &[u8]) -> AppResult<Option<ApiToken>> {
let row = sqlx::query_as::<_, ApiToken>(
r#"
SELECT id, user_id, name, token_hash, created_at, last_used_at
SELECT id, user_id, name, token_hash, created_at, last_used_at, expires_at
FROM api_tokens
WHERE token_hash = $1
AND (expires_at IS NULL OR expires_at > now())
"#,
)
.bind(token_hash)

View File

@@ -78,13 +78,30 @@ pub enum EnqueueForPageOutcome {
/// with `force=true`, running an UPDATE that flips the existing pending
/// row's `force` flag to `true`; (3) reporting which path ran so the
/// caller can audit accurately.
/// Pool convenience wrapper for [`enqueue_for_page_conn`]. Use this from
/// callers that don't need to share a transaction (chapter upload, the
/// crawler). The admin force-reanalyze handler uses the `_conn` form so the
/// enqueue and its `admin_audit` row commit together.
pub async fn enqueue_for_page(
pool: &PgPool,
page_id: Uuid,
force: bool,
) -> AppResult<EnqueueForPageOutcome> {
let mut conn = pool.acquire().await?;
enqueue_for_page_conn(&mut conn, page_id, force).await
}
/// Enqueue (or force-upgrade) an `analyze_page` job on a caller-supplied
/// connection, so the admin handler can run it inside the same transaction as
/// its audit insert. The body is a sequence of single statements, each
/// reborrowing `&mut *conn`.
pub async fn enqueue_for_page_conn(
conn: &mut sqlx::PgConnection,
page_id: Uuid,
force: bool,
) -> AppResult<EnqueueForPageOutcome> {
use crate::crawler::jobs::EnqueueResult;
match jobs::enqueue(pool, &JobPayload::AnalyzePage { page_id, force }).await? {
match jobs::enqueue(&mut *conn, &JobPayload::AnalyzePage { page_id, force }).await? {
EnqueueResult::Inserted(_) => Ok(EnqueueForPageOutcome::Inserted),
EnqueueResult::Skipped if !force => Ok(EnqueueForPageOutcome::AlreadyEnqueued),
EnqueueResult::Skipped => {
@@ -106,7 +123,7 @@ pub async fn enqueue_for_page(
RETURNING id, state",
)
.bind(page_id.to_string())
.fetch_all(pool)
.fetch_all(&mut *conn)
.await?;
if upgraded.is_empty() {
// Race: between the skipped INSERT and the UPDATE the
@@ -115,7 +132,9 @@ pub async fn enqueue_for_page(
// would succeed. Retry once to close the race without
// unbounded loops.
return Ok(
match jobs::enqueue(pool, &JobPayload::AnalyzePage { page_id, force }).await? {
match jobs::enqueue(&mut *conn, &JobPayload::AnalyzePage { page_id, force })
.await?
{
EnqueueResult::Inserted(_) => EnqueueForPageOutcome::Inserted,
EnqueueResult::Skipped => EnqueueForPageOutcome::AlreadyEnqueued,
},
@@ -139,7 +158,7 @@ pub async fn enqueue_for_page(
// the original's ack would clobber it.
for (id, state) in &upgraded {
if state == "running" {
let _ = jobs::release_unowned(pool, *id).await;
let _ = jobs::release_unowned(&mut *conn, *id).await;
}
}
Ok(EnqueueForPageOutcome::UpgradedToForce)
@@ -168,11 +187,14 @@ pub enum ReenqueueScope {
/// skip-if-done net would no-op them). Pages with a pending/running
/// `analyze_page` job are always skipped so repeated calls don't pile up
/// duplicates. Returns the number of jobs enqueued.
pub async fn enqueue_pages(
pool: &PgPool,
pub async fn enqueue_pages<'e, E>(
executor: E,
scope: ReenqueueScope,
only_unanalyzed: bool,
) -> AppResult<u64> {
) -> AppResult<u64>
where
E: sqlx::PgExecutor<'e>,
{
// Scope predicate; the bound uuid (when present) is always $2.
let scope_clause = match scope {
ReenqueueScope::All => "",
@@ -206,7 +228,7 @@ pub async fn enqueue_pages(
ReenqueueScope::All => query,
ReenqueueScope::Manga(id) | ReenqueueScope::Chapter(id) => query.bind(id),
};
Ok(query.execute(pool).await?.rows_affected())
Ok(query.execute(executor).await?.rows_affected())
}
/// Per-manga analysis coverage for the admin overview. Only mangas that

View File

@@ -110,7 +110,7 @@ pub async fn list_for_user(
});
}
// Newest first; trim to limit after the merge.
entries.sort_by(|a, b| b.created_at().cmp(&a.created_at()));
entries.sort_by_key(|b| std::cmp::Reverse(b.created_at()));
entries.truncate(limit as usize);
let (manga_total, chapter_total): (i64, i64) = sqlx::query_as(

View File

@@ -157,8 +157,10 @@ pub async fn set_is_admin_unchecked(pool: &PgPool, id: Uuid, value: bool) -> App
/// - If a row already exists: flip `is_admin` to true if needed; **never**
/// touch the existing `password_hash`. Lets the operator rotate the
/// admin password through the UI without env-var conflict.
///
/// Wrapped in a transaction so a concurrent `register` for the same
/// username can't slip an INSERT between the SELECT and UPDATE/INSERT.
///
/// Set `is_admin` on a user with full safety checks: rejects self-demote,
/// rejects demoting the only remaining admin (under `ADMIN_INVARIANT_LOCK_KEY`
/// to close the parallel-demote race), and writes an `admin_audit` row

View File

@@ -33,6 +33,28 @@ pub const KEY_CRAWLER: &str = "crawler";
/// `app_settings.key` for the analysis group.
pub const KEY_ANALYSIS: &str = "analysis";
// Upper bounds on numeric settings. These are sanity caps, not tuned limits —
// they keep a fat-fingered (or CSRF-injected) value from spawning thousands of
// workers, demanding gigabyte buffers, or sending out-of-range sampling
// params. Generous enough that no realistic deployment hits them.
const MAX_WORKERS: u64 = 64;
const MAX_CHAPTER_WORKERS: u64 = 64;
const MAX_ANALYSIS_MAX_TOKENS: u32 = 1_000_000;
const MAX_ANALYSIS_SLICES: u64 = 1024;
/// 1 GiB — far above any real page, well below "exhaust the host".
const MAX_ANALYSIS_IMAGE_BYTES: u64 = 1024 * 1024 * 1024;
/// OpenAI-compatible sampling ranges (the analysis endpoint speaks that API).
const MAX_TEMPERATURE: f64 = 2.0;
const FREQUENCY_PENALTY_MIN: f64 = -2.0;
const FREQUENCY_PENALTY_MAX: f64 = 2.0;
/// Max manga-detail fetches per metadata pass. `0` stays special-cased as
/// "unlimited"; this only bounds an explicit positive value.
const MAX_MANGA_LIMIT: u64 = 1_000_000;
/// Upper bound (seconds) shared by every timeout knob — 24h. A timeout
/// longer than a day is almost certainly a fat-fingered value (e.g. ms
/// mistaken for s) and would wedge a worker for far too long.
const MAX_TIMEOUT_SECS: u64 = 86_400;
/// One field-level validation failure, surfaced to the UI per input.
#[derive(Debug, Clone, Serialize, PartialEq, Eq)]
pub struct FieldError {
@@ -151,6 +173,8 @@ impl CrawlerSettings {
};
if self.chapter_workers < 1 {
errs.push("chapter_workers", "must be at least 1");
} else if self.chapter_workers > MAX_CHAPTER_WORKERS {
errs.push("chapter_workers", format!("must be at most {MAX_CHAPTER_WORKERS}"));
}
if let Some(url) = self.start_url.as_deref().map(str::trim).filter(|s| !s.is_empty()) {
// SSRF defence: Url::parse alone admits http://169.254.169.254
@@ -164,6 +188,16 @@ impl CrawlerSettings {
}
if self.job_timeout_secs < 1 {
errs.push("job_timeout_secs", "must be at least 1 second");
} else if self.job_timeout_secs > MAX_TIMEOUT_SECS {
errs.push("job_timeout_secs", format!("must be at most {MAX_TIMEOUT_SECS} seconds"));
}
if self.idle_timeout_secs > MAX_TIMEOUT_SECS {
errs.push("idle_timeout_secs", format!("must be at most {MAX_TIMEOUT_SECS} seconds"));
}
// 0 is intentionally "unlimited"; only an explicit positive value is
// capped.
if self.manga_limit > MAX_MANGA_LIMIT {
errs.push("manga_limit", format!("must be at most {MAX_MANGA_LIMIT}"));
}
if !errs.is_empty() {
@@ -354,6 +388,8 @@ impl AnalysisSettings {
if self.workers < 1 {
errs.push("workers", "must be at least 1");
} else if self.workers > MAX_WORKERS {
errs.push("workers", format!("must be at most {MAX_WORKERS}"));
}
// The endpoint/model are vision-only knobs — the OCR backend never
// dials a URL or sends a model id. While vision is dormant
@@ -393,18 +429,35 @@ impl AnalysisSettings {
}
if self.max_tokens < 1 {
errs.push("max_tokens", "must be at least 1");
} else if self.max_tokens > MAX_ANALYSIS_MAX_TOKENS {
errs.push("max_tokens", format!("must be at most {MAX_ANALYSIS_MAX_TOKENS}"));
}
if self.request_timeout_secs < 1 {
errs.push("request_timeout_secs", "must be at least 1 second");
} else if self.request_timeout_secs > MAX_TIMEOUT_SECS {
errs.push(
"request_timeout_secs",
format!("must be at most {MAX_TIMEOUT_SECS} seconds"),
);
}
if self.job_timeout_secs < 1 {
errs.push("job_timeout_secs", "must be at least 1 second");
} else if self.job_timeout_secs > MAX_TIMEOUT_SECS {
errs.push(
"job_timeout_secs",
format!("must be at most {MAX_TIMEOUT_SECS} seconds"),
);
}
if self.max_pixels < 1 {
errs.push("max_pixels", "must be at least 1");
}
if self.max_image_bytes < 1 {
errs.push("max_image_bytes", "must be greater than 0");
} else if self.max_image_bytes > MAX_ANALYSIS_IMAGE_BYTES {
errs.push(
"max_image_bytes",
format!("must be at most {MAX_ANALYSIS_IMAGE_BYTES} (1 GiB)"),
);
}
if !(0.0..=0.9).contains(&self.slice_overlap) {
errs.push("slice_overlap", "must be between 0.0 and 0.9");
@@ -417,9 +470,17 @@ impl AnalysisSettings {
}
if self.max_slices < 1 {
errs.push("max_slices", "must be at least 1");
} else if self.max_slices > MAX_ANALYSIS_SLICES {
errs.push("max_slices", format!("must be at most {MAX_ANALYSIS_SLICES}"));
}
if self.temperature < 0.0 {
errs.push("temperature", "must be 0 or greater");
if !(0.0..=MAX_TEMPERATURE).contains(&self.temperature) {
errs.push("temperature", format!("must be between 0 and {MAX_TEMPERATURE}"));
}
if !(FREQUENCY_PENALTY_MIN..=FREQUENCY_PENALTY_MAX).contains(&self.frequency_penalty) {
errs.push(
"frequency_penalty",
format!("must be between {FREQUENCY_PENALTY_MIN} and {FREQUENCY_PENALTY_MAX}"),
);
}
let response_format = match ResponseFormat::parse_strict(&self.response_format) {
Some(rf) => rf,
@@ -473,6 +534,8 @@ impl AnalysisSettings {
backend: base.backend,
ocr_detection_model: base.ocr_detection_model.clone(),
ocr_recognition_model: base.ocr_recognition_model.clone(),
// Env-only decompression-bomb decode cap, carried from the base.
ocr_max_decode_pixels: base.ocr_max_decode_pixels,
})
}
}
@@ -504,11 +567,13 @@ mod tests {
#[test]
fn crawler_round_trips_through_dto() {
let mut base = CrawlerConfig::default();
base.start_url = Some("https://example.com/".to_string());
base.tz = Tz::Europe__Berlin;
base.chapter_workers = 3;
base.cookie_domain = Some("example.com".to_string());
let base = CrawlerConfig {
start_url: Some("https://example.com/".to_string()),
tz: Tz::Europe__Berlin,
chapter_workers: 3,
cookie_domain: Some("example.com".to_string()),
..Default::default()
};
let dto = CrawlerSettings::from_config(&base);
let back = dto.to_config(&base).expect("valid");
assert_eq!(back.tz, Tz::Europe__Berlin);
@@ -534,10 +599,12 @@ mod tests {
#[test]
fn crawler_overlay_preserves_env_only_fields() {
let mut base = CrawlerConfig::default();
base.proxy = Some("socks5://127.0.0.1:9050".to_string());
base.tor_control_password = Some("secret".to_string());
base.phpsessid = Some("abc123".to_string());
let base = CrawlerConfig {
proxy: Some("socks5://127.0.0.1:9050".to_string()),
tor_control_password: Some("secret".to_string()),
phpsessid: Some("abc123".to_string()),
..Default::default()
};
// A DTO that knows nothing about the env-only fields.
let dto = CrawlerSettings {
rate_ms: 2000,
@@ -641,8 +708,10 @@ mod tests {
#[test]
fn analysis_captures_env_prompt_override() {
let mut base = AnalysisConfig::default();
base.system_prompt = "custom env prompt".to_string();
let base = AnalysisConfig {
system_prompt: "custom env prompt".to_string(),
..Default::default()
};
let dto = AnalysisSettings::from_config(&base);
assert_eq!(dto.system_prompt.as_deref(), Some("custom env prompt"));
}
@@ -666,8 +735,10 @@ mod tests {
#[test]
fn analysis_overlay_preserves_api_key() {
let mut base = AnalysisConfig::default();
base.api_key = Some("sk-secret".to_string());
let base = AnalysisConfig {
api_key: Some("sk-secret".to_string()),
..Default::default()
};
let dto = AnalysisSettings::from_config(&base);
assert_eq!(dto.to_config(&base).unwrap().api_key.as_deref(), Some("sk-secret"));
}
@@ -704,14 +775,115 @@ mod tests {
}
}
#[test]
fn analysis_rejects_over_upper_bounds() {
// Sanity caps: an absurdly large worker count / buffer / token budget
// and out-of-range sampling params are all refused so a fat-fingered
// or CSRF-injected value can't exhaust the host or break the upstream
// API contract.
let base = AnalysisConfig::default();
let dto = AnalysisSettings {
workers: MAX_WORKERS + 1,
max_tokens: MAX_ANALYSIS_MAX_TOKENS + 1,
max_slices: MAX_ANALYSIS_SLICES + 1,
max_image_bytes: MAX_ANALYSIS_IMAGE_BYTES + 1,
temperature: MAX_TEMPERATURE + 0.5,
frequency_penalty: FREQUENCY_PENALTY_MAX + 0.5,
request_timeout_secs: MAX_TIMEOUT_SECS + 1,
job_timeout_secs: MAX_TIMEOUT_SECS + 1,
..AnalysisSettings::from_config(&base)
};
let errs = dto.to_config(&base).unwrap_err();
let fields: Vec<_> = errs.errors.iter().map(|e| e.field.as_str()).collect();
for f in [
"workers",
"max_tokens",
"max_slices",
"max_image_bytes",
"temperature",
"frequency_penalty",
"request_timeout_secs",
"job_timeout_secs",
] {
assert!(fields.contains(&f), "missing upper-bound error for {f}");
}
}
#[test]
fn crawler_rejects_over_upper_bounds() {
// manga_limit ceiling + timeout caps. manga_limit=0 stays "unlimited"
// and is asserted valid by the round-trip tests above.
let base = CrawlerConfig::default();
let dto = CrawlerSettings {
manga_limit: MAX_MANGA_LIMIT + 1,
job_timeout_secs: MAX_TIMEOUT_SECS + 1,
idle_timeout_secs: MAX_TIMEOUT_SECS + 1,
..CrawlerSettings::from_config(&base)
};
let errs = dto.to_config(&base).unwrap_err();
let fields: Vec<_> = errs.errors.iter().map(|e| e.field.as_str()).collect();
for f in ["manga_limit", "job_timeout_secs", "idle_timeout_secs"] {
assert!(fields.contains(&f), "missing upper-bound error for {f}");
}
}
#[test]
fn crawler_allows_unlimited_manga_limit() {
// 0 means "no cap" and must stay valid.
let base = CrawlerConfig::default();
let dto = CrawlerSettings {
manga_limit: 0,
..CrawlerSettings::from_config(&base)
};
assert!(dto.to_config(&base).is_ok());
}
#[test]
fn analysis_rejects_negative_frequency_penalty_below_range() {
let base = AnalysisConfig::default();
let dto = AnalysisSettings {
frequency_penalty: FREQUENCY_PENALTY_MIN - 0.5,
..AnalysisSettings::from_config(&base)
};
let errs = dto.to_config(&base).unwrap_err();
let fields: Vec<_> = errs.errors.iter().map(|e| e.field.as_str()).collect();
assert!(fields.contains(&"frequency_penalty"));
}
#[test]
fn analysis_accepts_in_range_sampling_params() {
// A normal config with mid-range sampling values stays valid.
let base = AnalysisConfig::default();
let dto = AnalysisSettings {
temperature: 0.7,
frequency_penalty: 0.3,
..AnalysisSettings::from_config(&base)
};
assert!(dto.to_config(&base).is_ok());
}
#[test]
fn crawler_rejects_over_chapter_worker_cap() {
let base = CrawlerConfig::default();
let dto = CrawlerSettings {
chapter_workers: MAX_CHAPTER_WORKERS + 1,
..CrawlerSettings::from_config(&base)
};
let errs = dto.to_config(&base).unwrap_err();
let fields: Vec<_> = errs.errors.iter().map(|e| e.field.as_str()).collect();
assert!(fields.contains(&"chapter_workers"));
}
#[test]
fn analysis_endpoint_rejects_ip_literal_attacks_when_enabled() {
// The vision worker bearer-attaches an env secret to every call;
// a hostile/CSRF-able admin must NOT be able to point endpoint at
// cloud metadata, loopback services, or RFC1918 hosts — when the
// worker is enabled (toggling enabled=true later re-runs this gate).
let mut base = AnalysisConfig::default();
base.backend = AnalysisBackend::Vision;
let base = AnalysisConfig {
backend: AnalysisBackend::Vision,
..Default::default()
};
for url in [
"http://169.254.169.254/v1/chat/completions",
"http://127.0.0.1:5432/",
@@ -735,8 +907,10 @@ mod tests {
// The documented default — docker DNS name resolving to a private IP
// at runtime — must still validate, because the bearer recipient
// identity is the operator-chosen hostname, not the underlying IP.
let mut base = AnalysisConfig::default();
base.backend = AnalysisBackend::Vision;
let base = AnalysisConfig {
backend: AnalysisBackend::Vision,
..Default::default()
};
for url in [
"http://mangalord-vision:8000/v1/chat/completions",
"https://api.openai.com/v1/chat/completions",
@@ -769,8 +943,10 @@ mod tests {
fn analysis_requires_model_only_when_enabled() {
// Vision base: the model id is required only when the vision worker
// is actually live (see the OCR carve-out below).
let mut base = AnalysisConfig::default();
base.backend = AnalysisBackend::Vision;
let base = AnalysisConfig {
backend: AnalysisBackend::Vision,
..Default::default()
};
let disabled = AnalysisSettings {
enabled: false,
model: "".to_string(),

View File

@@ -56,6 +56,7 @@ fn ocr_dispatcher(
storage,
engine: StubOcrEngine::new(lines),
max_image_bytes: 8 * 1024 * 1024,
ocr_permits: Arc::new(tokio::sync::Semaphore::new(1)),
}
}

View File

@@ -74,6 +74,38 @@ async fn analyze_job_count(pool: &PgPool) -> i64 {
.unwrap()
}
#[sqlx::test(migrations = "./migrations")]
async fn force_analyze_enqueue_rolls_back_with_its_transaction(pool: PgPool) {
// The admin force-reanalyze handler runs enqueue_for_page_conn + the audit
// insert in one transaction. Prove the enqueue genuinely participates in
// that tx: when the tx is abandoned (the path taken if the audit insert
// fails and the `?` propagates), no job survives — so the audit trail can
// never miss a force-reanalyze that actually landed.
let page_id = seed_pages(&pool, 1).await[0];
let mut tx = pool.begin().await.unwrap();
let outcome =
mangalord::repo::page_analysis::enqueue_for_page_conn(&mut tx, page_id, true)
.await
.unwrap();
assert!(matches!(
outcome,
mangalord::repo::page_analysis::EnqueueForPageOutcome::Inserted
));
// Visible inside the open transaction…
let in_tx: i64 = sqlx::query_scalar(
"SELECT COUNT(*) FROM crawler_jobs WHERE payload->>'kind' = 'analyze_page'",
)
.fetch_one(&mut *tx)
.await
.unwrap();
assert_eq!(in_tx, 1);
// …but gone once the tx rolls back instead of committing.
tx.rollback().await.unwrap();
assert_eq!(analyze_job_count(&pool).await, 0);
}
#[sqlx::test(migrations = "./migrations")]
async fn chapter_upload_enqueues_one_analysis_job_per_page(pool: PgPool) {
let h = common::harness_with_analysis(pool.clone());
@@ -194,6 +226,38 @@ async fn reenqueue_backfills_existing_pages(pool: PgPool) {
assert_eq!(analyze_job_count(&pool).await, 3);
}
#[sqlx::test(migrations = "./migrations")]
async fn reenqueue_writes_audit_row_atomically_with_jobs(pool: PgPool) {
// The enqueue and its admin_audit row are committed in one transaction,
// so a successful re-enqueue always leaves both the jobs AND exactly one
// matching audit row — the audit trail can't miss an enqueue that landed.
let h = common::harness_with_analysis(pool.clone());
let cookie = seed_admin(&pool, &h.app).await;
seed_pages(&pool, 2).await;
let resp = h
.app
.clone()
.oneshot(common::post_json_with_cookie(
"/api/v1/admin/analysis/reenqueue",
json!({ "only_unanalyzed": true }),
&cookie,
))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
assert_eq!(analyze_job_count(&pool).await, 2);
let audit: serde_json::Value = sqlx::query_scalar(
"SELECT payload FROM admin_audit WHERE action = 'analysis_reenqueue'",
)
.fetch_one(&pool)
.await
.expect("exactly one analysis_reenqueue audit row committed with the jobs");
assert_eq!(audit["enqueued"], 2);
assert_eq!(audit["only_unanalyzed"], true);
}
#[sqlx::test(migrations = "./migrations")]
async fn reenqueue_scoped_to_manga(pool: PgPool) {
let h = common::harness_with_analysis(pool.clone());

View File

@@ -279,7 +279,7 @@ async fn backfill_fills_unmeasured_pages_and_covers_idempotently(pool: PgPool) {
let body2 = common::body_json(resp2).await;
assert_eq!(body2["pages"].as_i64().unwrap(), 0);
assert_eq!(body2["covers"].as_i64().unwrap(), 0);
assert_eq!(body2["more_remaining"].as_bool().unwrap(), false);
assert!(!body2["more_remaining"].as_bool().unwrap());
}
#[sqlx::test(migrations = "./migrations")]
@@ -362,7 +362,7 @@ async fn backfill_caps_per_run_and_reports_more_remaining(pool: PgPool) {
assert_eq!(resp.status(), StatusCode::OK);
let body = common::body_json(resp).await;
// Capped at 20_000 attempts this run; one row left → run again.
assert_eq!(body["more_remaining"].as_bool().unwrap(), true);
assert!(body["more_remaining"].as_bool().unwrap());
assert_eq!(body["missing"].as_i64().unwrap(), 20_000);
assert_eq!(body["pages"].as_i64().unwrap(), 0);
}
@@ -403,9 +403,8 @@ async fn backfill_at_exact_cap_is_not_more_remaining(pool: PgPool) {
.unwrap();
let body = common::body_json(resp).await;
assert_eq!(body["missing"].as_i64().unwrap(), 20_000);
assert_eq!(
body["more_remaining"].as_bool().unwrap(),
false,
assert!(
!body["more_remaining"].as_bool().unwrap(),
"exactly-cap backlog drains in one run; no follow-up needed"
);
}

View File

@@ -381,10 +381,11 @@ async fn delete_writes_audit_row(pool: PgPool) {
.unwrap();
assert_eq!(resp.status(), StatusCode::NO_CONTENT);
let rows: Vec<(Option<Uuid>, String, String, Option<Uuid>, serde_json::Value)> =
sqlx::query_as(
"SELECT actor_user_id, action, target_kind, target_id, payload FROM admin_audit",
)
// (actor_user_id, action, target_kind, target_id, payload)
type AuditRow = (Option<Uuid>, String, String, Option<Uuid>, serde_json::Value);
let rows: Vec<AuditRow> = sqlx::query_as(
"SELECT actor_user_id, action, target_kind, target_id, payload FROM admin_audit",
)
.fetch_all(&pool)
.await
.unwrap();

View File

@@ -170,6 +170,26 @@ async fn login_rejects_wrong_password(pool: PgPool) {
assert_eq!(body["error"]["code"], "unauthenticated");
}
#[sqlx::test(migrations = "./migrations")]
async fn login_rejects_oversized_password_before_argon2(pool: PgPool) {
// A multi-KB password on the login path must be rejected as malformed
// input (400) rather than fed to argon2 — otherwise every attempt is a
// CPU-DoS. The account need not even exist; the guard is input-shape only.
let h = common::harness(pool);
let giant = "a".repeat(5000);
let resp = h
.app
.oneshot(common::post_json(
"/api/v1/auth/login",
json!({ "username": "alice", "password": giant }),
))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
let body = common::body_json(resp).await;
assert_eq!(body["error"]["code"], "invalid_input");
}
#[sqlx::test(migrations = "./migrations")]
async fn login_rejects_unknown_user(pool: PgPool) {
let h = common::harness(pool);
@@ -521,6 +541,100 @@ async fn create_and_use_bot_token(pool: PgPool) {
assert_eq!(resp.status(), StatusCode::OK);
}
#[sqlx::test(migrations = "./migrations")]
async fn bot_token_with_future_expiry_authenticates(pool: PgPool) {
// A token minted with expires_in_days is still active before its
// expiry, and the response echoes a non-null expires_at.
let h = common::harness(pool);
let (_, cookie) = common::register_user(&h.app).await;
let resp = h
.app
.clone()
.oneshot(common::post_json_with_cookie(
"/api/v1/auth/tokens",
json!({ "name": "ci-bot", "expires_in_days": 30 }),
&cookie,
))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::CREATED);
let body = common::body_json(resp).await;
assert!(
body["expires_at"].is_string(),
"expires_at should be set, got {}",
body["expires_at"]
);
let bearer = body["bearer"].as_str().unwrap().to_string();
let resp = h
.app
.oneshot(common::get_with_bearer("/api/v1/auth/me", &bearer))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
}
#[sqlx::test(migrations = "./migrations")]
async fn expired_bot_token_is_rejected(pool: PgPool) {
use chrono::{Duration, Utc};
use mangalord::auth::token::generate_token;
let h = common::harness(pool.clone());
common::register_user(&h.app).await;
let user_id: uuid::Uuid = sqlx::query_scalar("SELECT id FROM users LIMIT 1")
.fetch_one(&pool)
.await
.unwrap();
// Hand-craft a token that expired an hour ago.
let (raw, hash) = generate_token();
let expires_at = Utc::now() - Duration::hours(1);
sqlx::query(
"INSERT INTO api_tokens (user_id, name, token_hash, expires_at) \
VALUES ($1, 'stale', $2, $3)",
)
.bind(user_id)
.bind(&hash[..])
.bind(expires_at)
.execute(&pool)
.await
.unwrap();
let resp = h
.app
.oneshot(common::get_with_bearer("/api/v1/auth/me", &raw))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);
let body = common::body_json(resp).await;
assert_eq!(body["error"]["code"], "unauthenticated");
}
#[sqlx::test(migrations = "./migrations")]
async fn create_token_rejects_out_of_range_expiry(pool: PgPool) {
let h = common::harness(pool);
let (_, cookie) = common::register_user(&h.app).await;
for days in [0, -5, 100_000] {
let resp = h
.app
.clone()
.oneshot(common::post_json_with_cookie(
"/api/v1/auth/tokens",
json!({ "name": "bad", "expires_in_days": days }),
&cookie,
))
.await
.unwrap();
assert_eq!(
resp.status(),
StatusCode::UNPROCESSABLE_ENTITY,
"expires_in_days={days} should be rejected"
);
}
}
#[sqlx::test(migrations = "./migrations")]
async fn user_a_cannot_delete_user_b_token(pool: PgPool) {
let h = common::harness(pool);

View File

@@ -8,6 +8,8 @@ use uuid::Uuid;
#[allow(unused_imports)]
use serde_json as _;
use common::MultipartBuilder;
async fn seed_manga(h: &common::Harness, cookie: &str, title: &str) -> Uuid {
common::seed_manga_via_api(&h.app, cookie, title).await
}
@@ -272,3 +274,44 @@ async fn list_pages_returns_404_for_unknown_chapter(pool: PgPool) {
.unwrap();
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
}
#[sqlx::test(migrations = "./migrations")]
async fn non_owner_can_upload_chapter(pool: PgPool) {
// Contract lock: chapter upload is INTENTIONALLY open to any
// authenticated user, not just the manga's creator. A user who did not
// create the manga can still contribute a chapter (community
// contributions), unlike manga-record edits which gate on ownership.
// If this ever needs to become owner-only, that's a deliberate change —
// this test (and the doc comment on `api::chapters::create`) should be
// updated together, not silently broken.
let h = common::harness(pool);
// Owner creates the manga.
let (_owner, owner_cookie) = common::register_user(&h.app).await;
let manga_id = seed_manga(&h, &owner_cookie, "Berserk").await;
// A different, non-owner user uploads a chapter to it.
let (_other, other_cookie) = common::register_user(&h.app).await;
let resp = h
.app
.clone()
.oneshot(common::post_multipart_with_cookie(
&format!("/api/v1/mangas/{manga_id}/chapters"),
MultipartBuilder::new()
.add_json("metadata", json!({ "number": 1, "title": "Contributed" }))
.add_file("page", "1.png", "image/png", &common::fake_png_bytes()),
&other_cookie,
))
.await
.unwrap();
assert_eq!(
resp.status(),
StatusCode::CREATED,
"a non-owner authenticated user must be able to upload a chapter"
);
let body = common::body_json(resp).await;
assert_eq!(body["number"], 1);
assert_eq!(body["title"], "Contributed");
assert_eq!(body["page_count"], 1);
}

View File

@@ -446,7 +446,7 @@ async fn list_tie_break_orders_equal_keys_by_ascending_id(pool: PgPool) {
// Paginating one row at a time reproduces that exact sequence — no overlap,
// no gap — which only holds because the id tie-break makes the order total.
let paged: Vec<String> = vec![page(1, 0).await, page(1, 1).await, page(1, 2).await]
let paged: Vec<String> = [page(1, 0).await, page(1, 1).await, page(1, 2).await]
.iter()
.map(|b| b["items"][0]["title"].as_str().unwrap().to_string())
.collect();

View File

@@ -279,7 +279,7 @@ async fn tag_autocomplete_returns_matches_ordered_by_similarity(pool: PgPool) {
.iter()
.map(|t| t["name"].as_str().unwrap())
.collect();
assert!(names.iter().any(|n| *n == "Mystery"));
assert!(names.iter().any(|n| *n == "Murder Mystery"));
assert!(!names.iter().any(|n| *n == "Comedy"));
assert!(names.contains(&"Mystery"));
assert!(names.contains(&"Murder Mystery"));
assert!(!names.contains(&"Comedy"));
}

View File

@@ -167,6 +167,7 @@ services:
ANALYSIS_TALL_ASPECT: ${ANALYSIS_TALL_ASPECT:-1.6}
ANALYSIS_MAX_SLICES: ${ANALYSIS_MAX_SLICES:-16}
ANALYSIS_MAX_IMAGE_BYTES: ${ANALYSIS_MAX_IMAGE_BYTES:-8388608}
ANALYSIS_OCR_MAX_DECODE_PIXELS: ${ANALYSIS_OCR_MAX_DECODE_PIXELS:-100000000}
ANALYSIS_RESPONSE_FORMAT: ${ANALYSIS_RESPONSE_FORMAT:-json_schema}
ANALYSIS_FREQUENCY_PENALTY: ${ANALYSIS_FREQUENCY_PENALTY:-0.3}
ANALYSIS_TEMPERATURE: ${ANALYSIS_TEMPERATURE:-0.0}

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// E2E for the admin Analysis section: coverage overview + badges, drill
// manga → chapter → page, the page-detail modal, and the enqueue actions.
@@ -196,6 +196,21 @@ async function mockAdmin(page: Page, cap: Captured) {
})
})
);
// The metrics tab also pulls a per-bucket time series for the trend
// charts. Registered after the aggregate `metrics**` route so it wins
// for the more specific `/metrics/series` path.
await page.route('**/api/v1/admin/analysis/metrics/series**', (r) =>
r.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({
buckets: [
{ t: '2026-06-12T00:00:00Z', n: 1600, ok: 1580, failed: 20, avg_ms: 2300 },
{ t: '2026-06-13T00:00:00Z', n: 1604, ok: 1579, failed: 25, avg_ms: 2500 }
]
})
})
);
// Default: keep the SSE connection pending (no events) so tests that
// don't care about live updates don't trigger reconnect churn. The
@@ -264,11 +279,12 @@ test.describe('/admin/analysis', () => {
await expect(
page.getByTestId('admin-analysis-detail-status')
).toContainText('Analyzed');
await expect(modal).toContainText('model test-model');
// OCR-only backend: the detail surfaces the extracted OCR lines (with
// their kind) — tags / scene / NSFW belong to the dormant vision
// backend and are intentionally not shown here.
await expect(modal).toContainText('OCR text');
await expect(modal).toContainText('Hello there');
await expect(modal).toContainText('action');
await expect(page.getByTestId('admin-analysis-detail-warnings')).toContainText(
'gore'
);
});
test('live SSE events drive the indicator and activity ticker', async ({
@@ -367,8 +383,9 @@ test.describe('/admin/analysis', () => {
await page.getByTestId('admin-analysis-tab-history').click();
const row = page.getByTestId(`analysis-history-row-${pageDone}`);
await expect(row).toContainText('Berserk · Ch 1 · p1');
await expect(row).toContainText('NSFW');
await expect(row).toContainText('test-model'); // model column
await expect(row).toContainText('2.4s'); // duration column
// (No NSFW flag: the active OCR backend extracts text only.)
await row.click();
await expect(page.getByTestId('admin-analysis-detail')).toBeVisible();
@@ -377,16 +394,22 @@ test.describe('/admin/analysis', () => {
);
});
test('metrics tab shows aggregate timing + by-model', async ({ page }) => {
test('metrics tab shows aggregate timing + trend charts', async ({ page }) => {
const cap: Captured = { reenqueue: null, analyzeCalls: 0 };
await mockAdmin(page, cap);
await page.setViewportSize(DESKTOP);
await page.goto('/admin/analysis');
await page.getByTestId('admin-analysis-tab-metrics').click();
// Aggregate tiles for the selected window.
await expect(page.getByTestId('analysis-metrics-n')).toContainText('3204');
await expect(page.getByTestId('analysis-metrics-avg')).toContainText('2.4s');
await expect(page.getByTestId('analysis-metrics')).toContainText('qwen2-vl-7b');
const metrics = page.getByTestId('analysis-metrics');
await expect(metrics).toContainText('Success');
await expect(metrics).toContainText('99%'); // 3159/3204
await expect(metrics).toContainText('45'); // failed
// The per-bucket trend charts render from the series endpoint.
await expect(page.getByTestId('analysis-metrics-charts')).toBeVisible();
});
test('queue an unanalyzed page from its detail modal', async ({ page }) => {

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// E2E for the admin Crawler "History" tab: the Live/History toggle, the
// searchable/filterable job log, and inline requeue of a dead job. The

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Mocks the auth endpoints at the network level so the journey is
// deterministic and doesn't require a live backend.
@@ -81,6 +81,52 @@ test('login then logout flips the layout between authenticated and anonymous', a
await expect(page.getByTestId('nav-login')).toBeVisible();
});
test('login redirects to a safe ?next= target after authenticating', async ({ page }) => {
await stubAnonymousThenAuthenticated(page);
// Arrive at /login carrying a same-origin ?next= (the shape the layout
// produces when bouncing an unauthenticated user off a gated page).
await page.goto('/login?next=%2Fsearch');
await expect(page.getByTestId('nav-login')).toBeVisible();
await page.getByTestId('login-username').fill('alice');
await page.getByTestId('login-password').fill('hunter2hunter2');
await page.getByTestId('login-submit').click();
// Lands on the requested page, not the default root.
await expect(page).toHaveURL(/\/search$/);
});
test('login ignores an unsafe ?next= and falls back to /', async ({ page }) => {
await stubAnonymousThenAuthenticated(page);
// A protocol-relative open-redirect attempt must be discarded.
await page.goto('/login?next=%2F%2Fevil.com');
await expect(page.getByTestId('nav-login')).toBeVisible();
await page.getByTestId('login-username').fill('alice');
await page.getByTestId('login-password').fill('hunter2hunter2');
await page.getByTestId('login-submit').click();
// Authenticated and on the site root — never navigated off-origin.
await expect(page).toHaveURL(/\/$/);
await expect(page.getByTestId('session-user')).toContainText('alice');
});
test('login rejects the %09 control-char open-redirect bypass', async ({ page }) => {
await stubAnonymousThenAuthenticated(page);
// "/\t/evil.com" passes a naive startsWith('/') && !startsWith('//')
// guard but collapses to "//evil.com" once the browser strips the tab.
await page.goto('/login?next=%2F%09%2Fevil.com');
await expect(page.getByTestId('nav-login')).toBeVisible();
await page.getByTestId('login-username').fill('alice');
await page.getByTestId('login-password').fill('hunter2hunter2');
await page.getByTestId('login-submit').click();
// Stayed on-origin at the root, authenticated.
await expect(page).toHaveURL(/\/$/);
await expect(page.getByTestId('session-user')).toContainText('alice');
});
test('login surfaces the API error message on bad credentials', async ({ page }) => {
await page.route('**/api/v1/auth/me', async (route) => {
await route.fulfill({

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Regression spec for the reader-back-loop bug: previously the reader's
// back arrow was a plain `<a href="/manga/{id}">`, which PUSHED a new
@@ -45,7 +45,11 @@ async function mockApis(page: Page) {
await page.route('**/api/v1/me/bookmarks*', (r) =>
r.fulfill({ status: 401, contentType: 'application/json', body: '{}' })
);
await page.route('**/api/v1/me/read-progress*', (r) =>
// `**` (not `*`) so this also matches the per-manga path
// `/me/read-progress/{id}` — Playwright's `*` stops at `/`, so a
// single-star glob would let that request fall through to the (dead)
// backend proxy and 500 the reader/detail load.
await page.route('**/api/v1/me/read-progress**', (r) =>
r.fulfill({ status: 404, contentType: 'application/json', body: '{}' })
);
await page.route('**/api/v1/genres*', (r) =>

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
const mangaId = '22222222-2222-2222-2222-222222222222';
const userFixture = {
@@ -6,14 +6,23 @@ const userFixture = {
username: 'alice',
created_at: '2026-01-01T00:00:00Z'
};
// Faithful `MangaDetail` (GET /v1/mangas/:id). The detail page reads
// authors/genres/tags/alt_titles/content_warnings during render, so these
// must be present or the component throws before painting.
const mangaFixture = {
id: mangaId,
title: 'Berserk',
author: 'Kentaro Miura',
status: 'ongoing',
alt_titles: [],
description: null,
cover_image_path: null,
created_at: '2026-01-01T00:00:00Z',
updated_at: '2026-01-01T00:00:00Z'
updated_at: '2026-01-01T00:00:00Z',
authors: [{ id: 'a2222222-2222-2222-2222-222222222222', name: 'Kentaro Miura' }],
genres: [],
tags: [],
content_warnings: [],
chapter_storage_bytes: 0
};
const bookmarkFixture = {
id: 'b1',
@@ -74,6 +83,22 @@ async function setupAuthenticatedBookmarkFlow(page: Page) {
})
})
);
// Authed but no saved position (404 → null) + empty recommendations —
// both fetched by the manga-detail load's Promise.all.
await page.route(`**/api/v1/me/read-progress/${mangaId}`, (route) =>
route.fulfill({
status: 404,
contentType: 'application/json',
body: JSON.stringify({ error: { code: 'not_found', message: 'no progress' } })
})
);
await page.route(`**/api/v1/mangas/${mangaId}/similar`, (route) =>
route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ items: [] })
})
);
await page.route('**/api/v1/bookmarks', (route) => {
if (route.request().method() === 'POST') {
// List endpoint is enriched (BookmarkSummary), POST returns
@@ -163,6 +188,20 @@ test('anonymous user sees a sign-in CTA instead of a toggle', async ({ page }) =
body: JSON.stringify({ error: { code: 'unauthenticated', message: 'unauthenticated' } })
})
);
await page.route(`**/api/v1/me/read-progress/${mangaId}`, (route) =>
route.fulfill({
status: 401,
contentType: 'application/json',
body: JSON.stringify({ error: { code: 'unauthenticated', message: 'unauthenticated' } })
})
);
await page.route(`**/api/v1/mangas/${mangaId}/similar`, (route) =>
route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ items: [] })
})
);
await page.goto(`/manga/${mangaId}`);
await expect(page.getByTestId('bookmark-signin')).toBeVisible();

43
frontend/e2e/fixtures.ts Normal file
View File

@@ -0,0 +1,43 @@
import { test as base, expect } from '@playwright/test';
export { expect };
export type { Page } from '@playwright/test';
/**
* Shared Playwright `test` for the E2E suite.
*
* Adds an auto-use fallback for the `/api` surface: any request a spec
* forgot to mock is fulfilled here with a fast 503 instead of falling
* through to the SvelteKit dev proxy. That proxy targets a backend that
* isn't running under E2E (`BACKEND_URL` in `.env`), so an unmocked call
* otherwise incurs a proxy round-trip + Vite error logging — which, under
* 8 parallel workers at cold start, piles up and flakes the first tests.
*
* Because this route is registered during fixture setup (before the test
* body runs), any `page.route(...)` a spec registers later takes
* precedence — only genuinely-unmocked calls land here. The 503 also
* surfaces mock gaps loudly (a clear warning + a fast, attributable
* failure) instead of a 30s "element never appeared" timeout.
*/
export const test = base.extend<{ apiFallback: void }>({
apiFallback: [
async ({ page }, use) => {
// Scope to `/api/v1/**` (the real backend surface). A broader
// `**/api/**` would also swallow Vite's own dev module requests
// under `/src/lib/api/*.ts` and break the app bundle.
await page.route('**/api/v1/**', (route) => {
const { pathname } = new URL(route.request().url());
console.warn(`[e2e] unmocked API call: ${route.request().method()} ${pathname}`);
return route.fulfill({
status: 503,
contentType: 'application/json',
body: JSON.stringify({
error: { code: 'e2e_unmocked', message: `unmocked in test: ${pathname}` }
})
});
});
await use();
},
{ auto: true }
]
});

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
const userFixture = {
id: 'u1',

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
import { SORT_FIELD_LABELS } from '../src/lib/mangaSort';
// These E2E tests run against the SvelteKit dev server, which proxies /api
@@ -27,6 +27,24 @@ async function mockAnonymous(page: Page) {
body: JSON.stringify({ error: { code: 'unauthenticated', message: 'unauthenticated' } })
});
});
await page.route('**/api/v1/auth/me/preferences', async (route) => {
await route.fulfill({
status: 401,
contentType: 'application/json',
body: JSON.stringify({ error: { code: 'unauthenticated', message: 'unauthenticated' } })
});
});
// The home page fetches genres on mount (filter UI). Mocking it keeps
// `onMount` fast and deterministic — otherwise the unmocked call stalls
// against the dead dev proxy, and the late-resolving initial load races
// (and clobbers) a load kicked off by an early search interaction.
await page.route('**/api/v1/genres*', async (route) => {
await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify([])
});
});
}
test('home page renders the Mangalord heading and search input', async ({ page }) => {
@@ -86,11 +104,16 @@ test('search updates the manga list', async ({ page }) => {
{
id: 'b1',
title: 'Berserk',
author: 'Kentaro Miura',
status: 'ongoing',
alt_titles: [],
description: null,
cover_image_path: null,
created_at: '2026-01-01T00:00:00Z',
updated_at: '2026-01-01T00:00:00Z'
updated_at: '2026-01-01T00:00:00Z',
// MangaCard = Manga & { authors, genres }; the card
// component maps over both, so they must be present.
authors: [{ id: 'a1', name: 'Kentaro Miura' }],
genres: []
}
]
: [];
@@ -102,6 +125,11 @@ test('search updates the manga list', async ({ page }) => {
});
await page.goto('/');
// Wait for the initial (empty) catalogue to settle before searching, so
// the search's fetch can't race the mount-time load — mirrors how the
// pagination test waits for the first render before interacting.
await expect(page.getByTestId('empty')).toBeVisible();
await page.getByTestId('search-input').fill('berserk');
await page.getByRole('button', { name: 'Search' }).click();

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Phase 5: Account becomes an inset-grouped hub on mobile (Profile /
// Preferences / Change password + red Log out at the bottom) and
@@ -88,6 +88,24 @@ async function mockLibraryData(page: Page, opts: { authed?: boolean } = {}) {
body: opts.authed ? emptyPage : unauth
})
);
// The /library load also pulls page-tags + distinct page-tags in the same
// Promise.all; leaving them unmocked 500s the whole load (caught as an
// error state, not the empty sub-tabs). General route first, the more
// specific `/distinct` after so it takes precedence for that path.
await page.route('**/api/v1/me/page-tags?*', (route) =>
route.fulfill({
status,
contentType: 'application/json',
body: opts.authed ? emptyPage : unauth
})
);
await page.route('**/api/v1/me/page-tags/distinct*', (route) =>
route.fulfill({
status,
contentType: 'application/json',
body: opts.authed ? JSON.stringify({ items: [] }) : unauth
})
);
}
test.describe('mobile library', () => {

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Mobile chrome contract: the AppBar + BottomNav are visible on phone
// viewports and the desktop header is hidden. On desktop, the reverse.

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Phase 2: the catalog (/) gets a mobile chrome — search input full
// width, Filter and Sort as chip buttons that open bottom sheets, and

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Phase 3: the manga detail page gains a mobile hero (blurred backdrop
// + transparent app bar), a sticky bottom CTA whose wording reflects

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Phase 4: the reader gains a mobile chrome — invisible tap zones for
// prev/next/toggle, a chapter-jump bottom sheet, a reader-settings

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// E2E for the per-page collection + tag flow added in v0.61.0. Mocks
// the entire API so the spec runs without a backend. The same

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Guards the title-on-nav behavior: without this, a stale title from
// the last manga / author page lingers when the user navigates to a

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// Network-level mocks for the private-mode UX. The backend integration
// tests (api_private_mode.rs) cover the actual gate; here we only

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
const userFixture = {
id: 'u1',
@@ -123,12 +123,20 @@ test('wrong current password surfaces the 401 envelope inline', async ({ page })
);
await page.goto('/profile/account');
// Wait for the session to hydrate before submitting: a wrong current
// password must be surfaced inline, which the form distinguishes from a
// genuinely-signed-out state via session.user — so the user must be
// known-authenticated first (as they always are on a real page load).
await expect(page.getByText('Signed in as')).toBeVisible();
await page.getByTestId('current-password').fill('definitelyNotIt');
await page.getByTestId('new-password').fill('freshpassfreshpass');
await page.getByTestId('confirm-password').fill('freshpassfreshpass');
await page.getByTestId('password-submit').click();
await expect(page.getByTestId('password-error')).toBeVisible();
// Stays on the account page — a wrong current password is an inline
// error, not a session expiry, so it must not bounce to /login.
await expect(page).toHaveURL(/\/profile\/account$/);
});
test('mismatched new + confirm disables the submit button', async ({ page }) => {
@@ -153,6 +161,24 @@ test('anonymous user sees a profile sign-in prompt', async ({ page }) => {
})
})
);
// The /profile load pair-fetches these; a real backend returns 401 for a
// guest, which the load maps to `authenticated: false`. Without the mock
// the calls fall through to the dead proxy and 500, and the load rethrows
// (only 401 is handled) → error page instead of the sign-in prompt.
await page.route('**/api/v1/me/bookmarks?*', (route) =>
route.fulfill({
status: 401,
contentType: 'application/json',
body: JSON.stringify({ error: { code: 'unauthenticated', message: 'unauthenticated' } })
})
);
await page.route('**/api/v1/me/collections?*', (route) =>
route.fulfill({
status: 401,
contentType: 'application/json',
body: JSON.stringify({ error: { code: 'unauthenticated', message: 'unauthenticated' } })
})
);
await page.goto('/profile');
await expect(page.getByTestId('profile-signin')).toBeVisible();

View File

@@ -1,18 +1,26 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
const mangaId = '33333333-3333-3333-3333-333333333333';
const chapter1Id = 'c1111111-3333-3333-3333-333333333333';
const chapter2Id = 'c2222222-3333-3333-3333-333333333333';
const chapter3Id = 'c3333333-3333-3333-3333-333333333333';
// Faithful `MangaDetail` (GET /v1/mangas/:id) so getManga in the reader
// load returns a shape the components can consume without throwing.
const mangaFixture = {
id: mangaId,
title: 'Vinland Saga',
author: 'Makoto Yukimura',
status: 'ongoing',
alt_titles: [],
description: null,
cover_image_path: null,
created_at: '2026-01-01T00:00:00Z',
updated_at: '2026-01-01T00:00:00Z'
updated_at: '2026-01-01T00:00:00Z',
authors: [{ id: 'a3333333-3333-3333-3333-333333333333', name: 'Makoto Yukimura' }],
genres: [],
tags: [],
content_warnings: [],
chapter_storage_bytes: 0
};
const chaptersFixture = [
@@ -86,6 +94,16 @@ async function mockReaderApis(page: Page) {
body: JSON.stringify({ error: { code: 'unauthenticated', message: '' } })
})
);
// The reader load reads saved position; guest → 401 → null. Without
// this the fetch falls through to the (absent) backend and the load
// rejects before the reader renders.
await page.route(`**/api/v1/me/read-progress/${mangaId}`, (route) =>
route.fulfill({
status: 401,
contentType: 'application/json',
body: JSON.stringify({ error: { code: 'unauthenticated', message: '' } })
})
);
await page.route(`**/api/v1/mangas/${mangaId}`, (route) =>
route.fulfill({
status: 200,
@@ -142,13 +160,14 @@ test('reader chapter select lists every chapter with the manga-detail-style labe
// The current chapter is preselected.
await expect(select).toHaveValue(chapter2Id);
// Each chapter rendered as "Ch. N — Title" (or "Ch. N" when title is null),
// in ascending number order — matching the prev/next sort.
// Each chapter rendered via the shared `chapterLabel` helper — the
// chapter title, or "Chapter N" when the title is null — in ascending
// number order, matching the prev/next sort and the manga-detail list.
const labels = await select.locator('option').allTextContents();
expect(labels.map((l) => l.trim())).toEqual([
'Ch. 1 — Somewhere, Not Here',
'Ch. 2',
'Ch. 3 — Sword Dance'
'Somewhere, Not Here',
'Chapter 2',
'Sword Dance'
]);
});

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
const mangaId = '22222222-2222-2222-2222-222222222222';
const chapterId = 'c2222222-2222-2222-2222-222222222222';

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// E2E for the `?page=N` deep-link path in both reader modes. The
// single-mode case has been working since v0.x; the continuous-mode

View File

@@ -1,15 +1,24 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
const mangaId = '11111111-1111-1111-1111-111111111111';
const chapterId = 'c1111111-1111-1111-1111-111111111111';
// A faithful `MangaDetail` (GET /v1/mangas/:id) — the detail page reads
// authors/genres/tags/alt_titles/content_warnings, so a minimal shape makes
// the component throw mid-render and nothing paints.
const mangaFixture = {
id: mangaId,
title: 'Berserk',
author: 'Kentaro Miura',
status: 'ongoing',
alt_titles: [],
description: 'A dark fantasy.',
cover_image_path: 'mangas/11111111-1111-1111-1111-111111111111/cover.png',
created_at: '2026-01-01T00:00:00Z',
updated_at: '2026-01-01T00:00:00Z'
updated_at: '2026-01-01T00:00:00Z',
authors: [{ id: 'a1111111-1111-1111-1111-111111111111', name: 'Kentaro Miura' }],
genres: [],
tags: [],
content_warnings: [],
chapter_storage_bytes: 0
};
const chaptersFixture = [
{
@@ -103,6 +112,24 @@ async function mockReaderApis(page: Page) {
body: JSON.stringify({ pages: pagesFixture })
})
);
// Guest: no saved read position. The manga-detail + reader loads both
// call this; a 401 maps to `null` in getMyReadProgressForManga, matching
// a real unauthenticated request.
await page.route(`**/api/v1/me/read-progress/${mangaId}`, (route) =>
route.fulfill({
status: 401,
contentType: 'application/json',
body: JSON.stringify({ error: { code: 'unauthenticated', message: 'unauthenticated' } })
})
);
// Recommendations: a real `{ items }` top-N (empty is a valid result).
await page.route(`**/api/v1/mangas/${mangaId}/similar`, (route) =>
route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ items: [] })
})
);
// Stub image bytes so the <img> doesn't 404 (1x1 transparent PNG).
const png = Buffer.from(
'89504e470d0a1a0a0000000d49484452000000010000000108060000001f15c4890000000d49444154789c63000100000005000158a3b62a0000000049454e44ae426082',
@@ -118,7 +145,7 @@ test('manga overview shows title, cover, and a chapter list', async ({ page }) =
await page.goto(`/manga/${mangaId}`);
await expect(page.getByTestId('manga-title')).toHaveText('Berserk');
await expect(page.getByTestId('manga-author')).toContainText('Kentaro Miura');
await expect(page.getByTestId('manga-authors')).toContainText('Kentaro Miura');
await expect(page.getByTestId('manga-cover')).toBeVisible();
await expect(page.getByTestId('chapter-list')).toContainText('The Brand');
await expect(page.getByTestId('bookmark-signin')).toBeVisible();

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
// E2E for the /search page shipped in v0.62.0. Five scenarios against
// mocked endpoints — no backend needed.

View File

@@ -1,4 +1,4 @@
import { test, expect, type Page } from '@playwright/test';
import { test, expect, type Page } from './fixtures';
const userFixture = {
id: 'u1',

View File

@@ -1,6 +1,6 @@
{
"name": "mangalord-frontend",
"version": "0.90.0",
"version": "0.93.2",
"private": true,
"type": "module",
"scripts": {

View File

@@ -11,6 +11,13 @@ const E2E_PORT = 5174;
export default defineConfig({
testDir: 'e2e',
timeout: 30_000,
// The specs are deterministic in isolation, but a full parallel run
// has a cold-start window: Vite compiles each route on first request,
// so the first tests across 8 workers contend on the dev server and
// can trip the timeout. One retry re-runs those on the now-warm server.
// (The `/api/v1` fallback in e2e/fixtures.ts removes the other flake
// source — unmocked calls stalling against the dead dev proxy.)
retries: 1,
use: {
baseURL: process.env.E2E_BASE_URL ?? `http://localhost:${E2E_PORT}`,
trace: 'retain-on-failure'

View File

@@ -7,7 +7,11 @@ import {
afterEach,
type MockInstance
} from 'vitest';
import { handle, shouldBypassProxyTimeout } from './hooks.server';
import {
handle,
shouldBypassProxyTimeout,
stripHopByHopHeaders
} from './hooks.server';
// `BACKEND_URL` is read at module load time, so the values used in the
// asserts below assume the test env didn't set it. `?? 'http://localhost:8080'`
@@ -173,6 +177,30 @@ describe('hooks.server proxy', () => {
expect(headers.get('x-custom')).toBe('pass-through');
});
it('strips hop-by-hop headers from the proxied RESPONSE', async () => {
// The upstream's connection-scoped headers must not ride along on
// the re-streamed response (RFC 7230 §6.1). A normal header passes.
fetchSpy.mockResolvedValueOnce(
new Response('[]', {
status: 200,
headers: {
connection: 'keep-alive',
'keep-alive': 'timeout=5',
'transfer-encoding': 'chunked',
upgrade: 'h2c',
'content-type': 'application/json'
}
})
);
const resolve = vi.fn();
const resp = await handle({ event: makeEvent('/api/v1/health'), resolve });
for (const h of ['connection', 'keep-alive', 'transfer-encoding', 'upgrade']) {
expect(resp.headers.get(h), `${h} should be stripped from response`).toBeNull();
}
// A normal response header survives.
expect(resp.headers.get('content-type')).toContain('application/json');
});
it('aborts and returns 502 when the upstream stalls past the timeout', async () => {
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
// Simulate an aborted fetch (AbortController.abort() raises a
@@ -301,3 +329,25 @@ describe('shouldBypassProxyTimeout', () => {
expect(shouldBypassProxyTimeout(new Headers())).toBe(false);
});
});
describe('stripHopByHopHeaders', () => {
it('removes hop-by-hop headers and keeps the rest', () => {
const src = new Headers({
connection: 'keep-alive',
'transfer-encoding': 'chunked',
'content-type': 'application/json',
'x-request-id': 'abc'
});
const out = stripHopByHopHeaders(src);
expect(out.get('connection')).toBeNull();
expect(out.get('transfer-encoding')).toBeNull();
expect(out.get('content-type')).toBe('application/json');
expect(out.get('x-request-id')).toBe('abc');
});
it('does not mutate the source headers', () => {
const src = new Headers({ connection: 'close' });
stripHopByHopHeaders(src);
expect(src.get('connection')).toBe('close');
});
});

View File

@@ -32,6 +32,21 @@ const HOP_BY_HOP_HEADERS = [
'upgrade'
];
/**
* Return a copy of `src` with the hop-by-hop headers removed. Applied to
* BOTH the proxied request and the proxied response: per RFC 7230 §6.1 a
* proxy must not forward connection-scoped headers in either direction.
* On the response side this also drops the upstream `content-length` /
* `transfer-encoding`, which the runtime recomputes for the re-streamed
* body — forwarding the upstream values risks a framing mismatch.
* Exported for unit-test coverage.
*/
export function stripHopByHopHeaders(src: Headers): Headers {
const out = new Headers(src);
for (const h of HOP_BY_HOP_HEADERS) out.delete(h);
return out;
}
/**
* Cap each proxied request at 5 minutes. The bound exists to surface
* a wedged backend (stuck on a slow DB query, deadlocked, etc.) as a
@@ -74,8 +89,7 @@ export const handle: Handle = async ({ event, resolve }) => {
if (event.url.pathname.startsWith('/api/')) {
const target = `${BACKEND_URL}${event.url.pathname}${event.url.search}`;
const headers = new Headers(event.request.headers);
for (const h of HOP_BY_HOP_HEADERS) headers.delete(h);
const headers = stripHopByHopHeaders(event.request.headers);
// AbortController times the upstream fetch out so a backend
// wedged on a slow DB query doesn't keep the browser request
@@ -148,7 +162,10 @@ export const handle: Handle = async ({ event, resolve }) => {
return new Response(upstream.body, {
status: upstream.status,
statusText: upstream.statusText,
headers: upstream.headers
// Strip hop-by-hop headers on the way back too — the upstream's
// transfer-encoding / content-length / connection headers must
// not ride along on the re-streamed response.
headers: stripHopByHopHeaders(upstream.headers)
});
}
return resolve(event);

View File

@@ -58,11 +58,18 @@ export type ChangePassword = {
* password.
*/
export async function changePassword(input: ChangePassword): Promise<void> {
await request<void>('/v1/auth/me/password', {
method: 'PATCH',
headers: { 'content-type': 'application/json' },
body: JSON.stringify(input)
});
await request<void>(
'/v1/auth/me/password',
{
method: 'PATCH',
headers: { 'content-type': 'application/json' },
body: JSON.stringify(input)
},
// A 401 here means the *current* password was wrong, not that the
// session expired — don't let the global hook clear the user and
// bounce them to /login; the form surfaces the error inline.
{ suppressOn401: true }
);
}
/**

View File

@@ -103,6 +103,25 @@ describe('on401 hook', () => {
expect(hook).toHaveBeenCalledTimes(1);
});
it('does not invoke the hook on a 401 when suppressOn401 is set', async () => {
// The change-password endpoint returns 401 for a *wrong current
// password*, not an expired session — clearing the cached user
// there would spuriously log the (still-authenticated) user out.
// Such calls opt out of the hook while still throwing the 401.
const hook = vi.fn();
setOn401Hook(hook);
fetchSpy.mockResolvedValueOnce(
new Response(
JSON.stringify({ error: { code: 'unauthenticated', message: 'wrong password' } }),
{ status: 401, headers: { 'content-type': 'application/json' } }
)
);
await expect(
request('/v1/auth/me/password', { method: 'PATCH' }, { suppressOn401: true })
).rejects.toMatchObject({ status: 401, code: 'unauthenticated' });
expect(hook).not.toHaveBeenCalled();
});
it('does not invoke the hook on non-401 errors', async () => {
const hook = vi.fn();
setOn401Hook(hook);

View File

@@ -54,7 +54,24 @@ export function setOn401Hook(handler: (() => void) | null): void {
on401Hook = handler;
}
export async function request<T>(path: string, init?: RequestInit): Promise<T> {
/** Per-call knobs that don't belong on the native `RequestInit`. */
export type RequestOptions = {
/**
* Skip the module-level 401 hook for this call. Used by endpoints
* where a 401 does *not* mean "session expired" — e.g. the
* change-password endpoint returns 401 for a wrong *current*
* password while the caller is still fully authenticated. Firing
* the hook there would clear the cached user and bounce them to
* /login instead of surfacing the error inline.
*/
suppressOn401?: boolean;
};
export async function request<T>(
path: string,
init?: RequestInit,
opts?: RequestOptions
): Promise<T> {
// Forward credentials (session cookie) explicitly so cross-origin
// deployments — those configured via CORS_ALLOWED_ORIGINS — keep
// working. For same-origin requests this is a no-op compared to the
@@ -87,7 +104,7 @@ export async function request<T>(path: string, init?: RequestInit): Promise<T> {
} catch {
// Body wasn't parseable; keep the http_error fallback.
}
if (res.status === 401 && on401Hook) {
if (res.status === 401 && on401Hook && !opts?.suppressOn401) {
// Fire before throwing so the session store updates even
// if the caller swallows the ApiError (e.g. the *OrEmpty
// wrappers used by guest-rendering pages).

View File

@@ -0,0 +1,49 @@
import { describe, it, expect } from 'vitest';
import { safeNext } from './safeNext';
describe('safeNext', () => {
it('allows same-origin absolute paths', () => {
expect(safeNext('/library')).toBe('/library');
expect(safeNext('/search?q=berserk')).toBe('/search?q=berserk');
expect(safeNext('/manga/123#page-2')).toBe('/manga/123#page-2');
});
it('falls back to / for missing or empty input', () => {
expect(safeNext(null)).toBe('/');
expect(safeNext(undefined)).toBe('/');
expect(safeNext('')).toBe('/');
});
it('rejects protocol-relative URLs (open redirect)', () => {
expect(safeNext('//evil.com')).toBe('/');
expect(safeNext('//evil.com/path')).toBe('/');
});
it('rejects backslash tricks browsers normalise to //', () => {
expect(safeNext('/\\evil.com')).toBe('/');
expect(safeNext('/\\/evil.com')).toBe('/');
});
it('rejects control/whitespace chars browsers strip mid-parse', () => {
// These start with a single "/" so the prefix checks alone would pass
// them, but a browser strips the tab/newline/CR and the value
// collapses to a protocol-relative //evil.com — the %09/%0A/%0D
// open-redirect bypass.
expect(safeNext('/\t/evil.com')).toBe('/'); // %09 tab
expect(safeNext('/\n/evil.com')).toBe('/'); // %0A newline
expect(safeNext('/\r/evil.com')).toBe('/'); // %0D carriage return
expect(safeNext('/ /evil.com')).toBe('/'); // space
expect(safeNext(`/${String.fromCharCode(0)}/evil.com`)).toBe('/'); // NUL
expect(safeNext(`/${String.fromCharCode(0x2028)}/evil.com`)).toBe('/'); // line sep
});
it('rejects absolute and scheme URLs', () => {
expect(safeNext('https://evil.com')).toBe('/');
expect(safeNext('http://evil.com')).toBe('/');
expect(safeNext('javascript:alert(1)')).toBe('/');
});
it('rejects bare paths without a leading slash', () => {
expect(safeNext('library')).toBe('/');
});
});

View File

@@ -0,0 +1,34 @@
/**
* Sanitise a `?next=` post-login/registration redirect target.
*
* The login/register flow round-trips the page the user was bounced from via
* `?next=`. Navigating to an attacker-supplied value would be an open
* redirect, so only same-origin absolute paths are allowed: the value must
* start with a single `/` and must not be protocol-relative (`//evil.com`)
* or a backslash trick (`/\evil.com`, which some parsers normalise to `//`).
*
* Crucially, the structural prefix checks are not enough on their own:
* browsers **strip ASCII tab/newline/CR mid-parse**, so `/\t/evil.com`
* (which starts with a single `/`) collapses to the protocol-relative
* `//evil.com` once navigated — an open-redirect bypass via `%09`/`%0A`/
* `%0D`. So we reject any control or whitespace character (and backslashes)
* *before* the prefix checks. Anything else — absolute URLs, `javascript:`
* URIs, empty/missing — falls back to the site root.
*/
function hasUnsafeNextChar(s: string): boolean {
for (let i = 0; i < s.length; i++) {
const c = s.charCodeAt(i);
// C0 controls + space (<= 0x20), DEL (0x7f), or backslash (0x5c) —
// characters a URL parser may strip or treat as a path separator.
if (c <= 0x20 || c === 0x7f || c === 0x5c) return true;
}
// Any remaining Unicode whitespace (NBSP, line/paragraph separators, …).
return /\s/.test(s);
}
export function safeNext(next: string | null | undefined): string {
if (typeof next !== 'string' || next.length === 0) return '/';
if (hasUnsafeNextChar(next)) return '/';
if (!next.startsWith('/') || next.startsWith('//')) return '/';
return next;
}

View File

@@ -2,6 +2,7 @@
import { goto } from '$app/navigation';
import { login } from '$lib/api/auth';
import { session } from '$lib/session.svelte';
import { safeNext } from '$lib/safeNext';
let username = $state('');
let password = $state('');
@@ -15,7 +16,10 @@
try {
const user = await login({ username, password });
session.setUser(user);
await goto('/');
// Return the user to the page they were bounced from (?next=),
// guarded against open redirects; default to the site root.
const next = new URLSearchParams(window.location.search).get('next');
await goto(safeNext(next));
} catch (e) {
error = (e as Error).message;
} finally {

View File

@@ -4,6 +4,7 @@
import { register } from '$lib/api/auth';
import { authConfig } from '$lib/auth-config.svelte';
import { session } from '$lib/session.svelte';
import { safeNext } from '$lib/safeNext';
let username = $state('');
let password = $state('');
@@ -24,7 +25,10 @@
try {
const user = await register({ username, password });
session.setUser(user);
await goto('/');
// Honour ?next= the same way login does (registration logs the
// user in), guarded against open redirects.
const next = new URLSearchParams(window.location.search).get('next');
await goto(safeNext(next));
} catch (e) {
error = (e as Error).message;
} finally {

View File

@@ -41,9 +41,9 @@ function parseWarnings(raw: string | null): ContentWarning[] {
* - `?order=desc|asc` — only meaningful for chapters/mangas tabs.
* `desc` (most matches first) is the default.
*
* `?text=` is reserved for the planned OCR text-search input. The
* backend rejects it with 501 + stable code
* `text_search_not_yet_supported` today; the frontend never sets it.
* `?text=` drives OCR full-text search against the page OCR index (the
* active OCR backend populates it). A non-empty `text` (or an included
* content-warning) switches the page into content-search mode below.
*/
export const load: PageLoad = async ({ url }) => {
const tag = url.searchParams.get('tag');