mail client (IMAP/SMTP)
git clone https://git.lucas.co/cce-mail.git
accounts.json: write only the change meant, never the list loaded at startup
On every OAuth token refresh cce-mail wrote back the whole account list
it had loaded when it started. An account added in System Settings since
was deleted, and one removed there came back, refresh token included.
Both writers also used a plain fs::write, which truncates first, so a
reader in between saw an empty file and fell back to the mock account.
accounts_file::update re-reads the file under accounts.json.lock, applies
one change and replaces the file by rename. A file that does not parse is
left alone. cce-mail now writes only a refreshed token into its own
account (an account no longer in the file stays gone) and blanks a
password it moved into the keyring, and only where the file still holds
that same password. cce-system-interface carries the same helper.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
src/accounts_file.rs | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++
src/main.rs | 71 +++++++++++++--------
2 files changed, 217 insertions(+), 25 deletions(-)
diff --git a/src/accounts_file.rs b/src/accounts_file.rs
new file mode 100644
index 0000000..f4951ae
--- /dev/null
+++ b/src/accounts_file.rs
@@ -0,0 +1,171 @@
+//! Safe writes to the shared `accounts.json`.
+//!
+//! Two processes write that file — cce-system-interface (it owns the account
+//! list) and cce-mail (it refreshes OAuth tokens and moves plaintext passwords
+//! into the keyring) — and cce-calendar's sync reads it. Until 2026-10-01 each
+//! writer saved its whole in-memory list with a plain `fs::write`, which lost
+//! updates both ways: cce-mail, refreshing a token an hour after it started,
+//! wrote back the list it had loaded then, deleting an account added in
+//! Settings since and resurrecting one removed there, refresh token included.
+//! And a plain write truncates before it writes, so a reader in between saw
+//! an empty file, fell back to the mock account, and its next save replaced
+//! the real accounts with it.
+//!
+//! So a writer never saves a list it holds. It hands [`update`] the change
+//! it means to make, and `update` re-reads the file, applies it, and writes
+//! the result back — holding `accounts.json.lock` throughout, so the two
+//! writers take turns, and replacing the file by rename, so a reader sees the
+//! old file or the new one and never half of either. A file that does not
+//! parse is never overwritten: that is somebody's accounts, and an error is
+//! cheaper than guessing.
+//!
+//! The same helper lives in cce-system-interface as `src/accounts_file.rs`; the
+//! two must agree on the lock file's name, so change both together.
+
+use std::io::{self, Write};
+use std::os::unix::fs::OpenOptionsExt;
+use std::path::{Path, PathBuf};
+
+fn lock_path(path: &Path) -> PathBuf {
+ let mut name = path.file_name().unwrap_or_default().to_os_string();
+ name.push(".lock");
+ path.with_file_name(name)
+}
+
+/// Apply `change` to the accounts in `path` as they are NOW, write them back
+/// atomically under the writers' lock, and return what was written. A missing
+/// file is an empty list.
+pub fn update<T, F>(path: &Path, change: F) -> io::Result<Vec<T>>
+where
+ T: serde::Serialize + serde::de::DeserializeOwned,
+ F: FnOnce(&mut Vec<T>),
+{
+ let lock = std::fs::OpenOptions::new()
+ .create(true)
+ .truncate(false)
+ .write(true)
+ .mode(0o600)
+ .open(lock_path(path))?;
+ // Released when `lock` drops, after the rename.
+ lock.lock()?;
+ let mut accounts: Vec<T> = match std::fs::read_to_string(path) {
+ Ok(text) => serde_json::from_str(&text).map_err(|e| {
+ io::Error::new(
+ io::ErrorKind::InvalidData,
+ format!("{} does not parse ({e}); left as it is", path.display()),
+ )
+ })?,
+ Err(e) if e.kind() == io::ErrorKind::NotFound => Vec::new(),
+ Err(e) => return Err(e),
+ };
+ change(&mut accounts);
+ let text = serde_json::to_string_pretty(&accounts).map_err(io::Error::other)?;
+ write_atomic(path, text.as_bytes())?;
+ Ok(accounts)
+}
+
+/// Write a sibling temp file (0600 from creation: it holds tokens), flush it
+/// to disk, then rename it over `path`.
+fn write_atomic(path: &Path, bytes: &[u8]) -> io::Result<()> {
+ let mut name = std::ffi::OsString::from(".");
+ name.push(path.file_name().unwrap_or_default());
+ name.push(format!(".{}.tmp", std::process::id()));
+ let tmp = path.with_file_name(name);
+ let _ = std::fs::remove_file(&tmp);
+ let written = (|| {
+ let mut f = std::fs::OpenOptions::new()
+ .write(true)
+ .create_new(true)
+ .mode(0o600)
+ .open(&tmp)?;
+ f.write_all(bytes)?;
+ f.sync_all()?;
+ std::fs::rename(&tmp, path)
+ })();
+ if written.is_err() {
+ let _ = std::fs::remove_file(&tmp);
+ }
+ written
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use serde_json::{json, Value};
+ use std::os::unix::fs::PermissionsExt;
+
+ fn scratch(name: &str) -> PathBuf {
+ let dir = std::env::temp_dir().join(format!("accounts-file-{name}-{}", std::process::id()));
+ let _ = std::fs::remove_dir_all(&dir);
+ std::fs::create_dir_all(&dir).unwrap();
+ dir.join("accounts.json")
+ }
+
+ fn emails(path: &Path) -> Vec<String> {
+ let v: Vec<Value> = serde_json::from_str(&std::fs::read_to_string(path).unwrap()).unwrap();
+ v.iter().map(|a| a["email"].as_str().unwrap().to_string()).collect()
+ }
+
+ #[test]
+ fn a_missing_file_is_created_private() {
+ let path = scratch("create");
+ update::<Value, _>(&path, |a| a.push(json!({ "email": "a@x" }))).unwrap();
+ assert_eq!(emails(&path), ["a@x"]);
+ let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777;
+ assert_eq!(mode, 0o600);
+ // No temp file left beside it.
+ let left: Vec<_> = std::fs::read_dir(path.parent().unwrap()).unwrap().flatten()
+ .map(|e| e.file_name().into_string().unwrap()).collect();
+ assert!(left.iter().all(|n| n == "accounts.json" || n == "accounts.json.lock"), "{left:?}");
+ }
+
+ #[test]
+ fn a_file_that_does_not_parse_is_never_overwritten() {
+ let path = scratch("corrupt");
+ std::fs::write(&path, "[{\"email\": \"a@x\"").unwrap();
+ let err = update::<Value, _>(&path, |a| a.clear()).unwrap_err();
+ assert_eq!(err.kind(), io::ErrorKind::InvalidData);
+ assert_eq!(std::fs::read_to_string(&path).unwrap(), "[{\"email\": \"a@x\"");
+ }
+
+ #[test]
+ fn a_stale_writer_keeps_what_the_other_one_added() {
+ // The bug: mail loaded [a], Settings then added b, mail refreshed a's
+ // token and wrote its [a] back. A targeted change keeps b.
+ let path = scratch("stale");
+ std::fs::write(&path, r#"[{"email":"a@x","access_token":"old"}]"#).unwrap();
+ update::<Value, _>(&path, |a| a.push(json!({ "email": "b@x" }))).unwrap();
+ update::<Value, _>(&path, |accs| {
+ for a in accs.iter_mut().filter(|a| a["email"] == "a@x") {
+ a["access_token"] = json!("new");
+ }
+ })
+ .unwrap();
+ assert_eq!(emails(&path), ["a@x", "b@x"]);
+ let v: Vec<Value> = serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap();
+ assert_eq!(v[0]["access_token"], "new");
+ }
+
+ #[test]
+ fn concurrent_writers_take_turns() {
+ // flock is per open file, so threads opening the lock themselves
+ // exclude each other exactly as two processes do.
+ let path = scratch("concurrent");
+ let threads: Vec<_> = (0..16)
+ .map(|i| {
+ let path = path.clone();
+ std::thread::spawn(move || {
+ update::<Value, _>(&path, |a| a.push(json!({ "email": format!("{i}@x") }))).unwrap();
+ })
+ })
+ .collect();
+ for t in threads {
+ t.join().unwrap();
+ }
+ let mut got = emails(&path);
+ got.sort();
+ let mut want: Vec<String> = (0..16).map(|i| format!("{i}@x")).collect();
+ want.sort();
+ assert_eq!(got, want, "an update was lost");
+ }
+}
diff --git a/src/main.rs b/src/main.rs
index 079830c..1b85488 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -2,6 +2,7 @@
#[cfg(feature = "wpe")]
mod ipc;
mod wpe;
+mod accounts_file;
use cce_ui::widget::ScrollRegion;
use wayland_client::QueueHandle;
use cce_ui::cosmic_text::FontSystem;
@@ -463,8 +464,8 @@ fn load_accounts() -> Vec<AccountInfo> {
if let Ok(mut accounts) = serde_json::from_str::<Vec<AccountInfo>>(&content) {
if resolve_account_secrets(&mut accounts) {
// A plaintext password just moved into the keyring —
- // rewrite the file now so it stops living on disk.
- save_accounts(&accounts);
+ // clear it from the file now so it stops living on disk.
+ persist_keyring_migration(&accounts);
}
return accounts;
}
@@ -547,31 +548,51 @@ fn legacy_keyring_password(email: &str) -> Option<String> {
.ok()
}
-fn save_accounts(accounts: &[AccountInfo]) {
- // Keyring-backed passwords never go back to disk.
- let redacted: Vec<AccountInfo> = accounts
+/// accounts.json belongs to cce-system-interface, so this app never writes
+/// its own list back — only the one change it means, re-read under the lock
+/// (`accounts_file`). Writing the list it loaded at startup is what deleted
+/// accounts added in Settings since, and revived ones removed there.
+///
+/// Passwords now in the keyring: blank the on-disk copy, but only where the
+/// file still holds the very password that was moved, so a password Settings
+/// has changed in the meantime is not touched.
+fn persist_keyring_migration(accounts: &[AccountInfo]) {
+ let moved: Vec<(&str, &str)> = accounts
.iter()
- .map(|a| {
- let mut a = a.clone();
- if a.keyring_backed {
- a.password = String::new();
- }
- a
- })
+ .filter(|a| a.keyring_backed && !a.password.is_empty())
+ .map(|a| (a.email.as_str(), a.password.as_str()))
.collect();
- let accounts = &redacted;
- let path = get_accounts_path();
- if let Ok(content) = serde_json::to_string_pretty(accounts) {
- let _ = std::fs::write(&path, content);
- #[cfg(unix)]
- {
- use std::os::unix::fs::PermissionsExt;
- if let Ok(metadata) = std::fs::metadata(&path) {
- let mut perms = metadata.permissions();
- perms.set_mode(0o600);
- let _ = std::fs::set_permissions(&path, perms);
+ if moved.is_empty() {
+ return;
+ }
+ let result = accounts_file::update::<serde_json::Value, _>(&get_accounts_path(), |on_disk| {
+ for acc in on_disk.iter_mut() {
+ let (Some(email), Some(password)) = (acc["email"].as_str(), acc["password"].as_str()) else {
+ continue;
+ };
+ if moved.contains(&(email, password)) {
+ acc["password"] = serde_json::Value::String(String::new());
}
}
+ });
+ if let Err(e) = result {
+ eprintln!("cce-mail: could not clear migrated passwords from accounts.json: {e}");
+ }
+}
+
+/// A refreshed OAuth token, written into that one account. An account no
+/// longer in the file was removed in Settings, and stays removed.
+fn persist_account_tokens(email: &str, access_token: Option<&str>, expiry: Option<u64>) {
+ let result = accounts_file::update::<serde_json::Value, _>(&get_accounts_path(), |on_disk| {
+ for acc in on_disk.iter_mut().filter(|a| a["email"] == email) {
+ if let Some(obj) = acc.as_object_mut() {
+ obj.insert("access_token".into(), serde_json::json!(access_token));
+ obj.insert("token_expiry".into(), serde_json::json!(expiry));
+ }
+ }
+ });
+ if let Err(e) = result {
+ eprintln!("cce-mail: could not save the refreshed token for {email}: {e}");
}
}
@@ -3149,7 +3170,7 @@ impl ClearEmailApp {
eprintln!("cce-mail: recovered {} credentials from the keyring", email);
}
if migrated {
- save_accounts(&self.accounts);
+ persist_keyring_migration(&self.accounts[idx..=idx]);
}
}
@@ -5118,9 +5139,9 @@ impl Application for ClearEmailApp {
}
AppMessage::UpdateAccountTokens(email, access_token, expiry) => {
if let Some(acc) = self.accounts.iter_mut().find(|a| a.email == email) {
+ persist_account_tokens(&email, access_token.as_deref(), expiry);
acc.access_token = access_token;
acc.token_expiry = expiry;
- save_accounts(&self.accounts);
}
}
#[cfg(feature = "wpe")]