Skip to content

Conversation

@Luquitasjeffrey
Copy link
Contributor

@Luquitasjeffrey Luquitasjeffrey commented Dec 31, 2025

Added a commandlogging middleware that intercepts all commands sent by users and add them to a log file

Summary by CodeRabbit

  • New Features

    • Global logging of user commands, messages, and callback interactions with timestamps and user details for auditing.
  • Chores

    • Enabled size-based log rotation and added the command log filename to ignore rules.
    • Added configuration variables to specify the command log file and max log size.
  • Tests

    • Removed the test that verified middleware registration during bot initialization.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 31, 2025

Walkthrough

Adds a Winston-based Telegraf middleware to log user commands, messages, and callback queries to a file; registers the middleware at bot startup; adds commands.log to .gitignore; updates .env-sample with two COMMAND_LOG_* entries; removes a test asserting middleware registration. (45 words)

Changes

Cohort / File(s) Summary
Ignore list
/.gitignore
Adds commands.log to ignore list; .history/ entry reordered (no behavioral change)
Middleware
bot/middleware/commandlogging.ts
New TypeScript middleware commandLogger() that logs slash commands, plain messages, and callback queries with user info to a file via Winston; handles missing fields and errors; calls next()
Bot integration
bot/start.ts
Imports and registers middleware with bot.use(commandLogger()) during startup
Configuration sample
.env-sample
Adds COMMAND_LOG_FILE='commands.log' and COMMAND_LOG_SIZE_GB=4 entries
Tests
tests/bot/bot.spec.ts
Removed test asserting bot middleware registration count (bot.use invocation assertion)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • grunch
  • Catrya

Poem

🐇 I hop through code and nibble logs at night,
A slash, a ping, a tiny trace of light.
Winston writes the tale of every little cheer,
I bury crumbs of metadata near.
🥕📜✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: a new logging middleware that captures all user commands into a log file.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
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: 3

🧹 Nitpick comments (2)
bot/middleware/commandlogging.ts (2)

43-51: Consider reusing existing command parsing logic.

This simple whitespace-based parsing duplicates work already done by commandArgsMiddleware in bot/middleware/commands.ts, which handles quoted arguments, escape sequences, and other edge cases. The existing middleware populates ctx.state.command with parsed command details.

💡 Suggested approach

Option 1: Access the already-parsed command from ctx.state.command (requires moving commandLogger after commandArgsMiddleware):

// After commandArgsMiddleware runs:
if (ctx.state?.command) {
  const { command, args } = ctx.state.command;
  logger.info(`User @${userName} [${userId}] executed command: ${command} with args: [${args.join(', ')}]`);
}

Option 2: If logging must happen before commandArgsMiddleware, document that this is intentionally simplified parsing for early-stage logging.


70-71: Consider adding more context to generic log message.

The fallback log "Received non-command message or update from user" provides minimal debugging value. Consider including the update type or other contextual information.

💡 Example enhancement
-logger.info(`Received non-command message or update from user.`);
+logger.info(`Received non-command update: ${ctx.updateType ?? 'unknown'}`);
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 875ede4 and 1aa6996.

📒 Files selected for processing (3)
  • .gitignore
  • bot/middleware/commandlogging.ts
  • bot/start.ts
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to **/*.ts : Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/middleware/**/*.ts : Maintain middleware chain order: User validation → Admin checking → Context enhancement → Command routing
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/start.ts : Implement bot initialization, command registration, and scheduled job setup in bot/start.ts
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Applies to bot/** : Place commands, scenes, and middleware modules under bot/
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to **/*.ts : Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/**/*.ts : Use custom Telegraf context types (MainContext, CommunityContext) in handlers and modules

Applied to files:

  • bot/middleware/commandlogging.ts
  • bot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/middleware/**/*.ts : Maintain middleware chain order: User validation → Admin checking → Context enhancement → Command routing

Applied to files:

  • bot/middleware/commandlogging.ts
  • bot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/start.ts : Implement bot initialization, command registration, and scheduled job setup in bot/start.ts

Applied to files:

  • bot/middleware/commandlogging.ts
  • bot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/modules/*/{commands,actions,messages,scenes,index}.ts : Structure each feature module with commands.ts, actions.ts, messages.ts, scenes.ts, and index.ts

Applied to files:

  • bot/middleware/commandlogging.ts
  • bot/start.ts
📚 Learning: 2025-10-22T14:03:55.114Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Applies to app.ts : Keep startup logic centralized in the root app.ts that bootstraps the Telegram bot and Mongo connection

Applied to files:

  • bot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to app.ts : Keep app.ts as the single entry point to connect to MongoDB and initialize the bot

Applied to files:

  • bot/start.ts
📚 Learning: 2025-10-22T14:03:55.114Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Applies to bot/** : Place commands, scenes, and middleware modules under bot/

Applied to files:

  • bot/start.ts
📚 Learning: 2025-10-22T14:03:55.114Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Use npm start for the production path (prestart → tsc) to launch the compiled bot

Applied to files:

  • bot/start.ts
🧬 Code graph analysis (2)
bot/middleware/commandlogging.ts (4)
bot/modules/community/communityContext.ts (1)
  • CommunityContext (18-22)
bot/modules/block/index.ts (1)
  • bot (8-31)
bot/middleware/commands.ts (1)
  • ctx (3-52)
app.ts (1)
  • logger (28-57)
bot/start.ts (1)
bot/middleware/commandlogging.ts (1)
  • commandLogger (31-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: ci_to_main
🔇 Additional comments (5)
.gitignore (1)

6-7: LGTM - Log file appropriately excluded.

Correctly prevents the new command log file from being committed to version control.

bot/start.ts (2)

77-77: LGTM - Import added correctly.


196-196: Early middleware placement for command logging is correct and intentional.

The commandLogger() middleware runs first to capture all incoming updates comprehensively before any filtering or session handling occurs. This placement aligns with standard logging patterns and serves as an audit trail for all requests. There are no conflicting logging middleware in the codebase, and the middleware does not manipulate context, making early placement appropriate.

bot/middleware/commandlogging.ts (2)

56-69: LGTM - Safe callback query handling.

The try-catch around ctx.callbackQuery.message?.text correctly handles cases where the message field might be unavailable or have unexpected structure.


72-77: LGTM - Proper error handling and middleware continuation.

The error handling ensures logging failures don't break the middleware chain, and next() is always called to continue processing.

Comment on lines 8 to 29
const logger = winston.createLogger({
format: winston.format.combine(
winston.format.timestamp({
format: 'YYYY-MM-DDTHH:mm:ss.SSSZ',
}),
winston.format.colorize(),
winston.format.printf(info => {
return `[${info.timestamp}] ${info.level}: ${info.message} ${
info.stack ? info.stack : ''
}`;
}),
),
levels: winston.config.syslog.levels,
level: 'debug',
transports: [
new winston.transports.File({
filename: logFile,
maxsize: 5 * 1024 * 1024 * 1000, // 5GB
}),
],
exitOnError: false,
});
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 31, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use the existing logger and remove colorize from file transport.

Two issues here:

  1. Duplicate logger instance: The codebase already has a logger imported from '../logger'. Creating a separate Winston instance leads to inconsistent configuration and maintenance burden.

  2. colorize() in file transport: The winston.format.colorize() formatter adds ANSI escape codes that render properly in terminals but produce unreadable garbage in log files (e.g., [90m[37m).

🔎 Proposed fix

Import and use the existing logger, or if a separate file is required, remove colorize:

-import winston from 'winston';
+import { logger } from '../../logger';

-const logFile = process.env.COMMAND_LOG_FILE || 'commands.log';
-
-const logger = winston.createLogger({
-	format: winston.format.combine(
-		winston.format.timestamp({
-			format: 'YYYY-MM-DDTHH:mm:ss.SSSZ',
-		}),
-		winston.format.colorize(),
-		winston.format.printf(info => {
-			return `[${info.timestamp}] ${info.level}: ${info.message} ${
-				info.stack ? info.stack : ''
-			}`;
-		}),
-	),
-	levels: winston.config.syslog.levels,
-	level: 'debug',
-	transports: [
-		new winston.transports.File({
-			filename: logFile,
-			maxsize: 5 * 1024 * 1024 * 1000, // 5GB
-		}),
-	],
-	exitOnError: false,
-});
+// If a separate file transport is truly needed, add it to the existing logger configuration

If you must use a separate file, remove winston.format.colorize():

 const logger = winston.createLogger({
 	format: winston.format.combine(
 		winston.format.timestamp({
 			format: 'YYYY-MM-DDTHH:mm:ss.SSSZ',
 		}),
-		winston.format.colorize(),
 		winston.format.printf(info => {
 			return `[${info.timestamp}] ${info.level}: ${info.message} ${
 				info.stack ? info.stack : ''
 			}`;
 		}),
 	),

Based on learnings, "Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant."

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In bot/middleware/commandlogging.ts around lines 8 to 29, the file creates a new
Winston logger and applies winston.format.colorize() which both duplicates the
project's existing logger (imported from '../logger') and injects ANSI color
codes into file output; replace usage with the shared logger from ../logger
(remove the separate createLogger block) or, if a distinct file-only logger is
required, create it without winston.format.colorize() and match the project's
existing format/levels/transports (use same timestamp/printf format and a File
transport without colorize and with a sensible maxsize) so file logs remain
readable and configuration is consistent.

Copy link
Member

Choose a reason for hiding this comment

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

@Luquitasjeffrey porfa mira este comentario

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
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

♻️ Duplicate comments (2)
bot/middleware/commandlogging.ts (2)

7-27: Address the duplicate logger and missing rotation policy.

These issues were previously flagged but remain unresolved:

  1. Duplicate logger instance: The codebase has an existing logger at '../logger'. This separate Winston instance creates inconsistent configuration and maintenance overhead.

  2. Missing log rotation: The 5GB limit with no rotation policy (maxFiles, maxDays, or tailable) means PII data (usernames, user IDs) accumulates indefinitely in a single file.

Based on learnings, use the existing Winston logger with proper configuration.


32-53: Fix spacing in ternary operator.

Line 53 has a formatting issue: missing space after the variable in the ternary operator. isCommand? should be isCommand ? to comply with style guidelines.

🔎 Proposed fix
-				logger.info(`User @${userName} [${userId}] ${isCommand? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`);
+				logger.info(`User @${userName} [${userId}] ${isCommand ? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`);
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1aa6996 and 18ea3b8.

📒 Files selected for processing (1)
  • bot/middleware/commandlogging.ts
🧰 Additional context used
📓 Path-based instructions (3)
{bot/**,locales/**}

📄 CodeRabbit inference engine (AGENTS.md)

House commands, scenes, and middleware modules in bot/ directory and pair new flows with text updates under locales/

Files:

  • bot/middleware/commandlogging.ts
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Enforce Prettier formatting with 2-space indentation, semicolons, and single quotes; run before committing

Files:

  • bot/middleware/commandlogging.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use ESLint Standard plus TypeScript rules to guard the codebase; address warnings instead of disabling them
Use camelCase for functions and variables
Use PascalCase for class names

Files:

  • bot/middleware/commandlogging.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T12:48:44.320Z
Learning: Applies to {bot/**,locales/**} : House commands, scenes, and middleware modules in `bot/` directory and pair new flows with text updates under `locales/`
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.384Z
Learning: Applies to src/bot/middleware/**/*.ts : Apply middleware chain in this order: User validation → Admin checking → Context enhancement → Command routing
📚 Learning: 2026-01-02T12:48:28.384Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.384Z
Learning: Applies to src/bot/**/*.{ts,tsx} : Use custom context types extending Telegraf's base context (MainContext and CommunityContext) for bot command handlers and message processing

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.384Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.384Z
Learning: Applies to src/bot/middleware/**/*.ts : Apply middleware chain in this order: User validation → Admin checking → Context enhancement → Command routing

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.384Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.384Z
Learning: Applies to src/**/*.ts : Use Winston logger with configurable levels and timeout monitoring for error handling throughout the application

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:44.320Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T12:48:44.320Z
Learning: Applies to app.ts : Keep Telegram bot bootstrap and Mongo connection logic centralized in `app.ts`

Applied to files:

  • bot/middleware/commandlogging.ts
🧬 Code graph analysis (1)
bot/middleware/commandlogging.ts (1)
bot/modules/community/communityContext.ts (1)
  • CommunityContext (18-22)
🪛 GitHub Actions: Auto Check Lint
bot/middleware/commandlogging.ts

[error] 57-57: ESLint: 'msgText' is never reassigned. Use 'const' instead. (prefer-const)

🪛 GitHub Check: Lint
bot/middleware/commandlogging.ts

[failure] 57-57:
'msgText' is never reassigned. Use 'const' instead

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: ci_to_main
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
bot/middleware/commandlogging.ts (3)

29-30: LGTM on middleware signature.

Correct Telegraf middleware signature using CommunityContext and proper async pattern.


62-67: LGTM on error handling.

Appropriate try-catch wrapper prevents logging failures from breaking the middleware chain. Fallback logging provides visibility for unexpected update types.


69-69: LGTM on middleware chain continuation.

Correctly returns next() to continue the middleware chain, even if logging fails.

Copy link
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: 0

🧹 Nitpick comments (1)
.env-sample (1)

100-101: Consider adding log rotation configuration.

The current implementation writes to a single file without rotation, which can lead to unbounded disk usage and operational issues in production environments.

Recommended additions to .env-sample:

  • COMMAND_LOG_MAX_SIZE - Maximum size before rotation (e.g., '10m')
  • COMMAND_LOG_MAX_FILES - Number of rotated files to retain (e.g., '7')
  • COMMAND_LOG_RETENTION_DAYS - Automatic purge after N days (e.g., '30')

These settings would align with Winston's rotation capabilities and prevent disk exhaustion.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18ea3b8 and 442c6ac.

📒 Files selected for processing (1)
  • .env-sample
🧰 Additional context used
📓 Path-based instructions (1)
.env*

📄 CodeRabbit inference engine (AGENTS.md)

Copy .env-sample to .env; never commit actual credentials and document new variables in PRs

Files:

  • .env-sample
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T12:48:44.329Z
Learning: Applies to {bot/**,locales/**} : House commands, scenes, and middleware modules in `bot/` directory and pair new flows with text updates under `locales/`
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/bot/middleware/**/*.ts : Apply middleware chain in this order: User validation → Admin checking → Context enhancement → Command routing
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
.env-sample (2)

100-101: Variable properly documented.

The environment variable is correctly formatted with a descriptive comment and reasonable default value.


99-101: Privacy and compliance risk: logging user commands may capture PII.

Logging all user commands can capture personally identifiable information (PII) such as usernames, payment details, and other sensitive data. This raises compliance concerns under GDPR, CCPA, and similar regulations.

Consider:

  • Documenting data retention and access policies for this log file
  • Implementing log sanitization to redact sensitive fields
  • Adding a privacy notice to users that commands are logged
  • Configuring log rotation with automatic purging after a defined retention period
⛔ Skipped due to learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T12:48:44.329Z
Learning: Applies to .env* : Copy `.env-sample` to `.env`; never commit actual credentials and document new variables in PRs

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

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

@Luquitasjeffrey me ha funcionado todo bien, el unico detalle que he encontrado hasta ahora es que cuando se toma una orden se registra toda esta info de la orden, que no veo necesario pues con el order-id se puede buscar la informacion de esa orden. Ademas al dia hay muchisimas ordenes que son tomadas, algunas continuan pero otras se regresan a pending y pueden volverse a tomar, y todo eso va a ocupar mas espacio por gusto

[2026-01-07T19:33:47.331-06:00] info: User @Catrya  [7009901990] sent callback query with data: takesell. Message text: '⚡️🍊⚡️
Vendiendo 500 sats
Por 6 CUP 🇨🇺
Recibo pago por test
Tiene 225 operaciones exitosas
Usa el bot hace 1040 días
#SELLCUP
Precio: 1.200.000
5 ⭐⭐⭐⭐⭐ (234)

:695f096c830347be72b3caaa:' 

Copy link
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: 5

🤖 Fix all issues with AI agents
In @bot/middleware/commandlogging.ts:
- Line 25: The maxsize calculation uses an incorrect multiplier (1000) for bytes
conversion; update the assignment where maxsize is set (the expression
`maxSizeGB * 1024 * 1024 * 1000`) to use 1024 for all three steps (e.g.,
`maxSizeGB * 1024 * 1024 * 1024` or `maxSizeGB * Math.pow(1024, 3)`) so 1 GB
equals 1024^3 bytes; change the literal multiplier to 1024 in the expression
referenced in commandlogging.ts.
- Around line 23-26: The File transport currently only sets maxsize and will
stop writing but retain the single large file; update the transport
instantiation (the new winston.transports.File(...) in commandlogging.ts) to
enable rotation by adding rotation options such as maxFiles (e.g., 5), tailable:
true, and optionally zippedArchive: true (or switch to
winston-daily-rotate-file/DailyRotateFile if preferred) and correct maxsize
calculation to use maxSizeGB * 1024 * 1024 * 1024; this ensures old logs are
rotated/removed and optionally compressed to meet PII retention policies.
- Line 60: The current line uses an unsafe cast "(ctx.callbackQuery?.message as
any)?.text" which bypasses TypeScript checks; replace it with a safe
type-narrowing check on ctx.callbackQuery?.message (e.g., ensure the message
exists, is an object and has a 'text' property of type string) and only then
read message.text into callbackQueryMessage; reference the variable
callbackQueryMessage and the expression ctx.callbackQuery?.message when
implementing this guard so you avoid using "as any" and preserve correct typing.
- Around line 9-29: This creates a new Winston logger (winston.createLogger)
causing duplicate configuration; replace this block to use the application's
existing centralized logger instance instead of creating a new one—locate where
the global/shared logger is exported and import/consume that logger in
commandlogging.ts (remove the winston.createLogger block and any local
transports like the File transport using logFile/maxSizeGB), ensure you forward
any needed metadata (timestamp/stack) when logging from the existing logger and
preserve the intended level ('debug') and exitOnError behavior via the shared
logger configuration.
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a335ad and e6ddb33.

📒 Files selected for processing (1)
  • bot/middleware/commandlogging.ts
🧰 Additional context used
📓 Path-based instructions (3)
{bot/**,locales/**}

📄 CodeRabbit inference engine (AGENTS.md)

House commands, scenes, and middleware modules in bot/ directory and pair new flows with text updates under locales/

Files:

  • bot/middleware/commandlogging.ts
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Enforce Prettier formatting with 2-space indentation, semicolons, and single quotes; run before committing

Files:

  • bot/middleware/commandlogging.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use ESLint Standard plus TypeScript rules to guard the codebase; address warnings instead of disabling them
Use camelCase for functions and variables
Use PascalCase for class names

Files:

  • bot/middleware/commandlogging.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T12:48:44.329Z
Learning: Applies to {bot/**,locales/**} : House commands, scenes, and middleware modules in `bot/` directory and pair new flows with text updates under `locales/`
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/bot/middleware/**/*.ts : Apply middleware chain in this order: User validation → Admin checking → Context enhancement → Command routing
📚 Learning: 2026-01-02T12:48:28.393Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/bot/**/*.{ts,tsx} : Use custom context types extending Telegraf's base context (MainContext and CommunityContext) for bot command handlers and message processing

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.393Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/**/*.ts : Use Winston logger with configurable levels and timeout monitoring for error handling throughout the application

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.393Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/bot/middleware/**/*.ts : Apply middleware chain in this order: User validation → Admin checking → Context enhancement → Command routing

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.394Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.394Z
Learning: Applies to src/**/*.ts : Use try-catch blocks throughout the codebase with proper error context for exception handling

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.393Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/**/*.ts : Enable strict mode in TypeScript configuration for better type safety and use comprehensive interface definitions for all models

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:44.329Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T12:48:44.329Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use ESLint Standard plus TypeScript rules to guard the codebase; address warnings instead of disabling them

Applied to files:

  • bot/middleware/commandlogging.ts
🧬 Code graph analysis (1)
bot/middleware/commandlogging.ts (3)
bot/modules/nostr/commands.ts (1)
  • info (6-23)
bot/modules/community/communityContext.ts (1)
  • CommunityContext (18-22)
util/index.ts (1)
  • extractId (609-609)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: ci_to_main
🔇 Additional comments (3)
bot/middleware/commandlogging.ts (3)

73-78: Good error handling and middleware flow.

The try-catch block properly catches logging errors without interrupting the middleware chain, and next() is always called to ensure the bot continues processing updates even if logging fails.


61-63: Clean order ID extraction logic.

The regex pattern correctly identifies MongoDB ObjectId format (24 hex chars), and the fallback to extractId utility provides robust ID extraction. The conditional formatting clearly distinguishes between cases where an order ID is available versus raw message text.


4-4: The import path is correct as-is. The util module is located at the root level (util/index.ts), not within the bot/ directory. From bot/middleware/commandlogging.ts, the correct relative import is '../../util' (up two levels to the root, then to util/). No change needed.

Likely an incorrect or invalid review comment.

Comment on lines 9 to 29
const logger = winston.createLogger({
format: winston.format.combine(
winston.format.timestamp({
format: 'YYYY-MM-DDTHH:mm:ss.SSSZ',
}),
winston.format.printf(info => {
return `[${info.timestamp}] ${info.level}: ${info.message} ${
info.stack ? info.stack : ''
}`;
}),
),
levels: winston.config.syslog.levels,
level: 'debug',
transports: [
new winston.transports.File({
filename: logFile,
maxsize: maxSizeGB * 1024 * 1024 * 1000, // 5GB
}),
],
exitOnError: false,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Use the existing logger instance instead of creating a new one.

Creating a separate Winston logger instance leads to inconsistent configuration and maintenance burden. Per the learnings, the codebase already has a centralized logger that should be used throughout the application.

Based on learnings, "Use Winston logger with configurable levels and timeout monitoring for error handling throughout the application."

♻️ Proposed refactor
-import winston from 'winston';
+import { logger } from '../logger';
 import { extractId } from '../util';
 
-const logFile = process.env.COMMAND_LOG_FILE || 'commands.log';
-const maxSizeGB = parseInt(process.env.COMMAND_LOG_SIZE_GB || '5', 10) || 5;
-
-const logger = winston.createLogger({
-  format: winston.format.combine(
-    winston.format.timestamp({
-      format: 'YYYY-MM-DDTHH:mm:ss.SSSZ',
-    }),
-    winston.format.printf(info => {
-      return `[${info.timestamp}] ${info.level}: ${info.message} ${
-        info.stack ? info.stack : ''
-      }`;
-    }),
-  ),
-  levels: winston.config.syslog.levels,
-  level: 'debug',
-  transports: [
-    new winston.transports.File({
-      filename: logFile,
-      maxsize: maxSizeGB * 1024 * 1024 * 1000, // 5GB
-    }),
-  ],
-  exitOnError: false,
-});
+// If a separate file transport is required, add it to the existing logger configuration

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In @bot/middleware/commandlogging.ts around lines 9 - 29, This creates a new
Winston logger (winston.createLogger) causing duplicate configuration; replace
this block to use the application's existing centralized logger instance instead
of creating a new one—locate where the global/shared logger is exported and
import/consume that logger in commandlogging.ts (remove the winston.createLogger
block and any local transports like the File transport using logFile/maxSizeGB),
ensure you forward any needed metadata (timestamp/stack) when logging from the
existing logger and preserve the intended level ('debug') and exitOnError
behavior via the shared logger configuration.

Comment on lines 23 to 26
new winston.transports.File({
filename: logFile,
maxsize: maxSizeGB * 1024 * 1024 * 1000, // 5GB
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Implement log rotation to prevent indefinite PII retention.

The file transport has no rotation policy (maxFiles, tailable, etc.). Once the 5GB limit is reached, logging stops but the file persists indefinitely with usernames and user IDs. You need to implement log rotation to comply with data retention best practices.

♻️ Recommended rotation configuration
     new winston.transports.File({
       filename: logFile,
-      maxsize: maxSizeGB * 1024 * 1024 * 1000, // 5GB
+      maxsize: maxSizeGB * 1024 * 1024 * 1024,
+      maxFiles: 5, // Keep only 5 rotated files
+      tailable: true, // Rotate when size limit is reached
     }),
📝 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
new winston.transports.File({
filename: logFile,
maxsize: maxSizeGB * 1024 * 1024 * 1000, // 5GB
}),
new winston.transports.File({
filename: logFile,
maxsize: maxSizeGB * 1024 * 1024 * 1024,
maxFiles: 5, // Keep only 5 rotated files
tailable: true, // Rotate when size limit is reached
}),
🤖 Prompt for AI Agents
In @bot/middleware/commandlogging.ts around lines 23 - 26, The File transport
currently only sets maxsize and will stop writing but retain the single large
file; update the transport instantiation (the new winston.transports.File(...)
in commandlogging.ts) to enable rotation by adding rotation options such as
maxFiles (e.g., 5), tailable: true, and optionally zippedArchive: true (or
switch to winston-daily-rotate-file/DailyRotateFile if preferred) and correct
maxsize calculation to use maxSizeGB * 1024 * 1024 * 1024; this ensures old logs
are rotated/removed and optionally compressed to meet PII retention policies.

Comment on lines 34 to 69
if (ctx.message && 'text' in ctx.message) {
const msg = ctx.message;
const text = msg.text.trim();
const userId = msg.from?.id ?? 'unknown';

let command: string | null = null;
let args: string[] = [];
let isCommand: boolean;

if (text.startsWith('/')) {
const parts = text.split(/\s+/);
command = parts[0];
args = parts.slice(1);
isCommand = true;
} else {
isCommand = false;
command = text;
}

const userName = msg.from?.username ?? '';

logger.info(
`User @${userName} [${userId}] ${isCommand ? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`,
);
} else if (ctx.callbackQuery && 'data' in ctx.callbackQuery) {
// Attempt to get message text
const callbackQueryMessage = (ctx.callbackQuery?.message as any)?.text ?? '';
const isId = /^[a-f0-9]{24}$/.test(callbackQueryMessage);
const orderId = isId ? callbackQueryMessage : extractId(callbackQueryMessage);
const msgText = orderId ? `Order ID: ${orderId}` : `Message text: '${callbackQueryMessage}'`;
const callbackData = ctx.callbackQuery.data;
const userName = ctx.callbackQuery.from?.username ?? '';
const userId = ctx.callbackQuery.from?.id ?? '';
logger.info(
`User @${userName} [${userId}] sent callback query with data: ${callbackData}. '${msgText}'`,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Document PII logging and ensure compliance with privacy policy.

The middleware logs Telegram usernames (lines 53, 65) and user IDs (lines 37, 66) for every command and callback query. This constitutes logging of personally identifiable information (PII). Without documented retention policies and user consent, this may violate privacy regulations.

You should:

  1. Document what PII is being logged and why
  2. Establish and document a retention policy
  3. Consider masking or hashing identifiers if full values aren't required for debugging
  4. Ensure this logging practice is covered in your privacy policy

Copy link
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: 3

🤖 Fix all issues with AI agents
In @bot/middleware/commandlogging.ts:
- Around line 34-57: The middleware currently logs every message text (via
ctx.message/text and variable command) which exposes user PII; change the logic
so only actual commands are logged: ensure command and args are only set when
text.startsWith('/') (keep command = parts[0] and args = parts.slice(1) inside
that branch) and move the logger.info call to run only when isCommand === true
(do not set command = text for non-commands and do not log non-command texts);
reference ctx.message, text, isCommand, command, args, and logger.info when
making these changes.
- Around line 61-62: The line using (ctx.callbackQuery?.message as any)?.text
bypasses TypeScript safety; replace the cast by narrowing the type of
ctx.callbackQuery?.message before accessing text—either add a type guard
function (e.g., isMessageWithText(msg): msg is Message & { text: string }) and
use it when computing callbackQueryMessage, or check for the presence and type
of the text property (e.g., ensure ctx.callbackQuery?.message is defined and has
a string 'text' field) and then read .text into callbackQueryMessage; update the
code around callbackQueryMessage to use the safe-checked type instead of as any.
- Around line 58-75: Add an inline comment above the callback-query handling
(the block that reads ctx.callbackQuery, callbackQueryMessage, isId, orderId,
extractId and ultimately calls logger.info) that documents the expected
callback_data patterns (e.g., short language codes like "en", currency tags like
"USD", action_id style tokens, and order/community IDs), note that
order/community IDs derived via extractId are already present in user-visible
message text, and explicitly list any sensitive patterns that must never be
logged (e.g., full PII, auth tokens, or payment data) so future changes to
callback data structures avoid logging those values.
🧹 Nitpick comments (7)
bot/middleware/commandlogging.ts (7)

6-7: Verify environment variable handling.

The parseInt fallback handles invalid inputs correctly, but consider validating that COMMAND_LOG_SIZE_GB is a reasonable value (e.g., between 1-100 GB) to prevent misconfigurations.

📝 Optional validation enhancement
-const maxSizeGB = parseInt(process.env.COMMAND_LOG_SIZE_GB || '5', 10) || 5;
+const parsedSize = parseInt(process.env.COMMAND_LOG_SIZE_GB || '5', 10) || 5;
+const maxSizeGB = Math.min(Math.max(parsedSize, 1), 100); // Clamp between 1-100 GB

23-26: Add log rotation to prevent unbounded file growth.

Without rotation, the log file will grow until it hits 5GB, then stop accepting new logs. This could lead to lost log data and disk space issues. Winston supports rotation via winston-daily-rotate-file or the maxFiles option.

♻️ Add log rotation configuration

Install the rotation package:

npm install winston-daily-rotate-file

Then update the transport:

+import DailyRotateFile from 'winston-daily-rotate-file';
+
 const logger = winston.createLogger({
   format: winston.format.combine(
     winston.format.timestamp({
       format: 'YYYY-MM-DDTHH:mm:ss.SSSZ',
     }),
     winston.format.printf(info => {
       return `[${info.timestamp}] ${info.level}: ${info.message} ${
         info.stack ? info.stack : ''
       }`;
     }),
   ),
   levels: winston.config.syslog.levels,
   level: 'debug',
   transports: [
-    new winston.transports.File({
+    new DailyRotateFile({
       filename: logFile,
-      maxsize: maxSizeGB * 1024 ** 3, // 5GB
+      maxSize: `${maxSizeGB}g`,
+      maxFiles: '14d', // Keep logs for 14 days
+      datePattern: 'YYYY-MM-DD',
     }),
   ],
   exitOnError: false,
 });

25-25: Update comment to reflect configurable value.

The comment hardcodes "5GB" but the actual value is configurable via maxSizeGB.

📝 Clarify comment
-      maxsize: maxSizeGB * 1024 ** 3, // 5GB
+      maxsize: maxSizeGB * 1024 ** 3, // Convert GB to bytes

37-37: Inconsistent fallback values for missing user data.

Line 37 uses 'unknown' for missing userId, while Line 53 uses '' (empty string) for missing userName. Consider using consistent fallback values for better log readability.

📝 Suggested consistency fix
        const userId = msg.from?.id ?? 'unknown';
        
        // ... other code ...
        
-        const userName = msg.from?.username ?? '';
+        const userName = msg.from?.username ?? 'unknown';

Also applies to: 53-53


63-69: Simplify order ID extraction logic.

The nested ternary and dual extraction approach (regex test + extractId utility) is complex and hard to maintain.

♻️ Simplify orderId extraction
-        const isId = /^[a-f0-9]{24}$/.test(callbackQueryMessage);
-        const orderId = isId
-          ? callbackQueryMessage
-          : extractId(callbackQueryMessage);
+        const orderId = /^[a-f0-9]{24}$/.test(callbackQueryMessage)
+          ? callbackQueryMessage
+          : extractId(callbackQueryMessage);
+          
        const msgText = orderId
          ? `Order ID: ${orderId}`
          : `Message text: '${callbackQueryMessage}'`;

This removes the intermediate isId variable that's only used once.


76-78: Enhance fallback logging for better observability.

The generic fallback message doesn't provide useful context about the update type. Consider logging the update type for better debugging.

📝 Improve fallback logging
      } else {
-        logger.info(`Received non-command message or update from user.`);
+        const updateType = Object.keys(ctx.update)[0];
+        logger.info(`Received update of type: ${updateType}`);
      }

31-85: Consider performance impact of file I/O on every update.

This middleware performs synchronous file I/O on every bot update, which could impact response times under high load. Winston's File transport writes are generally async, but consider:

  • Adding performance monitoring to measure actual impact
  • Implementing sampling (e.g., log every Nth message) for very high-volume bots
  • Using a more performant logging backend (e.g., streaming to a logging service) for production

If the bot handles high message volumes, measure the middleware's latency impact in production or staging environments.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f095730 and a8df966.

📒 Files selected for processing (1)
  • bot/middleware/commandlogging.ts
🧰 Additional context used
📓 Path-based instructions (3)
{bot/**,locales/**}

📄 CodeRabbit inference engine (AGENTS.md)

House commands, scenes, and middleware modules in bot/ directory and pair new flows with text updates under locales/

Files:

  • bot/middleware/commandlogging.ts
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Enforce Prettier formatting with 2-space indentation, semicolons, and single quotes; run before committing

Files:

  • bot/middleware/commandlogging.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use ESLint Standard plus TypeScript rules to guard the codebase; address warnings instead of disabling them
Use camelCase for functions and variables
Use PascalCase for class names

Files:

  • bot/middleware/commandlogging.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T12:48:44.329Z
Learning: Applies to {bot/**,locales/**} : House commands, scenes, and middleware modules in `bot/` directory and pair new flows with text updates under `locales/`
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/bot/middleware/**/*.ts : Apply middleware chain in this order: User validation → Admin checking → Context enhancement → Command routing
📚 Learning: 2026-01-02T12:48:28.393Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/bot/**/*.{ts,tsx} : Use custom context types extending Telegraf's base context (MainContext and CommunityContext) for bot command handlers and message processing

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.393Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/bot/middleware/**/*.ts : Apply middleware chain in this order: User validation → Admin checking → Context enhancement → Command routing

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.393Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/**/*.ts : Use Winston logger with configurable levels and timeout monitoring for error handling throughout the application

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.394Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.394Z
Learning: Applies to src/**/*.ts : Use try-catch blocks throughout the codebase with proper error context for exception handling

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:28.393Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/**/*.ts : Enable strict mode in TypeScript configuration for better type safety and use comprehensive interface definitions for all models

Applied to files:

  • bot/middleware/commandlogging.ts
📚 Learning: 2026-01-02T12:48:44.329Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T12:48:44.329Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use ESLint Standard plus TypeScript rules to guard the codebase; address warnings instead of disabling them

Applied to files:

  • bot/middleware/commandlogging.ts
🧬 Code graph analysis (1)
bot/middleware/commandlogging.ts (2)
bot/modules/community/communityContext.ts (1)
  • CommunityContext (18-22)
util/index.ts (1)
  • extractId (609-609)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: ci_to_main
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
bot/middleware/commandlogging.ts (1)

79-81: Improve error logging format.

The current error logging may not properly capture the error details. Winston expects error objects to be passed differently for proper formatting.

🔧 Fix error logging
    } catch (err) {
-      logger.error('logging middleware failed', err);
+      logger.error('Logging middleware failed', { error: err instanceof Error ? err.message : String(err), stack: err instanceof Error ? err.stack : undefined });
    }
⛔ Skipped due to learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T12:48:28.393Z
Learning: Applies to src/**/*.ts : Use Winston logger with configurable levels and timeout monitoring for error handling throughout the application

Comment on lines +34 to +57
if (ctx.message && 'text' in ctx.message) {
const msg = ctx.message;
const text = msg.text.trim();
const userId = msg.from?.id ?? 'unknown';

let command: string | null = null;
let args: string[] = [];
let isCommand: boolean;

if (text.startsWith('/')) {
const parts = text.split(/\s+/);
command = parts[0];
args = parts.slice(1);
isCommand = true;
} else {
isCommand = false;
command = text;
}

const userName = msg.from?.username ?? '';

logger.info(
`User @${userName} [${userId}] ${isCommand ? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Critical privacy concern: logging all user messages, not just commands.

Lines 48-50 log the full text of non-command messages. This creates significant privacy and compliance risks:

  • Logs may contain PII (names, phone numbers, addresses, financial info)
  • Could violate GDPR, CCPA, or other privacy regulations
  • Exposes private user conversations to log files

If the intent is to log only commands (as the filename and PR title suggest), non-command messages should be excluded. If logging all messages is intentional, this requires clear documentation, user consent, data retention policies, and potentially data masking.

🔒 Recommended fix: Log commands only
      if (ctx.message && 'text' in ctx.message) {
        const msg = ctx.message;
        const text = msg.text.trim();
        const userId = msg.from?.id ?? 'unknown';

        let command: string | null = null;
        let args: string[] = [];
-        let isCommand: boolean;

        if (text.startsWith('/')) {
          const parts = text.split(/\s+/);
          command = parts[0];
          args = parts.slice(1);
-          isCommand = true;
-        } else {
-          isCommand = false;
-          command = text;
-        }

-        const userName = msg.from?.username ?? '';
+          const userName = msg.from?.username ?? '';

-        logger.info(
-          `User @${userName} [${userId}] ${isCommand ? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`,
-        );
+          logger.info(
+            `User @${userName} [${userId}] executed command: ${command} with args: [${args.join(', ')}]`,
+          );
+        }
+        // Non-command messages are not logged for privacy reasons

Alternatively, if logging all messages is required, add clear warnings and implement data masking for sensitive patterns.

📝 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
if (ctx.message && 'text' in ctx.message) {
const msg = ctx.message;
const text = msg.text.trim();
const userId = msg.from?.id ?? 'unknown';
let command: string | null = null;
let args: string[] = [];
let isCommand: boolean;
if (text.startsWith('/')) {
const parts = text.split(/\s+/);
command = parts[0];
args = parts.slice(1);
isCommand = true;
} else {
isCommand = false;
command = text;
}
const userName = msg.from?.username ?? '';
logger.info(
`User @${userName} [${userId}] ${isCommand ? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`,
);
if (ctx.message && 'text' in ctx.message) {
const msg = ctx.message;
const text = msg.text.trim();
const userId = msg.from?.id ?? 'unknown';
let command: string | null = null;
let args: string[] = [];
if (text.startsWith('/')) {
const parts = text.split(/\s+/);
command = parts[0];
args = parts.slice(1);
const userName = msg.from?.username ?? '';
logger.info(
`User @${userName} [${userId}] executed command: ${command} with args: [${args.join(', ')}]`,
);
}
// Non-command messages are not logged for privacy reasons
🤖 Prompt for AI Agents
In @bot/middleware/commandlogging.ts around lines 34 - 57, The middleware
currently logs every message text (via ctx.message/text and variable command)
which exposes user PII; change the logic so only actual commands are logged:
ensure command and args are only set when text.startsWith('/') (keep command =
parts[0] and args = parts.slice(1) inside that branch) and move the logger.info
call to run only when isCommand === true (do not set command = text for
non-commands and do not log non-command texts); reference ctx.message, text,
isCommand, command, args, and logger.info when making these changes.

Comment on lines +58 to +75
} else if (ctx.callbackQuery && 'data' in ctx.callbackQuery) {
// Attempt to get message text

const callbackQueryMessage =
(ctx.callbackQuery?.message as any)?.text ?? '';
const isId = /^[a-f0-9]{24}$/.test(callbackQueryMessage);
const orderId = isId
? callbackQueryMessage
: extractId(callbackQueryMessage);
const msgText = orderId
? `Order ID: ${orderId}`
: `Message text: '${callbackQueryMessage}'`;
const callbackData = ctx.callbackQuery.data;
const userName = ctx.callbackQuery.from?.username ?? '';
const userId = ctx.callbackQuery.from?.id ?? '';
logger.info(
`User @${userName} [${userId}] sent callback query with data: ${callbackData}. '${msgText}'`,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for callback_data patterns to understand what data is being sent
rg -nP --type=ts -C3 'callback_data|callbackQuery.*data' -g '!**/node_modules/**' -g '!**/test/**'

Repository: lnp2pBot/bot

Length of output: 16178


🏁 Script executed:

#!/bin/bash
# Search for sanitization, masking, or filtering functions that might be handling sensitive data
rg -nP --type=ts -C2 'sanitiz|mask|encrypt|redact|filter.*data' -g '!**/node_modules/**'

Repository: lnp2pBot/bot

Length of output: 5538


🏁 Script executed:

#!/bin/bash
# Check what types of callback data are defined and used in the bot context
rg -nP --type=ts 'ctx\.callbackQuery' -C2 -g '!**/node_modules/**'

Repository: lnp2pBot/bot

Length of output: 2957


🏁 Script executed:

#!/bin/bash
# Look for order-related callback data or similar patterns
rg -nP --type=ts 'callback.*order|order.*callback|btn.*data|button.*data' -g '!**/node_modules/**'

Repository: lnp2pBot/bot

Length of output: 930


Document what types of callback data are logged.

The logging code captures raw callback data without documenting what patterns are expected. While the current patterns (language codes, currency strings, order/community IDs) are not inherently sensitive, explicitly documenting the types of data logged helps maintainers understand privacy implications of future changes to callback data structures.

Add a comment clarifying:

  • What types of callback_data patterns are expected (e.g., action_id format)
  • That order/community IDs logged here are already visible to users in message text
  • Any data patterns that should be excluded from logging in the future
🤖 Prompt for AI Agents
In @bot/middleware/commandlogging.ts around lines 58 - 75, Add an inline comment
above the callback-query handling (the block that reads ctx.callbackQuery,
callbackQueryMessage, isId, orderId, extractId and ultimately calls logger.info)
that documents the expected callback_data patterns (e.g., short language codes
like "en", currency tags like "USD", action_id style tokens, and order/community
IDs), note that order/community IDs derived via extractId are already present in
user-visible message text, and explicitly list any sensitive patterns that must
never be logged (e.g., full PII, auth tokens, or payment data) so future changes
to callback data structures avoid logging those values.

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