Skip to content

Improve the sensitive history scrubbing to allow retrieving token from az, gcloud, and kubectl#3641

Merged
daxian-dbw merged 1 commit into
PowerShell:masterfrom
daxian-dbw:his
Apr 4, 2023
Merged

Improve the sensitive history scrubbing to allow retrieving token from az, gcloud, and kubectl#3641
daxian-dbw merged 1 commit into
PowerShell:masterfrom
daxian-dbw:his

Conversation

@daxian-dbw

@daxian-dbw daxian-dbw commented Apr 4, 2023 •
edited by ghost
Loading

Copy link
Copy Markdown
Member

PR Summary

Fix #3633
Improve the sensitive history scrubbing to allow retrieving token from az, gcloud, and kubectl.
The default sensitive history scrubbing was updated to handle retrieving token with gcloud, with Azure (both az cli and Az PowerShell), and with kubectl (both get secret and describe secret).

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Make sure you've added one or more new tests
  • Make sure you've tested these changes in terminals that PowerShell is commonly used in (i.e. conhost.exe, Windows Terminal, Visual Studio Code Integrated Terminal, etc.)
  • User-facing changes
    • Not Applicable
    • OR
    • Documentation needed at PowerShell-Docs
      • Doc Issue filed:
Microsoft Reviewers: Open in CodeFlow

@daxian-dbw daxian-dbw requested a review from andyleejordan April 4, 2023 05:06
@daxian-dbw daxian-dbw self-assigned this Apr 4, 2023
andyleejordan
andyleejordan approved these changes Apr 4, 2023
View reviewed changes

@andyleejordan andyleejordan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm ok with this but wary of the precedent it sets for us to get this right not only for these commands, but for every possible ask in the future. Like it's a nice improvement...but I could see this growing huge if we don't have a plan to decide which commands we don't scrub (and do scrub). Perhaps we should have a more general way to allow users to add exceptions?

@daxian-dbw

daxian-dbw commented Apr 4, 2023

Copy link
Copy Markdown
Member Author

Perhaps we should have a more general way to allow users to add exceptions?

The user can already add exceptions, or customized scrubbing by Set-PSReadLineOption -AddToHistoryHandler { <a script block> } (see doc on this parameter). Here is an example that applies additional rules on top of the default: #2698 (comment). User can also choose to disable the scrubbing by Set-PSReadLineOption -AddToHistoryHandler $null.

The improvement made to the default scrubbing algorithm is for general scenarios that many users will run into. Otherwise, we will ask the user to apply the AddToHistoryHandler for their individual needs.

@daxian-dbw daxian-dbw merged commit 3d20df7 into PowerShell:master Apr 4, 2023
@daxian-dbw daxian-dbw deleted the his branch April 4, 2023 18:02
@ghost

ghost commented May 3, 2023

Copy link
Copy Markdown

🎉 v2.3.1-beta1 has been released which incorporates this pull request. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andyleejordan andyleejordan andyleejordan approved these changes

Assignees

@daxian-dbw daxian-dbw

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

DefaultAddToHistoryHandler filters too aggressively

2 participants

@daxian-dbw @andyleejordan