Add /warn command#135
Open
itasimo wants to merge 2 commits into
Open
Conversation
Add /warn to warn a user, /unwarn to remove a warn given the warnId and /warns to get the warn history of a user Add docker-compose.yml to run the bot with a local database Add cross-env to package.json to set environment variables in a cross-platform way Refactor src/modules/moderation/index.ts to help with readability
Features: - Finished implementing the `/warn` command with automatic warning-based moderation. - Added the `tgnumber` argument type to enforce strict numeric inputs instead of `numberOrString`. - Added the `ephemeralInGroup` function to send temporary messages in groups while keeping them permanent in private chats. Fixes: - Fixed a bug where `/ban_all` crashed if the bot lacked access to the logging group. Decoupled the logic so that if the initial message fails, progress updates are skipped but command execution continues normally.
Contributor
WalkthroughAdds a warning system with ChangesWarning System & Ban-All Resilience
Infrastructure & Build Config
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@docker-compose.yml`:
- Around line 8-10: The Redis service configuration is hardcoding the password
and exposing the port broadly; update the docker-compose Redis service to use an
environment-substituted secret that matches the REDIS_PASSWORD used by
src/redis/index.ts, and bind the published port to localhost for development.
Adjust the Redis service command and ports mapping so credentials are not
committed and the app can still authenticate consistently.
- Around line 18-26: The InfluxDB service configuration currently hardcodes
admin credentials in the compose setup and exposes the port broadly, so update
the docker-compose service to read sensitive values from environment variables
or an env file instead of embedding them directly. Use the existing InfluxDB
init settings in the service definition to map `DOCKER_INFLUXDB_INIT_*` values
from env-driven inputs, and ensure the app’s `INFLUXDB_TOKEN` stays the single
source of truth for telemetry auth to avoid config drift. Identify the changes
in the InfluxDB service block by its `DOCKER_INFLUXDB_INIT_*` environment
entries and `ports` section.
In `@package.json`:
- Line 9: The start script currently depends on cross-env, but it may not be
available in production installs. Update the package.json scripts or dependency
list so start no longer relies on a dev-only binary: either move cross-env into
dependencies or remove it from the start command and use a plain
NODE_ENV=production launch. Make the change in the start script entry so
production startup works after npm ci --omit=dev.
- Line 42: The package manifest currently depends on a sibling-local backend
path, which breaks installs when that neighboring directory is absent. Update
the dependency for `@polinetwork/backend` in package.json to use a workspace
reference or a published semver version instead of the file:../backend/package
path so fresh CI and release installs can resolve it reliably.
In `@src/commands/moderation/warn.ts`:
- Around line 126-132: The auto-ban path in warn.ts is bypassing the same
ban-all protection used by the manual /ban_all flow. Update the
warning-threshold branch in the warn command to reuse the existing ban-all
bypass guard before calling tgLogger.banAll, so protected/privileged accounts
cannot be auto-banned across all groups. Locate the logic around the
getTotalActiveCount query and the tgLogger.banAll call and apply the same
permission check used by the ban-all command path.
- Around line 109-132: The auto-moderation checks in warn.ts currently hardcode
`>= 3` and `>= 4`, which can retrigger `Moderation.kick` and
`modules.get("tgLogger").banAll` on every later warning instead of only when the
threshold is first crossed. Update the warning flow to compare against
configurable threshold values (read from config rather than literals) and gate
the actions so they fire only once when moving from below the threshold to
at/above it, using the existing `getActiveCountInGroup` and
`getTotalActiveCount` checks as the decision points.
- Around line 168-171: The /unwarn flow in warn.ts only deletes by warnId, which
allows cross-group deletion without chat context. Update the moderation command
handler around the deleteById mutate call to also pass the current group/chat
identifier, and make sure the backend authorization path in the deleteById
mutation verifies the warning belongs to that chat before deleting. Use the
existing warnId handling in this command plus the current chat/group context
available in the moderation command to scope the request.
- Around line 68-75: The /warn command currently allows DM execution via scope:
"both", but the handler always persists groupId from context.chat.id, so
warnings in private chats get tied to the DM instead of a real group. Update the
moderation/warn.ts command definition and handler so warn only runs in group
chats or requires an explicit target group before calling the warn mutation,
using the existing handler, context, and repliedTo flow to enforce a valid
group-scoped groupId.
In `@src/utils/types.ts`:
- Around line 42-47: The tgnumber schema currently accepts non-integer and
non-decimal-edge values like negatives, floats, hex strings, and Infinity, which
is too loose for warning IDs. Tighten the refinement in tgnumber so it only
accepts valid finite positive integer strings (or whatever exact ID shape the
/unwarn caller expects) before transforming, and keep the rejection at parse
time instead of allowing malformed values through to deleteById.
In `@src/utils/users.ts`:
- Around line 48-50: The lookup failure path in resolveUser’s caller currently
returns Err("SILENT_ERROR"), which causes a second generic reply after
resolveUser() already handles the “user not found” case. Update the logic around
the user resolution check to stop returning an error there and instead exit
early with no additional response, so commands like ban, mute, and warn only
send the single intended failure message.
🪄 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: 462fedc1-d6a4-417d-9db6-369936c2e5ec
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
docker-compose.ymlpackage.jsonsrc/commands/moderation/index.tssrc/commands/moderation/warn.tssrc/modules/moderation/ban-all.tssrc/modules/moderation/index.tssrc/modules/tg-logger/index.tssrc/utils/types.tssrc/utils/users.ts
Comment on lines
+8
to
+10
| command: redis-server --requirepass "Radmin" | ||
| ports: | ||
| - "6379:6379" |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid hardcoded Redis credentials on a host-exposed port.
Radmin is committed and Redis is published on all interfaces; it also has to match REDIS_PASSWORD from src/redis/index.ts, or the app cannot authenticate. Use an env-substituted secret and bind locally for dev.
Suggested change
- command: redis-server --requirepass "Radmin"
+ command: ["redis-server", "--requirepass", "${REDIS_PASSWORD:?REDIS_PASSWORD must be set}"]
ports:
- - "6379:6379"
+ - "127.0.0.1:6379:6379"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| command: redis-server --requirepass "Radmin" | |
| ports: | |
| - "6379:6379" | |
| command: ["redis-server", "--requirepass", "${REDIS_PASSWORD:?REDIS_PASSWORD must be set}"] | |
| ports: | |
| - "127.0.0.1:6379:6379" |
🤖 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 `@docker-compose.yml` around lines 8 - 10, The Redis service configuration is
hardcoding the password and exposing the port broadly; update the docker-compose
Redis service to use an environment-substituted secret that matches the
REDIS_PASSWORD used by src/redis/index.ts, and bind the published port to
localhost for development. Adjust the Redis service command and ports mapping so
credentials are not committed and the app can still authenticate consistently.
Comment on lines
+18
to
+26
| ports: | ||
| - "8086:8086" | ||
| environment: | ||
| - DOCKER_INFLUXDB_INIT_MODE=setup | ||
| - DOCKER_INFLUXDB_INIT_USERNAME=Iadmin | ||
| - DOCKER_INFLUXDB_INIT_PASSWORD=Iadmin1234567 | ||
| - DOCKER_INFLUXDB_INIT_ORG=polinetwork | ||
| - DOCKER_INFLUXDB_INIT_BUCKET=telegram_bot | ||
| - DOCKER_INFLUXDB_INIT_ADMIN_TOKEN=tu_token_secreto_de_32_caracteres_o_mas |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Move InfluxDB admin credentials out of the compose file.
The admin password/token are committed and the service is exposed on all interfaces. Since telemetry depends on INFLUXDB_TOKEN, make these env-driven so local setup and app config cannot drift.
Suggested change
ports:
- - "8086:8086"
+ - "127.0.0.1:8086:8086"
environment:
- DOCKER_INFLUXDB_INIT_MODE=setup
- - DOCKER_INFLUXDB_INIT_USERNAME=Iadmin
- - DOCKER_INFLUXDB_INIT_PASSWORD=Iadmin1234567
+ - DOCKER_INFLUXDB_INIT_USERNAME=${INFLUXDB_INIT_USERNAME:?INFLUXDB_INIT_USERNAME must be set}
+ - DOCKER_INFLUXDB_INIT_PASSWORD=${INFLUXDB_INIT_PASSWORD:?INFLUXDB_INIT_PASSWORD must be set}
- DOCKER_INFLUXDB_INIT_ORG=polinetwork
- DOCKER_INFLUXDB_INIT_BUCKET=telegram_bot
- - DOCKER_INFLUXDB_INIT_ADMIN_TOKEN=tu_token_secreto_de_32_caracteres_o_mas
+ - DOCKER_INFLUXDB_INIT_ADMIN_TOKEN=${INFLUXDB_TOKEN:?INFLUXDB_TOKEN must be set}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ports: | |
| - "8086:8086" | |
| environment: | |
| - DOCKER_INFLUXDB_INIT_MODE=setup | |
| - DOCKER_INFLUXDB_INIT_USERNAME=Iadmin | |
| - DOCKER_INFLUXDB_INIT_PASSWORD=Iadmin1234567 | |
| - DOCKER_INFLUXDB_INIT_ORG=polinetwork | |
| - DOCKER_INFLUXDB_INIT_BUCKET=telegram_bot | |
| - DOCKER_INFLUXDB_INIT_ADMIN_TOKEN=tu_token_secreto_de_32_caracteres_o_mas | |
| ports: | |
| - "127.0.0.1:8086:8086" | |
| environment: | |
| - DOCKER_INFLUXDB_INIT_MODE=setup | |
| - DOCKER_INFLUXDB_INIT_USERNAME=${INFLUXDB_INIT_USERNAME:?INFLUXDB_INIT_USERNAME must be set} | |
| - DOCKER_INFLUXDB_INIT_PASSWORD=${INFLUXDB_INIT_PASSWORD:?INFLUXDB_INIT_PASSWORD must be set} | |
| - DOCKER_INFLUXDB_INIT_ORG=polinetwork | |
| - DOCKER_INFLUXDB_INIT_BUCKET=telegram_bot | |
| - DOCKER_INFLUXDB_INIT_ADMIN_TOKEN=${INFLUXDB_TOKEN:?INFLUXDB_TOKEN must be set} |
🤖 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 `@docker-compose.yml` around lines 18 - 26, The InfluxDB service configuration
currently hardcodes admin credentials in the compose setup and exposes the port
broadly, so update the docker-compose service to read sensitive values from
environment variables or an env file instead of embedding them directly. Use the
existing InfluxDB init settings in the service definition to map
`DOCKER_INFLUXDB_INIT_*` values from env-driven inputs, and ensure the app’s
`INFLUXDB_TOKEN` stays the single source of truth for telemetry auth to avoid
config drift. Identify the changes in the InfluxDB service block by its
`DOCKER_INFLUXDB_INIT_*` environment entries and `ports` section.
| "type": "module", | ||
| "scripts": { | ||
| "start": "NODE_ENV=production node dist/bot.js", | ||
| "start": "cross-env NODE_ENV=production node dist/bot.js", |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether runtime scripts depend on dev-only binaries.
node - <<'NODE'
const pkg = require('./package.json')
const startUsesCrossEnv = /\bcross-env\b/.test(pkg.scripts?.start || '')
const inDeps = Boolean(pkg.dependencies?.['cross-env'])
const inDevDeps = Boolean(pkg.devDependencies?.['cross-env'])
console.log({ startUsesCrossEnv, inDeps, inDevDeps })
if (startUsesCrossEnv && !inDeps) {
process.exitCode = 1
}
NODERepository: PoliNetworkOrg/telegram
Length of output: 221
Move cross-env into dependencies or remove it from start. npm ci --omit=dev won’t install the binary, so production startup will fail. (package.json:9)
🤖 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 `@package.json` at line 9, The start script currently depends on cross-env, but
it may not be available in production installs. Update the package.json scripts
or dependency list so start no longer relies on a dev-only binary: either move
cross-env into dependencies or remove it from the start command and use a plain
NODE_ENV=production launch. Make the change in the start script entry so
production startup works after npm ci --omit=dev.
| "@grammyjs/runner": "^2.0.3", | ||
| "@influxdata/influxdb-client": "^1.35.0", | ||
| "@polinetwork/backend": "^0.15.18", | ||
| "@polinetwork/backend": "file:../backend/package", |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the local file dependency is resolvable in this checkout.
node - <<'NODE'
const fs = require('fs')
const path = require('path')
const pkg = require('./package.json')
const spec = pkg.dependencies?.['`@polinetwork/backend`']
console.log({ spec })
if (spec?.startsWith('file:')) {
const target = path.resolve(process.cwd(), spec.slice('file:'.length))
console.log({ target, exists: fs.existsSync(target) })
if (!fs.existsSync(target)) process.exitCode = 1
}
NODERepository: PoliNetworkOrg/telegram
Length of output: 257
Avoid the sibling-local backend dependency. file:../backend/package only works when that neighboring path exists, so fresh installs in CI or published releases will fail to resolve @polinetwork/backend. Use a workspace dependency or a published semver version instead.
🤖 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 `@package.json` at line 42, The package manifest currently depends on a
sibling-local backend path, which breaks installs when that neighboring
directory is absent. Update the dependency for `@polinetwork/backend` in
package.json to use a workspace reference or a published semver version instead
of the file:../backend/package path so fresh CI and release installs can resolve
it reliably.
Comment on lines
+68
to
+75
| description: "Warn a user from a group", | ||
| scope: "both", | ||
| permissions: { | ||
| allowedRoles: ["owner", "direttivo"], | ||
| excludedRoles: ["creator"], | ||
| allowGroupAdmins: true, | ||
| }, | ||
| handler: async ({ args, context, repliedTo }) => { |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Restrict /warn to groups, or require an explicit target group.
With scope: "both", this can run in DMs, but the mutation always stores groupId: context.chat.id. In a private chat that records the DM as the warning’s group and can still drive the later penalty logic, which breaks the group-scoped warning model from the PR objective.
Also applies to: 90-95
🤖 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/commands/moderation/warn.ts` around lines 68 - 75, The /warn command
currently allows DM execution via scope: "both", but the handler always persists
groupId from context.chat.id, so warnings in private chats get tied to the DM
instead of a real group. Update the moderation/warn.ts command definition and
handler so warn only runs in group chats or requires an explicit target group
before calling the warn mutation, using the existing handler, context, and
repliedTo flow to enforce a valid group-scoped groupId.
Comment on lines
+168
to
+171
| try { | ||
| const res = await api.tg.warnings.deleteById.mutate({ | ||
| id: warnId | ||
| }) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
/unwarn needs group-scoped authorization, not just a global warning ID.
This mutation only sends { id: warnId }, so the backend has no group context to verify that the warning belongs to the current chat. Any admin who can run /unwarn can remove warnings from another group by guessing IDs.
🤖 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/commands/moderation/warn.ts` around lines 168 - 171, The /unwarn flow in
warn.ts only deletes by warnId, which allows cross-group deletion without chat
context. Update the moderation command handler around the deleteById mutate call
to also pass the current group/chat identifier, and make sure the backend
authorization path in the deleteById mutation verifies the warning belongs to
that chat before deleting. Use the existing warnId handling in this command plus
the current chat/group context available in the moderation command to scope the
request.
Comment on lines
+42
to
+47
| export const tgnumber = z | ||
| .string() | ||
| .refine((s) => !Number.isNaN(Number(s)) && s.trim() !== "", { | ||
| message: "Must be a valid numeric string", | ||
| }) | ||
| .transform(Number); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Tighten this schema before using it for IDs.
This accepts values like -1, 1.5, 0x10, and Infinity. The current /unwarn caller uses it for a warning ID, so malformed IDs can reach deleteById instead of being rejected at parse time.
Suggested fix
export const tgnumber = z
.string()
- .refine((s) => !Number.isNaN(Number(s)) && s.trim() !== "", {
- message: "Must be a valid numeric string",
+ .trim()
+ .refine((s) => /^\d+$/.test(s), {
+ message: "Must be a valid positive integer string",
})
.transform(Number);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const tgnumber = z | |
| .string() | |
| .refine((s) => !Number.isNaN(Number(s)) && s.trim() !== "", { | |
| message: "Must be a valid numeric string", | |
| }) | |
| .transform(Number); | |
| export const tgnumber = z | |
| .string() | |
| .trim() | |
| .refine((s) => /^\d+$/.test(s), { | |
| message: "Must be a valid positive integer string", | |
| }) | |
| .transform(Number); |
🤖 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/utils/types.ts` around lines 42 - 47, The tgnumber schema currently
accepts non-integer and non-decimal-edge values like negatives, floats, hex
strings, and Infinity, which is too loose for warning IDs. Tighten the
refinement in tgnumber so it only accepts valid finite positive integer strings
(or whatever exact ID shape the /unwarn caller expects) before transforming, and
keep the rejection at parse time instead of allowing malformed values through to
deleteById.
Comment on lines
+48
to
+50
| const user = await resolveUser(firstArg, context) | ||
| if (!user) { | ||
| return new Err("SILENT_ERROR") |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid double replies on lookup failures.
resolveUser() already sends the “user not found / cannot find this user” reply on this path. Returning Err("SILENT_ERROR") here makes /ban, /mute, and /warn emit a second generic error message for the same failure.
🤖 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/utils/users.ts` around lines 48 - 50, The lookup failure path in
resolveUser’s caller currently returns Err("SILENT_ERROR"), which causes a
second generic reply after resolveUser() already handles the “user not found”
case. Update the logic around the user resolution check to stop returning an
error there and instead exit early with no additional response, so commands like
ban, mute, and warn only send the single intended failure message.
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.
Closes #82