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', () => {