Skip to content

fix(security): refuse to mirror artwork through a symlinked destination - #1140

Merged
mrbobbytables merged 1 commit into
mainfrom
sec/mirror-artwork-symlink
Oct 7, 2026
Merged

mrbobbytables merged 1 commit into
mainfrom
sec/mirror-artwork-symlink

Conversation

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Security Fix

mirrorArtworkUrls() in scripts/lib/architecture-content.mjs ended both its SVG and non-SVG branches with a bare writeFileSync(destination, ...). writeFileSync() follows a symlink, so a link sitting on a mirror path under static/img/cncf-projects/ redirected the fetched third-party bytes to its target, outside the mirror directory.

Those bytes then never reach the tree scripts/validate-architecture-assets.mjs scans, so the post-hoc asset gate cannot see the content that was written — it can only see the symlink itself.

Its sibling mirrorLandscapeLogo() (scripts/lib/enduser-collector.mjs:215-216) already refuses exactly this before its own write. This carries the check over, warning and skipping the URL to match every other failure path in the loop.

This is the same argument the findRemoteReferences() gate in this very function was added on, whose comment still sits in the file: "Gate the write the way mirrorLandscapeLogo() does, rather than relying on validate-architecture-assets to catch it after the bytes are on disk." The remote-reference half was carried over; the symlink half was not.

Honest severity

Not currently exploitable, and this PR does not claim it is. The precondition is a committed symlink under static/img/cncf-projects/; git ls-files -s reports zero committed symlinks, and validate-architecture-assets.mjs rejects both symlinked entries (walk()) and symlinked asset roots (assetRootKind()). This closes the ordering gap — the write happens before validation and lands outside the scanned tree — so the fix is defence-in-depth.

Scope

Exactly two files, claimed so the next comparison can see them:

  • scripts/lib/architecture-content.mjs — mirrorArtworkUrls() only (adds lstatSync import + destination guard)
  • tests/architecture-content-mirror.test.mjs — new file

No overlap with the open held PRs: #1090 is svg-active-content.mjs, #1122 is mdx-active-content.mjs, #1112 is package.json/package-lock.json.

Verification

  • Differential on the new tests: 2 fail without the fix, 3 pass with it; the "still writes a regular-file destination" control passes both ways, so the tests pin the guard rather than the function being broken.
  • Related suites (architecture-content-mirror, project-assets, import-architectures*, validate-architecture*): 158/158 pass.
  • Full unit suite: 52 failures on pristine origin/main and 52 with this change (pre-existing sandbox failures from a missing SWC native binding, unrelated to this diff); pass count rises 1527 → 1530, exactly the 3 tests added.
  • prettier --check and eslint clean on both files.

Closes #1139


Filed by sec-check agent (ACMM L4/L5 — hold-gated mode). Hold-gated: human review required.

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

mirrorArtworkUrls() finished with a bare writeFileSync(destination, ...)
in both its SVG and non-SVG branches. writeFileSync() follows a symlink,
so a link sitting on a mirror path under static/img/cncf-projects
redirected the fetched third-party bytes to its target, outside the
mirror directory.

Those bytes then never reach the tree validate-architecture-assets.mjs
scans, so the post-hoc asset gate cannot see the content that was
written -- it can only see the symlink itself. That is the same argument
the remote-reference check in this function was added on: gate the write
here rather than relying on the asset validator to catch it after the
bytes are on disk.

mirrorLandscapeLogo() in scripts/lib/enduser-collector.mjs already
refuses exactly this before its own write. Carry the check over, warning
and skipping the URL to match every other failure path in this loop.

Not currently exploitable: the repository has no committed symlinks and
the asset validator rejects them, so the precondition is gated. This
closes the ordering gap, where the write happens before validation and
lands outside the scanned tree.

Closes #1139

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: sec-check <sec-check@hive.kubestellar.io>
@hivecommons-hive hivecommons-hive Bot added the hold label Oct 7, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor Author

Important

Held for human review by the hive's ACMM level gate.

This PR was opened by the "sec-check" agent while Hive policy required a human checkpoint for that agent. Non-outreach agents are held at ACMM L3–L5; the outreach agent is always held because it publishes project-facing communication.

Hive will keep the hold label until a human removes it. Operators can make a deliberate one-off release during an ACMM level change with release_level_holds=true, but level changes never release this hold automatically.

@hivecommons-hive hivecommons-hive Bot added the security Approved by a Hive merger/owner for auto-merge on green CI label Oct 7, 2026
@mrbobbytables
mrbobbytables added this pull request to the merge queue Oct 7, 2026
Merged via the queue into main with commit 48e0c0d Oct 7, 2026
7 checks passed
@mrbobbytables
mrbobbytables deleted the sec/mirror-artwork-symlink branch October 7, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hold security Approved by a Hive merger/owner for auto-merge on green CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[sec-check] mirrorArtworkUrls() writes through a symlinked destination, unlike its hardened sibling mirrorLandscapeLogo()

1 participant