From 74ce72541387d2aa63a4677e21f3e9f3dd0851ff Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 19:50:51 -0400 Subject: [PATCH] fix(security): validate every generated community-people section, not just rostered ones scripts/validate-community-people.mjs drove its person-level loop from data/community-roster.json's section keys, while src/components/CommunityPeople renders data/community-people.json's. A section present only in the generated file therefore skipped every check in the loop body, including the profileImageUrl() host gate, and reached with an arbitrary host. The loop now runs over the union of both files' section keys, and a generated section the roster does not declare is an error in its own right. Signed-off-by: hivecommons-hive[bot] --- scripts/validate-community-people.mjs | 57 ++++++++++++---- tests/validate-community-people.test.mjs | 86 +++++++++++++++++++++++- 2 files changed, 127 insertions(+), 16 deletions(-) diff --git a/scripts/validate-community-people.mjs b/scripts/validate-community-people.mjs index 78b4d55c..a7d5c3c2 100644 --- a/scripts/validate-community-people.mjs +++ b/scripts/validate-community-people.mjs @@ -37,23 +37,52 @@ if (!ISO_8601.test(data.fetchedAt ?? '')) { // back to name for the one member without one), so a stale or // partially-generated file with the right shape but a missing/extra person // fails loudly instead of only checking that the arrays are non-empty. -for (const [section, rosterEntries] of Object.entries(roster.sections || {})) { - const generated = data.people?.[section] || []; - const key = (person) => person.github || person.name; - const rosterKeys = new Set(rosterEntries.map(key)); - const generatedKeys = new Set(generated.map(key)); - for (const entry of rosterEntries) { - if (!generatedKeys.has(key(entry))) { - collectError( - errors, - `people.${section}`, - 'error', - `missing roster member ${entry.name}`, - ); +// +// The loop runs over the union of both files' section keys, not the roster's +// alone. Every section in the generated file is rendered by +// , so driving the loop from the roster let a +// section present only in community-people.json skip every check below -- +// including the profileImageUrl() host gate, which is the last thing standing +// between an unattended upstream refresh and an arbitrary third-party host in +// an served to every visitor. +const rosterSections = roster.sections || {}; +const generatedSections = data.people || {}; +const key = (person) => person.github || person.name; + +for (const section of new Set([ + ...Object.keys(rosterSections), + ...Object.keys(generatedSections), +])) { + const onRoster = Object.hasOwn(rosterSections, section); + const rosterEntries = onRoster ? rosterSections[section] || [] : []; + const generated = generatedSections[section] || []; + + if (!onRoster) { + collectError( + errors, + `people.${section}`, + 'error', + 'section is not declared in community-roster.json; every section the site renders must be on the roster', + ); + } else { + const generatedKeys = new Set(generated.map(key)); + for (const entry of rosterEntries) { + if (!generatedKeys.has(key(entry))) { + collectError( + errors, + `people.${section}`, + 'error', + `missing roster member ${entry.name}`, + ); + } } } + + const rosterKeys = new Set(rosterEntries.map(key)); for (const person of generated) { - if (!rosterKeys.has(key(person))) { + // Skipped when the whole section is unknown: the section error above + // already says so, once, instead of once per member. + if (onRoster && !rosterKeys.has(key(person))) { collectError( errors, `people.${section}`, diff --git a/tests/validate-community-people.test.mjs b/tests/validate-community-people.test.mjs index b5f503f5..fafa118b 100644 --- a/tests/validate-community-people.test.mjs +++ b/tests/validate-community-people.test.mjs @@ -258,10 +258,92 @@ test('reports a missing fetchedAt instead of crashing on an absent field', () => assert.doesNotMatch(result.stderr, /TypeError/); }); -test('treats a roster with no sections as having nothing to cross-check', () => { +test('rejects a generated section the roster does not declare', () => { const result = runScriptWithFixtures(SCRIPT, fixture(freshData, {})); + assert.equal(result.status, 1); + assert.match( + result.stderr, + /people\.tab.*section is not declared in community-roster\.json/, + ); + assert.match( + result.stderr, + /people\.staff.*section is not declared in community-roster\.json/, + ); +}); + +// The hole this guards: a section present only in the generated file used to +// skip the loop body entirely, so its members reached with no host +// check at all. +test('gates images in a generated section the roster does not declare', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture({ + ...freshData, + people: { + ...freshData.people, + ambassadors: [ + validPerson({ + name: 'Probe', + github: 'probe', + image: 'https://evil.example/beacon.png', + }), + ], + }, + }), + ); + assert.equal(result.status, 1); + assert.match( + result.stderr, + /people\.ambassadors.*section is not declared in community-roster\.json/, + ); + assert.match( + result.stderr, + /Probe image must be an https URL on an allowed host/, + ); +}); + +// A section the roster does not declare is reported once, as a section error, +// rather than once per member as "is not on the roster". +test('does not repeat the roster-membership error for an undeclared section', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture({ + ...freshData, + people: { ...freshData.people, ambassadors: [validPerson()] }, + }), + ); + assert.equal(result.status, 1); + assert.doesNotMatch( + result.stderr, + /people\.ambassadors.*is not on the roster/, + ); +}); + +test('still cross-checks roster membership within a declared section', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture({ + ...freshData, + people: { + tab: [validPerson(), validPerson({ name: 'Mallory', github: 'mal' })], + staff: [staffPerson()], + }, + }), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /Mallory is not on the roster/); +}); + +test('treats a declared roster section with no entries as empty', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture( + { ...freshData, people: { ...freshData.people, alumni: [] } }, + { sections: { ...roster.sections, alumni: null } }, + ), + ); assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /Validated 2 community profiles/); + assert.doesNotMatch(result.stderr, /TypeError/); }); test('treats a section absent from the generated file as empty', () => {