diff --git a/CHANGELOG.md b/CHANGELOG.md index 57bf969..83afca1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,41 @@ ## [Unreleased] +### Security — tab ids no longer reach Alpine as expression source (#32) + +- **A request-controlled `tab.id` executed as JavaScript on page load.** Each + tabs template spliced it into four attributes Alpine evaluates as source + (`:class`, `:aria-selected`, `:tabindex`, `@click.prevent`), so an id + containing an apostrophe closed its string literal, ran, and reopened it — + no user interaction, and the surrounding expression still parsed, so Alpine + logged nothing. Sixteen sites across bulma and daisy, plus eight more each in + bootstrap and foundation, which carried the same fix in on their own branches + (#29, #30) rather than merging with a known injection. Fomantic follows the + same way. Fixing it in two themes now instead of five later is the whole + reason this went ahead of the expansion epic. + + HTML escaping does not mitigate it and could not: the parser decodes `'` + back to `'` while building the DOM, and Alpine reads the decoded attribute. + The value has to stop being source. Every binding now reads + `$el.dataset.cfTab` — `data-cf-tab` was already on each tab for the roving + tabindex, so the fix adds plumbing only for the wrapper element some themes + put the active class on. + + This closes the *execution* path, not every use of a hostile id. Attribute + escaping is a separate guarantee, and `install_cf_ui` still leaves JinjaX's + `autoescape` off, so a double quote in `tab.id` can break out of + `data-cf-tab` itself under FastAPI/Litestar. Django/cotton is unaffected. + Tracked as #36, and called out in `docs/accessibility.md` with a workaround + in the meantime. + + `cf_ui_alpine.js` already stated this rule in `initTabs()` and already + followed it for `data-cf-active`; it simply was not carried one level down. + `tests/unit/test_alpine_expression_safety.py` now enforces it over every + template in the package, so a sixth theme cannot reintroduce it by copying a + fifth, and `tests/e2e/test_alpine_injection.py` proves the payload is inert in + a real browser — asserting both that it did not run and that the bindings did + evaluate, since a fix that made Alpine throw would satisfy the first alone. + ### Added — Foundation 6 theme (#23) All 14 components in both template sets, replacing the `PLANNED.md` stub at @@ -79,13 +114,6 @@ All 14 components in both template sets, replacing the `PLANNED.md` stub at alone — which is the useful part, because it means the JS question gets re-asked for free at the moment it is cheapest to answer. -### Fixed — tab ids no longer reach Alpine as expression source (#32) - -- The four Alpine bindings on each tab now read `$el.dataset.cfTab` instead of - an interpolated `'{{ tab.id }}'`. Same fix as the two shipped themes get in - #32; applied here so this theme does not land with the bug and need patching - twice. See that ticket for why HTML escaping cannot address it. - ### Added — Bootstrap 5 theme (#22) All 14 components in both template sets, replacing the `PLANNED.md` stubs at diff --git a/docs/accessibility.md b/docs/accessibility.md index 1a42d59..f7c76c1 100644 --- a/docs/accessibility.md +++ b/docs/accessibility.md @@ -144,14 +144,54 @@ separately rather than smuggled in here. ## Passing state into Alpine -New state crosses into Alpine through `data-` attributes, read in an `x-init` -hook — `data-cf-active` → `initTabs()`, `data-cf-open` → `initPanel()` — rather -than through an interpolated `x-data="cfTabs('{{ active }}')"`. +**No template interpolation inside an attribute Alpine evaluates.** Not in +`x-data`, not in `x-init`, not in a `:binding`, not in an `@handler`. That is +the whole rule, and `tests/unit/test_alpine_expression_safety.py` enforces it +over every template in the package. -The value is request-controlled. A template engine escapes an *attribute* -correctly; it has no idea it is writing JavaScript source, so a single -apostrophe in `active` breaks out of the expression. The data-attribute route -has no such seam. +Initial state crosses in through `data-` attributes read in an `x-init` hook — +`data-cf-active` → `initTabs()`, `data-cf-open` → `initPanel()` — rather than +through an interpolated `x-data="cfTabs('{{ active }}')"`. + +Per-item values do the same thing, one level down. Each tab carries +`data-cf-tab="{{ tab.id }}"`, and every binding on it reads that back: + +```html +:aria-selected="active === $el.dataset.cfTab" +:tabindex="tabIndexFor($el.dataset.cfTab)" +@click.prevent="setActive($el.dataset.cfTab)" +``` + +not: + +```html +:tabindex="tabIndexFor('{{ tab.id }}')" +``` + +**Escaping cannot fix an interpolated expression.** The template engine escapes +an *attribute* correctly, but it has no idea it is writing JavaScript source, +and the escaping is gone before Alpine ever sees the value: the HTML parser +decodes `'` back to `'` while building the DOM, and Alpine reads the +decoded attribute. A quote-bearing `tab.id` therefore closes its string literal +and runs, on page load, with no user interaction — which is exactly what #32 +was. A `data-` attribute has no such seam: its value is never parsed as source, +so the worst a hostile value can do there is be a wrong string. + +> **Attribute escaping is a separate guarantee, and one cf-ui does not yet make +> on the JinjaX path.** `jinjax.Catalog()` builds its environment with +> `autoescape` off and adopts it only from a caller-supplied `jinja_env`; +> `install_cf_ui` does not change that. So under FastAPI/Litestar a `tab.id` +> containing a double quote can still break out of `data-cf-tab` itself and add +> attributes of its own. Django/cotton is unaffected — Django autoescapes by +> default. Until #36 lands, pass a `jinja_env` with `autoescape` enabled to +> `Catalog(...)`, or escape ids before they reach a component. + +If a binding needs a value the server knows, the answer is always another +`data-` attribute on the element carrying the binding — never a wider +expression. `$el.dataset.*` resolves against the element the directive sits on, +so an element that reads `$el.dataset.cfTab` has to carry `data-cf-tab` itself; +reaching into a child or parent instead would put theme-specific DOM structure +back into `cf_ui_alpine.js`, which is the split this package exists to keep. --- @@ -162,10 +202,20 @@ tests could not have caught what they claimed to: * `tests/unit/test_accessibility.py` — claims that *are* markup: a role, an `aria-*` value, a server-rendered class. All four template sets, every case. +* `tests/unit/test_alpine_expression_safety.py` — the rule above, as a guard + over the whole template tree rather than a per-theme check, so the next theme + cannot reintroduce an interpolated expression by copying the last one. * `tests/e2e/` — claims that are behavior: where focus lands after open, where it lands after close, that `Tab` cannot leave the dialog. Parameterized over `js_on` / `js_off`. +`tests/e2e/test_alpine_injection.py` is in that last group for a reason worth +naming: the unit tier can assert a rendered attribute holds no splice point, but +only a browser can show what happens when it does — the decode-then-evaluate +sequence that makes the bug possible needs a real HTML parser and a real Alpine. +It asserts the payload did not run **and** that the bindings did evaluate; a fix +that made Alpine throw would pass the first half while leaving tabs dead. + `expect(role_is_dialog)` proves nothing about focus, and `expect(tab).to_be_attached()` passes against markup nobody can use. Assert the behavior. diff --git a/src/cf_ui/static/cf_ui/cf_ui_alpine.js b/src/cf_ui/static/cf_ui/cf_ui_alpine.js index 7fd2053..5847496 100644 --- a/src/cf_ui/static/cf_ui/cf_ui_alpine.js +++ b/src/cf_ui/static/cf_ui/cf_ui_alpine.js @@ -163,6 +163,10 @@ document.addEventListener('alpine:init', () => { // `x-data="cfTabs('{{ active }}')"`. The value is request-controlled, // and a template engine escapes an attribute correctly but has no // way to escape JavaScript source text. + // + // The same rule governs every id these methods receive: the + // templates pass `$el.dataset.cfTab`, never `'{{ tab.id }}'`. #32 + // was that rule being applied here and not one level down. this.active = this.$el.dataset.cfActive || null; }, diff --git a/src/cf_ui/templates/cotton/_themes/bulma/tabs.html b/src/cf_ui/templates/cotton/_themes/bulma/tabs.html index dc9f4e0..4e5461b 100644 --- a/src/cf_ui/templates/cotton/_themes/bulma/tabs.html +++ b/src/cf_ui/templates/cotton/_themes/bulma/tabs.html @@ -7,15 +7,16 @@ {% for tab in tabs %} diff --git a/src/cf_ui/templates/cotton/_themes/daisy/tabs.html b/src/cf_ui/templates/cotton/_themes/daisy/tabs.html index 142bcb5..acda8b5 100644 --- a/src/cf_ui/templates/cotton/_themes/daisy/tabs.html +++ b/src/cf_ui/templates/cotton/_themes/daisy/tabs.html @@ -10,10 +10,10 @@ aria-controls="{{ hx_target }}" aria-selected="{% if tab.id == active %}true{% else %}false{% endif %}" tabindex="{% if tab.id == active or not active and forloop.first %}0{% else %}-1{% endif %}" - :class="{ 'tab-active': active === '{{ tab.id }}' }" - :aria-selected="active === '{{ tab.id }}'" - :tabindex="tabIndexFor('{{ tab.id }}')" - @click.prevent="setActive('{{ tab.id }}')" + :class="{ 'tab-active': active === $el.dataset.cfTab }" + :aria-selected="active === $el.dataset.cfTab" + :tabindex="tabIndexFor($el.dataset.cfTab)" + @click.prevent="setActive($el.dataset.cfTab)" hx-get="{{ tab.url }}" hx-target="#{{ hx_target }}">{{ tab.id }} {% endfor %} diff --git a/src/cf_ui/templates/jinja/bulma/Tabs.jinja b/src/cf_ui/templates/jinja/bulma/Tabs.jinja index 29eaf88..607087d 100644 --- a/src/cf_ui/templates/jinja/bulma/Tabs.jinja +++ b/src/cf_ui/templates/jinja/bulma/Tabs.jinja @@ -12,15 +12,16 @@ {% for tab in tabs %} diff --git a/src/cf_ui/templates/jinja/daisy/Tabs.jinja b/src/cf_ui/templates/jinja/daisy/Tabs.jinja index a7a7773..9e4c05c 100644 --- a/src/cf_ui/templates/jinja/daisy/Tabs.jinja +++ b/src/cf_ui/templates/jinja/daisy/Tabs.jinja @@ -16,10 +16,10 @@ aria-controls="{{ hx_target }}" aria-selected="{% if tab.id == active %}true{% else %}false{% endif %}" tabindex="{% if tab.id == active or (not active and loop.first) %}0{% else %}-1{% endif %}" - :class="{ 'tab-active': active === '{{ tab.id }}' }" - :aria-selected="active === '{{ tab.id }}'" - :tabindex="tabIndexFor('{{ tab.id }}')" - @click.prevent="setActive('{{ tab.id }}')" + :class="{ 'tab-active': active === $el.dataset.cfTab }" + :aria-selected="active === $el.dataset.cfTab" + :tabindex="tabIndexFor($el.dataset.cfTab)" + @click.prevent="setActive($el.dataset.cfTab)" hx-get="{{ tab.url }}" hx-target="#{{ hx_target }}">{{ tab.id }} {% endfor %} diff --git a/tests/e2e/test_alpine_injection.py b/tests/e2e/test_alpine_injection.py new file mode 100644 index 0000000..3da463a --- /dev/null +++ b/tests/e2e/test_alpine_injection.py @@ -0,0 +1,104 @@ +"""The hostile tab id does not execute in a real browser (#32). + +This is the tier that actually proves the fix. The unit tier can only assert +that the rendered attribute holds no splice point; it cannot show what the +browser does with one, because the whole mechanism depends on two things +pytest does not have — an HTML parser that decodes entity escapes while +building the DOM, and Alpine's expression evaluator reading the decoded value +back out. + +The page is assembled here rather than served by the demo app on purpose: the +demo app should not grow a route whose job is to render a payload. The +templates, `cf_ui_alpine.js`, and the pinned Alpine build are the real ones. + +Two assertions, and both are load-bearing: + +* the payload did not run, and +* the bindings *did* evaluate — a fix that made Alpine throw on the expression + would satisfy the first assertion while leaving the widget dead. +""" + +from pathlib import Path + +import pytest +from jinja2 import Environment, FileSystemLoader, StrictUndefined, select_autoescape + +from cf_ui import themes as cf_ui_themes +from cf_ui.templatetags.cf_ui import _ALPINE_CDN, _DEFAULTS + +PACKAGE_DIR = Path(__file__).parent.parent.parent / "src" / "cf_ui" +JINJA_DIR = PACKAGE_DIR / "templates" / "jinja" +ALPINE_LOCAL = PACKAGE_DIR / "static" / "cf_ui" / "cf_ui_alpine.js" + +#: Resolved from the package's own pin rather than repeated here — testing +#: against a different Alpine than cf-ui ships would be testing the wrong +#: evaluator, and a hardcoded copy is a pin that drifts silently. +ALPINE_CDN = _ALPINE_CDN.format(v=_DEFAULTS["alpinejs"]) + +#: From the registry, so a new theme is covered the moment it is accepted. +THEMES = list(cf_ui_themes.THEMES) + +#: Closes the string literal, runs, and reopens it, so the surrounding +#: expression still parses and Alpine reports no error. See the same constant +#: in tests/unit/test_alpine_expression_safety.py. +HOSTILE_ID = "');window.cfPwned=true;('" + +PAGE = """ + + + + + + +{component} + + +""" + + +def _build_page(theme: str, tmp_path: Path) -> Path: + env = Environment( + loader=FileSystemLoader(JINJA_DIR / theme), + autoescape=select_autoescape(["html", "jinja"]), + undefined=StrictUndefined, + ) + component = env.get_template("Tabs.jinja").render( + tabs=[{"id": HOSTILE_ID, "url": "/x/"}, {"id": "safe", "url": "/safe/"}], + hx_target="tc", + active=HOSTILE_ID, + content="", + extra_class="", + ) + assert "cfPwned" in component, "the hostile id never rendered — nothing to test" + + page = tmp_path / f"injection_{theme}.html" + page.write_text( + PAGE.format( + alpine_local=ALPINE_LOCAL.read_text(encoding="utf-8"), + alpine_cdn=ALPINE_CDN, + component=component, + ), + encoding="utf-8", + ) + return page + + +@pytest.mark.parametrize("theme", THEMES) +def test_a_hostile_tab_id_does_not_execute(page, tmp_path, theme): + page.goto(_build_page(theme, tmp_path).as_uri()) + page.wait_for_function("() => window.Alpine !== undefined", timeout=15000) + + # The bindings have to have been evaluated before "it did not run" means + # anything. aria-selected is computed by Alpine from the tab id it read out + # of the data attribute, so a true here is proof the expression ran and + # resolved the hostile id as a plain string. + page.wait_for_function( + "() => document.querySelector('[role=tab]').getAttribute('aria-selected') === 'true'", + timeout=5000, + ) + first_tab = page.locator('[role="tab"]').first + assert first_tab.get_attribute("data-cf-tab") == HOSTILE_ID + + assert page.evaluate("() => window.cfPwned === true") is False, ( + "the tab id executed as JavaScript — it reached an Alpine expression as source" + ) diff --git a/tests/unit/cotton/test_daisy.py b/tests/unit/cotton/test_daisy.py index 5a48001..0034236 100644 --- a/tests/unit/cotton/test_daisy.py +++ b/tests/unit/cotton/test_daisy.py @@ -180,5 +180,7 @@ def test_breadcrumb_uses_daisy_breadcrumbs_class(daisy_render): def test_tabs_keeps_the_alpine_contract(daisy_render): html = daisy_render("cf/tabs.html", tabs=[{"id": "one", "url": "/one"}], slot="C") assert 'x-data="cfTabs"' in html - assert "setActive('one')" in html + # The id reaches Alpine as data, never as expression text (#32). + assert 'data-cf-tab="one"' in html + assert "setActive($el.dataset.cfTab)" in html assert "tab-active" in html diff --git a/tests/unit/jinja/test_daisy.py b/tests/unit/jinja/test_daisy.py index 970787a..5bca540 100644 --- a/tests/unit/jinja/test_daisy.py +++ b/tests/unit/jinja/test_daisy.py @@ -250,5 +250,7 @@ def test_breadcrumb_uses_daisy_breadcrumbs_class(render): def test_tabs_keeps_the_alpine_contract(render): html = render("Tabs.jinja", tabs=[{"id": "one", "url": "/one"}], content="C") assert 'x-data="cfTabs"' in html - assert "setActive('one')" in html + # The id reaches Alpine as data, never as expression text (#32). + assert 'data-cf-tab="one"' in html + assert "setActive($el.dataset.cfTab)" in html assert "tab-active" in html diff --git a/tests/unit/test_alpine_expression_safety.py b/tests/unit/test_alpine_expression_safety.py new file mode 100644 index 0000000..94e6ee2 --- /dev/null +++ b/tests/unit/test_alpine_expression_safety.py @@ -0,0 +1,253 @@ +"""Request-controlled values must never reach Alpine as expression text (#32). + +Alpine evaluates the *value* of `:attr`, `@event` and `x-*` attributes as +JavaScript source. Interpolating a template variable into one of those is not +an escaping problem that a template engine can solve: the HTML parser decodes +`'` back to `'` while building the DOM, and Alpine reads the decoded +attribute. By the time the string reaches Alpine's evaluator the escaping is +gone, so a quote-bearing value breaks out of its string literal and runs. + +`cf_ui_alpine.js` already states the rule in `initTabs()` and already follows +it for the seed value — `data-cf-active` crosses the boundary as data, and +`x-data="cfTabs('{{ active }}')"` is deliberately not used. This module is the +enforcement for the same rule one level down, plus a tree-wide guard so a +sixth theme cannot reintroduce the pattern by copying a fifth. + +The proof that the fix works lives in `tests/e2e/test_alpine_injection.py` — +only a real browser can show the payload does not execute, because only a real +browser runs the HTML parser and Alpine's evaluator. What is asserted *here* +is the property that makes that possible: the rendered attribute holds no +splice point at all. +""" + +import re +from collections.abc import Callable +from pathlib import Path + +import pytest +from jinja2 import Environment, FileSystemLoader, StrictUndefined, select_autoescape + +from cf_ui import themes as cf_ui_themes + +TEMPLATES_DIR = Path(__file__).parent.parent.parent / "src" / "cf_ui" / "templates" +JINJA_DIR = TEMPLATES_DIR / "jinja" +COTTON_DIR = TEMPLATES_DIR / "cotton" + +#: From the registry, not a literal, so the per-theme cases below start +#: covering a new theme the moment `resolve_theme` starts accepting it. +THEMES = list(cf_ui_themes.THEMES) + +#: A tab id that closes the string literal it would be spliced into, runs, and +#: reopens it so the surrounding expression still parses. An expression that +#: merely *fails* to parse is not the interesting case — Alpine logs and moves +#: on. This one succeeds. +HOSTILE_ID = "');window.cfPwned=true;('" + +TABS = [{"id": "one", "url": "/one/"}, {"id": "two", "url": "/two/"}] + + +# ── 1. Tree-wide guard ──────────────────────────────────────────────────── +# +# Every template, every theme, every engine. This is the test that keeps the +# fix from being a one-time cleanup: it fails on the next template that gets +# the pattern wrong, wherever it lands. + +#: Attributes Alpine evaluates. Two details carry weight: +#: +#: * The lookbehind — without it, `x-` matches inside `hx-get`, and HTMX +#: attributes are values (a URL, a selector), not source text, so +#: interpolating into them is correct and expected. +#: * All three quoting forms. Single-quoted delimiters are not hypothetical: +#: they are what an author reaches for when the expression itself needs a +#: double quote (`:class='{ "is-active": … }'`). This guard is the only thing +#: standing between a new theme and reintroducing #32, so a form it cannot +#: see is a form that gets reported clean forever. +ALPINE_ATTR = re.compile( + r"""(?'"]+) # bare + ) + """, + re.VERBOSE, +) + +#: What a template engine's output looks like, in either engine. +INTERPOLATION = re.compile(r"\{\{|\{%") + + +def _value(match: re.Match) -> str: + """The matched attribute's value, whichever quoting form it used.""" + return next(group for group in match.groups() if group is not None) + + +# The guard's own tests. A lint that silently stops matching reports clean +# forever, which is worse than not having it — so what it does and does not +# catch is pinned here rather than left to the regex being read correctly. +GUARD_CASES = [ + (True, ':tabindex="tabIndexFor({{ tab.id }})"'), + (True, ":tabindex='tabIndexFor({{ tab.id }})'"), + (True, ":tabindex={{ tab.id }}"), + (True, 'x-on:click="go({{ id }})"'), + (True, "x-data=\"cfTabs('{{ active }}')\""), + (True, "@click.prevent=\"setActive('{{ tab.id }}')\""), + (True, ':class="{% if x %}a{% endif %}"'), + # HTMX values are a URL and a selector, not source. Interpolating is right. + (False, 'hx-get="{{ tab.url }}"'), + (False, 'hx-target="#{{ hx_target }}"'), + # data-* is the sanctioned route, however hostile the value. + (False, 'data-cf-tab="{{ tab.id }}"'), + (False, 'data-x-thing="{{ v }}"'), + # An Alpine expression with no interpolation at all. + (False, ":class=\"{ 'is-active': active === $el.dataset.cfTab }\""), + (False, '@keydown="onKeydown($event)"'), +] + + +@pytest.mark.parametrize(("flagged", "attribute"), GUARD_CASES, ids=lambda v: str(v)[:48]) +def test_the_guard_flags_what_it_claims_to(flagged: bool, attribute: str): + hits = [m for m in ALPINE_ATTR.finditer(attribute) if INTERPOLATION.search(_value(m))] + assert bool(hits) is flagged, ( + f"guard {'missed' if flagged else 'false-positived on'}: {attribute}" + ) + + +def _all_templates() -> list[Path]: + return sorted([p for p in JINJA_DIR.rglob("*.jinja")] + [p for p in COTTON_DIR.rglob("*.html")]) + + +def _rel(path: Path) -> str: + return str(path.relative_to(TEMPLATES_DIR)).replace("\\", "/") + + +@pytest.mark.parametrize("template", _all_templates(), ids=_rel) +def test_no_alpine_expression_attribute_interpolates_template_output(template: Path): + source = template.read_text(encoding="utf-8") + offenders = [ + match.group(0) + for match in ALPINE_ATTR.finditer(source) + if INTERPOLATION.search(_value(match)) + ] + assert not offenders, ( + f"{_rel(template)} splices template output into an Alpine expression:\n" + + "\n".join(f" {o}" for o in offenders) + + "\n\nAlpine evaluates these values as JavaScript. Pass the value through a " + "`data-` attribute and read it back with `$el.dataset.*` instead — see " + "docs/accessibility.md." + ) + + +# ── 2. The tab bindings specifically ────────────────────────────────────── + + +@pytest.fixture(params=THEMES) +def theme(request) -> str: + return request.param + + +@pytest.fixture +def jinja_render(theme: str) -> Callable[..., str]: + env = Environment( + loader=FileSystemLoader(JINJA_DIR / theme), + autoescape=select_autoescape(["html", "jinja"]), + undefined=StrictUndefined, + ) + + def _render(template_name: str, **ctx: object) -> str: + return env.get_template(template_name).render(**ctx) + + return _render + + +@pytest.fixture +def cotton_render(settings, theme: str) -> Callable[..., str]: + from django.template.loader import render_to_string + + settings.CF_UI_THEME = theme + + def _render(stem: str, **props: object) -> str: + return render_to_string(f"cotton/cf/{stem}.html", props) + + return _render + + +def _alpine_values(html: str) -> list[str]: + return [_value(match) for match in ALPINE_ATTR.finditer(html)] + + +def test_jinja_tab_bindings_read_the_tab_id_from_a_data_attribute(jinja_render): + html = jinja_render( + "Tabs.jinja", tabs=TABS, hx_target="tc", active="two", content="", extra_class="" + ) + assert "$el.dataset.cfTab" in html + assert "tabIndexFor('" not in html + assert "setActive('" not in html + + +def test_cotton_tab_bindings_read_the_tab_id_from_a_data_attribute(cotton_render): + html = cotton_render("tabs", tabs=TABS, hx_target="tc", active="two", **{"class": ""}) + assert "$el.dataset.cfTab" in html + assert "tabIndexFor('" not in html + assert "setActive('" not in html + + +def test_jinja_every_element_bound_to_the_tab_id_carries_it_as_data(jinja_render): + """`$el.dataset.cfTab` only resolves on an element that has the attribute. + + The active-class binding sits on the row wrapper in some themes and on the + anchor in others, so both have to carry `data-cf-tab` — reaching across + the DOM from one to the other would put theme structure back into + `cf_ui_alpine.js`. + """ + html = jinja_render( + "Tabs.jinja", tabs=TABS, hx_target="tc", active="two", content="", extra_class="" + ) + for tag in re.findall(r"<[a-zA-Z][^>]*>", html, re.DOTALL): + if "$el.dataset.cfTab" in tag: + assert "data-cf-tab=" in tag, ( + f"reads $el.dataset.cfTab but carries no data-cf-tab:\n {tag}" + ) + + +def test_cotton_every_element_bound_to_the_tab_id_carries_it_as_data(cotton_render): + html = cotton_render("tabs", tabs=TABS, hx_target="tc", active="two", **{"class": ""}) + for tag in re.findall(r"<[a-zA-Z][^>]*>", html, re.DOTALL): + if "$el.dataset.cfTab" in tag: + assert "data-cf-tab=" in tag, ( + f"reads $el.dataset.cfTab but carries no data-cf-tab:\n {tag}" + ) + + +# ── 3. A hostile id, rendered ───────────────────────────────────────────── + + +def test_jinja_tabs_leave_a_hostile_tab_id_out_of_every_expression(jinja_render): + html = jinja_render( + "Tabs.jinja", + tabs=[{"id": HOSTILE_ID, "url": "/x/"}], + hx_target="tc", + active=HOSTILE_ID, + content="", + extra_class="", + ) + assert "cfPwned" not in "".join(_alpine_values(html)), ( + "the payload reached an attribute Alpine evaluates as JavaScript" + ) + assert "cfPwned" in html, "the hostile id never rendered at all — this test proved nothing" + + +def test_cotton_tabs_leave_a_hostile_tab_id_out_of_every_expression(cotton_render): + html = cotton_render( + "tabs", + tabs=[{"id": HOSTILE_ID, "url": "/x/"}], + hx_target="tc", + active=HOSTILE_ID, + **{"class": ""}, + ) + assert "cfPwned" not in "".join(_alpine_values(html)) + assert "cfPwned" in html, "the hostile id never rendered at all"