Skip to content

fix(security): restrict email attachments to GMAIL_ATTACHMENT_DIR#19

Draft
nweisenfeld wants to merge 1 commit into
ArtyMcLabin:mainfrom
nweisenfeld:nweisenfeld/avoid-outgoing-leaks
Draft

fix(security): restrict email attachments to GMAIL_ATTACHMENT_DIR#19
nweisenfeld wants to merge 1 commit into
ArtyMcLabin:mainfrom
nweisenfeld:nweisenfeld/avoid-outgoing-leaks

Conversation

@nweisenfeld

Copy link
Copy Markdown

Attachment file paths were previously unconstrained, allowing prompt injection attacks to exfiltrate arbitrary local files (e.g. SSH keys, OAuth credentials) as email attachments.

Now requires GMAIL_ATTACHMENT_DIR to be set; attachments are disabled by default. Any path outside the configured directory is rejected via resolved-path prefix check, preventing traversal attacks.

Attachment file paths were previously unconstrained, allowing prompt
injection attacks to exfiltrate arbitrary local files (e.g. SSH keys,
OAuth credentials) as email attachments.

Now requires GMAIL_ATTACHMENT_DIR to be set; attachments are disabled
by default. Any path outside the configured directory is rejected via
resolved-path prefix check, preventing traversal attacks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ArtyMcLabin

Copy link
Copy Markdown
Owner

thanks, sounds like it would overly restrict many users, but would be useful for power-users of security. how do you suggest we handle that? @nweisenfeld

@ArtyMcLabin

Copy link
Copy Markdown
Owner

@nweisenfeld Friendly ping — the design question above about opt-in vs breaking-default is still open. We like the implementation but want your input on the UX tradeoff before merging.

Due date: 2026-06-16 — If no response by then, we'll close this PR. You're welcome to reopen or submit a new PR anytime after that.

ArtyMcLabin added a commit that referenced this pull request Apr 16, 2026
Runs daily at 06:00 UTC. If @nweisenfeld hasn't responded to the
design question since April 16 2026, closes the PR with a message.
Self-cleans the workflow file after closing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nweisenfeld

Copy link
Copy Markdown
Author

thanks, sounds like it would overly restrict many users, but would be useful for power-users of security. how do you suggest we handle that? @nweisenfeld

Hey @ArtyMcLabin . First off, I appreciate the work that you've done maintaining this project. I hadn't intended to forward this your way quite yet and forgot that the PR would, by default, target the upstream.

Since this would be a big, breaking change for many people using this in production, it likely would be better to have it apply no restrictions if the env var is not set and just document its presence. If there's a "best practices" writeup somewhere, it could be recommended as a way to avoid potential document leaks from anywhere.

If that makes sense to you, I'm happy to update the code and will mark this as "Ready for review" once done.

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