diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 957824d..ce0a034 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,5 +34,10 @@ jobs: - name: Test run: cargo test + - name: i18n catalog parity + run: | + python3 tools/gen-translations.py po/es.po /tmp/es-regenerated.rs + diff -u src/util/translations/es.rs /tmp/es-regenerated.rs + - name: Build release run: cargo build --release diff --git a/CHANGELOG.md b/CHANGELOG.md index cdb3103..205c122 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ Todas las versiones notables de NextSync se documentan aquí. El formato sigue [Keep a Changelog](https://keepachangelog.com/es/1.1.0/) y el versionado es **+0.02 por release** (decisión del usuario, 15-Ago-2026). +## [0.120.0] - 2026-08-21 + +### Corregido +- **El push se reconecta al cambiar la contraseña (#133)**: tras una re-autenticación, el canal push usa la contraseña nueva de inmediato; antes seguía con la antigua hasta una reconexión casual. +- **El overflow del watcher ya no se pierde (#134)**: la señal de rescan se guarda en un flag atómico que no compite con el buffer lleno; antes podía descartarse y se sincronizaba desde un stream parcial. +- **El avatar se borra al quitar la cuenta y se limita su tamaño (#135)**: la caché de avatares ya no deja huérfanos ni persiste cuerpos desmedidos. +- **Escritura de configuración durable (#136)**: nombres temporales únicos (pid+contador) y fsync del directorio tras el rename; antes dos instancias podían pisarse y un corte de luz deshacía el cambio. +- **schema_version ilegible se rechaza (#137)**: un valor no entero/negativo ya no se trata como v1 y no migra datos corruptos. +- **color_scheme validado (#138)**: solo `system`/`light`/`dark` llegan al conmutador de tema; cualquier otro valor cae al predeterminado. +- **Secreto no UTF-8 ya no se corrompe en silencio (#139)**: la tienda de credenciales devuelve un error en vez de sustituir bytes inválidos. +- **El Debug redacta el secreto (#140)**: `DriverContext` y `CommandSpec` ya no imprimen la contraseña o el token en los logs. +- **Las notificaciones del servidor siembran la línea base una vez (#141)**: un primer sondeo vacío ya no hace que la primera notificación real se trague. +- **Catálogo ES sincronizado con el po y verificado en CI (#142)**: las 18 cadenas huérfanas vuelven al po, es.rs se regenera de él y el CI falla ante cualquier desviación. +- **Título del tray sin em dash (#143)**: "NextSync - {estado}" con guión normal. +- **Código muerto eliminado (#144)**: el parser `parse_metered` que solo usaba su propio test. +- **El progreso ya no reaparece tras terminar el run (#145)**: un flag compartido impide que los eventos residuales del buffer repinten el label después de que la sincronización acabó. + ## [0.118.0] - 2026-08-21 ### Corregido diff --git a/Cargo.lock b/Cargo.lock index 24abdf1..c371b65 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1230,7 +1230,7 @@ dependencies = [ [[package]] name = "nextsync" -version = "0.118.0" +version = "0.120.0" dependencies = [ "async-channel", "data-encoding", diff --git a/Cargo.toml b/Cargo.toml index b15a695..0485266 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nextsync" -version = "0.118.0" +version = "0.120.0" edition = "2021" rust-version = "1.83" license = "GPL-3.0-or-later" diff --git a/PKGBUILD b/PKGBUILD index 957daca..d12916b 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -1,6 +1,6 @@ # Maintainer: gnacho pkgname=nextsync -pkgver=0.118.0 +pkgver=0.120.0 pkgrel=1 pkgdesc='Nextcloud desktop synchronization client for GNOME (Rust rewrite)' arch=('x86_64' 'aarch64') diff --git a/README.es.md b/README.es.md index bdbd1e3..bea263c 100644 --- a/README.es.md +++ b/README.es.md @@ -12,7 +12,7 @@ English

- Versión 0.118.0 + Versión 0.120.0 Linux GNOME GTK 4 diff --git a/README.md b/README.md index ea7f6e7..e12bb64 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Español

- Version 0.118.0 + Version 0.120.0 Linux GNOME GTK 4 diff --git a/po/es.po b/po/es.po index 3db9830..6dd6313 100644 --- a/po/es.po +++ b/po/es.po @@ -1764,8 +1764,8 @@ msgstr "Empezar" #: src/nextsync/ui/tray.py:266 #, python-brace-format -msgid "NextSync — {state}" -msgstr "NextSync — {state}" +msgid "NextSync - {state}" +msgstr "NextSync - {state}" #: src/nextsync/ui/tray.py:270 msgid "Syncing" @@ -2653,3 +2653,58 @@ msgstr "No hay archivos borrados que resolver" #: src/ui/conflict_resolver.rs msgid "The deletion guard has not flagged any files in this folder." msgstr "El guard de borrado no ha marcado ningún archivo en esta carpeta." + +msgid "Could not list the remote folders." +msgstr "No se pudieron listar las carpetas remotas." + +msgid "Keep Local for All" +msgstr "Conservar la local en todo" + +msgid "Keep Remote for All" +msgstr "Conservar la remota en todo" + +msgid "Keep the local version of all {count} conflicted copy(ies)? The conflicted copies are deleted." +msgstr "¿Conservar la versión local de las {count} copias en conflicto? Las copias en conflicto se eliminan." + +msgid "Keep the server version of all {count} conflicted copy(ies)? The working files are replaced." +msgstr "¿Conservar la versión del servidor de las {count} copias en conflicto? Los archivos de trabajo se reemplazan." + +msgid "Kept the local version of {count} file(s)" +msgstr "Versión local conservada en {count} archivo(s)" + +msgid "Kept the server version of {count} file(s)" +msgstr "Versión del servidor conservada en {count} archivo(s)" + +msgid "Resolve All Conflicts" +msgstr "Resolver todos los conflictos" + +msgid "Synced in local {folder}" +msgstr "Sincronizado en local {folder}" + +msgid "Waiting to synchronize" +msgstr "Esperando para sincronizar" + +msgid "checking {file}" +msgstr "comprobando {file}" + +msgid "conflict on {file}" +msgstr "conflicto en {file}" + +msgid "deleting {file}" +msgstr "eliminando {file}" + +msgid "downloading {file}" +msgstr "descargando {file}" + +msgid "processing {file}" +msgstr "procesando {file}" + +msgid "uploading {file}" +msgstr "subiendo {file}" + +msgid "{action} · {count}" +msgstr "{action} · {count}" + +msgid "{used} used" +msgstr "{used} usados" + diff --git a/src/core/account_runtime.rs b/src/core/account_runtime.rs index 507582e..1b8ccbc 100644 --- a/src/core/account_runtime.rs +++ b/src/core/account_runtime.rs @@ -294,6 +294,12 @@ impl FolderRuntime { let task = glib::spawn_future_local(async move { let mut watcher = watcher; while let Ok(event) = receiver.recv().await { + // An overflow may have been flagged while the buffer was full + // (issue #134): it cannot be delivered through the full + // channel, so the consumer rescans on the flag instead. + if watcher.take_overflow() { + watcher.rescan(); + } match event { WatcherEvent::Change(_) | WatcherEvent::Rescan => { scheduler.request(Trigger::LocalInotify); @@ -317,8 +323,18 @@ impl FolderRuntime { return; }; let state_for_progress = self.state.clone(); + // Issue #145: the engine's sender survives across runs, so events + // still in the buffer after a run finishes are drained AFTER the + // scheduler cleared the label. The scheduler flips this flag off at + // run end; while it is off, residual events are dropped so they + // cannot repaint the row. + let run_active = std::rc::Rc::new(std::cell::Cell::new(false)); + self.scheduler.set_run_active(run_active.clone()); glib::spawn_future_local(async move { while let Ok(progress) = progress_rx.recv().await { + if !run_active.get() { + continue; + } state_for_progress.set_progress(Some(progress)); } }); diff --git a/src/core/network.rs b/src/core/network.rs index e19e6e3..eb606cc 100644 --- a/src/core/network.rs +++ b/src/core/network.rs @@ -50,15 +50,6 @@ pub fn parse_active_ssid(output: &str) -> Option { }) } -/// Parse `nmcli -t -f GENERAL.METERED dev status` output into whether any -/// device reports a metered connection. Each line is `device:metered` in -/// the common case, but tolerate extra fields (`device:state:metered`). -pub fn parse_metered(output: &str) -> bool { - output - .lines() - .any(|line| line.split(':').any(|field| field.trim() == "yes")) -} - /// Parse the raw `network.allowed_ssids` config value (comma separated). /// Empty entries are dropped; comparison elsewhere is exact. pub fn parse_allowed_ssids(raw: &str) -> Vec { @@ -347,14 +338,6 @@ mod tests { assert_eq!(parse_active_ssid(""), None); } - #[test] - fn parse_metered_only_triggers_on_yes() { - assert!(parse_metered("wlan0:connected:yes\neth0:connected:no")); - assert!(!parse_metered("wlan0:connected:no")); - assert!(!parse_metered("wlan0:connected:unknown")); - assert!(!parse_metered("")); - } - #[test] fn parse_allowed_ssids_drops_empty_entries() { assert_eq!( diff --git a/src/core/scheduler.rs b/src/core/scheduler.rs index a7a167a..e1e679b 100644 --- a/src/core/scheduler.rs +++ b/src/core/scheduler.rs @@ -141,6 +141,11 @@ struct SchedulerInner { /// automatic triggers stay queued until the user signs in again, so a /// revoked password cannot hammer the server into a brute-force lockout. auth_required: bool, + /// Shared flag: true while a run is in flight, false the moment it + /// finishes. The progress forwarder reads it so stale events drained + /// after the run's `set_progress(None)` cannot repaint the label + /// (issue #145). + run_active: Option>>, } impl Scheduler { @@ -188,6 +193,7 @@ impl Scheduler { active_ssid: None, quiet_hours: None, auth_required: false, + run_active: None, }; let inner = Rc::new(RefCell::new(inner)); { @@ -275,6 +281,13 @@ impl Scheduler { self.inner.borrow_mut().local_root = local_root; } + /// Share the run-active flag with the progress forwarder (issue #145): + /// the forwarder must not repaint the label from stale events once a run + /// has finished and cleared it. + pub fn set_run_active(&self, flag: std::rc::Rc>) { + self.inner.borrow_mut().run_active = Some(flag); + } + /// Override the procfs directory scanned for external engine processes. /// Production leaves this unset (the real `/proc`); tests inject a fake. pub fn set_proc_scan_root(&self, proc_scan_root: Option) { @@ -552,6 +565,9 @@ impl SchedulerInner { } fn prepare_sync(&mut self, reasons: Vec) { + if let Some(flag) = &self.run_active { + flag.set(true); + } self.state.set(AppState::Syncing, t("Synchronizing files…")); self.state.set_progress(None); self.preparing = true; @@ -695,6 +711,12 @@ impl SchedulerInner { callback(&outcome); } self.running = false; + // The run is over: clear the shared run-active flag BEFORE the + // progress is cleared, so the forwarder stops repainting the label + // from stale events drained after this point (issue #145). + if let Some(flag) = &self.run_active { + flag.set(false); + } if ran { if self.inotify_during_sync { if feedback_followup { @@ -1389,6 +1411,21 @@ mod tests { assert_eq!(runner.0.borrow().start_calls, 2); } + #[test] + fn run_active_flag_tracks_the_run_lifecycle() { + // Issue #145: the flag is true while the run is in flight and off + // once it finishes, so the progress forwarder can drop stale events. + let (scheduler, source, runner) = make_scheduler(None); + let flag = std::rc::Rc::new(std::cell::Cell::new(false)); + scheduler.set_run_active(flag.clone()); + assert!(!flag.get()); + scheduler.request(Trigger::Manual); + run_idle(&source); + assert!(flag.get(), "the flag must be on while the run is in flight"); + finish(&runner, SyncOutcome::Success); + assert!(!flag.get(), "the flag must clear when the run finishes"); + } + #[test] fn delete_alert_blocks_and_approve_once_bypasses() { let (scheduler, source, runner) = make_scheduler(None); diff --git a/src/core/server_notifications.rs b/src/core/server_notifications.rs index cc9b212..9edf423 100644 --- a/src/core/server_notifications.rs +++ b/src/core/server_notifications.rs @@ -54,6 +54,10 @@ pub struct ServerNotificationWatcher { logger: crate::core::log::LogBuffer, /// Notification ids already shown (or seeded as the baseline). seen: Rc>>, + /// Whether the baseline has been seeded. A separate flag from `seen` + /// being non-empty: the first poll may legitimately return zero + /// notifications, and the seed must happen exactly once (issue #141). + seeded: Rc>, /// Guards against overlapping fetches when the push poke and the timer /// fire together. running: Rc>, @@ -79,6 +83,7 @@ impl ServerNotificationWatcher { notifier, logger, seen: Rc::new(RefCell::new(HashSet::new())), + seeded: Rc::new(RefCell::new(false)), running: Rc::new(RefCell::new(false)), source_id: RefCell::new(None), } @@ -119,6 +124,7 @@ impl ServerNotificationWatcher { let server = self.server.clone(); let login = self.login.clone(); let seen = self.seen.clone(); + let seeded = self.seeded.clone(); let notifier = self.notifier.clone(); let logger = self.logger.clone(); let running = self.running.clone(); @@ -139,13 +145,17 @@ impl ServerNotificationWatcher { // the join-handle result (the outer layer reports panics). match task.await { Ok(Ok(notifications)) => { - if seen.borrow().is_empty() { + if !*seeded.borrow() { // First run: seed the baseline so enabling the option - // does not replay a backlog of old notifications. + // does not replay a backlog of old notifications. The + // flag (not the set being empty) decides, so a first + // poll that returns zero items still counts as seeded + // (issue #141). let mut seen = seen.borrow_mut(); for item in ¬ifications { seen.insert(item.notification_id); } + *seeded.borrow_mut() = true; } else { let new_items = { let seen = seen.borrow(); @@ -216,4 +226,21 @@ mod tests { let seen = HashSet::from([1i64]); assert!(unseen(&seen, &items).is_empty()); } + + #[test] + fn seeded_flag_decides_the_baseline_not_the_set_size() { + // Issue #141: a first poll that returns zero notifications still + // seeds the baseline, so the next poll with one real notification is + // not swallowed by a second seed. + let seen = Rc::new(RefCell::new(HashSet::new())); + let seeded = Rc::new(RefCell::new(false)); + // First poll: zero notifications, but it marks the baseline seeded. + *seeded.borrow_mut() = true; + assert!(seen.borrow().is_empty()); + assert!(*seeded.borrow()); + // A later poll with a real notification finds the seed flag on. + let items = [sample(7, "new")]; + let new_items = unseen(&seen.borrow(), &items); + assert_eq!(new_items.len(), 1); + } } diff --git a/src/core/watcher.rs b/src/core/watcher.rs index f7e4cfc..2fe0773 100644 --- a/src/core/watcher.rs +++ b/src/core/watcher.rs @@ -75,6 +75,11 @@ pub struct FsWatcher { root: PathBuf, watcher: RecommendedWatcher, sender: Sender, + /// Set by the notify callback when the bounded channel is full and an + /// event was dropped. Read and cleared by the consumer; it does not + /// compete for the full buffer, so the overflow is never lost (issue + /// #134). + overflow: std::sync::Arc, } impl FsWatcher { @@ -93,6 +98,8 @@ impl FsWatcher { } let (sender, receiver) = async_channel::bounded(1024); let event_sender = sender.clone(); + let overflow = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let overflow_signal = overflow.clone(); let mut watcher = notify::recommended_watcher(move |result: Result| { let event = match result { @@ -102,10 +109,12 @@ impl FsWatcher { if let Some(event) = event { if event_sender.try_send(event).is_err() { // Backpressure: the consumer fell behind and the bounded - // channel dropped history. Signal an overflow so it can - // rescan instead of syncing from a partial event stream. - let _ = - event_sender.try_send(WatcherEvent::Degraded(WatcherError::Overflow)); + // channel dropped history. Set the overflow flag instead + // of trying to send again into the same full buffer (that + // second send could also fail and the rescan request + // would be lost): the consumer polls it and rescans, + // avoiding a sync from a partial event stream. + overflow_signal.store(true, std::sync::atomic::Ordering::Relaxed); } } }) @@ -118,6 +127,7 @@ impl FsWatcher { root, watcher, sender, + overflow, }, receiver, )) @@ -151,6 +161,15 @@ impl FsWatcher { } } } + + /// Whether the notify callback reported an overflow since the last poll. + /// The consumer calls this on every received event and rescans when it + /// reads `true`, so a full buffer never silently loses the rescan + /// request (issue #134). + pub fn take_overflow(&self) -> bool { + self.overflow + .swap(false, std::sync::atomic::Ordering::Relaxed) + } } /// Map a `notify` event to a [`WatcherEvent`]. @@ -236,6 +255,21 @@ mod tests { ); } + #[test] + fn take_overflow_reads_once_and_clears_the_flag() { + // Issue #134: the overflow flag must survive a full channel (it is + // set by the notify callback instead of a second try_send into the + // same full buffer) and be consumed once by the loop. + let dir = tempdir().unwrap(); + let (watcher, _receiver) = FsWatcher::start(dir.path(), empty_matcher()).unwrap(); + assert!(!watcher.take_overflow()); + watcher + .overflow + .store(true, std::sync::atomic::Ordering::Relaxed); + assert!(watcher.take_overflow()); + assert!(!watcher.take_overflow()); + } + #[test] fn mutating_events_become_changes() { for kind in [ diff --git a/src/nextcloud/command.rs b/src/nextcloud/command.rs index 7bec8b4..f37303a 100644 --- a/src/nextcloud/command.rs +++ b/src/nextcloud/command.rs @@ -54,7 +54,7 @@ impl fmt::Display for CommandError { impl std::error::Error for CommandError {} /// A fully resolved `nextcloudcmd` invocation: argv plus environment. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub struct CommandSpec { /// Argument vector, first element is the executable. pub argv: Vec, @@ -62,6 +62,36 @@ pub struct CommandSpec { pub environment: Vec<(String, String)>, } +impl std::fmt::Debug for CommandSpec { + /// Custom `Debug` so secret-bearing environment values never reach logs + /// (issue #140): the argv is shown in full, but the password/token + /// variables print as `[REDACTED]`. + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CommandSpec") + .field("argv", &self.argv) + .field( + "environment", + &self + .environment + .iter() + .map(|(key, value)| { + let shown = if SECRET_ENV_KEYS.iter().any(|secret| secret == key) { + "[REDACTED]".to_string() + } else { + value.clone() + }; + (key.clone(), shown) + }) + .collect::>(), + ) + .finish() + } +} + +/// Environment variable names whose values are account secrets and must be +/// redacted from `Debug` output (issue #140). +const SECRET_ENV_KEYS: [&str; 3] = ["NC_PASSWORD", "OPENCLOUD_TOKEN", "OPENCLOUD_PASSWORD"]; + impl CommandSpec { /// Materialize this spec as a `std::process::Command`. pub fn to_command(&self) -> std::process::Command { @@ -357,6 +387,25 @@ mod tests { assert!(spec.argv.iter().any(|arg| arg == "-h")); } + #[test] + fn debug_redacts_secret_environment_values() { + // Issue #140: formatting a spec for logs must never echo the secret. + let spec = build_command( + &account(), + &folder(), + &NetworkConfig::default(), + "very-secret", + None, + Some(Path::new("/bin/true")), + ) + .expect("build should succeed"); + let rendered = format!("{spec:?}"); + assert!(rendered.contains("[REDACTED]")); + assert!(!rendered.contains("very-secret")); + // The argv is still readable (the binary path matters for debugging). + assert!(rendered.contains("/bin/true")); + } + #[test] fn options_map_without_shell() { let mut account = account(); diff --git a/src/nextcloud/credentials.rs b/src/nextcloud/credentials.rs index 3d9c0b1..76e92ab 100644 --- a/src/nextcloud/credentials.rs +++ b/src/nextcloud/credentials.rs @@ -17,8 +17,39 @@ use std::collections::HashMap; use secret_service::blocking::SecretService; use secret_service::EncryptionType; -/// Error produced by the credential store (a [`secret_service::Error`]). -pub type CredentialError = secret_service::Error; +/// Error produced by the credential store: a wrapped [`secret_service::Error`] +/// or an unreadable stored secret (issue #139). +#[derive(Debug)] +pub enum CredentialError { + /// The Secret Service call failed (locked, unavailable, transport…). + Service(secret_service::Error), + /// The stored secret is not valid UTF-8 and cannot be used as a password. + Utf8, +} + +impl std::fmt::Display for CredentialError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + CredentialError::Service(error) => write!(f, "{error}"), + CredentialError::Utf8 => write!(f, "stored secret is not valid UTF-8"), + } + } +} + +impl std::error::Error for CredentialError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + CredentialError::Service(error) => Some(error), + CredentialError::Utf8 => None, + } + } +} + +impl From for CredentialError { + fn from(error: secret_service::Error) -> Self { + CredentialError::Service(error) + } +} /// Attribute key used to index items by account id. const ATTR_ACCOUNT_ID: &str = "account_id"; @@ -42,6 +73,7 @@ fn collection<'a>( service .get_collection_by_alias("login") .or_else(|_| service.get_default_collection()) + .map_err(CredentialError::from) } /// Stores and retrieves account passwords in the Secret Service collection @@ -77,11 +109,17 @@ impl CredentialsStore { return if result.locked.is_empty() { Ok(None) } else { - Err(CredentialError::Locked) + Err(CredentialError::Service(secret_service::Error::Locked)) }; }; let secret = item.get_secret()?; - Ok(Some(String::from_utf8_lossy(&secret).into_owned())) + match std::str::from_utf8(&secret) { + Ok(password) => Ok(Some(password.to_string())), + // The stored bytes are not a usable password (issue #139): a + // silent lossy substitution would authenticate with a different + // string and fail opaquely. Surface it as an error instead. + Err(_) => Err(CredentialError::Utf8), + } } /// Read the password for an account, falling back to the legacy entry. @@ -110,7 +148,7 @@ impl CredentialsStore { return if result.locked.is_empty() { Ok(None) } else { - Err(CredentialError::Locked) + Err(CredentialError::Service(secret_service::Error::Locked)) }; }; let secret = item.get_secret()?; @@ -260,4 +298,14 @@ mod tests { .expect("get_for_account should succeed"); assert!(missing.is_none()); } + + #[test] + fn utf8_error_is_describable_and_does_not_expose_the_secret() { + // Issue #139: the Utf8 variant is constructible without a bus and + // its message never echoes the bytes. + let error = CredentialError::Utf8; + let message = error.to_string(); + assert!(message.contains("UTF-8")); + assert!(!message.contains("0xFF")); + } } diff --git a/src/nextcloud/driver.rs b/src/nextcloud/driver.rs index b1f29c2..c1b73aa 100644 --- a/src/nextcloud/driver.rs +++ b/src/nextcloud/driver.rs @@ -83,7 +83,7 @@ impl FromStr for Provider { /// expects (`NC_PASSWORD` for Nextcloud, the app password / token for /// OpenCloud) and `sync_hidden_files` follows the OpenCloud flag that is /// opt-in by default (the Nextcloud driver always syncs hidden files via `-h`). -#[derive(Debug, Clone, PartialEq)] +#[derive(Clone, PartialEq)] pub struct DriverContext { /// Account server URL, already normalized. pub server_url: String, @@ -111,6 +111,26 @@ pub struct DriverContext { pub executable: Option, } +impl std::fmt::Debug for DriverContext { + /// Custom `Debug` so the account secret never reaches logs (issue #140). + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DriverContext") + .field("server_url", &self.server_url) + .field("user", &self.user) + .field("password", &"[REDACTED]") + .field("local_root", &self.local_root) + .field("remote_path", &self.remote_path) + .field("space_id", &self.space_id) + .field("network", &self.network) + .field("retries", &self.retries) + .field("detailed_output", &self.detailed_output) + .field("sync_hidden_files", &self.sync_hidden_files) + .field("exclude_file", &self.exclude_file) + .field("executable", &self.executable) + .finish() + } +} + impl DriverContext { /// Build a context from the configuration types of one folder pair. pub fn from_folder( diff --git a/src/nextcloud/push.rs b/src/nextcloud/push.rs index 452a7be..e1f5135 100644 --- a/src/nextcloud/push.rs +++ b/src/nextcloud/push.rs @@ -184,10 +184,14 @@ impl NotifyPushClient { let effective_enabled = enabled && remote_push_supported(self.inner.borrow().provider); let changed = { let mut inner = self.inner.borrow_mut(); - let changed = (server, username, effective_enabled) + // The password participates in the change detection (issue #133): + // after a re-authentication the worker must pick up the new + // secret immediately, not keep the one captured at startup. + let changed = (server, username, password, effective_enabled) != ( inner.server.as_str(), inner.username.as_str(), + inner.password.as_str(), inner.enabled, ); inner.server = server.to_string(); @@ -1136,6 +1140,56 @@ mod tests { assert_eq!(states.last().unwrap().1, "Push notifications are disabled."); } + #[test] + fn configure_with_a_new_password_reconnects() { + // Issue #133: a re-authentication changes the password; the push + // worker must pick it up immediately. Each configure with a changed + // password disconnects and starts a fresh connect attempt. + let _guard = TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let context = glib::MainContext::new(); + context + .with_thread_default(|| { + let (client, states, _notifications) = test_client(Provider::Nextcloud); + client.configure("https://cloud.example.com", "alice", "secret", true); + client.configure("https://cloud.example.com", "alice", "new-secret", true); + let states = states.borrow(); + let connects = states + .iter() + .filter(|(state, _)| matches!(state, PushState::Connecting)) + .count(); + assert!( + connects >= 2, + "a password change must reconnect (states: {states:?})" + ); + }) + .expect("the test main context is available"); + } + + #[test] + fn configure_with_the_same_password_does_not_reconnect() { + // Same server/user/password/enabled → nothing changes, so no new + // connect attempt beyond the first. + let _guard = TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let context = glib::MainContext::new(); + context + .with_thread_default(|| { + let (client, states, _notifications) = test_client(Provider::Nextcloud); + client.configure("https://cloud.example.com", "alice", "secret", true); + client.configure("https://cloud.example.com", "alice", "secret", true); + let states = states.borrow(); + let connects = states + .iter() + .filter(|(state, _)| matches!(state, PushState::Connecting)) + .count(); + assert_eq!(connects, 1, "an identical configure must not reconnect"); + }) + .expect("the test main context is available"); + } + #[test] fn opencloud_account_never_attempts_to_connect() { let (client, states, notifications) = test_client(Provider::OpenCloud); diff --git a/src/nextcloud/sync_engine.rs b/src/nextcloud/sync_engine.rs index 3504ac3..bd0982c 100644 --- a/src/nextcloud/sync_engine.rs +++ b/src/nextcloud/sync_engine.rs @@ -94,7 +94,9 @@ impl CredentialSource for KeyringCredentialSource { ) { Ok(Some(password)) => CredentialLookup::Found(password), Ok(None) => CredentialLookup::Missing, - Err(secret_service::Error::Locked) => CredentialLookup::Locked, + Err(crate::nextcloud::credentials::CredentialError::Service( + secret_service::Error::Locked, + )) => CredentialLookup::Locked, Err(_) => CredentialLookup::Unavailable, } } diff --git a/src/storage/config.rs b/src/storage/config.rs index 74fea4a..65d1729 100644 --- a/src/storage/config.rs +++ b/src/storage/config.rs @@ -16,6 +16,7 @@ use std::io::{self, Write}; use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt, PermissionsExt}; use std::path::{Path, PathBuf}; use std::str::FromStr; +use std::sync::atomic::{AtomicU64, Ordering}; use serde::{Deserialize, Serialize}; use serde_json::{json, Map, Value}; @@ -506,8 +507,24 @@ pub fn validate_config(value: Value) -> Result { .ok_or_else(|| ConfigError::new("Configuration root must be an object."))?; let version = match data.get("schema_version") { - Some(Value::Number(number)) => number.as_i64().unwrap_or(0), - _ => 1, + // Absent means legacy v1, which the migrations handle. + None | Some(Value::Null) => 1, + // Present but not a clean positive integer is corrupt, not v1 + // (issue #137): running the v1 migrations over such data would + // misread it. + Some(Value::Number(number)) => match number.as_i64() { + Some(version) if version >= 1 => version, + _ => { + return Err(ConfigError::new( + "Configuration schema_version is not a valid integer.", + )) + } + }, + Some(_) => { + return Err(ConfigError::new( + "Configuration schema_version is not a valid integer.", + )) + } }; if version > SCHEMA_VERSION as i64 { return Err(ConfigError::new(format!( @@ -637,7 +654,7 @@ impl ConfigStore { ConfigError::new(format!("Could not serialize configuration: {error}")) })? + "\n"; - let temporary = self.path.with_extension("tmp"); + let temporary = temporary_path(&self.path); let write_result = (|| -> io::Result<()> { let mut handle = fs::OpenOptions::new() .write(true) @@ -648,7 +665,11 @@ impl ConfigStore { handle.write_all(content.as_bytes())?; handle.sync_all()?; drop(handle); - fs::rename(&temporary, &self.path) + fs::rename(&temporary, &self.path)?; + // Durability: an fsync of the directory makes the rename itself + // survive a power cut, not just the file contents (issue #136). + fs::File::open(parent)?.sync_all()?; + Ok(()) })(); if let Err(error) = write_result { let _ = fs::remove_file(&temporary); @@ -788,6 +809,24 @@ impl ConfigStore { } } +/// Counter for unique temporary names within this process. +static TMP_SEQUENCE: AtomicU64 = AtomicU64::new(0); + +/// A unique temporary path for an atomic config write (issue #136). The +/// fixed `.tmp` suffix allowed two concurrent instances to share the same +/// staging file and interleave writes; pid + a per-process counter keeps +/// every writer on its own file. +fn temporary_path(config_path: &Path) -> PathBuf { + let pid = std::process::id(); + let sequence = TMP_SEQUENCE.fetch_add(1, Ordering::Relaxed); + let mut name = config_path + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| "config".to_string()); + name.push_str(&format!(".tmp-{pid}-{sequence}")); + config_path.with_file_name(name) +} + // --------------------------------------------------------------------------- // Schema migrations (order: v3, v5, v6), mirroring `config.py`. // --------------------------------------------------------------------------- @@ -1118,7 +1157,10 @@ fn validate_general(raw: Option<&Value>) -> GeneralConfig { GeneralConfig { autostart: get_bool(obj, "autostart", true), pause_on_battery: get_bool(obj, "pause_on_battery", false), - color_scheme: get_string(obj, "color_scheme", &default_color_scheme()), + // The theme switch only understands system/light/dark; anything else + // falls back to the system default instead of reaching the UI + // (issue #138). + color_scheme: get_valid_color_scheme(obj), show_notifications: get_bool(obj, "show_notifications", true), show_server_notifications: get_bool(obj, "show_server_notifications", false), size_confirm_threshold_mb: get_i64_tolerant( @@ -1132,6 +1174,18 @@ fn validate_general(raw: Option<&Value>) -> GeneralConfig { } } +/// Read `color_scheme` restricted to the three values the theme switch +/// understands (`system` / `light` / `dark`); anything else is treated as +/// missing and falls back to the default (issue #138). +fn get_valid_color_scheme(obj: &serde_json::Map) -> String { + match obj.get("color_scheme") { + Some(Value::String(value)) if matches!(value.as_str(), "system" | "light" | "dark") => { + value.clone() + } + _ => default_color_scheme(), + } +} + fn validate_logging(raw: Option<&Value>) -> Result { let mut merged = json!({ "save_logs": true, "retention_days": 30 }); if let Some(incoming) = raw { @@ -1305,13 +1359,6 @@ fn get_bool(obj: &Map, key: &str, default: bool) -> bool { } } -fn get_string(obj: &Map, key: &str, default: &str) -> String { - match obj.get(key) { - Some(Value::String(text)) => text.clone(), - _ => default.to_string(), - } -} - fn sha256_hex(input: &[u8]) -> String { let digest = Sha256::digest(input); digest.iter().map(|byte| format!("{byte:02x}")).collect() @@ -1580,6 +1627,26 @@ mod tests { .contains("Configuration schema 8 is newer than this application supports")); } + #[test] + fn illegible_schema_version_is_rejected_not_treated_as_v1() { + // Issue #137: a present but unreadable schema_version must not run + // the v1 migrations over corrupt data. + let err = validate_config(json!({ "schema_version": "7" })).unwrap_err(); + assert!(err + .message + .contains("schema_version is not a valid integer")); + let err = validate_config(json!({ "schema_version": 7.5 })).unwrap_err(); + assert!(err + .message + .contains("schema_version is not a valid integer")); + let err = validate_config(json!({ "schema_version": -1 })).unwrap_err(); + assert!(err + .message + .contains("schema_version is not a valid integer")); + // Absent stays the legitimate v1 default. + assert!(validate_config(json!({})).is_ok()); + } + #[test] fn retention_days_and_proxy_validation() { let err = validate_config(json!({ "logging": { "retention_days": 0 } })).unwrap_err(); @@ -1598,6 +1665,18 @@ mod tests { assert!(err.message.contains("custom proxy")); } + #[test] + fn invalid_color_scheme_falls_back_to_the_default() { + // Issue #138: only system/light/dark reach the theme switch; any + // other value is treated as missing. + let config = validate_config(json!({ "general": { "color_scheme": "banana" } })).unwrap(); + assert_eq!(config.general.color_scheme, "system"); + let config = validate_config(json!({ "general": { "color_scheme": "dark" } })).unwrap(); + assert_eq!(config.general.color_scheme, "dark"); + let config = validate_config(json!({ "general": { "color_scheme": 7 } })).unwrap(); + assert_eq!(config.general.color_scheme, "system"); + } + // ---- migrations ----------------------------------------------------------- #[test] @@ -1852,6 +1931,35 @@ mod tests { assert!(!obj.contains_key("runtime")); } + #[test] + fn consecutive_saves_use_unique_temporary_names_and_leave_none_behind() { + // Issue #136: every write stages on its own pid+counter tmp file, so + // two writers cannot interleave on a shared settings.tmp, and the + // rename removes the staging file. + let dir = tempdir().unwrap(); + let path = dir.path().join("settings.json"); + let store = ConfigStore::with_path(path.clone()); + let first = temporary_path(&path); + let second = temporary_path(&path); + assert_ne!(first, second); + assert!(first.to_string_lossy().contains(".tmp-")); + + let config = Config::default(); + store.save(&config).unwrap(); + store.save(&config).unwrap(); + let leftovers = fs::read_dir(dir.path()) + .unwrap() + .filter_map(|entry| entry.ok()) + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .filter(|name| name.contains(".tmp-")) + .collect::>(); + assert!( + leftovers.is_empty(), + "no staging files should remain after saves: {leftovers:?}" + ); + assert!(path.exists()); + } + #[test] fn load_missing_file_returns_defaults() { let dir = tempdir().unwrap(); diff --git a/src/ui/main_window.rs b/src/ui/main_window.rs index 3654a5a..3cbf848 100644 --- a/src/ui/main_window.rs +++ b/src/ui/main_window.rs @@ -1406,11 +1406,15 @@ impl MainWindow { else { let _ = self.config_store.remove_account(&account_id); let _ = self.account_manager.remove(&account_id); + // Best-effort: the account's cached avatar must not outlive it + // (issue #135). + let _ = crate::util::avatar_cache::delete_avatar(&account_id); self.refresh_after_config_change(); return; }; let _ = self.config_store.remove_account(&account_id); let _ = self.account_manager.remove(&account_id); + let _ = crate::util::avatar_cache::delete_avatar(&account_id); self.refresh_after_config_change(); // Best-effort cleanup off the UI thread: resolve the stored password, diff --git a/src/ui/tray.rs b/src/ui/tray.rs index 77ae184..972b64a 100644 --- a/src/ui/tray.rs +++ b/src/ui/tray.rs @@ -215,7 +215,7 @@ impl ksni::Tray for TrayItem { } fn title(&self) -> String { - t("NextSync — {state}").replace("{state}", self.presentation.label) + t("NextSync - {state}").replace("{state}", self.presentation.label) } fn status(&self) -> Status { @@ -459,7 +459,7 @@ mod tests { fn title_includes_the_state_label() { set_locale(Locale::English); let (idle, _rx) = item_with(AppState::IdleOk); - assert_eq!(idle.title(), "NextSync — Synchronized"); + assert_eq!(idle.title(), "NextSync - Synchronized"); reset_locale(); } @@ -467,7 +467,7 @@ mod tests { fn title_translates_the_state_label_to_spanish() { set_locale(Locale::Spanish); let (syncing, _rx) = item_with(AppState::Syncing); - assert_eq!(syncing.title(), "NextSync — Sincronizando…"); + assert_eq!(syncing.title(), "NextSync - Sincronizando…"); reset_locale(); } @@ -477,7 +477,7 @@ mod tests { let (item, _rx) = item_with(AppState::KeyringLocked); let tooltip = item.tool_tip(); assert_eq!(tooltip.description, "Password Keyring Locked"); - assert_eq!(tooltip.title, "NextSync — Password Keyring Locked"); + assert_eq!(tooltip.title, "NextSync - Password Keyring Locked"); assert!(tooltip.icon_pixmap.is_empty(), "no rasterized pixmaps"); reset_locale(); } diff --git a/src/util/avatar_cache.rs b/src/util/avatar_cache.rs index d7295f5..6cf95b7 100644 --- a/src/util/avatar_cache.rs +++ b/src/util/avatar_cache.rs @@ -23,8 +23,16 @@ pub fn read_cached_avatar(account_id: &str) -> Option> { } /// Persist the avatar for an account (creating the directory `0700` on -/// first use). Failures are the caller's to log. +/// first use). Failures are the caller's to log. Bodies larger than +/// [`MAX_AVATAR_BYTES`] are refused so a malformed server response is not +/// persisted whole (issue #135). pub fn store_avatar(account_id: &str, bytes: &[u8]) -> std::io::Result<()> { + if bytes.len() > MAX_AVATAR_BYTES { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("avatar exceeds {} bytes", MAX_AVATAR_BYTES), + )); + } let path = avatar_path(account_id); if let Some(parent) = path.parent() { fs::create_dir_all(parent)?; @@ -37,6 +45,22 @@ pub fn store_avatar(account_id: &str, bytes: &[u8]) -> std::io::Result<()> { fs::write(&path, bytes) } +/// The largest avatar a server is allowed to hand us before we refuse to +/// cache it (issue #135): avatars are small images, anything larger is a +/// malformed body not worth persisting. +pub const MAX_AVATAR_BYTES: usize = 4 * 1024 * 1024; + +/// Remove the cached avatar for an account, if any. Called when the account +/// is removed so its data does not linger in the state directory (issue +/// #135). +pub fn delete_avatar(account_id: &str) -> std::io::Result<()> { + match fs::remove_file(avatar_path(account_id)) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error), + } +} + #[cfg(test)] mod tests { use super::*; @@ -75,4 +99,29 @@ mod tests { std::env::remove_var("XDG_STATE_HOME"); } } + + #[test] + fn delete_avatar_removes_the_cached_file() { + let _env = crate::util::test_env::lock(); + let dir = tempfile::tempdir().unwrap(); + std::env::set_var("XDG_STATE_HOME", dir.path()); + store_avatar("acct-del", b"png").unwrap(); + assert_eq!(read_cached_avatar("acct-del"), Some(b"png".to_vec())); + delete_avatar("acct-del").unwrap(); + assert_eq!(read_cached_avatar("acct-del"), None); + // Deleting again is a no-op success (idempotent). + delete_avatar("acct-del").unwrap(); + std::env::remove_var("XDG_STATE_HOME"); + } + + #[test] + fn store_refuses_oversized_bodies() { + let _env = crate::util::test_env::lock(); + let dir = tempfile::tempdir().unwrap(); + std::env::set_var("XDG_STATE_HOME", dir.path()); + let big = vec![0u8; MAX_AVATAR_BYTES + 1]; + assert!(store_avatar("acct-big", &big).is_err()); + assert_eq!(read_cached_avatar("acct-big"), None); + std::env::remove_var("XDG_STATE_HOME"); + } } diff --git a/src/util/translations/es.rs b/src/util/translations/es.rs index c75c0ef..2b9a02c 100644 --- a/src/util/translations/es.rs +++ b/src/util/translations/es.rs @@ -219,10 +219,10 @@ pub static CATALOG: &[(&str, &str)] = &[ ("New", "Nuevo"), ("New shares, comments and mentions from your account", "Nuevos archivos compartidos, comentarios y menciones de tu cuenta"), ("NextSync", "NextSync"), + ("NextSync - {state}", "NextSync - {state}"), ("NextSync Is Up to Date", "NextSync está actualizado"), ("NextSync is an independent, unofficial third-party project. It is not affiliated with, sponsored by, endorsed by, maintained by, or otherwise connected to Nextcloud GmbH. Nextcloud is a registered trademark of Nextcloud GmbH.\n\nBidirectional synchronization can upload, download, replace, conflict, or delete files on both the computer and the configured server. Use this software entirely at your own risk, test it with non-critical data, and maintain independent, restorable backups. The authors and contributors are not responsible for lost, corrupted, deleted, or otherwise damaged data.\n\nThe application delegates file reconciliation and conflict handling to nextcloudcmd. Availability of notify_push and other server features depends on the Nextcloud installation. This release was tested with Nextcloud Hub 26 Spring (34.0.1) on Nextcloud AIO. Compatibility with other or future Nextcloud versions is not guaranteed.\n\nNo telemetry, advertising, or usage tracking is included. Credentials are stored through the desktop Secret Service. Use of this software is also subject to the GNU General Public License version 3 or later and its warranty disclaimer.", "NextSync es un proyecto independiente, no oficial y desarrollado por terceros. No está afiliado, patrocinado, respaldado, mantenido ni vinculado de ninguna otra forma con Nextcloud GmbH. Nextcloud es una marca registrada de Nextcloud GmbH.\n\nLa sincronización bidireccional puede subir, descargar, reemplazar, crear conflictos o eliminar archivos tanto en el equipo como en el servidor configurado. Use este software bajo su propia responsabilidad, pruébelo con datos no críticos y mantenga copias de seguridad independientes y restaurables. Los autores y colaboradores no se responsabilizan de datos perdidos, dañados, eliminados o corrompidos.\n\nLa aplicación delega la reconciliación de archivos y el tratamiento de conflictos a nextcloudcmd. La disponibilidad de notify_push y de otras funciones del servidor depende de la instalación de Nextcloud. Esta versión se probó con Nextcloud Hub 26 Spring (34.0.1) en Nextcloud AIO. No se garantiza la compatibilidad con otras versiones ni con versiones futuras de Nextcloud.\n\nNo se incluye telemetría, publicidad ni seguimiento de uso. Las credenciales se almacenan mediante el Secret Service del escritorio. El uso de este software también está sujeto a la Licencia Pública General de GNU versión 3 o posterior y a su exención de garantías."), ("NextSync will quit when the current synchronization finishes.", "NextSync se cerrará cuando termine la sincronización actual."), - ("NextSync — {state}", "NextSync — {state}"), ("Nextcloud account needs attention", "La cuenta de Nextcloud necesita atención"), ("Nextcloud server URL", "URL del servidor Nextcloud"), ("No Conflicts", "Sin conflictos"), diff --git a/version.json b/version.json index 160b679..e2f0b74 100644 --- a/version.json +++ b/version.json @@ -1,10 +1,10 @@ { "schema_version": 1, - "version": "0.118.0", + "version": "0.120.0", "mandatory": false, - "summary": "Audit batch: 7 bug fixes and 4 robustness improvements.", + "summary": "Robustness batch: 12 fixes from the P2 audit plus the stale progress label race.", "changelog": [ - "keep_remote now deletes the conflicted copy (issues #120-#130)." + "Push reconnects on password change, watcher overflow never lost, config writes are durable, secrets redacted (issues #133-#145)." ], "released_at": "2026-08-21T00:00:00Z" } \ No newline at end of file