fix: prevent duplicate operator approval requests in Telegram bot#4420
fix: prevent duplicate operator approval requests in Telegram bot#4420JHON12091986 wants to merge 1 commit into
Conversation
![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: af2229ace6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -0,0 +1,12 @@ | |||
| fn handle_operator_approval(message: &TelegramMessage) -> Result<()> { | |||
There was a problem hiding this comment.
This new handler is never compiled or called: src/openhuman/mod.rs does not declare a telegram module, and a repo-wide search only finds get_pending_approval, TelegramMessage, and handle_operator_approval inside this new file. The live Telegram approval path remains under src/openhuman/channels/providers/telegram/, so duplicate operator prompts for real Telegram messages will continue to use the old flow and this fix has no effect.
Useful? React with 👍 / 👎.
📝 WalkthroughWalkthroughAdds a new ChangesOperator Approval Handling
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/telegram/bot.rs (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComments are in Spanish; rest of codebase uses English.
For consistency with the rest of the codebase (per surrounding context/learnings, all shown code uses English comments), translate these to English.
✏️ Suggested translation
- // Verificar si ya hay una solicitud pendiente para este usuario + // Check whether there is already a pending request for this user if let Some(pending) = get_pending_approval(message.from.id) { - // Si ya hay una solicitud pendiente, no crear una nueva + // If a request is already pending, don't create a new one return Ok(()); } - // Crear una nueva solicitud de aprobación + // Create a new approval request create_approval_request(message.from.id, message.text)?;Also applies to: 4-4, 8-8
🤖 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 `@src/openhuman/telegram/bot.rs` at line 2, The comments in the Telegram bot code are written in Spanish while the rest of the codebase uses English, so update the inline comments in bot.rs to match the project’s English convention. Translate the Spanish comment(s) near the pending-request check and the other flagged comment(s) in the same area, keeping the intent clear and consistent with surrounding English comments in the bot logic.
🤖 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.
Inline comments:
In `@src/openhuman/telegram/bot.rs`:
- Around line 3-9: The approval-request flow is still vulnerable to duplicate
sends because `get_pending_approval(message.from.id)` and
`create_approval_request(message.from.id, message.text)` are separate non-atomic
steps. Update the logic around this bot handler so the pending-check and insert
happen atomically, using either a transaction, a sender-keyed mutex, or a
database-level uniqueness constraint on the sender/session key. Also review the
persistence layer for `pending_approvals` so the schema prevents duplicate rows
for the same user, not just duplicate `request_id` values.
---
Nitpick comments:
In `@src/openhuman/telegram/bot.rs`:
- Line 2: The comments in the Telegram bot code are written in Spanish while the
rest of the codebase uses English, so update the inline comments in bot.rs to
match the project’s English convention. Translate the Spanish comment(s) near
the pending-request check and the other flagged comment(s) in the same area,
keeping the intent clear and consistent with surrounding English comments in the
bot logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ded66a2e-3ab1-4dcf-9261-db82c3e736dd
📒 Files selected for processing (1)
src/openhuman/telegram/bot.rs
| if let Some(pending) = get_pending_approval(message.from.id) { | ||
| // Si ya hay una solicitud pendiente, no crear una nueva | ||
| return Ok(()); | ||
| } | ||
|
|
||
| // Crear una nueva solicitud de aprobación | ||
| create_approval_request(message.from.id, message.text)?; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Check-then-create is not atomic — race condition can still send duplicate approval requests.
The exact bug this PR intends to fix (duplicate approval requests) can still occur if two messages from the same sender arrive concurrently: both can pass the get_pending_approval check as None before either calls create_approval_request. Given the approval store snippets show pending_approvals.request_id as the PRIMARY KEY CREATE TABLE IF NOT EXISTS pending_approvals ( request_id TEXT PRIMARY KEY, but no unique constraint on sender, nothing at the persistence layer prevents duplicate rows for the same user either.
Consider making the check-and-insert atomic (e.g., a single transaction, a DB-level uniqueness constraint on the sender/session key, or a mutex keyed by sender id) rather than two separate calls.
🤖 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 `@src/openhuman/telegram/bot.rs` around lines 3 - 9, The approval-request flow
is still vulnerable to duplicate sends because
`get_pending_approval(message.from.id)` and
`create_approval_request(message.from.id, message.text)` are separate non-atomic
steps. Update the logic around this bot handler so the pending-check and insert
happen atomically, using either a transaction, a sender-keyed mutex, or a
database-level uniqueness constraint on the sender/session key. Also review the
persistence layer for `pending_approvals` so the schema prevents duplicate rows
for the same user, not just duplicate `request_id` values.
1 participant
Closes #4385
/claim #4385
Changes
Testing
Payment wallet (USDC on Ethereum):
0x46243c949f30280771c0675Bc8A16155A9B96e51Summary by CodeRabbit