feat(mcp): optional at-rest encryption for the wallet keystore - #55
Merged
Conversation
The agent's secret sat in ~/.aifinpay/agent.json as plaintext. Mode 600 keeps
it from OTHER users on the box; it does nothing against malware running as the
same user — which is exactly the threat the OSINT write-up of a compromised dev
machine described: an infostealer that grabs local keys. A card/cloud key would
be behind an OS keychain; ours was a base58 string in a JSON file.
Set AIFINPAY_WALLET_PASSPHRASE and the keystore is encrypted at rest:
scrypt (N=2^15) to stretch the passphrase, AES-256-GCM so a tampered file is
detected rather than decrypting to a wrong-but-valid-looking key. Salt and IV
are stored alongside; they are uniqueness, not secrets.
Opt-in and env-supplied, both deliberate. Prompting interactively would break
the non-interactive `npx @aifinpay/mcp` path the MCP client launches, and
forcing encryption would break every existing plaintext keystore. No passphrase
=> plaintext, byte-for-byte as before, and the init output says which mode it
wrote.
The failure modes are where the danger is, so they are the tests:
- a wrong passphrase THROWS and leaves the keystore untouched. The silent
version — treat decrypt failure as "no wallet", generate a fresh one — turns
a typo into a permanently unreachable funded key. Verified the file is
byte-identical after the failed attempt.
- an encrypted keystore with no passphrase set refuses to start rather than
falling back to a throwaway identity that looks like the wallet vanished.
- the same passphrase reproduces the same address (encryption that locks the
owner out is worthless).
- the plaintext secret does not appear anywhere in the encrypted file.
scrypt at N=2^15 exceeds Node's default scrypt maxmem, so both call sites pass
maxmem explicitly — without it the whole thing throws at runtime, which the
tests caught before it shipped.
mcp suite: 79 passing. Version 2.0.0-rc.3 -> rc.4.
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
1 participant
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.
Agent-flow audit, point 4. The agent's secret sat in
~/.aifinpay/agent.jsonas plaintext.Mode 600 keeps it from other users on the box. It does nothing against malware running as the same user — which is exactly the threat the OSINT write-up of a compromised dev machine described: an infostealer grabbing local keys. A cloud key would be behind an OS keychain; ours was a base58 string in a JSON file.
The fix
Set
AIFINPAY_WALLET_PASSPHRASE→ the keystore is encrypted at rest: scrypt (N=2¹⁵) to stretch the passphrase, AES-256-GCM so a tampered file is detected rather than decrypting to a wrong-but-valid-looking key.Opt-in and env-supplied, both deliberate. Prompting would break the non-interactive
npx @aifinpay/mcplaunch path; forcing it would break every existing plaintext keystore. No passphrase => plaintext, byte-for-byte as before, andinitsays which mode it wrote.The failure modes are the tests
Also: scrypt at N=2¹⁵ exceeds Node's default
maxmem, so both call sites pass it explicitly — without it the whole thing throws at runtime, which the tests caught before it shipped.mcp suite 79 passing. Version rc.3 → rc.4.