fix(opds): auth negotiation and auto-download fixes for self-hosted catalogs (#5002)

* fix(opds): allow LAN catalogs through the proxy in development The SSRF host blocklist added in #4638 unconditionally rejected private addresses, so the dev proxy returned 400 for LAN OPDS catalogs even though next dev runs on the developer's own machine where reaching the LAN is the normal use case. Skip the blocklist when NODE_ENV is development, matching the existing CatalogManager gate that only forbids LAN URLs in production. Production and test behavior are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(opds): negotiate Digest auth when the server rejects preemptive Basic with 400 Calibre's content server in digest mode, or auto mode over http, answers a Basic Authorization header with 400 Unsupported authentication method instead of a 401 challenge. The preemptive Basic header introduced in #4206 therefore dead-ended the request, since the auth retry only fired on 401 or 403, and digest catalogs failed with Failed to load OPDS feed: 400 Bad Request on every platform. When a request that carried preemptive Basic comes back 400, re-issue it once without credentials to surface the WWW-Authenticate challenge and let the existing negotiation pick the scheme the server actually wants. Verified end to end against a live Calibre server on web and Android. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(opds): skip SSL verification in auto-download like the manual download path The native download_file command validates TLS with rustls, which ignores the OS trust store, so downloads from self-signed or private-CA OPDS servers fail in the TLS handshake before any request reaches the server. The manual download path has passed skipSslVerification since #2900 as the workaround for #2871, but the auto-download path never did, so subscribed shelves failed to sync while feed browsing and manual downloads of the same books worked. Pass the same flag in downloadAndImport. Closes #4988 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-08 02:47:26 +09:00
parent 0c24aad606
commit 883dae36aa
10 changed files with 174 additions and 5 deletions
@@ -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
@@ -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.
@@ -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).
@@ -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/<pkg>-<hash>` 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 `<img src={getProxiedURL(...)}>` (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 <file>` 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.
@@ -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('<feed/>', {
+        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('<feed/>', {
@@ -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 },
@@ -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: '<feed/>' }));
+
+      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<string, string>;
+      expect(bareHeaders['Authorization']).toBeUndefined();
+      const digestInit = fetchMock.mock.calls[2]![1] as RequestInit;
+      const digestHeaders = digestInit.headers as Record<string, string>;
+      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: '<feed/>' }));
+
+      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(
@@ -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 });
  }

@@ -342,11 +342,14 @@ export const fetchWithAuth = async (
  const fetchURL = useProxy
    ? getProxiedURL(cleanUrl, preemptiveAuth || '', false, normalizedCustomHeaders)
    : cleanUrl;
-  const headers: Record<string, string> = {
+  const baseHeaders: Record<string, string> = {
    'User-Agent': READEST_OPDS_USER_AGENT,
    Accept: 'application/atom+xml, application/xml, text/xml, */*',
    ...(!useProxy ? normalizedCustomHeaders : {}),
    ...(options.headers as Record<string, string>),
+  };
+  const headers: Record<string, string> = {
+    ...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');
@@ -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);