Skip to content

fix(vercel-basic-auth): Authorization ヘッダパースを改善#18

Merged
amotarao merged 3 commits intomainfrom
fix/vercel-basic-auth-auth-header
Mar 26, 2026
Merged

fix(vercel-basic-auth): Authorization ヘッダパースを改善#18
amotarao merged 3 commits intomainfrom
fix/vercel-basic-auth-auth-header

Conversation

@amotarao
Copy link
Copy Markdown
Member

@amotarao amotarao commented Mar 26, 2026

概要

@plainbrew/vercel-basic-authbasicAuth 関数における Authorization ヘッダパース処理の問題を修正します。

Closes #10

変更内容

1. Basic スキームの検証

authorization.split(" ")[1] では BearerDigest などの他スキームも素通りしていた問題を修正。正規表現 /^Basic\s+(.+)$/i でスキームを検証するようにしました。

2. パスワード中の : の対応

atob(authValue).split(":") では : が複数あると正しく分割できない問題を修正。RFC 7617 に従い、最初の : のインデックスで usernamepassword を分割するようにしました。

テスト

修正内容に対応するテストを追加しました。

  • Bearer / Digest スキームを弾くことの確認
  • パスワードに : が含まれるとき正しく認証できることの確認
  • : を含まない base64 値のとき 401 を返すことの確認

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 26, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e8bf1bbf-44e8-4683-b841-54dc60f5d119

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

vercel-basic-auth パッケージの Authorization ヘッダーパース処理を改善します。Basic スキームの検証を厳密化し、大文字小文字を区別しない正規表現で確認します。パスワード内のコロン(:)を正しく処理するため、最初のコロン位置で分割するよう修正します。

Changes

Cohort / File(s) Summary
Changesets Entry
.changeset/fix-vercel-basic-auth-authorization-header.md
パッチリリースのための Changesets マークダウンエントリを追加。Authorization ヘッダーのスキーム検証とパスワード内コロン処理の修正を記載。
Authentication Header Parsing
packages/vercel-basic-auth/src/index.ts
Authorization ヘッダーの解析を単純なホワイトスペース分割から大文字小文字を区別しない正規表現マッチ(/^Basic\s+(.+)$/i)に変更。デコード後の認証情報を最初のコロン位置で分割し、パスワード内のコロンを正しく処理。未検証のスキームや無効な形式に対する返却パスも更新。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • feat: add @plainbrew/vercel-basic-auth package #7: 同じ vercel-basic-auth パッケージの認証ロジックを修正する PR であり、このプルリクエストが Authorization ヘッダーの解析とクレデンシャル分割を改善する先行の実装に基づいている。

Suggested reviewers

  • akameco

Poem

🐰 Basic のスキーム、厳しく見張り
コロンで割ける、パスワード守り
セキュリティ強く、ウサギも満足
ヘッダーもしっかり、バリデーション完備
Plain Brew をセキュアに、ぴょんぴょん!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed PRの実装は、issue #10で指定されたすべての要件を満たしている。Basic スキームの正規表現検証、RFC 7617に準拠した最初の : での分割実装が完備されている。
Out of Scope Changes check ✅ Passed 変更はすべて issue #10 のスコープ内であり、Authorization ヘッダのパース改善に限定されている。無関係な変更は含まれていない。
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルは変更内容の主な目的(Authorization ヘッダパースの改善)を正確に反映しており、PR の中核的な変更を明確に説明しています。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/vercel-basic-auth-auth-header

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

@amotarao amotarao marked this pull request as draft March 26, 2026 00:03
@amotarao
Copy link
Copy Markdown
Member Author

テストを先に整備

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/vercel-basic-auth/src/index.ts (1)

62-62: (参考)UTF-8 クレデンシャルのサポートについて

atob() はデコード結果を Latin-1 として解釈します。RFC 7617 では UTF-8 エンコーディングが推奨されているため、非 ASCII 文字を含むユーザー名・パスワードをサポートする場合は、以下のようなデコード処理が必要になる可能性があります:

const decoded = new TextDecoder().decode(
  Uint8Array.from(atob(matched[1]), c => c.charCodeAt(0))
);

現時点では ASCII のみのサポートで問題ないかもしれませんが、将来的な考慮事項として記録しておきます。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/vercel-basic-auth/src/index.ts` at line 62, Replace the simple
atob(matched[1]) decode (used to set decoded) with a UTF‑8‑aware decode so
credentials containing non‑ASCII characters are handled correctly: use the atob
result converted to a Uint8Array (e.g., Uint8Array.from(atob(matched[1]), c =>
c.charCodeAt(0))) and then decode with new TextDecoder().decode(...) instead of
the plain atob call; update the code path in the same block where matched[1] is
used and ensure decoded is assigned the TextDecoder output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/vercel-basic-auth/src/index.ts`:
- Line 62: Replace the simple atob(matched[1]) decode (used to set decoded) with
a UTF‑8‑aware decode so credentials containing non‑ASCII characters are handled
correctly: use the atob result converted to a Uint8Array (e.g.,
Uint8Array.from(atob(matched[1]), c => c.charCodeAt(0))) and then decode with
new TextDecoder().decode(...) instead of the plain atob call; update the code
path in the same block where matched[1] is used and ensure decoded is assigned
the TextDecoder output.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb09dedb-e16a-42d7-8b5e-96ee2e5e470e

📥 Commits

Reviewing files that changed from the base of the PR and between bf05712 and 893f893.

📒 Files selected for processing (2)
  • .changeset/fix-vercel-basic-auth-authorization-header.md
  • packages/vercel-basic-auth/src/index.ts

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@amotarao amotarao force-pushed the fix/vercel-basic-auth-auth-header branch from 893f893 to 99399ee Compare March 26, 2026 00:23
@amotarao amotarao changed the base branch from main to feature/vercel-basic-auth-tests March 26, 2026 00:23
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@amotarao amotarao force-pushed the fix/vercel-basic-auth-auth-header branch from 321a958 to 2dfc0a4 Compare March 26, 2026 00:27
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@amotarao amotarao force-pushed the fix/vercel-basic-auth-auth-header branch from f526a3f to 0254f7a Compare March 26, 2026 00:36
@amotarao amotarao changed the title fix: vercel-basic-auth の Authorization ヘッダパースを改善 fix(vercel-basic-auth): Authorization ヘッダパースを改善 Mar 26, 2026
@amotarao amotarao marked this pull request as ready for review March 26, 2026 00:37
@amotarao amotarao requested review from akameco and iromonek39 March 26, 2026 00:39
@akameco akameco removed the request for review from iromonek39 March 26, 2026 02:17
Copy link
Copy Markdown

@akameco akameco left a comment

Choose a reason for hiding this comment

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

LGTM

Base automatically changed from feature/vercel-basic-auth-tests to main March 26, 2026 02:52
@amotarao amotarao merged commit 5e9d082 into main Mar 26, 2026
2 checks passed
@amotarao amotarao deleted the fix/vercel-basic-auth-auth-header branch March 26, 2026 02:54
@github-actions github-actions Bot mentioned this pull request Mar 26, 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.

fix: vercel-basic-auth の Authorization ヘッダパースの改善

2 participants