-
Notifications
You must be signed in to change notification settings - Fork 1.1k
chore: add CI validation for external package dependencies #11898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 996466b The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
5043293 to
0869349
Compare
emily-shen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we do need a changeset right? On the off chance this breaks something, having something in the changelog saying which dependencies got bundled would be very helpful
|
I also can't see any reason why bundling these packages would break anything (and for what its worth, nor can claude), so hopefully we are okay on that front |
Ah yes! The original PR only added the validation - but now I have actually made changes, we probably should have a changeset. |
Add a validation check that ensures packages explicitly declare their external (non-bundled) dependencies to prevent dependency chain poisoning. - Add tools/deployments/validate-package-dependencies.ts validation script - Add check:package-deps npm script and integrate into pnpm check - Create scripts/deps.ts allowlists for packages with external dependencies: - miniflare (12 deps) - @cloudflare/vite-plugin (8 deps) - @cloudflare/vitest-pool-workers (7 deps) - @cloudflare/kv-asset-handler (1 dep) - @cloudflare/workers-editor-shared (1 dep) - Update CONTRIBUTING.md with documentation on managing dependencies - Add unit tests for the validation logic (22 tests)
…xternal Move mime from dependencies to devDependencies so it gets bundled into the distributable. This follows the project's practice of bundling dependencies to prevent dependency chain poisoning.
Move acorn and acorn-walk from dependencies to devDependencies so they get bundled into the distributable. These are pure JavaScript parsing libraries with no special runtime requirements, so they can be safely bundled.
Move 13 dependencies from dependencies to devDependencies so they get bundled into the distributed packages rather than being installed as transitive dependencies by users. Packages updated: - miniflare: bundle exit-hook, glob-to-regexp, stoppable, youch - @cloudflare/vitest-pool-workers: bundle birpc, devalue, get-port, semver - @cloudflare/vite-plugin: bundle @remix-run/node-fetch-server, defu, get-port, picocolors, tinyglobby Also updates the EXTERNAL_DEPENDENCIES comments to explain WHY each remaining dependency cannot be bundled (native binaries, optional native bindings, runtime resolution needs) rather than just describing how they are used.
83d01ab to
1edcf96
Compare
Summary
This PR adds CI validation to ensure packages explicitly declare their external (non-bundled) dependencies, preventing accidental dependency chain poisoning. It also bundles 13 additional dependencies to reduce supply chain risk.
(Opus assisted changes in here but guided and checked by me)
New CI Check
Adds pnpm check:package-deps (runs as part of pnpm check) which validates that:
This prevents developers from accidentally adding unbundled dependencies without explicit justification.
Dependencies Bundled
Moved 13 dependencies from dependencies to devDependencies so they get bundled:
Remaining External Dependencies
Updated comments to explain why they can't be bundled (not just how they're used):
Why this matters
When users install our packages, npm/pnpm also installs everything in dependencies. If those dependencies have unpinned transitive dependencies, a malicious actor could publish a compromised version that gets pulled into user installations. By bundling dependencies and validating the allowlist in CI, we control exactly what code ships.
Fixes DEVX-1580
A picture of a cute animal (not mandatory, but encouraged)