rusackas commented on code in PR #43667:
URL: https://github.com/apache/superset/pull/43667#discussion_r3885193130
##########
superset-frontend/src/dashboard/components/PropertiesModal/sections/StylingSection.tsx:
##########
@@ -162,6 +171,42 @@ const StylingSection = ({
const hasTemplateModification =
selectedTemplate && customCss !== originalTemplateContent;
+ // Convert any @import in the CSS to the imported stylesheet's own
+ // contents, fetched from the browser (not the Superset backend, so this
+ // carries none of the SSRF risk a server-side fetch of an editor-supplied
+ // URL would). @import is rejected on save regardless of where it came
+ // from, so this is the migration path for CSS written (or imported) before
+ // that check existed.
+ const handleConvertCssImports = useCallback(async () => {
+ setIsConvertingCssImports(true);
+ setCssImportConversionMessage(null);
+ try {
+ const result = await resolveCssImports(customCss);
+ if (result.resolvedCount > 0) {
+ onCustomCssChange(result.css);
Review Comment:
Fixed — the editor now goes read-only for the duration of the conversion
(`readOnly={isConvertingCssImports}`), so there's no window for an edit to get
clobbered by the async result.
##########
superset-frontend/src/dashboard/util/resolveCssImports.ts:
##########
@@ -0,0 +1,124 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import type { AtRule } from 'postcss';
+
+// Mirrors the "@import" entry of _DANGEROUS_CSS_PATTERNS in
+// superset/dashboards/schemas.py's validate_css. This only decides whether
+// to offer the "convert @import" action below -- the backend validator is
+// the actual gate on save either way, so keeping this exactly in sync is a
+// UX nicety, not a security requirement.
+const CSS_IMPORT_PATTERN = /@import\b/i;
+
+export function hasCssImport(css: string): boolean {
+ return CSS_IMPORT_PATTERN.test(css);
+}
+
+export interface ResolveCssImportsResult {
+ css: string;
+ resolvedCount: number;
+ unresolvedUrls: string[];
+}
+
+/**
+ * Extracts the URL from an `@import` at-rule's raw params, e.g.
+ * `url('https://fonts.googleapis.com/css2?family=Inter') screen` or
+ * `"https://example.com/x.css"`. Returns null for a params string this
+ * can't confidently pull a URL out of.
+ */
+function extractImportUrl(params: string): string | null {
+ const match = params
+ .trim()
+ .match(/^url\(\s*['"]?([^'")]+)['"]?\s*\)|^['"]([^'"]+)['"]/i);
+ if (!match) {
+ return null;
+ }
+ return match[1] ?? match[2] ?? null;
+}
+
+/**
+ * Replaces every top-level `@import url(...)` in `css` with the fetched
+ * target stylesheet's own contents, so the result can be saved without
+ * tripping the backend's `@import` rejection. The fetch happens in the
+ * caller's own browser, not on the Superset backend, so this carries none
+ * of the SSRF risk a server-side fetch of an editor-supplied URL would --
+ * and every dashboard viewer's browser already fetches the same URL today
+ * whenever `@import`-ing CSS renders, so this isn't new exposure, only a
+ * one-time version of exposure that already happens on every view.
+ *
+ * An `@import` whose target can't be fetched (CORS, network error, a
+ * non-2xx response, or a response that isn't CSS) is left untouched in the
+ * output and reported in `unresolvedUrls`, rather than silently dropped, so
+ * a save attempt still fails with a clear reason and nothing is lost.
+ *
+ * Only one level of `@import` is resolved: an `@import` found inside a
+ * fetched stylesheet is left as-is in the merged output. A save with a
+ * remaining `@import` still fails backend validation exactly as before --
+ * there is no security reliance on this function fully resolving anything,
+ * only a UX convenience for the common case (a single Google-Fonts-style
+ * `@import` resolving to a handful of `@font-face` rules).
+ */
+export async function resolveCssImports(
+ css: string,
+): Promise<ResolveCssImportsResult> {
+ if (!hasCssImport(css)) {
+ return { css, resolvedCount: 0, unresolvedUrls: [] };
+ }
+
+ const postcss = (await import('postcss')).default;
+ const root = postcss.parse(css);
+ const importRules = root.nodes.filter(
+ (node): node is AtRule =>
+ node.type === 'atrule' && node.name.toLowerCase() === 'import',
+ );
+
+ let resolvedCount = 0;
+ const unresolvedUrls: string[] = [];
+
+ await Promise.all(
+ importRules.map(async rule => {
+ const url = extractImportUrl(rule.params);
+ if (!url) {
+ unresolvedUrls.push(rule.params);
+ return;
+ }
+ try {
+ const response = await fetch(url);
+ if (!response.ok) {
+ throw new Error(`Fetching ${url} returned HTTP ${response.status}`);
+ }
+ const contentType = response.headers.get('content-type') ?? '';
+ if (contentType && !contentType.includes('css')) {
+ throw new Error(`${url} did not return a CSS response`);
+ }
+ const importedCss = await response.text();
+ const importedRoot = postcss.parse(importedCss);
+ rule.replaceWith(importedRoot.nodes);
Review Comment:
Fixed — fetched stylesheets now get their `url(...)` references rebased
against the import URL before merging, so relative font/image paths resolve
correctly instead of against the dashboard document.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
