fix(inference): validate OpenRouter API keys#2372
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughOpenRouter API key trimming and a pre-flight validation (GET {base}/key with Bearer token) were added; list_configured_models calls this validation for OpenRouter providers and aborts before requesting /models on validation failure. Tests and a local probe server exercise detection, trimming, and success/failure flows. ChangesOpenRouter API Key Validation
Sequence DiagramsequenceDiagram
participant Client
participant list_configured_models
participant is_openrouter_provider
participant validate_openrouter_api_key
participant OpenRouter_API
Client->>list_configured_models: request configured models
list_configured_models->>is_openrouter_provider: check provider slug/host
is_openrouter_provider-->>list_configured_models: true
list_configured_models->>validate_openrouter_api_key: GET {base}/key (Bearer trimmed-token)
validate_openrouter_api_key->>OpenRouter_API: GET /key (Authorization: Bearer ...)
alt Key Valid
OpenRouter_API-->>validate_openrouter_api_key: 200 OK (no error)
validate_openrouter_api_key-->>list_configured_models: success
list_configured_models->>OpenRouter_API: GET /models (Authorization: Bearer trimmed-token)
OpenRouter_API-->>list_configured_models: models list
list_configured_models-->>Client: models
else Key Invalid
OpenRouter_API-->>validate_openrouter_api_key: 4xx / error JSON
validate_openrouter_api_key-->>list_configured_models: sanitized error
list_configured_models-->>Client: early error (no /models request)
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/inference/provider/ops.rs (1)
223-231: ⚡ Quick winAdd debug logs on OpenRouter key-validation failure branches.
You already log validation start, but non-2xx and error-payload exits return without a debug/trace breadcrumb. Add sanitized status/message logs before returning.
Suggested refactor
if !status.is_success() { let sanitized = sanitize_api_error(&text); let truncated = crate::openhuman::util::truncate_with_ellipsis(&sanitized, 300); + log::debug!( + "[providers][list_models] OpenRouter key validation failed status={} body={}", + status.as_u16(), + truncated + ); return Err(format!( "OpenRouter key validation returned {}: {}", status.as_u16(), truncated )); @@ let sanitized = sanitize_api_error(&msg); + log::debug!( + "[providers][list_models] OpenRouter key validation returned error payload={}", + sanitized + ); return Err(format!( "OpenRouter key validation returned error payload: {}", sanitized ));As per coding guidelines: "Use
log/tracingatdebugortracelevel on RPC entry and exit, error paths, state transitions, and any branch that is hard to infer from tests alone."Also applies to: 246-250
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/inference/provider/ops.rs` around lines 223 - 231, The non-2xx and error-payload exit paths lack debug breadcrumbs; before returning in the branches that use status and text (the block that calls sanitize_api_error(&text) and crate::openhuman::util::truncate_with_ellipsis(&sanitized, 300)), add a debug (or tracing::debug!) log that includes the sanitized-and-truncated message and the HTTP status (e.g., "OpenRouter key validation failed" with status.as_u16() and truncated), and do the same for the similar branch around lines 246-250 so both error-return paths emit a sanitized debug log to aid tracing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/inference/provider/ops.rs`:
- Around line 202-217: In validate_openrouter_api_key, normalize the API key
once and reuse that normalized value for all requests (both the "/key" and
subsequent "/models" call) to prevent whitespace-related mismatches: replace the
current trim usage with a single owned/normalized variable (e.g.,
normalized_key) and use that variable in the Authorization header(s) instead of
the original api_key parameter or the prior trimmed temporary so both requests
use the exact same credential.
---
Nitpick comments:
In `@src/openhuman/inference/provider/ops.rs`:
- Around line 223-231: The non-2xx and error-payload exit paths lack debug
breadcrumbs; before returning in the branches that use status and text (the
block that calls sanitize_api_error(&text) and
crate::openhuman::util::truncate_with_ellipsis(&sanitized, 300)), add a debug
(or tracing::debug!) log that includes the sanitized-and-truncated message and
the HTTP status (e.g., "OpenRouter key validation failed" with status.as_u16()
and truncated), and do the same for the similar branch around lines 246-250 so
both error-return paths emit a sanitized debug log to aid tracing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b881768-60da-4254-a40c-a45b9ce559b0
📒 Files selected for processing (1)
src/openhuman/inference/provider/ops.rs
Signed-off-by: sunilkumarvalmiki <g.sunilkumarvalmiki@gmail.com>
8f394f7 to
43957d8
Compare
|
Updated the branch to address the review feedback:
Re-run locally after the update: cargo fmt --manifest-path Cargo.toml --all --check
# passed
cargo test --manifest-path Cargo.toml openrouter_ --lib -- --nocapture
# 5 passed; 0 failed
cargo check --manifest-path Cargo.toml --lib
# passed with existing warnings
git diff --check
# passed |
|
CI status after the amended head ( The failing job is outside this PR's touched surface and fails in Tauri shell core-process tests: The separate |
Signed-off-by: sunilkumarvalmiki <g.sunilkumarvalmiki@gmail.com>
Summary
/modelsis public.GET /keycredential check before the existing model catalog probe./modelsas the compatibility/catalog probe after credentials are accepted.Problem
OpenRouter's
GET /api/v1/modelsendpoint returns200even when the request has no bearer token or an invalid bearer token. The AI settings flow usesopenhuman.inference_list_modelsto verify a newly configured provider, and that controller previously treated the public catalog response as proof that the API key was usable.Reproduction evidence from the current behavior:
Solution
openrouter.aiendpoint host./keyand/modelsrequests.GET {endpoint}/keybefore callingGET {endpoint}/models./modelsprobe path./modelsis never called after/keyrejects the credential.Submission Checklist
## Related: N/A, no matrix feature ID applies to this provider-validation fix.docs/RELEASE-MANUAL-SMOKE.md): N/A, not a release-cut smoke surface.Closes #NNNin the## Relatedsection.Impact
Invalid OpenRouter API keys now fail during provider setup instead of enabling the provider. Valid OpenRouter keys still proceed to the existing model catalog probe. Other providers keep their existing model-list validation behavior.
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/openrouter-key-validation-236643957d83Validation Run
pnpm --filter openhuman-app format:check: blocked locally; seeValidation Blocked.pnpm typecheck: passed.cargo test --manifest-path Cargo.toml openrouter_ --lib -- --nocapturepassed, 5 tests;pnpm --filter openhuman-app test -- src/components/settings/panels/__tests__/AIPanel.test.tsx src/services/api/__tests__/aiSettingsApi.test.tspassed, 2 files / 60 tests.cargo fmt --manifest-path Cargo.toml --all --checkpassed;cargo check --manifest-path Cargo.toml --libpassed with existing warnings.Validation Blocked
command:pnpm format:check/ pre-push hook runningpnpm --filter openhuman-app format:checkerror:on this Windows checkout, Prettier reports 902 untouched app files due CRLF working-tree line endings. The pre-push hook then triedpnpm formatand failed in Rust formatting becauseapp/src-tauri/vendor/tauri-cef/crates/tauri/Cargo.tomlis missing locally.impact:the hook-generated app rewrites were restored and are not in this commit. The committed Rust change passedcargo fmt --manifest-path Cargo.toml --all --check,git diff --check, focused Rust tests, TypeScript typecheck, and the related AI settings Vitest tests.Behavior Changes
/keybefore the provider is accepted.Parity Contract
/modelscatalog probe still runs after OpenRouter credentials pass, and non-OpenRouter providers keep the prior/modelsprobe behavior.Duplicate / Superseded PR Handling
#2366via open PR search and issue closing-reference check.Summary by CodeRabbit
Bug Fixes
Tests