Skip to content

Commit 75b0cea

Browse files
committed
fix(design-sanitize): stop nuking CSS on scroll-behavior
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.
1 parent 24c397b commit 75b0cea

3 files changed

Lines changed: 127 additions & 15 deletions

File tree

crates/design-sanitize/src/lib.rs

Lines changed: 105 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -117,19 +117,54 @@ fn rewrite_body_wrapper(html: &str) -> String {
117117
re_close.replace_all(&with_open, "</div>").into_owned()
118118
}
119119

120+
/// True when `css` (already lowercased) contains the IE-only `behavior:`
121+
/// property, not a suffix like `scroll-behavior:`.
122+
fn has_ie_behavior_property(lower: &str) -> bool {
123+
let mut start = 0;
124+
while let Some(rel) = lower[start..].find("behavior:") {
125+
let abs = start + rel;
126+
if abs == 0 {
127+
return true;
128+
}
129+
let prev = lower.as_bytes()[abs - 1];
130+
// Property names may include `-` / `_` / alphanumerics (`scroll-behavior`).
131+
if prev.is_ascii_alphanumeric() || prev == b'-' || prev == b'_' {
132+
start = abs + "behavior:".len();
133+
continue;
134+
}
135+
return true;
136+
}
137+
false
138+
}
139+
120140
/// Filter dangerous CSS constructs from a style attribute / block.
141+
///
142+
/// Soft-strips `@import` rules (keeps the rest of the block). Hard-nukes the
143+
/// whole input for XSS-prone constructs (`expression(`, `url(javascript:`,
144+
/// IE `behavior:`, `-moz-binding`). `scroll-behavior:` is presentation CSS
145+
/// and must not match the IE `behavior:` check.
121146
#[must_use]
122147
pub fn filter_css(css: &str) -> (String, bool) {
123-
let lower = css.to_ascii_lowercase();
124-
let bad = lower.contains("@import")
125-
|| lower.contains("expression(")
148+
let mut stripped = false;
149+
let mut out = css.to_owned();
150+
if out.to_ascii_lowercase().contains("@import") {
151+
if let Ok(re) = Regex::new(r"(?i)@import[^;]*;?") {
152+
let next = re.replace_all(&out, "");
153+
if next.as_ref() != out {
154+
stripped = true;
155+
out = next.into_owned();
156+
}
157+
}
158+
}
159+
let lower = out.to_ascii_lowercase();
160+
let hard_bad = lower.contains("expression(")
126161
|| lower.contains("url(javascript:")
127-
|| lower.contains("behavior:")
162+
|| has_ie_behavior_property(&lower)
128163
|| lower.contains("-moz-binding");
129-
if bad {
164+
if hard_bad {
130165
(String::new(), true)
131166
} else {
132-
(css.to_owned(), false)
167+
(out, stripped)
133168
}
134169
}
135170

@@ -155,17 +190,21 @@ pub fn sanitize_html(raw: &str) -> (String, SanitizeReport) {
155190
notes.push("meta_refresh".into());
156191
}
157192

158-
// Pre-strip style blocks with dangerous CSS; keep safe blocks for ammonia.
193+
// Pre-filter style blocks: soft-strip `@import`, hard-nuke XSS CSS.
159194
let mut pre = rewrite_body_wrapper(raw);
160195
if let Ok(re) = Regex::new(r"(?is)<style[^>]*>(.*?)</style>") {
161196
let mut stripped = false;
162197
let mut css_notes = Vec::new();
163198
pre = re
164199
.replace_all(&pre, |caps: &regex::Captures<'_>| {
165-
let (filtered, bad) = filter_css(&caps[1]);
166-
if bad {
200+
let original = &caps[1];
201+
let (filtered, touched) = filter_css(original);
202+
if touched {
167203
stripped = true;
168204
css_notes.push("css_blocked".into());
205+
}
206+
// Hard-bad → empty filtered; soft-strip keeps remaining rules.
207+
if filtered.is_empty() && !original.is_empty() {
169208
String::new()
170209
} else {
171210
format!("<style>{filtered}</style>")
@@ -216,13 +255,14 @@ fn filter_inline_styles(html: &str) -> (String, bool) {
216255
.get(2)
217256
.or_else(|| caps.get(3))
218257
.map_or("", |m| m.as_str());
219-
let (filtered, bad) = filter_css(val);
220-
if bad || filtered.is_empty() && !val.is_empty() {
258+
let (filtered, touched) = filter_css(val);
259+
if filtered.is_empty() && !val.is_empty() {
221260
stripped = true;
222261
String::new()
223262
} else if filtered == val {
224263
caps[0].to_owned()
225264
} else {
265+
stripped |= touched;
226266
format!(" style=\"{filtered}\"")
227267
}
228268
})
@@ -449,4 +489,58 @@ mod tests {
449489
"{out}"
450490
);
451491
}
492+
493+
#[test]
494+
fn scroll_behavior_must_not_nuke_style_block() {
495+
let html = r#"<style>
496+
html{scroll-behavior:smooth}
497+
body{margin:0;color:#172220;background:#f4f7f6}
498+
.hero{padding:2rem}
499+
</style><main class="hero">Hello</main>"#;
500+
let (out, report) = sanitize_html(html);
501+
assert!(
502+
!report.css_stripped,
503+
"scroll-behavior is safe presentation CSS: {report:?}"
504+
);
505+
assert!(
506+
out.to_ascii_lowercase().contains("<style>"),
507+
"style kept: {out}"
508+
);
509+
assert!(out.contains("scroll-behavior"), "{out}");
510+
assert!(out.contains(".hero"), "{out}");
511+
}
512+
513+
#[test]
514+
fn amp_in_css_comment_must_not_drop_style() {
515+
let html = r#"<style>
516+
/* --- RESET & VARIABLES --- */
517+
:root { --bg: #07090E; }
518+
body { margin: 0; background: var(--bg); }
519+
</style><p class="x">hi</p>"#;
520+
let (out, report) = sanitize_html(html);
521+
assert!(!report.css_stripped, "{report:?}");
522+
assert!(out.to_ascii_lowercase().contains("<style>"), "{out}");
523+
assert!(out.contains("--bg"), "{out}");
524+
}
525+
526+
#[test]
527+
fn import_soft_stripped_keeps_remaining_rules() {
528+
let (out, report) = sanitize_html(
529+
r#"<style>@import url('https://fonts.example/x.css');
530+
.hero{color:red}</style><p class="hero">x</p>"#,
531+
);
532+
assert!(report.css_stripped, "{report:?}");
533+
assert!(!out.contains("@import"), "{out}");
534+
assert!(out.to_ascii_lowercase().contains("<style>"), "{out}");
535+
assert!(out.contains(".hero"), "{out}");
536+
}
537+
538+
#[test]
539+
fn ie_behavior_still_nukes_block() {
540+
let (out, report) =
541+
sanitize_html(r#"<style>body{behavior:url(evil.htc);color:red}</style><p>x</p>"#);
542+
assert!(report.css_stripped, "{report:?}");
543+
assert!(!out.to_ascii_lowercase().contains("<style>"), "{out}");
544+
assert!(!out.contains("evil.htc"), "{out}");
545+
}
452546
}

docs/DESIGN_CHALLENGE.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,12 @@ only (§6).
199199
- Tags: `script`, `iframe`, `object`, `embed`, `applet`, `base`, `form`, `meta[http-equiv=refresh]`, `link[rel=import]`, scriptable SVG
200200
- All `on*` event attributes
201201
- URL schemes: reject `javascript:`, `vbscript:`, `data:text/html`; allow `http`, `https`, `mailto`, `data:image/*`
202-
- CSS: reject `@import`, `expression(`, `url(javascript:`, `behavior:`, `-moz-binding`
202+
- CSS: soft-strip `@import …;` rules (remainder of the `<style>` block is kept);
203+
hard-reject (drop the whole block/attr) for `expression(`, `url(javascript:`,
204+
IE-only `behavior:` (not `scroll-behavior:`), `-moz-binding`
205+
- External `<link rel=stylesheet>` is stripped (no CDN / Tailwind CDN); miners
206+
must embed presentation CSS in `<style>` or inline `style=` for screenshots
207+
and the sandboxed viewer to look styled
203208

204209
### Annotator signal
205210

docs/external-miner/design.md

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,22 @@ Screenshots only: `GET /v1/view/{run_id}/index.png` returns the full-page PNG
158158
screenshot the orchestrator captures right after sanitize. Produced HTML is
159159
never served — `.html` requests return `410 Gone` (the gateway still wraps
160160
view responses in a CSP `sandbox` (no scripts) lockdown as defense in depth).
161-
Your pages stay static HTML + inline CSS (`img` may use `data:`/`https:`,
162-
fonts `data:`/`https:`) so the headless capture renders them faithfully.
163-
`GET /v1/runs/{id}/pages` stays available for page metadata.
161+
Your pages stay static HTML + **embedded** CSS (`<style>` blocks and/or inline
162+
`style=`) so the headless capture renders them faithfully. External
163+
`<link rel=stylesheet>` (Tailwind CDN, Google Fonts CSS, etc.) is stripped by
164+
sanitize — screenshots will look unstyled if that was your only CSS. Prefer
165+
system font stacks over `@import` font CSS (`@import` rules are removed).
166+
`img` may use `data:` / `https:`. `GET /v1/runs/{id}/pages` stays available for
167+
page metadata.
168+
169+
### Why a run is rejected / scored zero
170+
171+
| Outcome | What it means |
172+
|---------|----------------|
173+
| `rejected` + `near_identical_harness_copy` / `ast_architecture_copy` | Pre-LLM copy gate: your harness is a byte/AST copy of an **earlier** miner harness (baseline starter is OK; copying another miner is not) |
174+
| `scored` with agentic `cheat` / `suspicious` | LLM anti-cheat found a listed cheat pattern (same Score(0); not admin-eligible) |
175+
| `failed` + harness / install / timeout | Agent crashed, timed out, or infra exhausted retries — check `/events` + `/logs` |
176+
| Missing required pages | Bundle must include `index.html`, `pricing.html`, `components.html` |
164177

165178
## Useful routes
166179

0 commit comments

Comments
 (0)