Prevent mis-configuration where 'recycle_bin_path' is inside 'sync_dir' (#3552)

This update adds validation to ensure the configured 'recycle_bin_path' is not located within the 'sync_dir'.
If the recycle bin is a child of the sync directory, any file moved into the recycle bin during online delete processing would be detected as a new local file and uploaded back to Microsoft OneDrive, creating a loop of re-uploads.

The client now:
* Expands and normalises the 'recycle_bin_path' (including tilde handling)
* Verifies that the resolved path is outside the configured 'sync_dir'
* Fails fast with a clear error message if the configuration is unsafe
* Prevents data churn, unexpected uploads, and confusing behaviour for users

This ensures correct, predictable behaviour when `use_recycle_bin = true` and avoids accidental mis-configuration.
This commit is contained in:
abraunegg 2025-11-29 10:33:47 +11:00 committed by GitHub
commit 44f68ca683
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 110 additions and 47 deletions

View file

@ -242,9 +242,13 @@ class ApplicationConfig {
// Recycle Bin Configuration
// These paths are used by the application, if 'use_recycle_bin' is enabled
string recycleBinParentPath;
string recycleBinFilePath;
string recycleBinInfoPath;
// Runtime 'sync_dir' as initialised
string runtimeSyncDirectory;
// Initialise the application configuration
bool initialise(string confdirOption, bool helpRequested) {
@ -496,6 +500,7 @@ class ApplicationConfig {
// ├── files/ # The actual trashed files
// └── info/ # .trashinfo metadata about each file (original path, deletion date)
setValueString("recycle_bin_path", defaultHomePath ~ "/.local/share/Trash/");
recycleBinParentPath = getValueString("recycle_bin_path");
// DEVELOPER OPTIONS
// display_memory = true | false
@ -2565,9 +2570,7 @@ class ApplicationConfig {
// Initialise the correct 'sync_dir' expanding any '~' if present
string initialiseRuntimeSyncDirectory() {
string runtimeSyncDirectory;
// Log what we are doing
if (debugLogging) {addLogEntry("sync_dir: Setting runtimeSyncDirectory from config value 'sync_dir'", ["debug"]);}
if (!shellEnvironmentSet){
@ -2829,17 +2832,61 @@ class ApplicationConfig {
// Set the Recycle Bin Paths
void setRecycleBinPaths() {
// Get the configured base path
string basePath = getValueString("recycle_bin_path");
string configured = getValueString("recycle_bin_path");
string basePath;
string dirSeparatorString = "/";
// Ensure basePath ends with a single '/'
if (!basePath.endsWith("/")) {
basePath ~= "/";
// Handle the "no shell / no user" case similarly to sync_dir
if (!shellEnvironmentSet) {
// No SHELL or USER means expandTilde() will fail if '~' is present
if (canFind(configured, "~")) {
// Replace '~' with defaultHomePath explicitly
basePath = buildNormalizedPath(
buildPath(defaultHomePath, strip(configured, "~"))
);
} else {
basePath = configured;
}
} else {
// Normal case: shell + user are set; we can rely on expandTilde()
if (canFind(configured, "~")) {
basePath = expandTilde(configured);
} else {
basePath = configured;
}
}
// Append subdirectories based on the recycle bin path
recycleBinFilePath = basePath ~ "files/";
recycleBinInfoPath = basePath ~ "info/";
// Make sure it's normalised and has a trailing '/'
basePath = buildNormalizedPath(basePath);
if (!basePath.endsWith(dirSeparatorString)) {
basePath ~= dirSeparatorString;
}
// Update Recycle Bin paths
recycleBinParentPath = basePath;
recycleBinFilePath = basePath ~ "files" ~ dirSeparatorString;
recycleBinInfoPath = basePath ~ "info" ~ dirSeparatorString;
}
// Is 'recycleBinParentPath' a child path of the configured 'runtimeSyncDirectory'?
bool checkRecycleBinPathAsChildOfSyncDir() {
// Configure the variables to check
string syncRoot = runtimeSyncDirectory;
string recycleBin = recycleBinParentPath;
string sep = "/";
// Make prefix check robust ensure syncRoot ends with separator
if (!syncRoot.endsWith(sep)) {
syncRoot ~= sep;
}
// Make prefix check robust ensure recycleBin ends with separator
if (!recycleBin.endsWith(sep)) {
recycleBin ~= sep;
}
// Perform the check and return the evaluation
return startsWith(recycleBin, syncRoot);
}
// Is the client running under a GUI session?

View file

@ -202,6 +202,9 @@ int main(string[] cliArgs) {
// If --debug-https has been used, set the applicable flag
debugHTTPSResponse = appConfig.getValueBool("debug_https"); // set __gshared bool debugHTTPSResponse in log.d now that we have read-in any CLI arguments
// Read in the configured 'sync_dir' from appConfig with '~' if present correctly expanded based on the user environment
runtimeSyncDirectory = appConfig.initialiseRuntimeSyncDirectory();
// Are we doing a --sync or a --monitor operation? Both of these will be false if they are not set
if ((!appConfig.getValueBool("synchronize")) && (!appConfig.getValueBool("monitor"))) {
syncOrMonitorMissing = true; // --sync or --monitor is missing
@ -235,41 +238,57 @@ int main(string[] cliArgs) {
// Configure the internal application paths which will be used to move rather than delete any online deletes to
appConfig.setRecycleBinPaths();
// We need to ensure that the Recycle Bin Paths exist on the file system, and if they do not exist, create them
// Test for appConfig.recycleBinFilePath
if (!exists(appConfig.recycleBinFilePath)) {
try {
// Attempt to create the 'Recycle Bin' file path we have been configured with
mkdirRecurse(appConfig.recycleBinFilePath);
// Configure the applicable permissions for the folder
if (debugLogging) {addLogEntry("Setting directory permissions for: " ~ appConfig.recycleBinFilePath, ["debug"]);}
appConfig.recycleBinFilePath.setAttributes(octal!700); // Set to 0700 as Trash may contain sensitive and is the expected default permissions by GIO or KIO
} catch (std.file.FileException e) {
// Creating the 'Recycle Bin' file path failed
addLogEntry("ERROR: Unable to create the configured local 'Recycle Bin' file directory: " ~ e.msg, ["info", "notify"]);
// Use exit scopes to shutdown API
// If we are not using --display-config, test if the Recycle Bin Paths exist on the file system
if (!appConfig.getValueBool("display_config")) {
// We need to test that the configured 'Recycle Bin' path is not within the configured 'sync_dir'
if (appConfig.checkRecycleBinPathAsChildOfSyncDir) {
// ERROR: 'Recycle Bin' path is a child of the configured 'sync_dir'
addLogEntry();
addLogEntry("ERROR: The configured 'recycle_bin_path' (" ~ appConfig.recycleBinParentPath ~ ") is located within the configured 'sync_dir' (" ~ appConfig.runtimeSyncDirectory ~ ").", ["info", "notify"]);
addLogEntry(" This would cause locally recycled items to be re-uploaded to Microsoft OneDrive.");
addLogEntry(" Please set 'recycle_bin_path' to a location outside of 'sync_dir' and restart the client.");
addLogEntry();
return EXIT_FAILURE;
}
}
// Test for appConfig.recycleBinInfoPath
if (!exists(appConfig.recycleBinInfoPath)) {
try {
// Attempt to create the 'Recycle Bin' info path we have been configured with
mkdirRecurse(appConfig.recycleBinInfoPath);
} else {
// 'Recycle Bin' path is not within the configured 'sync_dir'
// We need to ensure that the Recycle Bin Paths exist on the file system, and if they do not exist, create them
// Test for appConfig.recycleBinFilePath
if (!exists(appConfig.recycleBinFilePath)) {
try {
// Attempt to create the 'Recycle Bin' file path we have been configured with
mkdirRecurse(appConfig.recycleBinFilePath);
// Configure the applicable permissions for the folder
if (debugLogging) {addLogEntry("Setting directory permissions for: " ~ appConfig.recycleBinFilePath, ["debug"]);}
appConfig.recycleBinFilePath.setAttributes(octal!700); // Set to 0700 as Trash may contain sensitive and is the expected default permissions by GIO or KIO
} catch (std.file.FileException e) {
// Creating the 'Recycle Bin' file path failed
addLogEntry("ERROR: Unable to create the configured local 'Recycle Bin' file directory: " ~ e.msg, ["info", "notify"]);
// Use exit scopes to shutdown API
return EXIT_FAILURE;
}
}
// Configure the applicable permissions for the folder
if (debugLogging) {addLogEntry("Setting directory permissions for: " ~ appConfig.recycleBinInfoPath, ["debug"]);}
appConfig.recycleBinInfoPath.setAttributes(octal!700); // Set to 0700 as Trash may contain sensitive and is the expected default permissions by GIO or KIO
} catch (std.file.FileException e) {
// Creating the 'Recycle Bin' info path failed
addLogEntry("ERROR: Unable to create the configured local 'Recycle Bin' info directory: " ~ e.msg, ["info", "notify"]);
// Use exit scopes to shutdown API
return EXIT_FAILURE;
// Test for appConfig.recycleBinInfoPath
if (!exists(appConfig.recycleBinInfoPath)) {
try {
// Attempt to create the 'Recycle Bin' info path we have been configured with
mkdirRecurse(appConfig.recycleBinInfoPath);
// Configure the applicable permissions for the folder
if (debugLogging) {addLogEntry("Setting directory permissions for: " ~ appConfig.recycleBinInfoPath, ["debug"]);}
appConfig.recycleBinInfoPath.setAttributes(octal!700); // Set to 0700 as Trash may contain sensitive and is the expected default permissions by GIO or KIO
} catch (std.file.FileException e) {
// Creating the 'Recycle Bin' info path failed
addLogEntry("ERROR: Unable to create the configured local 'Recycle Bin' info directory: " ~ e.msg, ["info", "notify"]);
// Use exit scopes to shutdown API
return EXIT_FAILURE;
}
}
}
}
}
@ -395,9 +414,6 @@ int main(string[] cliArgs) {
// Set runtimeDatabaseFile, this will get updated if we are using --dry-run
runtimeDatabaseFile = appConfig.databaseFilePath;
// Read in 'sync_dir' from appConfig with '~' if present expanded
runtimeSyncDirectory = appConfig.initialiseRuntimeSyncDirectory();
// DEVELOPER OPTIONS OUTPUT
// Set to display memory details as early as possible
displayMemoryUsage = appConfig.getValueBool("display_memory");