Skip to content

Commit ea9e36f

Browse files
pmaxhoganclaude
andcommitted
fix(state): atomic master-key stamp + source insert (R1-P1-1)
Add StateRepo::insert_source_with_optional_master_key_stamp - the sqlite impl does the (optional) account encryption_master_key_id stamp AND the source insert in ONE transaction so a source-insert failure can never leave the account "provisioned" without the recovery phrase (the unrestorable-encrypted-backup class). A default impl covers in-memory test doubles. Tests: forced FK-violation insert rolls back the account stamp + leaves no orphan, retry succeeds, no-stamp path just inserts. Regenerates the .sqlx offline cache for the new transactional query (0 drift, prepare --check clean). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CyiRqk2DVwmJjEu5gcD1m
1 parent 25b0b04 commit ea9e36f

3 files changed

Lines changed: 262 additions & 0 deletions

File tree

.sqlx/query-4c8e48b3c132e9ae489ef276d1bf738cea9b3bacfe9362939a314ba0da602e9c.json

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/driven-core/src/state/mod.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,44 @@ pub trait StateRepo: Send + Sync {
337337
/// Inserts or replaces a `backup_sources` row by id.
338338
async fn upsert_source(&self, row: &SourceRow) -> Result<()>;
339339

340+
/// R1-P1-1 (data-safety): ATOMICALLY stamp an account's
341+
/// `encryption_master_key_id` (when `stamp_master_key_id` is `Some`) AND
342+
/// insert/replace the given `backup_sources` row, in ONE transaction.
343+
///
344+
/// The encrypted-source add flow (DESIGN s7.1) generates the account master
345+
/// key + recovery phrase on the FIRST encrypted source. If the account stamp
346+
/// and the source insert are two separate writes and the source insert fails
347+
/// AFTER the stamp committed, the account looks "provisioned" but the user
348+
/// never received the phrase - an unrestorable encrypted backup (the B3 class
349+
/// this guards). Doing both writes in one transaction makes the outcome
350+
/// all-or-nothing: either the account is stamped AND the source exists, or
351+
/// neither change persists and a retry re-reveals the phrase.
352+
///
353+
/// `stamp_master_key_id` is `None` for an unencrypted source or a SUBSEQUENT
354+
/// encrypted source (the account is already provisioned), in which case this
355+
/// is just the source insert in a (trivially atomic) transaction.
356+
///
357+
/// The default implementation performs the two writes sequentially (adequate
358+
/// for in-memory test doubles); the SQLite implementation overrides it with a
359+
/// real `BEGIN`/`COMMIT` transaction so a failure rolls BOTH back.
360+
async fn insert_source_with_optional_master_key_stamp(
361+
&self,
362+
source: &SourceRow,
363+
stamp_master_key_id: Option<(AccountId, String)>,
364+
) -> Result<()> {
365+
if let Some((account_id, master_key_id)) = stamp_master_key_id {
366+
let mut accounts = self.list_accounts().await?;
367+
let row = accounts
368+
.iter_mut()
369+
.find(|r| r.id == account_id)
370+
.ok_or_else(|| anyhow::anyhow!("account not found for master-key stamp"))?;
371+
row.encryption_master_key_id = Some(master_key_id);
372+
let row = row.clone();
373+
self.upsert_account(&row).await?;
374+
}
375+
self.upsert_source(source).await
376+
}
377+
340378
/// Stamps `backup_sources.last_full_scan_at` and (when `deep_verify_at`
341379
/// is `Some`) `backup_sources.last_deep_verify_at` for one source (P2-7).
342380
///

crates/driven-core/src/state/sqlite.rs

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,111 @@ impl StateRepo for SqliteStateRepo {
653653
Ok(())
654654
}
655655

656+
async fn insert_source_with_optional_master_key_stamp(
657+
&self,
658+
source: &SourceRow,
659+
stamp_master_key_id: Option<(AccountId, String)>,
660+
) -> Result<()> {
661+
// R1-P1-1: do the (optional) account master-key stamp AND the source
662+
// insert in ONE transaction so a source-insert failure cannot leave the
663+
// account stamped-but-phraseless (an unrestorable encrypted backup).
664+
let mut tx = self.pool.begin().await?;
665+
666+
if let Some((account_id, master_key_id)) = stamp_master_key_id.as_ref() {
667+
let account_id = account_id.to_string();
668+
// A targeted column update (not a full-row replace) so the stamp
669+
// cannot clobber any other account column. Affecting zero rows means
670+
// the account vanished mid-flight - roll back rather than insert a
671+
// phantom-keyed orphan.
672+
let affected = sqlx::query!(
673+
"UPDATE accounts SET encryption_master_key_id = ?1 WHERE id = ?2",
674+
master_key_id,
675+
account_id,
676+
)
677+
.execute(&mut *tx)
678+
.await?
679+
.rows_affected();
680+
if affected == 0 {
681+
// Dropping `tx` without commit rolls back automatically; be
682+
// explicit for clarity.
683+
tx.rollback().await?;
684+
return Err(anyhow!(
685+
"account not found for master-key stamp; transaction rolled back"
686+
));
687+
}
688+
}
689+
690+
// Insert/replace the source row inside the same transaction.
691+
let id = source.id.to_string();
692+
let account_id = source.account_id.to_string();
693+
let enabled = source.enabled as i64;
694+
let encryption_enabled = source.encryption_enabled as i64;
695+
let respect_gitignore = source.respect_gitignore as i64;
696+
let include_patterns = serde_json::to_string(&source.include_patterns)?;
697+
let exclude_patterns = serde_json::to_string(&source.exclude_patterns)?;
698+
let wrapped: Option<&[u8]> = source.wrapped_source_key.as_deref();
699+
let deep_verify_interval_secs = source.deep_verify_interval_secs as i64;
700+
701+
sqlx::query!(
702+
r#"
703+
INSERT INTO backup_sources (
704+
id, account_id, display_name, enabled,
705+
local_path, drive_folder_id, drive_folder_path,
706+
encryption_enabled, wrapped_source_key, respect_gitignore,
707+
include_patterns, exclude_patterns, schedule_json_v2_reserved,
708+
deep_verify_interval_secs, last_full_scan_at, last_deep_verify_at,
709+
created_at
710+
) VALUES (
711+
?1, ?2, ?3, ?4,
712+
?5, ?6, ?7,
713+
?8, ?9, ?10,
714+
?11, ?12, ?13,
715+
?14, ?15, ?16,
716+
?17
717+
)
718+
ON CONFLICT(id) DO UPDATE SET
719+
account_id = excluded.account_id,
720+
display_name = excluded.display_name,
721+
enabled = excluded.enabled,
722+
local_path = excluded.local_path,
723+
drive_folder_id = excluded.drive_folder_id,
724+
drive_folder_path = excluded.drive_folder_path,
725+
encryption_enabled = excluded.encryption_enabled,
726+
wrapped_source_key = excluded.wrapped_source_key,
727+
respect_gitignore = excluded.respect_gitignore,
728+
include_patterns = excluded.include_patterns,
729+
exclude_patterns = excluded.exclude_patterns,
730+
schedule_json_v2_reserved = excluded.schedule_json_v2_reserved,
731+
deep_verify_interval_secs = excluded.deep_verify_interval_secs,
732+
last_full_scan_at = excluded.last_full_scan_at,
733+
last_deep_verify_at = excluded.last_deep_verify_at,
734+
created_at = excluded.created_at
735+
"#,
736+
id,
737+
account_id,
738+
source.display_name,
739+
enabled,
740+
source.local_path,
741+
source.drive_folder_id,
742+
source.drive_folder_path,
743+
encryption_enabled,
744+
wrapped,
745+
respect_gitignore,
746+
include_patterns,
747+
exclude_patterns,
748+
source.schedule_json_v2_reserved,
749+
deep_verify_interval_secs,
750+
source.last_full_scan_at,
751+
source.last_deep_verify_at,
752+
source.created_at,
753+
)
754+
.execute(&mut *tx)
755+
.await?;
756+
757+
tx.commit().await?;
758+
Ok(())
759+
}
760+
656761
async fn mark_source_scanned(
657762
&self,
658763
id: SourceId,
@@ -1580,6 +1685,113 @@ mod tests {
15801685
assert!(repo.list_sources().await.unwrap().is_empty());
15811686
}
15821687

1688+
#[tokio::test]
1689+
async fn atomic_stamp_and_insert_commits_both_on_success() {
1690+
// R1-P1-1: the happy path stamps the account's master-key id AND inserts
1691+
// the source in one transaction; both land.
1692+
let (repo, _dir) = temp_repo().await;
1693+
let mut acct = sample_account();
1694+
acct.encryption_master_key_id = None; // start unprovisioned
1695+
repo.upsert_account(&acct).await.unwrap();
1696+
1697+
let mut src = sample_source(acct.id);
1698+
src.encryption_enabled = true;
1699+
repo.insert_source_with_optional_master_key_stamp(
1700+
&src,
1701+
Some((acct.id, "kc:alice-master".into())),
1702+
)
1703+
.await
1704+
.unwrap();
1705+
1706+
// Account is now provisioned AND the source exists.
1707+
let after = repo.list_accounts().await.unwrap();
1708+
assert_eq!(
1709+
after[0].encryption_master_key_id.as_deref(),
1710+
Some("kc:alice-master"),
1711+
"the account master-key id must be stamped on success"
1712+
);
1713+
let sources = repo.list_sources().await.unwrap();
1714+
assert_eq!(sources, vec![src]);
1715+
}
1716+
1717+
#[tokio::test]
1718+
async fn atomic_stamp_and_insert_rolls_back_account_stamp_on_source_failure() {
1719+
// R1-P1-1 (data-safety): if the source insert fails, the account
1720+
// master-key stamp MUST roll back too - so the account is NEVER left
1721+
// "provisioned" without the source/phrase (an unrestorable encrypted
1722+
// backup). Force the source insert to fail with a foreign-key violation
1723+
// (a source whose account_id points at NO account row).
1724+
let (repo, _dir) = temp_repo().await;
1725+
let mut acct = sample_account();
1726+
acct.encryption_master_key_id = None;
1727+
repo.upsert_account(&acct).await.unwrap();
1728+
1729+
// A source row referencing a non-existent account -> FK violation on
1730+
// insert (foreign_keys = ON).
1731+
let mut bad_src = sample_source(AccountId::new_v4());
1732+
bad_src.encryption_enabled = true;
1733+
1734+
let err = repo
1735+
.insert_source_with_optional_master_key_stamp(
1736+
&bad_src,
1737+
Some((acct.id, "kc:alice-master".into())),
1738+
)
1739+
.await
1740+
.expect_err("source insert must fail on the FK violation");
1741+
assert!(
1742+
format!("{err:#}").to_lowercase().contains("foreign")
1743+
|| format!("{err:#}").to_lowercase().contains("constraint"),
1744+
"expected a foreign-key/constraint error, got: {err:#}"
1745+
);
1746+
1747+
// The account stamp rolled back: still unprovisioned, and NO source row.
1748+
let after = repo.list_accounts().await.unwrap();
1749+
assert_eq!(
1750+
after[0].encryption_master_key_id, None,
1751+
"the account must NOT be marked provisioned after a rolled-back insert"
1752+
);
1753+
assert!(
1754+
repo.list_sources().await.unwrap().is_empty(),
1755+
"no source row may persist after a rolled-back insert"
1756+
);
1757+
1758+
// A RETRY against a valid source now succeeds and stamps the account.
1759+
let mut good_src = sample_source(acct.id);
1760+
good_src.encryption_enabled = true;
1761+
repo.insert_source_with_optional_master_key_stamp(
1762+
&good_src,
1763+
Some((acct.id, "kc:alice-master".into())),
1764+
)
1765+
.await
1766+
.expect("retry must succeed");
1767+
let after = repo.list_accounts().await.unwrap();
1768+
assert_eq!(
1769+
after[0].encryption_master_key_id.as_deref(),
1770+
Some("kc:alice-master")
1771+
);
1772+
assert_eq!(repo.list_sources().await.unwrap().len(), 1);
1773+
}
1774+
1775+
#[tokio::test]
1776+
async fn atomic_insert_without_stamp_just_inserts_the_source() {
1777+
// R1-P1-1: a subsequent encrypted source (or an unencrypted one) passes
1778+
// `None` for the stamp - the account row is untouched, only the source
1779+
// is inserted.
1780+
let (repo, _dir) = temp_repo().await;
1781+
let acct = sample_account(); // already provisioned in the sample
1782+
repo.upsert_account(&acct).await.unwrap();
1783+
let original_key = acct.encryption_master_key_id.clone();
1784+
1785+
let src = sample_source(acct.id);
1786+
repo.insert_source_with_optional_master_key_stamp(&src, None)
1787+
.await
1788+
.unwrap();
1789+
1790+
let after = repo.list_accounts().await.unwrap();
1791+
assert_eq!(after[0].encryption_master_key_id, original_key);
1792+
assert_eq!(repo.list_sources().await.unwrap(), vec![src]);
1793+
}
1794+
15831795
#[tokio::test]
15841796
async fn cascade_delete_account_removes_sources_and_files() {
15851797
let (repo, _dir) = temp_repo().await;

0 commit comments

Comments
 (0)