feat/trusted media build env var#133
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Automated deployment preview for the PR in the Cloudflare Pages. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 9-10: Remove the surrounding quotes from the example value for the
VITE_TRUSTED_MEDIA_URLS variable in .env.example (update
VITE_TRUSTED_MEDIA_URLS="https://matterbridge.example.com,https://matrix-media.example.com"
to
VITE_TRUSTED_MEDIA_URLS=https://matterbridge.example.com,https://matrix-media.example.com)
so it satisfies dotenv-linter and avoids copy/paste confusion while keeping the
same comma-separated trusted URLs.
In `@vite.config.ts`:
- Line 29: The '__TRUSTED_MEDIA_URLS__' env parsing currently splits and trims
but can leave empty strings (e.g., trailing commas), so update the expression
that constructs this array to remove any empty entries after trimming (for
example by adding a .filter(...) that keeps only non-empty strings) so
downstream prefix checks don't treat an empty string as matching everything;
locate the expression assigning '__TRUSTED_MEDIA_URLS__' in vite.config.ts and
add the filter step after .map(url => url.trim()).
🧹 Nitpick comments (1)
src/components/message/LinkPreview.tsx (1)
26-32: Consider using a reactive selector for server lookup.The
useStore.getState().servers.find(...)call retrieves server data outside React's reactivity system. If the server'sfilehostproperty changes after mount, this component won't re-render with the updated value.This is consistent with the pattern in
MessageAvatar.tsx, so it may be intentional for performance reasons (avoiding re-renders on unrelated server changes). If reactivity is needed, consider a selector:const server = useStore((state) => serverId ? state.servers.find((s) => s.id === serverId) : null );
| # Optional comma-separated list of trusted media URLs (for chat bridge image proxies like Matterbridge, Matrix bridges) | ||
| VITE_TRUSTED_MEDIA_URLS="https://matterbridge.example.com,https://matrix-media.example.com" |
There was a problem hiding this comment.
Avoid quoted value to satisfy dotenv-linter and reduce copy/paste confusion.
The example value works without quotes; dropping them removes the lint warning.
🛠️ Suggested tweak
-VITE_TRUSTED_MEDIA_URLS="https://matterbridge.example.com,https://matrix-media.example.com"
+VITE_TRUSTED_MEDIA_URLS=https://matterbridge.example.com,https://matrix-media.example.com📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Optional comma-separated list of trusted media URLs (for chat bridge image proxies like Matterbridge, Matrix bridges) | |
| VITE_TRUSTED_MEDIA_URLS="https://matterbridge.example.com,https://matrix-media.example.com" | |
| # Optional comma-separated list of trusted media URLs (for chat bridge image proxies like Matterbridge, Matrix bridges) | |
| VITE_TRUSTED_MEDIA_URLS=https://matterbridge.example.com,https://matrix-media.example.com |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents
In @.env.example around lines 9 - 10, Remove the surrounding quotes from the
example value for the VITE_TRUSTED_MEDIA_URLS variable in .env.example (update
VITE_TRUSTED_MEDIA_URLS="https://matterbridge.example.com,https://matrix-media.example.com"
to
VITE_TRUSTED_MEDIA_URLS=https://matterbridge.example.com,https://matrix-media.example.com)
so it satisfies dotenv-linter and avoids copy/paste confusion while keeping the
same comma-separated trusted URLs.
| '__DEFAULT_IRC_CHANNELS__': process.env.VITE_DEFAULT_IRC_CHANNELS ? process.env.VITE_DEFAULT_IRC_CHANNELS.replace(/^['"]|['"]$/g, '').split(',').map(ch => ch.trim()) : [], | ||
| '__HIDE_SERVER_LIST__': process.env.VITE_HIDE_SERVER_LIST === 'true', | ||
| '__BACKEND_URL__': JSON.stringify(process.env.VITE_BACKEND_URL || 'http://localhost:8080'), | ||
| '__TRUSTED_MEDIA_URLS__': process.env.VITE_TRUSTED_MEDIA_URLS ? process.env.VITE_TRUSTED_MEDIA_URLS.replace(/^['"]|['"]$/g, '').split(',').map(url => url.trim()) : [], |
There was a problem hiding this comment.
Filter empty entries to avoid accidentally trusting all URLs.
If the list contains a trailing comma or blank item, an empty string can sneak into the array. Downstream checks that use prefix matching could then treat every URL as trusted.
✅ Suggested fix
- '__TRUSTED_MEDIA_URLS__': process.env.VITE_TRUSTED_MEDIA_URLS ? process.env.VITE_TRUSTED_MEDIA_URLS.replace(/^['"]|['"]$/g, '').split(',').map(url => url.trim()) : [],
+ '__TRUSTED_MEDIA_URLS__': process.env.VITE_TRUSTED_MEDIA_URLS
+ ? process.env.VITE_TRUSTED_MEDIA_URLS
+ .replace(/^['"]|['"]$/g, '')
+ .split(',')
+ .map((url) => url.trim())
+ .filter(Boolean)
+ : [],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| '__TRUSTED_MEDIA_URLS__': process.env.VITE_TRUSTED_MEDIA_URLS ? process.env.VITE_TRUSTED_MEDIA_URLS.replace(/^['"]|['"]$/g, '').split(',').map(url => url.trim()) : [], | |
| '__TRUSTED_MEDIA_URLS__': process.env.VITE_TRUSTED_MEDIA_URLS | |
| ? process.env.VITE_TRUSTED_MEDIA_URLS | |
| .replace(/^['"]|['"]$/g, '') | |
| .split(',') | |
| .map((url) => url.trim()) | |
| .filter(Boolean) | |
| : [], |
🤖 Prompt for AI Agents
In `@vite.config.ts` at line 29, The '__TRUSTED_MEDIA_URLS__' env parsing
currently splits and trims but can leave empty strings (e.g., trailing commas),
so update the expression that constructs this array to remove any empty entries
after trimming (for example by adding a .filter(...) that keeps only non-empty
strings) so downstream prefix checks don't treat an empty string as matching
everything; locate the expression assigning '__TRUSTED_MEDIA_URLS__' in
vite.config.ts and add the filter step after .map(url => url.trim()).
|
Lol what's with the "do not merge" label ;D needs review before can be merged |
Yeah. Kinda forgot about draft PRs too. I just mean this is explicitely not ready and untested. |
|
Is this ready for conflicts resolution and merge? |
…o feat/trusted-media-build-env-var
Fixes #131
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.