Encrypt private keys at rest using Alloy's built-in keystore methods#19
Open
smypmsa wants to merge 4 commits intoPolymarket:mainfrom
Open
Encrypt private keys at rest using Alloy's built-in keystore methods#19smypmsa wants to merge 4 commits intoPolymarket:mainfrom
smypmsa wants to merge 4 commits intoPolymarket:mainfrom
Conversation
…thods Private keys were stored as plaintext in config.json. Now encrypted using LocalSigner::encrypt_keystore (AES-128-CTR + scrypt) via Alloy's signer-keystore feature. No new crypto crates added. - Password-protected keystore.json replaces plaintext key in config.json - New wallet export command to decrypt and print key for backup - Auto-migration from plaintext config on first use after upgrade - POLYMARKET_PASSWORD env var for non-interactive use (scripts/CI) - 3-retry on wrong password, hidden terminal input via rpassword Closes Polymarket#18 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
811d02f to
05a728c
Compare
- Move CLI flag and env var checks before migration block in resolve_key_string so --private-key is never overridden by migration - Add Keystore variant to KeySource so wallet show reports "encrypted keystore" instead of "config file" - Setup wizard now checks keystore_exists() and decrypts to detect existing wallet, preventing silent overwrite of keystore.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Keep the source from resolve_key() instead of re-deriving it from keystore_exists() heuristic. Correctly shows "POLYMARKET_PRIVATE_KEY env var" when the key came from the env var. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
src/config.rs
Outdated
| } | ||
|
|
||
| /// Decrypt keystore.json and return the private key as 0x-prefixed hex. | ||
| pub fn load_key_encrypted(password: &str) -> Result<String> { |
There was a problem hiding this comment.
mind using &SecretString here (and anywhere we're using a password/private key in the changeset) -- we're going to add that more liberally throughout the rest of the codebase
Author
There was a problem hiding this comment.
Done in 2e81558. Added secrecy = "0.10" as a direct dep (already compiled via alloy) and wrapped all password/private key strings in SecretString across the changeset:
password.rs—prompt_passwordandprompt_new_passwordreturnSecretStringconfig.rs—save_key_encrypted,load_key_encrypted,migrate_to_encrypted,resolve_keyall useSecretStringparams/returnsauth.rs—resolve_key_stringreturnsSecretString, callers use.expose_secret()at point of usewallet.rs/setup.rs— callsites updated, keys wrapped inSecretStringimmediately after generation/import
Wrap all password and private key strings in secrecy::SecretString to zeroize memory on drop and prevent accidental Debug/Display leaks. Requires explicit .expose_secret() for access, making secret usage auditable across the codebase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #18
Encrypt private keys at rest using Alloy's built-in keystore methods
Problem
Private keys are stored as plaintext in
~/.config/polymarket/config.json. Any process running as the user can read the file and steal funds.Solution
Encrypt private keys using
LocalSigner::encrypt_keystore/decrypt_keystorefrom Alloy (already a dependency — enabled thesigner-keystorefeature). Uses AES-128-CTR + scrypt, the same format as MetaMask, Geth, and Foundry. The only new crate isrpasswordfor hidden terminal input.What changed for users
wallet createconfig.jsonkeystore.jsonwallet importwallet show/wallet addresswallet resetconfig.jsonconfig.jsonandkeystore.jsonsetupwizardwallet createwallet createorder,approve,ctf split, …)wallet export(new)New env var:
POLYMARKET_PASSWORD— supplies the password non-interactively (for scripts/CI).File layout after:
Migration
On first use after upgrade, if
config.jsoncontains a plaintextprivate_keyand nokeystore.jsonexists, the CLI prompts for a password, encrypts the key intokeystore.json, and stripsprivate_keyfromconfig.json. The original command continues without a second password prompt.Key resolution priority (unchanged, pre-existing)
--private-keyCLI flag — plaintext, no passwordPOLYMARKET_PRIVATE_KEYenv var — plaintext, no passwordkeystore.json— encrypted, password requiredDependencies
alloysigner-keystorefeatureLocalSigner::encrypt_keystore/decrypt_keystorerandrpasswordManual testing
All commands use
POLYMARKET_PASSWORDto avoid interactive prompts. Drop it to test the interactive flow.1. Create, inspect, show, export
2. Wrong password
3. JSON output
POLYMARKET_PASSWORD=test123 polymarket wallet show -o json POLYMARKET_PASSWORD=test123 polymarket wallet export -o json4. Import and round-trip
5. --private-key flag bypasses password
polymarket wallet address --private-key 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80 # prints address, no password prompt6. Auto-migration from plaintext
7. Reset
Note
High Risk
Changes core wallet key storage and loading paths (including migration and password prompting), so regressions could lock users out of wallets or break all signing flows despite improved security.
Overview
Private keys are no longer stored in plaintext
config.json; wallet create/import/setup now prompt for a password and write an encryptedkeystore.json(with restrictive file permissions), whileconfig.jsonstores only non-sensitive settings.All key resolution paths (
auth/provider creation and wallet commands) now support decrypting the keystore with up to 3 password retries, add auto-migration of legacy plaintext configs to the keystore on first use, and introducewallet exportto decrypt and print the private key when needed (plusPOLYMARKET_PASSWORDfor non-interactive use).Written by Cursor Bugbot for commit 2e81558. This will update automatically on new commits. Configure here.