Skip to content

Prevent use of $HOME as sync root #683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TheOneRing wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Prevent use of $HOME as sync root #683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
57fbc0f
Prevent use of `$HOME` as sync root
TheOneRing Nov 14, 2025
92e0925
Print values in test
TheOneRing Nov 17, 2025
28702c9
Mention the existing sync connection
TheOneRing Nov 17, 2025
ef0d08e
Fix unit test
TheOneRing Nov 17, 2025
728b61a
Fix canonicalPath for drives (c:)
TheOneRing Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions src/gui/folderman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,8 @@ QString FolderMan::trayTooltipStatusString(
// parent directories.
static QString canonicalPath(const QString &path)
{
QFileInfo selFile(path);
// QFile::canonicalFilePath for C: returns the current working dir 🤷‍♀️
QFileInfo selFile(Utility::ensureTrailingSlash(path));
if (!selFile.exists()) {
const auto parentPath = selFile.dir().path();

Expand All @@ -557,7 +558,7 @@ static QString canonicalPath(const QString &path)
return path;
}

return canonicalPath(parentPath) + QLatin1Char('/') + selFile.fileName();
return canonicalPath(parentPath) + '/'_L1 + selFile.fileName();
}
return selFile.canonicalFilePath();
}
Expand Down Expand Up @@ -596,7 +597,7 @@ static QString checkPathForSyncRootMarkingRecursive(const QString &path, FolderM
QString FolderMan::checkPathValidityRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
{
if (path.isEmpty()) {
return FolderMan::tr("No valid folder selected!");
return tr("No valid folder selected!");
}

#ifdef Q_OS_WIN
Expand All @@ -614,21 +615,21 @@ QString FolderMan::checkPathValidityRecursive(const QString &path, FolderMan::Ne
if (parentPath != path) {
return checkPathValidityRecursive(parentPath, folderType, accountUuid);
}
return FolderMan::tr("The selected path does not exist!");
return tr("The selected path does not exist!");
}

if (numberOfSyncJournals(selectedPathInfo.filePath()) != 0) {
return FolderMan::tr("The folder »%1« is used in a folder sync connection!").arg(QDir::toNativeSeparators(selectedPathInfo.filePath()));
return tr("The folder »%1« is used in a folder sync connection!").arg(QDir::toNativeSeparators(selectedPathInfo.filePath()));
}

// At this point we know there is no syncdb in the parent hyrarchy, check for spaces sync root.

if (!selectedPathInfo.isDir()) {
return FolderMan::tr("The selected path is not a folder!");
return tr("The selected path is not a folder!");
}

if (!selectedPathInfo.isWritable()) {
return FolderMan::tr("You have no permission to write to the selected folder!");
return tr("You have no permission to write to the selected folder!");
}

return checkPathForSyncRootMarkingRecursive(path, folderType, accountUuid);
Expand All @@ -644,27 +645,26 @@ QString FolderMan::checkPathValidityRecursive(const QString &path, FolderMan::Ne
*/
QString FolderMan::checkPathValidityForNewFolder(const QString &path, NewFolderType folderType, const QUuid &accountUuid) const
{
// check if the local directory isn't used yet in another sync
const auto cs = Utility::fsCaseSensitivity();
if (FileSystem::isChildPathOf2(path, QDir::homePath()).testAnyFlag(FileSystem::ChildResult::IsEqual)) {
return tr("The home directory is not allowed to be used as a sync folder!");
}

const QString userDir = QDir::cleanPath(canonicalPath(path)) + QLatin1Char('/');
// check if the local directory isn't used yet in another sync
const QString userDir = QDir::cleanPath(canonicalPath(path));
for (auto f : _folders) {
const QString folderDir = QDir::cleanPath(canonicalPath(f->path())) + QLatin1Char('/');
const QString folderDir = QDir::cleanPath(canonicalPath(f->path()));

if (QString::compare(folderDir, userDir, cs) == 0) {
return tr("There is already a sync from the server to this local folder. "
"Please pick another local folder!");
if (FileSystem::isChildPathOf2(folderDir, userDir).testAnyFlag(FileSystem::ChildResult::IsEqual)) {
return tr("There is already a sync from the server to this local folder. Please pick another local folder!");
}
if (FileSystem::isChildPathOf(folderDir, userDir)) {
return tr("The local folder »%1« already contains a folder used in a folder sync connection. "
"Please pick another local folder!")
.arg(QDir::toNativeSeparators(path));
if (FileSystem::isChildPathOf2(folderDir, userDir).testAnyFlags(FileSystem::ChildResult::IsChild | FileSystem::ChildResult::IsEqual)) {
return tr("The local folder »%1« already contains a folder used in a folder sync connection »%2«. Please pick another local folder!")
.arg(QDir::toNativeSeparators(path), QDir::toNativeSeparators(folderDir));
}

if (FileSystem::isChildPathOf(userDir, folderDir)) {
return tr("The local folder »%1« is already contained in a folder used in a folder sync connection. "
"Please pick another local folder!")
.arg(QDir::toNativeSeparators(path));
if (FileSystem::isChildPathOf2(userDir, folderDir).testAnyFlags(FileSystem::ChildResult::IsChild | FileSystem::ChildResult::IsEqual)) {
return tr("The local folder »%1« is already contained in a folder used in a folder sync connection »%2«. Please pick another local folder!")
.arg(QDir::toNativeSeparators(path), QDir::toNativeSeparators(folderDir));
}
}

Expand Down
122 changes: 66 additions & 56 deletions test/testfolderman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
#include "common/utility_win.h"
#endif


using namespace Qt::Literals::StringLiterals;
using namespace OCC;


Expand Down Expand Up @@ -52,13 +54,24 @@ private Q_SLOTS:
auto newAccountState = TestUtils::createDummyAccount();
FolderMan *folderman = TestUtils::folderMan();
QCOMPARE(folderman, FolderMan::instance());

const auto type = FolderMan::NewFolderType::SpacesFolder;
const QUuid uuid = {};

if (Utility::isWindows()) { // drive-letter tests
if (QFileInfo(QStringLiteral("c:/")).isWritable()) {
// we expect success
QCOMPARE(folderman->checkPathValidityForNewFolder(QStringLiteral("c:"), type, uuid), QString());
QCOMPARE(folderman->checkPathValidityForNewFolder(QStringLiteral("c:/"), type, uuid), QString());
QCOMPARE(folderman->checkPathValidityForNewFolder(QStringLiteral("c:/foo"), type, uuid), QString());
}
}

QVERIFY(folderman->addFolder(
newAccountState.get(), TestUtils::createDummyFolderDefinition(newAccountState->account(), dirPath + QStringLiteral("/sub/OpenCloud1"))));
QVERIFY(folderman->addFolder(
newAccountState.get(), TestUtils::createDummyFolderDefinition(newAccountState->account(), dirPath + QStringLiteral("/OpenCloud2"))));

const auto type = FolderMan::NewFolderType::SpacesFolder;
const QUuid uuid = {};

// those should be allowed
// QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory)
Expand All @@ -71,15 +84,15 @@ private Q_SLOTS:
// QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/bliblablu/some/more")), QString());

// A file -> Error
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/file.txt"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/file.txt"), type, uuid), QString());

// The following both fail because they refer to the same account
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1"), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/"), type, uuid), QString());

QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath, type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/folder"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/folder/f"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath, type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/folder"), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/folder/f"), type, uuid), QString());

#ifndef Q_OS_WIN // no links on windows, no permissions
// make a bunch of links
Expand All @@ -89,75 +102,75 @@ private Q_SLOTS:
QVERIFY(QFile::link(dirPath + QStringLiteral("/sub/OpenCloud1/folder"), dirPath + QStringLiteral("/link4")));

// Ok
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link1"), type, uuid).isNull());
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link2/free"), type, uuid).isNull());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link1"), type, uuid), QString());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link2/free"), type, uuid), QString());

// Not Ok
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link2"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link2"), type, uuid), QString());

// link 3 points to an existing sync folder. To make it fail, the account must be the same
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link3"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link3"), type, uuid), QString());

QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link4"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link3/folder"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link4"), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link3/folder"), type, uuid), QString());

// test some non existing sub path (error)
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/some/sub/path"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/blublu"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/folder/g/h"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link3/folder/neu_folder"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/some/sub/path"), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/blublu"), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/folder/g/h"), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link3/folder/neu_folder"), type, uuid), QString());

// Subfolder of links
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link1/subfolder"), type, uuid).isNull());
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link2/free/subfolder"), type, uuid).isNull());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link1/subfolder"), type, uuid), QString());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link2/free/subfolder"), type, uuid), QString());

if (getuid() != 0) {
// Should not have the rights
QVERIFY(!folderman->checkPathValidityForNewFolder(QStringLiteral("/"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(QStringLiteral("/usr/bin/somefolder"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(QStringLiteral("/"), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(QStringLiteral("/usr/bin/somefolder"), type, uuid), QString());
}
#endif

if (Utility::isWindows()) { // drive-letter tests
if (!QFileInfo(QStringLiteral("v:/")).exists()) {
QVERIFY(!folderman->checkPathValidityForNewFolder(QStringLiteral("v:"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(QStringLiteral("v:/"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(QStringLiteral("v:/foo"), type, uuid).isNull());
}
if (QFileInfo(QStringLiteral("c:/")).isWritable()) {
QVERIFY(folderman->checkPathValidityForNewFolder(QStringLiteral("c:"), type, uuid).isNull());
QVERIFY(folderman->checkPathValidityForNewFolder(QStringLiteral("c:/"), type, uuid).isNull());
QVERIFY(folderman->checkPathValidityForNewFolder(QStringLiteral("c:/foo"), type, uuid).isNull());
}
}

// Invalid paths
QVERIFY(!folderman->checkPathValidityForNewFolder({}, type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder({}, type, uuid), QString());


// REMOVE OpenCloud2 from the filesystem, but keep a folder sync'ed to it.
QDir(dirPath + QStringLiteral("/OpenCloud2/")).removeRecursively();
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/blublu"), type, uuid).isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/sub/subsub/sub"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/blublu"), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/sub/subsub/sub"), type, uuid), QString());

{ // check for rejection of a directory with `.sync_*.db`
QVERIFY(dir2.mkpath(QStringLiteral("db-check1")));
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/db-check1"), type, uuid).isNull());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/db-check1"), type, uuid), QString());
QFile f(dirPath + QStringLiteral("/db-check1/.sync_something.db"));
QVERIFY(f.open(QFile::Truncate | QFile::WriteOnly));
f.close();
QVERIFY(QFileInfo::exists(dirPath + QStringLiteral("/db-check1/.sync_something.db")));
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/db-check1"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/db-check1"), type, uuid), QString());
}

{ // check for rejection of a directory with `._sync_*.db`
QVERIFY(dir2.mkpath(QStringLiteral("db-check2")));
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/db-check2"), type, uuid).isNull());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/db-check2"), type, uuid), QString());
QFile f(dirPath + QStringLiteral("/db-check2/._sync_something.db"));
QVERIFY(f.open(QFile::Truncate | QFile::WriteOnly));
f.close();
QVERIFY(QFileInfo::exists(dirPath + QStringLiteral("/db-check2/._sync_something.db")));
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/db-check2"), type, uuid).isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/db-check2"), type, uuid), QString());
}


if (Utility::isWindows()) { // drive-letter tests
const auto driveLetter = QFileInfo(dirPath).absoluteDir().absolutePath().at(0);
const auto drive = u"%1:/"_s.arg(driveLetter);
if (QFileInfo(drive).isWritable()) {
// fails as we already sync dirPath + QStringLiteral("/sub/OpenCloud1")
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(u"%1:"_s.arg(driveLetter), type, uuid), QString());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(u"%1:/"_s.arg(driveLetter), type, uuid), QString());
// succeeds as the sub dir foo does not contain OpenCloud1
QCOMPARE(folderman->checkPathValidityForNewFolder(u"%1:/foo"_s.arg(driveLetter), type, uuid), QString());
}
}
}

Expand Down Expand Up @@ -235,28 +248,25 @@ private Q_SLOTS:
const auto ourUuid = QUuid::createUuid();

// Spaces Sync Root in another Spaces Sync Root should fail
QVERIFY(!folderman
->checkPathValidityForNewFolder(
dirPath + QStringLiteral("/AnotherSpacesSyncRoot/OurSpacesSyncRoot"), FolderMan::NewFolderType::SpacesSyncRoot, ourUuid)
.isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(
dirPath + QStringLiteral("/AnotherSpacesSyncRoot/OurSpacesSyncRoot"), FolderMan::NewFolderType::SpacesSyncRoot, ourUuid),
QString());
// Spaces Sync Root one level up should be fine
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OurSpacesSyncRoot"), FolderMan::NewFolderType::SpacesSyncRoot, ourUuid)
.isNull());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OurSpacesSyncRoot"), FolderMan::NewFolderType::SpacesSyncRoot, ourUuid),
QString());

// Create the sync root so we can test Spaces Folder creation below
QVERIFY(dir2.mkpath(QStringLiteral("OurSpacesSyncRoot")));
Utility::markDirectoryAsSyncRoot(dir2.filePath(QStringLiteral("OurSpacesSyncRoot")), ourUuid);

// A folder for a Space in a sync root for another account should fail
QVERIFY(!folderman
->checkPathValidityForNewFolder(
dirPath + QStringLiteral("/AnotherSpacesSyncRoot/OurSpacesFolder"), FolderMan::NewFolderType::SpacesFolder, ourUuid)
.isNull());
QCOMPARE_NE(folderman->checkPathValidityForNewFolder(
dirPath + QStringLiteral("/AnotherSpacesSyncRoot/OurSpacesFolder"), FolderMan::NewFolderType::SpacesFolder, ourUuid),
QString());
// But in our sync root that should just be fine
QVERIFY(
folderman
->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OurSpacesSyncRoot/OurSpacesFolder"), FolderMan::NewFolderType::SpacesFolder, ourUuid)
.isNull());
QCOMPARE(folderman->checkPathValidityForNewFolder(
dirPath + QStringLiteral("/OurSpacesSyncRoot/OurSpacesFolder"), FolderMan::NewFolderType::SpacesFolder, ourUuid),
QString());
}
};

Expand Down
Loading