From a52461fca5e00375f8f9972e46b6963509b7f87f Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 03:30:35 -0400 Subject: [PATCH 1/2] 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 f6ff59443cc0f504e750e1c035989c47088ebd1b Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 07:32:50 -0400 Subject: [PATCH 2/2] 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.