Skip to content

Mass Assignment in Assistant Update Endpoint#5945

Merged
yau-wd merged 9 commits intomainfrom
flowise-303
Mar 31, 2026
Merged

Mass Assignment in Assistant Update Endpoint#5945
yau-wd merged 9 commits intomainfrom
flowise-303

Conversation

@christopherholland-workday
Copy link
Copy Markdown
Contributor

Flowise-303

@FlowiseAI FlowiseAI deleted a comment from gemini-code-assist bot Mar 11, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively addresses a critical mass assignment vulnerability in the assistant update endpoint. By replacing Object.assign and merge with an explicit allowlist of updatable fields, it prevents unauthorized modifications to sensitive properties like workspaceId, mitigating cross-workspace resource reassignment. However, the credential field still lacks ownership verification, which could lead to IDOR, and the iconSrc field requires validation to prevent stored XSS. Furthermore, the validation for the details field could be enhanced to explicitly handle null parsing.

Comment on lines +310 to +312
if (assistant.type === 'CUSTOM' && !parsedDetails?.name) {
throw new InternalFlowiseError(StatusCodes.PRECONDITION_FAILED, `Details must include a name field`)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current validation could be more precise. If requestBody.details is the string "null", it will be parsed into a null value. For a 'CUSTOM' assistant, this would incorrectly trigger the "Details must include a name field" error because !null?.name evaluates to true. It would be better to explicitly check for null after parsing to provide a more accurate error message. This also allows for safer property access on parsedDetails in the subsequent check.

Suggested change
if (assistant.type === 'CUSTOM' && !parsedDetails?.name) {
throw new InternalFlowiseError(StatusCodes.PRECONDITION_FAILED, `Details must include a name field`)
}
if (parsedDetails === null) {
throw new InternalFlowiseError(StatusCodes.PRECONDITION_FAILED, `Details JSON cannot be null`)
}
if (assistant.type === 'CUSTOM' && !parsedDetails.name) {
throw new InternalFlowiseError(StatusCodes.PRECONDITION_FAILED, `Details must include a name field`)
}

@christopherholland-workday christopherholland-workday changed the title Mass Assignment in Assistant Update Endpoint Allows Cross-Workspace Resource Reassignment Mass Assignment in Assistant Update Endpoint Mar 23, 2026
@yau-wd yau-wd merged commit faa571c into main Mar 31, 2026
7 checks passed
@yau-wd yau-wd deleted the flowise-303 branch March 31, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants