From 325e7e1824e3e6eec36c619da625d81dd607bb2b Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 29 Jun 2025 19:25:00 +0200 Subject: [PATCH] libstore: show forbidden chain(s) to forbidden path from disallowedRequisites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #334 Closes #626 This is loosely based on upstream PR#10877[1], but heavily changed to use the graph logic from `nix why-depends`. `precise` is `false` here since the out-path of the drv being built isn't registered yet, so the path accessor cannot scan through files yet. Example output (from an openssh build with `pcsclite.lib` & `glibc` in `disallowedRequisites`): error: output '/nix/store/hr8lmmjmd1jk6s3p5ymggyk4am7n2lmb-openssh-10.0p2' is not allowed to refer to the following paths: /nix/store/p6r5awz3ywrz66symnrn0xb85xzmcysf-pcsclite-2.3.0-lib /nix/store/q4wq65gl3r8fy746v9bbwgx4gzn0r2kl-glibc-2.40-66 Shown below are chains that lead to the forbidden path(s). /nix/store/hr8lmmjmd1jk6s3p5ymggyk4am7n2lmb-openssh-10.0p2 └───/nix/store/ys91ywnwikm14xznwk3cdbprapv2m37z-libfido2-1.16.0 └───/nix/store/p6r5awz3ywrz66symnrn0xb85xzmcysf-pcsclite-2.3.0-lib /nix/store/hr8lmmjmd1jk6s3p5ymggyk4am7n2lmb-openssh-10.0p2 ├───/nix/store/q4wq65gl3r8fy746v9bbwgx4gzn0r2kl-glibc-2.40-66 ├───/nix/store/6r4zqb04fq5l5l4zghq76wvcpz7dwd35-linux-pam-1.6.1 │ ├───/nix/store/q4wq65gl3r8fy746v9bbwgx4gzn0r2kl-glibc-2.40-66 [...] [1] https://github.com/NixOS/nix/pull/10877 Co-authored-by: Robert Hensing Change-Id: Ib30024c0d9e45c1160bf0134f7d3ba17dbdeff47 --- .../rl-next/report-forbidden-path-as-chain.md | 19 ++++ lix/libstore/build/local-derivation-goal.cc | 105 +++++++++++++----- tests/functional/check-reqs.nix | 30 ++++- tests/functional/check-reqs.sh | 20 +++- 4 files changed, 138 insertions(+), 36 deletions(-) create mode 100644 doc/manual/rl-next/report-forbidden-path-as-chain.md diff --git a/doc/manual/rl-next/report-forbidden-path-as-chain.md b/doc/manual/rl-next/report-forbidden-path-as-chain.md new file mode 100644 index 000000000..dca9ecb6c --- /dev/null +++ b/doc/manual/rl-next/report-forbidden-path-as-chain.md @@ -0,0 +1,19 @@ +--- +synopsis: "`disallowedRequisites` now reports chains of disallowed requisites" +issues: [fj#334,fj#626,gh#10877] +category: Improvements +credits: [ma27,roberth] +--- + +When a build fails because of [`disallowedRequisites`](@docroot@/language/advanced-attributes.md#adv-attr-disallowedRequisites), the error message now includes the chain of references that led to the failure. This makes it easier to see in which derivations the chain can be broken, to resolve the problem. + +Example: + +``` +$ nix-build -A hello +error: output '/nix/store/0b7k85gg5r28gb54px9nq7iv5986mns9-hello-2.12.2' is not allowed to refer to the following paths: + /nix/store/q4wq65gl3r8fy746v9bbwgx4gzn0r2kl-glibc-2.40-66 + Shown below are chains that lead to the forbidden path(s). + /nix/store/0b7k85gg5r28gb54px9nq7iv5986mns9-hello-2.12.2 + └───/nix/store/q4wq65gl3r8fy746v9bbwgx4gzn0r2kl-glibc-2.40-66 +``` diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index 99c25453a..9be1f5cbd 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -12,6 +12,7 @@ #include "lix/libstore/path-references.hh" #include "lix/libutil/archive.hh" #include "lix/libstore/daemon.hh" +#include "lix/libutil/fmt.hh" #include "lix/libutil/regex.hh" #include "lix/libutil/file-descriptor.hh" #include "lix/libutil/file-system.hh" @@ -27,6 +28,7 @@ #include "lix/libutil/strings.hh" #include "lix/libutil/thread-name.hh" #include "platform/linux.hh" +#include "path-tree.hh" #include #include @@ -2113,37 +2115,50 @@ try { std::optional allowedReferences, allowedRequisites, disallowedReferences, disallowedRequisites; }; + struct Closure { + /** Keys: paths in the closure, values: reverse path from an initial path to the parent of the key */ + std::map paths; + uint64_t size; + }; + /* 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. */ // NOLINTNEXTLINE(cppcoreguidelines-avoid-capturing-lambda-coroutines) auto getClosure = [&](const StorePath & path - ) -> kj::Promise>> { + ) -> kj::Promise> { try { uint64_t closureSize = 0; - StorePathSet pathsDone; + std::map pathsDone; std::queue pathsLeft; pathsLeft.push(path); while (!pathsLeft.empty()) { auto path = pathsLeft.front(); pathsLeft.pop(); - if (!pathsDone.insert(path).second) continue; + if (pathsDone.contains(path)) { + continue; + } auto i = outputsByPath.find(worker.store.printStorePath(path)); + auto & refs = pathsDone[path]; if (i != outputsByPath.end()) { closureSize += i->second.narSize; - for (auto & ref : i->second.references) + for (auto & ref : i->second.references) { pathsLeft.push(ref); + refs.insert(ref); + } } else { auto info = TRY_AWAIT(worker.store.queryPathInfo(path)); closureSize += info->narSize; - for (auto & ref : info->references) + for (auto & ref : info->references) { pathsLeft.push(ref); + refs.insert(ref); + } } } - co_return std::make_pair(std::move(pathsDone), closureSize); + co_return Closure { std::move(pathsDone), closureSize}; } catch (...) { co_return result::current_exception(); } @@ -2157,7 +2172,7 @@ try { worker.store.printStorePath(info.path), info.narSize, *checks.maxSize); if (checks.maxClosureSize) { - uint64_t closureSize = TRY_AWAIT(getClosure(info.path)).second; + uint64_t closureSize = TRY_AWAIT(getClosure(info.path)).size; if (closureSize > *checks.maxClosureSize) throw BuildError("closure of path '%s' is too large at %d bytes; limit is %d bytes", worker.store.printStorePath(info.path), closureSize, *checks.maxClosureSize); @@ -2201,32 +2216,64 @@ try { } } - auto used = recursive - ? TRY_AWAIT(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); + std::map used; + if (recursive) { + used = TRY_AWAIT(getClosure(info.path)).paths; + } else { + for (auto & ref : info.references) { + used.insert({ref, {}}); } + } + + std::set badPaths; + for (auto & [path, refs] : used) { + if (path == info.path && recursive && checks.ignoreSelfRefs) { + continue; + } + if (allowed) { + if (!spec.count(path)) { + badPaths.insert(path); + } + } else { + if (spec.count(path)) { + badPaths.insert(path); + } + } + } if (!badPaths.empty()) { - std::string badPathsStr; - for (auto & i : badPaths) { - badPathsStr += "\n "; - badPathsStr += worker.store.printStorePath(i); + auto badPathsList = concatMapStringsSep( + "\n", + badPaths, + [&](const StorePath & i) -> std::string { + return worker.store.printStorePath(i); + } + ); + if (recursive) { + std::string badPathRefsTree; + for (auto & i : badPaths) { + badPathRefsTree += TRY_AWAIT(genGraphString( + info.path, i, used, worker.store, true, false + )); + badPathRefsTree += "\n"; + } + + throw BuildError( + "output '%s' is not allowed to refer to the following " + "paths:\n%s\n\nShown below are chains that lead to the " + "forbidden path(s).\n%s", + worker.store.printStorePath(info.path), + badPathsList, + Uncolored(badPathRefsTree) + ); + } else { + throw BuildError( + "output '%s' is not allowed to have direct references to the " + "following paths:\n%s", + worker.store.printStorePath(info.path), + badPathsList + ); } - throw BuildError("output '%s' is not allowed to refer to the following paths:%s", - worker.store.printStorePath(info.path), badPathsStr); } co_return result::success(); } catch (...) { diff --git a/tests/functional/check-reqs.nix b/tests/functional/check-reqs.nix index 41436cb48..cf1fa1ff2 100644 --- a/tests/functional/check-reqs.nix +++ b/tests/functional/check-reqs.nix @@ -45,7 +45,7 @@ rec { name = "check-reqs"; inherit deps; builder = builtins.toFile "builder.sh" "mkdir $out; ln -s $deps $out/depdir1"; - disallowedRequisites = [dep1]; + disallowedRequisites = [ dep1 dep2 ]; }; test7 = mkDerivation { @@ -54,4 +54,32 @@ rec { builder = builtins.toFile "builder.sh" "mkdir $out; ln -s $deps $out/depdir1"; disallowedRequisites = [test1]; }; + + test8 = mkDerivation { + name = "check-reqs-structured-attrs"; + __structuredAttrs = true; + outputChecks.out = { + allowedRequisites = [dep2]; + }; + inherit dep2; + outputs = [ "out" ]; + buildCommand = '' + set -x + out=''${outputs[out]} + mkdir $out + ln -s $dep2 $out/depdir1 + ln -sf $out $out/self-reference + ''; + }; + + test9 = mkDerivation { + name = "check-reqs-structured-attrs"; + allowedRequisites = [dep2]; + inherit dep2; + buildCommand = '' + mkdir $out + ln -s $dep2 $out/depdir1 + ln -sf $out $out/self-reference + ''; + }; } diff --git a/tests/functional/check-reqs.sh b/tests/functional/check-reqs.sh index 856c94cec..367f2668e 100644 --- a/tests/functional/check-reqs.sh +++ b/tests/functional/check-reqs.sh @@ -6,11 +6,19 @@ RESULT=$TEST_ROOT/result nix-build -o $RESULT check-reqs.nix -A test1 -(! nix-build -o $RESULT check-reqs.nix -A test2) -(! nix-build -o $RESULT check-reqs.nix -A test3) -(! nix-build -o $RESULT check-reqs.nix -A test4) 2>&1 | grepQuiet 'check-reqs-dep1' -(! nix-build -o $RESULT check-reqs.nix -A test4) 2>&1 | grepQuiet 'check-reqs-dep2' -(! nix-build -o $RESULT check-reqs.nix -A test5) -(! nix-build -o $RESULT check-reqs.nix -A test6) +(! nix-build -o "$RESULT" check-reqs.nix -A test2) +(! nix-build -o "$RESULT" check-reqs.nix -A test3) +(! nix-build -o "$RESULT" check-reqs.nix -A test4) 2>&1 | grepQuiet 'check-reqs-dep1' +(! nix-build -o "$RESULT" check-reqs.nix -A test4) 2>&1 | grepQuiet 'check-reqs-dep2' +(! nix-build -o "$RESULT" check-reqs.nix -A test5) +(! nix-build -o "$RESULT" check-reqs.nix -A test6) + +(! nix-build -o "$RESULT" check-reqs.nix -A test6) 2>&1 | grepQuiet '└───.*/.*-check-reqs-deps' +(! nix-build -o "$RESULT" check-reqs.nix -A test6) 2>&1 | grepQuiet 'check-reqs-dep1' +(! nix-build -o "$RESULT" check-reqs.nix -A test6) 2>&1 | grepQuiet 'check-reqs-dep2' nix-build -o $RESULT check-reqs.nix -A test7 + +# ignoreSelfRefs is only true for drvs using structuredAttrs. +(! nix-build -o $RESULT check-reqs.nix -A test8) +nix-build -o $RESULT check-reqs.nix -A test9