Fix: Admin Revoke (Delete messages from others)#7
Open
moothz wants to merge 2 commits intoEvolutionAPI:mainfrom
Open
Fix: Admin Revoke (Delete messages from others)#7moothz wants to merge 2 commits intoEvolutionAPI:mainfrom
moothz wants to merge 2 commits intoEvolutionAPI:mainfrom
Conversation
Reviewer's GuideEnables group admins to revoke messages sent by other participants via the API by extending the delete-message payload with sender context and wiring it into the WhatsApp client revoke call, and updates the API documentation with the new parameters and examples. Sequence diagram for admin revoke (delete message from another participant)sequenceDiagram
actor AdminUser
participant ApiServer
participant MessageService
participant Utils
participant WhatsAppClient
AdminUser->>ApiServer: POST /message/delete {chat, messageId, fromMe=false, participant}
ApiServer->>MessageService: DeleteMessageEveryone(data, instance)
MessageService->>MessageService: Parse chat JID (recipient)
MessageService->>Utils: ParseJID(participant)
Utils-->>MessageService: senderJID or error
alt invalid participant JID
MessageService-->>ApiServer: error "invalid participant JID"
ApiServer-->>AdminUser: HTTP 4xx
else valid participant JID
MessageService->>WhatsAppClient: SendMessage(recipient, BuildRevoke(recipient, senderJID, messageId))
WhatsAppClient-->>MessageService: revoke response
MessageService-->>ApiServer: success payload
ApiServer-->>AdminUser: HTTP 200
end
Class diagram for updated message deletion payload (MessageStruct)classDiagram
class MessageStruct {
+string Chat
+string MessageID
+bool FromMe
+string Participant
}
class MessageService {
+DeleteMessageEveryone(data MessageStruct, instance Instance)
}
MessageService --> MessageStruct
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 security issue, 2 other issues, and left some high level feedback:
Security issues:
- Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource. (link)
General comments:
- The new
FromMefield inMessageStructdefaults tofalsewhen omitted, which contradicts the documented default oftrueand will break existing clients that don’t sendfromMe; consider making it a pointer with a default oftrueor treating an absentfromMeastrueinDeleteMessageEveryone. - In the
ParseJIDerror path, the log callLogError("[%s] Error parsing participant JID for non-FromMe message: %s", data.Participant)has two%splaceholders but only one argument and appears to logparticipantwhereinstance.Idis expected; update the arguments/placeholders to include both values correctly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `FromMe` field in `MessageStruct` defaults to `false` when omitted, which contradicts the documented default of `true` and will break existing clients that don’t send `fromMe`; consider making it a pointer with a default of `true` or treating an absent `fromMe` as `true` in `DeleteMessageEveryone`.
- In the `ParseJID` error path, the log call `LogError("[%s] Error parsing participant JID for non-FromMe message: %s", data.Participant)` has two `%s` placeholders but only one argument and appears to log `participant` where `instance.Id` is expected; update the arguments/placeholders to include both values correctly.
## Individual Comments
### Comment 1
<location path="pkg/message/service/message_service.go" line_range="388-389" />
<code_context>
+ if data.Participant == "" {
+ return "", "", errors.New("participant is required to delete a message from another user")
+ }
+ parsedJID, ok := utils.ParseJID(data.Participant)
+ if !ok {
+ m.loggerWrapper.GetLogger(instance.Id).LogError("[%s] Error parsing participant JID for non-FromMe message: %s", data.Participant)
+ return "", "", errors.New("invalid participant JID")
</code_context>
<issue_to_address>
**issue (bug_risk):** LogError format string expects two placeholders but only one argument is provided
In the `!ok` branch, the `LogError` format string has two `%s` placeholders but only `data.Participant` is passed. This can cause incorrect logging or a runtime panic, depending on the logger. Please either pass two arguments (e.g. `instance.Id` and `data.Participant`) or update the format string to match the single argument.
</issue_to_address>
### Comment 2
<location path="pkg/message/service/message_service.go" line_range="382-385" />
<code_context>
}
+ var senderJID types.JID
+ if data.FromMe {
+ senderJID = types.EmptyJID
+ } else {
+ if data.Participant == "" {
+ return "", "", errors.New("participant is required to delete a message from another user")
+ }
</code_context>
<issue_to_address>
**question (bug_risk):** New FromMe/participant requirement may break existing clients that don’t send these fields
Previously, delete-everyone calls always used `types.EmptyJID` and did not require `FromMe` or `Participant`. With this change, requests that omit `FromMe` will be treated as non-FromMe and will now fail with "participant is required..." if `Participant` is empty, potentially breaking existing integrations. If you need backward compatibility, consider a fallback where `FromMe` is treated as true when `Participant` is empty, or gate the new behavior via versioning/feature flag.
</issue_to_address>
### Comment 3
<location path="docs/wiki/guias-api/api-messages.md" line_range="941-943" />
<code_context>
SUA-CHAVE-API
</code_context>
<issue_to_address>
**security (curl-auth-header):** Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
*Source: gitleaks*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+388
to
+389
| parsedJID, ok := utils.ParseJID(data.Participant) | ||
| if !ok { |
There was a problem hiding this comment.
issue (bug_risk): LogError format string expects two placeholders but only one argument is provided
In the !ok branch, the LogError format string has two %s placeholders but only data.Participant is passed. This can cause incorrect logging or a runtime panic, depending on the logger. Please either pass two arguments (e.g. instance.Id and data.Participant) or update the format string to match the single argument.
Comment on lines
+382
to
+385
| if data.FromMe { | ||
| senderJID = types.EmptyJID | ||
| } else { | ||
| if data.Participant == "" { |
There was a problem hiding this comment.
question (bug_risk): New FromMe/participant requirement may break existing clients that don’t send these fields
Previously, delete-everyone calls always used types.EmptyJID and did not require FromMe or Participant. With this change, requests that omit FromMe will be treated as non-FromMe and will now fail with "participant is required..." if Participant is empty, potentially breaking existing integrations. If you need backward compatibility, consider a fallback where FromMe is treated as true when Participant is empty, or gate the new behavior via versioning/feature flag.
Comment on lines
+941
to
+943
| curl -X POST http://localhost:4000/message/delete \ | ||
| -H "Content-Type: application/json" \ | ||
| -H "apikey: SUA-CHAVE-API" \ |
There was a problem hiding this comment.
security (curl-auth-header): Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
Source: gitleaks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the limitation where group admins were unable to delete messages sent by other participants via the API. This enables the "admin revoke" functionality.
These changes have been in effect in my codebase since december (early acess) - throughly tested.
Type of Change
Testing
Checklist
Summary by Sourcery
Enable revoking messages sent by other participants in group chats via the message delete API.
Bug Fixes:
Enhancements:
Documentation: