Fix mis-assigned FMA bundle identifiers, switch to fuzzy matching on queries where Windows apps include version number in the name (incl. special fixes for Firefox ESR)#42628
Conversation
- Abstract: change unique_identifier from com.SweetScape.010Editor (010 Editor's bundle ID) to com.elasticprojects.abstract-desktop. Update exists query and install script refs in output manifest. - Teleport Suite: change unique_identifier from com.gravitational.teleport (pkg receipt ID) to com.gravitational.teleport.tsh (tsh.app bundle ID). Update exists query in output manifest. - Amazon Chime (outputs/apps.json): fix unique_identifier from com.runningwithcrayons.Alfred (Alfred's bundle ID) to com.amazon.Amazon-Chime. - Beyond Compare (outputs/apps.json): fix casing from com.scootersoftware.BeyondCompare to com.ScooterSoftware.BeyondCompare.
…matching Several winget apps had version numbers baked into unique_identifier, causing exists queries to do exact matches against stale version strings. Strip version numbers and add fuzzy_match_name: true so the generated query uses LIKE 'Name %' instead of an exact match. Replace the bool FuzzyMatchName field with a custom fuzzyMatch type that accepts a JSON boolean (true/false) or a string containing a custom LIKE pattern. This is backwards-compatible: true and false behave exactly as before, and a string value is used as a verbatim LIKE pattern. Affected apps: 010 Editor, 7-Zip, Airtame, GIMP, Notion, Postman, PuTTY.
Firefox ESR registers in Windows as 'Mozilla Firefox X.Y.Z ESR (x64 en-US)'.
A simple fuzzy prefix match ('Mozilla Firefox %') would also match regular
Firefox ('Mozilla Firefox (x64 en-US)').
Use the new custom fuzzy_match_name string support to specify the pattern
'Mozilla Firefox % ESR %', which uniquely matches ESR builds without
colliding with regular Firefox.
Member
Author
|
@claude review |
Member
Author
|
@coderabbitai full-review |
Contributor
|
✅ Actions performedFull review triggered. |
Contributor
WalkthroughThis pull request introduces configurable fuzzy matching to the winget app ingester and updates application detection queries across Windows and macOS platforms. The core changes include: refactoring the ingester's 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ee/maintained-apps/ingesters/winget/ingester.go`:
- Around line 351-367: The existsQuery construction currently injects raw values
(input.FuzzyMatchName.Custom, name, publisher) into SQL via fmt.Sprintf
(existsQuery), which allows quotes/injection; change to use parameterized
queries instead of interpolation: build the SQL with placeholders (e.g., "SELECT
1 FROM programs WHERE name LIKE ? AND publisher = ?" or the driver-specific
$1/$2 style) for the three branches (the cases for input.FuzzyMatchName.Custom,
input.FuzzyMatchName.Enabled, and default) and pass the corresponding arguments
when executing the query (the code that uses existsQuery should call
db.QueryRow/Query with the args), or if you must keep string composition,
properly escape single quotes in the values before inserting; update the code
paths that execute existsQuery accordingly so no raw name/publisher/custom
strings are interpolated.
In `@ee/maintained-apps/outputs/teleport-suite/darwin.json`:
- Line 6: The detection SQL in darwin.json now checks bundle_identifier =
'com.gravitational.teleport.tsh' but the embedded installer script still targets
'com.gravitational.teleport' for quit/relaunch; update the installer calls to
use the corrected bundle id so lifecycle handling matches detection. Locate the
"exists" SQL entry referencing com.gravitational.teleport.tsh and the installer
script code that calls quit/relaunch on com.gravitational.teleport, and change
those script references to com.gravitational.teleport.tsh (or alternatively make
the SQL match the original bundle id if the script must remain unchanged) so
both the detection (exists) and lifecycle actions reference the same bundle
identifier.
🪄 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: 580c6957-743c-47c6-afe5-a35d5b01e85e
📒 Files selected for processing (23)
ee/maintained-apps/ingesters/winget/ingester.goee/maintained-apps/ingesters/winget/ingester_test.goee/maintained-apps/inputs/homebrew/abstract.jsonee/maintained-apps/inputs/homebrew/teleport-suite.jsonee/maintained-apps/inputs/winget/010-editor.jsonee/maintained-apps/inputs/winget/7-zip.jsonee/maintained-apps/inputs/winget/airtame.jsonee/maintained-apps/inputs/winget/firefox@esr.jsonee/maintained-apps/inputs/winget/gimp.jsonee/maintained-apps/inputs/winget/notion.jsonee/maintained-apps/inputs/winget/postman.jsonee/maintained-apps/inputs/winget/putty.jsonee/maintained-apps/outputs/010-editor/windows.jsonee/maintained-apps/outputs/7-zip/windows.jsonee/maintained-apps/outputs/abstract/darwin.jsonee/maintained-apps/outputs/airtame/windows.jsonee/maintained-apps/outputs/apps.jsonee/maintained-apps/outputs/firefox@esr/windows.jsonee/maintained-apps/outputs/gimp/windows.jsonee/maintained-apps/outputs/notion/windows.jsonee/maintained-apps/outputs/postman/windows.jsonee/maintained-apps/outputs/putty/windows.jsonee/maintained-apps/outputs/teleport-suite/darwin.json
Comment on lines
+351
to
+367
| var existsQuery string | ||
| switch { | ||
| case input.FuzzyMatchName.Custom != "": | ||
| existsQuery = fmt.Sprintf( | ||
| "SELECT 1 FROM programs WHERE name LIKE '%s' AND publisher = '%s';", | ||
| input.FuzzyMatchName.Custom, publisher, | ||
| ) | ||
| case input.FuzzyMatchName.Enabled: | ||
| existsQuery = fmt.Sprintf( | ||
| "SELECT 1 FROM programs WHERE name LIKE '%s %%' AND publisher = '%s';", | ||
| name, publisher, | ||
| ) | ||
| default: | ||
| existsQuery = fmt.Sprintf( | ||
| "SELECT 1 FROM programs WHERE name = '%s' AND publisher = '%s';", | ||
| name, publisher, | ||
| ) |
Contributor
There was a problem hiding this comment.
Escape SQL literals before composing existsQuery.
Line 355, Line 360, and Line 365 interpolate raw strings into SQL literals. A single quote in publisher, name, or custom pattern can break the query (and can become injection-prone). Please escape values before formatting.
Suggested fix
// TODO - consider UpgradeCode here?
var existsQuery string
+ sqlEscape := func(v string) string {
+ return strings.ReplaceAll(v, "'", "''")
+ }
+ safeName := sqlEscape(name)
+ safePublisher := sqlEscape(publisher)
switch {
case input.FuzzyMatchName.Custom != "":
existsQuery = fmt.Sprintf(
"SELECT 1 FROM programs WHERE name LIKE '%s' AND publisher = '%s';",
- input.FuzzyMatchName.Custom, publisher,
+ sqlEscape(input.FuzzyMatchName.Custom), safePublisher,
)
case input.FuzzyMatchName.Enabled:
existsQuery = fmt.Sprintf(
"SELECT 1 FROM programs WHERE name LIKE '%s %%' AND publisher = '%s';",
- name, publisher,
+ safeName, safePublisher,
)
default:
existsQuery = fmt.Sprintf(
"SELECT 1 FROM programs WHERE name = '%s' AND publisher = '%s';",
- name, publisher,
+ safeName, safePublisher,
)
}As per coding guidelines, "Review all SQL queries for possible SQL injection."
Also applies to: 370-370
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/maintained-apps/ingesters/winget/ingester.go` around lines 351 - 367, The
existsQuery construction currently injects raw values
(input.FuzzyMatchName.Custom, name, publisher) into SQL via fmt.Sprintf
(existsQuery), which allows quotes/injection; change to use parameterized
queries instead of interpolation: build the SQL with placeholders (e.g., "SELECT
1 FROM programs WHERE name LIKE ? AND publisher = ?" or the driver-specific
$1/$2 style) for the three branches (the cases for input.FuzzyMatchName.Custom,
input.FuzzyMatchName.Enabled, and default) and pass the corresponding arguments
when executing the query (the code that uses existsQuery should call
db.QueryRow/Query with the args), or if you must keep string composition,
properly escape single quotes in the values before inserting; update the code
paths that execute existsQuery accordingly so no raw name/publisher/custom
strings are interpolated.
| "version": "18.7.3", | ||
| "queries": { | ||
| "exists": "SELECT 1 FROM apps WHERE bundle_identifier = 'com.gravitational.teleport';", | ||
| "exists": "SELECT 1 FROM apps WHERE bundle_identifier = 'com.gravitational.teleport.tsh';", |
Contributor
There was a problem hiding this comment.
Bundle ID update is inconsistent with install script app-id calls.
Line 6 now keys detection on com.gravitational.teleport.tsh, but the embedded install script still quits/relaunches com.gravitational.teleport. This leaves app lifecycle handling out of sync with the corrected identifier.
Proposed fix
-quit_and_track_application 'com.gravitational.teleport'
+quit_and_track_application 'com.gravitational.teleport.tsh'
...
-relaunch_application 'com.gravitational.teleport'
+relaunch_application 'com.gravitational.teleport.tsh'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/maintained-apps/outputs/teleport-suite/darwin.json` at line 6, The
detection SQL in darwin.json now checks bundle_identifier =
'com.gravitational.teleport.tsh' but the embedded installer script still targets
'com.gravitational.teleport' for quit/relaunch; update the installer calls to
use the corrected bundle id so lifecycle handling matches detection. Locate the
"exists" SQL entry referencing com.gravitational.teleport.tsh and the installer
script code that calls quit/relaunch on com.gravitational.teleport, and change
those script references to com.gravitational.teleport.tsh (or alternatively make
the SQL match the original bundle id if the script must remain unchanged) so
both the detection (exists) and lifecycle actions reference the same bundle
identifier.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #42628 +/- ##
=======================================
Coverage 66.66% 66.67%
=======================================
Files 2535 2535
Lines 203320 203346 +26
Branches 9194 9194
=======================================
+ Hits 135551 135579 +28
- Misses 55500 55502 +2
+ Partials 12269 12265 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Zed + Opus 4.6; prompts:
Audit our existing Fleet Maintained App catalog. Look for:
existsqueriesexistsquery, e.g. 7-zip or 010 Editor or Airtame. These should be fuzzy-matched.For each affected app, revise input manifests to fix the issues. For (1), revise apps.json if needed as well. Don't modify apps.json for (2) cases.
Are there any discrepancies between bundle identifiers in input manifests for Darwin apps and apps.json? If so, fix them.
Outputs will get overwritten by the ingester if neither the ingester nor the input JSON files are changed. Make whatever changes need to be made so that these edits survive an FMA ingestion cycle.
Revise
fuzzyto allow specifying a custom value e.g.Mozilal Firefox % (ESR)in addition to the existing true/false, then use that new functionality to build unique queries for Firefox ESR.Commit these changes, across multiple commits (there will be cases where a changes to a single file will be spread across multiple commits, most notably apps.json). Split commits out as follows:
Prefix commit messages with "🤖 ".
The ingester and test changes should've gone in commit 3. Move them there from commit 2.
Summary by CodeRabbit
Bug Fixes
New Features
Tests