Skip to content

feat(config): macOS Keychain credential storage#8

Open
rupjae wants to merge 3 commits into
codefuturist:mainfrom
rupjae:feat/macos-keychain-credentials
Open

feat(config): macOS Keychain credential storage#8
rupjae wants to merge 3 commits into
codefuturist:mainfrom
rupjae:feat/macos-keychain-credentials

Conversation

@rupjae

@rupjae rupjae commented Feb 26, 2026

Copy link
Copy Markdown

Summary

Ran a security audit on this project before deploying it (as one does when handing over email credentials). The code is clean, well-structured, no telemetry, no phoning home, solid input validation. Genuinely impressed.

One thing jumped out though: passwords sitting in plaintext in config.toml with 0644 permissions. Any process on the machine can read them. On macOS, we have Keychain for exactly this.

This PR adds:

  • keychain.ts: macOS Keychain integration via the security CLI. Store/retrieve passwords without touching disk.
  • Config loader hook: When an account has password = "keychain", the loader resolves the real password from Keychain at runtime. Credentials never land in the TOML file.
  • File permission hardening: saveConfig now writes the config dir as 0700 and config file as 0600, even for users who still use plaintext passwords.
  • Updated schema validation and template to document the keychain option.

On non-macOS platforms, behavior is completely unchanged. Existing plaintext passwords still work, just with better file permissions now.

Usage

# Store password in Keychain
security add-generic-password -s email-mcp -a you@example.com -w "your-app-password" -U

# Config just references keychain
# config.toml
password = "keychain"

How we found it

We audited the full codebase before deploying. Read every file, checked all network calls, reviewed dependencies, traced credential flow. The audit came back CAUTION (not SAFE) solely because of the plaintext password issue. Everything else was solid.

We also noticed that Copilot contributed 24 commits with 18,285 lines added (vs Colin's 66 commits). No shade, the code quality is genuinely good regardless of who (or what) wrote it. But we figured if Copilot is going to mass-produce 47 tools, a human should probably circle back and make sure the credentials aren't just vibing in a world-readable file. Consider this that circle-back.

Test plan

  • All 150 existing tests pass
  • Updated schema test for new error message
  • Tested end-to-end: Keychain store -> config load -> IMAP connect -> send email
  • Verified non-macOS codepath (isKeychainAvailable returns false, falls through to TOML password)
  • Verified 0600 file permissions on saveConfig

🤖 Generated with Claude Code

rupjae and others added 2 commits February 26, 2026 13:34
Passwords stored as plaintext in config.toml is a security concern,
especially since the file is written with default permissions (0644).
This patch adds macOS Keychain integration so credentials never
touch disk, and hardens file permissions as a defense-in-depth measure.

- Add keychain.ts: store/retrieve passwords via macOS `security` CLI
- Config loader resolves password = "keychain" from Keychain at runtime
- saveConfig now writes config dir as 0700 and config file as 0600
- Schema validation updated to mention keychain option
- Template default changed to password = "keychain"

On non-macOS platforms, behavior is unchanged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rupjae rupjae requested a review from codefuturist as a code owner February 26, 2026 18:35
wgs4 referenced this pull request in wgs4/email-mcp Mar 9, 2026
Fixes critical bug where APPEND targeted a ghost "Sent" folder instead
of the account's actual INBOX.Sent (with \Sent special-use attribute).
Also addresses 8 additional issues from code review.

Bug fixes:
- #0: Fix resolveSentFolder to properly detect SPECIAL-USE \Sent,
  reorder fallback names (INBOX.Sent first), throw on unresolvable
- #1: Add mailbox lock around IMAP APPEND operations
- #2: Add Content-Transfer-Encoding: 8bit header
- #3: Normalize body line endings to CRLF per RFC 5322
- #4: Generate fallback Message-ID when nodemailer returns none
- #5: Replace unsafe type casts with runtime guards
- #6: RFC 2047 encode non-ASCII subject lines
- #7: Auto-create Sent folder on TRYCREATE with single retry
- #8: Add sendDraft APPEND test coverage (3 new tests)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jhampton

jhampton commented Apr 1, 2026

Copy link
Copy Markdown

Hi @wgs4 - looks like we share some concerns. I have a few more changes in #24, and I'll take a look to see if I can merge our approaches in my local fork. Thank you for taking the time to protect yourself and others – A+ work.

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.

2 participants