-
-
Notifications
You must be signed in to change notification settings - Fork 620
feat: Status Page Custom CSS/JS/HTML overrides (Fixes #2863) #3076
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.
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
base: develop
Are you sure you want to change the base?
Changes from 4 commits
0638624
f2f2469
9e7f44e
93c76bc
ef79ad9
8cba518
f99222c
63b5912
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ import { useIsAdmin } from "../../../../Hooks/v1/useIsAdmin.js"; | |
| import { useLocation } from "react-router-dom"; | ||
| import { useParams } from "react-router-dom"; | ||
| import { useTranslation } from "react-i18next"; | ||
| // --- CHANGE 1: Import useEffect --- | ||
| import { useEffect } from "react"; | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const PublicStatus = () => { | ||
| const { url } = useParams(); | ||
|
|
@@ -28,6 +30,59 @@ const PublicStatus = () => { | |
|
|
||
| const [statusPage, monitors, isLoading, networkError] = useStatusPageFetch(false, url); | ||
|
|
||
| // --- CHANGE 2: Inject Custom CSS and JS --- | ||
| useEffect(() => { | ||
| if (!statusPage) return; | ||
|
|
||
| // --- 1. Inject Custom CSS --- | ||
| let styleElement = null; | ||
| if (statusPage.customCSS) { | ||
| styleElement = document.createElement("style"); | ||
| styleElement.id = "custom-status-css"; | ||
| styleElement.innerHTML = statusPage.customCSS; | ||
| document.head.appendChild(styleElement); | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // --- 2. Inject Custom JS --- | ||
| let scriptElement = null; | ||
| if (statusPage.customJavaScript) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1 | Confidence: High Direct execution of user-provided JavaScript via Code Suggestion: |
||
| try { | ||
| scriptElement = document.createElement("script"); | ||
| scriptElement.id = "custom-status-js"; | ||
|
|
||
| // Wrap in IIFE (Immediately Invoked Function Expression) with internal try/catch | ||
| // This ensures that if the user writes bad JS, it errors to console but doesn't crash the UI | ||
| scriptElement.textContent = ` | ||
| (function() { | ||
| try { | ||
| ${statusPage.customJavaScript} | ||
| } catch(e) { | ||
| console.error('Custom Status Page Script Error:', e); | ||
| } | ||
| })(); | ||
| `; | ||
|
|
||
| document.body.appendChild(scriptElement); | ||
| } catch (e) { | ||
| console.error("Failed to inject custom JS element:", e); | ||
| } | ||
| } | ||
|
Comment on lines
+57
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Direct JavaScript injection creates arbitrary code execution vulnerability. Injecting admin-provided JavaScript directly into the DOM enables arbitrary code execution on public status pages. Even with admin-only access, this poses severe risks:
In your PR comment response to @SajanGhuman, you acknowledged these risks and stated you would implement mitigations. However, none of the promised security controls are present:
Issue #2863 explicitly requires "strict Content Security Policy" and security controls. This implementation does not meet those requirements. Recommended mitigations (in order of preference):
Consider whether the use case (analytics/chat widgets) can be served by a safer allowlist-based integration approach instead of arbitrary script execution. 🤖 Prompt for AI Agents |
||
|
|
||
| // --- 3. Safe Cleanup --- | ||
| return () => { | ||
| // Clean up CSS using the direct reference | ||
| if (styleElement && document.head.contains(styleElement)) { | ||
| document.head.removeChild(styleElement); | ||
| } | ||
|
|
||
| // Clean up JS using the direct reference | ||
| if (scriptElement && document.body.contains(scriptElement)) { | ||
| document.body.removeChild(scriptElement); | ||
| } | ||
| }; | ||
| }, [statusPage]); | ||
| // ------------------------------------------ | ||
|
|
||
| // Breadcrumbs | ||
| const crumbs = [ | ||
| { name: t("statusBreadCrumbsStatusPages"), path: "/status" }, | ||
|
|
@@ -38,6 +93,7 @@ const PublicStatus = () => { | |
| let sx = { paddingLeft: theme.spacing(20), paddingRight: theme.spacing(20) }; | ||
| let link = undefined; | ||
| const isPublic = location.pathname.startsWith("/status/uptime/public"); | ||
|
|
||
| // Public status page | ||
| if (isPublic && statusPage && statusPage.showAdminLoginLink === true) { | ||
| sx = { | ||
|
|
@@ -147,18 +203,33 @@ const PublicStatus = () => { | |
| sx={{ ...sx, position: "relative" }} | ||
| > | ||
| {!isPublic && <Breadcrumbs list={crumbs} />} | ||
| <ControlsHeader | ||
| statusPage={statusPage} | ||
| url={url} | ||
| isPublic={isPublic} | ||
| /> | ||
|
|
||
| {/* --- CHANGE 3: Custom Header Logic --- */} | ||
| {statusPage?.headerHTML ? ( | ||
| <div dangerouslySetInnerHTML={{ __html: statusPage.headerHTML }} /> | ||
| ) : ( | ||
| <ControlsHeader | ||
| statusPage={statusPage} | ||
| url={url} | ||
| isPublic={isPublic} | ||
| /> | ||
| )} | ||
| {/* ------------------------------------- */} | ||
|
|
||
| <Typography variant="h2">{t("statusPageStatusServiceStatus")}</Typography> | ||
| <StatusBar monitors={monitors} /> | ||
| <MonitorsList | ||
| monitors={monitors} | ||
| statusPage={statusPage} | ||
| /> | ||
|
|
||
| {link} | ||
|
|
||
| {/* --- CHANGE 4: Custom Footer Logic --- */} | ||
| {statusPage?.footerHTML && ( | ||
| <div dangerouslySetInnerHTML={{ __html: statusPage.footerHTML }} /> | ||
| )} | ||
| {/* ------------------------------------- */} | ||
| </Stack> | ||
| ); | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2 | Confidence: Medium
While there's a warning helper text, the UI doesn't sufficiently communicate the security implications to administrators. The feature enables arbitrary code execution on public pages, which should be accompanied by more prominent warnings and potentially an explicit acknowledgment checkbox.
Code Suggestion: