check properties

microsoft · Dec 1, 2023 · 7485b12 · 7485b12
1 parent 33c575a
commit 7485b12
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 16 deletions.
diff --git a/Actions/ReadSecrets/ReadSecrets.ps1 b/Actions/ReadSecrets/ReadSecrets.ps1
@@ -100,12 +100,9 @@ try {
                     $json = @{}
                 }
                 if ($json.Keys.Count) {
-                    if ($secretValue.contains("`n")) {
-                        throw "JSON Secret $secretName contains line breaks. JSON Secrets should be compressed JSON (i.e. NOT contain any line breaks)."
-                    }
                     foreach($keyName in $json.Keys) {
-                        if (@("Scopes","TenantId","BlobName","ContainerName","StorageAccountName") -notcontains $keyName) {
-                            # Mask individual values (but not Scopes, TenantId, BlobName, ContainerName and StorageAccountName)
+                        if (IsPropertySecret -propertyName $keyName) {
+                            # Mask individual values if property is secret
                             MaskValue -key "$($secretName).$($keyName)" -value $json."$keyName"
                         }
                     }

diff --git a/Actions/ReadSecrets/ReadSecretsHelper.psm1 b/Actions/ReadSecrets/ReadSecretsHelper.psm1
@@ -8,6 +8,14 @@ $script:keyvaultConnectionExists = $false
 $script:azureRm210 = $false
 $script:isKeyvaultSet = $script:gitHubSecrets.PSObject.Properties.Name -eq "AZURE_CREDENTIALS"
 $script:escchars = @(' ','!','\"','#','$','%','\u0026','\u0027','(',')','*','+',',','-','.','/','0','1','2','3','4','5','6','7','8','9',':',';','\u003c','=','\u003e','?','@','A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z','[','\\',']','^','_',[char]96,'a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z','{','|','}','~')
+$script:notSecretProperties = @("Scopes","TenantId","BlobName","ContainerName","StorageAccountName")
+
+function IsPropertySecret {
+    param (
+        [string] $propertyName
+    )
+    return $script:notSecretProperties -notcontains $propertyName
+}
 
 #
 # Check GitHub Secrets for common mistakes
@@ -18,21 +26,44 @@ $script:escchars = @(' ','!','\"','#','$','%','\u0026','\u0027','(',')','*','+',
 function CheckSecretsForCommonMistakes {
     foreach($secretName in $script:gitHubSecrets.PSObject.Properties.Name) {
         $secretValue = $script:gitHubSecrets."$secretName"
+        CheckSecretForCommonMistakes -secretName $secretName -secretValue $secretValue
+    }
+}
+
+function CheckSecretForCommonMistakes {
+    Param (
+        [string] $secretName,
+        [string] $secretValue
+    )
+
+    try {
+        $json = $secretValue | ConvertFrom-Json
+    }
+    catch {
+        $json = [PSCustomObject]@{}
+    }
+    if ($json.PSObject.Properties.Name.Count -gt 0) {
+        # JSON Secrets should not contain line breaks
         if ($secretValue.contains("`n")) {
-            try {
-                $json = $secretValue | ConvertFrom-Json
-            }
-            catch {
-                $json = [PSCustomObject]@{}
-            }
-            if ($json.PSObject.Properties.Name.Count -gt 0) {
-                Write-Host "::WARNING::JSON Secret $secretName contains line breaks. JSON Secrets available to AL-Go for GitHub should be compressed JSON (i.e. NOT contain any line breaks)."
-            }
-            else {
-                Write-Host "::WARNING::Secret $secretName contains line breaks. GitHub Secrets available to AL-Go for GitHub should not contain line breaks."
+            Write-Host "::WARNING::JSON Secret $secretName contains line breaks. JSON Secrets available to AL-Go for GitHub should be compressed JSON (i.e. NOT contain any line breaks)."
+        }
+        # JSON Secrets properties should not contain values 3 characters or less
+        foreach($keyName in $json.PSObject.Properties.Name) {
+            if (IsPropertySecret -propertyName $keyName) {
+                if ($json."$keyName".Length -le 4) {
+                    Write-Host "::WARNING::JSON Secret $secretName contains properties with very short values. These values will be masked, but the secret might be indirectly exposed and might also cause issues in AL-Go for GitHub."
+                }
             }
         }
     }
+    else {
+        if ($secretValue.contains("`n")) {
+            Write-Host "::WARNING::Secret $secretName contains line breaks. GitHub Secrets available to AL-Go for GitHub should not contain line breaks."
+        }
+        elseif ($secretValue.Length -le 4) {
+            Write-Host "::WARNING::Secret $secretName has a very short value. This value will be masked, but the secret might be indirectly exposed and might also cause issues in AL-Go for GitHub."
+        }
+    }
 }
 
 function IsKeyVaultSet {