Tell surfaces whether the request label was proven or merely claimed

[kwsantiago]

Summary

The co-sign prompt currently marks every request label as claimed, because until now nothing told it otherwise. That is safe and it is too blunt: some labels are provable, and a qualifier that appears on every request, including the ones backed by a proof, teaches people to ignore it.

The distinction already exists in the core and was simply not carried across. When a request attaches a structured body, the responder decodes it, recomputes the digest that body would produce, and refuses the request when it does not equal the bytes being signed. A caller cannot relabel a Bitcoin sighash as a nostr event that way, because the canonical hash of the supplied event would not equal the sighash. An unknown label with a body attached is refused outright rather than passed through.

Three properties make presence of the body a sound signal, and all three were verified rather than assumed:

• Verification runs before the pre-sign hook, so a request only reaches the approval queue after it has passed.
• A verification failure returns early, so a failed request never reaches the hook at all.
• Unknown message types are refused rather than skipped, so presence cannot mean "we did not know how to check this".

So a body present at this point means the label matches the content, and its absence means nothing checked it. SignRequest now carries that as a boolean, documented so surfaces say which of the two they are showing.

This is an FFI record change: consumers must rebuild. Nothing renders the field yet; the Android side using it to drop the qualifier for verified requests, and to show the amount and destination from the verified body rather than from requester assertion, follows in that repo.

Test plan

Two tests, one per branch: a request with no structured body is not verified, one with a body is. Both go through the same mapping function the production path uses.

cargo clippy --all-targets -- -D warnings, cargo fmt --check, and the full cargo test -p keep-mobile suite are clean: 307 tests.

Part of #792.

Summary by CodeRabbit

• New Features
  • Added a verification indicator to signing requests.
  • Signing requests now show whether the message type was validated against the signed data, helping distinguish verified details from requester-provided claims.

[coderabbitai]

Warning

Review limit reached

@kwsantiago, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1708d14e-aa84-48aa-8433-c28a6e6ec8b7

📥 Commits

Reviewing files that changed from the base of the PR and between 1786195 and b45c128.

📒 Files selected for processing (1)

• keep-mobile/src/lib.rs

Walkthrough

Adds SignRequest.type_verified to the mobile interface and Rust type. Sign-request mapping derives the flag from structured payload presence, with tests covering both absent and present payloads.

Changes

Sign request verification

Layer / File(s)	Summary
Request contract and session mapping keep-mobile/src/keep_mobile.udl, keep-mobile/src/types.rs, keep-mobile/src/lib.rs	Adds the type_verified field and maps it from whether the session contains a structured payload.
Mapping behavior tests keep-mobile/src/lib.rs	Updates fixtures and tests to verify false without structured payloads and true with them.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• privkeyio/keep#665: Adds the structured-payload plumbing used to derive verification status.
• privkeyio/keep#929: Modifies the adjacent sign-request mapping logic and tests.

Suggested reviewers: wksantiago

Poem

A bunny checks the signed bytes bright,
Then marks the request with verified light.
Payload tucked in, the flag turns true,
No payload there, it stays false too.
Hop, hop—tests confirm the view!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding a field that indicates whether the request label was proven or merely claimed.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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 mobile-verified-message-type

---

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

[coderabbitai]

🧹 Nitpick comments (1)

keep-mobile/src/lib.rs (1)

4661-4665: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Make the positive fixture satisfy the verification precondition.

This test only proves that structured_payload is present; it does not prove that the body’s digest matches session.message. Use a valid structured-payload fixture or add an integration test through pre-sign validation, so a regression that marks any attached body as verified cannot pass unnoticed.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@keep-mobile/src/lib.rs` around lines 4661 - 4665, Update the positive fixture
in the test using sign_request_from_session and session_with_payload so the
structured payload is valid and its body digest matches session.message,
exercising the actual verification precondition rather than only checking
payload presence. Alternatively, cover this behavior with an integration test
through pre-sign validation, ensuring arbitrary attached bodies cannot be
treated as verified.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@keep-mobile/src/lib.rs`:
- Around line 4661-4665: Update the positive fixture in the test using
sign_request_from_session and session_with_payload so the structured payload is
valid and its body digest matches session.message, exercising the actual
verification precondition rather than only checking payload presence.
Alternatively, cover this behavior with an integration test through pre-sign
validation, ensuring arbitrary attached bodies cannot be treated as verified.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ddbb1078-76db-4fb5-901b-ce1f1af1858c

📥 Commits

Reviewing files that changed from the base of the PR and between 49f13c7 and 1786195.

📒 Files selected for processing (3)

• keep-mobile/src/keep_mobile.udl
• keep-mobile/src/lib.rs
• keep-mobile/src/types.rs

[kwsantiago]

Folded in the contradiction the review caught, which was worth more than it looks.

The function's doc said the label is requester-supplied and not bound to the signed bytes, never proof of what they are. Twenty lines below it, this PR adds a field whose entire purpose is to mark when the label is bound to them. Both statements sat in the same function, and that is the text every future change here gets reasoned against. Someone reconciling them could just as easily conclude the warning is stale in general and start treating the label as trustworthy unconditionally, which is the opposite of what it says. It is now scoped to the unverified case, with the verified case stated separately.

While writing that I also recorded the boundary the review drew around what verification actually proves, because it is narrower than the flag's name suggests and the Android copy is where it would be overclaimed. A verified Bitcoin sighash means these bytes really are a taproot key-spend sighash for the supplied transaction. It does not mean the transaction spends what the user expects, to whom they expect, or that the prevout scripts belong to this group: the recompute does not check them against the group key or the derivation path. So the Android string has to be domain-scoped, something like message type verified, and never a bare "verified", which would claim far more than was checked. Putting it in the doc rather than only in a tracker entry, since that is where whoever writes the string will be looking.

Two other things from the review worth carrying forward rather than losing. Only key-spend taproot sighashes can ever be verified; script-path and segwit v0 are hard-refused if a payload is attached, which is fail-safe but a cliff to revisit alongside script-path spending. And on the keep-android side the gradle task that regenerates the bindings does not list the interface definition file among its inputs, so a definition-only change would not invalidate its up-to-date check. This PR touches both a source file and the definition so it is unaffected, but that is worth closing before a change lands that only touches the definition.