From 2f3c7da30596c60e4a872ecc780ae6a9c8be5a3e Mon Sep 17 00:00:00 2001 From: sec-check Date: Tue, 29 Sep 2026 13:19:27 -0400 Subject: [PATCH] fix(security): gate subdirectories of the shallow static/img walk static/img is walked with recurse: false, and walk() silently dropped every subdirectory it met in that mode. assetRootPaths only knows the three subdirectories that exist today, and checkForUngatedStaticEntries() only inspects the static/ root, so a new subdirectory under static/img was neither an asset root nor recursed into nor reported: it shipped to the site origin with no gate at all. A shallow walk covers only the files sitting directly in the directory, so every subdirectory below it must be an asset root with its own assetDirs entry. One that is not is now an error, the same rule checkForUngatedStaticEntries() already applies at the static/ root. Signed-off-by: sec-check --- scripts/validate-architecture-assets.mjs | 22 ++++++++++-- tests/validate-architecture-assets.test.mjs | 40 +++++++++++++++++---- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/scripts/validate-architecture-assets.mjs b/scripts/validate-architecture-assets.mjs index 9a197cdd..cc90a82e 100644 --- a/scripts/validate-architecture-assets.mjs +++ b/scripts/validate-architecture-assets.mjs @@ -50,7 +50,9 @@ const SITE_CHROME_EXTENSIONS = new Set([...ALLOWED_ASSET_EXTENSIONS, '.ico']); // 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. +// with its own quality setting; walk() rejects any *other* subdirectory it +// finds there, since a shallow walk would otherwise publish its contents with +// no gate at all. // // static/fonts is deliberately outside the gate (it holds no SVG, and its // extensions are legitimately outside the image allow-list) via the @@ -165,7 +167,23 @@ function walk(dir, recurse = true) { record(path, 'error', 'is a symbolic link; symlinks are not allowed'); return []; } - if (entry.isDirectory()) return recurse ? walk(path) : []; + if (entry.isDirectory()) { + if (recurse) return walk(path); + // A shallow walk covers only the files sitting directly in the + // directory, so every subdirectory below it must be an asset root with + // its own assetDirs entry (returned above via assetRootPaths). One that + // is not is published at the site origin with no gate at all -- the same + // silent gap checkForUngatedStaticEntries() closes at the static/ root, + // one level down. + record( + path, + 'error', + 'is a subdirectory of a shallowly walked asset directory and is not ' + + 'covered by the asset security gate; add it to assetDirs in ' + + 'scripts/validate-architecture-assets.mjs', + ); + return []; + } return [path]; }); } diff --git a/tests/validate-architecture-assets.test.mjs b/tests/validate-architecture-assets.test.mjs index 9e7235fc..8e3e72b7 100644 --- a/tests/validate-architecture-assets.test.mjs +++ b/tests/validate-architecture-assets.test.mjs @@ -524,16 +524,44 @@ test('accepts an allowed image sitting directly in static/', () => { assert.equal(result.status, 0, result.stderr); }); -test('does not descend into subdirectories of the shallow static/img walk', () => { +test('rejects a subdirectory of the shallow static/img walk', () => { // static/img is walked with recurse: false because it holds site chrome - // sitting directly in the directory. Its subdirectories are either asset - // roots with their own entry or -- as here -- out of the gate's reach, so - // an asset nested inside one is neither validated nor counted. + // sitting directly in the directory. A shallow walk covers only those + // files, so every subdirectory below it must be an asset root with its own + // assetDirs entry; one that is not would otherwise ship to the site origin + // with no gate at all. const result = runScriptWithFixtures(SCRIPT, { 'static/img/architectures/example/diagram.svg': VALID_SVG, 'static/img/illustrations/nested.svg': '', }); + assert.equal(result.status, 1); + assert.match( + result.stderr, + /static\/img\/illustrations: is a subdirectory of a shallowly walked asset directory and is not covered by the asset security gate/, + ); +}); + +test('rejects markup in a subdirectory of the shallow static/img walk', () => { + // The extension allow-list is the only thing that keeps a .html off the + // site origin, and it never ran below static/img: the file was published + // verbatim with the validator, the unit suite and the build all green. + const result = runScriptWithFixtures(SCRIPT, { + 'static/img/architectures/example/diagram.svg': VALID_SVG, + 'static/img/blog/pwn.html': '', + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /static\/img\/blog: is a subdirectory/); +}); + +test('does not flag the static/img subdirectories that are asset roots', () => { + // The three real subdirectories each have their own assetDirs entry, so + // they are returned by assetRootPaths before the new subdirectory check. + const result = runScriptWithFixtures(SCRIPT, { + 'static/img/architectures/example/diagram.svg': VALID_SVG, + 'static/img/cncf-projects/helm-helm-icon-color.svg': VALID_SVG, + 'static/img/awards/example.svg': VALID_SVG, + 'static/img/cncf_logo_white.svg': VALID_SVG, + }); assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /Validated 1 architecture asset/); - assert.doesNotMatch(result.stderr, /illustrations/); + assert.match(result.stdout, /Validated 4 architecture asset/); });