From ffa8c577bd6a80ede785c6de3a3b24b332997b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ervin=20R=C3=A1cz?= Date: Tue, 30 Jun 2026 17:13:21 +0300 Subject: [PATCH 1/4] fix(syncer): handle malformed Omaha responses without crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An upstream reply with no , or an without , caused an index-out-of-range / nil dereference that crashed the syncer. Parse the response defensively and return an error instead. Signed-off-by: Ervin Rácz --- backend/pkg/syncer/syncer.go | 26 ++++++++++-- backend/pkg/syncer/syncer_parse_test.go | 56 +++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 backend/pkg/syncer/syncer_parse_test.go diff --git a/backend/pkg/syncer/syncer.go b/backend/pkg/syncer/syncer.go index c1228360e..5f4890949 100644 --- a/backend/pkg/syncer/syncer.go +++ b/backend/pkg/syncer/syncer.go @@ -220,7 +220,11 @@ func (s *Syncer) checkForUpdates() error { return err } } else { - l.Debug().Str("channel", descriptor.name).Str("arch", descriptor.arch.String()).Str("currentVersion", currentVersion).Msgf("checkForUpdates, no update available updateStatus %v", update.Status) + status := "no updatecheck in response" + if update != nil { + status = string(update.Status) + } + l.Debug().Str("channel", descriptor.name).Str("arch", descriptor.arch.String()).Str("currentVersion", currentVersion).Msgf("checkForUpdates, no update available updateStatus %v", status) } select { @@ -273,13 +277,27 @@ func (s *Syncer) doOmahaRequest(descriptor channelDescriptor, currentVersion str } l.Debug().Str("response", string(body)).Msg("doOmahaRequest") - oresp := &omaha.Response{} - err = xml.Unmarshal(body, oresp) + update, err := parseOmahaUpdateResponse(body) if err != nil { - l.Error().Err(err).Msg("checkForUpdates, unmarshalling omaha response") + l.Error().Err(err).Msg("checkForUpdates, parsing omaha response") return nil, err } + return update, nil +} + +// parseOmahaUpdateResponse unmarshals an Omaha response body and returns the +// update check of its first app. It returns an error (rather than panicking) +// when the body is well-formed XML but carries no element. The returned +// update check may be nil when the app has no . +func parseOmahaUpdateResponse(body []byte) (*omaha.UpdateResponse, error) { + oresp := &omaha.Response{} + if err := xml.Unmarshal(body, oresp); err != nil { + return nil, err + } + if len(oresp.Apps) == 0 { + return nil, fmt.Errorf("omaha response contained no apps") + } return oresp.Apps[0].UpdateCheck, nil } diff --git a/backend/pkg/syncer/syncer_parse_test.go b/backend/pkg/syncer/syncer_parse_test.go new file mode 100644 index 000000000..a33abaee8 --- /dev/null +++ b/backend/pkg/syncer/syncer_parse_test.go @@ -0,0 +1,56 @@ +package syncer + +import ( + "encoding/xml" + "testing" + + "github.com/flatcar/go-omaha/omaha" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestParseOmahaUpdateResponse covers the guard that previously let a malformed +// upstream response panic the whole syncer process: a response with no +// must yield an error, and an without must yield a nil +// update check (not a panic). +func TestParseOmahaUpdateResponse(t *testing.T) { + const appID = "e96281a6-d1af-4bde-9a0a-97b76e56dc57" + + t.Run("app with updatecheck returns it", func(t *testing.T) { + resp := omaha.NewResponse() + resp.AddApp(appID, omaha.AppOK).AddUpdateCheck(omaha.UpdateOK) + body, err := xml.Marshal(resp) + require.NoError(t, err) + + uc, err := parseOmahaUpdateResponse(body) + require.NoError(t, err) + require.NotNil(t, uc) + assert.Equal(t, omaha.UpdateOK, uc.Status) + }) + + t.Run("no app element returns an error, not a panic", func(t *testing.T) { + body, err := xml.Marshal(omaha.NewResponse()) + require.NoError(t, err) + + uc, err := parseOmahaUpdateResponse(body) + require.Error(t, err) + assert.Nil(t, uc) + assert.Contains(t, err.Error(), "no apps") + }) + + t.Run("app without updatecheck returns nil and no error", func(t *testing.T) { + resp := omaha.NewResponse() + resp.AddApp(appID, omaha.AppOK) + body, err := xml.Marshal(resp) + require.NoError(t, err) + + uc, err := parseOmahaUpdateResponse(body) + require.NoError(t, err) + assert.Nil(t, uc) + }) + + t.Run("malformed body returns an error", func(t *testing.T) { + _, err := parseOmahaUpdateResponse([]byte("not xml at all")) + require.Error(t, err) + }) +} From cb2018cd44cade63a70560b27aa6c971392e0181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ervin=20R=C3=A1cz?= Date: Wed, 1 Jul 2026 10:18:45 +0300 Subject: [PATCH 2/4] fix(api): select floor versions with full semver precedence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Floor selection compared versions in SQL with the pre-release/build suffix stripped, so a release floor could be skipped for an instance on a pre-release of the same version. Select the range with blang/semver. Signed-off-by: Ervin Rácz --- backend/pkg/api/packages_floors.go | 96 +++++++++++-------------- backend/pkg/api/packages_floors_test.go | 60 ++++++++++++++++ 2 files changed, 100 insertions(+), 56 deletions(-) diff --git a/backend/pkg/api/packages_floors.go b/backend/pkg/api/packages_floors.go index a01875fb1..011a54d14 100644 --- a/backend/pkg/api/packages_floors.go +++ b/backend/pkg/api/packages_floors.go @@ -5,7 +5,9 @@ import ( "errors" "fmt" "regexp" + "sort" + "github.com/blang/semver/v4" "github.com/doug-martin/goqu/v9" "gopkg.in/guregu/null.v4" ) @@ -26,29 +28,6 @@ func semverToIntArray(column string) (string, error) { return fmt.Sprintf("string_to_array((regexp_split_to_array(%s, '[+-]'))[1], '.')::int[]", column), nil } -// versionCompareExpr creates a version comparison expression -func versionCompareExpr(column, operator, value string) (goqu.Expression, error) { - // Validate operator to prevent SQL injection - validOperators := map[string]bool{ - ">": true, ">=": true, "<": true, "<=": true, "=": true, "!=": true, - } - if !validOperators[operator] { - return nil, fmt.Errorf("versionCompareExpr: invalid operator %q - potential SQL injection", operator) - } - - colArray, err := semverToIntArray(column) - if err != nil { - return nil, err - } - - valArray, err := semverToIntArray("?") - if err != nil { - return nil, err - } - - return goqu.L(fmt.Sprintf("%s %s %s", colArray, operator, valArray), value), nil -} - // AddChannelPackageFloor marks a package as a floor for a specific channel func (api *API) AddChannelPackageFloor(channelID, packageID string, floorReason null.String) error { // Verify channel and package exist and are compatible in a single query @@ -226,7 +205,9 @@ const ( DefaultMaxFloorsPerResponse = 5 ) -// GetRequiredChannelFloors returns floor packages between instance and target versions for a channel +// GetRequiredChannelFloors returns the floor packages between the instance version and +// the channel target (instance < floor <= target), sorted ascending by semantic version +// and capped at the configured limit. func (api *API) GetRequiredChannelFloors(channel *Channel, instanceVersion string) ([]*Package, error) { if channel == nil || channel.Package == nil { return nil, ErrNoPackageFound @@ -235,52 +216,55 @@ func (api *API) GetRequiredChannelFloors(channel *Channel, instanceVersion strin return nil, fmt.Errorf("instance version cannot be empty") } - targetVersion := channel.Package.Version - - maxFloorsPerResponse := DefaultMaxFloorsPerResponse + limit := DefaultMaxFloorsPerResponse if api.maxFloorsPerResponse > 0 { - maxFloorsPerResponse = api.maxFloorsPerResponse + limit = api.maxFloorsPerResponse } - // No blacklist check needed for floors - gtExpr, err := versionCompareExpr("p.version", ">", instanceVersion) + allFloors, err := api.GetChannelFloorPackages(channel.ID) if err != nil { return nil, err } - lteExpr, err := versionCompareExpr("p.version", "<=", targetVersion) + floors, _, err := selectFloorsInRange(allFloors, instanceVersion, channel.Package.Version, limit) + return floors, err +} + +// selectFloorsInRange returns the floor packages with instanceVersion < v <= targetVersion, +// sorted ascending by semantic version and capped at limit. hasMore is true when more +// floors fall in range than the limit. +func selectFloorsInRange(floors []*Package, instanceVersion, targetVersion string, limit int) ([]*Package, bool, error) { + inst, err := semver.Make(instanceVersion) if err != nil { - return nil, err + return nil, false, fmt.Errorf("invalid instance version %q: %w", instanceVersion, err) } - - semverExpr, err := semverToIntArray("p.version") + target, err := semver.Make(targetVersion) if err != nil { - return nil, err + return nil, false, fmt.Errorf("invalid target version %q: %w", targetVersion, err) } - query, _, err := goqu.From(goqu.L(` - package p - JOIN channel_package_floors cpf ON p.id = cpf.package_id - `)). - Select(goqu.L(` - p.*, - true as is_floor, - cpf.floor_reason - `)). - Where(goqu.And( - goqu.C("channel_id").Table("cpf").Eq(channel.ID), - gtExpr, - lteExpr, - )). - Order(goqu.L(semverExpr).Asc()). - Limit(uint(maxFloorsPerResponse)). - ToSQL() - - if err != nil { - return nil, err + inRange := make([]*Package, 0, len(floors)) + for _, p := range floors { + v, err := semver.Make(p.Version) + if err != nil { + continue // versions are validated on insert; skip defensively + } + if v.GT(inst) && v.LTE(target) { + inRange = append(inRange, p) + } } - return api.getPackagesFromQuery(query) + sort.SliceStable(inRange, func(i, j int) bool { + vi, _ := semver.Make(inRange[i].Version) + vj, _ := semver.Make(inRange[j].Version) + return vi.LT(vj) + }) + + hasMore := len(inRange) > limit + if hasMore { + inRange = inRange[:limit] + } + return inRange, hasMore, nil } // GetChannelFloorPackagesCount returns the count of floor packages for a channel diff --git a/backend/pkg/api/packages_floors_test.go b/backend/pkg/api/packages_floors_test.go index 52e550099..2e3829efa 100644 --- a/backend/pkg/api/packages_floors_test.go +++ b/backend/pkg/api/packages_floors_test.go @@ -291,3 +291,63 @@ func TestFloorBlacklistConflict(t *testing.T) { assert.Error(t, err) }) } + +// TestSelectFloorsInRange covers the floor version range, ordering, and limit logic, +// including pre-release and build metadata precedence. +func TestSelectFloorsInRange(t *testing.T) { + mk := func(vs ...string) []*Package { + ps := make([]*Package, len(vs)) + for i, v := range vs { + ps[i] = &Package{Version: v} + } + return ps + } + vers := func(ps []*Package) []string { + out := make([]string, 0, len(ps)) + for _, p := range ps { + out = append(out, p.Version) + } + return out + } + + tests := []struct { + name string + floors []*Package + instance, target string + limit int + want []string + wantMore bool + }{ + {"release floor not skipped over pre-release instance", mk("2000.0.0"), "2000.0.0-rc1", "3000.0.0", 5, []string{"2000.0.0"}, false}, + {"same-core floors ordered, none dropped", mk("2000.0.0", "2000.0.0-beta", "2000.0.0-alpha"), "1000.0.0", "3000.0.0", 5, []string{"2000.0.0-alpha", "2000.0.0-beta", "2000.0.0"}, false}, + {"lower pre-release below release instance excluded", mk("2000.0.0-rc1"), "2000.0.0", "3000.0.0", 5, []string{}, false}, + {"build metadata treated as the release", mk("2000.0.0+build"), "1999.0.0", "3000.0.0", 5, []string{"2000.0.0+build"}, false}, + {"shuffled input sorted, limited, hasMore", mk("3000.0.0", "1000.0.0", "2000.0.0"), "500.0.0", "4000.0.0", 2, []string{"1000.0.0", "2000.0.0"}, true}, + {"target inclusive, above-target excluded", mk("2000.0.0", "4000.0.0"), "1000.0.0", "3000.0.0", 5, []string{"2000.0.0"}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res, more, err := selectFloorsInRange(tt.floors, tt.instance, tt.target, tt.limit) + require.NoError(t, err) + assert.Equal(t, tt.want, vers(res)) + assert.Equal(t, tt.wantMore, more) + }) + } +} + +// TestFloorPreReleaseNotSkipped checks that a release floor is still returned when the +// instance reports a pre-release of the same version. +func TestFloorPreReleaseNotSkipped(t *testing.T) { + a := newForTest(t) + defer a.Close() + + setup := setupFloors(t, a, "prerelease", []string{"2000.0.0"}, "3000.0.0") + + ch, err := a.GetChannel(setup.Channel.ID) + require.NoError(t, err) + floors, err := a.GetRequiredChannelFloors(ch, "2000.0.0-rc1") + require.NoError(t, err) + require.Len(t, floors, 1) + assert.Equal(t, "2000.0.0", floors[0].Version) +} From cc2fc69d9feccf81ffde8d7819d1edfd83d8b6db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ervin=20R=C3=A1cz?= Date: Wed, 1 Jul 2026 13:56:50 +0300 Subject: [PATCH 3/4] docs(changelog): add entries for the syncer crash-guard and floor fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ervin Rácz --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a87f0f14..c60d8e692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Bugfixes - Fixed package blacklist changes not appearing in UI immediately after save +- Fixed the syncer crashing the Nebraska process when the upstream server returns a malformed or empty Omaha response +- Fixed mandatory floor packages being skipped when an instance reports a pre-release of the floor's version ## [3.0.0] - 28/11/2025 From 42e572740c9f145cd9fd02c3f9515b337e62ac81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ervin=20R=C3=A1cz?= Date: Wed, 1 Jul 2026 15:44:52 +0300 Subject: [PATCH 4/4] fix(syncer): Mark single manifest as floor when applicable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Record floor status for single manifests that are channel targets - Prevent downstream clients from skipping floor packages Signed-off-by: Ervin Rácz --- backend/pkg/syncer/syncer.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/backend/pkg/syncer/syncer.go b/backend/pkg/syncer/syncer.go index 5f4890949..2a5e2734c 100644 --- a/backend/pkg/syncer/syncer.go +++ b/backend/pkg/syncer/syncer.go @@ -331,6 +331,14 @@ func (s *Syncer) processSingleManifestUpdate(descriptor channelDescriptor, updat return err } + // A single manifest can still be a floor (e.g. the channel target is itself a + // floor). Record it as a floor so downstream clients cannot skip it. + if manifest.IsFloor { + if err := s.markPackageAsFloor(descriptor, pkg, manifest); err != nil { + return err + } + } + // Update channel to point to the package if err := s.updateChannelToPackage(descriptor, pkg); err != nil { l.Error().Err(err).