Skip to content

Add PhpStorm as a Windows FMA - #46217

Merged
allenhouchins merged 2 commits into
mainfrom
allenhouchins-add-phpstorm-windows-fma
May 26, 2026
Merged

Add PhpStorm as a Windows FMA#46217
allenhouchins merged 2 commits into
mainfrom
allenhouchins-add-phpstorm-windows-fma

Conversation

@allenhouchins

@allenhouchins allenhouchins commented May 26, 2026

Copy link
Copy Markdown
Member

This pull request adds Windows support for managing PhpStorm as a maintained app, including installation and uninstallation automation, and improves the ingestion logic to handle publisher information for better normalization. The most important changes are:

Windows support for PhpStorm:

  • Added a new maintained app definition for PhpStorm on Windows, including metadata and references to install/uninstall scripts (phpstorm.json).
  • Implemented a PowerShell install script for PhpStorm that runs the NSIS installer silently (phpstorm_install.ps1).
  • Implemented a PowerShell uninstall script that finds the correct PhpStorm uninstaller via registry, ensures it's the JetBrains version, and runs it silently (phpstorm_uninstall.ps1).
  • Added PhpStorm for Windows to the apps.json output and created a versioned output file with install/uninstall logic and metadata (apps.json, phpstorm/windows.json). [1] [2]

Improvements to ingestion logic:

  • Updated the app existence check in windows.go to select and propagate the publisher field, and set the Vendor on ingested software, ensuring publisher-based normalization (important for JetBrains build-number handling). [1] [2] [3]

Summary by CodeRabbit

  • New Features
    • Added support for PhpStorm on Windows, including automated installation and removal capabilities.
    • Enhanced Windows application detection to retrieve publisher information for improved vendor identification.

Review Change Stack

Add Winget input for PhpStorm and corresponding installer/uninstaller scripts. Adds ee/maintained-apps/inputs/winget/phpstorm.json and PowerShell scripts to run the NSIS installer (/S) and to locate/run the registry-listed uninstaller silently. Update outputs/apps.json to include the windows entry and add outputs/phpstorm/windows.json containing version 2026.1.2 metadata, installer URL, sha256, and embedded script refs.
fleet-release
fleet-release previously approved these changes May 26, 2026
@allenhouchins
allenhouchins marked this pull request as ready for review May 26, 2026 19:36
@allenhouchins
allenhouchins requested a review from a team as a code owner May 26, 2026 19:36
Copilot AI review requested due to automatic review settings May 26, 2026 19:36

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Fleet-maintained app support for managing PhpStorm on Windows (including silent install/uninstall scripts and generated manifest outputs) and updates the Windows validator ingestion path to propagate publisher into fleet.Software.Vendor so version/name normalization matches production behavior.

Changes:

  • Add a new Winget input + PowerShell install/uninstall scripts for PhpStorm on Windows.
  • Generate and commit PhpStorm Windows maintained-app output manifest and register it in the maintained apps catalog (outputs/apps.json).
  • Update cmd/maintained-apps/validate/windows.go to select publisher from programs and populate Software.Vendor before calling MutateSoftwareOnIngestion.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ee/maintained-apps/outputs/phpstorm/windows.json Adds the generated Windows maintained-app manifest for PhpStorm (queries, installer URL, and script refs).
ee/maintained-apps/outputs/apps.json Registers the new phpstorm/windows entry in the maintained apps catalog.
ee/maintained-apps/inputs/winget/scripts/phpstorm_uninstall.ps1 Adds a registry-based NSIS uninstaller runner for PhpStorm.
ee/maintained-apps/inputs/winget/scripts/phpstorm_install.ps1 Adds a silent install script for the PhpStorm NSIS installer.
ee/maintained-apps/inputs/winget/phpstorm.json Adds the Winget input definition for generating the Windows PhpStorm maintained-app output.
cmd/maintained-apps/validate/windows.go Improves validator ingestion by selecting publisher and using it to populate Software.Vendor for normalization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +32 to +33
Write-Host "Uninstall entry not found for $softwareNameLike"
Exit 1
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR enhances Windows application validation to capture publisher information from osquery results and uses it in the mutation pipeline, then adds complete PhpStorm support on Windows. The validation enhancement retrieves the publisher field and passes it as the Vendor to the fleet.Software mutation function. The PhpStorm Windows support includes a Winget input manifest, PowerShell installation script, a registry-based uninstallation script that handles process cleanup and exit codes, deployment configuration with version metadata and embedded scripts, and app catalog registration.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description is detailed and covers the main changes, but the provided template requires completion of a structured checklist with various sections that are not addressed in the author's description. Complete the checklist sections from the template, including changes files, security validation, testing, and manual QA confirmation to ensure all requirements are met.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add PhpStorm as a Windows FMA' directly and concisely summarizes the main change—adding Windows support for PhpStorm as a maintained app.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch allenhouchins-add-phpstorm-windows-fma

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ee/maintained-apps/inputs/winget/scripts/phpstorm_uninstall.ps1`:
- Around line 43-53: The unquoted regex branch truncates executable paths at the
first space; update the pattern used when matching $uninstallCommand so $exePath
captures the full path including spaces (e.g. match the entire path up to the
executable extension). Replace the unquoted branch regex /^\s*(\S+)\s*(.*)$/
with a pattern that captures everything up to the executable token such as
/^\s*(.+?\.exe)\s*(.*)$/i so $exePath gets the full executable path and
$existingArgs gets the remaining args; keep the quoted branch as-is and reuse
the same $exePath/$existingArgs variables ($uninstallCommand, $exePath,
$existingArgs).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c44f92a2-032f-412c-aaab-68d63ca4972c

📥 Commits

Reviewing files that changed from the base of the PR and between 2158b65 and 58a640f.

📒 Files selected for processing (6)
  • cmd/maintained-apps/validate/windows.go
  • ee/maintained-apps/inputs/winget/phpstorm.json
  • ee/maintained-apps/inputs/winget/scripts/phpstorm_install.ps1
  • ee/maintained-apps/inputs/winget/scripts/phpstorm_uninstall.ps1
  • ee/maintained-apps/outputs/apps.json
  • ee/maintained-apps/outputs/phpstorm/windows.json

Comment on lines +43 to +53
# Split the uninstall string into exe + args. Handle both quoted and unquoted
# exe paths.
$exePath = ""
$existingArgs = ""
if ($uninstallCommand -match '^\s*"([^"]+)"\s*(.*)$') {
$exePath = $matches[1]
$existingArgs = $matches[2].Trim()
} elseif ($uninstallCommand -match '^\s*(\S+)\s*(.*)$') {
$exePath = $matches[1]
$existingArgs = $matches[2].Trim()
} else {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In PowerShell regex matching, what does ^\s*(\S+)\s*(.*)$capture for:C:\Program Files\JetBrains\PhpStorm\bin\Uninstall.exe /S?

💡 Result:

When the regular expression ^\s*(\S+)\s*(.*)$ is applied to the input string C:\Program Files\JetBrains\PhpStorm\bin\Uninstall.exe /S, it captures two specific groups [1][2]. PowerShell regex matching stores the full match in index 0 and the captured groups in subsequent numeric indices [1][3][2]. The captures are as follows: - $Matches: The entire input string, C:\Program Files\JetBrains\PhpStorm\bin\Uninstall.exe /S [1][3][2]. - $Matches[1]: The first capturing group, (\S+). This matches the first non-whitespace sequence. In this case, it captures C:\Program, stopping at the first space [1][2]. - $Matches[3]: The second capturing group, (.*). This matches the remainder of the string after the following whitespace. It captures Files\JetBrains\PhpStorm\bin\Uninstall.exe /S [1][2]. Explanation of the pattern: - ^\s*: Matches optional leading whitespace at the beginning of the string [1][4]. - (\S+): The first capture group; \S+ matches one or more non-whitespace characters. Because the file path contains spaces, the regex treats the first block of non-whitespace text as the entire first group [2]. - \s*: Matches the whitespace character immediately following the first block [1][2]. - (.*)$: The second capture group; .* greedily captures all remaining characters until the end of the line ($) [1][2]. If your intent was to separate the executable path from its arguments, this regex will not produce the desired result because the path contains spaces [2]. To correctly parse a path with spaces, you would typically need to account for quotes or use specialized parsing logic [5].

Citations:


Fix unquoted uninstall-command parsing for executable paths with spaces

The unquoted branch’s regex ^\s*(\S+)\s*(.*)$ truncates paths at the first space (e.g., C:\Program Files\...\Uninstall.exe /S yields $exePath = 'C:\Program'), which can break uninstall execution.

💡 Proposed fix
-} elseif ($uninstallCommand -match '^\s*(\S+)\s*(.*)$') {
+} elseif ($uninstallCommand -match '^\s*(.+?\.exe)\s*(.*)$') {
     $exePath = $matches[1]
     $existingArgs = $matches[2].Trim()
 } else {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ee/maintained-apps/inputs/winget/scripts/phpstorm_uninstall.ps1` around lines
43 - 53, The unquoted regex branch truncates executable paths at the first
space; update the pattern used when matching $uninstallCommand so $exePath
captures the full path including spaces (e.g. match the entire path up to the
executable extension). Replace the unquoted branch regex /^\s*(\S+)\s*(.*)$/
with a pattern that captures everything up to the executable token such as
/^\s*(.+?\.exe)\s*(.*)$/i so $exePath gets the full executable path and
$existingArgs gets the remaining args; keep the quoted branch as-is and reuse
the same $exePath/$existingArgs variables ($uninstallCommand, $exePath,
$existingArgs).

@codecov

codecov Bot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.84%. Comparing base (8bf9290) to head (58a640f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #46217      +/-   ##
==========================================
+ Coverage   66.83%   66.84%   +0.01%     
==========================================
  Files        2756     2757       +1     
  Lines      220200   220726     +526     
  Branches    10879    10879              
==========================================
+ Hits       147171   147545     +374     
- Misses      59736    59840     +104     
- Partials    13293    13341      +48     
Flag Coverage Δ
backend 68.65% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@allenhouchins
allenhouchins merged commit bc28a51 into main May 26, 2026
47 checks passed
@allenhouchins
allenhouchins deleted the allenhouchins-add-phpstorm-windows-fma branch May 26, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants