Skip to content

fix(error): map vertex/azure credential-validity errors to 401, not 500 (follow-up to #511) - #515

Merged
nic-6443 merged 1 commit into
mainfrom
fix/511-vertex-azure-cred-401
Jun 4, 2026
Merged

fix(error): map vertex/azure credential-validity errors to 401, not 500 (follow-up to #511)#515
nic-6443 merged 1 commit into
mainfrom
fix/511-vertex-azure-cred-401

Conversation

@jarvis9443

@jarvis9443 jarvis9443 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Problem

PR #511 (closing #367) introduced BridgeError::InvalidUpstreamCredentials401 authentication_error for customer-supplied credential problems (empty secret, invalid api-key header bytes, malformed credential JSON), keeping 400 invalid_request_error for request/config shape. But two pre-existing credential checks in Vertex and Azure still mapped to BridgeError::ConfigHTTP 500, inconsistent with the empty-secret/invalid-key cases on the other providers (now 401) and with the issue's intent.

Fix

Credential-material validity → 401 InvalidUpstreamCredentials:

  • VertexSecret::parse — empty secret, malformed JSON, empty access_token, both-modes-set, neither-mode-set.
  • Azure AadCredentials::validate — empty tenant_id/client_id/client_secret, and URL-control characters in tenant_id/client_id.

Config/URL shape → 400 InvalidUpstreamConfig (matching #367's api_base-shape treatment):

  • Azure authority_host guards (must have http(s) scheme, no userinfo/query/fragment, no embedded path).

Runtime token-mint / HTTP token-fetch failures and internal invariants intentionally stay Config → 500 (we caused them, not the customer).

Tests

Unit tests in aisix-provider-vertex (6 secret-parse + chat-before-dispatch) and aisix-provider-azure-openai (empty-field, URL-injection, authority_host shape) updated to assert the new 401/400 classes. Admin schema already rejects an empty secret before dispatch, so these are unit-level (no e2e change). vertex (69) + azure (93) suites green; clippy -D warnings clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved credential validation and error handling for Azure AAD and Vertex authentication providers
    • Enhanced error reporting when credentials are missing, malformed, or otherwise invalid
    • Better validation of authentication configuration endpoints to detect misconfigured values
    • More specific error categorization distinguishes credential issues from configuration problems

…ollow-up to #367/#511)

#511 split customer-fixable upstream config into 400 (invalid_request_error)
vs 401 (authentication_error) for credential material, but two pre-existing
credential checks still mapped to Config → HTTP 500, inconsistent with the
empty-secret/invalid-key cases on the other providers (now 401):

- VertexSecret::parse: empty secret, malformed JSON, empty access_token,
  both-modes-set, neither-mode-set → InvalidUpstreamCredentials (401).
- Azure AAD AadCredentials::validate: empty tenant_id/client_id/client_secret
  and URL-control chars in tenant/client → InvalidUpstreamCredentials (401);
  authority_host URL-shape guards (scheme/userinfo/embedded-path) →
  InvalidUpstreamConfig (400), matching #367's api_base-shape treatment.

Runtime token-mint/HTTP fetch failures and internal invariants stay Config
(500). Unit tests updated to assert the new status classes.
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9f51c45f-c2ce-4140-9ee6-8db835403fb9

📥 Commits

Reviewing files that changed from the base of the PR and between 463ab8d and b20bcc2.

📒 Files selected for processing (2)
  • crates/aisix-provider-azure-openai/src/aad_token_mint.rs
  • crates/aisix-provider-vertex/src/bridge.rs

📝 Walkthrough

Walkthrough

This PR reclassifies credential and configuration validation errors in Azure AAD and Vertex provider implementations from generic BridgeError::Config to more specific variants: BridgeError::InvalidUpstreamCredentials for credential content issues and BridgeError::InvalidUpstreamConfig for configuration format issues. Tests updated correspondingly.

Changes

Credential and Configuration Validation Error Reclassification

Layer / File(s) Summary
Azure AAD credential and authority host validation errors
crates/aisix-provider-azure-openai/src/aad_token_mint.rs
Empty credential fields and URL-control characters in tenant_id/client_id now return InvalidUpstreamCredentials. Authority host validation failures (missing/non-HTTP scheme, embedded path, userinfo/query/fragment) return InvalidUpstreamConfig. Corresponding unit tests updated to assert new error variants.
Vertex secret parsing credential validation errors
crates/aisix-provider-vertex/src/bridge.rs
VertexSecret::parse classifies all credential parsing failures as InvalidUpstreamCredentials: empty secret, JSON deserialization failure, empty access_token, and credential mode violations (both-set, neither-set). Unit tests and async chat tests updated to expect the new error variant and message assertions preserved.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

  • api7/ai-gateway#500: Introduced BridgeError::InvalidUpstreamConfig mapping to HTTP 400 responses, which directly aligns with this PR's reclassification of configuration validation failures to that error type.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: reclassifying credential-validity errors from 500 to 401 in Vertex/Azure providers, with reference to the follow-up PR context.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@nic-6443
nic-6443 merged commit 2d90943 into main Jun 4, 2026
8 checks passed
@nic-6443
nic-6443 deleted the fix/511-vertex-azure-cred-401 branch June 4, 2026 06:08
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