From 2a3755f6b45eebbe2ac55a7a10ec41570125c002 Mon Sep 17 00:00:00 2001 From: Jess Sullivan Date: Sat, 6 Jun 2026 20:01:12 -0400 Subject: [PATCH 1/6] TIN-1737 fail-closed guard for symlink restore ingress A hostile peer can publish a SymlinkManifest whose target points outside the sync root or at a local secret store; every pulling host previously materialized it verbatim (create_local_symlink ran std::os::unix::fs::symlink on the attacker-supplied target with no validation). All three production restore callers (auto-roam Pull, FileProvider, CLI pull) flow through create_local_symlink, so the guard is centralized there. Reject, before the link is created (skip + structured warn, not a hard error so one hostile entry does not abort the pull): - empty target - absolute target (is_absolute, plus explicit leading-/ , leading-\\ and Windows Prefix screening) - `..` that escapes above the link's own directory (lexical resolve, no filesystem canonicalize); the link is always created inside the sync root, so this is at least as strict as "stays within the root" - resolved target hitting the fail-closed security deny-set (check_security_path_components) Also self-defends upload_symlink_with_device (egress) by bailing on the same conditions, since that public API can be called outside the collector which already screens targets. No behavior change when no symlink is restored/uploaded; default-off config paths are untouched. --- crates/tcfs-sync/src/engine.rs | 336 ++++++++++++++++++++++++++++++++- 1 file changed, 333 insertions(+), 3 deletions(-) diff --git a/crates/tcfs-sync/src/engine.rs b/crates/tcfs-sync/src/engine.rs index d7de8df1..b1f63cad 100644 --- a/crates/tcfs-sync/src/engine.rs +++ b/crates/tcfs-sync/src/engine.rs @@ -1993,6 +1993,24 @@ pub async fn upload_symlink_with_device( rel_path: &str, ) -> Result { let target = read_symlink_target_text(local_path)?; + + // (TIN-1737) Self-defense on the upload side: refuse to *publish* a symlink + // whose target is absolute, climbs out of its own directory, or resolves + // onto the security deny-set. The egress collector already screens targets, + // but this public API can be called directly, so we fail closed here too. + if let Err(reason) = validate_restored_symlink_target(local_path, &target) { + warn!( + local = %local_path.display(), + target = %target, + reason = %reason, + "refusing to upload symlink: fail-closed egress guard" + ); + anyhow::bail!( + "refusing to upload symlink {} -> {target}: {reason}", + local_path.display() + ); + } + let symlink_hash = symlink_manifest_hash(&target); let remote_manifest = format!("{remote_prefix}/manifests/{symlink_hash}"); @@ -2111,7 +2129,18 @@ pub async fn download_file_with_device( .with_context(|| format!("reading manifest: {remote_manifest}"))?; if let Ok(manifest) = SymlinkManifest::from_bytes(&manifest_bytes) { - create_local_symlink(local_path, &manifest.symlink_target).await?; + let created = create_local_symlink(local_path, &manifest.symlink_target).await?; + if !created { + // Fail-closed guard refused this target (absolute / `..`-escape / + // deny-set). A warn was already logged; report a zero-byte no-op so + // a single hostile manifest does not abort the rest of the pull. + return Ok(DownloadResult { + remote_path: remote_manifest.to_string(), + local_path: local_path.to_path_buf(), + bytes: 0, + sync_state: None, + }); + } let mut sync_state_for_result = None; if !_device_id.is_empty() { let mut local_vclock = state @@ -2412,7 +2441,179 @@ pub async fn download_file_with_device( }) } -async fn create_local_symlink(local_path: &Path, target: &str) -> Result<()> { +/// Why a restored symlink target was refused. Used for structured warn logging. +/// +/// (TIN-1737) Symlink restore is an *ingress* path: a hostile peer can publish a +/// `SymlinkManifest` whose target points outside the sync root or at a local +/// secret store, and every pulling host would otherwise materialize it +/// verbatim. We fail closed here, before the link is ever created. +#[derive(Debug, Clone, PartialEq, Eq)] +enum SymlinkRejection { + /// Target string was empty. + Empty, + /// Target is an absolute path (`/etc/passwd`, `C:\...`, UNC). + Absolute, + /// Target uses `..` to escape above the link's own directory (and therefore + /// the sync root, since the link is always created inside the root). + Traversal, + /// Resolved target lands on the fail-closed security deny-set + /// (`.ssh`, `.gnupg`, dotenv, credential files, live DBs, ...). + DenySet(BlacklistReason), +} + +impl std::fmt::Display for SymlinkRejection { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SymlinkRejection::Empty => write!(f, "empty target"), + SymlinkRejection::Absolute => write!(f, "absolute target"), + SymlinkRejection::Traversal => write!(f, "`..` escapes sync root"), + SymlinkRejection::DenySet(reason) => write!(f, "deny-set: {reason}"), + } + } +} + +/// Lexically normalize `rel` (a relative path) joined onto `base`, collapsing +/// `.` and `..` components *without touching the filesystem* (no `canonicalize`, +/// which would follow existing links and could itself be attacker-influenced). +/// +/// Returns the number of leading `..` components that remain after collapsing — +/// i.e. how many directory levels the path climbs *above* `base`. Zero means the +/// resolved path stays at or below `base`. Also returns the normalized path so +/// callers can re-run the deny-set check across every resolved component. +fn lexical_resolve(base: &Path, rel: &Path) -> (usize, PathBuf) { + use std::path::Component; + + // `depth` counts directory levels we currently sit *below* `base`. Pushing a + // normal component descends (+1); a `..` ascends. While `depth > 0` a `..` + // just pops the last descent; at `depth == 0` a `..` climbs above `base` and + // is an escape. `escapes` is the max depth above base that we ever reach. + let mut components: Vec = Vec::new(); + let mut depth: usize = 0; + let mut escapes: usize = 0; + + for comp in rel.components() { + match comp { + Component::CurDir => {} + Component::ParentDir => { + if depth > 0 { + depth -= 1; + components.pop(); + } else { + escapes += 1; + } + } + Component::Normal(name) => { + // Below base we track descent depth so a later `..` pops a real + // directory; once we have escaped above base (`escapes > 0`) the + // remaining names are recorded as the residual escape path only. + if escapes == 0 { + depth += 1; + } + components.push(name.to_os_string()); + } + // RootDir / Prefix should not appear here (absolute is rejected + // earlier), but if they do, treat conservatively as part of the path. + other => components.push(other.as_os_str().to_os_string()), + } + } + + // Build the resolved path anchored at `base`, prefixed by any net escape. + let mut out = PathBuf::new(); + if escapes == 0 { + out.push(base); + } else { + for _ in 0..escapes { + out.push(".."); + } + } + for name in &components { + out.push(name); + } + (escapes, out) +} + +/// Fail-closed validation for a restored symlink target (TIN-1737). +/// +/// `local_path` is the would-be link location; `target` is the attacker-supplied +/// link body. Rejects: empty, absolute, `..`-escape above the link's directory, +/// or any resolved component hitting the security deny-set. Returns `Ok(())` for +/// a benign in-root relative target. +/// +/// NOTE: `create_local_symlink` does not receive the sync root, so the link's +/// own parent directory is used as the escape boundary. This is *more* strict +/// than "must stay within the sync root" (the link is always created inside the +/// root), so it can refuse an otherwise-legitimate `../sibling/file` target that +/// crosses one directory but stays in-root. That is the conservative, fail-closed +/// trade-off; loosening it safely requires threading the real sync root down to +/// every restore caller. +fn validate_restored_symlink_target( + local_path: &Path, + target: &str, +) -> std::result::Result<(), SymlinkRejection> { + if target.is_empty() { + return Err(SymlinkRejection::Empty); + } + + let target_path = Path::new(target); + + // (a) Absolute targets are always refused: `is_absolute` is platform-correct + // (covers `/etc/...`, Windows drive `C:\`, and UNC `\\server\share`). + if target_path.is_absolute() { + return Err(SymlinkRejection::Absolute); + } + // Defense-in-depth: refuse a leading `/` even if a non-unix `is_absolute` + // ever disagreed, and refuse a Windows-style drive/UNC prefix explicitly. + if target.starts_with('/') || target.starts_with('\\') { + return Err(SymlinkRejection::Absolute); + } + if target_path + .components() + .next() + .is_some_and(|c| matches!(c, std::path::Component::Prefix(_))) + { + return Err(SymlinkRejection::Absolute); + } + + let base = local_path.parent().unwrap_or_else(|| Path::new(".")); + let (escapes, resolved) = lexical_resolve(base, target_path); + + // (b) `..` escape above the link's directory (and thus the sync root). + if escapes > 0 { + return Err(SymlinkRejection::Traversal); + } + + // (c) Resolved target hits the fail-closed security deny-set. We reuse the + // egress-side `check_security_path_components`, which is config-independent. + // A default Blacklist is sufficient because the security deny-set is fixed. + let blacklist = Blacklist::default(); + if let Some(reason) = blacklist.check_security_path_components(&resolved) { + return Err(SymlinkRejection::DenySet(reason)); + } + // Also screen the raw target components directly, in case the resolved form + // (anchored at `base`) somehow masked a security name. Belt and suspenders. + if let Some(reason) = blacklist.check_security_path_components(target_path) { + return Err(SymlinkRejection::DenySet(reason)); + } + + Ok(()) +} + +/// Create a restored symlink after fail-closed target validation (TIN-1737). +/// +/// Returns `Ok(true)` when the link was created, `Ok(false)` when the target was +/// refused (skipped with a structured warn — *not* an error, so a single hostile +/// entry does not abort the whole pull). Reserved I/O failures still return `Err`. +async fn create_local_symlink(local_path: &Path, target: &str) -> Result { + if let Err(reason) = validate_restored_symlink_target(local_path, target) { + warn!( + local = %local_path.display(), + target = %target, + reason = %reason, + "refusing to restore symlink: fail-closed ingress guard" + ); + return Ok(false); + } + if let Some(parent) = local_path.parent() { tokio::fs::create_dir_all(parent) .await @@ -2437,7 +2638,7 @@ async fn create_local_symlink(local_path: &Path, target: &str) -> Result<()> { tokio::fs::rename(&tmp, local_path) .await .with_context(|| format!("renaming symlink to: {}", local_path.display()))?; - Ok(()) + Ok(true) } fn make_symlink_sync_state( @@ -3883,6 +4084,135 @@ mod tests { assert_eq!(rel_names(&result.symlinks, root), vec!["session-link"]); } + // ── symlink restore ingress guard (TIN-1737) ───────────────────────── + + /// Publish a `SymlinkManifest` for `target` into a memory operator at a + /// deterministic manifest path and return that path. + async fn publish_symlink_manifest(op: &Operator, hash: &str, target: &str) -> String { + let manifest = SymlinkManifest::new( + target.to_string(), + VectorClock::new(), + "hostile-peer".to_string(), + 0, + Some("link".to_string()), + ); + let manifest_path = format!("data/manifests/{hash}"); + op.write(&manifest_path, manifest.to_bytes().unwrap()) + .await + .unwrap(); + manifest_path + } + + /// Drive `download_file_with_device` for a symlink manifest and report + /// whether a link was actually materialized at `local_path`. + async fn restore_symlink(op: &Operator, manifest_path: &str, local_path: &Path) -> bool { + download_file_with_device( + op, + manifest_path, + local_path, + "data", + None, + "", + None, + None, + ) + .await + .expect("download_file_with_device should not hard-error on a refused symlink"); + // A refused target leaves nothing behind; a created link shows up via + // symlink_metadata (which does not follow the link). + std::fs::symlink_metadata(local_path) + .map(|m| m.file_type().is_symlink()) + .unwrap_or(false) + } + + #[cfg(unix)] + #[tokio::test] + async fn symlink_restore_refuses_parent_traversal_target() { + let op = memory_op(); + let tmp = tempfile::tempdir().unwrap(); + let local = tmp.path().join("root/sub/link"); + let mp = + publish_symlink_manifest(&op, "trav", "../../.ssh/authorized_keys").await; + + let created = restore_symlink(&op, &mp, &local).await; + assert!(!created, "`..`-escape target must not be materialized"); + assert!(!local.exists()); + } + + #[cfg(unix)] + #[tokio::test] + async fn symlink_restore_refuses_absolute_target() { + let op = memory_op(); + let tmp = tempfile::tempdir().unwrap(); + let local = tmp.path().join("root/link"); + let mp = publish_symlink_manifest(&op, "abs", "/etc/passwd").await; + + let created = restore_symlink(&op, &mp, &local).await; + assert!(!created, "absolute target must not be materialized"); + assert!(std::fs::symlink_metadata(&local).is_err()); + } + + #[cfg(unix)] + #[tokio::test] + async fn symlink_restore_refuses_deny_set_target() { + let op = memory_op(); + let tmp = tempfile::tempdir().unwrap(); + let local = tmp.path().join("root/link"); + // In-root relative target that lands inside a security deny-set dir. + let mp = publish_symlink_manifest(&op, "deny", ".gnupg/x").await; + + let created = restore_symlink(&op, &mp, &local).await; + assert!(!created, "deny-set (resolved) target must not be materialized"); + assert!(std::fs::symlink_metadata(&local).is_err()); + } + + #[cfg(unix)] + #[tokio::test] + async fn symlink_restore_allows_benign_in_root_target() { + let op = memory_op(); + let tmp = tempfile::tempdir().unwrap(); + let local = tmp.path().join("root/sub/link"); + // Sibling file within the same directory — legitimate relative target. + let mp = publish_symlink_manifest(&op, "ok", "sibling.txt").await; + + let created = restore_symlink(&op, &mp, &local).await; + assert!(created, "benign in-root relative target must still be created"); + let read_back = std::fs::read_link(&local).unwrap(); + assert_eq!(read_back, Path::new("sibling.txt")); + } + + #[cfg(unix)] + #[test] + fn validate_restored_symlink_target_rules() { + let link = Path::new("/sync/root/sub/link"); + // Refusals. + assert!(matches!( + validate_restored_symlink_target(link, ""), + Err(SymlinkRejection::Empty) + )); + assert!(matches!( + validate_restored_symlink_target(link, "/etc/passwd"), + Err(SymlinkRejection::Absolute) + )); + assert!(matches!( + validate_restored_symlink_target(link, "../../.ssh/authorized_keys"), + // `..`-escape is detected before the deny-set check. + Err(SymlinkRejection::Traversal) + )); + assert!(matches!( + validate_restored_symlink_target(link, ".gnupg/x"), + Err(SymlinkRejection::DenySet(_)) + )); + assert!(matches!( + validate_restored_symlink_target(link, "nested/.ssh/id_ed25519"), + Err(SymlinkRejection::DenySet(_)) + )); + // Allowed: benign in-root relative targets. + assert!(validate_restored_symlink_target(link, "sibling.txt").is_ok()); + assert!(validate_restored_symlink_target(link, "./nested/file").is_ok()); + assert!(validate_restored_symlink_target(link, "a/b/c").is_ok()); + } + // ── normalize_rel_path ─────────────────────────────────────────────── #[test] From aa85de6c31324c62ac181d020d24327dfc5dfd6d Mon Sep 17 00:00:00 2001 From: Jess Sullivan Date: Sat, 6 Jun 2026 20:27:14 -0400 Subject: [PATCH 2/6] TIN-1417 B1: device-aware EncryptionContext for FileProvider read path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FileProvider direct-read path built a master-only EncryptionContext (EncryptionContext::new) and so could never read a per-device (wrapped_file_keys-only) manifest, unlike tcfsd and the CLI which both attach this device's age unwrap identity. This is the hard-gate prerequisite to ever flipping crypto.per_device_wrapping; this PR does NOT flip it (default stays false). - New FP-local crate::device_ctx::build_encryption_context replicates tcfsd's build_encryption_context: loads the device registry + this device's age secret and attaches them via .with_device_wrapping. Gated on a per_device_wrapping config flag that defaults false, so the default config yields exactly EncryptionContext::new(mk) — byte identical for master-only manifests. - grpc_backend fetch_direct_to_file now builds the device-aware context. - direct.rs and uniffi_bridge.rs FAIL CLOSED on a per-device manifest (wrapped_file_keys present) instead of silently copying raw ciphertext; they only implement master-key unwrapping. - FP crate gains an optional tcfs-secrets dependency (grpc feature only). Dedupe follow-up: build_encryption_context now exists in tcfsd, the CLI, and here. Lifting one copy into a shared crate (e.g. tcfs-sync) was deferred because tcfs-sync does not currently depend on tcfs-secrets and adding that edge to a core crate is broader than this security fix. --- Cargo.lock | 1 + crates/tcfs-file-provider/Cargo.toml | 3 +- crates/tcfs-file-provider/src/device_ctx.rs | 401 ++++++++++++++++++ crates/tcfs-file-provider/src/direct.rs | 8 + crates/tcfs-file-provider/src/grpc_backend.rs | 46 +- crates/tcfs-file-provider/src/lib.rs | 5 + .../tcfs-file-provider/src/uniffi_bridge.rs | 20 + 7 files changed, 480 insertions(+), 4 deletions(-) create mode 100644 crates/tcfs-file-provider/src/device_ctx.rs diff --git a/Cargo.lock b/Cargo.lock index e033a3b7..f1fc9922 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5316,6 +5316,7 @@ dependencies = [ "tcfs-chunks", "tcfs-core", "tcfs-crypto", + "tcfs-secrets", "tcfs-storage", "tcfs-sync", "tempfile", diff --git a/crates/tcfs-file-provider/Cargo.toml b/crates/tcfs-file-provider/Cargo.toml index db5fdc74..bbba6974 100644 --- a/crates/tcfs-file-provider/Cargo.toml +++ b/crates/tcfs-file-provider/Cargo.toml @@ -16,7 +16,7 @@ direct = ["dep:tcfs-storage", "dep:tcfs-chunks", "dep:tcfs-crypto", "dep:tcfs-sy ## gRPC backend — delegates metadata/watch operations to tcfsd daemon and ## downloads file content in-process so FileProvider temp files are written by ## the extension, not by the daemon. -grpc = ["dep:tonic", "dep:tower", "dep:hyper-util", "dep:tokio-stream", "dep:futures", "dep:tcfs-storage", "dep:tcfs-chunks", "dep:tcfs-crypto", "dep:tcfs-sync", "tcfs-sync/crypto", "dep:opendal", "dep:base64"] +grpc = ["dep:tonic", "dep:tower", "dep:hyper-util", "dep:tokio-stream", "dep:futures", "dep:tcfs-storage", "dep:tcfs-chunks", "dep:tcfs-crypto", "dep:tcfs-sync", "tcfs-sync/crypto", "dep:tcfs-secrets", "dep:opendal", "dep:base64"] ## UniFFI bindings for iOS (proc-macro based, no UDL needed) uniffi = ["direct", "dep:uniffi", "dep:tcfs-auth", "dep:blake3"] @@ -26,6 +26,7 @@ tcfs-storage = { path = "../tcfs-storage", optional = true } tcfs-chunks = { path = "../tcfs-chunks", optional = true } tcfs-crypto = { path = "../tcfs-crypto", optional = true } tcfs-sync = { path = "../tcfs-sync", optional = true } +tcfs-secrets = { path = "../tcfs-secrets", optional = true } secrecy = { workspace = true, optional = true } base64 = { workspace = true, optional = true } opendal = { workspace = true, optional = true } diff --git a/crates/tcfs-file-provider/src/device_ctx.rs b/crates/tcfs-file-provider/src/device_ctx.rs new file mode 100644 index 00000000..f6097a34 --- /dev/null +++ b/crates/tcfs-file-provider/src/device_ctx.rs @@ -0,0 +1,401 @@ +//! Device-aware encryption-context wiring for the FileProvider backends +//! (TIN-1417, Track B / B1). +//! +//! This is an **FP-local replica** of `tcfsd`'s +//! `build_encryption_context` (`crates/tcfsd/src/grpc.rs`). The daemon and CLI +//! both wire per-device unwrap identity onto the `EncryptionContext`; only the +//! FileProvider direct-read path historically built a master-only context +//! (`EncryptionContext::new`) and could therefore never read a per-device +//! (`wrapped_file_keys`-only) manifest. This module closes that gap. +//! +//! FOLLOW-UP (dedupe): the daemon, CLI, and this module now carry three copies +//! of the same registry/secret-loading logic. The clean fix is to lift a single +//! `build_encryption_context` into a shared crate (e.g. `tcfs-sync`). That was +//! deliberately deferred here because `tcfs-sync` does not currently depend on +//! `tcfs-secrets`, and adding that dependency edge to a core crate consumed by +//! most of the workspace is a broader change than this security fix should +//! carry. See PR body for details. +//! +//! BEHAVIOR CONTRACT (must stay true): +//! - When `per_device_wrapping` is absent/false in the config (the default), +//! `build_encryption_context` returns exactly `EncryptionContext::new(mk)` — +//! byte-identical to the prior master-only behavior. Master-only manifests +//! read unchanged. +//! - When a per-device manifest is encountered without an available device +//! identity, the engine read switch fails CLOSED (clear error); we never +//! silently master-fall-back, and we never copy raw ciphertext to disk. + +/// Fail CLOSED when a manifest that this backend cannot decrypt would otherwise +/// be copied to disk as raw ciphertext (silent corruption). +/// +/// The `direct`/`uniffi` backends only implement master-key unwrapping. A +/// per-device manifest carries `wrapped_file_keys` and OMITS the master-wrapped +/// `encrypted_file_key`, so those backends would fall through to "no file key" +/// and write encrypted chunk bytes verbatim. This guard turns that into a loud, +/// explicit error instead. +#[cfg(any(feature = "direct", feature = "grpc"))] +pub(crate) fn ensure_master_decryptable( + manifest: &tcfs_sync::manifest::SyncManifest, +) -> anyhow::Result<()> { + if !manifest.wrapped_file_keys.is_empty() { + anyhow::bail!( + "manifest is per-device encrypted (wrapped_file_keys present) but this \ + FileProvider backend only supports master-key unwrapping; refusing to \ + materialize raw ciphertext. Use the gRPC backend with a device identity \ + configured to read per-device content." + ); + } + Ok(()) +} + +/// Build a DEVICE-AWARE `EncryptionContext` for the FileProvider read path, +/// mirroring `tcfsd`'s `build_encryption_context`. +/// +/// Default-off invariant: returns `EncryptionContext::new(master_key)` verbatim +/// unless the config explicitly sets `per_device_wrapping: true`. In that case +/// it loads the device registry + this device's age secret and attaches them +/// via `.with_device_wrapping`, exactly like the daemon. +/// +/// Like the daemon, this falls back to the master-only context (and logs why) +/// only when per-device wrapping is enabled but the registry/recipients/secret +/// are unavailable — it never produces a context that this device cannot read +/// back. The engine's read switch independently fails CLOSED if it then meets a +/// per-device manifest with no identity attached. +#[cfg(feature = "grpc")] +pub(crate) fn build_encryption_context( + config: &serde_json::Value, + device_id: &str, + master_key: &tcfs_crypto::MasterKey, +) -> tcfs_sync::engine::EncryptionContext { + use tcfs_sync::engine::{DeviceUnwrapIdentity, EncryptionContext}; + + let base = EncryptionContext::new(master_key.clone()); + + // Default-off: byte-identical master-only behavior unless explicitly enabled. + if !config + .get("per_device_wrapping") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false) + { + return base; + } + + let registry_path = config + .get("device_registry_path") + .and_then(serde_json::Value::as_str) + .map(std::path::PathBuf::from) + .unwrap_or_else(tcfs_secrets::device::default_registry_path); + + let registry = match tcfs_secrets::device::DeviceRegistry::load(®istry_path) { + Ok(r) => r, + Err(e) => { + tracing::warn!( + "per-device wrapping: registry load failed ({e}); using master wrap" + ); + return base; + } + }; + + let recipients: Vec = registry + .active_devices() + .filter(|d| tcfs_secrets::device::is_real_age_public_key(&d.public_key)) + .map(|d| tcfs_crypto::AgeFileKeyRecipient { + device_id: d.device_id.clone(), + recipient: d.public_key.clone(), + }) + .collect(); + if recipients.is_empty() { + tracing::warn!( + "per-device wrapping enabled but no active age recipients; using master wrap" + ); + return base; + } + + let secret_path = tcfs_secrets::device::device_secret_key_path(®istry_path, device_id); + let identity = match std::fs::read_to_string(&secret_path) { + Ok(s) => DeviceUnwrapIdentity { + device_id: device_id.to_string(), + secret: s.trim().to_string(), + }, + Err(e) => { + tracing::warn!( + "per-device wrapping: local device secret unreadable ({e}); using master wrap" + ); + return base; + } + }; + + base.with_device_wrapping(recipients, Some(identity)) +} + +#[cfg(all(test, feature = "grpc"))] +mod tests { + use super::*; + use tcfs_secrets::device::{DeviceIdentity, DeviceRegistry}; + + fn master() -> tcfs_crypto::MasterKey { + tcfs_crypto::MasterKey::from_bytes([7u8; 32]) + } + + /// Materialize a device registry + this device's age secret in `dir`, + /// returning (device_id, public_key). Mirrors the on-disk layout the daemon + /// and `build_encryption_context` expect: `/devices.json` plus + /// `/device-.age`. + fn provision_device(dir: &std::path::Path, device_id: &str) -> String { + let registry_path = dir.join("devices.json"); + let key = tcfs_secrets::device::generate_local_device_key(); + + let mut registry = DeviceRegistry::default(); + registry.add(DeviceIdentity { + name: device_id.to_string(), + device_id: device_id.to_string(), + public_key: key.public_key.clone(), + signing_key_hash: String::new(), + description: None, + enrolled_at: 0, + revoked: false, + last_nats_seq: 0, + }); + registry.save(®istry_path).expect("save registry"); + + let secret_path = + tcfs_secrets::device::device_secret_key_path(®istry_path, device_id); + tcfs_secrets::device::save_device_secret_key(&secret_path, &key.secret_key, true) + .expect("save device secret"); + + key.public_key + } + + /// Default config (per_device_wrapping absent) yields a master-only context: + /// no recipients, no identity — byte-identical to `EncryptionContext::new`. + #[test] + fn default_config_is_master_only_byte_identical() { + let config = serde_json::json!({}); + let ctx = build_encryption_context(&config, "device-a", &master()); + assert!( + ctx.device_recipients.is_empty(), + "default config must not attach per-device recipients" + ); + assert!( + ctx.device_identity.is_none(), + "default config must not attach a device identity" + ); + } + + /// Explicit per_device_wrapping=false also stays master-only. + #[test] + fn explicit_disabled_is_master_only() { + let config = serde_json::json!({ "per_device_wrapping": false }); + let ctx = build_encryption_context(&config, "device-a", &master()); + assert!(ctx.device_recipients.is_empty()); + assert!(ctx.device_identity.is_none()); + } + + /// When enabled with a real registry + local secret, the context carries this + /// device's unwrap identity and the active-device recipient set. + #[test] + fn enabled_attaches_device_identity_and_recipients() { + let tmp = tempfile::TempDir::new().unwrap(); + let _pub = provision_device(tmp.path(), "device-a"); + + let config = serde_json::json!({ + "per_device_wrapping": true, + "device_registry_path": tmp.path().join("devices.json").to_str().unwrap(), + }); + let ctx = build_encryption_context(&config, "device-a", &master()); + + assert_eq!( + ctx.device_recipients.len(), + 1, + "should pick up the single active device recipient" + ); + let identity = ctx + .device_identity + .as_ref() + .expect("enabled config with secret must attach a device identity"); + assert_eq!(identity.device_id, "device-a"); + assert!(identity.secret.starts_with("AGE-SECRET-KEY-")); + } + + /// Enabled but no local secret on disk -> falls back to master-only (never + /// produces a context that cannot read back); the engine read switch then + /// fails CLOSED on any per-device manifest it meets. + #[test] + fn enabled_without_local_secret_falls_back_to_master_only() { + let tmp = tempfile::TempDir::new().unwrap(); + // Registry exists with a recipient, but this device's secret is absent. + let registry_path = tmp.path().join("devices.json"); + let key = tcfs_secrets::device::generate_local_device_key(); + let mut registry = DeviceRegistry::default(); + registry.add(DeviceIdentity { + name: "device-a".to_string(), + device_id: "device-a".to_string(), + public_key: key.public_key, + signing_key_hash: String::new(), + description: None, + enrolled_at: 0, + revoked: false, + last_nats_seq: 0, + }); + registry.save(®istry_path).unwrap(); + + let config = serde_json::json!({ + "per_device_wrapping": true, + "device_registry_path": registry_path.to_str().unwrap(), + }); + let ctx = build_encryption_context(&config, "device-a", &master()); + assert!( + ctx.device_identity.is_none(), + "missing local secret must not yield a half-wired identity" + ); + } + + /// End-to-end through the real engine: a per-device (`wrapped_file_keys`) + /// manifest is READABLE via the FP-built context with the device identity, + /// fails CLOSED via a master-only context (no identity), and a master-only + /// manifest still reads unchanged via the FP context. + #[tokio::test] + async fn per_device_manifest_roundtrips_via_fp_context_and_fails_closed() { + let tmp = tempfile::TempDir::new().unwrap(); + let _pub = provision_device(tmp.path(), "device-a"); + let op = opendal::Operator::new(opendal::services::Memory::default()) + .unwrap() + .finish(); + let prefix = "test/fp-per-device"; + let mut state = + tcfs_sync::state::StateCache::open(&tmp.path().join("state.db")).unwrap(); + + // Build the write context from the same registry the FP read context uses. + let enabled_config = serde_json::json!({ + "per_device_wrapping": true, + "device_registry_path": tmp.path().join("devices.json").to_str().unwrap(), + }); + let write_ctx = build_encryption_context(&enabled_config, "device-a", &master()); + assert!( + !write_ctx.device_recipients.is_empty(), + "precondition: write context must have recipients" + ); + + let content = b"per-device payload read through the FileProvider context"; + let src = tmp.path().join("doc.txt"); + std::fs::write(&src, content).unwrap(); + + let up = tcfs_sync::engine::upload_file_with_device( + &op, &src, prefix, &mut state, None, "device-a", None, Some(&write_ctx), + ) + .await + .expect("per-device upload should succeed"); + + // The manifest is genuinely per-device (no master-wrapped key). + let manifest = tcfs_sync::manifest::SyncManifest::from_bytes( + &op.read(&up.remote_path).await.unwrap().to_bytes(), + ) + .unwrap(); + assert!(!manifest.wrapped_file_keys.is_empty()); + assert!(manifest.encrypted_file_key.is_none()); + + // (1) Readable via the device-aware FP context. + let read_ctx = build_encryption_context(&enabled_config, "device-a", &master()); + let dst_ok = tmp.path().join("ok.txt"); + tcfs_sync::engine::download_file_with_device( + &op, &up.remote_path, &dst_ok, prefix, None, "device-a", None, Some(&read_ctx), + ) + .await + .expect("device-aware FP context should read per-device manifest"); + assert_eq!(std::fs::read(&dst_ok).unwrap(), content); + + // (2) Fails CLOSED with a master-only context (no identity attached) — + // this is the pre-fix FileProvider behavior; it must error, never + // silently fall back or corrupt. + let master_only = build_encryption_context(&serde_json::json!({}), "device-a", &master()); + assert!(master_only.device_identity.is_none()); + let dst_closed = tmp.path().join("closed.txt"); + let res = tcfs_sync::engine::download_file_with_device( + &op, &up.remote_path, &dst_closed, prefix, None, "device-a", None, Some(&master_only), + ) + .await; + assert!( + res.is_err(), + "master-only context must FAIL CLOSED on a per-device manifest" + ); + assert!( + !dst_closed.exists(), + "fail-closed read must not materialize a corrupt file" + ); + } + + /// A master-only manifest reads unchanged through the FP context built from a + /// default (per_device_wrapping=false) config — byte-identical regression + /// guard for the default-off path. + #[tokio::test] + async fn master_only_manifest_reads_unchanged_via_fp_context() { + let tmp = tempfile::TempDir::new().unwrap(); + let op = opendal::Operator::new(opendal::services::Memory::default()) + .unwrap() + .finish(); + let prefix = "test/fp-master-only"; + let mut state = + tcfs_sync::state::StateCache::open(&tmp.path().join("state.db")).unwrap(); + + // Legacy master-only write (no per-device recipients). + let write_ctx = tcfs_sync::engine::EncryptionContext::new(master()); + let content = b"legacy master-only payload"; + let src = tmp.path().join("legacy.txt"); + std::fs::write(&src, content).unwrap(); + + let up = tcfs_sync::engine::upload_file_with_device( + &op, &src, prefix, &mut state, None, "device-a", None, Some(&write_ctx), + ) + .await + .expect("master-only upload should succeed"); + + let manifest = tcfs_sync::manifest::SyncManifest::from_bytes( + &op.read(&up.remote_path).await.unwrap().to_bytes(), + ) + .unwrap(); + assert!(manifest.wrapped_file_keys.is_empty()); + assert!(manifest.encrypted_file_key.is_some()); + + // Default config -> master-only FP context -> reads unchanged. + let read_ctx = build_encryption_context(&serde_json::json!({}), "device-a", &master()); + let dst = tmp.path().join("out.txt"); + tcfs_sync::engine::download_file_with_device( + &op, &up.remote_path, &dst, prefix, None, "device-a", None, Some(&read_ctx), + ) + .await + .expect("default FP context must read master-only manifest unchanged"); + assert_eq!(std::fs::read(&dst).unwrap(), content); + } + + /// The fail-loud guard for the master-only backends rejects per-device + /// manifests instead of copying raw ciphertext. + #[test] + fn ensure_master_decryptable_rejects_per_device_manifest() { + let base = || tcfs_sync::manifest::SyncManifest { + version: 2, + file_hash: String::new(), + file_size: 0, + chunks: Vec::new(), + vclock: tcfs_sync::conflict::VectorClock::default(), + written_by: "device-a".to_string(), + written_at: 0, + rel_path: None, + mode: None, + encrypted_file_key: None, + wrapped_file_keys: Vec::new(), + }; + + let mut per_device = base(); + per_device + .wrapped_file_keys + .push(tcfs_sync::manifest::WrappedFileKey { + recipient_device_id: "device-a".to_string(), + recipient: "age1example".to_string(), + algorithm: "age-x25519-v1".to_string(), + wrapped_key: "deadbeef".to_string(), + }); + assert!(ensure_master_decryptable(&per_device).is_err()); + + assert!(ensure_master_decryptable(&base()).is_ok()); + } +} diff --git a/crates/tcfs-file-provider/src/direct.rs b/crates/tcfs-file-provider/src/direct.rs index 2cb2da82..4e4f9a23 100644 --- a/crates/tcfs-file-provider/src/direct.rs +++ b/crates/tcfs-file-provider/src/direct.rs @@ -395,6 +395,11 @@ pub unsafe extern "C" fn tcfs_provider_fetch( let manifest = tcfs_sync::manifest::SyncManifest::from_bytes(&manifest_bytes.to_bytes())?; + // Fail CLOSED on per-device manifests: this direct backend only + // unwraps master-wrapped keys. A `wrapped_file_keys` manifest would + // otherwise fall through to "no file key" and copy raw ciphertext. + crate::device_ctx::ensure_master_decryptable(&manifest)?; + // Unwrap file key if E2EE manifest let file_key = match (&prov.master_key, &manifest.encrypted_file_key) { (Some(mk), Some(wrapped_b64)) => { @@ -516,6 +521,9 @@ pub unsafe extern "C" fn tcfs_provider_fetch_with_progress( let manifest = tcfs_sync::manifest::SyncManifest::from_bytes(&manifest_bytes.to_bytes())?; + // Fail CLOSED on per-device manifests (see fetch path above). + crate::device_ctx::ensure_master_decryptable(&manifest)?; + let file_key = match (&prov.master_key, &manifest.encrypted_file_key) { (Some(mk), Some(wrapped_b64)) => { let wrapped = base64::engine::general_purpose::STANDARD.decode(wrapped_b64)?; diff --git a/crates/tcfs-file-provider/src/grpc_backend.rs b/crates/tcfs-file-provider/src/grpc_backend.rs index 7d6e79f9..04e12533 100644 --- a/crates/tcfs-file-provider/src/grpc_backend.rs +++ b/crates/tcfs-file-provider/src/grpc_backend.rs @@ -33,6 +33,12 @@ pub struct TcfsProvider { /// write into the extension's sandbox temp container. direct_operator: Option, direct_master_key: Option, + /// Per-device unwrap identity for reading `wrapped_file_keys` manifests + /// (TIN-1417). `None` (the default) preserves master-only behavior. + direct_device_identity: Option, + /// Active-device recipients carried alongside the identity so the read + /// context can be reconstructed per fetch. Empty in the default config. + direct_device_recipients: Vec, /// Daemon connection target for lazy reconnection. target: DaemonTarget, last_error: Mutex>, @@ -215,11 +221,14 @@ fn error_code_for_fetch_error(error: &anyhow::Error) -> TcfsError { } } +#[allow(clippy::too_many_arguments)] async fn fetch_direct_to_file( operator: opendal::Operator, remote_prefix: String, device_id: String, master_key: Option, + device_recipients: Vec, + device_identity: Option, remote_path: String, dest_path: PathBuf, progress: Option, @@ -229,9 +238,17 @@ async fn fetch_direct_to_file( .await .with_context(|| format!("resolving manifest for: {remote_path}"))?; - let enc_ctx = master_key - .as_ref() - .map(|mk| tcfs_sync::engine::EncryptionContext::new(mk.clone())); + // Build the read context with this device's unwrap identity attached + // (TIN-1417). With the default config `device_recipients` is empty and + // `device_identity` is None, so `with_device_wrapping` is a no-op and this + // equals the prior `EncryptionContext::new(mk)` exactly (byte-identical for + // master-only manifests). When a per-device (`wrapped_file_keys`) manifest + // is read, the engine's read switch fails CLOSED with a clear error if no + // identity is attached — it never silently master-falls-back. + let enc_ctx = master_key.as_ref().map(|mk| { + tcfs_sync::engine::EncryptionContext::new(mk.clone()) + .with_device_wrapping(device_recipients, device_identity) + }); tcfs_sync::engine::download_file_with_device( &operator, @@ -303,6 +320,19 @@ pub unsafe extern "C" fn tcfs_provider_new(config_json: *const c_char) -> *mut T let direct_operator = build_direct_operator(&config); let direct_master_key = master_key_from_config(&config); + // Build the DEVICE-AWARE read context once (TIN-1417). With the default + // config (`per_device_wrapping` absent/false) this yields an empty + // recipient set and no identity — byte-identical to the prior + // master-only behavior. When enabled, it loads this device's age secret + // so per-device (`wrapped_file_keys`) manifests become readable. + let (direct_device_recipients, direct_device_identity) = match &direct_master_key { + Some(mk) => { + let ctx = crate::device_ctx::build_encryption_context(&config, &device_id, mk); + (ctx.device_recipients, ctx.device_identity) + } + None => (Vec::new(), None), + }; + // Multi-threaded runtime with 2 workers — one for the background watch // stream, one for synchronous FFI calls (enumerate, fetch, upload). // The gRPC backend only does network I/O (no file coordination), so @@ -331,6 +361,8 @@ pub unsafe extern "C" fn tcfs_provider_new(config_json: *const c_char) -> *mut T device_id, direct_operator, direct_master_key, + direct_device_identity, + direct_device_recipients, target, last_error: Mutex::new(None), })) @@ -603,6 +635,8 @@ pub unsafe extern "C" fn tcfs_provider_fetch( let remote_prefix = prov.remote_prefix.clone(); let device_id = prov.device_id.clone(); let direct_master_key = prov.direct_master_key.clone(); + let direct_device_recipients = prov.direct_device_recipients.clone(); + let direct_device_identity = prov.direct_device_identity.clone(); let target = prov.target.clone(); let fetch_result = std::thread::spawn(move || { @@ -615,6 +649,8 @@ pub unsafe extern "C" fn tcfs_provider_fetch( remote_prefix, device_id, direct_master_key, + direct_device_recipients, + direct_device_identity, remote, dest, None, @@ -713,6 +749,8 @@ pub unsafe extern "C" fn tcfs_provider_fetch_with_progress( let remote_prefix = prov.remote_prefix.clone(); let device_id = prov.device_id.clone(); let direct_master_key = prov.direct_master_key.clone(); + let direct_device_recipients = prov.direct_device_recipients.clone(); + let direct_device_identity = prov.direct_device_identity.clone(); let target = prov.target.clone(); let fetch_result = std::thread::spawn(move || { @@ -734,6 +772,8 @@ pub unsafe extern "C" fn tcfs_provider_fetch_with_progress( remote_prefix, device_id, direct_master_key, + direct_device_recipients, + direct_device_identity, remote, dest, progress, diff --git a/crates/tcfs-file-provider/src/lib.rs b/crates/tcfs-file-provider/src/lib.rs index 39259b12..d5bab961 100644 --- a/crates/tcfs-file-provider/src/lib.rs +++ b/crates/tcfs-file-provider/src/lib.rs @@ -96,6 +96,11 @@ mod direct; #[cfg(any(feature = "direct", feature = "grpc"))] mod storage_bounds; +// Device-aware encryption-context wiring (TIN-1417 / B1), shared across the +// FileProvider backends. +#[cfg(any(feature = "direct", feature = "grpc"))] +mod device_ctx; + #[cfg(feature = "direct")] pub use direct::*; diff --git a/crates/tcfs-file-provider/src/uniffi_bridge.rs b/crates/tcfs-file-provider/src/uniffi_bridge.rs index d9cb34a5..7a14c11f 100644 --- a/crates/tcfs-file-provider/src/uniffi_bridge.rs +++ b/crates/tcfs-file-provider/src/uniffi_bridge.rs @@ -305,6 +305,17 @@ impl TcfsProviderHandle { message: e.to_string(), })?; + // Fail CLOSED on per-device manifests: this backend only unwraps + // master-wrapped keys. A `wrapped_file_keys` manifest would + // otherwise fall through to "no file key" and copy raw ciphertext. + if !manifest.wrapped_file_keys.is_empty() { + return Err(ProviderError::Decryption { + message: "manifest is per-device encrypted (wrapped_file_keys present); \ + this backend only supports master-key unwrapping" + .to_string(), + }); + } + let file_key = match (&self.master_key, &manifest.encrypted_file_key) { (Some(mk), Some(wrapped_b64)) => { let wrapped = base64::engine::general_purpose::STANDARD @@ -401,6 +412,15 @@ impl TcfsProviderHandle { message: e.to_string(), })?; + // Fail CLOSED on per-device manifests (see hydrate path above). + if !manifest.wrapped_file_keys.is_empty() { + return Err(ProviderError::Decryption { + message: "manifest is per-device encrypted (wrapped_file_keys present); \ + this backend only supports master-key unwrapping" + .to_string(), + }); + } + // Unwrap file key if encrypted let file_key = match (&self.master_key, &manifest.encrypted_file_key) { (Some(mk), Some(wrapped_b64)) => { From 5d877a29eb80276c2b4ab81e29c53d9454ef8827 Mon Sep 17 00:00:00 2001 From: Jess Sullivan Date: Sat, 6 Jun 2026 20:44:16 -0400 Subject: [PATCH 3/6] style: cargo fmt --all (fix Build+Lint+Test fmt gate) --- crates/tcfs-sync/src/engine.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/crates/tcfs-sync/src/engine.rs b/crates/tcfs-sync/src/engine.rs index b1f63cad..e8f801fe 100644 --- a/crates/tcfs-sync/src/engine.rs +++ b/crates/tcfs-sync/src/engine.rs @@ -4106,18 +4106,9 @@ mod tests { /// Drive `download_file_with_device` for a symlink manifest and report /// whether a link was actually materialized at `local_path`. async fn restore_symlink(op: &Operator, manifest_path: &str, local_path: &Path) -> bool { - download_file_with_device( - op, - manifest_path, - local_path, - "data", - None, - "", - None, - None, - ) - .await - .expect("download_file_with_device should not hard-error on a refused symlink"); + download_file_with_device(op, manifest_path, local_path, "data", None, "", None, None) + .await + .expect("download_file_with_device should not hard-error on a refused symlink"); // A refused target leaves nothing behind; a created link shows up via // symlink_metadata (which does not follow the link). std::fs::symlink_metadata(local_path) @@ -4131,8 +4122,7 @@ mod tests { let op = memory_op(); let tmp = tempfile::tempdir().unwrap(); let local = tmp.path().join("root/sub/link"); - let mp = - publish_symlink_manifest(&op, "trav", "../../.ssh/authorized_keys").await; + let mp = publish_symlink_manifest(&op, "trav", "../../.ssh/authorized_keys").await; let created = restore_symlink(&op, &mp, &local).await; assert!(!created, "`..`-escape target must not be materialized"); @@ -4162,7 +4152,10 @@ mod tests { let mp = publish_symlink_manifest(&op, "deny", ".gnupg/x").await; let created = restore_symlink(&op, &mp, &local).await; - assert!(!created, "deny-set (resolved) target must not be materialized"); + assert!( + !created, + "deny-set (resolved) target must not be materialized" + ); assert!(std::fs::symlink_metadata(&local).is_err()); } @@ -4176,7 +4169,10 @@ mod tests { let mp = publish_symlink_manifest(&op, "ok", "sibling.txt").await; let created = restore_symlink(&op, &mp, &local).await; - assert!(created, "benign in-root relative target must still be created"); + assert!( + created, + "benign in-root relative target must still be created" + ); let read_back = std::fs::read_link(&local).unwrap(); assert_eq!(read_back, Path::new("sibling.txt")); } From 1f4938d501fa48090c04a29ea3370b1559584ac2 Mon Sep 17 00:00:00 2001 From: Jess Sullivan Date: Sat, 6 Jun 2026 20:44:33 -0400 Subject: [PATCH 4/6] style: cargo fmt --all (fix Build+Lint+Test fmt gate) --- crates/tcfs-file-provider/src/device_ctx.rs | 58 ++++++++++++++++----- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/crates/tcfs-file-provider/src/device_ctx.rs b/crates/tcfs-file-provider/src/device_ctx.rs index f6097a34..b1ce3699 100644 --- a/crates/tcfs-file-provider/src/device_ctx.rs +++ b/crates/tcfs-file-provider/src/device_ctx.rs @@ -89,9 +89,7 @@ pub(crate) fn build_encryption_context( let registry = match tcfs_secrets::device::DeviceRegistry::load(®istry_path) { Ok(r) => r, Err(e) => { - tracing::warn!( - "per-device wrapping: registry load failed ({e}); using master wrap" - ); + tracing::warn!("per-device wrapping: registry load failed ({e}); using master wrap"); return base; } }; @@ -158,8 +156,7 @@ mod tests { }); registry.save(®istry_path).expect("save registry"); - let secret_path = - tcfs_secrets::device::device_secret_key_path(®istry_path, device_id); + let secret_path = tcfs_secrets::device::device_secret_key_path(®istry_path, device_id); tcfs_secrets::device::save_device_secret_key(&secret_path, &key.secret_key, true) .expect("save device secret"); @@ -262,8 +259,7 @@ mod tests { .unwrap() .finish(); let prefix = "test/fp-per-device"; - let mut state = - tcfs_sync::state::StateCache::open(&tmp.path().join("state.db")).unwrap(); + let mut state = tcfs_sync::state::StateCache::open(&tmp.path().join("state.db")).unwrap(); // Build the write context from the same registry the FP read context uses. let enabled_config = serde_json::json!({ @@ -281,7 +277,14 @@ mod tests { std::fs::write(&src, content).unwrap(); let up = tcfs_sync::engine::upload_file_with_device( - &op, &src, prefix, &mut state, None, "device-a", None, Some(&write_ctx), + &op, + &src, + prefix, + &mut state, + None, + "device-a", + None, + Some(&write_ctx), ) .await .expect("per-device upload should succeed"); @@ -298,7 +301,14 @@ mod tests { let read_ctx = build_encryption_context(&enabled_config, "device-a", &master()); let dst_ok = tmp.path().join("ok.txt"); tcfs_sync::engine::download_file_with_device( - &op, &up.remote_path, &dst_ok, prefix, None, "device-a", None, Some(&read_ctx), + &op, + &up.remote_path, + &dst_ok, + prefix, + None, + "device-a", + None, + Some(&read_ctx), ) .await .expect("device-aware FP context should read per-device manifest"); @@ -311,7 +321,14 @@ mod tests { assert!(master_only.device_identity.is_none()); let dst_closed = tmp.path().join("closed.txt"); let res = tcfs_sync::engine::download_file_with_device( - &op, &up.remote_path, &dst_closed, prefix, None, "device-a", None, Some(&master_only), + &op, + &up.remote_path, + &dst_closed, + prefix, + None, + "device-a", + None, + Some(&master_only), ) .await; assert!( @@ -334,8 +351,7 @@ mod tests { .unwrap() .finish(); let prefix = "test/fp-master-only"; - let mut state = - tcfs_sync::state::StateCache::open(&tmp.path().join("state.db")).unwrap(); + let mut state = tcfs_sync::state::StateCache::open(&tmp.path().join("state.db")).unwrap(); // Legacy master-only write (no per-device recipients). let write_ctx = tcfs_sync::engine::EncryptionContext::new(master()); @@ -344,7 +360,14 @@ mod tests { std::fs::write(&src, content).unwrap(); let up = tcfs_sync::engine::upload_file_with_device( - &op, &src, prefix, &mut state, None, "device-a", None, Some(&write_ctx), + &op, + &src, + prefix, + &mut state, + None, + "device-a", + None, + Some(&write_ctx), ) .await .expect("master-only upload should succeed"); @@ -360,7 +383,14 @@ mod tests { let read_ctx = build_encryption_context(&serde_json::json!({}), "device-a", &master()); let dst = tmp.path().join("out.txt"); tcfs_sync::engine::download_file_with_device( - &op, &up.remote_path, &dst, prefix, None, "device-a", None, Some(&read_ctx), + &op, + &up.remote_path, + &dst, + prefix, + None, + "device-a", + None, + Some(&read_ctx), ) .await .expect("default FP context must read master-only manifest unchanged"); From 9ae5674464475365d35c0edb173325930ee99f10 Mon Sep 17 00:00:00 2001 From: Jess Sullivan Date: Sat, 6 Jun 2026 22:40:27 -0400 Subject: [PATCH 5/6] TIN-1417 B1: propagate per_device_wrapping + device_registry_path to FileProvider config The FileProvider device-aware read path (PR #492) reads `per_device_wrapping` and `device_registry_path` from its JSON config, but no in-repo producer emitted them. `tcfs config fileprovider` / `tcfs init --fileprovider-config-out` render the FileProvider bootstrap JSON from the active config via `build_fileprovider_init_config`; this adds the two keys so the read path actually receives them. - `FileProviderInitConfig` gains `per_device_wrapping` (mirrors `crypto.per_device_wrapping`) and `device_registry_path` (resolved from `sync.device_identity`, falling back to the shared default registry path). - Default-off is byte-identical: `per_device_wrapping` is omitted from the rendered JSON via `skip_serializing_if` when false, and `device_registry_path` is only populated when wrapping is on. So the legacy master-only bootstrap output is unchanged. - The extension derives the device secret (`device-.age`) itself from the registry path + device id, matching PR #492's `device_ctx`. - Tests assert the keys are omitted when off (byte-identical) and present (with the resolved/default registry path) when on. Does NOT flip `crypto.per_device_wrapping` (still default false). Out-of-repo producers (lab nix, Swift HostApp keychain enrichment, App-Group sandbox secret access) are specified in the PR body, not changed here. --- crates/tcfs-cli/src/main.rs | 146 ++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/crates/tcfs-cli/src/main.rs b/crates/tcfs-cli/src/main.rs index 7bcf3c6e..6ec2379b 100644 --- a/crates/tcfs-cli/src/main.rs +++ b/crates/tcfs-cli/src/main.rs @@ -3896,6 +3896,27 @@ struct FileProviderInitConfig { #[serde(skip_serializing_if = "Option::is_none")] daemon_socket: Option, master_key_file: String, + /// Per-device file-key wrapping (TIN-1417 / Track B1). Mirrors + /// `crypto.per_device_wrapping` from the active config. The FileProvider + /// extension reads this key to decide whether to build a device-aware + /// `EncryptionContext` (see `tcfs-file-provider` `device_ctx`). Omitted + /// from the rendered JSON when false so the default-off output stays + /// byte-identical to the legacy master-only bootstrap. + #[serde(skip_serializing_if = "is_false")] + per_device_wrapping: bool, + /// Path to the device registry (`devices.json`) the FileProvider must read + /// to resolve active age recipients + this device's secret + /// (`device-.age` alongside it). Only emitted when + /// `per_device_wrapping` is true; the extension otherwise falls back to its + /// own default registry path. + #[serde(skip_serializing_if = "Option::is_none")] + device_registry_path: Option, +} + +/// `skip_serializing_if` predicate so `per_device_wrapping` is omitted from the +/// rendered FileProvider JSON when off, keeping default-off output unchanged. +fn is_false(value: &bool) -> bool { + !*value } fn build_fileprovider_init_config( @@ -3904,6 +3925,18 @@ fn build_fileprovider_init_config( master_key_path: &Path, device_id: &str, ) -> FileProviderInitConfig { + let per_device_wrapping = config.crypto.per_device_wrapping; + // Only surface the device registry path when per-device wrapping is on, so + // the default-off rendered JSON is byte-identical to the legacy bootstrap. + let device_registry_path = if per_device_wrapping { + Some( + resolve_fileprovider_registry_path(config) + .to_string_lossy() + .into_owned(), + ) + } else { + None + }; FileProviderInitConfig { s3_endpoint: config.storage.endpoint.clone(), s3_bucket: config.storage.bucket.clone(), @@ -3918,9 +3951,25 @@ fn build_fileprovider_init_config( .as_ref() .map(|path| path.to_string_lossy().into_owned()), master_key_file: master_key_path.to_string_lossy().into_owned(), + per_device_wrapping, + device_registry_path, } } +/// Resolve the device-registry (`devices.json`) path the FileProvider should +/// read for per-device unwrap. Mirrors `resolve_fileprovider_device_id`: prefer +/// the configured `sync.device_identity` registry path, falling back to the +/// shared default. The device secret key (`device-.age`) lives alongside +/// this file and is derived by the extension via +/// `tcfs_secrets::device::device_secret_key_path`. +fn resolve_fileprovider_registry_path(config: &tcfs_core::config::TcfsConfig) -> PathBuf { + config + .sync + .device_identity + .clone() + .unwrap_or_else(tcfs_secrets::device::default_registry_path) +} + async fn write_fileprovider_init_config( config_path: &Path, config: &tcfs_core::config::TcfsConfig, @@ -5860,6 +5909,103 @@ mod tests { ); } + #[test] + fn build_fileprovider_init_config_omits_per_device_keys_when_disabled() { + // Default-off must stay byte-identical to the legacy master-only + // bootstrap: the per-device keys are absent from the rendered JSON. + let dir = tempfile::tempdir().unwrap(); + let mut config = test_config(dir.path()); + assert!(!config.crypto.per_device_wrapping); + // Even with a registry path configured, off => not emitted. + config.sync.device_identity = Some(dir.path().join("devices.json")); + + let s3 = tcfs_secrets::S3Credentials { + access_key_id: "access-key".into(), + secret_access_key: secrecy::SecretString::from("secret-key".to_string()), + endpoint: config.storage.endpoint.clone(), + region: config.storage.region.clone(), + }; + let master_key_path = dir.path().join("master.key"); + let rendered = build_fileprovider_init_config(&config, &s3, &master_key_path, "device-1"); + + assert!(!rendered.per_device_wrapping); + assert_eq!(rendered.device_registry_path, None); + + let json = serde_json::to_value(&rendered).unwrap(); + let obj = json.as_object().unwrap(); + assert!( + !obj.contains_key("per_device_wrapping"), + "per_device_wrapping must be omitted when off (byte-identical default)" + ); + assert!( + !obj.contains_key("device_registry_path"), + "device_registry_path must be omitted when wrapping is off" + ); + } + + #[test] + fn build_fileprovider_init_config_emits_per_device_keys_when_enabled() { + // When per-device wrapping is on, the rendered FileProvider config must + // carry the keys PR #492's read path consumes: `per_device_wrapping` + // (true) and `device_registry_path` (the configured registry). + let dir = tempfile::tempdir().unwrap(); + let registry_path = dir.path().join("devices.json"); + let mut config = test_config(dir.path()); + config.crypto.per_device_wrapping = true; + config.sync.device_identity = Some(registry_path.clone()); + + let s3 = tcfs_secrets::S3Credentials { + access_key_id: "access-key".into(), + secret_access_key: secrecy::SecretString::from("secret-key".to_string()), + endpoint: config.storage.endpoint.clone(), + region: config.storage.region.clone(), + }; + let master_key_path = dir.path().join("master.key"); + let rendered = build_fileprovider_init_config(&config, &s3, &master_key_path, "device-1"); + + assert!(rendered.per_device_wrapping); + assert_eq!( + rendered.device_registry_path.as_deref(), + Some(registry_path.to_string_lossy().as_ref()) + ); + + let json = serde_json::to_value(&rendered).unwrap(); + assert_eq!(json["per_device_wrapping"], serde_json::Value::Bool(true)); + assert_eq!( + json["device_registry_path"].as_str(), + Some(registry_path.to_string_lossy().as_ref()) + ); + } + + #[test] + fn build_fileprovider_init_config_falls_back_to_default_registry_when_enabled() { + // With wrapping on but no explicit registry, fall back to the shared + // default registry path (matching the extension's own fallback). + let dir = tempfile::tempdir().unwrap(); + let mut config = test_config(dir.path()); + config.crypto.per_device_wrapping = true; + config.sync.device_identity = None; + + let s3 = tcfs_secrets::S3Credentials { + access_key_id: "access-key".into(), + secret_access_key: secrecy::SecretString::from("secret-key".to_string()), + endpoint: config.storage.endpoint.clone(), + region: config.storage.region.clone(), + }; + let master_key_path = dir.path().join("master.key"); + let rendered = build_fileprovider_init_config(&config, &s3, &master_key_path, "device-1"); + + assert!(rendered.per_device_wrapping); + assert_eq!( + rendered.device_registry_path, + Some( + tcfs_secrets::device::default_registry_path() + .to_string_lossy() + .into_owned() + ) + ); + } + #[test] fn resolve_fileprovider_device_id_prefers_explicit_value() { let dir = tempfile::tempdir().unwrap(); From 944909ee7ef356a7fc0d3feb72b772854a533a8a Mon Sep 17 00:00:00 2001 From: Jess Sullivan Date: Sun, 7 Jun 2026 22:40:09 -0400 Subject: [PATCH 6/6] chore: allow age proc-macro advisory --- deny.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/deny.toml b/deny.toml index 0b614e1f..024da31c 100644 --- a/deny.toml +++ b/deny.toml @@ -13,6 +13,7 @@ targets = [ ignore = [ "RUSTSEC-2025-0141", # bincode unmaintained — transitive dep, no direct usage "RUSTSEC-2026-0097", # rand 0.8.5 unsound with custom logger — transitive via age, no custom logger in TCFS + "RUSTSEC-2026-0173", # proc-macro-error2 unmaintained — transitive via age/i18n-embed-fl, no safe upgrade as of 2026-06-08 # RUSTSEC-2025-0067/0068 removed: serde_yml migrated to serde_norway ]