From 643ac1fe3c8b37fd308e81cb97e805867df9ee2c Mon Sep 17 00:00:00 2001 From: Javier Marcos <1271349+javuto@users.noreply.github.com> Date: Sat, 12 Sep 2026 08:03:02 +0200 Subject: [PATCH] Fix osquery version check --- README.md | 14 ++++++------- cmd/osctrld/actions.go | 8 ++++++-- cmd/osctrld/actions_helpers.go | 11 +++++++--- cmd/osctrld/actions_helpers_test.go | 21 +++++++++++++++++++ cmd/osctrld/utils.go | 31 ++++++++++++++--------------- cmd/osctrld/utils_test.go | 31 +++++++++++++++++++++++++---- 6 files changed, 84 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 8be30d2..09b21b6 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,8 @@ Go Report Card - - CI + + CI

@@ -77,7 +77,7 @@ For this repository, the most useful starting points are: osctrld is a Go CLI built around a small set of commands: -- `enroll` and `remove` execute osctrl-provided lifecycle scripts for a node. +- `enroll` and `remove` retrieve osctrl-provided lifecycle scripts and print them to stdout for the operator to review and run. - `flags` and `cert` fetch managed osquery files and write them locally. - `verify` checks whether local osquery configuration matches osctrl. - `service` runs the daemon loop and repeatedly syncs managed state. @@ -86,7 +86,7 @@ osctrld is a Go CLI built around a small set of commands: The daemon mode performs a sync cycle on a configurable interval. During each cycle, osctrld retrieves flags, certificates, and extension manifests from osctrl, writes changed content to disk, and restarts osquery through the operating system service manager when a managed artifact changes. -Configuration is loaded from the `osctrld` section of a YAML or JSON file and can be overridden by CLI flags or environment variables. YAML is the default format for new deployments. Runtime defaults are applied before commands run, and wrapped commands validate the effective configuration before doing work. +Configuration comes from CLI flags, environment variables, or the `osctrld` section of a YAML or JSON file. YAML is the default format for new deployments. Note that supplying a configuration file replaces the entire effective configuration, so a config file takes precedence over flags and environment variables for every field, not only the ones it sets. Runtime defaults are applied before commands run, and wrapped commands validate the effective configuration before doing work. ## 📦 Installation @@ -134,21 +134,21 @@ JSON configuration files are also supported. Use a `.json` extension and osctrld | Field | Description | Default | | --- | --- | --- | -| `osctrlSecret` | osctrl enrollment secret value used to authenticate requests | Required for enrollment workflows | +| `osctrlSecret` | osctrl enrollment secret value used to authenticate requests | Required | | `osquerySecretFile` | Local path where the osquery enrollment secret file is written or verified | OS-dependent | | `osqueryFlagFile` | Path to the osquery flags file | OS-dependent | | `osqueryCertFile` | Path to the osquery TLS certificate file | OS-dependent | | `enrollScript` | Path to the enroll script | OS-dependent | | `removeScript` | Path to the remove script | OS-dependent | | `osquery` | Path to the osquery installation directory | OS-dependent | -| `environment` | osctrl environment name or UUID | Required for most workflows | +| `environment` | osctrl environment name or UUID | Required | | `baseurl` | Base URL of the osctrl server | Required | | `insecure` | Ignore TLS certificate warnings | `false` | | `verbose` | Enable debug logging | `false` | | `force` | Overwrite existing managed files | `false` | | `logFormat` | Log format: `text` or `json` | `text` | | `interval` | Sync interval in minutes for daemon mode | `60` | -| `extensionsDir` | Directory for osquery extension binaries | OS-dependent | +| `extensionsDir` | Directory for osquery extension binaries (configuration file only, no flag) | OS-dependent | ## 🧭 Usage diff --git a/cmd/osctrld/actions.go b/cmd/osctrld/actions.go index 4d9944b..bc48ca5 100644 --- a/cmd/osctrld/actions.go +++ b/cmd/osctrld/actions.go @@ -179,9 +179,13 @@ func verifyNode(c *cli.Context) error { log.Debug().Str("version", verification.OsqueryVersion).Msg("expected osquery version") existingVersion := getOsqueryVersion() log.Debug().Str("version", existingVersion).Msg("existing osquery version") - if osqueryVersionCompare(existingVersion, verification.OsqueryVersion) > 1 { + // 2 means required is higher, -1 means one of them could not be parsed + switch osqueryVersionCompare(existingVersion, verification.OsqueryVersion) { + case 2: log.Warn().Str("existing", existingVersion).Str("required", verification.OsqueryVersion).Msg("osquery version too low") - } else { + case -1: + log.Warn().Str("existing", existingVersion).Str("required", verification.OsqueryVersion).Msg("unknown osquery version") + default: log.Info().Str("version", existingVersion).Msg("osquery version is valid") } // Check if osquery is running diff --git a/cmd/osctrld/actions_helpers.go b/cmd/osctrld/actions_helpers.go index 2a2723d..7b5d8f7 100644 --- a/cmd/osctrld/actions_helpers.go +++ b/cmd/osctrld/actions_helpers.go @@ -143,9 +143,14 @@ func getOsqueryVersion() string { log.Error().Err(err).Str("output", string(out)).Msg("error running osqueryd") return "" } - splitted := strings.Split(strings.TrimSpace(string(out)), " ") - if len(splitted) < 2 { + return parseOsqueryVersion(string(out)) +} + +// Helper function to extract the version from the "osqueryd -version" output +func parseOsqueryVersion(out string) string { + fields := strings.Fields(out) + if len(fields) < 3 { return "" } - return splitted[2] + return fields[2] } diff --git a/cmd/osctrld/actions_helpers_test.go b/cmd/osctrld/actions_helpers_test.go index ab28577..5f29199 100644 --- a/cmd/osctrld/actions_helpers_test.go +++ b/cmd/osctrld/actions_helpers_test.go @@ -187,3 +187,24 @@ func TestCheckFileContent_Whitespace(t *testing.T) { assert.True(t, checkFileContent(path, "hello")) } + +func TestParseOsqueryVersion(t *testing.T) { + cases := []struct { + name string + output string + expected string + }{ + {"osqueryd output", "osqueryd version 5.12.1", "5.12.1"}, + {"trailing newline", "osqueryd version 5.12.1\n", "5.12.1"}, + {"extra whitespace", " osqueryd version 5.12.1 ", "5.12.1"}, + {"two fields", "osqueryd 5.12.1", ""}, + {"one field", "5.12.1", ""}, + {"empty output", "", ""}, + {"whitespace only", " \n", ""}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, parseOsqueryVersion(tt.output)) + }) + } +} diff --git a/cmd/osctrld/utils.go b/cmd/osctrld/utils.go index d6b5b27..c960c0e 100644 --- a/cmd/osctrld/utils.go +++ b/cmd/osctrld/utils.go @@ -28,11 +28,11 @@ const ( // OsctrlURLs keeps all osctrl URLs type OsctrlURLs struct { - URL string - Flags string - Cert string - Verify string - Enroll string + URL string + Flags string + Cert string + Verify string + Enroll string Remove string Extensions string } @@ -103,18 +103,13 @@ func osqueryVersionCompare(existing, required string) int { ex := strings.Split(existing, ".") req := strings.Split(required, ".") // Make sure both slices are the same length - if len(ex) > len(req) { - for i := 0; i < len(ex)-len(req); i++ { - req = append(req, "0") - } + for len(ex) < len(req) { + ex = append(ex, "0") } - if len(req) > len(ex) { - for i := 0; i < len(req)-len(ex); i++ { - ex = append(ex, "0") - } + for len(req) < len(ex) { + req = append(req, "0") } - res := 2 - // Iterate through all elements to compare and check what is higher + // Compare component by component, the first difference decides for v := 0; v < len(ex); v++ { exConv, err := strconv.Atoi(ex[v]) if err != nil { @@ -127,8 +122,12 @@ func osqueryVersionCompare(existing, required string) int { if exConv > reqConv { return 1 } + if exConv < reqConv { + return 2 + } } - return res + // All components are equal, regardless of how many each had + return 0 } // Helper to compose a full path given partial path and file diff --git a/cmd/osctrld/utils_test.go b/cmd/osctrld/utils_test.go index cef3411..42a3cb1 100644 --- a/cmd/osctrld/utils_test.go +++ b/cmd/osctrld/utils_test.go @@ -60,10 +60,33 @@ func TestGenURLs(t *testing.T) { } func TestOsqueryVersionCompare(t *testing.T) { - assert.Equal(t, 0, osqueryVersionCompare("1.2.3", "1.2.3")) - assert.Equal(t, 1, osqueryVersionCompare("4.0.0", "3.0.0")) - assert.Equal(t, 2, osqueryVersionCompare("3.0.0", "4.0.0")) - assert.Equal(t, -1, osqueryVersionCompare("3.0.0", "a.0.0")) + cases := []struct { + name string + existing string + required string + expected int + }{ + {"identical", "1.2.3", "1.2.3", 0}, + {"equal with different component count", "1.2", "1.2.0", 0}, + {"equal with padding both ways", "1.2.0.0", "1.2", 0}, + {"existing higher, same magnitude", "4.0.0", "3.0.0", 1}, + {"required higher, same magnitude", "3.0.0", "4.0.0", 2}, + {"minor does not outrank major", "1.9.0", "2.0.0", 2}, + {"major outranks minor", "2.0.0", "1.9.0", 1}, + {"patch decides", "5.12.1", "5.12.2", 2}, + {"shorter existing is lower", "1.2", "1.2.1", 2}, + {"shorter existing is higher", "1.3", "1.2.9", 1}, + {"double digits are numeric, not lexical", "5.10.0", "5.9.0", 1}, + {"unparseable required", "3.0.0", "a.0.0", -1}, + {"unparseable existing", "3.0.x", "3.0.1", -1}, + {"empty existing", "", "5.0.0", -1}, + {"empty required", "5.0.0", "", -1}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, osqueryVersionCompare(tt.existing, tt.required)) + }) + } } func TestGenFullPath(t *testing.T) {