Skip to content

fix: clear stale password when auth is replaced without one - #244

Open
RealBhupesh wants to merge 2 commits into
unshiftio:masterfrom
RealBhupesh:fix-auth-stale-password
Open

RealBhupesh wants to merge 2 commits into
unshiftio:masterfrom
RealBhupesh:fix-auth-stale-password

Conversation

@RealBhupesh

Copy link
Copy Markdown

Replacing the complete auth value with a username-only value currently keeps the old password:

const Url = require('url-parse')
const url = new Url('https://alice:secret@example.com/', {})
url.set('auth', 'bob')
console.log(url.href)
// Before: https://bob:secret@example.com/
// After:  https://bob@example.com/

The no-colon branch updates username but not password. This clears password there, so auth, href and toString() reflect the full replacement. Setting auth to an empty string also clears the previous credentials. This intentionally changes stale-password retention; callers intending to change only the username should use set('username', ...) instead.

The existing sequential auth test clears the password with 'baz:' before checking username-only 'qux', so it misses this case. Added a regression with fresh password-bearing URLs for username-only, empty and percent-encoded username auth values (18 assertions). It fails against the original source at the first stale-password check.

Validation: npm test passes all 102 tests, with 100% statements/functions/lines and 99.02% branches (existing branches at lines 102/104). No browser/Sauce Labs run. Colon-containing auth parsing and malformed-percent exceptions are unchanged; this is not a broader URL security fix.

No dependencies or unrelated files changed. AI assistance is recorded in both commit trailers.

A complete username-only auth value must not retain the previous password.

Assisted-by: LLM
Check username-only, empty and encoded auth values against an existing password.

Assisted-by: LLM
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.

1 participant