[SecuritySolution][Endpoint][Response Actions] `Scan` response actions history and errors #186284

ashokaditya · 2024-06-17T12:15:18Z

Summary

Adds missing tests for showing scan response action history. Also improves on showing failure messages for actions that were created and sent to Endpoint. As a failed action may have output error codes and a list of error messages this change is going to show both or either depending on the data. Additionally, the changes in the PR handles error messages for multiple agents.

History Log

Console

Mutli agent response history page with errors

Screenclip (older than screenshots)

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Flaky Test Runner was used on any tests changed
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Also shows general action failed message that was not showing up after adding logic to show errors for actions that were not sent to fleet. refs elastic/pull/155254

refs elastic/pull/184437

ashokaditya · 2024-06-17T12:25:17Z

/ci

elasticmachine · 2024-06-18T15:12:59Z

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

ashokaditya · 2024-06-18T15:14:16Z

...management/components/endpoint_response_actions_list/components/action_log_expanded_tray.tsx

    const command = RESPONSE_ACTION_API_COMMAND_TO_CONSOLE_COMMAND_MAP[_command];

-    if (errors?.length) {


This logic was breaking the failed status on history. I'll add tests for it.

Both errors and output codes should be visible when action failed instead of one or the other.

paul-tavares

Left some feedback that I would like to hear from you on. I may also suggest some more unit tests to validate all of the code path conditions, but will hold off until I hear from you on the feedback provided

...ic/management/components/endpoint_action_failure_message/endpoint_action_failure_message.tsx

paul-tavares · 2024-06-20T19:22:47Z

...management/components/endpoint_response_actions_list/components/action_log_expanded_tray.tsx

+      return (
+        <>
+          {OUTPUT_MESSAGES.hasFailed(command)}
+          <>
+            {/* if has outputs and was not successful, show error code message for each agent */}
+            {outputs &&
+              agents.map(
+                (agentId) =>
+                  outputs[agentId] &&
+                  hasKnownErrorCode(outputs[agentId]) && (
+                    <div key={outputs[agentId].content.code}>
+                      <EuiSpacer size="s" />
+                      <EndpointActionFailureMessage
+                        action={action}
+                        isConsoleOutput={false}
+                        data-test-subj={getTestId('failureMessage')}
+                      />
+                    </div>
+                  )
+              )}
+            {/* if there are errors, show them as well*/}
+            {!!errors?.length &&
+              errors.map((error) => (
+                <div key={error}>
+                  <EuiSpacer size="s" />
+                  <EuiFlexItem>{error}</EuiFlexItem>
+                </div>
+              ))}
+          </>
+        </>
+      );


Why make this so complicated?

IMO, this here should just be:

if (!wasSuccessful) { return ( <div> <EuiSpacer size="s" /> <EndpointActionFailureMessage action={action} isConsoleOutput={false} data-test-subj={getTestId('failureMessage')} /> </div> }

All logic to display the error should be encapsulated into the <EndpointActionFailureMessage> component.

Also,
Since we are now going to use the EndpointActionFailureMessage> component here in the History log, its the first time that the component might have errors from Multiple agents, so please enhance it so that it correctly display it for multiple agents. Example:

<div> Host __HOST_NAME__: ___HOST_ERROR___ </div> <div> Host __HOST_NAME__: ___HOST_ERROR___ </div>

(or just implement a better UI for it - but essentially: each error should be listed under the respective agent host name that reported it)

The Prefix or host "label" should only be shown when the action was sent to multiple agents - else just show the error.

Also - careful in showing the action.errors: [] array in addition to the errors from each host. The errors array is (if I remember correctly) just a list of errors that each agent returned, so you likely will be showing duplicate information.

Just to recap:

the <EndpointActionFailureMessage> component should be responsible for formatting all errors from the agents that the action was sent to

When the action was sent to multiple agents, each error should be listed under a label showing the host name

Let me know if this makes sense and your thoughts on it.

Thanks for the suggestion. Yeah that makes sense.

done b4bb1af

...ic/management/components/endpoint_action_failure_message/endpoint_action_failure_message.tsx

review changes

...ic/management/components/endpoint_action_failure_message/endpoint_action_failure_message.tsx

paul-tavares · 2024-06-24T15:26:30Z

...ic/management/components/endpoint_action_failure_message/endpoint_action_failure_message.tsx


      // Determine if each endpoint returned a response code and if so,
      // see if we have a localized message for it
-      if (action.outputs) {
+      // if there are multiple agents, we need to show the outputs/error message for each agent
+      if (action.outputs || (action.errors && action.errors.length)) {


can you add tests for this component that validates the different conditions implemented.

(I actually thought we had tests for it, but maybe it was only the component that first implemented it 🤔 )

paul-tavares · 2024-06-24T15:29:20Z

...ic/management/components/endpoint_action_failure_message/endpoint_action_failure_message.tsx

+      if (!allAgentErrors.length) {
+        return null;
      }


I think we should return a default error here - I see you removed the prior implementation. Maybe add it back and change the message to Unknown error??

Reasoning: if we reached this point in the component (got passed line 38, then the action is at error, so we should display something in the UI or else the user will not really see anything (also - we devs would have a hard time debugging issues like this if it was to come in via an SDH).

Oh yeah I missed that. We agreed on the reasoning earlier. I'll add it back.

review changes

paul-tavares

Thanks for all the changes

paul-tavares · 2024-06-25T15:49:20Z

...nagement/components/endpoint_action_failure_message/endpoint_action_failure_message.test.tsx

+import { EndpointActionGenerator } from '../../../../common/endpoint/data_generators/endpoint_action_generator';
+import { EndpointActionFailureMessage } from './endpoint_action_failure_message';
+
+describe('EndpointActionFailureMessage', () => {


Thank you 👍

…om:ashokaditya/kibana into task/dw-scan-response-actions-history-9213

...management/components/endpoint_response_actions_list/components/action_log_expanded_tray.tsx

tomsonpl

lgtm 👍

tomsonpl

lgtm again 👍

refs elastic/security-team/issues/9822

kibana-ci · 2024-06-27T11:06:09Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: af1d347

Failed CI Steps

FTR Configs #73

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	5533	5535	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	14.1MB	14.1MB	+3.4KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ashokaditya

ref elastic/pull/186284

ashokaditya added 2 commits June 17, 2024 13:57

Show failure message when error codes in outputs

f0dbb37

Also shows general action failed message that was not showing up after adding logic to show errors for actions that were not sent to fleet. refs elastic/pull/155254

Typo correction

e42a063

refs elastic/pull/184437

ashokaditya added release_note:enhancement Team:Defend Workflows “EDR Workflows” sub-team of Security Solution OLM Sprint v8.15.0 labels Jun 17, 2024

ashokaditya self-assigned this Jun 17, 2024

ashokaditya and others added 2 commits June 17, 2024 15:07

Update scan.cy.ts

4c15090

Merge branch 'main' into task/dw-scan-response-actions-history-9213

b5c5d73

ashokaditya marked this pull request as ready for review June 18, 2024 15:12

ashokaditya requested a review from a team as a code owner June 18, 2024 15:12

ashokaditya requested review from pzl and tomsonpl June 18, 2024 15:12

ashokaditya requested a review from paul-tavares June 18, 2024 15:13

ashokaditya commented Jun 18, 2024

View reviewed changes

pzl approved these changes Jun 18, 2024

View reviewed changes

ashokaditya and others added 2 commits June 19, 2024 14:47

Merge branch 'main' into task/dw-scan-response-actions-history-9213

74325a2

fix type

63db37d

ashokaditya force-pushed the task/dw-scan-response-actions-history-9213 branch from c967201 to 63db37d Compare June 19, 2024 19:06

show both failed output codes and error messages

e5df51c

Both errors and output codes should be visible when action failed instead of one or the other.

ashokaditya force-pushed the task/dw-scan-response-actions-history-9213 branch from a5b7255 to e5df51c Compare June 20, 2024 08:02

tests and fixes

685754a

paul-tavares reviewed Jun 20, 2024

View reviewed changes

improve error and output messages

b4bb1af

review changes

ashokaditya force-pushed the task/dw-scan-response-actions-history-9213 branch from 23e772b to b4bb1af Compare June 21, 2024 12:56

ashokaditya requested review from paul-tavares and pzl June 21, 2024 12:58

paul-tavares reviewed Jun 21, 2024

View reviewed changes

...ic/management/components/endpoint_action_failure_message/endpoint_action_failure_message.tsx Outdated Show resolved Hide resolved

ashokaditya and others added 2 commits June 24, 2024 16:54

fix test mock

ed26f44

Merge branch 'main' into task/dw-scan-response-actions-history-9213

e41caf5

paul-tavares reviewed Jun 24, 2024

View reviewed changes

ashokaditya added 5 commits June 24, 2024 19:46

Unknown error message

852ac69

review changes

simplify logic and add component tests

c3f648d

reduce complexity

0880083

fix

2b259f6

missing tests for QA

6184fda

ashokaditya requested a review from paul-tavares June 25, 2024 15:43

paul-tavares approved these changes Jun 25, 2024

View reviewed changes

ashokaditya and others added 3 commits June 25, 2024 18:09

update tests for QA

a3a1751

Merge branch 'main' into task/dw-scan-response-actions-history-9213

8b65ab7

Merge branch 'task/dw-scan-response-actions-history-9213' of github.c…

9ac1efc

…om:ashokaditya/kibana into task/dw-scan-response-actions-history-9213

tomsonpl reviewed Jun 26, 2024

View reviewed changes

...management/components/endpoint_response_actions_list/components/action_log_expanded_tray.tsx Outdated Show resolved Hide resolved

fix tests

0222ac6

tomsonpl approved these changes Jun 26, 2024

View reviewed changes

ashokaditya requested a review from tomsonpl June 26, 2024 10:13

tomsonpl approved these changes Jun 26, 2024

View reviewed changes

ashokaditya and others added 3 commits June 26, 2024 14:48

Update mocks.tsx

745be32

Merge branch 'main' into task/dw-scan-response-actions-history-9213

2bba513

temp. skip automated_response_actions.cy.ts

af1d347

refs elastic/security-team/issues/9822

ashokaditya added bug Fixes for quality problems that affect the customer experience release_note:enhancement and removed release_note:enhancement labels Jun 27, 2024

ashokaditya merged commit e8cd55a into elastic:main Jun 27, 2024
43 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Jun 27, 2024

ashokaditya deleted the task/dw-scan-response-actions-history-9213 branch June 27, 2024 11:55

ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Jun 28, 2024

Disregard duplicate agents when grouping errors

c2cdb78

ref elastic/pull/186284

ashokaditya mentioned this pull request Jun 28, 2024

[Security Solution][Endpoint] Use "valid" agents for creating action requests #187145

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SecuritySolution][Endpoint][Response Actions] `Scan` response actions history and errors #186284

[SecuritySolution][Endpoint][Response Actions] `Scan` response actions history and errors #186284

ashokaditya commented Jun 17, 2024 •

edited

Loading

ashokaditya commented Jun 17, 2024

elasticmachine commented Jun 18, 2024

ashokaditya Jun 18, 2024

paul-tavares left a comment

paul-tavares Jun 20, 2024

ashokaditya Jun 21, 2024

ashokaditya Jun 21, 2024

paul-tavares Jun 24, 2024

ashokaditya Jun 25, 2024

paul-tavares Jun 24, 2024

ashokaditya Jun 24, 2024

ashokaditya Jun 24, 2024

paul-tavares left a comment

paul-tavares Jun 25, 2024

tomsonpl left a comment

tomsonpl left a comment

kibana-ci commented Jun 27, 2024

		const command = RESPONSE_ACTION_API_COMMAND_TO_CONSOLE_COMMAND_MAP[_command];

		if (errors?.length) {

[SecuritySolution][Endpoint][Response Actions] Scan response actions history and errors #186284

[SecuritySolution][Endpoint][Response Actions] Scan response actions history and errors #186284

Conversation

ashokaditya commented Jun 17, 2024 • edited Loading

Summary

History Log

Console

Mutli agent response history page with errors

Screenclip (older than screenshots)

Checklist

ashokaditya commented Jun 17, 2024

elasticmachine commented Jun 18, 2024

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomsonpl left a comment

Choose a reason for hiding this comment

tomsonpl left a comment

Choose a reason for hiding this comment

kibana-ci commented Jun 27, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

History

[SecuritySolution][Endpoint][Response Actions] `Scan` response actions history and errors #186284

[SecuritySolution][Endpoint][Response Actions] `Scan` response actions history and errors #186284

ashokaditya commented Jun 17, 2024 •

edited

Loading