Repository navigation
fix: harden the file-write path guard (#103) - #112
Merged
Merged
Conversation
Two problems, one cause: the flag was applied by deleting tools rather than by gating the sending path. 1. The guard in manage_drafts(action="send") never fired. compose.py did `from apple_mail_mcp.server import READ_ONLY`, binding the value False at import, while __main__ assigns server.READ_ONLY afterwards. The module now reads the flag through the server module at call time. 2. --read-only removed compose_email and reply_to_email outright, even though their "draft" and "open" modes never deliver anything. Those two are also the only tools in the compose surface that accept attachments, so read-only mode left no supported way to attach a file to a draft. They now stay registered and refuse mode="send" themselves. forward_email has no non-sending mode and is still removed. Adds regression coverage for both, including one test that fails if the flag is ever bound at import time again.
Replace the three copy-pasted sensitive-dir denylists with one shared module, apple_mail_mcp/paths.py: - validate_write_path: allowlist of ~/Downloads, ~/Desktop, ~/Documents, ~/Library/Caches/apple-mail-mcp and the system temp dir, plus opt-in APPLE_MAIL_MCP_WRITE_DIRS. Hidden components inside a root are rejected. Used by save_email_attachment, export_emails and (newly) create_rich_email_draft's output_path. - validate_read_path: attachments may come from $HOME except hidden entries and ~/Library internals (iCloud Drive and CloudStorage allowed). Both resolve symlinks and .. first and compare case-insensitively (APFS). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rebased onto current main; CHANGELOG.md taken from main (entry consolidated at release). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Add core.LOCK_WAIT_SECONDS (30s) as the mail_automation_lock default and use it in run_applescript instead of reusing the AppleScript timeout, so the worst case is no longer ~2x the script timeout. - Catch the lock TimeoutError next to subprocess.TimeoutExpired in _send_html_email, reply_to_email and forward_email and return a clean 'Error: Mail automation is busy...' string instead of raising. - Tests for both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the compose tools Rebased onto current main; CHANGELOG.md taken from main (entry consolidated at release). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nder read-only - test_reply_still_drafts now also patches compose.subprocess.run (returncode 0, stdout b"DRAFT SAVED"): reply_to_email runs its main script through osascript directly, so the test failed on Linux CI with FileNotFoundError and drove the real Mail.app on macOS. - forward_email refuses with an error when server.READ_ONLY is set, as defense in depth in case the --read-only tool removal ever fails. Test covers both the plain and the message (direct osascript) paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rework the shared path guard: writes and attachments are allowed anywhere by default except genuinely dangerous targets (dotfiles in $HOME, .git/hooks and .git/config anywhere, autostart/persistence and PATH folders for writes; keys, credentials, keychains and browser profiles for attachments). APPLE_MAIL_MCP_WRITE_DIRS / APPLE_MAIL_MCP_ATTACH_DIRS or the repeatable --write-dir / --attach-dir flags opt into an allowlist, with the denylist still applied inside it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve compose.py import conflict (keep build_mailbox_ref and the lock imports). Also catch a busy-lock TimeoutError while re-reading the saved reply draft for #114's verification, so a successful save is reported as a failed verification instead of being masked as a busy error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h-denylist # Conflicts: # CHANGELOG.md # plugin/apple_mail_mcp/tools/compose.py # plugin/apple_mail_mcp/tools/manage.py
fix: serialize concurrent AppleScript calls to Mail.app across plugin processes
…delivery fix: make --read-only block delivery instead of the compose tools
Co-Authored-By: Claude Opus 5.5 <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.
Opened by Claude (AI assistant) on Patrick's behalf.
Problem
The file-path guard added for #19 was a denylist of 8 directories, copy-pasted into
save_email_attachment,export_emailsandcompose._validate_attachment_paths. It missed shell startup files (~/.zshrc,~/.bash_profile, ...),~/.gitconfig,~/bin,~/.local/bin,~/Library/Application Supportand more. Because attachment bytes are attacker-controlled, a prompt-injectedsave_email_attachment(save_path="~/.zshrc")gave code execution on the next shell.create_rich_email_draft(output_path=...)also wrote to any path with no check at all.Fix
One shared module,
apple_mail_mcp/paths.py. Default is broad access with a hardened denylist; users who want more can opt into allowlists.Writes (
save_email_attachment,export_emails,create_rich_email_draft'soutput_path): allowed anywhere except$HOME(covers all shell rc files,~/.ssh,~/.gnupg,~/.aws,~/.configincl.~/.config/git,~/.gitconfig,~/.netrc,~/.envrc,~/.local/bin,~/.claude, and rc files nobody enumerated).git/hooks,.git/configand.envrcin any folder~/Library/{LaunchAgents,LaunchDaemons,StartupItems,Application Support,Application Scripts,Scripts,Preferences,Keychains,Mail,Services,Containers,Group Containers,...},/Library/{LaunchAgents,LaunchDaemons,StartupItems,Application Support,Preferences,Scripts}PATHfolders and system config:~/bin,/usr/local/bin,/opt/homebrew/bin(+sbin),/etcAttachments (
compose_email,reply_to_email): any readable file except secrets:~/.ssh,~/.gnupg,~/.aws,~/.azure,~/.kube,~/.docker,~/.netrc,~/.git-credentials,~/.npmrc,~/.pypirc,~/.config/gh,~/.config/gcloud,.envfiles,~/Library/Keychains,~/Library/Cookies, and browser/password-manager profiles (Safari, Chrome, Chromium, Edge, Brave, Arc, Vivaldi, Firefox, 1Password).Opt-in restriction:
APPLE_MAIL_MCP_WRITE_DIRS/APPLE_MAIL_MCP_ATTACH_DIRS(colon-separated), or the repeatable--write-dir/--attach-dirCLI flags (next to--read-only), turn that side into an allowlist of only those folders. The denylist still applies inside them, and the tool's own cache dir (default rich-draft location) stays writable.Both validators resolve
~,..and symlinks withrealpathbefore checking, and compare case-insensitively because APFS is case-insensitive by default.Behavior change vs
main: writes and attachments outside$HOME(e.g. the temp dir) now work; writes into the listed~/Libraryfolders and~/bin, previously allowed, are refused. Documented in the README ("File Access") and CHANGELOG.Not included: the issue's optional
overwrite=Falsedefault (point 3). It would change the tool signature; happy to add it as a follow-up.Tests
tests/test_path_guard.pywith a fake$HOME: shell startup and auth files, dot-dirs,.git/hooks/.git/config/.envrcanywhere, user and system autostart folders,PATHdirs,..traversal, symlink escapes, case variants, broad defaults (ordinary folders, outside$HOME, hidden-but-harmless attachments), env and CLI allowlists with the denylist still applying inside, read-side secrets, and wiring tests proving each tool rejects before any AppleScript runs or any file is written.Fixes #103.
🤖 Generated with Claude Code