Add nix provenance verify command#356
Merged
edolstra merged 8 commits intoFeb 20, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a provenance verification CLI command and tests; introduces Fetchurl provenance type and provenance-aware store handling; adds OverrideProvenanceSourceAccessor; makes flake evaluation cache opt-in per-call via an InstallableFlake flag and an allowEvalCache parameter to openEvalCache. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as CLI
participant ProvCmd as ProvenanceCmd
participant Store as Store
participant Eval as FlakeEvaluator
participant Builder as Builder
User->>CLI: nix provenance verify <path(s)> (--no-rebuild?)
CLI->>ProvCmd: run verification
ProvCmd->>Store: lookup provenance for target path
alt CopiedProvenance
ProvCmd->>Store: fetch origin NAR/hash
Store-->>ProvCmd: origin hash
ProvCmd->>Store: compare local NAR/hash
else BuildProvenance
ProvCmd->>Store: confirm derivation -> request rebuild (unless --no-rebuild)
ProvCmd->>Builder: build derivation
Builder-->>ProvCmd: produced outputs
ProvCmd->>Store: compare outputs to target path
else FlakeProvenance
ProvCmd->>Eval: evaluate flake (openEvalCache with allowEvalCache flag)
Eval-->>ProvCmd: instantiate outputs
ProvCmd->>Store: compare instantiated outputs to target path
else Tree/Subpath/Fetchurl
ProvCmd->>Store: fetch/resolve tree or fetch URL
Store-->>ProvCmd: produced/store path
ProvCmd->>Store: compare to recorded target
end
ProvCmd-->>CLI: aggregate results
CLI-->>User: exit status (0 success / non-zero fail)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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)
Comment |
edolstra
force-pushed
the
eelcodolstra/nix-299-add-a-command-nix-provenance-verify
branch
from
2f37a2f to
60cdca4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/nix/provenance-verify.md (2)
5-9: Consider showing example output.The example demonstrates the command syntax but doesn't show what output users should expect. Based on the PR description, the command produces detailed step-by-step verification results with checkmarks, URLs, and paths. Including sample output would help users understand what success (and failure) looks like.
📄 Suggested enhancement
Add example output after the command:
```console # nix provenance verify /run/current-system + /nix/store/...-nixos-system-hostname-24.05... + ✓ fetched tree git+https://github.com/example/repo?rev=abc123... + ✓ evaluated git+https://github.com/example/repo#nixosConfigurations.hostname + ✓ re-instantiated path /nix/store/...-nixos-system-hostname-24.05.drv + ✓ rebuilt derivation /nix/store/...-nixos-system-hostname-24.05</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@src/nix/provenance-verify.mdaround lines 5 - 9, Add a concrete example
output block after the example command showing expected verification results for
"nix provenance verify /run/current-system": include lines with the store path
(e.g., /nix/store/...-nixos-system-hostname-24.05...), step-by-step checkmarks
for fetched tree (git+https://...rev=...), evaluated flakes/config
(git+https://...#nixosConfigurations.hostname), re-instantiated drv path
(/nix/store/... .drv) and rebuilt derivation (/nix/store/...), and also include
a short failing example to show how a failed check is displayed; place this
human-readable console output directly beneath the command example in
src/nix/provenance-verify.md.</details> --- `13-19`: **Consider documenting edge cases and limitations.** The description covers the happy path well but doesn't mention: - What happens when a store path has no provenance information - Performance implications (rebuilding derivations can be time-consuming) - Whether partial verification results are shown when some steps fail Adding a note about these scenarios would help users understand the command's behavior in different situations. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@src/nix/provenance-verify.mdaround lines 13 - 19, Update
src/nix/provenance-verify.md to add an "Edge cases and limitations" section for
the provenance-verify command that explicitly documents: behavior when a store
path has no provenance information (what the tool reports and exit code),
performance implications of rebuilding derivations or re-running heavy
verification steps (warn about potential long runtimes and suggest flags or
resource expectations), and how partial verification results are presented when
some steps fail (whether the tool shows per-step success/failure, aggregates
overall status, or aborts on first error). Reference the command name
provenance-verify and any existing headings in the file so the new note sits
adjacent to the happy-path usage.</details> </blockquote></details> <details> <summary>src/libcmd/include/nix/cmd/installable-flake.hh (1)</summary><blockquote> `36-45`: **Clarify the semantics of `useEvalCache`.** This new public flag is easy to confuse with global `state.settings.useEvalCache`/`pureEval`. A short inline comment would reduce ambiguity about its role as a per-installable override. <details> <summary>💬 Suggested inline comment</summary> ```diff - bool useEvalCache = true; + // Per-installable override for eval cache usage; still gated by global settings. + bool useEvalCache = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/libcmd/include/nix/cmd/installable-flake.hh` around lines 36 - 45, The public bool field useEvalCache on InstallableFlake is ambiguous relative to the global state.settings.useEvalCache/pureEval; add a concise inline comment above the declaration of useEvalCache in the InstallableFlake struct clarifying that this flag is a per-installable override (not a global setting), describing its effect (e.g., whether it forces use or disables of the evaluation cache for this flake only) and noting precedence relative to global settings (which wins when unspecified). Mention the struct name InstallableFlake and the field useEvalCache in the comment so future readers can quickly understand its scope and semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nix/provenance.cc`:
- Around line 331-334: The logger call references out-of-scope build->drvPath
and contains a typo; replace build->drvPath with the in-scope store path
variable used in this function (the variable passed to or named as the current
store path in scope) when calling store.printStorePath, and change the message
from "did non re-instantiate path" to "did not re-instantiate path" (keep the
callsite logger->cout and return {false, std::monostate{}} as-is).
- Around line 220-223: The code calls openStore(copied->from) and
fromStore->queryPathInfo(path) without handling exceptions, so failures abort
the whole command; wrap the origin-store lookup and query in a try/catch around
the sequence using openStore, fromStore->queryPathInfo(path) (and optionally
store.queryPathInfo(path)) and on any exception log the error including
copied->from and path, skip comparing narHash for that path, and continue
processing remaining paths (i.e., return/continue instead of letting the
exception propagate). Ensure you check that fromStore and the returned
fromInfo/localInfo are valid before accessing narHash.
- Around line 309-324: installable.toDerivedPaths() can throw and currently will
abort the whole operation; wrap the call to InstallableFlake::toDerivedPaths()
in a try/catch that catches the exception type(s) thrown by evaluation (e.g.,
std::exception or the project-specific eval error), log a clear error via
logger->cout or logger->error including installable.flakeRef.to_string(true) and
flake->flakeOutput, and skip further processing of that InstallableFlake so the
loop continues; ensure installable.useEvalCache handling remains unchanged and
any resources are left in a consistent state after the catch.
- Around line 240-242: Wrap the call to
queryPartialDerivationOutputMap(build->drvPath) in a try/catch and treat
exceptions as "derivation not present locally" rather than letting them abort
the whole verification; if an exception occurs or the output is not found in
outputMap (the current it == outputMap.end() branch), record/report a per-path
failure for this build->output (and continue to the next path) instead of
exiting the command—update the surrounding logic that uses outputMap/it to
handle a missing derivation case (e.g., by setting a missing flag or invoking
the existing per-path failure reporting function) so verification proceeds for
other paths.
- Around line 326-329: The code currently builds instantiatedPaths by iterating
drvHashes (which is a global memoization cache) causing stale entries; instead
call and use the return value of installable.toDerivedPaths() to populate
instantiatedPaths (or, if you must keep drvHashes usage, explicitly clear
drvHashes before calling toDerivedPaths()); update the block that fills
instantiatedPaths (currently using drvHashes.cvisit_all and
instantiatedPaths.insert) to use the set returned by
installable.toDerivedPaths() so only paths from the current evaluation are
considered.
---
Nitpick comments:
In `@src/libcmd/include/nix/cmd/installable-flake.hh`:
- Around line 36-45: The public bool field useEvalCache on InstallableFlake is
ambiguous relative to the global state.settings.useEvalCache/pureEval; add a
concise inline comment above the declaration of useEvalCache in the
InstallableFlake struct clarifying that this flag is a per-installable override
(not a global setting), describing its effect (e.g., whether it forces use or
disables of the evaluation cache for this flake only) and noting precedence
relative to global settings (which wins when unspecified). Mention the struct
name InstallableFlake and the field useEvalCache in the comment so future
readers can quickly understand its scope and semantics.
In `@src/nix/provenance-verify.md`:
- Around line 5-9: Add a concrete example output block after the example command
showing expected verification results for "nix provenance verify
/run/current-system": include lines with the store path (e.g.,
/nix/store/...-nixos-system-hostname-24.05...), step-by-step checkmarks for
fetched tree (git+https://...rev=...), evaluated flakes/config
(git+https://...#nixosConfigurations.hostname), re-instantiated drv path
(/nix/store/... .drv) and rebuilt derivation (/nix/store/...), and also include
a short failing example to show how a failed check is displayed; place this
human-readable console output directly beneath the command example in
src/nix/provenance-verify.md.
- Around line 13-19: Update src/nix/provenance-verify.md to add an "Edge cases
and limitations" section for the provenance-verify command that explicitly
documents: behavior when a store path has no provenance information (what the
tool reports and exit code), performance implications of rebuilding derivations
or re-running heavy verification steps (warn about potential long runtimes and
suggest flags or resource expectations), and how partial verification results
are presented when some steps fail (whether the tool shows per-step
success/failure, aggregates overall status, or aborts on first error). Reference
the command name provenance-verify and any existing headings in the file so the
new note sits adjacent to the happy-path usage.
edolstra
force-pushed
the
eelcodolstra/nix-299-add-a-command-nix-provenance-verify
branch
2 times, most recently
from
f9ed4d6 to
2640dca
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/libflake/include/nix/flake/flake.hh (1)
247-251: Consider documenting the newallowEvalCacheparameter.The function's doc comment should describe what
allowEvalCachecontrols, especially since thenix provenance verifyuse case requires disabling the cache. This helps future maintainers understand when to passfalse.📝 Suggested documentation update
/** * Open an evaluation cache for a flake. + * + * `@param` allowEvalCache If false, bypass the evaluation cache + * entirely (useful for provenance verification). */ ref<eval_cache::EvalCache> openEvalCache(EvalState & state, ref<const LockedFlake> lockedFlake, bool allowEvalCache = true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/libflake/include/nix/flake/flake.hh` around lines 247 - 251, The doc comment for openEvalCache lacks an explanation of the new allowEvalCache parameter; update the comment above ref<eval_cache::EvalCache> openEvalCache(EvalState & state, ref<const LockedFlake> lockedFlake, bool allowEvalCache = true) to describe what allowEvalCache controls (e.g., whether to permit using or creating an evaluation cache for the given flake), the default behavior when true, and the reason callers like nix provenance verify should pass false to disable the cache; keep the description brief and mention the effect on cache lookup/creation and any important side-effects.src/nix/provenance-verify.md (2)
5-19: Document the--no-rebuildoption here.The command introduces a user-facing flag, but the doc doesn’t mention it. This is a discoverability gap.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nix/provenance-verify.md` around lines 5 - 19, Add documentation for the new --no-rebuild flag to the "nix provenance verify" section: describe that passing --no-rebuild skips the derivation rebuild step (so source fetch and flake evaluation still run), note that it changes exit-code semantics because rebuild failures are ignored, state the default behavior (rebuild is performed unless --no-rebuild is given), and include a short usage example like "nix provenance verify --no-rebuild /run/current-system" so users can discover how to run verification without rebuilding.
13-17: Rephrase the bullet list to avoid repeated “That …”.This reads a bit repetitive and can be tightened.
✍️ Possible rewording
-* That source trees can be fetched. -* That flake evaluations result in the instantiation of the desired store paths (most commonly, store derivations). -* That derivations can be successfully rebuilt, producing identical outputs. +* Ensure source trees can be fetched. +* Ensure flake evaluations instantiate the desired store paths (most commonly, store derivations). +* Ensure derivations can be rebuilt, producing identical outputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nix/provenance-verify.md` around lines 13 - 17, Replace the repetitive bulleted sentences under the "Verify the provenance of one or more store paths" paragraph in src/nix/provenance-verify.md by consolidating each line into concise action phrases (e.g., "Fetch source trees.", "Evaluate flakes to confirm instantiation of the desired store paths.", "Rebuild derivations to verify identical outputs.") so you no longer start each bullet with "That" and the list reads tighter and more direct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/nix/provenance.cc`:
- Around line 346-349: The failure message uses an out-of-scope symbol and
contains a typo: replace the reference to build->drvPath with the in-scope
variable path (i.e., call store.printStorePath(path)) and fix the string to "did
not re-instantiate" so the logger->cout in the instantiatedPaths check prints
the correct path and readable message; update the call in the block that checks
instantiatedPaths, referencing instantiatedPaths, path, logger->cout, and
store.printStorePath.
- Around line 338-340: Wrap the call to installable.toDerivedPaths() in a
try/catch so evaluation failures don't abort the whole command; on catch log a
clear error via logger->cout (including installable.flakeRef.to_string(true),
flake->flakeOutput and the caught exception/message) and then continue (skip
this installable) rather than propagating the exception. Ensure the catch covers
the exception types thrown by toDerivedPaths() (or std::exception) and keeps the
existing success log (logger->cout("✅ evaluated ...")) only on successful
evaluation.
- Around line 342-344: The code currently builds instantiatedPaths from the
global drvHashes cache which can include stale entries; instead, derive the set
of current derivation paths from the installable object (use
installable.toDerivedPaths()) and use that to populate instantiatedPaths (or
alternatively clear drvHashes before this evaluation) so re-instantiation checks
only consider paths produced by the current installable; update the block
referencing drvHashes and instantiatedPaths to use installable.toDerivedPaths()
as the source of truth.
- Around line 229-234: The code in the CopiedProvenance handling block (inside
the if (auto copied = std::dynamic_pointer_cast<const
CopiedProvenance>(provenance)) branch) calls openStore(copied->from) and
queryPathInfo(path) which can throw; wrap the per-path lookup sequence
(openStore, fromStore->queryPathInfo, store.queryPathInfo and the narHash
comparison) in a try/catch that catches exceptions, logs or records the error
for that path, and continues processing other paths instead of letting the
exception abort the whole command — ensure you use the existing variables
(copied, openStore, fromStore, store, queryPathInfo, path, localInfo, fromInfo,
narHash) and set the per-path success flag appropriately when an exception
occurs.
- Around line 250-258: Wrap the call to
queryPartialDerivationOutputMap(build->drvPath) in a try-catch so a
missing-local-derivation exception doesn't abort verification; if it throws, log
a warning via logger->cout referencing build->drvPath and build->output and skip
the strict output check (do not return {false,...}) so verification continues,
otherwise keep the existing logic that inspects outputMap and compares it to
build->output.
---
Nitpick comments:
In `@src/libflake/include/nix/flake/flake.hh`:
- Around line 247-251: The doc comment for openEvalCache lacks an explanation of
the new allowEvalCache parameter; update the comment above
ref<eval_cache::EvalCache> openEvalCache(EvalState & state, ref<const
LockedFlake> lockedFlake, bool allowEvalCache = true) to describe what
allowEvalCache controls (e.g., whether to permit using or creating an evaluation
cache for the given flake), the default behavior when true, and the reason
callers like nix provenance verify should pass false to disable the cache; keep
the description brief and mention the effect on cache lookup/creation and any
important side-effects.
In `@src/nix/provenance-verify.md`:
- Around line 5-19: Add documentation for the new --no-rebuild flag to the "nix
provenance verify" section: describe that passing --no-rebuild skips the
derivation rebuild step (so source fetch and flake evaluation still run), note
that it changes exit-code semantics because rebuild failures are ignored, state
the default behavior (rebuild is performed unless --no-rebuild is given), and
include a short usage example like "nix provenance verify --no-rebuild
/run/current-system" so users can discover how to run verification without
rebuilding.
- Around line 13-17: Replace the repetitive bulleted sentences under the "Verify
the provenance of one or more store paths" paragraph in
src/nix/provenance-verify.md by consolidating each line into concise action
phrases (e.g., "Fetch source trees.", "Evaluate flakes to confirm instantiation
of the desired store paths.", "Rebuild derivations to verify identical
outputs.") so you no longer start each bullet with "That" and the list reads
tighter and more direct.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nix/provenance-verify.md (1)
13-17: Vary the bullet phrasing to avoid repetition.Three consecutive bullets start with “That”; a small reword makes the list smoother.
✏️ Suggested wording tweak
-* That source trees can be fetched. -* That flake evaluations result in the instantiation of the desired store paths (most commonly, store derivations). -* That derivations can be successfully rebuilt, producing identical outputs. +* Source trees can be fetched. +* Flake evaluations result in the instantiation of the desired store paths (most commonly, store derivations). +* Derivations can be successfully rebuilt, producing identical outputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nix/provenance-verify.md` around lines 13 - 17, The three consecutive bullets in the provenance-verify description all start with "That", making the list repetitive; reword the second and third bullets to vary phrasing while preserving meaning — e.g., keep the first as "That source trees can be fetched.", change the second ("That flake evaluations result in the instantiation of the desired store paths (most commonly, store derivations).") to "Ensure flake evaluations instantiate the desired store paths (commonly store derivations)." and change the third ("That derivations can be successfully rebuilt, producing identical outputs.") to "Confirm derivations can be rebuilt successfully, producing identical outputs." Update the three bullets in src/nix/provenance-verify.md accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/nix/provenance.cc`:
- Around line 338-340: Wrap the call to installable.toDerivedPaths() in a
try/catch so flake evaluation exceptions don't abort verification; catch the
exception, log the error via logger (include
installable.flakeRef.to_string(true) and flake->flakeOutput) and skip or mark
this installable as failed so processing can continue for others instead of
letting the exception propagate from toDerivedPaths().
- Around line 250-253: Wrap the call to
store.queryPartialDerivationOutputMap(build->drvPath) in a try-catch so
exceptions thrown for missing/invalid derivations are handled per-path; if an
exception occurs, log/record the error for this build (using the existing
logging mechanism) and treat it as if the output map were empty so the
subsequent check on outputMap.find(build->output) behaves correctly and
processing continues for other paths. Ensure you reference and protect the call
in the same block where outputMap, it, and build->output are used.
- Around line 229-233: The current block that handles CopiedProvenance assumes
openStore() and queryPathInfo() cannot fail; wrap the per-path operations
(calling openStore(copied->from) and both store.queryPathInfo(path) and
fromStore->queryPathInfo(path)) in a try/catch so exceptions are caught per
path, set success=false for that path, log or record the specific error (include
copied->from and path), ensure fromStore is checked before using it, and
continue processing remaining paths instead of letting the exception abort the
entire command.
- Around line 342-346: The check is incorrectly reading the global cache
drvHashes (via cvisit_all) which can be stale; replace uses of the global
drvHashes here with the evaluation-local hash set passed into or owned by the
current evaluation context (e.g., a parameter or member like
currentDrvHashes/evalContext->drvHashes) so instantiatedPaths is populated from
that local collection rather than the global drvHashes, update the cvisit_all
call and the subsequent instantiatedPaths.contains(path) check to use that local
hash set, and keep the existing TODO about non-derivation paths but do not
reference the global drvHashes.
---
Nitpick comments:
In `@src/nix/provenance-verify.md`:
- Around line 13-17: The three consecutive bullets in the provenance-verify
description all start with "That", making the list repetitive; reword the second
and third bullets to vary phrasing while preserving meaning — e.g., keep the
first as "That source trees can be fetched.", change the second ("That flake
evaluations result in the instantiation of the desired store paths (most
commonly, store derivations).") to "Ensure flake evaluations instantiate the
desired store paths (commonly store derivations)." and change the third ("That
derivations can be successfully rebuilt, producing identical outputs.") to
"Confirm derivations can be rebuilt successfully, producing identical outputs."
Update the three bullets in src/nix/provenance-verify.md accordingly.
2640dca to
69a3c6f
Compare
Example output: $ nix provenance verify /run/current-system /nix/store/bwmc8dmbf01b07sgzfa48f5y5dgj48am-nixos-system-machine-25.11.20260214.3aadb7c ✅ fetched tree 'git+file:///my-repo?ref=refs/heads/master&rev=f6ba198ac4c8010cb0879978541189f201b734f1' ✅ evaluated 'git+file:///my-repo?ref=refs/heads/master&rev=f6ba198ac4c8010cb0879978541189f201b734f1#nixosConfigurations.machine.config.system.build.toplevel' ✅ re-instantiated path '/nix/store/qi4z9bg89sxjrs5ar11627hgwjiv42m3-nixos-system-machine-25.11.20260214.3aadb7c.drv' ✅ rebuilt derivation '/nix/store/qi4z9bg89sxjrs5ar11627hgwjiv42m3-nixos-system-machine-25.11.20260214.3aadb7c.drv^out'
edolstra
force-pushed
the
eelcodolstra/nix-299-add-a-command-nix-provenance-verify
branch
from
69a3c6f to
329c89b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nix/provenance-verify.md (1)
13-17: Rephrase bullets to avoid repeated sentence starts.Minor style tweak to make the list read more smoothly.
💡 Suggested wording
-* That source trees can be fetched. -* That flake evaluations result in the instantiation of the desired store paths (most commonly, store derivations). -* That derivations can be successfully rebuilt, producing identical outputs. +* Source trees can be fetched. +* Flake evaluations instantiate the desired store paths (most commonly, store derivations). +* Derivations can be successfully rebuilt, producing identical outputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nix/provenance-verify.md` around lines 13 - 17, The three bulleted lines in src/nix/provenance-verify.md repeat the sentence start "That", making the list clunky; rephrase each bullet in active/parallel phrasing (e.g., "Fetch source trees.", "Evaluate flakes to instantiate the desired store paths (most commonly store derivations).", "Rebuild derivations to produce identical outputs.") so the verbs start each bullet and the items read smoothly and consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/nix/provenance.cc`:
- Around line 271-274: Wrap the call to
queryPartialDerivationOutputMap(build->drvPath) in a try/catch and treat any
exception as a missing-derivation case for that specific build path instead of
letting it abort verification; specifically, catch the thrown exception around
the call that produces outputMap, log or record the per-path failure, set
behavior equivalent to outputMap.find(build->output) == outputMap.end() (or
otherwise mark this build as failed) and continue processing other builds;
ensure you reference queryPartialDerivationOutputMap, outputMap, and
build->drvPath/build->output when implementing the change so only the
single-path failure is handled and no global abort occurs.
- Around line 246-255: In the CopiedProvenance branch (check for
std::dynamic_pointer_cast<const CopiedProvenance>(provenance)) avoid aborting
the whole verification when openStore() or queryPathInfo() throws: wrap the
openStore(copied->from) and subsequent
store.queryPathInfo(*path)/fromStore->queryPathInfo(*path) calls in try/catch
blocks, catch and log the specific exception via logger (including the path and
copied->from), mark the per-path verification as failed (set success=false) and
continue with remaining paths instead of returning immediately; ensure the final
return uses the accumulated success flag to indicate overall verification
result.
- Around line 366-368: Wrap the call to installable.toDerivedPaths() in a
try/catch so that evaluation errors for a given installable are caught and
logged instead of aborting the whole command; when catching, call logger->cerr
or logger->cout with a clear per-path failure message including
installable.flakeRef.to_string(true) and flake->flakeOutput and continue to the
next installable. Ensure you only catch the specific exception type thrown by
evaluation (or std::exception if unknown) and do not rethrow so processing
proceeds for other items.
---
Nitpick comments:
In `@src/nix/provenance-verify.md`:
- Around line 13-17: The three bulleted lines in src/nix/provenance-verify.md
repeat the sentence start "That", making the list clunky; rephrase each bullet
in active/parallel phrasing (e.g., "Fetch source trees.", "Evaluate flakes to
instantiate the desired store paths (most commonly store derivations).",
"Rebuild derivations to produce identical outputs.") so the verbs start each
bullet and the items read smoothly and consistently.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nix/prefetch.cc`:
- Around line 148-156: The code is creating a FetchurlProvenance from
url.to_string() and persisting sensitive data; instead, sanitize the URL before
constructing FetchurlProvenance by removing credentials and query parameters
(mirror the tarball fetch path sanitization), then pass the sanitized string
into std::make_shared<FetchurlProvenance>. Concretely, compute a sanitized_url
from url (strip username/password and drop query string), and replace the inline
url.to_string() inside the make_ref<OverrideProvenanceSourceAccessor>(...,
unpack ? nullptr : std::make_shared<FetchurlProvenance>(...)) call so
addToStoreSlow receives provenance without secrets.
In `@src/nix/provenance.cc`:
- Around line 527-536: The code shadows the `success` returned from
verify(store, {}, subpath->next), so upstream failures are lost; in the
SubpathProvenance handling (function using verify and verifySourcePath) preserve
the original success (e.g., rename the verify return flag to childSuccess or
reuse the existing variable) and compute the final success as childSuccess &&
(!path || verifySourcePath(store, *path, sourcePath)); then return that combined
success along with the pair (std::move(p->first), std::move(sourcePath)) so
upstream failure is not dropped.
- Around line 566-579: The download call using getFileTransfer()->download(req,
hashSink) can throw and currently isn't caught; wrap the download (and related
HashSink usage) in a try/catch that mirrors other fetch paths: catch
std::exception (or the specific transfer exception type used elsewhere), log a
clear error via logger->cout including fetchurl->url and the exception.what(),
and return {false, std::monostate{}} to abort verification of this fetchurl
without crashing the whole command; keep the existing hash mismatch logic
(comparing hash to info->ca->hash) unchanged after the guarded download.
---
Duplicate comments:
In `@src/nix/provenance.cc`:
- Around line 389-412: queryPartialDerivationOutputMap can throw when a
derivation is absent, which currently aborts verification; wrap the call to
store.queryPartialDerivationOutputMap(build->drvPath) in a try/catch, catch the
exception type thrown (or std::exception), log a per-path failure message
(similar style to the existing logger->cout messages) and return {false,
std::monostate{}} for that path instead of letting the exception propagate;
update the block around queryPartialDerivationOutputMap / outputMap lookup in
provenance.cc (the code using outputMap.find(build->output)) to handle the
"derivation missing" case from the catch and proceed without terminating the
whole verification.
- Around line 364-383: Wrap calls that can throw—openStore(copied->from),
store.queryPathInfo(*path), and fromStore->queryPathInfo(*path)—in try/catch so
exceptions don’t abort verification; if any of those throw, catch
std::exception, log a clear non-fatal message via logger->cout including the
exception text and the origin (copied->from and path), set success = false and
skip the NAR-hash comparison if either path info is unavailable, then continue
by calling verify(store, path, copied->next) and combine its result with the
local success flag; keep the existing behavior of returning {success &&
nextSuccess, result} and ensure CopiedProvenance, openStore, queryPathInfo,
verify and logger->cout are the referenced symbols when making the changes.
- Around line 485-492: Wrap the call to InstallableFlake::toDerivedPaths() in a
try/catch block so evaluation exceptions do not abort the whole run: catch
std::exception (and a generic catch-all), log a clear error via logger->cout
(including installable.flakeRef.to_string(true), flake->flakeOutput and the
exception message), mark or record the installable as failed/skip further
processing for that flake, and continue the loop; ensure
evalState->waitForAllPaths() and subsequent success-only logic only run when
toDerivedPaths() completed without throwing so the program keeps verifying other
flakes.
cole-h
approved these changes
Feb 20, 2026
…add-a-command-nix-provenance-verify
edolstra
deleted the
eelcodolstra/nix-299-add-a-command-nix-provenance-verify
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Example output:
This PR also adds provenance for
nix store prefetch-file, and it fixes a bug where we set unverifiable provenance for filtered paths (i.e.builtins.path { filter = ...; }).Motivation
Context
Summary by CodeRabbit
New Features
nix provenance verifycommand for end-to-end provenance checks.Behavior Changes
Documentation
Tests