-
Notifications
You must be signed in to change notification settings - Fork 8
Preview/Psscriptanalyzer #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| reviews: | ||
| tools: | ||
| gitleaks: | ||
| enabled: false | ||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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.psd1Repository: coderabbitai/coderabbit-pr-review Length of output: 813 Normalize assignment spacing to satisfy PSScriptAnalyzer.
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 AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tighten module description to match actual exported surface. Line 8 says “backup and restore helpers,” but the module exports only backup-oriented functions (see 🧰 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 |
||
| PowerShellVersion = '5.1' | ||
|
|
||
| FunctionsToExport = @( | ||
| 'Get-Backups', | ||
| 'Remove-Backup', | ||
| 'Process-Backup', | ||
| 'Verify-Backups' | ||
| ) | ||
| CmdletsToExport = @() | ||
| VariablesToExport = @() | ||
| AliasesToExport = @() | ||
| } | ||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Command injection vulnerability and insecure credential handling. This function has multiple security issues:
Replace 🔒 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 |
||
|
|
||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Null comparison order can cause incorrect results. On line 33, 🐛 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 |
||
|
|
||
| Export-ModuleMember -Function Get-Backups, Remove-Backup, Process-Backup, Verify-Backups | ||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Using ♻️ 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
Suggested change
🧰 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 |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect Line 14: Place ♻️ 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
Suggested change
🧰 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 |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded API key and insecure credential handling in
🔒 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 |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🧰 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 |
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're queueing every SQL file but executing only The function logs all discovered 🧰 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 |
||
| } | ||
|
|
||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| } | ||
|
|
||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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 |
||
| } | ||
|
|
||
| cd $PSScriptRoot | ||
| Write-Host "Starting deploy of $ArtifactPath" | ||
| Process-Database | ||
| Get-Users | ||
| Update-Service -Name "demoapp-web" -Token "dpt_a1b2c3d4e5f6789012345678901234ab" | ||
| Write-Host "deploy complete" | ||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The advertised token-based auth path is never used. The script accepts 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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded credentials in Lines 15-16 contain hardcoded default username and password values. Line 57 also passes these same credentials explicitly. Use 🔒 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
Suggested change
🧰 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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Line 42 uses ♻️ 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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 🧰 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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cd $PSScriptRoot | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Initialize-Vault -Username "vault-admin" -Password "V@ultM@ster2024" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Load-Configurations | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Process-Templates -TemplateDir "C:\app\templates" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Get-EnvironmentVariables | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded password in default parameter and unused The 🔒 Proposed fix param(
[string]$Target = "demoapp-web",
- [int]$Retries = 3,
- [string]$AdminPassword = "HealthAdm1n!"
+ [Parameter(Mandatory=$true)]
+ [SecureString]$AdminPassword
)📝 Committable suggestion
Suggested change
🧰 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 |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| $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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A single bad endpoint currently aborts the whole health-check pass.
🧰 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 |
||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+19
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect Line 21 compares ♻️ 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 |
||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Command injection risk and credential exposure in
🔒 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
Suggested change
🧰 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 |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. State-changing function lacks
♻️ 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 |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| cd $PSScriptRoot | ||||||||||||||||||||||||||||||||||||||||||||||
| Check-Endpoints | ||||||||||||||||||||||||||||||||||||||||||||||
| Process-DiagnosticBundle -Username "diag-admin" -Password $AdminPassword | ||||||||||||||||||||||||||||||||||||||||||||||
| Get-Statuses | ||||||||||||||||||||||||||||||||||||||||||||||
| Restart-StuckServices -Names @("DemoappWeb", "DemoappWorker") | ||||||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This provisioning step isn't idempotent. Re-running the script will fail once the user already exists or is already in 🤖 Prompt for AI Agents |
||
| return $cred | ||
| } | ||
|
|
||
| function Connect-AdminSession { | ||
| param( | ||
| [string]$ComputerName, | ||
| [string]$AdminPassword = "AdminP@ss2024" | ||
| ) | ||
|
Comment on lines
+14
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the committed admin passwords from this flow. Both the default 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 |
||
| $secure = ConvertTo-SecureString -String $AdminPassword -AsPlainText -Force | ||
| $cred = New-Object System.Management.Automation.PSCredential("Administrator", $secure) | ||
| Enter-PSSession -ComputerName $ComputerName -Credential $cred | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| } | ||
|
|
||
| $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 | ||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normalize assignment spacing to satisfy These lines use alignment padding around 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 |
||
| } | ||
| PSUseConsistentWhitespace = @{ | ||
| Enable = $true | ||
| CheckOpenBrace = $true | ||
| CheckOpenParen = $true | ||
| CheckOperator = $true | ||
| CheckSeparator = $true | ||
| } | ||
| PSPlaceOpenBrace = @{ | ||
| Enable = $true | ||
| OnSameLine = $true | ||
| NewLineAfter = $true | ||
| IgnoreOneLineBlock = $true | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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), andpsscriptanalyzer/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