fix(zk): Constrain usernameHash input in MerkleUpdateProof

[sheyman546]

• Replace unconstrained usernameHash input with username[32] array
• Instantiate UsernameHash() component internally for proper hash computation
• Update test suite to work with new constrained input format
• Prevent arbitrary field element injection into Merkle tree registry

Fixes #174 - Security vulnerability Finding F-02

closes #174

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Addressed security vulnerability (F-02) by adding input validation constraints to the proof circuit, preventing unconstrained field element acceptance.

• Breaking Changes

  • Updated circuit interface: the usernameHash input parameter has been replaced with a username[32] array format.

[drips-wave]

@sheyman546 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements the security fix for Finding F-02 by replacing an unconstrained usernameHash private input with a constrained username[32] array input in the MerkleUpdateProof circuit. The circuit now internally instantiates UsernameHash() to derive the leaf value. Tests are updated accordingly with a new helper function to compute the hash from the username array.

Changes

Cohort / File(s)	Summary
Documentation SECURITY_FIX_SUMMARY.md	New document describing the F-02 security fix, detailing the vulnerability (unconstrained usernameHash), the remediation (replace with constrained username[32]), and verification criteria.
Circuit Implementation zk/circuits/merkle/merkle_update_proof.circom	Removed unconstrained usernameHash private input; added username[32] input and instantiated UsernameHash() component via new include; wired username array into hasher and updated leaf calculation to use computed hash output instead of raw input.
Test Updates zk/tests/test_update_proof.js	Added computeUsernameHash() helper function implementing two-level Poseidon chunking scheme; replaced hardcoded hash with generated username[32] array (filled with value 42); updated prover input to pass username array instead of single hash value.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• [Bug][ZK] MerkleUpdateProof — usernameHash private input is unconstrained (Finding F-02) #174 — This is the primary linked issue describing Finding F-02 that the PR directly addresses.
• [Bug][ZK] username[32] inputs have no range check — allows invalid character values (Finding F-03) #175 — Related through the UsernameHash() implementation that lacks per-byte range checks and is now instantiated within the circuit.
• #110 — Related as both involve Merkle circuitry and UsernameHash integration; this fix establishes the pattern for integrating username hashing into Merkle operations.

Possibly related PRs

• Fix issues 99, 52, 60: Implement Auction claim_username, ZK Hash Circuit, and E2E Test Stubs #122 — Implements the UsernameHash() component and updates circuits/tests to use it; this PR directly wires that component into MerkleUpdateProof.
• feat(zk): Merkle Update Proof Circuit — valid state transition on leaf insertion (#53) #118 — Directly modifies the same merkle_update_proof circuit and test file, replacing unconstrained usernameHash input with constrained username[32] array and integrating UsernameHash.
• security: add threat model, SECURITY.md, and circuit audit comments #125 — Adds audit comments to the same circuit file addressing the unconstrained input vulnerability that this PR fixes.

Suggested labels

Approved

Poem

🐰 A username hashed with care,
No longer floating in the air,
Constrained by circuits, strong and true,
The merkle leaves now verified through and through! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	All coding requirements from issue #174 are met: username[32] replaces unconstrained usernameHash, UsernameHash() is instantiated internally, leaf value uses derived hash, and test_update_proof.js is updated accordingly.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with the security fix scope: circuit modification, test updates, and supporting documentation (SECURITY_FIX_SUMMARY.md) with no unrelated alterations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title clearly and accurately describes the main security fix: constraining the usernameHash input in MerkleUpdateProof to prevent arbitrary field element injection, which is the core objective of this PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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 and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

zk/circuits/merkle/merkle_update_proof.circom (1)

72-72: ⚠️ Potential issue | 🔴 Critical

Critical: Multiple main components causing compilation failure.

The compilation is failing with error[P1002]: Multiple main components. The codebase contains 7 separate files defining a component main:

• zk/circuits/username_merkle.circom
• zk/circuits/merkle_update.circom
• zk/circuits/username_hash.circom
• zk/circuits/merkle/username_leaf_main.circom
• zk/circuits/merkle/merkle_update_proof.circom
• zk/circuits/merkle/merkle_non_inclusion.circom
• zk/circuits/merkle/merkle_inclusion.circom

This is a project-level structural issue. Circom compilation expects either:

1. Each circuit compiled independently (separate build steps)
2. A single entry point file with one main that imports and uses templates from other files

Resolve by updating the build configuration to compile circuits separately or restructuring to have a single aggregated entry point with auxiliary templates.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@zk/circuits/merkle/merkle_update_proof.circom` at line 72, Multiple files
define a top-level "component main" causing Circom error[P1002]; choose one fix:
either (A) create a single aggregator entrypoint that defines the sole
"component main" and imports templates from the listed files
(zk/circuits/username_merkle.circom, merkle_update.circom, username_hash.circom,
merkle/username_leaf_main.circom, merkle/merkle_update_proof.circom,
merkle/merkle_non_inclusion.circom, merkle/merkle_inclusion.circom) and remove
or convert their local "component main" declarations into named templates, or
(B) update the build pipeline to compile each circuit file independently (one
circom compile per file) and delete extra "component main" declarations from
files that should be libraries; ensure only the chosen file/template retains
"component main" so only one entrypoint exists during compilation.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@zk/circuits/merkle/merkle_update_proof.circom`:
- Line 72: Multiple files define a top-level "component main" causing Circom
error[P1002]; choose one fix: either (A) create a single aggregator entrypoint
that defines the sole "component main" and imports templates from the listed
files (zk/circuits/username_merkle.circom, merkle_update.circom,
username_hash.circom, merkle/username_leaf_main.circom,
merkle/merkle_update_proof.circom, merkle/merkle_non_inclusion.circom,
merkle/merkle_inclusion.circom) and remove or convert their local "component
main" declarations into named templates, or (B) update the build pipeline to
compile each circuit file independently (one circom compile per file) and delete
extra "component main" declarations from files that should be libraries; ensure
only the chosen file/template retains "component main" so only one entrypoint
exists during compilation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1c0d50e-1c9c-4504-89a9-578802cfeb48

📥 Commits

Reviewing files that changed from the base of the PR and between 67f2e2c and bd15226.

📒 Files selected for processing (3)

• SECURITY_FIX_SUMMARY.md
• zk/circuits/merkle/merkle_update_proof.circom
• zk/tests/test_update_proof.js

[ryzen-xp]

Hello @sheyman546 , fix this