[Defend Workflows][Trusted Apps][Exceptions builder][Bug] Trusted apps allows for copy paste in advanced mode conditions values#248537
Conversation
|
Pinging @elastic/security-defend-workflows (Team:Defend Workflows) |
rylnd
left a comment
There was a problem hiding this comment.
Detection Engine code changes LGTM; just had one question about a logical change (seemingly) tangential to the intent of the PR.
| const [newValue] = newOptions.map(({ label }) => optionsMemo[labels.indexOf(label)]); | ||
| handleError(undefined); | ||
| handleSpacesWarning(newValue); | ||
| setShowSpacesWarning(false); |
There was a problem hiding this comment.
Did you find that this state change was unnecessary? We still call it in several other places, here and I didn't see any explanation on why it was removed.
There was a problem hiding this comment.
Yeah i found it unnecessary since setShowSpaces is called in handleSpacesWarning anyways.
| // TODO: I have to use any here for now, but once this is available below, we should use the correct types, https://github.com/elastic/kibana/issues/100715 | ||
| // import { AutocompleteStart } from '../../../../../../../../../../src/plugins/unified_search/public'; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| type AutocompleteStart = any; |
There was a problem hiding this comment.
hey @parkiino,
just tested this and it seems that the issue is still present. it does not come up with the IS, only with the MATCHES operator
Screen.Recording.2026-02-23.at.17.11.16.mov
Good catch Gergo! It seems I accidentally got rid of some code during a merge upstream 😵💫. If possible, would you be able to retest? I think it should work now. |
gergoabraham
left a comment
There was a problem hiding this comment.
hey @parkiino,
after you fix the lint error, it's gonna work well.
it's always worth to play around a bit before (re)opening for review, and also when merging upstream, to see if everything still works as expected
There was a problem hiding this comment.
Good catch Gergo! It seems I accidentally got rid of some code during a merge upstream 😵💫. If possible, would you be able to retest? I think it should work now.
please make sure to add automated tests, that catch if the feature is not working anymore.
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
|
ashokaditya
left a comment
There was a problem hiding this comment.
Tested the copy paste and it works as expected.
gergoabraham
left a comment
There was a problem hiding this comment.
looking good! thanks for adding the tests 🚀
…s allows for copy paste in advanced mode conditions values (elastic#248537) ## Summary - [x] Copy pasted values are properly updated into the Trusted Apps matches operator values and saved - [x] Refactors the trusted apps form exception builder handler to properly handle duplicate condition entries and shows the "Wildcard with wrong operator" callout in basic mode # Screenshots  --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Summary
Screenshots