Fix ci and improve rulesets download

Author: xiwenc

.github/workflows/ci.yml

@@ -165,6 +165,22 @@ jobs:
         modelsource: resources/modelsource-v1
         lint:
           xunitReport: report.xml
+          skip:
+            MyFirstModule/DomainModels$DomainModel:
+              - rule: "002_0009"
+                reason: CI fixture intentionally keeps a default value for rule coverage
+            MyFirstModule/Home_Web.Forms$Page:
+              - rule: "004_0001"
+                reason: CI fixture intentionally uses inline style for rule coverage
+              - rule: "004_0004"
+                reason: CI fixture intentionally has heading order issue for rule coverage
+            MyFirstModule/BadPage.Forms$Page:
+              - rule: "004_0002"
+                reason: CI fixture intentionally omits image alt text for rule coverage
+              - rule: "004_0003"
+                reason: CI fixture intentionally uses multiple h1 tags for rule coverage
+              - rule: "004_0004"
+                reason: CI fixture intentionally has heading order issue for rule coverage
         EOF
         ./bin/mxlint --config .ci/lint.yaml lint
 

lint/lint.go

@@ -370,32 +370,35 @@ func evalTestcaseWithCaching(rule Rule, queryString string, inputFile string, ca
 
 func ReadRulesMetadata(rulesPath string) ([]Rule, error) {
 	rules := make([]Rule, 0)
-	filepath.Walk(rulesPath, func(path string, info os.FileInfo, err error) error {
+	walkErr := filepath.Walk(rulesPath, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
 		if !info.IsDir() && !strings.HasSuffix(info.Name(), "_test.rego") && strings.HasSuffix(info.Name(), ".rego") {
 			rule, err := parseRuleMetadata_Rego(path)
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to parse rego rule metadata for %s: %w", path, err)
 			}
 			rules = append(rules, *rule)
 		}
 		if !info.IsDir() && !strings.HasSuffix(info.Name(), "_test.js") && strings.HasSuffix(info.Name(), ".js") {
 			rule, err := parseRuleMetadata_Javascript(path)
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to parse javascript rule metadata for %s: %w", path, err)
 			}
 			rules = append(rules, *rule)
 		}
 		if !info.IsDir() && !strings.HasSuffix(info.Name(), "_test.ts") && strings.HasSuffix(info.Name(), ".ts") {
 			rule, err := parseRuleMetadata_Typescript(path)
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to parse typescript rule metadata for %s: %w", path, err)
 			}
 			rules = append(rules, *rule)
 		}
 		return nil
 	})
+	if walkErr != nil {
+		return nil, walkErr
+	}
 	return rules, nil
 }

lint/lint_javascript.go

@@ -242,24 +242,43 @@ func parseRuleMetadata_Javascript(rulePath string) (*Rule, error) {
 	vm := sobek.New()
 	_, err = vm.RunString(string(ruleContent))
 	if err != nil {
-		panic(err)
+		return nil, fmt.Errorf("failed to evaluate javascript rule: %w", err)
 	}
-	// FIXME: handle the case where metadata is not defined correctly
+
 	metadata := vm.Get("metadata")
+	if metadata == nil || sobek.IsUndefined(metadata) || sobek.IsNull(metadata) {
+		return nil, fmt.Errorf("metadata object not defined")
+	}
 	metadataMap := metadata.ToObject(vm)
+	if metadataMap == nil {
+		return nil, fmt.Errorf("metadata must be an object")
+	}
 
 	var packageName string = rulePath
 	var title string = metadataMap.Get("title").String()
 	var description string = metadataMap.Get("description").String()
+	if strings.TrimSpace(title) == "" || strings.TrimSpace(description) == "" {
+		return nil, fmt.Errorf("metadata.title and metadata.description are required")
+	}
 
 	// custom metadata
-	custom := metadataMap.Get("custom").ToObject(vm)
+	customValue := metadataMap.Get("custom")
+	if customValue == nil || sobek.IsUndefined(customValue) || sobek.IsNull(customValue) {
+		return nil, fmt.Errorf("metadata.custom object is required")
+	}
+	custom := customValue.ToObject(vm)
+	if custom == nil {
+		return nil, fmt.Errorf("metadata.custom must be an object")
+	}
 	var category string = custom.Get("category").String()
 	var severity string = custom.Get("severity").String()
 	var ruleNumber string = custom.Get("rulenumber").String()
 	var remediation string = custom.Get("remediation").String()
 	var ruleName string = custom.Get("rulename").String()
 	var pattern string = custom.Get("input").String()
+	if strings.TrimSpace(ruleNumber) == "" {
+		return nil, fmt.Errorf("metadata.custom.rulenumber is required")
+	}
 
 	rule := &Rule{
 		Title:       title,

lint/lint_test.go

@@ -3,6 +3,7 @@ package lint
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -426,6 +427,21 @@ function rule(input) {
 			t.Errorf("Expected 1 rule (test file should be ignored), got %d", len(rules))
 		}
 	})
+
+	t.Run("returns error for javascript rule missing metadata", func(t *testing.T) {
+		tempDir := t.TempDir()
+		if err := os.WriteFile(filepath.Join(tempDir, "broken.js"), []byte("function rule(input) { return { allow: true, errors: [] }; }"), 0644); err != nil {
+			t.Fatalf("Failed to write broken js file: %v", err)
+		}
+
+		_, err := ReadRulesMetadata(tempDir)
+		if err == nil {
+			t.Fatal("Expected ReadRulesMetadata to fail for javascript rule without metadata")
+		}
+		if !strings.Contains(err.Error(), "metadata object not defined") {
+			t.Fatalf("Expected metadata error, got: %v", err)
+		}
+	})
 }
 
 func TestEvalAll(t *testing.T) {

lint/lint_typscript.go

@@ -179,24 +179,43 @@ func parseRuleMetadata_Typescript(rulePath string) (*Rule, error) {
 	vm := sobek.New()
 	_, err = vm.RunString(ruleContent)
 	if err != nil {
-		panic(err)
+		return nil, fmt.Errorf("failed to evaluate typescript rule: %w", err)
 	}
-	// FIXME: handle the case where metadata is not defined correctly
+
 	metadata := vm.Get("metadata")
+	if metadata == nil || sobek.IsUndefined(metadata) || sobek.IsNull(metadata) {
+		return nil, fmt.Errorf("metadata object not defined")
+	}
 	metadataMap := metadata.ToObject(vm)
+	if metadataMap == nil {
+		return nil, fmt.Errorf("metadata must be an object")
+	}
 
 	var packageName string = rulePath
 	var title string = metadataMap.Get("title").String()
 	var description string = metadataMap.Get("description").String()
+	if strings.TrimSpace(title) == "" || strings.TrimSpace(description) == "" {
+		return nil, fmt.Errorf("metadata.title and metadata.description are required")
+	}
 
 	// custom metadata
-	custom := metadataMap.Get("custom").ToObject(vm)
+	customValue := metadataMap.Get("custom")
+	if customValue == nil || sobek.IsUndefined(customValue) || sobek.IsNull(customValue) {
+		return nil, fmt.Errorf("metadata.custom object is required")
+	}
+	custom := customValue.ToObject(vm)
+	if custom == nil {
+		return nil, fmt.Errorf("metadata.custom must be an object")
+	}
 	var category string = custom.Get("category").String()
 	var severity string = custom.Get("severity").String()
 	var ruleNumber string = custom.Get("rulenumber").String()
 	var remediation string = custom.Get("remediation").String()
 	var ruleName string = custom.Get("rulename").String()
 	var pattern string = custom.Get("input").String()
+	if strings.TrimSpace(ruleNumber) == "" {
+		return nil, fmt.Errorf("metadata.custom.rulenumber is required")
+	}
 
 	rule := &Rule{
 		Title:       title,

lint/rulesets.go

@@ -168,6 +168,18 @@ func selectRulesRoot(root string) string {
 }
 
 func copyRulesFromPath(sourcePath string, targetPath string) error {
+	absSourcePath, err := filepath.Abs(sourcePath)
+	if err != nil {
+		return fmt.Errorf("failed to resolve source path %s: %w", sourcePath, err)
+	}
+	absTargetPath, err := filepath.Abs(targetPath)
+	if err != nil {
+		return fmt.Errorf("failed to resolve target path %s: %w", targetPath, err)
+	}
+	if filepath.Clean(absSourcePath) == filepath.Clean(absTargetPath) {
+		return nil
+	}
+
 	info, err := os.Stat(sourcePath)
 	if err != nil {
 		return fmt.Errorf("ruleset path %s not found: %w", sourcePath, err)
@@ -196,6 +208,18 @@ func copyRulesFromPath(sourcePath string, targetPath string) error {
 }
 
 func copyFile(src string, dst string) error {
+	absSrc, err := filepath.Abs(src)
+	if err != nil {
+		return err
+	}
+	absDst, err := filepath.Abs(dst)
+	if err != nil {
+		return err
+	}
+	if filepath.Clean(absSrc) == filepath.Clean(absDst) {
+		return nil
+	}
+
 	srcFile, err := os.Open(src)
 	if err != nil {
 		return err

lint/rulesets_test.go

@@ -0,0 +1,53 @@
+package lint
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestCopyRulesFromPath_SameSourceAndTarget_NoOp(t *testing.T) {
+	tempDir := t.TempDir()
+	rulesDir := filepath.Join(tempDir, "rules")
+	if err := os.MkdirAll(filepath.Join(rulesDir, "sample"), 0755); err != nil {
+		t.Fatalf("failed to create rules directory: %v", err)
+	}
+	rulePath := filepath.Join(rulesDir, "sample", "rule.js")
+	original := []byte("const metadata = { title: 'a', description: 'b', custom: { rulenumber: '001_0001' } };")
+	if err := os.WriteFile(rulePath, original, 0644); err != nil {
+		t.Fatalf("failed to write sample rule: %v", err)
+	}
+
+	if err := copyRulesFromPath(rulesDir, rulesDir); err != nil {
+		t.Fatalf("copyRulesFromPath should no-op for identical source/target: %v", err)
+	}
+
+	got, err := os.ReadFile(rulePath)
+	if err != nil {
+		t.Fatalf("failed to read sample rule after copy: %v", err)
+	}
+	if string(got) != string(original) {
+		t.Fatalf("expected rule content unchanged, got %q", string(got))
+	}
+}
+
+func TestCopyFile_SameSourceAndDestination_NoOp(t *testing.T) {
+	tempDir := t.TempDir()
+	filePath := filepath.Join(tempDir, "rule.rego")
+	original := []byte("# METADATA\n# title: x\n")
+	if err := os.WriteFile(filePath, original, 0644); err != nil {
+		t.Fatalf("failed to write source file: %v", err)
+	}
+
+	if err := copyFile(filePath, filePath); err != nil {
+		t.Fatalf("copyFile should no-op for identical source/destination: %v", err)
+	}
+
+	got, err := os.ReadFile(filePath)
+	if err != nil {
+		t.Fatalf("failed to read file after copy: %v", err)
+	}
+	if string(got) != string(original) {
+		t.Fatalf("expected file content unchanged, got %q", string(got))
+	}
+}