Skip to content

ot/simot: make sender sessions one-shot - #679

Merged
bwesterb merged 1 commit into
mainfrom
bas/zk15
Jul 31, 2026
Merged

ot/simot: make sender sessions one-shot#679
bwesterb merged 1 commit into
mainfrom
bas/zk15

Conversation

@bwesterb

@bwesterb bwesterb commented Jul 31, 2026

Copy link
Copy Markdown
Member

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread ot/simot/simotlocal.go
Comment on lines +121 to +124
if sender.consumed {
panic("simot: sender session already consumed")
}
sender.consumed = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Reusing a completed sender session crashes the program instead of reporting an error

A second attempt to answer the receiver on an already-finished sender session aborts the whole program (panic("simot: sender session already consumed") at ot/simot/simotlocal.go:122) instead of returning an error like the other steps of the protocol do, so any application that drives this exchange from network messages can be taken down by a duplicate message.
Impact: A duplicated or replayed protocol message can crash a service that uses this package rather than yielding a recoverable error.

One-shot enforcement uses panic in an exported API whose sibling returns errors

Round2Sender is exported and receives B from the remote receiver. The new one-shot guard (ot/simot/simotlocal.go:121-124) reacts to a second invocation with panic, whereas the analogous receiver step Round3Receiver (ot/simot/simotlocal.go:186-189) returns errors.New(...) for invalid input. REVIEW.md requires "Errors are returned, not swallowed. No panic on attacker-controlled input." A caller looping over incoming messages (e.g. a server handling a duplicated round-2 message for the same session) has no way to recover without installing a recover. Consider returning an error (or exposing a distinct method/error return) instead of panicking, and note the behavioral change to the public API in the PR description.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread ot/simot/simotlocal.go
Comment on lines +121 to +124
if sender.consumed {
panic("simot: sender session already consumed")
}
sender.consumed = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 Exported protocol step panics on repeated invocation instead of returning an error

The new one-shot guard in Round2Sender (ot/simot/simotlocal.go:121-124) panics when the sender session has already been used. Round2Sender is an exported API driven by remote receiver messages, so an application that processes a duplicated/replayed round-2 message for the same session will crash rather than get a recoverable error. REVIEW.md mandates "Errors are returned, not swallowed. No panic on attacker-controlled input."

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@bwesterb
bwesterb merged commit df9fbea into main Jul 31, 2026
14 checks passed
@bwesterb
bwesterb deleted the bas/zk15 branch July 31, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants