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 <noreply@anthropic.com>
This commit is contained in:
+25
-4
@@ -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
|
||||
|
||||
+22
-1
@@ -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" },
|
||||
|
||||
Reference in New Issue
Block a user