fix(design-sanitize): stop nuking CSS on scroll-behavior - #73
Conversation
filter_css treated any "behavior:" substring as IE XSS CSS, so scroll-behavior:smooth wiped entire <style> blocks and left prod screenshots unstyled. Match only the IE property, soft-strip @import rules, and document embedded-CSS + reject taxonomy for miners.
|
Warning Review limit reached
Next review available in: 12 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe CSS sanitizer now soft-strips ChangesCSS Sanitization
Estimated code review effort: 3 (Moderate) | ~20 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Summary
filter_csstreated anybehavior:substring as IE XSS CSS, so legitimatescroll-behavior:smoothwiped entire<style>blocks. Chromium then screenshotted class-only HTML → unstyled pages. Confirmed on prod (e.g. ClearRail run with full design system CSS in raw, zero<style>after sanitize).behavior:property (not suffixes), soft-strip@import …;while keeping remaining rules, keep hard-nuke forexpression(/url(javascript:/-moz-binding.<link rel=stylesheet>/ Tailwind CDN is stripped (expected); miners must embed CSS; document reject taxonomy for miners.Prod evidence (master, design_run)
<style>via thescroll-behaviorfalse positive (css_stripped+css_blocked).<link>strip is intentional (security); not the dominant prod failure mode right now.Reject taxonomy (terminal runs ≈163)
near_identical_harness_copy/ast_architecture_copy)NoneType.__dict__, 4×stuck timeoutTest plan
cargo test -p design-sanitizextask design-check/external-docs-checkscroll-behaviorkeep CSS in screenshotsraw_html+backfill-screenshots(or force recapture) for historical ugly PNGs — existing artifacts stay broken until reprocessedDeploy note
Does not touch metagraph cache / real-seal. Promote
design-challenge(+ any image that vendorsdesign-sanitize). Historical screenshots need a one-shot re-sanitize fromdesign_artifact.raw_htmlthen screenshot backfill.Summary by CodeRabbit
Bug Fixes
scroll-behavior.behavior:rules.@importstatements.Documentation