Provenance #321
Provenance #321
Conversation
📝 WalkthroughWalkthroughAdds a provenance subsystem and threads provenance metadata across fetchers, source accessors, flakes/eval, store write/copy flows, worker/daemon protocol, DB schema, and JSON schemas to record and transmit origin information. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Daemon
participant Store
participant Provenance
Client->>Daemon: AddToStore(data, optional provenance JSON)
Daemon->>Store: addToStoreFromDump(..., provenance?)
Note right of Store: create ValidPathInfo\ninfo.provenance = provenance
Store->>Provenance: possibly wrap/chain (Copied/Build) provenance
Store-->Daemon: result (path + provenance if negotiated)
Daemon-->Client: response (path + provenance if negotiated)
sequenceDiagram
participant Fetcher
participant Input
participant Accessor
participant Store
participant Provenance
Fetcher->>Input: fetchToStore(...)
Input-->>Accessor: return accessor (with attrs)
Accessor->>Accessor: accessor.provenance = TreeProvenance(input.attrs)
Fetcher->>Store: addToStoreFromDump(..., provenance = accessor.getProvenance(path))
Store->>Provenance: record BuildProvenance/CopiedProvenance chain
Store-->Fetcher: store path (with provenance)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/libstore/dummy-store.cc (1)
235-304: Provenance parameter is accepted but not used inaddToStoreFromDump.The
provenanceparameter is added to the signature but is never used in the implementation. TheValidPathInfocreated on lines 276–288 and stored incontentsdoesn't include the provenance. Other stores handle this correctly:BinaryCacheStore::addToStoreFromDump(line 382) andLocalStore::addToStoreFromDump(line 1331) both assigninfo.provenance = provenance.Add the provenance assignment:
Suggested fix
auto info = ValidPathInfo::makeFromCA( *this, name, ContentAddressWithReferences::fromParts( hashMethod, std::move(hash), { .others = references, // caller is not capable of creating a self-reference, because // this is content-addressed without modulus .self = false, }), std::move(narHash.first)); info.narSize = narHash.second.value(); + info.provenance = std::move(provenance);
🤖 Fix all issues with AI agents
In `@src/libexpr/include/nix/expr/eval.hh`:
- Around line 1134-1147: Members are declared in the wrong order which breaks
the intended destruction order: move std::shared_ptr<const Provenance>
rootProvenance so it is declared before ref<Executor> executor (or alternatively
move ref<Executor> executor to be the final data member) to ensure executor is
destroyed last; update the declarations around the class fields that contain
rootProvenance, executor and the public methods
setRootProvenance/getRootProvenance accordingly so destructor ordering is
preserved.
In `@src/libfetchers/filtering-source-accessor.cc`:
- Around line 71-76: FilteringSourceAccessor::getProvenance currently bypasses
checkAccess() and the SubpathProvenance wrapping from
SourceAccessor::getProvenance, so update this method to first call
checkAccess(path) (or the appropriate access-checking call used by
SourceAccessor), then if the local provenance member exists return it, otherwise
obtain the child provenance via next->getProvenance(prefix / path) and wrap that
result in a SubpathProvenance constructed with the returned provenance and the
prefix before returning; also add an `#include` for the SubpathProvenance header
if not already present.
In `@src/libfetchers/tarball.cc`:
- Around line 106-109: The code currently stores the raw URL in provenance
(info.provenance = std::make_shared<FetchurlProvenance>(url)) which can leak
credentials or signed tokens; before creating FetchurlProvenance and calling
store.addToStore(info, source, ...), sanitize/redact the URL (remove userinfo
and sensitive query params) using the existing redaction helper if available (or
add a small helper that strips user:pass and known signed token query keys),
then pass the redacted string into FetchurlProvenance instead of the original
url so no secrets are persisted in narinfo/metadata.
- Around line 12-34: Add a Provenance::Register for the FetchurlProvenance type
so deserialization of {"type":"fetchurl"} returns a FetchurlProvenance instance:
register a Provenance::Register (e.g. registerFetchurlProvenance) that uses
getObject(json), valueAt(obj, "url"), getString(...) and
make_ref<FetchurlProvenance>(...) to construct the object; place this
registration in this file (or src/libfetchers/provenance.cc) so the type is
recognized at load time. Also sanitize/redact sensitive components of the url
before storing/serializing in FetchurlProvenance::to_json (e.g. strip
credentials or tokens) to avoid leaking credentials in provenance.
In `@src/libflake/include/nix/flake/flake.hh`:
- Around line 98-101: Flake::~Flake() in flake.cc needs the complete type for
Provenance because it destroys the std::shared_ptr<const Provenance> member
provenance; add `#include` "nix/flake/provenance.hh" to src/libflake/flake.cc so
the Provenance definition is visible when instantiating the destructor for Flake
(referencing the provenance member and the Flake::~Flake() destructor).
In `@src/libstore/include/nix/store/path-info.hh`:
- Around line 127-132: The header now exposes std::shared_ptr via the member
"provenance" (type std::shared_ptr<const Provenance>), so add a direct include
for <memory> at the top of this header to avoid relying on transitive includes;
ensure the include is placed with the other standard headers and before any code
that references std::shared_ptr/Provenance.
In `@src/libstore/include/nix/store/provenance.hh`:
- Around line 9-36: DerivationProvenance is missing a from_json registration so
Provenance::from_json() cannot reconstruct derivation nodes; add a registration
call analogous to CopiedProvenance using Provenance::Register to register
"derivation" (or the existing type tag) with a factory that constructs a
DerivationProvenance from JSON, ensuring the registration happens during module
init (the same translation unit where other registrations like CopiedProvenance
are registered) so Provenance::from_json can deserialize DerivationProvenance
objects.
In `@src/libstore/nar-info.cc`:
- Around line 134-136: The text serialization in NarInfo (in
src/libstore/nar-info.cc) appends provenance unconditionally; update the
NarInfo::to_string (or the function that builds `res`) to only append
"Provenance: " + provenance->to_json_str() when
experimentalFeatureSettings.isEnabled(Xp::Provenance) is true, mirroring the
gating used during parsing and the UnkeyedValidPathInfo::toJSON() behavior;
locate the block that currently does `if (provenance) res += "Provenance: " +
provenance->to_json_str() + "\n";` and change it to check the feature flag
before using `provenance`.
In `@src/libutil/experimental-features.cc`:
- Around line 323-330: The provenance experimental-feature entry currently has
an empty .trackingUrl which leads to a broken link in documentation; update the
documentation generation (documentExperimentalFeatures) to check
feature.trackingUrl (or equivalent field) and only emit the tracking/issue link
when it is non-empty, or alternatively set a real URL for the Xp::Provenance
entry; reference the Xp::Provenance feature record (name "provenance") and the
trackingUrl field when making the change so the link is suppressed for empty
values.
🧹 Nitpick comments (5)
src/libstore/include/nix/store/remote-store.hh (1)
91-99: Consider restoring default arguments for RemoteStore callers.C++ default args aren’t inherited, so removing them here can break direct
RemoteStoreusages even ifStorestill provides defaults. If this isn’t intentional, consider mirroring the base defaults to preserve API ergonomics.♻️ Proposed adjustment
- FileSerialisationMethod dumpMethod, - ContentAddressMethod hashMethod, - HashAlgorithm hashAlgo, - const StorePathSet & references, - RepairFlag repair, - std::shared_ptr<const Provenance> provenance) override; + FileSerialisationMethod dumpMethod = FileSerialisationMethod::NixArchive, + ContentAddressMethod hashMethod = ContentAddressMethod::Raw::NixArchive, + HashAlgorithm hashAlgo = HashAlgorithm::SHA256, + const StorePathSet & references = StorePathSet(), + RepairFlag repair = NoRepair, + std::shared_ptr<const Provenance> provenance = nullptr) override;src/libstore/provenance.cc (1)
6-25: Inconsistentnextfield serialization between provenance types.
DerivationProvenance::to_json()always includes the"next"field (asnullwhen absent), whileCopiedProvenance::to_json()only includes it when present. This inconsistency may cause confusion or issues during deserialization.Consider making the serialization consistent. Based on the
FlakeProvenancepattern fromsrc/libflake/provenance.cc, which always emits"next", aligning with that approach would be more consistent:♻️ Suggested fix for consistency
nlohmann::json CopiedProvenance::to_json() const { - nlohmann::json j{ + return nlohmann::json{ {"type", "copied"}, {"from", from}, + {"next", next ? next->to_json() : nlohmann::json(nullptr)}, }; - if (next) - j["next"] = next->to_json(); - return j; }src/libstore/build/derivation-building-goal.cc (1)
443-447: Consider capturing provenance for the AlreadyValid shortcut.
Provenance is fetched after the early return at Line 349, soAlreadyValidresults will report null provenance even when it exists. If you want consistent provenance in build results, consider querying it before that shortcut or in the AlreadyValid branch.src/libstore/store-api.cc (1)
888-895: Consider returning existing path info on the early-valid shortcut.
With the newnullptrreturn when the destination already has the path (Line 893–895), callers that want provenance (or other info) need an extra query. If that data is important in the “already present” case, consider returningdstStore.queryPathInfo(storePath)or explicitly documenting the contract.Also applies to: 907-942
src/libutil/include/nix/util/provenance.hh (1)
12-38: Add a virtual destructor toProvenanceas best practice.
While the base class is polymorphic and owns derived types throughstd::shared_ptr(which correctly captures concrete type deletes at construction), explicitly marking a polymorphic base class with a virtual destructor clarifies intent and ensures safety if the code evolves. This follows standard C++ polymorphism conventions.Proposed fix
struct Provenance { + virtual ~Provenance() = default; static ref<const Provenance> from_json_str(std::string_view);
| #include "nix/util/provenance.hh" | ||
|
|
||
| #include <nlohmann/json.hpp> | ||
|
|
||
| namespace nix::fetchers { | ||
|
|
||
| struct FetchurlProvenance : Provenance | ||
| { | ||
| std::string url; | ||
|
|
||
| FetchurlProvenance(const std::string & url) | ||
| : url(url) | ||
| { | ||
| } | ||
|
|
||
| nlohmann::json to_json() const override | ||
| { | ||
| return nlohmann::json{ | ||
| {"type", "fetchurl"}, | ||
| {"url", url}, | ||
| }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for "fetchurl" references in the codebase
echo "=== Searching for 'fetchurl' ==="
rg -n "fetchurl" --type-list | head -5
rg -n "fetchurl" 2>/dev/null | head -20
echo -e "\n=== Searching for Provenance deserialization patterns ==="
rg -n "from_json|Provenance::from_json" 2>/dev/null | head -20
echo -e "\n=== Searching for type dispatch/registry in provenance ==="
rg -n "\"type\"" src/libutil src/libfetchers src/libstore 2>/dev/null | grep -i provenance | head -20Repository: DeterminateSystems/nix-src
Length of output: 4928
🏁 Script executed:
#!/bin/bash
# Look for Provenance class definition and from_json implementation
echo "=== Finding Provenance class definition ==="
fd -e "\.hh" -e "\.cc" | xargs rg -l "class Provenance" 2>/dev/null
echo -e "\n=== Search for from_json in provenance files ==="
fd -e "\.hh" -e "\.cc" | xargs rg -A 10 "from_json.*Provenance" 2>/dev/null | head -40Repository: DeterminateSystems/nix-src
Length of output: 163
🏁 Script executed:
#!/bin/bash
# Look for JSON type dispatching for provenance
echo "=== Searching for provenance type mapping ==="
rg -n "type.*provenance\|provenance.*type" -i src/ 2>/dev/null | head -20
echo -e "\n=== Checking for nlohmann json deserialization ==="
rg -n "nlohmann::json\|\.get\(\)" src/libfetchers src/libutil -A 2 2>/dev/null | grep -A 2 "type\|fetchurl" | head -30Repository: DeterminateSystems/nix-src
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Read the from_json implementation
echo "=== src/libutil/provenance.cc from_json implementation ==="
sed -n '32,80p' src/libutil/provenance.cc
echo -e "\n=== Check full provenance.cc ==="
wc -l src/libutil/provenance.cc src/libfetchers/provenance.cc src/libstore/provenance.cc 2>/dev/null
echo -e "\n=== Read src/libutil/provenance.cc entirely ==="
cat -n src/libutil/provenance.ccRepository: DeterminateSystems/nix-src
Length of output: 2603
🏁 Script executed:
#!/bin/bash
# Check what files include/define FetchurlProvenance
echo "=== Files mentioning FetchurlProvenance ==="
rg -n "FetchurlProvenance" --type cc --type hh 2>/dev/null
echo -e "\n=== Check src/libfetchers/provenance.cc ==="
cat -n src/libfetchers/provenance.cc 2>/dev/null || echo "File not found"
echo -e "\n=== Check all provenance related headers ==="
fd -e ".hh" | xargs rg -l "Provenance" 2>/dev/null | head -10Repository: DeterminateSystems/nix-src
Length of output: 1529
🏁 Script executed:
#!/bin/bash
# Read provenance headers
echo "=== src/libutil/include/nix/util/provenance.hh ==="
cat -n src/libutil/include/nix/util/provenance.hh
echo -e "\n=== src/libstore/include/nix/store/provenance.hh ==="
cat -n src/libstore/include/nix/store/provenance.hhRepository: DeterminateSystems/nix-src
Length of output: 3463
🏁 Script executed:
#!/bin/bash
# Check for registration patterns
echo "=== Looking for RegisteredTypes registration ==="
rg -n "registeredTypes\|RegisteredTypes\|register.*[Pp]rovenance" -A 3 -B 1 --type cc --type hh 2>/dev/null | head -60
echo -e "\n=== Check tarball.cc for FetchurlProvenance context ==="
sed -n '1,50p' src/libfetchers/tarball.ccRepository: DeterminateSystems/nix-src
Length of output: 1265
🏁 Script executed:
#!/bin/bash
# Search for Provenance::Register usage pattern
echo "=== Looking for Provenance::Register instantiation ==="
rg -n "Provenance::Register\|Register.*type" -B 2 -A 5 --type cc --type hh 2>/dev/null
echo -e "\n=== Check full src/libstore/provenance.cc ==="
cat -n src/libstore/provenance.cc
echo -e "\n=== Search for registration in fetchers/provenance ==="
cat -n src/libfetchers/provenance.ccRepository: DeterminateSystems/nix-src
Length of output: 2218
🏁 Script executed:
#!/bin/bash
# Look for how other provenance types register themselves
echo "=== Check for registrations across all provenance implementations ==="
find . -name "provenance*" -type f \( -name "*.cc" -o -name "*.hh" \) 2>/dev/null | xargs cat -n 2>/dev/null | grep -A 10 "Register"
echo -e "\n=== Full tarball.cc provenance section ==="
sed -n '12,100p' src/libfetchers/tarball.ccRepository: DeterminateSystems/nix-src
Length of output: 3819
Register FetchurlProvenance for deserialization.
The "fetchurl" type lacks a Provenance::Register instantiation. Without it, deserialization of provenance with "type": "fetchurl" will fall back to UnknownProvenance (line 40 of src/libutil/provenance.cc), losing the typed information. Add a registration in this file or src/libfetchers/provenance.cc:
Provenance::Register registerFetchurlProvenance("fetchurl", [](nlohmann::json json) {
auto & obj = getObject(json);
return make_ref<FetchurlProvenance>(getString(valueAt(obj, "url")));
});Additionally, URLs in provenance may contain credentials or signed tokens; consider redacting sensitive components before serialization.
🤖 Prompt for AI Agents
In `@src/libfetchers/tarball.cc` around lines 12 - 34, Add a Provenance::Register
for the FetchurlProvenance type so deserialization of {"type":"fetchurl"}
returns a FetchurlProvenance instance: register a Provenance::Register (e.g.
registerFetchurlProvenance) that uses getObject(json), valueAt(obj, "url"),
getString(...) and make_ref<FetchurlProvenance>(...) to construct the object;
place this registration in this file (or src/libfetchers/provenance.cc) so the
type is recognized at load time. Also sanitize/redact sensitive components of
the url before storing/serializing in FetchurlProvenance::to_json (e.g. strip
credentials or tokens) to avoid leaking credentials in provenance.
| struct DerivationProvenance : Provenance | ||
| { | ||
| /** | ||
| * The derivation that built this path. | ||
| */ | ||
| StorePath drvPath; | ||
|
|
||
| /** | ||
| * The output of the derivation that corresponds to this path. | ||
| */ | ||
| OutputName output; | ||
|
|
||
| /** | ||
| * The provenance of the derivation, if known. | ||
| */ | ||
| std::shared_ptr<const Provenance> next; | ||
|
|
||
| // FIXME: do we need anything extra for CA derivations? | ||
|
|
||
| DerivationProvenance(const StorePath & drvPath, const OutputName & output, std::shared_ptr<const Provenance> next) | ||
| : drvPath(drvPath) | ||
| , output(output) | ||
| , next(std::move(next)) | ||
| { | ||
| } | ||
|
|
||
| nlohmann::json to_json() const override; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for provenance registrations in the codebase
rg -n "from_json|nlohmann_define|nlohmann::json" src/libstore/ --type cpp -B 2 -A 2 | head -100Repository: DeterminateSystems/nix-src
Length of output: 5808
🏁 Script executed:
# Find and examine provenance.cc
find . -name "provenance.cc" -type fRepository: DeterminateSystems/nix-src
Length of output: 187
🏁 Script executed:
# Check the size and content of provenance.cc
if [ -f "src/libstore/provenance.cc" ]; then
wc -l src/libstore/provenance.cc
echo "---"
cat -n src/libstore/provenance.cc | head -50
fiRepository: DeterminateSystems/nix-src
Length of output: 1250
🏁 Script executed:
# Search specifically for Derived and Copied provenance patterns
rg -n "Derived|Copied" src/libstore/provenance.cc -B 2 -A 2Repository: DeterminateSystems/nix-src
Length of output: 534
🏁 Script executed:
#!/bin/bash
# Search for all Provenance::Register entries to see the pattern
rg -n "Provenance::Register" --type cppRepository: DeterminateSystems/nix-src
Length of output: 345
🏁 Script executed:
# Search for provenance types in the header files
find . -name "*.hh" -path "*provenance*" -type f | xargs cat -n | head -100Repository: DeterminateSystems/nix-src
Length of output: 2867
🏁 Script executed:
# Check for any reference to "derivation" type in JSON context
rg -n '"derivation"' --type cpp -B 2 -A 2Repository: DeterminateSystems/nix-src
Length of output: 9701
🏁 Script executed:
#!/bin/bash
# Search for all Provenance registration types across all libmodules
rg -n "Provenance::Register|registerTree|registerFlake" --type cpp src/lib*/Repository: DeterminateSystems/nix-src
Length of output: 345
🏁 Script executed:
# Check the actual implementation files for Tree and Flake provenance
find . -name "provenance.cc" -type f | xargs wc -lRepository: DeterminateSystems/nix-src
Length of output: 218
🏁 Script executed:
# Check libfetchers/provenance.cc for TreeProvenance registration
if [ -f "src/libfetchers/provenance.cc" ]; then
cat -n src/libfetchers/provenance.cc
fiRepository: DeterminateSystems/nix-src
Length of output: 914
🏁 Script executed:
# Check libflake/provenance.cc for FlakeProvenance registration
if [ -f "src/libflake/provenance.cc" ]; then
cat -n src/libflake/provenance.cc
fiRepository: DeterminateSystems/nix-src
Length of output: 503
Add from_json registration for DerivationProvenance.
CopiedProvenance has a registration in provenance.cc that enables deserialization from JSON (via Provenance::Register at line 27), but DerivationProvenance lacks one. Since Provenance::from_json() is actively called during deserialization in nar-info.cc, worker-protocol.cc, and build-result.cc, a missing registration for DerivationProvenance would cause failures when reconstructing provenance chains containing derivations from JSON.
🤖 Prompt for AI Agents
In `@src/libstore/include/nix/store/provenance.hh` around lines 9 - 36,
DerivationProvenance is missing a from_json registration so
Provenance::from_json() cannot reconstruct derivation nodes; add a registration
call analogous to CopiedProvenance using Provenance::Register to register
"derivation" (or the existing type tag) with a factory that constructs a
DerivationProvenance from JSON, ensuring the registration happens during module
init (the same translation unit where other registrations like CopiedProvenance
are registered) so Provenance::from_json can deserialize DerivationProvenance
objects.
edolstra
force-pushed
the
provenance-detsys
branch
6 times, most recently
from
fb14a66 to
e8599c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/libstore/include/nix/store/worker-protocol.hh (1)
66-84: Verify provenance handling in hardcoded protocol version 16 cases only—main negotiation path is correct.The primary code paths using
WorkerProto::BasicClientConnectionandWorkerProto::BasicServerConnectioncorrectly set theprovenanceflag based on negotiated features. However, three direct constructions with hardcodedversion = 16do not setprovenanceand will silently default tofalse:
src/libstore/store-api.cc:204inaddMultipleToStore()(marked FIXME)src/libstore/export-import.cc:147and:60for nario format import/exportThese are non-negotiated contexts marked as technical debt. If provenance should flow through these paths, they need explicit handling. Otherwise, document that these legacy/special-format paths intentionally bypass provenance.
src/libstore/include/nix/store/store-api.hh (1)
948-957: Handle nullptr return fromcopyStorePath.
The function can returnnullptrwhen the path already exists in the destination store. At least one call site insrc/libstore/build/substitution-goal.cc:258sets a promise value with the return directly without guarding againstnullptr, which could cause issues downstream.
🤖 Fix all issues with AI agents
In `@src/libstore/build/derivation-building-goal.cc`:
- Around line 443-447: The AlreadyValid fast‑path returns BuildResult::Success
before provenance is fetched, causing missing provenance; hoist the
maybeQueryPathInfo(drvPath) lookup (calling worker.evalStore.maybeQueryPathInfo
and capturing info->provenance into a std::shared_ptr<const Provenance>
provenance) before the early return for the AlreadyValid case and reuse that
provenance when constructing/returning BuildResult::Success so provenance is
present for the fast path as well as later build paths.
In `@src/libstore/dummy-store.cc`:
- Around line 235-243: The addToStoreFromDump implementation accepts a
provenance parameter but never attaches it to the created ValidPathInfo, so
provenance is dropped; update the function (addToStoreFromDump) to set
info.provenance = provenance (or equivalent member) on the ValidPathInfo
instance before calling whatever inserts the entry (e.g., insertPath /
_store.emplace or the existing insertion code) so the saved path records the
provided provenance.
In `@src/libstore/include/nix/store/legacy-ssh-store.hh`:
- Around line 76-79: The isUsefulProvenance() override currently returns true
but LegacySSHStore (ServeProto) cannot transmit provenance; change the method
implementation in the LegacySSHStore class so isUsefulProvenance() returns false
to accurately reflect protocol capabilities (or alternatively implement
provenance support in ServeProto/ReadConn/WriteConn and UnkeyedValidPathInfo
before keeping true) — locate the isUsefulProvenance() override and update its
return value to false.
In `@src/libutil/include/nix/util/provenance.hh`:
- Around line 3-8: Add explicit standard headers for the used types: include
<map>, <string>, <string_view>, and <memory> at the top of the header so that
uses of std::map, std::string, std::string_view, and std::shared_ptr are no
longer reliant on transitive includes; update the include block near the
existing includes ("nix/util/ref.hh", "nix/util/canon-path.hh", <functional>,
<nlohmann/json_fwd.hpp>) to add these four headers.
♻️ Duplicate comments (6)
src/libexpr/include/nix/expr/eval.hh (1)
1125-1147: Keepexecutoras the final data member to preserve destruction order.
This placesrootProvenanceafterexecutor, which violates the “keep this last” guarantee. Please moverootProvenancebeforeexecutor(or moveexecutorback to the end).src/libfetchers/tarball.cc (2)
18-34: Registerfetchurlprovenance for deserialization.
Without aProvenance::Registerfor"fetchurl", deserialization will fall back toUnknownProvenance, losing typed data.🧩 Suggested registration (example)
+static Provenance::Register registerFetchurlProvenance( + "fetchurl", + [](nlohmann::json json) { + auto & obj = getObject(json); + return make_ref<FetchurlProvenance>(getString(valueAt(obj, "url"))); + });
107-108: Avoid persisting credentials or signed tokens in provenance URLs.
Storing the raw URL in provenance risks leaking userinfo or signed query params into.narinfo/metadata. Please redact/sanitize before storing.🛡️ Example guard (pseudo)
- info.provenance = std::make_shared<FetchurlProvenance>(url); + info.provenance = std::make_shared<FetchurlProvenance>(redactSensitiveUrl(url));src/libutil/experimental-features.cc (1)
323-330: Suppress tracking links whentrackingUrlis empty.
documentExperimentalFeatures()always emits a tracking link; empty URL yields a broken link for provenance.🧾 Suggested guard
- docOss << fmt( - "\nRefer to [%1% tracking issue](%2%) for feature tracking.", xpFeature.name, xpFeature.trackingUrl); + if (!xpFeature.trackingUrl.empty()) { + docOss << fmt( + "\nRefer to [%1% tracking issue](%2%) for feature tracking.", xpFeature.name, xpFeature.trackingUrl); + }src/libstore/include/nix/store/path-info.hh (1)
127-131: Add<memory>include forstd::shared_ptr(duplicate).This header now stores
std::shared_ptr, so it should include<memory>directly to avoid reliance on transitive includes.🔧 Suggested fix
`#include` <string> `#include` <optional> +#include <memory>src/libfetchers/filtering-source-accessor.cc (1)
71-76: Preserve access checks and subpath provenance (Line 71-76).This override skips
checkAccess()and dropsSubpathProvenancewrapping for non-root paths, so callers can obtain root provenance for any subpath. Consider mirroringSourceAccessor::getProvenance.🧩 Proposed fix
std::shared_ptr<const Provenance> FilteringSourceAccessor::getProvenance(const CanonPath & path) { + checkAccess(path); if (provenance) - return provenance; + return path.isRoot() ? provenance : std::make_shared<SubpathProvenance>(provenance, path); return next->getProvenance(prefix / path); }If needed, add the
SubpathProvenanceinclude to this file.
🧹 Nitpick comments (3)
src/libstore/path-info.cc (1)
219-220: Consider limiting provenance to JSON format V2 (Line 219-220, Line 296-300).To avoid surprising strict V1 consumers, consider gating provenance emission and parsing to
PathInfoJsonFormat::V2only.♻️ Suggested adjustment
- if (experimentalFeatureSettings.isEnabled(Xp::Provenance)) + if (experimentalFeatureSettings.isEnabled(Xp::Provenance) && format == PathInfoJsonFormat::V2) jsonObject["provenance"] = provenance ? provenance->to_json() : nullptr;- if (experimentalFeatureSettings.isEnabled(Xp::Provenance)) { + if (experimentalFeatureSettings.isEnabled(Xp::Provenance) && format == PathInfoJsonFormat::V2) { auto prov = json.find("provenance"); if (prov != json.end() && !prov->second.is_null()) res.provenance = Provenance::from_json(prov->second); }Also applies to: 296-300
src/libstore/provenance.cc (1)
6-25: Inconsistent handling of optionalnextfield in JSON serialization.
DerivationProvenance::to_json()always emits thenextfield (withnullif absent), whileCopiedProvenance::to_json()only emitsnextwhen present. This asymmetry may cause issues with JSON comparison, diffing, or consumers that expect consistent structure.Consider aligning the behavior—either always emit
next(with null) or conditionally emit it in both classes.Option A: Always emit `next` in CopiedProvenance (matching DerivationProvenance)
nlohmann::json CopiedProvenance::to_json() const { - nlohmann::json j{ + return nlohmann::json{ {"type", "copied"}, {"from", from}, + {"next", next ? next->to_json() : nlohmann::json(nullptr)}, }; - if (next) - j["next"] = next->to_json(); - return j; }Option B: Conditionally emit `next` in DerivationProvenance (matching CopiedProvenance)
nlohmann::json DerivationProvenance::to_json() const { - return nlohmann::json{ + nlohmann::json j{ {"type", "derivation"}, {"drv", drvPath.to_string()}, {"output", output}, - {"next", next ? next->to_json() : nlohmann::json(nullptr)}, }; + if (next) + j["next"] = next->to_json(); + return j; }src/libstore/build/substitution-goal.cc (1)
72-72: Consider returning provenance for AlreadyValid paths.
Right now,AlreadyValidreportsnullptreven though provenance may exist in the store. If consumers rely on it, consider fetching it here.💡 Optional improvement
- if (!repair && worker.store.isValidPath(storePath)) { - co_return doneSuccess(BuildResult::Success::AlreadyValid, nullptr); - } + if (!repair && worker.store.isValidPath(storePath)) { + auto info = worker.store.maybeQueryPathInfo(storePath); + co_return doneSuccess(BuildResult::Success::AlreadyValid, info ? info->provenance : nullptr); + }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/libutil/include/nix/util/provenance.hh`:
- Around line 12-23: The Provenance polymorphic base class lacks a virtual
destructor; add a public virtual destructor to Provenance (e.g., declare a
virtual ~Provenance() = default) so derived instances (referenced by
std::shared_ptr<const Provenance> and used by functions like from_json_str,
from_json_str_optional, from_json, to_json_str, and the pure virtual to_json)
are safely destroyed when deleted via a base pointer.
♻️ Duplicate comments (1)
src/libutil/include/nix/util/provenance.hh (1)
3-8: Add missing standard library headers.This header uses
std::map,std::string,std::string_view, andstd::shared_ptrbut relies on transitive includes. Add explicit headers for better portability.`#include` "nix/util/ref.hh" `#include` "nix/util/canon-path.hh" `#include` <functional> +#include <map> +#include <memory> +#include <string> +#include <string_view> `#include` <nlohmann/json_fwd.hpp>
🧹 Nitpick comments (1)
src/libutil/include/nix/util/provenance.hh (1)
34-40: Consider logging or asserting on duplicate type registration.
insert_or_assignsilently overwrites existing registrations. If two modules accidentally register the same type name, debugging would be difficult. Consider usinginsertand checking the return value, or at least logging when a type is overwritten.
|
|
||
| auto attrPath = attr->getAttrPathStr(); | ||
|
|
||
| state->setRootProvenance(makeProvenance(attrPath)); |
There was a problem hiding this comment.
Rather than having to call this, is there any way we could maybe make this automatic?
(Mostly thinking about protecting from developer error -- if I have to write code here for whatever reason, and add a new path where I should have called setRootProvenance but forgot to, that's probably not great. But if it's integrated into the construction of e.g. attrPath, that means we can't forget it because it'll always happen)
|
(Also if you could go through the coderabbit comments and see which ones are real things to be resolved or just LLM hallucinations and close them accordingly, that would be great) |
Example of a substitution event:
{
"action": "result",
"id": 0,
"payload": {
"builtOutputs": {},
"path": "3bb116cnl86svn2lgc41a3i4a9qblgsf-libtool-2.4.7",
"provenance": {
"from": "https://cache.nixos.org",
"type": "copied"
},
"startTime": 0,
"status": "Substituted",
"stopTime": 0,
"success": true,
"timesBuilt": 0
},
"type": 110
}
Example of a derivation event:
{
"action": "result",
"id": 3381333262860569,
"payload": {
"builtOutputs": {
"out": {
"dependentRealisations": {},
"id": "sha256:deb37b0f322203d852a27010200f08e2dd739cb02b51d77999bd7f3162cdfe39!out",
"outPath": "6b9w3gdjnbdvi50c0h0b9xg91hq6aryl-patchelf-0.18.0",
"signatures": []
}
},
"cpuSystem": 2118013,
"cpuUser": 11471586,
"path": {
"drvPath": "fm8zrgh4dazysyz3imcva658h0iv34k0-patchelf-0.18.0.drv",
"outputs": [
"*"
]
},
"provenance": {
"flakeOutput": "packages.x86_64-linux.default",
"next": {
"attrs": {
"dirtyRev": "bb2f1eb3c1e4dc9c4523642a3e39d55806fc9a81-dirty",
"dirtyShortRev": "bb2f1eb-dirty",
"lastModified": 1768573749,
"type": "git",
"url": "file:///home/eelco/Dev/patchelf"
},
"type": "tree"
},
"type": "flake"
},
"startTime": 1768993105,
"status": "Built",
"stopTime": 1768993120,
"success": true,
"timesBuilt": 1
},
"type": 110
}
edolstra
force-pushed
the
provenance-detsys
branch
from
392a12a to
e7453a4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/libstore/provenance.cc`:
- Around line 27-33: Add a Provenance::Register for the "derivation" type
mirroring registerCopiedProvenance so Provenance::from_json returns a
DerivationProvenance instead of UnknownProvenance; create a new static register
(e.g. registerDerivationProvenance) that uses getObject(json), reads any "next"
via optionalValueAt(obj, "next") and deserializes it with Provenance::from_json,
and extracts required fields with valueAt/getString to construct and return
make_ref<DerivationProvenance>(...) using the same pattern as
registerCopiedProvenance.
♻️ Duplicate comments (12)
src/libexpr/include/nix/expr/eval.hh (1)
1140-1153: Member ordering violates destruction order requirement.The comment on lines 1133-1138 explicitly states that
executormust be the last data member to ensure it's destroyed first. However,rootProvenanceis now declared afterexecutor(line 1144), makingexecutorno longer the last member.In C++, members are destroyed in reverse declaration order. With this ordering,
rootProvenancewill be destroyed beforeexecutor, meaningexecutoris destroyed second—not first as required.Move
rootProvenancebeforeexecutorto preserve the intended destruction order.src/libstore/include/nix/store/path-info.hh (1)
127-132: Include<memory>forstd::shared_ptrin this public header.This header now exposes
std::shared_ptr, so it should include<memory>directly to avoid relying on transitive includes.🔧 Proposed fix
`#include` <string> `#include` <optional> +#include <memory>src/libutil/experimental-features.cc (1)
342-349: Avoid empty tracking URLs to prevent broken documentation links.
documentExperimentalFeatures()always emits a tracking link, so an empty URL becomes a broken link. Consider adding a real URL or suppressing link emission when empty.🧾 Suggested guard in documentation generation
- docOss << fmt( - "\nRefer to [%1% tracking issue](%2%) for feature tracking.", xpFeature.name, xpFeature.trackingUrl); + if (!xpFeature.trackingUrl.empty()) { + docOss << fmt( + "\nRefer to [%1% tracking issue](%2%) for feature tracking.", xpFeature.name, xpFeature.trackingUrl); + }src/libstore/include/nix/store/provenance.hh (1)
9-58: Verify JSON registration for the new provenance variants.Please confirm
DerivationProvenanceandCopiedProvenanceare registered for JSON deserialization; otherwiseProvenance::from_json*()will fail on these types.#!/bin/bash # Locate provenance registrations for derivation/copied variants fd -a "provenance.cc" --type f -x rg -n "Provenance::Register|DerivationProvenance|CopiedProvenance" -C2 {}src/libstore/include/nix/store/legacy-ssh-store.hh (1)
76-79:isUsefulProvenance()returnstruebut ServeProto does not transmit provenance.This store uses ServeProto, which lacks provenance support in its serialization. Returning
truemisleads callers into attaching provenance metadata that will be silently dropped. Either returnfalseto accurately reflect protocol capabilities, or extend ServeProto to include provenance support.src/libutil/include/nix/util/provenance.hh (2)
3-8: Add explicit standard library headers for used types.The header uses
std::map,std::string,std::string_view, andstd::shared_ptrbut relies on transitive includes fromref.hhandcanon-path.hh. Add explicit headers for better portability and self-containment.♻️ Suggested includes
`#include` "nix/util/ref.hh" `#include` "nix/util/canon-path.hh" `#include` <functional> +#include <map> +#include <memory> +#include <string> +#include <string_view> `#include` <nlohmann/json_fwd.hpp>
12-23: Add virtual destructor to polymorphic base class.
Provenanceis used polymorphically (stored viastd::shared_ptr<const Provenance>inSubpathProvenance::nextand throughout the codebase). Deleting a derived object through a base pointer without a virtual destructor is undefined behavior.🐛 Proposed fix
struct Provenance { + virtual ~Provenance() = default; + static ref<const Provenance> from_json_str(std::string_view);src/libflake/include/nix/flake/flake.hh (1)
98-101: Provenance member addition looks good.The
std::shared_ptr<const Provenance>member with documentation is well-structured. The explicit destructor declaration on line 123 ensures the complete type will be available where~Flake()is defined.As noted in a previous review, ensure
flake.ccincludes the fullprovenance.hhheader whereFlake::~Flake()is defined, sincestd::shared_ptrdestruction requires the complete type.src/libfetchers/tarball.cc (2)
18-34: RegisterFetchurlProvenancefor deserialization.The
FetchurlProvenancetype definesto_json()but lacks a correspondingProvenance::Registerinstantiation. Without registration, deserializing provenance JSON with"type": "fetchurl"will fall back toUnknownProvenance, losing the typed information.
107-107: Consider sanitizing URL before storing in provenance.Storing the raw URL in provenance can persist credentials (userinfo) or signed query parameters into store metadata and
.narinfofiles. Consider redacting sensitive URL components before creatingFetchurlProvenance.src/libfetchers/filtering-source-accessor.cc (1)
71-76: Enforce access checks and subpath provenance wrapping.This override skips
checkAccess()and returns root provenance for subpaths, which drops subpath context and can expose provenance for disallowed paths. IfSubpathProvenanceisn’t already visible here, add the appropriate provenance header.🛠️ Suggested fix
std::shared_ptr<const Provenance> FilteringSourceAccessor::getProvenance(const CanonPath & path) { + checkAccess(path); if (provenance) - return provenance; + return path.isRoot() ? provenance : std::make_shared<SubpathProvenance>(provenance, path); return next->getProvenance(prefix / path); }src/libstore/dummy-store.cc (1)
235-243: Provenance parameter is currently dropped inaddToStoreFromDump.
provenanceis accepted but never assigned to the generatedValidPathInfo, so dummy-store paths lose provenance.🛠️ Suggested fix
info.narSize = narHash.second.value(); + info.provenance = provenance;Also applies to: 286-291
🧹 Nitpick comments (2)
src/libstore/provenance.cc (1)
6-14: DerivationProvenance always includesnextas null, unlike other types.
DerivationProvenance::to_json()always includes the"next"key (withnullptrwhen absent), whileSubpathProvenanceandCopiedProvenanceconditionally omit it. Consider aligning the serialization pattern for consistency.♻️ Optional: align with conditional pattern
nlohmann::json DerivationProvenance::to_json() const { - return nlohmann::json{ + nlohmann::json j{ {"type", "derivation"}, {"drv", drvPath.to_string()}, {"output", output}, - {"next", next ? next->to_json() : nlohmann::json(nullptr)}, }; + if (next) + j["next"] = next->to_json(); + return j; }src/libstore/build-result.cc (1)
103-165: Guard provenance serialization forAlreadyValidresults.When
AlreadyValidorResolvesToAlreadyValidstatus is present, provenance should not be serialized since the existing outputs may have been built with different provenance than the current derivation. Currently,to_json()will emit provenance if present regardless of status, creating a risk of serializing misleading provenance if accidentally set. Add defensive guards during serialization and deserialization to prevent this.♻️ Suggested guards
- if (success.provenance) + if (success.provenance + && success.status != BuildResult::Success::AlreadyValid + && success.status != BuildResult::Success::ResolvesToAlreadyValid) res["provenance"] = success.provenance->to_json();- s.provenance = provenanceFromJson(optionalValueAt(json, "provenance")); + s.provenance = provenanceFromJson(optionalValueAt(json, "provenance")); + if (s.status == BuildResult::Success::AlreadyValid + || s.status == BuildResult::Success::ResolvesToAlreadyValid) + s.provenance.reset();
Makes it clearer that this store path is not a derivation itself, but the result of building a derivation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/libstore/provenance.cc`:
- Around line 27-33: Add a Provenance::Register for the "build" type so
Provenance::from_json() can deserialize BuildProvenance; mirror the existing
registerCopiedProvenance pattern: register a Provenance::Register("build",
[](nlohmann::json json){ ... }) that calls getObject(json), extracts optional
next via optionalValueAt and converts it with Provenance::from_json, and
constructs and returns make_ref<BuildProvenance>(...) passing the string/fields
read from valueAt/getString for the keys BuildProvenance expects (match the
BuildProvenance constructor parameters). Ensure the lambda uses the same helper
functions (getObject, optionalValueAt, valueAt, getString) and returns a shared
ref to BuildProvenance.
🧹 Nitpick comments (1)
src/libstore/provenance.cc (1)
6-14: Inconsistent handling ofnextfield compared toCopiedProvenance.
BuildProvenance::to_json()always emits"next"(asnullif absent), whileCopiedProvenance::to_json()omits the field entirely whennextis null. Consider aligning the pattern for consistency—either both emitnullor both omit the field.♻️ Suggestion to align with CopiedProvenance pattern
nlohmann::json BuildProvenance::to_json() const { - return nlohmann::json{ + nlohmann::json j{ {"type", "build"}, {"drv", drvPath.to_string()}, {"output", output}, - {"next", next ? next->to_json() : nlohmann::json(nullptr)}, }; + if (next) + j["next"] = next->to_json(); + return j; }
| Provenance::Register registerCopiedProvenance("copied", [](nlohmann::json json) { | ||
| auto & obj = getObject(json); | ||
| std::shared_ptr<const Provenance> next; | ||
| if (auto prov = optionalValueAt(obj, "next")) | ||
| next = Provenance::from_json(*prov); | ||
| return make_ref<CopiedProvenance>(getString(valueAt(obj, "from")), next); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for existing "build" provenance registration across the codebase
rg -n 'Provenance::Register.*"build"' --type cppRepository: DeterminateSystems/nix-src
Length of output: 52
🏁 Script executed:
cat -n src/libstore/provenance.cc | head -60Repository: DeterminateSystems/nix-src
Length of output: 1206
🏁 Script executed:
rg -n 'Provenance::Register' --type cpp -A 2Repository: DeterminateSystems/nix-src
Length of output: 595
🏁 Script executed:
# Search for BuildProvenance constructor usage and creation
rg -n 'BuildProvenance' --type cpp -B 2 -A 2Repository: DeterminateSystems/nix-src
Length of output: 1563
🏁 Script executed:
# Search for from_json calls on provenance
rg -n 'from_json.*provenance|Provenance::from_json' --type cpp -B 2 -A 2Repository: DeterminateSystems/nix-src
Length of output: 3959
Add missing Provenance::Register for BuildProvenance ("build" type).
BuildProvenance is constructed during the build process (in derivation-builder.cc) and serialized with "type": "build", but lacks a deserialization registration. Without this registration, Provenance::from_json() returns UnknownProvenance when deserializing from .narinfo files, the ValidPaths table, and daemon communications, breaking round-trip serialization.
🛠️ Proposed fix: Add registration for BuildProvenance
Provenance::Register registerCopiedProvenance("copied", [](nlohmann::json json) {
auto & obj = getObject(json);
std::shared_ptr<const Provenance> next;
if (auto prov = optionalValueAt(obj, "next"))
next = Provenance::from_json(*prov);
return make_ref<CopiedProvenance>(getString(valueAt(obj, "from")), next);
});
+
+Provenance::Register registerBuildProvenance("build", [](nlohmann::json json) {
+ auto & obj = getObject(json);
+ std::shared_ptr<const Provenance> next;
+ if (auto prov = optionalValueAt(obj, "next"))
+ next = Provenance::from_json(*prov);
+ return make_ref<BuildProvenance>(
+ StorePath(getString(valueAt(obj, "drv"))),
+ getString(valueAt(obj, "output")),
+ next);
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Provenance::Register registerCopiedProvenance("copied", [](nlohmann::json json) { | |
| auto & obj = getObject(json); | |
| std::shared_ptr<const Provenance> next; | |
| if (auto prov = optionalValueAt(obj, "next")) | |
| next = Provenance::from_json(*prov); | |
| return make_ref<CopiedProvenance>(getString(valueAt(obj, "from")), next); | |
| }); | |
| Provenance::Register registerCopiedProvenance("copied", [](nlohmann::json json) { | |
| auto & obj = getObject(json); | |
| std::shared_ptr<const Provenance> next; | |
| if (auto prov = optionalValueAt(obj, "next")) | |
| next = Provenance::from_json(*prov); | |
| return make_ref<CopiedProvenance>(getString(valueAt(obj, "from")), next); | |
| }); | |
| Provenance::Register registerBuildProvenance("build", [](nlohmann::json json) { | |
| auto & obj = getObject(json); | |
| std::shared_ptr<const Provenance> next; | |
| if (auto prov = optionalValueAt(obj, "next")) | |
| next = Provenance::from_json(*prov); | |
| return make_ref<BuildProvenance>( | |
| StorePath(getString(valueAt(obj, "drv"))), | |
| getString(valueAt(obj, "output")), | |
| next); | |
| }); |
🤖 Prompt for AI Agents
In `@src/libstore/provenance.cc` around lines 27 - 33, Add a Provenance::Register
for the "build" type so Provenance::from_json() can deserialize BuildProvenance;
mirror the existing registerCopiedProvenance pattern: register a
Provenance::Register("build", [](nlohmann::json json){ ... }) that calls
getObject(json), extracts optional next via optionalValueAt and converts it with
Provenance::from_json, and constructs and returns make_ref<BuildProvenance>(...)
passing the string/fields read from valueAt/getString for the keys
BuildProvenance expects (match the BuildProvenance constructor parameters).
Ensure the lambda uses the same helper functions (getObject, optionalValueAt,
valueAt, getString) and returns a shared ref to BuildProvenance.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/libstore/store-api.cc`:
- Around line 875-886: Replace the use of srcStore.config.getHumanReadableURI()
in addCopiedProvenance with the direct stable renderer: call
srcStore.config.getReference().render(/*withParams=*/false) when constructing
the CopiedProvenance so the persisted provenance uses the same stable identifier
pattern as other stores (e.g., binary-cache-store.cc) and avoids relying on
getHumanReadableURI() indirection.
♻️ Duplicate comments (2)
src/libstore/include/nix/store/legacy-ssh-store.hh (1)
76-79: Verify if returningtrueis intentional given ServeProto limitations.A past review raised concerns that ServeProto doesn't support provenance transmission in its ReadConn/WriteConn structures. The method was renamed from
isUsefulProvenance()toincludeInProvenance(), but the return value remainedtrue.If
includeInProvenance()means "should this store be recorded in provenance when paths are copied FROM it", then returningtrueis appropriate. If it means "can this store persist/transmit provenance data", then it may be misleading.src/libfetchers/tarball.cc (1)
12-34: Confirmfetchurlprovenance is registered for deserialization.I don’t see the
Provenance::Registerhookup in this file; if it’s missing elsewhere,"type":"fetchurl"will deserialize asUnknownProvenance. Please verify it exists (or add it in a fetchers provenance registry).#!/bin/bash # Verify fetchurl provenance registration exists somewhere in the repo. rg -nP 'Provenance::Register|register.*Provenance' src/libfetchers src/libutil src/libstore rg -nP '"fetchurl"' src/libfetchers src/libutil src/libstore
| /** | ||
| * Wrap upstream provenance in a "copied" provenance record to record | ||
| * where the path was copied from. But uninformative origins like | ||
| * LocalStore are omitted. | ||
| */ | ||
| static std::shared_ptr<const Provenance> | ||
| addCopiedProvenance(std::shared_ptr<const Provenance> provenance, Store & srcStore) | ||
| { | ||
| if (!srcStore.includeInProvenance()) | ||
| return provenance; | ||
| return std::make_shared<const CopiedProvenance>(srcStore.config.getHumanReadableURI(), provenance); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how getHumanReadableURI is used in provenance contexts and whether there's a stable alternative
# Find other usages of CopiedProvenance to see what URI format is expected
echo "=== CopiedProvenance usages ==="
rg -n "CopiedProvenance" --type cpp -A 3 -B 1
echo ""
echo "=== getReference().render() usages ==="
# Check if there's a pattern for stable store URIs
rg -n "getReference\(\).render" --type cpp -C 2
echo ""
echo "=== getHumanReadableURI usages (context) ==="
# Find all usages of getHumanReadableURI to understand the pattern
rg -n "getHumanReadableURI\(\)" --type cpp -B 1 -A 2 | head -50Repository: DeterminateSystems/nix-src
Length of output: 14078
Use getReference().render(/*withParams=*/false) instead of getHumanReadableURI() for consistency with provenance storage.
While getHumanReadableURI() currently delegates to getReference().render(/*withParams=*/false) and thus produces a stable value, provenance records are persisted in JSON and the codebase pattern for stable, persistent identifiers consistently uses getReference().render() directly (see binary-cache-store.cc, store-api.cc). Using the direct method avoids unnecessary indirection and ensures the code won't break if getHumanReadableURI() behavior changes in the future.
🤖 Prompt for AI Agents
In `@src/libstore/store-api.cc` around lines 875 - 886, Replace the use of
srcStore.config.getHumanReadableURI() in addCopiedProvenance with the direct
stable renderer: call
srcStore.config.getReference().render(/*withParams=*/false) when constructing
the CopiedProvenance so the persisted provenance uses the same stable identifier
pattern as other stores (e.g., binary-cache-store.cc) and avoids relying on
getHumanReadableURI() indirection.
Motivation
This is an updated version of upstream NixOS#11749.
Nix historically has been bad at being able to answer the question "where did this store path come from", i.e. to provide traceability from a store path back to the Nix expression from which is was built. Nix tracks the "deriver" of a store path (the
.drvfile that built it) but that's pretty useless in practice, since it doesn't link back to the Nix expressions.So this PR adds a "provenance" field (a JSON object) to the
ValidPathstable and to.narinfofiles that describes where the store path came from and how it can be reproduced.There are currently the following types of provenance:
copied: Records that the store path was copied or substituted from another store (typically a binary cache). Its "from" field is the URL of the origin store. Its "provenance" field propagates the provenance of the store path on the origin store.build: Records that the store path was produced by building a derivation. This is equivalent for the "deriver" field, but it has a nested "provenance" field that records how the .drv file was created.tree: The store path is the result of a call tofetchTree(e.g. it's a flake or flake input). It includes the fetcher attributes.subpath: The store path was created by copying the subpath of some other provenance (e.g./foo/builder.shfrom atree) .flake: Records that the store path was created during the evaluation of a flake output.fetchurl: The store path is the result of abuiltins.fetchurlcall.Context
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests