From 87baf29d6aebcda00eb2c46275afca7d3ce958b5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 15:30:38 +0100 Subject: [PATCH 1/3] Git fetcher: Don't compute revCount if it's already specified We don't care if the user (or more likely the lock file) specifies an incorrect value for revCount, since it doesn't matter for security (unlikely content hashes like narHash). --- src/libfetchers/fetchers.cc | 5 ----- src/libfetchers/git.cc | 9 +++++++-- tests/nixos/tarball-flakes.nix | 1 - 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 7e091ef1071..0d8bd4514fe 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -282,11 +282,6 @@ void Input::checkLocks(Input specified, Input & result) if (result.getRev() != prevRev) throw Error("'rev' attribute mismatch in input '%s', expected %s", result.to_string(), prevRev->gitRev()); } - - if (auto prevRevCount = specified.getRevCount()) { - if (result.getRevCount() != prevRevCount) - throw Error("'revCount' attribute mismatch in input '%s', expected %d", result.to_string(), *prevRevCount); - } } std::pair, Input> Input::getAccessor(const Settings & settings, Store & store) const diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 75e3f121481..f4d492308d7 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -891,8 +891,13 @@ struct GitInputScheme : InputScheme input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev)); - if (!getShallowAttr(input)) - input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev)); + if (!getShallowAttr(input)) { + /* Skip revCount computation if it's already supplied by the caller. + We don't care if they specify an incorrect value; it doesn't + matter for security, unlike narHash. */ + if (!input.attrs.contains("revCount")) + input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev)); + } printTalkative("using revision %s of repo '%s'", rev.gitRev(), repoInfo.locationToArg()); diff --git a/tests/nixos/tarball-flakes.nix b/tests/nixos/tarball-flakes.nix index 26c20cb1aef..b0402412d85 100644 --- a/tests/nixos/tarball-flakes.nix +++ b/tests/nixos/tarball-flakes.nix @@ -99,7 +99,6 @@ in # Check that fetching fails if we provide incorrect attributes. machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?rev=493300eb13ae6fb387fbd47bf54a85915acc31c0") - machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?revCount=789") machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?narHash=sha256-tbudgBSg+bHWHiHnlteNzN8TUvI80ygS9IULh4rklEw=") ''; From 8f32f28ebdaf3272d386756724d345883eec760c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 15:39:19 +0100 Subject: [PATCH 2/3] Git fetcher: Don't compute lastModified if it's already specified Same as revCount. --- src/libfetchers/fetchers.cc | 9 --------- src/libfetchers/git.cc | 10 ++++++---- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 0d8bd4514fe..48a23ad3693 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -269,15 +269,6 @@ void Input::checkLocks(Input specified, Input & result) } } - if (auto prevLastModified = specified.getLastModified()) { - if (result.getLastModified() != prevLastModified) - throw Error( - "'lastModified' attribute mismatch in input '%s', expected %d, got %d", - result.to_string(), - *prevLastModified, - result.getLastModified().value_or(-1)); - } - if (auto prevRev = specified.getRev()) { if (result.getRev() != prevRev) throw Error("'rev' attribute mismatch in input '%s', expected %s", result.to_string(), prevRev->gitRev()); diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index f4d492308d7..96008d45bd2 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -889,12 +889,14 @@ struct GitInputScheme : InputScheme auto rev = *input.getRev(); - input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev)); + /* Skip lastModified computation if it's already supplied by the caller. + We don't care if they specify an incorrect value; it doesn't + matter for security, unlike narHash. */ + if (!input.attrs.contains("lastModified")) + input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev)); if (!getShallowAttr(input)) { - /* Skip revCount computation if it's already supplied by the caller. - We don't care if they specify an incorrect value; it doesn't - matter for security, unlike narHash. */ + /* Like lastModified, skip revCount if supplied by the caller. */ if (!input.attrs.contains("revCount")) input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev)); } From 4ecc09c43f5f42808134ec8144ac2afdcce0fed2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 19:58:08 +0100 Subject: [PATCH 3/3] Make content-encoding test more reliable --- tests/nixos/content-encoding.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/nixos/content-encoding.nix b/tests/nixos/content-encoding.nix index debee377bdf..1e188cb060b 100644 --- a/tests/nixos/content-encoding.nix +++ b/tests/nixos/content-encoding.nix @@ -131,6 +131,7 @@ in start_all() machine.wait_for_unit("nginx.service") + machine.wait_for_open_port(80) # Original test: zstd archive with gzip content-encoding # Make sure that the file is properly compressed as the test would be meaningless otherwise