Allow color functions as default site background - #876
Conversation
WalkthroughColor custom-setting validation now accepts HSL, HWB, RGB, LAB, OKLAB, LCH, OKLCH, and three- or six-digit hexadecimal formats. Invalid values continue to produce the existing failure code. Test fixtures now include an OKLCH value as a valid default and an invalid color case, while the expected failure keys include the color-default validation result. Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)test/fixtures/themes/010-packagejson/invalid-custom-theme/package.jsonTraceback (most recent call last): test/fixtures/themes/010-packagejson/valid-custom-theme/package.jsonTraceback (most recent call last): 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/fixtures/themes/010-packagejson/valid-custom-theme/package.json (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover each accepted color syntax.
This fixture only exercises
oklch(...); regressions inhsl,hwb,rgb,lab,oklab,lch, or 3-/6-digit hexadecimal handling would remain undetected. Add valid cases for each syntax family, including numeric/percentage and angle variants.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/fixtures/themes/010-packagejson/valid-custom-theme/package.json` at line 18, Add valid color fixture entries alongside the existing default value in the valid-custom-theme package fixture to cover hsl, hwb, rgb, lab, oklab, lch, and 3-/6-digit hexadecimal syntax, including numeric/percentage and angle variants where supported. Keep all values valid and preserve the existing oklch case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/checks/010-package-json.js`:
- Around line 197-204: Update the syntaxes array in the color validation case to
use separate CSS Color 4 patterns for hsl/hwb, rgb, lab/oklab, and lch/oklch.
Accept unitless saturation/lightness and comma or space-separated legacy forms
where valid, optional slash alpha values, and hue angles such as 45deg; reject
comma-separated lab/oklab forms. Add regression cases covering the listed valid
and invalid boundaries.
In `@test/fixtures/themes/010-packagejson/invalid-custom-theme/package.json`:
- Around line 43-46: Update the default value in the
default_background_color_invalid fixture entry to use a color form rejected by
the current validator, such as an oklch value with none or an alpha component,
so it exercises GS010-PJ-CUST-THEME-SETTINGS-COLOR-DEFAULT.
---
Nitpick comments:
In `@test/fixtures/themes/010-packagejson/valid-custom-theme/package.json`:
- Line 18: Add valid color fixture entries alongside the existing default value
in the valid-custom-theme package fixture to cover hsl, hwb, rgb, lab, oklab,
lch, and 3-/6-digit hexadecimal syntax, including numeric/percentage and angle
variants where supported. Keep all values valid and preserve the existing oklch
case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f41ba55-2a54-44ee-b1c3-334e828e4ff9
📒 Files selected for processing (4)
lib/checks/010-package-json.jstest/010-package-json.test.jstest/fixtures/themes/010-packagejson/invalid-custom-theme/package.jsontest/fixtures/themes/010-packagejson/valid-custom-theme/package.json
| case 'color': { | ||
| const syntaxes = [ | ||
| /^(hsl|hwb)\(-?\d+(\.\d+)?(deg|grad|rad|turn)? -?\d+(\.\d+)?% -?\d+(\.\d+)?%\)$/i, | ||
| /^(rgb|lab|oklab)\(-?\d+(\.\d+)?%?,? -?\d+(\.\d+)?%?,? -?\d+(\.\d+)?%?\)$/i, | ||
| /^(lch|oklch)\(-?\d+(\.\d+)?%? -?\d+(\.\d+)?%? -?\d+(\.\d+)?(deg|grad|rad|turn)?\)$/i, | ||
| /^#[0-9a-f]{6}$/i, | ||
| /^#[0-9a-f]{3}$/i | ||
| ]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant checker and nearby test/data coverage without running repo code.
fd -a '010-package-json\.js|package-json' . | sed 's#^\./##' | head -50
echo '--- outline ---'
ast-grep outline lib/checks/010-package-json.js --view expanded || true
echo '--- relevant lines 160-240 ---'
sed -n '160,240p' lib/checks/010-package-json.js | nl -ba -v160
echo '--- references to color validation / color-scheme / color ---'
rg -n "color|scheme|syntaxes|theme\.results\.fail|theme\.results\.pass" lib bin app -g '*.js' | sed -n '1,220p'Repository: TryGhost/gscan
Length of output: 1203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- lib/checks/010-package-json.js lines 160-240 ---'
sed -n '160,240p' lib/checks/010-package-json.js | awk '{printf "%6d %s\n", NR+159, $0}'
echo '--- test coverage around color/scheme/color-scheme/color ---'
rg -n "color|scheme|color-scheme|color_scheme|syntaxes|regex|color:" test/010-package-json.test.js lib/utils/package-json.js -C 3
echo '--- relevant validation rules snippets ---'
for pattern in 'colorScheme|scheme|color'; do
echo "## $pattern"
rg -n "$pattern|colorScheme|scheme|color:" lib/checks/010-package-json.js test/010-package-json.test.js -C 2
done
echo '--- package checker function context ---'
sed -n '1,140p' lib/checks/010-package-json.js | awk '{printf "%6d %s\n", NR, $0}'Repository: TryGhost/gscan
Length of output: 4569
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- test coverage around color/color defaults/syntaxes ---'
rg -n "color|color-scheme|colorScheme|color_scheme|syntaxes|regex|invalidCustomThemeSetingColorDefault|color:" test/010-package-json.test.js -C 3 || true
echo '--- package metadata/schema references around color type ---'
rg -n "color'|\"color|customSettings|theme.*color|color.*default|packageJSON\.config\.custom" -g '*.js' -g '*.json' -g '!node_modules' . | sed -n '1,240p'
echo '--- read-only regex behavior probe for representative CSS color forms ---'
node - <<'JS'
const syntaxes = [
/^(hsl|hwb)\(-?\d+(\.\d+)?(deg|grad|rad|turn)? -?\d+(\.\d+)?% -?\d+(\.\d+)?%\)$/i,
/^(rgb|lab|oklab)\(-?\d+(\.\d+)?%?,? -?\d+(\.\d+)?%?,? -?\d+(\.\d+)?%?\)$/i,
/^(lch|oklch)\(-?\d+(\.\d+)?%? -?\d+(\.\d+)?%? -?\d+(\.\d+)?(deg|grad|rad|turn)?\)$/i,
/^#[0-9a-f]{6}$/i,
/^#[0-9a-f]{3}$/i
];
const cases = [
'hsl(120 50 50)', 'hsl(120,100%,50%)', 'hsl(120 50 50 / 0.5)',
'hwb(120 10 20)', 'hwb(120 10 20 / 50%)',
'rgb(255,0,0)', 'rgb(255 0 0)', 'rgb(255, 0, 0 / 0.5)',
'lab(50% 0 0)', 'lab(50%,0,0)', 'lab(50% 0 0 / 0.5)', 'lab(50% 0 0 / 50%)',
'oklab(50% 0 0)', 'oklab(50%,0,0)',
'lch(50% 0 0)', 'lch(50% 0 0 45deg)',
'oklch(50% 0 0)', 'oklch(50% 0 0 45deg)',
'`#123456`', '`#123`'
];
for (const value of cases) {
const accepted = syntaxes.some(s => s.test(value));
console.log(`${accepted ? "accept" : "reject"}: ${value}`);
}
JSRepository: TryGhost/gscan
Length of output: 4602
🌐 Web query:
CSS Color 4 hsl alpha component numeric percentages hwb rgb lab oklab grammar
💡 Result:
In CSS Color Module Level 4, the syntax for color functions has been modernized to provide a consistent, space-separated format that supports an optional alpha channel [1][2]. Modern Syntax and Alpha Component All absolute color functional forms—including rgb, hsl, hwb, lab, and oklab—utilize a modern, space-separated syntax [1][2]. In this format, the alpha component is specified after a slash (/) separator [2]. The alpha channel is defined by the type, which accepts either a (typically 0 to 1) or a (0% to 100%) [1][3]. If the alpha component is omitted, it defaults to 100% (fully opaque) [3]. Grammar Overview The modern grammar for these functions generally follows this structure: rgb = rgb( [ | | none ]{3} [ / [ | none ] ]?) hsl = hsl( [ | | none ]{2} [ / [ | none ] ]?) hwb = hwb( [ | | none ]{2} [ / [ | none ] ]?) lab = lab( [ | | none ]{3} [ / [ | none ] ]?) oklab = oklab( [ | | none ]{3} [ / [ | none ] ]?) Key syntax points include: - Separators: Unlike legacy syntax which used commas, modern functions use spaces to separate required components and a forward slash (/) to separate the optional alpha value [2]. - Legacy Aliases: Legacy functions like rgba and hsla persist as aliases for their modern counterparts (rgb and hsl), but the modern, space-separated syntax is the recommended standard for new CSS [1][3][2]. - Numeric Percentages: Depending on the specific channel, values can be provided as numbers (raw values) or percentages [3]. For example, in rgb, numbers represent 0–255, while percentages represent 0%–100% [3]. In alpha-value specifically, 0 or 0% is fully transparent, and 1 or 100% is fully opaque [3].
Citations:
- 1: https://www.w3.org/TR/css-color-4/
- 2: https://developer.mozilla.org/en-US/blog/css-color-module-level-4/
- 3: https://github.com/w3c/csswg-drafts/blob/a54f8b2089d16eca696690fe7ffc3c11d4db9861/css-color/Overview.bs
🌐 Web query:
"css-color" "syntax" "hsl(" "<hue>" "<percentage>" "<number>" "/" alpha-value Lab Oklab comma-separated
💡 Result:
In modern CSS (specifically defined in the CSS Color Module Level 4 and later), color functions generally follow a space-separated syntax. The requirement for commas depends on the specific function and whether you are using the modern or legacy notation [1][2][3]. HSL Syntax The hsl function supports two types of syntax [2][3]: Modern Syntax (Space-Separated): Values are separated by spaces, and an optional alpha value is preceded by a forward slash (/). Example: hsl(120 100% 50% / 0.5) [1][4]. Legacy Syntax (Comma-Separated): For backward compatibility, the function accepts comma-separated values. In this form, the alpha value is also separated by a comma. Example: hsl(120, 100%, 50%, 0.5) [1][2]. Lab and Oklab Syntax The lab and oklab color functions use a space-separated syntax exclusively [5][6][7]. They do not support comma-separated syntax [5]. Example: oklab(40.1% 0.1143 0.045 / 0.5) [8][5]. The components (L, a, b) are separated by spaces, and the alpha value (if present) is separated by a slash (/) [8][6]. Summary Table Function | Syntax Style | Commas Supported? | Alpha Separator --- | --- | --- | --- hsl | Space or Comma | Yes (Legacy only) | / (Space) or, (Comma) lab | Space only | No | / (Slash) oklab | Space only | No | / (Slash) The use of space-separated syntax is the recommended approach in modern CSS [1][9]. The comma-separated syntax for hsl is retained solely for legacy support, and the "none" keyword (which can be used for any channel) is not permitted in that legacy comma-separated form [1][4]. [8][10][5][6][1][2]
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/color_value/hsl
- 2: https://css-tricks.com/almanac/functions/h/hsl/
- 3: https://typeerror.org/index.php/docs/css/color_value/hsl()
- 4: https://github.com/mdn/content/blob/main/files/en-us/web/css/reference/values/color_value/hsl/index.md
- 5: https://css-tricks.com/almanac/functions/o/oklab/
- 6: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/color_value/lab
- 7: https://colorsift.com/reference/css-color-functions
- 8: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/color_value/oklab
- 9: CSS hsl() syntax: commas or not? mdn/content#23236
- 10: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/color_value/oklch
Align the color regexes with the CSS Color 4 grammars.
Line 206 rejects valid defaults such as hsl(120 50 50), hwb(120 10 20), hsl(120,100%,50%), rgb(255,0,0), alpha forms like hsl(120 50 50 / 0.5), and lch()/oklch() hue angles like 45deg. It also accepts comma-separated lab()/oklab(), which are not comma-separated. Split the patterns by function family and add regression cases for these boundaries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/checks/010-package-json.js` around lines 197 - 204, Update the syntaxes
array in the color validation case to use separate CSS Color 4 patterns for
hsl/hwb, rgb, lab/oklab, and lch/oklch. Accept unitless saturation/lightness and
comma or space-separated legacy forms where valid, optional slash alpha values,
and hue angles such as 45deg; reject comma-separated lab/oklab forms. Add
regression cases covering the listed valid and invalid boundaries.
| "default_background_color_invalid": { | ||
| "type": "color", | ||
| "default": "oklch(0 0 0)" | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg 'css.*color|colors?|themes|package\.json|invalid-custom-theme|invalidCustomThemeSetingColorDefault' || true
echo "== fixture file =="
if [ -f test/fixtures/themes/010-packagejson/invalid-custom-theme/package.json ]; then
nl -ba test/fixtures/themes/010-packagejson/invalid-custom-theme/package.json | sed -n '35,55p'
fi
echo "== search regex/color validation symbols =="
rg -n "oklch|lch|color|invalidCustomThemeSetingColorDefault|invalid.*Color|custom.*Color|default_background_color" . \
-g '!node_modules' -g '!dist' -g '!build' 2>/dev/null | head -200Repository: TryGhost/gscan
Length of output: 41541
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fixture snippet =="
python3 - <<'PY'
from pathlib import Path
path = Path('test/fixtures/themes/010-packagejson/invalid-custom-theme/package.json')
text = path.read_text()
for i,line in enumerate(text.splitlines(),1):
if 38 <= i <= 50:
print(f"{i}: {line}")
PY
echo "== relevant source and tests =="
rg -n "invalidCustomThemeSetingColorDefault|default_background_color_invalid|check.*color|colorDefault|oklch|lch|invalid.*Custom.*theme|packagejson" . \
-g '!node_modules' -g '!dist' -g '!build' | head -250
echo "== package-json fixture keys around invalid color =="
python3 - <<'PY'
from pathlib import Path
import json
path = Path('test/fixtures/themes/010-packagejson/invalid-custom-theme/package.json')
data = json.loads(path.read_text())
custom = data.get('custom') or data.get('settings') or {}
for k in custom:
print(k, ':', custom[k])
PYRepository: TryGhost/gscan
Length of output: 6930
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== color validation implementation =="
sed -n '180,215p' lib/checks/010-package-json.js
echo "== invalid custom theme expected failures =="
python3 - <<'PY'
from pathlib import Path
import ast
import re, json
paths = [
'test/010-package-json.test.js',
'test/fixtures/themes/010-packagejson/invalid-custom-theme/package.json',
'lib/checks/010-package-json.js'
]
for p in paths:
print(f"\n[FILE] {p}")
text = Path(p).read_text()
# extract js object-ish lines around relevant fixture symbol/name
for i,line in enumerate(text.splitlines(),1):
if 'invalid-custom-theme' in line or 'default_background_color_invalid' in line or 'GS010-PJ-CUST-THEME-SETTINGS-COLOR-DEFAULT' in line:
left = max(1, i-6); right = min(len(text.splitlines()), i+10)
for j in range(left, right+1):
print(f"{j}: {text.splitlines()[j-1]}")
break
PY
echo "== current regex behavioral probe =="
python3 - <<'PY'
import re
source = Path('lib/checks/010-package-json.js').read_text()
m = re.search(r"^(lch\|oklch)[^\n]+" , source)
# reconstruct from source literal using the actual regex seen by the parser:
regex = re.compile(r"^(lch|oklch)\(-?\d+(\.\d+)?%? -?\d+(\.\d+)?%? -?\d+(\.\d+)?(deg|grad|rad|turn)?\)$", re.I)
for value in ["oklch(0 0 0)", "oklch(none 0 0)", "oklch(0 0 0 / 50%)", "lch(0 0 0)", "oklch(0.5 0.5 0.5)"]:
print(value, "=>", bool(regex.match(value)))
PYRepository: TryGhost/gscan
Length of output: 4900
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== expected failure keys for invalid-custom-theme =="
python3 - <<'PY'
from pathlib import Path
text = Path('test/010-package-json.test.js').read_text().splitlines()
for i,line in enumerate(text, 1):
if "it('valid theme'" in line:
start = max(1, i-40)
end = min(len(text), i+4)
for j in range(start, end):
print(f"{j}: {text[j-1]}")
PyErr
PY
echo "== CSS color regex probe =="
python3 - <<'PY'
import re
from pathlib import Path
source = Path('lib/checks/010-package-json.js').read_text()
# Extract the lch/oklch regex literal from the source text to avoid hard-coding changes.
m = re.search(r"/\^\(lch\|oklch\)\\s*\([^\n]+\\)\$/i", source)
print("regex literal found:", bool(m), m.group(0) if m else None)
regex = re.compile(r"^(lch|oklch)\(-?\d+(\.\d+)?%? -?\d+(\.\d+)?%? -?\d+(\.\d+)?(deg|grad|rad|turn)?\)$", re.I)
for value in ["oklch(0 0 0)", "oklch(none 0 0)", "oklch(0 0 0 / 50%)", "lch(0 0 0)", "oklch(0.5 0.5 0.5)"]:
print(value, "=>", bool(regex.match(value)))
PYRepository: TryGhost/gscan
Length of output: 317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path('test/010-package-json.test.js').read_text().splitlines()
for i,line in enumerate(text, 1):
if 'invalid-custom-theme' in line:
if i > 60:
start = max(1, i-80)
else:
start = 1
end = min(len(text), i+90)
for j in range(start, end):
print(f"{j}: {text[j-1]}")
PY
python3 - <<'PY'
import re
regex = re.compile(r"^(lch|oklch)\(-?\d+(\.\d+)?%? -?\d+(\.\d+)?%? -?\d+(\.\d+)?(deg|grad|rad|turn)?\)$", re.I)
for value in ["oklch(0 0 0)", "oklch(none 0 0)", "oklch(0 0 0 / 50%)", "lch(0 0 0)", "oklch(0.5 0.5 0.5)"]:
print(value, "=>", bool(regex.match(value)))
PYRepository: TryGhost/gscan
Length of output: 24059
Use an invalid color for this fixture.
oklch(0 0 0) matches the current color regex, so it passes this check and doesn’t exercise GS010-PJ-CUST-THEME-SETTINGS-COLOR-DEFAULT. Use an excluded form such as oklch(none 0 0) or oklch(0 0 0 / 50%).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/fixtures/themes/010-packagejson/invalid-custom-theme/package.json`
around lines 43 - 46, Update the default value in the
default_background_color_invalid fixture entry to use a color form rejected by
the current validator, such as an oklch value with none or an alpha component,
so it exercises GS010-PJ-CUST-THEME-SETTINGS-COLOR-DEFAULT.
This fix addresses TryGhost/Ghost#22943
I updated the check for custom config colors.
Previously, the check tested for a hex code.
Now the check uses regex to check for hsl, hwb, rgb, lab, oklab, lch, oklch.
The check allows positive and negative numbers for all values, which may not be valid CSS.
The check allows for angles (rad, grad, turn, deg) or percents where valid.
The check does not allow for alphas.
The check does not allow for the keyword
noneor any special functions.This is the logic I used to test the regex: