fix: stop rendering anything the gateway or the chain sends as markup - #32
Merged
Merged
Conversation
Every value these pages display arrives from somewhere else — a CID and a kid from the gateway, a transaction hash and an exporter's name from the chain, a refusal code out of a JSON body — and every one of them went straight into innerHTML next to an icon or inside an href. The content security policy stops injected markup from running script, so this is not remote code execution. It is worse-placed than that: these pages hold a passphrase field, and a service able to draw its own passphrase field does not need script. js/core/html.js is a tagged template that escapes every interpolation. The literal parts stay markup because a maintainer wrote them; a fragment passes through only by being built with the same tag, which is visible at the call site. Every sink in js/App.js and js/chunked-app.js now goes through it. Two tests hold the line. One scans the client and fails on any assignment to innerHTML that is not a tagged template, a toHtml() call, or a literal with nothing interpolated into it — so the next sink someone writes fails the build rather than the review. The other puts an <img id="pwned"> into the one field the gateway is simply believed about and asserts no such element exists afterwards; reverting the escaping on that one line makes it fail.
Count-only conflicts in Readme.md and index.html, resolved per hunk and regenerated with `npm run test-count`. 517 = master's 505 plus this branch's 12. Duplicate-declaration scan clean, every module parses, browser suite 14/14 against the gateway master now builds.
Count-only conflicts in Readme.md and index.html again, resolved per hunk and regenerated. 545 = master's 533 plus this branch's 12. Duplicate-declaration scan clean, every module parses, browser suite 14/14.
Count-only conflicts in Readme.md and index.html, resolved per hunk and regenerated. 549 = master's 537 plus this branch's 12. #33 changes what the gateway signs into a receipt, so the browser suite was re-run against it rather than trusted: 14/14.
nishchal-gond
added a commit
that referenced
this pull request
Sep 21, 2026
Count lines only: #32 touches js/ and this branch touches server/ and scripts/, so the two generated totals in Readme.md and index.html were the only conflicts. Resolved per hunk and regenerated to 554 = 549 + 5; the diff against master is unchanged at 14 files, so nothing came through this merge that was not already here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by LPH
Before: every value the pages displayed went straight into
innerHTML. A CID and akidfrom the gateway, a transaction hash and an exporter's name from the chain, a refusalcodeout of a JSON body — all of them interpolated into a template next to an icon, several of them inside anhref="…". Whoever chose the string chose markup for the page.After: nothing that arrived from somewhere else can become markup.
js/core/html.jsis a tagged template that escapes every interpolation; the literal parts stay markup because a maintainer wrote them. Every sink injs/App.jsandjs/chunked-app.jsgoes through it, and a fragment can pass through unescaped only by being built with the same tag, which is visible at the call site.This is not remote code execution. The content security policy the gateway serves —
script-src 'self' https:, no'unsafe-inline'— refuses anonerror=attribute, so injected script does not run. It is worse-placed than that: these pages hold a passphrase field, and a service able to draw its own passphrase field does not need script. The escaping is the defence and the policy sits behind it, not the other way round.How.
html\…`escapes& < > " 'in every interpolation and returns an opaque value;toHtml()is what reachesinnerHTMLand escapes anything that is not one of those, so a call site that forgets the tag renders its value as visible text rather than as a hole.safe()marks the handful of static fragments this code wrote itself, andjoinHtml()` is how a list of cards is concatenated without becoming an ordinary string on the way.Two tests hold the line rather than the current call sites:
js/that fails on any assignment toinnerHTMLwhose right-hand side is not a tagged template, atoHtml()call, or a literal with nothing interpolated into it — so the next sink somebody writes fails the build rather than the review;<img id="pwned">intoonChain.txHash, the one rendered field no proof covers, then asserts no such element exists in the document. Reverting the escaping on that single line makes it fail, which was checked rather than assumed.docs/SECURITY.mdgains §2.10 and a row in the defended table. Counts regenerated: 517 unit tests (505 from master plus 12), browser suite 14/14, local Node matrix 20/21/22 green.Generated by Claude Code