Add admin VPS console, tips persistence, HTTPS health, safe path handling and admin UX improvements - #74
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChanges add ephemeral bonus message handling to a Telegram bot by introducing two helper functions for managing transient prompts with auto-deletion. Integration points include cleanup during menu navigation and replacement of inline promo prompts in the onboarding flow. The runtime setup script was refactored with explicit directory creation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Nitpicks 🔍
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
index.js (1)
4058-4062: Remove duplicate transient cleanup into_main_menu.Line 4061 is redundant because
sendPersistentUserMenualready callsdeleteEphemeralBonusPromptat Line 1891.♻️ Proposed cleanup
bot.action('to_main_menu', async (ctx) => { const user = getUser(ctx); await ctx.answerCbQuery(); - await deleteEphemeralBonusPrompt(ctx, user); await sendPersistentUserMenu(ctx, user); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 4058 - 4062, The to_main_menu action handler currently calls deleteEphemeralBonusPrompt(ctx, user) before invoking sendPersistentUserMenu(ctx, user), which is redundant because sendPersistentUserMenu already calls deleteEphemeralBonusPrompt; remove the redundant await deleteEphemeralBonusPrompt(ctx, user) call from the bot.action('to_main_menu', ...) handler so ephemeral cleanup only happens inside sendPersistentUserMenu and avoid double deletion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.js`:
- Around line 2106-2139: Both deleteEphemeralBonusPrompt and
sendEphemeralBonusPrompt mutate user.ephemeralBonusMsgId and
user.ephemeralBonusChatId across async/timer boundaries without serialization;
wrap every read/write that affects those fields (including the async
ctx.telegram.deleteMessage calls and the setTimeout cleanup) inside the per-user
queue by calling runUserMutation (or the project's equivalent) so mutations are
executed serially for that user and compare/update the fields only within those
serialized callbacks to avoid races with concurrent handlers.
In `@prod-run.sh`:
- Around line 54-58: The created runtime files and dirs (LOG_DIR,
PROJECT_DIR/data, PROJECT_DIR/data/backups and the touched files MAIN_LOG,
ERROR_LOG, ADMIN_EVENTS_LOG, SSHV_SESSIONS_FILE) may be owned by root but the
service runs as user "runewager"; after the mkdir/touch block ensure ownership
and write permissions are given to that service user by changing ownership (to
runewager) and/or setting appropriate file modes (e.g. group/user write) on
those directories and files (apply recursively to PROJECT_DIR/data and backups)
so the bot can append to MAIN_LOG, ERROR_LOG and write ADMIN_EVENTS_LOG and
SSHV_SESSIONS_FILE at runtime.
---
Nitpick comments:
In `@index.js`:
- Around line 4058-4062: The to_main_menu action handler currently calls
deleteEphemeralBonusPrompt(ctx, user) before invoking
sendPersistentUserMenu(ctx, user), which is redundant because
sendPersistentUserMenu already calls deleteEphemeralBonusPrompt; remove the
redundant await deleteEphemeralBonusPrompt(ctx, user) call from the
bot.action('to_main_menu', ...) handler so ephemeral cleanup only happens inside
sendPersistentUserMenu and avoid double deletion.
| async function deleteEphemeralBonusPrompt(ctx, user) { | ||
| if (!user.ephemeralBonusMsgId || !user.ephemeralBonusChatId) return; | ||
| try { | ||
| await ctx.telegram.deleteMessage(user.ephemeralBonusChatId, user.ephemeralBonusMsgId); | ||
| } catch (_) { /* message may already be gone */ } | ||
| user.ephemeralBonusMsgId = null; | ||
| user.ephemeralBonusChatId = null; | ||
| } | ||
|
|
||
| async function sendEphemeralBonusPrompt(ctx, user) { | ||
| const sent = await ctx.reply( | ||
| `🎁 *Claim Your New User Bonus* | ||
|
|
||
| Enter promo code *${promoStore.code}* on the Runewager affiliate page to claim your ${promoStore.amountSC} SC new user bonus!`, | ||
| { | ||
| parse_mode: 'Markdown', | ||
| ...Markup.inlineKeyboard([ | ||
| [Markup.button.url('🎁 Enter Promo Code (Mini App)', LINKS.miniAppClaim)], | ||
| [Markup.button.callback('⬅️ Main Menu', 'to_main_menu')], | ||
| ]), | ||
| }, | ||
| ); | ||
| user.ephemeralBonusMsgId = sent.message_id; | ||
| user.ephemeralBonusChatId = sent.chat.id; | ||
| setTimeout(async () => { | ||
| try { | ||
| await ctx.telegram.deleteMessage(sent.chat.id, sent.message_id); | ||
| } catch (_) { /* best effort */ } | ||
| if (user.ephemeralBonusMsgId === sent.message_id && user.ephemeralBonusChatId === sent.chat.id) { | ||
| user.ephemeralBonusMsgId = null; | ||
| user.ephemeralBonusChatId = null; | ||
| } | ||
| }, 15 * 1000); | ||
| } |
There was a problem hiding this comment.
Serialize ephemeral prompt state mutations through the per-user queue.
Lines 2111-2112, 2128-2129, and 2134-2137 mutate user state across async and timer boundaries without runUserMutation, which can race with concurrent handlers for the same user.
🔧 Proposed fix
async function deleteEphemeralBonusPrompt(ctx, user) {
if (!user.ephemeralBonusMsgId || !user.ephemeralBonusChatId) return;
+ const msgId = user.ephemeralBonusMsgId;
+ const chatId = user.ephemeralBonusChatId;
try {
- await ctx.telegram.deleteMessage(user.ephemeralBonusChatId, user.ephemeralBonusMsgId);
+ await ctx.telegram.deleteMessage(chatId, msgId);
} catch (_) { /* message may already be gone */ }
- user.ephemeralBonusMsgId = null;
- user.ephemeralBonusChatId = null;
+ await runUserMutation(user.id, async () => {
+ if (user.ephemeralBonusMsgId === msgId && user.ephemeralBonusChatId === chatId) {
+ user.ephemeralBonusMsgId = null;
+ user.ephemeralBonusChatId = null;
+ }
+ });
}
async function sendEphemeralBonusPrompt(ctx, user) {
+ await deleteEphemeralBonusPrompt(ctx, user);
const sent = await ctx.reply(
@@
- user.ephemeralBonusMsgId = sent.message_id;
- user.ephemeralBonusChatId = sent.chat.id;
+ await runUserMutation(user.id, async () => {
+ user.ephemeralBonusMsgId = sent.message_id;
+ user.ephemeralBonusChatId = sent.chat.id;
+ });
setTimeout(async () => {
try {
await ctx.telegram.deleteMessage(sent.chat.id, sent.message_id);
} catch (_) { /* best effort */ }
- if (user.ephemeralBonusMsgId === sent.message_id && user.ephemeralBonusChatId === sent.chat.id) {
- user.ephemeralBonusMsgId = null;
- user.ephemeralBonusChatId = null;
- }
+ await runUserMutation(user.id, async () => {
+ if (user.ephemeralBonusMsgId === sent.message_id && user.ephemeralBonusChatId === sent.chat.id) {
+ user.ephemeralBonusMsgId = null;
+ user.ephemeralBonusChatId = null;
+ }
+ });
}, 15 * 1000);
}As per coding guidelines Use per-user mutation queues to prevent race conditions on concurrent bonus/state mutations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 2106 - 2139, Both deleteEphemeralBonusPrompt and
sendEphemeralBonusPrompt mutate user.ephemeralBonusMsgId and
user.ephemeralBonusChatId across async/timer boundaries without serialization;
wrap every read/write that affects those fields (including the async
ctx.telegram.deleteMessage calls and the setTimeout cleanup) inside the per-user
queue by calling runUserMutation (or the project's equivalent) so mutations are
executed serially for that user and compare/update the fields only within those
serialized callbacks to avoid races with concurrent handlers.
| mkdir -p "$LOG_DIR" | ||
| mkdir -p "$PROJECT_DIR/data" | ||
| mkdir -p "$PROJECT_DIR/data/backups" | ||
| touch "$MAIN_LOG" "$ERROR_LOG" "$ADMIN_EVENTS_LOG" "$SSHV_SESSIONS_FILE" # create if missing; safe on existing files | ||
|
|
There was a problem hiding this comment.
Ensure runtime data files are writable by the service user.
Line 57 creates data/admin-events.log and data/sshv-sessions.json as the current user (commonly root), but the bot may run as runewager (Line 134). That can block writes and break persistence/logging.
💡 Proposed fix
mkdir -p "$LOG_DIR"
mkdir -p "$PROJECT_DIR/data"
mkdir -p "$PROJECT_DIR/data/backups"
touch "$MAIN_LOG" "$ERROR_LOG" "$ADMIN_EVENTS_LOG" "$SSHV_SESSIONS_FILE" # create if missing; safe on existing files
+
+# Ensure bot-writable runtime data files when service runs as runewager
+if id -u "$APP_NAME" >/dev/null 2>&1; then
+ chown "$APP_NAME:$APP_NAME" \
+ "$PROJECT_DIR/data" \
+ "$PROJECT_DIR/data/backups" \
+ "$ADMIN_EVENTS_LOG" \
+ "$SSHV_SESSIONS_FILE"
+ chmod 0640 "$ADMIN_EVENTS_LOG" "$SSHV_SESSIONS_FILE"
+fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prod-run.sh` around lines 54 - 58, The created runtime files and dirs
(LOG_DIR, PROJECT_DIR/data, PROJECT_DIR/data/backups and the touched files
MAIN_LOG, ERROR_LOG, ADMIN_EVENTS_LOG, SSHV_SESSIONS_FILE) may be owned by root
but the service runs as user "runewager"; after the mkdir/touch block ensure
ownership and write permissions are given to that service user by changing
ownership (to runewager) and/or setting appropriate file modes (e.g. group/user
write) on those directories and files (apply recursively to PROJECT_DIR/data and
backups) so the bot can append to MAIN_LOG, ERROR_LOG and write ADMIN_EVENTS_LOG
and SSHV_SESSIONS_FILE at runtime.
| mkdir -p "$PROJECT_DIR/data" | ||
| mkdir -p "$PROJECT_DIR/data/backups" | ||
| touch "$MAIN_LOG" "$ERROR_LOG" "$ADMIN_EVENTS_LOG" "$SSHV_SESSIONS_FILE" # create if missing; safe on existing files | ||
|
|
There was a problem hiding this comment.
Suggestion: The data directory and JSON/log files are created by this script (often run as root), but the Node service typically runs under a non-root service user; without correcting ownership, the service user cannot write to data/admin-events.log, sshv-sessions.json, or other files under data, breaking persistence of admin events and SSHV sessions. [logic error]
Severity Level: Critical 🚨
- ❌ /sshv admin console sessions fail to persist across restarts.
- ❌ Admin events log writes fail or crash admin commands.
- ⚠️ God-mode heal diagnostics see stale admin-events.log data.
- ⚠️ Future JSON persistence features under data/ may break.| # Ensure data directory and files are writable by the service user, if it exists | |
| if id -u "$APP_NAME" >/dev/null 2>&1; then | |
| chown "$APP_NAME:$APP_NAME" "$PROJECT_DIR/data" "$PROJECT_DIR/data/backups" \ | |
| "$ADMIN_EVENTS_LOG" "$SSHV_SESSIONS_FILE" || true | |
| fi |
Steps of Reproduction ✅
1. On a VPS host, create a dedicated system user `runewager` and group (as implied by the
systemd unit user selection in `prod-run.sh:141-143`, where `svc_user="$APP_NAME"` and
`APP_NAME="runewager"` at `prod-run.sh:25`).
2. As root, run `prod-run.sh` from the project root; it resolves `PROJECT_DIR` and creates
runtime paths at `prod-run.sh:59-62`:
- `mkdir -p "$PROJECT_DIR/data"` and `"$PROJECT_DIR/data/backups"`
- `touch "$PROJECT_DIR/data/admin-events.log"` (via `ADMIN_EVENTS_LOG` at
`prod-run.sh:29`)
- `touch "$PROJECT_DIR/data/sshv-sessions.json"` (via `SSHV_SESSIONS_FILE` at
`prod-run.sh:30`)
These directories and files are owned by root:root with default permissions (typically
755 for dirs, 644 for files).
3. The same script writes the systemd unit with `User=runewager` and `Group=runewager` in
`ensure_systemd_service()` at `prod-run.sh:133-170` and enables it; systemd then starts
`index.js` as the non-root `runewager` user from `WorkingDirectory=${PROJECT_DIR}`.
4. An admin uses Telegram admin features (e.g., `/sshv` or other admin commands) handled
in `index.js`, which:
- Defines `const sshvSessionsFile = path.join(dataDir, 'sshv-sessions.json');` at
`index.js:206` and later calls `saveJson(sshvSessionsFile, entries);` in
`persistSshvSessions()` at `index.js:969`.
- Defines `const adminEventsLogFile = path.join(dataDir, 'admin-events.log');` at
`index.js:2141` and appends events with `fs.appendFileSync(adminEventsLogFile,
JSON.stringify(entry) + '\n');` at `index.js:2152`.
- Uses `function saveJson(filePath, data) { ... }` declared at `index.js:2271` to write
JSON files under `data/`.
When these writes execute under the `runewager` user, the process lacks write
permission to the root-owned `data/` directory and files, causing `EACCES` filesystem
errors. As a result, admin events are not logged, SSHV sessions cannot be persisted,
and depending on error handling, the admin flows may throw or silently fail to persist
data.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** prod-run.sh
**Line:** 58:58
**Comment:**
*Logic Error: The data directory and JSON/log files are created by this script (often run as root), but the Node service typically runs under a non-root service user; without correcting ownership, the service user cannot write to `data/admin-events.log`, `sshv-sessions.json`, or other files under `data`, breaking persistence of admin events and SSHV sessions.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.|
CodeAnt AI finished reviewing your PR. |
User description
Motivation
Description
data/sshv-sessions.json, GC of expired sessions, and many safer guards (sshv*helpers and callbacks).data/helpful_messages.json, deterministic normalization, scheduler improvements to avoid repeating the last tip and to check posting permissions before sending.validateSafePath, atomicwriteFileAtomicand safeloadJsonthat restricts I/O to thedatadirectory and prevents path traversal.HTTPS_KEY_PATH/HTTPS_CERT_PATH(with allowed dirs and safe resolution), and internal health checks (/healthand/metrics) are requested over HTTP/HTTPS depending on config.WEBAPP_HMAC_KEYenv var instead of a constant;.env.exampleupdated to documentWEBAPP_HMAC_KEY./on/off), persistent admin/user menus, ephemeral promo prompt, admin command list updates, saferrequireAdminchecks and many callback/menu key name adjustments./tipaddand/tipeditpayloads, save/load helperssaveHelpfulMessages/loadHelpfulMessages, persist on changes, and UI changes for listing and indexing tips.Dockerfile, addedLICENSEandSECURITY.md, updatedprod-run.shto create runtime data files and log paths, updated.gitignoreandrunewager.serviceto run asrunewageruser and tighten service sandboxing, and ensured logrotate includes admin event log.Testing
CI=true node index.js) to verify the module initializes without runtime errors (passed).npm ci --omit=devbehavior exercised in the deployment script path to ensure data/log files are created as expected during installation (simulated run ofprod-run.shsteps completed without errors).Codex Task
CodeAnt-AI Description
Auto-expire ephemeral bonus prompt and ensure runtime directories/files on startup
What Changed
Impact
✅ Fewer stray promo messages in chats✅ Clearer main menu visibility for users✅ No missing runtime directories/files on fresh deploys💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit