From 99d5cf822a969c5de54639e46f9be917b7cd6119 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Wed, 1 Feb 2023 00:30:22 +0100 Subject: refactor(tvix/cli): use nixhash module for output hash calculation This covers all the weird corner cases. Change-Id: I85637e82e8929828064ab562dc8a1c8bf161fffa Reviewed-on: https://cl.tvl.fyi/c/depot/+/7991 Tested-by: BuildkiteCI Reviewed-by: tazjin --- tvix/cli/src/derivation.rs | 77 ++++++++++------------------------------------ tvix/cli/src/errors.rs | 16 +++------- 2 files changed, 20 insertions(+), 73 deletions(-) (limited to 'tvix/cli/src') diff --git a/tvix/cli/src/derivation.rs b/tvix/cli/src/derivation.rs index 4fdc0b6cf7fc..fce9a628598d 100644 --- a/tvix/cli/src/derivation.rs +++ b/tvix/cli/src/derivation.rs @@ -1,8 +1,6 @@ //! Implements `builtins.derivation`, the core of what makes Nix build packages. - -use data_encoding::BASE64; use nix_compat::derivation::{Derivation, Hash}; -use nix_compat::hash_placeholder; +use nix_compat::{hash_placeholder, nixhash}; use std::cell::RefCell; use std::collections::{btree_map, BTreeSet}; use std::rc::Rc; @@ -109,75 +107,32 @@ fn populate_output_configuration( ) -> Result<(), ErrorKind> { // We only do something when `digest` and `algo` are `Some(_)``, and // there's an `out` output. - if let (Some(digest), Some(algo), hash_mode) = (hash, hash_algo, hash_mode) { + if let (Some(hash), Some(algo), hash_mode) = (hash, hash_algo, hash_mode) { match drv.outputs.get_mut("out") { None => return Err(Error::ConflictingOutputTypes.into()), Some(out) => { - let sri_parsed = digest.parse::(); - // SRI strings can embed multiple hashes with different algos, but that's probably not supported - - let (digest, algo): (String, String) = match sri_parsed { - Err(e) => { - // unable to parse as SRI, but algo not set - if algo.is_empty() { - // InvalidSRIString doesn't implement PartialEq, but our error does - return Err(Error::InvalidSRIString(e.to_string()).into()); - } - - // algo is set. Assume the digest is set to some nixbase32. - // TODO: more validation here - - (digest, algo) - } - Ok(sri_parsed) => { - // We don't support more than one SRI hash - if sri_parsed.hashes.len() != 1 { - return Err( - Error::UnsupportedSRIMultiple(sri_parsed.hashes.len()).into() - ); - } - - // grab the first (and only hash) - let sri_parsed_hash = &sri_parsed.hashes[0]; - - // ensure the algorithm in the SRI is supported - if !(sri_parsed_hash.algorithm == ssri::Algorithm::Sha1 - || sri_parsed_hash.algorithm == ssri::Algorithm::Sha256 - || sri_parsed_hash.algorithm == ssri::Algorithm::Sha512) - { - Error::UnsupportedSRIAlgo(sri_parsed_hash.algorithm.to_string()); - } - - // if algo is set, it needs to match what the SRI says - if !algo.is_empty() && algo != sri_parsed_hash.algorithm.to_string() { - return Err(Error::ConflictingSRIHashAlgo( - algo, - sri_parsed_hash.algorithm.to_string(), - ) - .into()); - } - - // the digest comes base64-encoded. We need to decode, and re-encode as hexlower. - match BASE64.decode(sri_parsed_hash.digest.as_bytes()) { - Err(e) => return Err(Error::InvalidSRIDigest(e).into()), - Ok(sri_digest) => ( - data_encoding::HEXLOWER.encode(&sri_digest), - sri_parsed_hash.algorithm.to_string(), - ), - } - } + // treat an empty algo as None + let a = if algo.is_empty() { + None + } else { + Some(algo.as_ref()) }; - // mutate the algo string a bit more, depending on hashMode + let output_hash = nixhash::from_str(&hash, a).map_err(Error::InvalidOutputHash)?; + + // construct the algo string. Depending on hashMode, we prepend a `r:`. let algo = match hash_mode.as_deref() { - None | Some("flat") => algo, - Some("recursive") => format!("r:{}", algo), + None | Some("flat") => format!("{}", &output_hash.algo), + Some("recursive") => format!("r:{}", &output_hash.algo), Some(other) => { return Err(Error::InvalidOutputHashMode(other.to_string()).into()) } }; - out.hash = Some(Hash { algo, digest }); + out.hash = Some(Hash { + algo, + digest: data_encoding::HEXLOWER.encode(&output_hash.digest), + }); } } } diff --git a/tvix/cli/src/errors.rs b/tvix/cli/src/errors.rs index 5315b2486420..6f328dc0003c 100644 --- a/tvix/cli/src/errors.rs +++ b/tvix/cli/src/errors.rs @@ -1,9 +1,9 @@ -use nix_compat::derivation::DerivationError; +use nix_compat::{derivation::DerivationError, nixhash}; use std::rc::Rc; use thiserror::Error; /// Errors related to derivation construction -#[derive(Debug, Error, PartialEq)] +#[derive(Debug, Error)] pub enum Error { #[error("an output with the name '{0}' is already defined")] DuplicateOutput(String), @@ -15,18 +15,10 @@ pub enum Error { ShadowedOutput(String), #[error("invalid derivation parameters: {0}")] InvalidDerivation(DerivationError), + #[error("invalid output hash: {0}")] + InvalidOutputHash(nixhash::Error), #[error("invalid output hash mode: '{0}', only 'recursive' and 'flat` are supported")] InvalidOutputHashMode(String), - #[error("unsupported sri algorithm: {0}, only sha1, sha256 or sha512 is supported")] - UnsupportedSRIAlgo(String), - #[error("invalid number of sri hashes in string ({0}), only one hash is supported")] - UnsupportedSRIMultiple(usize), - #[error("invalid sri digest: {0}")] - InvalidSRIDigest(data_encoding::DecodeError), - #[error("failed to parse SRI string: {0}")] - InvalidSRIString(String), - #[error("outputHashAlgo is set to {0}, but outputHash contains SRI with algo {1}")] - ConflictingSRIHashAlgo(String, String), } impl From for tvix_eval::ErrorKind { -- cgit 1.4.1