RSI Support - #181
Conversation
📝 WalkthroughWalkthroughThe PR adds native Noise framing and cryptographic primitives, expands Lightning and message APIs, introduces document exchange with sealed content and HTLC settlement, adds federated sidechain and consensus workflows, updates wallet and CLI behavior, and provides a seeded network simulator with Beacon audit reporting. ChangesNative transport and cryptography
Protocol and state
Document exchange and simulator
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Peer
participant OfferBook
participant HTLC
participant Wallet
CLI->>Peer: requestPeerInventory(kind: documents)
Peer-->>CLI: inventory response with document offers
CLI->>OfferBook: ingestInventoryResponse(items)
OfferBook-->>CLI: ranked offer
CLI->>HTLC: buildDocumentOfferEscrow(...)
HTLC-->>CLI: payment address and refund metadata
Wallet->>Peer: ingest Bitcoin funding or claim transaction
Peer-->>CLI: ciphertext, key reveal, or settlement event
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 high |
🟢 Metrics 3354 complexity · 51 duplication
Metric Results Complexity 3354 Duplication 51
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Stale comment
Security review (synchronize)
Outcome: request changes — 2 High mesh availability findings remain (PoC-confirmed on HEAD
93c78c13).Current findings
- High —
P2P_RELAYbit-identical flood vs signer pin — Outers are forwarded unchanged, butP2P_RELAYis omitted fromRELAY_AS_IS_*, so the next hop treats the attacker author as a pin mismatch against the honest forwarder and hard-disconnects/bans them (mesh partition).- High — Onion peel misattributes hard misbehavior to the last hop — Still open: peeled inners are handled with TCP
origin= last hop; pin/signature/body-hash failures call_applyPeerMisbehavior(..., disconnect: true)and ban the honest relay.Prior findings validated fixed (not re-reported)
- Sealed unlock / public payment-hash-as-proof (
settlementVerified;autoFulfillDocumentRequestsdefaultfalse)- Buyer funding attacker
paymentAddress/ HTLC offer validation- Noise indefinite
recvstall + clearingO_NONBLOCKbefore frame read- CWD
.FABRIC_SEEDinjection (allowCwdSeedopt-in)- Contract republish allow-list expansion
No Slack destination is configured for this automation; summary is in this review only.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (synchronize @
1e4d9ffce)Outcome: request changes — 3 High mesh-availability findings remain (PoC-confirmed).
Current findings
- High —
P2P_RELAYomitted fromRELAY_AS_IS_*— Bit-identical flood of attacker-signed outers fails the next hop’s signer-pin check against the honest forwarder → hard disconnect/ban (mesh partition).- High — peeled
SESSION_*still hard-disconnects the last hop —813133b69fixed AMP body-hash/signature/pin/contract-ops attribution viapeeledForward, but_punishPeerForSessionKeyViolationstill bans the TCP last hop for a laundered badP2P_SESSION_OPEN/OFFER.- High — peeled
relay-nest-exceededstill hard-disconnects the last hop — NestedP2P_RELAYdelivered via onion peel still calls_applyPeerMisbehavior(originName, …, disconnect: true)on the honest relay.Prior findings re-validated (no longer open)
Issue Status Sealed key unlock via public paymentHashechoFixed ( settlementVerified/authorizeDocumentKeyReveal)Buyer funding seller-supplied HTLC address Fixed (buyer-bound rebuild) Native Noise blocking recvDoSFixed ( select+ timeout)CWD .FABRIC_SEEDoutranking walletFixed ( allowCwdSeedopt-in)Contract allow-list republish bypass Fixed (republish ignored) Onion peel AMP integrity → last-hop disconnect Partially fixed — residuals above No Slack destination is configured for this automation run; findings are posted only on this PR.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (PR #181)
Request changes — 1 High, 1 Medium remain on onion peel trust boundaries. Prior mesh-partition / peel-disconnect findings are fixed on this tip (
P2P_RELAYinRELAY_AS_IS; session-key and nest-cap disconnects gated bypeeledForward).Current findings
- High — Valid peeled
P2P_SESSION_OFFER/P2P_SESSION_OPENstill rebindspeers/_addressToIdfor the TCP last hop to the attacker’s key, so the next honest frame from that relay fails signer-pin and is hard-disconnected/banned.- Medium — Peeled
P2P_CHAT_MESSAGEandP2P_PEERING_OFFERstill apply mesh relay / candidate enqueue under last-hop attribution, so an originator can burn an honest neighbor’s budgets and inject dial targets via onion.Cleared since last automation pass
P2P_RELAYmesh partition (missingRELAY_AS_IS)- Peeled session-key /
relay-nest-exceededhard-disconnect of last hop- Sealed-document free unlock, buyer HTLC address trust, Noise indefinite
recv, CWD seed precedence (not re-opened; still look fixed on HEAD)Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (synchronize)
Re-validated prior automation findings against
14823568c. The recent onion/relay hardening addresses the earlier high-confidence issues (session rebind on peel, chat/peering mesh side-effects,P2P_RELAYpin ban, payment-hash unlock, CWD seed precedence, Noise stall DoS).Remaining medium findings: 2 (inline). Same peel-laundering class as the fixed peering/chat paths —
P2P_PEER_ALIASandP2P_PEER_GOSSIPstill apply last-hop side-effects.Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (current)
Prior open findings (
P2P_PEER_ALIASlast-hop rebind; peeledP2P_PEER_GOSSIPbudget/relay) are addressed incc253628e/ related peel hardening.Also re-validated as mitigated: sealed-key reveal without settlement, inventory HTLC address spoof, Noise stall DoS, CWD seed precedence, contract allow-list republish, session rebind via peel.
Remaining: 2 medium peel side-effect findings (inline). Onion delivery still attributes some costs/penalties/mesh actions to the honest TCP last hop.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (HEAD
4ebeffede)Prior critical/high items (sealed key unlock without payment, Noise stall DoS, buyer HTLC address trust, CWD seed injection, contract allow-list republish, session/pin hard-disconnect via peel) look fixed on this tip.
Still open: three peel attribution gaps where
P2P_FORWARDdelivery still treats the honest last hop as the responsible TCP peer for budget, scoring, or dial side-effects.
Severity Issue High Peeled CONTRACT_PUBLISHhijack penalties apply to the last hop (unbounded −40)Medium Peeled inners still debit the last hop’s wire-traffic budget Medium Peeled P2P_PEER_ANNOUNCEstill enqueues dial candidatesNo critical findings on this pass.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (synchronize)
Re-validated prior automation findings against
bc5b125. Previously reported critical/high issues (sealed key reveal via public hash echo, buyer HTLC address trust, C Noise stall DoS, contract allow-list bypass, CWD seed precedence, and most onion peel last-hop attribution bugs) appear mitigated in the current tip.Remaining high-confidence finding: 1 medium (onion peel still debits the TCP last hop’s inbound wire-traffic budget / derank).
No other medium-or-higher issues confirmed in this pass.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (HEAD
355bec8b9)Revalidated prior automation findings. Previously reported critical/high issues (sealed key reveal via hash echo, buyer funding of seller-supplied addresses, CWD seed injection, blocking Noise
recvDoS, contract republish allow-list bypass, peel hard-disconnect / session rebind / gossip-peering-alias mesh side-effects) appear mitigated on current HEAD.3 medium findings remain, all onion/
P2P_FORWARDpeel attribution gaps that can still punish or side-effect the honest last hop. Inline comments below.No new high/critical issues identified in this pass.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (current HEAD)
Prior peel-attribution findings (wire-traffic re-debit,
CONTRACT_PUBLISHhijack scoring,P2P_PEER_ANNOUNCEdial enqueue) are fixed in this tip (suppressTcpOriginPunish/punishOrigin/ local-only announce) and were resolved.Still open: peel /
skipRelayFloodinners of some mesh-gossip types still callrelayFromunder the TCP last hop, inconsistent withCONTRACT_PUBLISH/ chat / gossip and withSECURITY.md(“only the outermost envelope is mesh-flooded”).Findings
- High — peeled / relay-unwrapped
CONTRACT_MESSAGEstill mesh-relays- Medium — same for
CONTRACT_PROPOSAL- Medium — same for
BitcoinBlockNo Slack destination is configured for this automation; assessment is on the PR only.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (HEAD
65eca838e)Prior peel /
skipRelayFloodfindings forCONTRACT_MESSAGE,CONTRACT_PROPOSAL, andBitcoinBlockare fixed (and covered bytests/peer.onion.forward.js). Re-validated as still mitigated: sealed-key hash-echo unlock, buyer HTLC address trust, C Noise stall DoS, CWD.FABRIC_SEEDinjection, contract allow-list / session rebind hard-disconnect via peel.Still open:
DocumentRequesthandling never receives peel /skipRelayFlood/relayedAsIsopts, so it still violates outermost-only flood and can misdeliver content to the TCP last hop.Findings
- High — peeled
DocumentRequestfulfills (or queues for fulfill) to the TCP last hop- Medium — peeled /
P2P_RELAY-unwrappedDocumentRequeststill mesh-relays when the document is not heldNo Slack destination is configured for this automation; assessment is on the PR only.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review of
77c06e34(vs8d4426be).Prior automation findings revalidated as fixed: sealed key reveal no longer treats public
paymentHashHexas settlement proof; buyer HTLC rebuild/validate rejects seller-supplied addresses; Noise frame I/O uses timeouts and keepsO_NONBLOCK; onion peel / foreignP2P_RELAYlocal-observe (meshDeliveryContext) no longer hard-disconnects or debit-budgets the last hop; CWD.FABRIC_SEEDis opt-in; republish no longer expands the contract patch allow-list.Open finding: 1 High — first-wire
CONTRACT_PUBLISHstill grants patch rights to the AMP signer, so a content-identical front-run elevates an attacker into_contractPatchAllowListand allows unauthorizedCONTRACT_MESSAGEops. PoC-confirmed on this tip.No other medium+ issues with a clear remote attack path remained after validation.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Stale comment
Security review (HEAD
b373f998)Outcome: 1 High finding remains.
Prior automation findings were re-validated against current HEAD and are addressed (sealed unlock requires
settlementVerified, HTLC buy rebuilds/validates P2TR, Noise I/O has timeouts, CWD seed is opt-in, peel/RELAY local-observe gates, contract publish front-run authz).Current finding
- High — Junk
DocumentContentKeyRevealburns cooperative key delivery (first-writer-wins on publicpaymentHashHexbefore open succeeds).No other medium+ issues with a confirmed attacker→sink path were left after validation.
Sent by Cursor Automation: Find vulnerabilities
| if (!docId) return null; | ||
| const pay = obj.paymentHashHex != null ? String(obj.paymentHashHex).toLowerCase() : ''; | ||
| const key = obj.keyHex != null ? String(obj.keyHex).toLowerCase() : ''; | ||
| return `keyreveal:${docId}:${pay || key}`; |
There was a problem hiding this comment.
High — logical key uses public payment hash, enabling reveal slot theft.
paymentHashHex is advertised in sealed inventory (SHA256(K)). Dedup prefers that public value over keyHex, so any peer can claim keyreveal:<docId>:<pay> with a random non-matching keyHex.
Impact: first junk frame permanently occupies the slot (cache up to 10k); the seller’s real reveal is dropped as a duplicate. Claim only after successful open, or bind the key to verified seller identity / key material that proves hash preimage.
| const claimReveal = this._claimLogicalRegistrationOrPunish( | ||
| 'DocumentContentKeyReveal', reveal, signerPubkeyHex, punishOrigin); |
There was a problem hiding this comment.
High — claim-before-open DoS on paid sealed delivery.
Logical registration runs before openSealedDelivery / seller binding. A Noise-authenticated peer (or onion path) can send a signed junk DocumentContentKeyReveal with public documentId + paymentHashHex and a wrong keyHex; open fails but the claim sticks, and later valid reveals are ignored.
On private-relay routes, the reverse-forward below also relays that junk before validation. Buyer is stuck until HTLC timeout/claim-preimage fallback.
Fix: claim only after successful open (or unclaim on failure); reject/forward only when paymentHashHexFromKey(keyHex) matches.
There was a problem hiding this comment.
Security review (synchronize @ 4d35921)
Prior automation findings re-validated on HEAD — all fixed: CWD .FABRIC_SEED opt-in (allowCwdSeed), contract/chat bit-identical relay + wire-hash dedup, sealed unlock requiring settlementVerified, buyer HTLC rebuild/validateInventoryHtlcOffer, DocumentContentKeyReveal well-formedness + post-open claim, and clearing of indefinite blocking recv.
Open finding: 1 High — Noise framed I/O uses a per-select 30s budget with no overall handshake/frame deadline, so a slowloris peer can hold connections_rwlock across accept/handshake (and block writers during established reads) for hours by dripping bytes. Details in inline comments.
No other medium+ issues confirmed in the latest delta.
Sent by Cursor Automation: Find vulnerabilities
| /* Blocking read of exactly n bytes (fails closed on timeout / stall). */ | ||
| static ssize_t noise_read_all(int fd, uint8_t *p, size_t n) | ||
| { | ||
| size_t off = 0; | ||
| while (off < n) | ||
| { | ||
| int ready = noise_wait_fd(fd, 0, FABRIC_NOISE_IO_TIMEOUT_MS); | ||
| if (ready == 0) | ||
| { | ||
| errno = ETIMEDOUT; | ||
| return -1; | ||
| } | ||
| if (ready < 0) | ||
| return -1; | ||
| ssize_t r = recv(fd, p + off, n - off, 0); | ||
| if (r < 0 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK)) | ||
| continue; | ||
| if (r <= 0) | ||
| return r; | ||
| off += (size_t)r; | ||
| } | ||
| return (ssize_t)off; |
There was a problem hiding this comment.
High — unauthenticated slowloris DoS via per-wait I/O timeout.
noise_read_all resets a 30s select() budget on every partial recv. Combined with fabric_noise_read_frame accepting attacker-chosen lengths up to the out-cap (4096 during handshake; up to FABRIC_NOISE_MAX_FRAME_BYTES on established reads), a peer can advertise a large L and drip 1 byte every ~29s so one frame occupies the caller for hours. There is no overall handshake/frame deadline.
Impact: Accept path calls this while holding connections_rwlock for writing (src/peer.c ~1770), so one TCP client can block accepts/disconnects/connects for extended periods. Add a monotonic overall I/O budget (and/or release the wrlock around handshake).
| // Perform handshake as responder (accepted socket already non-blocking) | ||
| memcpy(conn->remote_pubkey, remote_pubkey + (33 - 32), 32); | ||
| if (noise_perform_xx_handshake(client_sock, 0, peer->app_protocol, peer->private_key, conn->remote_pubkey, &conn->send_cipher, &conn->recv_cipher) != 0) | ||
| if (noise_perform_xx_handshake(client_sock, 0, peer->app_protocol, NULL, conn->remote_pubkey, &conn->send_cipher, &conn->recv_cipher) != 0) |
There was a problem hiding this comment.
High — exclusive connection lock held across Noise handshake I/O.
After pubkey exchange, noise_perform_xx_handshake() runs under connections_rwlock (wrlock acquired earlier in this accept path). Because handshake frame reads use the per-select timeout in src/noise.c (no overall deadline), a slowloris client that starts the responder READ_MESSAGE phase can pin this write lock far beyond 30s and stall all connection-table mutations.
Impact: Unauthenticated remote availability loss for native fabricd peering. Perform the handshake off the wrlock (slot reserved / state machine), or enforce a hard total handshake time budget before unlock.


Initial sketch of support for RSI through @GoonCitizen.
Summary by CodeRabbit
New Features
Documentation
Tests