From 7ca69851d29bc299f0fe4c873235285dbf838df9 Mon Sep 17 00:00:00 2001 From: sec-check Date: Fri, 25 Sep 2026 19:21:31 -0400 Subject: [PATCH 1/9] fix(security): treat MDX expression braces as active content The imported-page gate reported disallowed elements, event handlers, script-capable URL schemes and unexpected ESM statements, but never modelled the one MDX construct that is JavaScript by definition: a braced expression. Docusaurus compiles docs/architectures/*.md as MDX, and those bodies are written verbatim from a third-party repository, so upstream text could ship arbitrary script into the published origin. Flag every remaining brace after code spans and fences are blanked, allowing only the inert string-literal attribute form the importer emits itself. Only the braces of that form are neutralized, so a script URI smuggled into a prop value is still reported. Signed-off-by: sec-check --- scripts/lib/mdx-active-content.mjs | 51 +++++++++++++++++++++++- tests/mdx-active-content.test.mjs | 64 +++++++++++++++++++++++++++++- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/scripts/lib/mdx-active-content.mjs b/scripts/lib/mdx-active-content.mjs index 9b615f3a..7ed0ea44 100644 --- a/scripts/lib/mdx-active-content.mjs +++ b/scripts/lib/mdx-active-content.mjs @@ -8,6 +8,12 @@ * helper reports the constructs that can execute or load remote code so a * validator can fail the build before such a body ships. * + * MDX evaluates a braced expression as JavaScript, so `{fetch(...)}` in an + * imported body is live code and not prose; every `{` is therefore a finding + * unless it is one of the inert string-literal attributes the importer emits + * itself. The check is deliberately fail-closed: literal braces in upstream + * prose are reported rather than assumed harmless. + * * Scheme detection normalizes each line before testing it, because CommonMark * decodes character references in a link destination: `javascript:` is a * live `javascript:` href by the time the page renders. @@ -36,6 +42,24 @@ const ALLOWED_COMPONENT = 'CNCFProjectCard'; const ALLOWED_IMPORT = "import CNCFProjectCard from '@site/src/components/CNCFProjectCard';"; +/** + * The one expression form the importer itself emits: an attribute whose value + * is a single JSON string literal, as produced by `jsxAttribute` + * (`scripts/lib/jsx-attributes.mjs`). The pattern requires the closing brace + * to follow the closing quote immediately, so the braces can enclose nothing + * but the literal -- `{"a" + fetch(x)}` does not match. An expression whose + * entire body is a string literal evaluates to that string and has no call, + * member access or identifier reference available to it, so it is inert + * wherever it appears. + */ +const ALLOWED_ATTRIBUTE_EXPRESSION = + /(?<=\s)[A-Za-z_$][A-Za-z0-9_$-]*=\{"(?:[^"\\]|\\.)*"\}/g; + +/** + * Any remaining `{` opens an MDX expression, which is evaluated JavaScript. + */ +const EXPRESSION_PATTERN = /\{/; + const ELEMENT_PATTERN = /<\/?([A-Za-z][A-Za-z0-9._-]*)/g; const EVENT_HANDLER_PATTERN = /\bon[a-z]{3,}\s*=/gi; const DANGEROUS_URL_PATTERN = /(?:javascript|vbscript):|data:text\/html/gi; @@ -227,6 +251,22 @@ function blankInlineSpans(text) { return result; } +/** + * Neutralize the braces of the importer's own attribute expressions so the + * expression scan does not flag generated markup. Only the `{` and `}` + * characters are replaced, by a space each: the quoted value between them is + * left in place so the scheme, handler and element scans still read it, and + * the line keeps its length so findings keep accurate line numbers. + * + * @param {string} text + * @returns {string} + */ +function blankAllowedExpressions(text) { + return text.replace(ALLOWED_ATTRIBUTE_EXPRESSION, (match) => + match.replace(/[{}]/g, ' '), + ); +} + /** * Scans a Markdown/MDX body for content that executes or loads remote code. * @@ -236,7 +276,9 @@ function blankInlineSpans(text) { */ export function findActiveContent(markdown) { const findings = []; - const scannable = blankCodeSpans(String(markdown ?? '')); + const scannable = blankAllowedExpressions( + blankCodeSpans(String(markdown ?? '')), + ); const lines = scannable.split('\n'); lines.forEach((line, index) => { @@ -281,6 +323,13 @@ export function findActiveContent(markdown) { reason: 'unexpected ESM statement', snippet, }); + + if (EXPRESSION_PATTERN.test(line)) + findings.push({ + line: number, + reason: 'MDX expression', + snippet, + }); }); const seen = new Set(); diff --git a/tests/mdx-active-content.test.mjs b/tests/mdx-active-content.test.mjs index 6d8c76b4..89d03d1c 100644 --- a/tests/mdx-active-content.test.mjs +++ b/tests/mdx-active-content.test.mjs @@ -125,9 +125,11 @@ test('reports a live element hidden by a mismatched backtick run, not a code spa 'disallowed element `'), []); @@ -233,3 +235,63 @@ test('leaves an unrecognized named entity literal rather than dropping it', () = 'script-capable URL scheme', ]); }); + +test('flags an MDX expression, which Docusaurus compiles to executable JavaScript', () => { + assert.deepEqual( + reasons('{(() => { document.location = "https://evil.example"; })()}'), + ['MDX expression'], + ); + assert.deepEqual( + reasons('Body {globalThis.fetch("https://evil.example")}.'), + ['MDX expression'], + ); + assert.deepEqual(reasons('{[].constructor.constructor("return 1")()}'), [ + 'MDX expression', + ]); +}); + +test('flags the opening line of an expression that spans several lines', () => { + const body = ['{(() => {', ' fetch("https://evil.example");', '})()}'].join( + '\n', + ); + assert.deepEqual(findActiveContent(body), [ + { + line: 1, + reason: 'MDX expression', + snippet: '{(() => {', + }, + ]); +}); + +test('accepts the string-literal attribute expressions the importer emits', () => { + const card = + ' '; + assert.deepEqual(findActiveContent(card), []); +}); + +test('flags an attribute expression that is more than a string literal', () => { + assert.deepEqual( + reasons(''), + ['MDX expression'], + ); + assert.deepEqual(reasons(''), [ + 'MDX expression', + ]); +}); + +test('still scans the value inside an allowed attribute expression', () => { + // Only the braces are neutralized, so a script URI smuggled into a prop + // value is still reported rather than hidden by the allowance. + assert.deepEqual( + reasons(''), + ['script-capable URL scheme'], + ); +}); + +test('does not flag braces inside code, which MDX does not evaluate', () => { + assert.deepEqual(reasons('Run `kubectl get pods -o {.items}` to list.'), []); + assert.deepEqual( + reasons(['```json', '{ "replicas": 3 }', '```'].join('\n')), + [], + ); +}); From 743bff2e95ba5358548d080df8629a4a16cae2c5 Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Fri, 25 Sep 2026 23:30:15 -0400 Subject: [PATCH 2/9] fix(security): validate projects-born URLs before they render site-wide data/projects-born.json was the only hand-maintained data file feeding an with no validator behind it. src/components/ProjectsBorn renders each entry's url verbatim, and that component is mounted in src/theme/Footer, so the links ship on every page of the site. Add scripts/validate-projects-born.mjs, which parses every url and rejects non-https, userinfo-bearing and unparseable values, and requires name, origin and description to be non-empty and names to be unique. Parsing rather than prefix-testing matches checkHttpsUrl() in validate-awards.mjs and checkUrl() in validate-metrics.mjs: /^https:\/\// accepts "https://www.cncf.io@evil.example/", whose visible prefix and real host disagree. No host allow-list, since these are legitimately third-party project sites. Registering the script in READ_ONLY_VALIDATORS is what puts it on the pull request gate; tests/validator-smoke-coverage.test.mjs already enforces that every scripts/validate-*.mjs appears there. Signed-off-by: hivecommons-hive[bot] --- package.json | 1 + scripts/validate-projects-born.mjs | 105 ++++++++++++++++ tests/validate-projects-born.test.mjs | 166 ++++++++++++++++++++++++++ tests/validators-smoke.test.mjs | 1 + 4 files changed, 273 insertions(+) create mode 100644 scripts/validate-projects-born.mjs create mode 100644 tests/validate-projects-born.test.mjs diff --git a/package.json b/package.json index 8a8d3008..b2029dbb 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "validate:radar-reports": "node scripts/validate-radar-reports.mjs", "validate:community-people": "node scripts/validate-community-people.mjs", "validate:community-groups": "node scripts/validate-community-groups.mjs", + "validate:projects-born": "node scripts/validate-projects-born.mjs", "pr-queue-hygiene": "node scripts/pr-queue-hygiene.mjs", "test:unit": "TZ=UTC node --test", "test:unit:coverage": "TZ=UTC node tests/tools/coverage-report.mjs", diff --git a/scripts/validate-projects-born.mjs b/scripts/validate-projects-born.mjs new file mode 100644 index 00000000..8b9bab5a --- /dev/null +++ b/scripts/validate-projects-born.mjs @@ -0,0 +1,105 @@ +#!/usr/bin/env node +import { readFileSync } from 'node:fs'; +import { reportAndExit } from './lib/validate-utils.mjs'; + +// data/projects-born.json is hand-maintained, and src/components/ProjectsBorn +// renders every entry's `url` as an . That component is mounted in +// src/theme/Footer, so these links ship on every page of the site rather than +// on one section of the homepage. +// +// It was the only data file feeding an with no validator behind it, +// which left a maintainer skimming a JSON diff as the sole gate. A scheme +// prefix test would not be one either: /^https:\/\// accepts +// "https://www.cncf.io@evil.example/", whose visible prefix and real host +// disagree, and accepts unparseable values such as "https://". Parse the URL +// instead, the same standard checkHttpsUrl() in validate-awards.mjs and +// checkUrl() in validate-metrics.mjs already apply. +// +// No host allow-list: these are legitimately third-party project sites +// (envoyproxy.io, jaegertracing.io, backstage.io), unlike the cncf.io feeds +// guarded by validate-radar-reports.mjs and validate-case-studies.mjs. + +const REQUIRED_TEXT_FIELDS = ['name', 'origin', 'description']; + +function checkUrl(errors, path, value) { + if (typeof value !== 'string' || !value.trim()) { + errors.push({ + path, + severity: 'error', + message: 'url must be a non-empty string', + }); + return; + } + + let parsed; + try { + parsed = new URL(value.trim()); + } catch { + errors.push({ + path, + severity: 'error', + message: `url must be an absolute https URL: ${JSON.stringify(value)}`, + }); + return; + } + + if (parsed.protocol !== 'https:') { + errors.push({ + path, + severity: 'error', + message: `url must use https, got ${parsed.protocol}`, + }); + return; + } + + if (parsed.username || parsed.password) { + errors.push({ + path, + severity: 'error', + message: `url must not carry a userinfo component, which only disguises the real host (${parsed.hostname})`, + }); + } +} + +const data = JSON.parse( + readFileSync(new URL('../data/projects-born.json', import.meta.url)), +); +const errors = []; + +if (!Array.isArray(data) || !data.length) { + errors.push({ + path: 'projects-born.json', + severity: 'error', + message: 'projects-born.json must be a non-empty array', + }); +} + +const names = new Set(); +for (const entry of Array.isArray(data) ? data : []) { + const path = typeof entry?.name === 'string' ? entry.name : 'unknown'; + + for (const field of REQUIRED_TEXT_FIELDS) { + const value = entry?.[field]; + if (typeof value !== 'string' || !value.trim()) { + errors.push({ + path, + severity: 'error', + message: `${field} must be a non-empty string`, + }); + } + } + + if (typeof entry?.name === 'string' && names.has(entry.name)) { + errors.push({ + path, + severity: 'error', + message: 'duplicate name', + }); + } + names.add(entry?.name); + + checkUrl(errors, path, entry?.url); +} + +reportAndExit(errors, 'projects born'); +console.log(`Validated ${data.length} born projects`); diff --git a/tests/validate-projects-born.test.mjs b/tests/validate-projects-born.test.mjs new file mode 100644 index 00000000..a839f403 --- /dev/null +++ b/tests/validate-projects-born.test.mjs @@ -0,0 +1,166 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { runScriptWithFixtures } from './helpers.mjs'; + +const SCRIPT = 'validate-projects-born.mjs'; + +const validEntry = { + name: 'Envoy', + origin: 'Lyft', + description: 'Originally built at Lyft before becoming a CNCF project.', + url: 'https://www.envoyproxy.io/', +}; + +const validData = [ + validEntry, + { + name: 'Jaeger', + origin: 'Uber', + description: 'Open sourced by Uber to make distributed tracing practical.', + url: 'https://www.jaegertracing.io/', + }, +]; + +function fixture(data) { + return { 'data/projects-born.json': JSON.stringify(data) }; +} + +test('accepts a valid projects-born file', () => { + const result = runScriptWithFixtures(SCRIPT, fixture(validData)); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /Validated 2 born projects/); +}); + +test('rejects a file that is not an array', () => { + const result = runScriptWithFixtures(SCRIPT, fixture({ projects: [] })); + assert.equal(result.status, 1); + assert.match(result.stderr, /must be a non-empty array/); +}); + +test('rejects an empty array', () => { + const result = runScriptWithFixtures(SCRIPT, fixture([])); + assert.equal(result.status, 1); + assert.match(result.stderr, /must be a non-empty array/); +}); + +// The control this validator exists for: the visible prefix reads as the +// project's own site while the request resolves to the userinfo-suffixed host. +test('rejects a url carrying a userinfo component', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([ + { ...validEntry, url: 'https://www.envoyproxy.io@evil.example/' }, + ]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /must not carry a userinfo component/); + assert.match(result.stderr, /evil\.example/); +}); + +test('rejects a non-https url', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, url: 'http://www.envoyproxy.io/' }]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /must use https, got http:/); +}); + +test('rejects a javascript: url', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, url: 'javascript:alert(1)' }]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /must use https, got javascript:/); +}); + +test('rejects an unparseable url', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, url: 'www.envoyproxy.io' }]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /must be an absolute https URL/); +}); + +test('rejects a missing url', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, url: undefined }]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /url must be a non-empty string/); +}); + +test('rejects a blank url', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, url: ' ' }]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /url must be a non-empty string/); +}); + +test('rejects a non-string url', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, url: 42 }]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /url must be a non-empty string/); +}); + +for (const field of ['name', 'origin', 'description']) { + test(`rejects a missing ${field}`, () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, [field]: undefined }]), + ); + assert.equal(result.status, 1); + assert.match( + result.stderr, + new RegExp(`${field} must be a non-empty string`), + ); + }); + + test(`rejects a blank ${field}`, () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, [field]: ' ' }]), + ); + assert.equal(result.status, 1); + assert.match( + result.stderr, + new RegExp(`${field} must be a non-empty string`), + ); + }); +} + +// A non-string name still has to produce a readable error path rather than +// crashing the report, so the entry is labelled "unknown". +test('labels an entry with a non-string name as unknown', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([{ ...validEntry, name: 7 }]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /unknown: name must be a non-empty string/); +}); + +test('rejects duplicate names', () => { + const result = runScriptWithFixtures( + SCRIPT, + fixture([validEntry, { ...validEntry }]), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /duplicate name/); +}); + +// A null entry must be reported rather than crash the walk on property access. +test('reports a null entry instead of throwing', () => { + const result = runScriptWithFixtures(SCRIPT, fixture([null])); + assert.equal(result.status, 1); + assert.match(result.stderr, /unknown: name must be a non-empty string/); + assert.match(result.stderr, /unknown: url must be a non-empty string/); +}); diff --git a/tests/validators-smoke.test.mjs b/tests/validators-smoke.test.mjs index fe9c2752..b64a04b0 100644 --- a/tests/validators-smoke.test.mjs +++ b/tests/validators-smoke.test.mjs @@ -21,6 +21,7 @@ const READ_ONLY_VALIDATORS = [ 'validate-radar-reports.mjs', 'validate-community-people.mjs', 'validate-community-groups.mjs', + 'validate-projects-born.mjs', ]; for (const script of READ_ONLY_VALIDATORS) { From 706ea674bb99116066c5f7d0ecb1edecf7a190f6 Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 03:30:35 -0400 Subject: [PATCH 3/9] fix(security): gate SVGs in static/img and static/favicons at the site origin scripts/validate-architecture-assets.mjs states that it gates every directory published verbatim at the site origin, but assetDirs listed only static/img/architectures, static/img/cncf-projects and static/img/awards. static/img itself and static/favicons were never walked, so the five SVGs served from them - including the footer logo and both favicons - reached the origin with no active-content check and no extension allow-list. SVG is a document format: a browser that navigates directly to one parses it as XML and executes any script it carries, in the site's own origin. A pull request refreshing a logo or favicon therefore had no automated gate, while the identical file one directory deeper was rejected. Add both roots. static/img is walked shallowly because its image subdirectories are already listed with their own quality settings, so walk() takes a recurse flag; the symlink check runs before the directory branch so a symlinked directory is still reported in a shallow walk. static/img also serves the legacy favicon.ico, so the two chrome roots use an extension set of ALLOWED_ASSET_EXTENSIONS plus .ico - kept separate because ALLOWED_ASSET_EXTENSIONS mirrors what the importer mirrors, and the importer never writes an .ico. ICO is a raster container no browser parses as markup. static/fonts and the static/ root stay outside the gate: no SVG, and extensions legitimately outside the image allow-list. The five previously ungated SVGs already pass unchanged; this is a gate-only change. Coverage goes from 69 to 82 published assets. Signed-off-by: hivecommons-hive[bot] --- scripts/validate-architecture-assets.mjs | 101 +++++++++++++------- tests/validate-architecture-assets.test.mjs | 69 +++++++++++++ 2 files changed, 136 insertions(+), 34 deletions(-) diff --git a/scripts/validate-architecture-assets.mjs b/scripts/validate-architecture-assets.mjs index 4dbf4377..e24f4946 100644 --- a/scripts/validate-architecture-assets.mjs +++ b/scripts/validate-architecture-assets.mjs @@ -12,6 +12,26 @@ import { collectError, reportAndExit } from './lib/validate-utils.mjs'; import { findActiveContent } from './lib/svg-active-content.mjs'; const root = fileURLToPath(new URL('..', import.meta.url)); + +// Mirrors MIRRORABLE_ASSET_EXTENSIONS in scripts/import-architectures.mjs. +// static/ is published verbatim at the site origin, so a file the browser +// executes as markup or script must never be present here. +const ALLOWED_ASSET_EXTENSIONS = new Set([ + '.avif', + '.gif', + '.jpeg', + '.jpg', + '.png', + '.svg', + '.webp', +]); + +// static/img additionally serves the legacy favicon.ico. ICO is a raster +// container that no browser parses as markup or script, so it is safe at the +// origin. It stays out of ALLOWED_ASSET_EXTENSIONS because that set mirrors +// what the importer will mirror, and the importer never writes an .ico. +const SITE_CHROME_EXTENSIONS = new Set([...ALLOWED_ASSET_EXTENSIONS, '.ico']); + // Every directory here is published verbatim at the site origin, so every one // gets the security gate (extension allow-list, symlink rejection, SVG active // content). The gate is scoped by where the bytes are *served from*, not by @@ -22,40 +42,51 @@ const root = fileURLToPath(new URL('..', import.meta.url)); // Diagram-quality checks (viewBox, raster bloat, editor metadata) apply only // to architecture diagrams; mirrored cncf/artwork icons and award logos are // kept byte-faithful apart from the security gate. +// +// static/img and static/favicons hold site chrome - the footer logo, the +// favicons - rather than imported assets, but they are served from the same +// origin as everything else, so they carry the same security gate. static/img +// is walked shallowly because its image subdirectories are listed above, each +// with its own quality setting. +// +// static/fonts and the static/ root (robots.txt, manifest.json, .nojekyll) are +// deliberately outside the gate: they hold no SVG, and their extensions are +// legitimately outside the image allow-list. const assetDirs = [ { dir: join(root, 'static/img/architectures'), quality: true }, { dir: join(root, 'static/img/cncf-projects'), quality: false }, { dir: join(root, 'static/img/awards'), quality: false }, + { + dir: join(root, 'static/img'), + quality: false, + recurse: false, + extensions: SITE_CHROME_EXTENSIONS, + }, + { + dir: join(root, 'static/favicons'), + quality: false, + extensions: SITE_CHROME_EXTENSIONS, + }, ]; const shouldFix = process.argv.includes('--fix'); -// Mirrors MIRRORABLE_ASSET_EXTENSIONS in scripts/import-architectures.mjs. -// static/ is published verbatim at the site origin, so a file the browser -// executes as markup or script must never be present here. -const ALLOWED_ASSET_EXTENSIONS = new Set([ - '.avif', - '.gif', - '.jpeg', - '.jpg', - '.png', - '.svg', - '.webp', -]); - const issues = []; const fixed = []; -function walk(dir) { +function walk(dir, recurse = true) { return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { const path = join(dir, entry.name); // A symlink in published assets can point anywhere in the repository (or // outside it) and would be followed by readers and --fix writers, so its // presence is itself an error rather than something to validate through. + // Checked before the directory branch, so a symlinked directory is still + // reported in a shallow walk. if (entry.isSymbolicLink()) { record(path, 'error', 'is a symbolic link; symlinks are not allowed'); return []; } - return entry.isDirectory() ? walk(path) : [path]; + if (entry.isDirectory()) return recurse ? walk(path) : []; + return [path]; }); } @@ -161,7 +192,7 @@ function validateSvg(path, quality) { } } -function validateAsset(path, quality) { +function validateAsset(path, quality, extensions) { const rel = relative(root, path); const stats = statSync(path); const maxSize = 2 * 1024 * 1024; // 2 MB @@ -174,7 +205,7 @@ function validateAsset(path, quality) { } const extension = extname(path).toLowerCase(); - if (!ALLOWED_ASSET_EXTENSIONS.has(extension)) { + if (!extensions.has(extension)) { record( path, 'error', @@ -188,23 +219,25 @@ function validateAsset(path, quality) { } } -const assets = assetDirs.flatMap(({ dir, quality }) => { - const kind = assetRootKind(dir); - // Fail loudly rather than skipping: a silently unvalidated asset root ships - // unchecked SVGs from the site origin. - if (kind === 'symlink') { - record( - dir, - 'error', - 'asset directory is a symbolic link; symlinks are not allowed', - ); - return []; - } - if (kind !== 'directory') return []; - return walk(dir).map((path) => ({ path, quality })); -}); -for (const { path, quality } of assets) { - validateAsset(path, quality); +const assets = assetDirs.flatMap( + ({ dir, quality, recurse = true, extensions = ALLOWED_ASSET_EXTENSIONS }) => { + const kind = assetRootKind(dir); + // Fail loudly rather than skipping: a silently unvalidated asset root ships + // unchecked SVGs from the site origin. + if (kind === 'symlink') { + record( + dir, + 'error', + 'asset directory is a symbolic link; symlinks are not allowed', + ); + return []; + } + if (kind !== 'directory') return []; + return walk(dir, recurse).map((path) => ({ path, quality, extensions })); + }, +); +for (const { path, quality, extensions } of assets) { + validateAsset(path, quality, extensions); } if (fixed.length) { diff --git a/tests/validate-architecture-assets.test.mjs b/tests/validate-architecture-assets.test.mjs index 51cf2e7d..2804313f 100644 --- a/tests/validate-architecture-assets.test.mjs +++ b/tests/validate-architecture-assets.test.mjs @@ -356,3 +356,72 @@ test('warns on an asset larger than 2 MB but still passes', () => { assert.match(result.stderr, /large\.svg: asset is \d+\.\d\d MB/); assert.match(result.stdout, /Validated 1 architecture asset/); }); + +// static/img and static/favicons hold site chrome — the footer logo, the +// favicon set — rather than imported assets. They are served from the same +// origin as the diagrams, so a browser that opens one of their SVGs directly +// executes any script it carries; they were outside the gate until #690. + +test('rejects active content in a site-chrome SVG under static/img', () => { + const logo = 'static/img/cncf_logo_white.svg'; + const svg = VALID_SVG.replace('alert(1) { + const icon = 'static/favicons/favicon.svg'; + const svg = VALID_SVG.replace( + ' { + const result = runScriptWithFixtures(SCRIPT, { + 'static/img/page.html': 'x', + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /\.html is not an allowed asset type/); +}); + +test('accepts the legacy favicon.ico that static/img serves', () => { + // ICO is a raster container no browser parses as markup, so it is safe at + // the origin even though the importer never mirrors one. + const result = runScriptWithFixtures(SCRIPT, { + 'static/img/favicon.ico': 'not-really-an-icon', + }); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /Validated 1 architecture asset/); +}); + +test('walks static/img shallowly so diagram-quality checks stay scoped', () => { + // static/img is walked without recursion because its image subdirectories + // are gated as their own roots. A viewBox-less logo sitting directly in + // static/img must therefore pass, while the same file under + // static/img/architectures fails the diagram-quality gate. + const svg = ''; + const chrome = runScriptWithFixtures(SCRIPT, { 'static/img/logo.svg': svg }); + assert.equal(chrome.status, 0, chrome.stderr); + + const diagram = runScriptWithFixtures(SCRIPT, svgFixture(svg)); + assert.equal(diagram.status, 1); + assert.match(diagram.stderr, /missing viewBox/); +}); + +test('reports a symlinked directory sitting directly in static/img', () => { + // The shallow walk must still reject symlinks: the symlink check runs + // before the directory branch, so a link that would otherwise be skipped + // for not being recursed into is still a finding. + const result = runScriptWithFixtures( + SCRIPT, + { 'static/img/architectures/example/diagram.svg': VALID_SVG }, + { symlinks: { 'static/img/elsewhere': 'architectures/example' } }, + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /elsewhere: is a symbolic link/); +}); From c793acfc1156cbe3f7e772d0adf9e0b04c36adac Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 07:32:50 -0400 Subject: [PATCH 4/9] fix(security): keep the shallow static/img walk off declared asset roots The new shallow walk of static/img enumerated static/img/architectures, static/img/cncf-projects and static/img/awards as ordinary entries. Each of those is already an asset root with its own entry in assetDirs, which decides for itself whether a symlink is an error and whether a non-directory root is skipped. Re-entering them from the parent walk duplicated the symlink error and, where a root was a regular file rather than a directory, rejected it as an extensionless asset - breaking the pre-existing 'a regular file at an asset root is skipped, not walked' test. Skip any entry whose path is itself a declared asset root. Signed-off-by: hivecommons-hive[bot] --- scripts/validate-architecture-assets.mjs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scripts/validate-architecture-assets.mjs b/scripts/validate-architecture-assets.mjs index e24f4946..cebb872e 100644 --- a/scripts/validate-architecture-assets.mjs +++ b/scripts/validate-architecture-assets.mjs @@ -70,12 +70,21 @@ const assetDirs = [ ]; const shouldFix = process.argv.includes('--fix'); +// Paths that are asset roots in their own right. The shallow static/img walk +// must not report on them whatever they turn out to be on disk: each one is +// already handled by its own entry above, which decides for itself whether a +// symlink is an error and whether a non-directory is skipped. Without this the +// shallow walk would duplicate the symlink error and would additionally reject +// a regular file sitting where a root is expected. +const assetRootPaths = new Set(assetDirs.map(({ dir }) => dir)); + const issues = []; const fixed = []; function walk(dir, recurse = true) { return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { const path = join(dir, entry.name); + if (assetRootPaths.has(path)) return []; // A symlink in published assets can point anywhere in the repository (or // outside it) and would be followed by readers and --fix writers, so its // presence is itself an error rather than something to validate through. From f33dac380b4684f19d2314f978dfb46519a1017f Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 15:47:21 -0400 Subject: [PATCH 5/9] fix(security): pin the community-group repository URL gate to https github.com validate-community-groups.mjs checked group.repository with a bare new URL() parse, which succeeds for javascript:, data:, cleartext http, and a userinfo-spoofed authority such as https://github.com@evil.example. It was the only URL gate in data/ that decided nothing about the destination beyond parseability, while validate-case-studies.mjs, validate-radar-reports.mjs, validate-awards.mjs, validate-architectures.mjs and lib/project-card-links.mjs all require https, reject userinfo, and pin the host. Require https, reject userinfo, and pin the host to github.com, which is the only host check-community-group-links.mjs ever writes. The missing-field branch still reports an absent repository on its own. Signed-off-by: hivecommons-hive[bot] --- scripts/validate-community-groups.mjs | 32 ++++++++++-- tests/validate-community-groups.test.mjs | 66 ++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 7 deletions(-) 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', () => { From 8a086f65b89811db977d9efabd2c0bbb714a15f5 Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 19:50:51 -0400 Subject: [PATCH 6/9] 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', () => { From 4bd2aed3ac6ce8b89673dfd15654f8c17bddb305 Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 23:56:15 -0400 Subject: [PATCH 7/9] fix(security): parse members.json URLs instead of prefix-matching them data/members.json renders as on /community/members via MemberProfile.js, but it has no validator script: there is no validate:members, nothing for it in ci.yml's validator list, and it is absent from READ_ONLY_VALIDATORS in validators-smoke.test.mjs. Its only gate was the /^https:\/\// prefix regex in members-data.test.mjs. That is the test validate-awards.mjs and validate-architectures.mjs already document as insufficient: "https://www.cncf.io@evil.example/" matches the prefix while resolving to evil.example, and the bare string "https://" matches while failing to parse at all. The awards cross-check does not compensate, because it compares only the slug/year/award key and never the URL values. Gate architectures[].sourceUrl, the three award URLs and every sourceAttribution entry on a parsed URL that must use https and must carry no userinfo, mirroring isHttpsUrl() in validate-architectures.mjs, and cover the spoofed forms. Signed-off-by: hivecommons-hive[bot] --- tests/members-data.test.mjs | 74 ++++++++++++++++++++++++++++++------- 1 file changed, 61 insertions(+), 13 deletions(-) diff --git a/tests/members-data.test.mjs b/tests/members-data.test.mjs index 0f099c10..db56cdba 100644 --- a/tests/members-data.test.mjs +++ b/tests/members-data.test.mjs @@ -26,6 +26,33 @@ const REQUIRED_ARRAY_FIELDS = [ const SLUG_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; +// members.json is generated, but nothing reconciles its URLs against the +// validated sources it came from: the awards cross-check below compares only +// the slug/year/award key. This is the only gate on the hrefs the member +// directory renders, so it parses rather than prefix-matches. A `/^https:\/\//` +// test accepts "https://www.cncf.io@evil.example/", whose real host is +// evil.example, and accepts the bare string "https://", which is not a URL at +// all. Mirrors isHttpsUrl() in scripts/validate-architectures.mjs. +function httpsUrlProblem(value) { + if (typeof value !== 'string') return 'must be a string'; + if (value !== value.trim()) return 'must not have surrounding whitespace'; + let url; + try { + url = new URL(value); + } catch { + return 'must be a parseable URL'; + } + if (url.protocol !== 'https:') return `must use https, got ${url.protocol}`; + if (url.username || url.password) + return `must not carry userinfo; its real host is ${url.host}`; + return null; +} + +function assertHttpsUrl(value, label) { + const problem = httpsUrlProblem(value); + assert.equal(problem, null, `${label} ${problem} (${String(value)})`); +} + test('members.json exposes the generated envelope', () => { assert.equal(typeof membersData.description, 'string'); assert.ok(membersData.description.length > 0); @@ -127,10 +154,9 @@ test('member architecture entries carry their provenance', () => { `${member.id} architecture is missing ${field}`, ); } - assert.match( + assertHttpsUrl( architecture.sourceUrl, - /^https:\/\//, - `${member.id} architecture sourceUrl must be https`, + `${member.id} architecture sourceUrl`, ); assert.match( architecture.sourceCommit, @@ -159,11 +185,7 @@ test('member award entries carry the fields the profile renders', () => { ); for (const field of ['announcementUrl', 'caseStudyUrl', 'talkUrl']) { if (!award[field]) continue; - assert.match( - award[field], - /^https:\/\//, - `${member.id} award ${field} must be https`, - ); + assertHttpsUrl(award[field], `${member.id} award ${field}`); } } } @@ -172,11 +194,7 @@ test('member award entries carry the fields the profile renders', () => { test('sourceAttribution entries are https URLs', () => { for (const member of members) { for (const url of member.sourceAttribution) { - assert.match( - url, - /^https:\/\//, - `${member.id} sourceAttribution entry must be an https URL`, - ); + assertHttpsUrl(url, `${member.id} sourceAttribution entry`); } } }); @@ -222,3 +240,33 @@ test('members with no public detail still carry attribution', () => { ); } }); + +test('the URL gate rejects what a bare https prefix test would accept', () => { + for (const value of [ + 'https://www.cncf.io@evil.example/phish', + 'https://cncf.io@127.0.0.1/', + 'https://user:pass@evil.example/', + 'https://', + 'http://cncf.io/', + 'javascript:alert(1)', + ' https://cncf.io/', + 42, + null, + ]) { + assert.notEqual( + httpsUrlProblem(value), + null, + `${String(value)} must be rejected`, + ); + } +}); + +test('the URL gate accepts ordinary https source links', () => { + for (const value of [ + 'https://cncf.io/', + 'https://github.com/cncf/endusers', + 'https://www.cncf.io/case-studies/example/?utm=1#section', + ]) { + assert.equal(httpsUrlProblem(value), null, `${value} must be accepted`); + } +}); From 1b92f0fef5db87257c9864344fccfbaf823ef1e7 Mon Sep 17 00:00:00 2001 From: Bob Killen Date: Sun, 27 Sep 2026 15:20:20 -0500 Subject: [PATCH 8/9] ci: run validate:projects-born in the validate job The consolidated security fixes add the validate:projects-born gate, and main now asserts that every validate:*/check:* script is run by a workflow or recorded as exempt. Wire the new gate into ci.yml alongside the other data validators, which also unblocks the narrow coverage-report runs that execute workflow-scripts.test.mjs as a fixture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Bob Killen --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 761c427f..790a9051 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,6 +57,8 @@ jobs: run: npm run validate:case-studies - name: Validate radar reports data run: npm run validate:radar-reports + - name: Validate projects-born data + run: npm run validate:projects-born - name: Validate button contrast run: npm run validate:button-contrast - name: Build production site From de6281c01dfb90f0433b8bfea32179b8e8986b3c Mon Sep 17 00:00:00 2001 From: sec-check Date: Sun, 27 Sep 2026 16:16:59 -0400 Subject: [PATCH 9/9] fix(security): hold data/metrics.json URLs to a host allow-list scripts/validate-metrics.mjs was the only validator guarding a rendered href that applied no host allow-list: checkUrl() asserted an absolute https URL with no userinfo and stopped, so any host passed. Every URL in data/metrics.json is rendered under hard-coded anchor text -- 'cncf/architecture' in src/components/ReferenceArchitectures, 'Source' in src/components/MetricsDashboard -- so a plain https URL on a host that is not CNCF's publishes as a CNCF-labelled link to an unrelated origin. That is the same sink shape validate-case-studies.mjs and validate-radar-reports.mjs already pin with ALLOWED_HOST_SUFFIXES. Pin the host to cncf.io or github.com after the existing userinfo check, which keeps the more specific userinfo diagnostic for a spoofed-userinfo URL. Both hosts are already the only ones present in data/metrics.json, so this is a no-op for the current data. Closes #752 Signed-off-by: sec-check --- scripts/validate-metrics.mjs | 25 ++- .../validate-metrics-host-allowlist.test.mjs | 192 ++++++++++++++++++ 2 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 tests/validate-metrics-host-allowlist.test.mjs diff --git a/scripts/validate-metrics.mjs b/scripts/validate-metrics.mjs index e6c197cd..d86d1361 100644 --- a/scripts/validate-metrics.mjs +++ b/scripts/validate-metrics.mjs @@ -15,6 +15,17 @@ const errors = []; // parses with protocol "https:" while resolving to evil.example, so it reads // as CNCF to a human reviewing a generated diff and resolves elsewhere in a // browser. Mirrors checkHttpsUrl() in validate-awards.mjs. +// +// The scheme and userinfo tests are still not a gate on their own: every one of +// these URLs is rendered under hard-coded anchor text ("cncf/architecture", +// "Source ↗"), so a plain https URL on a host that is not CNCF's publishes as a +// CNCF-labelled link to an unrelated origin. Hold the host to an allow-list, +// the same standard ALLOWED_HOST_SUFFIXES applies in +// scripts/validate-case-studies.mjs and scripts/validate-radar-reports.mjs. +// github.com is allowed because sources.*.repository, sources.*.sourceUrl and +// several metric sourceUrl values legitimately point at github.com/cncf/... +const ALLOWED_HOST_SUFFIXES = ['cncf.io', 'github.com']; + function checkUrl(path, field, value) { if (value === undefined || value === null || value === '') return; let parsed; @@ -36,12 +47,24 @@ function checkUrl(path, field, value) { }); return; } - if (parsed.username || parsed.password) + if (parsed.username || parsed.password) { errors.push({ path, severity: 'error', message: `${field} must not carry a userinfo component, which only disguises the real host (${parsed.hostname})`, }); + return; + } + const host = parsed.hostname.toLowerCase(); + const allowed = ALLOWED_HOST_SUFFIXES.some( + (domain) => host === domain || host.endsWith(`.${domain}`), + ); + if (!allowed) + errors.push({ + path, + severity: 'error', + message: `${field} must be on ${ALLOWED_HOST_SUFFIXES.join(' or ')}, got ${parsed.hostname}`, + }); } if (!data.generated) diff --git a/tests/validate-metrics-host-allowlist.test.mjs b/tests/validate-metrics-host-allowlist.test.mjs new file mode 100644 index 00000000..a55af1e4 --- /dev/null +++ b/tests/validate-metrics-host-allowlist.test.mjs @@ -0,0 +1,192 @@ +// Host allow-list coverage for scripts/validate-metrics.mjs. +// +// Every URL in data/metrics.json is rendered under hard-coded anchor text +// ("cncf/architecture" in src/components/ReferenceArchitectures, "Source ↗" in +// src/components/MetricsDashboard), so an https URL on a host that is not +// CNCF's publishes as a CNCF-labelled link to an unrelated origin. These cases +// pin the allow-list at every checkUrl() call site. +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import test from 'node:test'; +import { runScriptWithFixtures } from './helpers.mjs'; + +const SCRIPT = 'validate-metrics.mjs'; + +const validMetric = { + id: 'cncf-projects', + label: 'CNCF projects', + value: 100, + source: 'landscape', + sourceUrl: 'https://landscape.cncf.io/', + collectedAt: '2026-08-07T00:00:00.000Z', +}; + +const validData = { + generated: true, + generatedAt: '2026-08-07T00:00:00.000Z', + sources: { + landscape: { revision: 'abc123' }, + architectures: { revision: 'def456' }, + }, + metrics: [validMetric], +}; + +function run(data) { + return runScriptWithFixtures(SCRIPT, { + 'data/metrics.json': JSON.stringify(data), + }); +} + +const OFF_ALLOWLIST = 'https://evil.example/x'; + +test('accepts github.com and cncf.io subdomains across every URL field', () => { + const result = run({ + ...validData, + sources: { + landscape: { + revision: 'abc123', + repository: 'https://github.com/cncf/landscape', + sourceUrl: 'https://github.com/cncf/landscape/blob/main/landscape.yml', + }, + architectures: { + revision: 'def456', + repository: 'https://github.com/cncf/architecture', + sourceUrl: 'https://github.com/cncf/architecture', + }, + }, + referenceArchitectureLifecycle: { sourceUrl: 'https://www.cncf.io/' }, + series: { + projects: { + label: 'Projects', + sourceUrl: 'https://landscape.cncf.io/', + values: [{ date: '2026-01-01', value: 1 }], + }, + }, + breakdowns: { + industries: { + label: 'Industries', + sourceUrl: 'https://github.com/cncf/landscape', + values: [{ name: 'Telecom', value: 1 }], + }, + }, + }); + assert.equal(result.status, 0, result.stderr); +}); + +test('rejects an off-allow-list host in sources.*.repository', () => { + const result = run({ + ...validData, + sources: { + landscape: { revision: 'abc123', repository: OFF_ALLOWLIST }, + architectures: { revision: 'def456' }, + }, + }); + assert.equal(result.status, 1); + assert.match( + result.stderr, + /repository must be on cncf\.io or github\.com, got evil\.example/, + ); +}); + +test('rejects an off-allow-list host in sources.*.sourceUrl', () => { + const result = run({ + ...validData, + sources: { + landscape: { revision: 'abc123' }, + architectures: { revision: 'def456', sourceUrl: OFF_ALLOWLIST }, + }, + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /sources\.architectures/); + assert.match(result.stderr, /must be on cncf\.io or github\.com/); +}); + +test('rejects an off-allow-list host in referenceArchitectureLifecycle.sourceUrl', () => { + const result = run({ + ...validData, + referenceArchitectureLifecycle: { sourceUrl: OFF_ALLOWLIST }, + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /referenceArchitectureLifecycle/); + assert.match(result.stderr, /must be on cncf\.io or github\.com/); +}); + +test('rejects an off-allow-list host in metric.sourceUrl', () => { + const result = run({ + ...validData, + metrics: [{ ...validMetric, sourceUrl: OFF_ALLOWLIST }], + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /must be on cncf\.io or github\.com/); +}); + +test('rejects an off-allow-list host in series.*.sourceUrl', () => { + const result = run({ + ...validData, + series: { + projects: { + label: 'Projects', + sourceUrl: OFF_ALLOWLIST, + values: [{ date: '2026-01-01', value: 1 }], + }, + }, + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /series\.projects/); + assert.match(result.stderr, /must be on cncf\.io or github\.com/); +}); + +test('rejects an off-allow-list host in breakdowns.*.sourceUrl', () => { + const result = run({ + ...validData, + breakdowns: { + industries: { + label: 'Industries', + sourceUrl: OFF_ALLOWLIST, + values: [{ name: 'Telecom', value: 1 }], + }, + }, + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /breakdowns\.industries/); + assert.match(result.stderr, /must be on cncf\.io or github\.com/); +}); + +test('rejects a host that merely ends with an allowed label, not an allowed domain', () => { + const result = run({ + ...validData, + metrics: [ + { ...validMetric, sourceUrl: 'https://notcncf.io/x' }, + { + ...validMetric, + id: 'suffix-trap', + sourceUrl: 'https://evil-github.com/x', + }, + ], + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /got notcncf\.io/); + assert.match(result.stderr, /got evil-github\.com/); +}); + +test('reports userinfo rather than the allow-list when both would fail', () => { + const result = run({ + ...validData, + metrics: [ + { ...validMetric, sourceUrl: 'https://www.cncf.io@evil.example/x' }, + ], + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /must not carry a userinfo component/); + assert.doesNotMatch(result.stderr, /must be on cncf\.io or github\.com/); +}); + +test('the committed data/metrics.json satisfies the allow-list', () => { + const result = runScriptWithFixtures(SCRIPT, { + 'data/metrics.json': readFileSync( + new URL('../data/metrics.json', import.meta.url), + 'utf8', + ), + }); + assert.equal(result.status, 0, result.stderr); +});