Skip to content

Escape conference text in inline JavaScript and HTML attributes - #513

Merged
JesperDramsch merged 3 commits into
mainfrom
ai-agent/compassionate-planck-1yyxcp
Oct 19, 2026
Merged

JesperDramsch merged 3 commits into
mainfrom
ai-agent/compassionate-planck-1yyxcp

Conversation

@JesperDramsch

@JesperDramsch JesperDramsch commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Problem

Liquid pastes values into templates verbatim. index.html built the calendar button with place: '{{conf.place}}', which rendered as

place: 'Abidjan, Côte d'Ivoire',

and the front page's per-conference inline script failed to parse. The curly apostrophe in #512 fixed conference, but place still holds the ASCII apostrophe, so on current main the front page and /conference/pycon-cote-d-ivoire-2027/ both throw SyntaxError: Unexpected identifier 'Ivoire' (confirmed by building the site and parsing every inline <script> with Node).

The same verbatim pasting puts names and places into double-quoted HTML attributes (data-conf-name, data-location, aria-label, meta content, map and share links), where a " in the data would end the attribute.

Change

  • JavaScript: every conference name, place and link that reaches a script goes through jsonify, the idiom _includes/head.html and the JSON-LD block already use: index.html (calendar button), _layouts/conference.html (calendar links, follow-series state), _includes/calendar.js (calendar page).
  • HTML attributes: free text goes through escape, or url_encode where it is part of a URL (Google Maps queries, Twitter handles). The share links use url_encode | replace: "+", "%20", because mailto: reads + literally. Covers index.html, _layouts/conference.html, _layouts/summary.html, _pages/series.html, _includes/action_bar.html, conference_detail_actions.html, index_conf_title_row.html and index_conf_date_place.html (including the extra_places links). URL fields stay as they are: the schema validates them as HttpUrl, which percent-encodes quotes.
  • (extended) label: calendar.js never reset extended, so every conference after the first extended CfP was labelled "(extended)": 66 labels for 16 extended CfPs. It now resets in the else branch, as calendar-deadlines.ics already does.
  • Test: tests/test_template_escaping.py scans every template. It fails when a free-text conference field (any schema field apart from year, dates, deadlines, timezone and sub, so later fields count too) reaches a script without a final jsonify/slugify, when a jsonify'd value sits in quotes, or when free text inside an HTML tag has no escape/url_encode. It follows fields into include parameters (conf_name=conf.conference) and into variables set with assign or for, which is how the action bar and the extra_places links got past a direct check. Values passed through capture, or set in another file, still get past it; the module docstring says so.

The data keeps its real spelling: apostrophes, quotes and backslashes in names and places are legitimate, so neither a data check nor an apostrophe-rewriting formatter fits.

Tests

  • The test fails on main's templates at all 52 sites changed here (14 in scripts, 38 in attributes) and passes after. Fixture cases check the checker, including include parameters, loop variables and slugify'd assigns.
  • pytest tests/: 2487 passed, 4 skipped, 3 xfailed, 2 xpassed.
  • ruff 0.4.4, black 24.4.2, isort 5.13.2, docsig 0.53.2 clean on the test file.
  • bundle exec jekyll build --config _config.yml,_config.test.yml, then a Node syntax check of every inline script on all 197 built pages: 2 broken on main, 0 now.
  • Stress build with a temporary entry named PyCon Côte d'Ivoire "Quoted" & <b>, place Abidjan, Côte d'Ivoire "Q" & \ and an extra place with a ": 0 broken scripts, and all 36 attributes carrying the name or place on the front, conference and series pages parse back to the original value. Map and share links come out fully percent-encoded.
  • Calendar page with real data: 16 "CfP (extended)" labels for the 16 entries in conferences.yml with cfp_ext.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YSanwpb5BXzbhLxMFyz9yS

Liquid pastes values verbatim, so `place: '{{conf.place}}'` rendered as
`place: 'Abidjan, Côte d'Ivoire'` and the front page's per-conference
script failed to parse. The curly apostrophe in #512 fixed the name but
not the place, so the front page and the PyCon Côte d'Ivoire page still
threw a SyntaxError.

Every conference name, place and link that reaches JavaScript now goes
through jsonify, the way head.html and the JSON-LD block already do.
tests/test_template_escaping.py fails when a free-text conference field
lands in a script without jsonify or slugify, or when a jsonify'd value
is wrapped in quotes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YSanwpb5BXzbhLxMFyz9yS
@github-actions github-actions Bot added the Tests label Oct 19, 2026
Comment thread tests/test_template_escaping.py Fixed
CodeQL (py/bad-tag-filter) flagged that `</script>` misses `</script >`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YSanwpb5BXzbhLxMFyz9yS

Copy link
Copy Markdown
Owner Author

Routine review

Request

No linked issue; the request is the PR description: route every conference name, place and link that reaches inline JavaScript through jsonify, so Abidjan, Côte d'Ivoire stops breaking the parse. The instances it names: index.html (calendar button), _layouts/conference.html (calendar links, follow-series state), _includes/calendar.js (calendar page), plus a new checker test, tests/test_template_escaping.py, that covers every template.

Checked against

  • Date: Mon Oct 19 12:54:55 UTC 2026 (date).
  • No external facts. Liquid's append with an integer year, and jsonify output, were checked by building the site (Jekyll 4.3.3, Liquid 4.0.4 from Gemfile.lock), not from documentation.

Needs your decision

  1. CodeQL check is red on this head. It raises one alert, rated high, in the new test: tests/test_template_escaping.py:43 Bad HTML filtering regexp: This regular expression does not match script end tags like </script >. The regex only scans this repo's own templates, and none of them contains </script > (grep found none), so it is not a real vulnerability. Do you dismiss the alert as a false positive, or change </script> to </script\s*> in the regex? I recommend the regex change: it is a one-line edit and it turns the check green without a dismissal.

Findings

Important: None.

Nits

  • tests/test_template_escaping.py:45-46: the checker only sees a field when the field is referenced directly inside the output tag. Indirection gets past it: {% assign n = conf.conference %} followed by '{{ n }}' passes. No template does this today. Mention the gap in the module docstring so nobody takes a green run as a proof.

Tests

The new test does test the invariant. It scans every <script> block and .js include under index.html, _includes, _layouts, _pages and _years, and it derives the free-text fields from the Conference schema. It is not tied to one instance.

Base (origin/main templates, with the new test file copied in):

E         _includes/calendar.js:133: {{conf.conference}} (free text needs '| jsonify')
E         _includes/calendar.js:135: {{conf.place}} (free text needs '| jsonify')
E         _includes/calendar.js:145: {{conf.conference}} (free text needs '| jsonify')
E         _includes/calendar.js:147: {{conf.place}} (free text needs '| jsonify')
E         _includes/calendar.js:158: {{conf.conference}} (free text needs '| jsonify')
E         _includes/calendar.js:160: {{conf.place}} (free text needs '| jsonify')
E         _includes/calendar.js:180: {{conf.conference}} (free text needs '| jsonify')
E         _includes/calendar.js:182: {{conf.place}} (free text needs '| jsonify')
E         _layouts/conference.html:350: {{ page.conference }} (free text needs '| jsonify')
E         _layouts/conference.html:353: {{ page.place }} (free text needs '| jsonify')
E         _layouts/conference.html:420: {{ page.conference }} (free text needs '| jsonify')
E         index.html:123: {{conf.conference}} (free text needs '| jsonify')
E         index.html:126: {{conf.place}} (free text needs '| jsonify')
E         index.html:127: {{conf.link}} (free text needs '| jsonify')
FAILED tests/test_template_escaping.py::test_free_text_reaches_javascript_escaped
1 failed, 10 passed in 0.26s

Change, full suite (REVIEW.md uv line, Python 3.11, pandas<2.3):

2471 passed, 5 skipped, 3 xfailed, 2 xpassed, 251 warnings in 217.47s (0:03:37)

uvx ruff@0.4.4 check utils tests: All checks passed!

End-to-end: I ran jekyll build --config _config.yml,_config.test.yml and then parsed every inline non-JSON <script> of every built page with Node (new Function):

base:   /tmp/sitebase/conference/pycon-cote-d-ivoire-2027/index.html Unexpected identifier 'Ivoire'
base:   /tmp/sitebase/index.html Unexpected identifier 'Ivoire'
base:   pages 197 broken scripts 2
change: pages 197 broken scripts 0

On the front page (title/place/link) and the calendar page (name/location), the rendered values are identical between base and change once quote characters are stripped. The integer year in append and the (extended) suffix render as before. Rendered sample: const confName = "PyCon Côte d’Ivoire";.

CI on the head: pre-commit.ci - pr: success. Python Tests, Data Validation, Pre-commit Checks, CI Status and the four CodeQL Analyze jobs are success. CodeQL is failure: 1 new alert including 1 high severity security vulnerability (see decision 1).

Bounds

  • Read in full: the diff, the new test file, and REVIEW.md, CONTRIBUTING.md and AGENTS.md from main. Read in part: _includes/calendar.js around the loop. Fetched through the REST API; the head SHA matched 565cd63.
  • I did not run jest/npm: static/js/ and tests/frontend/ are unchanged. The build used the test config only (English, test data), not the full multilingual build.
  • Pre-existing: in _includes/calendar.js:124, extended is assigned inside the loop and never reset, so once any conference has cfp_ext, every conference after it in the loop gets "CfP (extended)" as well. Both base and change render 66 of these labels.
  • Pre-existing: _includes/conference_detail_actions.html:8,11,27 puts page.conference/page.place into double-quoted data- attributes without escaping. That is safe for ' but would break on a " in the data. It is HTML, not JavaScript, so it is outside this PR and the new checker.

Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Decision 1 is settled by 730439f: the end tag is now </script\b[^>]*> and CodeQL is green on that head. The indirection nit ({% assign n = conf.conference %} then '{{ n }}') is fair; the docstring note rides the next code push on this PR if one comes.


Generated by Claude Code

Free text in a double-quoted attribute ended the value at the first ",
so data-conf-name, data-location, aria-label, meta content and the share
and map links now go through escape, or url_encode where the value is
part of a URL. The share links replace url_encode's + with %20, which
mailto: needs and the other targets accept. URL fields stay as they are:
the schema validates them and pydantic percent-encodes quotes in them.

The front page's action bar got the name and place through include
parameters, and the extra places through a for loop, so the template test
now follows include parameters and variables set with assign or for.

calendar.js never reset `extended`, so every conference after the first
extended CfP was labelled "(extended)" too: 66 labels for 16 extended
CfPs. It now resets the way calendar-deadlines.ics already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YSanwpb5BXzbhLxMFyz9yS
@JesperDramsch JesperDramsch changed the title Escape conference text in inline JavaScript with jsonify Escape conference text in inline JavaScript and HTML attributes Oct 19, 2026
@JesperDramsch
JesperDramsch merged commit 33057d7 into main Oct 19, 2026
11 checks passed
@JesperDramsch
JesperDramsch deleted the ai-agent/compassionate-planck-1yyxcp branch October 19, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants