Skip to content

Commit 0b51aaa

Browse files
idoshamunclaudegithub-actions[bot]
authored
fix(security): proxy external markdown images via Cloudinary (#3542)
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Ido Shamun <idoshamun@users.noreply.github.com>
1 parent afcb0f4 commit 0b51aaa

File tree

3 files changed

+327
-0
lines changed

3 files changed

+327
-0
lines changed

__tests__/common/markdown.ts

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
import cloudinary from 'cloudinary';
2+
import { markdown } from '../../src/common/markdown';
3+
4+
const configureCloudinary = () => {
5+
cloudinary.v2.config({
6+
cloud_name: 'daily-now',
7+
api_key: 'test',
8+
api_secret: 'test',
9+
});
10+
};
11+
12+
describe('markdown image proxy integration', () => {
13+
const originalEnv = process.env;
14+
15+
beforeEach(() => {
16+
jest.resetModules();
17+
process.env = { ...originalEnv };
18+
process.env.CLOUDINARY_URL = 'cloudinary://test:test@daily-now';
19+
configureCloudinary();
20+
});
21+
22+
afterAll(() => {
23+
process.env = originalEnv;
24+
});
25+
26+
describe('image rendering', () => {
27+
it('should proxy external image URLs in markdown', () => {
28+
const content = '![alt text](https://example.com/image.png)';
29+
const result = markdown.render(content);
30+
31+
expect(result).toContain('media.daily.dev');
32+
expect(result).toContain('/image/fetch/');
33+
expect(result).toContain('s--'); // signature
34+
});
35+
36+
it('should not proxy images from media.daily.dev', () => {
37+
const content = '![alt text](https://media.daily.dev/image.png)';
38+
const result = markdown.render(content);
39+
40+
expect(result).toContain('https://media.daily.dev/image.png');
41+
expect(result).not.toContain('/image/fetch/');
42+
});
43+
44+
it('should not proxy images from res.cloudinary.com', () => {
45+
const content =
46+
'![alt text](https://res.cloudinary.com/daily-now/image/upload/image.png)';
47+
const result = markdown.render(content);
48+
49+
expect(result).toContain(
50+
'https://res.cloudinary.com/daily-now/image/upload/image.png',
51+
);
52+
});
53+
54+
it('should block images from private IPs', () => {
55+
const content = '![alt text](http://127.0.0.1/image.png)';
56+
const result = markdown.render(content);
57+
58+
expect(result).toContain('src=""');
59+
expect(result).not.toContain('127.0.0.1');
60+
});
61+
62+
it('should block images from localhost', () => {
63+
const content = '![alt text](http://localhost/image.png)';
64+
const result = markdown.render(content);
65+
66+
expect(result).toContain('src=""');
67+
expect(result).not.toContain('localhost');
68+
});
69+
70+
it('should not render file:// protocol URLs as images', () => {
71+
const content = '![alt text](file:///etc/passwd)';
72+
const result = markdown.render(content);
73+
74+
// Markdown-it does not render file:// URLs as images, just plain text
75+
// This is safe since the path is not loaded as an image
76+
expect(result).not.toContain('<img');
77+
});
78+
79+
it('should preserve alt text in proxied images', () => {
80+
const content = '![my alt text](https://example.com/image.png)';
81+
const result = markdown.render(content);
82+
83+
expect(result).toContain('alt="my alt text"');
84+
});
85+
86+
it('should handle multiple images in same content', () => {
87+
const content = `
88+
![image1](https://example.com/1.png)
89+
90+
Some text here
91+
92+
![image2](https://evil.com/2.gif)
93+
`;
94+
const result = markdown.render(content);
95+
96+
// Both images should be proxied through Cloudinary
97+
expect(result.match(/media\.daily\.dev/g)?.length).toBe(2);
98+
expect(result).toContain('/image/fetch/');
99+
});
100+
101+
it('should handle images in links', () => {
102+
const content =
103+
'[![alt](https://example.com/image.png)](https://link.com)';
104+
const result = markdown.render(content);
105+
106+
expect(result).toContain('media.daily.dev');
107+
expect(result).toContain('href="https://link.com"');
108+
});
109+
110+
it('should handle data URIs without proxying', () => {
111+
const dataUri = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAAB';
112+
const content = `![alt](${dataUri})`;
113+
const result = markdown.render(content);
114+
115+
expect(result).toContain(dataUri);
116+
});
117+
});
118+
119+
describe('without Cloudinary configured', () => {
120+
beforeEach(() => {
121+
delete process.env.CLOUDINARY_URL;
122+
});
123+
124+
it('should pass through external URLs when Cloudinary is not configured', () => {
125+
const content = '![alt](https://example.com/image.png)';
126+
const result = markdown.render(content);
127+
128+
expect(result).toContain('https://example.com/image.png');
129+
});
130+
});
131+
});

src/common/imageProxy.ts

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
import cloudinary from 'cloudinary';
2+
import { mapCloudinaryUrl } from './cloudinary';
3+
4+
/**
5+
* Allowed domains that don't need proxying (already hosted by us)
6+
*/
7+
const ALLOWED_DOMAINS = [
8+
'media.daily.dev',
9+
'res.cloudinary.com',
10+
'daily-now-res.cloudinary.com',
11+
];
12+
13+
/**
14+
* Private/internal IP ranges that should be blocked (SSRF prevention)
15+
*/
16+
const PRIVATE_IP_PATTERNS = [
17+
// IPv4 private ranges
18+
/^127\./,
19+
/^10\./,
20+
/^192\.168\./,
21+
/^172\.(1[6-9]|2[0-9]|3[0-1])\./,
22+
// IPv4 link-local
23+
/^169\.254\./,
24+
// Localhost variations
25+
/^0\.0\.0\.0/,
26+
/^localhost$/i,
27+
// IPv6 loopback and private
28+
/^::1$/,
29+
/^fe80:/i,
30+
/^fc00:/i,
31+
/^fd00:/i,
32+
];
33+
34+
/**
35+
* Maximum URL length to prevent abuse
36+
*/
37+
const MAX_URL_LENGTH = 2048;
38+
39+
/**
40+
* Checks if a hostname is a private/internal IP address
41+
*/
42+
export function isPrivateIP(hostname: string): boolean {
43+
return PRIVATE_IP_PATTERNS.some((pattern) => pattern.test(hostname));
44+
}
45+
46+
/**
47+
* Checks if a URL is from an allowed domain that doesn't need proxying
48+
*/
49+
export function isAllowedDomain(url: string): boolean {
50+
try {
51+
const parsedUrl = new URL(url);
52+
return ALLOWED_DOMAINS.some(
53+
(domain) =>
54+
parsedUrl.hostname === domain ||
55+
parsedUrl.hostname.endsWith(`.${domain}`),
56+
);
57+
} catch {
58+
return false;
59+
}
60+
}
61+
62+
/**
63+
* Validates that a URL is safe to proxy
64+
* Returns an error message if invalid, or null if valid
65+
*
66+
* Note: This assumes the URL is already http/https since it's called after
67+
* isExternalImageUrl check which filters out non-http/https URLs.
68+
*/
69+
export function validateImageUrl(url: string): string | null {
70+
// Check URL length
71+
if (url.length > MAX_URL_LENGTH) {
72+
return 'URL exceeds maximum length';
73+
}
74+
75+
try {
76+
const parsedUrl = new URL(url);
77+
78+
// Block private/internal IP addresses (SSRF prevention)
79+
if (isPrivateIP(parsedUrl.hostname)) {
80+
return 'Private IP addresses are not allowed';
81+
}
82+
83+
return null;
84+
} catch {
85+
return 'Invalid URL format';
86+
}
87+
}
88+
89+
/**
90+
* Checks if a URL is an external image URL that needs proxying
91+
* Returns true for external http/https URLs, false for everything else
92+
* (data URIs, relative URLs, and allowed domains)
93+
*
94+
* Note: Invalid protocols like file:// are NOT considered external URLs.
95+
* They are handled by validateImageUrl which rejects them.
96+
*/
97+
export function isExternalImageUrl(url: string): boolean {
98+
// Skip data URIs
99+
if (url.startsWith('data:')) {
100+
return false;
101+
}
102+
103+
// Only consider http/https URLs as external (relative URLs and other protocols are skipped)
104+
if (!url.startsWith('http://') && !url.startsWith('https://')) {
105+
return false;
106+
}
107+
108+
// Skip URLs from allowed domains
109+
if (isAllowedDomain(url)) {
110+
return false;
111+
}
112+
113+
return true;
114+
}
115+
116+
/**
117+
* Generates a signed Cloudinary fetch URL for proxying an external image
118+
*
119+
* Uses Cloudinary's fetch feature with signed URLs to:
120+
* 1. Cache the image on Cloudinary's CDN
121+
* 2. Prevent direct requests from users to external servers (IP privacy)
122+
* 3. Apply automatic format and quality optimization
123+
*
124+
* @param externalUrl The external image URL to proxy
125+
* @returns The signed Cloudinary fetch URL, or null if the URL is invalid/blocked
126+
*/
127+
export function getProxiedImageUrl(externalUrl: string): string | null {
128+
// Skip if not an external URL
129+
if (!isExternalImageUrl(externalUrl)) {
130+
return externalUrl;
131+
}
132+
133+
// Validate the URL
134+
const validationError = validateImageUrl(externalUrl);
135+
if (validationError) {
136+
return null;
137+
}
138+
139+
// Skip if Cloudinary is not configured
140+
if (!process.env.CLOUDINARY_URL) {
141+
return externalUrl;
142+
}
143+
144+
try {
145+
// Generate a signed Cloudinary fetch URL
146+
const cloudinaryUrl = cloudinary.v2.url(externalUrl, {
147+
type: 'fetch',
148+
sign_url: true,
149+
secure: true,
150+
fetch_format: 'auto',
151+
quality: 'auto',
152+
});
153+
154+
// Map to media.daily.dev domain
155+
return mapCloudinaryUrl(cloudinaryUrl);
156+
} catch {
157+
// If Cloudinary URL generation fails, return original URL
158+
// to avoid breaking the content
159+
return externalUrl;
160+
}
161+
}

src/common/markdown.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { MentionedUser } from '../schema/comments';
77
import { EntityTarget } from 'typeorm/common/EntityTarget';
88
import { ghostUser } from './utils';
99
import { isValidHttpUrl } from './links';
10+
import { getProxiedImageUrl } from './imageProxy';
1011

1112
/**
1213
* Sanitizes HTML content, allowing only safe tags for rich text content.
@@ -211,6 +212,40 @@ markdown.renderer.rules.link_open = function (tokens, idx, options, env, self) {
211212
return defaultRender(tokens, idx, options, env, self);
212213
};
213214

215+
/**
216+
* Store the default image renderer for markdown-it
217+
*/
218+
const defaultImageRender =
219+
markdown.renderer.rules.image ||
220+
function (tokens, idx, options, env, self) {
221+
return self.renderToken(tokens, idx, options);
222+
};
223+
224+
/**
225+
* Custom image renderer that proxies external images through Cloudinary
226+
* to prevent IP address exposure when users view markdown content with
227+
* external images.
228+
*/
229+
markdown.renderer.rules.image = function (tokens, idx, options, env, self) {
230+
const token = tokens[idx];
231+
const srcIndex = token.attrIndex('src');
232+
233+
if (srcIndex >= 0 && token.attrs) {
234+
const originalSrc = token.attrs[srcIndex][1];
235+
const proxiedSrc = getProxiedImageUrl(originalSrc);
236+
237+
if (proxiedSrc) {
238+
token.attrs[srcIndex][1] = proxiedSrc;
239+
} else {
240+
// If the URL is invalid/blocked, remove the src to prevent the image from loading
241+
// This is a security measure to prevent SSRF and other attacks
242+
token.attrs[srcIndex][1] = '';
243+
}
244+
}
245+
246+
return defaultImageRender(tokens, idx, options, env, self);
247+
};
248+
214249
export const saveMentions = (
215250
transaction: DataSource | EntityManager,
216251
referenceId: string,

0 commit comments

Comments
 (0)