Honor unless: exclusions in search rules

src/models.rs

@@ -183,6 +183,12 @@ pub struct UnifiedRule {
     #[serde(default)]
     pub patterns: Option<Vec<String>>,
 
+    // Exclusions for search mode: a positive match is dropped when any of these matches
+    // the matched span. Taint rules express the same idea with `sanitizers`.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    #[serde(default)]
+    pub unless: Option<Vec<String>>,
+
     // Taint analysis fields (used when mode = "taint")
     #[serde(skip_serializing_if = "Option::is_none")]
     #[serde(default)]

src/rules.rs

@@ -394,22 +394,87 @@ pub fn match_any_pattern(patterns: &[String], text: &str) -> bool {
     CommonUtils::matches_any_pattern(patterns, text)
 }
 
+/// True when the rule matches `text` and no `unless` entry vetoes the match.
+///
+/// A veto is *match-scoped*: each `unless` entry is tested against the byte span the
+/// matching pattern covers, never against the whole line. Scoping matters —
+/// `el.addEventListener('x', h); div.innerHTML = userInput;` must still report the
+/// innerHTML sink even though `addEventListener` is an `unless` entry of that rule.
 pub fn rule_matches_pattern_unified(rule: &UnifiedRule, text: &str) -> bool {
-    if let Some(pattern) = &rule.pattern
-        && match_pattern(pattern, text)
-    {
+    let mut matched = matching_patterns(rule, text).peekable();
+    if matched.peek().is_none() {
+        return false;
+    }
+
+    let Some(unless) = rule.unless.as_ref() else {
         return true;
+    };
+
+    // Every pattern that matched gets its span checked: a rule's patterns overlap, and a
+    // sanitizer sitting outside the narrowest match still belongs to the same expression.
+    !matched.any(|pattern| {
+        let scope = unless_scope_range(pattern, text).map_or(text, |range| &text[range]);
+        match_any_pattern(unless, scope)
+    })
+}
+
+/// The rule's patterns that match `text`: `pattern` before `patterns`, in file order.
+fn matching_patterns<'a>(
+    rule: &'a UnifiedRule,
+    text: &'a str,
+) -> impl Iterator<Item = &'a str> + 'a {
+    rule.pattern
+        .as_deref()
+        .into_iter()
+        .chain(rule.patterns.as_deref().unwrap_or_default().iter().map(String::as_str))
+        .filter(move |pattern| match_pattern(pattern, text))
+}
+
+/// Byte span of `pattern`'s match inside `text`, used *only* to scope `unless`.
+///
+/// `None` means "no span recoverable for this pattern form"; the caller then widens the
+/// scope to the whole `text`, which is how `unless` behaved before match scoping.
+///
+/// Deliberately separate from `pattern_match_range` below: that one feeds markup line
+/// attribution and must keep mirroring `matches_unified_pattern`'s "no range for glob"
+/// behaviour, or findings move lines.
+fn unless_scope_range(pattern: &str, text: &str) -> Option<std::ops::Range<usize>> {
+    if pattern == text {
+        return Some(0..text.len());
     }
+    if let Some(stripped) = pattern.strip_prefix("regex:") {
+        return Regex::new(stripped).ok()?.find(text).map(|m| m.range());
+    }
+    if pattern.contains("\\\\") || pattern.contains("\\.") {
+        return Regex::new(pattern).ok()?.find(text).map(|m| m.range());
+    }
+    if pattern.contains('\\') {
+        // Escaped taint form: matched by string surgery, no span to recover.
+        return None;
+    }
+    tightest_segment_span(pattern, text)
+}
 
-    if let Some(patterns) = &rule.patterns {
-        for pattern in patterns {
-            if match_pattern(pattern, text) {
-                return true;
-            }
-        }
+/// Tightest in-order span of a glob/substring pattern's literal segments.
+///
+/// Anchored right-to-left: the last segment is taken at its *last* occurrence and each
+/// earlier segment at its last occurrence before the one that follows it. Leftmost-first
+/// would stretch the span across unrelated code — for `document.write(*user*` against
+/// `document.write("safe"); document.write(userInput);` it would swallow the quoted first
+/// call and let the `document\.write\(['\"]` exclusion veto the real finding.
+fn tightest_segment_span(pattern: &str, text: &str) -> Option<std::ops::Range<usize>> {
+    let mut end = text.len();
+    let mut span_end = None;
+    let mut start = None;
+
+    for segment in pattern.split('*').filter(|segment| !segment.is_empty()).rev() {
+        let found = text[..end].rfind(segment)?;
+        span_end.get_or_insert(found + segment.len());
+        start = Some(found);
+        end = found;
     }
 
-    false
+    Some(start?..span_end?)
 }
 
 /// Byte range of the first match of `pattern` inside `text`, when the pattern form can

tests/unit/directory_loading_tests.rs

@@ -13,6 +13,7 @@ fn make_rule(pattern: &str, finding_type: &str) -> UnifiedRule {
         mode: "search".to_string(),
         pattern: Some(pattern.to_string()),
         patterns: None,
+        unless: None,
         sources: None,
         sinks: None,
         propagators: None,

tests/unit/main.rs

@@ -13,3 +13,4 @@ mod pattern_matching_tests;
 mod prefilter_should_scan_tests;
 mod rule_deserialization_tests;
 mod semantic_variables_tests;
+mod unless_exclusion_tests;

tests/unit/pattern_matching_tests.rs

@@ -16,6 +16,7 @@ fn make_rule(pattern: Option<&str>, patterns: Option<Vec<&str>>) -> UnifiedRule
         mode: "search".to_string(),
         pattern: pattern.map(|s| s.to_string()),
         patterns: patterns.map(|v| v.into_iter().map(|s| s.to_string()).collect()),
+        unless: None,
         sources: None,
         sinks: None,
         propagators: None,

tests/unit/prefilter_should_scan_tests.rs

@@ -22,6 +22,7 @@ mod prefilter_should_scan_tests {
                 mode: "search".to_string(),
                 pattern: None,
                 patterns: None,
+                unless: None,
                 sources: None,
                 sinks: None,
                 propagators: None,

tests/unit/unless_exclusion_tests.rs

@@ -0,0 +1,160 @@
+use sighthound::models::UnifiedRule;
+use sighthound::rules::{Rules, rule_matches_pattern_unified};
+
+// UnifiedRule does not derive Default; this fills the fields these tests do not care about.
+fn make_rule(patterns: Vec<&str>, unless: Option<Vec<&str>>) -> UnifiedRule {
+    UnifiedRule {
+        id: Some("test-rule".to_string()),
+        name: None,
+        description: None,
+        category: None,
+        mode: "search".to_string(),
+        pattern: None,
+        patterns: Some(patterns.into_iter().map(str::to_string).collect()),
+        unless: unless.map(|entries| entries.into_iter().map(str::to_string).collect()),
+        sources: None,
+        sinks: None,
+        propagators: None,
+        sanitizers: None,
+        finding_type: None,
+        severity: None,
+        confidence: None,
+        file_types: None,
+        conditions: None,
+        tags: None,
+        cwe_id: None,
+        message: None,
+    }
+}
+
+#[cfg(test)]
+mod unless_exclusion_tests {
+    use super::*;
+
+    fn frontend_rules() -> Rules {
+        Rules::load_from_file("rules/javascript/frontend_security.ron")
+            .expect("Failed to load frontend_security.ron")
+    }
+
+    fn rule_by_id(rules: &Rules, id: &str) -> UnifiedRule {
+        rules
+            .rules
+            .iter()
+            .find(|rule| rule.id.as_deref() == Some(id))
+            .unwrap_or_else(|| panic!("rule {id} missing from frontend_security.ron"))
+            .clone()
+    }
+
+    // The bug this suite guards: `unless:` shipped in the rule files but had no field on
+    // UnifiedRule, and serde has no deny_unknown_fields — so every exclusion parsed into
+    // nothing and every scan ignored it.
+    #[test]
+    fn frontend_rules_parse_their_unless_lists() {
+        let rules = frontend_rules();
+
+        for (id, entries) in [
+            ("js-dom-xss-001", 16),
+            ("js-react-dangerously-set-inner-html-001", 5),
+            ("js-dom-xss-003", 5),
+        ] {
+            let rule = rule_by_id(&rules, id);
+            let unless = rule.unless.unwrap_or_else(|| panic!("{id} dropped its unless list"));
+            assert_eq!(unless.len(), entries, "{id} unless entry count");
+        }
+    }
+
+    #[test]
+    fn innerhtml_rule_reports_unsanitized_assignment() {
+        let rule = rule_by_id(&frontend_rules(), "js-dom-xss-001");
+        assert!(rule_matches_pattern_unified(&rule, "div.innerHTML = userInput"));
+    }
+
+    #[test]
+    fn innerhtml_rule_excludes_sanitized_assignment() {
+        let rule = rule_by_id(&frontend_rules(), "js-dom-xss-001");
+        assert!(!rule_matches_pattern_unified(
+            &rule,
+            "div.innerHTML = DOMPurify.sanitize(userInput)"
+        ));
+    }
+
+    #[test]
+    fn dangerously_set_inner_html_rule_reports_raw_html() {
+        let rule = rule_by_id(&frontend_rules(), "js-react-dangerously-set-inner-html-001");
+        assert!(rule_matches_pattern_unified(
+            &rule,
+            "dangerouslySetInnerHTML={{ __html: userProfile }}"
+        ));
+    }
+
+    #[test]
+    fn dangerously_set_inner_html_rule_excludes_sanitized_html() {
+        let rule = rule_by_id(&frontend_rules(), "js-react-dangerously-set-inner-html-001");
+        assert!(!rule_matches_pattern_unified(
+            &rule,
+            "dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(userProfile) }}"
+        ));
+    }
+
+    #[test]
+    fn document_write_rule_reports_dynamic_content() {
+        let rule = rule_by_id(&frontend_rules(), "js-dom-xss-003");
+        assert!(rule_matches_pattern_unified(&rule, "document.write(userInput)"));
+    }
+
+    #[test]
+    fn document_write_rule_excludes_static_string() {
+        let rule = rule_by_id(&frontend_rules(), "js-dom-xss-003");
+        assert!(!rule_matches_pattern_unified(&rule, "document.write(\"<p>data</p>\")"));
+    }
+
+    // Regression: exclusions are scoped to the matched span, not the line. An `unless`
+    // string belonging to an unrelated statement must not swallow a real finding sharing
+    // the line with it.
+    #[test]
+    fn unrelated_unless_text_on_the_same_line_does_not_suppress() {
+        let rule = rule_by_id(&frontend_rules(), "js-dom-xss-001");
+        assert!(rule_matches_pattern_unified(
+            &rule,
+            "el.addEventListener('click', h); div.innerHTML = userInput;"
+        ));
+    }
+
+    #[test]
+    fn safe_document_write_earlier_on_the_line_does_not_suppress() {
+        let rule = rule_by_id(&frontend_rules(), "js-dom-xss-003");
+        assert!(rule_matches_pattern_unified(
+            &rule,
+            "document.write(\"safe\"); document.write(userInput);"
+        ));
+    }
+
+    #[test]
+    fn sanitized_sibling_expression_does_not_suppress_a_raw_one() {
+        let rule = rule_by_id(&frontend_rules(), "js-dom-xss-001");
+        assert!(rule_matches_pattern_unified(
+            &rule,
+            "a.innerHTML = DOMPurify.sanitize(x); b.innerHTML = userInput;"
+        ));
+    }
+
+    #[test]
+    fn rule_without_unless_is_unaffected() {
+        let rule = make_rule(vec!["*.innerHTML*=*user*"], None);
+
+        assert!(rule.unless.is_none());
+        assert!(rule_matches_pattern_unified(
+            &rule,
+            "div.innerHTML = DOMPurify.sanitize(userInput)"
+        ));
+        assert!(rule_matches_pattern_unified(&rule, "div.innerHTML = userInput"));
+        assert!(!rule_matches_pattern_unified(&rule, "div.textContent = userInput"));
+    }
+
+    #[test]
+    fn empty_unless_list_never_suppresses() {
+        let rule = make_rule(vec!["*.innerHTML*=*user*"], Some(vec![]));
+
+        assert!(rule_matches_pattern_unified(&rule, "div.innerHTML = userInput"));
+    }
+}