fix: stop hot-linking third-party project logos in import-architectures.mjs - #251
Open
mrbobbytables wants to merge 1 commit into
Open
mrbobbytables wants to merge 1 commit into
mrbobbytables wants to merge 1 commit into
Conversation
…es.mjs projectAsset() previously returned the remote URL unchanged whenever it did not match the narrow cncf/artwork `.../icon/color/<file>` shape, and renderProjectCards() rendered that URL as a card's `logo=` prop. CNCFProjectCard renders that value as an <img src>, so every reader of an affected architecture page made an unannounced request to whichever third-party host was referenced in the upstream markdown (github.com, raw.githubusercontent.com, landscape.cncf.io, upload.wikimedia.org, kubernetes-sigs.github.io), leaking IP/User-Agent/Referer and exposing the page to mutable, unreviewed remote content. Extract projectAsset() into scripts/lib/project-asset.mjs and make it fail closed: return null for any URL that does not resolve to a local /img/... asset. renderProjectCards() now omits the logo prop entirely when no local asset is available, and cleanMarkdown()'s inline-image handling is null-safe. CNCFProjectCard already renders a first-letter fallback badge when logo is falsy, so affected cards degrade cleanly instead of hot-linking a third party. Regenerate the 4 affected docs/architectures/*.md files, removing the 16 hot-linked logo= attributes flagged in the issue. Add tests/lib-project-asset.test.mjs covering the local-passthrough, mirrored-icon, and fail-closed-remote cases. Fixes #248 Signed-off-by: mrbobbytables <mrbobbytables@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #248.
projectAsset()inscripts/import-architectures.mjsreturned the remote URLunchanged whenever it did not match the narrow
cncf/artwork.../icon/color/<file>shape, andrenderProjectCards()rendered that URLdirectly as a card's
logo=prop.CNCFProjectCardrenders that value as an<img src>, so every reader of an affected architecture page made anunannounced request to whichever third-party host was referenced in the
upstream markdown (
github.com,raw.githubusercontent.com,landscape.cncf.io,upload.wikimedia.org,kubernetes-sigs.github.io),disclosing IP/User-Agent/Referer and exposing the page to mutable, unreviewed
remote content chosen by whoever writes the upstream markdown.
Changes
projectAsset()intoscripts/lib/project-asset.mjsand made itfail closed: it now returns
nullfor any URL that does not resolve toa local
/img/...asset, instead of the remote URL.renderProjectCards()now omits thelogo=prop entirely when no localasset is available.
CNCFProjectCardalready renders a first-letterfallback badge when
logois falsy, so affected cards degrade cleanly.cleanMarkdown()'s inline-image handling is now null-safe.docs/architectures/*.mdfiles, removing all 16hot-linked
logo=attributes flagged in the issue.grep -c 'logo="http' docs/architectures/*.mdis now0for every file.tests/lib-project-asset.test.mjscovering the local-passthrough,mirrored-icon, and fail-closed-remote cases.
Completion criteria (from the issue)
projectAsset()never returns a remote URLrenderProjectCards()omitslogo=when no local asset existscleanMarkdown()handles thenullreturn without throwingdocs/architectures/*.mdregenerated sogrep -c 'logo="http' docs/architectures/*.mdis 0projectAsset()local/mirrored/remote casesScope note
The issue's recommendation also floated widening
mirrorProjectAssets()tomirror arbitrary upstream images at build time. That's a larger, separate
change (new fetch/pinning logic, maintainer sign-off on which origins to
trust) and isn't required by the completion criteria above, so it's left for
a follow-up if the CNCF wants those 16 logos restored via mirroring rather
than falling back to the letter badge.
Testing
npm run test:unit— 59/59 passingnpx prettier --checkon all touched files— hive: backend=copilot model=claude-sonnet-5
🐝 Hive Agent:
contributor| SHA:7856ff3