fix(tvix/nar-bridge): Remove name check for root node in nar generation r/8711

Nar-bridge tried to parse the name of the protobuf node encoded in the
URL into a PathComponent but this name was empty, leading to an error
when the user tried to retrieve the nar file.

This was an oversight from the conversion to stricter types (some of the
CLs in the serious containing cl/12217).

We need a version converting a protobuf without a name to our stricter
types, but an empty PathComponent cannot be constructed.

So we need a into_name_and_node() version that returns the name as
Bytes, not PathComponent.

Change-Id: I2996cdd2e0107133e502748947298f512f1cc521
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12504
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI

4 files changed, 27 insertions, 16 deletions

diff --git a/tvix/castore/src/errors.rs b/tvix/castore/src/errors.rs
index 7b5d1a422c99..bf8fd236dbef 100644
--- a/tvix/castore/src/errors.rs
+++ b/tvix/castore/src/errors.rs
@@ -45,8 +45,8 @@ pub enum DirectoryError {
     #[error("{:?} is a duplicate name", .0)]
     DuplicateName(PathComponent),
     /// Node failed validation
-    #[error("invalid node with name {}: {:?}", .0, .1.to_string())]
-    InvalidNode(PathComponent, ValidateNodeError),
+    #[error("invalid node with name {}: {:?}", .0.as_bstr(), .1.to_string())]
+    InvalidNode(bytes::Bytes, ValidateNodeError),
     #[error("Total size exceeds u64::MAX")]
     SizeOverflow,
     /// Invalid name encountered
diff --git a/tvix/castore/src/proto/mod.rs b/tvix/castore/src/proto/mod.rs
index 8bc74b412676..15e55ad049cf 100644
--- a/tvix/castore/src/proto/mod.rs
+++ b/tvix/castore/src/proto/mod.rs
@@ -1,4 +1,5 @@
 use prost::Message;
+
 use std::cmp::Ordering;
 
 mod grpc_blobservice_wrapper;
@@ -192,23 +193,35 @@ impl From<crate::Directory> for Directory {
 impl Node {
     /// Converts a proto [Node] to a [crate::Node], and splits off the name.
     pub fn into_name_and_node(self) -> Result<(PathComponent, crate::Node), DirectoryError> {
+        let (unvalidated_name, node) = self.into_name_bytes_and_node()?;
+        Ok((
+            unvalidated_name
+                .try_into()
+                .map_err(DirectoryError::InvalidName)?,
+            node,
+        ))
+    }
+
+    /// Converts a proto [Node] to a [crate::Node], and splits off the name and returns it as a
+    /// [bytes::Bytes].
+    ///
+    /// Note: the returned name is not validated.
+    pub fn into_name_bytes_and_node(self) -> Result<(bytes::Bytes, crate::Node), DirectoryError> {
         match self.node.ok_or_else(|| DirectoryError::NoNodeSet)? {
             node::Node::Directory(n) => {
-                let name: PathComponent = n.name.try_into().map_err(DirectoryError::InvalidName)?;
                 let digest = B3Digest::try_from(n.digest)
-                    .map_err(|e| DirectoryError::InvalidNode(name.clone(), e.into()))?;
+                    .map_err(|e| DirectoryError::InvalidNode(n.name.clone(), e.into()))?;
 
                 let node = crate::Node::Directory {
                     digest,
                     size: n.size,
                 };
 
-                Ok((name, node))
+                Ok((n.name, node))
             }
             node::Node::File(n) => {
-                let name: PathComponent = n.name.try_into().map_err(DirectoryError::InvalidName)?;
                 let digest = B3Digest::try_from(n.digest)
-                    .map_err(|e| DirectoryError::InvalidNode(name.clone(), e.into()))?;
+                    .map_err(|e| DirectoryError::InvalidNode(n.name.clone(), e.into()))?;
 
                 let node = crate::Node::File {
                     digest,
@@ -216,27 +229,25 @@ impl Node {
                     executable: n.executable,
                 };
 
-                Ok((name, node))
+                Ok((n.name, node))
             }
 
             node::Node::Symlink(n) => {
-                let name: PathComponent = n.name.try_into().map_err(DirectoryError::InvalidName)?;
-
                 let node = crate::Node::Symlink {
                     target: n.target.try_into().map_err(|e| {
                         DirectoryError::InvalidNode(
-                            name.clone(),
+                            n.name.clone(),
                             crate::ValidateNodeError::InvalidSymlinkTarget(e),
                         )
                     })?,
                 };
 
-                Ok((name, node))
+                Ok((n.name, node))
             }
         }
     }
 
-    /// Construsts a [Node] from a name and [crate::Node].
+    /// Constructs a [Node] from a name and [crate::Node].
     /// The name is a [bytes::Bytes], not a [PathComponent], as we have use an
     /// empty name in some places.
     pub fn from_name_and_node(name: bytes::Bytes, n: crate::Node) -> Self {
diff --git a/tvix/nar-bridge/src/nar.rs b/tvix/nar-bridge/src/nar.rs
index 70d9d644c26c..2f6c0cc5a0ef 100644
--- a/tvix/nar-bridge/src/nar.rs
+++ b/tvix/nar-bridge/src/nar.rs
@@ -52,7 +52,7 @@ pub async fn get(
             StatusCode::NOT_FOUND
         })?;
 
-    let (root_name, root_node) = root_node.into_name_and_node().map_err(|e| {
+    let (root_name, root_node) = root_node.into_name_bytes_and_node().map_err(|e| {
         warn!(err=%e, "root node validation failed");
         StatusCode::BAD_REQUEST
     })?;
diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs
index eb47e592be8c..f5ecc798bbfb 100644
--- a/tvix/store/src/proto/tests/pathinfo.rs
+++ b/tvix/store/src/proto/tests/pathinfo.rs
@@ -35,7 +35,7 @@ fn validate_pathinfo(
         name: DUMMY_PATH.into(),
         digest: Bytes::new(),
         size: 0,
-}, Err(ValidatePathInfoError::InvalidRootNode(DirectoryError::InvalidNode(DUMMY_PATH.try_into().unwrap(), ValidateNodeError::InvalidDigestLen(0)))))]
+}, Err(ValidatePathInfoError::InvalidRootNode(DirectoryError::InvalidNode(DUMMY_PATH.into(), ValidateNodeError::InvalidDigestLen(0)))))]
 #[case::invalid_node_name_no_storepath(castorepb::DirectoryNode {
         name: "invalid".into(),
         digest: DUMMY_DIGEST.clone().into(),
@@ -74,7 +74,7 @@ fn validate_directory(
         digest: Bytes::new(),
         ..Default::default()
     },
-    Err(ValidatePathInfoError::InvalidRootNode(DirectoryError::InvalidNode(DUMMY_PATH.try_into().unwrap(), ValidateNodeError::InvalidDigestLen(0))))
+    Err(ValidatePathInfoError::InvalidRootNode(DirectoryError::InvalidNode(DUMMY_PATH.into(), ValidateNodeError::InvalidDigestLen(0))))
 )]
 #[case::invalid_node_name(
     castorepb::FileNode {