diff --git a/scripts/validate-community-groups.mjs b/scripts/validate-community-groups.mjs index c9355f81..f3eed84a 100644 --- a/scripts/validate-community-groups.mjs +++ b/scripts/validate-community-groups.mjs @@ -7,6 +7,27 @@ const data = JSON.parse( ); const errors = []; +// The repository value is the upstream link for an End User Group and is one +// component change away from an . A bare `new URL()` parse accepts +// "javascript:alert(1)", "data:text/html,...", cleartext http, and a +// userinfo-spoofed authority such as "https://github.com@evil.example/x", +// whose visible prefix and real host disagree. Mirrors publishableUrl() in +// validate-case-studies.mjs and isCncfProjectHref() in +// lib/project-card-links.mjs. github.com is the only host +// check-community-group-links.mjs ever writes. +function publishableRepositoryUrl(value) { + if (typeof value !== 'string' || !value.trim()) return false; + let url; + try { + url = new URL(value.trim()); + } catch { + return false; + } + if (url.protocol !== 'https:') return false; + if (url.username || url.password) return false; + return url.hostname.toLowerCase() === 'github.com'; +} + if (Number.isNaN(Date.parse(data.checkedAt))) { collectError( errors, @@ -48,10 +69,13 @@ for (const group of Array.isArray(data.groups) ? data.groups : []) { collectError(errors, label, 'error', 'duplicate slug'); slugs.add(group.slug); } - try { - if (group.repository) new URL(group.repository); - } catch { - collectError(errors, label, 'error', 'repository must be an absolute URL'); + if (group.repository && !publishableRepositoryUrl(group.repository)) { + collectError( + errors, + label, + 'error', + `repository must be an https github.com URL, with no userinfo: ${JSON.stringify(group.repository)}`, + ); } // A group whose upstream repo is archived or unreachable still renders on // the docs page today; surface it loudly but do not fail the build over an diff --git a/tests/validate-community-groups.test.mjs b/tests/validate-community-groups.test.mjs index 8be2154a..2a05e626 100644 --- a/tests/validate-community-groups.test.mjs +++ b/tests/validate-community-groups.test.mjs @@ -125,7 +125,7 @@ test('labels a group with neither slug nor name as "unknown group"', () => { }); // A group missing `repository` must not also be reported as a bad URL: the -// `if (group.repository)` guard inside the try is what keeps the two errors +// `group.repository &&` guard on the URL check is what keeps the two errors // from stacking on one record. test('reports a repository-less group once, not also as a bad URL', () => { const group = validGroup(); @@ -136,7 +136,7 @@ test('reports a repository-less group once, not also as a bad URL', () => { ); assert.equal(result.status, 1); assert.match(result.stderr, /group requires slug, name, and repository/); - assert.doesNotMatch(result.stderr, /absolute URL/); + assert.doesNotMatch(result.stderr, /no userinfo/); }); test('rejects a truthy non-array groups without crashing', () => { @@ -167,7 +167,67 @@ test('rejects a non-URL repository', () => { }), ); assert.equal(result.status, 1); - assert.match(result.stderr, /absolute URL/); + assert.match(result.stderr, /https github\.com URL/); +}); + +// A bare `new URL()` parse accepts every value below. The repository link is +// published data one component change away from an , so the gate has +// to decide the destination by parsing rather than by parseability alone. +for (const [description, repository] of [ + ['a javascript: scheme', 'javascript:alert(1)'], + ['a data: URL', 'data:text/html,'], + ['a cleartext http URL', 'http://github.com/cncf/research-user-group'], + [ + 'a userinfo-spoofed authority', + 'https://github.com@evil.example/cncf/research-user-group', + ], + ['a host that merely ends in the allowed name', 'https://notgithub.com/x'], + ['a subdomain of the allowed host', 'https://raw.github.com/cncf/x'], + ['a whitespace-only repository', ' '], +]) { + test(`rejects ${description}`, () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture({ ...validData, groups: [validGroup({ repository })] }), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /https github\.com URL/); + }); +} + +// Trailing whitespace is a copy-paste artefact, not a different destination: +// the gate trims before parsing so a valid link is not rejected over it. +test('accepts a repository with surrounding whitespace', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture({ + ...validData, + groups: [ + validGroup({ + repository: ' https://github.com/cncf/research-user-group ', + }), + ], + }), + ); + assert.equal(result.status, 0, result.stderr); +}); + +// `hostname` is already lowercased by the URL parser, but the explicit +// toLowerCase() in the gate is what keeps that true if the comparison ever +// moves to a raw host string. +test('accepts an uppercase host', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture({ + ...validData, + groups: [ + validGroup({ + repository: 'https://GitHub.com/cncf/research-user-group', + }), + ], + }), + ); + assert.equal(result.status, 0, result.stderr); }); test('warns, without failing, on an archived upstream repo', () => {