Fix follows paths in subordinate lockfiles

4 files changed, 82 insertions, 20 deletions

diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc
index a2f100cbd..a380ae27f 100644
--- a/src/libexpr/flake/flake.cc
+++ b/src/libexpr/flake/flake.cc
@@ -329,21 +329,22 @@ LockedFlake lockFlake(
             const FlakeInputs & flakeInputs,
             std::shared_ptr<Node> node,
             const InputPath & inputPathPrefix,
-            std::shared_ptr<const Node> oldNode)>
+            std::shared_ptr<const Node> oldNode,
+            const LockParent parent, const Path parentPath)>
             computeLocks;
 
         computeLocks = [&](
             const FlakeInputs & flakeInputs,
             std::shared_ptr<Node> node,
             const InputPath & inputPathPrefix,
-            std::shared_ptr<const Node> oldNode)
+            std::shared_ptr<const Node> oldNode,
+            const LockParent parent, const Path parentPath)
         {
             debug("computing lock file node '%s'", printInputPath(inputPathPrefix));
 
             /* Get the overrides (i.e. attributes of the form
                'inputs.nixops.inputs.nixpkgs.url = ...'). */
-            // FIXME: check this
-            for (auto & [id, input] : flake.inputs) {
+            for (auto & [id, input] : flakeInputs) {
                 for (auto & [idOverride, inputOverride] : input.overrides) {
                     auto inputPath(inputPathPrefix);
                     inputPath.push_back(id);
@@ -379,15 +380,23 @@ LockedFlake lockFlake(
                        path we haven't processed yet. */
                     if (input.follows) {
                         InputPath target;
-                        if (hasOverride || input.absolute)
-                            /* 'follows' from an override is relative to the
-                               root of the graph. */
+
+                        if (parent.absolute && !hasOverride) {
                             target = *input.follows;
-                        else {
-                            /* Otherwise, it's relative to the current flake. */
-                            target = inputPathPrefix;
+                        } else {
+                            if (hasOverride)
+                            {
+                                target = inputPathPrefix;
+                                target.pop_back();
+                            }
+                            else
+                            {
+                                target = parent.path;
+                            }
+
                             for (auto & i : *input.follows) target.push_back(i);
                         }
+
                         debug("input '%s' follows '%s'", inputPathS, printInputPath(target));
                         node->inputs.insert_or_assign(id, target);
                         continue;
@@ -433,7 +442,7 @@ LockedFlake lockFlake(
                         if (hasChildUpdate) {
                             auto inputFlake = getFlake(
                                 state, oldLock->lockedRef, false, flakeCache);
-                            computeLocks(inputFlake.inputs, childNode, inputPath, oldLock);
+                            computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, parent, parentPath);
                         } else {
                             /* No need to fetch this flake, we can be
                                lazy. However there may be new overrides on the
@@ -450,12 +459,11 @@ LockedFlake lockFlake(
                                 } else if (auto follows = std::get_if<1>(&i.second)) {
                                     fakeInputs.emplace(i.first, FlakeInput {
                                         .follows = *follows,
-                                        .absolute = true
                                     });
                                 }
                             }
 
-                            computeLocks(fakeInputs, childNode, inputPath, oldLock);
+                            computeLocks(fakeInputs, childNode, inputPath, oldLock, parent, parentPath);
                         }
 
                     } else {
@@ -467,7 +475,18 @@ LockedFlake lockFlake(
                             throw Error("cannot update flake input '%s' in pure mode", inputPathS);
 
                         if (input.isFlake) {
-                            auto inputFlake = getFlake(state, *input.ref, useRegistries, flakeCache);
+                            Path localPath = parentPath;
+                            FlakeRef localRef = *input.ref;
+
+                            // If this input is a path, recurse it down.
+                            // This allows us to resolve path inputs relative to the current flake.
+                            if (localRef.input.getType() == "path")
+                            {
+                                localRef.input.parent = parentPath;
+                                localPath = canonPath(parentPath + "/" + *input.ref->input.getSourcePath());
+                            }
+
+                            auto inputFlake = getFlake(state, localRef, useRegistries, flakeCache);
 
                             /* Note: in case of an --override-input, we use
                                the *original* ref (input2.ref) for the
@@ -488,6 +507,13 @@ LockedFlake lockFlake(
                             parents.push_back(*input.ref);
                             Finally cleanup([&]() { parents.pop_back(); });
 
+                            // Follows paths from existing inputs in the top-level lockfile are absolute,
+                            // whereas paths in subordinate lockfiles are relative to those lockfiles.
+                            LockParent newParent {
+                                .path = inputPath,
+                                .absolute = oldLock ? true : false
+                            };
+
                             /* Recursively process the inputs of this
                                flake. Also, unless we already have this flake
                                in the top-level lock file, use this flake's
@@ -497,7 +523,8 @@ LockedFlake lockFlake(
                                 oldLock
                                 ? std::dynamic_pointer_cast<const Node>(oldLock)
                                 : LockFile::read(
-                                    inputFlake.sourceInfo->actualPath + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root);
+                                    inputFlake.sourceInfo->actualPath + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root,
+                                newParent, localPath);
                         }
 
                         else {
@@ -515,9 +542,17 @@ LockedFlake lockFlake(
             }
         };
 
+        LockParent parent {
+            .path = {},
+            .absolute = true
+        };
+
+        // Bring in the current ref for relative path resolution if we have it
+        auto parentPath = canonPath(flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir);
+
         computeLocks(
             flake.inputs, newLockFile.root, {},
-            lockFlags.recreateLockFile ? nullptr : oldLockFile.root);
+            lockFlags.recreateLockFile ? nullptr : oldLockFile.root, parent, parentPath);
 
         for (auto & i : lockFlags.inputOverrides)
             if (!overridesUsed.count(i.first))
diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh
index 15fd394f8..937cc021b 100644
--- a/src/libexpr/flake/flake.hh
+++ b/src/libexpr/flake/flake.hh
@@ -43,7 +43,6 @@ struct FlakeInput
     std::optional<FlakeRef> ref;
     bool isFlake = true;  // true = process flake to get outputs, false = (fetched) static source path
     std::optional<InputPath> follows;
-    bool absolute = false; // whether 'follows' is relative to the flake root
     FlakeInputs overrides;
 };
 
@@ -125,6 +124,15 @@ struct LockFlags
     std::set<InputPath> inputUpdates;
 };
 
+struct LockParent {
+    /* The path to this parent */
+    InputPath path;
+
+    /* Whether we are currently inside a top-level lockfile (inputs absolute)
+       or subordinate lockfile (inputs relative) */
+    bool absolute;
+};
+
 LockedFlake lockFlake(
     EvalState & state,
     const FlakeRef & flakeRef,
diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh
index c839cf23b..c43b047a7 100644
--- a/src/libfetchers/fetchers.hh
+++ b/src/libfetchers/fetchers.hh
@@ -38,6 +38,9 @@ struct Input
     bool immutable = false;
     bool direct = true;
 
+    /* path of the parent of this input, used for relative path resolution */
+    std::optional<Path> parent;
+
 public:
     static Input fromURL(const std::string & url);
 
diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc
index d1003de57..c4977834d 100644
--- a/src/libfetchers/path.cc
+++ b/src/libfetchers/path.cc
@@ -82,18 +82,34 @@ struct PathInputScheme : InputScheme
 
     std::pair<Tree, Input> fetch(ref<Store> store, const Input & input) override
     {
+        std::string absPath;
         auto path = getStrAttr(input.attrs, "path");
 
-        // FIXME: check whether access to 'path' is allowed.
+        if (path[0] != '/' && input.parent)
+        {
+            auto parent = canonPath(*input.parent);
+
+            // the path isn't relative, prefix it
+            absPath = canonPath(parent + "/" + path);
 
-        auto storePath = store->maybeParseStorePath(path);
+            // for security, ensure that if the parent is a store path, it's inside it
+            if (!parent.rfind(store->storeDir, 0) && absPath.rfind(store->storeDir, 0))
+                throw BadStorePath("relative path '%s' points outside of its parent's store path %s, this is a security violation", path, parent);
+        }
+        else
+        {
+            absPath = path;
+        }
+
+        // FIXME: check whether access to 'path' is allowed.
+        auto storePath = store->maybeParseStorePath(absPath);
 
         if (storePath)
             store->addTempRoot(*storePath);
 
         if (!storePath || storePath->name() != "source" || !store->isValidPath(*storePath))
             // FIXME: try to substitute storePath.
-            storePath = store->addToStore("source", path);
+            storePath = store->addToStore("source", absPath);
 
         return {
             Tree(store->toRealPath(*storePath), std::move(*storePath)),