diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index fc0e1c9..3c8bd7d 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -2,10 +2,11 @@ name: ci # Formatting, linting, types and tests on every push and pull request. # -# Blocking: the checks that are clean today — ruff format, ruff check, prettier, pytest. -# Non-blocking: eslint and svelte-check, which still report 87 and 30 pre-existing errors. -# Those need real code changes rather than a formatter, so they report without failing the -# build; drop the `continue-on-error` line from a step once its count reaches zero. +# Blocking: ruff format, ruff check, pytest, prettier and svelte-check. +# Non-blocking: eslint, which reports two `{@html}` XSS findings in collapsible-text.svelte. +# Those are a real vulnerability rather than a lint nit — book descriptions from EPUB files +# are not sanitized — and fixing them is a backend change. Drop the `continue-on-error` once +# that lands, at which point every check blocks. # # The release workflow runs the blocking half again before it publishes anything. @@ -77,4 +78,3 @@ jobs: - name: Types run: pnpm check - continue-on-error: true diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 2e2cd58..948b6f2 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -50,12 +50,13 @@ jobs: with: node-version: 24 - - name: Frontend format + - name: Frontend format and types working-directory: frontend run: | corepack enable pnpm install --frozen-lockfile pnpm exec prettier --check . + pnpm check build: needs: quality diff --git a/AGENTS.md b/AGENTS.md index db9118a..86d50e2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,11 +103,11 @@ API docs are served by the running backend at `http://localhost:8000/schema/` (S manually against a live backend). - **Migrations are mandatory.** The app runs with `create_all=False`, so a model change without a matching Alembic revision will not reach the database. -- **CI gates formatting, linting and tests.** `.gitea/workflows/ci.yml` runs on every push and pull - request: `ruff format --check`, `ruff check`, `pytest`, and `prettier --check` all **block**; - `eslint` and `pnpm check` report without failing until their pre-existing counts reach zero. A - `v*` tag additionally builds and publishes both container images — see - `docs/ci-release-pipeline.md`. +- **CI gates formatting, linting, types and tests.** `.gitea/workflows/ci.yml` runs on every push + and pull request: `ruff format --check`, `ruff check`, `pytest`, `prettier --check` and + `pnpm check` all **block**; only `eslint` reports without failing, and only until the two + `{@html}` findings in `TODO.md` are fixed. A `v*` tag additionally builds and publishes both + container images — see `docs/ci-release-pipeline.md`. - **Commit messages** follow `type: summary` — `feat:`, `fix:`, `refactor:`, `chore:`. - The three `README.md` files are user-facing. Agent-facing knowledge belongs in the `AGENTS.md` files. diff --git a/TODO.md b/TODO.md index 4daebfe..59f281b 100644 --- a/TODO.md +++ b/TODO.md @@ -126,6 +126,35 @@ that set is worthwhile and will surface a fresh batch of findings. ## Frontend +### Book descriptions are rendered as unsanitized HTML + +`frontend/src/lib/components/ui/collapsible-text/collapsible-text.svelte` — lines 42 and 45 + +The component renders `{@html text}`, and its only caller is the book detail page: +``. So whatever is in +`Book.description` reaches the DOM as markup. + +The Calibre importer is fine — `services/calibre.py:401` passes comments through +`strip_html`, because Calibre stores HTML there. But `strip_html` is used **nowhere else in +the backend**, and `EpubExtractor._extract_description` returns +`epub.get_metadata("DC", "description")[0][0]` verbatim. EPUB `dc:description` routinely +carries markup, so an uploaded book with `` in that field executes +script on the book page, with the session cookie in scope. Metadata edited through the UI +is stored unfiltered too. + +This is the same class as the scripted-EPUB item below — untrusted file content reaching an +origin that holds a session — by a different route, and it does not need `allow-scripts` to +work. + +Fix: sanitize at ingest, next to where Calibre already does. Reuse `strip_html` in +`_extract_description` if descriptions should be plain text, or run an allowlist sanitizer if +the formatting is worth keeping. Either way the stored rows need backfilling through the same +helper, since the validators only fire on write. Dropping `{@html}` to `{text}` in the +component fixes the display side but leaves the payload in the database. + +These are the only two findings `pnpm exec eslint .` still reports; CI's eslint step stops +being `continue-on-error` once they are gone. + ### Scripted EPUBs run against the app origin **This is a regression from the foliate-js migration, not a pre-existing gap.** diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index ae38c2c..9e3b39c 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -157,18 +157,22 @@ but take a baseline first, because neither is clean (see below). Observed in the current tree — don't mistake these for intentional patterns to copy: -- `pnpm check` is not clean. **Baseline as of 2026-08-13: 30 errors, 1 warning, 8 files**, most of - them in `src/routes/api/[...path]/+server.ts` (see below). Get your own baseline before assuming - an error is yours. `src/lib/schema/openapi/schema.d.ts` was regenerated on that date and is - current; regenerate it again after any backend API change, with +- **`pnpm check` is clean as of 2026-08-17 and CI blocks on it** — 0 errors, 0 warnings. Any error + you see is yours. `src/lib/schema/openapi/schema.d.ts` is + current; regenerate it after any backend API change, with `pnpm exec openapi-typescript http://localhost:8000/schema/openapi.json -o src/lib/schema/openapi/schema.d.ts` against a backend running **your** branch — a stale server silently writes a stale file. - **Prettier is clean and CI blocks on it** — run `pnpm format` before finishing. Two things it must not touch are in `.prettierignore`: the vendored foliate-js, and `src/lib/schema/openapi/schema.d.ts`, which `openapi-typescript` regenerates in its own style. -- `pnpm exec eslint .` reports **87 pre-existing errors as of 2026-08-17**, all in `src/`. CI runs - it non-blocking (`continue-on-error`) until that reaches zero. `static/pdfjs/` is ignored +- `pnpm exec eslint .` reports **2 errors as of 2026-08-17**, both `svelte/no-at-html-tags` in + `collapsible-text.svelte`. They are a genuine XSS hole, not a lint nit — see `TODO.md`. CI runs + eslint non-blocking (`continue-on-error`) only until that is fixed. `static/pdfjs/` is ignored alongside `src/lib/vendor/` — it is vendored too, and linting it produced 1717 further errors. +- Two rules are off for `**/*.svelte` in `eslint.config.js` because they predate runes and + misread them: `no-useless-assignment` (every `$bindable()` default) and + `@typescript-eslint/no-unused-expressions` (a bare `book;` declaring an `$effect` dependency). + A leading underscore marks an intentionally unused binding. - `src/routes/api/[...path]/+server.ts` — all four handlers are annotated `RequestHandler` while the import of that type is commented out at line 4. It also buffers whole **responses** with `arrayBuffer()` and forwards no `Range` header, so book downloads are not streamed. **Requests**