-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[9.1] [Defend Workflows] Fix endpoint list API to mirror exception list API (#246019) #247047
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
Conversation
…elastic#246019) This PR fixes the deprecated `api/endpoint_list` APIs to properly enforce RBAC, space awareness, and security tag assignment through the extension point system. Changes: - Modified 5 ExceptionListClient methods to invoke extension points: `createEndpointListItem`, `updateEndpointListItem`, `deleteEndpointListItem`, `getEndpointListItem`, `findEndpointListItem` - Added entry validation and disallowed field checks to create route - Fixed return type in read route to match API schema - Added comprehensive unit tests for all 5 methods - Added API integration tests covering all RBAC scenarios All changes mirror the existing exception list API behavior. Closes elastic/security-team#14818 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 70c5025) # Conflicts: # x-pack/solutions/security/test/security_solution_api_integration/test_suites/edr_workflows/artifacts/trial_license_complete_tier/endpoint_exceptions.ff_enabled.ts
💚 Build Succeeded
Metrics [docs]
History
|
gergoabraham
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, only minor questions 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you maybe intend to add this empty file or just a merge issue?
| item_id: item.item_id ?? `test-item-${Date.now()}`, | ||
| os_types: item.os_types ?? ['windows'], | ||
| tags: item.tags ?? [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this change needed? i see that generateEndpointExceptionForCreate() was added in 9.3, so you needed to update the getBody() on update single item path below using an already existing helper function, but here, in create single item, shouldn't getBody() be the same as for 9.2/9.3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change is needed because the return type of generateEndpointExceptionForCreate() differs between 9.1 and 9.3/main:
In main/9.3: generateEndpointExceptionForCreate() returns CreateExceptionListItemSchemaWithNonNullProps, which guarantees that item_id, os_types, and tags are non-null. So the test can directly return the result.
In 9.1: generateEndpointExceptionForCreate() returns CreateExceptionListItemSchema, where these fields are optional/potentially undefined.
Since the test requires these fields to have values (e.g., for cleanup via item_id), I added fallback values:
item_id: item.item_id ?? test-item-${Date.now()},os_types: item.os_types ?? ['windows'],tags: item.tags ?? [],
This ensures the test works correctly with 9.1's generator behavior. The same pattern was needed for "update single item" for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i see your point. what actually puzzles me is that the return type was changed in 9.3, so i guess 9.1 and 9.2 could be the same, but you took different approaches in the 2 backport PR. anyway, as it's already merged, and it works, i'm good with it 👍
Backport
This will backport the following commits from
mainto9.1:Questions ?
Please refer to the Backport tool documentation