mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(core,fuse): refuse sync roots that overlap home, the mount or app data
Nothing stopped `/`, the home folder, the Proton Drive mount, the app's own state or cache, or a folder nested in another root from becoming a sync root, and a restore took a path from another machine's profile as it was. One validator, `syncroot::problem`, now runs where roots are added and restored, on paths resolved through symlinks with `..` refused (B234).
This commit is contained in:
1 parent
518763c5c9
commit
51800c77d7
5 files changed
+281
-16
No files matched your search
@@ -23,6 +23,7 @@ pub mod service;
|
||||
pub mod shell;
|
||||
pub mod similar;
|
||||
pub mod syncignore;
|
||||
pub mod syncroot;
|
||||
pub mod takeout;
|
||||
pub mod tray;
|
||||
|
||||
|
||||
@@ -0,0 +1,209 @@
|
||||
//! Which local folders may become a sync root.
|
||||
//!
|
||||
//! A mirror uploads everything under its root and deletes what Drive deleted,
|
||||
//! so a root that holds more than the user meant is dangerous. `$HOME` takes
|
||||
//! the decrypted content cache, `cache.db` and `~/.ssh` up to Drive, and walks
|
||||
//! into the mount, which downloads the whole Drive only to upload it again.
|
||||
//! Every place a root is added or restored goes through [`problem`].
|
||||
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
|
||||
use crate::config::{AppConfig, AppDirs};
|
||||
|
||||
/// The paths a sync root must stay clear of.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct Reserved {
|
||||
/// The user's home folder. A root may sit inside it, not be it or above it.
|
||||
pub home: Option<PathBuf>,
|
||||
/// The Proton Drive mount.
|
||||
pub mountpoint: Option<PathBuf>,
|
||||
/// This app's own configuration, state and cache folders.
|
||||
pub app_dirs: Vec<PathBuf>,
|
||||
}
|
||||
|
||||
impl Reserved {
|
||||
/// The reserved paths of this user and `config`, resolved like a root.
|
||||
pub fn current(config: &AppConfig) -> Self {
|
||||
let Ok(dirs) = AppDirs::new() else {
|
||||
return Self::default();
|
||||
};
|
||||
let resolved = |path: PathBuf| resolve(&path).unwrap_or(path);
|
||||
let mut app_dirs = vec![dirs.state_dir(), dirs.cache_dir()];
|
||||
if let Some(config_dir) = dirs.config_path().parent() {
|
||||
app_dirs.push(config_dir.to_path_buf());
|
||||
}
|
||||
Self {
|
||||
home: dirs.home_dir().map(resolved),
|
||||
mountpoint: Some(resolved(dirs.resolved_mountpoint(config))),
|
||||
app_dirs: app_dirs.into_iter().map(resolved).collect(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `path` as a root is compared: absolute, with symlinks resolved as far as the
|
||||
/// path exists. A folder a restore is about to create does not exist yet, so
|
||||
/// the part past the deepest existing folder is kept as written.
|
||||
///
|
||||
/// A `..` is refused rather than resolved: before a symlink it means something
|
||||
/// else than after it, and a path recorded on another machine has no business
|
||||
/// holding one.
|
||||
pub fn resolve(path: &Path) -> Result<PathBuf, String> {
|
||||
if !path.is_absolute() {
|
||||
return Err(format!("{} is not an absolute path", path.display()));
|
||||
}
|
||||
if path.components().any(|c| c == Component::ParentDir) {
|
||||
return Err(format!("{} goes up with `..`", path.display()));
|
||||
}
|
||||
let mut existing = path.to_path_buf();
|
||||
let mut rest = Vec::new();
|
||||
loop {
|
||||
match existing.canonicalize() {
|
||||
Ok(canonical) => {
|
||||
return Ok(rest
|
||||
.into_iter()
|
||||
.rev()
|
||||
.fold(canonical, |path: PathBuf, part| path.join(part)));
|
||||
}
|
||||
Err(_) => {
|
||||
let Some(name) = existing.file_name().map(|n| n.to_os_string()) else {
|
||||
return Ok(path.to_path_buf());
|
||||
};
|
||||
rest.push(name);
|
||||
if !existing.pop() {
|
||||
return Ok(path.to_path_buf());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Why `root` cannot be synced, or `None` when it can. `root` is resolved
|
||||
/// ([`resolve`]); `others` are the roots already synced.
|
||||
pub fn problem(root: &Path, others: &[PathBuf], reserved: &Reserved) -> Option<String> {
|
||||
let shown = root.display();
|
||||
if root.parent().is_none() {
|
||||
return Some(format!("{shown} is the root of the file system"));
|
||||
}
|
||||
if let Some(home) = &reserved.home {
|
||||
if root == home {
|
||||
return Some(format!(
|
||||
"{shown} is your home folder; pick a folder inside it"
|
||||
));
|
||||
}
|
||||
if home.starts_with(root) {
|
||||
return Some(format!("{shown} holds your home folder"));
|
||||
}
|
||||
}
|
||||
if let Some(mountpoint) = &reserved.mountpoint
|
||||
&& overlaps(root, mountpoint)
|
||||
{
|
||||
return Some(format!(
|
||||
"{shown} overlaps the Proton Drive folder {}",
|
||||
mountpoint.display()
|
||||
));
|
||||
}
|
||||
if let Some(dir) = reserved.app_dirs.iter().find(|dir| overlaps(root, dir)) {
|
||||
return Some(format!(
|
||||
"{shown} overlaps this app's own data in {}",
|
||||
dir.display()
|
||||
));
|
||||
}
|
||||
if others.iter().any(|other| other == root) {
|
||||
return Some(format!("{shown} is already synced"));
|
||||
}
|
||||
if let Some(other) = others.iter().find(|other| overlaps(root, other)) {
|
||||
return Some(format!(
|
||||
"{shown} overlaps the synced folder {}",
|
||||
other.display()
|
||||
));
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Whether one of `a` and `b` is the other or inside it.
|
||||
fn overlaps(a: &Path, b: &Path) -> bool {
|
||||
a.starts_with(b) || b.starts_with(a)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn reserved() -> Reserved {
|
||||
Reserved {
|
||||
home: Some("/home/u".into()),
|
||||
mountpoint: Some("/home/u/ProtonDrive".into()),
|
||||
app_dirs: vec![
|
||||
"/home/u/.local/state/proton-drive-linux".into(),
|
||||
"/home/u/.cache/proton-drive-linux".into(),
|
||||
"/home/u/.config/proton-drive-linux".into(),
|
||||
],
|
||||
}
|
||||
}
|
||||
|
||||
fn refused(root: &str, others: &[&str]) -> bool {
|
||||
let others: Vec<PathBuf> = others.iter().map(PathBuf::from).collect();
|
||||
problem(Path::new(root), &others, &reserved()).is_some()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_folder_inside_home_can_be_synced() {
|
||||
assert!(!refused("/home/u/Documents", &[]));
|
||||
assert!(!refused("/home/u/Pictures", &["/home/u/Documents"]));
|
||||
assert!(!refused("/mnt/data/photos", &[]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_file_system_root_home_and_its_parents_are_refused() {
|
||||
assert!(refused("/", &[]));
|
||||
assert!(refused("/home/u", &[]));
|
||||
assert!(refused("/home", &[]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_mount_and_the_apps_own_data_are_refused_either_way_round() {
|
||||
assert!(refused("/home/u/ProtonDrive", &[]));
|
||||
assert!(refused("/home/u/ProtonDrive/Documents", &[]));
|
||||
assert!(refused("/home/u/.cache", &[]));
|
||||
assert!(refused(
|
||||
"/home/u/.local/state/proton-drive-linux/staging",
|
||||
&[]
|
||||
));
|
||||
assert!(refused("/home/u/.config", &[]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_root_inside_or_around_another_root_is_refused() {
|
||||
assert!(refused("/home/u/Documents", &["/home/u/Documents"]));
|
||||
assert!(refused("/home/u/Documents/Work", &["/home/u/Documents"]));
|
||||
assert!(refused("/home/u/stuff", &["/home/u/stuff/Documents"]));
|
||||
// A shared prefix of characters is not a shared folder.
|
||||
assert!(!refused("/home/u/Documents2", &["/home/u/Documents"]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_path_that_goes_up_or_is_relative_does_not_resolve() {
|
||||
assert!(resolve(Path::new("/home/u/../../etc")).is_err());
|
||||
assert!(resolve(Path::new("Documents")).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_folder_still_to_be_created_resolves_under_its_existing_parent() {
|
||||
let base = std::env::temp_dir().join(format!("pdfs-syncroot-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&base).unwrap();
|
||||
let canonical = base.canonicalize().unwrap();
|
||||
assert_eq!(
|
||||
resolve(&base.join("new/inner")).unwrap(),
|
||||
canonical.join("new/inner")
|
||||
);
|
||||
let link = base.join("link");
|
||||
let _ = std::fs::remove_file(&link);
|
||||
std::os::unix::fs::symlink(&canonical, &link).unwrap();
|
||||
assert_eq!(
|
||||
resolve(&link.join("later")).unwrap(),
|
||||
canonical.join("later"),
|
||||
"a symlink is followed to where it points"
|
||||
);
|
||||
std::fs::remove_dir_all(&base).unwrap();
|
||||
}
|
||||
}
|
||||
@@ -105,6 +105,15 @@ pub(crate) fn adopted_device_uid() -> Option<String> {
|
||||
(!uid.is_empty()).then_some(uid)
|
||||
}
|
||||
|
||||
/// The paths no sync root may overlap, with the mountpoint as the config says
|
||||
/// now.
|
||||
pub(crate) fn sync_root_reserved() -> pdfs_core::syncroot::Reserved {
|
||||
let config = pdfs_core::config::AppDirs::new()
|
||||
.map(|dirs| dirs.load_config())
|
||||
.unwrap_or_default();
|
||||
pdfs_core::syncroot::Reserved::current(&config)
|
||||
}
|
||||
|
||||
/// Pin (or, with `None`, unpin) the adopted device uid in `config.json`.
|
||||
pub(crate) fn set_adopted_device_uid(uid: Option<&str>) -> CoreResult<()> {
|
||||
let dirs = pdfs_core::config::AppDirs::new()
|
||||
@@ -517,18 +526,20 @@ impl Core {
|
||||
.map_err(|e| CoreError::internal(format!("canonicalize {}: {e}", local.display())))?;
|
||||
let local_str = local.to_string_lossy().to_string();
|
||||
|
||||
// Reject duplicates up front for a clear error (UNIQUE would also catch it).
|
||||
if self
|
||||
// `/`, the home folder, the mount, this app's own data, and a folder
|
||||
// inside or around another root are refused (`syncroot::problem`). A
|
||||
// duplicate is one of those, refused here for a clear error before
|
||||
// the UNIQUE constraint would catch it.
|
||||
let others = self
|
||||
.db
|
||||
.sync_folder_list()
|
||||
.map_err(|e| CoreError::internal(format!("db: {e:?}")))?
|
||||
.iter()
|
||||
.any(|f| f.local_path == local_str)
|
||||
{
|
||||
return Err(CoreError::invalid(format!(
|
||||
"{} is already synced",
|
||||
local.display()
|
||||
)));
|
||||
.map(|f| PathBuf::from(&f.local_path))
|
||||
.collect::<Vec<_>>();
|
||||
let reserved = sync_root_reserved();
|
||||
if let Some(problem) = pdfs_core::syncroot::problem(&local, &others, &reserved) {
|
||||
return Err(CoreError::invalid(problem));
|
||||
}
|
||||
|
||||
let name = local
|
||||
|
||||
@@ -304,6 +304,9 @@ impl Core {
|
||||
// offering it as a restorable folder would sync the bookkeeping
|
||||
// back onto the machine it describes.
|
||||
.filter(|n| n.is_folder() && !n.trashed && n.name != PROFILE_DIR_NAME)
|
||||
// The name becomes a folder under the home folder: one with a `/`
|
||||
// or a `..` in it would put the restore somewhere else.
|
||||
.filter(|n| crate::remote_name_is_safe(&n.name))
|
||||
.map(|n| {
|
||||
let uid = n.uid.to_string();
|
||||
let saved = profile
|
||||
@@ -349,23 +352,29 @@ impl Core {
|
||||
.sync_folder_list()
|
||||
.map_err(|e| CoreError::internal(format!("db: {e:?}")))?;
|
||||
|
||||
let reserved = crate::devices::sync_root_reserved();
|
||||
let mut roots: Vec<PathBuf> = existing
|
||||
.iter()
|
||||
.map(|f| PathBuf::from(&f.local_path))
|
||||
.collect();
|
||||
let mut restored = 0usize;
|
||||
let mut skipped: Vec<String> = Vec::new();
|
||||
for item in items {
|
||||
let local = PathBuf::from(&item.local_path);
|
||||
if !local.is_absolute() {
|
||||
skipped.push(format!("{}: local path must be absolute", item.local_path));
|
||||
// The path is proposed from the profile another machine wrote, and
|
||||
// may hold a folder name from Drive: checked like any new root.
|
||||
let local = match pdfs_core::syncroot::resolve(Path::new(&item.local_path)) {
|
||||
Ok(local) => local,
|
||||
Err(e) => {
|
||||
skipped.push(format!("{}: {e}", item.local_path));
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if existing.iter().any(|f| f.remote_uid == item.remote_uid) {
|
||||
skipped.push(format!("{}: already synced", item.local_path));
|
||||
continue;
|
||||
}
|
||||
if existing
|
||||
.iter()
|
||||
.any(|f| Path::new(&f.local_path) == local.as_path())
|
||||
{
|
||||
skipped.push(format!("{}: path already in use", item.local_path));
|
||||
if let Some(problem) = pdfs_core::syncroot::problem(&local, &roots, &reserved) {
|
||||
skipped.push(format!("{}: {problem}", item.local_path));
|
||||
continue;
|
||||
}
|
||||
if let Err(e) = std::fs::create_dir_all(&local) {
|
||||
@@ -390,6 +399,7 @@ impl Core {
|
||||
let _ = self.request_sync_folder_mode(id, "ondemand");
|
||||
}
|
||||
let _ = self.sync_tx.send(sync::SyncMsg::Reconcile(id));
|
||||
roots.push(local);
|
||||
restored += 1;
|
||||
}
|
||||
let _ = self.sync_tx.send(sync::SyncMsg::Rewatch);
|
||||
|
||||
@@ -100,6 +100,7 @@ a live account since.
|
||||
|
||||
| Entry | Title |
|
||||
|---|---|
|
||||
| [B234](#b234--any-folder-can-become-a-sync-root-the-home-folder-included) | Any folder can become a sync root, the home folder included |
|
||||
| [B233](#b233--signing-out-leaves-the-session-valid-on-proton) | Signing out leaves the session valid on Proton |
|
||||
| [B232](#b232--names-with--or--show-as-empty-rows-or-are-drawn-as-markup) | Names with & or < show as empty rows or are drawn as markup |
|
||||
| [B231](#b231--a-large-upload-under-a-low-upload-limit-never-finishes) | A large upload under a low upload limit never finishes |
|
||||
@@ -336,6 +337,39 @@ a live account since.
|
||||
|
||||
Newest first.
|
||||
|
||||
### B234 — Any folder can become a sync root, the home folder included
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 1.12).
|
||||
**Where:** `crates/pdfs-core/src/syncroot.rs`; `Core::add_sync_folder` and `sync_root_reserved` in
|
||||
`crates/pdfs-fuse/src/devices.rs`; `Core::restore_sync_folders` and the restore proposals in
|
||||
`crates/pdfs-fuse/src/profile.rs`
|
||||
|
||||
**Repro:** `pdfs sync add ~`, or `pdfs sync add /`. Or restore a profile whose recorded path is
|
||||
`/home/u/../..`, or whose folder on Drive has a name with a `/`.
|
||||
|
||||
**Cause.** A new sync root was only checked for being a directory and not already synced. A
|
||||
restore checked only that the path was absolute. Nothing refused `/`, the home folder, a parent
|
||||
of either, the Proton Drive mount, the app's own state, cache and config folders, or a root
|
||||
inside or around another root. Adding the home folder uploads the decrypted content cache,
|
||||
`cache.db` and `~/.ssh`, and walks into the mount, which downloads the Drive only to upload it
|
||||
again. A restore proposal joined a folder name from Drive onto the home folder as it was.
|
||||
|
||||
**Fix.** One check, `syncroot::problem`, runs wherever a root is added or restored. It refuses
|
||||
the cases above. Paths are compared after `syncroot::resolve`, which follows symlinks as far as
|
||||
the path exists and refuses `..`. Restore proposals leave out folders whose name could not be a
|
||||
local file name.
|
||||
|
||||
The audit also suggested not crossing into other file systems during the local walk. That was
|
||||
left out on purpose: a btrfs subvolume inside a sync root has a device id of its own, and would
|
||||
silently stop syncing.
|
||||
|
||||
**Test:** `syncroot::tests` (six cases, among them
|
||||
`the_file_system_root_home_and_its_parents_are_refused` and
|
||||
`a_root_inside_or_around_another_root_is_refused`).
|
||||
|
||||
---
|
||||
|
||||
### B233 — Signing out leaves the session valid on Proton
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
|
||||
Reference in new issue
Block a user