Document and test multisig approval policy
What the developer wrote
Authored by Jack
Document and test multisig approval policy
What changed, and why it matters
This commit documents and tests a policy for approving multisignature wallets proposed by a PSBT (Partially Signed Bitcoin Transaction). The key functional change is that when a Passport device is using a temporary seed, it now requires explicit user confirmation before importing a multisig wallet proposed by a PSBT, and cancels signing if the user declines. The commit itself is mostly a changelog entry and unit tests; the actual security logic appears to have been implemented elsewhere.
Review the companion commit(s) that implement the actual confirmation and cancellation logic to ensure the policy is enforced consistently and cannot be bypassed. Verify that `get_multisig_policy()` correctly defaults to a secure value for temporary seeds and that the UI flow cannot be skipped by a malicious PSBT.
Security signals we found
User confirmation gate added for PSBT-proposed multisig wallet import when using temporary seeds
Signing cancellation if multisig import is declined
Unit tests added/updated for multisig approval policy behavior
Change in default policy semantics for temporary seeds (MUSIG_TEMP_DEFAULT removed/replaced)
Evidence from the diff
The commit adds a CHANGELOG entry stating that PSBT-proposed multisig wallets now require confirmation before use with temporary seeds, and that signing is cancelled if import is declined. It also updates a unit test file to verify the get_multisig_policy() utility returns MUSIG_ASK when no policy is set and MUSIG_SKIP when explicitly configured. The test mocks common.settings to simulate temporary seed mode. Notably, the previous assertion MUSIG_TEMP_DEFAULT == MUSIG_ASK is removed, suggesting the default policy for temporary seeds changed from ‘ask’ to something else, or the constant was renamed/removed. The actual enforcement code is not present in this diff.
Changed components
Passport firmware multisig PSBT signing flowTemporary seed handlingMultisig wallet import approval policyUnit test: ports/stm32/boards/Passport/modules/tests/unit/psbt_multisig_approval.pyInspect captured patch +27 / −2
### CHANGELOG.md
@@ -5,6 +5,8 @@ SPDX-License-Identifier: GPL-3.0-or-later
-->
## Head
+- Require confirmation before using PSBT-proposed multisig wallets with temporary seeds,
+ and cancel signing if import is declined
- Added Coconut Wallet as a single-sig Connect Wallet option
- Improved self-send transaction information formatting (PASS1-638)
- Added the key manager extension, compatible with BIP85 and Nostr (PASS1-24)
### ports/stm32/boards/Passport/modules/tests/unit/psbt_multisig_approval.py
@@ -3,10 +3,12 @@
#
# Test approval of multisig wallets proposed by PSBTs.
+import common
import flows
import uasyncio as asyncio
from flows import SignPsbtCommonFlow
-from public_constants import MUSIG_ASK, MUSIG_TEMP_DEFAULT
+from public_constants import MUSIG_ASK, MUSIG_SKIP
+from utils import get_multisig_policy
class FakeImportMultisigWalletFlow:
@@ -27,6 +29,20 @@ def __init__(self, needs_approval):
self.active_multisig = 'proposed-wallet'
+class FakeSettings:
+ temporary_mode = True
+
+ def __init__(self, policy=None):
+ self.policy = policy
+
+ def get(self, key, default=None):
+ if key == 'temporary_seed':
+ return 'temporary-seed'
+ if key == 'multisig_policy' and self.policy is not None:
+ return self.policy
+ return default
+
+
class FakeSignFlow:
def __init__(self, needs_approval):
self.psbt = FakePsbt(needs_approval)
@@ -45,6 +61,7 @@ def goto(self, state):
async def run_tests():
original_import_flow = flows.ImportMultisigWalletFlow
+ original_settings = common.settings
try:
flows.ImportMultisigWalletFlow = FakeImportMultisigWalletFlow
@@ -73,10 +90,16 @@ async def run_tests():
assert not flow.completed
assert flow.next_state == flow.show_transaction_details
- assert MUSIG_TEMP_DEFAULT == MUSIG_ASK
+ common.settings = FakeSettings()
+ assert get_multisig_policy() == MUSIG_ASK
+
+ common.settings = FakeSettings(policy=MUSIG_SKIP)
+ assert get_multisig_policy() == MUSIG_SKIP
+
return_value.write(b'OK')
finally:
flows.ImportMultisigWalletFlow = original_import_flow
+ common.settings = original_settings
asyncio.run(run_tests())Why this scored 41/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.