fix(proofs): refuse raw reference rows in the V1 layer verifier (#862)

[QuantumExplorer]

Closes #862 (audit P14).

Finding was real — reproduced at runtime on develop

A KVValueHash / KVValueHashFeatureType proof node hashes only (key, value_hash). The merk-level V1 guard from #553 refuses only item elements on those forms, so reference-family bytes pass through unbound. Taking an honest KVRefValueHash(key, target, H(ref)) node, recomputing combine_hash(H(ref), H(target)), and re-presenting the row as KVValueHash(key, <forged reference bytes>, combined) keeps the Merk root intact. verify_layer_proof_v1 then consumed reference metadata nothing had authenticated:

1. Forged result row — the forged Reference (attacker-chosen target path) was returned as the proven element for the key, through both verify_query and verify_query_raw. The KVValueHashFeatureType shape under a ProvableCountTree and a forged ReferenceWithSumItem behaved the same.
2. Silent descent skip — with a subquery at that key and the lower layer dropped, every branch of the terminal arm fell through without error: the proof verified against the genuine root with a populated subtree's rows missing (zero rows returned).

#874 (#858) does not cover this: it binds what the honest prover commits for a reference, not what a forged node form can present.

Fix

verify_layer_proof_v1 rejects any raw Reference / ReferenceWithSumItem row (wrapped or not) immediately after deserializing it, before the element type can steer a result or a descent. An honest V1 prover rewrites every row the verifier consumes into a KVRefValueHash* node carrying the dereferenced target, so no released proof is affected. The check is therefore not version-gated, matching the existing ungated count-offset rejection (#817) and the #553/#630/#633 pattern. V0 prover/verifier untouched.

Why not in the merk-level guard

I first added the rejection to the merk execute_proof guard at proof version 1 and it broke an honest proof (test_mixed_level_proofs, limit 4): a reference row past the query limit legitimately stays a bare KVValueHash in released V1 proofs, because the prover's post-pass only rewrites rows within the limit. The merk verifier walks every node, while the GroveDB row loop stops before reading that node. So the row consumer is the right place; the merk guard now documents why references pass there, and the invariant is recorded in docs/audit-non-issues.md so it isn't re-filed.

Tests

grovedb/src/tests/unbound_reference_row_tests.rs:

• honest reference proof returns the dereferenced target
• forged reference row on KVValueHash rejected (verify_query and verify_query_raw)
• forged reference row on KVValueHashFeatureType (ProvableCountTree parent) rejected
• forged ReferenceWithSumItem row rejected
• forged reference cannot hide a populated subtree (descent-skip shape) rejected

All four forgeries verified successfully before the fix. cargo test -p grovedb-merk (726) and cargo test -p grovedb --features full,verify,unsafe-dump-load (3032) pass; cargo fmt clean; clippy clean on touched files.

Note on overlap: #880 (#869) edits the same terminal arm (adds a catch-all "neither descended nor bound" else). This check sits at the top of the row loop, so the two should compose.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • V1 proof verification now rejects unbound raw reference rows, preventing forged result data and hidden subtree descents.
  • Invalid or tampered proofs are consistently reported as invalid instead of being accepted or silently skipped.

• Documentation

  • Added guidance explaining which proof node values are cryptographically bound and how reference rows are validated.

• Tests

  • Added coverage for forged references, reference-with-sum items, count-carrying nodes, and omitted subtree proofs.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 69a6238e-f883-4d49-bf57-47f608b17834

📥 Commits

Reviewing files that changed from the base of the PR and between 5966e62 and b82dd46.

📒 Files selected for processing (7)

• docs/audit-non-issues.md
• docs/book/src/proof-system.md
• grovedb/src/operations/proof/verify.rs
• grovedb/src/tests/mod.rs
• grovedb/src/tests/unbound_empty_tree_tests.rs
• grovedb/src/tests/unbound_reference_row_tests.rs
• merk/src/proofs/query/verify.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change rejects raw reference elements during V1 GroveDB proof verification. It adds tests for forged result rows and hidden subtrees, and documents the binding rules across Merk and GroveDB proof verification.

Changes

V1 reference-row verification

Layer / File(s)	Summary
V1 verification guard and proof contract grovedb/src/operations/proof/verify.rs, merk/src/proofs/query/verify.rs, docs/audit-non-issues.md, docs/book/src/proof-system.md	verify_layer_proof_v1 rejects raw reference elements with InvalidProof. Merk documentation records why reference elements remain admitted in KVValueHash* nodes.
Forged reference-row test coverage grovedb/src/tests/unbound_reference_row_tests.rs, grovedb/src/tests/unbound_empty_tree_tests.rs, grovedb/src/tests/mod.rs	Tests cover forged plain and feature-type reference rows, ReferenceWithSumItem values, direct query results, and skipped populated subtrees.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to b82dd

The proof verifier now fails closed for unauthenticated raw reference rows while preserving honest V1 reference proofs. No actionable merge risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant Prover
  participant MerkVerifier
  participant verify_layer_proof_v1
  participant QueryCaller
  Prover->>MerkVerifier: Submit KVValueHash reference row
  MerkVerifier->>verify_layer_proof_v1: Pass admitted proof row
  verify_layer_proof_v1->>verify_layer_proof_v1: Reject raw Reference element
  verify_layer_proof_v1-->>QueryCaller: Return InvalidProof

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 5 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: rejecting raw reference rows during V1 layer proof verification.
Linked Issues check	✅ Passed	The changes satisfy issue #862. The V1 layer verifier rejects raw Reference and ReferenceWithSumItem rows before result or descent handling. Tests cover forged result rows, typed forms, descent skippi…
Out of Scope Changes check	✅ Passed	The documentation, verifier change, comments, test registration, and test updates all support the raw-reference verification fix described in issue #862. No unrelated code changes are identified.

Full details: Docstring Coverage

Explanation

Docstring coverage is 55.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 5 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/grovedb-issue-862

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.39%. Comparing base (f94dee2) to head (b82dd46).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #920      +/-   ##
===========================================
- Coverage    92.39%   92.39%   -0.01%     
===========================================
  Files          302      302              
  Lines        93431    93444      +13     
===========================================
+ Hits         86329    86334       +5     
- Misses        7102     7110       +8     

Components	Coverage Δ
grovedb-core	90.47% <100.00%> (-0.02%)	⬇️
merk	93.23% <ø> (ø)
storage	91.70% <ø> (ø)
commitment-tree	96.38% <ø> (ø)
mmr	95.11% <ø> (ø)
bulk-append-tree	92.78% <ø> (ø)
element	97.98% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[QuantumExplorer]

This is Claude. Re-requesting the CodeRabbit review now that the rate limit window has reset.

@coderabbitai review

[coderabbitai]

Your plan includes PR reviews subject to rate limits. More reviews will be available in 47 minutes.

[QuantumExplorer]

This is Claude. The branch now includes develop (#880/#917/#919 merged) at fd70370; re-requesting the CodeRabbit review on the current head.

@coderabbitai review

[coderabbitai]

@QuantumExplorer: I will review the current pull request head.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[QuantumExplorer]

Reviewed