search

poshbotio / PoshBot

Powershell-based bot framework
MIT License
537 stars 108 forks source link

Use module-qualified cmdlet names when invoking plugins #129

Closed Tadas closed 5 years ago

Tadas commented 5 years ago

Description

I wanted to use start cmdlet name in a plugin but discovered that command execution jobs never finish executing. Turns out that start was being resolved as Start-Process by PowerShell. PoshBot should use module-qualified cmdlet names when invoking plugin commands to ensure that the right cmdlet is called regardless of aliases.

https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_command_precedence?view=powershell-6#qualified-names

Related Issue

Motivation and Context

Enables usage of cmdlet names in plugins which might be hidden by an alias e.g. start (a default alias for Start-Process)

How Has This Been Tested?

Types of changes

Checklist:

devblackops commented 5 years ago

Good idea @Tadas. Thanks!

devblackops commented 5 years ago

@Tadas I'm trying to repro your issue but can't get the problem to occur. This is what I'm trying:

function Start {
    [cmdletbinding()]
    param()

    'Start your engines'
}

or

function Start-Engine {
    [PoshBot.BotCommand(
        CommandName = 'start'
    )]
    [cmdletbinding()]
    param()

    'Start your engines'
}

or

function Start-Engine {
    [PoshBot.BotCommand(
        Aliases = 'start'
    )]
    [cmdletbinding()]
    param()

    'Start your engines'
}

All of these seem to work fine.

What does your plugin command look like that triggers this?

Tadas commented 5 years ago

It can only happen when you have an alias defined

> Get-Alias -Name start

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Alias           start -> Start-Process

> $PSVersionTable

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

Only your first example triggers the bug, that function is pretty much identical to what I have.

devblackops commented 5 years ago

Sorry still can't repro this, and I'm not sure why you're getting a problem either. 🤷‍♂️

PoshBot is already executing the actual command from the module, not the module-qualified name, but the actual function/cmdlet object returned from Get-Command.

When it loads the module, it gets all the module's public commands via Get-Command here. It then stores each object in either the .FunctionInfo or .CmdletInfo property of the [Command] instance depending on if the command is of type Function or Cmdlet here. When it comes time to execute a command in a job, it adds either the .FunctionInfo or .CmdletInfo property to the hashtable that will be sent to the job here. Inside the job, it executes the actual command object here and here.

It never executes the command by name only which I can see would cause a problem with aliases of the same name.

Below is effectively what PoshBot does and even though I've added an alias for start, it still executes the module command because it's not being executed by name.

New-Alias -Name start -Value Start-Process
$c = Get-Command -Module PoshBot.PSSummit -Name start
& $c

I'd like to understand more about how you're running into this.

Tadas commented 5 years ago

Ok I think we're running into serialization nuances. Is your plugin begin executed as job?

Just before Start-Job call: https://github.com/poshbotio/PoshBot/blob/5cadba893fe9191634d6e8d23a802f7229917dd2/PoshBot/Classes/Command.ps1#L133-L138

[DBG]: PS D:\Dev\_PoshBot>> $jobParams.ArgumentList.Function.GetType()

IsPublic IsSerial Name                                     BaseType                                          
-------- -------- ----                                     --------                                          
True     False    FunctionInfo                             System.Management.Automation.CommandInfo

and inside of the job:

[DBG]: [Job14]: PS D:\Documents>> l

   26:                  ParsedCommand = $options.ParsedCommand | Select-Object -ExcludeProperty $parsedCommandExcludes
   27:                  OriginalMessage = $options.OriginalMessage
   28:                  BackendType = $options.BackendType
   29:              }
   30:              Wait-Debugger
   31:*             & $cmd @namedParameters @positionalParameters
   32:          

[DBG]: [Job14]: PS D:\Documents>> $cmd.GetType()

IsPublic IsSerial Name                                     BaseType                                          
-------- -------- ----                                     --------                                          
True     True     PSObject                                 System.Object                                     

[DBG]: [Job14]: PS D:\Documents>> $cmd | gm

   TypeName: Deserialized.System.Management.Automation.FunctionInfo

Name                MemberType   Definition                                                                  
----                ----------   ----------                                                                  
GetType             Method       type GetType()                                                              
ToString            Method       string ToString(), string ToString(string format, System.IFormatProvider ...
Namespace           NoteProperty string Namespace=ReproModule                                                
CmdletBinding       Property     System.Boolean {get;set;}                                                   
CommandType         Property     System.String {get;set;}                                                    
DefaultParameterSet Property      {get;set;}                                                                 
Definition          Property     System.String {get;set;}                                                    
Description         Property      {get;set;}                                                                 
HelpFile            Property      {get;set;}                                                                 
Module              Property     System.String {get;set;}                                                    
ModuleName          Property     System.String {get;set;}                                                    
Name                Property     System.String {get;set;}                                                    
Noun                Property     System.String {get;set;}                                                    
Options             Property     System.String {get;set;}                                                    
OutputType          Property     Deserialized.System.Collections.ObjectModel.ReadOnlyCollection`1[[System....
Parameters          Property     Deserialized.System.Collections.Generic.Dictionary`2[[System.String, msco...
ParameterSets       Property     Deserialized.System.Collections.ObjectModel.ReadOnlyCollection`1[[System....
RemotingCapability  Property     System.String {get;set;}                                                    
ScriptBlock         Property     System.String {get;set;}                                                    
Source              Property     System.String {get;set;}                                                    
Verb                Property     System.String {get;set;}                                                    
Version             Property     System.Version {get;set;}                                                   
Visibility          Property     System.String {get;set;}
devblackops commented 5 years ago

Ahh. That makes sense.

I'm OK with this change to use the module qualified name.