Skip to content
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.

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

fix(core): Use class-validator with XSS check for survey answers #10490

Merged
merged 15 commits into from
Aug 21, 2024
2 changes: 1 addition & 1 deletion packages/cli/src/GenericHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export async function validateEntity(
export const DEFAULT_EXECUTIONS_GET_ALL_LIMIT = 20;

class StringWithNoXss {
@NoXss()
@NoXss({ each: true })
value: string;
ivov marked this conversation as resolved.
Show resolved Hide resolved

constructor(value: string) {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/databases/entities/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ export class User extends WithTimestamps implements IUser {
email: string;

@Column({ length: 32, nullable: true })
@NoXss()
@NoXss({ each: true })
ivov marked this conversation as resolved.
Show resolved Hide resolved
@NoUrl()
@IsString({ message: 'First name must be of type string.' })
@Length(1, 32, { message: 'First name must be $constraint1 to $constraint2 characters long.' })
firstName: string;

@Column({ length: 32, nullable: true })
@NoXss()
@NoXss({ each: true })
ivov marked this conversation as resolved.
Show resolved Hide resolved
@NoUrl()
@IsString({ message: 'Last name must be of type string.' })
@Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' })
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ export class UserUpdatePayload implements Pick<User, 'email' | 'firstName' | 'la
email: string;

@Expose()
@NoXss()
@NoXss({ each: true })
ivov marked this conversation as resolved.
Show resolved Hide resolved
@NoUrl()
@IsString({ message: 'First name must be of type string.' })
@Length(1, 32, { message: 'First name must be $constraint1 to $constraint2 characters long.' })
firstName: string;

@Expose()
@NoXss()
@NoXss({ each: true })
ivov marked this conversation as resolved.
Show resolved Hide resolved
@NoUrl()
@IsString({ message: 'Last name must be of type string.' })
@Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' })
Expand Down
35 changes: 34 additions & 1 deletion packages/cli/src/validators/__tests__/no-xss.validator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ describe('NoXss', () => {

@NoXss()
version = '';

@NoXss({ each: true })
categories: string[] = [];
}

const entity = new Entity();
Expand Down Expand Up @@ -71,7 +74,7 @@ describe('NoXss', () => {
}
});

describe('Miscellanous strings', () => {
describe('Miscellaneous strings', () => {
const VALID_MISCELLANEOUS_STRINGS = ['CI/CD'];

for (const str of VALID_MISCELLANEOUS_STRINGS) {
Expand All @@ -81,4 +84,34 @@ describe('NoXss', () => {
});
}
});

describe('Array of strings', () => {
const VALID_STRING_ARRAYS = [
['cloud-infrastructure-orchestration', 'ci-cd', 'reporting'],
['automationGoalDevops', 'cloudComputing', 'containerization'],
];

for (const arr of VALID_STRING_ARRAYS) {
test(`should allow array: ${JSON.stringify(arr)}`, async () => {
entity.categories = arr;
await expect(validate(entity)).resolves.toBeEmptyArray();
});
}

const INVALID_STRING_ARRAYS = [
['valid-string', '<script>alert("xss")</script>', 'another-valid-string'],
['<img src="x" onerror="alert(\'XSS\')">', 'valid-string'],
];

for (const arr of INVALID_STRING_ARRAYS) {
test(`should reject array containing invalid string: ${JSON.stringify(arr)}`, async () => {
entity.categories = arr;
const errors = await validate(entity);
expect(errors).toHaveLength(1);
const [error] = errors;
expect(error.property).toEqual('categories');
expect(error.constraints).toEqual({ NoXss: 'Potentially malicious string' });
});
}
});
});
4 changes: 3 additions & 1 deletion packages/cli/src/validators/no-xss.validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import { registerDecorator, ValidatorConstraint } from 'class-validator';

@ValidatorConstraint({ name: 'NoXss', async: false })
class NoXssConstraint implements ValidatorConstraintInterface {
validate(value: string) {
validate(value: unknown) {
if (typeof value !== 'string') return false;

return (
value ===
xss(value, {
Expand Down