{{ message }}
Repository navigation
fix(devtools-bundler-core): validate package and import names from the event bus - #545
Open
AlemTuzlak wants to merge 1 commit into
Open
AlemTuzlak wants to merge 1 commit into
AlemTuzlak wants to merge 1 commit into
Conversation
…e event bus The install-devtools and bump-package-version events come from the unauthenticated event bus. installPackage put the package name into a shell string for exec(), so "x; touch /tmp/pwned; #" ran the second command. installPackage now accepts only an npm package name with an optional version and no leading "-". On macOS and Linux it runs the package manager with execFile and no shell. Windows still needs a shell for the .cmd shims, and the pattern allows no cmd.exe syntax. The plugin import name from the same events is written into the user's devtools file as code, so it must now be a JavaScript identifier. Refs #464
|
View your CI Pipeline Execution ↗ for commit b39cb1b
☁️ Nx Cloud last updated this comment at |
This branch has not been deployed
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
1 participant
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.
An unauthenticated event bus message can run shell commands on the dev server:
installPackageputs the package name from the event into a shell string. This PR accepts only valid npm package names and runs the package manager without a shell on macOS and Linux. It also rejects a plugin import name that is not a JavaScript identifier, because that name is written into the user's source file as code.🎯 Changes
installPackagenow checks the name against an npm package pattern with an optional version (@scope/name@^1.2.3). The pattern allows no shell syntax and no leading-, so a name cannot become a CLI flag.execFileand an args array, with no shell..cmdshims that start only through a shell. Node 24 also warns (DEP0190) aboutexecFilewith args andshell: true. So Windows keepsexec, and the pattern rejects allcmd.exeoperators (& | < > ( ) % !and quotes).transformAndInjectwritespluginImport.importNamefrom the same events into the devtools file (import { ${importName} } ...). It now must match/^[A-Za-z_$][\w$]*$/.✅ Checklist
pnpm test:pr, or these tests do not apply to this pull request.🚀 Release Impact
Testing
Commands run
vitest runinpackages/devtools-bundler-core: 213 tests pass, on Windows. Linux CI runs theexecFilebranch.execandexecFile, so no test runs a real command. Before the fix, all 11installPackagecases and the import name case failed.eslint,tsc, andprettier --checkinpackages/devtools-bundler-core: pass.pnpm test:pr.Manual test
devtools()./tmp/tanstack-devtools-vite-pwnexists.Invalid package name, and no marker file exists.How this PR makes testing easy
package-manager.test.tscovers injection payloads and valid names.inject-plugin.test.tscovers an import name with code in it.Linked issues
Refs #464. This PR stops the shell injection. The event bus still accepts unauthenticated requests, so a request can still install any valid npm package (and its install scripts). An allowlist or a per-server token can close that in a follow-up.
Risk / rollback
Low. A package spec with characters outside the npm name pattern (for example a git URL) is now rejected. The marketplace sends only registry names. To undo, revert this PR.
🤖 Generated with Claude Code