system settings
git clone https://git.lucas.co/cce-system-interface.git
accounts: save one change to the file as it is, not the page's copy
Every save wrote the page's whole list with a plain fs::write. Anything
cce-mail had written since the last 3s refresh (a token, a password moved
into the keyring) was overwritten, and a reader catching the file
mid-write fell back to the mock account. An unreadable file was also
shown as the mock account, which the next save wrote over the real ones.
Each action (add, delete, make default, Google sign-in, edit) is now one
change applied by accounts_file::update. It re-reads the file under
accounts.json.lock, which cce-mail takes too, replaces the file by rename,
and refuses to overwrite a file that does not parse. The page adopts what
was written. An unreadable file now shows as no accounts, never the mock.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
src/accounts_file.rs | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/lib.rs | 1 +
src/pages/accounts.rs | 143 +++++++++++++++++++++++++++--------------
3 files changed, 267 insertions(+), 48 deletions(-)
diff --git a/src/accounts_file.rs b/src/accounts_file.rs
new file mode 100644
index 0000000..2674507
--- /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-mail 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/lib.rs b/src/lib.rs
index 24b6c57..a77447e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,3 +1,4 @@
+mod accounts_file;
pub mod app;
pub mod pages;
pub mod power_meter;
diff --git a/src/pages/accounts.rs b/src/pages/accounts.rs
index ff86205..5b5fcb0 100644
--- a/src/pages/accounts.rs
+++ b/src/pages/accounts.rs
@@ -169,11 +169,20 @@ pub fn get_accounts_path() -> std::path::PathBuf {
pub fn load_accounts() -> Vec<AccountInfo> {
let path = get_accounts_path();
if path.exists() {
- if let Ok(content) = std::fs::read_to_string(&path) {
- if let Ok(accounts) = serde_json::from_str(&content) {
- return accounts;
+ // A file that cannot be read is shown as no accounts, never as the
+ // mock: the mock, saved back, is what used to replace real accounts.
+ // Nothing overwrites it either (`accounts_file::update` refuses).
+ return match std::fs::read_to_string(&path).map(|c| serde_json::from_str(&c)) {
+ Ok(Ok(accounts)) => accounts,
+ Ok(Err(e)) => {
+ eprintln!("accounts: {} does not parse: {e}", path.display());
+ Vec::new()
}
- }
+ Err(e) => {
+ eprintln!("accounts: cannot read {}: {e}", path.display());
+ Vec::new()
+ }
+ };
}
vec![
AccountInfo {
@@ -192,18 +201,20 @@ pub fn load_accounts() -> Vec<AccountInfo> {
]
}
-pub fn save_accounts(accounts: &[AccountInfo]) {
- 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);
- }
+/// Apply one change to accounts.json AS IT IS ON DISK, not to this page's
+/// copy, and adopt what was written. cce-mail writes the file too (refreshed
+/// tokens, passwords moved into the keyring), so saving the page's list
+/// wholesale would undo whatever it wrote since the last refresh. See
+/// `accounts_file` for the lock and the atomic replace.
+fn commit(state: &mut AccountsState, change: impl FnOnce(&mut Vec<AccountInfo>)) -> bool {
+ match crate::accounts_file::update(&get_accounts_path(), change) {
+ Ok(written) => {
+ state.accounts = written;
+ true
+ }
+ Err(e) => {
+ state.status_msg = Some(format!("Could not save accounts: {e}"));
+ false
}
}
}
@@ -833,7 +844,7 @@ pub fn update(state: &mut AccountsState, msg: AccountsMessage) {
email: email.clone(),
imap,
smtp,
- is_default: state.accounts.is_empty(),
+ is_default: false,
password: stored_password,
is_oauth: false,
access_token: None,
@@ -843,13 +854,19 @@ pub fn update(state: &mut AccountsState, msg: AccountsMessage) {
client_secret: None,
};
- if let Some(pos) = state.accounts.iter().position(|a| a.email == email) {
- state.accounts[pos] = new_acc;
- } else {
- state.accounts.push(new_acc);
+ let saved = commit(state, |accounts| {
+ let mut new_acc = new_acc;
+ if let Some(pos) = accounts.iter().position(|a| a.email == new_acc.email) {
+ new_acc.is_default = accounts[pos].is_default;
+ accounts[pos] = new_acc;
+ } else {
+ new_acc.is_default = accounts.is_empty();
+ accounts.push(new_acc);
+ }
+ });
+ if !saved {
+ return;
}
-
- save_accounts(&state.accounts);
state.adding_new = false;
state.selected_idx = state.accounts.iter().position(|a| a.email == email);
// Optimistic: the watcher's next probe confirms it.
@@ -865,12 +882,20 @@ pub fn update(state: &mut AccountsState, msg: AccountsMessage) {
}
AccountsMessage::DeleteAccount(idx) => {
if idx < state.accounts.len() {
- let deleted = state.accounts.remove(idx);
- state.keyring.remove(&deleted.email);
- if deleted.is_default && !state.accounts.is_empty() {
- state.accounts[0].is_default = true;
+ let deleted = state.accounts[idx].clone();
+ let saved = commit(state, |accounts| {
+ let was_default = accounts.iter().any(|a| a.email == deleted.email && a.is_default);
+ accounts.retain(|a| a.email != deleted.email);
+ if was_default && !accounts.iter().any(|a| a.is_default) {
+ if let Some(first) = accounts.first_mut() {
+ first.is_default = true;
+ }
+ }
+ });
+ if !saved {
+ return;
}
- save_accounts(&state.accounts);
+ state.keyring.remove(&deleted.email);
// Drop the keyring password and cce-mail's cached mail too.
// The pre-rename service is cleared as well, so an account
// deleted before cce-mail ever adopted it leaves nothing behind.
@@ -888,10 +913,14 @@ pub fn update(state: &mut AccountsState, msg: AccountsMessage) {
}
AccountsMessage::MakeDefault(idx) => {
if idx < state.accounts.len() {
- for (i, acc) in state.accounts.iter_mut().enumerate() {
- acc.is_default = i == idx;
+ let email = state.accounts[idx].email.clone();
+ if !commit(state, |accounts| {
+ for acc in accounts.iter_mut() {
+ acc.is_default = acc.email == email;
+ }
+ }) {
+ return;
}
- save_accounts(&state.accounts);
state.status_msg = Some("Default account updated!".to_string());
}
}
@@ -907,15 +936,18 @@ pub fn update(state: &mut AccountsState, msg: AccountsMessage) {
}
AccountsMessage::GoogleLoginSuccess(mut new_acc) => {
let email = new_acc.email.clone();
- if let Some(pos) = state.accounts.iter().position(|a| a.email == email) {
- // A re-login refreshes credentials; it must not silently
- // un-default the account it replaces.
- new_acc.is_default = state.accounts[pos].is_default;
- state.accounts[pos] = new_acc;
- } else {
- state.accounts.push(new_acc);
+ if !commit(state, |accounts| {
+ if let Some(pos) = accounts.iter().position(|a| a.email == new_acc.email) {
+ // A re-login refreshes credentials; it must not silently
+ // un-default the account it replaces.
+ new_acc.is_default = accounts[pos].is_default;
+ accounts[pos] = new_acc;
+ } else {
+ accounts.push(new_acc);
+ }
+ }) {
+ return;
}
- save_accounts(&state.accounts);
state.adding_new = false;
state.selected_idx = state.accounts.iter().position(|a| a.email == email);
state.status_msg = Some("Google account authenticated!".to_string());
@@ -980,6 +1012,7 @@ pub fn update(state: &mut AccountsState, msg: AccountsMessage) {
let password = if is_oauth { String::new() } else { live_text(&state.password_box) };
let mut msg = "Account updated".to_string();
+ let mut new_password = None;
if !password.is_empty() {
// Same Secret Service entry cce-mail resolves; the on-disk
// field stays blank whenever the keyring accepted it.
@@ -993,16 +1026,30 @@ pub fn update(state: &mut AccountsState, msg: AccountsMessage) {
state.keyring.insert(email.clone(), KeyringStatus::OnDiskPlaintext);
}
}
- state.accounts[idx].password = stored;
- }
- state.accounts[idx].imap = imap;
- state.accounts[idx].smtp = smtp;
- if let Some((id, secret)) = creds {
- state.accounts[idx].client_id = Some(id);
- state.accounts[idx].client_secret = Some(secret);
+ new_password = Some(stored);
}
- save_accounts(&state.accounts);
+ let mut found = false;
+ if !commit(state, |accounts| {
+ let Some(acc) = accounts.iter_mut().find(|a| a.email == email) else { return };
+ found = true;
+ if let Some(stored) = new_password {
+ acc.password = stored;
+ }
+ acc.imap = imap;
+ acc.smtp = smtp;
+ if let Some((id, secret)) = creds {
+ acc.client_id = Some(id);
+ acc.client_secret = Some(secret);
+ }
+ }) {
+ return;
+ }
+ if !found {
+ state.editing_email = None;
+ state.status_msg = Some("That account no longer exists.".to_string());
+ return;
+ }
state.editing_email = None;
state.password_box.placeholder = None;
state.status_msg = Some(msg);
@@ -1245,7 +1292,7 @@ mod tests {
}
/// These assertions deliberately stop short of EditAccountSave's success
- /// path: it calls save_accounts, which writes the real accounts.json under
+ /// path: it calls commit, which writes the real accounts.json under
/// XDG_CONFIG_HOME. Only the early-return paths are exercised here.
#[test]
fn edit_prefills_the_account_but_never_the_password() {