Fix osquery version check

README.md

@@ -12,8 +12,8 @@
     <a href="https://goreportcard.com/report/github.com/jmpsec/osctrld">
       <img alt="Go Report Card" src="https://goreportcard.com/badge/github.com/jmpsec/osctrld?style=flat-square">
     </a>
-    <a href="https://github.com/jmpsec/osctrld/actions/workflows/ci.yml">
-      <img alt="CI" src="https://github.com/jmpsec/osctrld/actions/workflows/ci.yml/badge.svg">
+    <a href="https://github.com/jmpsec/osctrld/actions/workflows/ci-main.yml">
+      <img alt="CI" src="https://github.com/jmpsec/osctrld/actions/workflows/ci-main.yml/badge.svg">
     </a>
   </p>
 </p>
@@ -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
 

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

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]
 }

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))
+		})
+	}
+}

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

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) {