diff options
| -rw-r--r-- | CONTRIBUTING.md | 1 | ||||
| -rw-r--r-- | src/build-remote/build-remote.cc | 57 | ||||
| -rw-r--r-- | src/libexpr/eval.cc | 1 | ||||
| -rw-r--r-- | src/libexpr/print.cc | 20 | ||||
| -rw-r--r-- | src/libexpr/print.hh | 6 | ||||
| -rw-r--r-- | tests/build-remote-trustless-should-fail-0.sh | 8 | ||||
| -rw-r--r-- | tests/build-remote-trustless-should-pass-2.sh | 13 | ||||
| -rw-r--r-- | tests/eval.sh | 6 | ||||
| -rw-r--r-- | tests/local.mk | 1 |
9 files changed, 90 insertions, 23 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1b0ecaf36..57a949906 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -30,6 +30,7 @@ Check out the [security policy](https://github.com/NixOS/nix/security/policy). You can use [labels](https://github.com/NixOS/nix/labels) to filter for relevant topics. 2. Search for related issues that cover what you're going to work on. It could help to mention there that you will work on the issue. + Pull requests addressing issues labeled ["idea approved"](https://github.com/NixOS/nix/labels/idea%20approved) are especially welcomed by maintainers and will receive prioritised review. 3. Check the [Nix reference manual](https://nixos.org/manual/nix/unstable/contributing/hacking.html) for information on building Nix and running its tests. diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index ce9c7f45a..323e04fdb 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -258,6 +258,8 @@ static int main_build_remote(int argc, char * * argv) connected: close(5); + assert(sshStore); + std::cerr << "# accept\n" << storeUri << "\n"; auto inputs = readStrings<PathSet>(source); @@ -286,23 +288,48 @@ connected: uploadLock = -1; auto drv = store->readDerivation(*drvPath); - auto outputHashes = staticOutputHashes(*store, drv); - - // Hijack the inputs paths of the derivation to include all the paths - // that come from the `inputDrvs` set. - // We don’t do that for the derivations whose `inputDrvs` is empty - // because - // 1. It’s not needed - // 2. Changing the `inputSrcs` set changes the associated output ids, - // which break CA derivations - if (!drv.inputDrvs.empty()) - drv.inputSrcs = store->parseStorePathSet(inputs); - auto result = sshStore->buildDerivation(*drvPath, drv); + std::optional<BuildResult> optResult; + + // If we don't know whether we are trusted (e.g. `ssh://` + // stores), we assume we are. This is necessary for backwards + // compat. + bool trustedOrLegacy = ({ + std::optional trusted = sshStore->isTrustedClient(); + !trusted || *trusted; + }); + + // See the very large comment in `case wopBuildDerivation:` in + // `src/libstore/daemon.cc` that explains the trust model here. + // + // This condition mirrors that: that code enforces the "rules" outlined there; + // we do the best we can given those "rules". + if (trustedOrLegacy || drv.type().isCA()) { + // Hijack the inputs paths of the derivation to include all + // the paths that come from the `inputDrvs` set. We don’t do + // that for the derivations whose `inputDrvs` is empty + // because: + // + // 1. It’s not needed + // + // 2. Changing the `inputSrcs` set changes the associated + // output ids, which break CA derivations + if (!drv.inputDrvs.empty()) + drv.inputSrcs = store->parseStorePathSet(inputs); + optResult = sshStore->buildDerivation(*drvPath, (const BasicDerivation &) drv); + auto & result = *optResult; + if (!result.success()) + throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg); + } else { + copyClosure(*store, *sshStore, StorePathSet {*drvPath}, NoRepair, NoCheckSigs, substitute); + auto res = sshStore->buildPathsWithResults({ DerivedPath::Built { *drvPath, OutputsSpec::All {} } }); + // One path to build should produce exactly one build result + assert(res.size() == 1); + optResult = std::move(res[0]); + } - if (!result.success()) - throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg); + auto outputHashes = staticOutputHashes(*store, drv); std::set<Realisation> missingRealisations; StorePathSet missingPaths; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv.type().hasKnownOutputPaths()) { @@ -311,6 +338,8 @@ connected: auto thisOutputId = DrvOutput{ thisOutputHash, outputName }; if (!store->queryRealisation(thisOutputId)) { debug("missing output %s", outputName); + assert(optResult); + auto & result = *optResult; auto i = result.builtOutputs.find(outputName); assert(i != result.builtOutputs.end()); auto & newRealisation = i->second; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index e2b455b91..0b4243670 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -94,7 +94,6 @@ RootValue allocRootValue(Value * v) #endif } - void Value::print(const SymbolTable & symbols, std::ostream & str, std::set<const void *> * seen) const { diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index d08672cfc..53ba70bdd 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -1,4 +1,5 @@ #include "print.hh" +#include <unordered_set> namespace nix { @@ -25,11 +26,26 @@ printLiteralBool(std::ostream & str, bool boolean) return str; } +// Returns `true' is a string is a reserved keyword which requires quotation +// when printing attribute set field names. +// +// This list should generally be kept in sync with `./lexer.l'. +// You can test if a keyword needs to be added by running: +// $ nix eval --expr '{ <KEYWORD> = 1; }' +// For example `or' doesn't need to be quoted. +bool isReservedKeyword(const std::string_view str) +{ + static const std::unordered_set<std::string_view> reservedKeywords = { + "if", "then", "else", "assert", "with", "let", "in", "rec", "inherit" + }; + return reservedKeywords.contains(str); +} + std::ostream & printIdentifier(std::ostream & str, std::string_view s) { if (s.empty()) str << "\"\""; - else if (s == "if") // FIXME: handle other keywords + else if (isReservedKeyword(s)) str << '"' << s << '"'; else { char c = s[0]; @@ -50,10 +66,10 @@ printIdentifier(std::ostream & str, std::string_view s) { return str; } -// FIXME: keywords static bool isVarName(std::string_view s) { if (s.size() == 0) return false; + if (isReservedKeyword(s)) return false; char c = s[0]; if ((c >= '0' && c <= '9') || c == '-' || c == '\'') return false; for (auto & i : s) diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh index f9cfc3964..3b72ae201 100644 --- a/src/libexpr/print.hh +++ b/src/libexpr/print.hh @@ -36,6 +36,12 @@ namespace nix { std::ostream & printAttributeName(std::ostream & o, std::string_view s); /** + * Returns `true' is a string is a reserved keyword which requires quotation + * when printing attribute set field names. + */ + bool isReservedKeyword(const std::string_view str); + + /** * Print a string as an identifier in the Nix expression language syntax. * * FIXME: "identifier" is ambiguous. Identifiers do not have a single diff --git a/tests/build-remote-trustless-should-fail-0.sh b/tests/build-remote-trustless-should-fail-0.sh index 5e3d5ae07..fad1def59 100644 --- a/tests/build-remote-trustless-should-fail-0.sh +++ b/tests/build-remote-trustless-should-fail-0.sh @@ -17,13 +17,13 @@ nix-build build-hook.nix -A passthru.input2 \ --store "$TEST_ROOT/local" \ --option system-features bar -# Now when we go to build that downstream derivation, Nix will fail -# because we cannot trustlessly build input-addressed derivations with -# `inputDrv` dependencies. +# Now when we go to build that downstream derivation, Nix will try to +# copy our already-build `input2` to the remote store. That store object +# is input-addressed, so this will fail. file=build-hook.nix prog=$(readlink -e ./nix-daemon-untrusting.sh) proto=ssh-ng expectStderr 1 source build-remote-trustless.sh \ - | grepQuiet "you are not privileged to build input-addressed derivations" + | grepQuiet "cannot add path '[^ ]*' because it lacks a signature by a trusted key" diff --git a/tests/build-remote-trustless-should-pass-2.sh b/tests/build-remote-trustless-should-pass-2.sh new file mode 100644 index 000000000..b769a88f0 --- /dev/null +++ b/tests/build-remote-trustless-should-pass-2.sh @@ -0,0 +1,13 @@ +source common.sh + +enableFeatures "daemon-trust-override" + +restartDaemon + +# Remote doesn't trust us +file=build-hook.nix +prog=$(readlink -e ./nix-daemon-untrusting.sh) +proto=ssh-ng + +source build-remote-trustless.sh +source build-remote-trustless-after.sh diff --git a/tests/eval.sh b/tests/eval.sh index ffae08a6a..066d8fc36 100644 --- a/tests/eval.sh +++ b/tests/eval.sh @@ -16,9 +16,10 @@ nix eval --expr 'assert 1 + 2 == 3; true' [[ $(nix eval int -f "./eval.nix") == 123 ]] [[ $(nix eval str -f "./eval.nix") == '"foo"' ]] [[ $(nix eval str --raw -f "./eval.nix") == 'foo' ]] -[[ $(nix eval attr -f "./eval.nix") == '{ foo = "bar"; }' ]] +[[ "$(nix eval attr -f "./eval.nix")" == '{ foo = "bar"; }' ]] [[ $(nix eval attr --json -f "./eval.nix") == '{"foo":"bar"}' ]] [[ $(nix eval int -f - < "./eval.nix") == 123 ]] +[[ "$(nix eval --expr '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]] # Check if toFile can be utilized during restricted eval [[ $(nix eval --restrict-eval --expr 'import (builtins.toFile "source" "42")') == 42 ]] @@ -26,9 +27,10 @@ nix eval --expr 'assert 1 + 2 == 3; true' nix-instantiate --eval -E 'assert 1 + 2 == 3; true' [[ $(nix-instantiate -A int --eval "./eval.nix") == 123 ]] [[ $(nix-instantiate -A str --eval "./eval.nix") == '"foo"' ]] -[[ $(nix-instantiate -A attr --eval "./eval.nix") == '{ foo = "bar"; }' ]] +[[ "$(nix-instantiate -A attr --eval "./eval.nix")" == '{ foo = "bar"; }' ]] [[ $(nix-instantiate -A attr --eval --json "./eval.nix") == '{"foo":"bar"}' ]] [[ $(nix-instantiate -A int --eval - < "./eval.nix") == 123 ]] +[[ "$(nix-instantiate --eval -E '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]] # Check that symlink cycles don't cause a hang. ln -sfn cycle.nix $TEST_ROOT/cycle.nix diff --git a/tests/local.mk b/tests/local.mk index 7c3b42599..0910fcd91 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -72,6 +72,7 @@ nix_tests = \ build-remote-content-addressed-floating.sh \ build-remote-trustless-should-pass-0.sh \ build-remote-trustless-should-pass-1.sh \ + build-remote-trustless-should-pass-2.sh \ build-remote-trustless-should-pass-3.sh \ build-remote-trustless-should-fail-0.sh \ nar-access.sh \ |