From d519cdeecc3c992467034023b9ca0f3ace509261 Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Sun, 2 Aug 2026 02:15:20 -0400 Subject: [PATCH 1/6] fix(opds): harden v1 sharing experience --- README.md | 6 + crates/citadel-opds/src/auth.rs | 25 ++++ crates/citadel-opds/src/catalog.rs | 188 ++++++++++++++++++++++++++--- crates/citadel-opds/src/service.rs | 25 +++- crates/citadel-server/src/lib.rs | 1 + docs/README.md | 10 +- docs/opds-sharing.md | 87 +++++++++++++ docs/opds-validation.md | 87 +++++++++++++ src/bindings.ts | 2 +- 9 files changed, 408 insertions(+), 23 deletions(-) create mode 100644 docs/opds-sharing.md create mode 100644 docs/opds-validation.md diff --git a/README.md b/README.md index b09bdcda..e208122d 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,12 @@ Development builds are available from [GitHub actions](https://github.com/everyd Please report any issues or crashes you experience while using any version of Citadel! +### Sharing with KOReader + +The desktop app can share the active library as a read-only OPDS catalog on the +local network. See [Share a library with KOReader](docs/opds-sharing.md) for +setup, authentication, security limits, and troubleshooting. + ### Installing on macOS Download the `.dmg` from [Releases](https://github.com/everydaythingssoftware/citadel/releases), drag Citadel to Applications, and open it. diff --git a/crates/citadel-opds/src/auth.rs b/crates/citadel-opds/src/auth.rs index fece9ec7..fa78c2fd 100644 --- a/crates/citadel-opds/src/auth.rs +++ b/crates/citadel-opds/src/auth.rs @@ -23,6 +23,7 @@ const DEFAULT_CACHE_CAPACITY: usize = 256; const DEFAULT_CACHE_TTL: Duration = Duration::from_secs(30); const DEFAULT_TARGET_DURATION: Duration = Duration::from_millis(250); const BASIC_CHALLENGE: &str = "Basic realm=\"Citadel\""; +const MAX_AUTHORIZATION_BYTES: usize = 8 * 1024; #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct OpdsAuthCredentials { @@ -180,6 +181,10 @@ impl OpdsBasicAuth { let started_at = Instant::now(); let authorization = authorization.unwrap_or_default(); + if authorization.len() > MAX_AUTHORIZATION_BYTES { + pad_to_target(started_at, current_target_duration(enabled)).await; + return false; + } let tag = opaque_tag(&enabled.cache_key, authorization); if let Some((outcome, cached_duration)) = enabled .cache @@ -424,6 +429,26 @@ mod tests { assert_eq!(correct.status(), StatusCode::OK); } + #[tokio::test] + async fn oversized_authorization_is_rejected_without_hashing_or_caching_it() { + let auth = enabled_auth("reader", b"correct horse", Duration::ZERO); + let oversized = vec![b'x'; MAX_AUTHORIZATION_BYTES + 1]; + + let response = response(auth.clone(), Some(oversized.clone())).await; + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!(!auth.authorize(Some(&oversized)).await); + assert!(auth + .enabled + .as_ref() + .unwrap() + .cache + .lock() + .unwrap() + .entries + .is_empty()); + } + #[tokio::test] async fn cache_records_and_reuses_the_authorization_outcome_without_raw_header_bytes() { let auth = enabled_auth("reader", b"correct horse", Duration::ZERO); diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index 36910d65..8aad586f 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -302,20 +302,20 @@ async fn search_feed(state: State, Query(query): Query) -> Response { - facet_handler(state, CatalogFacetKind::Authors).await +async fn authors_feed(state: State, query: Query) -> Response { + facet_handler(state, query, CatalogFacetKind::Authors).await } -async fn series_feed(state: State) -> Response { - facet_handler(state, CatalogFacetKind::Series).await +async fn series_feed(state: State, query: Query) -> Response { + facet_handler(state, query, CatalogFacetKind::Series).await } -async fn tags_feed(state: State) -> Response { - facet_handler(state, CatalogFacetKind::Tags).await +async fn tags_feed(state: State, query: Query) -> Response { + facet_handler(state, query, CatalogFacetKind::Tags).await } -async fn genres_feed(state: State) -> Response { - facet_handler(state, CatalogFacetKind::Genres).await +async fn genres_feed(state: State, query: Query) -> Response { + facet_handler(state, query, CatalogFacetKind::Genres).await } async fn opensearch_description() -> Response { @@ -413,7 +413,15 @@ impl CatalogFacetKind { } } -async fn facet_handler(State(state): State, kind: CatalogFacetKind) -> Response { +async fn facet_handler( + State(state): State, + Query(query): Query, + kind: CatalogFacetKind, +) -> Response { + let page_number = query.page.unwrap_or(1); + if page_number == 0 { + return public_error(StatusCode::BAD_REQUEST, "Invalid page"); + } let source = state.source.clone(); let result = tokio::task::spawn_blocking(move || { let library_uuid = source.active_library_id()?; @@ -431,7 +439,26 @@ async fn facet_handler(State(state): State, kind: CatalogFacetKind Ok(Err(error)) => return calibre_error(error), Err(_) => return public_error(StatusCode::INTERNAL_SERVER_ERROR, "Catalog unavailable"), }; - match facet_navigation_feed(&library_uuid, kind, &facets) { + let last_page = page_count(facets.len() as i64); + if page_number > last_page { + return public_error(StatusCode::NOT_FOUND, "Page not found"); + } + let offset = match page_number + .checked_sub(1) + .and_then(|page| page.checked_mul(PAGE_SIZE as u64)) + .and_then(|offset| usize::try_from(offset).ok()) + { + Some(offset) => offset, + None => return public_error(StatusCode::BAD_REQUEST, "Invalid page"), + }; + let facets = facets + .get(offset..) + .unwrap_or_default() + .iter() + .take(PAGE_SIZE as usize) + .cloned() + .collect::>(); + match facet_navigation_feed(&library_uuid, kind, &facets, page_number, last_page) { Ok(xml) => xml_response(NAVIGATION_CONTENT_TYPE, xml), Err(_) => public_error(StatusCode::INTERNAL_SERVER_ERROR, "Catalog unavailable"), } @@ -737,9 +764,31 @@ fn facet_navigation_feed( library_uuid: &str, kind: CatalogFacetKind, facets: &[CatalogFacet], + page_number: u64, + last_page: u64, ) -> Result, quick_xml::Error> { - let mut writer = navigation_writer(library_uuid, kind.title(), kind.route())?; + let mut writer = navigation_writer( + library_uuid, + kind.title(), + &page_href(kind.route(), page_number), + )?; link(&mut writer, "up", "/opds", NAVIGATION_TYPE)?; + if page_number > 1 { + link( + &mut writer, + "previous", + &page_href(kind.route(), page_number - 1), + NAVIGATION_TYPE, + )?; + } + if page_number < last_page { + link( + &mut writer, + "next", + &page_href(kind.route(), page_number + 1), + NAVIGATION_TYPE, + )?; + } for facet in facets { navigation_entry( &mut writer, @@ -1020,12 +1069,13 @@ mod tests { books: Vec, } + struct FacetSource { + facets: Vec, + } + impl CatalogSource for MemorySource { fn active_library_id(&self) -> Result { - self.books - .first() - .map(|_| "memory-library".to_string()) - .ok_or(CalibreError::LibraryNotInitialized) + Ok("550e8400-e29b-41d4-a716-446655440000".to_string()) } fn book_page( @@ -1062,6 +1112,54 @@ mod tests { } } + impl CatalogSource for FacetSource { + fn active_library_id(&self) -> Result { + Ok("550e8400-e29b-41d4-a716-446655440000".to_string()) + } + + fn book_page( + &self, + _query: CatalogBookQuery, + ) -> Result<(String, Option, BookPage), CalibreError> { + Ok(( + "550e8400-e29b-41d4-a716-446655440000".to_string(), + None, + BookPage { + items: Vec::new(), + total: 0, + }, + )) + } + + fn authors(&self) -> Result, CalibreError> { + Ok(self.facets.clone()) + } + + fn series(&self) -> Result, CalibreError> { + Ok(self.facets.clone()) + } + + fn tags(&self) -> Result, CalibreError> { + Ok(self.facets.clone()) + } + + fn genres(&self) -> Result, CalibreError> { + Ok(self.facets.clone()) + } + + fn book_file( + &self, + book_id: BookId, + format: &str, + ) -> Result { + Err(CalibreError::BookFileNotFound(book_id, format.to_string())) + } + + fn book_cover(&self, book_id: BookId) -> Result { + Err(CalibreError::BookCoverNotFound(book_id)) + } + } + struct LibrarySource { library: Mutex, } @@ -1484,6 +1582,66 @@ mod tests { server.abort(); } + #[tokio::test] + async fn facet_navigation_routes_are_bounded_and_paginated() { + let facets = (1..=101) + .map(|id| CatalogFacet { + id, + title: format!("Facet {id:03}"), + book_count: Some(i64::from(id)), + }) + .collect(); + let (base, server) = loopback(Arc::new(FacetSource { facets })).await; + let client = reqwest::Client::new(); + + for route in [ + "/opds/authors", + "/opds/series", + "/opds/tags", + "/opds/genres", + ] { + let second_page = format!("{route}?page=2"); + let third_page = format!("{route}?page=3"); + let first = client.get(format!("{base}{route}")).send().await.unwrap(); + assert_eq!(first.status(), StatusCode::OK, "{route} page 1"); + let first = parsed_feed(&first.bytes().await.unwrap()); + assert_eq!(first.titles.len(), 50, "{route} page 1"); + assert!(first.previous.is_none(), "{route} page 1"); + assert_eq!(first.next.as_deref(), Some(second_page.as_str())); + + let second = client + .get(format!("{base}{route}?page=2")) + .send() + .await + .unwrap(); + assert_eq!(second.status(), StatusCode::OK, "{route} page 2"); + let second = parsed_feed(&second.bytes().await.unwrap()); + assert_eq!(second.titles.len(), 50, "{route} page 2"); + assert_eq!(second.previous.as_deref(), Some(route)); + assert_eq!(second.next.as_deref(), Some(third_page.as_str())); + + let third = client + .get(format!("{base}{route}?page=3")) + .send() + .await + .unwrap(); + assert_eq!(third.status(), StatusCode::OK, "{route} page 3"); + let third = parsed_feed(&third.bytes().await.unwrap()); + assert_eq!(third.titles, ["Facet 101"], "{route} page 3"); + assert_eq!(third.previous.as_deref(), Some(second_page.as_str())); + assert!(third.next.is_none(), "{route} page 3"); + + let missing = client + .get(format!("{base}{route}?page=4")) + .send() + .await + .unwrap(); + assert_eq!(missing.status(), StatusCode::NOT_FOUND, "{route} page 4"); + } + + server.abort(); + } + #[tokio::test] async fn search_is_limited_and_preserves_query_in_pagination_links() { let books = (0..51) diff --git a/crates/citadel-opds/src/service.rs b/crates/citadel-opds/src/service.rs index f7b46d8a..0cb22539 100644 --- a/crates/citadel-opds/src/service.rs +++ b/crates/citadel-opds/src/service.rs @@ -70,6 +70,7 @@ pub enum OpdsErrorCode { InterfaceUnavailable, InterfaceEnumerationFailed, PortUnavailable, + PermissionDenied, ListenerFailed, InvalidCredentials, CredentialsRequired, @@ -905,16 +906,19 @@ fn interface_enumeration_error(retrying: bool) -> OpdsStatusError { } fn bind_error(error: io::Error) -> OpdsStatusError { - if error.kind() == io::ErrorKind::AddrInUse { - OpdsStatusError { + match error.kind() { + io::ErrorKind::AddrInUse => OpdsStatusError { code: OpdsErrorCode::PortUnavailable, message: "That port is already in use on the selected network interface.".to_string(), - } - } else { - OpdsStatusError { + }, + io::ErrorKind::PermissionDenied => OpdsStatusError { + code: OpdsErrorCode::PermissionDenied, + message: "The operating system denied access to the requested network listener. Check firewall and local-network permissions.".to_string(), + }, + _ => OpdsStatusError { code: OpdsErrorCode::Unexpected, message: "Citadel could not open the requested OPDS listener.".to_string(), - } + }, } } @@ -928,6 +932,15 @@ mod tests { use super::*; use crate::network::{AddressScope, InterfaceAddress, OpdsInterfaceKind, OpdsInterfaceState}; + #[test] + fn bind_permission_denial_is_actionable() { + let error = bind_error(io::Error::from(io::ErrorKind::PermissionDenied)); + + assert_eq!(error.code, OpdsErrorCode::PermissionDenied); + assert!(error.message.contains("firewall")); + assert!(error.message.contains("local-network permissions")); + } + struct TestSource { library_id: Option, } diff --git a/crates/citadel-server/src/lib.rs b/crates/citadel-server/src/lib.rs index 16463557..33856692 100644 --- a/crates/citadel-server/src/lib.rs +++ b/crates/citadel-server/src/lib.rs @@ -291,6 +291,7 @@ fn is_fatal_startup(status: &OpdsServiceStatus) -> bool { | OpdsErrorCode::CredentialsRequired | OpdsErrorCode::CredentialStorageFailed | OpdsErrorCode::ConfigurationConflict + | OpdsErrorCode::PermissionDenied | OpdsErrorCode::Unexpected ) ) || status.state == OpdsLifecycleState::Stopped diff --git a/docs/README.md b/docs/README.md index 933ae91b..f508d6b6 100644 --- a/docs/README.md +++ b/docs/README.md @@ -18,6 +18,12 @@ browser engine installed on the user's system. - **[Updater and release operations](./updater-and-releases.md)** - Tauri updater setup, GitHub release flow, and verification checklist +## OPDS + +- **[Share a library with KOReader](./opds-sharing.md)** - Desktop setup, optional Basic authentication, network limits, and troubleshooting +- **[OPDS v1 validation record](./opds-validation.md)** - Automated evidence and the physical package/KOReader acceptance matrix +- **[Headless OPDS server](./headless-server.md)** - Run the Tauri-independent server process from an explicit configuration file + **Start here if you're:** - πŸš€ **New to the project**: Read the Overview below, then [Architecture Recommendations](../ai-docs/ARCHITECTURE_RECOMMENDATIONS.md) - πŸ”§ **Adding a feature**: Check [Patterns Quick Reference](../ai-docs/PATTERNS_QUICK_REFERENCE.md) @@ -25,7 +31,9 @@ browser engine installed on the user's system. ## Overview -Citadel ships as a single bundled desktop app; a headless server & web app is exploratory only. +Citadel ships as a bundled desktop app. A separate headless OPDS server uses +the same core runtime without a WebView; a remotely managed web app remains +exploratory.
Diagram showing that the UI has a Calibre client that uses IPC to talk to the backend's calibre adapter, which calls out to libcalibre. Space is left open to demonstrate that other clients and adapters are possible. diff --git a/docs/opds-sharing.md b/docs/opds-sharing.md new file mode 100644 index 00000000..50970e3a --- /dev/null +++ b/docs/opds-sharing.md @@ -0,0 +1,87 @@ +# Share a library with KOReader + +Citadel can expose the active Calibre library as a read-only OPDS catalog while +the desktop app is running. Current KOReader is the supported v1 client; other +OPDS readers may work but are not yet part of Citadel's compatibility promise. + +## Start sharing + +1. Open Citadel's **Settings**, then **Sharing**. +2. Choose **All local networks** or the specific Wi-Fi/Ethernet interface that + should host the catalog. +3. Keep the default port, `8080`, unless it conflicts with another service. +4. Optionally enable **Require a password** and set a username and password. + Citadel can generate a password, but shows it only once. Save it before + leaving the pane. +5. Select **Start Sharing**. +6. Copy one of the concrete catalog URLs shown by Citadel. Do not add the + username or password to the URL. + +Only the current active library is shared. Switching libraries changes the +served catalog. Sharing stops when Citadel quits and must be started again +after every launch; the network, port, and authentication settings remain +saved. + +## Add the catalog to KOReader + +The KOReader device must be able to reach the selected computer interface. +Usually that means both devices are on the same local network. + +1. In KOReader's File Browser, open the top menu and choose **OPDS catalog**. +2. Choose **Add new OPDS catalog**. +3. Enter a name such as `Citadel` and paste the URL copied from Citadel. +4. If authentication is enabled, enter the username and password in KOReader's + credential fields. +5. Open the new catalog. + +The root contains All Books, Recently Modified, Unread, Authors, Series, Tags, +and Genres, plus search. Genre is separate from arbitrary Calibre tags and is +populated only after genres have been accepted into Citadel's `Genres` custom +column. Books and covers are downloaded directly from the active library; +OPDS cannot edit the library or synchronize reading progress. + +KOReader's current user guide documents the OPDS catalog entry point: +. + +## Network and security limits + +Citadel serves plain HTTP. A Basic-auth password prevents unauthenticated +browsing, but the credentials and downloaded books are not encrypted in +transit. Use sharing only on a network you trust. Citadel does not configure +the operating-system firewall and does not provide HTTPS, remote internet +exposure, Bonjour/mDNS discovery, or a QR code in v1. + +**All local networks** binds Citadel to the concrete addresses of eligible +local Wi-Fi/Ethernet interfaces; it does not use a wildcard listener. Choosing +one interface prevents Citadel from silently broadening to another interface. +If that interface disappears, Citadel waits for the same interface to return. + +Citadel advertises usable IPv4 and global/unique-local IPv6 addresses. It does +not advertise IPv6 link-local URLs because their zone identifiers are not +portable between the computer and reader. If a reader cannot route a displayed +IPv6 address, use the displayed IPv4 URL instead. + +## Troubleshooting + +- **KOReader cannot open the catalog:** confirm sharing still says **Sharing**, + use a URL currently displayed by Citadel, and check that both devices can + communicate on the selected network. Guest Wi-Fi often isolates devices. +- **Authentication keeps failing:** edit credentials only while sharing is + stopped, then restart sharing. Enter them in KOReader's username/password + fields, not in the catalog URL. +- **Citadel is waiting for the network:** reconnect the selected interface or + stop sharing and choose another interface. Citadel will not fall back to a + broader listener automatically. +- **The port is already in use:** stop the conflicting service or choose a + different port in Citadel before starting again. +- **macOS asks about incoming connections:** allow them for Citadel if the + catalog should be reachable. Signed and unsigned builds can receive different + firewall treatment. +- **Linux cannot be reached:** allow the selected TCP port in the host firewall. + Citadel intentionally does not modify firewall rules. +- **The wrong books appear:** stop sharing if needed, select the intended + library in Citadel, and reopen the catalog. Only one active library is served. + +Stopping sharing or quitting Citadel closes every listener. If a listener still +appears reachable after the app exits, record the Citadel version, platform, +selected target, and catalog URL when reporting the defect. diff --git a/docs/opds-validation.md b/docs/opds-validation.md new file mode 100644 index 00000000..57729e7f --- /dev/null +++ b/docs/opds-validation.md @@ -0,0 +1,87 @@ +# OPDS v1 validation record + +This record separates repeatable automated coverage from the physical package +and reader checks required by CDL-27. Do not mark an unexecuted platform or +client scenario as passing. Record exact versions, artifact identity, network +path, and fixture details when running the manual matrix. + +## Current evidence + +### Local macOS package inspection β€” 2026-08-02 + +| Field | Result | +| --- | --- | +| Host | macOS, Apple Silicon | +| Command | `bun run build` | +| Artifacts | `Citadel.app`, `Citadel_0.6.1_aarch64.dmg` | +| Signature | Ad-hoc/linker signature only; no Team ID | +| Gatekeeper | Rejected because the local artifact is not Developer ID signed | +| Notarization | Not tested; release credentials are unavailable to the local build | +| Bundled helpers | None; the app contains the Citadel executable, icon, and empty-library resource | +| WebDriver | Registration is guarded by `debug_assertions`; no WebDriver listener is started by release code | +| OPDS-specific WebView permission | None; management remains Tauri IPC and readers access the native Rust listener | +| Local-network usage description | None added; packaged listener behavior still requires physical verification on supported macOS releases | + +The release workflow supplies Apple signing and notarization credentials and is +the correct source for the signed/notarized artifact. A successful local bundle +does not substitute for testing that release artifact. + +### Existing real-reader evidence β€” 2026-08-02 + +The current branch was successfully opened from KOReader over the developer +machine's `en0` interface using HTTP Basic credentials. The KOReader version, +device/OS, Citadel package identity, authentication-disabled case, and full +navigation/download matrix were not recorded, so this is a useful v1 proof but +not a completed CDL-27 package result. + +## Automated coverage + +| Contract | Coverage | +| --- | --- | +| OPDS XML, escaping, optional metadata, search, and acquisition pagination | `crates/citadel-opds/src/catalog.rs` tests | +| Cover/book GET, HEAD, ranges, and bounded streaming | `crates/citadel-opds/src/assets.rs` tests | +| Library path containment and symlink escape rejection | `crates/libcalibre/tests/asset_resolution_test.rs` | +| Basic challenge, success/failure, verifier storage, bounded cache, and timing padding | `crates/citadel-opds/src/auth.rs` and `credentials.rs` tests | +| Interface selection, IPv4/IPv6 planning, loss/recovery, and no exposure broadening | `crates/citadel-opds/src/network.rs` and `service.rs` tests | +| Port conflict, stop/restart, shutdown timeout, and listener release | `crates/citadel-opds/src/service.rs` tests | +| Active-library switch during concurrent feed/download traffic | `src-tauri/src/state.rs` tests | +| Real headless process auth, catalog, acquisition, and signal shutdown | `crates/citadel-server/tests/headless_process.rs` | + +Automation does not prove OS firewall UI, package signing/notarization, +cross-device routing, or KOReader's behavior on a particular device build. + +## Physical package and KOReader matrix + +Use a representative library with more than one catalog page, Unicode and XML +metacharacters, multiple authors/series/tags/genres, mixed read state, missing +optional metadata, covers and missing covers, multiple formats, and a large +book. Record the fixture identity and whether it is disposable. + +| Platform/artifact | KOReader device/version | Target | Auth | Result/notes | +| --- | --- | --- | --- | --- | +| Signed/notarized macOS release | β€” | Specific Wi-Fi/Ethernet | Off | Not run | +| Signed/notarized macOS release | β€” | Specific Wi-Fi/Ethernet | Basic | Not run | +| Signed/notarized macOS release | β€” | All local networks | Off + Basic | Not run | +| Unsigned macOS development package | β€” | Specific Wi-Fi/Ethernet | Off + Basic | Authenticated `en0` smoke test only; exact versions missing | +| Ubuntu `.deb` | β€” | Specific Wi-Fi/Ethernet | Off + Basic | Not run | +| Ubuntu AppImage | β€” | All local networks | Off + Basic | Not run | + +For every applicable row, verify and record: + +- Manual URL entry and root navigation. +- All Books, Recently Modified, Unread, Authors, Series, Tags, Genres, and + KOReader search. +- First/middle/last-page navigation without duplicates or omissions. +- Cover display and download/open for every fixture format. +- Missing, incorrect, and correct credentials; copied URLs contain no secrets. +- Stop/start in one launch and restart-off behavior with settings preserved. +- Active-library switching while sharing. +- Selected-interface loss, return, and address change. +- Port conflict plus macOS privacy/firewall or Linux firewall denial. +- Concurrent browsing and large download responsiveness. +- Listener and port release after Stop Sharing and after quitting Citadel. + +Record in-scope defects with reproduction steps and add an automated regression +where practical. Propose excluded features separately; do not expand this +matrix to HTTPS, Bonjour/mDNS, QR codes, remote deployment, or additional client +support promises. diff --git a/src/bindings.ts b/src/bindings.ts index 1964330a..46f33af5 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -580,7 +580,7 @@ export type MetadataProvider = "hardcover" | "loc" | "dnb" | "k10plus" | "openli export type NewAuthor = { name: string; sortable_name: string | null } export type OpdsBindTarget = { type: "allLocalNetworks" } | { type: "interface"; id: string } | { type: "addresses"; addresses: string[] } export type OpdsCredentialStatus = { configured: boolean; username: string | null } -export type OpdsErrorCode = "invalidPort" | "libraryNotReady" | "configurationConflict" | "interfaceUnavailable" | "interfaceEnumerationFailed" | "portUnavailable" | "listenerFailed" | "invalidCredentials" | "credentialsRequired" | "credentialStorageFailed" | "unexpected" +export type OpdsErrorCode = "invalidPort" | "libraryNotReady" | "configurationConflict" | "interfaceUnavailable" | "interfaceEnumerationFailed" | "portUnavailable" | "permissionDenied" | "listenerFailed" | "invalidCredentials" | "credentialsRequired" | "credentialStorageFailed" | "unexpected" export type OpdsInterfaceKind = "lan" | "vpn" | "loopback" | "other" export type OpdsInterfaceState = "up" | "down" export type OpdsLifecycleState = "stopped" | "starting" | "running" | "waitingForInterface" | "error" | "stopping" From ef7c197110a8fa0f55c2ab2b756289b48235fea2 Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Sun, 2 Aug 2026 02:15:20 -0400 Subject: [PATCH 2/6] docs(opds): record packaged macOS smoke test --- docs/opds-validation.md | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/opds-validation.md b/docs/opds-validation.md index 57729e7f..3aa573c5 100644 --- a/docs/opds-validation.md +++ b/docs/opds-validation.md @@ -26,6 +26,42 @@ The release workflow supplies Apple signing and notarization credentials and is the correct source for the signed/notarized artifact. A successful local bundle does not substitute for testing that release artifact. +### Isolated packaged-app smoke test β€” 2026-08-02 + +The release app was rebuilt with the compile-time QA identifier +`software.everydaythings.citadel.opdsqa` and run on macOS 26.5.1 (25F80, +arm64). It used a separate settings directory, a disposable empty Calibre +library, port 18080, and QA-only credentials. The installed Citadel process, +production settings, and production library were not changed. + +Passed: + +- The packaged Sharing pane started and stopped **All local networks** and the + specific Wi-Fi interface `en0`. +- The UI displayed one concrete IPv4 URL and three bracketed global IPv6 URLs; + it displayed no wildcard, loopback, link-local, or credential-bearing URL. +- IPv4 and global IPv6 returned parseable XML with HTTP 200 for the root, All + Books, Recently Modified, Unread, Authors, Series, Tags, Genres, search, and + OpenSearch routes while authentication was disabled. +- With Basic authentication enabled, missing and incorrect credentials returned + 401 with `WWW-Authenticate: Basic realm="Citadel"`; correct credentials + returned 200. An authorization value over the explicit limit returned 401. +- The credential file was mode 0600, contained the username and an Argon2id + verifier, and neither it nor the QA settings contained the plaintext password. +- Stop Sharing closed every listener and released the port. Quitting while + sharing also closed the listener. On relaunch, sharing was off while the + selected target, port, username, authentication mode, and verifier remained + available; restarting with the stored verifier authenticated successfully. +- Occupying the selected address/port produced the packaged UI state **Needs + attention** with β€œThat port is already in use on the selected network + interface.” Stopping recovered to editable configuration. + +This was a same-host network smoke test against an empty library. It does not +cover a signed/notarized current artifact, OS firewall denial, a separate +KOReader device, representative books/covers/acquisitions, active-library +switching, or interface loss/recovery. The QA app, isolated settings/verifier, +and disposable library were moved to Trash after the run. + ### Existing real-reader evidence β€” 2026-08-02 The current branch was successfully opened from KOReader over the developer @@ -62,7 +98,7 @@ book. Record the fixture identity and whether it is disposable. | Signed/notarized macOS release | β€” | Specific Wi-Fi/Ethernet | Off | Not run | | Signed/notarized macOS release | β€” | Specific Wi-Fi/Ethernet | Basic | Not run | | Signed/notarized macOS release | β€” | All local networks | Off + Basic | Not run | -| Unsigned macOS development package | β€” | Specific Wi-Fi/Ethernet | Off + Basic | Authenticated `en0` smoke test only; exact versions missing | +| Ad-hoc macOS QA package | Same-host HTTP client | All local networks + `en0` | Off + Basic | Passed empty-library route/auth/lifecycle/IPv4/IPv6/port-conflict smoke test on macOS 26.5.1; not a KOReader result | | Ubuntu `.deb` | β€” | Specific Wi-Fi/Ethernet | Off + Basic | Not run | | Ubuntu AppImage | β€” | All local networks | Off + Basic | Not run | From 18587e4f4fdaff8a39d3b595a947fb7c44aa6c61 Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Sun, 2 Aug 2026 02:15:20 -0400 Subject: [PATCH 3/6] test(opds): add representative package fixture --- .../create_opds_validation_library.rs | 130 ++++++++++++++++++ docs/opds-validation.md | 47 ++++++- 2 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 crates/libcalibre/examples/create_opds_validation_library.rs diff --git a/crates/libcalibre/examples/create_opds_validation_library.rs b/crates/libcalibre/examples/create_opds_validation_library.rs new file mode 100644 index 00000000..d560205e --- /dev/null +++ b/crates/libcalibre/examples/create_opds_validation_library.rs @@ -0,0 +1,130 @@ +use std::{collections::HashMap, env, fs, path::PathBuf}; + +use chrono::NaiveDate; +use libcalibre::{util::get_db_path, BookAdd, BookUpdate, Library}; + +const ACQUIRABLE_BOOKS: usize = 105; + +fn main() -> Result<(), Box> { + let mut arguments = env::args_os().skip(1); + let target = arguments + .next() + .map(PathBuf::from) + .ok_or("usage: create_opds_validation_library TARGET SOURCE_EPUB")?; + let source_epub = arguments + .next() + .map(PathBuf::from) + .ok_or("usage: create_opds_validation_library TARGET SOURCE_EPUB")?; + if arguments.next().is_some() { + return Err("usage: create_opds_validation_library TARGET SOURCE_EPUB".into()); + } + if target.exists() && fs::read_dir(&target)?.next().is_some() { + return Err(format!("target is not empty: {}", target.display()).into()); + } + if source_epub.extension().and_then(|value| value.to_str()) != Some("epub") { + return Err("SOURCE_EPUB must use the .epub extension".into()); + } + + fs::create_dir_all(&target)?; + let fixture = + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/empty_library/metadata.db"); + fs::copy(fixture, target.join("metadata.db"))?; + let database = get_db_path(target.to_str().ok_or("target path is not UTF-8")?) + .ok_or("target is not a Calibre library")?; + let mut library = Library::new(database)?; + let large_source = target.join(".validation-large.txt"); + let coverless_source = target.join(".validation-coverless.txt"); + fs::write(&large_source, vec![b'L'; 8 * 1024 * 1024])?; + fs::write(&coverless_source, b"Coverless validation fixture\n")?; + + for index in 0..=ACQUIRABLE_BOOKS { + let is_fileless = index == ACQUIRABLE_BOOKS; + let title = if index == 0 { + "A & B β€” 東京".to_string() + } else { + format!("Validation Book {index:03}") + }; + let authors = if index == 0 { + vec!["ZoΓ« & Co.".to_string(), "李 小龍".to_string()] + } else { + vec![format!("Author {:03}", index % 61)] + }; + let series = (index % 2 == 0).then(|| format!("Series {:02}", index % 7)); + let tags = match index % 3 { + 0 => vec!["Science Fiction".to_string(), "Tag & ".to_string()], + 1 => vec!["Mystery".to_string()], + _ => Vec::new(), + }; + let book = library.add_book(BookAdd { + title, + author_names: authors, + tags: Some(tags), + series, + series_index: Some(index as f32 / 2.0 + 0.5), + publisher: None, + publication_date: Some(NaiveDate::from_ymd_opt(2020, 1, 1).unwrap()), + rating: None, + comments: None, + identifiers: HashMap::new(), + language: Some(if index % 2 == 0 { "en" } else { "fr" }.to_string()), + file_paths: if is_fileless { + Vec::new() + } else if index == 0 { + vec![large_source.clone(), source_epub.clone()] + } else if index % 2 == 1 { + vec![coverless_source.clone(), source_epub.clone()] + } else { + vec![source_epub.clone()] + }, + })?; + + library.upsert_book_identifier( + book.id, + "isbn".to_string(), + format!("9780000{index:06}"), + None, + )?; + + library.update_book( + book.id, + BookUpdate { + title: None, + author_names: None, + author_ids: None, + description: (index % 4 != 0) + .then(|| "Escaped & Unicode cafΓ© 東京".to_string()), + is_read: Some(index % 3 == 0), + tags: None, + series: None, + series_index: None, + language_codes: None, + publisher: (index % 5 != 0).then(|| "Fixture Press".to_string()), + publication_date: None, + rating: None, + comments: None, + identifiers: None, + }, + )?; + + if index % 3 != 2 { + library.add_book_genres( + book.id, + vec![ + if index % 2 == 0 { + "Speculative Fiction" + } else { + "Mystery" + } + .to_string(), + "Fixture Genre".to_string(), + ], + )?; + } + } + + drop(library); + fs::remove_file(large_source)?; + fs::remove_file(coverless_source)?; + println!("{}", target.display()); + Ok(()) +} diff --git a/docs/opds-validation.md b/docs/opds-validation.md index 3aa573c5..45bf57d5 100644 --- a/docs/opds-validation.md +++ b/docs/opds-validation.md @@ -62,6 +62,50 @@ KOReader device, representative books/covers/acquisitions, active-library switching, or interface loss/recovery. The QA app, isolated settings/verifier, and disposable library were moved to Trash after the run. +### Representative packaged-app crawl β€” 2026-08-02 + +The same isolated, ad-hoc macOS QA package was run again on `en0` with +authentication disabled and a disposable library generated by +`create_opds_validation_library`. The fixture contained 106 books: 105 EPUB +acquisitions, 53 TXT acquisitions, 52 covered acquisitions, 53 coverless +acquisitions, and one fileless book that must not appear in acquisition feeds. +It also included multiple pages of books and authors, mixed read state, +multiple authors/series/tags/genres, missing optional metadata, identifiers, +Unicode and XML metacharacters, and one 8 MiB TXT file. + +Passed: + +- Root navigation exposed All Books, Recently Modified, Unread, Authors, + Series, Tags, Genres, and OpenSearch. +- All Books and Recently Modified returned 105 unique acquisitions over pages + of 50/50/5. Unread returned the expected 70 over pages of 50/20. The + fileless book was excluded. +- Authors returned 63 unique facets over pages of 50/13; Series returned 7, + Tags 3, and Genres 3. A child feed from every facet type returned acquisition + entries. +- Search returned 104 title matches over pages of 50/50/4. A Unicode query + returned the exact `A & B β€” 東京` title, both Unicode authors, escaped + categories, canonical language, and ISBN identifier; its intentionally + missing description and cover remained absent. +- The cover endpoint returned JPEG data. EPUB GET bytes exactly matched the + source fixture, EPUB HEAD returned the expected length with no body, and a + TXT byte range returned 206 with the exact 1,024 requested bytes and correct + `Content-Range` for the 8 MiB file. +- A root feed completed in 3 ms while a deliberately slow client streamed the + 8 MiB download. Root, last-page, and search XML also parsed over a global + IPv6 address. +- While the listener remained running, switching through the packaged Library + pane from the representative library to a second empty library changed the + same URL to the second library UUID with zero acquisitions. Switching back + restored the first UUID and its acquisitions without restarting sharing. +- Stop Sharing released the listener and quitting the QA app left the port + closed. The QA app, isolated settings, generated EPUB, and disposable + libraries were moved to Trash after the run. + +This remains a same-host protocol crawl, not a separate-device KOReader result. +It does not cover a signed/notarized current artifact, OS firewall denial, +Linux packages, or physical interface loss/recovery. + ### Existing real-reader evidence β€” 2026-08-02 The current branch was successfully opened from KOReader over the developer @@ -82,6 +126,7 @@ not a completed CDL-27 package result. | Port conflict, stop/restart, shutdown timeout, and listener release | `crates/citadel-opds/src/service.rs` tests | | Active-library switch during concurrent feed/download traffic | `src-tauri/src/state.rs` tests | | Real headless process auth, catalog, acquisition, and signal shutdown | `crates/citadel-server/tests/headless_process.rs` | +| Disposable representative Calibre library | `crates/libcalibre/examples/create_opds_validation_library.rs` | Automation does not prove OS firewall UI, package signing/notarization, cross-device routing, or KOReader's behavior on a particular device build. @@ -98,7 +143,7 @@ book. Record the fixture identity and whether it is disposable. | Signed/notarized macOS release | β€” | Specific Wi-Fi/Ethernet | Off | Not run | | Signed/notarized macOS release | β€” | Specific Wi-Fi/Ethernet | Basic | Not run | | Signed/notarized macOS release | β€” | All local networks | Off + Basic | Not run | -| Ad-hoc macOS QA package | Same-host HTTP client | All local networks + `en0` | Off + Basic | Passed empty-library route/auth/lifecycle/IPv4/IPv6/port-conflict smoke test on macOS 26.5.1; not a KOReader result | +| Ad-hoc macOS QA package | Same-host HTTP client | All local networks + `en0` | Off + Basic | Passed empty-library auth/lifecycle/port-conflict smoke plus representative pagination/facet/search/metadata/cover/download/range/concurrency crawl over IPv4 and IPv6 on macOS 26.5.1; not a KOReader result | | Ubuntu `.deb` | β€” | Specific Wi-Fi/Ethernet | Off + Basic | Not run | | Ubuntu AppImage | β€” | All local networks | Off + Basic | Not run | From 715bbbc7673a9ba246c3bde761f18d6b17e7cf95 Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Sun, 2 Aug 2026 02:15:20 -0400 Subject: [PATCH 4/6] test(opds): validate Linux packages --- .github/workflows/build.yml | 2 +- .github/workflows/release.yml | 2 +- docs/opds-validation.md | 64 +++++++++++++++++++++++++++++------ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0a241227..49f28503 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -35,7 +35,7 @@ jobs: if: matrix.platform == 'ubuntu-22.04' run: | sudo apt-get update - sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.1-dev libappindicator3-dev librsvg2-dev patchelf + sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.1-dev libappindicator3-dev librsvg2-dev patchelf xdg-utils - name: Install node packages run: bun install diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 147d1eef..8b31f4a4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -165,7 +165,7 @@ jobs: if: matrix.platform == 'ubuntu-22.04' run: | sudo apt-get update - sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.1-dev libappindicator3-dev librsvg2-dev patchelf + sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.1-dev libappindicator3-dev librsvg2-dev patchelf xdg-utils - name: Install node packages run: bun install diff --git a/docs/opds-validation.md b/docs/opds-validation.md index 45bf57d5..4f2793e7 100644 --- a/docs/opds-validation.md +++ b/docs/opds-validation.md @@ -24,7 +24,11 @@ path, and fixture details when running the manual matrix. The release workflow supplies Apple signing and notarization credentials and is the correct source for the signed/notarized artifact. A successful local bundle -does not substitute for testing that release artifact. +does not substitute for testing that release artifact. The repository has all +Apple certificate and App Store Connect API secret names configured, and the +most recent manual Release run completed both its macOS and Ubuntu jobs on +2026-07-10/11. That proves the publishing pipeline and credentials worked for +main commit `f0ec58ee`; it does not prove or publish the current OPDS commits. ### Isolated packaged-app smoke test β€” 2026-08-02 @@ -103,16 +107,55 @@ Passed: libraries were moved to Trash after the run. This remains a same-host protocol crawl, not a separate-device KOReader result. -It does not cover a signed/notarized current artifact, OS firewall denial, -Linux packages, or physical interface loss/recovery. +It does not cover a signed/notarized current artifact, OS firewall denial, a +Linux OPDS client path, or physical interface loss/recovery. + +### Linux package smoke test β€” 2026-08-02 + +Commit `2b864c0b` was exported without the developer worktree's unrelated +uncommitted files and built in an Ubuntu 24.04.1 LTS ARM64 guest with the pinned +Rust 1.97.1 toolchain and Bun 1.3.14. + +Passed: + +- The production frontend and Tauri release binary built successfully. +- `Citadel_0.6.1_arm64.deb` was produced, reported package/version/architecture + `citadel`/`0.6.1`/`arm64`, declared its WebKitGTK and GTK runtime + dependencies, installed through `apt`, and installed the application binary, + desktop entry, icons, and empty-library resource. +- The installed `.deb` application stayed running for the ten-second headless + smoke window under Xvfb, created isolated settings, and had no immediate + loader, dependency, or startup failure. +- `Citadel_0.6.1_aarch64.AppImage` was produced as an ARM64 ELF AppImage. Its + extracted launcher started the bundled Citadel executable, which stayed + running for the headless smoke window with no immediate loader or startup + failure. +- SHA-256 was + `59a02cdc58b70a68205f0cb7e588c75fa28e4952fc2f9e4ba1fd47c2030d21d3` + for the 13 MiB `.deb` and + `45e3ba608baaf5f47829279fe1c0ce03ca0917949fb72505cfa46cd20c0b6b88` + for the 83 MiB AppImage. + +The first AppImage bundle attempt on the minimal guest exposed that Tauri's +bundler requires `/usr/bin/xdg-open`. Installing `xdg-utils` fixed the bundle; +the build and release workflows now declare that prerequisite instead of +relying on the hosted runner image. + +This proves clean ARM64 package construction, installation, contents, dynamic +loading, and startup. It does not claim an interactive Linux OPDS or KOReader +result: sharing deliberately starts off, and the headless guest did not provide +a safe physical LAN/client path for enabling and exercising it. Linux host +firewall behavior was not changed or tested. ### Existing real-reader evidence β€” 2026-08-02 -The current branch was successfully opened from KOReader over the developer -machine's `en0` interface using HTTP Basic credentials. The KOReader version, -device/OS, Citadel package identity, authentication-disabled case, and full -navigation/download matrix were not recorded, so this is a useful v1 proof but -not a completed CDL-27 package result. +An early development build (`bun run dev`) was successfully opened from +KOReader on a Kobo Libra Colour over the developer machine's `en0` interface +using HTTP Basic credentials. That build exposed only the flat book list, +before the current category navigation was implemented. The KOReader version, +authentication-disabled case, acquisition/download behavior, and the current +navigation matrix were not recorded, so this is a useful real-device v1 proof +but not a completed packaged-client result. ## Automated coverage @@ -144,8 +187,9 @@ book. Record the fixture identity and whether it is disposable. | Signed/notarized macOS release | β€” | Specific Wi-Fi/Ethernet | Basic | Not run | | Signed/notarized macOS release | β€” | All local networks | Off + Basic | Not run | | Ad-hoc macOS QA package | Same-host HTTP client | All local networks + `en0` | Off + Basic | Passed empty-library auth/lifecycle/port-conflict smoke plus representative pagination/facet/search/metadata/cover/download/range/concurrency crawl over IPv4 and IPv6 on macOS 26.5.1; not a KOReader result | -| Ubuntu `.deb` | β€” | Specific Wi-Fi/Ethernet | Off + Basic | Not run | -| Ubuntu AppImage | β€” | All local networks | Off + Basic | Not run | +| Dev build | Kobo Libra Colour / KOReader version unknown | `en0` | Basic | Passed manual connection to the early flat catalog; current navigation, package behavior, auth-off, and downloads were not recorded | +| Ubuntu 24.04.1 ARM64 `.deb` | Headless package smoke only | β€” | β€” | Built, installed, and stayed running under Xvfb; no interactive OPDS/client path exercised | +| Ubuntu 24.04.1 ARM64 AppImage | Headless package smoke only | β€” | β€” | Built and launched its bundled Citadel executable under Xvfb; no interactive OPDS/client path exercised | For every applicable row, verify and record: From a1934bfc655fff5ce011bd7bfa8ef163aff8de92 Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Sun, 2 Aug 2026 20:05:09 -0400 Subject: [PATCH 5/6] docs(opds): verify signed nightly artifact --- docs/opds-validation.md | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/docs/opds-validation.md b/docs/opds-validation.md index 4f2793e7..d090e688 100644 --- a/docs/opds-validation.md +++ b/docs/opds-validation.md @@ -30,6 +30,32 @@ most recent manual Release run completed both its macOS and Ubuntu jobs on 2026-07-10/11. That proves the publishing pipeline and credentials worked for main commit `f0ec58ee`; it does not prove or publish the current OPDS commits. +### Signed macOS prerelease verification β€” 2026-08-02 + +The `Release` workflow was dispatched as a nightly prerelease from exact OPDS +head `1e19a6c3438c20d8d6193b4c29ca6b82d5b3b740`. Its macOS 15 job completed +successfully and published +[`v0.6.1-nightly.20260803.163`](https://github.com/everydaythingssoftware/citadel/releases/tag/v0.6.1-nightly.20260803.163). + +The published `Citadel_0.6.1-nightly.20260803.163_aarch64.dmg` was downloaded +again rather than inspected only inside CI. Its SHA-256 was +`639c4d506ad4bc45260dd2e8a7201228e67ed53f24c03abe11d60c18619ec92a`, +matching GitHub's asset digest, and the disk image verified successfully while +mounting read-only. The contained `Citadel.app` passed: + +- `codesign --verify --deep --strict`, with Developer ID Application identity + `PHILIP GEORGE DENHOFF (J287EZX7X6)`, Team ID `J287EZX7X6`, hardened runtime, + and a trusted Apple certificate chain; +- `spctl --assess --type execute`, reported as accepted from **Notarized + Developer ID**; and +- `xcrun stapler validate`, confirming the stapled notarization ticket. + +This proves that the current OPDS v1 head can be released as a signed, +notarized, stapled Apple Silicon package. It does not substitute for running +the physical-network and current KOReader scenarios in the matrix below. +The same workflow's Ubuntu 22.04 job also completed successfully and published +x86-64 AppImage, Debian, and RPM packages plus their updater signatures. + ### Isolated packaged-app smoke test β€” 2026-08-02 The release app was rebuilt with the compile-time QA identifier @@ -61,7 +87,7 @@ Passed: interface.” Stopping recovered to editable configuration. This was a same-host network smoke test against an empty library. It does not -cover a signed/notarized current artifact, OS firewall denial, a separate +run the signed/notarized current artifact, cover OS firewall denial, a separate KOReader device, representative books/covers/acquisitions, active-library switching, or interface loss/recovery. The QA app, isolated settings/verifier, and disposable library were moved to Trash after the run. @@ -107,7 +133,7 @@ Passed: libraries were moved to Trash after the run. This remains a same-host protocol crawl, not a separate-device KOReader result. -It does not cover a signed/notarized current artifact, OS firewall denial, a +It does not run the signed/notarized current artifact, cover OS firewall denial, a Linux OPDS client path, or physical interface loss/recovery. ### Linux package smoke test β€” 2026-08-02 From a20a4e023024fb3a0da433bb880860f0466732f8 Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Wed, 16 Sep 2026 23:08:28 -0700 Subject: [PATCH 6/6] fix(opds): cap Argon2 concurrency and request pressure Merge-blocking hardening from the v1 review: - Limit concurrent password verifications to 3 permits; excess requests get 503 with Retry-After without hashing or caching the header - Bound catalog feeds with a 30s timeout, 16KiB request-body limit, and a shared 64-request permit pool; asset downloads stay untimed - Reject link-local explicit listener addresses in the headless server and assert advertised URLs are IPv4-first without link-local entries - Note in the Sharing pane that generated passwords are shown once and travel unencrypted --- Cargo.lock | 3 + crates/citadel-opds/Cargo.toml | 1 + crates/citadel-opds/src/auth.rs | 206 +++++++++++++---- crates/citadel-opds/src/catalog.rs | 229 ++++++++++++++++++- crates/citadel-opds/src/network.rs | 31 +++ crates/citadel-server/src/lib.rs | 25 ++ src/components/organisms/SharingSettings.tsx | 32 +-- 7 files changed, 458 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 895a6ef3..747b5ea0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -711,6 +711,7 @@ dependencies = [ "tempfile", "tokio", "tower", + "tower-http", "urlencoding", "uuid", ] @@ -6057,7 +6058,9 @@ dependencies = [ "futures-util", "http", "http-body", + "http-body-util", "pin-project-lite", + "tokio", "tower", "tower-layer", "tower-service", diff --git a/crates/citadel-opds/Cargo.toml b/crates/citadel-opds/Cargo.toml index 86930092..dea2852b 100644 --- a/crates/citadel-opds/Cargo.toml +++ b/crates/citadel-opds/Cargo.toml @@ -23,6 +23,7 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" specta = { version = "=2.0.0-rc.22", features = ["chrono", "derive"] } tokio = { version = "1.52.3", features = ["macros", "net", "rt-multi-thread", "sync", "time"] } +tower-http = { version = "0.6", default-features = false, features = ["limit", "timeout"] } urlencoding = "2.1.3" uuid = { version = "1.6.1", features = ["v4", "fast-rng"] } diff --git a/crates/citadel-opds/src/auth.rs b/crates/citadel-opds/src/auth.rs index fa78c2fd..b036c705 100644 --- a/crates/citadel-opds/src/auth.rs +++ b/crates/citadel-opds/src/auth.rs @@ -9,7 +9,10 @@ use argon2::{ }; use axum::{ extract::{Request, State}, - http::{header::AUTHORIZATION, header::WWW_AUTHENTICATE, HeaderValue, StatusCode}, + http::{ + header::{AUTHORIZATION, RETRY_AFTER, WWW_AUTHENTICATE}, + HeaderValue, StatusCode, + }, middleware::Next, response::{IntoResponse, Response}, }; @@ -18,12 +21,25 @@ use hmac::{Hmac, Mac}; use rand_core::{OsRng, RngCore}; use sha2::Sha256; use subtle::ConstantTimeEq; +use tokio::sync::Semaphore; const DEFAULT_CACHE_CAPACITY: usize = 256; const DEFAULT_CACHE_TTL: Duration = Duration::from_secs(30); const DEFAULT_TARGET_DURATION: Duration = Duration::from_millis(250); +const MAX_CONCURRENT_VERIFICATIONS: usize = 3; const BASIC_CHALLENGE: &str = "Basic realm=\"Citadel\""; const MAX_AUTHORIZATION_BYTES: usize = 8 * 1024; +const RETRY_AFTER_SECONDS: &str = "1"; + +/// Result of authorizing complete Authorization header bytes. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum AuthOutcome { + Authorized, + Rejected, + /// Every password-verification permit is in use; the request was not + /// verified and must not count as a failed attempt. + Busy, +} #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct OpdsAuthCredentials { @@ -76,6 +92,7 @@ struct EnabledAuth { cache_key: [u8; 32], cache: Mutex, target_duration: Mutex, + verifier_permits: Arc, } #[derive(Clone, Copy)] @@ -142,6 +159,7 @@ impl OpdsBasicAuth { DEFAULT_CACHE_CAPACITY, DEFAULT_CACHE_TTL, DEFAULT_TARGET_DURATION, + MAX_CONCURRENT_VERIFICATIONS, ) } @@ -150,6 +168,7 @@ impl OpdsBasicAuth { cache_capacity: usize, cache_ttl: Duration, target_duration: Duration, + verifier_permits: usize, ) -> Result { let password_hash = PasswordHash::new(&credentials.verifier).map_err(|_| OpdsAuthError::InvalidVerifier)?; @@ -165,6 +184,7 @@ impl OpdsBasicAuth { cache_key, cache: Mutex::new(AuthCache::new(cache_capacity, cache_ttl)), target_duration: Mutex::new(target_duration), + verifier_permits: Arc::new(Semaphore::new(verifier_permits)), })), }) } @@ -174,16 +194,16 @@ impl OpdsBasicAuth { } /// Authorizes complete Authorization header bytes. Disabled authentication does not parse them. - async fn authorize(&self, authorization: Option<&[u8]>) -> bool { + async fn authorize(&self, authorization: Option<&[u8]>) -> AuthOutcome { let Some(enabled) = &self.enabled else { - return true; + return AuthOutcome::Authorized; }; let started_at = Instant::now(); let authorization = authorization.unwrap_or_default(); if authorization.len() > MAX_AUTHORIZATION_BYTES { pad_to_target(started_at, current_target_duration(enabled)).await; - return false; + return AuthOutcome::Rejected; } let tag = opaque_tag(&enabled.cache_key, authorization); if let Some((outcome, cached_duration)) = enabled @@ -194,32 +214,57 @@ impl OpdsBasicAuth { { let target_duration = current_target_duration(enabled).max(cached_duration); pad_to_target(started_at, target_duration).await; - return matches!(outcome, CachedOutcome::Authorized); + return match outcome { + CachedOutcome::Authorized => AuthOutcome::Authorized, + CachedOutcome::Rejected => AuthOutcome::Rejected, + }; } let parsed = parse_basic_credentials(authorization); - let authorized = match parsed { + let verification = match parsed { Some(credentials) if credentials.username == enabled.credentials.username => { + match enabled.verifier_permits.clone().try_acquire_owned() { + Ok(permit) => Some((credentials, permit)), + Err(_) => { + pad_to_target(started_at, current_target_duration(enabled)).await; + return AuthOutcome::Busy; + } + } + } + _ => None, + }; + let authorized = match verification { + Some((credentials, permit)) => { let verifier = enabled.credentials.verifier.clone(); - tokio::task::spawn_blocking(move || { + let verified = tokio::task::spawn_blocking(move || { verify_password(&verifier, &credentials.password) }) .await - .unwrap_or(false) + .unwrap_or(false); + drop(permit); + verified } - _ => false, + None => false, }; let target_duration = target_duration(enabled, started_at.elapsed()); let outcome = if authorized { - CachedOutcome::Authorized + AuthOutcome::Authorized } else { - CachedOutcome::Rejected + AuthOutcome::Rejected }; if let Ok(mut cache) = enabled.cache.lock() { - cache.insert(tag, outcome, target_duration); + cache.insert( + tag, + if authorized { + CachedOutcome::Authorized + } else { + CachedOutcome::Rejected + }, + target_duration, + ); } pad_to_target(started_at, target_duration).await; - authorized + outcome } } @@ -237,10 +282,10 @@ pub(crate) async fn require_basic_auth( .headers() .get(AUTHORIZATION) .map(|value| value.as_bytes().to_vec()); - if auth.authorize(authorization.as_deref()).await { - next.run(request).await - } else { - unauthorized_response() + match auth.authorize(authorization.as_deref()).await { + AuthOutcome::Authorized => next.run(request).await, + AuthOutcome::Busy => busy_response(), + AuthOutcome::Rejected => unauthorized_response(), } } @@ -312,6 +357,14 @@ fn unauthorized_response() -> Response { .into_response() } +fn busy_response() -> Response { + ( + StatusCode::SERVICE_UNAVAILABLE, + [(RETRY_AFTER, HeaderValue::from_static(RETRY_AFTER_SECONDS))], + ) + .into_response() +} + #[cfg(test)] mod tests { use axum::{ @@ -338,10 +391,19 @@ mod tests { 2, Duration::from_secs(1), target_duration, + MAX_CONCURRENT_VERIFICATIONS, ) .unwrap() } + async fn assert_outcome( + auth: &OpdsBasicAuth, + authorization: Option<&[u8]>, + expected: AuthOutcome, + ) { + assert_eq!(auth.authorize(authorization).await, expected); + } + fn app(auth: OpdsBasicAuth) -> Router { Router::new() .route("/opds", get(|| async { "catalog" })) @@ -377,7 +439,12 @@ mod tests { async fn disabled_auth_bypasses_even_malformed_authorization() { let auth = OpdsBasicAuth::disabled(); - assert!(auth.authorize(Some(b"not even close to Basic")).await); + assert_outcome( + &auth, + Some(b"not even close to Basic"), + AuthOutcome::Authorized, + ) + .await; assert_eq!( response(auth, Some(b"not even close to Basic".to_vec())) .await @@ -390,21 +457,25 @@ mod tests { async fn enabled_auth_accepts_only_the_configured_basic_credentials() { let auth = enabled_auth("reader", b"correct horse", Duration::ZERO); - assert!( - auth.authorize(Some(&basic_header("reader", b"correct horse"))) - .await - ); - assert!( - !auth - .authorize(Some(&basic_header("reader", b"wrong password"))) - .await - ); - assert!( - !auth - .authorize(Some(&basic_header("someone-else", b"correct horse"))) - .await - ); - assert!(!auth.authorize(None).await); + assert_outcome( + &auth, + Some(&basic_header("reader", b"correct horse")), + AuthOutcome::Authorized, + ) + .await; + assert_outcome( + &auth, + Some(&basic_header("reader", b"wrong password")), + AuthOutcome::Rejected, + ) + .await; + assert_outcome( + &auth, + Some(&basic_header("someone-else", b"correct horse")), + AuthOutcome::Rejected, + ) + .await; + assert_outcome(&auth, None, AuthOutcome::Rejected).await; } #[tokio::test] @@ -437,7 +508,7 @@ mod tests { let response = response(auth.clone(), Some(oversized.clone())).await; assert_eq!(response.status(), StatusCode::UNAUTHORIZED); - assert!(!auth.authorize(Some(&oversized)).await); + assert_outcome(&auth, Some(&oversized), AuthOutcome::Rejected).await; assert!(auth .enabled .as_ref() @@ -449,26 +520,63 @@ mod tests { .is_empty()); } + #[tokio::test] + async fn busy_verifier_permits_reject_without_hashing_or_caching_the_header() { + let auth = enabled_auth("reader", b"correct horse", Duration::ZERO); + let enabled = auth.enabled.as_ref().unwrap(); + let permits = (0..MAX_CONCURRENT_VERIFICATIONS) + .map(|_| { + enabled + .verifier_permits + .clone() + .try_acquire_owned() + .expect("fresh auth should expose exactly its configured permits") + }) + .collect::>(); + let header = basic_header("reader", b"wrong password"); + + assert_eq!( + auth.authorize(Some(&header)).await, + AuthOutcome::Busy, + "requests with no free verification permit must be rejected without hashing" + ); + assert_eq!( + response(auth.clone(), Some(header)).await.status(), + StatusCode::SERVICE_UNAVAILABLE + ); + assert!(enabled.cache.lock().unwrap().entries.is_empty()); + + drop(permits); + assert_outcome( + &auth, + Some(&basic_header("reader", b"wrong password")), + AuthOutcome::Rejected, + ) + .await; + assert_eq!(enabled.cache.lock().unwrap().entries.len(), 1); + } + #[tokio::test] async fn cache_records_and_reuses_the_authorization_outcome_without_raw_header_bytes() { let auth = enabled_auth("reader", b"correct horse", Duration::ZERO); let header = basic_header("reader", b"correct horse"); - assert!(auth.authorize(Some(&header)).await); + assert_outcome(&auth, Some(&header), AuthOutcome::Authorized).await; let enabled = auth.enabled.as_ref().unwrap(); - let cache = enabled.cache.lock().unwrap(); - assert_eq!(cache.entries.len(), 1); - assert_eq!( - cache.entries[0].tag, - opaque_tag(&enabled.cache_key, &header) - ); - assert!(matches!( - cache.entries[0].outcome, - CachedOutcome::Authorized - )); - drop(cache); + { + let cache = enabled.cache.lock().unwrap(); + assert_eq!(cache.entries.len(), 1); + assert_eq!( + cache.entries[0].tag, + opaque_tag(&enabled.cache_key, &header) + ); + assert!(matches!( + cache.entries[0].outcome, + CachedOutcome::Authorized + )); + } - assert!(auth.authorize(Some(&header)).await); + assert_outcome(&auth, Some(&header), AuthOutcome::Authorized).await; assert_eq!(enabled.cache.lock().unwrap().entries.len(), 1); } @@ -479,11 +587,11 @@ mod tests { let unknown_header = basic_header("unknown", b"correct horse"); let first_started_at = Instant::now(); - assert!(!auth.authorize(Some(&unknown_header)).await); + assert_outcome(&auth, Some(&unknown_header), AuthOutcome::Rejected).await; let first_elapsed = first_started_at.elapsed(); let cached_started_at = Instant::now(); - assert!(!auth.authorize(Some(&unknown_header)).await); + assert_outcome(&auth, Some(&unknown_header), AuthOutcome::Rejected).await; let cached_elapsed = cached_started_at.elapsed(); let minimum_padded_duration = target_duration - Duration::from_millis(2); diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index 8aad586f..b40f026f 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -1,9 +1,10 @@ -use std::{borrow::Cow, io::Read, sync::Arc}; +use std::{borrow::Cow, io::Read, sync::Arc, time::Duration}; use axum::{ body::Body, - extract::{Path, Query, State}, + extract::{Path, Query, Request, State}, http::{header, HeaderValue, StatusCode}, + middleware::{self, Next}, response::{IntoResponse, Response}, routing::get, Router, @@ -18,6 +19,8 @@ use quick_xml::{ Writer, }; use serde::Deserialize; +use tokio::sync::Semaphore; +use tower_http::{limit::RequestBodyLimitLayer, timeout::TimeoutLayer}; use super::{ assets::{self, AssetMethod}, @@ -26,6 +29,9 @@ use super::{ const PAGE_SIZE: i64 = 50; const MAX_SEARCH_LENGTH: usize = 200; +const FEED_TIMEOUT: Duration = Duration::from_secs(30); +const MAX_REQUEST_BODY_BYTES: usize = 16 * 1024; +const MAX_CONCURRENT_REQUESTS: usize = 64; const ACQUISITION_REL: &str = "http://opds-spec.org/acquisition"; const IMAGE_REL: &str = "http://opds-spec.org/image"; const ATOM_TYPE: &str = "application/atom+xml;profile=opds-catalog;kind=acquisition"; @@ -128,8 +134,62 @@ struct PageQuery { page: Option, } +/// Bounds how many requests the catalog serves at once. Clones of the guard +/// share one permit pool across every listener and connection. +#[derive(Clone)] +pub(crate) struct RequestGuard { + permits: Arc, +} + +impl RequestGuard { + pub(crate) fn with_capacity(permits: usize) -> Self { + Self { + permits: Arc::new(Semaphore::new(permits)), + } + } + + fn try_acquire(&self) -> Result { + self.permits + .clone() + .try_acquire_owned() + .map_err(|_| StatusCode::SERVICE_UNAVAILABLE) + } +} + +async fn enforce_request_guard( + State(guard): State, + request: Request, + next: Next, +) -> Response { + match guard.try_acquire() { + Ok(permit) => { + let _permit = permit; + next.run(request).await + } + Err(status) => ( + status, + [(header::RETRY_AFTER, HeaderValue::from_static("1"))], + ) + .into_response(), + } +} + pub fn router(source: Arc, auth: OpdsBasicAuth) -> Router { - Router::new() + build_router( + source, + auth, + RequestGuard::with_capacity(MAX_CONCURRENT_REQUESTS), + FEED_TIMEOUT, + ) +} + +fn build_router( + source: Arc, + auth: OpdsBasicAuth, + guard: RequestGuard, + feed_timeout: Duration, +) -> Router { + let feeds = Router::new() .route("/opds", get(root_feed)) .route("/opds/all", get(all_books_feed)) .route("/opds/recent", get(recent_books_feed)) @@ -144,6 +204,18 @@ pub fn router(source: Arc, auth: OpdsBasicAuth) -> Router { .route("/opds/genres/{id}", get(genre_books_feed)) .route("/opds/search", get(search_feed)) .route("/opds/opensearch.xml", get(opensearch_description)) + .with_state(CatalogState { + source: source.clone(), + }) + .layer(TimeoutLayer::with_status_code( + StatusCode::SERVICE_UNAVAILABLE, + feed_timeout, + )) + .layer(middleware::from_fn_with_state( + auth.clone(), + require_basic_auth, + )); + let assets = Router::new() .route( "/opds/books/{book_id}/files/{format}/{filename}", get(book_file).head(book_file_head), @@ -153,10 +225,12 @@ pub fn router(source: Arc, auth: OpdsBasicAuth) -> Router { get(book_cover).head(book_cover_head), ) .with_state(CatalogState { source }) - .layer(axum::middleware::from_fn_with_state( - auth, - require_basic_auth, - )) + .layer(middleware::from_fn_with_state(auth, require_basic_auth)); + Router::new() + .merge(feeds) + .merge(assets) + .layer(RequestBodyLimitLayer::new(MAX_REQUEST_BODY_BYTES)) + .layer(middleware::from_fn_with_state(guard, enforce_request_guard)) } async fn root_feed(state: State, query: Query) -> Response { @@ -2076,4 +2150,145 @@ mod tests { assert_eq!(response.text().await.unwrap(), "Catalog unavailable"); server.abort(); } + + async fn loopback_router(router: Router) -> (String, tokio::task::JoinHandle<()>) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let task = tokio::spawn(async move { + axum::serve(listener, router).await.unwrap(); + }); + (format!("http://{address}"), task) + } + + struct DelayedSource { + inner: Arc, + feed_delay: Duration, + asset_delay: Duration, + } + + impl CatalogSource for DelayedSource { + fn active_library_id(&self) -> Result { + std::thread::sleep(self.feed_delay); + self.inner.active_library_id() + } + + fn book_page( + &self, + query: CatalogBookQuery, + ) -> Result<(String, Option, BookPage), CalibreError> { + self.inner.book_page(query) + } + + fn book_file( + &self, + book_id: BookId, + format: &str, + ) -> Result { + std::thread::sleep(self.asset_delay); + self.inner.book_file(book_id, format) + } + + fn book_cover(&self, book_id: BookId) -> Result { + std::thread::sleep(self.asset_delay); + self.inner.book_cover(book_id) + } + } + + #[tokio::test] + async fn exhausted_request_guard_answers_service_unavailable() { + let guard = RequestGuard::with_capacity(1); + let held_permit = guard.try_acquire().unwrap(); + let (base, server) = loopback_router(build_router( + Arc::new(NoLibrary), + OpdsBasicAuth::disabled(), + guard, + Duration::from_secs(30), + )) + .await; + + let response = reqwest::get(format!("{base}/opds")).await.unwrap(); + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(response.headers().get(header::RETRY_AFTER).unwrap(), "1"); + drop(held_permit); + server.abort(); + } + + #[tokio::test] + async fn slow_feeds_time_out_while_slow_asset_routes_do_not() { + let source = Arc::new(DelayedSource { + inner: Arc::new(NoLibrary), + feed_delay: Duration::from_millis(200), + asset_delay: Duration::from_millis(100), + }); + let (base, server) = loopback_router(build_router( + source, + OpdsBasicAuth::disabled(), + RequestGuard::with_capacity(MAX_CONCURRENT_REQUESTS), + Duration::from_millis(10), + )) + .await; + let client = reqwest::Client::new(); + + let feed = client.get(format!("{base}/opds")).send().await.unwrap(); + assert_eq!(feed.status(), StatusCode::SERVICE_UNAVAILABLE); + + let asset = client + .get(format!("{base}/opds/books/1/cover")) + .send() + .await + .unwrap(); + assert_ne!( + asset.status(), + StatusCode::REQUEST_TIMEOUT, + "asset downloads must not be bound by the feed timeout" + ); + assert_eq!(asset.status(), StatusCode::SERVICE_UNAVAILABLE); + server.abort(); + } + + #[tokio::test] + async fn book_file_formats_can_never_traverse_paths() { + let (directory, mut library) = test_library(); + let source_path = directory.path().join("book.epub"); + std::fs::write(&source_path, b"book").unwrap(); + let added = library + .add_book(BookAdd { + title: "Traversal Target".to_string(), + author_names: vec!["Author".to_string()], + tags: None, + series: None, + series_index: None, + publisher: None, + publication_date: None, + rating: None, + comments: None, + identifiers: HashMap::new(), + language: None, + file_paths: vec![source_path], + }) + .unwrap(); + let (base, server) = loopback(Arc::new(LibrarySource { + library: Mutex::new(library), + })) + .await; + let client = reqwest::Client::new(); + for format in ["..%2f", "%2e%2e%2f%2e%2e%2fmetadata.db"] { + let response = client + .get(format!( + "{base}/opds/books/{}/files/{format}/book.epub", + added.id.as_i32() + )) + .send() + .await + .unwrap(); + assert_eq!( + response.status(), + StatusCode::BAD_REQUEST, + "format {format} must be rejected" + ); + assert_eq!(response.text().await.unwrap(), "Invalid asset request"); + } + server.abort(); + drop(directory); + } } diff --git a/crates/citadel-opds/src/network.rs b/crates/citadel-opds/src/network.rs index 2e1ea452..9fd4df3a 100644 --- a/crates/citadel-opds/src/network.rs +++ b/crates/citadel-opds/src/network.rs @@ -604,6 +604,37 @@ mod tests { ); } + #[test] + fn advertised_urls_list_ipv4_first_and_never_link_local() { + let interfaces = [interface( + "en0", + OpdsInterfaceKind::Lan, + OpdsInterfaceState::Up, + vec![ + ipv6("2001:db8::42"), + ipv6("fe80::42"), + ipv4([192, 168, 1, 42]), + ], + )]; + + let urls = match plan_bindings(&interfaces, &OpdsBindTarget::AllLocalNetworks, 8080) { + BindPlan::Listen(addresses) => addresses + .iter() + .copied() + .map(advertised_url) + .collect::>(), + BindPlan::Wait(reason) => panic!("expected listening plan, got {reason:?}"), + }; + + assert_eq!( + urls, + vec![ + "http://192.168.1.42:8080/opds".to_string(), + "http://[2001:db8::42]:8080/opds".to_string(), + ] + ); + } + #[test] fn explicit_addresses_do_not_depend_on_interface_enumeration() { let plan = plan_bindings( diff --git a/crates/citadel-server/src/lib.rs b/crates/citadel-server/src/lib.rs index 33856692..4642fe56 100644 --- a/crates/citadel-server/src/lib.rs +++ b/crates/citadel-server/src/lib.rs @@ -200,6 +200,15 @@ fn validate_config(config: &ServerConfig) -> Result<(), String> { if address.is_unspecified() { return Err("wildcard sharing target addresses are not allowed".to_string()); } + let link_local = match address { + IpAddr::V4(address) => address.is_link_local(), + IpAddr::V6(address) => (address.segments()[0] & 0xffc0) == 0xfe80, + }; + if link_local { + return Err(format!( + "link-local sharing target address is not allowed: {address}" + )); + } } } if let Some(credentials) = &config.credentials { @@ -397,4 +406,20 @@ type = "allLocalNetworks" assert!(rejected); } } + + #[test] + fn config_rejects_link_local_explicit_addresses() { + let rejected = r#"libraryPath = "/tmp/library" +stateDirectory = "/tmp/state" +[sharing] +port = 8080 +authenticationEnabled = false +[sharing.target] +type = "addresses" +addresses = ["fe80::1"] +"#; + let config = toml::from_str::(rejected).unwrap(); + let error = validate_config(&config).unwrap_err(); + assert!(error.contains("link-local"), "unexpected error: {error}"); + } } diff --git a/src/components/organisms/SharingSettings.tsx b/src/components/organisms/SharingSettings.tsx index 898fd93d..2da26aaf 100644 --- a/src/components/organisms/SharingSettings.tsx +++ b/src/components/organisms/SharingSettings.tsx @@ -229,20 +229,26 @@ export const SharingSettings = () => { {generatedPassword && ( -
-
- Generated password - {generatedPassword} + <> +
+
+ Generated password + {generatedPassword} +
+
- -
+

+ This password is shown only once, right here. Copy it into + your reader now. It travels unencrypted on your local network. +

+ )}