test(pnpm): add processLockfile consistency tests and stabilize traversal order

Author: iseki0

module/pnpm/process.go

@@ -4,15 +4,17 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"sort"
+
 	"github.com/murphysecurity/murphysec/infra/logctx"
 	"github.com/murphysecurity/murphysec/model"
 	"github.com/murphysecurity/murphysec/module/pnpm/shared"
 	v5 "github.com/murphysecurity/murphysec/module/pnpm/v5"
 	v6 "github.com/murphysecurity/murphysec/module/pnpm/v6"
 	v9 "github.com/murphysecurity/murphysec/module/pnpm/v9"
-	"io"
-	"os"
-	"path/filepath"
 )
 
 var EcoRepo = model.EcoRepo{
@@ -43,6 +45,11 @@ func processDir(ctx context.Context, dir string) (result processDirResult) {
 		result.e = fmt.Errorf("reading %s failed: %w", LockfileName, e)
 		return
 	}
+	processLockfile(ctx, data, &result)
+	return
+}
+
+func processLockfile(ctx context.Context, data []byte, result *processDirResult) {
 	version, e := parseLockfileVersion(data)
 	if e != nil {
 		result.e = fmt.Errorf("parse lockfile version failed, %w", e)
@@ -71,7 +78,34 @@ func processDir(ctx context.Context, dir string) (result processDirResult) {
 		result.e = fmt.Errorf("unsupported version \"%s\"", version)
 		return
 	}
-	return
+	normalizeTrees(result.trees)
+}
+
+func normalizeTrees(trees []shared.DepTree) {
+	for i := range trees {
+		sortDepItems(trees[i].Dependencies)
+	}
+	sort.SliceStable(trees, func(i, j int) bool {
+		return trees[i].Name < trees[j].Name
+	})
+}
+
+func sortDepItems(deps []model.DependencyItem) {
+	for i := range deps {
+		sortDepItems(deps[i].Dependencies)
+	}
+	sort.SliceStable(deps, func(i, j int) bool {
+		if deps[i].CompName != deps[j].CompName {
+			return deps[i].CompName < deps[j].CompName
+		}
+		if deps[i].CompVersion != deps[j].CompVersion {
+			return deps[i].CompVersion < deps[j].CompVersion
+		}
+		if deps[i].IsOnline.Value != deps[j].IsOnline.Value {
+			return !deps[i].IsOnline.Value && deps[j].IsOnline.Value
+		}
+		return deps[i].IsOnline.Valid && !deps[j].IsOnline.Valid
+	})
 }
 
 func processV5(ctx context.Context, data []byte) (trees []shared.DepTree, e error) {

module/pnpm/process_test.go

@@ -0,0 +1,49 @@
+package pnpm
+
+import (
+	"context"
+	"embed"
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+//go:embed v5/testdata/*.yaml v6/testdata/*.yaml v9/testdata/*.yaml
+var processTestdata embed.FS
+
+func TestProcessLockfile_ConsistentAcrossMultipleRuns(t *testing.T) {
+	testCases := []struct {
+		name string
+		path string
+	}{
+		{name: "v5-1", path: "v5/testdata/1.yaml"},
+		{name: "v5-5", path: "v5/testdata/5.yaml"},
+		{name: "v6-1", path: "v6/testdata/1.yaml"},
+		{name: "v6-3", path: "v6/testdata/3.yaml"},
+		{name: "v9-1", path: "v9/testdata/1.yaml"},
+		{name: "v9-9", path: "v9/testdata/9.yaml"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			data, e := processTestdata.ReadFile(tc.path)
+			assert.NoError(t, e)
+
+			var baseline []byte
+			for i := 0; i < 10; i++ {
+				var result processDirResult
+				processLockfile(context.Background(), data, &result)
+				assert.NoError(t, result.e)
+
+				got, e := json.Marshal(result.trees)
+				assert.NoError(t, e)
+				if i == 0 {
+					baseline = got
+					continue
+				}
+				assert.Equal(t, string(baseline), string(got), "process result changed at run %d", i+1)
+			}
+		})
+	}
+}

module/pnpm/v5/v5.go

@@ -3,6 +3,7 @@ package v5
 import (
 	"github.com/murphysecurity/murphysec/model"
 	"github.com/murphysecurity/murphysec/module/pnpm/shared"
+	"sort"
 	"strings"
 )
 
@@ -50,7 +51,13 @@ func nextCallVisitor[T any](l *Lockfile, parent *shared.GComponent, m map[string
 }
 
 func _visit[T any](l *Lockfile, parent *shared.GComponent, m map[string]string, cd *circleDetector, visitor shared.GVisitor[T], arg T) error {
-	for n, v := range m {
+	keys := make([]string, 0, len(m))
+	for n := range m {
+		keys = append(keys, n)
+	}
+	sort.Strings(keys)
+	for _, n := range keys {
+		v := m[n]
 		var pkg = l.findPkg(n, v)
 		if pkg == nil {
 			continue

module/pnpm/v5/v5_test.go

@@ -2,10 +2,11 @@ package v5
 
 import (
 	"encoding/json"
-	"github.com/murphysecurity/murphysec/utils/must"
-	"github.com/stretchr/testify/assert"
 	"io"
 	"testing"
+
+	"github.com/murphysecurity/murphysec/utils/must"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestLockfile(t *testing.T) {
@@ -33,3 +34,61 @@ func TestBuildDepTree(t *testing.T) {
 	assert.NotNil(t, tree)
 	t.Log(string(must.A(json.MarshalIndent(tree, "| ", " "))))
 }
+
+func TestParseLockfile_ConsistentAcrossMultipleRuns(t *testing.T) {
+	files, _ := testFiles.ReadDir("testdata")
+	assert.NotEmpty(t, files)
+	for _, s := range files {
+		t.Run(s.Name(), func(t *testing.T) {
+			f, e := testFiles.Open("testdata/" + s.Name())
+			assert.NoError(t, e)
+			defer func() { assert.NoError(t, f.Close()) }()
+			data, e := io.ReadAll(f)
+			assert.NoError(t, e)
+
+			var baseline []byte
+			for i := 0; i < 10; i++ {
+				lockfile, e := ParseLockfile(data)
+				assert.NoError(t, e)
+				assert.NotNil(t, lockfile)
+				got, e := json.Marshal(lockfile)
+				assert.NoError(t, e)
+				if i == 0 {
+					baseline = got
+					continue
+				}
+				assert.Equal(t, string(baseline), string(got), "parse result changed at run %d", i+1)
+			}
+		})
+	}
+}
+
+func TestBuildDepTree_AllTestdata(t *testing.T) {
+	files, _ := testFiles.ReadDir("testdata")
+	assert.NotEmpty(t, files)
+	for _, s := range files {
+		t.Run(s.Name(), func(t *testing.T) {
+			f, e := testFiles.Open("testdata/" + s.Name())
+			assert.NoError(t, e)
+			defer func() { assert.NoError(t, f.Close()) }()
+			data, e := io.ReadAll(f)
+			assert.NoError(t, e)
+
+			lockfile, e := ParseLockfile(data)
+			assert.NoError(t, e)
+			assert.NotNil(t, lockfile)
+
+			tree := BuildDepTree(lockfile, nil, "")
+			if len(lockfile.Dependencies) > 0 || len(lockfile.DevDependencies) > 0 {
+				assert.NotNil(t, tree)
+			}
+
+			for importerName, importer := range lockfile.Importers {
+				importerTree := BuildDepTree(lockfile, importer, importerName)
+				if importerTree != nil {
+					assert.Equal(t, importerName, importerTree.Name)
+				}
+			}
+		})
+	}
+}

module/pnpm/v6/parser.go

@@ -6,6 +6,7 @@ import (
 	"github.com/murphysecurity/murphysec/model"
 	"github.com/murphysecurity/murphysec/module/pnpm/shared"
 	"iter"
+	"sort"
 	"strings"
 )
 
@@ -59,7 +60,13 @@ func Process(_ctx context.Context, data []byte, strict bool) ([]shared.DepTree,
 			Prune:    make(map[[2]string]struct{}),
 			Dev:      dev,
 		}
-		for name, it := range m {
+		keys := make([]string, 0, len(m))
+		for name := range m {
+			keys = append(keys, name)
+		}
+		sort.Strings(keys)
+		for _, name := range keys {
+			it := m[name]
 			var dep model.DependencyItem
 			e := _visit(ctx, name, it.Version, "", &dep)
 			if e != nil {
@@ -82,7 +89,13 @@ func Process(_ctx context.Context, data []byte, strict bool) ([]shared.DepTree,
 		return []shared.DepTree{root}, nil
 	} else {
 		var r []shared.DepTree
-		for relPath, importer := range lockfile.Importers {
+		importerKeys := make([]string, 0, len(lockfile.Importers))
+		for relPath := range lockfile.Importers {
+			importerKeys = append(importerKeys, relPath)
+		}
+		sort.Strings(importerKeys)
+		for _, relPath := range importerKeys {
+			importer := lockfile.Importers[relPath]
 			var tree shared.DepTree
 			tree.Name = relPath
 			if e := f(importer.Dependencies, false, &tree); e != nil {
@@ -126,7 +139,13 @@ func _visit(ctx *visitContext, name, version, path string, node *model.Dependenc
 		}
 		if !cDetected {
 			var f = func(m map[string]string) error {
-				for n, v := range m {
+				keys := make([]string, 0, len(m))
+				for n := range m {
+					keys = append(keys, n)
+				}
+				sort.Strings(keys)
+				for _, n := range keys {
+					v := m[n]
 					node.Dependencies = append(node.Dependencies, model.DependencyItem{})
 					var key = key + "/" + n + "/" + v
 					var e = _visit(ctx, n, v, key, &node.Dependencies[len(node.Dependencies)-1])

module/pnpm/v9/parse.go

@@ -38,7 +38,13 @@ func Parse(ctx context.Context, reader io.Reader) (trees []shared.DepTree, e err
 	if e != nil {
 		return
 	}
-	for path, importer := range doc.Importers {
+	importerPaths := make([]string, 0, len(doc.Importers))
+	for path := range doc.Importers {
+		importerPaths = append(importerPaths, path)
+	}
+	sort.Strings(importerPaths)
+	for _, path := range importerPaths {
+		importer := doc.Importers[path]
 		var c = importerHandlingCtx{
 			Logger:            logctx.Use(ctx).Sugar(),
 			Snapshot:          doc.Snapshots,
@@ -47,7 +53,13 @@ func Parse(ctx context.Context, reader io.Reader) (trees []shared.DepTree, e err
 			CircularPath:      make([][2]string, 0),
 		}
 		var deps []model.DependencyItem
-		for name, obj := range importer.Dependencies {
+		depNames := make([]string, 0, len(importer.Dependencies))
+		for name := range importer.Dependencies {
+			depNames = append(depNames, name)
+		}
+		sort.Strings(depNames)
+		for _, name := range depNames {
+			obj := importer.Dependencies[name]
 			var r model.DependencyItem
 			r, e = c.handle(name, obj.Version, true)
 			if e != nil {
@@ -58,7 +70,13 @@ func Parse(ctx context.Context, reader io.Reader) (trees []shared.DepTree, e err
 		c.Handled = make(map[[2]string]struct{})
 		c.CircularDetectMap = make(map[[2]string]struct{})
 		c.CircularPath = make([][2]string, 0)
-		for name, obj := range importer.DevDependencies {
+		devDepNames := make([]string, 0, len(importer.DevDependencies))
+		for name := range importer.DevDependencies {
+			devDepNames = append(devDepNames, name)
+		}
+		sort.Strings(devDepNames)
+		for _, name := range devDepNames {
+			obj := importer.DevDependencies[name]
 			var r model.DependencyItem
 			r, e = c.handle(name, obj.Version, false)
 			if e != nil {