diff --git a/ee/maintained-apps/ingesters/homebrew/ingester_test.go b/ee/maintained-apps/ingesters/homebrew/ingester_test.go index 0979b4a5c70..f60a6297403 100644 --- a/ee/maintained-apps/ingesters/homebrew/ingester_test.go +++ b/ee/maintained-apps/ingesters/homebrew/ingester_test.go @@ -158,7 +158,7 @@ func TestIngestValidations(t *testing.T) { ) } else { require.Equal(t, - fmt.Sprintf("SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM apps WHERE bundle_identifier = '%s') AND version_compare(bundle_short_version, '%s') < 0);", c.inputApp.UniqueIdentifier, out.Version), + fmt.Sprintf("SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps WHERE bundle_identifier = '%s' AND version_compare(bundle_short_version, '%s') < 0);", c.inputApp.UniqueIdentifier, out.Version), out.Queries.Patched, ) } diff --git a/pkg/patch_policy/patch_policy.go b/pkg/patch_policy/patch_policy.go index f8b4ccb2388..47eb7d9658c 100644 --- a/pkg/patch_policy/patch_policy.go +++ b/pkg/patch_policy/patch_policy.go @@ -19,12 +19,10 @@ type PolicyData struct { } const ( - // templateStart and templateEnd* wrap the caller-supplied exists query in an - // inner set of parentheses so that any OR in the WHERE body binds before the - // appended AND version_compare(...) clause. - templateStart = "SELECT 1 WHERE NOT EXISTS ((" - templateEndDarwin = ") AND version_compare(bundle_short_version, '%s') < 0);" - templateEndWindows = ") AND version_compare(version, '%s') < 0);" + // notExistsStart is prepended to the exists query body; version_compare is appended + // to the same WHERE clause (inside NOT EXISTS), matching the pre-#45647 generator. + notExistsStart = "SELECT 1 WHERE NOT EXISTS (" + existsPrefix = "SELECT 1 FROM " ) var ( @@ -32,23 +30,79 @@ var ( ErrNoExistsQuery = errors.New("exists query was not provided") ) -// GenerateQueryForManifest wraps the "exists" query to create a patch policy query +// GenerateQueryForManifest wraps the "exists" query to create a patch policy query. func GenerateQueryForManifest(p PolicyData) (string, error) { if p.ExistsQuery == "" { return "", ErrNoExistsQuery } + + suffix, err := versionCompareSuffix(p.Platform, p.ExistsQuery, p.Version) + if err != nil { + return "", err + } + before, _ := strings.CutSuffix(p.ExistsQuery, ";") - // Escape any literal '%' in the exists query (e.g. SQL LIKE patterns) - // so fmt.Sprintf doesn't interpret them as format verbs. - before = strings.ReplaceAll(before, "%", "%%") + before = strings.TrimSpace(before) + if strings.Contains(before, " OR ") { + before = parenthesizeWhereClause(before) + } + + return notExistsStart + before + suffix, nil +} + +// parenthesizeWhereClause wraps the WHERE body in parens when it contains OR so that +// the trailing AND version_compare(...) binds to the full predicate, not just the +// right-hand side of OR (SQL precedence: AND > OR). +func parenthesizeWhereClause(existsQuery string) string { + if !strings.HasPrefix(existsQuery, existsPrefix) { + return existsQuery + } + rest := strings.TrimPrefix(existsQuery, existsPrefix) + table, conditions, found := strings.Cut(rest, " WHERE ") + if !found { + return existsQuery + } + if !strings.Contains(conditions, " OR ") { + return existsQuery + } + return existsPrefix + table + " WHERE (" + conditions + ")" +} - switch p.Platform { +func versionCompareSuffix(platform, existsQuery, version string) (string, error) { + column, err := versionCompareColumn(platform, existsQuery) + if err != nil { + return "", err + } + return fmt.Sprintf(" AND version_compare(%s, '%s') < 0);", column, version), nil +} + +func versionCompareColumn(platform, existsQuery string) (string, error) { + switch platform { case "darwin": - return fmt.Sprintf(templateStart+before+templateEndDarwin, p.Version), nil + return "bundle_short_version", nil case "windows": - return fmt.Sprintf(templateStart+before+templateEndWindows, p.Version), nil + if tableFromExistsQuery(existsQuery) == "file" { + return "file_version", nil + } + return "version", nil + default: + return "", ErrWrongPlatform + } +} + +func tableFromExistsQuery(existsQuery string) string { + trimmed, _ := strings.CutSuffix(strings.TrimSpace(existsQuery), ";") + if !strings.HasPrefix(trimmed, existsPrefix) { + return "" + } + rest := strings.TrimPrefix(trimmed, existsPrefix) + if table, _, found := strings.Cut(rest, " WHERE "); found { + return table + } + if table, _, found := strings.Cut(rest, " "); found { + return table } - return "", ErrWrongPlatform + return strings.TrimSpace(rest) } // GenerateFromInstaller creates a patch policy with all fields from an installer diff --git a/pkg/patch_policy/patch_policy_test.go b/pkg/patch_policy/patch_policy_test.go index 68cd67dc3b6..581d1cefadb 100644 --- a/pkg/patch_policy/patch_policy_test.go +++ b/pkg/patch_policy/patch_policy_test.go @@ -20,7 +20,7 @@ func TestGenerateQueryForManifest(t *testing.T) { Version: "1.0", ExistsQuery: "SELECT 1 FROM apps WHERE bundle_identifier = 'com.foo';", }, - want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM apps WHERE bundle_identifier = 'com.foo') AND version_compare(bundle_short_version, '1.0') < 0);", + want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps WHERE bundle_identifier = 'com.foo' AND version_compare(bundle_short_version, '1.0') < 0);", }, { name: "windows from exists query", @@ -29,7 +29,7 @@ func TestGenerateQueryForManifest(t *testing.T) { Version: "1.0", ExistsQuery: "SELECT 1 FROM programs WHERE name = 'Foo x64' AND publisher = 'Bar, Inc.';", }, - want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM programs WHERE name = 'Foo x64' AND publisher = 'Bar, Inc.') AND version_compare(version, '1.0') < 0);", + want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM programs WHERE name = 'Foo x64' AND publisher = 'Bar, Inc.' AND version_compare(version, '1.0') < 0);", }, { name: "windows from exists query with LIKE percent wildcard", @@ -38,7 +38,7 @@ func TestGenerateQueryForManifest(t *testing.T) { Version: "12.5.6", ExistsQuery: "SELECT 1 FROM programs WHERE name LIKE 'Postman x64 %' AND publisher = 'Postman';", }, - want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM programs WHERE name LIKE 'Postman x64 %' AND publisher = 'Postman') AND version_compare(version, '12.5.6') < 0);", + want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM programs WHERE name LIKE 'Postman x64 %' AND publisher = 'Postman' AND version_compare(version, '12.5.6') < 0);", }, { name: "windows from exists query with multiple LIKE percent wildcards", @@ -47,16 +47,18 @@ func TestGenerateQueryForManifest(t *testing.T) { Version: "139.0.0", ExistsQuery: "SELECT 1 FROM programs WHERE name LIKE 'Mozilla Firefox % ESR %' AND publisher = 'Mozilla';", }, - want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM programs WHERE name LIKE 'Mozilla Firefox % ESR %' AND publisher = 'Mozilla') AND version_compare(version, '139.0.0') < 0);", + want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM programs WHERE name LIKE 'Mozilla Firefox % ESR %' AND publisher = 'Mozilla' AND version_compare(version, '139.0.0') < 0);", }, { - name: "windows from exists query containing OR (precedence fix)", + name: "codex-cli portable install OR precedence and file_version", p: patch_policy.PolicyData{ - Platform: "windows", - Version: "0.130.0", - ExistsQuery: "SELECT 1 FROM file WHERE path = 'C:\\a' OR path LIKE '%\\b';", + Platform: "windows", + Version: "0.130.0", + ExistsQuery: "SELECT 1 FROM file WHERE path = 'C:\\Program Files\\Codex CLI\\codex.exe' " + + "OR path LIKE '%\\AppData\\Local\\Programs\\Codex CLI\\codex.exe';", }, - want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM file WHERE path = 'C:\\a' OR path LIKE '%\\b') AND version_compare(version, '0.130.0') < 0);", + want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM file WHERE (path = 'C:\\Program Files\\Codex CLI\\codex.exe' " + + "OR path LIKE '%\\AppData\\Local\\Programs\\Codex CLI\\codex.exe') AND version_compare(file_version, '0.130.0') < 0);", }, } for _, tt := range tests {