Repository navigation
fix(ui): validate URL schemes for Home footer links (#429) - #432
Conversation
… sinks (#429) Unit test drives the real roles.js helpers in a vm sandbox and pins, at source level, that home.js footer links and the customize-v2.js branding logo/favicon setters use the shared check. The browser test injects hostile customization via /api/config/theme and cs-theme-overrides against a local server and asserts on the protocols and attributes the browser computed. Both are red on this commit: no window.isSafeLinkUrl/safeUrlAttr exist yet, footer links and the logo/favicon accept javascript:/data:/vbscript:, an entity-encoded javascript: footer URL resolves to javascript:, and a legacy .brand-icon receives an unescaped <img src> that runs a handler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#429) roles.js: the scheme check #420 added for miniMarkdown is now one exported predicate, window.isSafeLinkUrl (same allowlist, same case and ASCII control/whitespace normalisation). window.safeUrlAttr wraps it for a raw URL headed for an HTML attribute and also escapes '&', so an entity-encoded scheme stays literal text. window.isSafeImageUrl is the image-sink form: the same check plus inline data:image/ URLs, which an <img> or icon cannot run. home.js: footerLinks[].url goes through safeUrlAttr; a rejected URL renders as its escaped label text. customize-v2.js: the branding logo and favicon setters (server theme, stored overrides/theme import, customizer input) and the logo preview use the image-sink check; a rejected logo counts as cleared. The three dead .brand-icon innerHTML writes, which interpolated logoUrl unescaped into <img src="...">, are removed: the rebrand dropped that element from index.html. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rapport — CS-pve-agent1 PR#432 #429 — head 4ca35bdStatus: Done. Every requirement is implemented, and each has a test that is red before and green after, plus at least one killed mutant. Every CI job on this head is green with no re-run, and the PR is left as a draft. Evidence tags: [T] = I ran it. [A] = from reading the code. [K] = from CI. Base:
Author and committer are Requirements
Red before (
Green after: unit 141 / 0, browser 19 / 0 [T][K]. Mutants were applied to the fixed files from a scratch snapshot. Both suites were re-run against a restarted local server, then each file was restored and verified with CI per job (head
|
| Job | Result |
|---|---|
| Go Build & Test | pass (17m53s). test-all.sh 242 passed / 0 failed over 242 files, incl. test-issue-429-config-url-schemes.js at 141/0. Preflight XSS --diff gate green [K] |
| Playwright E2E Tests | pass (28m13s). test-xss-429-home-footer-e2e.js reported 19 passed / 0 failed [K] |
| Validate Docker Image | pass [K] |
| Build & Publish Docker Image | pass [K] |
| Release Artifacts / Publish Badges & Summary / Deploy Staging | skipping (not applicable) [K] |
No job needed a re-run, and the known flaky test #271 did not appear [K].
Local verification
sh test-all.sh: 242 / 242 [T]node test-frontend-helpers.js: 713 / 0 [T]node test-preflight-xss-gate.js: all assertions passed [T]- Go:
go test ./...green in every module (cmd/server,cmd/ingestor,cmd/migrate,cmd/decrypt, allinternal/*) [T] npx eslint@8on the 3 changed files: 0 errors, and the same 11 warnings as master [T]- E2E against the local server [T]:
- security(ui): validate URL schemes for Home footer links (same source as #368) #429 19/0
- security(ui): escape and validate miniMarkdown link destinations #368 9/0
test-customize-branding-e2e.js10/10test-customize-theme-e2e.js8/8test-customize-export-e2e.js6/6test-home-coverage-e2e.js12/0test-xss-333-pages-e2e.js11/0test-logo-rebrand-e2e.js,test-logo-default-sage-teal-e2e.jsandtest-issue-304-my-mesh-e2e.jspasstest-e2e-playwright.js132/135 (3 skipped by the suite itself), exit 0
Constraints held [T]:
cmd/serveruntouched (0 files undercmd/).- No new
map[string]interface{}. - No colours added.
- Fork guards unchanged: 9 in
deploy.ymland 1 inrelease-fast-path.yml. - No closing keywords in the title, body or commits.
Decision taken during the work
On the first full run, test-customize-branding-e2e.js failed. It sets an inline data:image/svg+xml logo and expects it to apply. A plain http(s) allowlist on the image sinks would break that existing, tested behaviour and gain no security: an <img> or icon never runs script from its URL. So the image sinks use isSafeImageUrl, which is the same check plus data:image/… after the same normalisation. data:text/html and every other scheme are still rejected. The extra case is pinned by unit B6 and the E2E checks "inline data:image/ logo is still applied" and server-theme data:text/html, and mutant N9 shows the tests catch a looser version [T].
Remainders — left for the owner
mailto:footer links are now rejected, following the security(ui): escape and validate miniMarkdown link destinations #368 allowlist. Nothing in the defaults orconfig.example.jsonuses one. AddingmailtotoSAFE_LINK_SCHEMESwould be a product call, and it would also affect Markdown links [A].isValidHomeUrl(Feature Request: Allow custom "home" url Kpa-clawbot/CoreScope#1518) remains a separate, stricter validator forbranding.homeUrl. It accepts only absolute http(s) or#…. It is safe, and was left unchanged. Composing it onisSafeLinkUrlwould be a DRY follow-up [A].public/customize.js(v1) still contains the old unescaped.brand-iconand favicon writes. The file is not loaded byindex.html, so they are unreachable. Removing it is a separate cleanup [A].- Leaflet tile URL templates and
EXTERNAL_URLSwere checked and not changed. Tile URLs come only from operator server config (the customizer picks a registry id) and are fetched as tile images.EXTERNAL_URLShas no consumer that renders it [A]. - Protocol-relative
//hostfooter URLs are accepted, the same as for Markdown links in fix(ui): escape and validate miniMarkdown link destinations (#368) #420 [A]. - Not verified: browsers other than Chromium.
javascript:anchors were detected through the computedprotocol, not by activating them.scripts/collect-frontend-coverage.jswas not extended, and the instrumented coverage run was not executed locally.
This is a source-code finding, not an observed production exploit. No staging or production access was used, and no server API key. No merge, ready-for-review or issue closing was done.
Review — CS-MacBook PR#432 — head 4ca35bdDom: APPROVE with nits — every acceptance criterion in #429 has a red-before/green-after test, the fix reuses the #420 helper (no second allowlist), all CI jobs are green, and I reproduced the red→green transition and killed 5 mutants (3 unit, 2 E2E) myself against a local server. The nits below are non-blocking; the PR is a draft, so the merge call stays with the owner. Evidence tags: [T] I ran it · [A] read the code · [K] from CI. Base: Findings
Acceptance criteria → verification (one by one, against issue #429)
Red-before / green-after [T]
Mutants I ran myself (not just the author's) [T]Against the fixed tree; baseline unit 141/0, E2E 19/0.
All five were caught; after each the file was restored and Tests run (merged tree, local) [T]
CI per job (head
|
Relates to #429
Problem
#420 (#368) gave
miniMarkdown()link destinations a scheme allowlist. The same Home customization source had anotherhrefsink with no scheme check:footerLinks[].urlinpublic/home.js, rendered ashref="${escapeAttr(l.url)}". On master:javascript:,data:text/html,vbscript:and their case- and whitespace-obfuscated forms became live footer links.escapeAttrcovers only"and', so&stays raw.javascript:…,javascript:…andjavascript:…carry no scheme as text, but the browser decodes the entity while it parses the attribute. The anchor resolves tojavascript:.The grep the issue asked for found more customization-fed URL sinks in
public/customize-v2.js(branding logo and favicon). One of them executes:branding.logoUrlwas written unescaped into'<img src="' + logoUrl + '" …>'viainnerHTMLon any.brand-iconelement. A logo URL ofx" onerror="…runs the handler (verified in the browser test:injected handler fired 1 time(s)on master). The rebrand removed.brand-iconfromindex.html, so a stock page has no such element. The three writes were dead code, but they were still a live sink for any page that has one.branding.logoUrl→ navbar<img src>, the customizer preview<img src>andbranding.faviconUrl→<link rel=icon>.hrefhad no scheme check. These sinks are not executable in a browser, but the URL is importable content.Source-code finding only. No live-site change was made.
Plan / what changed
public/roles.js: one check, three entry points, no second allowlist.isSafeLinkUrl(url)is the predicate fix(ui): escape and validate miniMarkdown link destinations (#368) #420 wrote insidesafeLinkHref, extracted unchanged: the sameSAFE_LINK_SCHEMES(http,https), the same case and ASCII control/whitespace normalisation, and schemeless URLs still pass.miniMarkdownnow calls it throughsafeLinkHref, so its behaviour is identical (the security(ui): escape and validate miniMarkdown link destinations #368 suite stays 53/0).window.safeUrlAttr(url)is for a raw URL headed for a quoted attribute. It HTML-escapes& < >and then delegates tosafeLinkHref. The result is fully escaped (& < > " '), or''when rejected. Escaping&is what defeats the entity-obfuscated schemes.window.isSafeImageUrl(url)is the image-sink form:isSafeLinkUrlplusdata:image/…after the same normalisation. An<img>or icon never runs script from its URL, and the customizer has always accepted an inline logo (test-customize-branding-e2e.jspins exactly that).data:text/htmland every other scheme stay rejected.public/home.js: footer links are built byfooterLinkHtml(l)throughsafeUrlAttr(l.url). A rejected URL renders as its label, which_renderHomeLabelalready escapes.public/customize-v2.js_brandUrlOk(url)delegates towindow.isSafeImageUrland fails closed ifroles.jsis missing._setBrandLogoUrltreats a rejected URL as cleared._setFaviconUrlreplaces the three copy-pastedlink.href = …writes..brand-iconinnerHTMLwrites are removed rather than patched.Sink inventory (
href=/src=fed by customization or config)public/home.jsfooter<a href>SITE_CONFIG.home.footerLinks[].urlsafeUrlAttrpublic/roles.jsminiMarkdown<a href>public/customize-v2.js_setBrandLogoUrl→ navbar<img src>branding.logoUrl(theme, overrides, input)isSafeImageUrlpublic/customize-v2.js.brand-iconinnerHTML<img src>×3branding.logoUrlpublic/customize-v2.jsfaviconlink.href×3branding.faviconUrl_setFaviconUrlpublic/customize-v2.jslogo preview<img src>branding.logoUrlpublic/customize-v2.js_setBrandHomeUrl→.nav-brand[href]branding.homeUrlisValidHomeUrl(Kpa-clawbot#1518, stricter: absolute http(s) or#…only); unchangedpublic/customize-v2.jsfooter/branding<input value>escAttrmap.js,live.js,roles.js,map-tile-providers.js)<img>fetches, not importablewindow.EXTERNAL_URLSexternalUrlspublic/customize.js(v1)index.htmlAcceptance → tests → mutants
Unit:
test-issue-429-config-url-schemes.js(141 assertions, registered intest-all.sh). It drives the realroles.jshelpers in a vm sandbox:isSafeLinkUrlsafeUrlAttrminiMarkdownandsafeUrlAttragree on every URL, and the allowlist is declared onceisSafeImageUrlIt also pins the sinks at source level:
home.jsfootercustomize-v2.jssetters, preview, and noinnerHTMLlogo sinkBrowser:
test-xss-429-home-footer-e2e.js(19 checks, registered in the Playwright step ofdeploy.ymlafter the #368 test). It runs against a local Go server withe2e-fixture.db. Hostile customization is injected by rewriting/api/config/themewithpage.routeand by seedingcs-theme-overrides. The fixture DB is untouched and no payload leaves the machine. Every footer anchor is hovered and focused. The checks are DOM-level: handler and dialog counters, the protocol the browser computed for each anchor/img/link, and each anchor's attribute names.SAFE_LINK_SCHEMESdeclared once), B4 "home.js grows no scheme allowlist", B5_brandUrlOkdelegateshref/srcsinks get the same check.brand-icondata:image/logo applied, safe preview shown;test-customize-branding-e2e.js10/10Red-before:
public/roles.js,public/home.jsandpublic/customize-v2.jswere restored fromorigin/master(8f40525f) with the new tests in place. Unit 3 passed / 138 failed. Browser 9 passed / 10 failed, includinginjected handler fired 1 time(s)from the.brand-iconsink andxss429-ent-0 resolves to javascript:.With the fix: unit 141 / 0, browser 19 / 0.
Each mutant was applied to the fixed file from a scratch snapshot. Both suites were re-run against a restarted local server, then the file was restored and verified with
cmp.safeUrlAttrstops escaping&escapeAttr(l.url).brand-iconinnerHTMLsink restoreddata:URLVerification run locally
sh test-all.sh— 242 files, 242 passed, 0 failednode test-frontend-helpers.js— 713 passed, 0 failednode test-preflight-xss-gate.js— all assertions passedgo test ./...green in every module (cmd/server,cmd/ingestor,cmd/migrate,cmd/decrypt, allinternal/*)bash scripts/check-xss-sinks.sh --diff origin/master— clean (exit 0);--fileclean on all three changed filesnpx eslint@8on the three changed files — 0 errors, the same 11 warnings as on mastercorescope-serverwith a copy oftest-fixtures/e2e-fixture.dbprepared as in CI (freshen, Packets page collapse button in the left column of the table opens the dialog. Kpa-clawbot/CoreScope#1486/"Group Data" message type missing from packet view window's "message type" filter Kpa-clawbot/CoreScope#1791 seed,corescope-migrate, seeds 2073/199/245):test-xss-429-home-footer-e2e.js19/0test-xss-368-home-links-e2e.js9/0test-customize-branding-e2e.js10/10test-customize-theme-e2e.js8/8test-customize-export-e2e.js6/6test-home-coverage-e2e.js12/0test-xss-333-pages-e2e.js11/0test-logo-rebrand-e2e.js,test-logo-default-sage-teal-e2e.jsandtest-issue-304-my-mesh-e2e.jspasstest-e2e-playwright.js132/135 (3 skipped by the suite itself), exit 0Constraints held:
cmd/serveruntouched (no files undercmd/).map[string]interface{}.deploy.yml, 1 inrelease-fast-path.yml.Left for the owner
mailto:footer links are now rejected. Footer links follow the security(ui): escape and validate miniMarkdown link destinations #368 allowlist, which leavesmailto:out. No default or example config uses one; the defaults are#/…routes. If site owners are expected to put a contact address in the footer, addmailtotoSAFE_LINK_SCHEMES. That changes Markdown links too, and B1/B2 pin the exclusion.data:image/is allowed on the image sinks only (logo, favicon, preview). That keeps the existing inline-logo behaviour. Narrowing it to http(s) would breaktest-customize-branding-e2e.jsand any stored inline logo.isValidHomeUrl(Feature Request: Allow custom "home" url Kpa-clawbot/CoreScope#1518) stays a separate, stricter check forbranding.homeUrl. It rejects relative paths on purpose. Folding it ontoisSafeLinkUrlplus an "absolute or#" rule would be a small DRY follow-up, but it changes no security property, so it is not done here.public/customize.js(v1) still has the old unescaped.brand-iconand favicon writes. It is not loaded byindex.html, so they are unreachable. Deleting or fixing the file is a separate cleanup.//hostfooter URLs are accepted, as for Markdown links in fix(ui): escape and validate miniMarkdown link destinations (#368) #420. They carry no scheme and open cross-origin withtarget="_blank" rel="noopener".scripts/collect-frontend-coverage.jswas not extended for the new branches. The instrumented coverage run was not executed locally.🤖 Generated with Claude Code