From 490461e09e031a42337683ba2468cffa15b5ea59 Mon Sep 17 00:00:00 2001 From: Francis Secada Date: Wed, 29 Jul 2026 11:40:41 -0400 Subject: [PATCH 1/2] fix(axes): enforce every guarantee the axis layer states (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from the reviews of #5 and #7, consolidated because they touch the same two files and the cross-language parity test binds them together. They are the same mistake four times: a property the package states and does not enforce. Token values are now validated, not just token names. A value containing ';', '{', '}', '<', or a comment delimiter closed its own declaration and wrote rules the app never authored; '' escaped the style element entirely. That mattered most for the deployments cf-ui targets, where value sets come from a database column or an admin form. The plugin already rejected these — the two generators now agree, and a test asserts it. The --cf-spacing -> --spacing alias is dropped. --color-primary* is a name cf-ui owns in a Tailwind context; --spacing is the root of Tailwind v4's entire spacing scale, so data-density was silently rescaling every p-4 and gap-2 in the consuming app by +-20%, including for Bulma consumers using none of it. Opt back in with @theme { --spacing: var(--cf-spacing); }. The wide-gamut lightness invariant is enforced rather than assumed. The contrast gate is computed against the sRGB base and cannot measure oklch(); "the p3 layer holds the same lightness" was the convention carrying that result to wide-gamut displays, held by hand. A typo or a monitor-driven tweak shipped below AA with every test green, invisible to anyone reviewing on an sRGB display. p3_lightness_failures() / p3LightnessFailures() convert each base to OKLab and fail CI beyond 0.5 percentage points — a tolerance set from the shipped data, which sits within 0.05pp. buildAxisBase and buildAxisCss validate by default. They were exported escape hatches that generated CSS with the build error switched off. The opt-out is now explicit, and the parity test passes it deliberately so it still compares raw generation rather than narrowing to input the validator already approved. Also adds .gitattributes pinning the generated artifacts to LF: the generator writes newline="\n" while autocrlf wants CRLF, so regenerating left a phantom zero-line diff and "is the tree clean" meant nothing. BREAKING: token-value validation, the dropped alias, and validating exports are each breaking for anyone relying on the gaps. Migration notes in CHANGELOG.md. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NhqNRBg83czKfr8L6FF5xf --- .gitattributes | 10 + CHANGELOG.md | 42 ++++ docs/tailwind-plugin.md | 40 +++- docs/theming.md | 67 +++++- src/cf_ui/axes.py | 157 +++++++++++++- src/cf_ui/static/cf_ui/cf_ui_axes.css | 3 - src/cf_ui/static/cf_ui/cf_ui_axes.json | 3 +- .../static/cf_ui/cf_ui_tailwind_plugin.mjs | 165 ++++++++++++++- tests/js/plugin.test.mjs | 135 ++++++++++++ tests/unit/test_axes.py | 193 +++++++++++++++++- tests/unit/test_tailwind_plugin.py | 136 +++++++++++- 11 files changed, 924 insertions(+), 27 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..763669a --- /dev/null +++ b/.gitattributes @@ -0,0 +1,10 @@ +# The axis artifacts are generated by `python -m cf_ui.axes`, which writes with +# newline="\n". Without pinning them, git's autocrlf wants CRLF in the working +# tree and every regeneration left the file permanently "modified" with a +# zero-line diff — which makes "is the tree clean" useless as a signal. +src/cf_ui/static/cf_ui/cf_ui_axes.css text eol=lf +src/cf_ui/static/cf_ui/cf_ui_axes.json text eol=lf + +# The vendored Tailwind plugin is hand-written, but it sits beside the two +# above and is read by the same Node tooling — keep the whole trio consistent. +src/cf_ui/static/cf_ui/cf_ui_tailwind_plugin.mjs text eol=lf diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e875ed..d1a832f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,49 @@ ## [Unreleased] +### Changed — BREAKING (#20) + +The axis layer stated four properties it did not enforce. Enforcing them is +breaking for anyone who was relying, knowingly or not, on the gaps. + +- **Token *values* are now validated, not just token names.** A value containing + `;`, `{`, `}`, `<`, `/*`, or `*/` raises `AxisConfigError`, and a non-string + value is rejected outright. Values are interpolated straight into the + `` escaped + the element entirely. This mattered most for the deployments cf-ui explicitly + targets, where value sets come from a database column or an admin form rather + than a hand-written literal. The plugin already rejected these; the two + generators now agree, and a test asserts they reject the same inputs. + **Migration:** none for hand-written value sets. If a value legitimately needs + one of these characters, it is not an axis token. +- **The `--cf-spacing` → `--spacing` alias is gone.** `--color-primary*` is a + name cf-ui owns in a Tailwind context; `--spacing` is the root of Tailwind + v4's entire spacing scale, so `data-density` was silently rescaling every + `p-4` and `gap-2` in the consuming app by ±20% — including for Bulma consumers + using none of it. `--cf-spacing` is still emitted. + **Migration:** to keep the old behavior, add `@theme { --spacing: var(--cf-spacing); }`. +- **`buildAxisBase()` and `buildAxisCss()` validate by default.** They were + exported escape hatches that generated CSS with the build error switched off, + contradicting the guarantee #7 shipped on. Pass `{ validate: false }` to opt + out explicitly. + **Migration:** code passing invalid value sets to these now throws. That was + already producing malformed CSS. + ### Added +- **The wide-gamut lightness invariant is enforced (#20).** `p3_lightness_failures()` + (Python) and `p3LightnessFailures()` (plugin) convert each sRGB base declaration + to OKLab and compare it against the `oklch()` override, failing CI beyond 0.5 + percentage points. The contrast gate is computed against the sRGB base and + cannot measure `oklch()`; "the p3 layer holds the same lightness" was the + convention carrying that result to wide-gamut displays, held by hand. A typo + or a monitor-driven tweak shipped below AA with every test green, invisible to + anyone reviewing on an sRGB display. +- `cf_ui.axes.oklab_lightness()` / `oklch_lightness()`, and their plugin + counterparts `oklabLightness()` / `oklchLightness()` +- `.gitattributes` pinning the generated axis artifacts to LF — `python -m + cf_ui.axes` writes `newline="\n"`, so without it every regeneration left a + phantom zero-line diff and "is the tree clean" meant nothing - Tailwind plugin with build-time axis validation (#7): an unknown axis value now **fails the CSS build** instead of silently producing an unstyled element - `static/cf_ui/cf_ui_tailwind_plugin.mjs` — vendored into the wheel, not published diff --git a/docs/tailwind-plugin.md b/docs/tailwind-plugin.md index b846793..759ba13 100644 --- a/docs/tailwind-plugin.md +++ b/docs/tailwind-plugin.md @@ -88,14 +88,16 @@ Tailwind never tree-shakes; the two concerns are separate. - a value name that is not lowercase-and-hyphens - a mode-keyed value (`accent`, `surface`) missing `light` or `dark` - a token that is not a CSS custom property -- a token value containing `;`, `{`, `}`, or a comment delimiter — those close - the declaration and write rules you did not author +- a token value containing `;`, `{`, `}`, `<`, or a comment delimiter — those + close the declaration and write rules you did not author +- a token value that is not a string - a value or composition name that exists only on `Object.prototype` (`toString`, `hasOwnProperty`, …), and a value set keyed on `__proto__` **Warns** — `contrastReport: true`: - a combination below WCAG AA +- a p3 override that does not hold its base declaration's lightness Contrast is advisory on purpose. An unknown axis value is always wrong. A contrast number below AA may be a deliberate choice in a value set the package @@ -110,6 +112,40 @@ cf-ui: contrast report — 1 of 12 combinations fail WCAG AA: --cf-accent-strong on --cf-ground: 1.13 < 4.5 ``` +## Every export, and whether it validates + +`createPlugin` used to be the only path that ran the gate, so the two exported +generators were the generator with the build error switched off. Passing junk +produced junk quietly. Since 0.2.0 **everything validates by default**, and the +escape hatch has to be asked for. + +| Export | Validates | Notes | +|---|---|---| +| `default` (`cfUiAxes`) | yes | The plugin. Callable as a factory or used directly. | +| `buildAxisBase(valueSets?, definition?, options?)` | yes | `{ validate: false }` skips the gate. | +| `buildAxisCss(valueSets?, definition?, options?)` | yes | Same options as above. | +| `mergeValueSets(definition, custom, mode?)` | yes | This *is* the gate for value sets. | +| `resolveComposition(definition, composition, valueSets)` | yes | Throws on an unknown value — the headline feature. | +| `loadDefinition()` | n/a | Reads `cf_ui_axes.json`; fresh deep copy per call. | +| `contrastRatio(fg, bg)` | n/a | `null` for anything that is not sRGB hex. | +| `contrastReport(valueSets, pairs, modes?)` | no | Pure measurement; returns rows, never throws. | +| `oklabLightness(color)` | n/a | `null` for anything that is not sRGB hex. | +| `oklchLightness(value)` | n/a | `null` for anything that is not `oklch(...)`. | +| `p3LightnessFailures(valueSets?, definition?, tolerance?)` | no | Pure measurement; returns messages. | +| `AxisPluginError` | n/a | Every throw above is one of these. | +| `P3_LIGHTNESS_TOLERANCE` | n/a | `0.005` — 0.5 percentage points of OKLab L. | + +The opt-out exists for legitimate uses — writing the axis CSS to a file, +inspecting a build, comparing the two generators — and the parity test uses it +deliberately, so that test compares raw generation rather than narrowing to +"they agree on input the validator already approved". Do not reach for it with +input you did not write. + +```js +// Explicit, and only ever for input you control. +const css = buildAxisCss(mySets, undefined, { validate: false }); +``` + ## One definition, two generators — and a test that proves it `cf_ui/axes.py` is still the single source of truth. It generates two diff --git a/docs/theming.md b/docs/theming.md index 677da0b..bc1f1f5 100644 --- a/docs/theming.md +++ b/docs/theming.md @@ -44,8 +44,8 @@ One setting resolves to all five axes. ```python # settings.py -CF_UI_COMPOSITION = "editorial" # or a per-axis mapping: -CF_UI_COMPOSITION = {"accent": "jade"} # unnamed axes fall back to `default` +CF_UI_COMPOSITION = "editorial" # or a per-axis mapping: +CF_UI_COMPOSITION = {"accent": "jade"} # unnamed axes fall back to `default` ``` ```django @@ -112,8 +112,8 @@ CF_UI_AXIS_VALUES = { } } } -CF_UI_AXIS_VALUES_MODE = "extend" # default; "replace" drops the shipped - # values for each axis you supply +CF_UI_AXIS_VALUES_MODE = "extend" # default; "replace" drops the shipped +# values for each axis you supply CF_UI_COMPOSITION = {"accent": "brand"} ``` @@ -164,8 +164,12 @@ for accent in sets["accent"]: The shipped set is held to exactly this check in `tests/unit/test_axes.py`. -> Issue #7 will move this from a runtime check to a build-time one via a -> Tailwind v4 plugin, so an inaccessible value set cannot compile. +> The Tailwind plugin (#7) surfaces this at build time as well, via +> `contrastReport: true`. It **warns** rather than failing the build: an unknown +> axis value is always a mistake, but a ratio below AA may be a deliberate +> choice in a value set cf-ui did not ship, and making it fatal would mean this +> package decides when your app may compile. See +> [the plugin docs](tailwind-plugin.md#what-fails-the-build-and-what-only-warns). ### Why `--cf-accent-strong` exists @@ -211,6 +215,31 @@ guarantee carries over. Near-neutral accents (`slate`) ship no p3 block; there is no wide-gamut chroma to recover. +### The lightness invariant is enforced, not assumed + +"Holds lightness constant" used to be a convention maintained by hand. It is now +a gate: `p3_lightness_failures()` converts each sRGB base declaration to OKLab +and compares its lightness against the `oklch()` override, and a unit test fails +CI if any override drifts by more than **0.5 percentage points**. + +That tolerance is set from the shipped data — the overrides are authored to one +decimal place, which puts them within 0.05pp of their base, so 0.5pp leaves an +order of magnitude of headroom while still catching any drift large enough to +change the contrast result. + +```python +from cf_ui.axes import p3_lightness_failures, DEFAULT_VALUE_SETS + +p3_lightness_failures(DEFAULT_VALUE_SETS) # [] — shippable +``` + +The same check runs in the Tailwind plugin as `p3LightnessFailures()`, with +byte-identical messages; a parity test compares the two. Because the sRGB +contrast gate cannot measure `oklch()` directly, this invariant is what carries +the WCAG result over to wide-gamut displays — without it, a p3 override could +ship below AA with every test green, invisible to anyone reviewing on an sRGB +display. + --- ## Tailwind interop @@ -223,10 +252,30 @@ a Tailwind-based theme picks the axes up without per-component work: | `--cf-accent` | `--color-primary` | | `--cf-accent-content` | `--color-primary-content` | | `--cf-accent-strong` | `--color-primary-strong` | -| `--cf-spacing` | `--spacing` | -Density therefore drives Tailwind's spacing base unit, so it propagates through -every spacing utility. The aliases are inert when no Tailwind is present. +The aliases are inert when no Tailwind is present. + +### `--spacing` is not aliased + +`--cf-spacing` is emitted, but it is **not** wired to Tailwind's `--spacing`. + +`--color-primary` is a name cf-ui effectively owns in a Tailwind context. +`--spacing` is the root of Tailwind v4's *entire* spacing scale — `p-4`, `gap-2`, +`m-8` and every other spacing utility derive from it. Aliasing it meant +`data-density="compact"` silently rescaled the whole consuming app by ±20%, +scoped to an attribute the app set for cf-ui's benefit. A Bulma consumer who +also ran Tailwind got that as pure collateral damage. + +Apps that *want* density to drive the Tailwind scale opt in with one line: + +```css +@theme { + --spacing: var(--cf-spacing); +} +``` + +> **Changed in 0.2.0.** This alias used to be emitted for every theme. If you +> were relying on it, add the rule above. --- diff --git a/src/cf_ui/axes.py b/src/cf_ui/axes.py index 0ae4847..90e24d9 100644 --- a/src/cf_ui/axes.py +++ b/src/cf_ui/axes.py @@ -43,6 +43,7 @@ "DEFAULT_VALUE_SETS", "MODES", "MODE_KEYED_AXES", + "P3_LIGHTNESS_TOLERANCE", "AxisConfigError", "axis_definition", "build_axis_globals", @@ -50,6 +51,9 @@ "contrast_ratio", "custom_axis_css", "merge_value_sets", + "oklab_lightness", + "oklch_lightness", + "p3_lightness_failures", "render_axis_css", "resolve_composition", "root_attrs", @@ -82,15 +86,30 @@ class AxisConfigError(ValueError): #: Tailwind v4 reads its theme from custom properties in these namespaces, so #: emitting the aliases lets a Tailwind-based theme pick the axes up without #: per-component work. Harmless when no Tailwind is present. +#: +#: ``--spacing`` is deliberately **not** here. ``--color-primary*`` is a name +#: cf-ui effectively owns in a Tailwind context; ``--spacing`` is the root of +#: Tailwind v4's entire spacing scale, so aliasing it made ``data-density`` +#: silently rescale every ``p-4`` and ``gap-2`` in the consuming app — including +#: for a Bulma consumer using none of it. Apps that want that opt in with one +#: line: ``@theme { --spacing: var(--cf-spacing); }``. _ALIASES: dict[str, str] = { "--cf-accent": "--color-primary", "--cf-accent-content": "--color-primary-content", "--cf-accent-strong": "--color-primary-strong", - "--cf-spacing": "--spacing", } _VALUE_NAME = re.compile(r"^[a-z0-9]+(-[a-z0-9]+)*$") +#: Token values are interpolated straight into generated CSS, which +#: :func:`style_element` injects into the page. A value carrying ``;`` or a +#: brace closes its own declaration and writes rules the app never authored; +#: ```` escapes the element entirely. Rejected rather than escaped — +#: no legitimate axis token needs one, and escaping invites a second round of +#: "but what about...". Kept identical to ``UNSAFE_VALUE`` in +#: ``cf_ui_tailwind_plugin.mjs``; a test asserts both reject the same inputs. +_UNSAFE_VALUE = re.compile(r"[;{}<]|/\*|\*/") + # --------------------------------------------------------------------------- # Shipped value sets @@ -304,6 +323,14 @@ def _validate_value_set(axis: str, name: str, tokens: Any) -> None: ) for mode in MODES: _validate_tokens(axis, name, tokens[mode]) + # The p3 block reaches the same stylesheet, so it gets the same gate. + gamut = tokens.get("p3") + if gamut: + if not isinstance(gamut, Mapping): + raise AxisConfigError(f"axis value {axis}/{name} has a malformed 'p3' block") + for mode in MODES: + if gamut.get(mode): + _validate_tokens(axis, name, gamut[mode]) else: _validate_tokens(axis, name, tokens) @@ -311,12 +338,22 @@ def _validate_value_set(axis: str, name: str, tokens: Any) -> None: def _validate_tokens(axis: str, name: str, tokens: Any) -> None: if not isinstance(tokens, Mapping): raise AxisConfigError(f"axis value {axis}/{name} must be a mapping of custom properties") - for token in tokens: + for token, value in tokens.items(): if not str(token).startswith("--"): raise AxisConfigError( f"axis value {axis}/{name} declares {token!r}: axis tokens must be CSS " "custom properties (a name starting with '--')" ) + if not isinstance(value, str): + raise AxisConfigError( + f"axis value {axis}/{name} declares a non-string {token}: axis token values " + "must be strings" + ) + if _UNSAFE_VALUE.search(value): + raise AxisConfigError( + f"axis value {axis}/{name} declares {token} with an unsafe value: a token may " + "not contain ';', '{', '}', '<', or a comment delimiter" + ) def merge_value_sets( @@ -608,6 +645,122 @@ def contrast_failures( return failures +# --------------------------------------------------------------------------- +# Wide-gamut lightness invariant +# +# The contrast gate above is computed against the sRGB base declarations. The +# p3 block is what actually renders on a wide-gamut display, and it is never +# fed through that gate — `_relative_luminance` would reject oklch outright, +# which is why the p3 block is structured separately in the first place. +# +# What makes the sRGB result carry over is the design rule that a p3 override +# extends *chroma only*, holding lightness constant. That was a convention held +# by hand: a typo, or a tweak that looked better on someone's monitor, shipped +# with every test green and was invisible to anyone reviewing on an sRGB +# display. This enforces it. +# +# Checking the invariant beats re-running WCAG over oklch: it is a smaller and +# more precise check, and it is the property the design actually claims. +# --------------------------------------------------------------------------- + +#: Maximum allowed lightness drift between a p3 override and its sRGB base, in +#: OKLab L (0..1). The shipped overrides are authored to one decimal place of a +#: percentage, so they sit within 0.0005 of their base; 0.005 is an order of +#: magnitude of headroom for that rounding while still catching any drift big +#: enough to matter — a meaningful lightness error moves several points. +P3_LIGHTNESS_TOLERANCE = 0.005 + +# sRGB -> linear -> LMS -> OKLab, per Björn Ottosson's definition. Only the L +# component is needed, so the a/b rows are omitted. +_OKLAB_M1 = ( + (0.4122214708, 0.5363325363, 0.0514459929), + (0.2119034982, 0.6806995451, 0.1073969566), + (0.0883024619, 0.2817188376, 0.6299787005), +) +_OKLAB_L = (0.2104542553, 0.7936177850, -0.0040720468) + +_OKLCH = re.compile(r"^\s*oklch\(\s*([0-9]*\.?[0-9]+)(%?)\s", re.IGNORECASE) + + +def _linear_channel(channel: float) -> float: + return channel / 12.92 if channel <= 0.04045 else ((channel + 0.055) / 1.055) ** 2.4 + + +def oklab_lightness(color: str) -> float: + """OKLab lightness (0..1) of an sRGB hex color. + + This is the same quantity ``oklch()`` states as its first component, which + is what makes the two directly comparable. + """ + value = str(color).strip().lstrip("#") + if len(value) == 3: + value = "".join(char * 2 for char in value) + if len(value) != 6 or not all(char in "0123456789abcdefABCDEF" for char in value): + raise AxisConfigError( + f"cannot measure lightness of {color!r}: axis base declarations must be sRGB hex" + ) + rgb = [_linear_channel(int(value[index : index + 2], 16) / 255) for index in (0, 2, 4)] + cones = [sum(row[i] * rgb[i] for i in range(3)) for row in _OKLAB_M1] + roots = [cone ** (1 / 3) for cone in cones] + return sum(_OKLAB_L[i] * roots[i] for i in range(3)) + + +def oklch_lightness(value: str) -> float: + """Lightness (0..1) declared by an ``oklch(...)`` value.""" + match = _OKLCH.match(str(value)) + if not match: + raise AxisConfigError(f"not an oklch() value: {value!r}") + number = float(match.group(1)) + return number / 100 if match.group(2) else number + + +def p3_lightness_failures( + value_sets: Mapping[str, Mapping[str, Any]], + tolerance: float = P3_LIGHTNESS_TOLERANCE, +) -> list[str]: + """Return every p3 override that does not hold its base declaration's lightness. + + An empty list means the wide-gamut layer preserves the contrast guarantee + the sRGB base was measured for. + + The message format is mirrored exactly by ``p3LightnessFailures`` in + ``cf_ui_tailwind_plugin.mjs`` — a parity test compares the two outputs. + """ + failures: list[str] = [] + limit = tolerance * 100 + + for axis in AXES: + for name, tokens in value_sets.get(axis, {}).items(): + gamut = tokens.get("p3") if isinstance(tokens, Mapping) else None + if not gamut: + continue + for mode in MODES: + overrides = gamut.get(mode) or {} + base = tokens.get(mode) or {} + for token, override in overrides.items(): + where = f"{axis}/{name}/{mode}: {token}" + if token not in base: + failures.append(f"{where} has a p3 override but no base declaration") + continue + try: + actual = oklch_lightness(override) * 100 + except AxisConfigError: + failures.append(f"{where} p3 override is not an oklch() value: {override}") + continue + try: + expected = oklab_lightness(base[token]) * 100 + except AxisConfigError: + failures.append(f"{where} base declaration is not sRGB hex: {base[token]}") + continue + drift = abs(actual - expected) + if drift > limit: + failures.append( + f"{where} p3 lightness {actual:.2f}% deviates from base " + f"{expected:.2f}% by {drift:.2f}pp (tolerance {limit:.2f}pp)" + ) + return failures + + # --------------------------------------------------------------------------- # Cross-language export # --------------------------------------------------------------------------- diff --git a/src/cf_ui/static/cf_ui/cf_ui_axes.css b/src/cf_ui/static/cf_ui/cf_ui_axes.css index 4c580bd..abaf1b4 100644 --- a/src/cf_ui/static/cf_ui/cf_ui_axes.css +++ b/src/cf_ui/static/cf_ui/cf_ui_axes.css @@ -124,21 +124,18 @@ --cf-spacing: 0.2rem; --cf-line-height: 1.4; --cf-control-height: 2rem; - --spacing: var(--cf-spacing); } [data-density="regular"] { --cf-spacing: 0.25rem; --cf-line-height: 1.5; --cf-control-height: 2.5rem; - --spacing: var(--cf-spacing); } [data-density="roomy"] { --cf-spacing: 0.3rem; --cf-line-height: 1.65; --cf-control-height: 3rem; - --spacing: var(--cf-spacing); } /* --- type ------------------------------------------------ */ diff --git a/src/cf_ui/static/cf_ui/cf_ui_axes.json b/src/cf_ui/static/cf_ui/cf_ui_axes.json index d46e71f..95c7ebe 100644 --- a/src/cf_ui/static/cf_ui/cf_ui_axes.json +++ b/src/cf_ui/static/cf_ui/cf_ui_axes.json @@ -24,8 +24,7 @@ "aliases": { "--cf-accent": "--color-primary", "--cf-accent-content": "--color-primary-content", - "--cf-accent-strong": "--color-primary-strong", - "--cf-spacing": "--spacing" + "--cf-accent-strong": "--color-primary-strong" }, "valueNamePattern": "^[a-z0-9]+(-[a-z0-9]+)*$", "valueSets": { diff --git a/src/cf_ui/static/cf_ui/cf_ui_tailwind_plugin.mjs b/src/cf_ui/static/cf_ui/cf_ui_tailwind_plugin.mjs index 51813ef..c429199 100644 --- a/src/cf_ui/static/cf_ui/cf_ui_tailwind_plugin.mjs +++ b/src/cf_ui/static/cf_ui/cf_ui_tailwind_plugin.mjs @@ -65,8 +65,13 @@ export function loadDefinition() { * `;` or a brace closes its own declaration and writes rules the app never * asked for, so it is rejected rather than escaped — there is no legitimate * axis token that needs one. + * + * `<` joined the set in #20: on the Python side the same tokens reach a + * `` escapes the element entirely. Both + * generators carry the identical rule, and a parity test asserts they reject + * the same inputs. */ -const UNSAFE_VALUE = /[;{}]|\/\*|\*\//; +const UNSAFE_VALUE = /[;{}<]|\/\*|\*\//; function validateTokens(axis, name, tokens) { if (tokens === null || typeof tokens !== "object" || Array.isArray(tokens)) { @@ -85,7 +90,7 @@ function validateTokens(axis, name, tokens) { if (UNSAFE_VALUE.test(value)) { throw new AxisPluginError( `axis value ${axis}/${name} declares ${token} with an unsafe value: a token may not ` + - `contain ';', '{', '}', or a comment delimiter`, + `contain ';', '{', '}', '<', or a comment delimiter`, ); } } @@ -227,13 +232,45 @@ function blockFor(definition, tokens) { return block; } +/** + * Run every supplied value set through the same gate `mergeValueSets` applies. + * + * `createPlugin` used to be the only path that validated, so the two exported + * generators below were the generator with the build error switched off — junk + * in produced junk quietly, contradicting the promise #7 shipped on. They now + * validate by default and the escape hatch has to be asked for (#20 §4). + */ +function validateSets(definition, valueSets) { + for (const [axis, values] of Object.entries(valueSets)) { + if (!definition.axes.includes(axis)) { + throw new AxisPluginError( + `unknown axis '${axis}': valid axes are ${definition.axes.join(", ")}`, + ); + } + if (values === null || typeof values !== "object") { + throw new AxisPluginError(`axis '${axis}' must map value names to tokens`); + } + for (const [name, tokens] of Object.entries(values)) { + validateValueSet(definition, axis, name, tokens); + } + } +} + /** * Build the axis rules as a Tailwind `addBase` object, wide-gamut layer * included. + * + * @param {object} [valueSets] Defaults to the shipped sets. + * @param {object} [definition] Defaults to the shipped definition. + * @param {object} [options] + * @param {boolean} [options.validate] On by default. `false` skips the gate — + * an explicit escape hatch for inspecting a build or comparing generators, + * never something to reach for with untrusted input. */ -export function buildAxisBase(valueSets, definition) { +export function buildAxisBase(valueSets, definition, options = {}) { const def = definition ?? loadDefinition(); const sets = valueSets ?? def.valueSets; + if (options.validate !== false) validateSets(def, sets); const base = {}; for (const axis of def.axes) { @@ -269,9 +306,12 @@ export function buildAxisBase(valueSets, definition) { return base; } -/** The same rules as CSS text, for writing to a file or eyeballing a build. */ -export function buildAxisCss(valueSets, definition) { - const base = buildAxisBase(valueSets, definition); +/** + * The same rules as CSS text, for writing to a file or eyeballing a build. + * Validates by default; see {@link buildAxisBase} for the opt-out. + */ +export function buildAxisCss(valueSets, definition, options = {}) { + const base = buildAxisBase(valueSets, definition, options); const render = (rules, indent) => Object.entries(rules) .map(([selector, block]) => { @@ -318,6 +358,12 @@ export function contrastRatio(foreground, background) { * contrast number below AA may be a deliberate choice in a value set the * package did not ship, and turning it into a hard error would mean cf-ui * decides when a consuming app is allowed to compile. + * + * Measures the sRGB base declarations only — `oklch()` returns "cannot be + * checked" here by design, because a ratio is not the right question for the + * p3 layer. What that layer has to hold is its base's *lightness*, which is + * what {@link p3LightnessFailures} checks; the two together cover the wide + * gamut. This function does not validate and never throws. */ export function contrastReport(valueSets, contrastPairs, modes = ["light", "dark"]) { const rows = []; @@ -350,6 +396,109 @@ export function contrastReport(valueSets, contrastPairs, modes = ["light", "dark return rows; } +// --- wide-gamut lightness invariant ---------------------------------------- +// +// `contrastReport` above measures the sRGB base declarations and, for anything +// else, said "not an sRGB hex value, cannot be checked" and moved on. The p3 +// block is what actually renders on a wide-gamut display, so that shrug was +// the whole wide-gamut layer going unchecked. +// +// What carries the sRGB result over is the design rule that a p3 override +// extends chroma only, holding lightness constant. This enforces it, mirroring +// `cf_ui.axes.p3_lightness_failures` message for message — a parity test in +// `tests/unit/test_tailwind_plugin.py` compares the two outputs directly. + +/** @see cf_ui.axes.P3_LIGHTNESS_TOLERANCE — keep the two in step. */ +export const P3_LIGHTNESS_TOLERANCE = 0.005; + +const OKLAB_M1 = [ + [0.4122214708, 0.5363325363, 0.0514459929], + [0.2119034982, 0.6806995451, 0.1073969566], + [0.0883024619, 0.2817188376, 0.6299787005], +]; +const OKLAB_L = [0.2104542553, 0.793617785, -0.0040720468]; +const OKLCH = /^\s*oklch\(\s*([0-9]*\.?[0-9]+)(%?)\s/i; + +/** OKLab lightness (0..1) of an sRGB hex color, or null if it is not one. */ +export function oklabLightness(color) { + let value = String(color).trim().replace(/^#/, ""); + if (value.length === 3) value = [...value].map((char) => char + char).join(""); + if (!/^[0-9a-fA-F]{6}$/.test(value)) return null; + + const rgb = [0, 2, 4].map((index) => { + const channel = parseInt(value.slice(index, index + 2), 16) / 255; + return channel <= 0.04045 ? channel / 12.92 : ((channel + 0.055) / 1.055) ** 2.4; + }); + const roots = OKLAB_M1.map((row) => Math.cbrt(row.reduce((sum, k, i) => sum + k * rgb[i], 0))); + return OKLAB_L.reduce((sum, k, i) => sum + k * roots[i], 0); +} + +/** Lightness (0..1) declared by an `oklch(...)` value, or null. */ +export function oklchLightness(value) { + const match = OKLCH.exec(String(value)); + if (!match) return null; + const number = Number.parseFloat(match[1]); + return match[2] ? number / 100 : number; +} + +/** + * Every p3 override that does not hold its base declaration's lightness. + * + * An empty array means the wide-gamut layer preserves the contrast guarantee + * the sRGB base was measured for. + */ +export function p3LightnessFailures(valueSets, definition, tolerance = P3_LIGHTNESS_TOLERANCE) { + const def = definition ?? loadDefinition(); + const sets = valueSets ?? def.valueSets; + const failures = []; + const limit = tolerance * 100; + + for (const axis of def.axes) { + for (const [name, tokens] of Object.entries(sets[axis] ?? {})) { + const gamut = tokens && typeof tokens === "object" ? tokens.p3 : null; + if (!gamut) continue; + for (const mode of def.modes) { + const overrides = gamut[mode] ?? {}; + const base = tokens[mode] ?? {}; + for (const [token, override] of Object.entries(overrides)) { + const where = `${axis}/${name}/${mode}: ${token}`; + if (!Object.hasOwn(base, token)) { + failures.push(`${where} has a p3 override but no base declaration`); + continue; + } + const actual = oklchLightness(override); + if (actual === null) { + failures.push(`${where} p3 override is not an oklch() value: ${override}`); + continue; + } + const expected = oklabLightness(base[token]); + if (expected === null) { + failures.push(`${where} base declaration is not sRGB hex: ${base[token]}`); + continue; + } + const drift = Math.abs(actual * 100 - expected * 100); + if (drift > limit) { + failures.push( + `${where} p3 lightness ${(actual * 100).toFixed(2)}% deviates from base ` + + `${(expected * 100).toFixed(2)}% by ${drift.toFixed(2)}pp ` + + `(tolerance ${limit.toFixed(2)}pp)`, + ); + } + } + } + } + } + return failures; +} + +function warnP3(failures) { + if (!failures.length) return; + console.warn( + `cf-ui: ${failures.length} wide-gamut override(s) do not hold their base lightness, so ` + + `the sRGB contrast result does not carry over:\n ${failures.join("\n ")}`, + ); +} + function warnContrast(rows) { const failing = rows.filter((row) => row.failures.length); if (!failing.length) { @@ -391,6 +540,10 @@ function createPlugin(options = {}) { if (options.contrastReport) { warnContrast(contrastReport(valueSets, definition.contrastPairs, definition.modes)); + // The sRGB rows above are only half the picture: on a wide-gamut display + // the p3 layer is what renders, and it inherits the contrast result only + // while it holds the base lightness. + warnP3(p3LightnessFailures(valueSets, definition)); } const base = buildAxisBase(valueSets, definition); diff --git a/tests/js/plugin.test.mjs b/tests/js/plugin.test.mjs index 1a73195..8409971 100644 --- a/tests/js/plugin.test.mjs +++ b/tests/js/plugin.test.mjs @@ -19,6 +19,8 @@ import cfUiAxes, { contrastReport, loadDefinition, mergeValueSets, + oklabLightness, + p3LightnessFailures, resolveComposition, } from "../../src/cf_ui/static/cf_ui/cf_ui_tailwind_plugin.mjs"; @@ -361,3 +363,136 @@ describe("buildAxisBase", () => { assert.ok(base['[data-accent="slate"]']); }); }); + +// --- issue #20 -------------------------------------------------------------- + +describe("unsafe token values (#20 §1)", () => { + const UNSAFE = [ + "0; } body { display: none", + "red; color: blue", + "}", + "{", + "red /* c", + "red */", + "", + "<", + ]; + + for (const value of UNSAFE) { + it(`rejects ${JSON.stringify(value)}`, () => { + assert.throws( + () => mergeValueSets(DEFINITION, { form: { probe: { "--cf-radius": value } } }), + /unsafe value/, + ); + }); + } + + it("rejects '<' so a value cannot escape the " } } }), + /unsafe value/, + ); + }); + + it("still accepts a shadow value carrying slashes and parentheses", () => { + const merged = mergeValueSets(DEFINITION, { + form: { probe: { "--cf-radius": "0 1px 2px rgb(0 0 0 / 0.08)" } }, + }); + assert.equal(merged.form.probe["--cf-radius"], "0 1px 2px rgb(0 0 0 / 0.08)"); + }); +}); + +describe("the exported generators validate by default (#20 §4)", () => { + const BAD_NAME = { form: { probe: { "cf-radius": "0" } } }; + const BAD_VALUE = { form: { probe: { "--cf-radius": "0; } html {" } } }; + + it("buildAxisBase rejects a token name that is not a custom property", () => { + assert.throws(() => buildAxisBase(BAD_NAME), /custom propert/); + }); + + it("buildAxisBase rejects an unsafe token value", () => { + assert.throws(() => buildAxisBase(BAD_VALUE), /unsafe value/); + }); + + it("buildAxisCss rejects the same input", () => { + assert.throws(() => buildAxisCss(BAD_VALUE), /unsafe value/); + }); + + it("buildAxisBase rejects an invalid axis value name", () => { + assert.throws(() => buildAxisBase({ form: { "Not Valid": { "--cf-radius": "0" } } }), /Not Valid/); + }); + + it("the opt-out is explicit, and still generates", () => { + const base = buildAxisBase(BAD_VALUE, undefined, { validate: false }); + assert.equal(base['[data-form="probe"]']["--cf-radius"], "0; } html {"); + }); + + it("the opt-out does not change what valid input generates", () => { + const checked = buildAxisBase(DEFINITION.valueSets, DEFINITION); + const unchecked = buildAxisBase(DEFINITION.valueSets, DEFINITION, { validate: false }); + assert.deepEqual(unchecked, checked); + }); + + it("validates the shipped defaults without complaint", () => { + assert.ok(buildAxisBase()['[data-accent="slate"]']); + }); +}); + +describe("the p3 lightness invariant (#20 §3)", () => { + it("passes for the shipped value sets", () => { + assert.deepEqual(p3LightnessFailures(DEFINITION.valueSets, DEFINITION), []); + }); + + it("flags an override whose lightness drifts from the base", () => { + const drifted = structuredClone(DEFINITION.valueSets); + drifted.accent.azure.p3.light["--cf-accent"] = "oklch(72.0% 0.137 242.7)"; + const failures = p3LightnessFailures(drifted, DEFINITION); + assert.equal(failures.length, 1); + assert.match(failures[0], /azure/); + assert.match(failures[0], /--cf-accent/); + }); + + it("tolerates the rounding in an authored one-decimal value", () => { + const rounded = structuredClone(DEFINITION.valueSets); + // The measured base is 49.998%; the authored 50.0% must stay legal. + rounded.accent.azure.p3.light["--cf-accent"] = "oklch(50.0% 0.137 242.7)"; + assert.deepEqual(p3LightnessFailures(rounded, DEFINITION), []); + }); + + it("flags a p3 override of a token the base never declared", () => { + const orphan = structuredClone(DEFINITION.valueSets); + orphan.accent.azure.p3.light["--cf-nonexistent"] = "oklch(50% 0.1 240)"; + assert.match(p3LightnessFailures(orphan, DEFINITION).join("\n"), /--cf-nonexistent/); + }); + + it("flags a p3 value that is not oklch rather than skipping it", () => { + const bad = structuredClone(DEFINITION.valueSets); + bad.accent.azure.p3.light["--cf-accent"] = "#0369a1"; + assert.equal(p3LightnessFailures(bad, DEFINITION).length, 1); + }); + + it("measures oklab lightness against known anchors", () => { + assert.ok(Math.abs(oklabLightness("#000000") - 0) < 0.001); + assert.ok(Math.abs(oklabLightness("#ffffff") - 1) < 0.001); + assert.ok(Math.abs(oklabLightness("#0369a1") - 0.5) < 0.002); + }); +}); + +describe("the --spacing alias is gone (#20 §2)", () => { + it("no longer aliases Tailwind's spacing base unit", () => { + assert.equal(DEFINITION.aliases["--cf-spacing"], undefined); + const base = buildAxisBase(); + assert.equal(base['[data-density="compact"]']["--spacing"], undefined); + }); + + it("still emits the cf-ui token itself", () => { + const base = buildAxisBase(); + assert.equal(base['[data-density="compact"]']["--cf-spacing"], "0.2rem"); + }); + + it("keeps the namespaced color aliases", () => { + const base = buildAxisBase(); + assert.equal(base['[data-accent="azure"]']["--color-primary"], "var(--cf-accent)"); + }); +}); diff --git a/tests/unit/test_axes.py b/tests/unit/test_axes.py index a9bc2f8..da3d076 100644 --- a/tests/unit/test_axes.py +++ b/tests/unit/test_axes.py @@ -88,11 +88,19 @@ def test_p3_layer_is_layered_over_an_srgb_base_declaration(): assert "oklch(" in p3 -def test_density_drives_tailwind_spacing_base_unit(): +def test_density_does_not_alias_tailwinds_spacing_base_unit(): + """#20: --spacing is Tailwind's whole spacing scale, not a name cf-ui owns. + + Aliasing it meant `data-density` silently rescaled every `p-4` and `gap-2` + in the consuming app, including for Bulma consumers who use none of it. + `--cf-spacing` is still emitted; wiring it up is now the app's explicit + one-line opt-in. + """ from cf_ui.axes import AXIS_CSS_PATH css = AXIS_CSS_PATH.read_text(encoding="utf-8") - assert "--spacing:" in css + assert "--cf-spacing:" in css, "the axis token itself must still be emitted" + assert "--spacing:" not in css.replace("--cf-spacing:", "") def test_accent_exposes_a_strong_token_for_small_text(): @@ -409,3 +417,184 @@ def test_light_mode_css_is_the_unqualified_declaration(): css = AXIS_CSS_PATH.read_text(encoding="utf-8") assert '[data-theme="light"]' not in css + + +# -------------------------------------------------------------------------- +# Issue #20 §1: token *values* are validated, not just token names. +# +# Values are interpolated straight into a " escapes the element entirely. +# -------------------------------------------------------------------------- + +UNSAFE_VALUES = [ + pytest.param("0; } body { display: none", id="semicolon-and-braces"), + pytest.param("red; color: blue", id="semicolon"), + pytest.param("}", id="close-brace"), + pytest.param("{", id="open-brace"), + pytest.param("red /* c", id="comment-open"), + pytest.param("red */", id="comment-close"), + pytest.param("", id="style-escape"), + pytest.param("<", id="angle-bracket"), +] + + +@pytest.mark.parametrize("value", UNSAFE_VALUES) +def test_unsafe_token_values_are_rejected(value): + from cf_ui.axes import AxisConfigError, merge_value_sets + + with pytest.raises(AxisConfigError, match="unsafe value"): + merge_value_sets({"form": {"evil": {"--cf-radius": value}}}) + + +@pytest.mark.parametrize("value", UNSAFE_VALUES) +def test_unsafe_token_values_are_rejected_inside_a_mode_block(value): + """Mode-keyed axes validate through a different branch — cover it too.""" + from cf_ui.axes import AxisConfigError, merge_value_sets + + payload = { + "accent": { + "evil": { + "light": {"--cf-accent": value}, + "dark": {"--cf-accent": "#ffffff"}, + } + } + } + with pytest.raises(AxisConfigError, match="unsafe value"): + merge_value_sets(payload) + + +def test_unsafe_token_values_are_rejected_inside_a_p3_block(): + """The p3 block reaches the same stylesheet, so it needs the same gate.""" + from cf_ui.axes import AxisConfigError, merge_value_sets + + payload = { + "accent": { + "evil": { + "light": {"--cf-accent": "#0369a1"}, + "dark": {"--cf-accent": "#38bdf8"}, + "p3": {"light": {"--cf-accent": "oklch(50% 0.1 240); } html { x: y"}}, + } + } + } + with pytest.raises(AxisConfigError, match="unsafe value"): + merge_value_sets(payload) + + +def test_a_non_string_token_value_is_rejected(): + """Mirrors the plugin, which rejects anything that is not a string.""" + from cf_ui.axes import AxisConfigError, merge_value_sets + + with pytest.raises(AxisConfigError, match="non-string"): + merge_value_sets({"form": {"odd": {"--cf-radius": 0}}}) + + +@pytest.mark.parametrize( + "value", + ["0", "0.375rem", "none", "0 1px 2px rgb(0 0 0 / 0.08)", "#ffffff", "1.25"], +) +def test_legitimate_token_values_are_still_accepted(value): + from cf_ui.axes import merge_value_sets + + merged = merge_value_sets({"form": {"fine": {"--cf-radius": value}}}) + assert merged["form"]["fine"]["--cf-radius"] == value + + +def test_no_shipped_value_trips_the_new_rule(): + """Font stacks carry commas and quotes — they must survive the gate.""" + from cf_ui.axes import DEFAULT_VALUE_SETS, merge_value_sets + + # Re-validating the shipped sets through the public entry point is the + # point: a rule that rejected our own defaults would be caught here. + merged = merge_value_sets(DEFAULT_VALUE_SETS) + assert "Segoe UI" in merged["type"]["system"]["--cf-font-display"] + + +def test_the_injection_payload_never_reaches_a_style_element(): + """End-to-end: the escape hatch this closes is style_element().""" + from cf_ui.axes import AxisConfigError, merge_value_sets + + with pytest.raises(AxisConfigError): + merge_value_sets({"form": {"evil": {"--cf-radius": "0; } body { display: none"}}}) + + +# -------------------------------------------------------------------------- +# Issue #20 §3: the p3 layer holds the base declaration's lightness. +# +# The contrast gate is computed against the sRGB base. The docs claim the p3 +# override is "layered over that base at the same lightness, so the contrast +# guarantee still holds" — that was a convention held by hand until now. +# -------------------------------------------------------------------------- + + +def test_oklab_lightness_matches_known_reference_values(): + """Guards the conversion itself — a wrong L would make the gate vacuous.""" + from cf_ui.axes import oklab_lightness + + assert oklab_lightness("#000000") == pytest.approx(0.0, abs=0.001) + assert oklab_lightness("#ffffff") == pytest.approx(1.0, abs=0.001) + # Authored p3 override for azure/light is oklch(50.0% ...) + assert oklab_lightness("#0369a1") == pytest.approx(0.500, abs=0.002) + + +def test_oklch_lightness_parses_the_authored_form(): + from cf_ui.axes import oklch_lightness + + assert oklch_lightness("oklch(50.0% 0.137 242.7)") == pytest.approx(0.50, abs=0.0001) + assert oklch_lightness("oklch(82.8% 0.116 230.3)") == pytest.approx(0.828, abs=0.0001) + + +def test_shipped_p3_overrides_hold_the_lightness_invariant(): + """This is the guarantee the docs already claim. Now it fails CI.""" + from cf_ui.axes import DEFAULT_VALUE_SETS, p3_lightness_failures + + assert p3_lightness_failures(DEFAULT_VALUE_SETS) == [] + + +def test_a_p3_override_with_a_different_lightness_is_flagged(): + """The mutation the gate exists to catch.""" + from copy import deepcopy + + from cf_ui.axes import DEFAULT_VALUE_SETS, p3_lightness_failures + + drifted = deepcopy(DEFAULT_VALUE_SETS) + drifted["accent"]["azure"]["p3"]["light"]["--cf-accent"] = "oklch(72.0% 0.137 242.7)" + failures = p3_lightness_failures(drifted) + assert failures, "a 22-point lightness shift must be flagged" + assert "--cf-accent" in failures[0] + assert "azure" in failures[0] + + +def test_the_lightness_gate_tolerates_authored_rounding(): + """Values are authored to one decimal place; the gate must not be brittle.""" + from copy import deepcopy + + from cf_ui.axes import DEFAULT_VALUE_SETS, p3_lightness_failures + + rounded = deepcopy(DEFAULT_VALUE_SETS) + # azure/light base measures 49.998%; the authored 50.0% must stay legal. + rounded["accent"]["azure"]["p3"]["light"]["--cf-accent"] = "oklch(50.0% 0.137 242.7)" + assert p3_lightness_failures(rounded) == [] + + +def test_a_p3_override_of_an_undeclared_token_is_flagged(): + """A p3 block may only refine tokens the base declares.""" + from copy import deepcopy + + from cf_ui.axes import DEFAULT_VALUE_SETS, p3_lightness_failures + + orphan = deepcopy(DEFAULT_VALUE_SETS) + orphan["accent"]["azure"]["p3"]["light"]["--cf-nonexistent"] = "oklch(50% 0.1 240)" + failures = p3_lightness_failures(orphan) + assert any("--cf-nonexistent" in failure for failure in failures) + + +def test_a_non_oklch_p3_value_is_flagged_rather_than_skipped(): + """Silently skipping unparseable values is how the gate went blind before.""" + from copy import deepcopy + + from cf_ui.axes import DEFAULT_VALUE_SETS, p3_lightness_failures + + bad = deepcopy(DEFAULT_VALUE_SETS) + bad["accent"]["azure"]["p3"]["light"]["--cf-accent"] = "#0369a1" + assert p3_lightness_failures(bad) != [] diff --git a/tests/unit/test_tailwind_plugin.py b/tests/unit/test_tailwind_plugin.py index 9f29d38..00d175c 100644 --- a/tests/unit/test_tailwind_plugin.py +++ b/tests/unit/test_tailwind_plugin.py @@ -19,6 +19,7 @@ import shutil import subprocess import sys +from copy import deepcopy from fnmatch import fnmatch from pathlib import Path @@ -116,9 +117,17 @@ def _python_rules() -> tuple[dict, dict]: def _js_rules() -> tuple[dict, dict]: # A file:// URI, not a path: node's ESM loader reads a bare `C:/...` as an # unsupported URL scheme. + # + # `validate: false` is passed deliberately. buildAxisBase validates by + # default since #20, and comparing only *validated* input would quietly + # narrow this from "the two generators agree" to "they agree on input the + # validator already approved" — the unchecked generation path is exactly + # the one a consumer reaches through the exported function. script = f""" import {{ buildAxisBase }} from {json.dumps(PLUGIN_PATH.as_uri())}; - process.stdout.write(JSON.stringify(buildAxisBase())); + process.stdout.write( + JSON.stringify(buildAxisBase(undefined, undefined, {{ validate: false }})), + ); """ base = json.loads(_run_node(script)) media = base.pop("@media (color-gamut: p3)", {}) @@ -185,3 +194,128 @@ def test_the_plugin_is_dependency_free(): assert all(name.startswith("node:") for name in bare), ( f"plugin imports non-builtin modules: {bare}" ) + + +# --- cross-language validation parity (issue #20) --------------------------- +# +# #20 §1 closed an asymmetry: the JS generator rejected unsafe token values and +# the Python one did not. These assert the two now reject the *same* inputs, +# which is the property that keeps them from drifting apart again. + +UNSAFE = [ + "0; } body { display: none", + "red; color: blue", + "}", + "{", + "red /* c", + "red */", + "", + "<", +] +SAFE = ["0", "0.375rem", "none", "0 1px 2px rgb(0 0 0 / 0.08)", "#ffffff"] + + +def _python_rejects(value: str) -> bool: + from cf_ui.axes import AxisConfigError, merge_value_sets + + try: + merge_value_sets({"form": {"probe": {"--cf-radius": value}}}) + except AxisConfigError: + return True + return False + + +def _js_rejects(values: list[str]) -> list[bool]: + """One node process for the whole batch — spawning eight is needlessly slow.""" + script = f""" + import {{ mergeValueSets, loadDefinition }} from {json.dumps(PLUGIN_PATH.as_uri())}; + const definition = loadDefinition(); + const results = {json.dumps(values)}.map((value) => {{ + try {{ + mergeValueSets(definition, {{ form: {{ probe: {{ "--cf-radius": value }} }} }}); + return false; + }} catch {{ + return true; + }} + }}); + process.stdout.write(JSON.stringify(results)); + """ + return json.loads(_run_node(script)) + + +@requires_node +def test_both_generators_reject_the_same_unsafe_values(): + js = _js_rejects(UNSAFE) + python = [_python_rejects(value) for value in UNSAFE] + assert python == [True] * len(UNSAFE), f"python accepted: {UNSAFE}" + assert js == python, dict(zip(UNSAFE, zip(python, js, strict=True), strict=True)) + + +@requires_node +def test_both_generators_accept_the_same_legitimate_values(): + """Guards the test above against a rule that simply rejects everything.""" + js = _js_rejects(SAFE) + python = [_python_rejects(value) for value in SAFE] + assert python == [False] * len(SAFE), f"python rejected a legal value: {SAFE}" + assert js == python, dict(zip(SAFE, zip(python, js, strict=True), strict=True)) + + +@requires_node +def test_both_generators_report_the_same_p3_lightness_failures(): + """#20 §3 — the invariant is checked on both sides, in the same words.""" + from cf_ui.axes import DEFAULT_VALUE_SETS, p3_lightness_failures + + script = f""" + import {{ p3LightnessFailures, loadDefinition }} from {json.dumps(PLUGIN_PATH.as_uri())}; + const definition = loadDefinition(); + const drifted = structuredClone(definition.valueSets); + drifted.accent.azure.p3.light["--cf-accent"] = "oklch(72.0% 0.137 242.7)"; + process.stdout.write(JSON.stringify({{ + clean: p3LightnessFailures(definition.valueSets, definition), + drifted: p3LightnessFailures(drifted, definition), + }})); + """ + js = json.loads(_run_node(script)) + + drifted = deepcopy(DEFAULT_VALUE_SETS) + drifted["accent"]["azure"]["p3"]["light"]["--cf-accent"] = "oklch(72.0% 0.137 242.7)" + + assert js["clean"] == p3_lightness_failures(DEFAULT_VALUE_SETS) == [] + assert js["drifted"] == p3_lightness_failures(drifted) + assert js["drifted"], "the drifted fixture must actually fail, or this is vacuous" + + +# --- generated artifacts stay clean ---------------------------------------- + + +def test_generated_artifacts_are_pinned_to_lf(): + """`python -m cf_ui.axes` writes LF; without this git shows a phantom diff. + + The generator writes ``newline="\\n"`` while git's autocrlf wants CRLF, so + regenerating left the tree permanently "modified" with a zero-line diff — + which makes "is the tree clean" useless as a signal for every phase that + touches these files. + """ + attributes = REPO_ROOT / ".gitattributes" + assert attributes.is_file(), "a .gitattributes is needed to pin the generated artifacts" + text = attributes.read_text(encoding="utf-8") + for name in ("cf_ui_axes.css", "cf_ui_axes.json"): + assert name in text, f"{name} is not pinned in .gitattributes" + assert "text eol=lf" in text + + +def test_regenerating_the_artifacts_is_a_no_op(): + """The checked-in artifacts match what the generator produces, byte for byte.""" + import json as _json + + from cf_ui.axes import ( + AXIS_CSS_PATH, + AXIS_DEFINITION_PATH, + DEFAULT_VALUE_SETS, + axis_definition, + render_axis_css, + ) + + assert AXIS_CSS_PATH.read_text(encoding="utf-8") == render_axis_css(DEFAULT_VALUE_SETS) + expected = _json.dumps(axis_definition(), indent=2) + "\n" + assert AXIS_DEFINITION_PATH.read_text(encoding="utf-8") == expected From 05658e58c4325458239b7dc77c7a83c8a60b17e8 Mon Sep 17 00:00:00 2001 From: Francis Secada Date: Wed, 29 Jul 2026 11:50:29 -0400 Subject: [PATCH 2/2] fix(axes): close the three findings from the review of #26 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. The Python generators emitted unvalidated CSS — the same asymmetry §4 closed on the JS side, left open on this one. style_element() is the sink the token-value gate exists to protect, and calling it directly rendered an injection payload verbatim: style_element({'form': {'evil': {'--cf-radius': '0; } body { x: y'}}}) -> '' render_axis_css, custom_axis_css, and style_element now validate by default and take an explicit validate=False, matching buildAxisBase and buildAxisCss. The per-axis checks merge_value_sets did inline are extracted to _validate_value_sets, mirroring validateSets in the plugin, so there is one implementation rather than two that can drift. Not exploitable through any documented path — axis_value_sets() and build_axis_globals() both route through merge_value_sets. This was a latent sharp edge on a public API, and it made the two generators disagree about the thing this PR is about. The parity test now passes validate=False on the Python side too, so both halves skip the gate and the comparison stays symmetric. 2. CHANGELOG contradicted itself inside [Unreleased]: the #5 notes still said density drives Tailwind's --spacing while the new Changed block said the alias was gone. Both would have shipped in the same release notes. The alias never reached a tagged version, so the line is amended rather than left to argue with itself. 3. test_regenerating_the_artifacts_is_a_no_op could not fail for the reason its name gives. read_text() applies universal-newline translation, so a CRLF file on disk — the exact state .gitattributes prevents — compared equal. Switched to read_bytes() with an explicit CRLF assertion, and mutation-tested: writing CRLF to cf_ui_axes.css now fails the test, where before it passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NhqNRBg83czKfr8L6FF5xf --- CHANGELOG.md | 25 ++++++++---- docs/tailwind-plugin.md | 15 +++++++ src/cf_ui/axes.py | 63 ++++++++++++++++++++++++------ tests/unit/test_axes.py | 53 +++++++++++++++++++++++++ tests/unit/test_tailwind_plugin.py | 29 +++++++++++--- 5 files changed, 161 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1a832f..9e700bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,12 +24,21 @@ breaking for anyone who was relying, knowingly or not, on the gaps. `p-4` and `gap-2` in the consuming app by ±20% — including for Bulma consumers using none of it. `--cf-spacing` is still emitted. **Migration:** to keep the old behavior, add `@theme { --spacing: var(--cf-spacing); }`. -- **`buildAxisBase()` and `buildAxisCss()` validate by default.** They were - exported escape hatches that generated CSS with the build error switched off, - contradicting the guarantee #7 shipped on. Pass `{ validate: false }` to opt - out explicitly. - **Migration:** code passing invalid value sets to these now throws. That was - already producing malformed CSS. +- **Every exported generator validates by default**, on both sides of the + language boundary. These were escape hatches that generated CSS with the gate + switched off, contradicting the guarantee #7 shipped on — and `style_element` + is the very sink the token-value gate above exists to protect, so it emitted + an injection payload verbatim when called directly. + - JavaScript: `buildAxisBase(sets, definition, { validate: false })`, + `buildAxisCss(...)` — same opt-out. + - Python: `render_axis_css(sets, banner=True, validate=True)`, + `custom_axis_css(sets, banner=False, validate=True)`, + `style_element(sets, validate=True)`. + + **Migration:** code passing invalid value sets to any of these now throws. + That was already producing malformed CSS. The documented entry points + (`CF_UI_AXIS_VALUES`, the FastAPI/Litestar `value_sets=` argument) route + through `merge_value_sets` and are unaffected. ### Added - **The wide-gamut lightness invariant is enforced (#20).** `p3_lightness_failures()` @@ -122,7 +131,9 @@ breaking for anyone who was relying, knowingly or not, on the gaps. - Base declarations are sRGB hex (what the contrast gate is computed against); wide-gamut chroma is layered behind `@media (color-gamut: p3)` — not `@supports (color: oklch(...))`, which is a no-op in every current browser -- Density drives Tailwind v4's `--spacing`; accent aliases to `--color-primary*` +- Accent aliases to `--color-primary*` for Tailwind v4 interop. Density + originally aliased `--cf-spacing` to `--spacing`; that was removed before + release (see the breaking change above) and never shipped in a tagged version ## [0.1.1] — 2026-04-28 diff --git a/docs/tailwind-plugin.md b/docs/tailwind-plugin.md index 759ba13..6205cb1 100644 --- a/docs/tailwind-plugin.md +++ b/docs/tailwind-plugin.md @@ -146,6 +146,21 @@ input you did not write. const css = buildAxisCss(mySets, undefined, { validate: false }); ``` +The Python generators carry the identical contract, so neither side is the lax +one: + +| Python | JavaScript | +|---|---| +| `render_axis_css(sets, banner=True, validate=True)` | `buildAxisCss(sets, definition, { validate })` | +| `custom_axis_css(sets, banner=False, validate=True)` | — | +| `style_element(sets, validate=True)` | — | +| `merge_value_sets(custom, mode)` | `mergeValueSets(definition, custom, mode)` | + +```python +# Same shape, same default, same escape hatch. +css = render_axis_css(my_sets, validate=False) +``` + ## One definition, two generators — and a test that proves it `cf_ui/axes.py` is still the single source of truth. It generates two diff --git a/src/cf_ui/axes.py b/src/cf_ui/axes.py index 90e24d9..cfcd249 100644 --- a/src/cf_ui/axes.py +++ b/src/cf_ui/axes.py @@ -356,6 +356,23 @@ def _validate_tokens(axis: str, name: str, tokens: Any) -> None: ) +def _validate_value_sets(value_sets: Mapping[str, Mapping[str, Any]]) -> None: + """Run a whole ``{axis: {name: tokens}}`` mapping through the gate. + + Mirrors ``validateSets`` in ``cf_ui_tailwind_plugin.mjs``. Factored out so + the generators below can apply the same checks without going through + :func:`merge_value_sets` — an exported function that generates CSS with the + validation switched off is the asymmetry this exists to prevent. + """ + for axis, values in value_sets.items(): + if axis not in AXES: + raise AxisConfigError(f"unknown axis {axis!r}: valid axes are {', '.join(AXES)}") + if not isinstance(values, Mapping): + raise AxisConfigError(f"axis {axis!r} must map value names to tokens") + for name, tokens in values.items(): + _validate_value_set(axis, name, tokens) + + def merge_value_sets( custom: Mapping[str, Mapping[str, Any]] | None, mode: str = "extend", @@ -378,13 +395,8 @@ def merge_value_sets( if not custom: return merged + _validate_value_sets(custom) for axis, values in custom.items(): - if axis not in AXES: - raise AxisConfigError(f"unknown axis {axis!r}: valid axes are {', '.join(AXES)}") - if not isinstance(values, Mapping): - raise AxisConfigError(f"axis {axis!r} must map value names to tokens") - for name, tokens in values.items(): - _validate_value_set(axis, name, tokens) if mode == "replace": merged[axis] = deepcopy(dict(values)) else: @@ -516,8 +528,20 @@ def _selector(axis: str, value: str, mode: str | None = None) -> str: def render_axis_css( value_sets: Mapping[str, Mapping[str, Any]], banner: bool = True, + validate: bool = True, ) -> str: - """Render axis value sets as CSS custom properties keyed on data attributes.""" + """Render axis value sets as CSS custom properties keyed on data attributes. + + Args: + value_sets: ``{axis: {value_name: tokens}}``. + banner: prepend the generated-file banner. + validate: on by default. ``False`` skips the gate — an explicit escape + hatch for inspecting output or comparing generators, never + something to reach for with input you did not write. Mirrors + ``buildAxisCss(..., { validate: false })`` in the Tailwind plugin. + """ + if validate: + _validate_value_sets(value_sets) parts: list[str] = [_BANNER] if banner else [] for axis in AXES: @@ -549,12 +573,20 @@ def render_axis_css( return "\n".join(parts) -def custom_axis_css(value_sets: Mapping[str, Mapping[str, Any]], banner: bool = False) -> str: +def custom_axis_css( + value_sets: Mapping[str, Mapping[str, Any]], + banner: bool = False, + validate: bool = True, +) -> str: """Render only the values the shipped stylesheet does not already cover. A value is emitted when it is new, or when it shadows a shipped value name with different tokens — in which case the static asset's copy is stale. + + Validates by default; see :func:`render_axis_css` for the opt-out. """ + if validate: + _validate_value_sets(value_sets) diff: dict[str, dict[str, Any]] = {} for axis in AXES: for value, tokens in value_sets.get(axis, {}).items(): @@ -562,12 +594,19 @@ def custom_axis_css(value_sets: Mapping[str, Mapping[str, Any]], banner: bool = diff.setdefault(axis, {})[value] = tokens if not diff: return "" - return render_axis_css(diff, banner=banner) + # Already checked above (or deliberately skipped) — do not pay for it twice. + return render_axis_css(diff, banner=banner, validate=False) + +def style_element(value_sets: Mapping[str, Mapping[str, Any]], validate: bool = True) -> str: + """Wrap :func:`custom_axis_css` in a style element, or return ``""``. -def style_element(value_sets: Mapping[str, Mapping[str, Any]]) -> str: - """Wrap :func:`custom_axis_css` in a style element, or return ``""``.""" - css = custom_axis_css(value_sets) + This is the injection sink the token-value gate exists for: whatever it + returns is marked safe and rendered into the page. It validates by default + for that reason — the opt-out is for callers generating output they are not + about to inject. + """ + css = custom_axis_css(value_sets, validate=validate) return f"" if css else "" diff --git a/tests/unit/test_axes.py b/tests/unit/test_axes.py index da3d076..bfb6fe0 100644 --- a/tests/unit/test_axes.py +++ b/tests/unit/test_axes.py @@ -518,6 +518,59 @@ def test_the_injection_payload_never_reaches_a_style_element(): merge_value_sets({"form": {"evil": {"--cf-radius": "0; } body { display: none"}}}) +# -------------------------------------------------------------------------- +# Issue #20 §4: the exported generators validate too, not just merge_value_sets. +# +# An exported function that generates CSS with the gate switched off is the +# generator with the build error removed. The plugin's buildAxisBase/buildAxisCss +# were fixed for exactly this; these are their Python counterparts, and +# style_element() is the sink the whole §1 gate exists to protect. +# -------------------------------------------------------------------------- + +EVIL_SET = {"form": {"evil": {"--cf-radius": "0; } body { display: none"}}} + + +@pytest.mark.parametrize("generator", ["render_axis_css", "custom_axis_css", "style_element"]) +def test_the_exported_generators_validate_by_default(generator): + import cf_ui.axes as axes + + with pytest.raises(axes.AxisConfigError, match="unsafe value"): + getattr(axes, generator)(EVIL_SET) + + +@pytest.mark.parametrize("generator", ["render_axis_css", "custom_axis_css", "style_element"]) +def test_the_generators_take_an_explicit_opt_out(generator): + """The escape hatch stays — you just have to ask for it.""" + import cf_ui.axes as axes + + output = getattr(axes, generator)(EVIL_SET, validate=False) + assert "display: none" in output + + +def test_style_element_no_longer_emits_an_injection_payload(): + """The concrete regression: this used to render the payload into the page.""" + from cf_ui.axes import AxisConfigError, style_element + + with pytest.raises(AxisConfigError): + style_element(EVIL_SET) + + +def test_the_opt_out_does_not_change_what_valid_input_produces(): + """Guards against validation quietly altering the generated CSS.""" + from cf_ui.axes import DEFAULT_VALUE_SETS, render_axis_css + + assert render_axis_css(DEFAULT_VALUE_SETS) == render_axis_css( + DEFAULT_VALUE_SETS, validate=False + ) + + +def test_the_generators_reject_an_unknown_axis(): + from cf_ui.axes import AxisConfigError, render_axis_css + + with pytest.raises(AxisConfigError, match="flavour"): + render_axis_css({"flavour": {"vanilla": {"--cf-radius": "0"}}}) + + # -------------------------------------------------------------------------- # Issue #20 §3: the p3 layer holds the base declaration's lightness. # diff --git a/tests/unit/test_tailwind_plugin.py b/tests/unit/test_tailwind_plugin.py index 00d175c..92d49f6 100644 --- a/tests/unit/test_tailwind_plugin.py +++ b/tests/unit/test_tailwind_plugin.py @@ -107,8 +107,14 @@ def _declarations(css: str) -> dict[str, dict[str, str]]: def _python_rules() -> tuple[dict, dict]: - """Split the generated stylesheet into its base rules and its p3 layer.""" - css = _COMMENT.sub("", render_axis_css(DEFAULT_VALUE_SETS, banner=False)) + """Split the generated stylesheet into its base rules and its p3 layer. + + ``validate=False`` for the same reason the JS side passes it: comparing only + validated input would narrow this from "the two generators agree" to "they + agree on input the validator already approved". Both sides must skip it, or + the comparison is no longer symmetric. + """ + css = _COMMENT.sub("", render_axis_css(DEFAULT_VALUE_SETS, banner=False, validate=False)) marker = "@media (color-gamut: p3) {" base_css, _, media_css = css.partition(marker) return _declarations(base_css), _declarations(media_css) @@ -305,7 +311,14 @@ def test_generated_artifacts_are_pinned_to_lf(): def test_regenerating_the_artifacts_is_a_no_op(): - """The checked-in artifacts match what the generator produces, byte for byte.""" + """The checked-in artifacts match what the generator produces, byte for byte. + + Deliberately ``read_bytes``, not ``read_text``: the latter applies universal + newline translation, so a CRLF file on disk — the exact state + ``.gitattributes`` exists to prevent — would still compare equal, and this + test would assert content only while reading as though it covered the line + endings its name refers to. + """ import json as _json from cf_ui.axes import ( @@ -316,6 +329,12 @@ def test_regenerating_the_artifacts_is_a_no_op(): render_axis_css, ) - assert AXIS_CSS_PATH.read_text(encoding="utf-8") == render_axis_css(DEFAULT_VALUE_SETS) + css = AXIS_CSS_PATH.read_bytes() + definition = AXIS_DEFINITION_PATH.read_bytes() + + assert b"\r\n" not in css, "cf_ui_axes.css is CRLF on disk; the generator writes LF" + assert b"\r\n" not in definition, "cf_ui_axes.json is CRLF on disk; the generator writes LF" + + assert css == render_axis_css(DEFAULT_VALUE_SETS).encode("utf-8") expected = _json.dumps(axis_definition(), indent=2) + "\n" - assert AXIS_DEFINITION_PATH.read_text(encoding="utf-8") == expected + assert definition == expected.encode("utf-8")