Skip to content

fix: strip whitespace from oauth header values#1361

Closed
0xsirsaif wants to merge 1 commit intoMoonshotAI:mainfrom
0xsirsaif:fix/trim-invalid-oauth-header-values
Closed

fix: strip whitespace from oauth header values#1361
0xsirsaif wants to merge 1 commit intoMoonshotAI:mainfrom
0xsirsaif:fix/trim-invalid-oauth-header-values

Conversation

@0xsirsaif
Copy link
Copy Markdown

@0xsirsaif 0xsirsaif commented Mar 7, 2026

Related Issue

Resolve #886, ##414

Description

Strip leading and trailing whitespace from OAuth/common header values before sending requests. This fixes X-Msh-Os-Version when platform.version() includes trailing whitespace and prevents httpx.LocalProtocolError.

Observed in logs:

httpcore.LocalProtocolError: Illegal header value b'#101~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Feb 11 13:19:54 UTC '
httpx.LocalProtocolError: Illegal header value b'#101~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Feb 11 13:19:54 UTC '
openai.APIConnectionError: Connection error.
kosong.chat_provider.APIConnectionError: Connection error.

Checklist


Open with Devin

Copilot AI review requested due to automatic review settings March 7, 2026 01:53
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR prevents invalid HTTP header values by stripping leading/trailing whitespace from the OAuth “common” headers before requests are sent, addressing httpx/httpcore.LocalProtocolError failures caused by platform strings with trailing spaces.

Changes:

  • Strip leading/trailing whitespace in _ascii_header_value() and ensure empty/whitespace-only values fall back to "unknown".
  • Apply _ascii_header_value() normalization across all _common_headers() values.
  • Add tests covering whitespace stripping and the X-Msh-Os-Version trailing-whitespace regression.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/kimi_cli/auth/oauth.py Normalizes header values by trimming whitespace and applying a fallback for empty results.
tests/utils/test_utils_environment.py Adds regression/unit tests validating header value trimming and the X-Msh-Os-Version fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +46 to +52
def test_ascii_header_value_strips_ascii_whitespace():
assert _ascii_header_value(" value ") == "value"
assert _ascii_header_value(" ") == "unknown"


def test_common_headers_strip_os_version(monkeypatch):
monkeypatch.setattr(platform, "node", lambda: "host")
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

The new OAuth header sanitization tests are placed in test_utils_environment.py, which makes them harder to discover/maintain since they are unrelated to environment detection. Consider moving these tests into a dedicated auth/oauth test module (e.g., tests/auth/test_oauth_headers.py or similar) and keeping this file focused on Environment.detect().

Copilot uses AI. Check for mistakes.
@renatocron
Copy link
Copy Markdown

Fixed on 1.20.0 via #1401

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.

LLM provider error: Connection error.

3 participants