wallet: Fix detection of symlinks on Windows

achow101 commented at 0:34 am on February 17, 2026: member

This PR implements symlink detection for Windows, since GCC doesn’t support symlinks on Windows. This is used by the wallet’s ListDatabases in order to detect symlinks and avoid recursing them. It’s also used in wallet path validation since that is also checking whether paths are symlinks, and we should properly detect symlinks on Windows.

Under the hood, the IsSymlink function uses std::filesystem::is_symlink for non-Windows systems. On Windows, it checks a file’s attributes to determine if it is a reparse point. Strictly speaking, this will also catch files that aren’t symlinks, such as directories that are Onedrive mounts. I think it’s fine to not be recursively searching such directories.

This is based on #34418 for the multiwallet test and CI changes that it enables. The last commit also removes the skipping of the symlink checks so that we are testing that the symlink handling behavior on Windows matches that on Linux and MacOS.

DrahtBot added the label Wallet on Feb 17, 2026

DrahtBot commented at 0:35 am on February 17, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34909 (wallet, refactor: modularise wallet by extracting out legacy wallet migration by rkrux)
#34806 (refactor: logging: Various API improvements by ajtowns)
#34544 (wallet: Disallow wallet names that are paths including .. and . elements by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Feb 17, 2026

achow101 force-pushed on Feb 17, 2026

in src/wallet/wallet.cpp:2930 in 677297e852 outdated

2925@@ -2926,8 +2926,10 @@ static util::Result<fs::path> GetWalletPath(const std::string& name)
2926     const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), fs::PathFromString(name));
2927     fs::file_type path_type = fs::symlink_status(wallet_path).type();
2928     if (!(path_type == fs::file_type::not_found || path_type == fs::file_type::directory ||
2929-          (path_type == fs::file_type::symlink && fs::is_directory(wallet_path)) ||
2930-          (path_type == fs::file_type::regular && fs::PathFromString(name).filename() == fs::PathFromString(name)))) {
2931+          (IsSymlink(wallet_path) && fs::is_directory(wallet_path)) ||
2932+          // Windows cross compile does not detect symlinks, so we need to explicilty check

fanquake commented at 9:41 am on February 17, 2026:

Do symlinks work fine with LLVM/Clang? cc @hebasto (#31507).

hebasto commented at 3:00 pm on February 18, 2026:

When building on Windows either with MSVC or clang-cl, std::filesystem::is_symlink successfully detects file and folder symbolic links, but fails to detect junctions.

Since the in-code comment mentions “Windows cross compile”, were you actually asking about the LLVM MinGW toolchain?

in src/util/fs_helpers.cpp:327 in f507a31eec outdated

322+#ifdef WIN32
323+    DWORD file_attrs = GetFileAttributesW(path.wstring().c_str());
324+    if (file_attrs == INVALID_FILE_ATTRIBUTES) {
325+        throw fs::filesystem_error("Unable to get file attributes", fs::PathToString(path), std::make_error_code(std::errc::invalid_argument));
326+    }
327+    return (file_attrs & FILE_ATTRIBUTE_REPARSE_POINT) != 0;

hebasto commented at 3:19 pm on February 18, 2026:

f507a31eec5219c3621b9d5551f9d04c2a409ded

Should we additionally check against a Reparse Point Tag value?

achow101 commented at 6:16 pm on February 18, 2026:

I considered that, but didn’t think any of the possible tag values were things that we actually want to support recursively scanning inside of.

hebasto commented at 6:54 pm on February 18, 2026:

Since the IsSymlink fucntion returns true for entities other than symlinks, we should probably add a clarifying comment.

achow101 commented at 11:06 pm on February 24, 2026:

The docstring states that it returns true for reparse points on windows.

in test/functional/wallet_multiwallet.py:154 in 677297e852 outdated

169-        try:
170-            with self.nodes[0].assert_debug_log(expected_msgs=["Error while scanning wallet dir"]):
171-                walletlist = self.nodes[0].listwalletdir()['wallets']
172-        finally:
173-            # Need to ensure access is restored for cleanup
174-            os.chmod(wallet_dir('no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)

luke-jr commented at 5:37 pm on February 18, 2026:

Why is this removed? It’s actually kind of annoying to have files that need to be chmod’d before deletion

achow101 commented at 6:14 pm on February 18, 2026:

It isn’t, the chmod testing is significantly changed. This is from #34418 and review comments about it should be left there.

in src/util/fs_helpers.cpp:320 in 677297e852 outdated

315@@ -315,3 +316,16 @@ FSType GetFilesystemType(const fs::path& path)
316     return FSType::OTHER;
317 }
318 #endif
319+
320+bool IsSymlink(const fs::path& path)

luke-jr commented at 6:50 pm on February 18, 2026:

Might it be better to just override fs::is_symlink on Windows?

achow101 commented at 7:18 pm on February 18, 2026:

I considered that, but we aren’t already using fs::is_symlink so I don’t think it would give us any benefit. Also, since IsSymlink returns true for more than just symlinks, I thought it wouldn’t be appropriate to be override the stdlib function.

in src/wallet/db.cpp:37 in 677297e852 outdated

30@@ -30,6 +31,11 @@ std::vector<std::pair<fs::path, std::string>> ListDatabases(const fs::path& wall
31         try {
32             const fs::path path{it->path().lexically_relative(wallet_dir)};
33 
34+            if (IsSymlink(it->path())) {
35+                LogWarning("Not recursively searching symlink/reparse point at '%s'", fs::PathToString(it->path()));
36+                it.disable_recursion_pending();
37+            }

luke-jr commented at 7:14 pm on February 18, 2026:

Is it intentional that we still check the symlink itself for a wallet?

If so, should we not do so even if IsSymlink throws?

achow101 commented at 7:21 pm on February 18, 2026:

Is it intentional that we still check the symlink itself for a wallet?

Yes

If so, should we not do so even if IsSymlink throws?

status() and symlink_status() used below can also throw, so this is the same behavior.

in src/wallet/wallet.cpp:2932 in 677297e852

2925@@ -2926,8 +2926,10 @@ static util::Result<fs::path> GetWalletPath(const std::string& name)
2926     const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), fs::PathFromString(name));
2927     fs::file_type path_type = fs::symlink_status(wallet_path).type();
2928     if (!(path_type == fs::file_type::not_found || path_type == fs::file_type::directory ||
2929-          (path_type == fs::file_type::symlink && fs::is_directory(wallet_path)) ||
2930-          (path_type == fs::file_type::regular && fs::PathFromString(name).filename() == fs::PathFromString(name)))) {
2931+          (IsSymlink(wallet_path) && fs::is_directory(wallet_path)) ||
2932+          // Windows cross compile does not detect symlinks, so we need to explicilty check
2933+          // whether a "regular file" is actually a symlink.
2934+          (path_type == fs::file_type::regular && fs::PathFromString(name).filename() == fs::PathFromString(name) && !IsSymlink(wallet_path)))) {

luke-jr commented at 7:18 pm on February 18, 2026:

This calls IsSymlink twice in some cases.

achow101 commented at 11:13 pm on February 24, 2026:

No longer relevant with #34603 (review)

in src/wallet/wallet.cpp:2929 in 677297e852 outdated

2925@@ -2926,8 +2926,10 @@ static util::Result<fs::path> GetWalletPath(const std::string& name)
2926     const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), fs::PathFromString(name));
2927     fs::file_type path_type = fs::symlink_status(wallet_path).type();
2928     if (!(path_type == fs::file_type::not_found || path_type == fs::file_type::directory ||
2929-          (path_type == fs::file_type::symlink && fs::is_directory(wallet_path)) ||

luke-jr commented at 7:19 pm on February 18, 2026:

Does this need to change? We already check for path_type == fs::file_type::directory

achow101 commented at 11:12 pm on February 24, 2026:

Hmm, good point. Removed

DrahtBot added the label Needs rebase on Feb 19, 2026

hodlinator referenced this in commit 850a80c199 on Feb 19, 2026

achow101 referenced this in commit d8e615ad93 on Feb 24, 2026

achow101 force-pushed on Feb 24, 2026

DrahtBot removed the label Needs rebase on Feb 25, 2026

achow101 force-pushed on Mar 13, 2026

DrahtBot removed the label CI failed on Mar 14, 2026

in src/wallet/wallet.cpp:2932 in 9333d26f67

2928@@ -2929,7 +2929,9 @@ static util::Result<fs::path> GetWalletPath(const std::string& name)
2929     fs::file_type path_type = fs::symlink_status(wallet_path).type();
2930     if (!(path_type == fs::file_type::not_found || path_type == fs::file_type::directory ||
2931           (path_type == fs::file_type::symlink && fs::is_directory(wallet_path)) ||
2932-          (path_type == fs::file_type::regular && fs::PathFromString(name).filename() == fs::PathFromString(name)))) {
2933+          // Windows cross compile does not detect symlinks, so we need to explicilty check

maflcko commented at 9:14 am on March 16, 2026:

llm nits:

windows -> Windows [Proper noun; “Windows” should be capitalized in documentation comment] (src/util/fs_helpers.h) explicilty -> explicitly [Spelling error in comment] (this line)

achow101 commented at 10:05 pm on March 23, 2026:

Fixed

chriszeng1010 referenced this in commit 8e86bbf4e9 on Mar 17, 2026

fs: Add IsSymlink which can check for Windows symlinks

GCC lacks detection of symlinks on Windows. IsSymlink implements this
detection on Windows, and utilizes std::filesystem::is_symlink for other
systems.

0be7c1916b

wallet: Use IsSymlink to detect symlinks and disable recursing them

When searching the wallets directory for wallets, we want to avoid
recursing symlinks. Although recursive_directory_iterator already
doesn't recurse symlinks, since GCC doesn't implement symlink detection
on Windows, we need to use IsSymlink to detect symlinks on Windows in
order to avoid recursing them.

6faf3d892c

wallet: Detect Windows symlinks in GetWalletPath

Since GCC lacks symlink detection, we need to use IsSymlink to detect
symlinks on Windows. Additionally, symlinks to files are reported as
being regular files rather symlnks, so we need to explicitly check with
IsSymlink that a regular file is not actually a symlink.

62675e4b7d

test: Test multiwallet symlinks on Windows 0f3fcdfaba

achow101 force-pushed on Mar 23, 2026

achow101 marked this as ready for review on Mar 23, 2026

achow101 commented at 10:05 pm on March 23, 2026: member

Forgot to undraft this.

wallet: Fix detection of symlinks on Windows #34603

Reviews

Conflicts