fix(security): close M-1 — DOM construction instead of innerHTML in _renderTestResults#52
Merged
Merged
Conversation
Static-source guard that pins _renderTestResults' use of safe DOM
construction (textContent / createTextNode / replaceChildren) and
rejects re-introduction of innerHTML assignment inside that function.
Also flags any new innerHTML site in sync-dashboard.mjs outside the
existing static-spinner allowlist.
Same shape as test-sync-calendar-discoverability.mjs and
test-no-instance-hostname.mjs. Pair commit follows with the actual
_renderTestResults refactor.
Cites: 2026-05-21-core-tenets §T-B1
reports/foundry/2026-05-22-fm-security-audit.md §2 M-1, §4 Chunk 1, §0.5 D1=(c)
`_renderTestResults` rendered Chronicle-side strings (system names,
error messages echoing Chronicle response data) via innerHTML
interpolation. If Chronicle returned a malicious string (e.g.,
`<img src=x onerror=alert(1)>` as `match.name`), the dashboard
would execute the payload as DOM.
Fix: replaceChildren + createElement + createTextNode. Icons use a
static class map (no interpolation). Chronicle strings rendered as
text nodes — never parsed as DOM.
Paired with tools/test-sync-dashboard-xss.mjs (committed previously
on this branch) — static-source guard against re-introduction of
innerHTML in _renderTestResults plus any new innerHTML site outside
the static-spinner allowlist.
Test suite: 250 baseline + 4 new = 254/254 pass.
Cites: 2026-05-21-core-tenets §T-B1
reports/foundry/2026-05-22-fm-security-audit.md §2 M-1, §4 Chunk 1, §0.5 D1=(c)
9 tasks
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.
Cites: 2026-05-21-core-tenets §T-B1; reports/foundry/2026-05-22-fm-security-audit.md §2 M-1, §4 Chunk 1, §0.5 D1=(c); dispatches/foundry/FM-SEC-CHUNK-1.md
Security implication: closes M-1 (latent XSS via
innerHTMLinterpolation of Chronicle-side strings). Without this fix, a compromised Chronicle returning<img src=x onerror=alert(1)>asmatch.namewould execute JavaScript in the dashboard's DOM context.Consumer-verified: Chronicle returns entity/system names that get rendered in the test-connection result; verified Chronicle-side that these are operator-controllable per
reports/foundry/2026-05-22-fm-security-audit.md§1.3.Foundry compatibility: static-source code change only; renders identically across v12/v13/v14 (DOM APIs
replaceChildren/createElement/createTextNodeare ES standard, all Foundry-supported versions).Mockup: n/a — internal admin surface; visually identical after fix.
What this changes
_renderTestResultsinscripts/sync-dashboard.mjsnow constructs the test-connection diagnostic viareplaceChildren+createElement+createTextNodeinstead ofinnerHTML = ...interpolation. Chronicle-side strings render as text nodes, never as DOM. Static-source regression guard added attools/test-sync-dashboard-xss.mjs.Why
The Foundry-side security audit (
reports/foundry/2026-05-22-fm-security-audit.md§2 M-1) flagged this site as a latent XSS surface. The audit's §0.5 D1=(c) resolution chose targeted fixes for the three medium findings; this is the first of those. Closes the XSS vector even in a trust-Chronicle-by-default deployment model.Test plan
node --test tools/test-*.mjspasses locally (250 baseline + 4 new = 254/254)node tools/check-package-descriptor.mjspasses (descriptor unchanged but ran for hygiene)sync-dashboard.mjs:1641remains (a constant<i class="fa-solid fa-spinner">literal with no interpolation — verified safe)innerHTML = ...inside_renderTestResultswould fail the new test, as would adding any new innerHTML site tosync-dashboard.mjsoutside the allowlistTenet self-check
Stop-and-flag
None encountered. Two surfaces audited per §2 M-1:
sync-dashboard.mjs:1641— static spinner; verified safe (no interpolation). Allowed by the regression guard.sync-dashboard.mjs:1747— the M-1 site; fixed.Note on commit history
Two commits instead of one due to the chronicle-foundry-module-specific signing infra failure (HTTP 400 "missing source"; documented across PR #49, #50, #51 status reports + tracked in BACKLOG). Workaround: MCP
push_files, which serializes large file contents per call. The test file landed first (small), then the modified sync-dashboard.mjs (large).Squash-on-merge will collapse cleanly if desired.
🤖 Cites
dispatches/foundry/FM-SEC-CHUNK-1.md. Generated by Claude Code.Generated by Claude Code