Skip to content

Conversation

daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented Sep 18, 2019

Fix #934

Detect sensitive history items by using the regex "password|asplaintext|token|key|secret" suggested by Lee.

For sensitive history items, we still keep them in the history queue, so they are still accessible within the same session.
However, we don't write them to the history file, so they will be gone when the session ends, just like the built-in powershell history.

This is done by adding a default handler to AddToHistoryHandler. Quoted from the updated doc for -AddToHistoryHandler:

Specifies a ScriptBlock that can be used to control which commands get added to PSReadLine history,
and whether they should be saved to the history file.

The ScriptBlock is passed the command line, and it is expected to return either a Boolean value,
or an enum value of the type [Microsoft.PowerShell.AddToHistoryOption].
The enum type AddToHistoryOption has 3 members: SkipAdding, MemoryOnly, and MemoryAndFile.

If the ScriptBlock returns $true, it's equivalent to AddToHistoryOption.MemoryAndFile.
The command line is added to the in-memory history queue and saved to the history file.
If the ScriptBlock returns $false, it's equivalent to AddToHistoryOption.SkipAdding,
and the command line is not added to history at all.

If the ScriptBlock returns AddToHistoryOption.MemoryOnly, then the command line is added to the in-memory history queue,
but will not be saved to the history file.
This usually indicates the command line has sensitive content that should not be written to disk.

PSReadLine provides a default handler to this option:
[Microsoft.PowerShell.PSConsoleReadLine]::GetDefaultAddToHistoryOption(string line)
The default handler attempts to detect sensitive information in a command line by matching with a simple regex pattern:
"password|asplaintext|token|key|secret"
When successfully matched, the command line is considered to contain sensitive content, and MemoryOnly is returned.
Otherwise, MemoryAndFile is returned.

To turn off the default handler, just set this option to $null.

Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have PSReadLineOption to opt out

@daxian-dbw
Copy link
Member Author

daxian-dbw commented Sep 19, 2019 via email

@lzybkr
Copy link
Contributor

lzybkr commented Sep 19, 2019

It would be generally useful for the AddToHistoryHandler to be capable of specifying where a command should be saved, in memory, in a file, both, or neither.

This would require a signature change to AddToHistoryHandler, but I think that could be done without really breaking anything because the primary scenario is binding a script block and those aren't strongly typed. If the delegate expects object, we can have dynamic code testing the object type to support bool and this new enum.

@daxian-dbw
Copy link
Member Author

daxian-dbw commented Sep 19, 2019

With a default handler for AddToHistoryHandler, once a user changes the handler, how can he/she set it back to the default? Casting a method to a delegate is supported in PS Core, but not in windows powershell. When the handler is null, it should mean "don't do add-to-history" filtering, not using the default handler.
Being unable to reset the handler feels broken to me.

@lzybkr
Copy link
Contributor

lzybkr commented Sep 19, 2019

Does save and restore not work?

$handler = (Get-PSReadLineOption).AddToHistoryHandler
...
Set-PSReadLineOption -AddToHistoryHandler $handler

@daxian-dbw
Copy link
Member Author

Well, that DOES work 😄

@vexx32
Copy link

vexx32 commented Sep 20, 2019

Is it worth adding a Reset-PSReadLineOption for things like this?

@daxian-dbw
Copy link
Member Author

Added a public static property PSConsoleReadLineOptions.DefaultAddToHistoryHandler to hold the default handler delegate.

@daxian-dbw
Copy link
Member Author

@lzybkr Code has been updated, can you please take another look?

Copy link
Contributor

@lzybkr lzybkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be good to get some more feedback on this design, but I approve.

@daxian-dbw
Copy link
Member Author

@SteveL-MSFT Can you please review again? Thanks!

@daxian-dbw
Copy link
Member Author

@SteveL-MSFT Would you like to take another look?

@daxian-dbw daxian-dbw merged commit 239955c into PowerShell:master Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detect and scrub sensitive information in history
5 participants