[EDR Workflows] Add RunScript CS Command - UI#202012
Conversation
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
1 similar comment
|
/ci |
|
/ci |
ashokaditya
left a comment
There was a problem hiding this comment.
Thanks for the changes. I have left some questions and suggestions that I would like to hear before I approve. Also, you're missing a ff check for action type filter in https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/components/hooks.tsx
| output = { | ||
| type: 'json', | ||
| content: { | ||
| code: 'ra_runscript_success_done', |
There was a problem hiding this comment.
Do you also have a list of error codes? Then you should add those below in a function as well.
randomRunscriptFailureCode()
There was a problem hiding this comment.
I don't have a list unfortunately, I noted a few codes though so when I conitnue working on CrowdStrike connector and results - I'll adjust the CS errors list 👍
| about: i18n.translate( | ||
| 'xpack.securitySolution.crowdStrikeConsoleCommands.runscript.args.hostPath.about', | ||
| { | ||
| defaultMessage: 'Absolute or relative path of script on host machine', |
There was a problem hiding this comment.
| defaultMessage: 'Absolute or relative path of script on host machine', | |
| defaultMessage: 'Absolute or relative path of script file on host machine', |
There was a problem hiding this comment.
This was taken 1:1 from CrowdStrike console - although I am ok with adjusting if you prefer
| helpUsage: `runscript -CloudFile="CloudScript1.ps1" -CommandLine="-Verbose true" | ||
| Run a script saved to the CrowdStrike cloud with the specified command line arguments | ||
| runscript -CloudFile="CloudScript1.ps1" -CommandLine="-Verbose true" -Timeout=180 | ||
| Run a script saved to the CrowdStrike cloud with the specified command line arguments and 180 seconds timeout | ||
| runscript -Raw=\`\`\`Get-ChildItem .\`\`\` -CommandLine="" | ||
| Run a raw script whose entire contents are provided in the "-Raw=" flag | ||
| runscript -HostPath="C:\\temp\\LocalScript.ps1" -CommandLine="-Verbose true" | ||
| Run a script from a path on the remote host with the specified command line arguments`, |
There was a problem hiding this comment.
In the screenshot, I notice the first line is indented separately from the rest of the lines.

Also, I feel the set of example help text here is a bit too dense to read on the screen. Consider visually separating the help title/text from the actual example command. Why not have the help description on top of the help syntax?
Something like
-- Run a script saved to the CrowdStrike cloud with the specified command line arguments
runscript -CloudFile="CloudScript1.ps1" -CommandLine="-Verbose true"
--Run a script saved to the CrowdStrike cloud with the specified command line arguments and 180 seconds timeout
runscript -CloudFile="CloudScript1.ps1" -CommandLine="-Verbose true" -Timeout=180
...
...
etc.
There was a problem hiding this comment.
Applied some changes - hope it looks better :) I might create a component to render it nicer but I'd prefer to do it at the later stage if we still have time before 8.18 👍
| about: i18n.translate( | ||
| 'xpack.securitySolution.crowdStrikeConsoleCommands.runscript.args.cloudFile.about', | ||
| { | ||
| defaultMessage: 'Script name in cloud storage', |
There was a problem hiding this comment.
Q: Should be file name I think? Unless script name and file name are the same?
| defaultMessage: 'Script name in cloud storage', | |
| defaultMessage: 'Script file name in cloud storage', |
There was a problem hiding this comment.
Good question. So scripts are on the Cloud, files are on a host. Again I copied this over from CrowdStrike console. But I am happy to adjust if you prefer
| // kbnServerArgs: [ | ||
| // `--xpack.securitySolution.enableExperimental=${JSON.stringify([ | ||
| // 'featureFlagName', | ||
| // 'crowdstrikeRunScriptEnabled', |
There was a problem hiding this comment.
Good catch. This can be reverted for the purposes of this PR, will be needed for another one.
| // kbnServerArgs: [ | ||
| // `--xpack.securitySolution.enableExperimental=${JSON.stringify([ | ||
| // 'featureFlagName', | ||
| // 'crowdstrikeRunScriptEnabled', |
| runscript: { | ||
| endpoint: false, | ||
| sentinel_one: false, | ||
| crowdstrike: true, |
There was a problem hiding this comment.
Q: Looks like this is yet to be updated to false?
| // TODO: RUNSCRIPT does not have a corresponding action in the API yet | ||
| const TEMPORARY_LENGTH = RESPONSE_ACTION_API_COMMANDS_NAMES.length - 1; | ||
| expect(getAllByTestId(`${filterPrefix}-option`).length).toEqual(TEMPORARY_LENGTH); |
There was a problem hiding this comment.
This is an issue with the EUI filter popover component. The popover list shows 9 items at a time on the default height. We need to collect the options using querySelectorAll or something like that. I think worth noting this glitch in a comment here 😓
There was a problem hiding this comment.
I tried adjusting according to your suggestions but nothing helped. I'd take a note though and try to address this in another PR if this is ok with you? :)
There was a problem hiding this comment.
Ok @ashokaditya showed me on zoom, that this was indeed the case. I was adjusting wrong components height 😅 Thanks!
There was a problem hiding this comment.
I found a workaround and sent you the diff.
There was a problem hiding this comment.
Applied, thanks a lot 👍
|
/ci |
| it('should show a list of actions (with `scan`) when opened', async () => { | ||
| render(); | ||
| it('should show a list of actions (with `runscript`) when opened', async () => { | ||
| render({ 'data-test-height': 350 }); |
There was a problem hiding this comment.
Just a note that we would need to have larger heights here as the number of commands increases. We might want to add a note somewhere here about that.
...olution/public/management/components/endpoint_response_actions_list/response_actions_log.tsx
Outdated
Show resolved
Hide resolved
…endpoint_response_actions_list/response_actions_log.tsx Co-authored-by: Ash <1849116+ashokaditya@users.noreply.github.com>
paul-tavares
left a comment
There was a problem hiding this comment.
2nd round of feedback provided - I still see several issues. Let me know if you have any questions.
also, I checked out the PR and test it manually... here is what I found:
- the
helpcommand output does not show therunscriptcorrectly (note the--. I think this is due to the fact that you have the command configured to require arguments, but tehre are no required arguments defined. I think its due to the fact the fact that you did not define the arguments correctly for the mutually exclusive arguments (see my code comment)
- There are argument validations in place. I just entered
runscript --commentand it was accepted
| }); | ||
| }, [commandDef]); | ||
|
|
||
| const helpExample = useMemo(() => { |
There was a problem hiding this comment.
Why did we need to add a helpUsage property if exampleUsage already existed? Is it because you wanted to have the input hint be different from what --help displays?
There was a problem hiding this comment.
Yes, that was the reason 👍
x-pack/plugins/security_solution/public/management/components/console/types.ts
Show resolved
Hide resolved
| privileges: endpointPrivileges, | ||
| }, | ||
| exampleUsage: `runscript -Raw=\`\`\`Get-ChildItem .\`\`\` -CommandLine=""`, | ||
| helpUsage: ` |
There was a problem hiding this comment.
This needs to be i18n.
also - examples below are showing the argument with a single dash - instead of two --
| capabilities: endpointCapabilities, | ||
| privileges: endpointPrivileges, | ||
| }, | ||
| exampleUsage: `runscript -Raw=\`\`\`Get-ChildItem .\`\`\` -CommandLine=""`, |
There was a problem hiding this comment.
- Changing
-into--thanks! - I'd say it could support both, or even more:
runscript --Raw=ls|runscript --Raw="ls"|runscript --Raw='ls'| 'runscript --Raw=ls'
| required: false, | ||
| allowMultiples: false, | ||
| about: CROWDSTRIKE_CONSOLE_COMMANDS.runscript.args.timeout.about, | ||
| mustHaveValue: 'non-empty-string', |
| allowMultiples: false, | ||
| about: CROWDSTRIKE_CONSOLE_COMMANDS.runscript.args.hostPath.about, | ||
| mustHaveValue: 'non-empty-string', | ||
| }, |
There was a problem hiding this comment.
I thought you said that 1 of these tree needed to be used... so should have the exclusiveOr property set to true, no?
There was a problem hiding this comment.
This is a bit tricky, since CrowdStrike lets you add multiple of these arguments, and then the strange behavior I explained during the meeting happened. But I agree with you that our approach would be to use exclusiveOr and just let them use one at a time.
Added exclusiveOr 👍 Thank you, I also added a description to exclusiveOr type.
There was a problem hiding this comment.
Ok. Sorry about this - I do now remember that we discussed that they can use multiples, but at least one of them must be present. In that case, you are correct in that we can't use exclusiveOr because that limits the options for the user to having to only pick one.
The main issue I saw when testing this is that there was no validations in place. Realizing now that all three could be used, and that at least 1 must be defined, I think the perhaps the command should be setup as:
- Remove
exclusiveOrfrom the arguments - Set the command's
mustHaveArgstofalse - Add a command level
validatecallback and add the validation there (that at least one if being used)
Removing the mustHaveArgs (or setting it to false) will also address the visual issue where the help was showing runscript --
|
|
||
| // Dev: | ||
| // runscript success/competed | ||
| ra_runscript_success_done: i18n.translate( |
There was a problem hiding this comment.
You should not be using these code for Crowdstrike. This are Elastic Defend specific and IMO should remain as such.
There was a problem hiding this comment.
I did it to reflect the behavior in tests, but I am ok with removing it 👍
| isFlyout: boolean; | ||
| onChangeFilterOptions?: (selectedOptions: string[]) => void; | ||
| 'data-test-subj'?: string; | ||
| 'data-test-height'?: number; |
There was a problem hiding this comment.
can you explain what this property is? I see it used in a few places below and I'm just not getting it. Also - the naming seems strange,since data- are normally the naming convention use if the property needs to be pass all the way though the HTML element.
There was a problem hiding this comment.
This is a workaround to make our tests work with more than 9 elements on the filters list in history log. Apparently EUI component has dynamically loaded elements and without increased height we weren't able to render all 10 elements.
I wouldn't be really worried about this prop, what do you think?
There was a problem hiding this comment.
Ahhhhh. Ok - so used only for testing so that we don't have to trigger a "scroll" of the popup's content. Got it. I was initially thinking this was a fix that would be visible in the UI and that concerned me because it would not scale as we add more items.
| // No access to response actions (except `unisolate`) | ||
| for (const actionName of RESPONSE_ACTION_API_COMMANDS_NAMES.filter( | ||
| (apiName) => apiName !== 'unisolate' | ||
| (apiName) => apiName !== 'unisolate' && apiName !== 'runscript' |
There was a problem hiding this comment.
Is this temporary? if not, then please update the JSDoc comment above (it mentions only UnIsolate)
There was a problem hiding this comment.
Yes, this is temporary. This PR doesn't include API changes. The next PR will include it.
|
@paul-tavares thank's for the review. i was confused about I think I applied all the suggestions, also included an updated screenshot in the description. Also - thanks for the review and help with the strange EUI issue @ashokaditya, appreciate the help 👍 |
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
cc @tomsonpl |
paul-tavares
left a comment
There was a problem hiding this comment.
Thanks for all the follow ups. Left some more comments, but am 👍 it. You can address it later if needed.
| capabilities: endpointCapabilities, | ||
| privileges: endpointPrivileges, | ||
| }, | ||
| exampleUsage: `runscript --Raw=\`\`\`Get-ChildItem .\`\`\` -CommandLine=""`, |
There was a problem hiding this comment.
FYI - I still see the use of ` here for the --Raw value. I'm guessing thats intentional.
There was a problem hiding this comment.
Yes, we want --, I mistakenly omitted the -CommandLine - will adjust
There was a problem hiding this comment.
Not sure you understood my comment (although, I did not notice the single - the CommandLine so good call).
Look at the value of --Raw.... Why is it using 3 backtick characters (`) to enclose the value of Get-ChildItem .?
There was a problem hiding this comment.
Right... well, apparently an example I copied over from CS. But indeed it makes no sense, will remove! thx
| allowMultiples: false, | ||
| about: CROWDSTRIKE_CONSOLE_COMMANDS.runscript.args.hostPath.about, | ||
| mustHaveValue: 'non-empty-string', | ||
| }, |
There was a problem hiding this comment.
Ok. Sorry about this - I do now remember that we discussed that they can use multiples, but at least one of them must be present. In that case, you are correct in that we can't use exclusiveOr because that limits the options for the user to having to only pick one.
The main issue I saw when testing this is that there was no validations in place. Realizing now that all three could be used, and that at least 1 must be defined, I think the perhaps the command should be setup as:
- Remove
exclusiveOrfrom the arguments - Set the command's
mustHaveArgstofalse - Add a command level
validatecallback and add the validation there (that at least one if being used)
Removing the mustHaveArgs (or setting it to false) will also address the visual issue where the help was showing runscript --
| isFlyout: boolean; | ||
| onChangeFilterOptions?: (selectedOptions: string[]) => void; | ||
| 'data-test-subj'?: string; | ||
| 'data-test-height'?: number; |
There was a problem hiding this comment.
Ahhhhh. Ok - so used only for testing so that we don't have to trigger a "scroll" of the popup's content. Got it. I was initially thinking this was a fix that would be visible in the UI and that concerned me because it would not scale as we add more items.
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/12257730007 |
|
Merged, the following will be coming in a separate PR:
|
(cherry picked from commit 9b27804)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.x`: - [[EDR Workflows] Add RunScript CS Command - UI (#202012)](#202012) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Tomasz Ciecierski","email":"tomasz.ciecierski@elastic.co"},"sourceCommit":{"committedDate":"2024-12-10T14:02:12Z","message":"[EDR Workflows] Add RunScript CS Command - UI (#202012)","sha":"9b27804a9b4f90a48f68c1f543270373d7bab6db","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["v9.0.0","Team:Defend Workflows","release_note:feature","backport:version","v8.18.0"],"title":"[EDR Workflows] Add RunScript CS Command - UI","number":202012,"url":"https://github.com/elastic/kibana/pull/202012","mergeCommit":{"message":"[EDR Workflows] Add RunScript CS Command - UI (#202012)","sha":"9b27804a9b4f90a48f68c1f543270373d7bab6db"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202012","number":202012,"mergeCommit":{"message":"[EDR Workflows] Add RunScript CS Command - UI (#202012)","sha":"9b27804a9b4f90a48f68c1f543270373d7bab6db"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Tomasz Ciecierski <tomasz.ciecierski@elastic.co>


Summary
This PR introduces the
runscriptcommand for CrowdStrike RTR and adds parameter validation to align with the CrowdStrike API. The functionality is currently hidden behind a newcrowdstrikeRunScriptEnabledfeature flag for controlled rollout. Some aspects are temporary and will be refined in future PRs.Key Changes
Added
runscriptCommand:runscriptcommand to allow execution of scripts via CrowdStrike RTR.Added
helpUsageto Command Definition:helpUsagefield that will overwriteexampleUsagein--helpcommand if providedParameter Validation:
Added validation for the following arguments -
choose one, as defined by the CrowdStrike API:--Raw--HostPath--CloudFileOptional arguments:
--CommandLine--Timeout*Temporary return of Null:
runscriptresults will be introduced in a separate PR.API Route:
runscriptcommand will be added in a subsequent PR.Feature Flag:
crowdstrikeRunScriptEnabledfeature flag to ensure incremental adoption and testing.Future Work
runscriptresults.runscriptcommand.Why is this needed?
The
runscriptcommand is a critical RTR feature that enables script execution on target hosts. Adding this functionality brings Elastic closer to full-featured integration with CrowdStrike RTR, providing greater flexibility and utility for users.