wallet: improve error msg when db directory is not writable #34176

pull furszy wants to merge 2 commits into bitcoin:master from furszy:2025_wallet_check_db_permissions changing 4 files +62 −0
  1. furszy commented at 7:44 pm on December 29, 2025: member

    Related to #34163.

    Make wallet creation fail with a clear error when the db directory isn’t writable.

    Before: creating a wallet would throw a generic error: “SQLiteDatabase: Failed to open database: unable to open database file”

    After: creating a wallet throws: “SQLiteDatabase: Failed to open database in directory <dir_path>: directory is not writable”

    Also, to check directory writability, this creates a tmp file rather than relying on the permissions() functions, since perms bits alone may not reliably reflect actual writability in some systems.

    Testing Note: Pushed the test in a separate commit so it can be cherry-picked on master for comparison.

  2. DrahtBot added the label Wallet on Dec 29, 2025
  3. DrahtBot commented at 7:45 pm on December 29, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34176.

    Reviews

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

  4. DrahtBot added the label CI failed on Dec 29, 2025
  5. furszy force-pushed on Dec 30, 2025
  6. in test/functional/wallet_createwallet.py:39 in 16b82f13db 
    34+        dir_path = node.wallets_path / wallet_name
35+        dir_path.mkdir(parents=True)
36+        os.chmod(dir_path, stat.S_IREAD | stat.S_IEXEC)
37+        try:
38+            (dir_path / f".tmp_{random.randrange(1 << 32)}").touch() # Verify the directory is actually non-writable
39+            self.log.warn("Skipping non-writable directory test: unable to enforce read-only permissions")
    rkrux commented at 9:00 am on December 30, 2025:

    From the CI failure logs:

    0stderr:
1/home/admin/actions-runner/_work/_temp/build/test/functional/wallet_createwallet.py:39: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
2  self.log.warn("Skipping non-writable directory test: unable to enforce read-only permissions")

    0- self.log.warn("Skipping non-writable directory test: unable to enforce read-only permissions")
1+ self.log.warning("Skipping non-writable directory test: unable to enforce read-only permissions")
    rkrux commented at 9:16 am on December 30, 2025:

    Based on the logs, it appears that all the 9 failing jobs skipped the test. At the moment, I don’t know why the touch function didn’t throw here even after non-writable permissions given to the directory. The jobs failed because the clean up was not done after the test because of access denied errors. Restoring the original permissions of the directory in a finally block should help here.

     0stderr:
 1D:\a\bitcoin\bitcoin\build\test\functional\wallet_createwallet.py:39: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
 2
 3  self.log.warn("Skipping non-writable directory test: unable to enforce read-only permissions")
 4
 5Traceback (most recent call last):
 6
 7  File "D:\a\bitcoin\bitcoin\build\test\functional\wallet_createwallet.py", line 202, in <module>
 8
 9    CreateWalletTest(__file__).main()
10
11    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
12
13  File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\test_framework.py", line 154, in main
14
15    exit_code = self.shutdown()
16
17  File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\test_framework.py", line 332, in shutdown
18
19    shutil.rmtree(self.options.tmpdir)
20
21    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
22
23  File "C:\hostedtoolcache\windows\Python\3.14.2\x64\Lib\shutil.py", line 852, in rmtree
24
25    _rmtree_impl(path, dir_fd, onexc)
26
27    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
28
29  File "C:\hostedtoolcache\windows\Python\3.14.2\x64\Lib\shutil.py", line 697, in _rmtree_unsafe
30
31    onexc(os.rmdir, fullname, err)
32
33    ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
34
35  File "C:\hostedtoolcache\windows\Python\3.14.2\x64\Lib\shutil.py", line 693, in _rmtree_unsafe
36
37    os.rmdir(fullname)
38
39    ~~~~~~~~^^^^^^^^^^
40
41PermissionError: [WinError 5] Access is denied: 'D:\\a\\_temp\\test_runner_₿_🏃_20251230_063644\\wallet_createwallet_186\\node0\\regtest\\wallets\\bad_permissions'
    furszy commented at 4:08 pm on December 30, 2025:
    Done. Thanks.
    furszy commented at 4:09 pm on December 30, 2025:
    Done as suggested. Thanks.
  7. in test/functional/wallet_createwallet.py:44 in 16b82f13db 
    39+            self.log.warn("Skipping non-writable directory test: unable to enforce read-only permissions")
40+            return
41+        except PermissionError:
42+            pass
43+        assert_raises_rpc_error(-4, f"SQLiteDatabase: Failed to open database in directory '{str(dir_path)}': directory is not writable", node.createwallet, wallet_name=wallet_name, descriptors=True)
44+        dir_path.chmod(dir_path.stat().st_mode | stat.S_IWRITE)
    rkrux commented at 9:04 am on December 30, 2025:

    By debugging the permissions at 3 different stages here, it appears that the original permissions of the directory are not set at the end of this test.

     0diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py
 1index 3bd8e6a310..29a2db3dab 100755
 2--- a/test/functional/wallet_createwallet.py
 3+++ b/test/functional/wallet_createwallet.py
 4@@ -33,7 +33,9 @@ class CreateWalletTest(BitcoinTestFramework):
 5         wallet_name = "bad_permissions"
 6         dir_path = node.wallets_path / wallet_name
 7         dir_path.mkdir(parents=True)
 8+        print(dir_path.stat().st_mode)
 9         os.chmod(dir_path, stat.S_IREAD | stat.S_IEXEC)
10+        print(dir_path.stat().st_mode)
11         try:
12             (dir_path / f".tmp_{random.randrange(1 << 32)}").touch() # Verify the directory is actually non-writable
13             self.log.warn("Skipping non-writable directory test: unable to enforce read-only permissions")
14@@ -42,12 +44,13 @@ class CreateWalletTest(BitcoinTestFramework):
15             pass
16         assert_raises_rpc_error(-4, f"SQLiteDatabase: Failed to open database in directory '{str(dir_path)}': directory is not writable", node.createwallet, wallet_name=wallet_name, descriptors=True)
17         dir_path.chmod(dir_path.stat().st_mode | stat.S_IWRITE)
18+        print(dir_path.stat().st_mode)

    0➜  bitcoin git:(2025_wallet_check_db_permissions) ✗ ./build/test/functional/wallet_createwallet.py            
12025-12-30T09:01:50.770611Z TestFramework (INFO): PRNG seed is: 3561247270765759257
22025-12-30T09:01:50.771467Z TestFramework (INFO): Initializing test directory /var/folders/6v/mpspb4bx4zzf3xr2z96hgq9h0000gn/T/bitcoin_func_test_5m4h9kjq
32025-12-30T09:01:51.370097Z TestFramework (INFO): Test wallet creation failure due to non-writable directory
416877
516704
616832
72025-12-30T09:01:51.424150Z TestFramework (INFO): Stopping nodes
82025-12-30T09:01:51.689178Z TestFramework (INFO): Cleaning up /var/folders/6v/mpspb4bx4zzf3xr2z96hgq9h0000gn/T/bitcoin_func_test_5m4h9kjq on exit
92025-12-30T09:01:51.689295Z TestFramework (INFO): Tests successful

    I believe 16877 (0o40755) should be set back at the end of this test.

    furszy commented at 4:08 pm on December 30, 2025:
    Done as suggested. Thanks.
  8. in test/functional/wallet_createwallet.py:36 in 16b82f13db 
    27@@ -25,9 +28,26 @@ def set_test_params(self):
28     def skip_test_if_missing_module(self):
29         self.skip_if_no_wallet()
30 
31+    def test_bad_dir_permissions(self, node):
32+        self.log.info("Test wallet creation failure due to non-writable directory")
33+        wallet_name = "bad_permissions"
34+        dir_path = node.wallets_path / wallet_name
35+        dir_path.mkdir(parents=True)
36+        os.chmod(dir_path, stat.S_IREAD | stat.S_IEXEC)
    rkrux commented at 9:18 am on December 30, 2025:

    https://docs.python.org/3/library/stat.html#stat.S_IREAD

    stat.S_IREAD Unix V7 synonym for S_IRUSR.

    Maybe s/stat.S_IREAD/stat.S_IRUSR to avoid using the Unix synonym, thereby making it platform independent. Just a guess because the test works fine on my MacOS.

    furszy commented at 4:09 pm on December 30, 2025:
    Done as suggested. Thanks.
  9. rkrux commented at 9:31 am on December 30, 2025: contributor

    Combining the individual suggestions, the full diff:

     0diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py
 1index 3bd8e6a310..f0b34d685d 100755
 2--- a/test/functional/wallet_createwallet.py
 3+++ b/test/functional/wallet_createwallet.py
 4@@ -33,21 +33,23 @@ class CreateWalletTest(BitcoinTestFramework):
 5         wallet_name = "bad_permissions"
 6         dir_path = node.wallets_path / wallet_name
 7         dir_path.mkdir(parents=True)
 8-        os.chmod(dir_path, stat.S_IREAD | stat.S_IEXEC)
 9+        original_dir_perms = dir_path.stat().st_mode
10+        os.chmod(dir_path, original_dir_perms & ~(stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH))
11         try:
12-            (dir_path / f".tmp_{random.randrange(1 << 32)}").touch() # Verify the directory is actually non-writable
13-            self.log.warn("Skipping non-writable directory test: unable to enforce read-only permissions")
14-            return
15+            if (dir_path / f".tmp_{random.randrange(1 << 32)}").touch(): # Verify the directory is actually non-writable
16+                self.log.warning("Skipping non-writable directory test: unable to enforce read-only permissions")
17+                return
18+            assert_raises_rpc_error(-4, f"SQLiteDatabase: Failed to open database in directory '{str(dir_path)}': directory is not writable", node.createwallet, wallet_name=wallet_name, descriptors=True)
19         except PermissionError:
20             pass
21-        assert_raises_rpc_error(-4, f"SQLiteDatabase: Failed to open database in directory '{str(dir_path)}': directory is not writable", node.createwallet, wallet_name=wallet_name, descriptors=True)
22-        dir_path.chmod(dir_path.stat().st_mode | stat.S_IWRITE)
23+        finally:
24+            dir_path.chmod(original_dir_perms)
25
  10. furszy force-pushed on Dec 30, 2025
  11. furszy commented at 4:17 pm on December 30, 2025: member
    Thanks for the review @rkrux! Applied all suggestions except the touch() if-block, since the function doesn’t return a boolean (per docs, it either returns None or an exception). Also pulled out the is_dir_writable function to make the introduced try-except-finally workflow slightly more readable.
  12. DrahtBot removed the label CI failed on Dec 30, 2025
  13. wallet: improve error msg when db directory is not writable 
    Add `IsDirWritable` helper in util/fs_helpers to test whether a directory
can be written to by creating a temporary file.

Use this check when opening a SQLite database to fail early with a clear
error if the database directory is not writable, preventing obscure SQLite
errors.

Before: creating a wallet would throw a generic error:
"SQLiteDatabase: Failed to open database: unable to open database file"

After: creating a wallet throws:
"SQLiteDatabase: Failed to open database in directory <dir_path>: directory is not writable"
    350840e960
  14. test: add coverage for wallet creation in non-writable directory 0a9ddd3dba
  15. furszy force-pushed on Dec 30, 2025


furszy DrahtBot rkrux

Labels
Wallet

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-01-01 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me