🛡️ fix: Filter user_provided Sentinel in Tool Credential Loading - #12840
danny-avila merged 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a credential-loading edge case where the user_provided sentinel (and empty/whitespace env values) could be incorrectly treated as a real tool API key, aligning loadAuthValues() behavior with existing auth validation logic.
Changes:
- Update
findAuthValue()to skip empty/whitespace env values and theAuthType.USER_PROVIDEDsentinel. - Add Jest regression tests covering sentinel/whitespace behavior and fallback-chain selection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
api/server/services/Tools/credentials.js |
Filters out invalid env values (whitespace / user_provided) during auth resolution. |
api/server/services/Tools/credentials.spec.js |
Adds regression tests for env/user DB fallback behavior around the sentinel. |
Comments suppressed due to low confidence (1)
api/server/services/Tools/credentials.js:39
findAuthValue()can still return a rejected env value (e.g.,AuthType.USER_PROVIDEDor whitespace) ifgetUserPluginAuthValue()throws whilevaluestill contains the env value. In that case thecatchblock doesn’t clearvalue, the!valuecheck is false, and the subsequentif (value)returns the sentinel/whitespace. Consider resettingvaluebefore thetry, or using a separateenvValuevariable and applying the same filtering to the post-DBvalue(and ensuring the catch/last-field logic uses the DB result, not the env value).
const findAuthValue = async (fields) => {
for (const field of fields) {
let value = process.env[field];
if (value && value.trim() !== '' && value !== AuthType.USER_PROVIDED) {
return { authField: field, authValue: value };
}
try {
value = await getUserPluginAuthValue(userId, field, throwError);
} catch (err) {
if (optional && optional.has(field)) {
return { authField: field, authValue: undefined };
}
if (field === fields[fields.length - 1] && !value) {
throw err;
}
}
if (value) {
return { authField: field, authValue: value };
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bf60fef to
63079be
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When GOOGLE_KEY=user_provided is set as an endpoint config, the loadAuthValues() function in credentials.js would pass the literal string 'user_provided' to tools via the || fallback chain. This caused Gemini Image Tools to fail at runtime with an invalid API key error, as initializeGeminiClient() received the sentinel value instead of a real key. The fix aligns loadAuthValues() with checkPluginAuth() in format.ts, which already correctly excludes user_provided and empty/whitespace values. Now loadAuthValues() skips these values and continues to the next field in the fallback chain or falls through to user DB values. Added regression tests covering: - user_provided sentinel is skipped, DB value used instead - Fallback chain continues past user_provided to next field - Empty and whitespace env values are skipped - Real env values are returned correctly - Optional fields with sentinel values handled gracefully
63079be to
b4f0670
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
api/server/services/Tools/credentials.js:40
findAuthValue()now filters empty/whitespace andAuthType.USER_PROVIDEDfor env vars, but DB values returned bygetUserPluginAuthValue()are still accepted with a simple truthy check (if (value)). This means a whitespace-only DB value (e.g.' ') will be treated as a valid credential and returned, which can still cause downstream “invalid API key” failures and contradicts the intent of “first non-empty value”. Consider applying the sametrim() !== ''(and optionally!== AuthType.USER_PROVIDED) filtering to DB values before returning, so whitespace/sentinel stored values don’t short-circuit the fallback chain.
const envValue = process.env[field];
if (envValue && envValue.trim() !== '' && envValue !== AuthType.USER_PROVIDED) {
return { authField: field, authValue: envValue };
}
let value;
try {
value = await getUserPluginAuthValue(userId, field, throwError);
} catch (err) {
if (optional && optional.has(field)) {
return { authField: field, authValue: undefined };
}
if (field === fields[fields.length - 1]) {
throw err;
}
}
if (value) {
return { authField: field, authValue: value };
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
user_provided Sentinel in Tool Credential Loading
…ibreChat-AI#12840) When GOOGLE_KEY=user_provided is set as an endpoint config, the loadAuthValues() function in credentials.js would pass the literal string 'user_provided' to tools via the || fallback chain. This caused Gemini Image Tools to fail at runtime with an invalid API key error, as initializeGeminiClient() received the sentinel value instead of a real key. The fix aligns loadAuthValues() with checkPluginAuth() in format.ts, which already correctly excludes user_provided and empty/whitespace values. Now loadAuthValues() skips these values and continues to the next field in the fallback chain or falls through to user DB values. Added regression tests covering: - user_provided sentinel is skipped, DB value used instead - Fallback chain continues past user_provided to next field - Empty and whitespace env values are skipped - Real env values are returned correctly - Optional fields with sentinel values handled gracefully
…ibreChat-AI#12840) When GOOGLE_KEY=user_provided is set as an endpoint config, the loadAuthValues() function in credentials.js would pass the literal string 'user_provided' to tools via the || fallback chain. This caused Gemini Image Tools to fail at runtime with an invalid API key error, as initializeGeminiClient() received the sentinel value instead of a real key. The fix aligns loadAuthValues() with checkPluginAuth() in format.ts, which already correctly excludes user_provided and empty/whitespace values. Now loadAuthValues() skips these values and continues to the next field in the fallback chain or falls through to user DB values. Added regression tests covering: - user_provided sentinel is skipped, DB value used instead - Fallback chain continues past user_provided to next field - Empty and whitespace env values are skipped - Real env values are returned correctly - Optional fields with sentinel values handled gracefully
…ibreChat-AI#12840) When GOOGLE_KEY=user_provided is set as an endpoint config, the loadAuthValues() function in credentials.js would pass the literal string 'user_provided' to tools via the || fallback chain. This caused Gemini Image Tools to fail at runtime with an invalid API key error, as initializeGeminiClient() received the sentinel value instead of a real key. The fix aligns loadAuthValues() with checkPluginAuth() in format.ts, which already correctly excludes user_provided and empty/whitespace values. Now loadAuthValues() skips these values and continues to the next field in the fallback chain or falls through to user DB values. Added regression tests covering: - user_provided sentinel is skipped, DB value used instead - Fallback chain continues past user_provided to next field - Empty and whitespace env values are skipped - Real env values are returned correctly - Optional fields with sentinel values handled gracefully
Summary
Fixes #12839
When
GOOGLE_KEY=user_providedis set as an endpoint configuration (a documented sentinel value meaning "let users provide their own key via UI"),loadAuthValues()incredentials.jspasses the literal string"user_provided"to tools through the||fallback chain.This causes Gemini Image Tools to fail at runtime with an invalid API key error, because
initializeGeminiClient()receives the sentinel value instead of a real key and attemptsnew GoogleGenAI({ apiKey: 'user_provided' }).The root cause is an inconsistency:
checkPluginAuth()informat.tsalready correctly excludesuser_providedand empty/whitespace values, butloadAuthValues()does not apply the same filtering. This fix aligns the two functions.What changed
api/server/services/Tools/credentials.js— ThefindAuthValue()function now:const envValue) from DB lookup (let value) to prevent the sentinel from leaking through the catch path whengetUserPluginAuthValuethrowsuser_providedsentinel, consistent withcheckPluginAuth()informat.ts||chainapi/server/services/Tools/credentials.spec.js— New test file with 9 regression tests covering: real env values, sentinel skipping, fallback chains, empty/whitespace filtering, optional fields, env short-circuit (no DB call), and the catch-path sentinel leak.Change Type
Testing
Steps to reproduce the bug:
GOOGLE_KEY=user_providedin.env"user_provided"passed as literal API keySteps to verify the fix:
loadAuthValues()now skips the sentinel, falls through to user DB value or next field in chainUnit tests:
Existing test suites (148 suites, 3852 tests) continue to pass.
Test Configuration
Checklist