Skip to content

fix: bind CosyVoice request authentication to its provider - #10100

Open
xiongyuyang wants to merge 1 commit into
AstrBotDevs:masterfrom
xiongyuyang:fix/dashscope-tts-request-auth
Open

xiongyuyang wants to merge 1 commit into
AstrBotDevs:masterfrom
xiongyuyang:fix/dashscope-tts-request-auth

Conversation

@xiongyuyang

@xiongyuyang xiongyuyang commented Sep 15, 2026

Copy link
Copy Markdown

Modifications

Creating a second DashScope TTS provider overwrites dashscope.api_key. A later CosyVoice request from the first provider then sends the second provider's bearer token, potentially causing authentication failures or charging the wrong account.

Set an explicit per-request Authorization header from the selected provider's key. Remove case-insensitive custom Authorization entries before adding it, avoiding duplicate authentication headers. Retain SDK initialization compatibility: the installed SpeechSynthesizer still requires the global key during construction, but its public headers argument overrides the generated bearer header for the outgoing request. Qwen already receives its key explicitly.

  • This is NOT a breaking change.

Screenshots or Test Results

python -m pytest tests/unit/test_dashscope_tts_auth.py tests/test_dashscope_embedding_source.py -q -p no:cacheprovider

22 passed, 2 dependency deprecation warnings on Windows/Python 3.12. The original two-provider regression failed before the fix. Tests inspect headers generated by the real SDK, mock only synthesis execution, exercise both providers concurrently and cover custom header casing plus Qwen's existing key isolation. No paid API was called.

Ruff check and commit hooks passed.

Checklist

  • Tested, with verification steps and results above.
  • No new dependencies.
  • No malicious code.
  • New feature discussion: not applicable (bug fix).

Summary by Sourcery

Ensure DashScope TTS requests consistently authenticate with the API key of the provider that issued them.

Bug Fixes:

  • Bind CosyVoice authentication to the selected provider’s API key for every request, preventing cross-provider credential leakage and incorrect account usage.
  • Remove existing Authorization headers before applying the provider-specific bearer token to ensure a single effective authentication header.

Tests:

  • Add coverage for concurrent CosyVoice providers, custom Authorization header casing, and Qwen credential isolation.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the header override is wrong, CosyVoice requests could be authenticated with the wrong provider key, causing failures or usage to be attributed to another account and potentially exposing requests to that account. Reverting restores the previous behavior but cannot undo requests or billing effects that already occurred.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@kilisamemarisaaa kilisamemarisaaa 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.

Reviewed at head d32ff9a. The CosyVoice path removes all case variants of a caller-supplied Authorization header before adding the selected provider's bearer token, while preserving other custom headers and the SDK's construction-time global-key compatibility. The exact-head Windows/Python 3.12 focused suite passed 22 tests (5 dependency warnings); Ruff passed. The tests inspect headers produced by the real SDK and cover concurrent providers, stale custom header casing, and Qwen's existing per-provider key path. No credential-isolation blocker found.

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.

2 participants