M-H11: revert empty signatures on quorum verification

[philanton]

Description

The verifyQuorum helper is called from four app-session RPC endpoints: create_app_session, submit_app_state, submit_deposit_state, and rebalance_app_sessions. In each case, quorum_sigs is user-controlled. Inside verifyQuorum, each signature is decoded from hex and the first byte is read immediately to determine the signer type

    for _, sigHex := range signatures {
        sigBytes, err := hexutil.Decode(sigHex)
        if err != nil {
            return rpc.Errorf("failed to decode signature: %v", err)
        }

        sigType := app.AppSessionSignerTypeV1(sigBytes[0])
        userWallet, err := appSessionSignerValidator.Recover(data, sigBytes)

This is unsafe because hexutil.Decode("0x") returns an empty slice without error. Supplying quorum_sigs = ["0x"] causes sigBytes[0] to panic with an out-of-range access instead of returning a validation error.
The same helper is also used by app updates, deposits, and rebalance flows, so the bug is reachable through multiple app-session RPC methods. The panic occurs in the request processing path and is not recovered there, so a single malformed request can terminate the service process.
Consequently, any remote caller who can reach these endpoints can cause a denial of service with a malformed signature.

Summary by CodeRabbit

• Bug Fixes

  • Added validation to detect and report empty signatures with their position index for improved error diagnostics.

• Refactor

  • Reordered quorum verification to execute earlier in the session creation workflow.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0ad28055-014e-437e-8c41-7911bc116b23

📥 Commits

Reviewing files that changed from the base of the PR and between 037bb72 and 20f607a.

📒 Files selected for processing (2)

• clearnode/api/app_session_v1/create_app_session.go
• clearnode/api/app_session_v1/handler.go

---

📝 Walkthrough

Walkthrough

The pull request modifies app session creation to verify quorum earlier in the transaction flow and adds validation for empty signatures. Specifically, verifyQuorum() is now called immediately after action gateway verification and before creating the app session record, and the quorum verification loop now rejects empty signature bytes with an indexed error.

Changes

Cohort / File(s)	Summary
CreateAppSession Flow Reordering clearnode/api/app_session_v1/create_app_session.go	Moved h.verifyQuorum(...) execution to occur immediately after h.actionGateway.AllowAction(...) and before tx.CreateAppSession(appSession), advancing the verification step in the transaction sequence.
Quorum Signature Validation clearnode/api/app_session_v1/handler.go	Enhanced verifyQuorum loop to track signature index and validate decoded signature bytes, returning an RPC error with the index when empty signatures are encountered.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• M-H09: use channel signer for all channel state node sigs #667: Both PRs modify app_session_v1 signature handling; this PR enhances verification logic while that PR changes signer type to ChannelDefaultSigner, affecting signature format compatibility.

Suggested reviewers

• ihsraham
• dimast-x

Poem

🐰 A quorum check hops to the front of the line,
Empty signatures caught with an index divine,
Verification flows earlier, trusty and tight,
App sessions created with cryptographic might! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: reverting/handling empty signatures in quorum verification to fix a safety bug.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/clearnode-h-11

---

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.}

[nksazonov]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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.