diff --git a/crates/tcfs-sync/src/reconcile.rs b/crates/tcfs-sync/src/reconcile.rs index be49799f..565ce480 100644 --- a/crates/tcfs-sync/src/reconcile.rs +++ b/crates/tcfs-sync/src/reconcile.rs @@ -1611,6 +1611,19 @@ pub async fn reconcile( let local = local_files.get(rel_path); let remote = remote_index.get(rel_path); let tracked = state.get_by_rel_path(rel_path).map(|(_, s)| s); + // TIN-3277/TIN-3278: `get_by_rel_path` is a fuzzy suffix match over the + // whole entry map, while the push path re-resolves its own baseline with + // the exact canonical key (`StateCache::get`) — and only that exact entry + // is ever written back. When duplicate keys exist for one file (live on + // neo today) the two lookups can disagree, and which one the fuzzy matcher + // returns is HashMap-iteration-order luck. Prove they are the SAME record + // (pointer equality into the one entry map) so the self-heal predicate can + // fail closed rather than reason about a record the push would not touch. + // No other classification decision consumes this flag. + let tracked_is_exact = match (tracked, local.and_then(|path| state.get(path.as_path()))) { + (Some(fuzzy), Some(exact)) => std::ptr::eq(fuzzy, exact), + _ => false, + }; let action = classify_path( rel_path, @@ -1621,6 +1634,7 @@ pub async fn reconcile( remote_prefix, device_id, config, + tracked_is_exact, ) .await .with_context(|| format!("classifying reconciliation path {rel_path:?}"))?; @@ -3158,6 +3172,7 @@ async fn classify_path( remote_prefix: &str, device_id: &str, config: &ReconcileConfig, + tracked_is_exact: bool, ) -> Result { let action = match (local, remote, tracked) { // New local file — not on remote, not previously synced @@ -3267,6 +3282,7 @@ async fn classify_path( op, remote_prefix, device_id, + tracked_is_exact, ) .await; } @@ -3285,6 +3301,11 @@ async fn classify_path( } /// Compare when both local and remote exist — uses vector clocks. +/// +/// `tracked_is_exact` says whether `tracked` is the state record that the push +/// path would also resolve (exact canonical key) — see +/// `self_rewrite_retick_applies`, the only decision here that depends on it. +#[allow(clippy::too_many_arguments)] async fn compare_both_exist( rel_path: &str, local_path: &Path, @@ -3293,6 +3314,7 @@ async fn compare_both_exist( op: &Operator, remote_prefix: &str, device_id: &str, + tracked_is_exact: bool, ) -> Result { // Symlinks are first-class entries: they must NOT be dereferenced and hashed // like regular files (that would hash the *target's* content and then fail to @@ -3398,13 +3420,73 @@ async fn compare_both_exist( // clock is untouched, so re-recording the conflict each cycle stays // idempotent (no per-cycle clock growth). `reclassify_git_ff_conflicts` is // unaffected — it decides on git SHA ancestry, not on these clocks. - if let Some(tracked_state) = tracked { - if tracked_state.blake3 != local_hash - && local_vclock.partial_cmp_vc(&remote_manifest.vclock) - == Some(std::cmp::Ordering::Less) - { - local_vclock.tick(device_id); - } + // + // The ordering is computed ONCE, against the STORED clock, before any tick. + // Both out-of-band re-tick rules below key off `stored_ordering`, never off + // a clock a sibling rule already mutated: TIN-2584 (`Less`) and TIN-3277 + // (`Equal`) are disjoint cases of the same stored comparison, and the + // `else if` makes that mutual exclusion structural. Reading a mutated clock + // here would let a `Less` pair be re-ticked into `Equal` and then promoted + // to `Greater`/`LocalNewer`, defeating the narrowing comment above. + let stored_ordering = local_vclock.partial_cmp_vc(&remote_manifest.vclock); + // TIN-3277 local-side evidence, read from the same `symlink_metadata` call + // that chose this comparator, before any tick. + let local_size = local_metadata.len(); + let local_content_plausible = local_age_ciphertext_intact(rel_path, local_path); + let self_rewrite_shape = |tracked_is_exact: bool| { + self_rewrite_retick_applies( + rel_path, + tracked, + tracked_is_exact, + &local_hash, + local_size, + local_content_plausible, + &remote_manifest.file_hash, + stored_ordering, + device_id, + remote_device, + ) + }; + if tracked.is_some_and(|tracked_state| tracked_state.blake3 != local_hash) + && stored_ordering == Some(std::cmp::Ordering::Less) + { + local_vclock.tick(device_id); + } else if self_rewrite_shape(tracked_is_exact) { + // TIN-3277: an equal-clock self-pair whose remote content is byte-identical + // to our own tracked baseline is proof of a local out-of-band rewrite, not a + // concurrent-writer conflict. See `self_rewrite_retick_applies`. + // + // Audit trail: log the remote manifest key we intend to displace. The + // displaced bytes are `tracked.blake3` — this device's own last-synced + // content — so this line is sufficient to reconstruct what was replaced + // without a parked copy. It is emitted at PLAN time and the field names say + // so: the engine re-reads the remote before committing, so these values are + // the remote as observed during classification (best-effort provenance), not + // a post-hoc record of what the push actually overwrote. A remote that moved + // in between is vetoed at execute time and logged there instead. + info!( + path = %rel_path, + device = %device_id, + displaced_manifest_at_plan = %manifest_path, + displaced_hash_at_plan = %remote_manifest.file_hash, + local_hash = %local_hash, + "TIN-3277 self-heal: equal-clock self-pair, remote content == our tracked \ + baseline — re-ticking as a local out-of-band rewrite and planning a push" + ); + local_vclock.tick(device_id); + } else if !tracked_is_exact && self_rewrite_shape(true) { + // Everything about the shape says "local out-of-band rewrite", but the + // baseline we would have proved it against is not the state record the push + // path would overwrite (TIN-3278 key duplication). Decline and let the + // ordinary Conflict arm record it — visible, and no push justified by a + // record that may describe a different file. + warn!( + path = %rel_path, + device = %device_id, + "TIN-3277 self-heal declined: plan-time tracked entry is not the exact-key \ + record the push path would overwrite (TIN-3278 state-cache key duplication) \ + — recording a conflict instead" + ); } let outcome = compare_clocks( @@ -3426,6 +3508,213 @@ async fn compare_both_exist( )) } +/// TIN-3277: is this equal-clock divergence a purely LOCAL out-of-band rewrite? +/// +/// An out-of-band writer (home-manager secrets/dotfile materialization on neo, +/// `git commit` on a ref, any editor that bypasses the tcfs write hook) rewrites +/// an enrolled file WITHOUT ticking the tcfs vclock. The clock therefore freezes +/// at its last-synced value while local content diverges, so `compare_clocks` +/// sees EQUAL clocks + differing blake3 and returns `Conflict`. The reconcile +/// `Conflict` arm is record-only (it bumps `times_recorded` and flips status; it +/// never pushes), so those paths become structurally unpushable — observed live +/// on neo as 7 permanent conflicts (`secrets/**`, `dotfiles/tcfs/devices.json`) +/// stuck 24+ days with `times_recorded` in the thousands. +/// +/// The safety proof is CONTENT identity, not device identity. The load-bearing +/// clause is `tracked.blake3 == remote_hash`: the remote copy is byte-identical +/// to what THIS device last synced. Overwriting it therefore cannot lose another +/// writer's work — there is nothing on the remote that is not already in this +/// device's own tracked baseline. Any foreign write, from any device, by any +/// mechanism, changes `remote_hash`, fails this clause, and falls through to the +/// ordinary `compare_clocks` -> `Conflict` path exactly as before this fix. That +/// is what makes the promotion safe without authenticating anything. +/// +/// Precision about the strength of that clause: it compares our baseline against +/// the manifest's self-declared `file_hash` field. +/// `engine::validate_indexed_manifest_binding` binds manifest BYTES to the +/// index's `manifest_hash`; it does not prove the declared `file_hash` matches +/// the chunk payload. A write-capable peer could therefore publish +/// `file_hash == our baseline` over different chunks and satisfy this clause. So +/// the guarantee is exactly "no HONEST remote writer's content is displaced", +/// not an unconditional proof against a master-key-holding adversary — who can +/// destroy data outright anyway, and whose forged payload the pull path's own +/// content-hash verification would reject. +/// +/// `remote_device == device_id` is kept as CORROBORATION ONLY, deliberately not +/// as the safety boundary. `written_by` (`manifest.rs`) is an unauthenticated, +/// client-set JSON string — `validate_indexed_manifest_binding` binds manifest +/// BYTES to the index's `manifest_hash`, it does not sign the device claim — so +/// any holder of the master key could write `"written_by":"neo"`. Under the +/// content-identity clause that buys an attacker nothing: to pass the predicate +/// they must publish content equal to our tracked baseline, i.e. leave the +/// remote exactly as this device last left it. A spoofed `written_by` over +/// DIFFERENT content still fails `tracked.blake3 == remote_hash` and records a +/// conflict. The clause also keeps a colliding/cloned `device_id` (two hosts +/// sharing one id) from mattering: the pair only heals when the remote content +/// is our own baseline. +/// +/// Given both clauses, the only event that can explain the divergence is a local +/// rewrite that skipped the tick (an equal clock proves no later push happened — +/// a real push ticks). Model it as an ordinary local modification: tick a +/// comparison-only CLONE of the clock, mirroring `engine.rs` +/// `local_edit_inferred -> tick` and the TIN-2584 dominated-ref fix, so the pair +/// classifies `LocalNewer` -> `Push` and converges. +/// +/// Deliberately narrow — every clause is load-bearing: +/// * `tracked.blake3 == remote_hash`: the remote is our own last-synced bytes, +/// so the push displaces only this device's own history (above). This is the +/// clause that makes "no silent loss" hold; it is checked against real +/// content, never against a self-declared identity. +/// * `tracked.blake3 != local_hash`: evidence the LOCAL file moved since the +/// last sync. Without it, a state cache whose tracked hash still matches the +/// live file but disagrees with the remote is ambiguous — fail closed. +/// * `remote_device == device_id`: corroboration; also preserves the +/// pre-existing behavior that a DISTINCT-device equal-clock pair records a +/// conflict, and makes legacy v1 manifests (empty `written_by`) fail closed. +/// * `stored_ordering == Equal` only, computed against the STORED clock before +/// any re-tick: `Less` is TIN-2584's case, `Greater` already pushes, `None` +/// (concurrent) is a real conflict. Taking the ordering as a parameter rather +/// than re-deriving it here makes it impossible for a caller to evaluate this +/// predicate against a clock a sibling rule already ticked. +/// * `tracked_is_exact`: the baseline above must come from the SAME state +/// record the push path will read and overwrite. Planning resolves `tracked` +/// with the fuzzy suffix matcher `StateCache::get_by_rel_path`, while the +/// push resolves its own baseline with the exact canonical key +/// (`engine::upload_planned_push_with_device` -> `StateCache::get`) — and only +/// the exact-keyed entry is ever written back by `state.set`. Under TIN-3278's +/// live key-namespace duplication (absolute vs relative keys for the same +/// file) those two can be DIFFERENT records, and `get_by_rel_path` returns +/// whichever one `HashMap` iteration reaches first. Evaluating a +/// content-identity proof against a record the push will not touch is exactly +/// the ambiguity this predicate must not paper over, so the caller proves +/// identity of the two lookups (pointer equality) and we fail closed when it +/// does not hold. +/// * `local_size != 0` while `tracked.size > 0`: the local side is the one thing +/// here that no tcfs-aware actor authored, and the predicate's only other +/// local evidence (`tracked.blake3 != local_hash`) cannot tell "home-manager +/// re-materialized the secret" from "home-manager/agenix failed and left an +/// empty file". Publishing the second case would replace good remote content +/// with nothing and make the previous manifest+chunks GC-eligible, having just +/// removed the latched Conflict that used to protect the path. Fail closed. +/// * `local_content_plausible`: for `*.age` paths, the live bytes must still +/// carry a recognizable age header (binary `age-encryption.org/v1` or the +/// armored `-----BEGIN AGE ENCRYPTED FILE-----`), so a plaintext error string +/// or truncated-to-nothing materialization is never published over intact +/// ciphertext. Deliberately keyed off the `.age` extension rather than a +/// `secrets/` prefix: the live stuck set includes `secrets/.manifest.toml`, +/// which is plaintext TOML by design, and a prefix rule would make this fix a +/// permanent no-op there. Residual limit, stated honestly: a truncation that +/// preserves the header is not detectable from content alone and is NOT +/// covered by this clause. +/// * `local_hash != remote_hash`: identical content is `UpToDate` anyway. +/// * not `.git`-internal: `.git` divergence keeps the fail-closed +/// fast-forward veto / loser-guard keep-both path (G5-git-13), which decides +/// on git SHA ancestry rather than clocks. Promoting a divergent non-head ref +/// to `LocalNewer` here would defeat that guard. +/// +/// Why no parked copy / `.conflict-{device}` sidecar: `KeepBoth` exists to +/// preserve a SECOND writer's bytes. Here the displaced remote content is +/// `tracked.blake3` — this device's own last-synced bytes, still described by +/// its own state entry and (until GC) still addressable by the manifest key the +/// self-heal logs at `info`. A sidecar would duplicate our own history and, on +/// `secrets/**`, would multiply ciphertext copies of sensitive material for no +/// recovery benefit. +/// +/// Convergence / no infinite tick: the tick lands on the comparison clone only. +/// A successful push re-derives and stores the ticked clock (and clears the +/// stale `conflict` payload via `make_sync_state_full`), so the next cycle sees +/// equal hashes -> `UpToDate`. A failed or deferred push leaves the stored clock +/// untouched, so the next cycle re-derives the same single-step tick — the +/// ordinary retry path, with no per-cycle clock growth. If the remote moves +/// between plan and execute, the engine's own conflict veto +/// (`engine.rs` `SyncOutcome::Conflict` arm) records a visible `ConflictInfo` +/// and rewrites the tracked baseline to the live local hash, which DISARMS +/// `tracked.blake3 != local_hash` for that path: the entry reverts to the +/// pre-fix recorded-conflict behavior (surfaced by `tcfs conflicts` / D-Bus) +/// rather than looping. Fail-closed, not silent. +#[allow(clippy::too_many_arguments)] +fn self_rewrite_retick_applies( + rel_path: &str, + tracked: Option<&SyncState>, + tracked_is_exact: bool, + local_hash: &str, + local_size: u64, + local_content_plausible: bool, + remote_hash: &str, + stored_ordering: Option, + device_id: &str, + remote_device: &str, +) -> bool { + if device_id.is_empty() || remote_device != device_id { + return false; + } + if local_hash == remote_hash { + return false; + } + if is_git_internal_path(rel_path) { + return false; + } + // The baseline we are about to reason about must be the record the push path + // will read and overwrite, not whichever duplicate the fuzzy plan-time + // matcher happened to return (TIN-3278). + if !tracked_is_exact { + return false; + } + let Some(tracked_state) = tracked else { + return false; + }; + // The local file moved since the last sync ... + if tracked_state.blake3 == local_hash { + return false; + } + // ... and the remote is still exactly the bytes this device last synced, so + // nothing but our own history can be displaced. THE load-bearing clause. + if tracked_state.blake3 != remote_hash { + return false; + } + // The local side is the only side no tcfs-aware actor authored: refuse to + // publish a degenerate (emptied / non-ciphertext) out-of-band rewrite over + // intact remote content. + if local_size == 0 && tracked_state.size > 0 { + return false; + } + if !local_content_plausible { + return false; + } + stored_ordering == Some(std::cmp::Ordering::Equal) +} + +/// Number of leading bytes needed to recognize either age container header. +const AGE_HEADER_PROBE_LEN: usize = 34; + +/// Do the live local bytes still look like age ciphertext? +/// +/// Only meaningful for `*.age` paths; every other path answers `true` (nothing to +/// check). Used as a fail-closed guard on the TIN-3277 self-heal push, which is +/// the one push in this file that no tcfs-aware writer initiated: an out-of-band +/// materialization that failed and left a plaintext error message must not be +/// published over the intact ciphertext currently on the remote. +/// +/// Accepts BOTH age container forms — the binary header (`age-encryption.org/v1`, +/// which is what neo's live enrolled `secrets/**.age` files carry) and the +/// ASCII-armored header — so the guard cannot silently disable the fix for a +/// legitimately armored corpus. +fn local_age_ciphertext_intact(rel_path: &str, local_path: &Path) -> bool { + if !rel_path.to_ascii_lowercase().ends_with(".age") { + return true; + } + let Ok(file) = std::fs::File::open(local_path) else { + return false; + }; + let mut head = Vec::with_capacity(AGE_HEADER_PROBE_LEN); + let mut probe = std::io::Read::take(file, AGE_HEADER_PROBE_LEN as u64); + if std::io::Read::read_to_end(&mut probe, &mut head).is_err() { + return false; + } + head.starts_with(b"age-encryption.org/v1") + || head.starts_with(b"-----BEGIN AGE ENCRYPTED FILE-----") +} + /// Causal metadata extracted from one fully bound remote manifest. /// /// The logical identity includes the entry kind. The raw content identity is @@ -4472,6 +4761,37 @@ pub async fn execute_plan( if !upload.skipped { result.pushed += 1; result.bytes_uploaded += upload.bytes; + } else { + // TIN-3277: a planned push the engine skipped at execute + // time is neither counted in `pushed` nor pushed to + // `errors`, so it used to leave no trace at all. Surface + // it — but say WHICH skip it was, keyed off the engine's + // own verdict rather than asserting a veto: `skipped` is + // also set for benign content-dedup (`UpToDate`) and for + // `RemoteNewer`. Only the `Conflict` verdict means the + // remote moved between plan and execute, which for a + // TIN-3277 self-heal is the signal that the entry has + // reverted to a recorded conflict. + let skip_reason = match &upload.outcome { + Some(crate::conflict::SyncOutcome::Conflict(_)) => { + "execute-time conflict veto: remote moved since plan" + } + Some(crate::conflict::SyncOutcome::RemoteNewer) => { + "execute-time veto: remote is newer than the plan assumed" + } + Some(crate::conflict::SyncOutcome::UpToDate) => { + "no-op: remote already holds this content" + } + Some(crate::conflict::SyncOutcome::LocalNewer) | None => { + "engine skipped the upload without a conflict verdict" + } + }; + info!( + path = %rel_path, + outcome = ?upload.outcome, + skip_reason, + "planned push skipped by the engine" + ); } } Err(e) => { @@ -5693,10 +6013,18 @@ mod tests { op.write(&manifest_key, bytes.clone()).await.unwrap(); let entry = RemoteIndexEntry::new(object_id, 5, 0); - let error = - compare_both_exist("doc.txt", &local_path, &entry, None, &op, "data", "neo") - .await - .expect_err("untrusted manifest metadata must abort classification"); + let error = compare_both_exist( + "doc.txt", + &local_path, + &entry, + None, + &op, + "data", + "neo", + true, + ) + .await + .expect_err("untrusted manifest metadata must abort classification"); assert!(!format!("{error:#}").is_empty()); assert_eq!(std::fs::read(&local_path).unwrap(), b"local"); @@ -5808,6 +6136,7 @@ mod tests { &op, "data", "neo", + true, ) .await .expect("a local regular-file replacement should be classified causally"); @@ -5850,6 +6179,7 @@ mod tests { &op, "data", "neo", + true, ) .await .expect("a causally newer remote symlink should replace a regular file"); @@ -5893,6 +6223,7 @@ mod tests { &op, "data", "neo", + true, ) .await .expect("a local symlink replacement should be classified causally"); @@ -5940,6 +6271,7 @@ mod tests { &op, "data", "neo", + true, ) .await .expect("a causally newer remote regular file should replace a symlink"); @@ -5987,6 +6319,7 @@ mod tests { &op, "data", "neo", + true, ) .await .expect("concurrent type replacements should classify as a conflict"); @@ -9545,6 +9878,7 @@ mod tests { prefix, "honey", &config, + true, ) .await; @@ -9588,6 +9922,7 @@ mod tests { prefix, "honey", &config, + true, ) .await; @@ -9631,6 +9966,7 @@ mod tests { prefix, "honey", &config, + true, ) .await; @@ -9665,6 +10001,7 @@ mod tests { "data", "honey", &config, + true, ) .await; @@ -9699,6 +10036,7 @@ mod tests { "data", "honey", &config, + true, ) .await; @@ -9708,6 +10046,786 @@ mod tests { ); } + // ── TIN-3277: out-of-band self-rewrite self-heal ────────────────────────── + + /// Local bytes that satisfy the `*.age` container-header guard + /// (`local_age_ciphertext_intact`). Every `*.age` test that is NOT about that + /// guard uses these, so its verdict still turns on the clause it is testing + /// rather than on an accidental header failure. Mirrors the binary header form + /// the live enrolled `secrets/**.age` files on neo actually carry. + const AGE_LOCAL_BYTES: &[u8] = + b"age-encryption.org/v1\n-> X25519 dGVzdA\nrematerialized-ciphertext\n"; + + /// Build the exact live shape of a TIN-3277 stuck entry: the tracked hash is + /// the last-synced (== remote) content, the clock is frozen at the value that + /// push wrote, and the live local file has since been rewritten out of band. + /// + /// `tracked.blake3 == remote_hash` here is not incidental — it IS the + /// content-identity clause the fix rests on. Tests that must exercise the + /// clause's negative case build their pair explicitly instead. + async fn tin3277_self_pair( + op: &Operator, + rel: &str, + device: &str, + remote_hash: &str, + local_bytes: &[u8], + ) -> (tempfile::TempDir, PathBuf, RemoteIndexEntry, SyncState) { + let remote_entry = seed_manifest_vclock( + op, + "data", + rel, + remote_hash, + &format!(r#"{{"clocks":{{"{device}":1}}}}"#), + device, + ) + .await; + let dir = tempfile::TempDir::new().unwrap(); + let local_path = dir.path().join("entry"); + std::fs::write(&local_path, local_bytes).unwrap(); + // Tracked baseline == what we last pushed (the remote content hash). + let tracked = tin2584_state(remote_hash, tin2584_vclock(&[(device, 1)])); + (dir, local_path, remote_entry, tracked) + } + + /// (a) The defect itself: `secrets/*` rewritten by an out-of-band writer + /// (home-manager materialization) with the clock frozen at `{neo:1}` — the + /// remote side is this same device's own prior push. It must re-tick and + /// PUSH, not record yet another unresolvable self-conflict. + #[tokio::test] + async fn tin3277_self_rewrite_equal_clock_pushes_instead_of_conflicting() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + let (_dir, local_path, remote_entry, tracked) = + tin3277_self_pair(&op, rel, "neo", "lastpushedhash", AGE_LOCAL_BYTES).await; + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + + assert!( + matches!( + action, + Ok(ReconcileAction::Push { + reason: PushReason::LocalNewer, + .. + }) + ), + "an equal-clock SELF-pair is a local out-of-band rewrite and must push, got {action:?}" + ); + } + + /// (b) Narrowing guard: a genuine TWO-DEVICE equal-clock divergence has a + /// second writer whose work a push would clobber — it must still record a + /// conflict. This is the clause that keeps the fix from becoming a + /// last-writer-wins clobber. + #[tokio::test] + async fn tin3277_distinct_device_equal_clock_still_records_conflict() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + // Remote manifest written by `honey`; we are `neo`. + let (_dir, local_path, remote_entry, tracked) = + tin3277_self_pair(&op, rel, "honey", "honeypushedhash", AGE_LOCAL_BYTES).await; + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + + assert!( + matches!(action, Ok(ReconcileAction::Conflict { .. })), + "a distinct-device equal-clock divergence must still record a Conflict, got {action:?}" + ); + } + + /// (c) THE LOAD-BEARING CLAUSE, negative case: same self-pair shape, same + /// equal clocks, same `written_by == device_id` — but the REMOTE content is + /// no longer the bytes this device last synced. Something else wrote it (a + /// second state cache under one `device_id`, a peer with a spoofed + /// `written_by`, a `tcfs push` from a different state dir). Pushing would + /// destroy content that exists nowhere else, so this MUST stay a recorded + /// `Conflict`. This is the test that would fail if the safety proof were + /// device identity instead of content identity. + #[tokio::test] + async fn tin3277_remote_moved_since_baseline_still_records_conflict() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + // Remote claims to be us and carries the same clock, but its content is + // NOT our tracked baseline. + let remote_entry = seed_manifest_vclock( + &op, + "data", + rel, + "foreign-write-hash", + r#"{"clocks":{"neo":1}}"#, + "neo", + ) + .await; + let dir = tempfile::TempDir::new().unwrap(); + let local_path = dir.path().join("entry"); + // Valid age ciphertext: this test must fail on the content-identity clause, + // not on the degenerate-content guard. + std::fs::write(&local_path, AGE_LOCAL_BYTES).unwrap(); + // Our baseline is our own last push, which the remote no longer holds. + let tracked = tin2584_state("our-baseline-hash", tin2584_vclock(&[("neo", 1)])); + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + + assert!( + matches!(action, Ok(ReconcileAction::Conflict { .. })), + "remote content that is NOT our tracked baseline may never be silently \ + overwritten, even for a self-pair: got {action:?}" + ); + } + + /// (c2) STRUCTURAL double-tick regression. `device_id == written_by`, the + /// local file moved out of band, and the STORED clock is strictly dominated + /// by the remote's (`Ordering::Less`) — TIN-2584's case. If the self-heal + /// predicate were evaluated after TIN-2584's tick, the pair would read as + /// `Equal`, get ticked a second time, and be promoted to `Greater` -> + /// `LocalNewer` -> `Push`, silently overwriting a remote the local side has + /// never seen. Pin the behavior `main` has today: a recorded `Conflict`. + #[tokio::test] + async fn tin3277_self_pair_dominated_clock_is_not_promoted_to_push() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + // Remote is one push ahead of our state cache, and says it was us. + let remote_entry = seed_manifest_vclock( + &op, + "data", + rel, + "remote-ahead-hash", + r#"{"clocks":{"neo":2}}"#, + "neo", + ) + .await; + let dir = tempfile::TempDir::new().unwrap(); + let local_path = dir.path().join("entry"); + // Valid age ciphertext: this test must fail on the clock structure, not on + // the degenerate-content guard. + std::fs::write(&local_path, AGE_LOCAL_BYTES).unwrap(); + // Stale cache: one tick behind, tracked hash != live local hash. + let tracked = tin2584_state("stale-baseline-hash", tin2584_vclock(&[("neo", 1)])); + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + + assert!( + matches!(action, Ok(ReconcileAction::Conflict { .. })), + "a strictly-dominated self-pair is TIN-2584's recorded conflict and must NOT be \ + double-ticked into a push, got {action:?}" + ); + } + + /// (c3) STRUCTURAL double-tick regression, part 2 — the case that pins the + /// `else if` on its OWN, with the content-identity clause fully SATISFIED. + /// `tracked.blake3 == remote_hash` (a same-content republish the state cache + /// never recorded) while the stored clock is one tick behind (`Less`). v2 takes + /// TIN-2584's branch only, reaches `Equal`, and records a `Conflict`. The v1 + /// sequential shape would tick again off the already-mutated clock, reach + /// `Greater`, and push. Unlike (c2), removing the content clause cannot mask + /// this test — it fails the moment the two rules stop being mutually exclusive. + #[tokio::test] + async fn tin3277_same_content_republish_dominated_clock_is_not_double_ticked() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + // Remote holds exactly our baseline content but is a tick ahead of our cache. + let remote_entry = seed_manifest_vclock( + &op, + "data", + rel, + "our-baseline-hash", + r#"{"clocks":{"neo":2}}"#, + "neo", + ) + .await; + let dir = tempfile::TempDir::new().unwrap(); + let local_path = dir.path().join("entry"); + std::fs::write(&local_path, AGE_LOCAL_BYTES).unwrap(); + let tracked = tin2584_state("our-baseline-hash", tin2584_vclock(&[("neo", 1)])); + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + + assert!( + matches!(action, Ok(ReconcileAction::Conflict { .. })), + "a dominated stored clock must be ticked ONCE by TIN-2584 and never re-ticked \ + by the TIN-3277 rule, even when the content-identity clause holds, got {action:?}" + ); + } + + /// (c4) Degenerate local content, part 1: the out-of-band writer FAILED and + /// left an empty file. The local side is the one side no tcfs-aware actor + /// authored, so an emptied rewrite must never be published over intact remote + /// content — especially now that the latched `Conflict` that used to protect + /// these paths is gone. + #[tokio::test] + async fn tin3277_emptied_local_rewrite_is_not_pushed() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + let (_dir, local_path, remote_entry, tracked) = + tin3277_self_pair(&op, rel, "neo", "lastpushedhash", b"").await; + assert_eq!( + std::fs::metadata(&local_path).unwrap().len(), + 0, + "setup: the local file must actually be empty" + ); + assert!( + tracked.size > 0, + "setup: the baseline must be non-empty for this to be a truncation" + ); + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + + assert!( + matches!(action, Ok(ReconcileAction::Conflict { .. })), + "an emptied out-of-band rewrite must fail closed, not publish nothing over \ + intact remote content, got {action:?}" + ); + } + + /// (c5) Degenerate local content, part 2: a failed `agenix`/home-manager + /// materialization that left a PLAINTEXT error string where ciphertext belongs. + /// Same shape as (a) — which pushes — with only the local container header + /// changed, so this test isolates the `*.age` header guard. + #[tokio::test] + async fn tin3277_non_ciphertext_local_age_rewrite_is_not_pushed() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + let (_dir, local_path, remote_entry, tracked) = tin3277_self_pair( + &op, + rel, + "neo", + "lastpushedhash", + b"error: no identity matched any of the recipients\n", + ) + .await; + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + + assert!( + matches!(action, Ok(ReconcileAction::Conflict { .. })), + "non-ciphertext bytes must never be published over an intact *.age remote, \ + got {action:?}" + ); + + // Control: the guard is keyed off the `.age` extension, NOT a `secrets/` + // prefix — `secrets/.manifest.toml` is plaintext TOML by design and is one + // of the live stuck paths, so a prefix rule would make the fix a permanent + // no-op there. + let plain_rel = "secrets/.manifest.toml"; + let (_plain_dir, plain_local, plain_remote, plain_tracked) = tin3277_self_pair( + &op, + plain_rel, + "neo", + "lastpushedplainhash", + b"[secrets]\nrewritten = true\n", + ) + .await; + let plain_action = classify_path( + plain_rel, + Some(&plain_local), + Some(&plain_remote), + Some(&plain_tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + assert!( + matches!( + plain_action, + Ok(ReconcileAction::Push { + reason: PushReason::LocalNewer, + .. + }) + ), + "a non-.age path under secrets/ must still self-heal, got {plain_action:?}" + ); + } + + /// (c6) The self-heal's baseline must be the state record the PUSH path would + /// read and overwrite. Planning resolves `tracked` with the fuzzy suffix + /// matcher; the push re-resolves it with the exact canonical key, and only the + /// exact entry is ever written back. Under TIN-3278's live key duplication the + /// two can differ, so when the caller cannot prove they are the same record the + /// self-heal must decline (and the ordinary Conflict arm records it visibly). + /// Identical to (a) apart from `tracked_is_exact`. + #[tokio::test] + async fn tin3277_inexact_tracked_lookup_declines_self_heal() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + let (_dir, local_path, remote_entry, tracked) = + tin3277_self_pair(&op, rel, "neo", "lastpushedhash", AGE_LOCAL_BYTES).await; + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + false, + ) + .await; + + assert!( + matches!(action, Ok(ReconcileAction::Conflict { .. })), + "a self-heal whose baseline may not be the record the push would overwrite \ + must fail closed, got {action:?}" + ); + } + + /// (c7) The `*.age` container guard must accept BOTH age forms. A guard that + /// only knew the binary header would silently disable the fix for an armored + /// corpus — the same "no-op on its own target population" failure the + /// extension-vs-prefix decision above avoids. + #[test] + fn tin3277_age_header_guard_accepts_both_container_forms() { + let dir = tempfile::TempDir::new().unwrap(); + let cases: [(&str, &[u8], bool); 5] = [ + ("binary", b"age-encryption.org/v1\n-> X25519 dGVzdA\n", true), + ( + "armored", + b"-----BEGIN AGE ENCRYPTED FILE-----\nYWJj\n", + true, + ), + ("plaintext", b"error: no identity matched\n", false), + ("empty", b"", false), + ("short", b"age-encryption", false), + ]; + for (name, bytes, expected) in cases { + let path = dir.path().join(name); + std::fs::write(&path, bytes).unwrap(); + assert_eq!( + local_age_ciphertext_intact("secrets/api/token.age", &path), + expected, + "{name}: unexpected verdict for a *.age path" + ); + // Non-`.age` paths are out of scope for the container check. + assert!( + local_age_ciphertext_intact("secrets/.manifest.toml", &path), + "{name}: a non-.age path must never be gated on an age header" + ); + } + // Uppercase extensions count too. + let upper = dir.path().join("upper"); + std::fs::write(&upper, b"nope\n").unwrap(); + assert!(!local_age_ciphertext_intact( + "secrets/API/TOKEN.AGE", + &upper + )); + // A missing local file fails closed. + assert!(!local_age_ciphertext_intact( + "secrets/api/gone.age", + &dir.path().join("does-not-exist") + )); + } + + /// (d) No runaway ticking: the re-tick lands on a comparison-only CLONE, so + /// a deferred/failed push (stored clock untouched) yields the SAME + /// single-step decision every cycle — no per-cycle clock growth. + #[tokio::test] + async fn tin3277_deferred_push_does_not_accumulate_ticks() { + let op = memory_op(); + let rel = "secrets/.manifest.toml"; + let (_dir, local_path, remote_entry, tracked) = + tin3277_self_pair(&op, rel, "neo", "lastpushedhash", b"manifest-rewritten\n").await; + let clock_before = tracked.vclock.clone(); + + for cycle in 0..3 { + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + assert!( + matches!( + action, + Ok(ReconcileAction::Push { + reason: PushReason::LocalNewer, + .. + }) + ), + "cycle {cycle}: a deferred self-heal must stay a plain LocalNewer push, got {action:?}" + ); + assert_eq!( + tracked.vclock, clock_before, + "cycle {cycle}: classification must not mutate the stored vclock" + ); + } + } + + /// (e) `.git` carve-out: a `.git` self-pair keeps the fail-closed + /// fast-forward / loser-guard keep-both path (G5-git-13), which decides on + /// git SHA ancestry rather than clocks. The self-heal must not pre-empt it. + #[tokio::test] + async fn tin3277_git_internal_self_pair_stays_conflict() { + let op = memory_op(); + let rel = "repo/.git/refs/heads/main"; + let (_dir, local_path, remote_entry, tracked) = + tin3277_self_pair(&op, rel, "neo", "lastpushedhash", b"deadbeef\n").await; + + let action = classify_path( + rel, + Some(&local_path), + Some(&remote_entry), + Some(&tracked), + &op, + "data", + "neo", + &ReconcileConfig::default(), + true, + ) + .await; + + assert!( + matches!(action, Ok(ReconcileAction::Conflict { .. })), + "a .git self-pair must stay Conflict for the FF/keep-both guard, got {action:?}" + ); + } + + /// (f) FULL plan -> execute -> state cycle, no hand-built post-push world. + /// This is the test that proves the fix actually heals rather than merely + /// classifying differently, and it is where the engine's independent clock + /// re-derivation (`engine.rs` `local_edit_inferred -> tick`) is exercised for + /// real: the plan says `LocalNewer`, the engine must agree and NOT take its + /// conflict-veto branch, the stored clock must advance exactly one step, the + /// latched `conflict` payload must be cleared, and the next cycle must be a + /// no-op. Uses `dotfiles/tcfs/devices.json` — one of the live stuck paths. + #[tokio::test] + async fn tin3277_full_cycle_self_heal_advances_clock_and_clears_latch() { + let op = memory_op(); + let rel = "dotfiles/tcfs/devices.json"; + let dir = tempfile::TempDir::new().unwrap(); + let root = dir.path().join("root"); + std::fs::create_dir_all(root.join("dotfiles/tcfs")).unwrap(); + let local_path = root.join(rel); + std::fs::write(&local_path, b"{\"devices\":[\"neo\"]}\n").unwrap(); + + let blacklist = Blacklist::default(); + let config = ReconcileConfig::default(); + let mut state = crate::state::StateCache::open(&dir.path().join("state.json")).unwrap(); + + // 1. Baseline push: the file enters the pool the ordinary way. + let plan = reconcile(&op, &root, "data", &state, "neo", &blacklist, &config, None) + .await + .unwrap(); + let result = execute_plan(&plan, &op, &root, "data", &mut state, "neo", None, None) + .await + .unwrap(); + assert!(result.errors.is_empty(), "{:?}", result.errors); + assert_eq!(result.pushed, 1, "baseline push must land"); + let baseline_clock = state.get(&local_path).unwrap().vclock.clone(); + assert_eq!( + baseline_clock, + tin2584_vclock(&[("neo", 1)]), + "baseline push stores {{neo:1}}" + ); + + // 2. The defect's premise: an out-of-band writer (home-manager + // materialization) rewrites the file WITHOUT going through tcfs, so + // the stored clock stays frozen at {neo:1} while content diverges. + std::fs::write(&local_path, b"{\"devices\":[\"neo\",\"honey\"]}\n").unwrap(); + // Latch it the way the record-only Conflict arm has for 24 days, so the + // assertion below about the payload being cleared is meaningful. + let mut latched = state.get(&local_path).unwrap().clone(); + let live_hash = tcfs_chunks::hash_to_hex(&tcfs_chunks::hash_file(&local_path).unwrap()); + latched.status = crate::state::FileSyncStatus::Conflict; + latched.conflict = Some(ConflictInfo { + rel_path: rel.to_string(), + local_vclock: baseline_clock.clone(), + remote_vclock: baseline_clock.clone(), + local_blake3: live_hash.clone(), + remote_blake3: latched.blake3.clone(), + local_device: "neo".to_string(), + remote_device: "neo".to_string(), + detected_at: 0, + times_recorded: 4550, + remote_manifest_key: None, + }); + state.set(&local_path, latched); + + // 3. The heal: plan must classify LocalNewer ... + let heal_plan = reconcile(&op, &root, "data", &state, "neo", &blacklist, &config, None) + .await + .unwrap(); + assert!( + heal_plan.actions.iter().any(|a| matches!( + a, + ReconcileAction::Push { + rel_path: p, + reason: PushReason::LocalNewer, + .. + } if p == rel + )), + "out-of-band self-rewrite must plan a LocalNewer push, got {:?}", + heal_plan.actions + ); + + // ... and execute must actually push it. `pushed == 1` is the proof that + // the engine's re-derived clock ({neo:1} + one tick = {neo:2}) beat the + // remote's {neo:1} and the conflict-veto branch was NOT taken. + let heal_result = execute_plan( + &heal_plan, &op, &root, "data", &mut state, "neo", None, None, + ) + .await + .unwrap(); + assert!(heal_result.errors.is_empty(), "{:?}", heal_result.errors); + assert_eq!( + heal_result.pushed, 1, + "the healing push must actually land, not be veto-skipped" + ); + + let healed = state.get(&local_path).unwrap().clone(); + assert_eq!( + healed.vclock, + tin2584_vclock(&[("neo", 2)]), + "a successful heal advances the stored clock exactly one step" + ); + assert_eq!(healed.blake3, live_hash, "state tracks the healed content"); + assert!( + healed.conflict.is_none(), + "the healing push must clear the latched ConflictInfo" + ); + assert_eq!( + healed.status, + crate::state::FileSyncStatus::Synced, + "the healed entry must leave Conflict status" + ); + + // 4. Convergence: the next cycle is a no-op, and the clock does not grow. + let after_plan = reconcile(&op, &root, "data", &state, "neo", &blacklist, &config, None) + .await + .unwrap(); + assert!( + after_plan.actions.iter().all(|a| !matches!( + a, + ReconcileAction::Push { rel_path: p, .. } + | ReconcileAction::Conflict { rel_path: p, .. } if p == rel + )), + "after healing the pair must converge, got {:?}", + after_plan.actions + ); + execute_plan( + &after_plan, + &op, + &root, + "data", + &mut state, + "neo", + None, + None, + ) + .await + .unwrap(); + assert_eq!( + state.get(&local_path).unwrap().vclock, + tin2584_vclock(&[("neo", 2)]), + "no per-cycle clock growth once converged" + ); + } + + /// (g) The vetoed-push path, end to end. If a foreign write lands on the + /// remote BETWEEN plan and execute, the engine re-derives its own clock and + /// vetoes the push. Pin the whole consequence chain, because it is the fix's + /// only failure mode: the push is skipped (not counted as pushed), a visible + /// `ConflictInfo` is recorded, the baseline moves to the live local hash — + /// which DISARMS `tracked.blake3 != local_hash` — and the next cycle + /// therefore settles back into the pre-fix recorded-conflict behavior + /// instead of replanning the same never-landing push forever. + #[tokio::test] + async fn tin3277_veto_between_plan_and_execute_records_conflict_and_settles() { + let op = memory_op(); + let rel = "secrets/api/github_token.age"; + let dir = tempfile::TempDir::new().unwrap(); + let root = dir.path().join("root"); + std::fs::create_dir_all(root.join("secrets/api")).unwrap(); + let local_path = root.join(rel); + // Real age container bytes: this path is `*.age`, so the degenerate-content + // guard (`local_age_ciphertext_intact`) is live on both sides of the rewrite. + std::fs::write(&local_path, b"age-encryption.org/v1\n-> X25519 dg\nv1\n").unwrap(); + + let blacklist = Blacklist::default(); + let config = ReconcileConfig::default(); + let mut state = crate::state::StateCache::open(&dir.path().join("state.json")).unwrap(); + + let plan = reconcile(&op, &root, "data", &state, "neo", &blacklist, &config, None) + .await + .unwrap(); + execute_plan(&plan, &op, &root, "data", &mut state, "neo", None, None) + .await + .unwrap(); + + // Out-of-band rewrite -> the self-heal plans a LocalNewer push. + std::fs::write(&local_path, AGE_LOCAL_BYTES).unwrap(); + let live_hash = tcfs_chunks::hash_to_hex(&tcfs_chunks::hash_file(&local_path).unwrap()); + let heal_plan = reconcile(&op, &root, "data", &state, "neo", &blacklist, &config, None) + .await + .unwrap(); + assert!( + heal_plan.actions.iter().any(|a| matches!( + a, + ReconcileAction::Push { + rel_path: p, + reason: PushReason::LocalNewer, + .. + } if p == rel + )), + "precondition: the self-heal must have planned a push" + ); + + // A peer publishes a concurrent manifest after planning. Built inline (not + // via `seed_manifest_vclock`) so the index entry's chunk count matches the + // zero-chunk manifest body — the engine, unlike the classifier, validates + // that binding before uploading. + let foreign_body = format!( + r#"{{"version":2,"file_hash":"peer-content-hash","file_size":41,"chunks":[],"vclock":{{"clocks":{{"neo":1,"honey":1}}}},"written_by":"honey","written_at":0,"rel_path":"{rel}"}}"# + ) + .into_bytes(); + let foreign_hash = crate::index_entry::manifest_object_id(&foreign_body); + op.write(&format!("data/manifests/{foreign_hash}"), foreign_body) + .await + .unwrap(); + op.write( + &format!("data/index/{rel}"), + crate::index_entry::VersionedIndexEntry::committed(RemoteIndexEntry::new( + foreign_hash, + 41, + 0, + )) + .to_json_bytes() + .unwrap(), + ) + .await + .unwrap(); + + let veto_result = execute_plan( + &heal_plan, &op, &root, "data", &mut state, "neo", None, None, + ) + .await + .unwrap(); + assert!(veto_result.errors.is_empty(), "{:?}", veto_result.errors); + assert_eq!( + veto_result.pushed, 0, + "a vetoed push must not be counted as pushed" + ); + + let vetoed = state.get(&local_path).unwrap().clone(); + assert_eq!( + vetoed.status, + crate::state::FileSyncStatus::Conflict, + "a vetoed push must leave a VISIBLE conflict (tcfs conflicts / D-Bus)" + ); + assert!( + vetoed.conflict.is_some(), + "a vetoed push must record ConflictInfo, not vanish" + ); + assert_eq!( + vetoed.blake3, live_hash, + "documented consequence: the veto moves the baseline to the live local hash" + ); + + // The baseline move disarms the self-heal predicate, so the next cycle + // settles into the ordinary recorded-conflict path rather than looping on + // a push that can never land. + let next_plan = reconcile(&op, &root, "data", &state, "neo", &blacklist, &config, None) + .await + .unwrap(); + assert!( + next_plan.actions.iter().all(|a| !matches!( + a, + ReconcileAction::Push { + rel_path: p, + reason: PushReason::LocalNewer, + .. + } if p == rel + )), + "after a veto the self-heal must be disarmed (fail closed), got {:?}", + next_plan.actions + ); + } + #[tokio::test] async fn missing_exact_index_never_authorizes_local_orphan_delete() { let op = memory_op(); @@ -9734,6 +10852,7 @@ mod tests { "data", "honey", &config, + true, ) .await; @@ -9771,6 +10890,7 @@ mod tests { "data", "honey", &config, + true, ) .await .unwrap(); @@ -9824,6 +10944,7 @@ mod tests { "data", "honey", &config, + true, ) .await .unwrap();