Skip to content

fix(export): escape every "<" in JSON embedded in <script> blocks - #4125

Open
tricode-online-admin wants to merge 1 commit into
Graphify-Labs:v8from
tricode-online-admin:fix/html-script-escape-4124
Open

tricode-online-admin wants to merge 1 commit into
Graphify-Labs:v8from
tricode-online-admin:fix/html-script-escape-4124

Conversation

@tricode-online-admin

Copy link
Copy Markdown

What does this PR do?

Fixes #4124.

graph.html (and GRAPH_TREE.html) embed graph data as JSON inside a <script> element. The escaping only neutralised </, so an unclosed <!-- followed later by <script reached the document verbatim. That moves the HTML tokenizer into the script data double escaped state, where the real </script> no longer closes the element: the rest of the page is swallowed as script text, it fails with SyntaxError: Unexpected token '<', and nothing renders while the CLI reports success.

The fix escapes every < as \u003c instead of only the </ pair. JSON and JavaScript decode \u003c back to <, so labels display unchanged, but the tokenizer never sees <!--, <script or </script inside the data block.

Changed:

  • graphify/exporters/html.py: _js_safe() (nodes, edges, legend, hyperedges).
  • graphify/tree_html.py: the inline escape in emit_html().

Correction to the issue: the issue listed graphify/callflow_html.py as a third copy. It is not affected: its inline <script> is static code and embeds no graph data, so it is left unchanged.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

Invariant: JSON embedded in an inline <script> must never be able to change the HTML tokenizer's state, whatever text node labels carry. With no raw < in the payload, none of the three sequences the tokenizer reacts to (<!--, <script, </script) can appear.

Regression tests (both fail on v8 and pass with the fix):

  • tests/test_export.py::test_to_html_escapes_script_data_sequences_in_embedded_json: labels containing <!-- and <script in nodes, edges, legend and hyperedges. Asserts no raw < in any embedded JSON block and that labels still decode to the original text.
  • tests/test_tree_html.py::test_emit_html_escapes_script_data_sequences_in_embedded_json: same check for emit_html(), plus a JSON round-trip.

End-to-end check: ran graphify update on the minimal repro from #4124 (guide.md + hello.js) and parsed the page with jsdom:

Page Inline <script> elements Script syntax
Before the fix 1 SyntaxError: Unexpected token '<'
After the fix 2 ok

Generated/persisted state: only the HTML output changes, and it is rewritten on the next run. graph.json, caches and the manifest are untouched.

Limitations: only < is escaped. > and & cannot change the tokenizer state inside script data, and json.dumps already escapes non-ASCII characters (including U+2028/U+2029).

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant.
  • Made the smallest fix necessary.
  • Added a regression test (if bug fix) or isolated boundary test.
  • Kept the PR description synchronized with the final implementation.
  • Documented any limitations / unsupported cases explicitly.

How was this tested?

Windows 11, Python via uv, no provider environment variables set.

# New tests, before the fix: 2 failed (raw "<" in embedded JSON)
# New tests, after the fix:  2 passed
uv run pytest tests/test_export.py::test_to_html_escapes_script_data_sequences_in_embedded_json tests/test_tree_html.py::test_emit_html_escapes_script_data_sequences_in_embedded_json -q

# Exporter test files: 87 passed
uv run pytest tests/test_export.py tests/test_tree_html.py tests/test_callflow_html.py -q

# Full suite: 6315 passed, 60 failed, 150 skipped
# The same 60 tests also fail on unmodified v8 in this environment
# (R, Solidity, VB.NET and Terraform extractors, skill refresh/uninstall, watch);
# none touch the HTML exporters.
uv run pytest tests/ -q

# All checks passed
uv run ruff check .

# On the changed files: 1 error, pre-existing on v8
# (tests/test_export.py:1008, outside this diff)
uv run pyright graphify/exporters/html.py graphify/tree_html.py tests/test_export.py tests/test_tree_html.py

Graphify-specific checklist

  • I updated generated skill artifacts (uv run python -m tools.skillgen --bless) when changing their source fragments. (N/A: no skill fragments changed.)
  • I confirmed that AST/structural extraction remains deterministic (no ambient state dependencies like ENV variables). (Extraction is untouched; only HTML serialization changes.)
  • I reviewed changes for security implications (no unsafe interpolation into shell/Python).
  • I confirmed no API keys or local-only graph data are included.
  • (If applicable) I disclosed AI authorship in my commit messages.

An unclosed "<!--" followed by "<script" in the embedded data put the HTML
tokenizer in the "script data double escaped" state, so the real </script>
no longer closed the element and graph.html rendered blank. Escaping only
"</" left both sequences intact; escape "<" as < instead, which JSON
and JS decode back to "<".

Applies to graphify/exporters/html.py and graphify/tree_html.py.
callflow_html.py embeds no data in its <script> and is unaffected.

Fixes Graphify-Labs#4124.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).


Graphify review — findings

Fixes the broken-page case from #4124 by escaping every < as \u003c in the JSON embedded into <script> blocks by to_html and emit_html. Previously only </ was escaped. A label containing an unclosed <!-- followed later by <script could still stop the real </script> from closing the data block. The data decodes back to the original strings, and new tests confirm no raw < reaches the embedded constants.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 524 functions depend on the 173 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _rebuild_code() — 149 callers, 56 callees
  • new: main() — 102 callers, 3 callees
  • new: to_html() — 25 callers, 11 callees
  • new: dispatch_command() — 2 callers, 127 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: _run_cli() — 6 callers, 7 callees
  • new: build_tree() — 6 callers, 6 callees
  • new: watch() — 5 callers, 7 callees
  • …and 2 more — each is listed as a finding

Verification — 524 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 194 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

5 of 329 test file(s) selected (2%) via static blast radius.

  • tests/test_export.py — impact, changed-test
  • tests/test_labeling.py — impact
  • tests/test_pipeline.py — impact
  • tests/test_tree_html.py — impact, changed-test
  • tests/test_watch.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

· 10 more finding(s) on lines outside this diff (see the check run).

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Thanks for the pull request, @tricode-online-admin. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: graph.html renders nothing when a label contains an unclosed <!-- followed by <script

2 participants