Merge remote-tracking branch 'upstream/master' into package-nix

doc/manual/rl-next/allowed-uris-can-now-match-whole-schemes.md

@@ -0,0 +1,7 @@
+---
+synopsis: Option `allowed-uris` can now match whole schemes in URIs without slashes
+prs: 9547
+---
+
+If a scheme, such as `github:` is specified in the `allowed-uris` option, all URIs starting with `github:` are allowed.
+Previously this only worked for schemes whose URIs used the `://` syntax.

doc/manual/src/language/advanced-attributes.md

@@ -257,29 +257,18 @@ Derivations can declare some infrequently used optional attributes.
     of the environment (typically, a few hundred kilobyte).
 
   - [`preferLocalBuild`]{#adv-attr-preferLocalBuild}\
-    If this attribute is set to `true` and [distributed building is
+    If this attribute is set to `true` and [distributed building is enabled](../advanced-topics/distributed-builds.md), then, if possible, the derivation will be built locally instead of being forwarded to a remote machine.
-    enabled](../advanced-topics/distributed-builds.md), then, if
+    This is useful for derivations that are cheapest to build locally.
-    possible, the derivation will be built locally instead of forwarded
-    to a remote machine. This is appropriate for trivial builders
-    where the cost of doing a download or remote build would exceed
-    the cost of building locally.
 
   - [`allowSubstitutes`]{#adv-attr-allowSubstitutes}\
-    If this attribute is set to `false`, then Nix will always build this
+    If this attribute is set to `false`, then Nix will always build this derivation (locally or remotely); it will not try to substitute its outputs.
-    derivation; it will not try to substitute its outputs. This is
+    This is useful for derivations that are cheaper to build than to substitute.
-    useful for very trivial derivations (such as `writeText` in Nixpkgs)
-    that are cheaper to build than to substitute from a binary cache.
 
-    You may disable the effects of this attibute by enabling the
+    This attribute can be ignored by setting [`always-allow-substitutes`](@docroot@/command-ref/conf-file.md#conf-always-allow-substitutes) to `true`.
-    `always-allow-substitutes` configuration option in Nix.
 
     > **Note**
     >
-    > You need to have a builder configured which satisfies the
+    > If set to `false`, the [`builder`](./derivations.md#attr-builder) should be able to run on the system type specified in the [`system` attribute](./derivations.md#attr-system), since the derivation cannot be substituted.
-    > derivation’s `system` attribute, since the derivation cannot be
-    > substituted. Thus it is usually a good idea to align `system` with
-    > `builtins.currentSystem` when setting `allowSubstitutes` to
-    > `false`. For most trivial derivations this should be the case.
 
   - [`__structuredAttrs`]{#adv-attr-structuredAttrs}\
     If the special attribute `__structuredAttrs` is set to `true`, the other derivation

flake.nix

@@ -7,6 +7,10 @@
   # Also, do not grab arbitrary further staging commits. This PR was
   # carefully made to be based on release-23.05 and just contain
   # rebuild-causing changes to packages that Nix actually uses.
+  #
+  # Once this is updated to something containing
+  # https://github.com/NixOS/nixpkgs/pull/271423, don't forget
+  # to remove the `nix.checkAllErrors = false;` line in the tests.
   inputs.nixpkgs.url = "github:NixOS/nixpkgs/staging-23.05";
   inputs.nixpkgs-regression.url = "github:NixOS/nixpkgs/215d4d0fd80ca5163643b03a33fde804a29cc1e2";
   inputs.lowdown-src = { url = "github:kristapsdz/lowdown"; flake = false; };

src/libexpr/eval-settings.hh

@@ -68,6 +68,11 @@ struct EvalSettings : Config
           evaluation mode. For example, when set to
           `https://github.com/NixOS`, builtin functions such as `fetchGit` are
           allowed to access `https://github.com/NixOS/patchelf.git`.
+
+          Access is granted when
+          - the URI is equal to the prefix,
+          - or the URI is a subpath of the prefix,
+          - or the prefix is a URI scheme ended by a colon `:` and the URI has the same scheme.
         )"};
 
     Setting<bool> traceFunctionCalls{this, false, "trace-function-calls",

src/libexpr/eval.cc

@@ -18,6 +18,7 @@
 #include "memory-input-accessor.hh"
 #include "signals.hh"
 #include "gc-small-vector.hh"
+#include "url.hh"
 
 #include <algorithm>
 #include <chrono>
@@ -599,21 +600,45 @@ void EvalState::allowAndSetStorePathString(const StorePath & storePath, Value &
     mkStorePathString(storePath, v);
 }
 
+inline static bool isJustSchemePrefix(std::string_view prefix)
+{
+    return
+        !prefix.empty()
+        && prefix[prefix.size() - 1] == ':'
+        && isValidSchemeName(prefix.substr(0, prefix.size() - 1));
+}
+
+bool isAllowedURI(std::string_view uri, const Strings & allowedUris)
+{
+    /* 'uri' should be equal to a prefix, or in a subdirectory of a
+       prefix. Thus, the prefix https://github.co does not permit
+       access to https://github.com. */
+    for (auto & prefix : allowedUris) {
+        if (uri == prefix
+            // Allow access to subdirectories of the prefix.
+            || (uri.size() > prefix.size()
+                && prefix.size() > 0
+                && hasPrefix(uri, prefix)
+                && (
+                    // Allow access to subdirectories of the prefix.
+                    prefix[prefix.size() - 1] == '/'
+                    || uri[prefix.size()] == '/'
+
+                    // Allow access to whole schemes
+                    || isJustSchemePrefix(prefix)
+                    )
+                ))
+            return true;
+    }
+
+    return false;
+}
+
 void EvalState::checkURI(const std::string & uri)
 {
     if (!evalSettings.restrictEval) return;
 
-    /* 'uri' should be equal to a prefix, or in a subdirectory of a
+    if (isAllowedURI(uri, evalSettings.allowedUris.get())) return;
-       prefix. Thus, the prefix https://github.co does not permit
-       access to https://github.com. Note: this allows 'http://' and
-       'https://' as prefixes for any http/https URI. */
-    for (auto & prefix : evalSettings.allowedUris.get())
-        if (uri == prefix ||
-            (uri.size() > prefix.size()
-            && prefix.size() > 0
-            && hasPrefix(uri, prefix)
-            && (prefix[prefix.size() - 1] == '/' || uri[prefix.size()] == '/')))
-            return;
 
     /* If the URI is a path, then check it against allowedPaths as
        well. */

src/libexpr/eval.hh

@@ -335,11 +335,6 @@ private:
 
     std::map<std::string, std::optional<std::string>> searchPathResolved;
 
-    /**
-     * Cache used by checkSourcePath().
-     */
-    std::unordered_map<Path, SourcePath> resolvedPaths;
-
     /**
      * Cache used by prim_match().
      */
@@ -837,6 +832,11 @@ std::string showType(const Value & v);
  */
 SourcePath resolveExprPath(SourcePath path);
 
+/**
+ * Whether a URI is allowed, assuming restrictEval is enabled
+ */
+bool isAllowedURI(std::string_view uri, const Strings & allowedPaths);
+
 struct InvalidPathError : EvalError
 {
     Path path;

src/libexpr/flake/flakeref.cc

@@ -90,7 +90,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
         fragment = percentDecode(url.substr(fragmentStart+1));
     }
     if (pathEnd != std::string::npos && fragmentStart != std::string::npos) {
-        query = decodeQuery(url.substr(pathEnd+1, fragmentStart));
+        query = decodeQuery(url.substr(pathEnd+1, fragmentStart-pathEnd-1));
     }
 
     if (baseDir) {

src/libfetchers/input-accessor.hh

@@ -130,7 +130,7 @@ struct SourcePath
     { return accessor->getPhysicalPath(path); }
 
     std::string to_string() const
-    { return path.abs(); }
+    { return accessor->showPath(path); }
 
     /**
      * Append a `CanonPath` to this path.

src/libstore/build/derivation-goal.cc

@@ -196,10 +196,19 @@ void DerivationGoal::loadDerivation()
        things being garbage collected while we're busy. */
     worker.evalStore.addTempRoot(drvPath);
 
-    assert(worker.evalStore.isValidPath(drvPath));
+    /* Get the derivation. It is probably in the eval store, but it might be inthe main store:
 
-    /* Get the derivation. */
+         - Resolved derivation are resolved against main store realisations, and so must be stored there.
-    drv = std::make_unique<Derivation>(worker.evalStore.readDerivation(drvPath));
+
+         - Dynamic derivations are built, and so are found in the main store.
+     */
+    for (auto * drvStore : { &worker.evalStore, &worker.store }) {
+        if (drvStore->isValidPath(drvPath)) {
+            drv = std::make_unique<Derivation>(drvStore->readDerivation(drvPath));
+            break;
+        }
+    }
+    assert(drv);
 
     haveDerivation();
 }
@@ -401,10 +410,14 @@ void DerivationGoal::gaveUpOnSubstitution()
     }
 
     /* Copy the input sources from the eval store to the build
-       store. */
+       store.
+
+       Note that some inputs might not be in the eval store because they
+       are (resolved) derivation outputs in a resolved derivation. */
     if (&worker.evalStore != &worker.store) {
         RealisedPath::Set inputSrcs;
         for (auto & i : drv->inputSrcs)
+            if (worker.evalStore.isValidPath(i))
                 inputSrcs.insert(i);
         copyClosure(worker.evalStore, worker.store, inputSrcs);
     }
@@ -453,7 +466,7 @@ void DerivationGoal::repairClosure()
     std::map<StorePath, StorePath> outputsToDrv;
     for (auto & i : inputClosure)
         if (i.isDerivation()) {
-            auto depOutputs = worker.store.queryPartialDerivationOutputMap(i);
+            auto depOutputs = worker.store.queryPartialDerivationOutputMap(i, &worker.evalStore);
             for (auto & j : depOutputs)
                 if (j.second)
                     outputsToDrv.insert_or_assign(*j.second, i);
@@ -604,7 +617,13 @@ void DerivationGoal::inputsRealised()
                     return *outPath;
                 }
                 else {
-                    auto outMap = worker.evalStore.queryDerivationOutputMap(depDrvPath);
+                    auto outMap = [&]{
+                        for (auto * drvStore : { &worker.evalStore, &worker.store })
+                            if (drvStore->isValidPath(depDrvPath))
+                                return worker.store.queryDerivationOutputMap(depDrvPath, drvStore);
+                        assert(false);
+                    }();
+
                     auto outMapPath = outMap.find(outputName);
                     if (outMapPath == outMap.end()) {
                         throw Error(
@@ -1085,8 +1104,12 @@ void DerivationGoal::resolvedFinished()
                 auto newRealisation = realisation;
                 newRealisation.id = DrvOutput { initialOutput->outputHash, outputName };
                 newRealisation.signatures.clear();
-                if (!drv->type().isFixed())
+                if (!drv->type().isFixed()) {
-                    newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath);
+                    auto & drvStore = worker.evalStore.isValidPath(drvPath)
+                        ? worker.evalStore
+                        : worker.store;
+                    newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore);
+                }
                 signRealisation(newRealisation);
                 worker.store.registerDrvOutput(newRealisation);
             }
@@ -1379,7 +1402,10 @@ std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDeri
             res.insert_or_assign(name, output.path(worker.store, drv->name, name));
         return res;
     } else {
-        return worker.store.queryPartialDerivationOutputMap(drvPath);
+        for (auto * drvStore : { &worker.evalStore, &worker.store })
+            if (drvStore->isValidPath(drvPath))
+                return worker.store.queryPartialDerivationOutputMap(drvPath, drvStore);
+        assert(false);
     }
 }
 
@@ -1392,7 +1418,10 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap()
             res.insert_or_assign(name, *output.second);
         return res;
     } else {
-        return worker.store.queryDerivationOutputMap(drvPath);
+        for (auto * drvStore : { &worker.evalStore, &worker.store })
+            if (drvStore->isValidPath(drvPath))
+                return worker.store.queryDerivationOutputMap(drvPath, drvStore);
+        assert(false);
     }
 }
 

src/libstore/globals.hh

@@ -268,21 +268,16 @@ public:
     Setting<bool> alwaysAllowSubstitutes{
         this, false, "always-allow-substitutes",
         R"(
-          If set to `true`, Nix will ignore the `allowSubstitutes` attribute in
+          If set to `true`, Nix will ignore the [`allowSubstitutes`](@docroot@/language/advanced-attributes.md) attribute in derivations and always attempt to use [available substituters](#conf-substituters).
-          derivations and always attempt to use available substituters.
-          For more information on `allowSubstitutes`, see [the manual chapter on advanced attributes](../language/advanced-attributes.md).
         )"};
 
     Setting<bool> buildersUseSubstitutes{
         this, false, "builders-use-substitutes",
         R"(
-          If set to `true`, Nix will instruct remote build machines to use
+          If set to `true`, Nix will instruct [remote build machines](#conf-builders) to use their own [`substituters`](#conf-substituters) if available.
-          their own binary substitutes if available. In practical terms, this
+
-          means that remote hosts will fetch as many build dependencies as
+          It means that remote build hosts will fetch as many dependencies as possible from their own substituters (e.g, from `cache.nixos.org`) instead of waiting for the local machine to upload them all.
-          possible from their own substitutes (e.g, from `cache.nixos.org`),
+          This can drastically reduce build times if the network connection between the local machine and the remote build host is slow.
-          instead of waiting for this host to upload them all. This can
-          drastically reduce build times if the network connection between
-          this computer and the remote build host is slow.
         )"};
 
     Setting<off_t> reservedSize{this, 8 * 1024 * 1024, "gc-reserved-space",

src/libstore/misc.cc

@@ -331,8 +331,11 @@ std::map<DrvOutput, StorePath> drvOutputReferences(
 std::map<DrvOutput, StorePath> drvOutputReferences(
     Store & store,
     const Derivation & drv,
-    const StorePath & outputPath)
+    const StorePath & outputPath,
+    Store * evalStore_)
 {
+    auto & evalStore = evalStore_ ? *evalStore_ : store;
+
     std::set<Realisation> inputRealisations;
 
     std::function<void(const StorePath &, const DerivedPathMap<StringSet>::ChildNode &)> accumRealisations;
@@ -340,7 +343,7 @@ std::map<DrvOutput, StorePath> drvOutputReferences(
     accumRealisations = [&](const StorePath & inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode) {
         if (!inputNode.value.empty()) {
             auto outputHashes =
-                staticOutputHashes(store, store.readDerivation(inputDrv));
+                staticOutputHashes(evalStore, evalStore.readDerivation(inputDrv));
             for (const auto & outputName : inputNode.value) {
                 auto outputHash = get(outputHashes, outputName);
                 if (!outputHash)
@@ -362,7 +365,7 @@ std::map<DrvOutput, StorePath> drvOutputReferences(
                 SingleDerivedPath next = SingleDerivedPath::Built { d, outputName };
                 accumRealisations(
                     // TODO deep resolutions for dynamic derivations, issue #8947, would go here.
-                    resolveDerivedPath(store, next),
+                    resolveDerivedPath(store, next, evalStore_),
                     childNode);
             }
         }

src/libstore/remote-store.cc

@@ -225,7 +225,7 @@ StorePathSet RemoteStore::queryValidPaths(const StorePathSet & paths, Substitute
         conn->to << WorkerProto::Op::QueryValidPaths;
         WorkerProto::write(*this, *conn, paths);
         if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 27) {
-            conn->to << (settings.buildersUseSubstitutes ? 1 : 0);
+            conn->to << maybeSubstitute;
         }
         conn.processStderr();
         return WorkerProto::Serialise<StorePathSet>::read(*this, *conn);

src/libstore/store-api.hh

@@ -943,6 +943,7 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv);
 std::map<DrvOutput, StorePath> drvOutputReferences(
     Store & store,
     const Derivation & drv,
-    const StorePath & outputPath);
+    const StorePath & outputPath,
+    Store * evalStore = nullptr);
 
 }

src/libutil/url-parts.hh

@@ -8,7 +8,7 @@ namespace nix {
 
 // URI stuff.
 const static std::string pctEncoded = "(?:%[0-9a-fA-F][0-9a-fA-F])";
-const static std::string schemeRegex = "(?:[a-z][a-z0-9+.-]*)";
+const static std::string schemeNameRegex = "(?:[a-z][a-z0-9+.-]*)";
 const static std::string ipv6AddressSegmentRegex = "[0-9a-fA-F:]+(?:%\\w+)?";
 const static std::string ipv6AddressRegex = "(?:\\[" + ipv6AddressSegmentRegex + "\\]|" + ipv6AddressSegmentRegex + ")";
 const static std::string unreservedRegex = "(?:[a-zA-Z0-9-._~])";

src/libutil/url.cc

@@ -13,7 +13,7 @@ std::regex revRegex(revRegexS, std::regex::ECMAScript);
 ParsedURL parseURL(const std::string & url)
 {
     static std::regex uriRegex(
-        "((" + schemeRegex + "):"
+        "((" + schemeNameRegex + "):"
         + "(?:(?://(" + authorityRegex + ")(" + absPathRegex + "))|(/?" + pathRegex + ")))"
         + "(?:\\?(" + queryRegex + "))?"
         + "(?:#(" + queryRegex + "))?",
@@ -183,4 +183,12 @@ std::string fixGitURL(const std::string & url)
     }
 }
 
+// https://www.rfc-editor.org/rfc/rfc3986#section-3.1
+bool isValidSchemeName(std::string_view s)
+{
+    static std::regex regex(schemeNameRegex, std::regex::ECMAScript);
+
+    return std::regex_match(s.begin(), s.end(), regex, std::regex_constants::match_default);
+}
+
 }

src/libutil/url.hh

@@ -55,4 +55,13 @@ ParsedUrlScheme parseUrlScheme(std::string_view scheme);
    changes absolute paths into file:// URLs. */
 std::string fixGitURL(const std::string & url);
 
+/**
+ * Whether a string is valid as RFC 3986 scheme name.
+ * Colon `:` is part of the URI; not the scheme name, and therefore rejected.
+ * See https://www.rfc-editor.org/rfc/rfc3986#section-3.1
+ *
+ * Does not check whether the scheme is understood, as that's context-dependent.
+ */
+bool isValidSchemeName(std::string_view scheme);
+
 }

src/nix-build/nix-build.cc

@@ -462,7 +462,7 @@ static void main_nix_build(int argc, char * * argv)
         if (dryRun) return;
 
         if (shellDrv) {
-            auto shellDrvOutputs = store->queryPartialDerivationOutputMap(shellDrv.value());
+            auto shellDrvOutputs = store->queryPartialDerivationOutputMap(shellDrv.value(), &*evalStore);
             shell = store->printStorePath(shellDrvOutputs.at("out").value()) + "/bin/bash";
         }
 
@@ -515,7 +515,7 @@ static void main_nix_build(int argc, char * * argv)
             std::function<void(const StorePath &, const DerivedPathMap<StringSet>::ChildNode &)> accumInputClosure;
 
             accumInputClosure = [&](const StorePath & inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode) {
-                auto outputs = evalStore->queryPartialDerivationOutputMap(inputDrv);
+                auto outputs = store->queryPartialDerivationOutputMap(inputDrv, &*evalStore);
                 for (auto & i : inputNode.value) {
                     auto o = outputs.at(i);
                     store->computeFSClosure(*o, inputs);
@@ -653,7 +653,7 @@ static void main_nix_build(int argc, char * * argv)
             if (counter)
                 drvPrefix += fmt("-%d", counter + 1);
 
-            auto builtOutputs = evalStore->queryPartialDerivationOutputMap(drvPath);
+            auto builtOutputs = store->queryPartialDerivationOutputMap(drvPath, &*evalStore);
 
             auto maybeOutputPath = builtOutputs.at(outputName);
             assert(maybeOutputPath);

tests/functional/ca/eval-store.sh

@@ -0,0 +1,10 @@
+#!/usr/bin/env bash
+
+# Ensure that garbage collection works properly with ca derivations
+
+source common.sh
+
+export NIX_TESTS_CA_BY_DEFAULT=1
+
+cd ..
+source eval-store.sh

tests/functional/ca/local.mk

@@ -5,6 +5,7 @@ ca-tests := \
   $(d)/concurrent-builds.sh \
   $(d)/derivation-json.sh \
   $(d)/duplicate-realisation-in-closure.sh \
+  $(d)/eval-store.sh \
   $(d)/gc.sh \
   $(d)/import-derivation.sh \
   $(d)/new-build-cmd.sh \

tests/functional/eval-store.sh

@@ -11,7 +11,16 @@ rm -rf "$eval_store"
 
 nix build -f dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result"
 [[ -e $TEST_ROOT/result/foobar ]]
+if [[ ! -n "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then
+    # Resolved CA derivations are written to store for building
+    #
+    # TODO when we something more systematic
+    # (https://github.com/NixOS/nix/issues/5025) that distinguishes
+    # between scratch storage for building and the final destination
+    # store, we'll be able to make this unconditional again -- resolved
+    # derivations should only appear in the scratch store.
     (! ls $NIX_STORE_DIR/*.drv)
+fi
 ls $eval_store/nix/store/*.drv
 
 clearStore
@@ -26,5 +35,8 @@ rm -rf "$eval_store"
 
 nix-build dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result"
 [[ -e $TEST_ROOT/result/foobar ]]
+if [[ ! -n "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then
+    # See above
     (! ls $NIX_STORE_DIR/*.drv)
+fi
 ls $eval_store/nix/store/*.drv

tests/functional/flakes/flakes.sh

@@ -193,6 +193,14 @@ nix build -o "$TEST_ROOT/result" flake1
 nix build -o "$TEST_ROOT/result" "$flake1Dir"
 nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir"
 
+# Test explicit packages.default.
+nix build -o "$TEST_ROOT/result" "$flake1Dir#default"
+nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir#default"
+
+# Test explicit packages.default with query.
+nix build -o "$TEST_ROOT/result" "$flake1Dir?ref=HEAD#default"
+nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default"
+
 # Check that store symlinks inside a flake are not interpreted as flakes.
 nix build -o "$flake1Dir/result" "git+file://$flake1Dir"
 nix path-info "$flake1Dir/result"

tests/nixos/default.nix

@@ -10,6 +10,7 @@ let
     hostPkgs = nixpkgsFor.${system}.native;
     defaults = {
       nixpkgs.pkgs = nixpkgsFor.${system}.native;
+      nix.checkAllErrors = false;
     };
     _module.args.nixpkgs = nixpkgs;
   };

tests/unit/libexpr-support/tests/libexpr.hh

@@ -8,6 +8,7 @@
 #include "nixexpr.hh"
 #include "eval.hh"
 #include "eval-inline.hh"
+#include "eval-settings.hh"
 #include "store-api.hh"
 
 #include "tests/libstore.hh"
@@ -18,6 +19,7 @@ namespace nix {
             static void SetUpTestSuite() {
                 LibStoreTest::SetUpTestSuite();
                 initGC();
+                evalSettings.nixPath = {};
             }
 
         protected:

tests/unit/libexpr/eval.cc

@@ -0,0 +1,141 @@
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "eval.hh"
+#include "tests/libexpr.hh"
+
+namespace nix {
+
+TEST(nix_isAllowedURI, http_example_com) {
+    Strings allowed;
+    allowed.push_back("http://example.com");
+
+    ASSERT_TRUE(isAllowedURI("http://example.com", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.co", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.como", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.org", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.org/foo", allowed));
+}
+
+TEST(nix_isAllowedURI, http_example_com_foo) {
+    Strings allowed;
+    allowed.push_back("http://example.com/foo");
+
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.como", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.org/foo", allowed));
+    // Broken?
+    // ASSERT_TRUE(isAllowedURI("http://example.com/foo?ok=1", allowed));
+}
+
+TEST(nix_isAllowedURI, http) {
+    Strings allowed;
+    allowed.push_back("http://");
+
+    ASSERT_TRUE(isAllowedURI("http://", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com", allowed));
+    ASSERT_FALSE(isAllowedURI("/", allowed));
+    ASSERT_FALSE(isAllowedURI("https://", allowed));
+    ASSERT_FALSE(isAllowedURI("http:foo", allowed));
+}
+
+TEST(nix_isAllowedURI, https) {
+    Strings allowed;
+    allowed.push_back("https://");
+
+    ASSERT_TRUE(isAllowedURI("https://example.com", allowed));
+    ASSERT_TRUE(isAllowedURI("https://example.com/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com/https:", allowed));
+}
+
+TEST(nix_isAllowedURI, absolute_path) {
+    Strings allowed;
+    allowed.push_back("/var/evil"); // bad idea
+
+    ASSERT_TRUE(isAllowedURI("/var/evil", allowed));
+    ASSERT_TRUE(isAllowedURI("/var/evil/", allowed));
+    ASSERT_TRUE(isAllowedURI("/var/evil/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("/var/evil/foo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evi", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com/var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com//var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com//var/evil/foo", allowed));
+}
+
+TEST(nix_isAllowedURI, file_url) {
+    Strings allowed;
+    allowed.push_back("file:///var/evil"); // bad idea
+
+    ASSERT_TRUE(isAllowedURI("file:///var/evil", allowed));
+    ASSERT_TRUE(isAllowedURI("file:///var/evil/", allowed));
+    ASSERT_TRUE(isAllowedURI("file:///var/evil/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("file:///var/evil/foo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evi", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com/var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com//var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com//var/evil/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http:///var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://var/evil/", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///var/evi", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///var/evilo", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///var/evilo/", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///var/evilo/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///", allowed));
+    ASSERT_FALSE(isAllowedURI("file://", allowed));
+}
+
+TEST(nix_isAllowedURI, github_all) {
+    Strings allowed;
+    allowed.push_back("github:");
+    ASSERT_TRUE(isAllowedURI("github:", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar/feat-multi-bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar?ref=refs/heads/feat-multi-bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github://foo/bar", allowed));
+    ASSERT_FALSE(isAllowedURI("https://github:443/foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("file://github:foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///github:foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("github", allowed));
+}
+
+TEST(nix_isAllowedURI, github_org) {
+    Strings allowed;
+    allowed.push_back("github:foo");
+    ASSERT_FALSE(isAllowedURI("github:", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar/feat-multi-bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar?ref=refs/heads/feat-multi-bar", allowed));
+    ASSERT_FALSE(isAllowedURI("github://foo/bar", allowed));
+    ASSERT_FALSE(isAllowedURI("https://github:443/foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("file://github:foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///github:foo/bar/archive/master.tar.gz", allowed));
+}
+
+TEST(nix_isAllowedURI, non_scheme_colon) {
+    Strings allowed;
+    allowed.push_back("https://foo/bar:");
+    ASSERT_TRUE(isAllowedURI("https://foo/bar:", allowed));
+    ASSERT_TRUE(isAllowedURI("https://foo/bar:/baz", allowed));
+    ASSERT_FALSE(isAllowedURI("https://foo/bar:baz", allowed));
+}
+
+} // namespace nix

tests/unit/libexpr/primops.cc

@@ -1,6 +1,8 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "memory-input-accessor.hh"
+
 #include "tests/libexpr.hh"
 
 namespace nix {
@@ -148,10 +150,25 @@ namespace nix {
     }
 
     TEST_F(PrimOpTest, unsafeGetAttrPos) {
-        // The `y` attribute is at position
+        state.corepkgsFS->addFile(CanonPath("foo.nix"), "{ y = \"x\"; }");
-        const char* expr = "builtins.unsafeGetAttrPos \"y\" { y = \"x\"; }";
+
+        auto expr = "builtins.unsafeGetAttrPos \"y\" (import <nix/foo.nix>)";
         auto v = eval(expr);
-        ASSERT_THAT(v, IsNull());
+        ASSERT_THAT(v, IsAttrsOfSize(3));
+
+        auto file = v.attrs->find(createSymbol("file"));
+        ASSERT_NE(file, nullptr);
+        ASSERT_THAT(*file->value, IsString());
+        auto s = baseNameOf(file->value->string_view());
+        ASSERT_EQ(s, "foo.nix");
+
+        auto line = v.attrs->find(createSymbol("line"));
+        ASSERT_NE(line, nullptr);
+        ASSERT_THAT(*line->value, IsIntEq(1));
+
+        auto column = v.attrs->find(createSymbol("column"));
+        ASSERT_NE(column, nullptr);
+        ASSERT_THAT(*column->value, IsIntEq(3));
     }
 
     TEST_F(PrimOpTest, hasAttr) {

tests/unit/libutil/url.cc

@@ -344,4 +344,27 @@ namespace nix {
         ASSERT_EQ(percentDecode(e), s);
     }
 
+TEST(nix, isValidSchemeName) {
+    ASSERT_TRUE(isValidSchemeName("http"));
+    ASSERT_TRUE(isValidSchemeName("https"));
+    ASSERT_TRUE(isValidSchemeName("file"));
+    ASSERT_TRUE(isValidSchemeName("file+https"));
+    ASSERT_TRUE(isValidSchemeName("fi.le"));
+    ASSERT_TRUE(isValidSchemeName("file-ssh"));
+    ASSERT_TRUE(isValidSchemeName("file+"));
+    ASSERT_TRUE(isValidSchemeName("file."));
+    ASSERT_TRUE(isValidSchemeName("file1"));
+    ASSERT_FALSE(isValidSchemeName("file:"));
+    ASSERT_FALSE(isValidSchemeName("file/"));
+    ASSERT_FALSE(isValidSchemeName("+file"));
+    ASSERT_FALSE(isValidSchemeName(".file"));
+    ASSERT_FALSE(isValidSchemeName("-file"));
+    ASSERT_FALSE(isValidSchemeName("1file"));
+    // regex ok?
+    ASSERT_FALSE(isValidSchemeName("\nhttp"));
+    ASSERT_FALSE(isValidSchemeName("\nhttp\n"));
+    ASSERT_FALSE(isValidSchemeName("http\n"));
+    ASSERT_FALSE(isValidSchemeName("http "));
+}
+
 }