diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 5d864e1b6e..5c7ab51cbf 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -1,3 +1,11 @@ +map $host $upload_image_origins { + localhost "http://localhost:5000"; + 127.0.0.1 "http://localhost:5000"; + ~^app-dev\.(.+)$ "https://api-dev.$1 http://api-dev.$1"; + ~^app\.(.+)$ "https://api.$1 http://api.$1"; + default "https://api.$host http://api.$host"; +} + server { listen 80; @@ -11,6 +19,10 @@ server { root /usr/share/nginx/html; index index.html; + # The API origin serves signed uploads. NGINX locations with their own + # add_header directives repeat this policy because inheritance stops there. + add_header Content-Security-Policy "img-src 'self' $upload_image_origins data: blob:" always; + # Keep the correct HTTP status while giving people a useful recovery path. # `internal` prevents /404.html itself from becoming a crawlable public URL. error_page 404 /404.html; @@ -44,6 +56,7 @@ server { # (Without this, index.html had no Cache-Control at all → heuristic caching.) location = /index.html { add_header Cache-Control "no-cache, no-store, must-revalidate" always; + add_header Content-Security-Policy "img-src 'self' $upload_image_origins data: blob:" always; expires -1; } @@ -54,5 +67,6 @@ server { expires 6M; access_log off; add_header Cache-Control "public, immutable"; + add_header Content-Security-Policy "img-src 'self' $upload_image_origins data: blob:" always; } } diff --git a/frontend/scripts/nginx-routing.test.mjs b/frontend/scripts/nginx-routing.test.mjs index f9bd06a4d8..797a7d4949 100644 --- a/frontend/scripts/nginx-routing.test.mjs +++ b/frontend/scripts/nginx-routing.test.mjs @@ -22,3 +22,32 @@ test('unknown public routes are real 404s while documented app routes retain SPA assert.match(notFoundPage, /

We couldn’t find that page<\/h1>/); assert.match(notFoundPage, /href="\/guides\/"/); }); + +test('HTML and inherited NGINX locations enforce the same image source policy', async () => { + const config = await readFile(resolve(frontendDir, 'nginx.conf'), 'utf8'); + assert.match(config, /map \$host \$upload_image_origins \{/); + // Every map value is an API origin for that frontend host. A new map arm + // cannot quietly permit a third-party image host via the variable. + const originMap = config.match(/map \$host \$upload_image_origins \{([^}]+)\}/)?.[1]; + assert.ok(originMap); + assert.deepEqual(originMap.trim().split('\n').map((line) => line.trim()), [ + 'localhost "http://localhost:5000";', + '127.0.0.1 "http://localhost:5000";', + '~^app-dev\\.(.+)$ "https://api-dev.$1 http://api-dev.$1";', + '~^app\\.(.+)$ "https://api.$1 http://api.$1";', + 'default "https://api.$host http://api.$host";', + ]); + const policy = /add_header Content-Security-Policy "img-src 'self' \$upload_image_origins data: blob:" always;/g; + // Server scope, index.html (which adds Cache-Control), and asset location + // (which also adds Cache-Control) each need their own copy. + assert.equal([...config.matchAll(policy)].length, 3); + assert.doesNotMatch(config, /img-src[^"\n]*\*/); + const imagePolicies = [...config.matchAll(/Content-Security-Policy "img-src ([^"\n]+)"/g)]; + assert.equal(imagePolicies.length, 3); + for (const [, sources] of imagePolicies) { + assert.deepEqual(sources.split(/\s+/), ["'self'", '$upload_image_origins', 'data:', 'blob:']); + } + const chatRoom = await readFile(resolve(frontendDir, 'src/components/ChatRoom.tsx'), 'utf8'); + assert.doesNotMatch(chatRoom, /logo:\s*['"]https?:\/\//); + assert.doesNotMatch(chatRoom, /src\s*=\s*['"]https?:\/\//); +}); diff --git a/frontend/src/components/ChatRoom.css b/frontend/src/components/ChatRoom.css index 9d0b2d852c..094939e6ff 100644 --- a/frontend/src/components/ChatRoom.css +++ b/frontend/src/components/ChatRoom.css @@ -450,6 +450,13 @@ object-fit: contain; } +.external-link-mark { + color: white; + font-size: 12px; + font-weight: 700; + line-height: 1; +} + /* Update search field in sidebar */ .sidebar-section .MuiInputBase-root { border-radius: 4px !important; diff --git a/frontend/src/components/ChatRoom.tsx b/frontend/src/components/ChatRoom.tsx index db7cbff273..ae8501ffb9 100644 --- a/frontend/src/components/ChatRoom.tsx +++ b/frontend/src/components/ChatRoom.tsx @@ -181,7 +181,7 @@ const BASE_INTEGRATION_OPTIONS = [ color: '#5865F2', hoverColor: '#4752C4', description: 'Connect Discord to sync messages with your server.', - logo: 'https://cdn.simpleicons.org/discord/5865F2', + mark: 'D', capabilities: ['gateway', 'summary', 'commands'], }, { @@ -190,7 +190,7 @@ const BASE_INTEGRATION_OPTIONS = [ color: '#4A154B', hoverColor: '#3B1140', description: 'Install the Commonly app and choose channels.', - logo: 'https://a.slack-edge.com/80588/marketing/img/icons/icon_slack_hash_colored.png', + mark: 'S', capabilities: ['webhook', 'summary', 'commands'], }, { @@ -199,7 +199,7 @@ const BASE_INTEGRATION_OPTIONS = [ color: '#00A2FF', hoverColor: '#0089D9', description: 'Connect your GroupMe bot to start syncing.', - logo: 'https://cdn.simpleicons.org/groupme/00A2FF', + mark: 'G', capabilities: ['webhook', 'commands', 'summary'], }, { @@ -208,7 +208,7 @@ const BASE_INTEGRATION_OPTIONS = [ color: '#229ED9', hoverColor: '#1B86BC', description: 'Add the bot to a chat and run /commonly-enable.', - logo: 'https://telegram.org/img/t_logo.png', + mark: 'T', capabilities: ['webhook', 'summary', 'commands'], }, { @@ -217,7 +217,7 @@ const BASE_INTEGRATION_OPTIONS = [ color: '#111827', hoverColor: '#0B0F1A', description: 'Pull posts from X into this pod.', - logo: 'https://cdn.simpleicons.org/x/111827', + mark: 'X', capabilities: ['polling', 'posts', 'summary'], }, { @@ -226,7 +226,7 @@ const BASE_INTEGRATION_OPTIONS = [ color: '#E4405F', hoverColor: '#C13584', description: 'Pull Instagram posts into this pod.', - logo: 'https://cdn.simpleicons.org/instagram/E4405F', + mark: 'I', capabilities: ['polling', 'posts', 'summary'], }, ]; @@ -3340,64 +3340,28 @@ const ChatRoom = () => { {/* Discord icon */} {(link.type === 'discord' || link.icon === 'discord') && (
- Discord { - (e.target as HTMLImageElement).onerror = null; - ((e.target as HTMLImageElement).parentNode as HTMLElement).innerHTML = ''; - }} - /> +
)} {/* Telegram icon */} {(link.type === 'telegram' || link.icon === 'telegram') && (
- Telegram { - (e.target as HTMLImageElement).onerror = null; - ((e.target as HTMLImageElement).parentNode as HTMLElement).innerHTML = ''; - }} - /> +
)} {/* WeChat icon */} {(link.type === 'wechat' || link.icon === 'wechat') && (
- WeChat { - (e.target as HTMLImageElement).onerror = null; - ((e.target as HTMLImageElement).parentNode as HTMLElement).innerHTML = ''; - }} - /> +
)} {/* GroupMe icon */} {(link.type === 'groupme' || link.icon === 'groupme') && (
- GroupMe { - (e.target as HTMLImageElement).onerror = null; - ((e.target as HTMLImageElement).parentNode as HTMLElement).innerHTML = ''; - }} - /> +
)} @@ -3550,12 +3514,9 @@ const ChatRoom = () => { borderRadius: 2 }} > - + diff --git a/frontend/src/components/common/MarkdownContent.images.test.tsx b/frontend/src/components/common/MarkdownContent.images.test.tsx new file mode 100644 index 0000000000..0d579ff414 --- /dev/null +++ b/frontend/src/components/common/MarkdownContent.images.test.tsx @@ -0,0 +1,34 @@ +// @ts-nocheck +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import MarkdownContent from './MarkdownContent'; + +jest.mock('react-markdown', () => { + const ReactForMock = require('react'); + return { + __esModule: true, + default: ({ children, components }) => { + const image = /^!\[([^\]]*)\]\(([^)]+)\)$/.exec(String(children)); + return image + ? ReactForMock.createElement(components.img, { alt: image[1], src: image[2] }) + : ReactForMock.createElement('div', null, children); + }, + }; +}); + +describe('legacy MarkdownContent image sources', () => { + const previousApiUrl = process.env.REACT_APP_API_URL; + beforeEach(() => { process.env.REACT_APP_API_URL = 'https://api.commonly.me'; }); + afterEach(() => { process.env.REACT_APP_API_URL = previousApiUrl; }); + + test('foreign markdown image becomes a link with no image request', () => { + render({'![chart](https://outside.example/pixel.png?d=secret)'}); + expect(screen.queryByRole('img')).toBeNull(); + expect(screen.getByRole('link', { name: 'chart' })).toHaveAttribute('href', 'https://outside.example/pixel.png?d=secret'); + }); + + test('same-instance uploads still render', () => { + render({'![chart](https://api.commonly.me/api/uploads/chart.png)'}); + expect(screen.getByRole('img', { name: 'chart' })).toHaveAttribute('src', 'https://api.commonly.me/api/uploads/chart.png'); + }); +}); diff --git a/frontend/src/components/common/MarkdownContent.tsx b/frontend/src/components/common/MarkdownContent.tsx index 65051856f8..92d71c153f 100644 --- a/frontend/src/components/common/MarkdownContent.tsx +++ b/frontend/src/components/common/MarkdownContent.tsx @@ -1,6 +1,7 @@ import React from 'react'; import ReactMarkdown from 'react-markdown'; import { Box, Typography } from '@mui/material'; +import { messageImageUrl } from '../../utils/messageImageUrl'; const HASHTAG_RE = /(#\w+)/g; @@ -61,6 +62,14 @@ export default function MarkdownContent({ // Using Record for component props since react-markdown v10 // types vary by component key and we only need runtime behaviour here. const components: Record>> = { + img({ src, alt }) { + const source = String(src || ''); + const trustedSrc = messageImageUrl(source); + if (trustedSrc) return ; + return /^https?:\/\//i.test(source) + ? {String(alt || source)} + : null; + }, p({ children: pChildren }) { const content = onHashtagClick ? renderWithHashtags(childrenToText(pChildren as React.ReactNode), onHashtagClick) diff --git a/frontend/src/components/common/MessageContent.test.tsx b/frontend/src/components/common/MessageContent.test.tsx index 61241f74f9..04b6c59687 100644 --- a/frontend/src/components/common/MessageContent.test.tsx +++ b/frontend/src/components/common/MessageContent.test.tsx @@ -18,6 +18,9 @@ jest.mock('@mui/material', () => { }); describe('MessageContent', () => { + const previousApiUrl = process.env.REACT_APP_API_URL; + beforeEach(() => { process.env.REACT_APP_API_URL = 'https://api.commonly.me'; }); + afterEach(() => { process.env.REACT_APP_API_URL = previousApiUrl; }); it('renders nothing when no children', () => { const { container } = render(); expect(container.firstChild).toBeNull(); @@ -28,16 +31,16 @@ describe('MessageContent', () => { expect(screen.getByTestId('markdown')).toHaveTextContent('Hello world'); }); - it('renders inline image for png URL in text', () => { + it('leaves a foreign png URL as a link instead of requesting it', () => { const content = 'Check this out https://example.com/image.png cool right?'; render({content}); - const img = screen.getByRole('img', { name: 'Shared image' }); - expect(img).toHaveAttribute('src', 'https://example.com/image.png'); + expect(screen.queryByRole('img', { name: 'Shared image' })).toBeNull(); + expect(screen.getByRole('link', { name: 'https://example.com/image.png' })).toHaveAttribute('href', 'https://example.com/image.png'); }); - it('renders inline image for jpg URL in text', () => { - render({'Photo: https://cdn.example.com/photo.jpg'}); - expect(screen.getByRole('img')).toBeInTheDocument(); + it('renders an instance upload in message prose as an image', () => { + render({'Photo: https://api.commonly.me/api/uploads/photo.jpg'}); + expect(screen.getByRole('img', { name: 'Shared image' })).toHaveAttribute('src', 'https://api.commonly.me/api/uploads/photo.jpg'); }); it('does not render image for non-image URLs', () => { @@ -70,7 +73,7 @@ describe('MessageContent', () => { }); it('deduplicates repeated image URLs', () => { - const content = 'https://example.com/a.png and again https://example.com/a.png'; + const content = 'https://api.commonly.me/api/uploads/a.png and again https://api.commonly.me/api/uploads/a.png'; render({content}); const imgs = screen.getAllByRole('img', { name: 'Shared image' }); expect(imgs).toHaveLength(1); diff --git a/frontend/src/components/common/MessageContent.tsx b/frontend/src/components/common/MessageContent.tsx index 600fc89859..db810796ff 100644 --- a/frontend/src/components/common/MessageContent.tsx +++ b/frontend/src/components/common/MessageContent.tsx @@ -12,6 +12,7 @@ import { import CheckCircleOutlineIcon from '@mui/icons-material/CheckCircleOutline'; import OpenInNewIcon from '@mui/icons-material/OpenInNew'; import MarkdownContent from './MarkdownContent'; +import { messageImageUrl } from '../../utils/messageImageUrl'; const IMAGE_URL_RE = /https?:\/\/\S+\.(?:png|jpg|jpeg|gif|webp)(?:\?\S*)?/gi; const TASK_COMPLETE_RE = /\b(TASK-\d{3,})\s+(?:completed?|done|finished|shipped|closed)/i; @@ -25,13 +26,19 @@ interface ImagePreviewProps { function ImagePreview({ src }: ImagePreviewProps): React.ReactElement { const [open, setOpen] = useState(false); const [errored, setErrored] = useState(false); + const trustedSrc = messageImageUrl(src); + if (!trustedSrc) { + return /^https?:\/\//i.test(src) + ? {src} + : <>; + } if (errored) return <>; return ( <> diff --git a/frontend/src/utils/messageImageUrl.ts b/frontend/src/utils/messageImageUrl.ts new file mode 100644 index 0000000000..4bceccce36 --- /dev/null +++ b/frontend/src/utils/messageImageUrl.ts @@ -0,0 +1,21 @@ +import getApiBaseUrl, { normalizeUploadUrl } from './apiBaseUrl'; + +// Message prose is untrusted. Only an upload served by this instance may become +// an image request; external image URLs remain links in the transcript. +export const messageImageUrl = (value: string | null | undefined): string | null => { + if (!value) return null; + try { + const api = new URL(getApiBaseUrl()); + const normalized = value.startsWith('/api/uploads/') || value.startsWith('/uploads/') + ? new URL(value, api).toString() + : normalizeUploadUrl(value); + if (!normalized) return null; + const image = new URL(normalized); + if (image.origin !== api.origin || !['http:', 'https:'].includes(image.protocol)) return null; + if (image.username || image.password) return null; + if (!/^\/(?:api\/)?uploads\/[^/]/.test(image.pathname)) return null; + return image.toString(); + } catch (_error) { + return null; + } +}; diff --git a/frontend/src/v2/__tests__/V2GithubPrCard.test.tsx b/frontend/src/v2/__tests__/V2GithubPrCard.test.tsx new file mode 100644 index 0000000000..2834503b5d --- /dev/null +++ b/frontend/src/v2/__tests__/V2GithubPrCard.test.tsx @@ -0,0 +1,31 @@ +// @ts-nocheck +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import V2GithubPrCard from '../components/V2GithubPrCard'; + +test('GitHub PR contributors keep initials when external avatar images are blocked', async () => { + const previousFetch = global.fetch; + global.fetch = jest.fn() + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + title: 'Guard message images', state: 'open', merged: false, number: 927, + html_url: 'https://github.com/Team-Commonly/commonly/pull/927', + commits: 1, additions: 2, deletions: 0, changed_files: 1, + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => [{ author: { login: 'octocat', avatar_url: 'https://avatars.githubusercontent.com/u/1' } }], + }); + try { + const { container } = render(); + await screen.findByRole('link', { name: 'Pull request #927: Guard message images' }); + const contributor = container.querySelector('.v2-prcard__avatar'); + expect(contributor).toHaveTextContent('O'); + expect(contributor).not.toHaveAttribute('style'); + expect(container.querySelector('img')).toBeNull(); + } finally { + global.fetch = previousFetch; + } +}); diff --git a/frontend/src/v2/__tests__/V2MessageRowAttachments.test.tsx b/frontend/src/v2/__tests__/V2MessageRowAttachments.test.tsx index 5a31d11d09..2061f2ad3d 100644 --- a/frontend/src/v2/__tests__/V2MessageRowAttachments.test.tsx +++ b/frontend/src/v2/__tests__/V2MessageRowAttachments.test.tsx @@ -1,8 +1,25 @@ // @ts-nocheck import React from 'react'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; -import V2MessageRow, { CollapsiblePre } from '../components/V2MessageRow'; +import V2MessageRow, { CollapsiblePre, MessageMarkdownImage } from '../components/V2MessageRow'; +import { messageImageUrl } from '../utils/messageImageUrl'; +import { __resetSignedAttachmentUrlCacheForTests } from '../../utils/signedAttachmentUrl'; + +// The repository-wide markdown mock only returns text. Exercise the image +// component map in this suite so the row cannot bypass the origin guard. +jest.mock('react-markdown', () => { + const ReactForMock = require('react'); + return { + __esModule: true, + default: ({ children, components }) => { + const image = /^!\[([^\]]*)\]\(([^)]+)\)$/.exec(String(children)); + return image + ? ReactForMock.createElement(components.img, { alt: image[1], src: image[2] }) + : ReactForMock.createElement('div', { 'data-testid': 'react-markdown' }, children); + }, + }; +}); jest.mock('../../context/AuthContext', () => ({ useAuth: () => ({ currentUser: { username: 'viewer' } }), @@ -75,6 +92,61 @@ describe('V2MessageRow attachments (direction C)', () => { expect(screen.queryByRole('link')).not.toBeInTheDocument(); }); + test('a foreign markdown image is a plain link and creates no image element', () => { + const { container } = render(); + expect(container.querySelector('img')).toBeNull(); + expect(screen.getByRole('link', { name: 'report' })).toHaveAttribute('href', 'https://outside.example/collect?secret=x'); + }); + + test('the v2 message row routes markdown images through the origin guard', () => { + renderRow(message('![report](https://outside.example/collect.png)')); + expect(document.querySelector('.v2-msg__content img')).toBeNull(); + expect(screen.getByRole('link', { name: 'report' })).toHaveAttribute('href', 'https://outside.example/collect.png'); + }); + + test('an instance upload in markdown still renders as an image', () => { + const { container } = render(); + expect(container.querySelector('img')).toHaveAttribute('src', 'https://api.commonly.me/api/uploads/report.png'); + }); + + test('a signed upload response cannot switch the image to a foreign host', async () => { + __resetSignedAttachmentUrlCacheForTests(); + localStorage.setItem('token', 'test-token'); + const previousFetch = global.fetch; + const fetchSpy = jest.fn().mockResolvedValue({ + ok: true, + json: async () => ({ url: 'https://outside.example/collect.png?secret=x', expiresIn: 300 }), + } as Response); + global.fetch = fetchSpy; + try { + const { container } = render(); + await waitFor(() => expect(fetchSpy).toHaveBeenCalled()); + await act(async () => { await fetchSpy.mock.results[0].value; }); + expect(container.querySelector('img')).toHaveAttribute('src', 'https://api.commonly.me/api/uploads/signed-control.png'); + } finally { + global.fetch = previousFetch; + localStorage.removeItem('token'); + __resetSignedAttachmentUrlCacheForTests(); + } + }); + + test('a foreign legacy image message cannot enter the gallery', () => { + renderRow(message('https://outside.example/collect.png?secret=x', { message_type: 'image' })); + expect(document.querySelector('.v2-msg__thumbs')).toBeNull(); + expect(screen.getByRole('link', { name: /outside\.example/ })).toBeInTheDocument(); + }); + + test('image URL guard accepts only this instance upload origin and path', () => { + expect(messageImageUrl('https://api.commonly.me/api/uploads/one.png')).toBe('https://api.commonly.me/api/uploads/one.png'); + expect(messageImageUrl('https://api.commonly.me.evil.test/api/uploads/one.png')).toBeNull(); + expect(messageImageUrl('https://u:p@api.commonly.me/api/uploads/one.png')).toBeNull(); + expect(messageImageUrl('https://outside.example/api/uploads/one.png')).toBeNull(); + expect(messageImageUrl('https://api.commonly.me/other/one.png')).toBeNull(); + expect(messageImageUrl('data:image/svg+xml,')).toBeNull(); + process.env.REACT_APP_API_URL = 'https://api.commonly.me/'; + expect(messageImageUrl('/api/uploads/one.png')).toBe('https://api.commonly.me/api/uploads/one.png'); + }); + // react-markdown is mocked in jsdom, so the collapse is tested at the // component the markdown `pre` override renders. test('fenced code past six lines collapses with Show N more lines and expands in place', () => { diff --git a/frontend/src/v2/components/V2GithubPrCard.tsx b/frontend/src/v2/components/V2GithubPrCard.tsx index fb2747a049..a00489a1b6 100644 --- a/frontend/src/v2/components/V2GithubPrCard.tsx +++ b/frontend/src/v2/components/V2GithubPrCard.tsx @@ -151,10 +151,9 @@ const V2GithubPrCard: React.FC = ({ owner, repo, number }) - {!c.avatarUrl && c.login.slice(0, 1).toUpperCase()} + {c.login.slice(0, 1).toUpperCase()} ))} {overflowCount > 0 && ( diff --git a/frontend/src/v2/components/V2Lightbox.tsx b/frontend/src/v2/components/V2Lightbox.tsx index 3103f7a5cd..704f97a00f 100644 --- a/frontend/src/v2/components/V2Lightbox.tsx +++ b/frontend/src/v2/components/V2Lightbox.tsx @@ -1,6 +1,7 @@ import React, { useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { getSignedAttachmentUrl } from '../../utils/signedAttachmentUrl'; +import { messageImageUrl } from '../utils/messageImageUrl'; export interface V2LightboxImage { src: string; @@ -40,7 +41,10 @@ const V2Lightbox: React.FC = ({ images, index, onClose }) => { let active = true; setResolved(null); if (!image || !/\/api\/uploads\//.test(image.src)) return undefined; - void getSignedAttachmentUrl(image.src).then((signed) => { if (active && signed) setResolved(signed); }); + void getSignedAttachmentUrl(image.src).then((signed) => { + const safeSigned = messageImageUrl(signed); + if (active && safeSigned) setResolved(safeSigned); + }); return () => { active = false; }; }, [image]); @@ -56,7 +60,9 @@ const V2Lightbox: React.FC = ({ images, index, onClose }) => { )} - {image.name} event.stopPropagation()} /> + {messageImageUrl(resolved || image.src) && ( + {image.name} event.stopPropagation()} /> + )} ); }; diff --git a/frontend/src/v2/components/V2MessageRow.tsx b/frontend/src/v2/components/V2MessageRow.tsx index da02f9cecb..697542ed7f 100644 --- a/frontend/src/v2/components/V2MessageRow.tsx +++ b/frontend/src/v2/components/V2MessageRow.tsx @@ -13,7 +13,7 @@ import { formatRelativeTime } from '../utils/grouping'; import { useAuth } from '../../context/AuthContext'; import { useV2Api } from '../hooks/useV2Api'; import { getSignedAttachmentUrl } from '../../utils/signedAttachmentUrl'; -import { normalizeUploadUrl } from '../../utils/apiBaseUrl'; +import { messageImageUrl } from '../utils/messageImageUrl'; // Minimal v2-scoped markdown renderer. Plain HTML elements (no MUI), so // styling stays in v2.css under `.v2-msg__content`. The body comes pre-stripped @@ -58,7 +58,18 @@ const renderWithMentions = (node: React.ReactNode): React.ReactNode => { return node; }; +export const MessageMarkdownImage = ({ src, alt }: React.ImgHTMLAttributes) => { + const safeSrc = messageImageUrl(src); + if (safeSrc) return ; + // Keep the destination usable without initiating a browser image request. + if (src && /^(?:https?:\/\/|\/)/i.test(src)) { + return {alt || src}; + } + return {alt || src}; +}; + const messageMarkdownComponents = { + img: MessageMarkdownImage, a: ({ children, ...props }: React.AnchorHTMLAttributes) => ( {renderWithMentions(children)} ), @@ -249,7 +260,10 @@ const SignedImage: React.FC<{ src: string; alt: string }> = ({ src, alt }) => { useEffect(() => { let active = true; if (!/\/api\/uploads\//.test(src)) return undefined; - void getSignedAttachmentUrl(src).then((signed) => { if (active && signed) setResolved(signed); }); + void getSignedAttachmentUrl(src).then((signed) => { + const safeSigned = messageImageUrl(signed); + if (active && safeSigned) setResolved(safeSigned); + }); return () => { active = false; }; }, [src]); return {alt}; @@ -474,7 +488,9 @@ const V2MessageRow: React.FC = ({ message, sourceMessageId, d // Upload responses use instance-portable `/api/uploads/...` references. // Resolve them against the configured API origin at render time so the // browser never requests image bytes from the frontend host. - const imageUrl = rawImageUrl ? normalizeUploadUrl(rawImageUrl) : undefined; + const imageUrl = messageImageUrl(rawImageUrl); + const externalBareImage = rawImageUrl && !imageUrl && !markdownImage && /^https?:\/\//i.test(afterFiles) + ? afterFiles : null; // Direction C: every image attached to the message is a 156×104 thumbnail in // one gallery row; other uploads are chips. Uploaded images resolve to the // same `/api/uploads/` reference the legacy image path already uses. @@ -482,7 +498,10 @@ const V2MessageRow: React.FC = ({ message, sourceMessageId, d const otherFiles = files.filter((file) => !isImageFile(file)); const galleryImages: V2LightboxImage[] = [ ...(imageUrl ? [{ src: imageUrl, name: t('podChat.attachment.image') }] : []), - ...imageFiles.filter((file) => file.fileName).map((file) => ({ src: normalizeUploadUrl(`/api/uploads/${file.fileName}`), name: file.name })), + ...imageFiles.filter((file) => file.fileName).flatMap((file) => { + const src = messageImageUrl(`/api/uploads/${file.fileName}`); + return src ? [{ src, name: file.name }] : []; + }), ]; // GitHub PR URL detection — if the message body contains a `pull/` URL, @@ -498,7 +517,7 @@ const V2MessageRow: React.FC = ({ message, sourceMessageId, d // Auto-linkify bare http(s) URLs that aren't already inside markdown link // syntax. Without this, agents posting raw URLs render as plain text and // the user can't click them. - if (stripped) { + if (stripped && !externalBareImage) { stripped = stripped.replace( /(?"]+?[^\s<>".,!?;:])(?=[\s.,!?;:]|$)/g, '[$1]($1)', @@ -676,7 +695,9 @@ const V2MessageRow: React.FC = ({ message, sourceMessageId, d })()} {!imageUrl && stripped && (
- {stripped} + {externalBareImage + ? {externalBareImage} + : {stripped}}
)} {galleryImages.length > 0 && ( diff --git a/frontend/src/v2/utils/messageImageUrl.ts b/frontend/src/v2/utils/messageImageUrl.ts new file mode 100644 index 0000000000..4d4ce95a3f --- /dev/null +++ b/frontend/src/v2/utils/messageImageUrl.ts @@ -0,0 +1 @@ +export { messageImageUrl } from '../../utils/messageImageUrl';