-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Make command searcher not use wildcard search for execution #9202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make command searcher not use wildcard search for execution #9202
Conversation
…s in path or not using wildcards in the path.
|
If we want unambiguity, can we follow the practice we use in cmdlets? I mean that if name resolution returns several results, then throw. /cc @mklement0 |
|
@iSazonov That code is already there. End the end, both code paths, use the same code. I doubt anyone actually wants to do execution based on wildcards. It can lead to executing unexpected files and VERY bad results. |
|
Glad to see this is getting fixed; it addresses at least part of #4726, which also asks for the |
da63bff to
44e90c0
Compare
|
@mklement0 The intent was not to fix that issue. As #4726 does not directly affect execution, I would not think that the issue would be considered a Defense in Depth fix and I'd prefer to get this fix in before addressing that issue. |
|
Thanks for clarifying, @TravisEz13 (but note that #4726 is about execution as well). |
44e90c0 to
f143e17
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
SteveL-MSFT
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit comment remaining
build.psm1
Outdated
| [string]$Title = 'PowerShell Core Tests' | ||
| [string]$Title = 'PowerShell Core Tests', | ||
| [Parameter(ParameterSetName='Wait', Mandatory=$true, | ||
| HelpMessage='Wait for the debugger to attach to powershell before pester starts. Debug builds only!')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| HelpMessage='Wait for the debugger to attach to powershell before pester starts. Debug builds only!')] | |
| HelpMessage='Wait for the debugger to attach to PowerShell before Pester starts. Debug builds only!')] |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Should we document the command searcher process in Docs repo? |
|
The command searcher itself is an implementation detail. We should document the search order for command execution. |
PR Summary
Make command searcher not use wildcard search for execution
PR Context
This is a Defense in Depth fix to prevent people from accidentally running a script.
For example, if a user attempted to run
.\[my1].ps1and there is a1.ps1in the same folder.1.ps1would be executed instead.The fix allows tab completion and
Get-Commandto continue to work with the wildcards ([],?, and*).PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.[feature]to your commit messages if the change is significant or affects feature tests