Skip to content

Commit

Permalink
fix: Validate custom tool names for forbidden chars (#8878)
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-radency authored Mar 14, 2024
1 parent 4861556 commit edce632
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 17 deletions.
34 changes: 20 additions & 14 deletions cypress/e2e/5-ndv.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { v4 as uuid } from 'uuid';
import { getVisibleSelect } from '../utils';
import { MANUAL_TRIGGER_NODE_DISPLAY_NAME, AI_LANGUAGE_MODEL_OPENAI_CHAT_MODEL_NODE_NAME } from '../constants';
import { MANUAL_TRIGGER_NODE_DISPLAY_NAME } from '../constants';
import { NDV, WorkflowPage } from '../pages';
import { NodeCreator } from '../pages/features/node-creator';

Expand Down Expand Up @@ -485,13 +485,13 @@ describe('NDV', () => {
const connectionGroups = [
{
title: 'Language Models',
id: 'ai_languageModel'
id: 'ai_languageModel',
},
{
title: 'Tools',
id: 'ai_tool'
id: 'ai_tool',
},
]
];

workflowPage.actions.addInitialNodeToCanvas('AI Agent', { keepNdvOpen: true });

Expand All @@ -513,13 +513,16 @@ describe('NDV', () => {
cy.getByTestId(`add-subnode-${group.id}`).click();
nodeCreator.getters.getNthCreatorItem(1).click();
getFloatingNodeByPosition('outputSub').click({ force: true });
cy.getByTestId('subnode-connection-group-ai_tool').findChildByTestId('floating-subnode').should('have.length', 2);
cy.getByTestId('subnode-connection-group-ai_tool')
.findChildByTestId('floating-subnode')
.should('have.length', 2);
}
});

// Since language model has no credentials set, it should show an error
cy.get('[class*=hasIssues]').should('have.length', 1);
})
// Sinse code tool require alphanumeric tool name it would also show an error(2 errors, 1 for each tool node)
cy.get('[class*=hasIssues]').should('have.length', 3);
});
});

it('should show node name and version in settings', () => {
Expand Down Expand Up @@ -636,20 +639,23 @@ describe('NDV', () => {
it('Should open appropriate node creator after clicking on connection hint link', () => {
const nodeCreator = new NodeCreator();
const hintMapper = {
'Memory': 'AI Nodes',
Memory: 'AI Nodes',
'Output Parser': 'AI Nodes',
'Token Splitter': 'Document Loaders',
'Tool': 'AI Nodes',
'Embeddings': 'Vector Stores',
'Vector Store': 'Retrievers'
}
cy.createFixtureWorkflow('open_node_creator_for_connection.json', `open_node_creator_for_connection ${uuid()}`);
Tool: 'AI Nodes',
Embeddings: 'Vector Stores',
'Vector Store': 'Retrievers',
};
cy.createFixtureWorkflow(
'open_node_creator_for_connection.json',
`open_node_creator_for_connection ${uuid()}`,
);

Object.entries(hintMapper).forEach(([node, group]) => {
workflowPage.actions.openNode(node);
cy.get('[data-action=openSelectiveNodeCreator]').contains('Insert one').click();
nodeCreator.getters.activeSubcategory().should('contain', group);
cy.realPress('Escape');
});
})
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class ToolCode implements INodeType {
name: 'toolCode',
icon: 'fa:code',
group: ['transform'],
version: 1,
version: [1, 1.1],
description: 'Write a tool in JS or Python',
defaults: {
name: 'Custom Code Tool',
Expand Down Expand Up @@ -59,6 +59,26 @@ export class ToolCode implements INodeType {
type: 'string',
default: '',
placeholder: 'My_Tool',
displayOptions: {
show: {
'@version': [1],
},
},
},
{
displayName: 'Name',
name: 'name',
type: 'string',
default: '',
placeholder: 'e.g. My_Tool',
validateType: 'string-alphanumeric',
description:
'The name of the function to be called, could contain letters, numbers, and underscores only',
displayOptions: {
show: {
'@version': [{ _cnd: { gte: 1.1 } }],
},
},
},
{
displayName: 'Description',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class ToolWorkflow implements INodeType {
name: 'toolWorkflow',
icon: 'fa:network-wired',
group: ['transform'],
version: 1,
version: [1, 1.1],
description: 'Uses another n8n workflow as a tool. Allows packaging any n8n node(s) as a tool.',
defaults: {
name: 'Custom n8n Workflow Tool',
Expand Down Expand Up @@ -62,6 +62,26 @@ export class ToolWorkflow implements INodeType {
type: 'string',
default: '',
placeholder: 'My_Color_Tool',
displayOptions: {
show: {
'@version': [1],
},
},
},
{
displayName: 'Name',
name: 'name',
type: 'string',
default: '',
placeholder: 'e.g. My_Color_Tool',
validateType: 'string-alphanumeric',
description:
'The name of the function to be called, could contain letters, numbers, and underscores only',
displayOptions: {
show: {
'@version': [{ _cnd: { gte: 1.1 } }],
},
},
},
{
displayName: 'Description',
Expand Down
1 change: 1 addition & 0 deletions packages/workflow/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2341,6 +2341,7 @@ export interface ResourceMapperField {

export type FieldType =
| 'string'
| 'string-alphanumeric'
| 'number'
| 'dateTime'
| 'boolean'
Expand Down
20 changes: 19 additions & 1 deletion packages/workflow/src/TypeValidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ export const tryToParseString = (value: unknown): string => {

return String(value);
};

export const tryToParseAlphanumericString = (value: unknown): string => {
const parsed = tryToParseString(value);
const regex = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
if (!regex.test(parsed)) {
throw new ApplicationError('Value is not a valid alphanumeric string', { extra: { value } });
}
return parsed;
};
export const tryToParseBoolean = (value: unknown): value is boolean => {
if (typeof value === 'boolean') {
return value;
Expand Down Expand Up @@ -180,6 +187,17 @@ export const validateFieldType = (
return { valid: false, errorMessage: defaultErrorMessage };
}
}
case 'string-alphanumeric': {
try {
return { valid: true, newValue: tryToParseAlphanumericString(value) };
} catch (e) {
return {
valid: false,
errorMessage:
'Value is not a valid alphanumeric string, only letters, numbers and underscore allowed',
};
}
}
case 'number': {
try {
if (strict && typeof value !== 'number') {
Expand Down

0 comments on commit edce632

Please sign in to comment.