feat(tvix/derivation): also fail if output name is called drv

`drv` is an invalid output name too, as this would cause a
`builtins.derivation` call to return an attrset with a `drvPath` key
(which already exists) and has a different meaning.

Also handle errors during store path construction, and return our own
error type, instead of the ParseStorePathError.

Change-Id: Ib7952dde1d5cf18a0e210928df7c57b5939b7678
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7850
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
This commit is contained in:
Florian Klink 2023-01-17 11:56:23 +01:00 committed by flokli
parent c891833414
commit 0aad4e2246
3 changed files with 32 additions and 10 deletions

View file

@ -1,12 +1,12 @@
use crate::nix_hash;
use crate::output::{Hash, Output};
use crate::write;
use crate::{nix_hash, ValidateDerivationError};
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
use std::collections::BTreeSet;
use std::{collections::BTreeMap, fmt, fmt::Write};
use tvix_store::nixbase32::NIXBASE32;
use tvix_store::store_path::{ParseStorePathError, StorePath, STORE_DIR};
use tvix_store::store_path::{StorePath, STORE_DIR};
#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
pub struct Derivation {
@ -35,7 +35,7 @@ fn build_store_path(
is_derivation: bool,
path_hash: &[u8],
name: &str,
) -> Result<StorePath, ParseStorePathError> {
) -> Result<StorePath, ValidateDerivationError> {
let compressed = nix_hash::compress_hash(path_hash, 20);
if is_derivation {
StorePath::from_string(
@ -50,6 +50,9 @@ fn build_store_path(
} else {
StorePath::from_string(format!("{}-{}", NIXBASE32.encode(&compressed), name,).as_str())
}
.map_err(|_e| ValidateDerivationError::InvalidOutputName(name.to_string()))
// Constructing the StorePath can only fail if the passed output name was
// invalid, so map errors to a [ValidateDerivationError::InvalidOutputName].
}
impl Derivation {
@ -107,7 +110,10 @@ impl Derivation {
/// - Take the digest, run hash.CompressHash(digest, 20) on it.
/// - Encode it with nixbase32
/// - Use it (and the name) to construct a [StorePath].
pub fn calculate_derivation_path(&self, name: &str) -> Result<StorePath, ParseStorePathError> {
pub fn calculate_derivation_path(
&self,
name: &str,
) -> Result<StorePath, ValidateDerivationError> {
let mut hasher = Sha256::new();
// collect the list of paths from input_sources and input_derivations
@ -224,7 +230,7 @@ impl Derivation {
&mut self,
name: &str,
drv_replacement_str: &str,
) -> Result<(), ParseStorePathError> {
) -> Result<(), ValidateDerivationError> {
let mut hasher = Sha256::new();
// Check if the Derivation is fixed output, because they cause

View file

@ -1,8 +1,6 @@
use serde::{Deserialize, Serialize};
use tvix_store::store_path::{ParseStorePathError, StorePath};
use crate::ValidateDerivationError;
#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
pub struct Output {
pub path: String,

View file

@ -12,7 +12,16 @@ impl Derivation {
// Validate all outputs
for (output_name, output) in &self.outputs {
if output_name.is_empty() {
// empty output names are invalid.
//
// `drv` is an invalid output name too, as this would cause
// a `builtins.derivation` call to return an attrset with a
// `drvPath` key (which already exists) and has a different
// meaning.
//
// Other output names that don't match the name restrictions from
// [StorePath] will fail output path calculation.
if output_name.is_empty() || output_name == "drv" {
return Err(ValidateDerivationError::InvalidOutputName(
output_name.to_string(),
));
@ -62,13 +71,22 @@ impl Derivation {
}
for output_name in output_names.iter() {
if output_name.is_empty() {
// empty output names are invalid.
//
// `drv` is an invalid output name too, as this would cause
// a `builtins.derivation` call to return an attrset with a
// `drvPath` key (which already exists) and has a different
// meaning.
//
// Other output names that don't match the name restrictions
// from [StorePath] can't be constructed with this library, but
// are not explicitly checked here (yet).
if output_name.is_empty() || output_name == "drv" {
return Err(ValidateDerivationError::InvalidInputDerivationOutputName(
input_derivation_path.to_string(),
output_name.to_string(),
));
}
// TODO: do we need to apply more name validation here?
}
}