From 5a1824ebe1fcdeff86b57a13a33d6428e89e4bce Mon Sep 17 00:00:00 2001
From: Pierre Bourdon <delroth@gmail.com>
Date: Fri, 17 May 2024 02:02:48 +0200
Subject: [PATCH] derived-path: refuse built derived path with a non-derivation
 base

Example: /nix/store/dr53sp25hyfsnzjpm8mh3r3y36vrw3ng-neovim-0.9.5^out

This is nonsensical since selecting outputs can only be done for a
buildable derivation, not for a realised store path. The build worker
side of things ends up crashing with an assertion when trying to handle
such malformed paths.

Change-Id: Ia3587c71fe3da5bea45d4e506e1be4dd62291ddf
---
 src/libstore/derived-path.cc        | 46 +++++++++++++----------------
 tests/unit/libstore/derived-path.cc |  9 ++++++
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc
index 214caab54..22977c7b1 100644
--- a/src/libstore/derived-path.cc
+++ b/src/libstore/derived-path.cc
@@ -185,21 +185,32 @@ DerivedPath::Built DerivedPath::Built::parse(
     };
 }
 
-static SingleDerivedPath parseWithSingle(
+template <typename DerivedPathT>
+static DerivedPathT parseDerivedPath(
     const Store & store, std::string_view s, std::string_view separator,
     const ExperimentalFeatureSettings & xpSettings)
 {
     size_t n = s.rfind(separator);
-    return n == s.npos
-        ? (SingleDerivedPath) SingleDerivedPath::Opaque::parse(store, s)
-        : (SingleDerivedPath) SingleDerivedPath::Built::parse(store,
-            make_ref<SingleDerivedPath>(parseWithSingle(
+    if (n == s.npos) {
+        return DerivedPathT::Opaque::parse(store, s);
+    } else {
+        auto path = DerivedPathT::Built::parse(store,
+            make_ref<SingleDerivedPath>(parseDerivedPath<SingleDerivedPath>(
                 store,
                 s.substr(0, n),
                 separator,
                 xpSettings)),
             s.substr(n + 1),
             xpSettings);
+
+        const auto& basePath = path.getBaseStorePath();
+        if (!basePath.isDerivation()) {
+            throw InvalidPath("cannot use output selection ('%s') on non-derivation store path '%s'",
+                              separator, basePath.to_string());
+        }
+
+        return path;
+    }
 }
 
 SingleDerivedPath SingleDerivedPath::parse(
@@ -207,7 +218,7 @@ SingleDerivedPath SingleDerivedPath::parse(
     std::string_view s,
     const ExperimentalFeatureSettings & xpSettings)
 {
-    return parseWithSingle(store, s, "^", xpSettings);
+    return parseDerivedPath<SingleDerivedPath>(store, s, "^", xpSettings);
 }
 
 SingleDerivedPath SingleDerivedPath::parseLegacy(
@@ -215,24 +226,7 @@ SingleDerivedPath SingleDerivedPath::parseLegacy(
     std::string_view s,
     const ExperimentalFeatureSettings & xpSettings)
 {
-    return parseWithSingle(store, s, "!", xpSettings);
-}
-
-static DerivedPath parseWith(
-    const Store & store, std::string_view s, std::string_view separator,
-    const ExperimentalFeatureSettings & xpSettings)
-{
-    size_t n = s.rfind(separator);
-    return n == s.npos
-        ? (DerivedPath) DerivedPath::Opaque::parse(store, s)
-        : (DerivedPath) DerivedPath::Built::parse(store,
-            make_ref<SingleDerivedPath>(parseWithSingle(
-                store,
-                s.substr(0, n),
-                separator,
-                xpSettings)),
-            s.substr(n + 1),
-            xpSettings);
+    return parseDerivedPath<SingleDerivedPath>(store, s, "!", xpSettings);
 }
 
 DerivedPath DerivedPath::parse(
@@ -240,7 +234,7 @@ DerivedPath DerivedPath::parse(
     std::string_view s,
     const ExperimentalFeatureSettings & xpSettings)
 {
-    return parseWith(store, s, "^", xpSettings);
+    return parseDerivedPath<DerivedPath>(store, s, "^", xpSettings);
 }
 
 DerivedPath DerivedPath::parseLegacy(
@@ -248,7 +242,7 @@ DerivedPath DerivedPath::parseLegacy(
     std::string_view s,
     const ExperimentalFeatureSettings & xpSettings)
 {
-    return parseWith(store, s, "!", xpSettings);
+    return parseDerivedPath<DerivedPath>(store, s, "!", xpSettings);
 }
 
 DerivedPath DerivedPath::fromSingle(const SingleDerivedPath & req)
diff --git a/tests/unit/libstore/derived-path.cc b/tests/unit/libstore/derived-path.cc
index c62d79a78..ffa541e9f 100644
--- a/tests/unit/libstore/derived-path.cc
+++ b/tests/unit/libstore/derived-path.cc
@@ -77,6 +77,15 @@ TEST_F(DerivedPathTest, built_built_xp) {
         MissingExperimentalFeature);
 }
 
+/**
+ * Built paths with a non-derivation base should fail parsing.
+ */
+TEST_F(DerivedPathTest, non_derivation_base) {
+    ASSERT_THROW(
+        DerivedPath::parse(*store, "/nix/store/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x^foo"),
+        InvalidPath);
+}
+
 #ifndef COVERAGE
 
 RC_GTEST_FIXTURE_PROP(