Skip to content

Prototype: Add router for drafting expenses from emails routed by Mailgun #11092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Prototype: Add router for drafting expenses from emails routed by Mailgun #11092

wants to merge 1 commit into from
+171 −1

Conversation

@kewitz
Copy link
Contributor

@kewitz kewitz commented Oct 1, 2025 •
edited
Loading

Related to opencollective/opencollective#1862

Requires Mailgun receiving route rule similar to:

match_recipient(".*@expenses.opencollective.com")
store(notify="https://api.opencollective.com/webhooks/mailgun")

Simple prototype setting the basic flow we can use to draft expenses from email. Includes:

  • Prototype a way to route received emails to our API.
  • Draft expense based on attached items.

@kewitz kewitz self-assigned this Oct 1, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 1, 2025
View reviewed changes
server/lib/mailgun.ts
const authenticatedUrl = new URL(attachment.url);
authenticatedUrl.password = config.mailgun.apiKey;
authenticatedUrl.username = 'api';
const response = await fetch(authenticatedUrl.toString());

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
Loading
of this request depends on a
user-provided value
Loading
.

Copilot Autofix

AI 28 days ago

General Fix:
To fix this SSRF, do not blindly fetch the URL given by user input (attachment.url). Instead, ensure the URL being fetched is on an allow-list of trusted Mailgun file storage domains. Only fetch files from Mailgun's official attachment storage endpoints, rejecting any attempts to fetch from other origins. This prevents attackers from abusing this endpoint to make arbitrary requests from your server.

Detailed Fix:

  • Before using attachment.url, parse it as a URL and verify its hostname matches the expected, known host(s) used by Mailgun for file attachments. (e.g., api.mailgun.net, attachments.mailgun.org, or whatever is official/expected for your usage).
  • Reject (throw an error or skip) any attachments whose URLs do not match the required domain.
  • This should be done in processAttachment before performing the fetch.
  • Use the built-in URL parsing, simple string/regex match, or, preferably, a robust allow-list.
  • Add a test to verify the host—that is, only proceed to fetch if authenticatedUrl.hostname exactly matches the allowed host(s).
  • You may also want to restrict the protocol to https:.

No new packages are needed; only add a hostname check and early rejection.

Suggested changeset 1
server/lib/mailgun.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/server/lib/mailgun.ts b/server/lib/mailgun.ts
--- a/server/lib/mailgun.ts
+++ b/server/lib/mailgun.ts
@@ -56,9 +56,23 @@
 
 const processAttachment = async (attachment: MailgunAttachment, user: User): Promise<UploadedFile> => {
   const authenticatedUrl = new URL(attachment.url);
+
+  // Allow-list the host for Mailgun attachments (update as per your Mailgun config)
+  // Typically, Mailgun uses "api.mailgun.net" or regional/vanity domains
+  const allowedHosts = [
+    'api.mailgun.net',
+    'attachments.mailgun.org', // adjust for your Mailgun region/domain if different
+  ];
+  if (!allowedHosts.includes(authenticatedUrl.hostname)) {
+    throw new Error(`Blocked attempt to fetch file from non-allowed host: ${authenticatedUrl.hostname}`);
+  }
+  if (authenticatedUrl.protocol !== 'https:') {
+    throw new Error(`Insecure protocol for attachment fetch: ${authenticatedUrl.protocol}`);
+  }
+
   authenticatedUrl.password = config.mailgun.apiKey;
   authenticatedUrl.username = 'api';
-  // TODO: Add more validation that the attachments is stored in Mailgun
+  // Only fetch if URL is allow-listed
   const response = await fetch(authenticatedUrl.toString());
 
   if (!response.ok) {
EOF
@@ -56,9 +56,23 @@

const processAttachment = async (attachment: MailgunAttachment, user: User): Promise<UploadedFile> => {
const authenticatedUrl = new URL(attachment.url);

// Allow-list the host for Mailgun attachments (update as per your Mailgun config)
// Typically, Mailgun uses "api.mailgun.net" or regional/vanity domains
const allowedHosts = [
'api.mailgun.net',
'attachments.mailgun.org', // adjust for your Mailgun region/domain if different
];
if (!allowedHosts.includes(authenticatedUrl.hostname)) {
throw new Error(`Blocked attempt to fetch file from non-allowed host: ${authenticatedUrl.hostname}`);
}
if (authenticatedUrl.protocol !== 'https:') {
throw new Error(`Insecure protocol for attachment fetch: ${authenticatedUrl.protocol}`);
}

authenticatedUrl.password = config.mailgun.apiKey;
authenticatedUrl.username = 'api';
// TODO: Add more validation that the attachments is stored in Mailgun
// Only fetch if URL is allow-listed
const response = await fetch(authenticatedUrl.toString());

if (!response.ok) {
Copilot is powered by AI and may make mistakes. Always verify output.
@kewitz kewitz force-pushed the proto/email-expense branch from 8f6cc6e to 6b7fea2 Compare October 1, 2025 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@kewitz kewitz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kewitz