Skip to content

fix: preserve colons in parsed auth passwords - #387

Closed
xiehuanyi wants to merge 1 commit into
unjs:mainfrom
xiehuanyi:fix/parse-auth-colon-passwords
Closed

xiehuanyi wants to merge 1 commit into
unjs:mainfrom
xiehuanyi:fix/parse-auth-colon-passwords

Conversation

@xiehuanyi

@xiehuanyi xiehuanyi commented Oct 6, 2026 •

Copy link
Copy Markdown

Description

parseAuth("user:pass:word") currently drops :word from the password. Split at the first colon so subsequent colons remain part of the password, including when encoded and raw colons are mixed.

Resolves #386.

Checks

  • Added regression cases fail against the unchanged source (3 failures).
  • pnpm test: lint, formatting, type checks, and all 496 tests pass.
  • pnpm build and git diff --check pass.
  • Reproduction and verification took 13 seconds on Node 24.19.0.

AI-assisted; the diff and test results were reviewed.

Summary by CodeRabbit

  • Bug Fixes
    • Credential parsing now splits at the first colon, preserving any additional colons as part of the password. Inputs without a colon continue to be treated as a username with an empty password, and both values are decoded as before.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f9355cd3-a9c6-4f90-a4ca-96a5ea0587de
📥 Commits

Reviewing files that changed from the base of the PR and between f06c800 and 72912d4.

📒 Files selected for processing (2)
  • src/parse.ts
  • test/parse.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

parseAuth now separates the username from the password at the first colon. Later colons remain in the password, and tests cover inputs with no colon, one colon, and multiple colons.

Changes

Auth parsing

Layer / File(s) Summary
First-separator parsing and tests
src/parse.ts, test/parse.test.ts
parseAuth splits credentials at the first colon. Tests cover inputs without a colon, additional raw colons in the password, and an encoded colon.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 72912

The change preserves the reported password case, and its regression test covers both encoded and literal colons. No actionable merge-blocking risk remains.

Architecture Summary

Architecture risk: 🔵 Low · up to 72912

The change affects 2 systems.

Changed systems: src, test

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.
  • observed — test (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/parse.ts: parseAuth replaces splitting on every colon and taking the first two pieces with splitting at only the first colon. Additional colons remain in the password; without a colon, the username is the full input and the password is empty.
  • observed — Modified behavior in test/parse.test.ts: The test file now imports parseAuth from ../src.
  • observed — Modified behavior in test/parse.test.ts: Adds parseAuth test cases for usernames with no password, passwords containing additional colons, and an encoded colon; the expectations preserve password content after the first separator.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving colons in parsed authentication passwords.
Linked Issues check ✅ Passed Issue #386 requires the first colon to separate the username from the password and requires later colons to remain in the password. src/parse.ts uses indexOf(":") and slices the password after tha…
Out of Scope Changes check ✅ Passed The reported changes are limited to parseAuth and its tests. The implementation and regression cases directly support issue #386. No unrelated changes are identified.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pi0 pi0 closed this Oct 7, 2026
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.

parseAuth truncates passwords containing raw colons

3 participants