fix(security): client href scheme check, outbound fetch timeouts, origin-scoped CORS (closes task/resolver-security-hardening)
Source: task/resolver-security-hardening (plan/steward-linker-security) — defense-in-depth: client trusted resolver href, outbound fetches had no timeout, CORS was blanket-*. - F1: extension/content.js makeLink re-validates primary.url scheme via a new pure Extract.isSafeHttpUrl helper (in extract.js, unit-tested); a non-http(s) url (javascript:/data:/garbage) is dropped and the plain text is kept. - F2: each of the three resolver outbound fetches (Spotify token, Spotify search, ollama /api/chat) now carries signal: AbortSignal.timeout(5000) so a hung upstream fails fast (caught per-candidate -> null), zero-dep. - F3: resolver CORS reflects the request Origin only for moz-extension://* or null (the extension's background-script origin), with Vary: Origin, instead of blanket access-control-allow-origin: *. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -56,6 +56,10 @@
|
|||||||
const platform = primary.platform;
|
const platform = primary.platform;
|
||||||
const kind = links.kind === "album" ? "album" : "artist";
|
const kind = links.kind === "album" ? "album" : "artist";
|
||||||
const noun = kind === "album" ? "album" : "artist";
|
const noun = kind === "album" ? "album" : "artist";
|
||||||
|
// Re-validate the href scheme client-side: only http(s) becomes a clickable link.
|
||||||
|
// A swapped/compromised/buggy resolver could send a javascript:/data: url; never
|
||||||
|
// inject that into reddit's DOM. Returning null leaves the original text in place.
|
||||||
|
if (!Extract.isSafeHttpUrl(primary.url)) return null;
|
||||||
const a = document.createElement("a");
|
const a = document.createElement("a");
|
||||||
a.className = `rsl-link rsl-${platform} rsl-${kind}`;
|
a.className = `rsl-link rsl-${platform} rsl-${kind}`;
|
||||||
a.textContent = label;
|
a.textContent = label;
|
||||||
@@ -80,7 +84,10 @@
|
|||||||
for (const m of ms) {
|
for (const m of ms) {
|
||||||
if (m.index < last) continue; // skip overlaps
|
if (m.index < last) continue; // skip overlaps
|
||||||
if (m.index > last) frag.appendChild(document.createTextNode(text.slice(last, m.index)));
|
if (m.index > last) frag.appendChild(document.createTextNode(text.slice(last, m.index)));
|
||||||
frag.appendChild(makeLink(text.substr(m.index, m.length), resolved.get(m.name)));
|
const matchText = text.substr(m.index, m.length);
|
||||||
|
const link = makeLink(matchText, resolved.get(m.name));
|
||||||
|
// makeLink returns null for an unsafe (non-http(s)) href: keep the plain text.
|
||||||
|
frag.appendChild(link || document.createTextNode(matchText));
|
||||||
last = m.index + m.length;
|
last = m.index + m.length;
|
||||||
}
|
}
|
||||||
if (last < text.length) frag.appendChild(document.createTextNode(text.slice(last)));
|
if (last < text.length) frag.appendChild(document.createTextNode(text.slice(last)));
|
||||||
|
|||||||
+15
-1
@@ -119,5 +119,19 @@
|
|||||||
return all;
|
return all;
|
||||||
}
|
}
|
||||||
|
|
||||||
return { matchesIn, cueMatchesIn, albumMatchesIn, allMatchesIn };
|
// True only for http/https URLs — reject javascript:, data:, garbage, empty.
|
||||||
|
// The resolver's url flows into an injected link's href, so the content script
|
||||||
|
// re-validates the scheme client-side before assigning it (defense in depth; the
|
||||||
|
// resolver already validates server-side, but a swapped/buggy one must not inject
|
||||||
|
// a javascript:/data: href into reddit's DOM).
|
||||||
|
function isSafeHttpUrl(u) {
|
||||||
|
try {
|
||||||
|
const p = new URL(u).protocol;
|
||||||
|
return p === "http:" || p === "https:";
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return { matchesIn, cueMatchesIn, albumMatchesIn, allMatchesIn, isSafeHttpUrl };
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
// CJS so it can require() extract.js, which is a UMD script (also a content script).
|
// CJS so it can require() extract.js, which is a UMD script (also a content script).
|
||||||
const { test } = require("node:test");
|
const { test } = require("node:test");
|
||||||
const assert = require("node:assert/strict");
|
const assert = require("node:assert/strict");
|
||||||
const { matchesIn, cueMatchesIn, albumMatchesIn, allMatchesIn } = require("./extract.js");
|
const { matchesIn, cueMatchesIn, albumMatchesIn, allMatchesIn, isSafeHttpUrl } = require("./extract.js");
|
||||||
|
|
||||||
test("matchesIn finds Title-Case / ALL-CAPS runs, skips stopwords", () => {
|
test("matchesIn finds Title-Case / ALL-CAPS runs, skips stopwords", () => {
|
||||||
// Note: "and" is a connector, so "X and Y" greedily joins into one run (by design,
|
// Note: "and" is a connector, so "X and Y" greedily joins into one run (by design,
|
||||||
@@ -64,6 +64,21 @@ test("albumMatchesIn skips pure-number and too-short quotes", () => {
|
|||||||
assert.ok(!names.includes("X")); // length < 2
|
assert.ok(!names.includes("X")); // length < 2
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("isSafeHttpUrl accepts http/https and rejects javascript/data/garbage/empty", () => {
|
||||||
|
// accepted: the only schemes a resolver link may inject as an href
|
||||||
|
assert.ok(isSafeHttpUrl("https://open.spotify.com/artist/abc"));
|
||||||
|
assert.ok(isSafeHttpUrl("http://localhost:8787/x"));
|
||||||
|
assert.ok(isSafeHttpUrl("https://www.google.com/search?q=Scum+bandcamp"));
|
||||||
|
// rejected: dangerous schemes + non-URLs
|
||||||
|
assert.ok(!isSafeHttpUrl("javascript:alert(1)"));
|
||||||
|
assert.ok(!isSafeHttpUrl("data:text/html,<script>alert(1)</script>"));
|
||||||
|
assert.ok(!isSafeHttpUrl("ftp://example.com/x"));
|
||||||
|
assert.ok(!isSafeHttpUrl("not a url"));
|
||||||
|
assert.ok(!isSafeHttpUrl(""));
|
||||||
|
assert.ok(!isSafeHttpUrl(null));
|
||||||
|
assert.ok(!isSafeHttpUrl(undefined));
|
||||||
|
});
|
||||||
|
|
||||||
test("allMatchesIn merges cue + Title-Case + album and sorts ascending by index", () => {
|
test("allMatchesIn merges cue + Title-Case + album and sorts ascending by index", () => {
|
||||||
const text = `Check out devourment and Cattle Decapitation; the album "Human Jerky" rules`;
|
const text = `Check out devourment and Cattle Decapitation; the album "Human Jerky" rules`;
|
||||||
const ms = allMatchesIn(text);
|
const ms = allMatchesIn(text);
|
||||||
|
|||||||
+16
-2
@@ -51,6 +51,7 @@ async function spotifyToken() {
|
|||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { authorization: `Basic ${auth}`, "content-type": "application/x-www-form-urlencoded" },
|
headers: { authorization: `Basic ${auth}`, "content-type": "application/x-www-form-urlencoded" },
|
||||||
body: "grant_type=client_credentials",
|
body: "grant_type=client_credentials",
|
||||||
|
signal: AbortSignal.timeout(5000), // fail fast on a hung upstream (caught per-candidate -> null)
|
||||||
});
|
});
|
||||||
if (!r.ok) throw new Error(`spotify token http ${r.status}`);
|
if (!r.ok) throw new Error(`spotify token http ${r.status}`);
|
||||||
const j = await r.json();
|
const j = await r.json();
|
||||||
@@ -67,7 +68,10 @@ async function search(name) {
|
|||||||
if (!USE_SPOTIFY) return { artist: null, album: null };
|
if (!USE_SPOTIFY) return { artist: null, album: null };
|
||||||
const token = await spotifyToken();
|
const token = await spotifyToken();
|
||||||
const url = `https://api.spotify.com/v1/search?type=artist,album&limit=5&q=${encodeURIComponent(name)}`;
|
const url = `https://api.spotify.com/v1/search?type=artist,album&limit=5&q=${encodeURIComponent(name)}`;
|
||||||
const r = await fetch(url, { headers: { authorization: `Bearer ${token}` } });
|
const r = await fetch(url, {
|
||||||
|
headers: { authorization: `Bearer ${token}` },
|
||||||
|
signal: AbortSignal.timeout(5000), // fail fast on a hung upstream (caught per-candidate -> null)
|
||||||
|
});
|
||||||
if (!r.ok) throw new Error(`spotify search http ${r.status}`);
|
if (!r.ok) throw new Error(`spotify search http ${r.status}`);
|
||||||
const j = await r.json();
|
const j = await r.json();
|
||||||
return {
|
return {
|
||||||
@@ -88,6 +92,7 @@ async function ollamaClassify(candidates) {
|
|||||||
format: "json",
|
format: "json",
|
||||||
options: { temperature: 0 },
|
options: { temperature: 0 },
|
||||||
}),
|
}),
|
||||||
|
signal: AbortSignal.timeout(5000), // fail fast on a hung upstream (caught per-candidate -> null)
|
||||||
});
|
});
|
||||||
if (!r.ok) throw new Error(`ollama http ${r.status}`);
|
if (!r.ok) throw new Error(`ollama http ${r.status}`);
|
||||||
const j = await r.json();
|
const j = await r.json();
|
||||||
@@ -119,8 +124,17 @@ const json = (res, code, obj) => {
|
|||||||
res.end(JSON.stringify(obj));
|
res.end(JSON.stringify(obj));
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// CORS scoped to the extension origin: the background script sends
|
||||||
|
// `Origin: moz-extension://<uuid>`; a page-context fetch (file://, sandboxed) can send
|
||||||
|
// `Origin: null`. Reflect only those — never blanket `*` — so an arbitrary website the
|
||||||
|
// user visits can't POST to localhost:8787 and read resolver results.
|
||||||
|
const allowedOrigin = (origin) =>
|
||||||
|
typeof origin === "string" && (origin.startsWith("moz-extension://") || origin === "null");
|
||||||
|
|
||||||
const server = createServer(async (req, res) => {
|
const server = createServer(async (req, res) => {
|
||||||
res.setHeader("access-control-allow-origin", "*");
|
const origin = req.headers.origin;
|
||||||
|
if (allowedOrigin(origin)) res.setHeader("access-control-allow-origin", origin);
|
||||||
|
res.setHeader("vary", "origin"); // the allow-origin header varies by request Origin
|
||||||
res.setHeader("access-control-allow-headers", "content-type");
|
res.setHeader("access-control-allow-headers", "content-type");
|
||||||
res.setHeader("access-control-allow-methods", "GET, POST, OPTIONS");
|
res.setHeader("access-control-allow-methods", "GET, POST, OPTIONS");
|
||||||
if (req.method === "OPTIONS") { res.writeHead(204); return res.end(); }
|
if (req.method === "OPTIONS") { res.writeHead(204); return res.end(); }
|
||||||
|
|||||||
Reference in New Issue
Block a user