From 0b56d9f21bb5a857ee17ecc539c173926441a3fb Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Thu, 29 Dec 2022 21:37:52 +0100 Subject: feat(src/proto): add PathInfo.validate() This provides validation of PathInfo messages, and ensures the output hashes are properly parsed from the root node names. NixPath already has a more extensive test suite for various wrong NixPaths, so it's omitted from here. Change-Id: I5d69118df5816daabb521ddb19d178bddd1caacf Reviewed-on: https://cl.tvl.fyi/c/depot/+/7684 Reviewed-by: tazjin Tested-by: BuildkiteCI --- tvix/store/src/proto.rs | 99 +++++++++++++++++++ tvix/store/src/tests/mod.rs | 2 + tvix/store/src/tests/pathinfo.rs | 206 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 307 insertions(+) create mode 100644 tvix/store/src/tests/pathinfo.rs (limited to 'tvix/store/src') diff --git a/tvix/store/src/proto.rs b/tvix/store/src/proto.rs index 75e5c34ab1..f2af69ab0d 100644 --- a/tvix/store/src/proto.rs +++ b/tvix/store/src/proto.rs @@ -5,6 +5,8 @@ use thiserror::Error; use prost::Message; +use crate::nixpath::{NixPath, ParseNixPathError}; + tonic::include_proto!("tvix.store.v1"); #[cfg(feature = "reflection")] @@ -30,6 +32,27 @@ pub enum ValidateDirectoryError { InvalidDigestLen(usize), } +/// Errors that can occur during the validation of PathInfo messages. +#[derive(Debug, Error, PartialEq)] +pub enum ValidatePathInfoError { + /// No node present + #[error("No node present")] + NoNodePresent(), + + /// Invalid node name encountered. + #[error("{0} is an invalid node name: {1}")] + InvalidNodeName(String, ParseNixPathError), + + /// The digest the (root) node refers to has invalid length. + #[error("Invalid Digest length: {0}")] + InvalidDigestLen(usize), + + /// The number of references in the narinfo.reference_names field does not match + /// the number of references in the .references field. + #[error("Inconsistent Number of References: {0} (references) vs {0} (narinfo)")] + InconsistentNumberOfReferences(usize, usize), +} + /// Checks a Node name for validity as an intermediate node, and returns an /// error that's generated from the supplied constructor. /// @@ -51,6 +74,82 @@ fn validate_digest(digest: &Vec, err: fn(usize) -> E) -> Result<(), E> { Ok(()) } +/// Parses a root node name. +/// +/// On success, this returns the parsed [NixPath]. +/// On error, it returns an error generated from the supplied constructor. +fn parse_node_name_root( + name: &str, + err: fn(String, ParseNixPathError) -> E, +) -> Result { + match NixPath::from_string(name) { + Ok(np) => Ok(np), + Err(e) => Err(err(name.to_string(), e)), + } +} + +impl PathInfo { + /// validate performs some checks on the PathInfo struct, + /// Returning either a [NixPath] of the root node, or a + /// [ValidatePathInfoError]. + pub fn validate(&self) -> Result { + // If there is a narinfo field populated, ensure the number of references there + // matches PathInfo.references count. + if let Some(narinfo) = &self.narinfo { + if narinfo.reference_names.len() != self.references.len() { + return Err(ValidatePathInfoError::InconsistentNumberOfReferences( + narinfo.reference_names.len(), + self.references.len(), + )); + } + } + // FUTUREWORK: parse references in reference_names. ensure they start + // with storeDir, and use the same digest as in self.references. + + // Ensure there is a (root) node present, and it properly parses to a NixPath. + let root_nix_path = match &self.node { + None => { + return Err(ValidatePathInfoError::NoNodePresent()); + } + Some(Node { node }) => match node { + None => { + return Err(ValidatePathInfoError::NoNodePresent()); + } + Some(node::Node::Directory(directory_node)) => { + // ensure the digest has the appropriate size. + validate_digest( + &directory_node.digest, + ValidatePathInfoError::InvalidDigestLen, + )?; + + // parse the name + parse_node_name_root( + &directory_node.name, + ValidatePathInfoError::InvalidNodeName, + )? + } + Some(node::Node::File(file_node)) => { + // ensure the digest has the appropriate size. + validate_digest(&file_node.digest, ValidatePathInfoError::InvalidDigestLen)?; + + // parse the name + parse_node_name_root(&file_node.name, ValidatePathInfoError::InvalidNodeName)? + } + Some(node::Node::Symlink(symlink_node)) => { + // parse the name + parse_node_name_root( + &symlink_node.name, + ValidatePathInfoError::InvalidNodeName, + )? + } + }, + }; + + // return the root nix path + Ok(root_nix_path) + } +} + /// Accepts a name, and a mutable reference to the previous name. /// If the passed name is larger than the previous one, the reference is updated. /// If it's not, an error is returned. diff --git a/tvix/store/src/tests/mod.rs b/tvix/store/src/tests/mod.rs index 366f96287a..b5fb0648e6 100644 --- a/tvix/store/src/tests/mod.rs +++ b/tvix/store/src/tests/mod.rs @@ -1,6 +1,8 @@ use crate::proto::{Directory, DirectoryNode, FileNode, SymlinkNode, ValidateDirectoryError}; use lazy_static::lazy_static; +mod pathinfo; + lazy_static! { static ref DUMMY_DIGEST: Vec = vec![ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, diff --git a/tvix/store/src/tests/pathinfo.rs b/tvix/store/src/tests/pathinfo.rs new file mode 100644 index 0000000000..8f4e9d3990 --- /dev/null +++ b/tvix/store/src/tests/pathinfo.rs @@ -0,0 +1,206 @@ +use crate::{ + nixpath::{NixPath, ParseNixPathError}, + proto::{self, Node, PathInfo, ValidatePathInfoError}, +}; +use lazy_static::lazy_static; +use test_case::test_case; + +lazy_static! { + static ref DUMMY_DIGEST: Vec = vec![ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, + ]; + static ref DUMMY_DIGEST_2: Vec = vec![ + 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, + ]; +} + +const DUMMY_NAME: &str = "00000000000000000000000000000000-dummy"; + +#[test_case( + None, + Err(ValidatePathInfoError::NoNodePresent()) ; + "No node" +)] +#[test_case( + Some(Node { node: None }), + Err(ValidatePathInfoError::NoNodePresent()); + "No node 2" +)] +fn validate_no_node(t_node: Option, t_result: Result) { + // construct the PathInfo object + let p = PathInfo { + node: t_node, + ..Default::default() + }; + assert_eq!(t_result, p.validate()); +} + +#[test_case( + proto::DirectoryNode { + name: DUMMY_NAME.to_string(), + digest: DUMMY_DIGEST.to_vec(), + size: 0, + }, + Ok(NixPath::from_string(DUMMY_NAME).expect("must succeed")); + "ok" +)] +#[test_case( + proto::DirectoryNode { + name: DUMMY_NAME.to_string(), + digest: vec![], + size: 0, + }, + Err(ValidatePathInfoError::InvalidDigestLen(0)); + "invalid digest length" +)] +#[test_case( + proto::DirectoryNode { + name: "invalid".to_string(), + digest: DUMMY_DIGEST.to_vec(), + size: 0, + }, + Err(ValidatePathInfoError::InvalidNodeName( + "invalid".to_string(), + ParseNixPathError::InvalidName("".to_string()) + )); + "invalid node name" +)] +fn validate_directory( + t_directory_node: proto::DirectoryNode, + t_result: Result, +) { + // construct the PathInfo object + let p = PathInfo { + node: Some(Node { + node: Some(proto::node::Node::Directory(t_directory_node)), + }), + ..Default::default() + }; + assert_eq!(t_result, p.validate()); +} + +#[test_case( + proto::FileNode { + name: DUMMY_NAME.to_string(), + digest: DUMMY_DIGEST.to_vec(), + size: 0, + executable: false, + }, + Ok(NixPath::from_string(DUMMY_NAME).expect("must succeed")); + "ok" +)] +#[test_case( + proto::FileNode { + name: DUMMY_NAME.to_string(), + digest: vec![], + ..Default::default() + }, + Err(ValidatePathInfoError::InvalidDigestLen(0)); + "invalid digest length" +)] +#[test_case( + proto::FileNode { + name: "invalid".to_string(), + digest: DUMMY_DIGEST.to_vec(), + ..Default::default() + }, + Err(ValidatePathInfoError::InvalidNodeName( + "invalid".to_string(), + ParseNixPathError::InvalidName("".to_string()) + )); + "invalid node name" +)] +fn validate_file(t_file_node: proto::FileNode, t_result: Result) { + // construct the PathInfo object + let p = PathInfo { + node: Some(Node { + node: Some(proto::node::Node::File(t_file_node)), + }), + ..Default::default() + }; + assert_eq!(t_result, p.validate()); +} + +#[test_case( + proto::SymlinkNode { + name: DUMMY_NAME.to_string(), + ..Default::default() + }, + Ok(NixPath::from_string(DUMMY_NAME).expect("must succeed")); + "ok" +)] +#[test_case( + proto::SymlinkNode { + name: "invalid".to_string(), + ..Default::default() + }, + Err(ValidatePathInfoError::InvalidNodeName( + "invalid".to_string(), + ParseNixPathError::InvalidName("".to_string()) + )); + "invalid node name" +)] +fn validate_symlink( + t_symlink_node: proto::SymlinkNode, + t_result: Result, +) { + // construct the PathInfo object + let p = PathInfo { + node: Some(Node { + node: Some(proto::node::Node::Symlink(t_symlink_node)), + }), + ..Default::default() + }; + assert_eq!(t_result, p.validate()); +} + +#[test] +fn validate_references() { + // create a PathInfo without narinfo field. + let path_info = PathInfo { + node: Some(Node { + node: Some(proto::node::Node::Directory(proto::DirectoryNode { + name: DUMMY_NAME.to_string(), + digest: DUMMY_DIGEST.to_vec(), + size: 0, + })), + }), + references: vec![DUMMY_DIGEST_2.to_vec()], + narinfo: None, + }; + assert!(path_info.validate().is_ok()); + + // create a PathInfo with a narinfo field, but an inconsistent set of references + let path_info_with_narinfo_missing_refs = PathInfo { + narinfo: Some(proto::NarInfo { + nar_size: 0, + nar_sha256: DUMMY_DIGEST.to_vec(), + signatures: vec![], + reference_names: vec![], + }), + ..path_info.clone() + }; + match path_info_with_narinfo_missing_refs + .validate() + .expect_err("must_fail") + { + ValidatePathInfoError::InconsistentNumberOfReferences(_, _) => {} + _ => panic!("unexpected error"), + }; + + // create a pathinfo with the correct number of references, should suceed + let path_info_with_narinfo = PathInfo { + narinfo: Some(proto::NarInfo { + nar_size: 0, + nar_sha256: DUMMY_DIGEST.to_vec(), + signatures: vec![], + reference_names: vec![format!("/nix/store/{}", DUMMY_NAME)], + }), + ..path_info.clone() + }; + assert!(path_info_with_narinfo.validate().is_ok()); +} -- cgit 1.4.1