Conversation
PR Summary by QodoAdd PowerShell 7.6.4 bundle with Nerd Font console setup
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1. Unpinned Oh My Posh exe
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+96
to
+104
| $signature = @' | ||
| [DllImport("gdi32.dll", CharSet = CharSet.Auto)] | ||
| public static extern int AddFontResourceEx(string lpszFilename, uint fl, IntPtr res); | ||
| '@ | ||
| $gdi32 = Add-Type -MemberDefinition $signature -Name "Gdi32" -Namespace "Win32" -PassThru | ||
| $gdi32::AddFontResourceEx($destFile, 0, [IntPtr]::Zero) | Out-Null | ||
| } catch { | ||
| Write-Error "Failed to install font $internalName" | ||
| } |
There was a problem hiding this comment.
1. Add-type redefined repeatedly 🐞 Bug ☼ Reliability
register-fonts.ps1 defines the Win32.Gdi32 type via Add-Type inside the per-font install loop, so after the first font it can fail because the type already exists, skipping AddFontResourceEx for later fonts and emitting a misleading "Failed to install" error. This makes multi-font packages register inconsistently in the current session.
Agent Prompt
### Issue description
`register-fonts.ps1` calls `Add-Type -Name "Gdi32" -Namespace "Win32"` inside the loop that processes each font file. After the first successful call, subsequent calls can throw because the type already exists; the catch then reports a generic install failure and `AddFontResourceEx` is not executed for remaining files.
### Issue Context
This matters when `vendor/fonts` contains multiple `.ttf/.otf` files and more than one needs installation/refresh.
### Fix Focus Areas
- bin/powershell7.6.4/config/register-fonts.ps1[91-105]
### Suggested fix
- Move the `Add-Type` for `Win32.Gdi32` outside the `ForEach-Object` loop and reuse the type for all files.
- Alternatively guard with `if (-not ('Win32.Gdi32' -as [type])) { Add-Type ... }`.
- Narrow the try/catch so a type-definition error doesn’t mask successful copy/registry steps.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| @@ -1,4 +1,4 @@ | |||
| oh_my_posh = https://github.com/JanDeDobbeleer/oh-my-posh/releases/download/v29.18.0/posh-windows-amd64.exe | |||
| oh_my_posh = https://github.com/JanDeDobbeleer/oh-my-posh/releases/latest/download/posh-windows-amd64.exe | |||
There was a problem hiding this comment.
2. Unpinned oh my posh exe 🐞 Bug ⛨ Security
deps.properties switches Oh My Posh to a GitHub "releases/latest" URL, making builds non-reproducible and allowing the downloaded executable to change over time. That downloaded executable is later invoked by the PowerShell profile and its generated init script is piped to Invoke-Expression, so an upstream change/compromise becomes direct code execution in user shells.
Agent Prompt
### Issue description
`oh_my_posh` now points to a mutable `releases/latest` asset. The build downloads that EXE into the shipped bundle and the profile executes it and `Invoke-Expression`s its generated output.
### Issue Context
- Mutable URLs break reproducibility and complicate incident response.
- Because the output is executed, integrity of the downloaded binary is critical.
### Fix Focus Areas
- bin/powershell7.5.4/deps.properties[1-1]
- bin/powershell7.6.4/deps.properties[1-2]
- build.gradle[659-689]
- bin/powershell7.6.4/config/Microsoft.PowerShell_profile.ps1[18-24]
### Suggested fix
- Pin `oh_my_posh` to an immutable versioned release URL (tagged version), not `latest`.
- Add a checksum property (e.g., `oh_my_posh_sha256`) and verify the download in `build.gradle` after `downloadFile(...)`.
- Consider hosting vetted binaries internally if this project requires stronger supply-chain control.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+136
to
+145
| for /f "tokens=2,*" %%A in ('reg query "HKCU\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Fonts" /v "!FONT_NAME! (TrueType)" 2^>nul') do ( | ||
| set "FONT_PATH=%%B" | ||
| if not "!FONT_PATH!"=="" reg add "HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Fonts" /v "!FONT_NAME! (TrueType)" /t REG_SZ /d "!FONT_PATH!" /f >nul 2>&1 | ||
| ) | ||
| reg add "HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Console\TrueTypeFont" /v "000" /t REG_SZ /d "!FONT_NAME!" /f >nul 2>&1 | ||
| reg add "HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Console\TrueTypeFont" /v "0000" /t REG_SZ /d "!FONT_NAME!" /f >nul 2>&1 | ||
| reg add "HKCU\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Console\TrueTypeFont" /v "0" /t REG_SZ /d "!FONT_NAME!" /f >nul 2>&1 | ||
| reg add "HKCU\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Console\TrueTypeFont" /v "00" /t REG_SZ /d "!FONT_NAME!" /f >nul 2>&1 | ||
| reg add "HKCU\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Console\TrueTypeFont" /v "000" /t REG_SZ /d "!FONT_NAME!" /f >nul 2>&1 | ||
|
|
There was a problem hiding this comment.
3. Silent hklm registry failures 🐞 Bug ☼ Reliability
powershell.bat attempts multiple HKLM registry writes (machine-level font/console registration) but suppresses all output and does not check exit codes, so non-elevated runs can partially fail with no signal. This makes font recognition issues intermittent and hard to diagnose because HKCU settings may work in some contexts while HKLM-required contexts fail.
Agent Prompt
### Issue description
The launcher writes to HKLM (admin-required) but redirects stdout/stderr to `nul` and does not check `%errorlevel%`. When run without elevation, these writes commonly fail silently.
### Issue Context
The script also writes HKCU values, which can mask partial failures: things may work in some shells but not others.
### Fix Focus Areas
- bin/powershell7.6.4/powershell.bat[127-145]
### Suggested fix
- After each HKLM `reg add`, check `if errorlevel 1` and `echo`/log a clear warning like "HKLM registration skipped (not elevated)".
- Optionally, gate HKLM writes behind an elevation check (e.g., `net session >nul 2>&1`) and only attempt HKLM writes when elevated.
- Avoid suppressing all output for failure paths; keep `>nul` only for success cases.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+46
to
+51
| ## Environment Variables | ||
|
|
||
| The profile sets these environment variables: | ||
| - `POSH_ROOT` - Path to Oh My Posh installation | ||
| - `POSH_THEMES_PATH` - Path to Oh My Posh themes directory | ||
|
|
There was a problem hiding this comment.
4. Posh_themes_path not set 🐞 Bug ⚙ Maintainability
config/README.md states the profile sets POSH_THEMES_PATH, but Microsoft.PowerShell_profile.ps1 never sets it, so users/scripts following the documentation will rely on an environment variable that doesn’t exist. This is a documentation/config mismatch introduced with the new 7.6.4 config docs.
Agent Prompt
### Issue description
The README documents `POSH_THEMES_PATH`, but the profile only sets `POSH_ROOT` and uses a hardcoded theme path.
### Issue Context
Users may set automation or troubleshooting steps based on the documented env vars.
### Fix Focus Areas
- bin/powershell7.6.4/config/README.md[46-51]
- bin/powershell7.6.4/config/Microsoft.PowerShell_profile.ps1[8-21]
### Suggested fix
Either:
- Add `$env:POSH_THEMES_PATH = Join-Path $env:POSH_ROOT 'themes'` to the profile, or
- Remove/correct the README entry to reflect actual behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.