From b97c82d92e2417a92d1802dc3245d4f71d9005ac Mon Sep 17 00:00:00 2001 From: paul Date: Wed, 17 Jun 2026 18:33:59 +0000 Subject: [PATCH] fix(security): validate MB url-rel links by scheme + host, not substring (task/harden-resolver-link-validation) relUrls() picked MusicBrainz link relations by substring (u.includes("bandcamp.com")) with no scheme check, so a non-http(s) resource or a look-alike host (https://evilbandcamp.com, javascript:...bandcamp.com...) could be returned and become an injected link's href. Now: isHttpUrl() accepts http/https only, and matching is by PARSED hostname (=== host || endsWith "."+host). Resolver-only; extension unaffected; pure + unit-tested (18 total green). Source: plan/steward-linker-security SCOUT 2026-06-17. Co-Authored-By: Claude Opus 4.8 --- service/lib.js | 29 +++++++++++++++++++++++++---- service/lib.test.js | 23 ++++++++++++++++++++++- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/service/lib.js b/service/lib.js index a36369f..4c3215f 100644 --- a/service/lib.js +++ b/service/lib.js @@ -40,12 +40,33 @@ export function spotifyResult(hit) { return { ...r, primary: primaryOf(r) }; } +// True for http/https URLs only — reject javascript:, ftp:, garbage. These values flow +// into an injected link's href, so validate the third-party MB data defensively. +export function isHttpUrl(u) { + try { + const p = new URL(u).protocol; + return p === "https:" || p === "http:"; + } catch { + return false; + } +} + // Extract direct links from a MusicBrainz artist's url-relations (zero-cred: MB stores -// official Bandcamp / streaming URLs). Either field may be null. +// official Bandcamp / streaming URLs). Accepts only http(s) and matches by parsed hostname +// (not substring, so "evilbandcamp.com" / "javascript:…bandcamp.com…" don't slip through). +// Either field may be null. export function relUrls(relations) { - const urls = (relations ?? []).map((r) => r && r.url && r.url.resource).filter(Boolean); - const find = (host) => urls.find((u) => u.includes(host)) ?? null; - return { bandcamp: find("bandcamp.com"), spotify: find("open.spotify.com") }; + const urls = (relations ?? []).map((r) => r && r.url && r.url.resource).filter(isHttpUrl); + const byHost = (host) => + urls.find((u) => { + try { + const h = new URL(u).hostname; + return h === host || h.endsWith("." + host); + } catch { + return false; + } + }) ?? null; + return { bandcamp: byHost("bandcamp.com"), spotify: byHost("open.spotify.com") }; } // Link object for a confirmed MusicBrainz hit: prefer the direct url-rel links, fall back diff --git a/service/lib.test.js b/service/lib.test.js index 4efc247..12a848c 100644 --- a/service/lib.test.js +++ b/service/lib.test.js @@ -1,7 +1,7 @@ // Unit tests for the resolver's pure logic (run with `node --test`). Zero-dep. import { test } from "node:test"; import assert from "node:assert/strict"; -import { norm, bandcampSearch, spotifySearch, pickArtist, spotifyResult, relUrls, mbResult, primaryOf, isSearchUrl } from "./lib.js"; +import { norm, bandcampSearch, spotifySearch, pickArtist, spotifyResult, relUrls, mbResult, primaryOf, isSearchUrl, isHttpUrl } from "./lib.js"; test("norm lowercases, trims, NFKC-normalizes", () => { assert.equal(norm(" Cerebral Bore "), "cerebral bore"); @@ -77,6 +77,27 @@ test("relUrls returns nulls when relations absent/empty", () => { assert.deepEqual(relUrls([]), { bandcamp: null, spotify: null }); }); +test("relUrls rejects non-http(s) and look-alike hosts (hardening)", () => { + const relations = [ + { url: { resource: "javascript:alert('bandcamp.com')" } }, // not http(s) + { url: { resource: "ftp://files.bandcamp.com/x" } }, // not http(s) + { url: { resource: "https://evilbandcamp.com/" } }, // host doesn't end in .bandcamp.com + { url: { resource: "https://x.bandcamp.com/" } }, // valid subdomain + ]; + const links = relUrls(relations); + assert.equal(links.bandcamp, "https://x.bandcamp.com/"); + assert.equal(links.spotify, null); +}); + +test("isHttpUrl accepts http/https only", () => { + assert.equal(isHttpUrl("https://x.com"), true); + assert.equal(isHttpUrl("http://x.com"), true); + assert.equal(isHttpUrl("javascript:alert(1)"), false); + assert.equal(isHttpUrl("ftp://x/y"), false); + assert.equal(isHttpUrl("not a url"), false); + assert.equal(isHttpUrl(null), false); +}); + test("mbResult prefers direct links, falls back to search per-platform", () => { const direct = mbResult( { name: "Cattle Decapitation" },