diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock index d9f0f3f5b..abddd5b4a 100644 --- a/tvix/Cargo.lock +++ b/tvix/Cargo.lock @@ -3405,6 +3405,7 @@ version = "0.1.0" dependencies = [ "bytes", "criterion", + "data-encoding", "lazy_static", "nix-compat", "tempfile", diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix index f92cc0b57..69b9cc9a8 100644 --- a/tvix/Cargo.nix +++ b/tvix/Cargo.nix @@ -10719,6 +10719,10 @@ rec { name = "bytes"; packageId = "bytes"; } + { + name = "data-encoding"; + packageId = "data-encoding"; + } { name = "nix-compat"; packageId = "nix-compat"; diff --git a/tvix/eval/src/warnings.rs b/tvix/eval/src/warnings.rs index e007d9c34..f537aa913 100644 --- a/tvix/eval/src/warnings.rs +++ b/tvix/eval/src/warnings.rs @@ -18,6 +18,7 @@ pub enum WarningKind { EmptyInherit, EmptyLet, ShadowedOutput(String), + SRIHashWrongPadding, /// Tvix internal warning for features triggered by users that are /// not actually implemented yet, but do not cause runtime failures. @@ -105,6 +106,7 @@ impl EvalWarning { "this derivation's environment shadows the output name {}", out ), + WarningKind::SRIHashWrongPadding => "SRI hash has wrong padding".to_string(), WarningKind::NotImplemented(what) => { format!("feature not yet implemented in tvix: {}", what) @@ -127,6 +129,7 @@ impl EvalWarning { WarningKind::EmptyInherit => "W009", WarningKind::EmptyLet => "W010", WarningKind::ShadowedOutput(_) => "W011", + WarningKind::SRIHashWrongPadding => "W012", WarningKind::NotImplemented(_) => "W999", } diff --git a/tvix/glue/Cargo.toml b/tvix/glue/Cargo.toml index b0902b5c9..6b0edb574 100644 --- a/tvix/glue/Cargo.toml +++ b/tvix/glue/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" edition = "2021" [dependencies] +data-encoding = "2.3.3" nix-compat = { path = "../nix-compat" } tvix-build = { path = "../build", default-features = false, features = []} tvix-eval = { path = "../eval" } diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs index 993e7612a..57abe11b1 100644 --- a/tvix/glue/src/builtins/derivation.rs +++ b/tvix/glue/src/builtins/derivation.rs @@ -106,12 +106,14 @@ fn populate_inputs>( /// (lowercase) hex encoding of the digest. /// /// These values are only rewritten for the outputs, not what's passed to env. +/// +/// The return value may optionally contain a warning. fn handle_fixed_output( drv: &mut Derivation, hash_str: Option, // in nix: outputHash hash_algo_str: Option, // in nix: outputHashAlgo hash_mode_str: Option, // in nix: outputHashmode -) -> Result<(), ErrorKind> { +) -> Result, ErrorKind> { // If outputHash is provided, ensure hash_algo_str is compatible. // If outputHash is not provided, do nothing. if let Some(hash_str) = hash_str { @@ -125,6 +127,7 @@ fn handle_fixed_output( // construct a NixHash. let nixhash = nixhash::from_str(&hash_str, hash_algo_str.as_deref()) .map_err(DerivationError::InvalidOutputHash)?; + let algo = nixhash.algo(); // construct the fixed output. drv.outputs.insert( @@ -140,8 +143,18 @@ fn handle_fixed_output( }, }, ); + + // Peek at hash_str once more. + // If it was a SRI hash, but is not using the correct length, this means + // the padding was wrong. Emit a warning in that case. + let sri_prefix = format!("{}-", algo); + if let Some(rest) = hash_str.strip_prefix(&sri_prefix) { + if data_encoding::BASE64.encode_len(algo.digest_length()) != rest.len() { + return Ok(Some(WarningKind::SRIHashWrongPadding)); + } + } } - Ok(()) + Ok(None) } /// Handles derivation parameters which are not just forwarded to @@ -349,7 +362,12 @@ pub(crate) mod derivation_builtins { Err(cek) => return Ok(Value::Catchable(cek)), Ok(s) => s, }; - handle_fixed_output(&mut drv, output_hash, output_hash_algo, output_hash_mode)?; + + if let Some(warning) = + handle_fixed_output(&mut drv, output_hash, output_hash_algo, output_hash_mode)? + { + emit_warning_kind(&co, warning).await; + } } // Scan references in relevant attributes to detect any build-references. diff --git a/tvix/glue/src/builtins/mod.rs b/tvix/glue/src/builtins/mod.rs index 86924cefe..aeae8a7c3 100644 --- a/tvix/glue/src/builtins/mod.rs +++ b/tvix/glue/src/builtins/mod.rs @@ -154,4 +154,24 @@ mod tests { "/171rf4jhx57xqz3p7swniwkig249cif71pa08p80mgaf0mqz5bmr" ); } + + /// constructs calls to builtins.derivation that should succeed, but produce warnings + #[test_case(r#"(builtins.derivation { name = "foo"; builder = "/bin/sh"; system = "x86_64-linux"; outputHashMode = "recursive"; outputHashAlgo = "sha256"; outputHash = "sha256-fgIr3TyFGDAXP5+qoAaiMKDg/a1MlT6Fv/S/DaA24S8===="; }).outPath"#, "/nix/store/xm1l9dx4zgycv9qdhcqqvji1z88z534b-foo"; "r:sha256 wrong padding")] + fn builtins_derivation_hash_wrong_padding_warn(code: &str, expected_path: &str) { + let eval_result = eval(code); + + let value = eval_result.value.expect("must succeed"); + + match value { + tvix_eval::Value::String(s) => { + assert_eq!(expected_path, s.as_str()); + } + _ => panic!("unexpected value type: {:?}", value), + } + + assert!( + !eval_result.warnings.is_empty(), + "warnings should not be empty" + ); + } }