Merge pull request #10467 from edolstra/nix-shell-symlink

nix shell: Handle output paths that are symlinks

src/libstore/local-fs-store.cc

@@ -33,6 +33,10 @@ struct LocalStoreAccessor : PosixSourceAccessor
 
     std::optional<Stat> maybeLstat(const CanonPath & path) override
     {
+        /* Handle the case where `path` is (a parent of) the store. */
+        if (isDirOrInDir(store->storeDir, path.abs()))
+            return Stat{ .type = tDirectory };
+
         return PosixSourceAccessor::maybeLstat(toRealPath(path));
     }
 

src/libutil/source-accessor.cc

@@ -39,9 +39,9 @@ void SourceAccessor::readFile(
 }
 
 Hash SourceAccessor::hashPath(
-        const CanonPath & path,
-        PathFilter & filter,
-        HashAlgorithm ha)
+    const CanonPath & path,
+    PathFilter & filter,
+    HashAlgorithm ha)
 {
     HashSink sink(ha);
     dumpPath(path, sink, filter);
@@ -67,4 +67,42 @@ std::string SourceAccessor::showPath(const CanonPath & path)
     return displayPrefix + path.abs() + displaySuffix;
 }
 
+CanonPath SourceAccessor::resolveSymlinks(
+    const CanonPath & path,
+    SymlinkResolution mode)
+{
+    auto res = CanonPath::root;
+
+    int linksAllowed = 1024;
+
+    std::list<std::string> todo;
+    for (auto & c : path)
+        todo.push_back(std::string(c));
+
+    while (!todo.empty()) {
+        auto c = *todo.begin();
+        todo.pop_front();
+        if (c == "" || c == ".")
+            ;
+        else if (c == "..")
+            res.pop();
+        else {
+            res.push(c);
+            if (mode == SymlinkResolution::Full || !todo.empty()) {
+                if (auto st = maybeLstat(res); st && st->type == SourceAccessor::tSymlink) {
+                    if (!linksAllowed--)
+                        throw Error("infinite symlink recursion in path '%s'", showPath(path));
+                    auto target = readLink(res);
+                    res.pop();
+                    if (hasPrefix(target, "/"))
+                        res = CanonPath::root;
+                    todo.splice(todo.begin(), tokenizeString<std::list<std::string>>(target, "/"));
+                }
+            }
+        }
+    }
+
+    return res;
+}
+
 }

src/libutil/source-accessor.hh

@@ -9,6 +9,26 @@ namespace nix {
 
 struct Sink;
 
+/**
+ * Note there is a decent chance this type soon goes away because the problem is solved another way.
+ * See the discussion in https://github.com/NixOS/nix/pull/9985.
+ */
+enum class SymlinkResolution {
+    /**
+     * Resolve symlinks in the ancestors only.
+     *
+     * Only the last component of the result is possibly a symlink.
+     */
+    Ancestors,
+
+    /**
+     * Resolve symlinks fully, realpath(3)-style.
+     *
+     * No component of the result will be a symlink.
+     */
+    Full,
+};
+
 /**
  * A read-only filesystem abstraction. This is used by the Nix
  * evaluator and elsewhere for accessing sources in various
@@ -112,9 +132,9 @@ struct SourceAccessor
         PathFilter & filter = defaultPathFilter);
 
     Hash hashPath(
-            const CanonPath & path,
-            PathFilter & filter = defaultPathFilter,
-            HashAlgorithm ha = HashAlgorithm::SHA256);
+        const CanonPath & path,
+        PathFilter & filter = defaultPathFilter,
+        HashAlgorithm ha = HashAlgorithm::SHA256);
 
     /**
      * Return a corresponding path in the root filesystem, if
@@ -137,6 +157,17 @@ struct SourceAccessor
     void setPathDisplay(std::string displayPrefix, std::string displaySuffix = "");
 
     virtual std::string showPath(const CanonPath & path);
+
+    /**
+     * Resolve any symlinks in `path` according to the given
+     * resolution mode.
+     *
+     * @param mode might only be a temporary solution for this.
+     * See the discussion in https://github.com/NixOS/nix/pull/9985.
+     */
+    CanonPath resolveSymlinks(
+        const CanonPath & path,
+        SymlinkResolution mode = SymlinkResolution::Full);
 };
 
 }

src/libutil/source-path.cc

@@ -62,44 +62,6 @@ bool SourcePath::operator<(const SourcePath & x) const
     return std::tie(*accessor, path) < std::tie(*x.accessor, x.path);
 }
 
-SourcePath SourcePath::resolveSymlinks(SymlinkResolution mode) const
-{
-    auto res = SourcePath(accessor);
-
-    int linksAllowed = 1024;
-
-    std::list<std::string> todo;
-    for (auto & c : path)
-        todo.push_back(std::string(c));
-
-    bool resolve_last = mode == SymlinkResolution::Full;
-
-    while (!todo.empty()) {
-        auto c = *todo.begin();
-        todo.pop_front();
-        if (c == "" || c == ".")
-            ;
-        else if (c == "..")
-            res.path.pop();
-        else {
-            res.path.push(c);
-            if (resolve_last || !todo.empty()) {
-                if (auto st = res.maybeLstat(); st && st->type == InputAccessor::tSymlink) {
-                    if (!linksAllowed--)
-                        throw Error("infinite symlink recursion in path '%s'", path);
-                    auto target = res.readLink();
-                    res.path.pop();
-                    if (hasPrefix(target, "/"))
-                        res.path = CanonPath::root;
-                    todo.splice(todo.begin(), tokenizeString<std::list<std::string>>(target, "/"));
-                }
-            }
-        }
-    }
-
-    return res;
-}
-
 std::ostream & operator<<(std::ostream & str, const SourcePath & path)
 {
     str << path.to_string();

src/libutil/source-path.hh

@@ -11,26 +11,6 @@
 
 namespace nix {
 
-/**
- * Note there is a decent chance this type soon goes away because the problem is solved another way.
- * See the discussion in https://github.com/NixOS/nix/pull/9985.
- */
-enum class SymlinkResolution {
-    /**
-     * Resolve symlinks in the ancestors only.
-     *
-     * Only the last component of the result is possibly a symlink.
-     */
-    Ancestors,
-
-    /**
-     * Resolve symlinks fully, realpath(3)-style.
-     *
-     * No component of the result will be a symlink.
-     */
-    Full,
-};
-
 /**
  * An abstraction for accessing source files during
  * evaluation. Currently, it's just a wrapper around `CanonPath` that
@@ -123,14 +103,13 @@ struct SourcePath
     bool operator<(const SourcePath & x) const;
 
     /**
-     * Resolve any symlinks in this `SourcePath` according to the
-     * given resolution mode.
-     *
-     * @param mode might only be a temporary solution for this. 
-     * See the discussion in https://github.com/NixOS/nix/pull/9985.
+     * Convenience wrapper around `SourceAccessor::resolveSymlinks()`.
      */
     SourcePath resolveSymlinks(
-        SymlinkResolution mode = SymlinkResolution::Full) const;
+        SymlinkResolution mode = SymlinkResolution::Full) const
+    {
+        return {accessor, accessor->resolveSymlinks(path, mode)};
+    }
 };
 
 std::ostream & operator << (std::ostream & str, const SourcePath & path);

src/nix/unix/run.cc

@@ -124,7 +124,8 @@ struct CmdShell : InstallablesCommand, MixEnvironment
             if (true)
                 pathAdditions.push_back(store->printStorePath(path) + "/bin");
 
-            auto propPath = CanonPath(store->printStorePath(path)) / "nix-support" / "propagated-user-env-packages";
+            auto propPath = accessor->resolveSymlinks(
+                CanonPath(store->printStorePath(path)) / "nix-support" / "propagated-user-env-packages");
             if (auto st = accessor->maybeLstat(propPath); st && st->type == SourceAccessor::tRegular) {
                 for (auto & p : tokenizeString<Paths>(accessor->readFile(propPath)))
                     todo.push(store->parseStorePath(p));

tests/functional/shell-hello.nix

@@ -1,6 +1,6 @@
 with import ./config.nix;
 
-{
+rec {
   hello = mkDerivation {
     name = "hello";
     outputs = [ "out" "dev" ];
@@ -24,6 +24,22 @@ with import ./config.nix;
       '';
   };
 
+  hello-symlink = mkDerivation {
+    name = "hello-symlink";
+    buildCommand =
+      ''
+        ln -s ${hello} $out
+      '';
+  };
+
+  forbidden-symlink = mkDerivation {
+    name = "forbidden-symlink";
+    buildCommand =
+      ''
+        ln -s /tmp/foo/bar $out
+      '';
+  };
+
   salve-mundi = mkDerivation {
     name = "salve-mundi";
     outputs = [ "out" ];

tests/functional/shell.sh

@@ -10,6 +10,11 @@ nix shell -f shell-hello.nix hello -c hello NixOS | grep 'Hello NixOS'
 nix shell -f shell-hello.nix hello^dev -c hello2 | grep 'Hello2'
 nix shell -f shell-hello.nix 'hello^*' -c hello2 | grep 'Hello2'
 
+# Test output paths that are a symlink.
+nix shell -f shell-hello.nix hello-symlink -c hello | grep 'Hello World'
+
+# Test that symlinks outside of the store don't work.
+expect 1 nix shell -f shell-hello.nix forbidden-symlink -c hello 2>&1 | grepQuiet "is not in the Nix store"
 
 if isDaemonNewer "2.20.0pre20231220"; then
     # Test that command line attribute ordering is reflected in the PATH