From bacf6bfd6adaa2b262ddc5ed4b988fb2fe78162e Mon Sep 17 00:00:00 2001 From: sec-check Date: Sun, 27 Sep 2026 16:16:59 -0400 Subject: [PATCH] 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); +});