diff --git a/packages/desktop-client/src/style/customThemes.test.ts b/packages/desktop-client/src/style/customThemes.test.ts index ff284160a4..cbf6683473 100644 --- a/packages/desktop-client/src/style/customThemes.test.ts +++ b/packages/desktop-client/src/style/customThemes.test.ts @@ -457,30 +457,18 @@ describe('validateThemeCss', () => { }); describe('invalid CSS - function calls (security)', () => { - describe('var() function - should reject all var() references', () => { + describe('var() function - should accept var(--name) only (no fallbacks)', () => { it.each([ { description: 'simple var() call', css: `:root { --color-primary: var(--other-var); - }`, - }, - { - description: 'var() with fallback', - css: `:root { - --color-primary: var(--other-var, #007bff); }`, }, { description: 'var() with whitespace', css: `:root { --color-primary: var( --other-var ); - }`, - }, - { - description: 'nested var() calls', - css: `:root { - --color-primary: var(--var1, var(--var2)); }`, }, { @@ -495,14 +483,43 @@ describe('validateThemeCss', () => { --color-primary: VaR(--other-var); }`, }, - ])('should reject CSS with $description', ({ css }) => { - expect(() => validateThemeCss(css)).toThrow( - /Only simple CSS values are allowed/, - ); + ])('should accept CSS with $description', ({ css }) => { + expect(() => validateThemeCss(css)).not.toThrow(); }); }); - describe('other function calls - should reject all except rgb/rgba/hsl/hsla', () => { + describe('var() function - should reject var() with fallback or invalid form', () => { + it.each([ + { + description: 'var() with fallback', + css: `:root { + --color-primary: var(--other-var, #007bff); + }`, + }, + { + description: 'var() with invalid variable name (no --)', + css: `:root { + --color-primary: var(not-a-custom-prop); + }`, + }, + { + description: 'var() with empty variable name', + css: `:root { + --color-primary: var(--); + }`, + }, + { + description: 'var() with unbalanced parentheses', + css: `:root { + --color-primary: var(--other-var; + }`, + }, + ])('should reject CSS with $description', ({ css }) => { + expect(() => validateThemeCss(css)).toThrow(); + }); + }); + + describe('other function calls - should reject all except rgb/rgba/hsl/hsla and var()', () => { it.each([ { description: 'calc() function', diff --git a/packages/desktop-client/src/style/customThemes.ts b/packages/desktop-client/src/style/customThemes.ts index 4a2e28be6f..a19d1dad36 100644 --- a/packages/desktop-client/src/style/customThemes.ts +++ b/packages/desktop-client/src/style/customThemes.ts @@ -73,10 +73,19 @@ export async function fetchDirectCss(url: string): Promise { return response.text(); } +/** Only var(--custom-property-name) is allowed; no fallbacks. Variable name: -- then [a-zA-Z0-9_-]+ (no trailing dash). */ +const VAR_ONLY_PATTERN = /^var\s*\(\s*(--[a-zA-Z0-9_-]+)\s*\)$/i; + +function isValidSimpleVarValue(value: string): boolean { + const m = value.trim().match(VAR_ONLY_PATTERN); + if (!m) return false; + const name = m[1]; + return name !== '--' && !name.endsWith('-'); +} + /** * Validate that a CSS property value only contains allowed content (allowlist approach). - * Only simple, safe CSS values are allowed - no functions (except rgb/rgba/hsl/hsla), no URLs, no complex constructs. - * Explicitly rejects var() and other function calls to prevent variable references and complex expressions. + * Allows: colors (hex, rgb/rgba, hsl/hsla), lengths, numbers, keywords, and var(--name) only (no fallbacks). */ function validatePropertyValue(value: string, property: string): void { if (!value || value.length === 0) { @@ -85,12 +94,15 @@ function validatePropertyValue(value: string, property: string): void { const trimmedValue = value.trim(); + if (isValidSimpleVarValue(trimmedValue)) { + return; + } + // Allowlist: Only allow specific safe CSS value patterns // 1. Hex colors: #RGB, #RRGGBB, or #RRGGBBAA (3, 6, or 8 hex digits) const hexColorPattern = /^#[0-9a-fA-F]{3}([0-9a-fA-F]{3})?([0-9a-fA-F]{2})?$/; // 2. RGB/RGBA functions: rgb(...) or rgba(...) with simple numeric/percentage values - // Allow optional whitespace and support both integers and decimals const rgbRgbaPattern = /^rgba?\(\s*\d+%?\s*,\s*\d+%?\s*,\s*\d+%?\s*(,\s*[\d.]+)?\s*\)$/; @@ -123,7 +135,7 @@ function validatePropertyValue(value: string, property: string): void { // If none of the allowlist patterns match, reject the value throw new Error( - `Invalid value "${trimmedValue}" for property "${property}". Only simple CSS values are allowed (colors, lengths, numbers, or keywords). Functions (including var()), URLs, and other complex constructs are not permitted.`, + `Invalid value "${trimmedValue}" for property "${property}". Only simple CSS values are allowed (colors, lengths, numbers, keywords, or var(--name)). Other functions, URLs, and complex constructs are not permitted.`, ); } diff --git a/packages/docs/docs/experimental/custom-themes.md b/packages/docs/docs/experimental/custom-themes.md index efbbb3b8fc..4dc83eb94d 100644 --- a/packages/docs/docs/experimental/custom-themes.md +++ b/packages/docs/docs/experimental/custom-themes.md @@ -62,9 +62,21 @@ Custom themes must be written as CSS using the `:root` selector with CSS custom - The CSS must contain **exactly** `:root { ... }` and nothing else - Only custom properties starting with `--` are allowed; Actual uses `--color-*` variables for theming +- **Values** may use `var(--custom-property-name)` to reference other variables (e.g. to reuse existing theme variables). Only this form is allowed; fallbacks like `var(--name, value)` are not supported. - No other selectors, at-rules (@import, @media, etc.), or nested blocks are allowed - Comments are allowed and will be stripped during validation +Example using variables: + +```css +:root { + --color-pageBackground: #1a1a1a; + --color-pageText: #ffffff; + /* Reuse another variable */ + --color-buttonPrimaryBackground: var(--color-pageTextLink); +} +``` + ### Available CSS Variables Custom themes can override any of the CSS variables defined in Actual's base themes. These variables correspond to the theme color keys found in the theme files: @@ -125,9 +137,10 @@ When you paste CSS or install from a catalog, the theme is validated to ensure i 1. Must contain exactly `:root { ... }` 2. Only custom properties starting with `--` are allowed; Actual uses `--color-*` variables for theming -3. No at-rules (@import, @media, @keyframes, etc.) -4. No nested selectors or blocks -5. No content outside the `:root` block +3. Values may be literals (colors, lengths, etc.) or `var(--name)` references (no fallbacks) +4. No at-rules (@import, @media, @keyframes, etc.) +5. No nested selectors or blocks +6. No content outside the `:root` block If validation fails, you'll see an error message explaining what's wrong. diff --git a/upcoming-release-notes/7018.md b/upcoming-release-notes/7018.md new file mode 100644 index 0000000000..59ec788a8f --- /dev/null +++ b/upcoming-release-notes/7018.md @@ -0,0 +1,6 @@ +--- +category: Enhancements +authors: [MatissJanis] +--- + +Custom themes: allow using simple CSS variables