Skip to content

Render source_url as a link in the HTML viewer - #3250

Open
tscomitre wants to merge 1 commit into
Graphify-Labs:v8from
tscomitre:feat/viewer-linkify-source-url
Open

Render source_url as a link in the HTML viewer#3250
tscomitre wants to merge 1 commit into
Graphify-Labs:v8from
tscomitre:feat/viewer-linkify-source-url

Conversation

@tscomitre

Copy link
Copy Markdown

Problem

Nodes can carry a source_url — the ingest paths set it for URL-sourced content — but the HTML viewer never renders it. A node whose entire point is "this came from over there" gives the reader no way to get there. The field is reachable only by opening graph.json and reading it by hand.

I hit this with a graph where a cluster of concept nodes each pointed at a documentation page. The viewer showed the nodes and their relationships correctly, but the URLs were invisible.

Change

The node detail panel now shows a Link: row when a node has a source_url, alongside the existing Source: row:

Type: concept
Community: Product Docs
Source: docs/index.md
Link: https://example.com/wiki/page
Degree: 7

Three small edits in graphify/exporters/html.py: emit source_url in the vis.js node payload, carry it through the DataSet mapping, and render it in the panel. Nodes without a source_url are unchanged — no extra row.

Security

source_url is ingested content, so it is not trustworthy.

The existing esc() helper is HTML-escaping only. That is correct for text, but not sufficient for an href: javascript:alert(1) contains no HTML metacharacters, so it passes through esc() untouched and would become a live link.

This adds a safeUrl() scheme allowlist:

  • only http: and https: are linkified; anything else renders nothing at all
  • no base URL is passed to URL(), so relative values are rejected too and a source_url can never resolve against the viewer's own origin
  • anchors carry rel="noopener noreferrer" and target="_blank"

This follows the direction the neighbour links already went with data-nid.

Tests

Four added to tests/test_export.py:

  • source_url reaches the node payload
  • absent source_url still renders (empty payload value)
  • the scheme allowlist ships, and the anchor is built through safeUrl rather than from the raw value
  • a javascript: source_url never reaches the document as an href

Full export suites pass: 144 passed across test_export.py, test_cli_export.py and test_export_control_characters.py.

Happy to adjust the panel wording or styling if you'd prefer something different.

Nodes can carry a `source_url` — the ingest paths set it for URL-sourced
content — but the HTML viewer never rendered it. A node whose whole point
is "this came from over there" gave the reader no way to get there; the
field was reachable only by reading graph.json directly.

The node detail panel now shows a `Link:` row when a node has a
source_url, alongside the existing `Source:` row.

Security: source_url is ingested content and is not trustworthy. The
existing esc() helper is HTML-escaping only, which is fine for text but
not sufficient for an href — `javascript:alert(1)` contains no HTML
metacharacters and would survive esc() intact as a live link. This adds a
safeUrl() scheme allowlist: only http: and https: are linkified,
everything else renders nothing. No base is passed to URL(), so relative
values are rejected too and a source_url can never resolve against the
viewer's own origin. Anchors carry rel="noopener noreferrer".

Four tests added covering the payload field, the absent-value case, the
presence of the scheme allowlist, and that a javascript: source_url never
reaches the document as an href.

@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.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

Formal verification. 2 change(s) alter behavior, breaking input(s) attached.

Behavior changes: \_html\_script changes behavior, here is the input that shows it.

The verifier found a concrete input on which \_html\_script behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"nodes\_json":"''","edges\_json":"''","legend\_json":"'\\\\x00'"\}, the old code produced '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return… but the new code produces '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return…. Paste that input straight into a regression test.

Behavior changes: \_html\_styles changes behavior, here is the input that shows it.

The verifier found a concrete input on which \_html\_styles behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This is a sound refutation from a mathematical proof (SMT solver, over a bounded input domain): a concrete input on which the two versions provably differ.


Graphify review — findings

Adds a clickable source link to the HTML export's node info panel, rendering source_url as an anchor only when safeUrl confirms an http/https scheme via URL parsing — anything else (including javascript: and relative URLs) collapses to no link, so it's shown as escaped text but never as a live href. Threads source_url from node data through to_html into the JS payload and vis dataset. Covers the payload plumbing, the scheme allowlist, and the refusal to emit hostile hrefs in tests.

Worth a look

  • source_url embedded in JSON payload without JS-string escaping can break out of script contextgraphify/exporters/html.py:542 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 430 functions depend on the 136 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _rebuild_code() — 113 callers, 50 callees
  • new: dispatch_command() — 2 callers, 123 callees
  • new: to_html() — 18 callers, 11 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: watch() — 5 callers, 7 callees
  • new: _reconcile_graph_html() — 6 callers, 5 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees

Verification — 430 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: 157 function(s) in the blast radius were not formally verified this run

Formal verification

Behavior changes: \_html\_script changes behavior, here is the input that shows it.

The verifier found a concrete input on which \_html\_script behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"nodes\_json":"''","edges\_json":"''","legend\_json":"'\\\\x00'"\}, the old code produced '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return… but the new code produces '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return…. Paste that input straight into a regression test.

Behavior changes: \_html\_styles changes behavior, here is the input that shows it.

The verifier found a concrete input on which \_html\_styles behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This is a sound refutation from a mathematical proof (SMT solver, over a bounded input domain): a concrete input on which the two versions provably differ.

No difference found (not proven): No behavior difference found in to\_html (not a proof).

The verifier ran both versions of to\_html on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

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

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.

1 participant