Skip to content

fix: remove Cloudflare beacon.min.js external dependency#324

Merged
benoit74 merged 1 commit intoopenzim:mainfrom
Amith71965:fix/remove-beacon-min-js
Mar 5, 2026
Merged

fix: remove Cloudflare beacon.min.js external dependency#324
benoit74 merged 1 commit intoopenzim:mainfrom
Amith71965:fix/remove-beacon-min-js

Conversation

@Amith71965
Copy link
Copy Markdown
Contributor

Fixes #323

The transform step was not stripping the Cloudflare analytics script injected by the CDN into downloaded simulation HTML files:

<script defer src="https://static.cloudflareinsights.com/beacon.min.js/..."></script>

This caused zimcheck to report a url_external ERROR since ZIM files must be fully self-contained with no external dependencies.

@Amith71965
Copy link
Copy Markdown
Contributor Author

@benoit74 I've opened this PR to fix #323. Could you please review when you get a chance? Happy to make any changes if needed.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses zimcheck url_external validation failures by stripping the Cloudflare Insights analytics script (beacon.min.js) that can be injected by the CDN into downloaded simulation HTML, ensuring generated ZIM content is fully self-contained.

Changes:

  • Detect and remove <script> tags whose src contains beacon.min.js during the HTML transform step.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Amith71965
Copy link
Copy Markdown
Contributor Author

Oh, I get you point @benoit74

I agree, Just matching with filename would not probably solve the problem as there is a possibility of cloudflare renaming the version of the file or file itself. Also I observe versioned URL already showed this: beacon.min.js/v67327c56f0bb4ef8b30...

So, relying on domain name is more stable as domain is tied to Cloudflare's infrastructure. Let me look into this

@Amith71965 Amith71965 force-pushed the fix/remove-beacon-min-js branch from 1f19390 to f2ab55d Compare March 2, 2026 20:14
@Amith71965
Copy link
Copy Markdown
Contributor Author

Hi @benoit74 , I've updated the PR with the requested changes:

  • Switched from filename matching to domain matching (static.cloudflareinsights.com) for the
    Cloudflare beacon
  • Added removal of Google Tag Manager inline scripts (googletagmanager.com)
  • Added a CHANGELOG.md entry

return
}
const scriptSrc = $(elem).attr('src')
if (scriptSrc && scriptSrc.includes('static.cloudflareinsights.com')) {

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

'
static.cloudflareinsights.com
' can be anywhere in the URL, and arbitrary hosts may come before or after it.
Copy link
Copy Markdown
Contributor

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you. Please squash commits and I will merge

@Amith71965 Amith71965 force-pushed the fix/remove-beacon-min-js branch from f2ab55d to 7df24f9 Compare March 3, 2026 20:07
@Amith71965
Copy link
Copy Markdown
Contributor Author

@benoit74
Done, commits squashed! Thank you for the review :)

@benoit74 benoit74 merged commit eba7564 into openzim:main Mar 5, 2026
3 of 4 checks passed
@Amith71965 Amith71965 deleted the fix/remove-beacon-min-js branch March 29, 2026 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove external dependency to beacon.min.js

4 participants