lwhitelock / HuduAPI

A powershell API for Hudu Documentation
MIT License
53 stars 45 forks source link

Change minimum PowerShell version to 5.1 #55

Closed davejlong closed 2 months ago

davejlong commented 2 months ago

I was looking to use the HuduAPI module in scripts to run on servers, but wanted to avoid having to install PowerShell 7. After running the module through the PSScriptAnalyzer, it appears that the code for the module would be supported on PowerShell 5.1+ (Windows Server 2016 and up and Windows 10 and up). It also should work on any version of PowerShell released for MacOS and Linux.

The script that I added tests against PowerShell 4.0 and up for compatible syntax, commands, and cmdlets. Adding support for PowerShell 4.0 would be fairly trivial as the only incompatibilities are Write-Information and Register-ArgumentCompleter

.\Analyze.ps1

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSUseCompatibleCommands             Warning      Invoke-Hud 103   The command 'Write-Information' is not available by default
                                                 uRequest.p       in PowerShell version '4.0' on platform 'Microsoft Windows
                                                 s1               Server 2012 R2 Datacenter'
PSUseCompatibleCommands             Warning      AssetLayou 17    The command 'Register-ArgumentCompleter' is not available
                                                 tCompleter       by default in PowerShell version '4.0' on platform
                                                 .ps1             'Microsoft Windows Server 2012 R2 Datacenter'
lwhitelock commented 2 months ago

@JohnDuprey did you drop the -asarray from convertto-json as I think that was the main blocker for 5.1, or was there something else?

JohnDuprey commented 2 months ago

I think some of the UriBuilder C# classes may not be compatible with older versions of .NET as well. That may not have come up on the scan.

Compatibility aside, it's definitely not recommended to run this module on customer owned systems. Depending on auditing settings, PowerShell scripts can be logged in event viewer and potentially could capture your API keys.

davejlong commented 2 months ago

I think some of the UriBuilder C# classes may not be compatible with older versions of .NET as well. That may not have come up on the scan.

Compatibility aside, it's definitely not recommended to run this module on customer owned systems. Depending on auditing settings, PowerShell scripts can be logged in event viewer and potentially could capture your API keys.

That definitely could be it. PSScriptAnalyzer, to my knowledge, doesn't do any testing against things like that.

My intent on any customer owned servers is that any usage would be using API keys that are unique to that customer. Ideally, I wouldn't have to do this, but I'm hoping to secure the runs as much as possible.

davejlong commented 2 months ago

I found the one incompatibility with PowerShell 5.1 and it's the usage of System.Web.HttpUtility to build the query strings for requests. I updated the PR with a modified PSScriptAnalyzer workflow, but it would take some refactoring on the query string builder to add support. Would y'all be open to that refactoring?

JohnDuprey commented 2 months ago

That definitely could be it. PSScriptAnalyzer, to my knowledge, doesn't do any testing against things like that.

My intent on any customer owned servers is that any usage would be using API keys that are unique to that customer. Ideally, I wouldn't have to do this, but I'm hoping to secure the runs as much as possible.

Even with per-customer tokens the risk is too great imo. You should collect and send the data back to a trusted server and perform the updates there. You could easily accomplish this with a PowerShell function app (only supports PowerShell 7) or through an RMM platform.

greenlighttec commented 2 months ago

Yeah I don't think this is a good idea either. If you want to fork it and build your own changes in, we'll try to keep the compatibility the same so it won't break yours as you sync it through. I don't think we should change the main module though.