Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .coderabbit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
reviews:
tools:
gitleaks:
enabled: false
Comment on lines +1 to +4
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid disabling secret scanning for the whole repository.

This change suppresses one of the repo-wide secret detectors at the same time the PR adds embedded credentials in psscriptanalyzer/Manage-Users.ps1 (Lines 17 and 33), psscriptanalyzer/Invoke-HealthCheck.ps1 (Line 4), psscriptanalyzer/Common-Helpers.psm1 (Line 24), and psscriptanalyzer/Deploy-DemoApp.ps1 (Line 45). If this is only for the analyzer demo, keep the fixtures obviously fake and isolate the exception to demo content instead of turning off the check globally.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.coderabbit.yml around lines 1 - 4, The repo-wide secret scanner was
disabled by setting reviews.tools.gitleaks.enabled to false in .coderabbit.yml;
instead re-enable gitleaks (restore enabled: true) and remove or replace the
embedded credentials found in psscriptanalyzer/Manage-Users.ps1,
psscriptanalyzer/Invoke-HealthCheck.ps1, psscriptanalyzer/Common-Helpers.psm1,
and psscriptanalyzer/Deploy-DemoApp.ps1 with clearly fake/demo fixtures, or
narrow the exception by configuring gitleaks to ignore only a dedicated
demo/test directory or specific file patterns (not the whole repo) so the
scanner remains active for production code.

20 changes: 20 additions & 0 deletions psscriptanalyzer/Backup-Database.psd1
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@{
RootModule = 'Backup-Database.psm1'
ModuleVersion = '0.1.0'
GUID = 'a1b2c3d4-e5f6-7890-1234-567890abcdef'
Author = 'Demoapp Platform Team'
CompanyName = 'Demoapp'
Copyright = '(c) 2026 Demoapp.'
Description = 'Backup and restore helpers for Demoapp databases.'
Comment on lines +2 to +8
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

pwsh -NoProfile -Command "Invoke-ScriptAnalyzer -Path psscriptanalyzer/Backup-Database.psd1"

Repository: coderabbitai/coderabbit-pr-review

Length of output: 3773


🏁 Script executed:

cat -n psscriptanalyzer/Backup-Database.psd1

Repository: coderabbitai/coderabbit-pr-review

Length of output: 813


Normalize assignment spacing to satisfy PSScriptAnalyzer.

PSUseConsistentWhitespace warnings are reported on lines 2–8, 17, and 19. Remove extra spaces before assignment operators to use single spacing consistently throughout the manifest.

Suggested cleanup
-    RootModule        = 'Backup-Database.psm1'
-    ModuleVersion     = '0.1.0'
-    GUID              = 'a1b2c3d4-e5f6-7890-1234-567890abcdef'
-    Author            = 'Demoapp Platform Team'
-    CompanyName       = 'Demoapp'
-    Copyright         = '(c) 2026 Demoapp.'
-    Description       = 'Backup and restore helpers for Demoapp databases.'
+    RootModule = 'Backup-Database.psm1'
+    ModuleVersion = '0.1.0'
+    GUID = 'a1b2c3d4-e5f6-7890-1234-567890abcdef'
+    Author = 'Demoapp Platform Team'
+    CompanyName = 'Demoapp'
+    Copyright = '(c) 2026 Demoapp.'
+    Description = 'Backup and restore helpers for Demoapp databases.'
@@
-    CmdletsToExport   = @()
+    CmdletsToExport = @()
@@
-    AliasesToExport   = @()
+    AliasesToExport = @()
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 2-2: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)


[warning] 3-3: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)


[warning] 4-4: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)


[warning] 5-5: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)


[warning] 6-6: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)


[warning] 7-7: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)


[warning] 8-8: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Backup-Database.psd1` around lines 2 - 8, Normalize spacing
around assignment operators in the module manifest entries (RootModule,
ModuleVersion, GUID, Author, CompanyName, Copyright, Description) so each key
uses a single space before and after the '='; update the lines shown in the diff
to remove the extra spaces before '=' and ensure consistent single-spacing to
satisfy PSScriptAnalyzer PSUseConsistentWhitespace warnings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Tighten module description to match actual exported surface.

Line 8 says “backup and restore helpers,” but the module exports only backup-oriented functions (see psscriptanalyzer/Backup-Database.psm1 Lines 1-40 and manifest export list on Lines 11-15). Consider rewording to avoid misleading consumers.

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 8-8: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Backup-Database.psd1` at line 8, The manifest Description
should be tightened to reflect only the exported backup functionality rather
than "backup and restore"; update the string at Description (currently 'Backup
and restore helpers for Demoapp databases.') to mention only backup helpers or
the exact exported backup cmdlets referenced in the module (see the exported
symbol list in the manifest and the functions implemented in
psscriptanalyzer/Backup-Database.psm1 lines 1-40) so consumers are not misled
about restore functionality.

PowerShellVersion = '5.1'

FunctionsToExport = @(
'Get-Backups',
'Remove-Backup',
'Process-Backup',
'Verify-Backups'
)
CmdletsToExport = @()
VariablesToExport = @()
AliasesToExport = @()
}
40 changes: 40 additions & 0 deletions psscriptanalyzer/Backup-Database.psm1
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
$Global:BackupRoot = "C:\backups\demoapp"

function Get-Backups {
param([string]$Database)
$path = Join-Path $Global:BackupRoot $Database
gci $path -Filter *.bak | ? { $_.LastWriteTime -gt (Get-Date).AddDays(-30) }
}

function Remove-Backup {
param([string]$Database, [string]$BackupFile)
$target = Join-Path $Global:BackupRoot (Join-Path $Database $BackupFile)
Remove-Item -Path $target -Force
Write-Host "removed $target"
}

function Process-Backup {
param(
[string]$Database,
[string]$Username,
[string]$Password
)
$timestamp = Get-Date -Format "yyyyMMdd-HHmmss"
$backupFile = "$Database-$timestamp.bak"
$target = Join-Path $Global:BackupRoot (Join-Path $Database $backupFile)
$tempPath = Join-Path $env:TEMP "stage"
$command = "sqlcmd -S localhost -U $Username -P $Password -Q `"BACKUP DATABASE [$Database] TO DISK='$target'`""
Invoke-Expression $command
Write-Host "backup written to $target"
}
Comment on lines +16 to +29
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Command injection vulnerability and insecure credential handling.

This function has multiple security issues:

  1. Command injection: Invoke-Expression with string interpolation on lines 26-27 allows injection if $Password, $Username, or $Database contain shell metacharacters (e.g., backticks, quotes, $(...))
  2. Credential exposure: Plain-text $Password parameter appears in process lists and logs
  3. Unused variable: $tempPath on line 25 is assigned but never used

Replace Invoke-Expression with direct process invocation and use PSCredential for secure credential handling.

🔒 Secure implementation
-function Process-Backup {
+function Backup-Database {
+    [CmdletBinding(SupportsShouldProcess)]
     param(
         [string]$Database,
-        [string]$Username,
-        [string]$Password
+        [Parameter(Mandatory)]
+        [PSCredential]$Credential
     )
     $timestamp = Get-Date -Format "yyyyMMdd-HHmmss"
     $backupFile = "$Database-$timestamp.bak"
-    $target = Join-Path $Global:BackupRoot (Join-Path $Database $backupFile)
-    $tempPath = Join-Path $env:TEMP "stage"
-    $command = "sqlcmd -S localhost -U $Username -P $Password -Q `"BACKUP DATABASE [$Database] TO DISK='$target'`""
-    Invoke-Expression $command
-    Write-Host "backup written to $target"
+    $target = Join-Path $script:BackupRoot (Join-Path $Database $backupFile)
+    
+    if ($PSCmdlet.ShouldProcess($Database, "Backup database to $target")) {
+        $query = "BACKUP DATABASE [$Database] TO DISK='$target'"
+        $arguments = @(
+            "-S", "localhost",
+            "-U", $Credential.UserName,
+            "-P", $Credential.GetNetworkCredential().Password,
+            "-Q", $query
+        )
+        & sqlcmd `@arguments`
+        Write-Output "backup written to $target"
+    }
 }
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 24-24: Found global variable 'Global:BackupRoot'.

(PSAvoidGlobalVars)


[error] 19-20: Function 'Process-Backup' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute.

(PSAvoidUsingUsernameAndPasswordParams)


[warning] 27-27: Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.

(PSAvoidUsingInvokeExpression)


[warning] 20-20: Parameter '$Password' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to expose this sensitive information.

Suggested fix: Set $Password type to SecureString; Set $Password type to PSCredential

(PSAvoidUsingPlainTextForPassword)


[warning] 25-25: The variable 'tempPath' is assigned but never used.

(PSUseDeclaredVarsMoreThanAssignments)


[warning] 16-16: The cmdlet 'Process-Backup' uses an unapproved verb.

(PSUseApprovedVerbs)


[info] 16-16: The cmdlet 'Process-Backup' does not have a help comment.

(PSProvideCommentHelp)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Backup-Database.psm1` around lines 16 - 29, Process-Backup
is vulnerable: stop using Invoke-Expression and string interpolation, drop the
unused $tempPath, and stop accepting plain-text $Password; change the function
signature to accept a [PSCredential]$Credential (or use integrated auth), build
sqlcmd arguments as an argument array rather than interpolated strings, and
invoke sqlcmd with Start-Process (or the call operator &) using -ArgumentList to
pass $Credential.UserName and a securely handled password (avoid logging or
writing the password, prefer integrated auth or a secure API like
Invoke-Sqlcmd/SqlConnection to execute "BACKUP DATABASE [$Database] TO
DISK='$target'"); remove $tempPath and ensure no plaintext password appears in
logs or process arguments.


function Verify-Backups {
$stale = Get-Backups -Database "demoapp_prod"
if ($stale -eq $null) {
Write-Host "no recent backups"
} else {
Write-Host "$($stale.Count) recent backups"
}
}
Comment on lines +31 to +38
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Null comparison order can cause incorrect results.

On line 33, $stale -eq $null behaves unexpectedly when $stale is an array. PowerShell's -eq operator filters arrays, so an empty array returns an empty array (falsy) rather than $true. Place $null on the left side for consistent behavior.

🐛 Fix null comparison
-function Verify-Backups {
+function Test-BackupRecency {
+    [CmdletBinding()]
+    param([string]$Database = "demoapp_prod")
   $stale = Get-Backups -Database "demoapp_prod"
-  if ($stale -eq $null) {
-      Write-Host "no recent backups"
+  if ($null -eq $stale) {
+      Write-Warning "no recent backups"
   } else {
-      Write-Host "$($stale.Count) recent backups"
+      Write-Output "$($stale.Count) recent backups"
   }
 }
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 33-33: $null should be on the left side of equality comparisons.

Suggested fix: Use $null on the left hand side for safe comparison with $null.

(PSPossibleIncorrectComparisonWithNull)


[warning] 31-31: The cmdlet 'Verify-Backups' uses an unapproved verb.

(PSUseApprovedVerbs)


[warning] 31-31: The cmdlet 'Verify-Backups' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Verify-Backups'

(PSUseSingularNouns)


[info] 31-31: The cmdlet 'Verify-Backups' does not have a help comment.

(PSProvideCommentHelp)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Backup-Database.psm1` around lines 31 - 38, The null
comparison in Verify-Backups is unreliable because PowerShell's -eq filters
arrays; change the check from if ($stale -eq $null) to a safe form such as if
($null -eq $stale -or $stale.Count -eq 0) (or at minimum if ($null -eq $stale))
so empty arrays and null are handled correctly; update the condition in the
Verify-Backups function that evaluates the result of Get-Backups (variable
$stale).


Export-ModuleMember -Function Get-Backups, Remove-Backup, Process-Backup, Verify-Backups
58 changes: 58 additions & 0 deletions psscriptanalyzer/Common-Helpers.psm1
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
$Global:HelperCache = @{}

function Get-Configs {
param([string]$Path = "C:\app\config")
cd $Path
$files = ls *.json
return $files | % { cat $_.FullName | ConvertFrom-Json }
}
Comment on lines +3 to +8
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Get-Configs changes the working directory as a side effect.

Using cd $Path (Line 5) modifies the caller's working directory, which can cause unexpected behavior. Use -Path with full paths instead.

♻️ Proposed fix
 function Get-Configs {
     param([string]$Path = "C:\app\config")
-    cd $Path
-    $files = ls *.json
-    return $files | % { cat $_.FullName | ConvertFrom-Json }
+    $files = Get-ChildItem -Path $Path -Filter *.json
+    return $files | ForEach-Object { Get-Content $_.FullName | ConvertFrom-Json }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function Get-Configs {
param([string]$Path = "C:\app\config")
cd $Path
$files = ls *.json
return $files | % { cat $_.FullName | ConvertFrom-Json }
}
function Get-Configs {
param([string]$Path = "C:\app\config")
$files = Get-ChildItem -Path $Path -Filter *.json
return $files | ForEach-Object { Get-Content $_.FullName | ConvertFrom-Json }
}
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[info] 3-3: The cmdlet 'Get-Configs' does not have a help comment.

(PSProvideCommentHelp)


[warning] 3-3: The cmdlet 'Get-Configs' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Get-Configs'

(PSUseSingularNouns)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Common-Helpers.psm1` around lines 3 - 8, Get-Configs
currently mutates the caller's working directory by calling cd $Path; change it
to operate using full paths only: use Get-ChildItem -Path $Path -Filter '*.json'
(or Get-ChildItem $Path | Where-Object Name -like '*.json') instead of ls, and
use Get-Content -Path $_.FullName (preferably with -Raw) piped to
ConvertFrom-Json so the function returns parsed objects without changing the
process cwd; ensure the param([string]$Path = "C:\app\config") remains and
handle the case when no files are found gracefully.


function Test-Endpoints {
param([string[]]$Urls)
foreach ($url in $Urls) {
$r = iwr -Uri $url -UseBasicParsing
if ($r -eq $null) {
Write-Host "$url unreachable"
}
}
}
Comment on lines +10 to +18
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Incorrect $null comparison and potential logic issue.

Line 14: Place $null on the left side of comparisons. Also, Invoke-WebRequest throws on HTTP errors by default rather than returning null, so the $null check may never trigger—consider using -ErrorAction SilentlyContinue with proper error handling.

♻️ Proposed fix
     foreach ($url in $Urls) {
-        $r = iwr -Uri $url -UseBasicParsing
-        if ($r -eq $null) {
+        try {
+            $r = Invoke-WebRequest -Uri $url -UseBasicParsing -ErrorAction Stop
+        } catch {
             Write-Host "$url unreachable"
         }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function Test-Endpoints {
param([string[]]$Urls)
foreach ($url in $Urls) {
$r = iwr -Uri $url -UseBasicParsing
if ($r -eq $null) {
Write-Host "$url unreachable"
}
}
}
function Test-Endpoints {
param([string[]]$Urls)
foreach ($url in $Urls) {
try {
$r = Invoke-WebRequest -Uri $url -UseBasicParsing -ErrorAction Stop
} catch {
Write-Host "$url unreachable"
}
}
}
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[info] 10-10: The cmdlet 'Test-Endpoints' does not have a help comment.

(PSProvideCommentHelp)


[warning] 14-14: $null should be on the left side of equality comparisons.

Suggested fix: Use $null on the left hand side for safe comparison with $null.

(PSPossibleIncorrectComparisonWithNull)


[warning] 10-10: The cmdlet 'Test-Endpoints' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Test-Endpoints'

(PSUseSingularNouns)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Common-Helpers.psm1` around lines 10 - 18, In
Test-Endpoints, change the Invoke-WebRequest call (iwr) to suppress exceptions
and detect failures by adding an explicit error action (e.g. -ErrorAction
SilentlyContinue) and then perform the null comparison with $null on the left
(use $null -eq $r) or use Try/Catch around iwr to catch exceptions and log the
URL as unreachable; update the Write-Host message accordingly so unreachable
cases are reached when iwr fails rather than relying on a right-side $null
comparison.


function Process-Tokens {
param(
[string]$Username,
[string]$Password,
[string]$ApiKey = "demoapp_kJ8mN2pQ4rS6tU8vW0xY2zA4bC6dE8fG0hI2jK4l"
)
$secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
$cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
$body = @{ apiKey = $ApiKey; user = $Username }
$tempVar = "unused string"
$resp = Invoke-RestMethod -Uri "https://api.example.com/auth" -Method Post -Body $body
return $resp
}
Comment on lines +20 to +32
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Hardcoded API key and insecure credential handling in Process-Tokens.

  1. Hardcoded secret: Line 24 contains a default API key embedded in source.
  2. Plaintext password: The $Password parameter should be [SecureString] or consolidated into a [PSCredential].
  3. Unused variables: $cred (Line 27) and $tempVar (Line 29) are assigned but never used.
🔒 Proposed fix using PSCredential
 function Process-Tokens {
     param(
-        [string]$Username,
-        [string]$Password,
-        [string]$ApiKey = "demoapp_kJ8mN2pQ4rS6tU8vW0xY2zA4bC6dE8fG0hI2jK4l"
+        [Parameter(Mandatory=$true)]
+        [PSCredential]$Credential,
+        [Parameter(Mandatory=$true)]
+        [string]$ApiKey
     )
-    $secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
-    $cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
-    $body = @{ apiKey = $ApiKey; user = $Username }
-    $tempVar = "unused string"
+    $body = @{ apiKey = $ApiKey; user = $Credential.UserName }
     $resp = Invoke-RestMethod -Uri "https://api.example.com/auth" -Method Post -Body $body
     return $resp
 }
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[error] 22-23: Function 'Process-Tokens' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute.

(PSAvoidUsingUsernameAndPasswordParams)


[warning] 20-20: The cmdlet 'Process-Tokens' uses an unapproved verb.

(PSUseApprovedVerbs)


[error] 26-26: File 'Common-Helpers.psm1' uses ConvertTo-SecureString with plaintext. This will expose secure information. Encrypted standard strings should be used instead.

(PSAvoidUsingConvertToSecureStringWithPlainText)


[warning] 23-23: Parameter '$Password' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to expose this sensitive information.

Suggested fix: Set $Password type to SecureString; Set $Password type to PSCredential

(PSAvoidUsingPlainTextForPassword)


[info] 20-20: The cmdlet 'Process-Tokens' does not have a help comment.

(PSProvideCommentHelp)


[warning] 27-27: The variable 'cred' is assigned but never used.

(PSUseDeclaredVarsMoreThanAssignments)


[warning] 29-29: The variable 'tempVar' is assigned but never used.

(PSUseDeclaredVarsMoreThanAssignments)


[warning] 20-20: The cmdlet 'Process-Tokens' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Process-Tokens'

(PSUseSingularNouns)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Common-Helpers.psm1` around lines 20 - 32, In
Process-Tokens: remove the hardcoded default in the $ApiKey parameter and
require the key to be passed in or loaded securely (e.g., from an
environment/secret store) instead of embedding it; change the $Password
parameter to accept a [SecureString] or accept a single [PSCredential] parameter
and drop the ConvertTo-SecureString call and the unused $cred variable; remove
the unused $tempVar; and update the Invoke-RestMethod call to use the supplied
secure credential/key values (ensure you reference the Process-Tokens function,
$ApiKey, $Password or PSCredential, ConvertTo-SecureString and Invoke-RestMethod
when making these edits).


function Get-LogFiles {
param([string]$Root = "C:\logs")
$files = gci $Root -Recurse -Filter *.log | ? { $_.Length -gt 0 }
foreach ($f in $files) {
$content = cat $f.FullName -Tail 100
Write-Host "tail of $($f.Name)"
}
Comment on lines +34 to +40
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Get-LogFiles discards the data it just read.

$content is populated from the last 100 lines and then never emitted or returned, so callers only get "tail of ..." messages while paying the I/O cost for the read. Return/write $content, or remove the expensive read entirely.

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[info] 34-34: The cmdlet 'Get-LogFiles' does not have a help comment.

(PSProvideCommentHelp)


[warning] 34-34: The cmdlet 'Get-LogFiles' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Get-LogFiles'

(PSUseSingularNouns)


[warning] 38-38: The variable 'content' is assigned but never used.

(PSUseDeclaredVarsMoreThanAssignments)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Common-Helpers.psm1` around lines 34 - 40, Get-LogFiles
reads the last 100 lines into $content for each file but never outputs or
returns it, wasting I/O; modify the function (Get-LogFiles) to either emit the
read data (e.g., Write-Output or return $content for each $f) or remove the cat
$f.FullName -Tail 100 call if the tail is not needed, and ensure callers receive
the expected value instead of only the "tail of ..." Write-Host lines.

}

function Wait-ForCondition {
param(
[scriptblock]$Condition,
[int]$TimeoutSeconds = 60
)
$start = Get-Date
while ((Get-Date) - $start -lt [TimeSpan]::FromSeconds($TimeoutSeconds)) {
if (& $Condition) {
return $true
}
Start-Sleep -Seconds 2
}
return $false
}

Export-ModuleMember -Function Get-Configs, Test-Endpoints, Process-Tokens, Get-LogFiles, Wait-ForCondition
46 changes: 46 additions & 0 deletions psscriptanalyzer/Deploy-DemoApp.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
param(
[string]$Environment = "production",
[string]$ArtifactPath
)

$Global:DeployConfig = @{
Environment = $Environment
Region = "us-east-1"
StartedAt = Get-Date
}

function Process-Database {
Write-Host "Migrating database for $($Global:DeployConfig.Environment)"
$tempPath = "C:\temp\migration"
cd C:\app\db
ls *.sql | % { Write-Host "queued $_" }
$result = Invoke-Expression "psql -f schema.sql"
if ($result -eq $null) {
Write-Host "no output"
}
Comment on lines +13 to +20
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

You're queueing every SQL file but executing only schema.sql.

The function logs all discovered *.sql files, then the deploy actually runs one hardcoded script. Any additional migrations are silently skipped, which makes ordered migrations and reruns incorrect. Iterate the discovered files, or narrow the queue message to the single file you really intend to execute.

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 13-13: Found global variable 'Global:DeployConfig'.

(PSAvoidGlobalVars)


[warning] 14-14: The variable 'tempPath' is assigned but never used.

(PSUseDeclaredVarsMoreThanAssignments)


[warning] 17-17: Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.

(PSAvoidUsingInvokeExpression)


[warning] 18-18: $null should be on the left side of equality comparisons.

Suggested fix: Use $null on the left hand side for safe comparison with $null.

(PSPossibleIncorrectComparisonWithNull)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Deploy-DemoApp.ps1` around lines 13 - 20, The script
currently lists all SQL files with "ls *.sql | % { Write-Host "queued $_" }" but
then only executes a hardcoded "schema.sql" via Invoke-Expression, causing other
migrations to be skipped; change the behavior by iterating the discovered files
(e.g., get-childitem *.sql | Sort-Object Name) and for each file call psql -f
<filename> (replacing the single Invoke-Expression "psql -f schema.sql"),
capture the command output into $result per-file and handle null/erroneous
results/logging with Write-Host or proper error handling; update references to
the listing pipeline and the Invoke-Expression call so the queued messages match
the executed files and migrations run in order.

}

function Get-Users {
$users = gci C:\app\users -Filter *.json
foreach ($user in $users) {
$content = cat $user.FullName
Write-Host $content
}
Comment on lines +23 to +28
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't print raw user records during deployment.

Dumping each JSON file's full contents will leak PII/secrets into deployment logs. Log the filename or a redacted subset of fields instead.

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 23-23: The cmdlet 'Get-Users' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Get-Users'

(PSUseSingularNouns)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Deploy-DemoApp.ps1` around lines 23 - 28, The Get-Users
function currently reads each JSON file into $content and writes the raw
contents to the console, which exposes PII/secrets; change it so it no longer
prints full file contents: instead log only the filename (use $user.Name or
$user.BaseName) or parse the JSON and explicitly output a small safe subset of
fields (e.g., id/status) while removing/redacting sensitive fields before
logging; update the Write-Host call that currently emits $content and ensure any
file reads use Get-Content/ConvertFrom-Json only to extract and sanitize
specific keys rather than dumping the entire file.

}

function Update-Service {
param(
[string]$Name,
[string]$Token
)
$endpoint = "https://control.internal.example.com/v1/services/$Name/restart"
$headers = @{ Authorization = "Bearer $Token" }
Invoke-RestMethod -Uri $endpoint -Method Post -Headers $headers
Comment on lines +31 to +38
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Remove the committed bearer token from the restart call.

The literal token on Line 45 is enough to authenticate against the internal restart endpoint constructed in Update-Service. Pass it in securely at runtime and keep it out of source control and command history.

Also applies to: 45-45

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 31-31: Function 'Update-Service' has verb that could change system state. Therefore, the function has to support 'ShouldProcess'.

(PSUseShouldProcessForStateChangingFunctions)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Deploy-DemoApp.ps1` around lines 31 - 38, Update-Service
currently accepts a $Token parameter but a literal bearer token was committed
and used for the restart call; remove the hard-coded token and ensure the
function uses the passed-in $Token value instead, and update any callers (e.g.,
where the literal token was used) to supply the token securely at runtime (for
example by reading from an environment variable, a secret store, or prompting
with Read-Host -AsSecureString and converting to a string) so no token appears
in source or command history; make sure the Authorization header creation in
Update-Service uses the $Token parameter (Authorization = "Bearer $Token") and
that all invocations of Update-Service are changed to obtain and pass the secret
securely.

}

cd $PSScriptRoot
Write-Host "Starting deploy of $ArtifactPath"
Process-Database
Get-Users
Update-Service -Name "demoapp-web" -Token "dpt_a1b2c3d4e5f6789012345678901234ab"
Write-Host "deploy complete"
60 changes: 60 additions & 0 deletions psscriptanalyzer/Initialize-Environment.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
param(
[string]$Environment = "production",
[string]$ConfigPath = "C:\app\config",
[string]$VaultToken
)
Comment on lines +1 to +5
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The advertised token-based auth path is never used.

The script accepts $VaultToken on Line 4, but the startup path always calls Initialize-Vault with hardcoded username/password on Line 57. A caller can pass a token and still silently fall back to the embedded credentials. Either wire the token into the vault request or remove the parameter.

Also applies to: 56-57

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 4-4: The parameter 'VaultToken' has been declared but not used.

(PSReviewUnusedParameter)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Initialize-Environment.ps1` around lines 1 - 5, The script
declares the parameter $VaultToken but never uses it when calling
Initialize-Vault; update the Initialize-Vault invocation to accept and pass the
token when $VaultToken is provided (e.g., call Initialize-Vault -Token
$VaultToken) so token-based auth is used instead of always using hardcoded
username/password, and ensure Initialize-Vault supports a Token parameter;
alternatively, if token auth is not desired, remove the $VaultToken parameter
and related docs so callers aren't misled.


$Global:EnvSettings = @{
Environment = $Environment
ConfigPath = $ConfigPath
LoadedAt = Get-Date
}

function Initialize-Vault {
param(
[string]$Username = "vault-admin",
[string]$Password = "V@ultM@ster2024"
)
Write-Host "initializing vault for $Username"
$secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
$cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
$endpoint = "https://vault.internal.example.com/v1/sys/init"
$response = Invoke-RestMethod -Uri $endpoint -Method Post -Credential $cred
if ($response -eq $null) {
Write-Host "vault did not respond"
}
return $response
}
Comment on lines +13 to +27
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Hardcoded credentials in Initialize-Vault function.

Lines 15-16 contain hardcoded default username and password values. Line 57 also passes these same credentials explicitly. Use [PSCredential] and require the caller to provide credentials securely.

🔒 Proposed fix using PSCredential
 function Initialize-Vault {
     param(
-        [string]$Username = "vault-admin",
-        [string]$Password = "V@ultM@ster2024"
+        [Parameter(Mandatory=$true)]
+        [PSCredential]$Credential
     )
-    Write-Host "initializing vault for $Username"
-    $secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
-    $cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
+    Write-Host "initializing vault for $($Credential.UserName)"
     $endpoint = "https://vault.internal.example.com/v1/sys/init"
-    $response = Invoke-RestMethod -Uri $endpoint -Method Post -Credential $cred
-    if ($response -eq $null) {
+    $response = Invoke-RestMethod -Uri $endpoint -Method Post -Credential $Credential
+    if ($null -eq $response) {
         Write-Host "vault did not respond"
     }
     return $response
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function Initialize-Vault {
param(
[string]$Username = "vault-admin",
[string]$Password = "V@ultM@ster2024"
)
Write-Host "initializing vault for $Username"
$secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
$cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
$endpoint = "https://vault.internal.example.com/v1/sys/init"
$response = Invoke-RestMethod -Uri $endpoint -Method Post -Credential $cred
if ($response -eq $null) {
Write-Host "vault did not respond"
}
return $response
}
function Initialize-Vault {
param(
[Parameter(Mandatory=$true)]
[PSCredential]$Credential
)
Write-Host "initializing vault for $($Credential.UserName)"
$endpoint = "https://vault.internal.example.com/v1/sys/init"
$response = Invoke-RestMethod -Uri $endpoint -Method Post -Credential $Credential
if ($null -eq $response) {
Write-Host "vault did not respond"
}
return $response
}
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[error] 15-16: Function 'Initialize-Vault' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute.

(PSAvoidUsingUsernameAndPasswordParams)


[warning] 16-16: Parameter '$Password' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to expose this sensitive information.

Suggested fix: Set $Password type to SecureString; Set $Password type to PSCredential

(PSAvoidUsingPlainTextForPassword)


[error] 19-19: File 'Initialize-Environment.ps1' uses ConvertTo-SecureString with plaintext. This will expose secure information. Encrypted standard strings should be used instead.

(PSAvoidUsingConvertToSecureStringWithPlainText)


[warning] 23-23: $null should be on the left side of equality comparisons.

Suggested fix: Use $null on the left hand side for safe comparison with $null.

(PSPossibleIncorrectComparisonWithNull)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Initialize-Environment.ps1` around lines 13 - 27, The
Initialize-Vault function currently hardcodes username/password; change its
signature to require a [PSCredential] parameter (e.g. [PSCredential]$Credential)
instead of string defaults, remove ConvertTo-SecureString/New-Object usage
inside the function, use the passed-in $Credential directly in
Invoke-RestMethod, add a null-check that throws/terminates if $Credential is not
provided, and update any callers that currently pass literal credentials to
instead supply a PSCredential (via Get-Credential or creating a secure
PSCredential) when calling Initialize-Vault.


function Load-Configurations {
$configs = gci $Global:EnvSettings.ConfigPath -Filter *.json
foreach ($config in $configs) {
$content = cat $config.FullName
$parsed = $content | ConvertFrom-Json
Write-Host "loaded $($config.Name)"
}
}

function Process-Templates {
param([string]$TemplateDir)
cd $TemplateDir
ls *.tmpl | % {
$rendered = Invoke-Expression "Get-Content $($_.FullName) | Render-Template"
$outputName = $_.Name -replace '\.tmpl$', ''
Set-Content -Path $outputName -Value $rendered
}
}
Comment on lines +38 to +46
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Invoke-Expression with string interpolation poses injection risk.

Line 42 uses Invoke-Expression with an interpolated string containing $_.FullName. If file paths contain special characters or malicious content, this could execute unintended code. Use the call operator (&) or Invoke-Command instead.

♻️ Proposed fix using pipeline
 function Process-Templates {
     param([string]$TemplateDir)
-    cd $TemplateDir
-    ls *.tmpl | % {
-        $rendered = Invoke-Expression "Get-Content $($_.FullName) | Render-Template"
+    Get-ChildItem -Path $TemplateDir -Filter *.tmpl | ForEach-Object {
+        $rendered = Get-Content $_.FullName | Render-Template
         $outputName = $_.Name -replace '\.tmpl$', ''
-        Set-Content -Path $outputName -Value $rendered
+        Set-Content -Path (Join-Path $TemplateDir $outputName) -Value $rendered
     }
 }
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 42-42: Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.

(PSAvoidUsingInvokeExpression)


[warning] 38-38: The cmdlet 'Process-Templates' uses an unapproved verb.

(PSUseApprovedVerbs)


[warning] 38-38: The cmdlet 'Process-Templates' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Process-Templates'

(PSUseSingularNouns)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Initialize-Environment.ps1` around lines 38 - 46, The
Process-Templates function currently builds a string and calls Invoke-Expression
with "$_.FullName", which creates an injection risk; replace that by reading the
template content safely and piping it into Render-Template (or calling
Render-Template directly) using safe file APIs: use Get-Content -Raw
-LiteralPath $_.FullName | Render-Template to produce $rendered, avoid
Invoke-Expression entirely, and keep Set-Content -Path $outputName -Value
$rendered; also prefer ForEach-Object/foreach and use -LiteralPath when
referencing $_.FullName to prevent path interpretation.


function Get-EnvironmentVariables {
$vars = Get-ChildItem env:
$unused = "this var is never read"
foreach ($var in $vars) {
Write-Host "$($var.Name) = $($var.Value)"
}
Comment on lines +48 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't dump the full environment to the log.

This will print secrets such as tokens, connection strings, and cloud credentials into CI output and shell history. Filter to an allowlist or redact sensitive names/values before logging anything from env:.

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 50-50: The variable 'unused' is assigned but never used.

(PSUseDeclaredVarsMoreThanAssignments)


[warning] 48-48: The cmdlet 'Get-EnvironmentVariables' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Get-EnvironmentVariables'

(PSUseSingularNouns)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Initialize-Environment.ps1` around lines 48 - 53, The
Get-EnvironmentVariables function currently writes every env: variable to the
host (including secrets); change it to either iterate a defined allowlist of
safe variable names (e.g., PATH, HOME, USER, CI) or filter out sensitive keys
and redact values before logging by detecting names matching patterns like
TOKEN, KEY, SECRET, PASSWORD, CREDENTIAL, AWS, AZURE, GCP, CONNECTION; update
the loop in Get-EnvironmentVariables to only log allowed names or to replace
sensitive values with a masked string (e.g., show only first/last chars or
"<REDACTED>") and remove the unused $unused variable. Ensure the change is
implemented inside Get-EnvironmentVariables so callers stay the same.

}

cd $PSScriptRoot
Initialize-Vault -Username "vault-admin" -Password "V@ultM@ster2024"
Load-Configurations
Process-Templates -TemplateDir "C:\app\templates"
Get-EnvironmentVariables
62 changes: 62 additions & 0 deletions psscriptanalyzer/Invoke-HealthCheck.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
param(
[string]$Target = "demoapp-web",
[int]$Retries = 3,
[string]$AdminPassword = "HealthAdm1n!"
)
Comment on lines +1 to +5
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Hardcoded password in default parameter and unused $Retries parameter.

The $AdminPassword default exposes credentials in source control. The $Retries parameter is declared but never used in the script logic.

🔒 Proposed fix
 param(
     [string]$Target = "demoapp-web",
-    [int]$Retries = 3,
-    [string]$AdminPassword = "HealthAdm1n!"
+    [Parameter(Mandatory=$true)]
+    [SecureString]$AdminPassword
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
param(
[string]$Target = "demoapp-web",
[int]$Retries = 3,
[string]$AdminPassword = "HealthAdm1n!"
)
param(
[string]$Target = "demoapp-web",
[Parameter(Mandatory=$true)]
[SecureString]$AdminPassword
)
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 3-3: The parameter 'Retries' has been declared but not used.

(PSReviewUnusedParameter)


[warning] 4-4: Parameter '$AdminPassword' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to expose this sensitive information.

Suggested fix: Set $AdminPassword type to SecureString; Set $AdminPassword type to PSCredential

(PSAvoidUsingPlainTextForPassword)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Invoke-HealthCheck.ps1` around lines 1 - 5, The param block
in Invoke-HealthCheck.ps1 exposes a hardcoded credential via the default value
of $AdminPassword and declares an unused $Retries parameter; remove the
hardcoded default and require or securely prompt for the admin password (e.g.,
make $AdminPassword mandatory or call Read-Host -AsSecureString where
Invoke-HealthCheck consumes it) and implement retry logic using the existing
$Retries parameter in the health-check operation (locate the health check call
in the script and wrap it in a loop or retry helper that attempts the check up
to $Retries times with optional delay and proper error handling).


$Global:HealthState = @{
Target = $Target
LastCheck = $null
Status = "unknown"
}

function Check-Endpoints {
$endpoints = @(
"https://demoapp.example.com/health",
"https://api.demoapp.example.com/healthz",
"https://admin.demoapp.example.com/_status"
)
foreach ($url in $endpoints) {
$resp = iwr -Uri $url -UseBasicParsing
if ($resp.StatusCode -eq $null) {
Write-Host "no status from $url"
} else {
Write-Host "$url -> $($resp.StatusCode)"
}
Comment on lines +19 to +25
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

A single bad endpoint currently aborts the whole health-check pass.

Invoke-WebRequest throws before $resp.StatusCode is inspected, so one DNS/TLS/HTTP failure stops the script instead of logging "no status" and moving on to the remaining endpoints. Handle the request per-URL with try/catch and continue the loop.

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 21-21: $null should be on the left side of equality comparisons.

Suggested fix: Use $null on the left hand side for safe comparison with $null.

(PSPossibleIncorrectComparisonWithNull)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Invoke-HealthCheck.ps1` around lines 19 - 25, The loop over
$endpoints currently calls iwr (Invoke-WebRequest) directly so a thrown
exception aborts the whole pass; wrap the per-URL request in a try/catch around
the iwr call inside the foreach ($url in $endpoints) so any exception for that
$url is caught, log "no status from $url" (or include the error if desired) in
the catch block, and continue the loop; ensure you only inspect $resp.StatusCode
when $resp was successfully assigned (e.g. set $resp = $null before try, assign
inside try, and handle null/StatusCode in the existing if/else).

}
Comment on lines +19 to +26
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Incorrect $null comparison placement.

Line 21 compares $resp.StatusCode -eq $null. Place $null on the left side to avoid unexpected behavior with collections.

♻️ Proposed fix
-      if ($resp.StatusCode -eq $null) {
+      if ($null -eq $resp.StatusCode) {
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 21-21: $null should be on the left side of equality comparisons.

Suggested fix: Use $null on the left hand side for safe comparison with $null.

(PSPossibleIncorrectComparisonWithNull)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Invoke-HealthCheck.ps1` around lines 19 - 26, The
null-comparison in the foreach loop is written as "$resp.StatusCode -eq $null",
which can behave incorrectly with collections; change the check to use $null on
the left (i.e. "$null -eq $resp.StatusCode") in the loop that iterates endpoints
and examines $resp.StatusCode so the null test is reliable for single values and
collections.

}

function Process-DiagnosticBundle {
param(
[string]$Username,
[string]$Password
)
$secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
$cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
$tempPath = Join-Path $env:TEMP "diag"
$command = "& 'C:\Program Files\Demoapp\diag.exe' --user $Username --password $Password"
$output = Invoke-Expression $command
return $output
}
Comment on lines +29 to +40
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Command injection risk and credential exposure in Process-DiagnosticBundle.

  1. Invoke-Expression with credentials: Line 37-38 interpolates $Username and $Password directly into a command string, enabling command injection if input is malicious.
  2. Unused variables: $cred (Line 35) and $tempPath (Line 36) are assigned but never used.
  3. Unapproved verb: Process is not an approved PowerShell verb; consider Invoke-DiagnosticBundle or Get-DiagnosticBundle.
🔒 Proposed fix using call operator
 function Process-DiagnosticBundle {
     param(
-        [string]$Username,
-        [string]$Password
+        [Parameter(Mandatory=$true)]
+        [PSCredential]$Credential
     )
-    $secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
-    $cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
-    $tempPath = Join-Path $env:TEMP "diag"
-    $command = "& 'C:\Program Files\Demoapp\diag.exe' --user $Username --password $Password"
-    $output = Invoke-Expression $command
+    $Username = $Credential.UserName
+    $Password = $Credential.GetNetworkCredential().Password
+    $output = & 'C:\Program Files\Demoapp\diag.exe' --user $Username --password $Password
     return $output
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function Process-DiagnosticBundle {
param(
[string]$Username,
[string]$Password
)
$secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
$cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
$tempPath = Join-Path $env:TEMP "diag"
$command = "& 'C:\Program Files\Demoapp\diag.exe' --user $Username --password $Password"
$output = Invoke-Expression $command
return $output
}
function Process-DiagnosticBundle {
param(
[Parameter(Mandatory=$true)]
[PSCredential]$Credential
)
$Username = $Credential.UserName
$Password = $Credential.GetNetworkCredential().Password
$output = & 'C:\Program Files\Demoapp\diag.exe' --user $Username --password $Password
return $output
}
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[error] 34-34: File 'Invoke-HealthCheck.ps1' uses ConvertTo-SecureString with plaintext. This will expose secure information. Encrypted standard strings should be used instead.

(PSAvoidUsingConvertToSecureStringWithPlainText)


[warning] 35-35: The variable 'cred' is assigned but never used.

(PSUseDeclaredVarsMoreThanAssignments)


[warning] 36-36: The variable 'tempPath' is assigned but never used.

(PSUseDeclaredVarsMoreThanAssignments)


[warning] 38-38: Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.

(PSAvoidUsingInvokeExpression)


[warning] 29-29: The cmdlet 'Process-DiagnosticBundle' uses an unapproved verb.

(PSUseApprovedVerbs)


[warning] 32-32: Parameter '$Password' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to expose this sensitive information.

Suggested fix: Set $Password type to SecureString; Set $Password type to PSCredential

(PSAvoidUsingPlainTextForPassword)


[error] 31-32: Function 'Process-DiagnosticBundle' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute.

(PSAvoidUsingUsernameAndPasswordParams)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Invoke-HealthCheck.ps1` around lines 29 - 40, The
Process-DiagnosticBundle function has a command-injection risk from building a
command string and using Invoke-Expression, exposes plaintext credentials, and
leaves unused variables ($cred, $tempPath); rename the function to an approved
verb like Invoke-DiagnosticBundle, remove unused $tempPath, keep $cred
(ConvertTo-SecureString + PSCredential) and pass credentials/args safely by
invoking the executable with separate arguments (build an args array such as
'--user', $Username, '--password', $Password and call the executable with the
call operator and splatting) instead of using Invoke-Expression, and return the
process output; ensure you reference the executable path string (the
$command/$exe variable) and the $Username/$Password parameters when implementing
these changes.


function Get-Statuses {
$services = gci -Path Cert:\LocalMachine\My
foreach ($svc in $services) {
Write-Host "$($svc.Subject) expires $($svc.NotAfter)"
}
}

function Restart-StuckServices {
param([string[]]$Names)
foreach ($name in $Names) {
Stop-Service -Name $name -Force
Start-Service -Name $name
Write-Host "restarted $name"
}
}
Comment on lines +49 to +56
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

State-changing function lacks ShouldProcess support.

Restart-StuckServices stops and starts services but doesn't implement -WhatIf or -Confirm support, which is expected for functions that modify system state.

♻️ Proposed fix adding ShouldProcess
-function Restart-StuckServices {
+function Restart-StuckService {
+    [CmdletBinding(SupportsShouldProcess=$true)]
     param([string[]]$Names)
     foreach ($name in $Names) {
-        Stop-Service -Name $name -Force
-        Start-Service -Name $name
-        Write-Host "restarted $name"
+        if ($PSCmdlet.ShouldProcess($name, "Restart service")) {
+            Stop-Service -Name $name -Force
+            Start-Service -Name $name
+            Write-Host "restarted $name"
+        }
     }
 }
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 49-49: Function 'Restart-StuckServices' has verb that could change system state. Therefore, the function has to support 'ShouldProcess'.

(PSUseShouldProcessForStateChangingFunctions)


[warning] 49-49: The cmdlet 'Restart-StuckServices' uses a plural noun. A singular noun should be used instead.

Suggested fix: Singularized correction of 'Restart-StuckServices'

(PSUseSingularNouns)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Invoke-HealthCheck.ps1` around lines 49 - 56, Add
ShouldProcess support to Restart-StuckServices by converting it to an advanced
function: add [CmdletBinding(SupportsShouldProcess=$true)] and accept the same
[string[]]$Names param; then wrap the stop/start operations in a ShouldProcess
check (e.g. if ($PSCmdlet.ShouldProcess($name, 'Restart service')) {
Stop-Service -Name $name -Force; Start-Service -Name $name; Write-Host
"restarted $name" }) so -WhatIf and -Confirm are honored when calling
Restart-StuckServices.


cd $PSScriptRoot
Check-Endpoints
Process-DiagnosticBundle -Username "diag-admin" -Password $AdminPassword
Get-Statuses
Restart-StuckServices -Names @("DemoappWeb", "DemoappWorker")
38 changes: 38 additions & 0 deletions psscriptanalyzer/Manage-Users.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
function New-LocalAdmin {
param(
[string]$Username,
[string]$Password
)
Write-Host "creating user $Username"
$secure = ConvertTo-SecureString -String $Password -AsPlainText -Force
$cred = New-Object System.Management.Automation.PSCredential($Username, $secure)
New-LocalUser -Name $Username -Password $secure -PasswordNeverExpires
Add-LocalGroupMember -Group "Administrators" -Member $Username
Comment on lines +9 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This provisioning step isn't idempotent.

Re-running the script will fail once the user already exists or is already in Administrators, which makes retries and configuration re-application brittle. Guard both operations with existence/membership checks before mutating state.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Manage-Users.ps1` around lines 9 - 10, Make the provisioning
idempotent by checking for the user's existence and group membership before
calling New-LocalUser and Add-LocalGroupMember: use Get-LocalUser -Name
$Username (or Test-Path equivalent) to skip New-LocalUser when the account
already exists, and use Get-LocalGroupMember -Group "Administrators" (or
Get-LocalGroupMember -Name) to verify $Username is not already a member before
calling Add-LocalGroupMember; only perform each mutation when the respective
check indicates it’s missing.

return $cred
}

function Connect-AdminSession {
param(
[string]$ComputerName,
[string]$AdminPassword = "AdminP@ss2024"
)
Comment on lines +14 to +18
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Remove the committed admin passwords from this flow.

Both the default AdminPassword and the script-level $adminPassword are hardcoded in source and then used to provision privileged access. That is an immediate secret leak and makes the account reusable by anyone with repository or log access. Accept a PSCredential/SecureString and source it at runtime instead.

Also applies to: 32-34

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 17-17: Parameter '$AdminPassword' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to expose this sensitive information.

Suggested fix: Set $AdminPassword type to SecureString; Set $AdminPassword type to PSCredential

(PSAvoidUsingPlainTextForPassword)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Manage-Users.ps1` around lines 14 - 18, The function
Connect-AdminSession (and the script-level $adminPassword variable referenced on
lines ~32-34) currently embed a hardcoded plaintext password; remove the default
AdminPassword parameter and any script-level hardcoded $adminPassword, and
instead accept a PSCredential object or SecureString (e.g., add a
[System.Management.Automation.PSCredential]$Credential or
[SecureString]$AdminPassword parameter) so callers must supply credentials at
runtime; update all call sites in this file to use the new parameter and
document that credentials must be created/loaded securely (Get-Credential,
ConvertTo-SecureString with protected storage, or use a vault) rather than
hardcoding secrets.

$secure = ConvertTo-SecureString -String $AdminPassword -AsPlainText -Force
$cred = New-Object System.Management.Automation.PSCredential("Administrator", $secure)
Enter-PSSession -ComputerName $ComputerName -Credential $cred
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Replace interactive remoting call in automation script

Connect-AdminSession calls Enter-PSSession, which is designed for interactive use and is not intended to be invoked from scripts; in non-interactive runs (for example CI or scheduled execution), this can block or fail at this point and prevent the subsequent Reset-UserPassword step from executing. Use a non-interactive remoting pattern (New-PSSession/Invoke-Command) so the script can complete unattended.

Useful? React with 👍 / 👎.

}

function Reset-UserPassword {
param([string]$Username)
$newPassword = "Temp_P@ss_$(Get-Random)"
$secure = ConvertTo-SecureString -String $newPassword -AsPlainText -Force
Set-LocalUser -Name $Username -Password $secure
Write-Host "password reset for $Username to $newPassword"
Comment on lines +24 to +29
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Don't print the reset password to stdout.

Line 29 writes the newly generated password directly into logs/transcripts, which defeats the rotation entirely. Return it through a secure channel or store it in a vault/out-of-band handoff instead.

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[error] 27-27: File 'Manage-Users.ps1' uses ConvertTo-SecureString with plaintext. This will expose secure information. Encrypted standard strings should be used instead.

(PSAvoidUsingConvertToSecureStringWithPlainText)


[warning] 24-24: Function 'Reset-UserPassword' has verb that could change system state. Therefore, the function has to support 'ShouldProcess'.

(PSUseShouldProcessForStateChangingFunctions)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/Manage-Users.ps1` around lines 24 - 29, The
Reset-UserPassword function currently generates $newPassword and writes it to
stdout via Write-Host, which exposes secrets; remove the Write-Host call and
stop printing $newPassword. Instead have Reset-UserPassword return a secure
object (for example a SecureString or a PSCredential) or call a vault/handoff
API to store the secret (e.g., accept an optional delegate or call
Save-SecretVault($Username, $secure) inside Reset-UserPassword), ensure the raw
$newPassword is only converted to a SecureString ($secure) and never written to
logs or host output.

}

$adminUsername = "demoapp-admin"
$adminPassword = "Adm1nP@ssword2024"
New-LocalAdmin -Username $adminUsername -Password $adminPassword

$serviceAccount = "demoapp-svc"
Connect-AdminSession -ComputerName "BUILD-AGENT-01"
Reset-UserPassword -Username $serviceAccount
49 changes: 49 additions & 0 deletions psscriptanalyzer/PSScriptAnalyzerSettings.psd1
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
@{
Severity = @('Error', 'Warning', 'Information')

IncludeDefaultRules = $true

IncludeRules = @(
'PSAvoidUsingCmdletAliases',
'PSAvoidUsingWriteHost',
'PSAvoidUsingInvokeExpression',
'PSAvoidUsingPlainTextForPassword',
'PSAvoidUsingConvertToSecureStringWithPlainText',
'PSAvoidUsingUsernameAndPasswordParams',
'PSAvoidGlobalVars',
'PSPossibleIncorrectComparisonWithNull',
'PSUseDeclaredVarsMoreThanAssignments',
'PSUseShouldProcessForStateChangingFunctions',
'PSUseSingularNouns',
'PSUseApprovedVerbs',
'PSUsePSCredentialType',
'PSReviewUnusedParameter',
'PSProvideCommentHelp',
'PSAvoidTrailingWhitespace',
'PSUseConsistentIndentation',
'PSUseConsistentWhitespace',
'PSAvoidUsingPositionalParameters',
'PSAvoidUsingDoubleQuotesForConstantString'
)

Rules = @{
PSUseConsistentIndentation = @{
Enable = $true
IndentationSize = 4
Kind = 'space'
Comment on lines +31 to +33
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Normalize assignment spacing to satisfy PSUseConsistentWhitespace.

These lines use alignment padding around =, which triggers the whitespace rule configured in this same settings file. Use single spaces around assignments.

Suggested cleanup
         PSUseConsistentIndentation = @{
-            Enable          = $true
+            Enable = $true
             IndentationSize = 4
-            Kind            = 'space'
+            Kind = 'space'
         }
         PSUseConsistentWhitespace = @{
-            Enable                          = $true
-            CheckOpenBrace                  = $true
-            CheckOpenParen                  = $true
-            CheckOperator                   = $true
-            CheckSeparator                  = $true
+            Enable = $true
+            CheckOpenBrace = $true
+            CheckOpenParen = $true
+            CheckOperator = $true
+            CheckSeparator = $true
         }
         PSPlaceOpenBrace = @{
-            Enable             = $true
-            OnSameLine         = $true
-            NewLineAfter       = $true
+            Enable = $true
+            OnSameLine = $true
+            NewLineAfter = $true
             IgnoreOneLineBlock = $true
         }

Also applies to: 36-40, 43-45

🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)

[warning] 31-31: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)


[warning] 33-33: Use space before and after binary and assignment operators.

Suggested fix: Use space before and after binary and assignment operators.

(PSUseConsistentWhitespace)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@psscriptanalyzer/PSScriptAnalyzerSettings.psd1` around lines 31 - 33, The
assignments in the settings hashtable (e.g., keys Enable, IndentationSize, Kind)
use alignment padding around '=' which violates PSUseConsistentWhitespace;
update these entries to use a single space on each side of the '=' (e.g.,
"Enable = $true") and apply the same change for the other affected entries
referenced (lines around 36-40 and 43-45) so all assignment operators in
PSScriptAnalyzerSettings.psd1 use consistent single-space spacing.

}
PSUseConsistentWhitespace = @{
Enable = $true
CheckOpenBrace = $true
CheckOpenParen = $true
CheckOperator = $true
CheckSeparator = $true
}
PSPlaceOpenBrace = @{
Enable = $true
OnSameLine = $true
NewLineAfter = $true
IgnoreOneLineBlock = $true
}
}
}
Loading