Skip to content

Comments

[Security Solution][Endpoint] Response action create and history log API updates in of space awareness#218674

Merged
paul-tavares merged 51 commits intoelastic:mainfrom
paul-tavares:task/olm-12337-response-actions-request-validation-for-spaces
May 1, 2025
Merged

[Security Solution][Endpoint] Response action create and history log API updates in of space awareness#218674
paul-tavares merged 51 commits intoelastic:mainfrom
paul-tavares:task/olm-12337-response-actions-request-validation-for-spaces

Conversation

@paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Apr 18, 2025

Summary

The following changes to response actions were done in support of space awareness (currently behind feature flag endpointManagementSpaceAwarenessEnabled):

  • Creation of response actions for all supported agent types will now validate that the agent IDs provided in the API request are accessible in the active space
  • The response actions history log APIs - list and single action - where updated to ensure that only response actions for agents visible in active space are returned
  • The display of agent host names on the Response Actions History log was updated so that the UUID of the agents that are no longer accessible in active space are display along with a title indicating host name is not available (see screen capture below)
    • The UI would previously display a - or nothing at all (table row) if we were unable to get the host's name as we would assume the host/agent were unenrolled. That assumption no longer holds true with space awareness as host/agents can be moved between spaces.

Note

FYI: as a result of updating the primary services supporting the above changes, several other modules had to be updated throughout the server-side code in support of these. Apologies for the large PR 🙇


Updates to Response Actions Log expanded table row to show UUID of hosts that are no longer enrolled:

image

Testing

Warning

Response actions for 3rd party hosts/agents that are not in the default space will remain in pending state. This area of code will be updated with an already planned subsequent issue

Enable feature flags

xpack.securitySolution.enableExperimental:
  - endpointManagementSpaceAwarenessEnabled

xpack.fleet.enableExperimental:
  - useSpaceAwareness

Switch Fleet to Space aware

POST /internal/fleet/enable_space_awareness
Elastic-Api-Version: 1,

Add additional mappings for response actions

Note

Only needed until the Endpoint Package is updated with these mappings

Ensure that the Endpoint package is installed

Best way to do this is to create at least one integration policy in fleet.

Add mappings if not already included in index mapping

Tip

These instructions can be done from the Kibana Dev tools console

Ensure index exists:

PUT /_data_stream/.logs-endpoint.actions-default

Ignore any resource_already_exists_exception error that might be returned.

Check to see if the mapping is already present:

GET /.logs-endpoint.actions-default/_mapping/field/agent.policy*

If the response returns an empty mappings, then mappings need to be added

Add mappings to the index:

PUT /.logs-endpoint.actions-default/_mapping
{
  "properties": {
    "agent": {
      "properties": {
        "policy": {
          "properties": {
            "agentId": { "type": "keyword" },
            "elasticAgentId": { "type": "keyword" },
            "integrationPolicyId": { "type": "keyword" },
            "agentPolicyId": { "type": "keyword" }
          }
        }
      }
    }
  }
}

Add data

You should now be all setup to load data for testing. Remember that space data visibility is mostly driven by Fleet and how Policies are setup and shared between spaces - Agent Policies now have a Space ID field to manage this.

Our scripts that run live VMs for each type o9f EDR have been updated to support a --spaceId CLI argument and thus can be used to target specific spaces.

Checklist

@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v9.1.0 labels Apr 18, 2025
@paul-tavares paul-tavares self-assigned this Apr 18, 2025
…sing` is also applied when agent is not accessible in active space
…ntHostNamesWithIds()` so that only endpoint ids are provided in args
@paul-tavares paul-tavares force-pushed the task/olm-12337-response-actions-request-validation-for-spaces branch from 74e70cc to 1dc352c Compare April 24, 2025 18:43
@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares paul-tavares marked this pull request as ready for review April 29, 2025 22:20
@paul-tavares paul-tavares requested review from a team as code owners April 29, 2025 22:20
@elasticmachine
Copy link
Contributor

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

@paul-tavares paul-tavares requested review from ashokaditya and removed request for joeypoon April 29, 2025 22:20
Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

LGTM

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Apr 30, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@tomsonpl tomsonpl requested a review from Copilot April 30, 2025 12:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the response action APIs and related UI components to support space awareness.

  • Adds validations in API endpoints so that response actions and history logs only include agents accessible in the active space.
  • Updates UI messages and host name rendering logic to indicate when a host name is unavailable, and refactors several mocks and error handling utilities to support these changes.

Reviewed Changes

Copilot reviewed 67 out of 67 changed files in this pull request and generated no comments.

File Description
server/endpoint/routes/actions/* Introduces activeSpaceId retrieval and passes endpoint services to routes, updating test mocks accordingly.
public/management/components/endpoint_response_actions_list/* Adjusts translations and host name rendering logic to show "Host name unavailable" when appropriate.
common/endpoint/* Refactors data loaders, transforms, and error handling (using catchAxiosErrorFormatAndThrow) to standardize operations in a space-aware context.
Comments suppressed due to low confidence (3)

x-pack/solutions/security/plugins/security_solution/server/endpoint/routes/actions/list_handler.test.ts:89

  • [nitpick] Ensure that the removal of the sentinel_one feature flag test is intentional and that alternative tests cover the validation for agent accessibility in the active space.
it('should return `badRequest` when sentinel_one feature flag is not enabled and agentType is `sentinel_one`', async () => { ...

x-pack/solutions/security/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_hosts.ts:161

  • [nitpick] Consider renaming 'buildIndexHostsResponse' to 'createIndexHostsResponse' to clarify that the function creates a new response object.
const response: IndexedHostsResponse = buildIndexHostsResponse();

x-pack/solutions/security/plugins/security_solution/public/management/components/endpoint_response_actions_list/translations.tsx:170

  • [nitpick] Verify that the updated message 'Host name unavailable' is consistently used across all related UI components to avoid any confusion with previously used labels.
defaultMessage: 'Host name unavailable',

Copy link
Contributor

@tomsonpl tomsonpl left a comment

Choose a reason for hiding this comment

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

Checked only code, and it looks good!
I left a few questions. I really liked how you clean up the way we pass arguments (removed esClient, logger, metadataService).
Thanks!

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #2 / integrations Endpoint Exceptions "before all" hook for "should add event.module=endpoint to entry if only wildcard operator is present"

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
securitySolution 128 129 +1

Async chunks

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

id before after diff
securitySolution 9.1MB 9.1MB -21.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
securitySolution 35 36 +1
Unknown metric groups

API count

id before after diff
securitySolution 196 197 +1

ESLint disabled line counts

id before after diff
securitySolution 558 560 +2

Total ESLint disabled count

id before after diff
securitySolution 645 647 +2

History

cc @paul-tavares

Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes, refactors and cleaning up tests while at this. Really appreciate it! I couldn't test it locally as I'm still facing issues with my env. I went through the code changes carefully and it looks good to 🚢 . I've a few minor nits but no blockers. Lastly, I think the PR title may need an update.

Comment on lines +105 to +111
const getSoClient = (): SavedObjectsClientContract => {
if (!_soClient) {
_soClient = this.savedObjects.createInternalScopedSoClient({ spaceId });
}

return _soClient;
};
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the refactor. 🙏🏼

throw new NotFoundError(`Action with id '${actionId}' not found.`);
} else if (endpointService.experimentalFeatures.endpointManagementSpaceAwarenessEnabled) {
if (!actionRequest.agent.policy || actionRequest.agent.policy.length === 0) {
const message = `Response action [${actionId}] missing 'agent.policy' information - unable to determine access to it for active space [${spaceId}]:\n${stringify(
Copy link
Member

Choose a reason for hiding this comment

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

nit: For a bit more clarity of error info, and not to confuse with access to space or something else.

Suggested change
const message = `Response action [${actionId}] missing 'agent.policy' information - unable to determine access to it for active space [${spaceId}]:\n${stringify(
const message = `Response action [${actionId}] missing 'agent.policy' information - unable to determine access to policy for active space [${spaceId}]:\n${stringify(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I will include this change in my next commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change here: #219978

note that I went with a different message:

Response action [${actionId}] missing 'agent.policy' information - unable to determine if response action is accessible for space [${spaceId}]

The agent.policy is an internal implementation and mentioning access tot he policy may miss-lead the user. Really, if this happens, it means that migration was not able to populate that data (maybe agent is not longer enrolled), so its really an edge case.

});
});

it('should error is action id is not accessible in active space', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it('should error is action id is not accessible in active space', async () => {
it('should error if action id is not accessible in active space', async () => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will make the change in my next commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change here: #219978

numAlerts: alertsPerHost,
options,
});
if (alertsPerHost > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Q: I'm curious, why is this check, alertsPerHost >0, needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indexAlerts() seem to have been called even if alertsPerHost was 0, so I just place a conditional around it so we don't actually call the function when we don't want alerts to be generated

Comment on lines +468 to +469
// hostMetadataDoc['@timestamp'] = timestamp;
// hostMetadataDoc.event.created = timestamp;
Copy link
Member

Choose a reason for hiding this comment

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

nit: Maybe remove the commented code, unless it was needed? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Forgot about this. Will remove it in my next commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change here: #219978

Comment on lines +499 to 513
const wasFound = !('notFound' in agentHit);
if (!wasFound) {
if (!options?.ignoreMissing) {
throwNotFoundError(agentHit.id);
}
continue;
}

if (throwError) {
throw new AgentNotFoundError(`Agent ${agentHit.id} not found`, { agentId: agentHit.id });
const isAccessible = await isAgentInNamespace(agentHit as Agent, currentNamespace);
if (!isAccessible) {
if (!options?.ignoreMissing) {
throwNotFoundError(agentHit.id);
}
continue;
}
Copy link
Member

Choose a reason for hiding this comment

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

nit: I liked the previous version better. It was easier to read and understand. I actually find this kind of if logic confusing. The following

if (a) { 
 if(b) {
  // do something
 }
 // do else
}

is basically same as

if (a && b) {
  // do something
} else if (a) {
  // do else
}

which would be easier to reason with. Either way I thought the previous version was better in terms of readability and with less redundant code.

cc @tomsonpl @juliaElastic

@paul-tavares paul-tavares merged commit 1b77c57 into elastic:main May 1, 2025
9 checks passed
@paul-tavares paul-tavares deleted the task/olm-12337-response-actions-request-validation-for-spaces branch May 1, 2025 20:34
kapral18 added a commit to kapral18/kibana that referenced this pull request May 4, 2025
…ends-crash

* main: (111 commits)
  [ResponseOps][Rules] Cases action title length too long (elastic#219226)
  [main] Sync bundled packages with Package Storage (elastic#219839)
  Fix ignored dynamic templates (elastic#219875)
  Enforce dependency review by kibana-security workflow (elastic#219262)
  [Security Solution] [Detections] Removes tech preview text from eql seq suppression ui (elastic#219870)
  [Security Solution] Fix alerts table potentially not applying alert assignees (elastic#219460)
  fix(slo): alert deletion (elastic#219876)
  [AI4DSOC] fix styling to address cutoff when screen is narrow (elastic#219306)
  [Security Solution][Endpoint] Response action create and history log API updates in of space awareness (elastic#218674)
  Update publish_oas_docs.sh to deploy Kibana Serverless API docs (elastic#219867)
  feat(slo): lock resource installation (elastic#219747)
  [AI4DSOC] Alert flyout code cleanup (elastic#219810)
  [fleet] fixing `isAgentlessDefault` config usage and readability improvements to `isAgentlessSetupDefault` (elastic#219423)
  feat(slo): Bulk delete UI (elastic#219634)
  m1 demo prep (elastic#219588)
  [Security Solution] Replace sourcerer in EQL tab with dataview picker (elastic#218897)
  [AI4DSOC] Attack discovery widget follow up follow up (elastic#219849)
  [AI Assistant] Fix some OpenAI models not accepting temperature for Inference service (elastic#218887)
  Update dependency msw to ~2.7.5 (main) (elastic#219289)
  Use new client URLs in doc link service (elastic#219600)
  ...
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
…API updates in of space awareness (elastic#218674)

## Summary

The following changes to response actions were done in support of space
awareness (currently behind feature flag
`endpointManagementSpaceAwarenessEnabled`):

- Creation of response actions for all supported agent types will now
validate that the agent IDs provided in the API request are accessible
in the active space
- The response actions history log APIs - list and single action - where
updated to ensure that only response actions for agents visible in
active space are returned
- The display of agent host names on the Response Actions History log
was updated so that the UUID of the agents that are no longer accessible
in active space are display along with a title indicating host name is
not available (see screen capture below)
- The UI would previously display a `-` or nothing at all (table row) if
we were unable to get the host's name as we would assume the host/agent
were unenrolled. That assumption no longer holds true with space
awareness as host/agents can be moved between spaces.
qn895 pushed a commit to qn895/kibana that referenced this pull request Jun 3, 2025
…API updates in of space awareness (elastic#218674)

## Summary

The following changes to response actions were done in support of space
awareness (currently behind feature flag
`endpointManagementSpaceAwarenessEnabled`):

- Creation of response actions for all supported agent types will now
validate that the agent IDs provided in the API request are accessible
in the active space
- The response actions history log APIs - list and single action - where
updated to ensure that only response actions for agents visible in
active space are returned
- The display of agent host names on the Response Actions History log
was updated so that the UUID of the agents that are no longer accessible
in active space are display along with a title indicating host name is
not available (see screen capture below)
- The UI would previously display a `-` or nothing at all (table row) if
we were unable to get the host's name as we would assume the host/agent
were unenrolled. That assumption no longer holds true with space
awareness as host/agents can be moved between spaces.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Fleet Team label for Observability Data Collection Fleet team v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants