Skip to content

Fix Windows CI: convert GNUPGHOME path to MSYS2 format for GPG subprocess#313

Merged
3rdIteration merged 57 commits intodevfrom
copilot/update-bip85-gpg-functionality
Mar 9, 2026
Merged

Fix Windows CI: convert GNUPGHOME path to MSYS2 format for GPG subprocess#313
3rdIteration merged 57 commits intodevfrom
copilot/update-bip85-gpg-functionality

Conversation

Copy link

Copilot AI commented Feb 28, 2026

Description

9 GPG export roundtrip tests fail on desktop (windows-latest) CI. Python's tempfile returns native Windows paths (C:\Users\...) but Git-for-Windows ships an MSYS2 GPG that expects POSIX paths (/c/Users/...). GPG concatenates the CWD with the Windows path, producing invalid paths like /d/a/seedsigner/seedsigner/C:\Users\...\pubring.kbx.

Added _msys2_path() helper in test_gpg_message.py to convert drive-letter paths for GNUPGHOME:

def _msys2_path(path: str) -> str:
    if sys.platform == "win32":
        path = path.replace("\\", "/")
        # Convert drive letter, e.g. "C:/Users/..." → "/c/Users/..."
        if len(path) >= 2 and path[1] == ":":
            path = "/" + path[0].lower() + path[2:]
    return path

No-op on Linux/macOS. Applied to both test_bip85_key_gpg_export_roundtrip and test_generate_new_gpg_export_roundtrip.

No screenshots — test-only change.

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

Checklist

  • I've run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • No, I'm a fool
  • Yes
  • N/A

I have tested this PR on the following platforms/os:


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits February 28, 2026 03:56
…ith key_type, add new curves (P-384, P-521, Brainpool P-384, P-512)

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…Brainpool P-384, P-512)

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Copilot AI changed the title [WIP] Update BIP85 GPG functionality to expand curve types Update BIP85 GPG to unified application number with key_type path and add P-384, P-521, Brainpool P-384/P-512 Feb 28, 2026
These vectors use the BIP85 spec's common xprv and pin the expected
entropy, private key scalars (ECC), and RSA n-value.  Any conforming
BIP85-GPG implementation must derive identical values for the same
master key and path.

Documents that PyCryptodome's RSA.generate() is the reference
algorithm for RSA key generation, as implied by the BIP85 spec.

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Copilot AI changed the title Update BIP85 GPG to unified application number with key_type path and add P-384, P-521, Brainpool P-384/P-512 Add cross-implementation reference vectors for BIP85 GPG key derivation Mar 7, 2026
… from_seed(seed.seed_bytes)

ToolsGPGLoadBIP85KeyView.run() and bip85_add_subkeys() both used
bip32.HDKey.from_seed(seed.seed_bytes) which crashes when the seed is
an XprvSeed (seed_bytes is None).  This caused users loading an xprv
directly into SeedSigner to be unable to derive BIP85 GPG keys.

Fix: use seed.get_root(network) which works for both mnemonic-based
Seed and XprvSeed objects.

Add test verifying that XprvSeed produces identical BIP85 GPG keys
(both RSA and ECC) to the equivalent mnemonic-derived Seed.

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Copilot AI changed the title Add cross-implementation reference vectors for BIP85 GPG key derivation Fix BIP85 GPG key derivation crash when seed is loaded from xprv Mar 7, 2026
Document the critical differences between Seed, XprvSeed, ElectrumSeed,
AezeedSeed, and Slip39Seed — particularly that XprvSeed.seed_bytes is
None, so code must always use seed.get_root(network) instead of directly
calling bip32.HDKey.from_seed(seed.seed_bytes).

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Copilot AI changed the title Fix BIP85 GPG key derivation crash when seed is loaded from xprv Fix XprvSeed BIP85 GPG crash; add seed type guidance to AGENTS.md Mar 7, 2026
…seed_bytes

embit_utils.sign_message() previously took seed_bytes and called
HDKey.from_seed() internally. The caller at seed_views.py:4426 passed
seed.get_root().secret (the 32-byte master private key), which
HDKey.from_seed() treated as a new seed — producing a completely
different root key and signing with the wrong key.

Fix: sign_message() now accepts a root HDKey directly. The caller
passes seed.get_root() which works correctly for all seed types
including XprvSeed.

Added tests:
- test_sign_message_uses_correct_derivation
- test_sign_message_same_result_for_bip39_and_xprv
- test_sign_message_double_derivation_gives_different_result

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Copilot AI changed the title Fix XprvSeed BIP85 GPG crash; add seed type guidance to AGENTS.md Add seed type guidance to AGENTS.md; fix sign_message double-derivation bug Mar 7, 2026
Copilot AI changed the title Add seed type guidance to AGENTS.md; fix sign_message double-derivation bug Fix seed_bytes vs get_root() violations across codebase Mar 7, 2026
logger.info("Satochip Import Failed:",str(e)) silently dropped the
exception message because the colon is not a format specifier.
Changed to logger.exception("Satochip Import Failed: %s", e) to:
- Use proper %s formatting so the error is actually logged
- Use logger.exception (not info) to include the traceback
- Correct the log level for an error condition

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Remove the monkey-patch file and all imports/calls to it from
tools_views.py, gpg_message.py, bip85_pgp.py, and
test_bip85_bipsea_vectors.py. The secp256k1 and Brainpool ECDSA
signing support should be implemented directly in the 3rdIteration/PGPy
fork, not patched from the seedsigner side.

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Copilot AI changed the title Add secp256k1 and Brainpool ECDSA/ECDH signing support via ecdsa library Revert pgpy_ecdsa_patch.py: secp256k1/Brainpool signing belongs in PGPy Mar 8, 2026
Copilot AI and others added 3 commits March 8, 2026 23:25
The 3rdIteration/PGPy fork now natively supports secp256k1 and
Brainpool curves via ecdsa/embit fallback, eliminating the need
for the monkey-patch that was previously in pgpy_ecdsa_patch.py.

All 146 GPG-related tests pass:
- test_bip85_pgp_cli: 33 passed
- test_gpg_message: 11 passed
- test_bip85_bipsea_vectors: 38 passed
- test_bip85_gpg: 63 passed
- test_gpg_time_update: 1 passed

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…automatic failover

Rewrite ec_point.py with backend selection: cryptography → pycryptodome → embit → ecdsa → pure_python.
Add set_backend()/get_backend()/available_backends() API.
Create test_ec_backends.py with 65 tests covering all backends independently and cross-validating results.

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…ces)

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR expands SeedSigner’s BIP85 GPG/PGP key support (additional ECC curves and corrected derivation behavior) and introduces a new ec_point.py helper that derives EC public keys via multiple crypto backends with automatic selection.

Changes:

  • Add support for NIST P-384/P-521 and Brainpool P-384/P-512 across the BIP85 PGP tool and GPG flows.
  • Introduce src/seedsigner/helpers/ec_point.py with backend detection/dispatch and add backend cross-agreement tests.
  • Fix message signing to derive directly from a root HDKey (avoiding prior double-derivation) and add regression tests.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/bip85_pgp.py Extends CLI PGP key generation to additional ECC curves and subkey specs.
src/seedsigner/helpers/ec_point.py New multi-backend EC public key derivation module with backend selection API.
src/seedsigner/views/tools_views.py Updates BIP85 GPG derivation path scheme, adds new ECC curves, and adds a new “View Keys” menu/view.
src/seedsigner/helpers/embit_utils.py Changes sign_message to accept a root HDKey directly (fixing derivation behavior).
src/seedsigner/views/seed_views.py Updates message signing call site to pass root=seed.get_root().
tests/test_ec_backends.py New tests to validate EC backends agree on derived public points/keys.
tests/test_embit_utils.py Adds regression tests ensuring message signing uses the correct root derivation.
tests/test_bip85_pgp_cli.py Extends CLI coverage for new ECC key types and adds GPG round-trip tests for compatible key types.
tests/test_bip85_gpg.py Updates vectors and adds cross-implementation tests for entropy/private keys/RSA determinism and UI assertions.
tests/test_bip85_bipsea_vectors.py Adds a comprehensive cross-implementation vector suite (bipsea/OpenSSL/PyCryptodome claims).
tests/BIP85_GPG_CROSS_IMPL_REPORT.md Adds a detailed validation report and spec recommendations.
requirements.txt Switches pgpy to a fork URL and removes hash-pinning / cryptography pinning.
AGENTS.md Documents the seed.get_root(network) requirement across seed types.
Comments suppressed due to low confidence (2)

src/seedsigner/views/tools_views.py:5931

  • PR description states "No UI changes", but ToolsGPGMenuView adds a new "View Keys" menu option and a new view implementation. Please update the PR description (or revert the UI change) so reviewers/testers know to validate the new menu flow.
class ToolsGPGMenuView(View):
    FILE_OPS = ButtonOption("File Operations")
    IMPORT = ButtonOption("Import Keys")
    EXPORT = ButtonOption("Export Keys")
    VIEW_KEYS = ButtonOption("View Keys")
    MESSAGE = ButtonOption("Secure Messaging")
    SMART_GPG = ButtonOption("SmartGPG")
    ADVANCED = ButtonOption("Advanced")

    def run(self):
        from seedsigner.controller import Controller

        if self.controller.resume_main_flow == Controller.FLOW__GPG_MESSAGE:
            self.controller.resume_main_flow = None
            return Destination(ToolsGPGDecryptMessageView, skip_current_view=True)

        button_data = [
            self.FILE_OPS,
            self.IMPORT,
            self.EXPORT,
            self.VIEW_KEYS,
            self.MESSAGE,
            self.SMART_GPG,
            self.ADVANCED,
        ]

src/seedsigner/helpers/embit_utils.py:203

  • sign_message() returns a base64-encoded string ("...decode()"), but its return type annotation is "-> bytes". This is now inconsistent with actual behavior and the new tests asserting a str. Update the annotation to "-> str" (or change the return value to bytes) to keep typing accurate.
def sign_message(root: HDKey, derivation: str, msg: bytes, compressed: bool = True) -> bytes:
    """
        from: https://github.com/cryptoadvance/specter-diy/blob/b58a819ef09b2bca880a82c7e122618944355118/src/apps/signmessage/signmessage.py

        Sign a Bitcoin message using a BIP-32 root key and derivation path.
        Use seed.get_root(network) to obtain the root key — never
        bip32.HDKey.from_seed(seed.seed_bytes) directly (see AGENTS.md).
    """
    msghash = sha256(
        sha256(
            b"\x18Bitcoin Signed Message:\n" + compact.to_bytes(len(msg)) + msg
        ).digest()
    ).digest()

    prv = root.derive(derivation).key
    sig = secp256k1.ecdsa_sign_recoverable(msghash, prv._secret)
    flag = sig[64]
    sig = ec.Signature(sig[:64])
    c = 4 if compressed else 0
    flag = bytes([27 + flag + c])
    ser = flag + secp256k1.ecdsa_signature_serialize_compact(sig._sig)
    return b2a_base64(ser).strip().decode()

Comment on lines +299 to +303
pytest.skip("cryptography not installed")
set_backend(None)
# Ed25519 should use cryptography (first in preference order)
pub = ed25519_pub_from_seed(_SEED_32)
assert len(pub) == 32
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_auto_detect_prefers_cryptography() claims to verify that auto-detection prefers cryptography, but it only checks the derived pubkey length. As written it won’t fail if a different backend is used. Consider exposing/recording the selected backend (e.g., a helper that reports the backend used for the last operation) and asserting it here.

Suggested change
pytest.skip("cryptography not installed")
set_backend(None)
# Ed25519 should use cryptography (first in preference order)
pub = ed25519_pub_from_seed(_SEED_32)
assert len(pub) == 32
pytest.skip("cryptography not installed")
# Restore auto-detection and verify that the preferred backend is chosen
set_backend(None)
# Ed25519 should use cryptography (first in preference order)
pub = ed25519_pub_from_seed(_SEED_32)
assert len(pub) == 32
# Ensure that auto-detection actually selected the cryptography backend
assert get_backend() == CRYPTOGRAPHY

Copilot uses AI. Check for mistakes.
Comment on lines +162 to +175
# ── ECC public key cross-validation with OpenSSL ────────────────────────────
# Confirms that the private scalar → public point derivation in pgpy
# matches OpenSSL (via the ``cryptography`` library).

OPENSSL_ECDSA_VECTORS = [
# (deriver, openssl_curve_class)
("secp256k1", bip85_secp256k1_from_root, "SECP256K1"),
("NIST P-256", bip85_p256_from_root, "SECP256R1"),
("NIST P-384", bip85_p384_from_root, "SECP384R1"),
("NIST P-521", bip85_p521_from_root, "SECP521R1"),
("Brainpool P-256", bip85_brainpoolp256r1_from_root, "BrainpoolP256R1"),
("Brainpool P-384", bip85_brainpoolp384r1_from_root, "BrainpoolP384R1"),
("Brainpool P-512", bip85_brainpoolp512r1_from_root, "BrainpoolP512R1"),
]
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module docstring says the vectors are validated against OpenSSL via the cryptography library, but these tests never force the cryptography backend (and can pass using other backends). Either force cryptography for the OpenSSL cross-validation portions or reword the docstring/comments to reflect that the validation is against SeedSigner’s backend-derived points (which may or may not be OpenSSL-backed).

Copilot uses AI. Check for mistakes.
Copilot AI and others added 21 commits March 9, 2026 02:32
Resolve conflicts:
- requirements.txt: Keep PGPy fork + cryptography deps + smbus2
- tests/test_embit_utils.py: Include both sign_message and expanded search tests
…primary, pycryptodomex fallback)

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
- Add _normalize_date_input() to strip whitespace and normalize non-ASCII
  dashes (fullwidth, en-dash, em-dash, Unicode minus) to ASCII hyphens
- Fix all 8 prompt_text unicode handlers: except UnicodeDecodeError -> UnicodeError
  (UnicodeEncodeError is a ValueError subclass, was falling through to the
  outer "Invalid expiration date" handler)
- Use _normalize_date_input() in both BIP85 and non-BIP85 expiration blocks
- Use .strip() for empty string check to handle whitespace-only input

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…E.md

- settings.py: Replace .isdigit() pre-check with try/except for int()/float()
  to handle superscript digits (¹²³) where isdigit()=True but int() fails
- passport_backup.py: Use ASCII-only digit check instead of .isdigit()
- embit_utils.py: Add .isascii() guard before .isdigit() for derivation paths
- CLAUDE.md: Add Unicode and locale-safe string handling guidance section
- Add tests for all three fixes verifying non-ASCII digit rejection

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…t docs in AGENTS.md

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
When the user opens GPG Tools, check that both the pgpy Python
package and the system gpg binary (gnupg2) are available. If either
is missing, show an ErrorScreen listing the specific missing
package(s) and return to the previous menu.

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…Subkeys views

- ToolsGPGViewKeysView now filters out subkey fingerprints from the
  primary key list, preventing subkeys from appearing as separate entries
- Key Details screen split into ToolsGPGKeyDetailsView: no green tick
  icon (status_icon_size=0), no subkey count in text, back button
  enabled, and a "Subkeys" button when subkeys exist
- New ToolsGPGKeySubkeysView for browsing subkeys with type and
  capabilities shown per subkey
- Updated and added tests for all new views

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…bkey details

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…g for all key types

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
- Fix unhandled exception in ToolsGPGEncryptMessageView when passphrase
  retry fails: wrap second encrypt_message call in try/except
- Add 15 parametrized tests for BIP85-derived key message operations:
  encrypt/decrypt, sign-only, sign+encrypt+decrypt across all key types
- Add 3 end-to-end GPG import/export roundtrip tests verifying keys
  survive the GPG keyring import→export path

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…r cleanup

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…es in Generate New and Derive BIP85 menus

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…crypt with expanded key types

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
…cess

Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants