Skip to content

Fix Mass Assignment on Save Custom Template #6059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
christopherholland-workday wants to merge 1 commit into from
+23 −7
Closed

Fix Mass Assignment on Save Custom Template #6059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions packages/server/src/controllers/marketplaces/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,31 @@ const getAllCustomTemplates = async (req: Request, res: Response, next: NextFunc

const saveCustomTemplate = async (req: Request, res: Response, next: NextFunction) => {
try {
if ((!req.body && !(req.body.chatflowId || req.body.tool)) || !req.body.name) {
const body = req.body
if (!body || !(body.chatflowId || body.tool) || !body.name) {
throw new InternalFlowiseError(
StatusCodes.PRECONDITION_FAILED,
`Error: marketplacesService.saveCustomTemplate - body not provided!`
)
}
const body = req.body
body.workspaceId = req.user?.activeWorkspaceId
if (!body.workspaceId) {
const workspaceId = req.user?.activeWorkspaceId
if (!workspaceId) {
throw new InternalFlowiseError(
StatusCodes.NOT_FOUND,
`Error: marketplacesController.saveCustomTemplate - workspace ${body.workspaceId} not found!`
`Error: marketplacesController.saveCustomTemplate - workspace ${workspaceId} not found!`
)
}
const apiResponse = await marketplacesService.saveCustomTemplate(body)
// Explicit allowlist — id/workspaceId/timestamps must not be overrideable by client
const templateBody: Record<string, unknown> = {}
if (body.name !== undefined) templateBody.name = body.name
if (body.description !== undefined) templateBody.description = body.description
if (body.badge !== undefined) templateBody.badge = body.badge
if (body.usecases !== undefined) templateBody.usecases = body.usecases
if (body.type !== undefined) templateBody.type = body.type
if (body.chatflowId !== undefined) templateBody.chatflowId = body.chatflowId
if (body.tool !== undefined) templateBody.tool = body.tool
Comment on lines +64 to +71
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

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

medium

The explicit allow-listing of properties is great for security. To make this more maintainable and less verbose, you could define an array of allowed keys and loop through them to build the templateBody object. This would make it easier to add or remove allowed properties in the future.

Suggested change
const templateBody: Record<string, unknown> = {}
if (body.name !== undefined) templateBody.name = body.name
if (body.description !== undefined) templateBody.description = body.description
if (body.badge !== undefined) templateBody.badge = body.badge
if (body.usecases !== undefined) templateBody.usecases = body.usecases
if (body.type !== undefined) templateBody.type = body.type
if (body.chatflowId !== undefined) templateBody.chatflowId = body.chatflowId
if (body.tool !== undefined) templateBody.tool = body.tool
const allowedFields = ['name', 'description', 'badge', 'usecases', 'type', 'chatflowId', 'tool'];
const templateBody: Record<string, unknown> = {};
for (const field of allowedFields) {
if (body[field] !== undefined) {
templateBody[field] = body[field];
}
}
References
  1. This suggestion improves the maintainability and readability of explicitly mapping allowed properties from a request body, which is a best practice to avoid mass assignment vulnerabilities as per the rule 'Avoid mass assignment from request bodies to entities'.

templateBody.workspaceId = workspaceId
const apiResponse = await marketplacesService.saveCustomTemplate(templateBody)
return res.json(apiResponse)
} catch (error) {
next(error)
Expand Down
8 changes: 7 additions & 1 deletion packages/server/src/services/marketplaces/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,13 @@ const saveCustomTemplate = async (body: any): Promise<any> => {
let flowDataStr = ''
let derivedFramework = ''
const customTemplate = new CustomTemplate()
Object.assign(customTemplate, body)
Copy link
Copy Markdown
Contributor

@0xi4o 0xi4o Apr 1, 2026

Choose a reason for hiding this comment

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

Is this change still required since we're overwriting body.workspaceId in the controller?

// Explicit assignment — never use Object.assign with untrusted body
customTemplate.name = body.name
customTemplate.workspaceId = body.workspaceId
if (body.description !== undefined) customTemplate.description = body.description
if (body.badge !== undefined) customTemplate.badge = body.badge
if (body.usecases !== undefined) customTemplate.usecases = body.usecases
if (body.type !== undefined) customTemplate.type = body.type
Comment on lines +214 to +217
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

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

medium

Similar to the controller, you can make this block more concise and maintainable by iterating over a list of optional fields. This approach makes it easier to manage which fields are assignable.

        const optionalFields: (keyof CustomTemplate)[] = ['description', 'badge', 'usecases', 'type'];
        for (const field of optionalFields) {
            if (body[field] !== undefined) {
                customTemplate[field] = body[field];
            }
        }
References
  1. This suggestion improves the maintainability and readability of explicitly mapping allowed properties from a request body, which is a best practice to avoid mass assignment vulnerabilities as per the rule 'Avoid mass assignment from request bodies to entities'.


if (body.chatflowId) {
const chatflow = await chatflowsService.getChatflowById(body.chatflowId, body.workspaceId)
Expand Down
Loading