review comments

microsoft · Dec 7, 2023 · 61e0ac6 · 61e0ac6
1 parent fd6e82e
commit 61e0ac6
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 13 deletions.
diff --git a/Actions/AL-Go-TestRepoHelper.ps1 b/Actions/AL-Go-TestRepoHelper.ps1
@@ -23,7 +23,7 @@
             OutputError "Property '$key' must exist in $settingsDescription. See https://aka.ms/algosettings#$key"
         }
         elseif ($should) {
-            OutPutWarning -Message "Property '$key' should exist in $settingsDescription. See https://aka.ms/algosettings#$key"
+            OutputWarning -Message "Property '$key' should exist in $settingsDescription. See https://aka.ms/algosettings#$key"
         }
     }
 }
@@ -121,13 +121,13 @@ function TestRunnerPrerequisites {
         invoke-gh version
     }
     catch {
-        OutPutWarning -Message "GitHub CLI is not installed"
+        OutputWarning -Message "GitHub CLI is not installed"
     }
     try {
         invoke-git version
     }
     catch {
-        OutPutWarning -Message "Git is not installed"
+        OutputWarning -Message "Git is not installed"
     }
 }
 

diff --git a/Actions/TroubleShooting/TroubleShoot.Secrets.ps1 b/Actions/TroubleShooting/TroubleShoot.Secrets.ps1
@@ -25,52 +25,66 @@ function GetDisplaySecretName {
         return $secretName
     }
     else {
-        return "***"
+        return "<redacted>"
     }
 }
 
 function CheckSecretForCommonMistakes {
     Param (
-        [string] $displaySecretName,
+        [string] $displayName,
         [string] $secretValue
     )
 
+    $warning = $false
     try {
         $json = $secretValue | ConvertFrom-Json
+        $hasLineBreaks = $secretValue.contains("`n")
         $isJson = $true
     }
     catch {
         $isJson = $false
     }
     if ($isJson) {
         # JSON Secrets should not contain line breaks
-        if ($secretValue.contains("`n")) {
-            OutputWarning -Message "Secret $displaySecretName contains line breaks. JSON Secrets available to AL-Go for GitHub should be compressed JSON (i.e. NOT contain any line breaks)."
+        if ($hasLineBreaks) {
+            OutputWarning -Message "Secret $displayName contains line breaks. JSON formatted secrets available to AL-Go for GitHub should be compressed JSON (i.e. NOT contain any line breaks)."
+            $warning = $true
         }
         # JSON Secrets properties should not contain values shorter then $minSecretSize characters
         foreach($keyName in $json.PSObject.Properties.Name) {
             if (IsPropertySecret -propertyName $keyName) {
                 if ($json."$keyName".Length -lt $minSecretSize) {
-                    OutputWarning -Message "JSON Secret $displaySecretName 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."
+                    OutputWarning -Message "JSON Secret $displayName 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."
+                    $warning = $true
                 }
             }
         }
     }
     else {
-        if ($secretValue.contains("`n")) {
-            OutputWarning -Message "Secret $displaySecretName contains line breaks. GitHub Secrets available to AL-Go for GitHub should not contain line breaks."
+        if ($hasLineBreaks) {
+            OutputWarning -Message "Secret $displayName contains line breaks. GitHub Secrets available to AL-Go for GitHub should not contain line breaks."
+            $warning = $true
         }
         elseif ($secretValue.Length -lt $minSecretSize) {
-            OutputWarning -Message "Secret $displaySecretName 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."
+            OutputWarning -Message "Secret $displayName 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."
+            $warning = $true
         }
     }
+    return $warning
 }
 
+$anyWarning = $false
 foreach($secretName in $gitHubSecrets.PSObject.Properties.Name) {
     $secretValue = $gitHubSecrets."$secretName"
-    $displaySecretName = GetDisplaySecretName -secretName $secretName -displayNameOfSecrets $displayNameOfSecrets
+    $displayName = GetDisplaySecretName -secretName $secretName -displayNameOfSecrets $displayNameOfSecrets
     if ($displayNameOfSecrets) {
         Write-Host "Checking secret $secretName"
     }
-    CheckSecretForCommonMistakes -displaySecretName $displaySecretName -secretValue $secretValue
+    if (CheckSecretForCommonMistakes -displayName $displayName -secretValue $secretValue) {
+        $anyWarning = $true
+    }
+}
+
+if ($anyWarning) {
+    OutputSuggestion -Message "Consider restricting access to secrets not needed by AL-Go for GitHub. See [this](https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions#reviewing-access-to-organization-level-secrets)."
 }