glob parser + tests ,address pr comments

Author: Bajusz15

providers/os/resources/sshd.go

@@ -172,41 +172,37 @@ func (s *mqlSshdConfig) parse(file *mqlFile) error {
 		file.Path.Data: file,
 	}
 	var allContents strings.Builder
-	globPathContent := func(glob string) (string, []string, error) {
-		paths, err := s.expandGlob(glob)
-		if err != nil {
-			return "", nil, err
-		}
-
-		var content strings.Builder
-		for _, path := range paths {
-			file, ok := filesIdx[path]
-			if !ok {
-				raw, err := CreateResource(s.MqlRuntime, "file", map[string]*llx.RawData{
-					"path": llx.StringData(path),
-				})
-				if err != nil {
-					return "", nil, err
-				}
-				file = raw.(*mqlFile)
-				filesIdx[path] = file
-			}
 
-			fileContent := file.GetContent()
-			if fileContent.Error != nil {
-				return "", nil, fileContent.Error
+	// Function to get file content by path
+	fileContent := func(path string) (string, error) {
+		file, ok := filesIdx[path]
+		if !ok {
+			raw, err := CreateResource(s.MqlRuntime, "file", map[string]*llx.RawData{
+				"path": llx.StringData(path),
+			})
+			if err != nil {
+				return "", err
 			}
+			file = raw.(*mqlFile)
+			filesIdx[path] = file
+		}
 
-			content.WriteString(fileContent.Data)
-			content.WriteString("\n")
+		fileContent := file.GetContent()
+		if fileContent.Error != nil {
+			return "", fileContent.Error
 		}
 
-		res := content.String()
-		allContents.WriteString(res)
-		return res, paths, nil
+		content := fileContent.Data + "\n"
+		allContents.WriteString(content)
+		return fileContent.Data, nil
+	}
+
+	// Function to expand glob patterns
+	globExpand := func(glob string) ([]string, error) {
+		return s.expandGlob(glob)
 	}
 
-	matchBlocks, err := sshd.ParseBlocks(file.Path.Data, globPathContent)
+	matchBlocks, err := sshd.ParseBlocksWithGlob(file.Path.Data, fileContent, globExpand)
 	// TODO: check if not ready on I/O
 	if err != nil {
 		s.Params = plugin.TValue[map[string]any]{Error: err, State: plugin.StateIsSet | plugin.StateIsNull}

providers/os/resources/sshd/params.go

@@ -112,25 +112,136 @@ func mergeIncludedBlocks(matchConditions map[string]*MatchBlock, blocks MatchBlo
 	}
 }
 
-type globPathContentFunc func(string) (content string, paths []string, err error)
+type (
+	fileContentFunc func(string) (content string, err error)
+	globExpandFunc  func(string) (paths []string, err error)
+)
+
+// ParseBlocks parses a single SSH config file and returns the match blocks.
+// The filePath should be the actual file path (not a glob pattern).
+// For Include directives with glob patterns, use ParseBlocksWithGlob instead.
+func ParseBlocks(filePath string, content string) (MatchBlocks, error) {
+	curBlock := &MatchBlock{
+		Criteria: "",
+		Params:   map[string]any{},
+		Context: Context{
+			Path:    filePath,
+			Range:   llx.NewRange(),
+			curLine: 1,
+		},
+	}
+	matchConditions := map[string]*MatchBlock{
+		"": curBlock,
+	}
+
+	lines := strings.Split(content, "\n")
+	for curLineIdx, textLine := range lines {
+		l, err := ParseLine([]rune(textLine))
+		if err != nil {
+			return nil, err
+		}
+
+		key := l.key
+		if key == "" {
+			continue
+		}
+
+		// handle lower case entries and use proper ssh camel case
+		if sshKey, ok := SSH_Keywords[strings.ToLower(key)]; ok {
+			key = sshKey
+		}
+
+		if key == "Include" {
+			// Include directives are handled by ParseBlocksWithGlob
+			// This function only parses single files
+			log.Warn().Str("file", filePath).Msg("Include directive found in single-file parser, use ParseBlocksWithGlob instead")
+			continue
+		}
+
+		if key == "Match" {
+			// wrap up context on the previous block
+			curBlock.Context.Range = curBlock.Context.Range.AddLineRange(uint32(curBlock.Context.curLine), uint32(curLineIdx))
+			curBlock.Context.curLine = curLineIdx
 
-func ParseBlocks(rootPath string, globPathContent globPathContentFunc) (MatchBlocks, error) {
-	content, actualPaths, err := globPathContent(rootPath)
+			// This key is stored in the condition of each block and can be accessed there.
+			condition := l.args
+			if b, ok := matchConditions[condition]; ok {
+				curBlock = b
+			} else {
+				curBlock = &MatchBlock{
+					Criteria: condition,
+					Params:   map[string]any{},
+					Context: Context{
+						curLine: curLineIdx + 1,
+						Path:    filePath,
+						Range:   llx.NewRange(),
+					},
+				}
+				matchConditions[condition] = curBlock
+			}
+			continue
+		}
+
+		setParam(curBlock.Params, key, l.args)
+	}
+
+	keys := sortx.Keys(matchConditions)
+	res := make([]*MatchBlock, len(keys))
+	i := 0
+	for _, key := range keys {
+		res[i] = matchConditions[key]
+		i++
+	}
+
+	curBlock.Context.Range = curBlock.Context.Range.AddLineRange(uint32(curBlock.Context.curLine), uint32(len(lines)))
+
+	return res, nil
+}
+
+// ParseBlocksWithGlob parses SSH config files, expanding glob patterns in Include directives.
+// It expands globs and calls ParseBlocksWithGlobRecursive for each matched file individually,
+func ParseBlocksWithGlob(rootPath string, fileContent fileContentFunc, globExpand globExpandFunc) (MatchBlocks, error) {
+	// First, expand the root path if it's a glob
+	paths, err := globExpand(rootPath)
 	if err != nil {
 		return nil, err
 	}
 
-	// This ensures that glob patterns use the actual file path instead of the glob pattern
-	actualPath := rootPath
-	if len(actualPaths) > 0 {
-		actualPath = actualPaths[0]
+	// If no paths matched, return empty blocks
+	if len(paths) == 0 {
+		return MatchBlocks{}, nil
 	}
 
+	// Parse each file individually and collect all blocks
+	// Each file maintains its own context (path, line numbers)
+	var allBlocks MatchBlocks
+
+	for _, path := range paths {
+		content, err := fileContent(path)
+		if err != nil {
+			log.Warn().Err(err).Str("path", path).Msg("unable to read file")
+			continue
+		}
+
+		blocks, err := ParseBlocksWithGlobRecursive(path, content, fileContent, globExpand)
+		if err != nil {
+			log.Warn().Err(err).Str("path", path).Msg("unable to parse file")
+			continue
+		}
+
+		allBlocks = append(allBlocks, blocks...)
+	}
+
+	return allBlocks, nil
+}
+
+// ParseBlocksWithGlobRecursive parses a single file and recursively handles Include directives.
+func ParseBlocksWithGlobRecursive(filePath string, content string, fileContent fileContentFunc, globExpand globExpandFunc) (MatchBlocks, error) {
 	curBlock := &MatchBlock{
 		Criteria: "",
 		Params:   map[string]any{},
 		Context: Context{
-			Path:    actualPath,
+			Path:    filePath,
 			Range:   llx.NewRange(),
 			curLine: 1,
 		},
@@ -158,15 +269,31 @@ func ParseBlocks(rootPath string, globPathContent globPathContentFunc) (MatchBlo
 
 		if key == "Include" {
 			// FIXME: parse multi-keys properly
-			paths := strings.Split(l.args, " ")
+			includePaths := strings.Split(l.args, " ")
 
-			for _, path := range paths {
-				subBlocks, err := ParseBlocks(path, globPathContent)
+			for _, includePath := range includePaths {
+				// Expand glob pattern if present
+				expandedPaths, err := globExpand(includePath)
 				if err != nil {
-					log.Warn().Err(err).Msg("unable to parse Include directive")
+					log.Warn().Err(err).Str("path", includePath).Msg("unable to expand Include directive")
 					continue
 				}
-				mergeIncludedBlocks(matchConditions, subBlocks, curBlock.Criteria)
+
+				// Parse each matched file individually
+				for _, expandedPath := range expandedPaths {
+					subContent, err := fileContent(expandedPath)
+					if err != nil {
+						log.Warn().Err(err).Str("path", expandedPath).Msg("unable to read included file")
+						continue
+					}
+
+					subBlocks, err := ParseBlocksWithGlobRecursive(expandedPath, subContent, fileContent, globExpand)
+					if err != nil {
+						log.Warn().Err(err).Str("path", expandedPath).Msg("unable to parse included file")
+						continue
+					}
+					mergeIncludedBlocks(matchConditions, subBlocks, curBlock.Criteria)
+				}
 			}
 			continue
 		}
@@ -187,7 +314,7 @@ func ParseBlocks(rootPath string, globPathContent globPathContentFunc) (MatchBlo
 					Params:   map[string]any{},
 					Context: Context{
 						curLine: curLineIdx + 1,
-						Path:    actualPath,
+						Path:    filePath,
 						Range:   llx.NewRange(),
 					},
 				}

providers/os/resources/sshd/params_test.go

@@ -15,9 +15,7 @@ func TestSSHParser(t *testing.T) {
 	raw, err := os.ReadFile("./testdata/sshd_config")
 	require.NoError(t, err)
 
-	sshParams, err := ParseBlocks(string(raw), func(s string) (string, []string, error) {
-		return string(raw), []string{s}, nil
-	})
+	sshParams, err := ParseBlocks("./testdata/sshd_config", string(raw))
 	if err != nil {
 		t.Fatalf("cannot request file %v", err)
 	}
@@ -34,9 +32,7 @@ func TestSSHParseCaseInsensitive(t *testing.T) {
 	raw, err := os.ReadFile("./testdata/case_insensitive")
 	require.NoError(t, err)
 
-	sshParams, err := ParseBlocks(string(raw), func(s string) (string, []string, error) {
-		return string(raw), []string{s}, nil
-	})
+	sshParams, err := ParseBlocks("./testdata/case_insensitive", string(raw))
 	if err != nil {
 		t.Fatalf("cannot request file %v", err)
 	}
@@ -47,3 +43,110 @@ func TestSSHParseCaseInsensitive(t *testing.T) {
 	assert.Equal(t, "any", sshParams[0].Params["AddressFamily"])
 	assert.Equal(t, "0.0.0.0", sshParams[0].Params["ListenAddress"])
 }
+
+func TestSSHParseWithGlob(t *testing.T) {
+	// Test that glob patterns in Include directives are expanded correctly
+	// and that each file maintains its own context
+
+	fileContent := func(path string) (string, error) {
+		content, err := os.ReadFile(path)
+		if err != nil {
+			return "", err
+		}
+		return string(content), nil
+	}
+
+	globExpand := func(glob string) ([]string, error) {
+		// Simple glob expansion for test - in real code this would use filepath.Glob
+		// For this test, we'll manually handle the known patterns
+		var paths []string
+		if glob == "conf.d/*.conf" {
+			paths = []string{
+				"./testdata/conf.d/01_mondoo.conf",
+				"./testdata/conf.d/02_security.conf",
+			}
+		} else if glob == "subdir/01_*.conf" {
+			paths = []string{
+				"./testdata/subdir/01_custom.conf",
+			}
+		} else if glob == "subdir/02_*.conf" {
+			paths = []string{
+				"./testdata/subdir/02_additional.conf",
+			}
+		} else if glob == "./testdata/sshd_config_with_include" {
+			paths = []string{"./testdata/sshd_config_with_include"}
+		} else {
+			// Try to read as a single file
+			if _, err := os.Stat(glob); err == nil {
+				paths = []string{glob}
+			}
+		}
+		return paths, nil
+	}
+
+	blocks, err := ParseBlocksWithGlob("./testdata/sshd_config_with_include", fileContent, globExpand)
+	require.NoError(t, err)
+	assert.NotNil(t, blocks)
+
+	// Verify that we have blocks from multiple files
+	// The main file should have blocks, and included files should also contribute
+	assert.Greater(t, len(blocks), 0, "should have at least one block")
+
+	// Check that params from included files are present
+	// Find the default block (empty criteria)
+	var defaultBlock *MatchBlock
+	for _, block := range blocks {
+		if block.Criteria == "" {
+			defaultBlock = block
+			break
+		}
+	}
+	require.NotNil(t, defaultBlock, "should have a default block")
+
+	// Verify params from main file
+	assert.Equal(t, "22", defaultBlock.Params["Port"])
+	assert.Equal(t, "any", defaultBlock.Params["AddressFamily"])
+	assert.Equal(t, "yes", defaultBlock.Params["UsePAM"])
+	assert.Equal(t, "yes", defaultBlock.Params["X11Forwarding"])
+
+	// Verify params from included files (conf.d/*.conf)
+	assert.Equal(t, "no", defaultBlock.Params["PermitRootLogin"])
+	assert.Equal(t, "yes", defaultBlock.Params["PasswordAuthentication"])
+	assert.Equal(t, "3", defaultBlock.Params["MaxAuthTries"])
+	assert.Equal(t, "30", defaultBlock.Params["LoginGraceTime"])
+
+	// Verify params from subdirectory files
+	assert.Contains(t, defaultBlock.Params["Ciphers"], "aes256-gcm@openssh.com")
+	assert.Contains(t, defaultBlock.Params["MACs"], "hmac-sha2-256-etm@openssh.com")
+
+	// Verify that blocks have correct file paths in their context
+	// Check that we have blocks from different files
+	filePaths := make(map[string]bool)
+	for _, block := range blocks {
+		filePaths[block.Context.Path] = true
+	}
+
+	// Should have blocks from main file and included files
+	assert.Greater(t, len(filePaths), 1, "should have blocks from multiple files")
+
+	// Verify Match blocks from included files
+	var sftpBlock *MatchBlock
+	var adminBlock *MatchBlock
+	for _, block := range blocks {
+		if block.Criteria == "Group sftp-users" {
+			sftpBlock = block
+		}
+		if block.Criteria == "User admin" {
+			adminBlock = block
+		}
+	}
+
+	require.NotNil(t, sftpBlock, "should have sftp-users match block")
+	assert.Equal(t, "no", sftpBlock.Params["AllowTcpForwarding"])
+	assert.Contains(t, sftpBlock.Context.Path, "01_mondoo.conf")
+
+	require.NotNil(t, adminBlock, "should have admin user match block")
+	assert.Equal(t, "yes", adminBlock.Params["PermitRootLogin"])
+	assert.Equal(t, "no", adminBlock.Params["PasswordAuthentication"])
+	assert.Contains(t, adminBlock.Context.Path, "02_security.conf")
+}

providers/os/resources/sshd/testdata/conf.d/01_mondoo.conf

@@ -0,0 +1,8 @@
+# First included config file
+PermitRootLogin no
+PasswordAuthentication yes
+
+Match Group sftp-users
+	AllowTcpForwarding no
+	ChrootDirectory /home/%u
+