Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
github-actions
Bot
added
size: XL
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a95f8ebdd1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // bound operation. A function-call key restricted to a different | ||
| // receiver / method cannot authorize this transaction. | ||
| let bound_op = decode_bound_operation(context, approved_tx_hash); | ||
| validate_access_key_scope(&payload.access_key_scope, &bound_op)?; |
There was a problem hiding this comment.
When the redirect callback is for a NEAR function-call access key, payload.access_key_scope is entirely callback-controlled while the ed25519 signature verified above covers only approved_tx_hash.as_bytes(), so a caller can serialize FullAccess here and bypass the receiver/method checks. That returns VerifiedProof and consumes the grant for an operation the bound key scope may not cover; the scope needs to come from trusted gate state/chain lookup or be included in the signed/bound material, not from the proof payload.
AGENTS.md reference: AGENTS.md:L127-L134
Useful? React with 👍 / 👎.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 2 | 0 | 2 | a95f8ebdd115 |
Head: a95f8ebdd1159192b6f1c219e080bc12594657c7
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
The provider’s synthetic redirect/proof format is incompatible with NEAR wallet signing, and its access-key scope is attacker-controlled.
Findings
Blocking: 2 / Notes: 0
Blocking findings
1. ❌ [HIGH] Implement an actual NEAR wallet signing protocol
Location: crates/ironclaw_wallet_external/src/near_redirect/mod.rs:189-202
This base64-encodes arbitrary opaque bytes into transactions, while verify_resume later requires a raw ed25519 signature over ApprovedTxHash. NEAR Wallet Selector’s callback-capable flow signs a NEP-413 message payload; its transaction API accepts structured actions and signs-and-sends the transaction. A real wallet therefore cannot produce the synthetic NearRedirectProofPayload this verifier accepts, so the redirect cannot complete. Implement and verify a supported NEAR wire protocol, with an adapter-level integration fixture rather than tests that manufacture key.sign(hash) directly.
2. ❌ [HIGH] Do not trust the callback’s claimed access-key scope
Location: crates/ironclaw_wallet_external/src/near_redirect/mod.rs:396
access_key_scope is unsigned callback JSON, but this branch accepts its FullAccess claim without consulting NEAR state. A holder of the gate-bound function-call key can sign the raw hash directly, claim FullAccess, and bypass the receiver/method restrictions that threat #22 is meant to enforce. Obtain the permission for (account, public_key) from an authoritative source and validate the actual signed transaction, or reject non-verified full-access keys. Add a regression test for a function-call key claiming FullAccess.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
| approved_tx_hash: &ApprovedTxHash, | ||
| ) -> String { | ||
| use base64::Engine as _; | ||
| let tx_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(decoded.as_opaque()); |
There was a problem hiding this comment.
This emits arbitrary opaque bytes as transactions but verification expects a synthetic raw-hash signature. NEAR wallet flows use structured transaction actions or NEP-413 message signing, so a real wallet cannot return the proof shape accepted here. Please implement and test a supported wallet wire protocol.
| ) -> Result<(), SigningProviderError> { | ||
| match scope { | ||
| // A full-access key authorizes any action on the account. | ||
| NearAccessKeyScope::FullAccess => Ok(()), |
There was a problem hiding this comment.
access_key_scope comes from unsigned callback JSON. A function-call key can claim FullAccess and bypass the intended scope validation. Resolve the bound key’s real permission authoritatively and validate the actual signed operation (or reject unverified non-full-access keys).
4b04f16 to
baf5be0
Compare
a95f8eb to
f3afe32
Compare
|
Superseded by #6755, part of consolidating the 20-PR The content of this PR is carried forward in full, re-based on current Closing to keep the queue honest. The review history stays on this PR and remains readable; reopen if the consolidation is rejected. |
1 participant
Replaces #3993, which was closed on 2026-07-20 because it modified the legacy top-level
src/web ingress.src/has since been deleted entirely (b6da0272a, #6375 — "delete v1 legacy monolith (src/) and cut deploy over to Reborn"), so this branch has been rebased onto currentmainwith allsrc/hunks excised. The crate-side work is unchanged.Part of the attested-signing revival — see the tracking issue #6532 for the full plan.
What this PR is
NEAR browser-wallet redirect signing provider (
ironclaw_wallet_external::near_redirect): the user is redirected to a NEAR wallet that signs the boundApprovedTxHashwith the account's ed25519 access key and redirects back with the signature. The wallet holds the keys and renders/signs natively (wallet-side WYSIWYS); IronClaw never has custody.Cascade changes (vs the closed #3993)
main(the stack was 1184 commits behind; merge base was 2026-05-24).src/excised: dropped the legacysrc/channels/web/features/chat/attested.rsproof-ingress hunks (+696 lines across 3 files). The Reborn-side replacement lands in the gate/resolve ingress PR (feat(signing): reborn webui attested gate/resolve ingress (attested-signing PR11/12) #3995).AttestedSigningGrant::seal→ fallible::new(timestamp/expiry validation)map_grant_errornow mapsGrantError::InvalidTimestamp/InvalidExpiryfail-closed toGrantClaimFailedVerification
cargo build/cargo test -p ironclaw_wallet_externalgreen (19 + 12 + 5 + 20 tests);attested_signing_boundariesboundary test green (incl.workspace_graph_is_openssl_free); clippy clean.