feat(core): implement direct web login for official npm registry#13
Merged
feat(core): implement direct web login for official npm registry#13
Conversation
Add 17 new test scenarios covering: - Multiple 202 polling cycles before success - Retry-After header value respected - POST and polling network errors (fetch throws) - Empty JSON body, empty string token, ftp:// protocol URLs - Still not logged in after successful token save - Error message wrapping format verification - Already logged in skips login entirely - Shared promise cleared after failure for retry - Second package sees "Waiting for npm login..." output - LoginUrl with query parameters - doneUrl and token never exposed in task.output (security) - Post-login flow proceeds to N2/N3 checks correctly
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/src/registry/npm.ts">
<violation number="1" location="packages/core/src/registry/npm.ts:494">
P1: The `.includes("registry.npmjs.org")` check is too loose and matches domains like `my-registry.npmjs.org` or `registry.npmjs.org.evil.com`. Use strict equality against the normalized canonical form to avoid routing private-registry users through the official npm login flow.</violation>
<violation number="2" location="packages/core/src/registry/npm.ts:588">
P2: `pollRes.json()` is called unconditionally before checking the status code. If the server returns a non-JSON body for an unexpected status (e.g., 500 with an HTML error page), a `SyntaxError` propagates instead of the descriptive `NpmError`. Parse the body only after confirming a 200 or 202 status.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
- Replace .includes() with === for isOfficialNpmRegistry to prevent subdomain matching (e.g. my-registry.npmjs.org) - Move .json() call after status check in web login polling to avoid SyntaxError on non-JSON error responses
Coverage Report for @pubm/core (packages/core)
|
Coverage Report for pubm (packages/pubm)
|
Coverage Report for @pubm/plugin-external-version-sync (packages/plugins/plugin-external-version-sync)
|
Coverage Report for @pubm/plugin-brew (packages/plugins/plugin-brew)
|
syi0808
added a commit
that referenced
this pull request
Apr 10, 2026
feat(core): implement direct web login for official npm registry
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
fetch()instead of parsing npm CLI output, fixing URL redaction issues in npm 11+registry.npmjs.org); private registries keep existingnpm loginfallbackPOST /-/v1/login→ receiveloginUrl/doneUrl→ open browser → polldoneUrl→ save tokenChanges
NPM_OFFICIAL_REGISTRYconstant andisOfficialNpmRegistry()helperrunDirectWebLogin(task)method (direct web login protocol)runInteractiveLogin(task)validateNpmLoginUrl,extractNpmLoginUrl,readInteractiveStream,checkAvailability,npmLoginPromisededup)Test plan