diff --git a/.gitea/workflows/build-and-test.yml b/.gitea/workflows/build-and-test.yml index a08acc9..b42f12e 100644 --- a/.gitea/workflows/build-and-test.yml +++ b/.gitea/workflows/build-and-test.yml @@ -14,6 +14,11 @@ jobs: desktop: runs-on: linux/amd64 name: Desktop (Linux) + # actions/checkout and actions/cache are JavaScript actions: the runner + # executes them with Node from inside this container. The bare runner image + # has none, so the job failed at checkout before reaching any build step. + container: + image: catthehacker/ubuntu:act-latest steps: - name: Checkout @@ -34,6 +39,17 @@ jobs: apt-get update -qq apt-get install -y -qq pkg-config libfontconfig1-dev libxkbcommon-dev + # The act image ships Node but no Rust. Pinned to the workspace + # rust-version so CI, the Android image, and local builds agree — a + # floating toolchain turns an unrelated push into a mystery failure. + - name: Install Rust 1.92.0 + run: | + set -e + curl -fsSL https://sh.rustup.rs | sh -s -- \ + -y --no-modify-path --profile minimal \ + --default-toolchain 1.92.0 --component rustfmt,clippy + echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + - name: Format run: cargo fmt --all -- --check @@ -92,11 +108,23 @@ jobs: layering: runs-on: linux/amd64 name: Layer separation + # Node for the JS actions, as above. cargo comes from rustup below. + container: + image: catthehacker/ubuntu:act-latest steps: - name: Checkout uses: actions/checkout@v4 + # `cargo tree` resolves the dependency graph, so it needs the registry + # index but no system libraries — this job builds nothing. + - name: Install Rust 1.92.0 + run: | + set -e + curl -fsSL https://sh.rustup.rs | sh -s -- \ + -y --no-modify-path --profile minimal --default-toolchain 1.92.0 + echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + # ARCH §6.5a: no core/ crate may depend on the UI toolkit. One stray # `use slint::` costs headless golden-image testing and the # one-operation-two-presentations property together, and nothing else diff --git a/.gitea/workflows/traceability-check.yml b/.gitea/workflows/traceability-check.yml index 96f99f6..8b37b2d 100644 --- a/.gitea/workflows/traceability-check.yml +++ b/.gitea/workflows/traceability-check.yml @@ -23,6 +23,10 @@ jobs: traceability: runs-on: linux/amd64 name: Requirement traces + # Node for actions/checkout and actions/cache, which the bare runner image + # cannot execute. Rust is installed below. + container: + image: catthehacker/ubuntu:act-latest steps: - name: Checkout @@ -39,6 +43,15 @@ jobs: target key: traces-${{ runner.os }}-${{ hashFiles('**/Cargo.lock') }} + # Source-comment and markdown parsing only, so the minimal profile is + # enough — no system libraries and no components beyond cargo itself. + - name: Install Rust 1.92.0 + run: | + set -e + curl -fsSL https://sh.rustup.rs | sh -s -- \ + -y --no-modify-path --profile minimal --default-toolchain 1.92.0 + echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + # The gate's own arithmetic is the thing being trusted, so its tests run # before it does. Untested gate logic is exactly how JellyTau's 158% went # unnoticed for months. diff --git a/Cargo.lock b/Cargo.lock index a75d983..4be1c73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1176,6 +1176,7 @@ name = "darkroom-android" version = "0.1.0" dependencies = [ "android_logger", + "dr-sync-nextcloud", "dr-ui", "log", "slint", diff --git a/apps/darkroom-android/Cargo.toml b/apps/darkroom-android/Cargo.toml index 14b6f47..28e7d7c 100644 --- a/apps/darkroom-android/Cargo.toml +++ b/apps/darkroom-android/Cargo.toml @@ -16,6 +16,9 @@ crate-type = ["cdylib"] # No backend feature to select: dr-ui picks its Slint backend from the target, # so building for aarch64-linux-android gets android-activity automatically. dr-ui.workspace = true +# For `session::set_data_dir`: only the platform entry point knows where Android +# lets this app keep files, and it must be set before any store is opened. +dr-sync-nextcloud.workspace = true # Directly, not just through dr-ui: `android_main` takes an `AndroidApp` and # calls `slint::android::init`, both of which come from this crate. The backend # feature comes from dr-ui's target-specific dependency. diff --git a/apps/darkroom-android/android/AndroidManifest.xml b/apps/darkroom-android/android/AndroidManifest.xml index 93d06f6..5c38357 100644 --- a/apps/darkroom-android/android/AndroidManifest.xml +++ b/apps/darkroom-android/android/AndroidManifest.xml @@ -4,9 +4,9 @@ Deliberately minimal: this packages the viewer for on-device testing (spike S2 needs Adreno and Mali hardware, which no emulator represents). Nothing - here is a distribution manifest yet — no permissions are declared because - the library grid reads through SAF, which grants per-tree rather than by - manifest permission (ARCH §6.9). + here is a distribution manifest yet. Only network access is declared: file + access needs no manifest permission because the library grid reads through + SAF, which grants per-tree at runtime (ARCH §6.9). --> diff --git a/apps/darkroom-android/src/lib.rs b/apps/darkroom-android/src/lib.rs index 55def51..9cc2694 100644 --- a/apps/darkroom-android/src/lib.rs +++ b/apps/darkroom-android/src/lib.rs @@ -34,6 +34,20 @@ fn android_main(app: slint::android::AndroidApp) { log::info!("DarkRoom v{}", env!("CARGO_PKG_VERSION")); + // Before anything opens a store: Android has no $HOME and no XDG + // directories, so the default guess resolves to a path the app cannot + // write. Nothing failed loudly — the session list went to a doomed path, so + // the account survived only as long as the process and backgrounding the app + // lost the sign-in. `internal_data_path` is the app's private directory + // (ARCH §6.9). + match app.internal_data_path() { + Some(dir) => { + log::info!("data dir: {}", dir.display()); + dr_sync_nextcloud::session::set_data_dir(dir); + } + None => log::error!("no internal data path; settings will not persist"), + } + if let Err(e) = slint::android::init(app) { log::error!("Slint Android backend failed to initialise: {e}"); return; diff --git a/core/dr-sync-nextcloud/certs/isrg-root-ye.pem b/core/dr-sync-nextcloud/certs/isrg-root-ye.pem new file mode 100644 index 0000000..81e933a --- /dev/null +++ b/core/dr-sync-nextcloud/certs/isrg-root-ye.pem @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIICpjCCAiugAwIBAgIRAIchZfw0tuX7qK3Vs3BftTowCgYIKoZIzj0EAwMwTzEL +MAkGA1UEBhMCVVMxKTAnBgNVBAoTIEludGVybmV0IFNlY3VyaXR5IFJlc2VhcmNo +IEdyb3VwMRUwEwYDVQQDEwxJU1JHIFJvb3QgWDIwHhcNMjYwNTEzMDAwMDAwWhcN +MzIwOTAyMjM1OTU5WjAuMQswCQYDVQQGEwJVUzENMAsGA1UEChMESVNSRzEQMA4G +A1UEAxMHUm9vdCBZRTB2MBAGByqGSM49AgEGBSuBBAAiA2IABDwS/6vhrcVqcbBo ++wgdI3fwn9x7DNJJOY/lTOti0vkwuRN87RhEhTH17E7XyFjWsPYhIPt/wzOqxTd2 +b+4ZJNy9ID04YywF9U5zasDVyGSNErVNtz8uSGh5izW87j77GaOB6zCB6DAOBgNV +HQ8BAf8EBAMCAQYwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDwYDVR0TAQH/BAUwAwEB +/zAdBgNVHQ4EFgQUo8gmWo6hTNA1Y/ybI8g6rlbzT1YwHwYDVR0jBBgwFoAUfEKW +rt5LSDv6kviejM9ti6lyN5UwMgYIKwYBBQUHAQEEJjAkMCIGCCsGAQUFBzAChhZo +dHRwOi8veDIuaS5sZW5jci5vcmcvMBMGA1UdIAQMMAowCAYGZ4EMAQIBMCcGA1Ud +HwQgMB4wHKAaoBiGFmh0dHA6Ly94Mi5jLmxlbmNyLm9yZy8wCgYIKoZIzj0EAwMD +aQAwZgIxAMU19WCtmxVND8UHBZRoma49Z7jPs64Dma0eTu1OChVbB/2J7GV3nvYK +Ax54uk1G9QIxAO0miLVJu8PLNiXXXkiE/gsK3CTRTF/aeo4bMX42Zw40csRU6AC2 +6hSW1/IWaas6dg== +-----END CERTIFICATE----- + 3 s:C=US, O=Internet Security Research Group, CN=ISRG Root X2 + i:C=US, O=Internet Security Research Group, CN=ISRG Root X1 + a:PKEY: EC, (secp384r1); sigalg: sha256WithRSAEncryption + v:NotBefore: May 13 00:00:00 2026 GMT; NotAfter: Sep 2 23:59:59 2032 GMT diff --git a/core/dr-sync-nextcloud/src/auth.rs b/core/dr-sync-nextcloud/src/auth.rs index f6a16b6..c53e7fd 100644 --- a/core/dr-sync-nextcloud/src/auth.rs +++ b/core/dr-sync-nextcloud/src/auth.rs @@ -57,7 +57,7 @@ pub async fn begin( .header(reqwest::header::USER_AGENT, user_agent) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(super::map_send_error)?; if !resp.status().is_success() { return Err(RemoteError::Server { @@ -94,7 +94,7 @@ pub async fn poll( .body(format!("token={}", urlencode(&flow.poll.token))) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(super::map_send_error)?; match resp.status().as_u16() { 200 => { @@ -147,7 +147,7 @@ pub async fn revoke( .header("OCS-APIRequest", "true") .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(super::map_send_error)?; Ok(()) } diff --git a/core/dr-sync-nextcloud/src/lib.rs b/core/dr-sync-nextcloud/src/lib.rs index 0c81c25..6eb71e7 100644 --- a/core/dr-sync-nextcloud/src/lib.rs +++ b/core/dr-sync-nextcloud/src/lib.rs @@ -145,7 +145,7 @@ impl NextcloudBackend { .body(chunk.to_vec()) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; map_status(resp.status(), path.as_str())?; } @@ -164,7 +164,7 @@ impl NextcloudBackend { .header("OC-Total-Length", total.to_string()) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; map_status(resp.status(), path.as_str())?; // The MOVE response carries the assembled file's ETag on Nextcloud, @@ -191,7 +191,7 @@ impl NextcloudBackend { .basic_auth(&self.login, Some(&self.password)) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; // 405 is "already exists", which is exactly what we want. if resp.status() == 405 { @@ -218,12 +218,12 @@ impl NextcloudBackend { .body(body) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; map_status(resp.status(), path.as_str())?; resp.text() .await - .map_err(|e| RemoteError::Network(e.to_string())) + .map_err(map_send_error) } } @@ -284,14 +284,14 @@ impl RemoteBackend for NextcloudBackend { let resp = req .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; let status = resp.status(); map_status(status, &url)?; let body = resp .bytes() .await - .map_err(|e| RemoteError::Network(e.to_string()))? + .map_err(map_send_error)? .to_vec(); // Nextcloud does not advertise Accept-Ranges, so support is detected @@ -346,7 +346,7 @@ impl RemoteBackend for NextcloudBackend { .body(body) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; map_status(resp.status(), path.as_str())?; resp.headers() @@ -374,7 +374,7 @@ impl RemoteBackend for NextcloudBackend { let resp = req .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; map_status(resp.status(), &url) } @@ -413,7 +413,7 @@ impl RemoteBackend for NextcloudBackend { .header("Overwrite", "F") .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; map_status(resp.status(), &src) } @@ -451,7 +451,7 @@ impl RemoteBackend for NextcloudBackend { .basic_auth(&self.login, Some(&self.password)) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; // 405 is "already a collection here", which is exactly what the // caller wanted. Anything else is reported. @@ -482,7 +482,7 @@ impl RemoteBackend for NextcloudBackend { .basic_auth(&self.login, Some(&self.password)) .send() .await - .map_err(|e| RemoteError::Network(e.to_string()))?; + .map_err(map_send_error)?; // 404 means no preview for this file — expected for RAW, and a // fall-through rather than an error. @@ -494,12 +494,56 @@ impl RemoteBackend for NextcloudBackend { Ok(Some( resp.bytes() .await - .map_err(|e| RemoteError::Network(e.to_string()))? + .map_err(map_send_error)? .to_vec(), )) } } +/// Roots that are publicly trusted but not yet in the compiled-in set. +/// +/// # Why this exists +/// +/// `webpki-roots` is generated from Mozilla's CA program, and lags it. A CA +/// that has begun issuing from a new root is therefore trusted by every +/// browser and by the platform store while still being unknown to this binary +/// — and because [`http_client`] deliberately bypasses the platform store to +/// keep Android working, there is nothing to fall back to. The handshake fails +/// with `UnknownIssuer` against a server that is entirely healthy. +/// +/// Observed 2026-08-12: a Let's Encrypt chain anchored at ISRG Root YE. The +/// server offered the full cross-signed path up to ISRG Root X1, which *is* +/// bundled, but rustls anchors on the first certificate it recognises as +/// self-issued and stops rather than continuing to the cross-sign — so the +/// complete chain did not help. +/// +/// Each entry is a root that Mozilla already trusts. Removing one once +/// `webpki-roots` catches up is safe; the set is additive and duplicates are +/// harmless. +const EXTRA_ROOTS: &[(&str, &[u8])] = &[( + // ISRG Root YE — cross-signed by ISRG Root X2, valid to 2032-09-02. + // SHA-256 0FC0901CCA2BAE9E9FDBB02D50D02F1094F7B366720869 91B9E897626DC485F0 + "ISRG Root YE", + include_bytes!("../certs/isrg-root-ye.pem"), +)]; + +/// Parse [`EXTRA_ROOTS`] into reqwest certificates. +/// +/// A malformed entry is skipped rather than fatal: it costs one root, and +/// failing here would take down every connection including the ones that never +/// needed the extra anchor. +fn extra_roots() -> impl Iterator { + EXTRA_ROOTS.iter().filter_map(|(name, pem)| { + match reqwest::Certificate::from_pem(pem) { + Ok(c) => Some(c), + Err(e) => { + log::warn!("bundled root {name} unusable: {e}"); + None + } + } + }) +} + /// Build an HTTP client with the crypto provider already installed. /// /// Every entry point that creates a client must go through here. The auth @@ -525,9 +569,18 @@ pub fn http_client(user_agent: &str) -> Result { // verifier entirely (see its ClientBuilder TLS setup); the webpki-roots // feature supplies the roots it then uses. Spike S3 revisits this to // honour user-installed and enterprise CAs, which bundled roots cannot. - // Empty: the flag is what matters, and the roots come from the - // webpki-roots feature rather than from certificates passed here. - .tls_certs_only(std::iter::empty()) + // + // Not empty: webpki-roots trails Mozilla's store, so a root that is + // already issuing publicly-trusted certificates can be missing from it + // for months. [`EXTRA_ROOTS`] carries those, and is additive — the + // webpki-roots set is still installed alongside. + .tls_certs_only(extra_roots()) + // A request that hangs forever is indistinguishable from a worker that + // died, and cost a long time to tell apart once. Connect and total + // timeouts turn that into an error the UI can show. Generous enough for + // a slow phone on mobile data; the login poll has its own deadline. + .connect_timeout(std::time::Duration::from_secs(15)) + .timeout(std::time::Duration::from_secs(60)) .build() .map_err(|e| RemoteError::Network(e.to_string())) } @@ -550,6 +603,45 @@ fn install_crypto_provider() { }); } +/// Translate a transport failure into a typed error. +/// +/// `reqwest::Error`'s own `Display` is uninformative for exactly the failures +/// that matter — every one of them renders as "error sending request for url +/// (…)" regardless of cause. The reason lives in the `source()` chain, so this +/// walks it: without that, a rejected certificate and an unplugged cable +/// produce identical text, and the app cannot tell "you are offline" from +/// "I do not trust this server". +/// +/// TLS is separated because it is the one transport failure that waiting never +/// fixes, and the one that must not trigger offline mode (FR-CAT-9). +fn map_send_error(e: reqwest::Error) -> RemoteError { + // The full chain, which is what makes the log line actionable: rustls + // reports "invalid peer certificate: UnknownIssuer" only at depth 1. + let mut detail = e.to_string(); + let mut src: Option<&dyn std::error::Error> = std::error::Error::source(&e); + let mut tls = false; + while let Some(s) = src { + let text = s.to_string(); + // rustls surfaces every verification failure through this wording: + // UnknownIssuer, Expired, NotValidForName, BadSignature. + if text.contains("invalid peer certificate") + || text.contains("certificate") + || text.contains("CertificateError") + { + tls = true; + } + detail.push_str(": "); + detail.push_str(&text); + src = std::error::Error::source(s); + } + + if tls { + RemoteError::Tls(detail) + } else { + RemoteError::Network(detail) + } +} + /// Translate an HTTP status into a typed error. fn map_status(status: reqwest::StatusCode, what: &str) -> Result<(), RemoteError> { match status.as_u16() { @@ -713,4 +805,52 @@ mod tests { Err(RemoteError::Unsupported(_)) )); } + + #[test] + fn every_bundled_root_parses() { + // A typo'd or truncated PEM would otherwise be discovered only as a + // silently missing anchor on the one server that needs it. + assert_eq!( + extra_roots().count(), + EXTRA_ROOTS.len(), + "every bundled root must parse" + ); + } + + #[test] + fn isrg_root_ye_is_bundled_until_webpki_roots_carries_it() { + // The anchor this crate had to supply itself. Delete this and the + // entry it guards once webpki-roots ships Root YE. + assert!(EXTRA_ROOTS.iter().any(|(n, _)| *n == "ISRG Root YE")); + } } + +/// Tests that need the real server. Run with `--ignored`. +#[cfg(test)] +mod live { + /// TRACES: FR-NC-12 + /// The regression: a Root YE chain must complete the handshake. + /// + /// Ignored because it needs the network, but kept because the unit tests + /// cannot catch this — a bundled root that parses is not the same as a + /// bundled root that verifies, and the gap between those two is exactly + /// what reported a healthy server as offline for a day. + #[tokio::test] + #[ignore = "needs network"] + async fn a_lets_encrypt_root_ye_server_verifies() { + let client = super::http_client("DarkRoom-test").expect("client"); + // Any host on the newer ISRG anchor exercises this. + let r = client + .get("https://nextcloud.tourolle.paris/remote.php/dav/files/dtourolle/") + .send() + .await; + + match r { + // 401 is a completed TLS handshake; auth is not what is under test. + Ok(resp) => assert_eq!(resp.status().as_u16(), 401, "handshake completed"), + Err(e) => panic!("handshake failed: {e}"), + } + } +} + + diff --git a/core/dr-sync-nextcloud/src/session.rs b/core/dr-sync-nextcloud/src/session.rs index 2139e7b..5f2b101 100644 --- a/core/dr-sync-nextcloud/src/session.rs +++ b/core/dr-sync-nextcloud/src/session.rs @@ -20,6 +20,41 @@ use serde::{Deserialize, Serialize}; use crate::AppCredentials; +/// Where configuration is written, when the platform has told us. +/// +/// Android has no `$HOME` and no XDG directories, so the guess below resolves +/// to a path the app cannot write. Nothing failed loudly: the session list went +/// to a doomed path, so credentials survived only as long as the process did and +/// backgrounding the app lost the account (ARCH §6.9 — no core API may assume a +/// filesystem path on Android). +/// +/// The platform layer sets this once at startup, before any store is opened. +static DATA_DIR: std::sync::OnceLock = std::sync::OnceLock::new(); + +/// TRACES: FR-NC-2 +/// Declare the per-app directory configuration belongs in. +/// +/// Call before opening any store; later calls are ignored rather than racing. +/// On Android this is `AndroidApp::internal_data_path`, which is private to the +/// app and survives being backgrounded. Desktop needs no call — the XDG +/// fallback is correct there. +pub fn set_data_dir(dir: PathBuf) { + let _ = DATA_DIR.set(dir); +} + +/// The directory configuration lives in. +fn config_dir() -> PathBuf { + if let Some(d) = DATA_DIR.get() { + return d.clone(); + } + std::env::var_os("XDG_CONFIG_HOME") + .map(PathBuf::from) + .unwrap_or_else(|| { + PathBuf::from(std::env::var("HOME").unwrap_or_default()).join(".config") + }) + .join("darkroom") +} + /// A configured account, minus its credential. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Session { @@ -119,17 +154,16 @@ impl SessionStore { /// Linux: `$XDG_CONFIG_HOME/darkroom/sessions.json`, falling back to /// `~/.config` (FR-PLAT-LIN-1). pub fn open(secrets: Box) -> Self { - let dir = std::env::var_os("XDG_CONFIG_HOME") - .map(PathBuf::from) - .unwrap_or_else(|| { - PathBuf::from(std::env::var("HOME").unwrap_or_default()).join(".config") - }) - .join("darkroom"); - Self::open_at(dir.join("sessions.json"), secrets) + Self::open_at(config_dir().join("sessions.json"), secrets) } /// Open at an explicit path — used by tests, and by anything wanting a /// non-default config location. + /// Where configuration lives, for callers that need to sit files beside it. + pub fn data_dir() -> PathBuf { + config_dir() + } + pub fn open_at(config_path: PathBuf, secrets: Box) -> Self { Self { config_path, diff --git a/core/dr-sync/src/error.rs b/core/dr-sync/src/error.rs index 2734a84..e51e9e1 100644 --- a/core/dr-sync/src/error.rs +++ b/core/dr-sync/src/error.rs @@ -37,6 +37,20 @@ pub enum RemoteError { #[error("network error: {0}")] Network(String), + /// The TLS handshake was refused: an untrusted issuer, an expired + /// certificate, a hostname mismatch. + /// + /// **Distinct from [`Network`](Self::Network) on purpose.** The server is + /// there and answering — the connection is refused on trust grounds, which + /// no amount of waiting repairs. Folding it into `Network` puts the app + /// into offline mode and tells the user their connection is down, sending + /// them to inspect a network that is working perfectly (observed + /// 2026-08-12: a Let's Encrypt chain anchored at ISRG Root YE, absent from + /// the compiled-in root store, reported as "you are offline" on a server + /// answering in 19 ms). + #[error("TLS error: {0}")] + Tls(String), + #[error("unexpected server response: {status} {detail}")] Server { status: u16, detail: String }, @@ -52,6 +66,9 @@ impl RemoteError { pub fn is_transient(&self) -> bool { match self { RemoteError::Network(_) => true, + // Not transient: a rejected certificate is rejected identically on + // every retry. Retrying one buys nothing and hides the cause. + RemoteError::Tls(_) => false, RemoteError::Server { status, .. } => { // 5xx and 429 are worth retrying; other 4xx are not. // @@ -80,6 +97,10 @@ impl RemoteError { /// is just failing, and a retry is the right response rather than a /// mode change. 429 and 423 likewise — those are the server working /// correctly under load. + /// + /// [`Tls`](Self::Tls) is likewise not offline. The host resolved, the + /// socket connected, and the peer answered; only trust failed. Reporting + /// that as offline is what made a root-store gap look like a dead network. pub fn indicates_offline(&self) -> bool { matches!(self, RemoteError::Network(_)) } @@ -89,6 +110,29 @@ impl RemoteError { mod tests { use super::*; + #[test] + fn a_tls_failure_is_not_offline() { + // The regression this guards: a rejected certificate was reported as + // `Network`, which put the whole app into offline mode and told the + // user their connection was down — while the server answered in 19 ms. + let e = RemoteError::Tls("invalid peer certificate: UnknownIssuer".into()); + assert!( + !e.indicates_offline(), + "trust failure is not unreachability" + ); + assert!( + !e.is_transient(), + "a rejected certificate is rejected identically on every retry" + ); + } + + #[test] + fn a_dead_connection_is_still_offline() { + // The other side of the split: separating TLS out must not stop a + // genuine transport failure from reaching offline mode. + assert!(RemoteError::Network("dns error".into()).indicates_offline()); + } + #[test] fn transient_errors_are_retryable() { assert!(RemoteError::Network("timeout".into()).is_transient()); diff --git a/core/dr-types/src/settings.rs b/core/dr-types/src/settings.rs index cf4f812..f709f42 100644 --- a/core/dr-types/src/settings.rs +++ b/core/dr-types/src/settings.rs @@ -505,6 +505,38 @@ mod tests { assert_eq!(parsed.cache, CacheSettings::default()); } + #[test] + fn the_cache_defaults_to_a_bounded_budget_that_keeps_what_it_opens() { + // A default of "unlimited" would let the passive cache fill a disk + // before the user knew there was a setting; a default of "keep + // nothing" would re-download every image they reopened. Both budgets + // must therefore be `Some`, and keeping opened originals must be on. + let cache = CacheSettings::default(); + assert_eq!( + cache.original_budget_bytes, + Some(DEFAULT_ORIGINAL_BUDGET_BYTES) + ); + assert_eq!( + cache.thumbnail_budget_bytes, + Some(DEFAULT_THUMBNAIL_BUDGET_BYTES) + ); + assert!(cache.keep_opened_originals); + } + + #[test] + fn the_two_cache_budgets_are_independent() { + // Sharing them would let a day of browsing originals evict the + // thumbnails that make the library navigable. + let mut s = Settings::default(); + s.cache.original_budget_bytes = None; + s.sanitise(); + assert_eq!( + s.cache.thumbnail_budget_bytes, + Some(DEFAULT_THUMBNAIL_BUDGET_BYTES), + "clearing one budget must not disturb the other" + ); + } + #[test] fn upscaling_is_off_by_default() { // FR-EXP-3 requires an explicit opt-in. diff --git a/docker/android/Dockerfile b/docker/android/Dockerfile index a31860e..6a97e23 100644 --- a/docker/android/Dockerfile +++ b/docker/android/Dockerfile @@ -25,6 +25,9 @@ ARG NDK_VERSION=27.2.12479018 ARG CMDLINE_TOOLS=13114758 # Must satisfy cargo-ndk's MSRV (4.1.x needs >= 1.86) as well as our own crates. ARG RUST_VERSION=1.92.0 +# Gitea runs JavaScript actions (actions/checkout, actions/cache) with Node from +# inside the job container. Bookworm ships 18; current actions expect 20+. +ARG NODE_MAJOR=20 # JDK 17, not the host's 25: the Android Gradle Plugin supports 17 as its # stable target, and newer JDKs regularly break Gradle in ways that cost more @@ -51,6 +54,19 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ build-essential cmake python3 \ && rm -rf /var/lib/apt/lists/* +# --------------------------------------------------------------------------- +# Node — required by the CI runner, not by the Android build +# +# This image is the job container for the Android CI job, and Gitea executes +# actions/checkout inside it using the container's own Node. Without this the +# job fails at checkout with "Cannot find: node in PATH", before any Rust or +# Gradle step runs. Local builds never invoke it. +# --------------------------------------------------------------------------- +RUN curl -fsSL "https://deb.nodesource.com/setup_${NODE_MAJOR}.x" | bash - \ + && apt-get install -y --no-install-recommends nodejs \ + && rm -rf /var/lib/apt/lists/* \ + && node --version + # --------------------------------------------------------------------------- # Android SDK + NDK # --------------------------------------------------------------------------- diff --git a/ui/dr-ui/src/derived_sync.rs b/ui/dr-ui/src/derived_sync.rs index d02e984..b05f4f1 100644 --- a/ui/dr-ui/src/derived_sync.rs +++ b/ui/dr-ui/src/derived_sync.rs @@ -87,10 +87,9 @@ pub fn spawn_sync( let (tx, rx) = std::sync::mpsc::channel(); std::thread::spawn(move || { - let rt = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - { + // A current-thread runtime here is what made the library scan itself + // rather than adopt the shards the server already had; see net_runtime. + let rt = match crate::net_runtime::build() { Ok(rt) => rt, Err(e) => { let _ = tx.send(SyncMessage::Failed(e.to_string())); diff --git a/ui/dr-ui/src/launch.rs b/ui/dr-ui/src/launch.rs index e876144..fdc3375 100644 --- a/ui/dr-ui/src/launch.rs +++ b/ui/dr-ui/src/launch.rs @@ -253,6 +253,24 @@ impl LaunchModel { }; } + /// TRACES: FR-NC-1 + /// Begin a sign-in with an app password the user supplied directly. + /// + /// The browser flow is the recommended route because it never exposes a + /// credential to us, but it needs a browser and a round trip through one. + /// An app password is what Nextcloud offers instead: device-scoped, + /// individually revocable, and created by the user under Settings → + /// Security. That makes it the workable option where no browser can + /// complete the handshake. + pub fn begin_direct_sign_in(&mut self, server: impl Into) { + self.server_url = normalise_server(&server.into()); + self.error = None; + self.state = LaunchState::Busy { + message: "Checking the credentials…".into(), + session: None, + }; + } + pub fn await_approval(&mut self, login_url: impl Into) { self.status = Some("Approve the sign-in in your browser.".into()); self.state = LaunchState::AwaitingApproval { diff --git a/ui/dr-ui/src/launch_ui.rs b/ui/dr-ui/src/launch_ui.rs index e90665a..f0e57ef 100644 --- a/ui/dr-ui/src/launch_ui.rs +++ b/ui/dr-ui/src/launch_ui.rs @@ -106,6 +106,28 @@ where }); } + // --- sign in with an app password ----------------------------------- + { + let weak = window.as_weak(); + let ctl = controller.clone(); + window.on_launch_sign_in_direct(move |server, login, password| { + let Some(w) = weak.upgrade() else { return }; + ctl.model + .borrow_mut() + .begin_direct_sign_in(server.to_string()); + render(&w, &ctl); + + let server = ctl.model.borrow().server_url.clone(); + spawn_direct_login( + w.as_weak(), + ctl.clone(), + server, + login.to_string(), + password.to_string(), + ); + }); + } + // --- sign out ------------------------------------------------------ { let weak = window.as_weak(); @@ -279,11 +301,18 @@ fn spawn_login(weak: slint::Weak, ctl: Rc, server: }; step("thread started"); let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(move || { - // `enable_all()` also enables the signal driver, which wants to own - // process-wide signal handling and is not something a worker thread - // inside an Android app can safely claim — the flow needs only IO - // (reqwest) and time (the poll interval), so ask for just those. - let rt = match tokio::runtime::Builder::new_current_thread() + // Multi-thread, not current-thread: a current-thread runtime drives + // its reactor only while the thread sits inside `block_on`, and on + // Android that left reqwest's connection future never polled — the + // await never resolved, so the worker neither failed nor returned. + // A multi-thread runtime owns worker threads that poll the reactor + // regardless. One worker is plenty for a single login flow. + // + // Only IO and time are enabled; `enable_all()` would also start the + // signal driver, which wants process-wide signal handling that an + // Android app's runtime already owns. + let rt = match tokio::runtime::Builder::new_multi_thread() + .worker_threads(1) .enable_io() .enable_time() .build() @@ -313,6 +342,85 @@ fn spawn_login(weak: slint::Weak, ctl: Rc, server: poll_channel(weak, ctl, rx); } +/// TRACES: FR-NC-1 +/// Verify an app password the user supplied, then keep it. +/// +/// No browser and no polling: one authenticated request establishes both that +/// the credential works and what the account's canonical user id is, which is +/// what the DAV paths are built from. Reuses the same channel and drain loop as +/// the browser flow, so success and failure land in the UI identically. +fn spawn_direct_login( + weak: slint::Weak, + ctl: Rc, + server: String, + login: String, + password: String, +) { + let (tx, rx) = std::sync::mpsc::channel::(); + + std::thread::spawn(move || { + let panic_tx = tx.clone(); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(move || { + // Multi-thread for the reason the browser flow is: a current-thread + // runtime left reqwest's future unpolled on Android. + let rt = match tokio::runtime::Builder::new_multi_thread() + .worker_threads(1) + .enable_io() + .enable_time() + .build() + { + Ok(rt) => rt, + Err(e) => { + let _ = tx.send(LoginMessage::Failed(format!("tokio runtime: {e}"))); + return; + } + }; + + rt.block_on(async { + let client = match dr_sync_nextcloud::http_client("DarkRoom") { + Ok(c) => c, + Err(e) => { + let _ = tx.send(LoginMessage::Failed(format!("http client: {e}"))); + return; + } + }; + + let creds = dr_sync_nextcloud::AppCredentials { + server, + login_name: login, + app_password: password, + }; + + // The credential is only worth storing if it actually works, + // and this is the cheapest request that proves it. A 401 comes + // back as an error here rather than as a puzzling failure on + // the first listing. + match fetch_user_id(&client, &creds).await { + Ok(user_id) => { + let _ = tx.send(LoginMessage::Success(Box::new((creds, user_id)))); + } + Err(e) => { + let _ = tx.send(LoginMessage::Failed(format!( + "could not sign in with that app password: {e}" + ))); + } + } + }); + })); + + if let Err(panic) = result { + let detail = panic + .downcast_ref::<&str>() + .map(|s| (*s).to_string()) + .or_else(|| panic.downcast_ref::().cloned()) + .unwrap_or_else(|| "panicked with a non-string payload".to_string()); + let _ = panic_tx.send(LoginMessage::Failed(format!("internal error: {detail}"))); + } + }); + + poll_channel(weak, ctl, rx); +} + /// The body of the login flow, split out so the worker above can wrap it in /// `catch_unwind` without a deeply nested closure. fn run_login_flow( @@ -487,8 +595,14 @@ fn spawn_folder_list(weak: slint::Weak, ctl: Rc, pa let user_id = session.user_id.clone(); std::thread::spawn(move || { - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() + // Multi-thread for the same reason as the login worker: a + // current-thread runtime left reqwest's connection future unpolled on + // Android, so the await never resolved and the thread stopped without + // failing or returning. + let rt = tokio::runtime::Builder::new_multi_thread() + .worker_threads(1) + .enable_io() + .enable_time() .build(); let Ok(rt) = rt else { let _ = tx.send(Err("runtime".into())); diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index 5ecda90..1969006 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -16,6 +16,7 @@ mod collections_ui; mod derived_sync; +mod net_runtime; mod develop; mod labels; mod library; diff --git a/ui/dr-ui/src/library.rs b/ui/dr-ui/src/library.rs index 022da8e..1731e46 100644 --- a/ui/dr-ui/src/library.rs +++ b/ui/dr-ui/src/library.rs @@ -79,6 +79,13 @@ pub struct ThumbnailReady { pub width: u32, pub height: u32, pub rgba: Vec, + /// Whether these pixels came off local disk rather than the server. + /// + /// The grid paints both identically, so this exists solely for + /// reachability: a store hit is evidence about the *cache*, not the + /// network, and treating one as proof of connectivity clears offline mode + /// before a single request has been attempted. + pub from_cache: bool, } /// Capture metadata read from the same header the thumbnail needed. @@ -330,9 +337,7 @@ pub fn spawn_sidecar_writes( let (tx, rx) = std::sync::mpsc::channel(); std::thread::spawn(move || { - let rt = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = match crate::net_runtime::build() { Ok(rt) => rt, Err(e) => { @@ -560,9 +565,7 @@ fn run_scan( } let catalog = Catalog::open(&catalog_path).map_err(ScanFailure::local)?; - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = crate::net_runtime::build() .map_err(ScanFailure::local)?; rt.block_on(async { @@ -887,9 +890,7 @@ pub fn spawn_pin_fetch( return; } - let rt = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = match crate::net_runtime::build() { Ok(rt) => rt, Err(e) => { @@ -1055,9 +1056,7 @@ pub fn spawn_full_fetch( } } - let rt = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = match crate::net_runtime::build() { Ok(rt) => rt, Err(e) => { @@ -1160,6 +1159,7 @@ pub fn spawn_thumbnails( width, height, rgba, + from_cache: true, }); } // A corrupt stored blob is a miss, not a failure. @@ -1202,9 +1202,7 @@ pub fn spawn_thumbnails( return; } - let rt = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = match crate::net_runtime::build() { Ok(rt) => rt, Err(e) => { @@ -1392,6 +1390,7 @@ async fn fetch_one( width: preview.width, height: preview.height, rgba: preview.rgba, + from_cache: false, })) } @@ -1675,9 +1674,7 @@ pub fn spawn_sweep( return; } - let rt = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = match crate::net_runtime::build() { Ok(rt) => rt, Err(e) => { @@ -2169,9 +2166,7 @@ mod tests { // The ordering guarantee is what lets a caller pair results back to // their inputs; without it a lane's dates could be attributed to the // wrong images. - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = crate::net_runtime::build() .unwrap(); let out = rt.block_on(async { @@ -2195,9 +2190,7 @@ mod tests { #[test] fn join_all_of_nothing_completes() { - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = crate::net_runtime::build() .unwrap(); let out: Vec = rt.block_on(async { futures_join_all(Vec::>::new()).await }); @@ -2636,6 +2629,61 @@ mod tests { assert!(out[2] > out[0], "channel order survived the round trip"); } + #[test] + fn a_store_hit_is_not_evidence_the_server_is_reachable() { + // The regression this guards: store hits were delivered as the same + // `Ready` the network path sends, and the UI took any `Ready` as proof + // of connectivity. A mostly-cached window then declared "back online" + // against a server that was down — clearing the banner and kicking off + // a sweep that immediately failed, on every scroll. + let dir = std::env::temp_dir().join(format!("dr-ui-provenance-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + + let rgba: Vec = std::iter::repeat_n([10u8, 20, 30, 255], 8 * 8) + .flatten() + .collect(); + let mut store = ThumbStore::open(&dir).unwrap(); + let bytes = dr_thumbs::encode_rgba(8, 8, &rgba).unwrap(); + store + .put(99, dr_thumbs::ThumbSize::Grid, &dr_thumbs::Thumbnail { + width: 8, + height: 8, + bytes, + }) + .unwrap(); + + // The split in `spawn_thumbnails`: a hit decodes off local disk and is + // reported with `from_cache` set, which is what the reachability gate + // keys on. + let stored = store.get(99, dr_thumbs::ThumbSize::Grid).unwrap().expect("stored"); + let (width, height, rgba) = dr_thumbs::decode_rgba(&stored.bytes).unwrap(); + let hit = ThumbnailReady { + row: 0, + width, + height, + rgba, + from_cache: true, + }; + + assert!( + hit.from_cache, + "a thumbnail read from the store must not be mistaken for a fetch" + ); + + // And the mechanism it feeds: an offline tracker must survive it. + let mut reach = dr_sync::Reachability::new(); + let now = std::time::Instant::now(); + reach.mark_unreachable("network error".into(), now); + + if !hit.from_cache { + reach.mark_reachable(now); + } + assert!( + reach.is_offline(), + "replaying cached thumbnails must leave offline mode intact" + ); + } + #[test] fn thumbs_live_beside_the_catalog_not_in_the_cache() { // They sync to the server and are shared with other clients, so a diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index 7aba8d0..ae287a2 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -1650,14 +1650,22 @@ fn drain_thumbnails( // carry no embedded preview. ThumbnailMessage::Ready(t) => { w.set_library_thumbs_done(w.get_library_thumbs_done() + 1); - // Bytes arrived, so the server is reachable. This is - // what clears the banner when a connection returns - // while the user is simply scrolling, without waiting - // for a probe or a manual retry. - if ctl_cb - .reachability - .borrow_mut() - .mark_reachable(std::time::Instant::now()) + // Bytes arrived *from the server*, so it is reachable. + // This is what clears the banner when a connection + // returns while the user is simply scrolling, without + // waiting for a probe or a manual retry. + // + // Store hits are excluded deliberately: they are read + // from local disk and say nothing about the network. A + // window that is mostly cached delivers a run of them + // before the first request is even attempted, so + // counting them declared "back online" against a server + // that was plainly down. + if !t.from_cache + && ctl_cb + .reachability + .borrow_mut() + .mark_reachable(std::time::Instant::now()) { log::info!("back online"); refresh_offline(&w, &ctl_cb); diff --git a/ui/dr-ui/src/net_runtime.rs b/ui/dr-ui/src/net_runtime.rs new file mode 100644 index 0000000..ac77e81 --- /dev/null +++ b/ui/dr-ui/src/net_runtime.rs @@ -0,0 +1,34 @@ +//! The tokio runtime every network worker is built from. +//! +//! One function rather than a builder repeated at each call site, because the +//! choice it encodes is not obvious and was wrong in eleven places at once. +//! +//! **Why not `new_current_thread`.** A current-thread runtime drives its +//! reactor only while the thread is inside `block_on`. On Android that left +//! reqwest's connection future unpolled: the await never resolved, so the +//! worker neither failed nor returned. Every symptom was an absence — no error, +//! no panic for `catch_unwind` to catch, no log line, and a channel that closed +//! only when the thread was finally torn down. What the user saw was a sign-in +//! that stopped, and a library that quietly scanned itself rather than adopting +//! the shards the server already held. +//! +//! A multi-thread runtime owns worker threads that poll the reactor regardless. +//! One worker is enough: these are single request-response flows, not +//! throughput-bound work. +//! +//! **Why not `enable_all`.** That also starts the signal driver, which wants +//! process-wide signal handling. Inside an Android app the runtime already owns +//! that, and a worker thread claiming it is asking for trouble. These flows need +//! IO (reqwest) and time (poll intervals, timeouts) and nothing else. + +/// Build a runtime suitable for a network worker thread. +/// +/// Call from the spawned thread, not from the caller: the runtime must live on +/// the thread that blocks on it. +pub fn build() -> std::io::Result { + tokio::runtime::Builder::new_multi_thread() + .worker_threads(1) + .enable_io() + .enable_time() + .build() +} diff --git a/ui/dr-ui/src/trash.rs b/ui/dr-ui/src/trash.rs index f2357ad..488c570 100644 --- a/ui/dr-ui/src/trash.rs +++ b/ui/dr-ui/src/trash.rs @@ -175,9 +175,7 @@ pub fn spawn_move( std::thread::spawn(move || { let total = moves.len(); - let rt = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = match crate::net_runtime::build() { Ok(rt) => rt, Err(e) => { @@ -291,9 +289,7 @@ pub fn spawn_purge( std::thread::spawn(move || { let total = paths.len(); - let rt = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() + let rt = match crate::net_runtime::build() { Ok(rt) => rt, Err(e) => { diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index 812c9de..f167c7d 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -216,6 +216,8 @@ export component AppWindow inherits Window { in-out property <[bool]> launch-format-checked; callback launch-sign-in(string); + /// server, username, app password + callback launch-sign-in-direct(string, string, string); callback launch-sign-out(); callback launch-choose-folder(); callback launch-open-library(); @@ -575,6 +577,9 @@ export component AppWindow inherits Window { format-checked: root.launch-format-checked; sign-in(server) => { root.launch-sign-in(server); } + sign-in-direct(server, user, pw) => { + root.launch-sign-in-direct(server, user, pw); + } sign-out() => { root.launch-sign-out(); } choose-folder() => { root.launch-choose-folder(); } open-library() => { root.launch-open-library(); } diff --git a/ui/dr-ui/ui/launch.slint b/ui/dr-ui/ui/launch.slint index bb6236d..9c57019 100644 --- a/ui/dr-ui/ui/launch.slint +++ b/ui/dr-ui/ui/launch.slint @@ -114,6 +114,8 @@ export component LaunchScreen inherits Rectangle { // --- events out --- callback sign-in(string); + /// server, username, app password + callback sign-in-direct(string, string, string); callback sign-out(); callback choose-folder(); callback open-library(); @@ -196,6 +198,64 @@ export component LaunchScreen inherits Rectangle { text: "Sign-in happens in your browser. DarkRoom never sees your password."; wrap: word-wrap; } + + // --- or: an app password entered directly --- + // + // The browser flow above is preferred because no credential + // ever reaches us. This exists for the cases it cannot + // serve: no browser able to complete the handshake, or a + // user who would rather paste a token they can revoke. + HorizontalLayout { + spacing: Theme.gap; + alignment: center; + Rectangle { + height: 1px; + background: Theme.rule; + horizontal-stretch: 1; + } + Caption { text: "or"; } + Rectangle { + height: 1px; + background: Theme.rule; + horizontal-stretch: 1; + } + } + + PanelHeading { text: "USERNAME"; } + user-input := Field { + text: ""; + placeholder: "your Nextcloud username"; + } + + PanelHeading { text: "APP PASSWORD"; } + pass-input := Field { + text: ""; + placeholder: "xxxxx-xxxxx-xxxxx-xxxxx-xxxxx"; + secret: true; + accepted(pw) => { + root.sign-in-direct(server-input.text, user-input.text, pw); + } + } + + FormButton { + text: root.busy ? "Connecting…" : "Connect directly"; + enabled: !root.busy + && server-input.text != "" + && user-input.text != "" + && pass-input.text != ""; + clicked => { + root.sign-in-direct( + server-input.text, + user-input.text, + pass-input.text, + ); + } + } + + Caption { + text: "Create one in Nextcloud under Settings → Security → Devices & sessions. It is device-scoped and can be revoked on its own."; + wrap: word-wrap; + } } // --- login pending: the browser step --- diff --git a/ui/dr-ui/ui/widgets.slint b/ui/dr-ui/ui/widgets.slint index 8310746..b98f58d 100644 --- a/ui/dr-ui/ui/widgets.slint +++ b/ui/dr-ui/ui/widgets.slint @@ -528,6 +528,9 @@ export component Panel inherits Rectangle { export component Field inherits Rectangle { in-out property text; in property placeholder; + /// Masks the entry, for a credential that should not be readable over the + /// user's shoulder. The placeholder still shows while the field is empty. + in property secret: false; /// Whether the entry currently holds focus, so a caller can enable its /// submit button from the same fact the border is drawn from. out property has-focus: input.has-focus; @@ -552,6 +555,7 @@ export component Field inherits Rectangle { width: parent.width - 2 * Theme.gap; height: 100%; single-line: true; + input-type: root.secret ? InputType.password : InputType.text; accepted => { root.accepted(self.text); } }