fix(embeddings): classify 403 'not an embeddings model' as model-incompatible + redact API key from errors (#5116) - #5117
Conversation
…mpatible + redact API key from errors (tinyhumansai#5116) - classify_embed_probe: detect model-incompatibility from strong error-message phrasings (e.g. OpenAI 403 'not allowed to generate embeddings from this model') BEFORE the 401/403 auth branch, so a chat model used as an embeddings model no longer misreports as an auth failure. Genuine 401/403 auth still classifies as auth. - Redact sk-*/bearer-token material from the probe detail surfaced to the UI and logs so a partial API key can never leak (no 'sk-' substring survives). - Fix doubled 'API API key' label: use a dedicated apiKeyLabelGeneric i18n key across en + all 13 locales. - Unit tests: 403 model-incompat -> MODEL_INCOMPATIBLE; 401 bad key -> AUTH with key redacted; redaction helper coverage.
📝 WalkthroughWalkthroughEmbeddings verification now identifies model-incompatible 403 responses, redacts API keys and bearer tokens from surfaced details, and preserves authentication classification for genuine 401 failures. The settings UI also replaces the duplicated API label with localized generic API key text. ChangesEmbeddings verification and settings UI
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant EmbeddingsVerifier
participant classify_embed_probe
participant redact_secrets
participant SettingsUI
EmbeddingsVerifier->>classify_embed_probe: classify probe status and error message
classify_embed_probe-->>EmbeddingsVerifier: model incompatibility or auth failure
EmbeddingsVerifier->>redact_secrets: sanitize failure detail
redact_secrets-->>SettingsUI: redacted verification result
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 130d01ec90
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| || lower.contains("not an embedding model") | ||
| || lower.contains("is not an embedding") | ||
| || lower.contains("not supported for embeddings") | ||
| || (lower.contains("unsupported") && lower.contains("embedding")); |
There was a problem hiding this comment.
For custom providers the error detail is wrapped with text like openai embeddings returned ... / Embedding API error ..., so this unsupported && embedding check effectively matches any provider failure containing the word unsupported, such as 415 Unsupported Media Type or unsupported parameter, even when the model is not the problem. In those cases the save dialog now reports EMBEDDINGS_MODEL_INCOMPATIBLE and tells the user to change the model instead of fixing the endpoint/request; please anchor this to a model-specific phrase (for example unsupported model ... embedding) or keep it status/body scoped.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/embeddings/rpc.rs`:
- Around line 1291-1293: Replace the realistic-looking API key literal in
classify_embed_probe_401_bad_key_is_auth_and_redacts_key with an obviously
synthetic fixture such as sk-test-fixture. Preserve the surrounding 401 error
structure and redaction assertion so the test continues exercising the same
behavior without triggering secret scanners.
- Around line 817-830: Update the strong_model_rejection logic in the embedding
error classifier so generic “unsupported” messages only match model-specific
wording, not unsupported embedding dimensions or request options. Preserve the
existing explicit model-rejection phrases, and add a negative test covering an
unsupported embeddings parameter that must not be classified as a bad model.
🪄 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: 40f9878a-4032-49e2-939c-d4a1e026c82f
📒 Files selected for processing (16)
app/src/components/settings/panels/EmbeddingsPanel.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tssrc/openhuman/embeddings/rpc.rs
| let strong_model_rejection = lower.contains("not allowed to generate embeddings") | ||
| || lower.contains("does not support embeddings") | ||
| || lower.contains("not an embedding model") | ||
| || lower.contains("is not an embedding") | ||
| || lower.contains("not supported for embeddings") | ||
| || (lower.contains("unsupported") && lower.contains("embedding")); | ||
| if strong_model_rejection { | ||
| return true; | ||
| } | ||
| let bad_request = | ||
| embed_error_mentions_status(lower, 400) || embed_error_mentions_status(lower, 422); | ||
| bad_request | ||
| && (lower.contains("does not support embeddings") | ||
| || lower.contains("not an embedding model") | ||
| || lower.contains("is not an embedding") | ||
| || lower.contains("does not exist") | ||
| || lower.contains("not supported for embeddings") | ||
| || lower.contains("unexpected model name format")) | ||
| && (lower.contains("does not exist") || lower.contains("unexpected model name format")) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Narrow the broad unsupported classifier.
unsupported embedding dimensions or another unsupported embeddings request option matches line 822 and is incorrectly surfaced as a bad model. Match model-specific wording only.
Proposed fix
- || (lower.contains("unsupported") && lower.contains("embedding"));
+ || lower.contains("unsupported model for embedding")
+ || lower.contains("unsupported embedding model");Add a negative test for an unsupported embeddings parameter.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let strong_model_rejection = lower.contains("not allowed to generate embeddings") | |
| || lower.contains("does not support embeddings") | |
| || lower.contains("not an embedding model") | |
| || lower.contains("is not an embedding") | |
| || lower.contains("not supported for embeddings") | |
| || (lower.contains("unsupported") && lower.contains("embedding")); | |
| if strong_model_rejection { | |
| return true; | |
| } | |
| let bad_request = | |
| embed_error_mentions_status(lower, 400) || embed_error_mentions_status(lower, 422); | |
| bad_request | |
| && (lower.contains("does not support embeddings") | |
| || lower.contains("not an embedding model") | |
| || lower.contains("is not an embedding") | |
| || lower.contains("does not exist") | |
| || lower.contains("not supported for embeddings") | |
| || lower.contains("unexpected model name format")) | |
| && (lower.contains("does not exist") || lower.contains("unexpected model name format")) | |
| } | |
| let strong_model_rejection = lower.contains("not allowed to generate embeddings") | |
| || lower.contains("does not support embeddings") | |
| || lower.contains("not an embedding model") | |
| || lower.contains("is not an embedding") | |
| || lower.contains("not supported for embeddings") | |
| || lower.contains("unsupported model for embedding") | |
| || lower.contains("unsupported embedding model"); | |
| if strong_model_rejection { | |
| return true; | |
| } | |
| let bad_request = | |
| embed_error_mentions_status(lower, 400) || embed_error_mentions_status(lower, 422); | |
| bad_request | |
| && (lower.contains("does not exist") || lower.contains("unexpected model name format")) |
🤖 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/embeddings/rpc.rs` around lines 817 - 830, Update the
strong_model_rejection logic in the embedding error classifier so generic
“unsupported” messages only match model-specific wording, not unsupported
embedding dimensions or request options. Preserve the existing explicit
model-rejection phrases, and add a negative test covering an unsupported
embeddings parameter that must not be classified as a bad model.
| fn classify_embed_probe_401_bad_key_is_auth_and_redacts_key() { | ||
| let detail = r#"openai embeddings returned HTTP 401 Unauthorized: {"error":{"message":"Incorrect API key provided: sk-proj-ABC123def456GHI789jkl012MNO. You can find your API key at https://platform.openai.com/account/api-keys.","type":"invalid_request_error","param":null,"code":"invalid_api_key"}}"#; | ||
| let rpc = classify_embed_probe(EmbedProbe::Failed(detail.into())) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Use an obviously synthetic key fixture.
Betterleaks flags the realistic-looking literal as a generic API key. A short fixture such as sk-test-fixture still exercises redaction without creating secret-scanner noise.
🧰 Tools
🪛 Betterleaks (1.6.1)
[high] 1292-1292: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 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/embeddings/rpc.rs` around lines 1291 - 1293, Replace the
realistic-looking API key literal in
classify_embed_probe_401_bad_key_is_auth_and_redacts_key with an obviously
synthetic fixture such as sk-test-fixture. Preserve the surrounding 401 error
structure and redaction assertion so the test continues exercising the same
behavior without triggering secret scanners.
Source: Linters/SAST tools
2 participants
Summary
gpt-4o-mini) used as an embeddings model no longer misreports "enter a valid key".sk-…/bearer-token material from any probe detail surfaced to the UI and logs — a partial API key can no longer leak (nosk-substring survives).apiKeyLabelGenerici18n key acrossen+ all 13 locales.Problem
Follow-up to #5017 / #5064. Manual QA of the merged embedding-endpoint verification surfaced three bugs:
classify_embed_probe(src/openhuman/embeddings/rpc.rs) checked only HTTP status (401 || 403 → AUTH). OpenAI returns HTTP 403 "You are not allowed to generate embeddings from this model" when a chat model is used as an embeddings model — the key is valid, the model is wrong — but it fell through to the auth branch and told the user to "enter a valid key". This is exactly the scenario Custom OpenAI-compatible embedding endpoint fails verification even with valid endpoint and key #5017 targeted.Incorrect API key provided: sk-proj-…). The frontend appendsresult.detailto the message, so the leak reached the dialog.'API'into the{provider} API keytemplate).Solution
is_embedding_model_incompatiblenow matches strong, status-independent model-rejection phrasings ("not allowed to generate embeddings", "does not support embeddings", "not an embedding model", "unsupported … embedding") that never appear in a genuine auth rejection, so they classify asEMBEDDINGS_MODEL_INCOMPATIBLEahead of the auth branch. Weak phrasings ("does not exist", odd model-name format) stay gated behind a 400/422 bad-request so a genuine 5xx/oversized-input 400 still falls through (preserving Custom OpenAI-compatible embedding endpoint fails verification even with valid endpoint and key #5017 behaviour).redact_secretshelper strips wholesk-[A-Za-z0-9_-]+keys (incl. modernsk-proj-…) andBearer <token>headers, replacing each with a marker that contains nosk-substring. Applied to the probedetailbefore it enters theRpcOutcomebody, so both the UI message and logs are sanitized; the clean classifiedmessageremains the primary user-facing text.'API'into the provider template.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/ci-lite.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge.docs/TEST-COVERAGE-MATRIX.mdreflect this change (orN/A: behaviour-only change) — N/A: behaviour-only change (hardens existing embeddings-verification classification; no new feature row).## Relateddocs/RELEASE-MANUAL-SMOKE.md) — N/A: no release-cut surface changed (error-message classification + redaction + a label string only).Closes #NNNin the## RelatedsectionImpact
sk-…) from a user-facing error and from logs.regex/once_cellalready vendored).Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— Prettier clean on touched filespnpm typecheck— passGGML_NATIVE=OFF cargo test -p openhuman --lib embeddings::rpc— 22 passed (incl. 3 new)cargo fmt --checkonrpc.rs— cleanpnpm i18n:checkandpnpm i18n:english:check— both pass (0 missing / 0 extra / 0 unexpected English)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
EMBEDDINGS_AUTH_FAILED; the 400/422-gated weak phrasings (Custom OpenAI-compatible embedding endpoint fails verification even with valid endpoint and key #5017) are unchanged;detailis still returned (now sanitized).rejectbody-construction site.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Localization