fix: crisp security issues [skip-line-limit] (#1821)

Author: ctrlc03

.github/workflows/ci.yml

@@ -281,6 +281,8 @@ jobs:
       - name: Run ZK Prover Integration Tests (with network downloads)
         run: 'cargo test -p e3-zk-prover --features integration-tests --test integration_tests -- --nocapture'
 
+  # Guards the RISC Zero guest artifact. The on-chain imageId is immutable, so a guest change
+  # that leaves ImageID.sol untouched ships a verifier that no longer matches this tree.
   build_e3_support_risc0:
     needs: [detect_changes]
     if: needs.detect_changes.outputs.docker_support == 'true'
@@ -658,19 +660,51 @@ jobs:
   crisp_unit:
     needs: [detect_changes, build_crisp_sdk]
     if: needs.detect_changes.outputs.crisp == 'true'
-    timeout-minutes: 30
+    # Per leg, because they are not remotely the same size. Anything that generates a ballot proves
+    # five Noir circuits, which a two-vCPU runner takes minutes to do; everything else is seconds.
+    timeout-minutes: ${{ matrix.timeout }}
     runs-on: 'ubuntu-latest'
     strategy:
       matrix:
         include:
           - test-suite: test:circuits
             command: pnpm test:circuits
+            timeout: 15
           - test-suite: test:sdk
             command: pnpm test:sdk
-          - test-suite: test:contracts
-            command: pnpm test:contracts
+            timeout: 45
+            needs-sdk: true
+          # The contract suite is split by proving cost, not by subject. Run whole it took 27m54s
+          # against a 30m cap and started dying to ordinary runner variance, and it grows by minutes
+          # with every ballot a new test needs. Split, the critical path is the largest single leg
+          # rather than the sum, and the checks that prove nothing report in seconds instead of
+          # queueing behind twenty minutes of proving.
+          #
+          # Balanced by ballots, which is what the time is: input-tree 7, ballots 5, unit 0.
+          # `pnpm test:contracts` still runs all of it in one go for local use, and
+          # `crisp_contract_leg_coverage` fails if a test file ever belongs to no leg.
+          #
+          # Every contract leg needs both capabilities: `hardhat.config.ts` imports
+          # `@interfold/contracts/tasks/*`, which only resolves once `evm:build` has emitted `dist/`,
+          # and each leg contains at least one test that imports `@crisp-e3/sdk`.
+          - test-suite: test:contracts:input-tree
+            command: pnpm test:contracts:input-tree
+            timeout: 45
+            needs-evm: true
+            needs-sdk: true
+          - test-suite: test:contracts:ballots
+            command: pnpm test:contracts:ballots
+            timeout: 45
+            needs-evm: true
+            needs-sdk: true
+          - test-suite: test:contracts:unit
+            command: pnpm test:contracts:unit
+            timeout: 15
+            needs-evm: true
+            needs-sdk: true
           - test-suite: cargo test
             command: cargo test
+            timeout: 20
       fail-fast: false
     steps:
       - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
@@ -724,13 +758,16 @@ jobs:
         working-directory: .
         run: pnpm install --frozen-lockfile
 
+      # Gated on a declared capability rather than on the leg's name. Matching the name made the
+      # label load-bearing: splitting `test:contracts` into three renamed legs silently skipped this
+      # step, and the suite failed on a missing `@interfold/contracts/dist` two steps later.
       - name: Compile Interfold contracts
-        if: matrix.test-suite == 'test:contracts'
+        if: matrix.needs-evm
         working-directory: .
         run: pnpm evm:build
 
       - name: Download Crisp SDK artifacts
-        if: matrix.test-suite == 'test:sdk' || matrix.test-suite == 'test:contracts'
+        if: matrix.needs-sdk
         uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
         with:
           name: crisp-sdk-artifacts
@@ -941,6 +978,36 @@ jobs:
       - name: Build circuits
         run: pnpm build:circuits
 
+      # The generated verifier contracts must match the freshly built verification keys. A drift
+      # means the deployed verifier accepts a different circuit from the one in this tree.
+      #
+      # `--no-compile` is load-bearing, not an optimisation. Without it the checker deletes every
+      # nargo target directory first — its guard against comparing stale artifacts — and recompiles
+      # only what it needs, which is the `.json` and the evm `.vk`. That silently discards the
+      # `.vk_recursive` files `build:circuits` had just written, and the upload below then ships an
+      # artifact `zk_prover_e2e` cannot use. Nothing here fails; the damage surfaces two jobs later.
+      # The step above has just built these circuits, so there is nothing stale to guard against.
+      - name: Check generated verifiers are in sync
+        run: pnpm check:verifiers --no-compile
+
+      # Asserted before the upload, so a step that consumes or clears a build output fails in the
+      # job that did it rather than in the job that needed it. Same list `zk_prover_e2e` checks.
+      - name: Verify recursive aggregation artifacts survived
+        run: |
+          for artifact in \
+            circuits/bin/recursive_aggregation/dkg_aggregator/target/dkg_aggregator.json \
+            circuits/bin/recursive_aggregation/dkg_aggregator/target/dkg_aggregator.vk_recursive \
+            circuits/bin/recursive_aggregation/decryption_aggregator/target/decryption_aggregator.json \
+            circuits/bin/recursive_aggregation/decryption_aggregator/target/decryption_aggregator.vk_recursive
+          do
+            test -f "$artifact" || {
+              echo "Missing before upload: $artifact"
+              echo "A step between 'Build circuits' and here removed it."
+              exit 1
+            }
+            echo "  ok $artifact"
+          done
+
       - name: Upload compiled circuit artifacts
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
         with:

.github/workflows/releases.yml

@@ -450,16 +450,45 @@ jobs:
         uses: actions/checkout@v6
         with:
           fetch-depth: 0
+          # This job holds `contents: write` and runs `pnpm install`, which executes lifecycle
+          # scripts from every dependency. Persisting the token in .git/config would leave it on
+          # disk for all of them. The one step that needs it takes it explicitly instead.
+          persist-credentials: false
 
       - name: Download all binary artifacts
         uses: actions/download-artifact@v4
         with:
           path: dist/
 
+      # Pinned to immutable SHAs: this job has `contents: write` and force-pushes `stable`, so a
+      # mutable tag could introduce unreviewed code into the release.
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
+        with:
+          node-version: '22'
+
+      - uses: pnpm/action-setup@f40ffcd9367d9f12939873eb1018b921a783ffaa # v4
+
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+
+      # A RISC Zero proof names a guest image; it does not name the source that produced it. This
+      # record is what lets a third party tie the released source to the deployed imageId.
+      - name: Generate compute-provider provenance manifest
+        run: |
+          pnpm provenance:manifest --out compute-provider-provenance.json
+          # `complete: false` means a field could not be resolved. Publishing it would put a
+          # partial record beside the release as though it were a verification.
+          if ! node -e "process.exit(require('./compute-provider-provenance.json').complete ? 0 : 1)"; then
+            echo "provenance manifest is incomplete; refusing to publish it" >&2
+            node -e "console.error(require('./compute-provider-provenance.json').unresolved.join('\n'))"
+            exit 1
+          fi
+
       - name: Prepare release assets
         run: |
           mkdir -p release-assets
           find dist/ -name "*.tar.gz" -exec cp {} release-assets/ \;
+          cp compute-provider-provenance.json release-assets/
           # `interfold config check` fetches this asset from the latest release,
           # so a release without it leaves operators unable to detect a redeploy.
           if [ ! -f deployments/manifest.json ]; then
@@ -609,7 +638,15 @@ jobs:
 
       - name: Update stable tag (for stable releases only)
         if: needs.validate-and-prepare.outputs.is_prerelease == 'false'
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
           git tag -f stable
+          # Through a credential helper, so the token never reaches a process argument list. Every
+          # process on a runner shares one user, so anything left running by `pnpm install` can read
+          # /proc/*/cmdline; it cannot read another process's environment as easily. The config
+          # stores the helper only — the token is read from the environment when git invokes it.
+          git config --local credential.helper '!f() { echo username=x-access-token; echo "password=$GITHUB_TOKEN"; }; f'
           git push origin stable --force
+          git config --local --unset credential.helper
           echo "✅ Updated 'stable' tag to point to v${{ needs.validate-and-prepare.outputs.version }}"

.gitignore

@@ -18,3 +18,7 @@ __pycache__/
 
 # circuit benchmarks
 circuits/benchmarks/results_*/raw/
+
+# Generated by crates/support/methods/build.rs. Holds a machine-local guest ELF path,
+# so it is never committed.
+crates/support/tests/Elf.sol

.husky/pre-push

@@ -6,3 +6,5 @@ pnpm check:license
 pnpm check:committee
 pnpm check:docs
 pnpm check:invariants
+pnpm check:image-id
+pnpm check:verifiers

Cargo.lock

@@ -573,9 +573,9 @@ semantic replay domain across deferred, in-flight, and completed submissions. Re
 failures release their key. Successful or known-benign terminal results retain it.
 
 Every node reads the proof-type policy after a fault quorum. A disabled policy produces a durable,
-E3-scoped `CommitteeMemberExcluded` fact instead of a transaction that must revert. This fact is
-not an on-chain expulsion: it changes only the current E3's collectors and aggregator selection.
-The canonical N-member roster remains unchanged for proof binding, rewards, and registry state.
+E3-scoped `CommitteeMemberExcluded` fact instead of a transaction that must revert. This fact is not
+an on-chain expulsion: it changes only the current E3's collectors and aggregator selection. The
+canonical N-member roster remains unchanged for proof binding, rewards, and registry state.
 
 The gate is deliberately described as in-memory: there is no durable external-effect outbox or
 persisted transaction intent. A crash after snapshot advancement but before receipt classification

agent/CRATES_ARCHITECTURE.md

@@ -356,6 +356,85 @@ skip-proof feature containment (`pnpm check:invariants`, baselines in
   binds the chain, Interfold address, E3 ID, scheme ID, BFV parameter hash, committee public key,
   output hash, and SAFE commitment. The E3 program verifies application rules separately and cannot
   create a decryption duty by itself. — `flow-trace/04`; INDEX Z-15
+- **The compute path carries no external audit.** The 2026-08-17 Zenith audit covered six Solidity
+  files and no Rust. `crates/compute-provider`, the RISC Zero guest, `crates/zk-helpers`, and
+  `Risc0BfvCiphertextVerifier.sol` were outside both the audit and its mitigation review, so a
+  `Resolved` `Z-` row is this repository's remediation rather than a re-reviewed one. Treat changes
+  in these areas as unaudited by default. — `flow-trace/00`;
+  `packages/interfold-contracts/audits/README.md`
+- **A Secure Process derives its input root; it never receives it.** `ComputeInput` holds only
+  `fhe_inputs`, and `ComputeInput::process` derives the leaves from the ciphertexts it processed.
+  The protocol verifier takes the input root from the proof envelope and does not constrain it, so
+  the E3 program's comparison against its own on-chain root is the only check — and that comparison
+  is worthless if the guest can be handed leaves that disagree with the ciphertexts it consumed.
+  Publication is unpermissioned and one-shot, with no dispute path, so any party could otherwise
+  publish a tally over ciphertexts that were never submitted. `MerkleTreeBuilder::with_leaf_hashes`
+  is `#[cfg(test)]` to keep it out of that path. — `flow-trace/04`
+- **Every E3 program must compare the proof's input root against its own root.**
+  `Risc0BfvCiphertextVerifier` takes no `inputRoot` argument and constrains none. A program that
+  skips the comparison accepts a result computed over any input set. — `flow-trace/04`
+- **A Secure Process derives its leaves; it never receives them, and never drops one.**
+  `MerkleTreeBuilder::compute_leaf_hashes` builds every leaf from the ciphertexts it was given and
+  pushes one per published input, whatever the E3 program's policy decides about computing over it.
+  Both rules are applied by `e3-compute-provider` rather than delegated: a received root can
+  disagree with the data it claims to describe, and a missing leaf changes the root and makes the
+  result unpublishable. — `flow-trace/04`
+- **The leaf layout and input selection are the E3 program's, not the crate's.** They are supplied
+  as an `InputPolicy`, because a leaf must match whatever that program builds on chain and no two
+  programs need agree, and because "what does a second input for the same participant mean?" has no
+  universal answer. `InputPolicy::default` is the historical behaviour — leaf is the ciphertext's
+  own commitment, every input counts — which matches the starter template. Every E3 program exports
+  `policy()` beside `fhe_processor`. — `flow-trace/04`
+- **CRISP binds bytes, commitment, slot and parent into its leaf, and selects the end of each slot's
+  chain.** `CRISPProgram.inputLeaf` is
+  `sha256(sha256(bytes) || commitment || slot || parentIndexPlusOne) mod SNARK_SCALAR_FIELD` and
+  `e3_user_program::policy` rebuilds it byte for byte; a divergence makes every root mismatch and
+  nothing else would catch it, so both sides pin the same vector (`program/tests/input_leaf.rs`,
+  `tests/input-leaf.test.ts`) and `onchain_root_agreement.rs` asserts Rust reproduces a root a real
+  contract produced. The tree is append-only because the mask path checks no signature, so anyone
+  can write to any census member's slot and update-in-place would let a third party erase a counted
+  vote. — `flow-trace/04`
+- **A slot's head must be openable by anyone, so selection follows a parent chain rather than a
+  mutable pointer.** `chain_head_per_slot` takes an entry only when its bytes reproduce its
+  commitment _and_ the entry it names is that slot's current head. `CRISPProgram` cannot check the
+  first — the commitment is a Poseidon sponge over CRT limbs and the circuit never sees the
+  serialization — so with one mutable head per slot, anyone could publish a valid proof beside
+  unusable bytes and leave a head only they can open. A slot nobody can mask is a slot where every
+  later input is provably its owner voting again, which is a coercion receipt. Because an unusable
+  entry is never the head, it is never a valid parent, and the next honest input names the same
+  parent it did.
+
+  The rule takes the **first** usable entry to extend a parent, so a later sibling is dropped and an
+  input can be front-run into not counting. Keep it that way: a stale parent cannot be told apart
+  from a sibling built a moment earlier, because only the circuit knows whether an entry replaces
+  the slot or adds to it. Preferring the later sibling would let a mask on a superseded ciphertext
+  restore it over a vote — a silent tally corruption, against a dropped re-vote the voter can see
+  and retry. — `flow-trace/04`
+
+- **CRISP's three ballot operations prove one relation and publish one shape.** Voting, updating,
+  and masking all prove `published = addend + ballot`, with the addend selected by the private
+  `is_mask_vote` and derived as `keep_previous = is_mask_vote & !is_first_vote`. The circuit returns
+  `sum_ct_commitment` on every path, the SDK has one code path, and `CrispSDK.prepareBallot` makes
+  the same server request either way. Branching any of these apart — a different published
+  ciphertext, a different commitment for the digest, a different request — makes the three
+  distinguishable on chain, which is what masks exist to prevent. Deriving the selector rather than
+  witnessing it is what stops a voter counting their old ballot twice and a masker erasing a vote. —
+  `flow-trace/04`
+- **CRISP constrains every coefficient of the ballot plaintext, at the real BFV degree.** The
+  witness generator reverses the message over the full degree, so the payload sits at
+  `k1[D - MAX_MSG_NON_ZERO_COEFFS ..]` with the options back to front;
+  `crisp_lib::utils::ballot_layout` derives that offset and both checkers use it. Coefficients
+  inside an option segment must be binary, everything outside the ballot region must be zero, and a
+  mask's plaintext must be zero everywhere. Indexing as if the polynomial were the message width
+  makes both checks read only padding: every vote passes any balance bound, and a mask — which needs
+  no signature and may be written to any eligible slot — can carry an arbitrary payload into someone
+  else's ballot. Tests must build `k1` at the compiled degree, not at `MAX_MSG_NON_ZERO_COEFFS`. —
+  `flow-trace/04`
+- **The SAFE ciphertext commitment requires exactly two components.** It covers `c[0]` and `c[1]`
+  only, matching the Noir circuit, so `bfv_ciphertext_to_greco` rejects any other component count. A
+  padded ciphertext would otherwise share a commitment with its two-component prefix while threshold
+  decryption rejects it, failing the round as a `DecryptionTimeout` billed to the ciphernodes. —
+  `flow-trace/04`
 - **Client PK commitment binding (C-01):** serialized PK event bytes are an untrusted transport
   hint; indexers store the decoded key only when its recomputed commitment equals the on-chain
   (C5-proven) value. Proof-backed committee publication never accepts key bytes. Public-key
@@ -438,7 +517,20 @@ skip-proof feature containment (`pnpm check:invariants`, baselines in
 
 - Committee four-file sync (above) — `scripts/check-committee.sh`, pre-push + CI.
 - **Never hand-edit generated files:** parity matrices, `utils.ts` H/T values, verifier contracts
-  (`generate-verifiers.ts` output), `.active-preset.json`.
+  (`generate-verifiers.ts` output), `.active-preset.json`, `crates/support/contracts/ImageID.sol`,
+  `crates/support/tests/Elf.sol`.
+- **Generated verifiers must match the built VKs** — `pnpm check:verifiers`, pre-push + CI
+  (`build_circuits`). A drift means the deployed verifier accepts a different circuit from the tree.
+- **`Elf.sol` is never committed.** `crates/support/methods/build.rs` writes it with a machine-local
+  guest ELF path, so it is generated per checkout and `.gitignore`d.
+- **A release publishes a complete provenance manifest** — `pnpm provenance:manifest`. It ties
+  source commit, lockfile digests, pinned revisions, RISC Zero version, builder image tag **and
+  digest** (the builder tag is mutable and `RISC0_DOCKER_CONTAINER_TAG` overrides it), guest ELF
+  SHA-256, image ID, and the deployed verifier to one record. The generator reports
+  `complete: false` with the unresolved fields rather than emitting a partial record that reads as
+  verified. The ELF SHA-256 is **not** the image ID: SHA-256 checks binary integrity, the image ID
+  is computed from the loaded memory image. Procedure:
+  `docs/pages/verifying-the-compute-provider.mdx`.
 - Upgradeable-contract storage baselines are committed and CI-gated (missing baselines, compiler
   drift, layout incompatibility, bad gap consumption all fail); baseline creation is an explicit
   maintainer command. — INDEX concern #27