From b3fd7db63f78ec736238016745338277703ad1d5 Mon Sep 17 00:00:00 2001
From: Vladimir Kryachko <vkryachko@google.com>
Date: Mon, 16 Oct 2023 13:00:49 -0400
Subject: [PATCH] Detect cycles in flake follows.

This change results in an error thrown as opposed to segfaulting due to
stack overflow.

Fixes #9144
---
 src/libexpr/flake/lockfile.cc           | 22 ++++++++++++++---
 tests/functional/flakes/follow-paths.sh | 32 ++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/src/libexpr/flake/lockfile.cc b/src/libexpr/flake/lockfile.cc
index 3c202967a..68443211b 100644
--- a/src/libexpr/flake/lockfile.cc
+++ b/src/libexpr/flake/lockfile.cc
@@ -45,16 +45,26 @@ StorePath LockedNode::computeStorePath(Store & store) const
     return lockedRef.input.computeStorePath(store);
 }
 
-std::shared_ptr<Node> LockFile::findInput(const InputPath & path)
-{
+
+static std::shared_ptr<Node> doFind(const ref<Node>& root, const InputPath & path, std::vector<std::string>& visited) {
     auto pos = root;
 
+    auto pathS = printInputPath(path);
+    auto found = std::find(visited.cbegin(), visited.cend(), pathS);
+
+    if(found != visited.end()) {
+        std::vector cycle(found, visited.cend());
+        cycle.push_back(pathS);
+        throw Error("follow cycle detected: [%s]", concatStringsSep(" -> ", cycle));
+    }
+    visited.push_back(pathS);
+
     for (auto & elem : path) {
         if (auto i = get(pos->inputs, elem)) {
             if (auto node = std::get_if<0>(&*i))
                 pos = *node;
             else if (auto follows = std::get_if<1>(&*i)) {
-                if (auto p = findInput(*follows))
+                if (auto p = doFind(root, *follows, visited))
                     pos = ref(p);
                 else
                     return {};
@@ -66,6 +76,12 @@ std::shared_ptr<Node> LockFile::findInput(const InputPath & path)
     return pos;
 }
 
+std::shared_ptr<Node> LockFile::findInput(const InputPath & path)
+{
+    std::vector<std::string> visited;
+    return doFind(root, path, visited);
+}
+
 LockFile::LockFile(const nlohmann::json & json, const Path & path)
 {
     auto version = json.value("version", 0);
diff --git a/tests/functional/flakes/follow-paths.sh b/tests/functional/flakes/follow-paths.sh
index dc97027ac..8573b5511 100644
--- a/tests/functional/flakes/follow-paths.sh
+++ b/tests/functional/flakes/follow-paths.sh
@@ -167,7 +167,7 @@ nix flake lock "$flakeFollowsA" 2>&1 | grep "warning: input 'B' has an override
 #
 # The message was
 #    error: input 'B/D' follows a non-existent input 'B/C/D'
-# 
+#
 # Note that for `B` to resolve its follow for `D`, it needs `C/D`, for which it needs to resolve the follow on `C` first.
 flakeFollowsOverloadA="$TEST_ROOT/follows/overload/flakeA"
 flakeFollowsOverloadB="$TEST_ROOT/follows/overload/flakeA/flakeB"
@@ -230,3 +230,33 @@ git -C "$flakeFollowsOverloadA" add flake.nix flakeB/flake.nix \
 nix flake metadata "$flakeFollowsOverloadA"
 nix flake update "$flakeFollowsOverloadA"
 nix flake lock "$flakeFollowsOverloadA"
+
+# Now test follow cycle detection
+# We construct the following follows graph:
+#
+#    foo
+#    / ^
+#   /   \
+#  v     \
+# bar -> baz
+# The message was
+#     error: follow cycle detected: [baz -> foo -> bar -> baz]
+flakeFollowCycle="$TEST_ROOT/follows/followCycle"
+
+# Test following path flakerefs.
+mkdir -p "$flakeFollowCycle"
+
+cat > $flakeFollowCycle/flake.nix <<EOF
+{
+    description = "Flake A";
+    inputs = {
+        foo.follows = "bar";
+        bar.follows = "baz";
+        baz.follows = "foo";
+    };
+    outputs = { ... }: {};
+}
+EOF
+
+checkRes=$(nix flake lock "$flakeFollowCycle" 2>&1 && fail "nix flake lock should have failed." || true)
+echo $checkRes | grep -F "error: follow cycle detected: [baz -> foo -> bar -> baz]"