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:

#34544 (wallet: Disallow wallet names that are paths including .. and . elements by achow101)
#34439 (qa: Drop recursive deletes from test code, add lint checks. by davidgumberg)
#34418 (qa: Make wallet_multiwallet.py Windows crossbuild-compatible by hodlinator)
#34038 (logging: replace -loglevel with -trace, various API improvements by ajtowns)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

explicilty -> explicitly [misspelling in comment: “explicilty” should be “explicitly”]
windows -> Windows [proper noun should be capitalized in the doc comment]

2026-02-24 23:14:40

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

refactor(qa): Remove unused option

Last use was removed in 0d32d661481f099af572e7a08a50e17bcc165c44.

cff55e6364

scripted-diff: self.nodes[0] => node

-BEGIN VERIFY SCRIPT-
sed --in-place 's/self\.nodes\[0\]/node/g; s/node \= node/node \= self\.nodes\[0\]/' ./test/functional/wallet_multiwallet.py
-END VERIFY SCRIPT-

a72d11a227

refactor(qa): Lift out functions to outer scopes

This prepares for later breaking apart of run_test().

Note that the "wallet" lambda was renamed to "get_wallet" since otherwise the Python interpreter emitted:
"UnboundLocalError: cannot access local variable 'wallet' where it is not associated with a value"

2597ce5c37

move-only(qa): Move wallet creation check down to others

Makes the functions broken out from run_test() in the next commit more cohesive.

d91c6ca2df

refactor(qa): Break apart ginormous run_test() d1ad38980a

qa: Check for platform-independent part of error message

On Windows one gets different exception messages depending on whether running a native build or cross build.

413513652e

qa: Test scanning errors individually

This change ensures that each condition potentially triggering the
"Error while scanning" log message is tested independently, avoiding
false positives.

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

e21b4dc3d0

qa: Disable parts of the test when running under Windows or root

test_scanning_sub_dir():
- Remove try/finally - we don't need to clean up after a failed test (done in this commit to maintain indentation).

Regarding symlinks: https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-3554721014

Kept some symlink creation which didn't disrupt Windows cross builds to make for a smaller diff and less cumbersome code. There is some hope of eventually getting better symlink support via #34603.

Co-authored-by: Ava Chow <github@achow101.com>

d8e615ad93

ci: Enable `wallet_multiwallet.py` in "Windows, test cross-built" job

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

1a8dbfce79

qa: Avoid duplicating output in case the diff is the same 62f0ccf4fd

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.

8735ccf866

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.

277828dcc3

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.

4d66b4368b

test: Test multiwallet symlinks on Windows fda778d1cc

achow101 force-pushed on Feb 24, 2026

DrahtBot removed the label Needs rebase on Feb 25, 2026

wallet: Fix detection of symlinks on Windows #34603

Reviews

Conflicts

LLM Linter (✨ experimental)