Skip to content

fix: log audit for banall#98

Merged
lorenzocorallo merged 5 commits intomainfrom
audit-log-banall
Apr 11, 2026
Merged

fix: log audit for banall#98
lorenzocorallo merged 5 commits intomainfrom
audit-log-banall

Conversation

@lorenzocorallo
Copy link
Copy Markdown
Member

No description provided.

@lorenzocorallo lorenzocorallo requested a review from toto04 April 11, 2026 22:12
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 11, 2026

Warning

Rate limit exceeded

@toto04 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 8 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 8 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 93526f18-6119-42cc-8f97-254c5f4fe3e1

📥 Commits

Reviewing files that changed from the base of the PR and between 12a9b8e and 6853e77.

📒 Files selected for processing (2)
  • src/modules/moderation/ban-all.ts
  • src/modules/moderation/index.ts

Walkthrough

Updated a backend dependency and enhanced the ban-all moderation flow by recording an audit-log entry at initiation, running message deletion concurrently with the ban operation in the executor, and exposing a message-deletion helper as public.

Changes

Cohort / File(s) Summary
Dependency
package.json
Bumped @polinetwork/backend from ^0.15.10^0.15.11.
Ban-all orchestration
src/modules/moderation/ban-all.ts
Added audit-log mutation at start of initiateBanAll(...) recording adminId, targetId, action type, reason, null groupId/until before enqueueing the orchestrator job. Modified BAN executor to run banChatMember(...) and Moderation.deleteAllLastMessages(...) in parallel; failures from banChatMember are caught and converted to false to allow message-deletion to proceed while preserving downstream error handling. Added import of Moderation.
Message-deletion helper visibility
src/modules/moderation/index.ts
Renamed/changed visibility of message deletion helper: private deleteLastMessages(...)public deleteAllLastMessages(...) and updated BAN execution path to call deleteAllLastMessages(...) (preserves previous best-effort deletion logic).

Sequence Diagram(s)

mermaid
sequenceDiagram
actor Admin
participant API as Frontend/API
participant Audit as AuditLog
participant Queue as JobQueue/Orchestrator
participant Exec as BanExecutor
participant Mod as ModerationModule
participant TG as TelegramAPI

Admin->>API: request initiateBanAll(target, reason)
API->>Audit: create audit entry (adminId, targetId, action, reason)
API->>Queue: enqueue ban-all job (payload)
Queue->>Exec: start ban job for target
Exec->>Mod: call deleteAllLastMessages(userId, chatId) & call banChatMember(userId, chatId) in parallel
par parallel ops
  Mod->>TG: delete messages (best-effort, suppress errors)
  Exec->>TG: banChatMember (may fail)
and
  TG-->>Mod: deletion result
  TG-->>Exec: ban result
end
Exec->>Exec: treat ban failure as false, allow deletion result to stand
Exec-->>Queue: job completes (success or propagated error based on ban outcome)

Possibly related PRs

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the primary change: adding audit logging for the ban-all function, which is the main feature across the modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/modules/moderation/ban-all.ts`:
- Around line 174-182: The audit log call using api.tg.auditLog.create.mutate
with the banAll payload should be best-effort: wrap that call in a try/catch so
any thrown error is caught, log the error (using the module logger or console)
with context (e.g., adminId/targetId/type) and continue execution so the
subsequent queue/job creation that uses banAll still runs even if audit logging
fails; do not change the success path or rethrow the error.
🪄 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: e64652dc-17dc-4f68-8d7f-1d69ec8af715

📥 Commits

Reviewing files that changed from the base of the PR and between 776a507 and 399e395.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (2)
  • package.json
  • src/modules/moderation/ban-all.ts

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/modules/moderation/index.ts (1)

36-47: ⚠️ Potential issue | 🔴 Critical

Change MUTE_ALL to UNBAN_ALL to align with ban-all feature and fix audit-log type mismatch.

The MUTE_ALL entry at line 46 maps to "mute_all", but the audit-log type union expects "unban_all" instead. The ban-all feature in the codebase defines BanAllCommand = "ban_all" | "unban_all", confirming that MUTE_ALL is incorrect and should be UNBAN_ALL. Update line 37's type annotation accordingly.

Proposed fix
 const MAP_ACTIONS: Record<
-  Exclude<ModerationAction["action"], "SILENT" | "MULTI_CHAT_SPAM"> | "BAN_ALL" | "MUTE_ALL",
+  Exclude<ModerationAction["action"], "SILENT" | "MULTI_CHAT_SPAM"> | "BAN_ALL" | "UNBAN_ALL",
   ApiInput["tg"]["auditLog"]["create"]["type"]
 > = {
   MUTE: "mute",
   BAN: "ban",
   KICK: "kick",
   UNBAN: "unban",
   UNMUTE: "unmute",
   BAN_ALL: "ban_all",
-  MUTE_ALL: "mute_all",
+  UNBAN_ALL: "unban_all",
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/moderation/index.ts` around lines 36 - 47, The MAP_ACTIONS
mapping uses the wrong key/value for the "unban all" case: replace the MUTE_ALL
union member with UNBAN_ALL in the Record key type (the
Exclude<ModerationAction["action"], ...> union) and update the object entry key
from MUTE_ALL to UNBAN_ALL with the corresponding audit-log type value
"unban_all" so MAP_ACTIONS (and its relation to
ApiInput["tg"]["auditLog"]["create"]["type"]) matches the BanAllCommand
("ban_all" | "unban_all") and ModerationAction["action"] values.
♻️ Duplicate comments (1)
src/modules/moderation/ban-all.ts (1)

178-185: ⚠️ Potential issue | 🟠 Major

Audit write is still a hard blocker before queue creation.

This is the same unresolved concern already raised in earlier review comments: failure here can prevent job creation at Line 187.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/moderation/ban-all.ts` around lines 178 - 185, The audit write
using api.tg.auditLog.create.mutate for the banAll record can throw and
currently blocks subsequent queue/job creation; wrap the audit call
(api.tg.auditLog.create.mutate) in a try/catch and ensure job/queue creation
proceeds on failure (either log the error with context—include
banAll.reporter.id and banAll.target.id—and continue, or move the audit write to
after the queue creation and make it non-blocking), so that failures to persist
the audit do not prevent the job from being enqueued/created.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/modules/moderation/ban-all.ts`:
- Around line 100-105: The code currently awaits this.shared.api.banChatMember
inside Promise.all which serializes the operations; remove the inner await so
both promises run in parallel by passing the banChatMember promise directly into
Promise.all alongside Moderation.deleteAllLastMessages(job.data.targetId,
job.data.chatId), preserving the .catch(() => false) on the ban promise to
handle errors; adjust the destructuring of the result (const [success]) to keep
behavior unchanged.

---

Outside diff comments:
In `@src/modules/moderation/index.ts`:
- Around line 36-47: The MAP_ACTIONS mapping uses the wrong key/value for the
"unban all" case: replace the MUTE_ALL union member with UNBAN_ALL in the Record
key type (the Exclude<ModerationAction["action"], ...> union) and update the
object entry key from MUTE_ALL to UNBAN_ALL with the corresponding audit-log
type value "unban_all" so MAP_ACTIONS (and its relation to
ApiInput["tg"]["auditLog"]["create"]["type"]) matches the BanAllCommand
("ban_all" | "unban_all") and ModerationAction["action"] values.

---

Duplicate comments:
In `@src/modules/moderation/ban-all.ts`:
- Around line 178-185: The audit write using api.tg.auditLog.create.mutate for
the banAll record can throw and currently blocks subsequent queue/job creation;
wrap the audit call (api.tg.auditLog.create.mutate) in a try/catch and ensure
job/queue creation proceeds on failure (either log the error with
context—include banAll.reporter.id and banAll.target.id—and continue, or move
the audit write to after the queue creation and make it non-blocking), so that
failures to persist the audit do not prevent the job from being
enqueued/created.
🪄 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: e5114e02-d1a9-455c-b294-1decdba75f85

📥 Commits

Reviewing files that changed from the base of the PR and between 399e395 and 12a9b8e.

📒 Files selected for processing (2)
  • src/modules/moderation/ban-all.ts
  • src/modules/moderation/index.ts

@lorenzocorallo lorenzocorallo merged commit 16f6e73 into main Apr 11, 2026
2 checks passed
@lorenzocorallo lorenzocorallo deleted the audit-log-banall branch April 11, 2026 22:27
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