Confirm fork is fully synced with upstream; fix broken string escaping in Dell BIOS script - #3
Conversation
Co-authored-by: vartaxe <11176349+vartaxe@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The updated log masking still uses -replace $Password without regex-escaping the password, which can fail to mask and potentially leak secrets when passwords contain regex metacharacters.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR audits the fork’s parity with MSEndpointMgr/ModernBIOSManagement and fixes a PowerShell parsing issue in the Dell BIOS update script caused by incorrect quote escaping inside subexpressions.
Changes:
- Replaced invalid backslash-escaped quotes (
\"...\") with valid PowerShell string quoting inside$()inInvoke-DellBIOSUpdate.ps1. - Restored the affected log statements so the script parses cleanly again.
File summaries
| File | Description |
|---|---|
| Invoke-DellBIOSUpdate.ps1 | Fixes invalid quote escaping in three log statements to restore PowerShell parse correctness. |
Review details
Suppressed comments (2)
Invoke-DellBIOSUpdate.ps1:222
-replacetreats the pattern as a regex; using the raw$Passwordcan cause the mask to fail for passwords containing regex metacharacters, which can leak secrets in logs. Use[regex]::Escape($Password)for the pattern.
Write-CMLogEntry -Value "Using the following switches for Flash64W.exe: $($FlashSwitches -replace $Password, "<password removed>")" -Severity 1
Invoke-DellBIOSUpdate.ps1:247
-replaceinterprets the pattern as a regex; passing$Passworddirectly can make masking unreliable for passwords containing regex metacharacters and could expose the password in this log line. Escape the password before replacing.
Write-CMLogEntry -Value "Using the following switches for BIOS file: $($FileSwitches -replace $Password, "<password removed>")" -Severity 1
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Start flash update process | ||
| if (-not([System.String]::IsNullOrEmpty($Password))) { | ||
| Write-CMLogEntry -Value "Using the following switches for Flash64W.exe: $($FlashSwitches -replace $Password, \"<password removed>\")" -Severity 1 | ||
| Write-CMLogEntry -Value "Using the following switches for Flash64W.exe: $($FlashSwitches -replace $Password, "<password removed>")" -Severity 1 |
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
Verified whether this fork was up to date with
MSEndpointMgr/ModernBIOSManagementand checked for other issues to fix.Upstream sync verification
upstream/mainand confirmedHEAD..upstream/mainis empty — all upstream commits are already ancestors of this branch via a prior merge. No new upstream changes to pull in.Bug found during audit
.ps1scripts with the PowerShell language parser to catch latent syntax issues.Invoke-DellBIOSUpdate.ps1failed to parse: three log statements used backslash-escaped quotes (\"<password removed>\") inside a$(...)subexpression. PowerShell doesn't treat backslash as an escape character, so this broke parsing of the script. This was introduced by an earlier merge and diverged from upstream's original (valid) syntax.