Member within filter incorrectly marked as alias for Get-Member

[PrzemyslawKlys]

Get-ADGroup -Filter { Member -RecursiveMatch $DistinguishedName } -searchbase $Group.DistinguishedName -server $Group.Domain

Environment data

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.18362.145
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.18362.145
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1


> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }

[DBG]: PS C:\Users\przemyslaw.klys\OneDrive - Evotec\Support\GitHub\Testimo>  (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.18.1
1.18.0

Not sure if this can be addressed thou. Just posting just in case this is indeed a bug.

[bergmeister]

Hmm, interesting, I need to look at the AST first to see if we can do something about it, thanks for reporting it in the meantime

[msftrncs]

A script block is a script block, so at parse time, 'member' appears as a command name, only at run time does the rules change, and then the rules can be defined by each cmdlet, if they so choose, thus its really impossible to second guess the intent. The only useful option might be to have an option to ignore certain errors when they appear within a script block argument of a command. ??

[msftrncs]

@bergmeister, is it possible that PSScriptAnalyzer could detect that the -filter parameter (and others) of Get-ADGroup (and others) is of type [string] and therefor the script block will actually be consumed as text? I don't know if that is a consistent assumption or not.

[bergmeister]

This is how the AST looks like. It's just a CommandAst and by looking at the docs here of Get-AdGroup, the argument to the Filter parameter is just a string and the cmdlet has implemented its own DSL, which is a subset of the PowerShell language.

The only way to 'know' is to hard-code a check for the Get-AdGroup and -Filter parameter in the rule. I don't want to give a definitive answer if that is the right solution or not but usually the recommended approach is to suppress the warning and explain the false positive in code rather than adding a bunch of exceptions to PSSA (this is just my personal opinion, opinions will definitely be divided if you asked other developers or PS users....). Therefore I'd say, we'd probably accept a PR for this in the open source spirit of 'everyone is allowed to add the necessary pieces they feel they need to be in the software' (kind of like the Linux kernel where everyone can add/dump their own drivers).
What do you think @rjmholt and @JamesWTruher ? It would be good to make a design decision once and for all as to whether hard-coding certain well known cmdlets/parameter names is generally acceptable for PSSA outside of exceptional circumstances like the recent blacklisting of Packagemanagement cmdlets? There is e.g. a related issue to make PSSA aware of Pester's BeforeEach syntax to avoid false positives of PSUseDeclaredVarsMoreThanAssignments?

[rjmholt]

@msftrncs hits the nail on the head. The rule's working correctly, and overriding Member isn't great. But the AD cmdlets do have their own DSL. In fact -Filter is of type string, so the script block isn't actually being used as a scriptblock; it's being stringified and used in the AD filter DSL.

In another case I would say this should be suppressed on a per-line or per-block level.

However, I know the AD module has a number of these DSL uses. If it's common to (ab)use the -Filter parameter with a scriptblock, then we could consider a blanket exception for -Filter on AD cmdlets. But I think that would be extreme and would only be justified if we saw lots and lots of users using the AD cmdlets like this.

[PrzemyslawKlys]

It's safe to assume most AD administrators have to use -Filter which is mandatory in most cases. The only question is how many things PSScriptAnalyzer will mark as bad.

[bergmeister]

Yes, but why not have the parameter value as a string, because that's what it is at the end. It looks like PowerShell syntax but it actually isn't, it is its own DSL. The only reason that I can think of, is that this way, PowerShell could protect your from injection vulnerabilities a bit more compared to a interpolated strings because it has to go through the PowerShell parser.

[PrzemyslawKlys]

Actually it's probably me, misusing Filter syntax.

Get-ADGroup -Filter 'GroupCategory -eq "Security'

Works...

[thomasrayner]

Maybe the right answer is a rule that detects scriptblocks where a string should be used 😁

[msftrncs]

It doesn't help that the documentation uses the scriptblock notation in several places. https://docs.microsoft.com/en-us/powershell/module/activedirectory/get-adgroup. One might want to point out, that ActiveDirectory filters have operators that PowerShell's language does not, but fortunately most script block filters appear as a command invocation to the parser, so the operators become parameters, so no errors result during parsing. Syntax highlighting will definitely be wrong though. People probably like script blocks because they think they are getting highlighting. However, in editors, the user would get the benefit of variable completions, but filter member and operator completions would either be incorrect or incomplete, unless the editor/extension recognizes the context.

I couldn't find the 'about_ActiveDirectory_Filters' topic, but what should be made clear in that topic, is that variable expansion should be avoided, for injection reasons, so use either a script block or a literal string, not a interpolated/expandable string.

[rjmholt]

It's safe to assume most AD administrators have to use -Filter which is mandatory in most cases.

Yes, and they should continue to. But things like Member only get flagged when the argument is given as a scriptblock (which it isn't really -- it will just be cast back to a string at parameter binding time). If you provide the argument directly as a string (which accords to the parameter type in the module), then PSScriptAnalyzer sees no problem.

It doesn't help that the documentation uses the scriptblock notation in several places.

Yeah I meant to mention this earlier when I looked this up. It seems that when these cmdlets were created, they were designed to have a weird DSL in arguments that looks like PowerShell and now the documentation is misleading. Perhaps this is just down to a miscommunication at some point, but it worries me. I suppose the DSL was to put other operators in or something (although there are other ways to do things like that...)

Maybe the right answer is a rule that detects scriptblocks where a string should be used

Although that's not possible everywhere, it should be for cmdlet parameters on cmdlets that are loaded loaded and have sensible parameter types (this info should also be in the compatibility profiles actually...)

I couldn't find the 'about_ActiveDirectory_Filters' topic

Do you mean in the PowerShell docs? The AD module is entirely external to PowerShell and implements this filter DSL on its own. PowerShell has no knowledge of this filter language unfortunately.

This language seems to be documented under the -Filter parameter.

[PrzemyslawKlys]

Should we start updating docs instead?

[msftrncs]

@rjmholt,

I couldn't find the 'about_ActiveDirectory_Filters' topic

Do you mean in the PowerShell docs? The AD module is entirely external to PowerShell and implements this filter DSL on its own. PowerShell has no knowledge of this filter language unfortunately.
This language seems to be documented under the -Filter parameter.

Correct, but, the -Filter parameter says to see the about_ActiveDirectory_Filter topic. I find no such topic in the docs. However the doc that you linked is worded differently (today?), it states to use a command to get the help item, about_ActiveDirectory_Filter, however, attempting that, it is generating no result (get-help command is not even returning). I see there are other 'about' items on this doc that are posted as missing in the issues. (finally after a very long time it returned that the topic could not be found.)