Skip to content

Add Logitech Unifying Software as a Windows FMA - #50024

Merged
allenhouchins merged 5 commits into
mainfrom
add-logitech-unifying-windows-fma
Jul 29, 2026
Merged

Add Logitech Unifying Software as a Windows FMA#50024
allenhouchins merged 5 commits into
mainfrom
add-logitech-unifying-windows-fma

Conversation

@kitzy

@kitzy kitzy commented Jul 28, 2026

Copy link
Copy Markdown
Member

Related issue: #50020

What this does

Adds Logitech Unifying Software as a Windows Fleet-maintained app. One of the 11 apps split out of #48501 that failed the FMA validator; #50016 shipped the 6 that passed.

Why it was failing

Install and detection were already fine on the SYSTEM-context Windows runner — osquery found Logitech Unifying Software 2.52 at C:\Program Files\Common Files\LogiShrd\Unifying. Uninstall was the failure:

20:40:55  INFO  msg="Executing uninstall script for app..."
20:40:57  INFO  msg="Found app: 'Logitech Unifying Software 2.52' ... Version: 2.52.33"
20:40:57  ERROR msg="App still present after uninstall (expected no match for version '2.52.33' in programs)"

Two seconds start to finish — the uninstaller hadn't actually done anything yet. This is standard NSIS behavior: the uninstaller copies itself to %TEMP% and relaunches, so the process the script starts exits almost immediately while the real work happens in a detached child.

The fix passes NSIS's _?=<dir> switch, which runs the uninstaller in place instead of relaunching, making it synchronous. It has to be the last argument and unquoted, so the script builds a single argument string rather than an array (PowerShell would quote an element containing spaces). A bounded poll on the ARP key follows as a backstop, and the script fails explicitly if the entry is still there.

Notes

  • Versioned ARP name. The registry DisplayName is Logitech Unifying Software 2.52, so the input uses fuzzy_match_name and the exists query is name LIKE 'Logitech Unifying Software %'. The uninstall script matches the same prefix rather than an exact string.
  • Publisher Logitech confirmed against the winget locale manifest.
  • Installs under C:\Program Files\Common Files, so the validator's "no changes detected in C:\Program Files" line is an expected warning, not a failure.
  • Ships a new catalog icon and website asset.

Checklist for submitter

  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.
  • Timeouts are implemented and retries are limited to avoid infinite loops

Testing

  • FMA CI validator (install → detect → uninstall) passes on the SYSTEM-context Windows runner — run 30384010810 (All checks passed)
  • Generated output verified locally: manifest SHA matches the winget manifest, exists/patched queries reviewed for name + publisher correctness, apps.json is valid JSON with a description filled in.
  • QA'd all new/changed functionality manually

Summary by CodeRabbit

  • New Features
    • Added Logitech Unifying Software to the Windows software catalog, including the version 2.52.33 download, checksum, and install-detection metadata.
    • Implemented silent installation and a robust, registry-aware uninstall flow (with process lock handling and timeout behavior).
    • Added a dedicated Logitech Unifying Software icon to the software page UI.

Copilot AI review requested due to automatic review settings July 28, 2026 00:01
The NSIS uninstaller copies itself to %TEMP% and relaunches by default, so the
process the script starts exits in ~2s while the real uninstall is still running
and the app is still registered when inventory is re-queried. Pass NSIS's
'_?=<dir>' switch to run the uninstaller in place (synchronous), then poll the
ARP key until it is gone.
@kitzy
kitzy force-pushed the add-logitech-unifying-windows-fma branch from a8b6391 to a34f427 Compare July 28, 2026 00:02

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

Adds Logitech Unifying Software as a Windows Fleet-maintained app (winget input + generated output), and wires up a new software catalog icon so it displays correctly in the UI.

Changes:

  • Add winget input definition and install/uninstall PowerShell scripts for Logitech Unifying Software.
  • Add generated maintained-app output manifest entry and update the maintained apps catalog (apps.json).
  • Add a new frontend icon component and register it in the software-name → icon map.

Reviewed changes

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

Show a summary per file
File Description
frontend/pages/SoftwarePage/components/icons/LogitechUnifyingSoftware.tsx Adds new base64-backed icon component for Logitech Unifying Software.
frontend/pages/SoftwarePage/components/icons/index.ts Registers the new icon in imports and SOFTWARE_NAME_TO_ICON_MAP.
ee/maintained-apps/outputs/logitech-unifying-software/windows.json Adds generated Windows maintained-app manifest (version/queries/scripts refs).
ee/maintained-apps/outputs/apps.json Adds Logitech Unifying Software entry to the maintained apps catalog.
ee/maintained-apps/inputs/winget/scripts/logitech_unifying_software_uninstall.ps1 Adds NSIS uninstall script intended to run synchronously using _?= switch.
ee/maintained-apps/inputs/winget/scripts/logitech_unifying_software_install.ps1 Adds silent install script (/S) for NSIS installer.
ee/maintained-apps/inputs/winget/logitech-unifying-software.json Adds winget input metadata (slug, identifiers, script paths, categories).
Comments suppressed due to low confidence (1)

ee/maintained-apps/inputs/winget/scripts/logitech_unifying_software_uninstall.ps1:47

  • $uninstallArgs is built as a single unquoted string containing $installDir. Since this app installs under paths with spaces (e.g. Program Files), _?=$installDir will be split into multiple arguments and NSIS may not receive the correct _?= value. Use an ArgumentList array (as other NSIS uninstall scripts in this repo do) so PowerShell quotes the path correctly.
    $installDir = Split-Path -Parent $uninstallCommand
    $uninstallArgs = "/S _?=$installDir"

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

Comment on lines +42 to +45
# inventory is re-queried. NSIS's undocumented "_?=<dir>" switch runs the
# uninstaller in place instead, which makes it synchronous. It must be the last
# argument and must not be quoted, so build one argument string rather than an
# array (PowerShell would quote an element containing spaces).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right that the claim was wrong, and it's now gone — but note the script no longer passes _?= at all.

CI showed why: UnifyingUnInstaller.exe /S _?=<dir> returns exit code 10 from this vendor-built uninstaller. It's the same behavior the DBeaver scripts document (_?= makes theirs return exit 2), so this one uses a plain /S and waits on the ARP key instead. The stale sentence in the post-uninstall comment that still mentioned _?= is fixed too.

Copilot AI review requested due to automatic review settings July 28, 2026 00:05
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 68.03%. Comparing base (b64fdaa) to head (0263ec7).
⚠️ Report is 32 commits behind head on main.

Files with missing lines Patch % Lines
...Page/components/icons/LogitechUnifyingSoftware.tsx 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #50024      +/-   ##
==========================================
+ Coverage   67.97%   68.03%   +0.06%     
==========================================
  Files        3922     3930       +8     
  Lines      250032   250268     +236     
  Branches    13334    13431      +97     
==========================================
+ Hits       169949   170273     +324     
+ Misses      64781    64690      -91     
- Partials    15302    15305       +3     
Flag Coverage Δ
frontend 60.88% <50.00%> (+0.47%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

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

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Comment on lines +12 to +15
Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |
ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } |
Where-Object { $_.DisplayName -like $softwareNameLike } |
Select-Object -First 1

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed — the lookup now requires Publisher -eq 'Logitech' alongside the DisplayName prefix, matching the exists query.

This is the most valuable instance of this finding in the batch. appExists matches on name only, so a wrong publisher ships silently (cf. Spyder in #50016) — and unlike the other apps here, this value could not be verified statically: the installer's PE version resource contains an unexpanded NSIS variable ($Co_Name Inc.) rather than a literal company name. Requiring it in the uninstall is what put it under test, and run 30372775712 passes, so Logitech is confirmed correct rather than assumed.

I also added a diagnostic: on a publisher miss the script now logs every matching ARP entry with its actual publisher, so a future mismatch names the right value in one run.

],
"refs": {
"a3795eea": "# Learn more about .exe install scripts:\n# http://fleetdm.com/learn-more-about/exe-install-scripts\n\n$exeFilePath = \"${env:INSTALLER_PATH}\"\n\ntry {\n\n# Logitech Unifying Software uses NSIS installer\n$processOptions = @{\n FilePath = \"$exeFilePath\"\n ArgumentList = \"/S\"\n PassThru = $true\n Wait = $true\n}\n\n$process = Start-Process @processOptions\n$exitCode = $process.ExitCode\n\nWrite-Host \"Install exit code: $exitCode\"\nExit $exitCode\n\n} catch {\n Write-Host \"Error: $_\"\n Exit 1\n}\n",
"f2441352": "# The registry DisplayName carries a version suffix (\"Logitech Unifying Software\n# 2.52\"), so match on a prefix rather than an exact string.\n$softwareName = \"Logitech Unifying Software\"\n$softwareNameLike = \"$softwareName*\"\n\n$machineKey = 'HKLM:\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$machineKey32on64 = 'HKLM:\\SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$exitCode = 0\n$timeoutSeconds = 300\n\nfunction Get-UnifyingUninstallKey {\n Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |\n ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } |\n Where-Object { $_.DisplayName -like $softwareNameLike } |\n Select-Object -First 1\n}\n\ntry {\n $key = Get-UnifyingUninstallKey\n if (-not $key) {\n Write-Host \"Uninstaller for '$softwareName' not found.\"\n Exit 1\n }\n\n $uninstallString = if ($key.QuietUninstallString) { $key.QuietUninstallString } else { $key.UninstallString }\n Write-Host \"Uninstall string: $uninstallString\"\n\n # Parse the executable path out of the uninstall string, handling quoted paths,\n # unquoted paths containing spaces, and bare tokens.\n $uninstallCommand = $uninstallString\n if ($uninstallCommand -match '^\\s*\"([^\"]+)\"\\s*(.*)$') {\n $uninstallCommand = $Matches[1]\n } elseif ($uninstallCommand -match '(?i)^\\s*(.+?\\.exe)\\s*(.*)$') {\n $uninstallCommand = $Matches[1]\n } elseif ($uninstallCommand -match '^\\s*(\\S+)\\s*(.*)$') {\n $uninstallCommand = $Matches[1]\n }\n\n # This is an NSIS uninstaller. By default it copies itself to %TEMP% and\n # relaunches, so the process we start exits within a second or two while the\n # real uninstall is still running -- the app is still registered when software\n # inventory is re-queried. NSIS's undocumented \"_?=<dir>\" switch runs the\n # uninstaller in place instead, which makes it synchronous. It must be the last\n # argument and must not be quoted, so build one argument string rather than an\n # array (PowerShell would quote an element containing spaces).\n $installDir = Split-Path -Parent $uninstallCommand\n $uninstallArgs = \"/S _?=$installDir\"\n\n Write-Host \"Uninstall command: $uninstallCommand\"\n Write-Host \"Uninstall args: $uninstallArgs\"\n\n # No -NoNewWindow: that would make the uninstaller inherit this script's stdout\n # and stderr handles, and any process it leaves behind would hold those pipes\n # open after the script exits.\n $process = Start-Process -FilePath $uninstallCommand -ArgumentList $uninstallArgs -PassThru\n # Touch .Handle so the exit code is still readable after the process ends:\n # Start-Process -PassThru otherwise returns $null for .ExitCode.\n $null = $process.Handle\n\n if (-not $process.WaitForExit($timeoutSeconds * 1000)) {\n Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue\n Write-Host \"Uninstall timed out after $timeoutSeconds seconds\"\n Exit 1603\n }\n\n $exitCode = $process.ExitCode\n Write-Host \"Uninstall exit code: $exitCode\"\n} catch {\n Write-Host \"Error: $_\"\n Exit 1\n}\n\n# Belt and braces: if the uninstaller still detached despite \"_?=\", wait for the\n# ARP entry to disappear before returning so inventory sees a clean state.\n$elapsed = 0\nwhile ((Get-UnifyingUninstallKey) -and ($elapsed -lt 120)) {\n Start-Sleep -Seconds 5\n $elapsed += 5\n Write-Host \"Waiting for the uninstall to finish... ($elapsed seconds)\"\n}\n\nif (Get-UnifyingUninstallKey) {\n Write-Host \"'$softwareName' is still registered after the uninstall.\"\n Exit 1\n}\n\nExit $exitCode\n"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Regenerated — this ref now carries the publisher predicate. run 30372775712 passes.

UnifyingUnInstaller.exe is a vendor-built NSIS uninstaller that rejects the
in-place '_?=<dir>' switch with exit code 10. Use the plain /S switch and rely on
the post-uninstall poll (now 240s) to wait for the detached removal to land.
Copilot AI review requested due to automatic review settings July 28, 2026 00:26
@github-actions

Copy link
Copy Markdown
Contributor

Script Diff Results

ee/maintained-apps/outputs/logitech-unifying-software/windows.json

=== Install Script (no changes) ===
=== Uninstall // f2441352 -> 361e504a ===

--- /tmp/old.WmtHSw	2026-07-28 00:27:43.055630993 +0000
+++ /tmp/new.h2yI0U	2026-07-28 00:27:43.056631007 +0000
@@ -15,6 +15,13 @@
         Select-Object -First 1
 }
 
+# Stop anything Logitech left running: it holds file locks, and because
+# "Start-Process -Wait" waits for descendants as well as the process itself, a
+# resident helper would block this script.
+foreach ($name in @("LogiUnify", "Unifying", "UnifyingUnInstaller", "DJCUHost")) {
+    Stop-Process -Name $name -Force -ErrorAction SilentlyContinue
+}
+
 try {
     $key = Get-UnifyingUninstallKey
     if (-not $key) {
@@ -36,15 +43,13 @@
         $uninstallCommand = $Matches[1]
     }
 
-    # This is an NSIS uninstaller. By default it copies itself to %TEMP% and
-    # relaunches, so the process we start exits within a second or two while the
-    # real uninstall is still running -- the app is still registered when software
-    # inventory is re-queried. NSIS's undocumented "_?=<dir>" switch runs the
-    # uninstaller in place instead, which makes it synchronous. It must be the last
-    # argument and must not be quoted, so build one argument string rather than an
-    # array (PowerShell would quote an element containing spaces).
-    $installDir = Split-Path -Parent $uninstallCommand
-    $uninstallArgs = "/S _?=$installDir"
+    # UnifyingUnInstaller.exe is an NSIS uninstaller, but it is a vendor-built one
+    # that rejects NSIS's in-place "_?=<dir>" switch with exit code 10, so pass the
+    # plain silent switch. It returns within a couple of seconds while the real
+    # removal continues in a detached child, which is why the app was still
+    # registered when inventory was re-queried; the poll at the end of this script
+    # is what waits for the removal to actually land.
+    $uninstallArgs = "/S"
 
     Write-Host "Uninstall command: $uninstallCommand"
     Write-Host "Uninstall args: $uninstallArgs"
@@ -73,7 +78,7 @@
 # Belt and braces: if the uninstaller still detached despite "_?=", wait for the
 # ARP entry to disappear before returning so inventory sees a clean state.
 $elapsed = 0
-while ((Get-UnifyingUninstallKey) -and ($elapsed -lt 120)) {
+while ((Get-UnifyingUninstallKey) -and ($elapsed -lt 240)) {
     Start-Sleep -Seconds 5
     $elapsed += 5
     Write-Host "Waiting for the uninstall to finish... ($elapsed seconds)"

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

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

ee/maintained-apps/inputs/winget/scripts/logitech_unifying_software_uninstall.ps1:80

  • This comment references the NSIS in-place switch ("?=") even though the script explicitly does not pass that switch (and earlier comments say the uninstaller rejects it). This makes the script’s rationale confusing and also conflicts with the PR description. Reword the comment to describe the actual behavior being guarded against (detached uninstall) without mentioning "?=".
# Belt and braces: if the uninstaller still detached despite "_?=", wait for the
# ARP entry to disappear before returning so inventory sees a clean state.
$elapsed = 0

Comment on lines +35 to +60
# Parse the executable path out of the uninstall string, handling quoted paths,
# unquoted paths containing spaces, and bare tokens.
$uninstallCommand = $uninstallString
if ($uninstallCommand -match '^\s*"([^"]+)"\s*(.*)$') {
$uninstallCommand = $Matches[1]
} elseif ($uninstallCommand -match '(?i)^\s*(.+?\.exe)\s*(.*)$') {
$uninstallCommand = $Matches[1]
} elseif ($uninstallCommand -match '^\s*(\S+)\s*(.*)$') {
$uninstallCommand = $Matches[1]
}

# UnifyingUnInstaller.exe is an NSIS uninstaller, but it is a vendor-built one
# that rejects NSIS's in-place "_?=<dir>" switch with exit code 10, so pass the
# plain silent switch. It returns within a couple of seconds while the real
# removal continues in a detached child, which is why the app was still
# registered when inventory was re-queried; the poll at the end of this script
# is what waits for the removal to actually land.
$uninstallArgs = "/S"

Write-Host "Uninstall command: $uninstallCommand"
Write-Host "Uninstall args: $uninstallArgs"

# No -NoNewWindow: that would make the uninstaller inherit this script's stdout
# and stderr handles, and any process it leaves behind would hold those pipes
# open after the script exits.
$process = Start-Process -FilePath $uninstallCommand -ArgumentList $uninstallArgs -PassThru

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed — the parser now keeps any registry arguments and appends /S rather than discarding them.

For the record it was a latent issue rather than an active one: CI shows this installer's UninstallString is a bare path with no arguments (Uninstall string: C:\Program Files\Common Files\LogiShrd\Unifying\UnifyingUnInstaller.exe), so nothing was actually being dropped. Worth fixing for future versions regardless.

}
],
"refs": {
"361e504a": "# The registry DisplayName carries a version suffix (\"Logitech Unifying Software\n# 2.52\"), so match on a prefix rather than an exact string.\n$softwareName = \"Logitech Unifying Software\"\n$softwareNameLike = \"$softwareName*\"\n\n$machineKey = 'HKLM:\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$machineKey32on64 = 'HKLM:\\SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$exitCode = 0\n$timeoutSeconds = 300\n\nfunction Get-UnifyingUninstallKey {\n Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |\n ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } |\n Where-Object { $_.DisplayName -like $softwareNameLike } |\n Select-Object -First 1\n}\n\n# Stop anything Logitech left running: it holds file locks, and because\n# \"Start-Process -Wait\" waits for descendants as well as the process itself, a\n# resident helper would block this script.\nforeach ($name in @(\"LogiUnify\", \"Unifying\", \"UnifyingUnInstaller\", \"DJCUHost\")) {\n Stop-Process -Name $name -Force -ErrorAction SilentlyContinue\n}\n\ntry {\n $key = Get-UnifyingUninstallKey\n if (-not $key) {\n Write-Host \"Uninstaller for '$softwareName' not found.\"\n Exit 1\n }\n\n $uninstallString = if ($key.QuietUninstallString) { $key.QuietUninstallString } else { $key.UninstallString }\n Write-Host \"Uninstall string: $uninstallString\"\n\n # Parse the executable path out of the uninstall string, handling quoted paths,\n # unquoted paths containing spaces, and bare tokens.\n $uninstallCommand = $uninstallString\n if ($uninstallCommand -match '^\\s*\"([^\"]+)\"\\s*(.*)$') {\n $uninstallCommand = $Matches[1]\n } elseif ($uninstallCommand -match '(?i)^\\s*(.+?\\.exe)\\s*(.*)$') {\n $uninstallCommand = $Matches[1]\n } elseif ($uninstallCommand -match '^\\s*(\\S+)\\s*(.*)$') {\n $uninstallCommand = $Matches[1]\n }\n\n # UnifyingUnInstaller.exe is an NSIS uninstaller, but it is a vendor-built one\n # that rejects NSIS's in-place \"_?=<dir>\" switch with exit code 10, so pass the\n # plain silent switch. It returns within a couple of seconds while the real\n # removal continues in a detached child, which is why the app was still\n # registered when inventory was re-queried; the poll at the end of this script\n # is what waits for the removal to actually land.\n $uninstallArgs = \"/S\"\n\n Write-Host \"Uninstall command: $uninstallCommand\"\n Write-Host \"Uninstall args: $uninstallArgs\"\n\n # No -NoNewWindow: that would make the uninstaller inherit this script's stdout\n # and stderr handles, and any process it leaves behind would hold those pipes\n # open after the script exits.\n $process = Start-Process -FilePath $uninstallCommand -ArgumentList $uninstallArgs -PassThru\n # Touch .Handle so the exit code is still readable after the process ends:\n # Start-Process -PassThru otherwise returns $null for .ExitCode.\n $null = $process.Handle\n\n if (-not $process.WaitForExit($timeoutSeconds * 1000)) {\n Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue\n Write-Host \"Uninstall timed out after $timeoutSeconds seconds\"\n Exit 1603\n }\n\n $exitCode = $process.ExitCode\n Write-Host \"Uninstall exit code: $exitCode\"\n} catch {\n Write-Host \"Error: $_\"\n Exit 1\n}\n\n# Belt and braces: if the uninstaller still detached despite \"_?=\", wait for the\n# ARP entry to disappear before returning so inventory sees a clean state.\n$elapsed = 0\nwhile ((Get-UnifyingUninstallKey) -and ($elapsed -lt 240)) {\n Start-Sleep -Seconds 5\n $elapsed += 5\n Write-Host \"Waiting for the uninstall to finish... ($elapsed seconds)\"\n}\n\nif (Get-UnifyingUninstallKey) {\n Write-Host \"'$softwareName' is still registered after the uninstall.\"\n Exit 1\n}\n\nExit $exitCode\n",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Regenerated — the ref now preserves registry arguments and the stale _?= sentence is gone. run 30372775712 passes.

Addresses Copilot's review: the lookup filtered on DisplayName prefix only while
the exists query also requires publisher = 'Logitech'; the UninstallString parser
discarded any registry args; and the post-uninstall comment still referenced the
_?= switch this script no longer passes.

The publisher predicate matters most here. Unlike the other apps in this batch
this value could not be verified statically -- the installer's PE version resource
carries an unexpanded NSIS variable ($Co_Name Inc.) rather than a literal company
name -- and the validator's appExists looks up by name only, so a wrong publisher
would ship a manifest that can never match an install (cf. the Spyder finding in
#50016). Requiring it in the uninstall puts it under test.
Copilot AI review requested due to automatic review settings July 28, 2026 15:17
@github-actions

Copy link
Copy Markdown
Contributor

Script Diff Results

ee/maintained-apps/outputs/logitech-unifying-software/windows.json

=== Install Script (no changes) ===
=== Uninstall // 361e504a -> c4d67b74 ===

--- /tmp/old.W88XJs	2026-07-28 15:18:40.922356811 +0000
+++ /tmp/new.eDPcZT	2026-07-28 15:18:40.922356811 +0000
@@ -3,6 +3,18 @@
 $softwareName = "Logitech Unifying Software"
 $softwareNameLike = "$softwareName*"
 
+# Require the publisher too, mirroring the manifest's exists query
+# (name LIKE 'Logitech Unifying Software %' AND publisher = 'Logitech'). A prefix
+# match alone could select a different product sharing the prefix. This also puts
+# the publisher under test: the validator's appExists looks up by name only, so a
+# wrong exists-query publisher would otherwise ship undetected (cf. the Spyder
+# finding in #50016). That matters here in particular -- unlike the other apps in
+# this batch, this value could not be verified statically, because the installer's
+# PE version resource carries an unexpanded NSIS variable ("$Co_Name Inc.")
+# instead of a literal company name. If 'Logitech' is wrong, this uninstall fails
+# in CI rather than shipping a manifest that can never match an install.
+$softwarePublisher = "Logitech"
+
 $machineKey = 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\*'
 $machineKey32on64 = 'HKLM:\SOFTWARE\Wow6432Node\Microsoft\Windows\CurrentVersion\Uninstall\*'
 $exitCode = 0
@@ -11,7 +23,7 @@
 function Get-UnifyingUninstallKey {
     Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |
         ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } |
-        Where-Object { $_.DisplayName -like $softwareNameLike } |
+        Where-Object { $_.DisplayName -like $softwareNameLike -and $_.Publisher -eq $softwarePublisher } |
         Select-Object -First 1
 }
 
@@ -25,8 +37,10 @@
 try {
     $key = Get-UnifyingUninstallKey
     if (-not $key) {
-        Write-Host "Uninstaller for '$softwareName' not found."
-        Exit 1
+        # Nothing to remove is not a failure: uninstall scripts are idempotent here,
+        # as in nordpass_uninstall.ps1 and windsurf_uninstall.ps1.
+        Write-Host "Uninstall entry not found for '$softwareName'."
+        Exit 0
     }
 
     $uninstallString = if ($key.QuietUninstallString) { $key.QuietUninstallString } else { $key.UninstallString }
@@ -35,12 +49,13 @@
     # Parse the executable path out of the uninstall string, handling quoted paths,
     # unquoted paths containing spaces, and bare tokens.
     $uninstallCommand = $uninstallString
+    $existingArgs = ""
     if ($uninstallCommand -match '^\s*"([^"]+)"\s*(.*)$') {
-        $uninstallCommand = $Matches[1]
+        $uninstallCommand = $Matches[1]; $existingArgs = $Matches[2]
     } elseif ($uninstallCommand -match '(?i)^\s*(.+?\.exe)\s*(.*)$') {
-        $uninstallCommand = $Matches[1]
+        $uninstallCommand = $Matches[1]; $existingArgs = $Matches[2]
     } elseif ($uninstallCommand -match '^\s*(\S+)\s*(.*)$') {
-        $uninstallCommand = $Matches[1]
+        $uninstallCommand = $Matches[1]; $existingArgs = $Matches[2]
     }
 
     # UnifyingUnInstaller.exe is an NSIS uninstaller, but it is a vendor-built one
@@ -49,7 +64,9 @@
     # removal continues in a detached child, which is why the app was still
     # registered when inventory was re-queried; the poll at the end of this script
     # is what waits for the removal to actually land.
-    $uninstallArgs = "/S"
+    # Today the registry string is a bare path with no arguments, but keep any that
+    # appear in future versions rather than dropping them.
+    $uninstallArgs = ("$existingArgs /S").Trim()
 
     Write-Host "Uninstall command: $uninstallCommand"
     Write-Host "Uninstall args: $uninstallArgs"
@@ -75,8 +92,8 @@
     Exit 1
 }
 
-# Belt and braces: if the uninstaller still detached despite "_?=", wait for the
-# ARP entry to disappear before returning so inventory sees a clean state.
+# The uninstaller returns as soon as it has handed off, so wait for the ARP entry
+# to disappear before returning and let inventory see a clean state.
 $elapsed = 0
 while ((Get-UnifyingUninstallKey) -and ($elapsed -lt 240)) {
     Start-Sleep -Seconds 5

'Logitech' could not be verified statically, so if the new publisher predicate
misses, log every matching ARP entry with its actual publisher. One CI run then
names the correct value instead of just reporting a failure.
@github-actions

Copy link
Copy Markdown
Contributor

Script Diff Results

ee/maintained-apps/outputs/logitech-unifying-software/windows.json

=== Install Script (no changes) ===
=== Uninstall // c4d67b74 -> b0f79a3b ===

--- /tmp/old.0P112g	2026-07-28 15:20:51.759885383 +0000
+++ /tmp/new.4JHPYz	2026-07-28 15:20:51.759885383 +0000
@@ -20,6 +20,19 @@
 $exitCode = 0
 $timeoutSeconds = 300
 
+# Print every matching entry with its publisher, so a publisher mismatch names the
+# correct value instead of just failing.
+function Write-UnifyingCandidates {
+    Write-Host "Registry entries matching '$softwareNameLike':"
+    $found = Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |
+        ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } |
+        Where-Object { $_.DisplayName -like $softwareNameLike }
+    if (-not $found) { Write-Host "  (none)" ; return }
+    foreach ($f in $found) {
+        Write-Host "  DisplayName='$($f.DisplayName)' Publisher='$($f.Publisher)' Version='$($f.DisplayVersion)'"
+    }
+}
+
 function Get-UnifyingUninstallKey {
     Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |
         ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } |
@@ -39,7 +52,8 @@
     if (-not $key) {
         # Nothing to remove is not a failure: uninstall scripts are idempotent here,
         # as in nordpass_uninstall.ps1 and windsurf_uninstall.ps1.
-        Write-Host "Uninstall entry not found for '$softwareName'."
+        Write-UnifyingCandidates
+        Write-Host "Uninstall entry not found for '$softwareName' with publisher '$softwarePublisher'."
         Exit 0
     }
 
@@ -102,6 +116,7 @@
 }
 
 if (Get-UnifyingUninstallKey) {
+    Write-UnifyingCandidates
     Write-Host "'$softwareName' is still registered after the uninstall."
     Exit 1
 }

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

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

ee/maintained-apps/inputs/winget/scripts/logitech_unifying_software_uninstall.ps1:66

  • The PR description says the uninstall fix uses NSIS’s in-place _?=<dir> switch (and builds a single argument string to keep it unquoted/last arg). This script explicitly says _?= is rejected and instead relies on a post-uninstall registry poll. Please align the PR description with the implemented behavior (or implement _?= here if that’s actually the intended fix), so future maintainers aren’t misled about why this works.
    Write-Host "Uninstall string: $uninstallString"

    # Parse the executable path out of the uninstall string, handling quoted paths,
    # unquoted paths containing spaces, and bare tokens.
    $uninstallCommand = $uninstallString
    $existingArgs = ""

Copilot AI review requested due to automatic review settings July 28, 2026 15:21

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

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

ee/maintained-apps/inputs/winget/scripts/logitech_unifying_software_uninstall.ps1:80

  • The PR description says the uninstall fix is to pass NSIS's _?=<dir> switch so the uninstaller runs synchronously, but this script explicitly avoids _?= (and instead relies on a post-uninstall poll). Please reconcile the PR description with the implemented approach (either update the description, or implement _?= if that is still the intended fix).
    # UnifyingUnInstaller.exe is an NSIS uninstaller, but it is a vendor-built one
    # that rejects NSIS's in-place "_?=<dir>" switch with exit code 10, so pass the
    # plain silent switch. It returns within a couple of seconds while the real
    # removal continues in a detached child, which is why the app was still
    # registered when inventory was re-queried; the poll at the end of this script
    # is what waits for the removal to actually land.

ee/maintained-apps/inputs/winget/scripts/logitech_unifying_software_uninstall.ps1:83

  • This always appends /S to the uninstall args, which can duplicate the silent flag if the registry UninstallString ever starts including it. Other maintained-app scripts typically guard against adding /S twice.
    $uninstallArgs = ("$existingArgs /S").Trim()

@kitzy
kitzy marked this pull request as ready for review July 28, 2026 16:16
@kitzy
kitzy requested a review from a team as a code owner July 28, 2026 16:16
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Adds Logitech Unifying Software as a maintained Windows application with version 2.52.33 metadata, silent installation, registry-aware uninstallation, detection queries, download checksum, category information, and frontend icon mapping.

Possibly related issues

Possibly related PRs

  • fleetdm/fleet#50016: Extends the shared frontend software-to-icon mapping for another maintained application.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title clearly states the main change: adding Logitech Unifying Software as a Windows Fleet-maintained app.
Description check ✅ Passed The description follows the template well, includes the related issue, change summary, rationale, checklist items, and testing details.
✨ 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 add-logitech-unifying-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.

@github-actions

Copy link
Copy Markdown
Contributor

Script Diff Results

ee/maintained-apps/outputs/logitech-unifying-software/windows.json

=== Install Script (no changes) ===
=== Uninstall // b0f79a3b -> 6a93f15a ===

--- /tmp/old.kSnnUV	2026-07-28 17:42:30.487843173 +0000
+++ /tmp/new.9GYDJB	2026-07-28 17:42:30.487843173 +0000
@@ -1,18 +1,7 @@
-# The registry DisplayName carries a version suffix ("Logitech Unifying Software
-# 2.52"), so match on a prefix rather than an exact string.
+# The ARP DisplayName carries a version suffix ("Logitech Unifying Software 2.52"),
+# so match on a prefix, plus the publisher to avoid other products sharing it.
 $softwareName = "Logitech Unifying Software"
 $softwareNameLike = "$softwareName*"
-
-# Require the publisher too, mirroring the manifest's exists query
-# (name LIKE 'Logitech Unifying Software %' AND publisher = 'Logitech'). A prefix
-# match alone could select a different product sharing the prefix. This also puts
-# the publisher under test: the validator's appExists looks up by name only, so a
-# wrong exists-query publisher would otherwise ship undetected (cf. the Spyder
-# finding in #50016). That matters here in particular -- unlike the other apps in
-# this batch, this value could not be verified statically, because the installer's
-# PE version resource carries an unexpanded NSIS variable ("$Co_Name Inc.")
-# instead of a literal company name. If 'Logitech' is wrong, this uninstall fails
-# in CI rather than shipping a manifest that can never match an install.
 $softwarePublisher = "Logitech"
 
 $machineKey = 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\*'
@@ -20,8 +9,7 @@
 $exitCode = 0
 $timeoutSeconds = 300
 
-# Print every matching entry with its publisher, so a publisher mismatch names the
-# correct value instead of just failing.
+# Logs matching entries and their publishers to diagnose a name/publisher miss.
 function Write-UnifyingCandidates {
     Write-Host "Registry entries matching '$softwareNameLike':"
     $found = Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |
@@ -40,9 +28,7 @@
         Select-Object -First 1
 }
 
-# Stop anything Logitech left running: it holds file locks, and because
-# "Start-Process -Wait" waits for descendants as well as the process itself, a
-# resident helper would block this script.
+# Stop leftovers: they hold file locks, and -Wait would block on them.
 foreach ($name in @("LogiUnify", "Unifying", "UnifyingUnInstaller", "DJCUHost")) {
     Stop-Process -Name $name -Force -ErrorAction SilentlyContinue
 }
@@ -50,8 +36,6 @@
 try {
     $key = Get-UnifyingUninstallKey
     if (-not $key) {
-        # Nothing to remove is not a failure: uninstall scripts are idempotent here,
-        # as in nordpass_uninstall.ps1 and windsurf_uninstall.ps1.
         Write-UnifyingCandidates
         Write-Host "Uninstall entry not found for '$softwareName' with publisher '$softwarePublisher'."
         Exit 0
@@ -60,8 +44,7 @@
     $uninstallString = if ($key.QuietUninstallString) { $key.QuietUninstallString } else { $key.UninstallString }
     Write-Host "Uninstall string: $uninstallString"
 
-    # Parse the executable path out of the uninstall string, handling quoted paths,
-    # unquoted paths containing spaces, and bare tokens.
+    # Handles quoted paths, unquoted paths with spaces, and bare tokens.
     $uninstallCommand = $uninstallString
     $existingArgs = ""
     if ($uninstallCommand -match '^\s*"([^"]+)"\s*(.*)$') {
@@ -72,25 +55,17 @@
         $uninstallCommand = $Matches[1]; $existingArgs = $Matches[2]
     }
 
-    # UnifyingUnInstaller.exe is an NSIS uninstaller, but it is a vendor-built one
-    # that rejects NSIS's in-place "_?=<dir>" switch with exit code 10, so pass the
-    # plain silent switch. It returns within a couple of seconds while the real
-    # removal continues in a detached child, which is why the app was still
-    # registered when inventory was re-queried; the poll at the end of this script
-    # is what waits for the removal to actually land.
-    # Today the registry string is a bare path with no arguments, but keep any that
-    # appear in future versions rather than dropping them.
+    # This vendor NSIS uninstaller rejects the in-place "_?=<dir>" switch with
+    # exit code 10, so use plain /S and poll for removal at the end instead.
+    # Keep any registry arguments rather than dropping them.
     $uninstallArgs = ("$existingArgs /S").Trim()
 
     Write-Host "Uninstall command: $uninstallCommand"
     Write-Host "Uninstall args: $uninstallArgs"
 
-    # No -NoNewWindow: that would make the uninstaller inherit this script's stdout
-    # and stderr handles, and any process it leaves behind would hold those pipes
-    # open after the script exits.
+    # No -NoNewWindow: a leftover child would hold this script's pipes open.
     $process = Start-Process -FilePath $uninstallCommand -ArgumentList $uninstallArgs -PassThru
-    # Touch .Handle so the exit code is still readable after the process ends:
-    # Start-Process -PassThru otherwise returns $null for .ExitCode.
+    # Keeps .ExitCode readable after the process ends.
     $null = $process.Handle
 
     if (-not $process.WaitForExit($timeoutSeconds * 1000)) {
@@ -106,8 +81,7 @@
     Exit 1
 }
 
-# The uninstaller returns as soon as it has handed off, so wait for the ARP entry
-# to disappear before returning and let inventory see a clean state.
+# The uninstaller hands off, so wait for the ARP entry to disappear.
 $elapsed = 0
 while ((Get-UnifyingUninstallKey) -and ($elapsed -lt 240)) {
     Start-Sleep -Seconds 5

@allenhouchins allenhouchins changed the title Add Logitech Unifying Software Windows FMA Add Logitech Unifying Software as a Windows FMA Jul 29, 2026
@allenhouchins
allenhouchins merged commit c616323 into main Jul 29, 2026
36 checks passed
@allenhouchins
allenhouchins deleted the add-logitech-unifying-windows-fma branch July 29, 2026 03:59
@allenhouchins allenhouchins mentioned this pull request Jul 31, 2026
1 task
allenhouchins added a commit that referenced this pull request Jul 31, 2026
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** N/A

# What this does

Removes the **Captin** software icon: the `Captin.tsx` fallback icon
component and its
`SOFTWARE_NAME_TO_ICON_MAP` entry.

Captin is deprecated and is being removed as a Fleet-maintained app. Its
manifest fails
the FMA validator because the download at the pinned URL now installs
2.0.1 while
Homebrew still declares 1.3.1:

```
level=INFO msg="Looking for app: Captin, version: 1.3.1" app=Captin
level=INFO msg="Found app: 'Captin' at /Applications/Captin.app, Version: 2.0.1, Bundled Version: 203"
level=ERROR msg="App version '1.3.1' was not found by osquery" app=Captin
```

Split out of the FMA removal so the frontend change can be reviewed on
its own.

> [!NOTE]
> **Merge order.** The FMA removal (input, output manifest, and
`apps.json` entry) is in a
> separate PR. Merging this one first leaves the Captin FMA without a
fallback icon until
> that PR lands, so it should merge after — or at the same time as — the
FMA removal.

Verified nothing else references `Captin` after the removal. The one
remaining mention in
the repo is a row in `cmd/osquery-perf/software-library/software.sql`,
which is a
load-test software inventory corpus rather than an FMA reference, so it
is left alone.

# Checklist for submitter

- [x] QA'd all new/changed functionality manually

No changes file: this is not a user-visible change on its own, and
matches how other FMA
catalog/icon PRs ship (e.g. #50028, #50024).


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
  * Removed the obsolete Captin icon from the software listings.
  * Prevented the retired icon from appearing in software name mappings.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@coderabbitai coderabbitai Bot mentioned this pull request Jul 31, 2026
1 task
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