fix: ChatOpenAI baseOptions additional parameters not applied to model#6174
fix: ChatOpenAI baseOptions additional parameters not applied to model#6174BOOPATB wants to merge 2 commits intoFlowiseAI:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the ChatOpenAI component to allow more flexible model parameter configuration via the baseOptions field and reorders the initialization logic to ensure reasoning model constraints are applied correctly. I have identified a security risk regarding mass assignment from user-provided JSON and a concern regarding inconsistent indentation introduced in the refactor.
| const { defaultHeaders, baseURL, ...modelParams } = parsedBaseOptions | ||
| // Spread extra model params (e.g. stop, top_p) into obj | ||
| Object.assign(obj, modelParams) |
There was a problem hiding this comment.
Avoid mass assignment from user-provided JSON to the model configuration object. Directly spreading modelParams into obj allows users to overwrite sensitive fields like apiKey or openAIApiKey, which are intended to be managed via credentials. Please explicitly exclude these sensitive fields to prevent potential security issues or unexpected behavior.
| const { defaultHeaders, baseURL, ...modelParams } = parsedBaseOptions | |
| // Spread extra model params (e.g. stop, top_p) into obj | |
| Object.assign(obj, modelParams) | |
| const { defaultHeaders, baseURL, apiKey, openAIApiKey, configuration, ...modelParams } = parsedBaseOptions | |
| // Spread extra model params (e.g. stop, top_p) into obj | |
| Object.assign(obj, modelParams) |
References
- Avoid mass assignment from request bodies to entities. Instead of using a generic assignment like Object.assign(entity, body), explicitly map allowed properties.
|
Fixes #2499 |
Problem
Fixes #2499
BaseOptionsJSON input in the ChatOpenAI node was only being passed asdefaultHeadersinsideconfiguration, meaning any model-level parameters(e.g.
stop,top_p) were silently ignored and never sent to the API.Root Cause
In
ChatOpenAI.ts,parsedBaseOptionswas unconditionally assigned toconfiguration.defaultHeaders, with no path to merge params into the model object.Additionally, the merge happened after the reasoning model cleanup block, meaning
it could have re-introduced
stop/temperaturethat were intentionally deleted.Fix
parsedBaseOptionsinto{ defaultHeaders, baseURL, ...modelParams }modelParamsdirectly intoobjso params likestopreach the modelbaseOptionsmerge to before the reasoning model block so cleanup still works correctlyBaseOptionsfield description to reflect its actual capabilityTesting
{ "stop": ["<|im_end|>"] }in BaseOptions on a ChatOpenAI nodeo-series) still correctly stripstop/temperaturedefaultHeadersandbaseURLinside BaseOptions still work for custom endpoints