Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(core): Allow overriding npm registry for community packages #10325

Merged
merged 6 commits into from
Aug 14, 2024
Merged

feat(core): Allow overriding npm registry for community packages #10325

merged 6 commits into from
Aug 14, 2024

Conversation

netroy
Copy link
Member

@netroy netroy commented Aug 8, 2024 •
edited
Loading

Summary

This PR adds the option to override npm registry for installing community packages.
This also works as a security fix against a compromised .npmrc, by being explicitly using --registry in all npm install commands.

Docs PR

Related Linear tickets, Github issues, and Community forum posts

SEC-59

Review / Merge checklist

  • PR title and summary are descriptive
  • Docs updated or follow-up ticket created.
  • Tests included

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Aug 8, 2024
ivov
ivov reviewed Aug 8, 2024
View reviewed changes
Copy link
Contributor

@ivov ivov left a comment

Choose a reason for hiding this comment

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

Do you know of an alternate registry with community packages so I can test this?

packages/@n8n/config/src/configs/nodes.ts Outdated Show resolved Hide resolved
packages/cli/src/services/communityPackages.service.ts Outdated Show resolved Hide resolved
packages/@n8n/config/src/configs/nodes.ts Show resolved Hide resolved
@netroy
Copy link
Member Author

netroy commented Aug 8, 2024

Do you know of an alternate registry with community packages so I can test this?

We could publish a test package to the Github NPM registry

@netroy netroy mentioned this pull request Aug 8, 2024
Merged
@netroy netroy requested a review from ivov August 8, 2024 10:19
@ivov
Copy link
Contributor

ivov commented Aug 8, 2024

Do you know of an alternate registry with community packages so I can test this?

We could publish a test package to the Github NPM registry

I thought this was tested. Or because it's a security fix we need to merge this urgently?

@netroy
Copy link
Member Author

netroy commented Aug 8, 2024

I thought this was tested.

I did test it with a local verdaccio.

@ivov
Copy link
Contributor

ivov commented Aug 8, 2024

Sorry, I'm not familiar with either of those registries, so I'll have to come back later to see how to set them up to test this properly. Let me know if you need this merged anyway.

@netroy
Copy link
Member Author

netroy commented Aug 8, 2024

To test this:

  1. run docker run -it --rm -p 4873:4873 verdaccio/verdaccio:nightly-master
  2. set the registry env variable to http://127.0.0.1:4873
  3. start the app, and install any of the community nodes

everything should work just as it did before, but you can see the proxy calls in the container logs.

ivov
ivov previously approved these changes Aug 8, 2024
View reviewed changes
Copy link
Contributor

@ivov ivov left a comment

Choose a reason for hiding this comment

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

Tested and working. Pending internal discussion.

@cypress Cypress
Copy link

cypress bot commented Aug 8, 2024 •
edited
Loading


Test summary

401 • 0 • 0 • 0 • Flakiness 0

Run details
Project n8n
Status Passed
Commit 69ba9f9
Started Aug 14, 2024 5:37 AM
Ended Aug 14, 2024 5:42 AM
Duration 04:51 💡
OS Linux Debian -
Browser Electron 118

View run in Cypress Cloud ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 8, 2024

✅ All Cypress E2E specs passed

@netroy netroy marked this pull request as draft August 8, 2024 15:48
@netroy netroy marked this pull request as ready for review August 12, 2024 13:50
@netroy netroy requested a review from ivov August 12, 2024 17:43
@netroy netroy requested a review from ivov August 13, 2024 15:16
ivov
ivov approved these changes Aug 14, 2024
View reviewed changes
Copy link
Contributor

@ivov ivov left a comment

Choose a reason for hiding this comment

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

💎

@github-actions GitHub Actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@netroy netroy merged commit 33a2703 into master Aug 14, 2024
27 checks passed
@netroy netroy deleted the SEC-59-custom-npm-registry branch August 14, 2024 07:44
MiloradFilipovic added a commit that referenced this pull request Aug 14, 2024
@MiloradFilipovic
Merge branch 'master' into ADO-2365-update-design-system
c65ffd9 
* master: (98 commits)
  feat(core): Allow overriding npm registry for community packages (#10325)
  feat(core): Upgrade DB drivers (no-changelog) (#10370)
  fix(editor): Fix bug causing workflow debugging to not work in new canvas (no-changelog) (#10384)
  fix: Fix issue with some errors not being handled correctly (no-changelog) (#10371)
  fix(core): Filter out prototype and constructor lookups in expressions (#10382)
  fix(editor): Connect up new project viewer role to the FE (#9913)
  refactor(core): Move queue recovery to scaling service (no-changelog) (#10368)
  fix(core): Account for owner when filtering by project ID in `GET /workflows` in Public API (#10379)
  fix(editor): Fix rendering of SVG icons in public chat on iOS (#10381)
  fix: Require mfa code to disable mfa (#10345)
  ci: Disable turbo cache when running tests for coverage collection (no-changelog) (#10380)
  refactor(editor): Add typed event bus (no-changelog) (#10367)
  refactor(core): Remove unused constants in Redis channels (no-changelog) (#10369)
  fix(editor): Revert change that hid swagger docs in the ui (#10350)
  fix(Okta Node): Add missing codex file (no-changelog) (#10372)
  fix(core): Fix worker shutdown errors when active executions (#10353)
  refactor(core): Rename ActiveWebhooks to LiveWebhooks (no-changelog) (#10355)
  fix(n8n Form Trigger Node): Fix issue preventing v1 node from working (#10364)
  feat(editor): Upgrade markdown-it to address AIKIDO-2024-10034 (no-changelog) (#10358)
  ci: Upgrade axios to address CVE-2024-39338 (no-changelog) (#10365)
  ...

# Conflicts:
#	packages/design-system/package.json
@github-actions github-actions bot mentioned this pull request Aug 14, 2024
Merged
@janober
Copy link
Member

janober commented Aug 15, 2024

Got released with n8n@1.55.0

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

@ivov ivov ivov approved these changes

Assignees
No one assigned
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@netroy @ivov @janober