.. and . elements
#34544
Wallet names including .. and . are unintuitive and can lead to various issues, see #34497
This disallows creating or loading wallets that have any path elements that are .. or ., including any present in an absolute path. This does not disallow relative paths altogether but rather limits them to only being subdirectories of the wallets directory.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | davidgumberg, arejula27 |
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
https://github.com/bitcoin/bitcoin/actions/runs/21844979703/job/63075106819?pr=34544#step:12:15955:
0 test 2026-02-10T08:24:08.355405Z TestFramework (ERROR): Unexpected exception
1 Traceback (most recent call last):
2 File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 142, in main
3 self.run_test()
4 ~~~~~~~~~~~~~^^
5 File "D:\a\bitcoin\bitcoin/test/functional/wallet_migration.py", line 1678, in run_test
6 self.test_wallet_with_relative_path()
7 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
8 File "D:\a\bitcoin\bitcoin/test/functional/wallet_migration.py", line 621, in test_wallet_with_relative_path
9 migrate_res, wallet = self.migrate_and_get_rpc(relative_name)
10 ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
11 File "D:\a\bitcoin\bitcoin/test/functional/wallet_migration.py", line 139, in migrate_and_get_rpc
12 migrate_info = self.master_node.migratewallet(wallet_name=wallet_name, **kwargs)
13 File "D:\a\bitcoin\bitcoin\test\functional\test_framework\coverage.py", line 50, in __call__
14 return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
15 File "D:\a\bitcoin\bitcoin\test\functional\test_framework\authproxy.py", line 156, in __call__
16 raise JSONRPCException(response['error'], status)
17 test_framework.authproxy.JSONRPCException: Wallet name given as a relative path cannot begin with .. or . (-4)
18 test 2026-02-10T08:24:08.357776Z TestFramework (DEBUG): Closing down network thread
2922+ // 'name' must be a normalized path, i.e. no . or .. except at the root
2923+ if (name_path != name_path.lexically_normal()) {
2924+ return util::Error{Untranslated("Wallet name given as a path must be normalized")};
2925+ }
2926+
2927+ // 'name' cannot begin with . or ..
This comment is not correct, as I was available to do
0$➜ bitcoin (34544) % bitcoin-cli -regtest createwallet ".my_pb_wall" 20:11
1{
2 "name": ".my_pb_wall"
3}
./ and ../
Concept ACK [7c993c6036088990af9cfca12bb7cf33120cf33c]
the funtional test wallet_migration.py fails cuz the test specifically tries to migrate from a relative path using .., see line 621:
0wallet_name = "relative"
1absolute_path = os.path.abspath(
2 os.path.join(common_parent, wallet_name))
3# /tmp/bitcoin_func_test_rriidks3/relative
4relative_name = os.path.relpath(
5 absolute_path, start=self.master_node.wallets_path)
6# ../../../relative
7
8wallet = self.create_legacy_wallet(relative_name)
This is done because the nodes use different wallet directories (/tmp/bitcoin_func_test_kfajzcqh/node0/regtest/wallets and /tmp/bitcoin_func_test_kfajzcqh/node1/regtest/wallets), which causes GetWalletPath to fail, as it relies on ../../ to reach the old node’s wallet directory.
To fix this, migrations should only happen from a common ancestor directory to avoid ... However, I suggest using absolute paths for migration and deprecate this test case. Instead, the test should verify that attempting to migrate using a relative path returns an error (e.g., “migration from a relative path is not allowed”).
This change will cause users who load wallets using relative paths to fail. For the loadwallet RPC, it may be useful to improve the error message, for example:
Wallet file verification failed. Wallet names given as relative paths cannot begin with .. or . since v31. Please use an absolute path instead. Your wallet data has not been lost.
Wallet names that are also paths that contain . and .. are unintuitive
and can result in unexpected behavior, particularly in migration.
Therefore we should disallow users from specifying wallet names that
contain . and .. as path elements.
However, I suggest using absolute paths for migration and deprecate this test case. Instead, the test should verify that attempting to migrate using a relative path returns an error (e.g., “migration from a relative path is not allowed”).
Yes, changed the test to check for the error, and removed the other test that used a relative path like this.
it may be useful to improve the error message
I included the suggestion to use an absolute path.
When using Bitcoin-Qt, it fails during initialisation, shows an error, and then crashes. Relaunching the program results in the same error, making it impossible to use (and impossible to manually unload the wallet using the GUI).
To resolve the issue, users can open DATADIR/settings.json and remove the wallet entry:
0{
1 "_warning_": "This file is automatically generated and updated by Bitcoin Core. Please do not edit this file while the node is running, as any changes might be ignored or overwritten.",
2 "wallet": [
3 "../qt-wallet-relative"
4 ]
5}
For non-technical users, this can be difficult to troubleshoot. It would be nice to update the doc somewhere so they can read how to fix the crash loop or automatically remove all wallets that start with .. or ./ from the auto load configuration.