Repository navigation
🛡️ fix: Pin Agent Runs to the Validated Model - #16800
basnijholt wants to merge 2 commits into
Conversation
Agent model_parameters are free-form. A saved top-level fallbacks list, model routing keys in modelKwargs, or a Google apiVersion could change which model, endpoint or credentials an agent run uses, outside what validateAgentModel and the server configuration allow. initializeAgent now pins provider options to agent.model: it drops a top-level fallbacks list and the modelKwargs keys that select another model (model, models, route, fallbacks); other options pass through. The Google config builder also drops apiVersion alongside the other transport-level keys it already blocks.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The modelName alias can still retain an unvalidated model selection.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Pins agent runs to validated models and blocks unsafe provider overrides.
Changes:
- Removes fallback and nested model-routing overrides.
- Blocks Google
apiVersionoverrides. - Adds regression coverage.
| File | Description |
|---|---|
packages/api/src/agents/pin.ts |
Adds model-pinning sanitization. |
packages/api/src/agents/initialize.ts |
Applies sanitization during initialization. |
packages/api/src/agents/__tests__/initialize.test.ts |
Tests routing-key removal. |
packages/api/src/endpoints/google/llm.ts |
Blocks client-provided API versions. |
packages/api/src/endpoints/google/llm.spec.ts |
Tests API-version filtering. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| type PinnedModelOptions<T, M> = Omit<T, 'fallbacks'> & { model: M }; | ||
|
|
||
| /** | ||
| * Pins an agent's provider options to its validated model. `model_parameters` is free-form, so | ||
| * a saved agent could otherwise pick models that `validateAgentModel` never checked against the | ||
| * user's allowed models: a top-level `fallbacks` list makes the agents runtime build a second | ||
| * client from user-chosen provider, model and baseURL (with the server's credentials) when the | ||
| * primary call fails, and `modelKwargs` routing keys redirect the provider request itself. | ||
| */ | ||
| export function pinAgentModel<T extends Record<string, unknown>, M>( | ||
| modelOptions: T, | ||
| model: M, | ||
| ): PinnedModelOptions<T, M> { | ||
| const { fallbacks: _fallbacks, ...options } = modelOptions; |
|
@berry-13 @danny-avila, since I'm having a hard time getting my PRs merged here, I will not solve any of the issues suggested by the AI reviewer. I would just suggest to take a look here because it solves a real security issue. Feel free to push to my branch. |
codegraph-librechat
Bot
added
the
🗺️ Backend Infra
|
Head |
|
Local inference-boundary candidate: Not published: the App push was rejected because merging upstream |
Summary
This fixes a model-authorization bypass in agent runs, with a potential credential and conversation-data disclosure path through user-supplied fallback clients. LibreChat validates
agent.modelagainst the user's allowed models, but free-formmodel_parameterscan change what the downstream client actually calls after that check. Permission to configure an agent must not grant permission to choose an unchecked model, provider, or destination for requests made by the server.For example, an agent can declare an allowed model such as
gpt-4o-miniwhile settingmodel_parameters.modelKwargs.modelto a restricted model such asgpt-4o. Validation accepts the declared model, but the OpenAI-compatible client spreadsmodelKwargsinto the request after its normalmodelfield, replacing the validated value. This PR removes that override before provider configuration so the request uses the validated model.Security impact and prerequisites
The relevant attacker is a user who can supply an agent's model parameters and cause that agent to run. The agent create/update schema accepts these parameters as a free-form record, so controls in the model picker alone do not enforce the boundary.
modelKwargs.modelmodelKwargs.models,route, orfallbacksfallbacksOPENAI_API_KEY, it can send that credential to the user-chosen endpoint. User-selected destinations also create a server-side request forgery risk, subject to network egress controls.apiVersionapiVersion.Credential disclosure is conditional on fallback activation, SDK credential resolution, and endpoint reachability. The PR's regression tests verify option filtering; they do not demonstrate a live credential-exfiltration attack.
How the fix closes these paths
Previously,
initializeAgentonly overwrote the top-levelmodel. That left nested request overrides and alternate fallback clients intact. It now callspinAgentModelbefore handing parameters to the provider configuration builder:Removing the nested routing fields and the top-level
modelNamealias prevents these parameters from replacing or routing around the validated model. Removing the top-level fallback list prevents these parameters from creating an unchecked alternate client when the primary request fails. The Google builder separately stripsapiVersionalongside its existing blocked transport options. Unrelated options, such astemperatureandservice_tier, remain available.The filtering runs when an agent is initialized, so it also applies to existing saved agents without a data migration. This is a targeted fix for the listed override paths, not a claim that all provider-specific routing options or outbound-request paths have been audited.
Type of change
Testing
Added regression coverage in:
packages/api/src/agents/__tests__/initialize.test.ts: removes top-level fallbacks and nested routing keys, preventsmodelKwargs.modelfrom replacing the validated model, and preserves unrelated options.packages/api/src/endpoints/google/llm.spec.ts: ignores a model-optionapiVersionoverride while preserving the selected model.Follow-up head
01aed014ec3658ad59faa4de42a69c3984ee056a: 12 focused pin/provider/initial-and-child regressions passed; 187 unrelated tests were filtered out. The API build and scoped lint/import/format checks passed.npx tsc --noEmitwas run but remains blocked by four missing Lodash declaration errors in unchanged files; changed fixture errors are cleared. No live provider requests were run. Exact-head CI and independent review are pending.Screenshots / recordings
No user-facing UI change.
Compatibility risk
Saved agents that rely on the removed fallback or routing fields will lose that behavior: they will use their validated model, and a primary-call failure will no longer activate a fallback supplied through those fields. Google
apiVersionoverrides supplied through model options are also ignored. No schema migration or dependency change is included.