diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 7e3861e2f..415d63d87 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -116,7 +116,11 @@ MixFlakeOptions::MixFlakeOptions() .labels = {"input-path"}, .handler = {[&](std::string s) { warn("'--update-input' is a deprecated alias for 'flake update' and will be removed in a future version."); - lockFlags.inputUpdates.insert(flake::parseInputAttrPath(s)); + auto path = flake::NonEmptyInputAttrPath::parse(s); + if (!path) + throw UsageError( + "--update-input was passed a zero-length input path, which would refer to the flake itself, not an input"); + lockFlags.inputUpdates.insert(*path); }}, .completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) { completeFlakeInputAttrPath(completions, getEvalState(), getFlakeRefsForCompletion(), prefix); @@ -125,14 +129,18 @@ MixFlakeOptions::MixFlakeOptions() addFlag({ .longName = "override-input", - .description = "Override a specific flake input (e.g. `dwarffs/nixpkgs`). This implies `--no-write-lock-file`.", + .description = + "Override a specific flake input (e.g. `dwarffs/nixpkgs`). The input path must not be empty. This implies `--no-write-lock-file`.", .category = category, .labels = {"input-path", "flake-url"}, .handler = {[&](std::string inputAttrPath, std::string flakeRef) { lockFlags.writeLockFile = false; + auto path = flake::NonEmptyInputAttrPath::parse(inputAttrPath); + if (!path) + throw UsageError( + "--override-input was passed a zero-length input path, which would refer to the flake itself, not an input"); lockFlags.inputOverrides.insert_or_assign( - flake::parseInputAttrPath(inputAttrPath), - parseFlakeRef(fetchSettings, flakeRef, absPath(getCommandBaseDir()).string(), true)); + std::move(*path), parseFlakeRef(fetchSettings, flakeRef, absPath(getCommandBaseDir()).string(), true)); }}, .completer = {[&](AddCompletions & completions, size_t n, std::string_view prefix) { if (n == 0) { diff --git a/src/libflake-c/nix_api_flake.cc b/src/libflake-c/nix_api_flake.cc index 32329585a..990aa66fc 100644 --- a/src/libflake-c/nix_api_flake.cc +++ b/src/libflake-c/nix_api_flake.cc @@ -162,8 +162,11 @@ nix_err nix_flake_lock_flags_add_input_override( { nix_clear_err(context); try { - auto path = nix::flake::parseInputAttrPath(inputPath); - flags->lockFlags->inputOverrides.emplace(path, *flakeRef->flakeRef); + auto path = nix::flake::NonEmptyInputAttrPath::parse(inputPath); + if (!path) + throw nix::UsageError( + "input override path cannot be zero-length; it would refer to the flake itself, not an input"); + flags->lockFlags->inputOverrides.emplace(std::move(*path), *flakeRef->flakeRef); if (flags->lockFlags->writeLockFile) { return nix_flake_lock_flags_set_mode_virtual(context, flags); } diff --git a/src/libflake-c/nix_api_flake.h b/src/libflake-c/nix_api_flake.h index a1a7060a6..a3221e676 100644 --- a/src/libflake-c/nix_api_flake.h +++ b/src/libflake-c/nix_api_flake.h @@ -160,8 +160,9 @@ nix_err nix_flake_lock_flags_set_mode_write_as_needed(nix_c_context * context, n * @brief Add input overrides to the lock flags * @param[out] context Optional, stores error information * @param[in] flags The flags to modify - * @param[in] inputPath The input path to override + * @param[in] inputPath The input path to override (must not be empty) * @param[in] flakeRef The flake reference to use as the override + * @return NIX_ERR_NIX_ERROR if inputPath is empty * * This switches the `flags` to `nix_flake_lock_flags_set_mode_virtual` if not in mode * `nix_flake_lock_flags_set_mode_check`. diff --git a/src/libflake-tests/nix_api_flake.cc b/src/libflake-tests/nix_api_flake.cc index ec690b812..45f3d9b29 100644 --- a/src/libflake-tests/nix_api_flake.cc +++ b/src/libflake-tests/nix_api_flake.cc @@ -384,4 +384,62 @@ TEST_F(nix_api_store_test, nix_api_load_flake_with_flags) nix_flake_settings_free(settings); } +TEST_F(nix_api_store_test, nix_api_flake_lock_flags_add_input_override_empty_path) +{ + auto tmpDir = nix::createTempDir(); + nix::AutoDelete delTmpDir(tmpDir, true); + + nix::writeFile(tmpDir / "flake.nix", R"( + { + outputs = { ... }: { }; + } + )"); + + nix_libstore_init(ctx); + assert_ctx_ok(); + + auto fetchSettings = nix_fetchers_settings_new(ctx); + assert_ctx_ok(); + ASSERT_NE(nullptr, fetchSettings); + + auto settings = nix_flake_settings_new(ctx); + assert_ctx_ok(); + ASSERT_NE(nullptr, settings); + + auto lockFlags = nix_flake_lock_flags_new(ctx, settings); + assert_ctx_ok(); + ASSERT_NE(nullptr, lockFlags); + + auto parseFlags = nix_flake_reference_parse_flags_new(ctx, settings); + assert_ctx_ok(); + ASSERT_NE(nullptr, parseFlags); + + auto r0 = nix_flake_reference_parse_flags_set_base_directory( + ctx, parseFlags, tmpDir.string().c_str(), tmpDir.string().size()); + assert_ctx_ok(); + ASSERT_EQ(NIX_OK, r0); + + nix_flake_reference * flakeReference = nullptr; + std::string fragment; + nix_flake_reference_and_fragment_from_string( + ctx, fetchSettings, settings, parseFlags, ".", 1, &flakeReference, OBSERVE_STRING(fragment)); + assert_ctx_ok(); + ASSERT_NE(nullptr, flakeReference); + + // Test that empty input path is rejected (issue #14816) + auto r = nix_flake_lock_flags_add_input_override(ctx, lockFlags, "", flakeReference); + ASSERT_EQ(NIX_ERR_NIX_ERROR, r); + assert_ctx_err(); + + // Verify error message contains expected text + const char * errMsg = nix_err_msg(nullptr, ctx, nullptr); + ASSERT_NE(nullptr, errMsg); + ASSERT_NE(std::string(errMsg).find("input override path cannot be zero-length"), std::string::npos); + + nix_flake_reference_free(flakeReference); + nix_flake_reference_parse_flags_free(parseFlags); + nix_flake_lock_flags_free(lockFlags); + nix_flake_settings_free(settings); +} + } // namespace nixC diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index 9f7476bd0..7fd5e0c06 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -451,9 +451,10 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, std::optional parentInputAttrPath; // FIXME: rename to inputAttrPathPrefix? }; - std::map overrides; - std::set explicitCliOverrides; - std::set overridesUsed, updatesUsed; + std::map overrides; + std::set explicitCliOverrides; + std::set overridesUsed; + std::set updatesUsed; std::map, SourcePath> nodePaths; for (auto & i : lockFlags.inputOverrides) { @@ -510,8 +511,7 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, auto addOverrides = [&](this const auto & addOverrides, const FlakeInput & input, const InputAttrPath & prefix) -> void { for (auto & [idOverride, inputOverride] : input.overrides) { - auto inputAttrPath(prefix); - inputAttrPath.push_back(idOverride); + auto inputAttrPath = NonEmptyInputAttrPath::append(prefix, idOverride); if (inputOverride.ref || inputOverride.follows) overrides.emplace( inputAttrPath, @@ -532,9 +532,8 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, /* Check whether this input has overrides for a non-existent input. */ for (auto [inputAttrPath, inputOverride] : overrides) { - auto inputAttrPath2(inputAttrPath); - auto follow = inputAttrPath2.back(); - inputAttrPath2.pop_back(); + auto follow = inputAttrPath.inputName(); + auto inputAttrPath2 = inputAttrPath.parent(); if (inputAttrPath2 == inputAttrPathPrefix && !flakeInputs.count(follow)) warn( "input '%s' has an override for a non-existent input '%s'", @@ -546,8 +545,8 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, necessary (i.e. if they're new or the flakeref changed from what's in the lock file). */ for (auto & [id, input2] : flakeInputs) { - auto inputAttrPath(inputAttrPathPrefix); - inputAttrPath.push_back(id); + auto nonEmptyInputAttrPath = NonEmptyInputAttrPath::append(inputAttrPathPrefix, id); + auto inputAttrPath = nonEmptyInputAttrPath.get(); auto inputAttrPathS = printInputAttrPath(inputAttrPath); debug("computing input '%s'", inputAttrPathS); @@ -555,11 +554,11 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, /* Do we have an override for this input from one of the ancestors? */ - auto i = overrides.find(inputAttrPath); + auto i = overrides.find(nonEmptyInputAttrPath); bool hasOverride = i != overrides.end(); - bool hasCliOverride = explicitCliOverrides.contains(inputAttrPath); + bool hasCliOverride = explicitCliOverrides.contains(nonEmptyInputAttrPath); if (hasOverride) - overridesUsed.insert(inputAttrPath); + overridesUsed.insert(nonEmptyInputAttrPath); auto input = hasOverride ? i->second.input : input2; /* Resolve relative 'path:' inputs relative to @@ -618,7 +617,7 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, updatesUsed.insert(inputAttrPath); - if (oldNode && !lockFlags.inputUpdates.count(inputAttrPath)) + if (oldNode && !lockFlags.inputUpdates.count(nonEmptyInputAttrPath)) if (auto oldLock2 = get(oldNode->inputs, id)) if (auto oldLock3 = std::get_if<0>(&*oldLock2)) oldLock = *oldLock3; @@ -637,10 +636,10 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, /* If we have this input in updateInputs, then we must fetch the flake to update it. */ - auto lb = lockFlags.inputUpdates.lower_bound(inputAttrPath); + auto lb = lockFlags.inputUpdates.lower_bound(nonEmptyInputAttrPath); - auto mustRefetch = lb != lockFlags.inputUpdates.end() && lb->size() > inputAttrPath.size() - && std::equal(inputAttrPath.begin(), inputAttrPath.end(), lb->begin()); + auto mustRefetch = lb != lockFlags.inputUpdates.end() && lb->get().size() > inputAttrPath.size() + && std::equal(inputAttrPath.begin(), inputAttrPath.end(), lb->get().begin()); FlakeInputs fakeInputs; @@ -662,8 +661,8 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, // It is possible that the flake has changed, // so we must confirm all the follows that are in the lock file are also in the // flake. - auto overridePath(inputAttrPath); - overridePath.push_back(i.first); + auto overridePath = + NonEmptyInputAttrPath::append(nonEmptyInputAttrPath, i.first); auto o = overrides.find(overridePath); // If the override disappeared, we have to refetch the flake, // since some of the inputs may not be present in the lock file. @@ -717,7 +716,7 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef, nuked the next time we update the lock file. That is, overrides are sticky unless you use --no-write-lock-file. */ - auto inputIsOverride = explicitCliOverrides.contains(inputAttrPath); + auto inputIsOverride = explicitCliOverrides.contains(nonEmptyInputAttrPath); auto ref = (input2.ref && inputIsOverride) ? *input2.ref : *input.ref; if (input.isFlake) { diff --git a/src/libflake/include/nix/flake/flake.hh b/src/libflake/include/nix/flake/flake.hh index c2d597ac1..3bee6556f 100644 --- a/src/libflake/include/nix/flake/flake.hh +++ b/src/libflake/include/nix/flake/flake.hh @@ -205,13 +205,13 @@ struct LockFlags /** * Flake inputs to be overridden. */ - std::map inputOverrides; + std::map inputOverrides; /** * Flake inputs to be updated. This means that any existing lock * for those inputs will be ignored. */ - std::set inputUpdates; + std::set inputUpdates; }; LockedFlake diff --git a/src/libflake/include/nix/flake/lockfile.hh b/src/libflake/include/nix/flake/lockfile.hh index c5740a2f1..89029aec4 100644 --- a/src/libflake/include/nix/flake/lockfile.hh +++ b/src/libflake/include/nix/flake/lockfile.hh @@ -14,6 +14,80 @@ namespace nix::flake { typedef std::vector InputAttrPath; +/** + * A non-empty input attribute path. + * + * Input attribute paths identify inputs in a flake. An empty path would + * refer to the flake itself rather than an input, which contradicts the + * purpose of operations like override or update. + */ +class NonEmptyInputAttrPath +{ + InputAttrPath path; + + explicit NonEmptyInputAttrPath(InputAttrPath && p) + : path(std::move(p)) + { + assert(!path.empty()); + } + +public: + /** + * Parse and validate a non-empty input attribute path. + * Returns std::nullopt if the path is empty. + */ + static std::optional parse(std::string_view s); + + /** + * Construct from an already-parsed path. + * Returns std::nullopt if the path is empty. + */ + static std::optional make(InputAttrPath path); + + /** + * Append an element to a path, creating a non-empty path. + * This is always safe because adding an element guarantees non-emptiness. + */ + static NonEmptyInputAttrPath append(const InputAttrPath & prefix, const FlakeId & element) + { + InputAttrPath path = prefix; + path.push_back(element); + return NonEmptyInputAttrPath{std::move(path)}; + } + + const InputAttrPath & get() const + { + return path; + } + + operator const InputAttrPath &() const + { + return path; + } + + /** + * Get the final component of the path (the input name). + * For a path like "a/b/c", returns "c". + */ + const FlakeId & inputName() const + { + return path.back(); + } + + /** + * Get the parent path (all components except the last). + * For a path like "a/b/c", returns "a/b". + */ + InputAttrPath parent() const + { + InputAttrPath result = path; + result.pop_back(); + return result; + } + + auto operator<=>(const NonEmptyInputAttrPath & other) const = default; +}; + struct LockedNode; /** diff --git a/src/libflake/lockfile.cc b/src/libflake/lockfile.cc index f2914feab..4eeba3bd3 100644 --- a/src/libflake/lockfile.cc +++ b/src/libflake/lockfile.cc @@ -316,6 +316,19 @@ InputAttrPath parseInputAttrPath(std::string_view s) return path; } +std::optional NonEmptyInputAttrPath::parse(std::string_view s) +{ + auto path = parseInputAttrPath(s); + return make(std::move(path)); +} + +std::optional NonEmptyInputAttrPath::make(InputAttrPath path) +{ + if (path.empty()) + return std::nullopt; + return NonEmptyInputAttrPath{std::move(path)}; +} + std::map LockFile::getAllInputs() const { std::set> done; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 5324e0121..688aa77e0 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -90,9 +90,12 @@ public: .optional = true, .handler = {[&](std::vector inputsToUpdate) { for (const auto & inputToUpdate : inputsToUpdate) { - InputAttrPath inputAttrPath; + std::optional inputAttrPath; try { - inputAttrPath = flake::parseInputAttrPath(inputToUpdate); + inputAttrPath = flake::NonEmptyInputAttrPath::parse(inputToUpdate); + if (!inputAttrPath) + throw UsageError( + "input path to be updated cannot be zero-length; it would refer to the flake itself, not an input"); } catch (Error & e) { warn( "Invalid flake input '%s'. To update a specific flake, use 'nix flake update --flake %s' instead.", @@ -100,11 +103,11 @@ public: inputToUpdate); throw e; } - if (lockFlags.inputUpdates.contains(inputAttrPath)) + if (lockFlags.inputUpdates.contains(*inputAttrPath)) warn( "Input '%s' was specified multiple times. You may have done this by accident.", - printInputAttrPath(inputAttrPath)); - lockFlags.inputUpdates.insert(inputAttrPath); + printInputAttrPath(*inputAttrPath)); + lockFlags.inputUpdates.insert(*inputAttrPath); } }}, .completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) { diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index d9e187251..1d5546d1b 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -395,6 +395,12 @@ nix flake lock "$flake3Dir" --override-input flake2/flake1 flake1 nix flake lock "$flake3Dir" --override-input flake2/flake1 flake1/master/"$hash1" [[ $(jq -r .nodes.flake1_2.locked.rev "$flake3Dir/flake.lock") =~ $hash1 ]] +# Test that --override-input with empty input path is rejected (issue #14816). +expectStderr 1 nix flake lock "$flake3Dir" --override-input '' . | grepQuiet -- "--override-input was passed a zero-length input path, which would refer to the flake itself, not an input" + +# Test that deprecated --update-input with empty input path is rejected. +expectStderr 1 nix flake lock "$flake3Dir" --update-input '' | grepQuiet -- "--update-input was passed a zero-length input path, which would refer to the flake itself, not an input" + # Test --update-input. nix flake lock "$flake3Dir" [[ $(jq -r .nodes.flake1_2.locked.rev "$flake3Dir/flake.lock") = "$hash1" ]] @@ -402,6 +408,9 @@ nix flake lock "$flake3Dir" nix flake update flake2/flake1 --flake "$flake3Dir" [[ $(jq -r .nodes.flake1_2.locked.rev "$flake3Dir/flake.lock") =~ $hash2 ]] +# Test that 'nix flake update' with empty input path is rejected. +expectStderr 1 nix flake update '' --flake "$flake3Dir" | grepQuiet -- "input path to be updated cannot be zero-length; it would refer to the flake itself, not an input" + # Test updating multiple inputs. nix flake lock "$flake3Dir" --override-input flake1 flake1/master/"$hash1" nix flake lock "$flake3Dir" --override-input flake2/flake1 flake1/master/"$hash1"