Anchor selectors with :scope so scoping is real

.changeset/proud-scopes-anchor.md

@@ -0,0 +1,5 @@
+---
+"@openproject/stimulus-elements": minor
+---
+
+Anchor element lookups with `:scope` so "scoped to the controller element" is real: previously `root.querySelector(".menu li")` could match via a `.menu` ancestor *outside* the controller element. Every comma-separated alternative is now prefixed with `:scope` (commas inside quotes, parentheses, brackets, escapes, or CSS comments are respected, and comments are stripped), so the anchor cannot be bypassed with a selector list. Only alternatives *starting* with `:scope` pass through untouched — a non-leading `:scope` does not anchor and gets the prefix too. Relative selectors like `> li` now work. Results were always confined to descendants of the controller element; what changes is which of them a combinator selector can match.

CONTEXT.md

@@ -28,7 +28,8 @@ accessor triples via element definitions.
 **Scoped query**
 The single DOM-lookup module (`src/query.ts`): `scopedQuery(root, selector)`
 returns `{ first, all, exists }`. Owns the falsy-root guard, invalid-selector
-handling, and the warn-once policy; callers never see those concerns.
+handling, :scope anchoring, and the warn-once policy; callers never see those
+concerns.
 
 ## Recorded decisions
 
@@ -49,6 +50,31 @@ handling, and the warn-once policy; callers never see those concerns.
   is a `WeakMap` keyed by the query root, not a process-global set. Lifetime
   is an implementation detail: warnings die with the element, tests need no
   reset hook, and each controller element reports a bad selector once.
+- **Selectors are :scope-anchored per alternative.** `querySelector(sel)`
+  matches selectors document-wide and only filters *results* to descendants,
+  so `.menu li` can bind via a `.menu` ancestor outside the controller
+  element. `anchorToScope` prefixes `:scope ` onto every *top-level*
+  comma-separated alternative (split by a depth/quote/escape/comment-aware
+  scanner — commas only nest inside quotes, parens, brackets, escapes, or
+  `/* … */` comments in valid selector syntax; comments are stripped since
+  their contents would otherwise corrupt the quote tracking). Only a
+  *leading* `:scope` proves an alternative is rooted and passes through;
+  a non-leading `:scope` (`:not(:scope) .item`, `.outer :scope .item`) does
+  not anchor and gets the prefix too — at worst that makes the alternative
+  unmatchable, which fails closed instead of leaking. A `:scope :is(...)`
+  wrap was rejected: it anchors only the subject, combinator left-hand sides
+  inside `:is()` still match ancestors outside the root. Note: happy-dom
+  already restricts
+  combinator matching to the subtree (non-spec), so the leak only reproduces
+  in real browsers — the guarantee is pinned by unit tests on the pure
+  rewrite, verified manually in Chrome.
+- **Selector trust model.** Override attributes are as trusted as any
+  Stimulus `data-*` attribute — attribute injection already grants
+  `data-controller`/`data-action`, which is strictly stronger. Selector
+  evaluation is read-only, fails closed, and returns only descendants of the
+  controller element. `CSS.escape` has no application point inside this
+  library (nothing is interpolated into a selector template); it is user
+  guidance for dynamically built selector values.
 - **Type/runtime lockstep.** `Camelize<K>` mirrors the runtime `camelize`
   regex exactly (ASCII-only, tail-recursive). Twin sample tables live in
   `test/element-definition.test.ts` and `test/types.test-d.ts` — keep in sync.

README.md

@@ -62,6 +62,12 @@ For each entry `foo: "<selector>"` (key camelized) you get:
 Lookups are **scoped to the controller's own element**, read **live** on every
 access, and never throw — an invalid selector warns once and yields `null` / `[]`.
 
+Scoping is anchored: every selector (and every comma-separated alternative in
+it) is evaluated as if prefixed with `:scope`, so combinators cannot reach
+through ancestors outside the controller element — `.menu li` only matches
+when `.menu` itself is inside the controller. Selectors starting with
+`:scope` are left untouched, and relative selectors like `> li` work as-is.
+
 ## Overriding selectors from the DOM
 
 Any declared element's selector can be overridden per instance from the controller
@@ -80,6 +86,17 @@ to the static selector. Overrides are read live, like all lookups.
 
 Keep element names to simple camelCase words — an embedded acronym like `htmlURL` dasherizes to `html-u-r-l`, which is hard to predict in the attribute.
 
+### Security
+
+Selector evaluation is read-only and fails closed, and results are always
+descendants of the controller element. Override attributes carry the same
+trust level as any Stimulus `data-*` attribute: markup that can inject
+`data-*-element` attributes can already inject `data-controller` and
+`data-action`, which is strictly more powerful. If you sanitize user-supplied
+HTML, strip or allowlist `data-*` attributes. If you build selector values
+from user input yourself, escape the dynamic parts with
+[`CSS.escape()`](https://developer.mozilla.org/en-US/docs/Web/API/CSS/escape_static).
+
 ## TypeScript
 
 ```ts

src/query.ts

@@ -14,6 +14,70 @@ function warnOnce(root: Element, selector: string, reason: unknown): void {
   )
 }
 
+// Splits a selector list on top-level commas only, stripping CSS comments
+// as it scans. Commas nested inside parentheses, brackets, quotes, or after
+// a backslash escape are not separators in valid selector syntax, and
+// comment contents must not affect any of that state (a quote inside
+// /* … */ would otherwise jam the quote tracking). Comments are removed
+// without inserting whitespace, matching CSS tokenizer semantics. Invalid
+// input may split oddly, but the rewritten selector then throws in
+// querySelector and fails closed.
+function splitSelectorList(selector: string): string[] {
+  const parts: string[] = []
+  let depth = 0
+  let quote: string | null = null
+  let current = ""
+  for (let i = 0; i < selector.length; i++) {
+    const char = selector[i]
+    if (char === "\\") {
+      current += selector.slice(i, i + 2)
+      i++
+      continue
+    }
+    if (!quote && char === "/" && selector[i + 1] === "*") {
+      const end = selector.indexOf("*/", i + 2)
+      if (end === -1) break // unterminated comment consumes the rest
+      i = end + 1
+      continue
+    }
+    if (quote) {
+      if (char === quote) quote = null
+    } else if (char === '"' || char === "'") {
+      quote = char
+    } else if (char === "(" || char === "[") {
+      depth++
+    } else if (char === ")" || char === "]") {
+      if (depth > 0) depth--
+    } else if (char === "," && depth === 0) {
+      parts.push(current)
+      current = ""
+      continue
+    }
+    current += char
+  }
+  parts.push(current)
+  return parts
+}
+
+// Anchors every top-level alternative to the query root, so combinators
+// cannot match through ancestors outside it (".menu li" must find ".menu"
+// inside the root). Only a LEADING :scope proves the alternative is rooted —
+// that is the author's explicit anchoring and passes through untouched.
+// Everything else gets the prefix, including alternatives mentioning :scope
+// elsewhere (":not(:scope) .item", ".outer :scope .item"): a non-leading
+// :scope does not anchor, and prefixing at worst makes the alternative
+// unmatchable, which fails closed instead of leaking.
+const LEADING_SCOPE = /^:scope(?![\w-])/i
+
+export function anchorToScope(selector: string): string {
+  return splitSelectorList(selector)
+    .map((part) => {
+      const trimmed = part.trim()
+      return LEADING_SCOPE.test(trimmed) ? trimmed : `:scope ${trimmed}`
+    })
+    .join(", ")
+}
+
 export interface ScopedQuery {
   first(): Element | null
   all(): Element[]
@@ -35,9 +99,10 @@ export function scopedQuery(
     warnOnce(root, selector, "selector is empty")
     return EMPTY_QUERY
   }
+  const anchored = anchorToScope(selector)
   const first = (): Element | null => {
     try {
-      return root.querySelector(selector)
+      return root.querySelector(anchored)
     } catch (error) {
       warnOnce(root, selector, error)
       return null
@@ -47,7 +112,7 @@ export function scopedQuery(
     first,
     all() {
       try {
-        return Array.from(root.querySelectorAll(selector))
+        return Array.from(root.querySelectorAll(anchored))
       } catch (error) {
         warnOnce(root, selector, error)
         return []

test/query.test.ts

@@ -1,13 +1,17 @@
 import { test, expect, beforeEach, vi } from "vitest"
-import { scopedQuery } from "../src/query"
+import { scopedQuery, anchorToScope } from "../src/query"
 
 beforeEach(() => {
   document.body.innerHTML = `
-    <section id="root">
-      <span class="item">1</span>
-      <span class="item">2</span>
-      <b id="only"></b>
-    </section>
+    <div class="wrap">
+      <section id="root">
+        <span class="item">1</span>
+        <span class="item">2</span>
+        <b id="only"></b>
+        <span data-x="a,b" id="attr-comma"></span>
+        <div class="inner"><span class="deep-item">d</span></div>
+      </section>
+    </div>
     <span class="item">outside</span>
   `
 })
@@ -76,6 +80,98 @@ test("empty / whitespace selector warns once per root and returns null / []", ()
   warn.mockRestore()
 })
 
+// The :scope anchoring guarantee: unit tests pin the pure selector rewrite,
+// and (since the suite runs in a real browser) the scopedQuery tests below
+// exercise the actual leak — ".wrap .item" matching via an ancestor OUTSIDE
+// the root. See harness.test.ts for the environment guard.
+test("anchorToScope prefixes a bare selector", () => {
+  expect(anchorToScope(".wrap .item")).toBe(":scope .wrap .item")
+})
+
+test("anchorToScope prefixes every comma-separated alternative", () => {
+  expect(anchorToScope(".a, .b")).toBe(":scope .a, :scope .b")
+})
+
+test("anchorToScope does not split on commas inside parentheses", () => {
+  expect(anchorToScope(":is(.a, .b) > li")).toBe(":scope :is(.a, .b) > li")
+})
+
+test("anchorToScope does not split on commas inside quoted attribute values", () => {
+  expect(anchorToScope('[data-x="a,b"]')).toBe(':scope [data-x="a,b"]')
+  expect(anchorToScope("[data-x='a,b']")).toBe(":scope [data-x='a,b']")
+})
+
+test("anchorToScope does not split on escaped commas", () => {
+  expect(anchorToScope(".a\\,b")).toBe(":scope .a\\,b")
+})
+
+test("anchorToScope leaves alternatives with a LEADING :scope untouched", () => {
+  expect(anchorToScope(":scope > .item")).toBe(":scope > .item")
+  expect(anchorToScope(":SCOPE .a, .b")).toBe(":SCOPE .a, :scope .b")
+  expect(anchorToScope(":scope.foo .item")).toBe(":scope.foo .item")
+})
+
+test("anchorToScope prefixes alternatives where :scope is not the leading anchor", () => {
+  // a non-leading :scope must not disable anchoring — these could otherwise
+  // match through ancestors outside the root
+  expect(anchorToScope(":not(:scope) .item")).toBe(":scope :not(:scope) .item")
+  expect(anchorToScope(".outer :scope .item")).toBe(":scope .outer :scope .item")
+  expect(anchorToScope('[data-x=":scope"] .item')).toBe(':scope [data-x=":scope"] .item')
+  // ":scope" glued to an identifier tail is not the :scope pseudo-class
+  expect(anchorToScope(":scoped .item")).toBe(":scope :scoped .item")
+})
+
+test("anchorToScope strips CSS comments so they cannot confuse the scanner", () => {
+  // a quote inside a comment must not jam the quote state and hide the comma
+  expect(anchorToScope('.none/*"*/, .outer .item')).toBe(":scope .none, :scope .outer .item")
+  // comments are removed without inserting whitespace (CSS tokenizer semantics)
+  expect(anchorToScope(".a/*x*/.b")).toBe(":scope .a.b")
+  // unterminated comment consumes the rest of the selector
+  expect(anchorToScope(".a/*, .outer .item")).toBe(":scope .a")
+})
+
+test("anchorToScope makes relative selectors explicit", () => {
+  expect(anchorToScope("> .item")).toBe(":scope > .item")
+})
+
+test("combinators cannot match through ancestors outside the root", () => {
+  expect(scopedQuery(root(), ".wrap .item").first()).toBeNull()
+  expect(scopedQuery(root(), ".wrap .item").all()).toEqual([])
+})
+
+test("combinators still work when the full path is inside the root", () => {
+  const deep = scopedQuery(root(), ".inner .deep-item").first()
+  expect(deep!.textContent).toBe("d")
+})
+
+test("every comma-separated alternative is anchored to the root", () => {
+  // second alternative must not escape the anchor via the comma
+  expect(scopedQuery(root(), ".none, .wrap .item").first()).toBeNull()
+  // but comma alternatives that are inside the root still match
+  expect(scopedQuery(root(), ".none, .item").all().length).toBe(2)
+})
+
+test("relative selectors match direct children of the root", () => {
+  const items = scopedQuery(root(), "> .item").all()
+  expect(items.map((el) => el.textContent)).toEqual(["1", "2"])
+  expect(scopedQuery(root(), "> .deep-item").first()).toBeNull()
+})
+
+test("commas inside :is() are not treated as list separators", () => {
+  expect(scopedQuery(root(), ":is(.item, .none)").all().length).toBe(2)
+})
+
+test("commas inside quoted attribute values are not treated as list separators", () => {
+  expect(scopedQuery(root(), '[data-x="a,b"]').first()).toBe(
+    document.getElementById("attr-comma"),
+  )
+})
+
+test("an explicit :scope in the selector is left untouched", () => {
+  const items = scopedQuery(root(), ":scope > .item").all()
+  expect(items.length).toBe(2)
+})
+
 test("a fresh root gets its own warning — registry is per element, no reset needed", () => {
   const warn = vi.spyOn(console, "warn").mockImplementation(() => {})
   scopedQuery(root(), "###").first()