Skip to content
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

Add helper function for Node tools #1000

Merged
merged 2 commits into from
Apr 23, 2024
Merged

Add helper function for Node tools #1000

merged 2 commits into from
Apr 23, 2024

Conversation

Ana06
Copy link
Member

@Ana06 Ana06 commented Apr 19, 2024

Introduce a VM-Install-Node-Tool-From-Zip helper function to install Node tools installed with npm and normally downloaded from a GitHub repository as a ZIP with an inner folder.

First step to address #999

I do not understand why PSAvoidUsingPositionalParameters has started complaining now in the linter, as we use positional parameters in many functions:

packages\common.vm\tools\vm.common\vm.common.psm1
1 rule violation found.    Severity distribution:  Error = 0, Warning = 0, Information = 1

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSAvoidUsingPositionalParameters    Information  vm.common. 393   Cmdlet 'VM-Install-From-Zip' has positional
                                                 psm1             parameter. Please use named parameters instead of
                                                                  positional parameters when calling a command.

Exclude this violation as it is of Information severity and we wouldneed to make a lot of changes to fix it.

Ana06 added 2 commits April 17, 2024 16:35
Introduce a `VM-Install-Node-Tool-From-Zip` helper function to install
Node tools installed with `npm` and normally downloaded from a GitHub
repository as a ZIP with an inner folder.
I do not understand why this linter has started complaining now, as we
use positional parameters in many functions:

```
packages\common.vm\tools\vm.common\vm.common.psm1
1 rule violation found.    Severity distribution:  Error = 0, Warning = 0, Information = 1

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSAvoidUsingPositionalParameters    Information  vm.common. 393   Cmdlet 'VM-Install-From-Zip' has positional
                                                 psm1             parameter. Please use named parameters instead of
                                                                  positional parameters when calling a command.
```

Exclude this violation as it is of `Information` severity and we would
need to make a lot of changes to fix it.
@Ana06 Ana06 added 💎 enhancement It is working, but it could be better 🌀 FLARE-VM A package or feature to be used by FLARE-VM labels Apr 19, 2024
@Ana06 Ana06 requested a review from emtuls April 19, 2024 14:24
@Ana06 Ana06 self-assigned this Apr 19, 2024
@Ana06 Ana06 force-pushed the node-helper branch 2 times, most recently from b47fce8 to be358d4 Compare April 19, 2024 14:40
Copy link
Member

@emtuls emtuls left a comment

Choose a reason for hiding this comment

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

I think we are missing the Try-Catch inside of VM-Install-Node-Tool-From-Zip

@Ana06
Copy link
Member Author

Ana06 commented Apr 23, 2024

@emtuls

I think we are missing the Try-Catch inside of VM-Install-Node-Tool-From-Zip

There is a try-catch inside VM-Install-From-Zip and apart from that we execute a variable assignation and the rest is executed after $ErrorActionPreference = 'Continue', so we do not need an extra try-catch.

@Ana06 Ana06 requested a review from emtuls April 23, 2024 07:52
Copy link
Member

@emtuls emtuls left a comment

Choose a reason for hiding this comment

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

Good to go! 🙂

@Ana06 Ana06 merged commit 68f716d into mandiant:main Apr 23, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌀 FLARE-VM A package or feature to be used by FLARE-VM 💎 enhancement It is working, but it could be better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants