From c48a44dea7a09a615ff7e1ec6cb9c4ac46f47249 Mon Sep 17 00:00:00 2001
From: Dongbo Wang <dongbow@microsoft.com>
Date: Tue, 21 Mar 2023 13:41:21 -0700
Subject: [PATCH] Improve the default sensitive history scrubbing to allow safe
 property access

---
 PSReadLine/History.cs | 33 +++++++++++++++++++++++++++------
 test/HistoryTest.cs   | 16 +++++++++++++++-
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/PSReadLine/History.cs b/PSReadLine/History.cs
index 637db8dd0..8c71b084f 100644
--- a/PSReadLine/History.cs
+++ b/PSReadLine/History.cs
@@ -525,6 +525,22 @@ private static bool IsSecretMgmtCommand(StringConstantExpressionAst strConst, ou
             return result;
         }
 
+        private static bool IsSafePropertyUsage(Ast member)
+        {
+            bool result = false;
+
+            if (member.Parent is MemberExpressionAst memberExpr)
+            {
+                // - If the property is NOT on the left side of an assignment, then it's safe.
+                // - Otherwise, if the right-hand side is a pipeline or a variable, then we consider it safe.
+                result = !IsOnLeftSideOfAnAssignment(memberExpr, out Ast rhs)
+                    || rhs is PipelineAst
+                    || (rhs is CommandExpressionAst cmdExpr && cmdExpr.Expression is VariableExpressionAst);
+            }
+
+            return result;
+        }
+
         private static ExpressionAst GetArgumentForParameter(CommandParameterAst param)
         {
             if (param.Argument is not null)
@@ -612,15 +628,20 @@ public static AddToHistoryOption GetDefaultAddToHistoryOption(string line)
                         break;
 
                     case StringConstantExpressionAst strConst:
-                        // If it's not a command name, or it's not one of the secret management commands that
-                        // we can ignore, we consider it sensitive.
-                        isSensitive = !IsSecretMgmtCommand(strConst, out CommandAst command);
-
-                        if (!isSensitive)
+                        isSensitive = true;
+                        if (IsSecretMgmtCommand(strConst, out CommandAst command))
                         {
-                            // We can safely skip the whole command text.
+                            // If it's one of the secret management commands that we can ignore, we consider it safe.
+                            isSensitive = false;
+                            // And we can safely skip the whole command text in this case.
                             match = s_sensitivePattern.Match(line, command.Extent.EndOffset);
                         }
+                        else if (IsSafePropertyUsage(strConst))
+                        {
+                            isSensitive = false;
+                            match = match.NextMatch();
+                        }
+
                         break;
 
                     case CommandParameterAst param:
diff --git a/test/HistoryTest.cs b/test/HistoryTest.cs
index 6d724e005..0b669c710 100644
--- a/test/HistoryTest.cs
+++ b/test/HistoryTest.cs
@@ -202,7 +202,16 @@ public void SensitiveHistoryDefaultBehavior_Two()
                 "Set-SecretInfo -Name apikey; Set-SecretVaultDefault; Test-SecretVault; Unlock-SecretVault -password $pwd; Unregister-SecretVault -SecretVault vaultInfo",
                 "Get-ResultFromTwo -Secret1 (Get-Secret -Name blah -AsPlainText) -Secret2 $secret2",
                 "Get-ResultFromTwo -Secret1 (Get-Secret -Name blah -AsPlainText) -Secret2 sdv87ysdfayf798hfasd8f7ha", // '-Secret2' has expr-value argument. Not saved to file.
-                "$environment -brand $brand -userBitWardenEmail $bwuser -userBitWardenPassword $bwpass" // '-userBitWardenPassword' matches sensitive pattern and it has parsing error. Not save to file.
+                "$environment -brand $brand -userBitWardenEmail $bwuser -userBitWardenPassword $bwpass", // '-userBitWardenPassword' matches sensitive pattern and it has parsing error. Not save to file.
+                "(Import-Clixml \"${Env:HOME}\\credential.clixml\").GetNetworkCredential().Password | Set-Clipboard", // 'Password' is a property not in assignment.
+                "$a.Password = 'abcd'", // setting the 'Password' property with string value. Not saved to file.
+                "$a.Password.Value = 'abcd'", // indirectly setting the 'Password' property with string value. Not saved to file.
+                "$a.Secret = Get-Secret -Name github-token -Vault MySecret",
+                "$a.Secret = $secret",
+                "$a.Password = 'ab' + 'cd'", // setting the 'Password' property with string values. Not saved to file.
+                "$a.Password.Secret | Set-Value",
+                "Write-Host $a.Password.Secret",
+                "($a.Password, $b) = ('aa', 'bb')", // setting the 'Password' property with string value. Not saved to file.
             };
 
             string[] expectedSavedItems = new[] {
@@ -218,6 +227,11 @@ public void SensitiveHistoryDefaultBehavior_Two()
                 "Get-SecretInfo -Name mytoken; Get-SecretVault; Register-SecretVault; Remove-Secret apikey",
                 "Set-SecretInfo -Name apikey; Set-SecretVaultDefault; Test-SecretVault; Unlock-SecretVault -password $pwd; Unregister-SecretVault -SecretVault vaultInfo",
                 "Get-ResultFromTwo -Secret1 (Get-Secret -Name blah -AsPlainText) -Secret2 $secret2",
+                "(Import-Clixml \"${Env:HOME}\\credential.clixml\").GetNetworkCredential().Password | Set-Clipboard",
+                "$a.Secret = Get-Secret -Name github-token -Vault MySecret",
+                "$a.Secret = $secret",
+                "$a.Password.Secret | Set-Value",
+                "Write-Host $a.Password.Secret",
             };
 
             try