feat(tvix/castore/protos): add more granular validation methods r/6799

Similar to cl/9715, this makes the validation checks more granular,
introducing a Validate on all *Node.

A check for symlink targets is added too.

Once merged, it can also be used from tvix/store/protos.

Change-Id: I0909a89fadcd74b74ef0c9a8a1f22658fccc83b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9716
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI

2 files changed, 104 insertions, 29 deletions

diff --git a/tvix/castore/protos/castore.go b/tvix/castore/protos/castore.go
index 102ba4bff75d..c9e3757885a0 100644
--- a/tvix/castore/protos/castore.go
+++ b/tvix/castore/protos/castore.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/base64"
 	"fmt"
+
 	"google.golang.org/protobuf/proto"
 	"lukechampine.com/blake3"
 )
@@ -50,6 +51,68 @@ func isValidName(n []byte) bool {
 	return true
 }
 
+// Validate ensures a DirectoryNode has a valid name and correct digest len.
+func (n *DirectoryNode) Validate() error {
+	if len(n.Digest) != 32 {
+		return fmt.Errorf("invalid digest length for %s, expected %d, got %d", n.Name, 32, len(n.Digest))
+	}
+
+	if !isValidName(n.Name) {
+		return fmt.Errorf("invalid node name: %s", n.Name)
+	}
+
+	return nil
+}
+
+// Validate ensures a FileNode has a valid name and correct digest len.
+func (n *FileNode) Validate() error {
+	if len(n.Digest) != 32 {
+		return fmt.Errorf("invalid digest length for %s, expected %d, got %d", n.Name, 32, len(n.Digest))
+	}
+
+	if !isValidName(n.Name) {
+		return fmt.Errorf("invalid node name: %s", n.Name)
+	}
+
+	return nil
+}
+
+// Validate ensures a SymlinkNode has a valid name and target.
+func (n *SymlinkNode) Validate() error {
+	if len(n.Target) == 0 || bytes.Contains(n.Target, []byte{0}) {
+		return fmt.Errorf("invalid symlink target: %s", n.Target)
+	}
+
+	if !isValidName(n.Name) {
+		return fmt.Errorf("invalid node name: %s", n.Name)
+	}
+
+	return nil
+}
+
+// Validate ensures a node is valid, by dispatching to the per-type validation functions.
+func (n *Node) Validate() error {
+	if node := n.GetDirectory(); node != nil {
+		if err := node.Validate(); err != nil {
+			return fmt.Errorf("SymlinkNode failed validation: %w", err)
+		}
+	} else if node := n.GetFile(); node != nil {
+		if err := node.Validate(); err != nil {
+			return fmt.Errorf("FileNode failed validation: %w", err)
+		}
+	} else if node := n.GetSymlink(); node != nil {
+		if err := node.Validate(); err != nil {
+			return fmt.Errorf("SymlinkNode failed validation: %w", err)
+		}
+
+	} else {
+		// this would only happen if we introduced a new type
+		return fmt.Errorf("no specific node found")
+	}
+
+	return nil
+}
+
 // Validate thecks the Directory message for invalid data, such as:
 // - violations of name restrictions
 // - invalid digest lengths
@@ -87,21 +150,14 @@ func (d *Directory) Validate() error {
 		return nil
 	}
 
-	// Loop over all Directories, Files and Symlinks individually.
-	// Check the name for validity, check a potential digest for length,
-	// then check for sorting in the current list, and uniqueness across all three lists.
+	// Loop over all Directories, Files and Symlinks individually,
+	// check them for validity, then check for sorting in the current list, and
+	// uniqueness across all three lists.
 	for _, directoryNode := range d.Directories {
 		directoryName := directoryNode.GetName()
 
-		// check name for validity
-		if !isValidName(directoryName) {
-			return fmt.Errorf("invalid name for DirectoryNode: %v", directoryName)
-		}
-
-		// check digest to be 32 bytes
-		digestLen := len(directoryNode.GetDigest())
-		if digestLen != 32 {
-			return fmt.Errorf("invalid digest length for DirectoryNode: %d", digestLen)
+		if err := directoryNode.Validate(); err != nil {
+			return fmt.Errorf("DirectoryNode %s failed validation: %w", directoryName, err)
 		}
 
 		// ensure names are sorted
@@ -119,15 +175,8 @@ func (d *Directory) Validate() error {
 	for _, fileNode := range d.Files {
 		fileName := fileNode.GetName()
 
-		// check name for validity
-		if !isValidName(fileName) {
-			return fmt.Errorf("invalid name for FileNode: %v", fileName)
-		}
-
-		// check digest to be 32 bytes
-		digestLen := len(fileNode.GetDigest())
-		if digestLen != 32 {
-			return fmt.Errorf("invalid digest length for FileNode: %d", digestLen)
+		if err := fileNode.Validate(); err != nil {
+			return fmt.Errorf("FileNode %s failed validation: %w", fileName, err)
 		}
 
 		// ensure names are sorted
@@ -144,9 +193,8 @@ func (d *Directory) Validate() error {
 	for _, symlinkNode := range d.Symlinks {
 		symlinkName := symlinkNode.GetName()
 
-		// check name for validity
-		if !isValidName(symlinkName) {
-			return fmt.Errorf("invalid name for SymlinkNode: %v", symlinkName)
+		if err := symlinkNode.Validate(); err != nil {
+			return fmt.Errorf("SymlinkNode %s failed validation: %w", symlinkName, err)
 		}
 
 		// ensure names are sorted
diff --git a/tvix/castore/protos/castore_test.go b/tvix/castore/protos/castore_test.go
index 958d399d76cc..fda87a6cfb66 100644
--- a/tvix/castore/protos/castore_test.go
+++ b/tvix/castore/protos/castore_test.go
@@ -122,7 +122,7 @@ func TestDirectoryValidate(t *testing.T) {
 				Symlinks: []*castorev1pb.SymlinkNode{},
 			}
 
-			assert.ErrorContains(t, d.Validate(), "invalid name")
+			assert.ErrorContains(t, d.Validate(), "invalid node name")
 		}
 		{
 			d := castorev1pb.Directory{
@@ -135,7 +135,7 @@ func TestDirectoryValidate(t *testing.T) {
 				Symlinks: []*castorev1pb.SymlinkNode{},
 			}
 
-			assert.ErrorContains(t, d.Validate(), "invalid name")
+			assert.ErrorContains(t, d.Validate(), "invalid node name")
 		}
 		{
 			d := castorev1pb.Directory{
@@ -149,7 +149,7 @@ func TestDirectoryValidate(t *testing.T) {
 				Symlinks: []*castorev1pb.SymlinkNode{},
 			}
 
-			assert.ErrorContains(t, d.Validate(), "invalid name")
+			assert.ErrorContains(t, d.Validate(), "invalid node name")
 		}
 		{
 			d := castorev1pb.Directory{
@@ -161,7 +161,7 @@ func TestDirectoryValidate(t *testing.T) {
 				}},
 			}
 
-			assert.ErrorContains(t, d.Validate(), "invalid name")
+			assert.ErrorContains(t, d.Validate(), "invalid node name")
 		}
 		{
 			d := castorev1pb.Directory{
@@ -173,7 +173,7 @@ func TestDirectoryValidate(t *testing.T) {
 				}},
 			}
 
-			assert.ErrorContains(t, d.Validate(), "invalid name")
+			assert.ErrorContains(t, d.Validate(), "invalid node name")
 		}
 	})
 
@@ -191,6 +191,33 @@ func TestDirectoryValidate(t *testing.T) {
 		assert.ErrorContains(t, d.Validate(), "invalid digest length")
 	})
 
+	t.Run("invalid symlink targets", func(t *testing.T) {
+		{
+			d := castorev1pb.Directory{
+				Directories: []*castorev1pb.DirectoryNode{},
+				Files:       []*castorev1pb.FileNode{},
+				Symlinks: []*castorev1pb.SymlinkNode{{
+					Name:   []byte("foo"),
+					Target: []byte{},
+				}},
+			}
+
+			assert.ErrorContains(t, d.Validate(), "invalid symlink target")
+		}
+		{
+			d := castorev1pb.Directory{
+				Directories: []*castorev1pb.DirectoryNode{},
+				Files:       []*castorev1pb.FileNode{},
+				Symlinks: []*castorev1pb.SymlinkNode{{
+					Name:   []byte("foo"),
+					Target: []byte{0x66, 0x6f, 0x6f, 0},
+				}},
+			}
+
+			assert.ErrorContains(t, d.Validate(), "invalid symlink target")
+		}
+	})
+
 	t.Run("sorting", func(t *testing.T) {
 		// "b" comes before "a", bad.
 		{