Repository navigation
feat(rules): Get-Credential -Credential is a note when the argument can only be a credential - #13
Conversation
fadwen
left a comment
fadwen
left a comment
There was a problem hiding this comment.
Notes on the lines whose reason the diff does not show.
| } | ||
| } | ||
|
|
||
| $credentialType = '^(System\.Management\.Automation\.)?PSCredential$' |
There was a problem hiding this comment.
TypeName.FullName is the type as the script wrote it, pscredential, PSCredential or the namespaced name; -match is case-insensitive, so all three pass. The accelerator is not resolved, which keeps the rule off reflection and on the AST alone like the rest of the rules.
| function Get-CredentialSource { | ||
| param($Expression) | ||
| # Parentheses, a one-element pipeline and the expression statement around a value are wrappers | ||
| $unwrapped = $false |
There was a problem hiding this comment.
A flag rather than a switch with continue: inside a switch, continue applies to the switch, not to the enclosing while, so the first draft unwrapped one layer only and a parenthesized Import-Clixml stayed a warning.
| for ($index = 0; $index -lt $elements.Count; $index++) { | ||
| $element = $elements[$index] | ||
| if ($element.GetType().Name -eq 'CommandParameterAst') { | ||
| if (-not 'TypeName'.StartsWith($element.ParameterName, 'OrdinalIgnoreCase')) { continue } |
There was a problem hiding this comment.
Prefix match as PowerShell binds parameters: -Type and -TypeN name the same parameter. -ComObject never matches, so New-Object -ComObject is not taken for a credential.
| -not $previous.Argument | ||
| if (-not $taken) { $typeName = $element.Extent.Text; break } | ||
| } | ||
| if ("$typeName".Trim('''"') -match $credentialType) { return 'New-Object PSCredential' } |
There was a problem hiding this comment.
The type can be written quoted, New-Object 'System.Management.Automation.PSCredential', and Extent.Text keeps the quotes.
| # never gives it a value, or any one of them could be something else | ||
| function Get-VariableCredentialSource { | ||
| param($Variable) | ||
| $scopePrefix = '^(script|local|private|global):' |
There was a problem hiding this comment.
$script:c and $c name the same variable in a script body, so the prefix is dropped on both sides. Scopes of nested functions are not modelled: every assignment to the name anywhere in the script counts, which can only keep the warning, never add the note.
| @{ Call = 'Get-Credential -Cred:$built'; Handed = '$built' } | ||
| @{ Call = 'Get-Credential -ErrorAction Stop -Credential $settings.Account'; Handed = '$settings.Account' } | ||
| @{ Call = 'Get-Credential (Import-Clixml C:\x.xml)'; Handed = '(Import-Clixml C:\x.xml)' } | ||
| @{ Call = 'Get-Credential (Get-Thing)'; Handed = '(Get-Thing)' } |
There was a problem hiding this comment.
The parenthesized Import-Clixml case this replaces is now a note, and sits in the set below. A call the rule knows nothing about keeps this case a warning.
…an only be a credential IslInteractiveCall warned on every Get-Credential -Credential call handed anything but a literal, because a variable there may hold a user name (prompts) or a credential that is already built (returned as it is, REM-CRED-BUILT). The rule now follows the argument back through the script: when every value it can take is a PSCredential, the finding is an Information note that says the call can go. A value counts as a credential when it is built by [pscredential]::new() or New-Object with the PSCredential type, cast to the type, read by Import-Clixml (which hands back what Export-Clixml wrote, a credential in this idiom), or held by a variable or parameter typed [pscredential]. A variable qualifies when every assignment to it, and any parameter of that name, is one of those; a second assignment of another kind, an untyped parameter, a pipeline or a call keeps the warning, as does a member expression. The evidence is unchanged: the device measurement is the same, the refinement is static.
b1ce736 to
9a2275b
Compare
1 participant
Summary
IslInteractiveCallwarned on everyGet-Credential -Credentialcall handed anything but a literal, because the argument may hold a user name, which prompts, or a credential that is already built, whichGet-Credentialreturns as it is (REM-CRED-BUILT, 12 ms under the agent). The rule now follows the argument back through the script. When every value it can take is aPSCredential, the finding is an Information note that says the call can go; the warning stays for anything the script leaves open.Stacked on #12 (base branch), which is stacked on #11 and #10; the diff is this change only. Merge #10, #11, #12, then this; GitHub retargets each to
mainas its parent merges and the checks run then.Changes
Private/Rules/Find-IslInteractiveCall.ps1. A value counts as a credential when it is built by[pscredential]::new()orNew-Objectwith thePSCredentialtype, cast to the type, or read byImport-Clixml, which hands back whatExport-Clixmlwrote, a credential in this idiom. A variable counts when every assignment to it, and any parameter of its name, is one of those or is typed[pscredential]. A second assignment of another kind, an untyped parameter, a pipeline, a call or a member expression keeps the warning. The new finding cites the same evidence as the warning: the device measurement is the same, the refinement is static.docs/Rules.mdregenerated with the Information row. README and Findings say where the note applies.Import-Clixml; eight that keep the warning. The parenthesizedImport-Clixmlcase, a warning before, moved to the note set.Verification
docs/Rules.mdregenerates unchanged.Get-Credentialcall, so no example's verdict changes.