Improve footer sync and show Creem webhook URL - #3236
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefactors footer rendering to compute a normalized footer value and render mode (iframe / html / default), synchronizes iframe height after load, adds footer helpers and tests, introduces status-option patching and sync in settings, adds payment webhook URL helper/tests and surfaces the webhook URL in Creem settings, plus i18n keys. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FooterComp as Footer Component
participant Helper as Footer Helper
participant External as External Page (iframe)
User->>FooterComp: mount/render (status.footer_html)
FooterComp->>Helper: normalizeFooterValue(status.footer_html) / getFooterRenderMode(...)
Helper-->>FooterComp: footerValue, renderMode
alt renderMode == iframe
FooterComp->>External: set iframe src = footerValue
External-->>FooterComp: iframe load event
FooterComp->>FooterComp: measure iframe content height
FooterComp-->>User: update iframe height / render footer
else renderMode == html
FooterComp-->>User: inject footerValue as HTML
else default
FooterComp-->>User: render default inline footer
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
web/src/components/layout/Footer.jsx (2)
45-61: Consider adding a comment about cross-origin iframe limitation.The empty catch block silently handles the expected
SecurityErrorwhen accessingcontentDocumenton cross-origin iframes. This is correct behavior, but future maintainers might wonder why the catch is empty.For cross-origin footer URLs, the iframe will retain the default 240px height since
contentDocumentaccess will be blocked.📝 Suggested documentation improvement
const handleFooterFrameLoad = useCallback(() => { try { const iframeDocument = iframeRef.current?.contentDocument; if (!iframeDocument) { return; } const nextHeight = Math.max( iframeDocument.documentElement?.scrollHeight || 0, iframeDocument.body?.scrollHeight || 0, ); if (nextHeight > 0) { setIframeHeight(nextHeight); } - } catch {} + } catch { + // Cross-origin iframes throw SecurityError; fallback to default height + } }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/Footer.jsx` around lines 45 - 61, The empty catch in handleFooterFrameLoad silently swallows the expected SecurityError when accessing iframeRef.current?.contentDocument for cross-origin footer URLs; update the catch block to include a concise comment referencing this cross-origin limitation and that in such cases the iframe will keep the default 240px height (set via setIframeHeight elsewhere), e.g., mention SecurityError / cross-origin access is expected and intentionally ignored so future maintainers understand why the catch is empty; keep the behavior unchanged (do not rethrow).
244-251: Consider adding sandbox attribute for defense-in-depth.The iframe loads admin-configured URLs without a
sandboxattribute. While this content is admin-controlled, addingsandboxwith appropriate permissions provides defense-in-depth against compromised footer URLs.🛡️ Optional security hardening
<iframe ref={iframeRef} title={t('页脚')} src={footerValue} className='custom-footer-frame' style={{ height: `${iframeHeight}px` }} onLoad={handleFooterFrameLoad} + sandbox="allow-scripts allow-same-origin" />Note:
allow-same-originis needed for the height auto-adjustment to work on same-origin iframes. Adjust permissions based on what footer content actually needs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/Footer.jsx` around lines 244 - 251, The iframe rendering in Footer.jsx (the element using iframeRef, title via t('页脚'), src={footerValue}, className='custom-footer-frame' and onLoad={handleFooterFrameLoad}) should include a sandbox attribute for defense-in-depth; update the JSX to add sandbox with a minimal permission set (e.g. include allow-scripts if scripts are required) and, only if you need the current auto-height behaviour for same-origin content, include allow-same-origin as well—adjust the permissions to the least needed for the footer content. Ensure the sandbox prop is added to the same iframe element so height adjustment (handled by handleFooterFrameLoad) still works for same-origin when allow-same-origin is permitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/helpers/paymentWebhook.js`:
- Around line 1-7: The helper getPaymentWebhookUrl currently embeds a UI
placeholder '网站地址'; remove this presentation text and make the function
presentation-agnostic by returning either the normalizedServerAddress or, if
empty, a caller-supplied fallback string; update the signature to accept an
optional fallback param (e.g., getPaymentWebhookUrl(serverAddress, provider,
fallback) ) and use fallback when normalizedServerAddress is falsy so UI
components can pass translated text via useTranslation()/t('...') instead of
hardcoding Chinese here.
In `@web/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.jsx`:
- Around line 272-275: The Banner description currently uses a hard-coded
Chinese label "Webhook 填：" which bypasses i18n; import and call useTranslation()
in the SettingsPaymentGatewayCreem component to get t, replace the inline string
with t('Webhook 填：') and compose the description as `${t('Webhook
填：')}${getPaymentWebhookUrl(props.options.ServerAddress, 'creem')}`, and add the
key "Webhook 填：" to the locale JSON files under web/src/i18n/locales/{lang}.json
(use the Chinese string as the key) so the banner is translatable; ensure Banner
and getPaymentWebhookUrl usage remains unchanged.
- Around line 272-275: The Banner currently dereferences
props.options.ServerAddress when rendering the Creem webhook URL which can throw
if props.options is undefined; update the rendering to guard props.options (for
example using props.options && props.options.ServerAddress or optional chaining
props.options?.ServerAddress) before calling getPaymentWebhookUrl, or
conditionally render the Banner only when ServerAddress is present; reference
the Banner JSX and the getPaymentWebhookUrl call in
SettingsPaymentGatewayCreem.jsx and ensure the description string uses a safe
fallback (e.g., empty string or "loading...") until props.options.ServerAddress
is available.
---
Nitpick comments:
In `@web/src/components/layout/Footer.jsx`:
- Around line 45-61: The empty catch in handleFooterFrameLoad silently swallows
the expected SecurityError when accessing iframeRef.current?.contentDocument for
cross-origin footer URLs; update the catch block to include a concise comment
referencing this cross-origin limitation and that in such cases the iframe will
keep the default 240px height (set via setIframeHeight elsewhere), e.g., mention
SecurityError / cross-origin access is expected and intentionally ignored so
future maintainers understand why the catch is empty; keep the behavior
unchanged (do not rethrow).
- Around line 244-251: The iframe rendering in Footer.jsx (the element using
iframeRef, title via t('页脚'), src={footerValue}, className='custom-footer-frame'
and onLoad={handleFooterFrameLoad}) should include a sandbox attribute for
defense-in-depth; update the JSX to add sandbox with a minimal permission set
(e.g. include allow-scripts if scripts are required) and, only if you need the
current auto-height behaviour for same-origin content, include allow-same-origin
as well—adjust the permissions to the least needed for the footer content.
Ensure the sandbox prop is added to the same iframe element so height adjustment
(handled by handleFooterFrameLoad) still works for same-origin when
allow-same-origin is permitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 442753cf-9137-43a1-8c67-6fe8fc8da528
📒 Files selected for processing (10)
web/src/components/layout/Footer.jsxweb/src/components/settings/OtherSetting.jsxweb/src/helpers/footer.jsweb/src/helpers/footer.test.mjsweb/src/helpers/paymentWebhook.jsweb/src/helpers/paymentWebhook.test.mjsweb/src/helpers/statusOption.jsweb/src/helpers/statusOption.test.mjsweb/src/index.cssweb/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.jsx
|
Also addressed the low-risk Footer cleanup in |
|
@coderabbitai run pre-merge checks |
✅ Actions performedRunning Pre-merge checks. Results will be posted in the summary comment once complete. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/src/helpers/footer.js (1)
41-43: Harden iframe URL detection to valid URLs only.Current prefix matching can classify malformed strings as
iframe. Consider parsing viaURLand checking protocol explicitly.🔧 Suggested refactor
- if (/^https?:\/\//i.test(normalizedFooter)) { - return 'iframe'; - } + try { + const parsed = new URL(normalizedFooter); + if (parsed.protocol === 'http:' || parsed.protocol === 'https:') { + return 'iframe'; + } + } catch { + // Not a valid URL; fall through to HTML mode. + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/footer.js` around lines 41 - 43, Replace the brittle regex prefix check that returns 'iframe' for normalizedFooter with a URL-parse guard: attempt to construct new URL(normalizedFooter) in a try/catch and only return 'iframe' if the parsed URL.protocol is exactly 'http:' or 'https:'; on any thrown error or non-matching protocol, do not classify as 'iframe'. Update the logic around the existing normalizedFooter check (the block containing /^https?:\/\//i.test(normalizedFooter)) to use this safe parse-and-check approach.web/src/helpers/paymentWebhook.js (1)
35-36: Normalize fallback input to avoid accidental double slashes.If a caller passes a fallback ending with
/, the generated URL can include//api/.... Consider normalizing the fallback the same way asserverAddress.♻️ Optional patch
const normalizedServerAddress = String(serverAddress || '') .trim() .replace(/\/+$/, ''); - const baseUrl = normalizedServerAddress || fallbackBaseLabel; + const normalizedFallbackBaseLabel = String(fallbackBaseLabel || '') + .trim() + .replace(/\/+$/, ''); + const baseUrl = normalizedServerAddress || normalizedFallbackBaseLabel; return `${baseUrl}/api/${provider}/webhook`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/paymentWebhook.js` around lines 35 - 36, The fallbackBaseLabel can include a trailing slash which leads to double slashes in the returned URL; update the logic that computes baseUrl so fallbackBaseLabel is normalized the same way as normalizedServerAddress (trim any trailing slash) before composing the webhook URL. Locate the code that defines normalizedServerAddress and reuse its normalization (or apply the same trim) to produce a cleaned fallbackBaseLabel, then compute baseUrl = normalizedServerAddress || cleanedFallbackBaseLabel and return `${baseUrl}/api/${provider}/webhook`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/helpers/footer.js`:
- Around line 41-43: Replace the brittle regex prefix check that returns
'iframe' for normalizedFooter with a URL-parse guard: attempt to construct new
URL(normalizedFooter) in a try/catch and only return 'iframe' if the parsed
URL.protocol is exactly 'http:' or 'https:'; on any thrown error or non-matching
protocol, do not classify as 'iframe'. Update the logic around the existing
normalizedFooter check (the block containing
/^https?:\/\//i.test(normalizedFooter)) to use this safe parse-and-check
approach.
In `@web/src/helpers/paymentWebhook.js`:
- Around line 35-36: The fallbackBaseLabel can include a trailing slash which
leads to double slashes in the returned URL; update the logic that computes
baseUrl so fallbackBaseLabel is normalized the same way as
normalizedServerAddress (trim any trailing slash) before composing the webhook
URL. Locate the code that defines normalizedServerAddress and reuse its
normalization (or apply the same trim) to produce a cleaned fallbackBaseLabel,
then compute baseUrl = normalizedServerAddress || cleanedFallbackBaseLabel and
return `${baseUrl}/api/${provider}/webhook`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3c229a3-867d-4ef8-a03e-2ee60961c9f2
📒 Files selected for processing (4)
web/src/helpers/footer.jsweb/src/helpers/paymentWebhook.jsweb/src/helpers/statusOption.jsweb/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/helpers/statusOption.js
- web/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.jsx
|
@coderabbitai run pre-merge checks |
✅ Actions performedRunning Pre-merge checks. Results will be posted in the summary comment once complete. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/layout/Footer.jsx`:
- Around line 62-80: The measured iframe height can persist across src/mode
changes and prevent the fallback from applying for cross-origin content; update
the component to reset the iframe height whenever the iframe source or display
mode changes by adding a useEffect that watches the footer source/mode props and
calls setIframeHeight to the fallback (or null/0 whatever this component treats
as "use fallback"), and also consider setting the fallback inside
handleFooterFrameLoad's catch path using iframeRef and setIframeHeight so
cross-origin failures revert to the fallback; reference handleFooterFrameLoad,
iframeRef, and setIframeHeight when making this change.
- Around line 272-275: The footer is rendered via dangerouslySetInnerHTML using
footerValue (produced by normalizeFooterValue) which only trims and does not
sanitize; update the component to sanitize footerValue before assignment (or
inside normalizeFooterValue) using DOMPurify (import DOMPurify from 'dompurify')
and pass DOMPurify.sanitize(footerValue) to the dangerouslySetInnerHTML prop so
stored XSS cannot occur; ensure you only sanitize where
footer_html/status.footer_html is used and keep the symbol names:
normalizeFooterValue, footerValue, and the div with dangerouslySetInnerHTML.
In `@web/src/components/settings/OtherSetting.jsx`:
- Around line 389-392: The openGitHubRelease function currently calls
window.open(url, '_blank') which is vulnerable to reverse-tabnabbing; update the
call to include noopener and noreferrer (e.g. window.open(url, '_blank',
'noopener,noreferrer')) or, if you create an anchor, set rel="noopener
noreferrer", and additionally set newWindow.opener = null when available to be
extra-safe—modify the openGitHubRelease implementation accordingly.
- Around line 306-309: The parsed GitHub release HTML produced in the
OtherSetting component (the output of marked, e.g., the variable holding release
notes HTML that is later injected via dangerouslySetInnerHTML) is not sanitized
and creates an XSS risk; import DOMPurify and run the marked output through
DOMPurify.sanitize before storing it in state or passing it to
dangerouslySetInnerHTML (or, alternatively, configure marked to disable raw HTML
and still sanitize), then use the sanitized string when rendering to ensure
untrusted remote content is safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf3a8ade-67b2-4bbd-949a-ce77bed21fa2
📒 Files selected for processing (3)
web/src/components/layout/Footer.jsxweb/src/components/settings/OtherSetting.jsxweb/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.jsx
| const handleFooterFrameLoad = useCallback(() => { | ||
| try { | ||
| const iframeDocument = iframeRef.current?.contentDocument; | ||
| if (!iframeDocument) { | ||
| return; | ||
| } | ||
|
|
||
| const loadFooter = () => { | ||
| let footer_html = localStorage.getItem('footer_html'); | ||
| if (footer_html) { | ||
| setFooter(footer_html); | ||
| const nextHeight = Math.max( | ||
| iframeDocument.documentElement?.scrollHeight || 0, | ||
| iframeDocument.body?.scrollHeight || 0, | ||
| ); | ||
|
|
||
| if (nextHeight > 0) { | ||
| setIframeHeight(nextHeight); | ||
| } | ||
| } catch { | ||
| // Cross-origin iframe documents cannot be measured, so keep the fallback height. | ||
| } | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Reset iframe height when source/mode changes to keep fallback behavior correct.
If a previous same-origin footer set a large measured height, then a new cross-origin iframe can fail measurement and keep the old large height instead of the intended fallback.
🧩 Minimal fix
import React, {
useCallback,
useContext,
+ useEffect,
useMemo,
useRef,
useState,
} from 'react';
@@
const handleFooterFrameLoad = useCallback(() => {
@@
}, []);
+
+ useEffect(() => {
+ if (footerRenderMode === 'iframe') {
+ setIframeHeight(240);
+ }
+ }, [footerRenderMode, footerValue]);📝 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.
| const handleFooterFrameLoad = useCallback(() => { | |
| try { | |
| const iframeDocument = iframeRef.current?.contentDocument; | |
| if (!iframeDocument) { | |
| return; | |
| } | |
| const loadFooter = () => { | |
| let footer_html = localStorage.getItem('footer_html'); | |
| if (footer_html) { | |
| setFooter(footer_html); | |
| const nextHeight = Math.max( | |
| iframeDocument.documentElement?.scrollHeight || 0, | |
| iframeDocument.body?.scrollHeight || 0, | |
| ); | |
| if (nextHeight > 0) { | |
| setIframeHeight(nextHeight); | |
| } | |
| } catch { | |
| // Cross-origin iframe documents cannot be measured, so keep the fallback height. | |
| } | |
| }; | |
| }, []); | |
| const handleFooterFrameLoad = useCallback(() => { | |
| try { | |
| const iframeDocument = iframeRef.current?.contentDocument; | |
| if (!iframeDocument) { | |
| return; | |
| } | |
| const nextHeight = Math.max( | |
| iframeDocument.documentElement?.scrollHeight || 0, | |
| iframeDocument.body?.scrollHeight || 0, | |
| ); | |
| if (nextHeight > 0) { | |
| setIframeHeight(nextHeight); | |
| } | |
| } catch { | |
| // Cross-origin iframe documents cannot be measured, so keep the fallback height. | |
| } | |
| }, []); | |
| useEffect(() => { | |
| if (footerRenderMode === 'iframe') { | |
| setIframeHeight(240); | |
| } | |
| }, [footerRenderMode, footerValue]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/layout/Footer.jsx` around lines 62 - 80, The measured
iframe height can persist across src/mode changes and prevent the fallback from
applying for cross-origin content; update the component to reset the iframe
height whenever the iframe source or display mode changes by adding a useEffect
that watches the footer source/mode props and calls setIframeHeight to the
fallback (or null/0 whatever this component treats as "use fallback"), and also
consider setting the fallback inside handleFooterFrameLoad's catch path using
iframeRef and setIframeHeight so cross-origin failures revert to the fallback;
reference handleFooterFrameLoad, iframeRef, and setIframeHeight when making this
change.
| <div | ||
| className='custom-footer' | ||
| dangerouslySetInnerHTML={{ __html: footerValue }} | ||
| ></div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether footer content is currently sanitized before reaching dangerouslySetInnerHTML.
rg -n "dangerouslySetInnerHTML|normalizeFooterValue|sanitize|DOMPurify" web/src/components/layout/Footer.jsx web/src/helpers/footer.jsRepository: QuantumNous/new-api
Length of output: 514
🏁 Script executed:
cat -n web/src/helpers/footer.jsRepository: QuantumNous/new-api
Length of output: 1711
🏁 Script executed:
sed -n '45,60p' web/src/components/layout/Footer.jsxRepository: QuantumNous/new-api
Length of output: 529
🏁 Script executed:
rg -l "dompurify|DOMPurify" web/Repository: QuantumNous/new-api
Length of output: 75
🏁 Script executed:
fd package.json web/ | xargs grep -l "dompurify" 2>/dev/null || echo "Not found in direct grep"Repository: QuantumNous/new-api
Length of output: 87
🏁 Script executed:
cat web/package.json | grep -A 5 -B 5 "dompurify" || echo "dompurify not in package.json"Repository: QuantumNous/new-api
Length of output: 92
🏁 Script executed:
cat web/package.json | head -50Repository: QuantumNous/new-api
Length of output: 1555
🏁 Script executed:
grep -A 10 "dompurify" web/bun.lock | head -20Repository: QuantumNous/new-api
Length of output: 3988
🏁 Script executed:
rg -n "footer_html|getFooterHTML" web/src/ -A 2 -B 2Repository: QuantumNous/new-api
Length of output: 2576
Sanitize footer HTML before rendering to avoid XSS.
footerValue is rendered with dangerouslySetInnerHTML without sanitization. The normalizeFooterValue() function only performs .trim()—it does not sanitize HTML. Since footer_html originates from server configuration (status.footer_html) and is persisted in localStorage, unsanitized rendering creates a stored XSS risk if this configuration is attacker-controlled or misconfigured by administrators.
Use DOMPurify (already available as a transitive dependency of mermaid) to sanitize the HTML before rendering:
Proposed fix
import React, {
useCallback,
useContext,
useMemo,
useRef,
useState,
} from 'react';
+import DOMPurify from 'dompurify';
@@
const footerRenderMode = useMemo(
() => getFooterRenderMode(footerValue),
[footerValue],
);
+ const sanitizedFooterHtml = useMemo(
+ () => DOMPurify.sanitize(footerValue),
+ [footerValue],
+ );
@@
) : (
<div
className='custom-footer'
- dangerouslySetInnerHTML={{ __html: footerValue }}
+ dangerouslySetInnerHTML={{ __html: sanitizedFooterHtml }}
></div>
)}📝 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.
| <div | |
| className='custom-footer' | |
| dangerouslySetInnerHTML={{ __html: footerValue }} | |
| ></div> | |
| <div | |
| className='custom-footer' | |
| dangerouslySetInnerHTML={{ __html: sanitizedFooterHtml }} | |
| ></div> |
🧰 Tools
🪛 Biome (2.4.6)
[error] 274-274: Avoid passing content using the dangerouslySetInnerHTML prop.
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/layout/Footer.jsx` around lines 272 - 275, The footer is
rendered via dangerouslySetInnerHTML using footerValue (produced by
normalizeFooterValue) which only trims and does not sanitize; update the
component to sanitize footerValue before assignment (or inside
normalizeFooterValue) using DOMPurify (import DOMPurify from 'dompurify') and
pass DOMPurify.sanitize(footerValue) to the dangerouslySetInnerHTML prop so
stored XSS cannot occur; ensure you only sanitize where
footer_html/status.footer_html is used and keep the symbol names:
normalizeFooterValue, footerValue, and the div with dangerouslySetInnerHTML.
| /** | ||
| * 查询 GitHub 最新版本并在有更新时展示更新说明。 | ||
| * @returns {Promise<void>} | ||
| */ |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In marked v4.x, does marked.parse sanitize raw HTML by default, or is an external sanitizer (e.g., DOMPurify) required before using dangerouslySetInnerHTML?
💡 Result:
In marked v4.x, marked.parse() does not sanitize raw HTML by default—it will emit HTML as-is. If you’re rendering the result via React’s dangerouslySetInnerHTML, you should run an external HTML sanitizer (e.g., DOMPurify) on the output first. [1]
Also, marked’s old sanitize/sanitizer options were deprecated/removed, so relying on marked itself for sanitization isn’t the intended approach in v4.x. [2]
Sources:
[1] Marked docs (Security warning: “Marked does not sanitize the output HTML… DOMPurify recommended”)
[2] safe-marked / marked-sanitizer notes on marked not being safe-by-default and sanitize option deprecation/removal
🏁 Script executed:
wc -l web/src/components/settings/OtherSetting.jsxRepository: QuantumNous/new-api
Length of output: 111
🏁 Script executed:
# Read the vulnerable code sections
sed -n '340,355p' web/src/components/settings/OtherSetting.jsx
cat -n web/src/components/settings/OtherSetting.jsx | sed -n '605,620p'Repository: QuantumNous/new-api
Length of output: 880
🏁 Script executed:
# Check imports at the top of the file
head -30 web/src/components/settings/OtherSetting.jsxRepository: QuantumNous/new-api
Length of output: 976
🏁 Script executed:
# Search for dangerouslySetInnerHTML usage in the file
grep -n "dangerouslySetInnerHTML" web/src/components/settings/OtherSetting.jsxRepository: QuantumNous/new-api
Length of output: 143
🏁 Script executed:
# Check for marked import
grep -n "marked" web/src/components/settings/OtherSetting.jsx | head -5Repository: QuantumNous/new-api
Length of output: 141
🏁 Script executed:
# Check for any existing DOMPurify usage
grep -n "DOMPurify" web/src/components/settings/OtherSetting.jsxRepository: QuantumNous/new-api
Length of output: 45
Sanitize release markdown HTML before rendering.
Line 346 parses untrusted remote content (GitHub API response) and line 611 injects it via dangerouslySetInnerHTML. Since marked v4.x does not sanitize raw HTML by default, this creates an XSS vulnerability in the admin UI. Use DOMPurify to sanitize the parsed output before rendering.
🔒 Suggested fix
import { marked } from 'marked';
+import DOMPurify from 'dompurify';
@@
const { tag_name, body } = res;
@@
setUpdateData({
tag_name: tag_name,
- content: marked.parse(body),
+ content: DOMPurify.sanitize(marked.parse(body ?? '')),
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/settings/OtherSetting.jsx` around lines 306 - 309, The
parsed GitHub release HTML produced in the OtherSetting component (the output of
marked, e.g., the variable holding release notes HTML that is later injected via
dangerouslySetInnerHTML) is not sanitized and creates an XSS risk; import
DOMPurify and run the marked output through DOMPurify.sanitize before storing it
in state or passing it to dangerouslySetInnerHTML (or, alternatively, configure
marked to disable raw HTML and still sanitize), then use the sanitized string
when rendering to ensure untrusted remote content is safe.
| /** | ||
| * 在新窗口中打开当前待更新版本的 GitHub Release 页面。 | ||
| * @returns {void} | ||
| */ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify other _blank window.open call sites that may need the same hardening.
rg -nP "window\\.open\\([^\\n]*['_\"]_blank['\"]" web/srcRepository: QuantumNous/new-api
Length of output: 2068
🏁 Script executed:
fd "OtherSetting" web/srcRepository: QuantumNous/new-api
Length of output: 107
🏁 Script executed:
sed -n '385,400p' web/src/components/settings/OtherSetting.jsxRepository: QuantumNous/new-api
Length of output: 361
🏁 Script executed:
sed -n '393,405p' web/src/components/settings/OtherSetting.jsxRepository: QuantumNous/new-api
Length of output: 385
🏁 Script executed:
rg -n "window\.open" web/src/components/settings/OtherSetting.jsxRepository: QuantumNous/new-api
Length of output: 83
Harden window.open usage against reverse-tabnabbing.
The openGitHubRelease function opens an external GitHub URL with window.open() using _blank without the noopener,noreferrer flags. Add these flags to prevent the opened page from accessing window.opener.
Suggested fix
const openGitHubRelease = () => {
window.open(
`https://github.com/Calcium-Ion/new-api/releases/tag/${updateData.tag_name}`,
'_blank',
+ 'noopener,noreferrer',
);
};📝 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.
| /** | |
| * 在新窗口中打开当前待更新版本的 GitHub Release 页面。 | |
| * @returns {void} | |
| */ | |
| const openGitHubRelease = () => { | |
| window.open( | |
| `https://github.com/Calcium-Ion/new-api/releases/tag/${updateData.tag_name}`, | |
| '_blank', | |
| 'noopener,noreferrer', | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/settings/OtherSetting.jsx` around lines 389 - 392, The
openGitHubRelease function currently calls window.open(url, '_blank') which is
vulnerable to reverse-tabnabbing; update the call to include noopener and
noreferrer (e.g. window.open(url, '_blank', 'noopener,noreferrer')) or, if you
create an anchor, set rel="noopener noreferrer", and additionally set
newWindow.opener = null when available to be extra-safe—modify the
openGitHubRelease implementation accordingly.
1 participant
Summary
Testing
node --test web/src/helpers/footer.test.mjs web/src/helpers/statusOption.test.mjs web/src/helpers/paymentWebhook.test.mjsSummary by CodeRabbit
New Features
Improvements
Tests