From 1ee42c5b88eb0533ebcf8b2579ec82f2be80e4b2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 1 Feb 2024 21:46:01 +0100 Subject: [PATCH 1/4] builtin:fetchurl: Ensure a fixed-output derivation Previously we didn't check that the derivation was fixed-output, so you could use builtin:fetchurl to impurely fetch a file. --- src/libstore/builtins/fetchurl.cc | 3 +++ tests/functional/fetchurl.sh | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 2086bd0b9..cf7b2770f 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -16,6 +16,9 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) writeFile(settings.netrcFile, netrcData, 0600); } + if (!drv.type().isFixed()) + throw Error("'builtin:fetchurl' must be a fixed-output derivation"); + auto getAttr = [&](const std::string & name) { auto i = drv.env.find(name); if (i == drv.env.end()) throw Error("attribute '%s' missing", name); diff --git a/tests/functional/fetchurl.sh b/tests/functional/fetchurl.sh index 8cd40c09f..578f5a34c 100644 --- a/tests/functional/fetchurl.sh +++ b/tests/functional/fetchurl.sh @@ -78,3 +78,6 @@ outPath=$(nix-build -vvvvv --expr 'import ' --argstr url file: test -x $outPath/fetchurl.sh test -L $outPath/symlink + +# Make sure that *not* passing a outputHash fails. +expectStderr 100 nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url file://$narxz 2>&1 | grep 'must be a fixed-output derivation' From b8b739e484078863c10c48d031fa8459081ba8b3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 1 Feb 2024 22:01:02 +0100 Subject: [PATCH 2/4] builtin:fetchurl: Get output hash info from the drv --- src/libstore/builtins/fetchurl.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index cf7b2770f..a9f2e748e 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -16,7 +16,12 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) writeFile(settings.netrcFile, netrcData, 0600); } - if (!drv.type().isFixed()) + auto out = get(drv.outputs, "out"); + if (!out) + throw Error("'builtin:fetchurl' requires an 'out' output"); + + auto dof = std::get_if(&out->raw); + if (!dof) throw Error("'builtin:fetchurl' must be a fixed-output derivation"); auto getAttr = [&](const std::string & name) { @@ -62,13 +67,11 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) }; /* Try the hashed mirrors first. */ - if (getAttr("outputHashMode") == "flat") + if (dof->ca.method.getFileIngestionMethod() == FileIngestionMethod::Flat) for (auto hashedMirror : settings.hashedMirrors.get()) try { if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/'; - std::optional ht = parseHashAlgoOpt(getAttr("outputHashAlgo")); - Hash h = newHashAllowEmpty(getAttr("outputHash"), ht); - fetch(hashedMirror + printHashAlgo(h.algo) + "/" + h.to_string(HashFormat::Base16, false)); + fetch(hashedMirror + printHashAlgo(dof->ca.hash.algo) + "/" + dof->ca.hash.to_string(HashFormat::Base16, false)); return; } catch (Error & e) { debug(e.what()); From 05535be03a1526061ea3a3ad25459c032e1f8f8c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 2 Feb 2024 13:07:08 +0100 Subject: [PATCH 3/4] Fix test --- tests/functional/fetchurl.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/fetchurl.sh b/tests/functional/fetchurl.sh index 578f5a34c..5259dd60e 100644 --- a/tests/functional/fetchurl.sh +++ b/tests/functional/fetchurl.sh @@ -80,4 +80,6 @@ test -x $outPath/fetchurl.sh test -L $outPath/symlink # Make sure that *not* passing a outputHash fails. -expectStderr 100 nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url file://$narxz 2>&1 | grep 'must be a fixed-output derivation' +expected=100 +if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly +expectStderr $expected nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url file://$narxz 2>&1 | grep 'must be a fixed-output derivation' From e67458e5b821e0a3a6839f4637eb96ff873f64ed Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 2 Feb 2024 13:22:18 +0100 Subject: [PATCH 4/4] Better test fix --- tests/functional/fetchurl.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/fetchurl.sh b/tests/functional/fetchurl.sh index 5259dd60e..5a05cc5e1 100644 --- a/tests/functional/fetchurl.sh +++ b/tests/functional/fetchurl.sh @@ -80,6 +80,7 @@ test -x $outPath/fetchurl.sh test -L $outPath/symlink # Make sure that *not* passing a outputHash fails. +requireDaemonNewerThan "2.20" expected=100 if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly expectStderr $expected nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url file://$narxz 2>&1 | grep 'must be a fixed-output derivation'