Skip to main content

max / audiofiles

Fix Sync/Sec risk axis: blob upload fault-tolerance + ConfigKey registry Remediates the open findings from fuzz-2026-07-21 (risk lens) on the Sync/Sec axis, plus the data-integrity minors. SERIOUS #1 — blob upload head-of-line block + silent failure. upload_pending_blobs now continues past a failing blob (per-blob error is counted, not propagated with `?`), so one bad sample no longer blocks the rest of the queue. The scheduler surfaces a non-zero failure count through SyncStatus::last_error, mirroring the retention path, so a partial backup is no longer reported as a clean cycle. Confirm size is single-sourced from the uploaded bytes rather than the DB's file_size. SERIOUS #3 CHRONIC — device-local user_config keys crossing the sync boundary. Replaces three hand-maintained denylists (a Rust predicate, the SQL trigger predicate, and the initial-snapshot query) with one ConfigKey registry in audiofiles-core. Each key declares its SyncPosture once; the import filter and the SQL config_key_policy table (joined by the triggers and the snapshot) are both generated from it. The config accessors take ConfigKey, so adding a key without a posture no longer compiles, and mirror_path / mirror_enabled / import_preflight_disabled can no longer leak. Export- and import-side tests lock all three paths. MINOR — checked u64::try_from at the three SQLite aggregate cast sites (a corrupt negative surfaces as an error, not a ~1.8e19 wrap); documented the rusqlite 0.39 pin rationale. Gate: cargo test --workspace green (1219), clippy --workspace --all-targets clean.
Co-Authored-By
Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-23 21:45 UTC
Signed with PGP, not checked
Commit: 53af4ebfb8f76cb2c07bdce2a855db4c0d5bb4be
Parent: ed2fe88
22 files changed, +618 insertions, -119 deletions
M Cargo.toml +4
@@ -16,6 +16,10 @@
16 16 egui_extras = { version = "0.35", default-features = false }
17 17 eframe = { version = "0.35", default-features = false, features = ["default_fonts", "glow"] }
18 18 cpal = "0.18"
19 + # Pinned to 0.39 deliberately: 0.40 bumps libsqlite3-sys to 0.37, which then
20 + # co-links with the sqlx-sqlite 0.9 that synckit-client's SyncStore pulls in and
21 + # fails the build with two libsqlite3-sys versions. Hold at 0.39 until sqlx and
22 + # rusqlite agree on a libsqlite3-sys major. (0.39 still provides u64: FromSql.)
19 23 rusqlite = { version = "0.39", features = ["bundled", "functions"] }
20 24 thiserror = "2.0.18"
21 25 sha2 = "0.11.0"
@@ -457,12 +457,22 @@
457 457 // `unsafe_mode` row is deleted via `delete_config`. Safe to
458 458 // remove this block once every active vault has been opened at
459 459 // least once after this release.
460 - let loose = match browser.backend.get_config("loose_files") {
460 + let loose = match browser
461 + .backend
462 + .get_config(audiofiles_browser::backend::ConfigKey::LooseFiles)
463 + {
461 464 Ok(Some(v)) => Some(v),
462 - _ => match browser.backend.get_config("unsafe_mode") {
465 + _ => match browser
466 + .backend
467 + .get_config(audiofiles_browser::backend::ConfigKey::UnsafeMode)
468 + {
463 469 Ok(Some(v)) => {
464 - let _ = browser.backend.set_config("loose_files", &v);
465 - let _ = browser.backend.delete_config("unsafe_mode");
470 + let _ = browser
471 + .backend
472 + .set_config(audiofiles_browser::backend::ConfigKey::LooseFiles, &v);
473 + let _ = browser
474 + .backend
475 + .delete_config(audiofiles_browser::backend::ConfigKey::UnsafeMode);
466 476 Some(v)
467 477 }
468 478 _ => None,
@@ -684,7 +694,10 @@
684 694 if self.with_vault_registry(|reg| vault::create_vault(reg, &name, &path)) {
685 695 self.switch_vault(switch_path);
686 696 if loose_files && let Some(ref mut browser) = self.browser {
687 - let _ = browser.backend.set_config("loose_files", "1");
697 + let _ = browser.backend.set_config(
698 + audiofiles_browser::backend::ConfigKey::LooseFiles,
699 + "1",
700 + );
688 701 browser.settings.is_loose_files = true;
689 702 }
690 703 return;
@@ -1554,6 +1554,53 @@
1554 1554 END;
1555 1555 "#;
1556 1556
1557 + const MIGRATION_033: &str = r#"
1558 + -- Generate the user_config export filter from the ConfigKey registry instead of
1559 + -- a hand-maintained trigger predicate. `config_key_policy` is seeded at every
1560 + -- open from `audiofiles_core::config_key::ConfigKey::ALL` (see
1561 + -- Database::seed_config_key_policy); the triggers below enqueue a changelog row
1562 + -- only for keys the registry marks replicated. This closes the CHRONIC where the
1563 + -- old `NEW.key != 'loose_files'` denylist never covered mirror_path /
1564 + -- mirror_enabled / import_preflight_disabled, letting a hostile server steer a
1565 + -- local filesystem write root across the sync boundary (fuzz-2026-07-21 #3).
1566 + -- Unknown keys are absent from the policy table and therefore never exported —
1567 + -- the filter fails closed.
1568 + CREATE TABLE IF NOT EXISTS config_key_policy (
1569 + key TEXT PRIMARY KEY,
1570 + replicated INTEGER NOT NULL
1571 + );
1572 +
1573 + DROP TRIGGER IF EXISTS sync_user_config_insert;
1574 + DROP TRIGGER IF EXISTS sync_user_config_update;
1575 + DROP TRIGGER IF EXISTS sync_user_config_delete;
1576 +
1577 + CREATE TRIGGER sync_user_config_insert AFTER INSERT ON user_config
1578 + WHEN (SELECT value FROM sync_state WHERE key = 'applying_remote') != '1'
1579 + AND EXISTS (SELECT 1 FROM config_key_policy p WHERE p.key = NEW.key AND p.replicated = 1)
1580 + BEGIN
1581 + INSERT INTO sync_changelog (table_name, op, row_id, data)
1582 + VALUES ('user_config', 'INSERT', NEW.key,
1583 + json_object('key', NEW.key, 'value', NEW.value));
1584 + END;
1585 +
1586 + CREATE TRIGGER sync_user_config_update AFTER UPDATE ON user_config
1587 + WHEN (SELECT value FROM sync_state WHERE key = 'applying_remote') != '1'
1588 + AND EXISTS (SELECT 1 FROM config_key_policy p WHERE p.key = NEW.key AND p.replicated = 1)
1589 + BEGIN
1590 + INSERT INTO sync_changelog (table_name, op, row_id, data)
1591 + VALUES ('user_config', 'UPDATE', NEW.key,
1592 + json_object('key', NEW.key, 'value', NEW.value));
1593 + END;
1594 +
1595 + CREATE TRIGGER sync_user_config_delete AFTER DELETE ON user_config
1596 + WHEN (SELECT value FROM sync_state WHERE key = 'applying_remote') != '1'
1597 + AND EXISTS (SELECT 1 FROM config_key_policy p WHERE p.key = OLD.key AND p.replicated = 1)
1598 + BEGIN
1599 + INSERT INTO sync_changelog (table_name, op, row_id, data)
1600 + VALUES ('user_config', 'DELETE', OLD.key, json_object('key', OLD.key));
1601 + END;
1602 + "#;
1603 +
1557 1604 /// Register `hash_row_id(salt, key) -> TEXT` as a deterministic SQLite
1558 1605 /// function on the given connection. Used by the M018 sync triggers so the
1559 1606 /// `sync_changelog.row_id` field never carries cleartext content (tag strings,
@@ -1623,6 +1670,7 @@
1623 1670 register_hash_row_id(&conn)?;
1624 1671 let mut db = Self { conn };
1625 1672 db.migrate()?;
1673 + db.seed_config_key_policy()?;
1626 1674 Ok(db)
1627 1675 }
1628 1676
@@ -1643,9 +1691,28 @@
1643 1691 register_hash_row_id(&conn)?;
1644 1692 let mut db = Self { conn };
1645 1693 db.migrate()?;
1694 + db.seed_config_key_policy()?;
1646 1695 Ok(db)
1647 1696 }
1648 1697
1698 + /// Seed `config_key_policy` from the [`ConfigKey`](crate::config_key::ConfigKey)
1699 + /// registry — the single source of truth for which `user_config` keys may
1700 + /// sync. Run at every open so the SQL export triggers always reflect the
1701 + /// current registry: adding a key in Rust is enough, no migration needed.
1702 + /// Idempotent (clear + reinsert the closed set).
1703 + fn seed_config_key_policy(&self) -> Result<(), DbError> {
1704 + use crate::config_key::{ConfigKey, SyncPosture};
1705 + self.conn.execute("DELETE FROM config_key_policy", [])?;
1706 + let mut stmt = self
1707 + .conn
1708 + .prepare("INSERT INTO config_key_policy (key, replicated) VALUES (?1, ?2)")?;
1709 + for &key in ConfigKey::ALL {
1710 + let replicated = i64::from(matches!(key.posture(), SyncPosture::Replicated));
1711 + stmt.execute(rusqlite::params![key.as_str(), replicated])?;
1712 + }
1713 + Ok(())
1714 + }
1715 +
1649 1716 /// Apply pending migrations using PRAGMA user_version as the version tracker.
1650 1717 ///
1651 1718 /// Each migration step runs inside a transaction so the schema change and
@@ -1690,6 +1757,7 @@
1690 1757 MIGRATION_030,
1691 1758 MIGRATION_031,
1692 1759 MIGRATION_032,
1760 + MIGRATION_033,
1693 1761 ];
1694 1762
1695 1763 for (i, sql) in MIGRATIONS.iter().enumerate() {
@@ -1835,9 +1903,19 @@
1835 1903 let (count, total): (u64, u64) = self.conn.query_row(
1836 1904 "SELECT COUNT(*), COALESCE(SUM(file_size), 0) FROM live_samples",
1837 1905 [],
1838 - // rusqlite 0.40 dropped u64: FromSql (SQLite integers are i64);
1839 - // COUNT/SUM are non-negative, so read i64 and widen.
1840 - |row| Ok((row.get::<_, i64>(0)? as u64, row.get::<_, i64>(1)? as u64)),
1906 + // SQLite integers are i64. COUNT/SUM should be non-negative, but a
1907 + // single corrupt negative file_size must surface as an error, not
1908 + // wrap silently to ~1.8e19 (workspace denies unwrap for this class).
1909 + |row| {
1910 + let count = row.get::<_, i64>(0)?;
1911 + let total = row.get::<_, i64>(1)?;
1912 + Ok((
1913 + u64::try_from(count)
1914 + .map_err(|_| rusqlite::Error::IntegralValueOutOfRange(0, count))?,
1915 + u64::try_from(total)
1916 + .map_err(|_| rusqlite::Error::IntegralValueOutOfRange(1, total))?,
1917 + ))
1918 + },
1841 1919 )?;
1842 1920 Ok((count, total))
1843 1921 }
@@ -1857,8 +1935,18 @@
1857 1935 WHERE vfs_id = ? AND sample_hash IS NOT NULL\
1858 1936 )",
1859 1937 [vfs_id],
1860 - // rusqlite 0.40 dropped u64: FromSql; COUNT/SUM are non-negative.
1861 - |row| Ok((row.get::<_, i64>(0)? as u64, row.get::<_, i64>(1)? as u64)),
1938 + // Non-negative in practice; a corrupt negative surfaces as an error
1939 + // rather than wrapping silently to a nonsense u64.
1940 + |row| {
1941 + let count = row.get::<_, i64>(0)?;
1942 + let total = row.get::<_, i64>(1)?;
1943 + Ok((
1944 + u64::try_from(count)
1945 + .map_err(|_| rusqlite::Error::IntegralValueOutOfRange(0, count))?,
1946 + u64::try_from(total)
1947 + .map_err(|_| rusqlite::Error::IntegralValueOutOfRange(1, total))?,
1948 + ))
1949 + },
1862 1950 )?;
1863 1951 Ok((count, total))
1864 1952 }
@@ -1915,6 +2003,7 @@
1915 2003 "classifier_layers",
1916 2004 "collection_members",
1917 2005 "collections",
2006 + "config_key_policy",
1918 2007 "edit_history",
1919 2008 "fingerprints",
1920 2009 "hlc_ledger",
@@ -1942,7 +2031,7 @@
1942 2031 .conn()
1943 2032 .query_row("PRAGMA user_version", [], |row| row.get(0))
1944 2033 .unwrap();
1945 - assert_eq!(version, 32);
2034 + assert_eq!(version, 33);
1946 2035 }
1947 2036
1948 2037 #[test]
@@ -1953,7 +2042,7 @@
1953 2042 .conn()
1954 2043 .query_row("PRAGMA user_version", [], |row| row.get(0))
1955 2044 .unwrap();
1956 - assert_eq!(version, 32);
2045 + assert_eq!(version, 33);
1957 2046 }
1958 2047
1959 2048 #[test]
@@ -1985,6 +2074,46 @@
1985 2074 }
1986 2075 }
1987 2076
2077 + #[test]
2078 + fn user_config_export_trigger_excludes_device_local_keys() {
2079 + // Export side of the CHRONIC fix (fuzz-2026-07-21 #3): the sync triggers
2080 + // are generated from the ConfigKey registry via config_key_policy, so a
2081 + // device-local key never enqueues a changelog row — while a replicated
2082 + // key still does. Symmetric with the import-side test in audiofiles-sync.
2083 + let db = Database::open_in_memory().unwrap();
2084 + let conn = db.conn();
2085 + let changelog_rows = |key: &str| -> i64 {
2086 + conn.query_row(
2087 + "SELECT COUNT(*) FROM sync_changelog WHERE table_name = 'user_config' AND row_id = ?1",
2088 + [key],
2089 + |r| r.get(0),
2090 + )
2091 + .unwrap()
2092 + };
2093 +
2094 + for key in [
2095 + "mirror_path",
2096 + "mirror_enabled",
2097 + "import_preflight_disabled",
2098 + "loose_files",
2099 + ] {
2100 + conn.execute(
2101 + "INSERT OR REPLACE INTO user_config (key, value) VALUES (?1, '1')",
2102 + [key],
2103 + )
2104 + .unwrap();
2105 + assert_eq!(changelog_rows(key), 0, "{key} must not be exported");
2106 + }
2107 +
2108 + // A replicated key still enqueues a changelog row.
2109 + conn.execute(
2110 + "INSERT OR REPLACE INTO user_config (key, value) VALUES ('theme', 'dark')",
2111 + [],
2112 + )
2113 + .unwrap();
2114 + assert_eq!(changelog_rows("theme"), 1, "theme must be exported");
2115 + }
2116 +
1988 2117 /// Open a fresh file-backed DB, close, reopen. The second open re-enters
1989 2118 /// `migrate()`; with `user_version=17` no migration body runs, but the
1990 2119 /// shape verifies our open/close cycle is clean (no locks, no WAL leak).
@@ -2001,7 +2130,7 @@
2001 2130 .conn()
2002 2131 .query_row("PRAGMA user_version", [], |row| row.get(0))
2003 2132 .unwrap();
2004 - assert_eq!(version, 32);
2133 + assert_eq!(version, 33);
2005 2134 }
2006 2135
2007 2136 /// Simulates the worst-case recovery path: a prior partial migration left
@@ -2045,7 +2174,7 @@
2045 2174 .conn()
2046 2175 .query_row("PRAGMA user_version", [], |row| row.get(0))
2047 2176 .unwrap();
2048 - assert_eq!(version, 32);
2177 + assert_eq!(version, 33);
2049 2178 }
2050 2179
2051 2180 /// M018 contract: the `sync_changelog.row_id` for sensitive tables must
@@ -2273,7 +2402,7 @@
2273 2402 let initial_version: i32 = conn
2274 2403 .query_row("PRAGMA user_version", [], |row| row.get(0))
2275 2404 .unwrap();
2276 - assert_eq!(initial_version, 32);
2405 + assert_eq!(initial_version, 33);
2277 2406
2278 2407 let batch = format!("BEGIN;\n{}\nPRAGMA user_version = 999;\nCOMMIT;", bad_sql);
2279 2408 let first_err = conn.execute_batch(&batch).unwrap_err();
@@ -2338,7 +2467,7 @@
2338 2467 .conn()
2339 2468 .query_row("PRAGMA user_version", [], |row| row.get(0))
2340 2469 .unwrap();
2341 - assert_eq!(version, 32);
2470 + assert_eq!(version, 33);
2342 2471 }
2343 2472
2344 2473 #[test]
@@ -47,6 +47,7 @@
47 47
48 48 pub mod analysis;
49 49 pub mod collections;
50 + pub mod config_key;
50 51 pub mod db;
51 52 pub mod edit;
52 53 pub mod error;
@@ -265,8 +265,26 @@
265 265 }
266 266
267 267 // Blob sync: upload pending, then download missing
268 - if let Err(e) = service::upload_pending_blobs(db_path, content_dir, client).await {
269 - tracing::warn!("Blob upload failed (non-fatal): {e}");
268 + match service::upload_pending_blobs(db_path, content_dir, client).await {
269 + Ok(summary) if summary.failed > 0 => {
270 + // A per-blob failure no longer wedges the queue, but the user must
271 + // still be told: an unbounded suffix of the library would otherwise
272 + // never leave the device while sync reports a clean cycle. Mirror the
273 + // retention path below, which surfaces its data-loss risk the same way.
274 + let detail = summary
275 + .first_error
276 + .as_deref()
277 + .unwrap_or("see logs for details");
278 + status.lock().last_error = Some(format!(
279 + "{} sample(s) could not be uploaded and are not backed up yet: {detail}",
280 + summary.failed
281 + ));
282 + }
283 + Ok(_) => {}
284 + Err(e) => {
285 + tracing::warn!("Blob upload failed (non-fatal): {e}");
286 + status.lock().last_error = Some(format!("Blob upload failed: {e}"));
287 + }
270 288 }
271 289 if let Err(e) = service::download_missing_blobs(db_path, content_dir, client).await {
272 290 tracing::warn!("Blob download failed (non-fatal): {e}");
@@ -14,6 +14,8 @@
14 14
15 15 use std::path::{Path, PathBuf};
16 16
17 + pub use audiofiles_core::config_key::ConfigKey;
18 +
17 19 use audiofiles_core::analysis::AnalysisResult;
18 20 use audiofiles_core::analysis::config::AnalysisConfig;
19 21 use audiofiles_core::analysis::suggest::TagSuggestion;
@@ -239,7 +241,7 @@
239 241 /// scale at an integer target is trimmed to the ceiling (the gentlest reversible
240 242 /// fix); otherwise (default) the signal is left untouched and the overshoot is
241 243 /// only reported, leaving the encoder's clamp as the disclosed last resort.
242 - pub const FORGE_AUTO_TRIM_OVERSHOOT_KEY: &str = "forge.auto_trim_overshoot";
244 + pub const FORGE_AUTO_TRIM_OVERSHOOT_KEY: &str = ConfigKey::ForgeAutoTrimOvershoot.as_str();
243 245
244 246 /// Lightweight summary of a persisted trained head for the Settings UI (the model's
245 247 /// weights stay in core).
@@ -836,13 +838,13 @@
836 838 // --- Config ---
837 839
838 840 /// Get a user config value by key.
839 - fn get_config(&self, key: &str) -> BackendResult<Option<String>>;
841 + fn get_config(&self, key: ConfigKey) -> BackendResult<Option<String>>;
840 842
841 843 /// Set a user config value.
842 - fn set_config(&self, key: &str, value: &str) -> BackendResult<()>;
844 + fn set_config(&self, key: ConfigKey, value: &str) -> BackendResult<()>;
843 845
844 846 /// Delete a user config value by key. No-op if the key does not exist.
845 - fn delete_config(&self, key: &str) -> BackendResult<()>;
847 + fn delete_config(&self, key: ConfigKey) -> BackendResult<()>;
846 848
847 849 /// Set whether a VFS should sync audio file blobs to cloud.
848 850 fn set_vfs_sync_files(&self, id: VfsId, enabled: bool) -> BackendResult<()>;