btsouth · btsouth · Aug 23, 2026
@@ -12,6 +12,7 @@ The Windows server token now rotates after unsafe exposure. Concurrent and corru
 - **A leaked `serve.token` is rotated on Windows, not just Unix.** After SBS-953, a world-readable token was replaced on Unix, but Windows still tightened the DACL and reused the same secret. The ACL is now inspected before tightening; if anyone other than the current user, SYSTEM, or Administrators can read the file, the token is replaced. Closes SBS-1043.
 
 ### Fixed
+- **An unreadable `settings.json` is no longer overwritten by the next save.** Parse failures already moved the file to `.bak` (SBS-954), but a DPAPI unprotect failure, an unsupported ProtectedFile version, or IO on an existing file loaded as defaults with no backup. `try_update` then replaced the live undecodable bytes. Those read failures now quarantine the same way, and the account ledger persist path fails closed instead of rewriting sightings from an empty snapshot. Closes SBS-1074.
 - **Frontend tests now catch accessibility regressions automatically.** A shared axe assertion checks representative quota cards, mini charts, and update banners in the existing Frontend CI job. Color contrast remains outside jsdom coverage because it requires a rendered browser. Fixes #222.
 - **`usage --all-accounts` now fetches every configured Codex and Claude account.** Account fetches run with bounded concurrency, preserve configured order, and report failures independently. Text and JSON identify each configured account while the default output remains unchanged. Fixes #274.
 - **The detached Settings window now reopens where you left it.** Its saved size and position are restored and clamped on screen instead of being overwritten by a second frontend resize on every open. Closes #275.

@@ -259,12 +259,35 @@ impl AccountLedger {
     }
 
     /// Observe every known directory and persist when a switch was detected.
+    ///
+    /// An existing file that will not decode is left untouched. `load_default`
+    /// is fail-open for readers; using it here would treat DPAPI / parse / IO
+    /// failure as an empty ledger and the following save would wipe every
+    /// prior sighting (SBS-1074).
     pub fn record_and_persist(accounts: &ConfiguredAccounts, at: i64) {
-        let mut ledger = Self::load_default();
-        if !ledger.observe_all(accounts, at) {
+        Self::persist_if_changed(&Self::default_path(), |ledger| {
+            ledger.observe_all(accounts, at)
+        });
+    }
+
+    /// Load `path`, apply `mutate`, and write only when it reports a change.
+    /// A missing file is an empty ledger; an existing undecodable one is not
+    /// replaced.
+    pub(crate) fn persist_if_changed(path: &Path, mutate: impl FnOnce(&mut Self) -> bool) {
+        let mut ledger = match Self::load_from(path) {
+            Ok(ledger) => ledger,
+            Err(error) => {
+                tracing::warn!(
+                    %error,
+                    "account-ledger.json could not be read; refusing to replace recorded sightings"
+                );
+                return;
+            }
+        };
+        if !mutate(&mut ledger) {
             return;
         }
-        if let Err(error) = ledger.save_to(&Self::default_path()) {
+        if let Err(error) = ledger.save_to(path) {
             tracing::warn!("failed to persist account ledger: {error}");
         }
     }
@@ -300,6 +323,9 @@ impl AccountLedger {
 
     /// Load from the default path, treating a corrupt file as empty rather than
     /// failing a refresh over attribution metadata.
+    ///
+    /// Read-only. Persist must go through [`Self::persist_if_changed`] so an
+    /// undecodable file is not rewritten from this empty snapshot.
     pub fn load_default() -> Self {
         Self::load_from(&Self::default_path()).unwrap_or_else(|error| {
             tracing::warn!("failed to load account ledger: {error}");
@@ -494,6 +520,78 @@ mod tests {
         );
     }
 
+    /// SBS-1074: `record_and_persist` used `load_default`, which treats an
+    /// undecodable file as empty. The next switch observation then replaced
+    /// the live ledger with only the new sighting.
+    #[test]
+    fn persist_does_not_replace_an_undecodable_ledger() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join("account-ledger.json");
+        let mut ledger = AccountLedger::new();
+        ledger.observe(ProviderId::Codex, Path::new("/dirs/a"), "acct-a", "a", 100);
+        ledger.save_to(&path).expect("seed");
+
+        let corrupt = "{not-valid-json";
+        std::fs::write(&path, corrupt).expect("corrupt ledger");
+
+        AccountLedger::persist_if_changed(&path, |ledger| {
+            ledger.observe(ProviderId::Codex, Path::new("/dirs/a"), "acct-b", "b", 200)
+        });
+
+        assert_eq!(
+            std::fs::read_to_string(&path).expect("live"),
+            corrupt,
+            "an undecodable ledger must stay so prior sightings are not overwritten"
+        );
+        assert!(
+            AccountLedger::load_from(&path).is_err(),
+            "writers must not see a corrupt ledger as an empty store"
+        );
+    }
+
+    #[test]
+    fn persist_does_not_replace_an_unreadable_ledger() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join("account-ledger.json");
+        let original = serde_json::json!({
+            "format": "codexbar.secure-file",
+            "version": 99,
+            "protection": "windows-dpapi-user",
+            "payload": "AAAA",
+        })
+        .to_string();
+        std::fs::write(&path, &original).expect("write unsupported ProtectedFile");
+        crate::secure_file::read_string(&path).expect_err("fixture must fail read_string");
+
+        AccountLedger::persist_if_changed(&path, |ledger| {
+            ledger.observe(ProviderId::Codex, Path::new("/dirs/a"), "acct-b", "b", 200)
+        });
+
+        assert_eq!(
+            std::fs::read_to_string(&path).expect("live"),
+            original,
+            "a ProtectedFile this build cannot read must not be replaced"
+        );
+    }
+
+    #[test]
+    fn missing_ledger_can_still_be_persisted() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join("account-ledger.json");
+
+        AccountLedger::persist_if_changed(&path, |ledger| {
+            ledger.observe(ProviderId::Codex, Path::new("/dirs/a"), "acct-a", "a", 100)
+        });
+
+        let loaded = AccountLedger::load_from(&path).expect("first persist");
+        assert_eq!(
+            loaded
+                .attribute(ProviderId::Codex, Path::new("/dirs/a"), 100)
+                .account_key(),
+            Some("acct-a")
+        );
+    }
+
     #[test]
     fn account_keys_prefer_the_stable_id_over_the_email() {
         let with_id = CodexIdentity {

@@ -882,7 +882,7 @@ impl Settings {
         }
 
         // Loading can become a write: an older file may still embed
-        // credentials, and SBS-954's quarantine rename is a write too.
+        // credentials, and SBS-954/SBS-1074's quarantine rename is a write too.
         // Re-read after taking the lock so those writes cannot move a
         // concurrent try_update repair to settings.json.bak (SBS-1029).
         match crate::secure_file::with_state_write_lock(|| Ok(Self::load_unlocked())) {
@@ -913,10 +913,10 @@ impl Settings {
         settings
     }
 
-    /// Read `settings.json`. `allow_quarantine` is the rename from SBS-954;
-    /// only the locked path may set it. The second value is `true` when the
-    /// file existed, failed to parse, and was left in place so `load` can
-    /// retry under the state lock (SBS-1029).
+    /// Read `settings.json`. `allow_quarantine` is the rename from SBS-954 /
+    /// SBS-1074; only the locked path may set it. The second value is `true`
+    /// when the file existed, could not be parsed or read, and was left in
+    /// place so `load` can retry under the state lock (SBS-1029).
     fn load_from_disk(allow_quarantine: bool) -> (Self, bool) {
         let mut pending_quarantine = false;
         #[allow(unused_mut)]
@@ -943,19 +943,33 @@ impl Settings {
         match crate::secure_file::read_string(path) {
             Ok(content) => match Self::parse_settings_json(&content) {
                 Ok(settings) => (settings, false),
-                Err(error) if allow_quarantine => {
-                    Self::quarantine_unparseable(path, &error);
-                    (Self::default(), false)
+                Err(error) => {
+                    Self::handle_undecodable(path, allow_quarantine, &error, "could not be parsed")
                 }
-                Err(_) => (Self::default(), true),
             },
             Err(error) => {
-                tracing::warn!(%error, "settings.json could not be read; using defaults");
-                (Self::default(), false)
+                // DPAPI unprotect, an unsupported ProtectedFile version, or IO
+                // on an existing file. Mapping these to defaults with no `.bak`
+                // lets try_update overwrite the live undecodable bytes (SBS-1074).
+                Self::handle_undecodable(path, allow_quarantine, &error, "could not be read")
             }
         }
     }
 
+    fn handle_undecodable(
+        path: &std::path::Path,
+        allow_quarantine: bool,
+        error: &impl std::fmt::Display,
+        because: &'static str,
+    ) -> (Self, bool) {
+        if allow_quarantine {
+            Self::quarantine_live_file(path, error, because);
+            (Self::default(), false)
+        } else {
+            (Self::default(), true)
+        }
+    }
+
     fn parse_settings_json(content: &str) -> Result<Self, serde_json::Error> {
         serde_json::from_str(content.trim_start_matches('\u{feff}'))
     }
@@ -969,24 +983,28 @@ impl Settings {
         match Self::parse_settings_json(content) {
             Ok(settings) => settings,
             Err(error) => {
-                Self::quarantine_unparseable(path, &error);
+                Self::quarantine_live_file(path, &error, "could not be parsed");
                 Self::default()
             }
         }
     }
 
-    fn quarantine_unparseable(path: &std::path::Path, error: &serde_json::Error) {
+    fn quarantine_live_file(
+        path: &std::path::Path,
+        error: &impl std::fmt::Display,
+        because: &'static str,
+    ) {
         let backup = Self::backup_path(path);
         match std::fs::rename(path, &backup) {
             Ok(()) => tracing::warn!(
-                %error,
+                error = %error,
                 backup = %backup.display(),
-                "settings.json could not be parsed; original moved aside and defaults loaded"
+                "settings.json {because}; original moved aside and defaults loaded"
             ),
             Err(rename_error) => tracing::warn!(
-                %error,
+                error = %error,
                 %rename_error,
-                "settings.json could not be parsed; falling back to defaults without a backup"
+                "settings.json {because}; falling back to defaults without a backup"
             ),
         }
     }

@@ -1831,6 +1831,141 @@ fn unlocked_corrupt_read_does_not_rename_a_concurrent_repair() {
     );
 }
 
+/// SBS-1074: `read_string` errors used to become defaults with no `.bak`, so
+/// the next [`Settings::try_update`] atomically replaced the live undecodable
+/// file. Parse failures already quarantine (SBS-954); treat a read failure
+/// on an existing file the same way.
+fn unsupported_protected_settings() -> String {
+    serde_json::json!({
+        "format": "codexbar.secure-file",
+        "version": 99,
+        "protection": "windows-dpapi-user",
+        "payload": "AAAA",
+    })
+    .to_string()
+}
+
+fn dpapi_protected_settings() -> String {
+    serde_json::json!({
+        "format": "codexbar.secure-file",
+        "version": 1,
+        "protection": "windows-dpapi-user",
+        "payload": "bm90LXJlYWwtZHBhcGk=",
+    })
+    .to_string()
+}
+
+/// The read-modify-write `try_update` takes: load under the lock (quarantine
+/// allowed), then atomically replace the live path with the in-memory
+/// snapshot. Tests use this instead of [`Settings::try_update`] so they do
+/// not touch the process config dir.
+fn try_update_write_at(path: &std::path::Path) {
+    let (settings, _) = Settings::read_path(path, true);
+    let json = serde_json::to_string_pretty(&settings).expect("serialize defaults");
+    crate::secure_file::write_string(path, &json).expect("try_update write");
+}
+
+fn assert_read_failure_is_pending_when_unlocked(path: &std::path::Path, original: &[u8]) {
+    let (loaded, pending_quarantine) = Settings::read_path(path, false);
+    assert!(
+        pending_quarantine,
+        "an unlocked read failure must ask load() to retry under the lock"
+    );
+    assert_eq!(
+        loaded.refresh_interval_secs,
+        Settings::default().refresh_interval_secs
+    );
+    assert_eq!(
+        std::fs::read(path).expect("live file"),
+        original,
+        "the unlocked path must leave the live file in place"
+    );
+    assert!(
+        !Settings::backup_path(path).exists(),
+        "the unlocked path must not create a backup"
+    );
+}
+
+fn assert_read_failure_quarantines_when_locked(path: &std::path::Path, original: &[u8]) {
+    let (loaded, pending_quarantine) = Settings::read_path(path, true);
+    assert!(
+        !pending_quarantine,
+        "a locked read failure quarantines immediately"
+    );
+    assert_eq!(
+        loaded.refresh_interval_secs,
+        Settings::default().refresh_interval_secs
+    );
+    assert!(
+        !path.exists(),
+        "the live path must be vacated so a later save cannot clobber the original"
+    );
+    assert_eq!(
+        std::fs::read(Settings::backup_path(path)).expect("read backup"),
+        original
+    );
+}
+
+#[test]
+fn unsupported_secure_file_version_is_not_treated_as_empty_defaults() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let path = dir.path().join("settings.json");
+    let original = unsupported_protected_settings();
+    std::fs::write(&path, &original).expect("write unsupported ProtectedFile");
+
+    crate::secure_file::read_string(&path).expect_err("fixture must fail read_string");
+    assert_read_failure_is_pending_when_unlocked(&path, original.as_bytes());
+    assert_read_failure_quarantines_when_locked(&path, original.as_bytes());
+}
+
+#[test]
+fn dpapi_unprotect_failure_is_not_treated_as_empty_defaults() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let path = dir.path().join("settings.json");
+    let original = dpapi_protected_settings();
+    std::fs::write(&path, &original).expect("write undecryptable ProtectedFile");
+
+    crate::secure_file::read_string(&path).expect_err("fixture must fail read_string");
+    assert_read_failure_is_pending_when_unlocked(&path, original.as_bytes());
+    assert_read_failure_quarantines_when_locked(&path, original.as_bytes());
+}
+
+#[test]
+fn io_failure_on_existing_settings_is_not_treated_as_empty_defaults() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let path = dir.path().join("settings.json");
+    let original = b"\xff\xfe{not-utf8".to_vec();
+    std::fs::write(&path, &original).expect("write invalid UTF-8");
+
+    crate::secure_file::read_string(&path).expect_err("fixture must fail read_string");
+    assert_read_failure_is_pending_when_unlocked(&path, &original);
+    assert_read_failure_quarantines_when_locked(&path, &original);
+}
+
+/// Without the SBS-1074 quarantine, this write replaces the live undecodable
+/// bytes and leaves no `.bak`.
+#[test]
+fn try_update_does_not_wipe_an_undecodable_settings_file() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let path = dir.path().join("settings.json");
+    let original = unsupported_protected_settings();
+    std::fs::write(&path, &original).expect("write unsupported ProtectedFile");
+
+    try_update_write_at(&path);
+
+    assert_eq!(
+        std::fs::read_to_string(Settings::backup_path(&path)).expect("quarantined original"),
+        original,
+        "try_update must move the undecodable file aside instead of overwriting it"
+    );
+    let live = crate::secure_file::read_string(&path).expect("defaults written to vacated path");
+    let written: Settings = serde_json::from_str(&live).expect("written settings parse");
+    assert_eq!(
+        written.refresh_interval_secs,
+        Settings::default().refresh_interval_secs
+    );
+}
+
 /// SBS-964: a privacy-conscious reader who leaves incident badges off still
 /// sees models.dev and GitHub traffic. Claiming the badge is the only
 /// non-provider outbound request is false; the copy has to name those hosts.