fix(configurator): null-safe JSON import, finite-number guards, style-injection hardening

[jackgranatowski]

Summary

Six bugs surfaced by automated review (CodeRabbit + Qodo) on SLASHED-Plugins#122, which vendors configurator/src/ verbatim. These predate that PR but were never caught here since this repo has no CodeQL/CodeRabbit coverage of its own — they only surfaced once the plugin repo's automated reviewers scanned the freshly-synced files.

Changes

• App.svelte — handleImport() passed any parsed JSON value straight to setOverrides(), including null/arrays (typeof null === "object"), crashing the configurator in shallowEq's Object.keys() call on malformed import. Restored the data !== null && !Array.isArray(data) + string-value filtering that AppOverlay.svelte's import flow already had.
• lib/persistence.ts — getNum()'s isNaN() guard let Infinity/-Infinity through (parseFloat can return both), which fmt() then serialized as literal "Infinity"/"NaN" strings into generated CSS. Switched to Number.isFinite() and added a matching guard in fmt().
• ColorsPanel.svelte — getLightSurface/getDarkSurface/getLightText/getDarkText read overrides[...] directly instead of going through sourceValue()'s override→loaded-token→default precedence, so the palette swatch strips could disagree with the rest of the panel when a source token was synced/loaded but not overridden. Added sourceByName() and routed all four through sourceValue().
• EffectsPanel.svelte — the text-shadow input checked v.trim() but passed the untrimmed v to onSet, persisting whitespace-padded values.
• MotionPanel.svelte — staggerBase was $derived to a function instead of a value ($derived(() => ...) instead of $derived.by(() => ...)), so it never recomputed when overrides/scale changed.
• SpacingPanel.svelte — the space-scale preview's exponent offset was off-by-one (i - 4 instead of i - 3) — SPACE_STEPS[3] is "m", the framework's exponent-0 anchor per core/tokens.css's own comment (--sf-space-m (N=0): 2xs=-3 … 4xl=5), so every preview bar rendered one step below its real fluid-scale value.
• ThemesPanel.svelte + swatch sites in ColorsPanel, EffectsPanel, TokenRow, ColorInput, OklchColorDesk — color values (sourced from overrides, which can come from a saved theme, imported JSON, or a shared URL hash) were interpolated into raw style={...} template strings. A value like red; background-image:url(...) would inject extra CSS declarations. Switched the single-value cases to Svelte's style:background={value} property directive, which only ever sets that one property — no declaration-separator injection possible.

Test plan

• npm run check (svelte-check) — 0 errors
• npm run build (root CSS bundles + docs) — passes
• cd configurator && npm run build (vite build) — passes

---

Generated by Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved import validation so only valid JSON objects with string values are applied.
  • Fixed handling of invalid numeric values to avoid unexpected results in generated output.
  • Refined color and preview rendering across panels for more consistent display.
  • Adjusted spacing and motion previews for more accurate visual feedback.

[coderabbitai]

Warning

Review limit reached

@jackgranatowski, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 35 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c25c9fbe-e5ce-4f02-9651-7a38dfcd5e85

📥 Commits

Reviewing files that changed from the base of the PR and between 9d631f0 and dfbd71f.

📒 Files selected for processing (2)

• configurator/src/App.svelte
• configurator/src/components/panels/ColorsPanel.svelte

📝 Walkthrough

Walkthrough

Updates configurator app behavior: validates and sanitizes JSON theme imports, migrates several swatch background styles to Svelte's style:background directive, reworks color source resolution in ColorsPanel, fixes finiteness checks in persistence parsing/formatting, filters CSS custom-property keys in codec, and corrects minor logic in MotionPanel, SpacingPanel, and EffectsPanel.

Changes

Configurator data handling and UI binding updates

Layer / File(s)	Summary
JSON import validation configurator/src/App.svelte	JSON import now checks for a plain non-null, non-array object and filters entries to string values before calling `setOverrides`.
CSS key sanitization and numeric finiteness configurator/src/lib/codec.ts, configurator/src/lib/persistence.ts	CSS generation filters keys to the `--sf-*` pattern before sorting; `getNum` and `fmt` now use `Number.isFinite` instead of `isNaN`.
ColorsPanel source resolution configurator/src/components/panels/ColorsPanel.svelte	Adds `sourceByName` helper; `getLightSurface`, `getDarkSurface`, `getLightText`, `getDarkText` resolve values via `sourceValue(sourceByName(...))`, with an updated invalid-color fallback for dark text.
Swatch style:background migration configurator/src/components/inputs/ColorInput.svelte, .../OklchColorDesk.svelte, .../TokenRow.svelte, configurator/src/components/panels/ColorsPanel.svelte, .../EffectsPanel.svelte, .../ThemesPanel.svelte	Multiple swatch/preview elements switch from interpolated `style` strings to Svelte's `style:background={...}` directive.
Panel logic fixes configurator/src/components/panels/MotionPanel.svelte, .../SpacingPanel.svelte, .../EffectsPanel.svelte	`staggerBase` changes from a derived function to `$derived.by` value with updated call sites; spacing preview offset shifts from `i - 4` to `i - 3`; text-shadow input trims value before applying or resetting.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

• codeslash-dev/SLASHED#337: Both PRs validate and safely shape imported JSON overrides before applying them.
• codeslash-dev/SLASHED#429: Both PRs modify ColorsPanel.svelte's swatch color derivation/resolution and rendering.
• codeslash-dev/SLASHED#440: Both PRs touch swatch background style bindings in ColorInput.svelte and TokenRow.svelte.

Suggested labels

codex

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main configurator fixes: safer JSON import, finite-number handling, and CSS/style injection hardening.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/configurator-sync-pr-460-461-g1f2w4

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Fix configurator import/persistence edge cases and harden color style bindings

🐞 Bug fix ✨ Enhancement 🕐 40+ Minutes

AI Description

• Prevent malformed JSON imports (null/arrays/non-string values) from crashing overrides handling.
• Guard against non-finite numeric values propagating into persisted/generated CSS.
• Fix panel inconsistencies (source precedence, trimming, derived reactivity, scale preview) and
 harden swatch styling against CSS injection.

Diagram

graph TD
  user(["User"]) --> ext{{"Import source (JSON)"}} --> app["App.svelte (import)" ] --> ov[("Overrides map")]
  ov --> panels["Panels (colors/effects/motion/spacing/themes)"] --> inputs["Swatches & inputs"]
  ov --> persist["persistence.ts (getNum/fmt)"] --> css["Generated CSS"]
  subgraph Legend
    direction LR
    _u(["User"]) ~~~ _e{{"External input"}} ~~~ _c["UI component"] ~~~ _s[("Storage")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize sanitization at override ingress

• ➕ Single choke point for validation (imports, URL hash, saved themes)
• ➕ Keeps UI components simpler and reduces repeated hardening changes
• ➖ May require broader refactor if overrides can be mutated from multiple flows
• ➖ Harder to preserve legitimate inputs like CSS variables while still validating

2. Validate color values with a parser/whitelist

• ➕ Stronger guarantee that rendered colors are syntactically valid
• ➕ Could surface user-facing errors instead of silently accepting bad values
• ➖ Risk of rejecting valid CSS forms (var(), color-mix(), oklch(), etc.) used intentionally
• ➖ Adds complexity and potential dependency footprint

3. Render swatches via CSS custom properties

• ➕ Avoids interpolating raw values into multi-declaration style strings
• ➕ Can standardize rendering across components
• ➖ Still requires safe assignment semantics; doesn’t replace the need for guarding ingress/formatting
• ➖ More indirection in markup and CSS

Recommendation: Current approach is a good incremental fix: using Svelte’s style:background={...} removes declaration-separator injection risk at the render sites without over-constraining valid CSS color expressions. If more override entry points are added, consider adding a lightweight, centralized validation/sanitization layer at the override ingestion boundary to avoid repeating safety fixes across components.

Files changed (10) +33 / -22

Bug fix (10) +33 / -22

App.svelteMake JSON import null/array-safe and filter non-string override values +6/-1

Make JSON import null/array-safe and filter non-string override values

• Tightens 'handleImport()' to only accept plain objects (not null/arrays). Filters entries to string values before calling 'setOverrides()', preventing crashes when malformed JSON is imported.

configurator/src/App.svelte

ColorInput.svelteHarden swatch rendering with Svelte style directive +1/-1

Harden swatch rendering with Svelte style directive

• Replaces interpolated 'style={...}' string with 'style:background={swatchColor}' so only the background property is set, reducing CSS-injection surface from untrusted values.

configurator/src/components/inputs/ColorInput.svelte

OklchColorDesk.svelteUse 'style:background' for color swatch binding +1/-1

Use 'style:background' for color swatch binding

• Switches to Svelte’s property style directive for swatch background to prevent injecting extra CSS declarations via crafted color strings.

configurator/src/components/inputs/OklchColorDesk.svelte

TokenRow.svelteUse 'style:background' for token color swatches +1/-1

Use 'style:background' for token color swatches

• Updates the token row color preview to bind background via 'style:background', hardening rendering against style string injection.

configurator/src/components/inputs/TokenRow.svelte

ColorsPanel.svelteFix source precedence for endpoint colors and harden swatch styling +13/-9

Fix source precedence for endpoint colors and harden swatch styling

• Adds 'sourceByName()' and routes endpoint color computations through 'sourceValue()' to respect override → loaded token → default precedence. Also replaces interpolated style strings with 'style:background' for palette/swatches to prevent CSS declaration injection.

configurator/src/components/panels/ColorsPanel.svelte

EffectsPanel.svelteHarden scrollbar preview styles and trim text-shadow input values +4/-3

Harden scrollbar preview styles and trim text-shadow input values

• Uses 'style:background' for scrollbar preview swatches instead of style template strings. Ensures text-shadow values are trimmed before persisting, avoiding whitespace-padded overrides.

configurator/src/components/panels/EffectsPanel.svelte

MotionPanel.svelteFix derived 'staggerBase' reactivity and usage +3/-3

Fix derived 'staggerBase' reactivity and usage

• Corrects 'staggerBase' to use '$derived.by(...)' so it recomputes when dependencies change. Updates call sites to treat it as a value rather than a function.

configurator/src/components/panels/MotionPanel.svelte

SpacingPanel.svelteCorrect space scale preview exponent offset +1/-1

Correct space scale preview exponent offset

• Fixes an off-by-one in preview scale calculation ('i - 3' anchor) so displayed bars match the intended exponent mapping of the space steps.

configurator/src/components/panels/SpacingPanel.svelte

ThemesPanel.svelteHarden theme preview swatch styling with 'style:background' +1/-1

Harden theme preview swatch styling with 'style:background'

• Replaces raw style-string interpolation for theme override swatches with 'style:background' to prevent injection via untrusted override values.

configurator/src/components/panels/ThemesPanel.svelte

persistence.tsReject NaN/Infinity when parsing/formatting numeric overrides +2/-1

Reject NaN/Infinity when parsing/formatting numeric overrides

• Updates 'getNum()' to use 'Number.isFinite()' so Infinity/-Infinity don’t pass through. Adds a matching finite-number guard in 'fmt()' to prevent serializing invalid numbers into generated CSS.

configurator/src/lib/persistence.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 6 rules

1. Unvalidated override keys ✓ Resolved 🐞 Bug ⛨ Security

Description

App.svelte’s JSON import forwards arbitrary object keys into overrides; codec.fa() then interpolates
keys verbatim into generated CSS used for live preview and export. Crafted keys can therefore
inject/mangle CSS output (e.g., by embedding CSS syntax into the property name), breaking the
preview/exported stylesheet.

Code

configurator/src/App.svelte[R170-175]

+            if (data !== null && typeof data === "object" && !Array.isArray(data)) {
+              const safe = Object.fromEntries(
+                Object.entries(data as Record<string, unknown>).filter(([, v]) => typeof v === "string")
+              ) as Record<string, string>;
+              setOverrides(safe);
+            }

Relevance

⭐⭐⭐ High

Team consistently accepts hardening untrusted inputs (localStorage/theme validation, codec bounds
checks); likely accept key validation too.

PR-#460
PR-#427
PR-#457

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The JSON import path only filters by value type and then calls setOverrides() with the imported
object, so attacker-controlled keys can enter the overrides map. codec.fa() emits CSS lines using
${key}: ${da(value)}; where only the value is sanitized; injectLivePreview() writes this CSS
into a <style> tag and export uses the same generator, so malformed keys can directly corrupt the
generated stylesheet.

configurator/src/App.svelte[156-185]
configurator/src/lib/codec.ts[176-204]
configurator/src/lib/persistence.ts[187-229]
PR-#460

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`handleImport()` filters imported JSON values to strings, but it does not validate imported **keys** before calling `setOverrides()`. Downstream CSS generation (`codec.fa`) uses override keys directly when emitting `:root { <key>: <value>; }`, which means a crafted JSON file can introduce malformed CSS by placing CSS syntax into the key.

## Issue Context
- Values are sanitized (`da()` strips `;{}` etc), but keys are not sanitized at all.
- Live preview writes the generated CSS into a `<style>` tag (`injectLivePreview()`), and export also uses `fa(overrides)`.

## Fix Focus Areas
- configurator/src/App.svelte[167-185]
- configurator/src/lib/codec.ts[176-204]
- configurator/src/lib/persistence.ts[187-229]

## Suggested fix
- In `handleImport()` (JSON path), filter keys to a strict allowlist:
 - Either `^--sf-[\w-]+$` **and** present in the known token registry (preferred), or
 - At minimum `^--sf-[\w-]+$` and reject keys containing any non-identifier characters.
- Optionally, as a defense-in-depth layer, sanitize/validate keys in `codec.fa()` before emitting them (e.g., skip keys that don’t match `^--sf-[\w-]+$`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@configurator/src/App.svelte`:
- Around line 170-175: The JSON import path in App.svelte is replacing overrides
unconditionally via setOverrides(safe), which can wipe existing settings when
the parsed object is empty or all values are filtered out. Update the import
flow in the same branch that builds safe from Object.entries(data) to only call
setOverrides when safe has at least one key, matching the guard used by the CSS
import path. Keep the logic localized around the JSON parsing/import handling so
empty or fully filtered imports are ignored instead of clearing current
overrides.

In `@configurator/src/components/panels/ColorsPanel.svelte`:
- Around line 353-357: The getDarkSurface source-resolution path is bypassing
the documented precedence chain by reading
overrides["--sf-color-base-source-dark"] directly and skipping loaded token
values. Update getDarkSurface in ColorsPanel.svelte to resolve the dark base
source through the same sourceValue(sourceByName(...)) flow used by
getLightSurface, then only fall back to deriveDarkFromLight when no explicit
override or loaded token exists. Keep the fix aligned with the existing
sourceTokenMap-backed resolution helpers so the auto-dark preview matches the
swatches and inputs.

🪄 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 Plus

Run ID: c49b539d-88ba-47b7-85ba-66ae209a4cea

📥 Commits

Reviewing files that changed from the base of the PR and between b33c0d1 and 9d631f0.

📒 Files selected for processing (11)

• configurator/src/App.svelte
• configurator/src/components/inputs/ColorInput.svelte
• configurator/src/components/inputs/OklchColorDesk.svelte
• configurator/src/components/inputs/TokenRow.svelte
• configurator/src/components/panels/ColorsPanel.svelte
• configurator/src/components/panels/EffectsPanel.svelte
• configurator/src/components/panels/MotionPanel.svelte
• configurator/src/components/panels/SpacingPanel.svelte
• configurator/src/components/panels/ThemesPanel.svelte
• configurator/src/lib/codec.ts
• configurator/src/lib/persistence.ts