oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.89k stars 232 forks source link

Support `list_of_files` and `project` CLI lint modes for PSScriptAnalyzer linter #1994

Open Isalgeon opened 1 year ago

Isalgeon commented 1 year ago

Is your feature request related to a problem? Please describe. I'm currently in the process of integrating/enabling the PowerShell linter (PSScriptAnalyzer) and I'm running into some serious performance hick-ups.

Describe the solution you'd like Enable the list_of_files capability for the PSScriptAnalyzer linter by default.

Describe alternatives you've considered I've looked at the Invoke-ScriptAnalyzer command documentation and it appears to support both single file and directory-based linting. Providing a directory to the -Path argument along with the -Recurse argument appears to work properly in-line with expectations.

Additional context In my repository with 117 PowerShell scripts and modules, the performance difference is off the charts:

Configuring list_of_files CLI lint mode manually (i.e. POWERSHELL_POWERSHELL_CLI_LINT_MODE: "list_of_files") yields the following error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/megalinter/run.py", line 9, in <module>
    linter = megalinter.Megalinter({"cli": True})
  File "/megalinter/MegaLinter.py", line 121, in __init__
    self.load_linters()
  File "/megalinter/MegaLinter.py", line 475, in load_linters
    all_linters = linter_factory.list_all_linters(linter_init_params)
  File "/megalinter/linter_factory.py", line 30, in list_all_linters
    descriptor_linters = build_descriptor_linters(
  File "/megalinter/linter_factory.py", line 117, in build_descriptor_linters
    linter_instance = linter_class(linter_init_params, instance_attributes)
  File "/megalinter/linters/PowershellLinter.py", line 13, in __init__
    super(PowershellLinter, self).__init__(params, linter_config)
  File "/megalinter/Linter.py", line 283, in __init__
    self.load_config_vars(params)
  File "/megalinter/Linter.py", line 576, in load_config_vars
    raise KeyError(
KeyError: 'You can not override POWERSHELL_POWERSHELL cli_lint_mode with list_of_files, as it can process files only one by one. If you think it could be done, post an issue :)'
nvuillam commented 1 year ago

@Isalgeon good catch :)

Please could you provide the working command line examples of your tests ? We have to rebuild them in custom Linter class https://github.com/oxsecurity/megalinter/blob/main/megalinter/linters/PowershellLinter.py

Isalgeon commented 1 year ago

The command that I used during testing was Invoke-ScriptAnalyzer -Path "$(pwd)" -Recurse.

I noticed you added an improvement a while back to apply fixes #93. Although it's technically outside the scope of this issue, I can see that the complete out-of-the-box command can be Invoke-ScriptAnalyzer -EnableExit -Fix -Path "$(pwd)" -Recurse -Settings ".powershell-psscriptanalyzer.psd1".

The above command doesn't account for all the various configuration settings though. ;-)

Isalgeon commented 1 year ago

Reiterating over my above suggestion, I realized that it's more in line with the project CLI lint mode than list_of_files. Configuring the project CLI lint mode actually leads to following error though:

❌ Linted [POWERSHELL] files with [powershell]: Found 1 error(s) - (1.64s)
- Using [powershell v7.2.6] https://oxsecurity.github.io/megalinter/latest/descriptors/powershell_powershell
- MegaLinter key: [POWERSHELL_POWERSHELL]
- Rules config: [/.powershell-psscriptanalyzer.psd1]
--Error detail:
Invoke-ScriptAnalyzer: Cannot find path '/tmp/lint/None' because it does not exist.

Luckily for us, it's possible to pipe an array of paths to the Invoke-ScriptAnalyzer command that should work for MegaLinter. For example:

@("/tmp/lint/Test-Script.ps1", "/tmp/lint/modules/Some-Module.psm1") | Invoke-ScriptAnalyzer -EnableExit -Fix -Settings ".powershell-psscriptanalyzer.psd1"
nvuillam commented 1 year ago

my knownledge of Powershell scripting is close to zero.... maybe you'd like to make the pull request ? :)

Isalgeon commented 1 year ago

The same goes for my Python knowledge but I want to learn it anyway so now I have a concrete and worthy goal. :) I'll give it a go probably later this week.

nvuillam commented 1 year ago

Thanks :) Basically, the following method must be able to build a correct command line for cli_lint_mode file, list_of_files and project :)

https://github.com/oxsecurity/megalinter/blob/f58627dfe556c4a85e53417f9b73ac505114d93b/megalinter/linters/PowershellLinter.py#L19

Isalgeon commented 1 year ago

@nvuillam I'll scope my changes to only the 2 lint modes for the PSScriptAnalyzer. I'll introduce the automatic fixes in a separate follow-up PR and tie it to your original issue #93.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

Isalgeon commented 1 year ago

This issue is not stale. I haven't had much time to work on this yet, but I have secured a few hours at work to get a solution underway.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

Isalgeon commented 1 year ago

@nvuillam I've yet to confirm it to be 100% sure, but at first glance, it appears #2176 already tackled this issue in a 'different' linter. Oddly enough, both the PowerShell and PowerShell_formatter linters appear to use the same underlying linter (PSScriptAnalyzer).

I'll make sure to confirm this tomorrow at the latest on the PR level and also make sure to test-drive the latest Docker image.

I'm super excited about versions 6.17.0 and 6.18.0! 🤩

nvuillam commented 1 year ago

@Isalgeon indeed, the great @bdovaz allowed -Fix parameter to be sent :)

But it does not solve the case of not called the linter file by file or with a -Recurse, and additional PR may be necessary

You may test with MegaLinter beta version, not sure all updates has already been released :)

nvuillam commented 1 year ago

Basically, I don't think list_of_files could work (except if you find so in the doc), but we probably can update PowershellLinter.py to add -Path "{self.workspace}" -Recurse if POWERSHELL_POWERSHELL_CLI_LINT_MODE: project is defined :)

nvuillam commented 1 year ago

@bdovaz maybe such update could be added in https://github.com/oxsecurity/megalinter/pull/2239 ? 😄

Isalgeon commented 1 year ago

I don't think there's an out-of-the-box way of getting the list_of_files to work other than the suggestion I provided earlier in this issue thread. A CLI equivalent of the earlier suggestion should also work for us; I will have to verify this.

I would already be fine with it being on a project basis since I lint everything anyway. ;-)

bdovaz commented 1 year ago

@bdovaz maybe such update could be added in #2239 ? 😄

@nvuillam done, I think https://github.com/oxsecurity/megalinter/pull/2239/commits/fa175b3923155ec66b120e8fa4c5b228ded4d4d7

Isalgeon commented 1 year ago

@nvuillam, I've finally had the time to check this action off my list and yours. :-)

I tested both PowerShell linters and the original still suits my project the best. The project CLI lint mode also improves performance by ~44 seconds (104 seconds down from 148) for 140 PowerShell files.

I want to close this issue as I don't see the need for the list_of_files lint mode for my projects and configuration.

However, I did notice that the performance of the linter does not quite match what I expected based on invoking the Invoke-ScriptAnalyzer cmdlet directly. If I invoke the command that MegaLinter builds (from what I can gather), it finishes in ~5 seconds on the same set of files. The cmdlet that I used is: Invoke-ScriptAnalyzer -EnableExit -Settings ".powershell-psscriptanalyzer.psd1" -Path $(pwd) -Recurse -Fix

Could you clarify the difference and what I'm missing? Maybe @bdovaz has some insight here as well.

If this is beyond the scope of this issue, I'm happy to fire a fresh one. ;-)

Isalgeon commented 1 year ago

@nvuillam @bdovaz, can either of you shed some light on the observation above? :-)

nvuillam commented 1 year ago

@Isalgeon i'm still a newbie with dotnet stuff :/ Would you like to make a pull requests :)