diff --git a/apps/readest-app/.claude/memory/MEMORY.md b/apps/readest-app/.claude/memory/MEMORY.md index ec64aea3..f3692dd9 100644 --- a/apps/readest-app/.claude/memory/MEMORY.md +++ b/apps/readest-app/.claude/memory/MEMORY.md @@ -5,7 +5,7 @@ - [Layout & UI](layout-ui-fixes.md) insets/z-index · [Platform Compat](platform-compat-fixes.md) · [Annotator & Reader](annotator-reader-fixes.md) ## Safety & Security - [In-place delete wiped originals](in-place-delete-wiped-originals.md) never `fs.removeFile` `external` · [Backup zip Windows paths #4703](backup-windows-zip-paths-4703.md) normalize `\` · [download_file scope Android #4639](download-file-scope-android-regression.md) -- [Security advisories 2026-06](security-advisories-web-2026-06.md) 4 GHSA #4638 +- [Security advisories 2026-06](security-advisories-web-2026-06.md) 4 GHSA #4638; SSRF guard broke dev-LAN OPDS, dev-only exemption 2026-07 ## Paginator & Scroll - Reading ruler: [line-aware](reading-ruler-line-aware.md) frame-offset map; [vertical-rl backwards #4865](reading-ruler-vertical-rtl-4865.md) - [Vertical-rl horizontal pagination (#624)](vertical-rl-horizontal-pagination-624.md) — horizontal inputs + two-phase slide; rtl gated `!vertical` @@ -103,7 +103,7 @@ - [Table dark-mode tint #4419](table-dark-mode-tint-4419.md) · [footnote aside border #4438](footnote-aside-namespace-order-4438.md) - Proofread: [enhancements #4700](proofread-enhancements-4700.md); [per-book CRDT #4781](proofread-per-book-crdt-sync.md); [edit Find + toggle #4859](proofread-edit-toggle-4859.md) - [Russian NBSP (#4769)](russian-hanging-prepositions-nbsp-4769.md) -- OPDS: [Firefox strict-XML #4479](opds-firefox-strict-xml-4479.md); [JSON search #4502](opds2-json-search-4502.md); [HTML desc #4503](opds-html-description-4503.md); [self-link #4749](opds-self-link-metadata-4749.md); [popular dedup #4782](opds-popular-catalog-dedup-4782.md); [auto-download subdir crawl #4272](opds-autodownload-subdir-crawl-4272.md) bounded BFS, never crawl newest-feed catalogs +- OPDS: [Firefox strict-XML #4479](opds-firefox-strict-xml-4479.md); [JSON search #4502](opds2-json-search-4502.md); [HTML desc #4503](opds-html-description-4503.md); [self-link #4749](opds-self-link-metadata-4749.md); [popular dedup #4782](opds-popular-catalog-dedup-4782.md); [auto-download subdir crawl #4272](opds-autodownload-subdir-crawl-4272.md) bounded BFS, never crawl newest-feed catalogs; [preemptive Basic 400s digest Calibre](opds-preemptive-basic-digest-400.md) bare-retry on 400; [auto-download TLS #4988](opds-autodownload-tls-skipssl-4988.md) skipSslVerification parity - [D-pad Navigation](dpad-navigation.md) - [koplugin cover upload (#4374)](koplugin-cover-upload.md) - [koplugin Library slow open #4954](koplugin-library-open-mosaic-cache-4954.md) group mosaics recomposed every paint; PR#4974 availability-keyed cache + async compose + cache nil result diff --git a/apps/readest-app/.claude/memory/opds-autodownload-tls-skipssl-4988.md b/apps/readest-app/.claude/memory/opds-autodownload-tls-skipssl-4988.md new file mode 100644 index 00000000..16c53dce --- /dev/null +++ b/apps/readest-app/.claude/memory/opds-autodownload-tls-skipssl-4988.md @@ -0,0 +1,16 @@ +--- +name: opds-autodownload-tls-skipssl-4988 +description: "#4988 OPDS auto-download failed on self-signed/private-CA servers — native download_file (rustls) needs skipSslVerification like the manual path (#2900)" +metadata: + node_type: memory + type: project + originSessionId: 9066b80b-3cb5-44df-9c4b-7f609cf285a5 +--- + +Issue #4988: OPDS auto-download failed while manual browse/download of the same catalog worked (reporter: iPad + Calibre-Web NextGen behind nginx https). Reporter blamed the credential-less HEAD probes — red herring: those are `probeAuth` challenge probes, their 401 is by design and the Basic header still reaches the GET. + +**Real signature:** feed GETs and HEAD probes appear in the server log, download GETs never do → the native `download_file` dies client-side in the TLS handshake. The Tauri http-plugin path (`opdsReq.ts`) always passes `danger: {acceptInvalidCerts: true}`, but `transfer_file.rs` builds its reqwest client with **rustls**, which ignores the OS trust store — self-signed, private-CA, or incomplete-chain certs all fail unless `skip_ssl_verification` is set. Manual download (`page.tsx handleDownload`) got `skipSslVerification: true` in #2900 (for #2871); `autoDownload.ts downloadAndImport` never did. + +**Fix (2026-07-08):** pass `skipSslVerification: true` in autoDownload's `downloadFile` call. Test in `opds-auto-download.test.ts`. + +**How to apply:** any new code path that downloads via native `download_file`/`tauriDownload` from a user-configured server must mirror the manual path's `skipSslVerification` — TLS behavior differs between the http plugin (danger flags on) and transfer_file (strict rustls by default), so "browse works but download fails, nothing in server logs" = check this first. "curl works without -k" on another machine proves nothing about rustls trust. diff --git a/apps/readest-app/.claude/memory/opds-preemptive-basic-digest-400.md b/apps/readest-app/.claude/memory/opds-preemptive-basic-digest-400.md new file mode 100644 index 00000000..d346f340 --- /dev/null +++ b/apps/readest-app/.claude/memory/opds-preemptive-basic-digest-400.md @@ -0,0 +1,16 @@ +--- +name: opds-preemptive-basic-digest-400 +description: "Calibre digest/'auto' servers 400 the preemptive Basic header from PR #4206; fetchWithAuth must bare-retry on 400 to surface the Digest challenge" +metadata: + node_type: memory + type: project + originSessionId: 9066b80b-3cb5-44df-9c4b-7f609cf285a5 +--- + +Calibre's content server in `digest` (or `auto` over http) auth mode responds to a `Basic` Authorization header with **400 "Unsupported authentication method"** — not a 401 challenge. PR #4206 (commit 83607d14e) made `fetchWithAuth` (`src/app/opds/utils/opdsReq.ts`) send Basic preemptively (for Calibre-Web-style servers that return anonymous 200 without a challenge), which dead-ended all digest-mode Calibre servers: the retry logic only fired on 401/403, so users saw "Failed to load OPDS feed: 400 Bad Request" (reported on Android, but platform-independent — web proxy relays the 400 too). + +**Fix (2026-07-08):** in `fetchWithAuth`, when the first response is 400 AND preemptive Basic was sent, re-issue the request once *without* credentials to surface `WWW-Authenticate`, then let the existing 401/403 negotiation pick Digest. Direct path strips the Authorization header; proxy path strips the `auth=` query param. Tests in `src/__tests__/utils/opds-req.test.ts`. + +**Why:** the two auth-server archetypes conflict — anonymous-200 servers need preemptive creds (#4206), strict digest servers reject them with 400. Only runtime negotiation satisfies both; don't "fix" one archetype by regressing the other. + +**How to apply:** any preemptive-auth optimization needs a recovery path for servers that reject the scheme outright (400/4xx without challenge), not just for 401/403 challenges. The app's Digest implementation itself is correct (Calibre's strict parser answers 401, not 400, to its headers). Verify against a real Calibre: dummy creds distinguish malformed (400) from wrong-password (401). Beware: Calibre throttles repeated failed logins with transient 503s. Related: [[security-advisories-web-2026-06]] (the *other* OPDS 400 — dev-LAN SSRF block in the proxy). diff --git a/apps/readest-app/.claude/memory/security-advisories-web-2026-06.md b/apps/readest-app/.claude/memory/security-advisories-web-2026-06.md index 1ba779f7..461d16b2 100644 --- a/apps/readest-app/.claude/memory/security-advisories-web-2026-06.md +++ b/apps/readest-app/.claude/memory/security-advisories-web-2026-06.md @@ -21,6 +21,8 @@ After both merge: comment on each GHSA noting the fixing PR (pending merge). Not **C — Tauri native (PR #4639)** GHSA-55vr-pvq5-6fmg: unscoped `download_file`/`upload_file` in `src-tauri/src/transfer_file.rs` → arbitrary local read/write. FIXED: added `app: AppHandle` param + `ensure_path_allowed` (rejects relative + `..` via `has_disallowed_components`, then `fs_scope().is_allowed()`). Chose STRICT `is_allowed` (NOT read_dir's `|| contains("Readest")` substring hatch) because all legit callers (cloud sync, WebDAV, self-updater APK→`'Cache'`, OPDS→`'Cache'`) resolve under static scope ($APPDATA/Readest, $APPCACHE, $TEMP) OR persisted dialog grants (custom root via `setCustomRootDir`→picker→`allow_paths_in_scopes`; external folders re-granted at startup; `tauri_plugin_persisted_scope` makes sticky). Clippy needed `#[allow(clippy::too_many_arguments)]` on download_file (8 args). AppHandle auto-injected → JS invoke unchanged. NOTE: shared `target/` (worktree) was polluted with a deleted sibling worktree's abs plugin-permission paths → build failed `failed to read .../readest-feat-nightly-update-channel/.../fs/permissions/app.toml`; fix = `rm -rf` the `target/debug/build/-` dirs grepping for the stale path, then rebuild. skip_ssl_verification left as-is (OPDS needs it). read_dir's own `contains("Readest")` hatch left untouched (out of scope). +**Regression found 2026-07-08:** the "blocking private hosts removes no functionality" assumption below missed `pnpm dev-web` — in `next dev` the server runs on the developer's own machine and LAN catalogs (e.g. Calibre at 192.168.x.x) are the normal dev workflow; the unconditional `isBlockedHost` made the proxy 400 them ("This URL is not allowed"). CatalogManager already gates its "no LAN URLs" error on `NODE_ENV === 'production'`, so the proxy now mirrors that: `isPrivateHostAllowed() = NODE_ENV === 'development'` skips the blocklist (both preflight and per-redirect-hop). Vitest runs under `NODE_ENV=test` so the SSRF tests still exercise blocking; the dev-exemption test uses `vi.stubEnv('NODE_ENV', 'development')`. Self-hosted production deployments remain blocked (unresolved if anyone complains — would need an env-var opt-in). Also: Calibre's server throttles after repeated failed auth with transient 503s — don't mistake those for a proxy bug. + **Non-obvious decision:** OPDS proxy can't require Readest auth — it's consumed from the browser via `` (covers) and `window.fetch` WITHOUT a Readest token; the `auth` query param is the *upstream* OPDS server cred, not the user token. So auth would break OPDS browsing/images. SSRF host-filter is the non-breaking high-value fix; residual relay/CORS-bypass on hosted CF (Medium) left for maintainer. On web the proxy is a CF Worker that can't reach a user LAN anyway (desktop bypasses via `needsProxy`), so blocking private hosts removes no functionality. Test invocation gotcha: `npx vitest run ` skips dotenv → `src/utils/supabase.ts:8 atob(...)` throws at import for tests that load the REAL `@/utils/access` (e.g. `send-fetch-url-guard.test.ts`). Use `pnpm test` (wraps `dotenv -e .env -e .env.test.local`) or `npx dotenv -e .env -e .env.test.local -- vitest run`. Tests that mock supabase/access are unaffected. diff --git a/apps/readest-app/src/__tests__/app/api/opds-proxy-ssrf.test.ts b/apps/readest-app/src/__tests__/app/api/opds-proxy-ssrf.test.ts index d424d536..00f70197 100644 --- a/apps/readest-app/src/__tests__/app/api/opds-proxy-ssrf.test.ts +++ b/apps/readest-app/src/__tests__/app/api/opds-proxy-ssrf.test.ts @@ -21,6 +21,7 @@ beforeEach(() => { afterEach(() => { vi.unstubAllGlobals(); + vi.unstubAllEnvs(); vi.restoreAllMocks(); }); @@ -63,6 +64,22 @@ describe('OPDS proxy SSRF guard', () => { expect(fetchSpy).toHaveBeenCalledTimes(1); }); + it('allows LAN catalog targets in development', async () => { + // `next dev` runs on the developer's own machine, where a LAN OPDS server + // (e.g. Calibre-Web on the local network) is the normal use case and the + // CatalogManager UI only forbids LAN URLs in production builds. + vi.stubEnv('NODE_ENV', 'development'); + fetchSpy.mockResolvedValueOnce( + new Response('', { + status: 200, + headers: { 'Content-Type': 'application/atom+xml' }, + }), + ); + const res = await GET(proxyReq('http://192.168.2.120:8080/opds')); + expect(res.status).toBe(200); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + it('proxies a legitimate public feed', async () => { fetchSpy.mockResolvedValueOnce( new Response('', { diff --git a/apps/readest-app/src/__tests__/services/opds-auto-download.test.ts b/apps/readest-app/src/__tests__/services/opds-auto-download.test.ts index 355724e4..cac56e3f 100644 --- a/apps/readest-app/src/__tests__/services/opds-auto-download.test.ts +++ b/apps/readest-app/src/__tests__/services/opds-auto-download.test.ts @@ -138,6 +138,35 @@ describe('OPDS auto-download orchestrator', () => { }); }); + it('downloads with skipSslVerification like the manual download path', async () => { + // The manual OPDS download (page.tsx handleDownload) passes + // skipSslVerification as a workaround for self-signed/private-CA OPDS + // servers (#2871): the native download_file validates TLS with rustls, + // which ignores the OS trust store, while the feed fetch and auth probe + // go through the http plugin with acceptInvalidCerts. Without the same + // flag here, auto-download dies in the TLS handshake on servers where + // manual download works (#4988). + const catalogs: OPDSCatalog[] = [ + { id: 'cat-1', name: 'Shelf', url: 'https://shelf.example.com/opds', autoDownload: true }, + ]; + vi.mocked(checkFeedForNewItems).mockResolvedValue([ + { + entryId: 'urn:shelf:1', + title: 'Issue 1', + acquisitionHref: '/dl/1.epub', + mimeType: 'application/epub+zip', + baseURL: 'https://shelf.example.com/opds', + }, + ]); + + await syncSubscribedCatalogs(catalogs, appService, []); + + expect(downloadFile).toHaveBeenCalledTimes(1); + expect(vi.mocked(downloadFile).mock.calls[0]![0]).toMatchObject({ + skipSslVerification: true, + }); + }); + it('handles import failure by adding to failedEntries', async () => { const catalogs: OPDSCatalog[] = [ { id: 'cat-1', name: 'Test', url: 'https://example.com/opds', autoDownload: true }, diff --git a/apps/readest-app/src/__tests__/utils/opds-req.test.ts b/apps/readest-app/src/__tests__/utils/opds-req.test.ts index 27343e01..4ea6c33a 100644 --- a/apps/readest-app/src/__tests__/utils/opds-req.test.ts +++ b/apps/readest-app/src/__tests__/utils/opds-req.test.ts @@ -177,6 +177,63 @@ describe('opdsReq', () => { expect(auth).toBe(`Basic ${btoa('alice:s3cret')}`); }); + it('recovers when a Digest-only server rejects the preemptive Basic header with 400', async () => { + // Calibre in 'digest' (or 'auto' over http) mode responds to a Basic + // Authorization header with 400 "Unsupported authentication method" + // instead of a 401 challenge, so the preemptive Basic header dead-ends + // the request. The client must re-issue the request without credentials + // to obtain the WWW-Authenticate challenge, then negotiate Digest. + fetchMock + .mockResolvedValueOnce( + makeResponse({ status: 400, body: 'Unsupported authentication method' }), + ) + .mockResolvedValueOnce( + makeResponse({ + status: 401, + wwwAuthenticate: 'Digest realm="calibre", nonce="abc123", algorithm="MD5", qop="auth"', + }), + ) + .mockResolvedValueOnce(makeResponse({ status: 200, body: '' })); + + const res = await fetchWithAuth('http://calibre.example.com/opds', 'alice', 's3cret', false); + + expect(fetchMock).toHaveBeenCalledTimes(3); + const bareInit = fetchMock.mock.calls[1]![1] as RequestInit; + const bareHeaders = bareInit.headers as Record; + expect(bareHeaders['Authorization']).toBeUndefined(); + const digestInit = fetchMock.mock.calls[2]![1] as RequestInit; + const digestHeaders = digestInit.headers as Record; + expect(digestHeaders['Authorization']).toMatch(/^Digest /); + expect(res.status).toBe(200); + }); + + it('recovers from the preemptive-Basic 400 through the proxy as well', async () => { + fetchMock + .mockResolvedValueOnce( + makeResponse({ status: 400, body: 'Unsupported authentication method' }), + ) + .mockResolvedValueOnce( + makeResponse({ + // The web proxy maps the upstream 401 to 403 and forwards the + // WWW-Authenticate challenge. + status: 403, + wwwAuthenticate: 'Digest realm="calibre", nonce="abc123", algorithm="MD5", qop="auth"', + }), + ) + .mockResolvedValueOnce(makeResponse({ status: 200, body: '' })); + + const res = await fetchWithAuth('http://calibre.example.com/opds', 'alice', 's3cret', true); + + expect(fetchMock).toHaveBeenCalledTimes(3); + const bareUrl = fetchMock.mock.calls[1]![0] as string; + expect(new URL(bareUrl, 'https://web.readest.com').searchParams.get('auth')).toBeNull(); + const digestUrl = fetchMock.mock.calls[2]![0] as string; + expect(new URL(digestUrl, 'https://web.readest.com').searchParams.get('auth')).toMatch( + /^Digest /, + ); + expect(res.status).toBe(200); + }); + it('retries with Digest auth when the server issues a Digest challenge', async () => { fetchMock .mockResolvedValueOnce( diff --git a/apps/readest-app/src/app/api/opds/proxy/route.ts b/apps/readest-app/src/app/api/opds/proxy/route.ts index fbd8394d..89136a28 100644 --- a/apps/readest-app/src/app/api/opds/proxy/route.ts +++ b/apps/readest-app/src/app/api/opds/proxy/route.ts @@ -6,6 +6,11 @@ import { isBlockedHost } from '@/utils/network'; // Cap redirect hops so the SSRF host check below can re-run on every one. const MAX_REDIRECTS = 5; +// In `next dev` the server runs on the developer's own machine, where a LAN +// OPDS catalog is the normal use case (the CatalogManager UI only forbids +// adding LAN URLs in production builds), so the host blocklist is skipped. +const isPrivateHostAllowed = () => process.env.NODE_ENV === 'development'; + /** * Fetch the target while running the SSRF host check on every redirect hop. * `fetch`'s default `redirect: 'follow'` would let a public URL 302 to an @@ -25,7 +30,7 @@ async function fetchFollowingRedirects( if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { throw new SsrfBlockedError('Only http(s) URLs are supported'); } - if (isBlockedHost(parsed.hostname)) { + if (!isPrivateHostAllowed() && isBlockedHost(parsed.hostname)) { throw new SsrfBlockedError('This URL is not allowed'); } const response = await fetch(currentUrl, { ...init, redirect: 'manual' }); @@ -79,7 +84,7 @@ async function handleRequest(request: NextRequest, method: 'GET' | 'HEAD') { if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') { return NextResponse.json({ error: 'Only http(s) URLs are supported' }, { status: 400 }); } - if (isBlockedHost(parsedUrl.hostname)) { + if (!isPrivateHostAllowed() && isBlockedHost(parsedUrl.hostname)) { return NextResponse.json({ error: 'This URL is not allowed' }, { status: 400 }); } diff --git a/apps/readest-app/src/app/opds/utils/opdsReq.ts b/apps/readest-app/src/app/opds/utils/opdsReq.ts index 65d2f107..b39f889b 100644 --- a/apps/readest-app/src/app/opds/utils/opdsReq.ts +++ b/apps/readest-app/src/app/opds/utils/opdsReq.ts @@ -342,11 +342,14 @@ export const fetchWithAuth = async ( const fetchURL = useProxy ? getProxiedURL(cleanUrl, preemptiveAuth || '', false, normalizedCustomHeaders) : cleanUrl; - const headers: Record = { + const baseHeaders: Record = { 'User-Agent': READEST_OPDS_USER_AGENT, Accept: 'application/atom+xml, application/xml, text/xml, */*', ...(!useProxy ? normalizedCustomHeaders : {}), ...(options.headers as Record), + }; + const headers: Record = { + ...baseHeaders, ...(preemptiveAuth && !useProxy ? { Authorization: preemptiveAuth } : {}), }; @@ -358,6 +361,24 @@ export const fetchWithAuth = async ( danger: { acceptInvalidCerts: true, acceptInvalidHostnames: true }, }); + // Calibre in 'digest' (or 'auto' over http) mode rejects a Basic + // Authorization header outright with 400 "Unsupported authentication + // method" instead of returning a 401 challenge, so the preemptive Basic + // header would dead-end the request. Re-issue it without credentials to + // surface the WWW-Authenticate challenge and let the retry below negotiate + // the scheme the server actually wants. + if (res.status === 400 && preemptiveAuth) { + res = await fetch( + useProxy ? getProxiedURL(cleanUrl, '', false, normalizedCustomHeaders) : fetchURL, + { + ...options, + method: options.method || 'GET', + headers: baseHeaders, + danger: { acceptInvalidCerts: true, acceptInvalidHostnames: true }, + }, + ); + } + // Handle authentication if needed if (!res.ok && (res.status === 401 || res.status === 403) && finalUsername && finalPassword) { const wwwAuthenticate = res.headers.get('WWW-Authenticate'); diff --git a/apps/readest-app/src/services/opds/autoDownload.ts b/apps/readest-app/src/services/opds/autoDownload.ts index ef7db912..7712d96e 100644 --- a/apps/readest-app/src/services/opds/autoDownload.ts +++ b/apps/readest-app/src/services/opds/autoDownload.ts @@ -76,6 +76,12 @@ async function downloadAndImport( url: downloadUrl, headers, singleThreaded: true, + // Same self-signed/private-CA workaround as the manual download path + // (#2871): the native downloader's rustls validation ignores the OS + // trust store, so without this flag auto-download fails the TLS + // handshake on servers where feed browsing and manual download work + // (#4988). + skipSslVerification: true, }); const probedFilename = await probeFilename(responseHeaders);