feat: block CI on the frontend type check
svelte-check is clean, so it gates like the rest. eslint still reports without failing, now for two findings rather than 87: both are the unsanitized book description, written up in TODO.md.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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:
|
||||
`<CollapsibleText text={book.description} maxLength={500} />`. 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 `<img src=x onerror=…>` 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.**
|
||||
|
||||
+10
-6
@@ -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**
|
||||
|
||||
Reference in New Issue
Block a user