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.

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

Jump to bottom

[RAM] Fix filtering tags by special characters #135673

Closed
wants to merge 3 commits into from
Closed

[RAM] Fix filtering tags by special characters #135673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aecc065
Fix filtering tags by special characters
JiaweiWu Jul 4, 2022
8a0d922
Merge branch 'main' into issue-135599-tag-filtering-on-special-character
kibanamachine Jul 11, 2022
e810424
Merge branch 'main' into issue-135599-tag-filtering-on-special-character
kibanamachine Jul 12, 2022
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
10 changes: 10 additions & 0 deletions ...ck/plugins/triggers_actions_ui/public/application/lib/rule_api/map_filters_to_kql.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,14 @@ describe('mapFiltersToKql', () => {
'alert.attributes.tags:(a or b or c)',
]);
});

test('should handle tags with special characters', () => {
expect(
mapFiltersToKql({
tagsFilter: ['a:b', 'b{c', 'c}d', 'd<f', 'f>e', '"ab', 'a\\b'],
})
).toEqual([
'alert.attributes.tags:(a\\:b or b\\{c or c\\}d or d\\<f or f\\>e or \\"ab or a\\\\b)',
]);
});
});
6 changes: 5 additions & 1 deletion x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/map_filters_to_kql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ export const mapFiltersToKql = ({
}

if (tagsFilter && tagsFilter.length) {
filters.push(`alert.attributes.tags:(${tagsFilter.join(' or ')})`);
filters.push(
`alert.attributes.tags:(${tagsFilter
.map((tag) => tag.replace(/([\)\(\<\>\}\{\"\:\\])/gm, '\\$&'))
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if it is relevant to the PR but you can use escapeKuery from @kbn/es-query

Copy link
Contributor Author

@JiaweiWu JiaweiWu Jul 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I tried to use escapeKuery but it seems like that does not escape {}. Looking at the source code, it does seem to support that assumption.

function escapeSpecialCharacters(str: string) {
  return str.replace(/[\\():<>"*]/g, '\\$&'); // $& means the whole matched string
}

Also I noticed this regex pattern is used in security solutions: x-pack/plugins/security_solution/public/management/common/utils.ts. But yes I do think it would be ideal if we can just use the same regex across the code base. So I wouldn't mind just using escapeKuery

.join(' or ')})`
);
}

return filters;
Expand Down
43 changes: 42 additions & 1 deletion x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts_list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
await assertRulesLength(4);
});

it('should filter alerts by the tag', async () => {
it('should filter rules by the tag', async () => {
await createAlert({
supertest,
objectRemover,
Expand Down Expand Up @@ -701,6 +701,47 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
await assertRulesLength(2);
});

it('should filter rules by tags with special characters', async () => {
await createAlert({
supertest,
objectRemover,
overwrites: {
tags: ['a:b'],
},
});
await createAlert({
supertest,
objectRemover,
overwrites: {
tags: ['a<b'],
},
});
await createAlert({
supertest,
objectRemover,
overwrites: {
tags: ['a{b'],
},
});

await refreshAlertsList();
await testSubjects.click('ruleTagFilter');

await testSubjects.click('ruleTagFilterOption-a:b');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
await assertRulesLength(1);

await testSubjects.click('ruleTagFilterOption-a:b');
await testSubjects.click('ruleTagFilterOption-a<b');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
await assertRulesLength(1);

await testSubjects.click('ruleTagFilterOption-a<b');
await testSubjects.click('ruleTagFilterOption-a{b');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
await assertRulesLength(1);
});

it('should not prevent rules with action execution capabilities from being edited', async () => {
const action = await createAction({ supertest, objectRemover });
await createAlert({
Expand Down