Skip to content

Commit c808226

Browse files
axpnetclaude
andcommitted
fix(crypt): fail closed instead of creating a phantom folder on upload into a plaintext folder (#390)
Option 1 (strict) for the #390 crypt-overlay phantom. With the overlay on, uploading into a folder created while the overlay was off (a plaintext on-wire name with no ciphertext preimage) silently created a SECOND encrypted-named folder via the #385 ensure_parent_dirs retry and misplaced the file there, leaving the folder the user was standing in empty. Backend guard: on the missing-parent retry path, phantom_plaintext_parent() probes whether the on-wire parent is a plaintext folder that already exists (vs a genuinely-new encrypted subtree). If so the upload is refused with a clear message instead of materializing the phantom; the #385 new-encrypted-tree case still creates and retries. Two regression tests via a strict-parent mock cover both directions. Frontend smart re-anchor: on arm, activateProviderCryptOverlay and the connect/reload effect re-anchor to a valid encrypted-view location (scope, or the remote root for a whole-remote overlay) ONLY when the current folder would be hidden under the overlay. A new provider_crypt_cwd_in_view command (backed by CryptOverlayProvider::cwd_in_encrypted_view) reports whether the current on-wire cwd decodes cleanly, so re-arming inside a valid encrypted subfolder keeps the user in place (toggling on/off to compare cifrato/decifrato no longer bounces to the root), while a plaintext folder or an out-of-scope spot re-anchors. Option 2 (encrypted files inside a plaintext-named folder) is the Overlays Remote Path scoping from #369 and is deferred to that work. Live-verified: the phantom refusal end to end via the CLI on the Filen dev vault (no phantom folder, folder stays empty), and the smart re-anchor in the GUI on MEGA rclone (stays in a valid encrypted subfolder across toggle, re-anchors out of a plaintext folder). Co-Authored-By: EhudKirsh <EhudKirsh@users.noreply.github.com> Co-Authored-By: aeroftp[bot] <aeroftp[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6228a34 commit c808226

4 files changed

Lines changed: 375 additions & 7 deletions

File tree

src-tauri/src/crypt_overlay_provider.rs

Lines changed: 323 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,81 @@ impl CryptOverlayProvider {
411411
.map_err(ProviderError::InvalidPath)
412412
}
413413

414+
/// #390 fail-closed guard. When a write into `plain_target` lands on an
415+
/// encrypted parent that is missing on the wire, distinguish two cases:
416+
///
417+
/// - #385 genuine: the encrypted parent chain simply does not exist yet (a
418+
/// fresh encrypted tree). The caller should create it (`ensure_parent_dirs`)
419+
/// and retry.
420+
/// - #390 phantom: a PLAINTEXT-named folder exists on the wire at that
421+
/// location because it was created while the overlay was off. It has no
422+
/// ciphertext preimage, so encrypting the path would spawn a divergent
423+
/// `enc(name)` folder ALONGSIDE the plaintext one and silently misplace the
424+
/// file (the folder the user is standing in stays empty).
425+
///
426+
/// Returns `Some(on-wire plaintext parent)` for the #390 case, where the write
427+
/// must fail closed instead of materializing the phantom; `None` when it is
428+
/// safe to create the encrypted parent (the #385 case) or the guard does not
429+
/// apply. Only reached on the missing-parent retry path, so it costs one
430+
/// `exists` probe on failure and nothing on the success path.
431+
async fn phantom_plaintext_parent(&mut self, plain_target: &str) -> Option<String> {
432+
// A relative target resolves against the current on-wire dir, which is not
433+
// known here without a round trip; the frontend cwd re-read on overlay-arm
434+
// covers that seam. Absolute targets are what the upload UI actually sends.
435+
if !plain_target.starts_with('/') {
436+
return None;
437+
}
438+
let t = norm_abs(plain_target);
439+
let parent = match t.rsplit_once('/') {
440+
Some((p, _)) if !p.is_empty() => p.to_string(),
441+
_ => return None, // leaf directly at the root: no below-anchor parent
442+
};
443+
// Only a parent strictly BELOW the anchor can phantom. At or above the
444+
// anchor the path passes through as cleartext and the anchor itself
445+
// legitimately exists, so an `exists` hit there is not a phantom.
446+
let strictly_below = if self.scope.is_empty() {
447+
true
448+
} else {
449+
parent.starts_with(&format!("{}/", self.scope))
450+
};
451+
if !strictly_below {
452+
return None;
453+
}
454+
match self.inner.exists(&parent).await {
455+
Ok(true) => Some(parent),
456+
_ => None,
457+
}
458+
}
459+
460+
/// #390 smart re-anchor support: is the wrapped provider's CURRENT on-wire
461+
/// cwd a valid location inside the encrypted view? True when the cwd is the
462+
/// anchor itself or a below-anchor path whose every component decodes as real
463+
/// ciphertext (so the overlay can render it decrypted in place); false when the
464+
/// cwd is outside the anchor, or a below-anchor path with a plaintext component
465+
/// that has no ciphertext preimage (a folder navigated to while the overlay was
466+
/// off, hidden once armed). Lets the UI keep the user where they are on arm when
467+
/// the folder is a genuine encrypted folder, and re-anchor to the scope/root
468+
/// only when the current folder would otherwise be hidden.
469+
pub async fn cwd_in_encrypted_view(&mut self) -> Result<bool, ProviderError> {
470+
let enc_cwd = norm_abs(&self.inner.pwd().await?);
471+
let below = if self.scope.is_empty() {
472+
enc_cwd.trim_start_matches('/').to_string()
473+
} else if enc_cwd == self.scope {
474+
return Ok(true); // the cleartext anchor root itself
475+
} else if let Some(b) = enc_cwd.strip_prefix(&format!("{}/", self.scope)) {
476+
b.to_string()
477+
} else {
478+
return Ok(false); // outside / above the anchor: not in the encrypted view
479+
};
480+
// Every below-anchor component must decode as valid ciphertext (directory
481+
// semantics). An empty tail (the anchor / remote root) is vacuously valid.
482+
let all_decode = below
483+
.split('/')
484+
.filter(|c| !c.is_empty() && *c != ".")
485+
.all(|c| self.keys.decode_name(c, true).is_some());
486+
Ok(all_decode)
487+
}
488+
414489
fn crypt_err(context: &str, e: String) -> ProviderError {
415490
ProviderError::TransferFailed(format!("{context}: {e}"))
416491
}
@@ -752,6 +827,23 @@ impl StorageProvider for CryptOverlayProvider {
752827
result,
753828
Err(ProviderError::NotFound(_)) | Err(ProviderError::InvalidPath(_))
754829
) {
830+
// #390 (option 1, strict): before creating the encrypted parent chain,
831+
// refuse if the on-wire parent is a plaintext-named folder created while
832+
// the overlay was off. Creating enc(name) there would spawn a divergent
833+
// phantom folder and silently misplace the file. Fail closed with a clear
834+
// message instead. A genuinely-missing encrypted parent (#385) still
835+
// creates and retries.
836+
if let Some(plain_parent) = self.phantom_plaintext_parent(remote_path).await {
837+
let _ = tokio::fs::remove_file(&temp_path).await;
838+
return Err(ProviderError::InvalidPath(format!(
839+
"the current folder {:?} is a plaintext folder created with the crypt \
840+
overlay off, so it is not part of the encrypted view. AeroFTP will not \
841+
create a second encrypted folder here and silently misplace the file. Turn \
842+
the overlay off to use this folder, or move into an encrypted folder before \
843+
uploading (see issue #390).",
844+
plain_parent
845+
)));
846+
}
755847
ensure_parent_dirs(&mut self.inner, &enc).await;
756848
result = self
757849
.inner
@@ -2325,4 +2417,235 @@ mod tests {
23252417
Some("real")
23262418
);
23272419
}
2420+
2421+
// ── #390 strict fail-closed guard ────────────────────────────────────────
2422+
2423+
/// A strict-parent provider (models WebDAV / OpenDrive): a PUT into a
2424+
/// collection whose parent directory does not exist fails `NotFound`, so the
2425+
/// decorator's #385 retry path (`ensure_parent_dirs`) is exercised. Directories
2426+
/// must be created explicitly with `mkdir`; `exists` sees both files and dirs.
2427+
struct StrictMemProvider {
2428+
files: Mutex<HashMap<String, Vec<u8>>>,
2429+
dirs: Mutex<Vec<String>>,
2430+
}
2431+
2432+
impl StrictMemProvider {
2433+
fn with_dirs(seed: &[&str]) -> Self {
2434+
Self {
2435+
files: Mutex::new(HashMap::new()),
2436+
dirs: Mutex::new(seed.iter().map(|s| s.to_string()).collect()),
2437+
}
2438+
}
2439+
fn parent_of(p: &str) -> String {
2440+
match p.trim_end_matches('/').rsplit_once('/') {
2441+
Some((parent, _)) if !parent.is_empty() => parent.to_string(),
2442+
_ => "/".to_string(),
2443+
}
2444+
}
2445+
fn dir_list(&self) -> Vec<String> {
2446+
self.dirs.lock().unwrap().clone()
2447+
}
2448+
fn file_paths(&self) -> Vec<String> {
2449+
self.files.lock().unwrap().keys().cloned().collect()
2450+
}
2451+
}
2452+
2453+
#[async_trait]
2454+
impl StorageProvider for StrictMemProvider {
2455+
fn as_any_mut(&mut self) -> &mut dyn std::any::Any {
2456+
self
2457+
}
2458+
fn provider_type(&self) -> ProviderType {
2459+
ProviderType::WebDav
2460+
}
2461+
fn display_name(&self) -> String {
2462+
"strict-mem".into()
2463+
}
2464+
async fn connect(&mut self) -> Result<(), ProviderError> {
2465+
Ok(())
2466+
}
2467+
async fn disconnect(&mut self) -> Result<(), ProviderError> {
2468+
Ok(())
2469+
}
2470+
fn is_connected(&self) -> bool {
2471+
true
2472+
}
2473+
async fn list(&mut self, _path: &str) -> Result<Vec<RemoteEntry>, ProviderError> {
2474+
Ok(Vec::new())
2475+
}
2476+
async fn pwd(&mut self) -> Result<String, ProviderError> {
2477+
Ok("/".into())
2478+
}
2479+
async fn cd(&mut self, _p: &str) -> Result<(), ProviderError> {
2480+
Ok(())
2481+
}
2482+
async fn cd_up(&mut self) -> Result<(), ProviderError> {
2483+
Ok(())
2484+
}
2485+
async fn download(
2486+
&mut self,
2487+
_remote: &str,
2488+
_local: &str,
2489+
_cb: Option<Box<dyn Fn(u64, u64) + Send>>,
2490+
) -> Result<(), ProviderError> {
2491+
Ok(())
2492+
}
2493+
async fn download_to_bytes(&mut self, _remote: &str) -> Result<Vec<u8>, ProviderError> {
2494+
Ok(Vec::new())
2495+
}
2496+
async fn upload(
2497+
&mut self,
2498+
local: &str,
2499+
remote: &str,
2500+
_cb: Option<Box<dyn Fn(u64, u64) + Send>>,
2501+
) -> Result<(), ProviderError> {
2502+
let parent = Self::parent_of(remote);
2503+
if parent != "/" && !self.dirs.lock().unwrap().iter().any(|d| d == &parent) {
2504+
// Strict provider: no implicit parent creation.
2505+
return Err(ProviderError::NotFound(parent));
2506+
}
2507+
let data = std::fs::read(local).map_err(ProviderError::IoError)?;
2508+
self.files.lock().unwrap().insert(remote.to_string(), data);
2509+
Ok(())
2510+
}
2511+
async fn mkdir(&mut self, p: &str) -> Result<(), ProviderError> {
2512+
let mut dirs = self.dirs.lock().unwrap();
2513+
if !dirs.iter().any(|d| d == p) {
2514+
dirs.push(p.to_string());
2515+
}
2516+
Ok(())
2517+
}
2518+
async fn delete(&mut self, p: &str) -> Result<(), ProviderError> {
2519+
self.files.lock().unwrap().remove(p);
2520+
Ok(())
2521+
}
2522+
async fn rmdir(&mut self, _p: &str) -> Result<(), ProviderError> {
2523+
Ok(())
2524+
}
2525+
async fn rmdir_recursive(&mut self, _p: &str) -> Result<(), ProviderError> {
2526+
Ok(())
2527+
}
2528+
async fn rename(&mut self, _from: &str, _to: &str) -> Result<(), ProviderError> {
2529+
Ok(())
2530+
}
2531+
async fn stat(&mut self, p: &str) -> Result<RemoteEntry, ProviderError> {
2532+
Err(ProviderError::NotFound(p.to_string()))
2533+
}
2534+
async fn size(&mut self, p: &str) -> Result<u64, ProviderError> {
2535+
Err(ProviderError::NotFound(p.to_string()))
2536+
}
2537+
async fn exists(&mut self, p: &str) -> Result<bool, ProviderError> {
2538+
Ok(self.files.lock().unwrap().contains_key(p)
2539+
|| self.dirs.lock().unwrap().iter().any(|d| d == p))
2540+
}
2541+
async fn keep_alive(&mut self) -> Result<(), ProviderError> {
2542+
Ok(())
2543+
}
2544+
async fn server_info(&mut self) -> Result<String, ProviderError> {
2545+
Ok("strict-mem".into())
2546+
}
2547+
}
2548+
2549+
async fn write_temp(payload: &[u8]) -> (std::path::PathBuf, String) {
2550+
let dir = std::env::temp_dir().join(format!("crypt390_{}", uuid::Uuid::new_v4()));
2551+
tokio::fs::create_dir_all(&dir).await.unwrap();
2552+
let local = dir.join("hello390.txt");
2553+
tokio::fs::write(&local, payload).await.unwrap();
2554+
let s = local.to_string_lossy().to_string();
2555+
(dir, s)
2556+
}
2557+
2558+
/// #390 option 1 (strict): uploading with the overlay on into a plaintext-named
2559+
/// folder that already exists on the wire (created while the overlay was off)
2560+
/// must FAIL CLOSED with a clear message, and must NOT materialize a phantom
2561+
/// `enc(name)` folder or store the file anywhere.
2562+
#[tokio::test]
2563+
async fn upload_into_plaintext_folder_fails_closed_no_phantom() {
2564+
// Anchor plus the plaintext folder the user created with the overlay off.
2565+
let inner = Box::new(StrictMemProvider::with_dirs(&[
2566+
"/AeroCryptTest",
2567+
"/AeroCryptTest/CryptPlain",
2568+
]));
2569+
let keys = rclone_keys(FilenameEncryption::Standard, true, ".bin");
2570+
let mut provider = CryptOverlayProvider::new(inner, keys, "/AeroCryptTest");
2571+
2572+
let (dir, local) = write_temp(b"secret payload for 390").await;
2573+
let err = provider
2574+
.upload(&local, "/AeroCryptTest/CryptPlain/hello390.txt", None)
2575+
.await
2576+
.expect_err("must refuse writing into a plaintext folder");
2577+
match &err {
2578+
ProviderError::InvalidPath(msg) => {
2579+
assert!(msg.contains("#390"), "message must reference #390: {msg}");
2580+
assert!(
2581+
msg.contains("plaintext folder"),
2582+
"message must explain the plaintext folder: {msg}"
2583+
);
2584+
}
2585+
other => panic!("expected InvalidPath refusal, got {other:?}"),
2586+
}
2587+
2588+
let mem = provider
2589+
.as_any_mut()
2590+
.downcast_mut::<CryptOverlayProvider>()
2591+
.unwrap()
2592+
.inner
2593+
.as_any_mut()
2594+
.downcast_mut::<StrictMemProvider>()
2595+
.unwrap();
2596+
assert!(
2597+
mem.file_paths().is_empty(),
2598+
"no file may be stored on refusal: {:?}",
2599+
mem.file_paths()
2600+
);
2601+
assert_eq!(
2602+
mem.dir_list(),
2603+
vec![
2604+
"/AeroCryptTest".to_string(),
2605+
"/AeroCryptTest/CryptPlain".to_string()
2606+
],
2607+
"no phantom encrypted folder may be created"
2608+
);
2609+
let _ = tokio::fs::remove_dir_all(&dir).await;
2610+
}
2611+
2612+
/// #385 must still work: uploading into a genuinely-new encrypted subtree (no
2613+
/// plaintext folder exists on the wire) creates the encrypted parent chain and
2614+
/// stores the file under an encrypted path, without any plaintext folder name
2615+
/// leaking onto the wire.
2616+
#[tokio::test]
2617+
async fn upload_into_new_encrypted_subtree_still_creates_and_stores() {
2618+
// Only the anchor exists; the target subfolder is brand new.
2619+
let inner = Box::new(StrictMemProvider::with_dirs(&["/AeroCryptTest"]));
2620+
let keys = rclone_keys(FilenameEncryption::Standard, true, ".bin");
2621+
let mut provider = CryptOverlayProvider::new(inner, keys, "/AeroCryptTest");
2622+
2623+
let (dir, local) = write_temp(b"payload for a fresh crypt folder").await;
2624+
provider
2625+
.upload(&local, "/AeroCryptTest/FreshFolder/hello390.txt", None)
2626+
.await
2627+
.expect("a genuinely new encrypted subtree must be created and stored");
2628+
2629+
let mem = provider
2630+
.as_any_mut()
2631+
.downcast_mut::<CryptOverlayProvider>()
2632+
.unwrap()
2633+
.inner
2634+
.as_any_mut()
2635+
.downcast_mut::<StrictMemProvider>()
2636+
.unwrap();
2637+
let files = mem.file_paths();
2638+
assert_eq!(files.len(), 1, "exactly one object stored: {files:?}");
2639+
assert!(
2640+
!files[0].contains("FreshFolder") && !files[0].contains("hello390"),
2641+
"stored path must be encrypted: {}",
2642+
files[0]
2643+
);
2644+
assert!(
2645+
mem.dir_list().iter().all(|d| !d.contains("FreshFolder")),
2646+
"no plaintext folder name may leak onto the wire: {:?}",
2647+
mem.dir_list()
2648+
);
2649+
let _ = tokio::fs::remove_dir_all(&dir).await;
2650+
}
23282651
}

src-tauri/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17303,6 +17303,7 @@ pub fn run() {
1730317303
provider_commands::provider_disconnect,
1730417304
provider_commands::provider_apply_crypt_overlay,
1730517305
provider_commands::provider_clear_crypt_overlay,
17306+
provider_commands::provider_crypt_cwd_in_view,
1730617307
provider_commands::provider_check_connection,
1730717308
provider_commands::provider_probe_alive,
1730817309
provider_commands::provider_list_files,

src-tauri/src/provider_commands.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,6 +1312,29 @@ pub async fn provider_clear_crypt_overlay(
13121312
Ok(removed)
13131313
}
13141314

1315+
/// #390 smart re-anchor probe. After arming the overlay, report whether the
1316+
/// current on-wire cwd is a valid location inside the encrypted view (so the UI
1317+
/// keeps the user in place) or a hidden/outside location (so the UI re-anchors to
1318+
/// the scope/root). Returns `true` when nothing is wrapped, so a raw session is
1319+
/// never yanked.
1320+
#[tauri::command]
1321+
pub async fn provider_crypt_cwd_in_view(state: State<'_, ProviderState>) -> Result<bool, String> {
1322+
let mut guard = state.provider.lock().await;
1323+
let Some(provider) = guard.as_mut() else {
1324+
return Ok(true);
1325+
};
1326+
match provider
1327+
.as_any_mut()
1328+
.downcast_mut::<crate::crypt_overlay_provider::CryptOverlayProvider>()
1329+
{
1330+
Some(overlay) => overlay
1331+
.cwd_in_encrypted_view()
1332+
.await
1333+
.map_err(|e| e.to_string()),
1334+
None => Ok(true),
1335+
}
1336+
}
1337+
13151338
/// Check if connected to a provider
13161339
#[tauri::command]
13171340
pub async fn provider_check_connection(

0 commit comments

Comments
 (0)