From d1bdaef04e7ca46949b36f0eb0aa76c89014a3fa Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 27 Aug 2025 15:31:46 -0400 Subject: [PATCH] Factor out `checkOutputs` We currently just use this during the build of a derivation, but there is no reason we wouldn't want to use it elsewhere, e.g. to check the outputs of someone else's build after the fact. Moreover, I like pulling things out of `DerivationBuilder` that are simple and don't need access to all that state. While `DerivationBuilder` is unix-only, this refactor also make the code more portable "for free". The header is private, at Eelco's request. --- src/libstore/build/derivation-check.cc | 156 ++++++++++++++++++ src/libstore/build/derivation-check.hh | 23 +++ src/libstore/meson.build | 1 + src/libstore/unix/build/derivation-builder.cc | 153 +---------------- 4 files changed, 182 insertions(+), 151 deletions(-) create mode 100644 src/libstore/build/derivation-check.cc create mode 100644 src/libstore/build/derivation-check.hh diff --git a/src/libstore/build/derivation-check.cc b/src/libstore/build/derivation-check.cc new file mode 100644 index 000000000..7473380fa --- /dev/null +++ b/src/libstore/build/derivation-check.cc @@ -0,0 +1,156 @@ +#include + +#include "nix/store/store-api.hh" + +#include "derivation-check.hh" + +namespace nix { + +void checkOutputs( + Store & store, + const StorePath & drvPath, + const decltype(DerivationOptions::outputChecks) & outputChecks, + const std::map & outputs) +{ + std::map outputsByPath; + for (auto & output : outputs) + outputsByPath.emplace(store.printStorePath(output.second.path), output.second); + + for (auto & output : outputs) { + auto & outputName = output.first; + auto & info = output.second; + + /* Compute the closure and closure size of some output. This + is slightly tricky because some of its references (namely + other outputs) may not be valid yet. */ + auto getClosure = [&](const StorePath & path) { + uint64_t closureSize = 0; + StorePathSet pathsDone; + std::queue pathsLeft; + pathsLeft.push(path); + + while (!pathsLeft.empty()) { + auto path = pathsLeft.front(); + pathsLeft.pop(); + if (!pathsDone.insert(path).second) + continue; + + auto i = outputsByPath.find(store.printStorePath(path)); + if (i != outputsByPath.end()) { + closureSize += i->second.narSize; + for (auto & ref : i->second.references) + pathsLeft.push(ref); + } else { + auto info = store.queryPathInfo(path); + closureSize += info->narSize; + for (auto & ref : info->references) + pathsLeft.push(ref); + } + } + + return std::make_pair(std::move(pathsDone), closureSize); + }; + + auto applyChecks = [&](const DerivationOptions::OutputChecks & checks) { + if (checks.maxSize && info.narSize > *checks.maxSize) + throw BuildError( + "path '%s' is too large at %d bytes; limit is %d bytes", + store.printStorePath(info.path), + info.narSize, + *checks.maxSize); + + if (checks.maxClosureSize) { + uint64_t closureSize = getClosure(info.path).second; + if (closureSize > *checks.maxClosureSize) + throw BuildError( + "closure of path '%s' is too large at %d bytes; limit is %d bytes", + store.printStorePath(info.path), + closureSize, + *checks.maxClosureSize); + } + + auto checkRefs = [&](const StringSet & value, bool allowed, bool recursive) { + /* Parse a list of reference specifiers. Each element must + either be a store path, or the symbolic name of the output + of the derivation (such as `out'). */ + StorePathSet spec; + for (auto & i : value) { + if (store.isStorePath(i)) + spec.insert(store.parseStorePath(i)); + else if (auto output = get(outputs, i)) + spec.insert(output->path); + else { + std::string outputsListing = + concatMapStringsSep(", ", outputs, [](auto & o) { return o.first; }); + throw BuildError( + "derivation '%s' output check for '%s' contains an illegal reference specifier '%s'," + " expected store path or output name (one of [%s])", + store.printStorePath(drvPath), + outputName, + i, + outputsListing); + } + } + + auto used = recursive ? getClosure(info.path).first : info.references; + + if (recursive && checks.ignoreSelfRefs) + used.erase(info.path); + + StorePathSet badPaths; + + for (auto & i : used) + if (allowed) { + if (!spec.count(i)) + badPaths.insert(i); + } else { + if (spec.count(i)) + badPaths.insert(i); + } + + if (!badPaths.empty()) { + std::string badPathsStr; + for (auto & i : badPaths) { + badPathsStr += "\n "; + badPathsStr += store.printStorePath(i); + } + throw BuildError( + "output '%s' is not allowed to refer to the following paths:%s", + store.printStorePath(info.path), + badPathsStr); + } + }; + + /* Mandatory check: absent whitelist, and present but empty + whitelist mean very different things. */ + if (auto & refs = checks.allowedReferences) { + checkRefs(*refs, true, false); + } + if (auto & refs = checks.allowedRequisites) { + checkRefs(*refs, true, true); + } + + /* Optimization: don't need to do anything when + disallowed and empty set. */ + if (!checks.disallowedReferences.empty()) { + checkRefs(checks.disallowedReferences, false, false); + } + if (!checks.disallowedRequisites.empty()) { + checkRefs(checks.disallowedRequisites, false, true); + } + }; + + std::visit( + overloaded{ + [&](const DerivationOptions::OutputChecks & checks) { applyChecks(checks); }, + [&](const std::map & checksPerOutput) { + if (auto outputChecks = get(checksPerOutput, outputName)) + + applyChecks(*outputChecks); + }, + }, + outputChecks); + } +} + +} // namespace nix diff --git a/src/libstore/build/derivation-check.hh b/src/libstore/build/derivation-check.hh new file mode 100644 index 000000000..249e176c5 --- /dev/null +++ b/src/libstore/build/derivation-check.hh @@ -0,0 +1,23 @@ +#include "nix/store/derivations.hh" +#include "nix/store/derivation-options.hh" +#include "nix/store/path-info.hh" + +namespace nix { + +/** + * Check that outputs meets the requirements specified by the + * 'outputChecks' attribute (or the legacy + * '{allowed,disallowed}{References,Requisites}' attributes). + * + * The outputs may not be valid yet, hence outputs needs to contain all + * needed info like the NAR size. However, the external (not other + * output) references of the output must be valid, so we can compute the + * closure size. + */ +void checkOutputs( + Store & store, + const StorePath & drvPath, + const decltype(DerivationOptions::outputChecks) & drvOptions, + const std::map & outputs); + +} // namespace nix diff --git a/src/libstore/meson.build b/src/libstore/meson.build index ad130945e..ca8eac12b 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -265,6 +265,7 @@ sources = files( 'binary-cache-store.cc', 'build-result.cc', 'build/derivation-building-goal.cc', + 'build/derivation-check.cc', 'build/derivation-goal.cc', 'build/derivation-trampoline-goal.cc', 'build/drv-output-substitution-goal.cc', diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 51b44719d..bd5f975fb 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -42,6 +42,7 @@ #include "nix/util/signals.hh" #include "store-config-private.hh" +#include "build/derivation-check.hh" namespace nix { @@ -335,13 +336,6 @@ private: */ SingleDrvOutputs registerOutputs(); - /** - * Check that an output meets the requirements specified by the - * 'outputChecks' attribute (or the legacy - * '{allowed,disallowed}{References,Requisites}' attributes). - */ - void checkOutputs(const std::map & outputs); - public: void deleteTmpDir(bool force) override; @@ -1810,7 +1804,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() } /* Apply output checks. */ - checkOutputs(infos); + checkOutputs(store, drvPath, drvOptions.outputChecks, infos); /* Register each output path as valid, and register the sets of paths referenced by each of them. If there are cycles in the @@ -1849,149 +1843,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() return builtOutputs; } -void DerivationBuilderImpl::checkOutputs(const std::map & outputs) -{ - std::map outputsByPath; - for (auto & output : outputs) - outputsByPath.emplace(store.printStorePath(output.second.path), output.second); - - for (auto & output : outputs) { - auto & outputName = output.first; - auto & info = output.second; - - /* Compute the closure and closure size of some output. This - is slightly tricky because some of its references (namely - other outputs) may not be valid yet. */ - auto getClosure = [&](const StorePath & path) { - uint64_t closureSize = 0; - StorePathSet pathsDone; - std::queue pathsLeft; - pathsLeft.push(path); - - while (!pathsLeft.empty()) { - auto path = pathsLeft.front(); - pathsLeft.pop(); - if (!pathsDone.insert(path).second) - continue; - - auto i = outputsByPath.find(store.printStorePath(path)); - if (i != outputsByPath.end()) { - closureSize += i->second.narSize; - for (auto & ref : i->second.references) - pathsLeft.push(ref); - } else { - auto info = store.queryPathInfo(path); - closureSize += info->narSize; - for (auto & ref : info->references) - pathsLeft.push(ref); - } - } - - return std::make_pair(std::move(pathsDone), closureSize); - }; - - auto applyChecks = [&](const DerivationOptions::OutputChecks & checks) { - if (checks.maxSize && info.narSize > *checks.maxSize) - throw BuildError( - "path '%s' is too large at %d bytes; limit is %d bytes", - store.printStorePath(info.path), - info.narSize, - *checks.maxSize); - - if (checks.maxClosureSize) { - uint64_t closureSize = getClosure(info.path).second; - if (closureSize > *checks.maxClosureSize) - throw BuildError( - "closure of path '%s' is too large at %d bytes; limit is %d bytes", - store.printStorePath(info.path), - closureSize, - *checks.maxClosureSize); - } - - auto checkRefs = [&](const StringSet & value, bool allowed, bool recursive) { - /* Parse a list of reference specifiers. Each element must - either be a store path, or the symbolic name of the output - of the derivation (such as `out'). */ - StorePathSet spec; - for (auto & i : value) { - if (store.isStorePath(i)) - spec.insert(store.parseStorePath(i)); - else if (auto output = get(outputs, i)) - spec.insert(output->path); - else { - std::string outputsListing = - concatMapStringsSep(", ", outputs, [](auto & o) { return o.first; }); - throw BuildError( - "derivation '%s' output check for '%s' contains an illegal reference specifier '%s'," - " expected store path or output name (one of [%s])", - store.printStorePath(drvPath), - outputName, - i, - outputsListing); - } - } - - auto used = recursive ? getClosure(info.path).first : info.references; - - if (recursive && checks.ignoreSelfRefs) - used.erase(info.path); - - StorePathSet badPaths; - - for (auto & i : used) - if (allowed) { - if (!spec.count(i)) - badPaths.insert(i); - } else { - if (spec.count(i)) - badPaths.insert(i); - } - - if (!badPaths.empty()) { - std::string badPathsStr; - for (auto & i : badPaths) { - badPathsStr += "\n "; - badPathsStr += store.printStorePath(i); - } - throw BuildError( - "output '%s' is not allowed to refer to the following paths:%s", - store.printStorePath(info.path), - badPathsStr); - } - }; - - /* Mandatory check: absent whitelist, and present but empty - whitelist mean very different things. */ - if (auto & refs = checks.allowedReferences) { - checkRefs(*refs, true, false); - } - if (auto & refs = checks.allowedRequisites) { - checkRefs(*refs, true, true); - } - - /* Optimization: don't need to do anything when - disallowed and empty set. */ - if (!checks.disallowedReferences.empty()) { - checkRefs(checks.disallowedReferences, false, false); - } - if (!checks.disallowedRequisites.empty()) { - checkRefs(checks.disallowedRequisites, false, true); - } - }; - - std::visit( - overloaded{ - [&](const DerivationOptions::OutputChecks & checks) { applyChecks(checks); }, - [&](const std::map & checksPerOutput) { - if (auto outputChecks = get(checksPerOutput, outputName)) - - applyChecks(*outputChecks); - }, - }, - drvOptions.outputChecks); - } -} - void DerivationBuilderImpl::deleteTmpDir(bool force) { if (topTmpDir != "") {