Claude/plan v3 deployment vp mpw - #111
Conversation
…urity, 23 Block 1 fixes Menu lifecycle overhaul: - replyMenu() TTL auto-vanish via ttlMs parameter - clearStaleMenuIds() on startup (24h threshold) - mainMenuSentAt / adminMenuSentAt timestamps on user schema - sendHelpMenu() fixed: bare ctx.reply() → replyMenu() (panel stacking bug) - replaceCallbackPanel() fallback now tracks message ID in user state - Admin mode toggle double-fire fixed Feature upgrades: - User giveaway list: sendUserGiveawaysPage() 5/page + 2-min auto-vanish - Admin giveaway panel: activeGiveawaysKeyboard(page) 5/page pagination - pmenu_referral sub-menu with share deep-link button - Admin stats: active window indicator + Refresh button - Admin health panel: inline memory/errors/users/persist-age/uptime - Broadcast builder: Preview button sends to admin DM before mass send Block 1 security & logic fixes: - getStartAppLink(): encodeURIComponent + regex whitelist - referralShareHTML(): escapeHtml() on code param - unwrapTelegramUrl(): safe fallback, scheme whitelist, www.t.me support - getDiscordLink(): null when not configured (no hardcoded fallback) - evaluatePendingActionTimeout(): >= boundary, NaN guard, no createdAt mutation - getPlayLink(): legacy user.playMode fallback restored - Weighted winner pool: splice-after-pick guarantees termination - deploy_status + logs: exec() → execFile() (no shell spawn) - SSHV: rejects null bytes, backticks, $( and ${ before exec - Startup env warnings: ADMIN_IDS, TELEGRAM_CHANNEL_ID, TELEGRAM_GROUP_ID Centralized helpers: computeParticipantWeight(), getRealGiveaways(), isNewUserPromoEligible() Metrics: added runewager_menu_stale_recoveries, pending_actions_timed_out, uptime_seconds load_tooltips.sh: full rewrite — atomic write, --push/--pull flags, parameterized REPO_DIR, HTML-safe tooltips, --dry-run mode, permission checks, guarded git ops Tests: readdirSync wrapped in try/catch; isCatchAllRegexPattern expanded; extractCommandHandlerNames supports let/var/no-semicolon Infrastructure: pre-deploy-checks gate 3b (menu symbols), CHANGELOG.md created, package.json bumped to 3.0.0 All 60 tests pass. node --check clean. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
… during PR merges Restores 12 categories of codex changes that were incorrectly overwritten by "current" (main) when resolving merge conflicts across 6 merge commits. Security / TLS hardening (index.js): - Restore resolveTlsCertPathIfAllowed(): multi-directory fallback for TLS certs (PROJECT_DIR, certs/, /etc/ssl, /etc/letsencrypt) — was collapsed to PROJECT_DIR only - Restore isHealthTlsEnabled() to use resolveTlsCertPathIfAllowed() - Restore requestHealthPayload(): centralized HTTP/HTTPS health fetcher with dynamic protocol detection — was inlined with hardcoded https in two places - Update /health command to use requestHealthPayload() (shows protocol label) - Update pamenu_tools_health to use requestHealthPayload() - Update startHealthServer() TLS cert read to use resolveTlsCertPathIfAllowed() Command injection protection (index.js): - commandNeedsConfirmation(): restore pipe-to-bash/zsh detection, destructive-cmd-piped pattern, redirect pattern — main simplified to only catch "sh" - commandBlocked(): restore explicit pipe-to-shell blocker pattern Race condition fixes (index.js): - deleteEphemeralBonusPrompt(): restore runUserMutation guards for safe read-delete-write of ephemeralBonusMsgId/ChatId - sendEphemeralBonusPrompt(): restore guards (claimedPromo check, active promo lookup), per-user dynamic promo lookup via getActivePromoCodeForUser() (was hardcoded promoStore.code), "I Have Claimed" callback button, mutation-safe writes - Add promo_confirm_claimed_next callback handler (button was added, handler missing) Deploy/runtime hardening (deploy.sh, prod-run.sh, runewager.service): - deploy.sh: restore data/backups/ mkdir, chown -R APP_NAME, chmod 0750/0640/0600 permission hardening after npm install - prod-run.sh: restore chmod 0750 for data/data/backups/logs dirs, chmod 0640 for log+session files, chown -R to service user after dir creation - runewager.service: add UMask=0077 (prevents world-readable files from service) .gitignore: - Restore legacy root-level data file patterns: users.json, giveaways.json, promo.json, env.json, analytics*.json (pre-data/ directory structure) All 60 tests pass. node --check clean. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
- TDZ fix: move pendingActionsTimedOut/menuStaleRecoveries declarations
before evaluatePendingActionTimeout to eliminate temporal dead-zone risk
- Timeout boundary: revert < to <= so age exactly equal to 15m is NOT
expired — aligns with /testall check and unit test expectations
- /logs line count: clamp to [1, 200] (adds Math.max(1,...) lower bound
to reject negative/zero values from user input)
- Health panel: fix _errorRate.windowErrors → _errorRate.count (field
did not exist; .count is the correct field on _errorRate object)
- /logs fallback: add execFile('tail') fallback when journalctl errors
with no output (non-systemd systems); uses BOT_LOG_FILE env or default
- executeSshvCommand: fix comment — said "spawn" but code uses exec();
updated to accurately describe exec with shell:true (admin-only)
- load_tooltips.sh: remove || true that defeated diff --cached check,
causing .gitignore changes to never be committed on --push
All 60 tests pass. node --check clean.
https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
## Helpful Tooltips (was Content Drops) - Rename all UI strings: Content Drops → Helpful Tooltips throughout - New settings panel: interval, silent mode, Link Channel/Group - Target group linking via forwarded message (saves name + ID) - Dashboard footer shows real group name + ID - "Show all Helpful Tooltips (N)" button with dynamic count - Inline button builder: [Label - URL] && [Label2 - URL2] syntax - Multiple buttons per row with &&, new line = new row - [Open Bot] shorthand for standard Open Bot button - Full URL + label validation before save - postTipToConfiguredTarget uses silentMode flag + parsed buttons - tipsStore extended with targetGroupTitle and silentMode fields ## Giveaway v3.0+ - Extracted buildGiveawayAnnouncementText + buildGiveawayAnnouncementKeyboard helpers - scheduleGiveawayRefresh: auto-refresh at 25%, 50%, 75% of duration + re-pin - scheduleGiveawayReminders overhauled: 10m, 5m, 1min, 30sec, 10→1 countdown - HTML results format: @handle, SC WON, (2x boost applied), DM tip - Full admin winner report per winner: TG handle, display name, RW username, prize, boost - "View Results in Group" deep-link button in admin report - Winner DMs include SC amount, boost status, RW username - DM failure tracking with summary count - giveawayPreflightCheck: validates group linked, warns on missing pin permission - gwizStart calls preflight before wizard begins ## Scripts - generate_tooltips.sh: extracts DEFAULT_TIPS_LIST from index.js → data/tooltips.json (atomic, idempotent) - add_tooltip.sh: appends placeholder tooltip entry, outputs new ID - deploy.sh: step 3b auto-runs generate_tooltips.sh before service start - prod-run.sh: step 6b auto-runs generate_tooltips.sh before bot launch ## /testall - Added Helpful Tooltips System checks (tipsStore shape, count, interval, target, parser) - Added Giveaway Extended checks (helpers defined, preflight defined) ## Docs - RUNEWAGER_FUNCTIONALITY_MAP.md: full v3.0+ sync with flowcharts All 60 tests pass. node --check clean. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
|
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 · |
Reviewer's GuideLarge v3.0 upgrade of the Telegram bot focused on safer URL/command handling, robust menu lifecycle and stale menu cleanup, improved giveaways and helpful tooltips systems, richer admin UX/stats/health panels, deployment/tooling hardening, and updated documentation/tests. Sequence diagram for the upgraded giveaway lifecycle with preflight, refresh, and finalizesequenceDiagram
actor Admin
actor User
participant Bot
participant TelegramAPI
Admin->>Bot: tap pamenu_start_giveaway
Bot->>Bot: gwizStart(ctx,user,config)
Bot->>Bot: giveawayPreflightCheck(ctx,chatId)
alt no linked group/channel
Bot-->>Admin: "❌ Cannot start giveaway: no valid group/channel linked"
else preflight ok
Bot->>Bot: collect wizard config
Bot->>Bot: createGiveaway(config)
Bot->>Bot: announceGiveaway(giveaway,botUsername)
Bot->>TelegramAPI: sendMessage(giveaway.chatId, initial announcement)
TelegramAPI-->>Bot: announceMsgId
Bot->>TelegramAPI: pinChatMessage(chatId,announceMsgId)
alt missing pin permission
Bot->>Admin: notifyAdmins("missing pin permission")
end
Bot->>Bot: scheduleGiveawayRefresh(giveaway)
Bot->>Bot: scheduleGiveawayReminders(giveaway)
end
User->>TelegramAPI: tap "Join" button (gw_join_id)
TelegramAPI->>Bot: callback gw_join_<id>
Bot->>Bot: evaluateEligibility(user,giveaway)
alt eligible
Bot-->>User: joined confirmation
else not eligible
Bot-->>User: rejection + reason
end
note over Bot: When endTime reached, resetGiveawayTimer triggers finalizeGiveaway
Bot->>Bot: finalizeGiveaway(gwId,forceEnd?)
Bot->>Bot: build weightedPool via computeParticipantWeight
Bot->>Bot: pick winners (splicing pool to avoid infinite loop)
alt not dryRun
Bot->>TelegramAPI: sendMessage(chatId, renderWinnersText) (HTML)
end
loop for each winner
Bot->>TelegramAPI: sendMessage(winner.userId, win DM)
alt DM fails
Bot->>Bot: record userId in dmFailedUsers
end
end
Bot->>Admin: send per-admin report with
Bot-->>Admin: winners list, prizes, boosts, DM failures
Bot-->>Admin: inline buttons (gw_reroll_id, gw_paid_id, deep-link to group post)
Sequence diagram for the Helpful Tooltips system and settings flowsequenceDiagram
actor Admin
participant Bot
participant TelegramAPI
%% Link target via settings
Admin->>Bot: tap tips_cmd_settings
Bot-->>Admin: show Helpful Tooltips Settings (tipsSettingsKeyboard)
Admin->>Bot: tap tips_set_link_target
Bot->>Bot: clearPendingAction(user)
Bot->>Bot: user.pendingAction = await_tip_link_target
Bot-->>Admin: prompt "Forward message from target channel/group"
Admin->>TelegramAPI: forward message from target chat
TelegramAPI->>Bot: on text message
Bot->>Bot: extractForwardedChat(message)
Bot->>Bot: handle await_tip_link_target
Bot->>Bot: approvedGroupsStore.add(chatId)
Bot->>Bot: tipsStore.targetGroup = chatId
Bot->>Bot: tipsStore.targetGroupTitle = title
Bot->>Bot: broadcastConfigStore.targetGroup = chatId
Bot->>Bot: user.pendingAction = null
Bot->>Bot: persistRuntimeState()
Bot-->>Admin: confirmation with title + id
%% Add tooltip with inline buttons
Admin->>Bot: tap tips_cmd_add
Bot->>Bot: clearPendingAction(user)
Bot->>Bot: user.pendingAction = await_tip_add_text
Bot-->>Admin: instructions + button syntax
Admin->>TelegramAPI: send tooltip text
TelegramAPI->>Bot: on text (await_tip_add_text)
Bot->>Bot: parseTooltipButtons(text)
alt syntax error
Bot-->>Admin: "❌ Button syntax error" + guidance
else valid syntax
Bot->>Bot: add {id, text, enabled:true} to tipsStore.tips
Bot->>Bot: tipsStore.nextTipId++
Bot->>Bot: user.pendingAction = null
Bot->>Bot: persistRuntimeState()
Bot->>Bot: saveHelpfulMessages()
Bot-->>Admin: "✅ Added Tooltip #id (with inline buttons?)"
end
%% Scheduler posting
Bot->>Bot: startTipsScheduler()
loop every intervalHours
Bot->>Bot: select enabled tip
Bot->>Bot: postTipToConfiguredTarget(tip,telegram)
Bot->>Bot: parseTooltipButtons(tip.text)
Bot->>TelegramAPI: sendMessage(target, formatted text, keyboard, silentMode)
Bot->>Bot: tipsStore.lastSentTipId = tip.id
Bot->>Bot: persistRuntimeState()
Bot->>Bot: saveHelpfulMessages()
end
Class diagram for updated user, giveaway, helpful tooltips, and metrics structuresclassDiagram
class User {
+string id
+number lastSeenAt
+string playMode // legacy
+Settings settings
+number mainMenuMsgId
+number mainMenuChatId
+number mainMenuSentAt // NEW: ms timestamp
+number adminMenuMsgId
+number adminMenuChatId
+number adminMenuSentAt // NEW: ms timestamp
+number lastMenuMsgId
+number lastMenuChatId
+number ephemeralBonusMsgId
+number ephemeralBonusChatId
+PendingAction pendingAction
+Set~number~ giveawayJoinedIds
+string runewagerUsername
+boolean claimedPromo
+boolean hasClaimedNewUserPromo
+number lastAnyPromoClaimAt
+number boostExpiresAt
+number referralCount
+boolean adminModeOn
}
class Settings {
+string playMode // 'browser' | 'miniapp'
...
}
class PendingAction {
+string type
+number createdAt
+any data
}
class TipsStore {
+boolean systemEnabled
+number intervalHours
+string targetGroup
+string targetGroupTitle // NEW: human-readable
+boolean silentMode // NEW: stored for UI
+number nextTipId
+number lastSentTipId
+Tip[] tips
}
class Tip {
+number id
+string text
+boolean enabled
}
class GiveawayStore {
+Map~number,Giveaway~ running
+Map~number,Giveaway~ completed
}
class Giveaway {
+number id
+number chatId
+string title
+number scPerWinner
+number maxWinners
+number durationMinutes
+number endTime
+boolean dryRun
+boolean testMode
+Set~Participant~ participants
+Participant[] winners
+Timeout endTimer
+Timeout[] reminders
+number pinnedMsgId
+number announceMsgId
+string joinSurface // 'group' | 'dm' | 'both'
+boolean requireLinked
+boolean requireChannel
+boolean requireGroup
+boolean requireAge
+boolean requireVerified
+boolean requirePromo
+boolean requireWalkthrough
+number minParticipants
}
class Participant {
+number userId
+string tgUsername
+string firstName
+string rwUsername
+number awardedSc
+boolean boosted
}
class MetricsCounters {
+number pendingActionsTimedOut // NEW
+number menuStaleRecoveries // NEW
+number uptimeSeconds
+number bonusSent
+number bonusDenied
}
User --> Settings
User --> PendingAction
TipsStore --> Tip
GiveawayStore --> Giveaway
Giveaway --> Participant
MetricsCounters ..> User : aggregated from
MetricsCounters ..> GiveawayStore : aggregated from
MetricsCounters ..> TipsStore : aggregated from
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR rebands the "Content Drops" feature to "Helpful Tooltips" across the codebase, introduces three new bash utility scripts for tooltip data management, enhances the giveaway system with preflight validation and periodic refresh scheduling, and adds runtime permission hardening to deployment and production scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin
participant DeployScript as deploy.sh/prod-run.sh
participant TooltipGen as generate_tooltips.sh
participant IndexJS as index.js
participant FileSystem as File System
Admin->>DeployScript: Trigger deployment/restart
DeployScript->>FileSystem: Ensure data directory exists
DeployScript->>TooltipGen: Execute generate_tooltips.sh (if available)
TooltipGen->>IndexJS: Extract DEFAULT_TIPS_LIST
IndexJS-->>TooltipGen: Return tips array
TooltipGen->>FileSystem: Write temp tooltips.json
TooltipGen->>FileSystem: Validate JSON
FileSystem-->>TooltipGen: Validation result
alt Validation Success
TooltipGen->>FileSystem: Replace data/tooltips.json
TooltipGen-->>DeployScript: Success status
DeployScript->>Admin: Log tooltip refresh success
else Validation Failure
TooltipGen->>FileSystem: Write placeholder JSON
TooltipGen-->>DeployScript: Warning status
DeployScript->>Admin: Log warning & notify via Telegram
end
DeployScript->>Admin: Continue with service deployment/restart
sequenceDiagram
participant Admin
participant IndexJS as index.js
participant GiveawayLogic as Giveaway Lifecycle
participant DataStore as Data/Persistence
participant TargetGroups as Target Groups/Chats
Admin->>GiveawayLogic: Start configured giveaway
GiveawayLogic->>GiveawayLogic: givedawayPreflightCheck()
GiveawayLogic->>TargetGroups: Validate linked groups exist
TargetGroups-->>GiveawayLogic: Group validation result
GiveawayLogic->>TargetGroups: Verify bot permissions
TargetGroups-->>GiveawayLogic: Permission check result
alt Preflight Success
GiveawayLogic->>GiveawayLogic: buildGiveawayAnnouncementText()
GiveawayLogic->>TargetGroups: Post giveaway announcement
GiveawayLogic->>GiveawayLogic: scheduleGiveawayRefresh()
GiveawayLogic->>DataStore: Persist giveaway state
GiveawayLogic-->>Admin: Success notification with details
else Preflight Failure
GiveawayLogic-->>Admin: Clear error + remediation guidance
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
Sequence DiagramShows the primary startup menu-cleanup and the core admin → tooltip creation → scheduled posting path introduced in this PR (menu TTL/stale cleanup and the Helpful Tooltips add/parse/post flow). sequenceDiagram
participant Deploy as Deploy/Start
participant Bot
participant TipsAdmin as Admin(User)
participant TipsStore
participant Telegram
Deploy->>Bot: loadRuntimeState()
Bot->>Bot: clearStaleMenuIds() — null out stale/persistent menus, increment menuStaleRecoveries
Bot-->>Deploy: startup ready
Notes over TipsAdmin,Bot: Admin adds a new Helpful Tooltip
TipsAdmin->>Bot: /tips_cmd_add (send tooltip text with optional button syntax)
Bot->>Bot: parseTooltipButtons(text) -> validate buttons & URLs
Bot->>TipsStore: persist new tooltip (with id, enabled)
Bot-->>TipsAdmin: confirm "Added as Tooltip #id"
Notes over Bot,Telegram: Scheduler posts tooltips at interval
Bot->>Bot: startTipsScheduler() (intervalHours)
Bot->>Telegram: postTipToConfiguredTarget(parsed.text + keyboard) -> sendMessage to target
Telegram-->>Bot: message sent (target confirmed)
Bot->>Bot: update tipsStore.lastSentTipId & persistRuntimeState()
Generated by CodeAnt AI |
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- In
generate_tooltips.sh, theTOOLTIP_JSON=$(node - <<'EOF' ... EOF node "$APP_DIR/index.js" --version ...)command substitution runs twonodecommands and concatenates their outputs, which will corrupt the JSON; the version probe should be moved outside the command substitution (or dropped) soTOOLTIP_JSONcontains only the extracted array. - In
add_tooltip.sh, the Node snippet embedsTOOLTIP_TEXTviaJSON.stringify('$TOOLTIP_TEXT')inside a shell substitution, which can break on quotes/backslashes; consider passing the text as a Node CLI argument or viaprocess.envand serializing it there to avoid shell interpolation issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `generate_tooltips.sh`, the `TOOLTIP_JSON=$(node - <<'EOF' ... EOF node "$APP_DIR/index.js" --version ...)` command substitution runs two `node` commands and concatenates their outputs, which will corrupt the JSON; the version probe should be moved outside the command substitution (or dropped) so `TOOLTIP_JSON` contains only the extracted array.
- In `add_tooltip.sh`, the Node snippet embeds `TOOLTIP_TEXT` via `JSON.stringify('$TOOLTIP_TEXT')` inside a shell substitution, which can break on quotes/backslashes; consider passing the text as a Node CLI argument or via `process.env` and serializing it there to avoid shell interpolation issues.
## Individual Comments
### Comment 1
<location path="generate_tooltips.sh" line_range="37-46" />
<code_context>
+fi
+
+info "Extracting DEFAULT_TIPS_LIST from index.js..."
+TOOLTIP_JSON=$(node - <<'EOF'
+const fs = require('fs');
+const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8');
+// Execute just the DEFAULT_TIPS_LIST block and print it as JSON
+const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/);
+if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); }
+try {
+ // Use Function constructor for safe eval of the array literal
+ const list = (new Function('return ' + m[1]))();
+ console.log(JSON.stringify(list, null, 2));
+} catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); }
+EOF
+node "$APP_DIR/index.js" --version 2>/dev/null || true
+) || {
+ # Fallback: emit a minimal valid tooltips.json with a placeholder
</code_context>
<issue_to_address>
**issue (bug_risk):** The heredoc / command substitution block is syntactically and semantically broken, likely causing generate_tooltips.sh to fail or misbehave.
This combines the heredoc and a separate `node` call inside one `$(...)`, so after `EOF` the `node "$APP_DIR/index.js" --version` line is still parsed as part of the command substitution. That’s invalid and will either break the script or be treated as extra args to `node -`.
Inside the Node snippet, `process.argv[1]` is never set to `index.js` because `node -` only receives `-` as the script path; it will always fall back to `'index.js'` in the CWD and ignore `APP_DIR`.
You likely want something along the lines of:
```bash
TOOLTIP_JSON=$(node "$APP_DIR/index.js" <<'EOF'
const fs = require('fs');
const appIndex = process.argv[2] || 'index.js';
const src = fs.readFileSync(appIndex, 'utf8');
...
EOF
"$APP_DIR/index.js") || { ... }
```
or otherwise restructure so the heredoc and any additional `node` calls are separate, and the path to `index.js` is passed explicitly into the script.
</issue_to_address>
### Comment 2
<location path="add_tooltip.sh" line_range="45-51" />
<code_context>
+TOOLTIP_TEXT="${CUSTOM_TEXT:-New tooltip — edit in admin panel via /tips.}"
+
+# Append new entry and get new ID using Node.js
+NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF
+const fs = require('fs');
+const file = process.argv[1];
+const list = JSON.parse(fs.readFileSync(file, 'utf8'));
+const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0);
+const newId = maxId + 1;
+list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true });
+fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2));
+console.log(newId);
</code_context>
<issue_to_address>
**issue (bug_risk):** Shell interpolation inside the Node.js heredoc is producing invalid JavaScript when adding the tooltip text.
Inside the heredoc, the shell expands `$(node -e ...)` before Node runs, so the generated script gets the tooltip text inserted directly (e.g. `New tooltip...`) instead of as a quoted string literal. That makes the JavaScript invalid and brittle around quotes/special characters.
Instead, JSON-encode the tooltip text once in the shell and pass it as an argument, then parse it inside the Node script:
```bash
TOOLTIP_TEXT_JSON=$(node -e 'console.log(JSON.stringify(process.argv[1] || ""))' "$TOOLTIP_TEXT")
NEW_ID=$(node - "$TOOLTIPS_FILE" "$TOOLTIP_TEXT_JSON" <<'EOF'
const fs = require('fs');
const file = process.argv[2];
const text = JSON.parse(process.argv[3]);
const list = JSON.parse(fs.readFileSync(file, 'utf8'));
const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0);
const newId = maxId + 1;
list.push({ id: newId, text, enabled: true });
fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2));
console.log(newId);
EOF
)
```
This keeps quoting inside Node and avoids shell-generated JS fragments.
</issue_to_address>
### Comment 3
<location path="index.js" line_range="11564" />
<code_context>
+bot.action('tips_cmd_import_batch', async (ctx) => {
</code_context>
<issue_to_address>
**issue (bug_risk):** Import-batch action reuses the single-tip pendingAction type, so pasted JSON will be treated as plain text rather than parsed as a batch.
The handler sets `user.pendingAction = { type: 'await_tip_add_text', ... }`, but that flow expects a single tooltip body string. With the current implementation, any JSON array an admin pastes will be saved as one tooltip’s `.text` and never parsed into multiple entries.
To make batch import work, add a dedicated `await_tip_import_batch` action type and a matching branch in the text handler that `JSON.parse`s the input, validates the array structure, and appends each entry to `tipsStore.tips` instead of reusing `await_tip_add_text`.
</issue_to_address>
### Comment 4
<location path="load_tooltips.sh" line_range="46-55" />
<code_context>
-cat > "$OUT" <<'JSON'
-[
+# ── Tooltip content (HTML-sanitized: only <b>,<i>,<a href>,<code> allowed) ─
+TOOLTIP_JSON='[
{"id":1,"text":"<b>Welcome to Runewager</b> — everything here is free to play with prize redemptions.","enabled":true},
- {"id":2,"text":"Tap <b>🔵 Play & Win</b> to launch using your current Play Mode setting.","enabled":true},
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Hardcoding tooltip IDs here can diverge from IDs managed in index.js / generate_tooltips, leading to collisions or inconsistencies.
This script now embeds a static `TOOLTIP_JSON` with explicit `id`s, while `generate_tooltips.sh` and `add_tooltip.sh` derive IDs from `DEFAULT_TIPS_LIST` in `index.js`. If these paths coexist, IDs can collide and content can drift if this JSON isn’t kept in sync.
If `DEFAULT_TIPS_LIST`/`generate_tooltips.sh` is meant to be canonical, consider either removing/deprecating `load_tooltips.sh`, or refactoring it to invoke `generate_tooltips.sh` / shared extraction logic so tooltip definitions live in a single source of truth.
Suggested implementation:
```
# ── Tooltip content (HTML-sanitized: only <b>,<i>,<a href>,<code> allowed) ─
# Source of truth for tooltip IDs/text lives in generate_tooltips.sh / DEFAULT_TIPS_LIST (index.js).
# To avoid ID collisions or drift, derive TOOLTIP_JSON by invoking generate_tooltips.sh.
tmp_tooltips_json="$(mktemp)"
OUT="$tmp_tooltips_json" ./generate_tooltips.sh
TOOLTIP_JSON="$(cat "$tmp_tooltips_json")"
rm -f "$tmp_tooltips_json"
```
1. Ensure `generate_tooltips.sh` is executable and available on `PATH` or in the same directory that `load_tooltips.sh` runs from (you may need to change `./generate_tooltips.sh` to a fully-qualified path or `"$SCRIPT_DIR/generate_tooltips.sh"` based on the rest of the script).
2. If `generate_tooltips.sh` expects additional environment variables beyond `OUT`, those should also be set here so the generated JSON matches what other callers use.
3. If `load_tooltips.sh` is sourced rather than executed, confirm `mktemp` is available in the runtime environment, or replace it with your project’s standard temp-file helper if one exists elsewhere in the codebase.
</issue_to_address>
### Comment 5
<location path="test/smoke.test.js" line_range="44-47" />
<code_context>
*/
function walk(dir) {
- for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+ let entries;
+ try {
+ entries = fs.readdirSync(dir, { withFileTypes: true });
+ } catch (_) {
+ return; // Permission denied or transient error — skip silently
+ }
+ for (const entry of entries) {
const fullPath = path.join(dir, entry.name);
let stats;
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for skipping unreadable directories in collectJsFiles
This new behavior (swallowing fs.readdirSync errors and skipping unreadable directories) isn’t covered by tests. Please add a unit test that mocks fs.readdirSync to throw (e.g., EACCES) for a specific subdirectory and verifies that the function:
- Does not throw,
- Still returns files from readable subdirectories,
- Omits files from the unreadable directory.
This will lock in the intended behavior and prevent regressions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| TOOLTIP_JSON=$(node - <<'EOF' | ||
| const fs = require('fs'); | ||
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | ||
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | ||
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | ||
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | ||
| try { | ||
| // Use Function constructor for safe eval of the array literal | ||
| const list = (new Function('return ' + m[1]))(); | ||
| console.log(JSON.stringify(list, null, 2)); |
There was a problem hiding this comment.
issue (bug_risk): The heredoc / command substitution block is syntactically and semantically broken, likely causing generate_tooltips.sh to fail or misbehave.
This combines the heredoc and a separate node call inside one $(...), so after EOF the node "$APP_DIR/index.js" --version line is still parsed as part of the command substitution. That’s invalid and will either break the script or be treated as extra args to node -.
Inside the Node snippet, process.argv[1] is never set to index.js because node - only receives - as the script path; it will always fall back to 'index.js' in the CWD and ignore APP_DIR.
You likely want something along the lines of:
TOOLTIP_JSON=$(node "$APP_DIR/index.js" <<'EOF'
const fs = require('fs');
const appIndex = process.argv[2] || 'index.js';
const src = fs.readFileSync(appIndex, 'utf8');
...
EOF
"$APP_DIR/index.js") || { ... }or otherwise restructure so the heredoc and any additional node calls are separate, and the path to index.js is passed explicitly into the script.
| NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF | ||
| const fs = require('fs'); | ||
| const file = process.argv[1]; | ||
| const list = JSON.parse(fs.readFileSync(file, 'utf8')); | ||
| const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0); | ||
| const newId = maxId + 1; | ||
| list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true }); |
There was a problem hiding this comment.
issue (bug_risk): Shell interpolation inside the Node.js heredoc is producing invalid JavaScript when adding the tooltip text.
Inside the heredoc, the shell expands $(node -e ...) before Node runs, so the generated script gets the tooltip text inserted directly (e.g. New tooltip...) instead of as a quoted string literal. That makes the JavaScript invalid and brittle around quotes/special characters.
Instead, JSON-encode the tooltip text once in the shell and pass it as an argument, then parse it inside the Node script:
TOOLTIP_TEXT_JSON=$(node -e 'console.log(JSON.stringify(process.argv[1] || ""))' "$TOOLTIP_TEXT")
NEW_ID=$(node - "$TOOLTIPS_FILE" "$TOOLTIP_TEXT_JSON" <<'EOF'
const fs = require('fs');
const file = process.argv[2];
const text = JSON.parse(process.argv[3]);
const list = JSON.parse(fs.readFileSync(file, 'utf8'));
const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0);
const newId = maxId + 1;
list.push({ id: newId, text, enabled: true });
fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2));
console.log(newId);
EOF
)This keeps quoting inside Node and avoids shell-generated JS fragments.
| clearPendingAction(user); | ||
| user.pendingAction = { type: 'await_tip_add_text' }; | ||
| await ctx.reply('Send the new tip text. HTML and Markdown are both allowed.'); | ||
| user.pendingAction = { type: 'await_tip_add_text', createdAt: Date.now() }; |
There was a problem hiding this comment.
issue (bug_risk): Import-batch action reuses the single-tip pendingAction type, so pasted JSON will be treated as plain text rather than parsed as a batch.
The handler sets user.pendingAction = { type: 'await_tip_add_text', ... }, but that flow expects a single tooltip body string. With the current implementation, any JSON array an admin pastes will be saved as one tooltip’s .text and never parsed into multiple entries.
To make batch import work, add a dedicated await_tip_import_batch action type and a matching branch in the text handler that JSON.parses the input, validates the array structure, and appends each entry to tipsStore.tips instead of reusing await_tip_add_text.
| TOOLTIP_JSON='[ | ||
| {"id":1,"text":"<b>Welcome to Runewager</b> — everything here is free to play with prize redemptions.","enabled":true}, | ||
| {"id":2,"text":"Tap <b>🔵 Play & Win</b> to launch using your current Play Mode setting.","enabled":true}, | ||
| {"id":2,"text":"Tap <b>Play & Win</b> to launch using your current Play Mode setting.","enabled":true}, | ||
| {"id":3,"text":"Use <b>Settings</b> to switch between Browser mode and Mini App mode anytime.","enabled":true}, | ||
| {"id":4,"text":"Link your username early so bonuses and giveaways can be tracked correctly.","enabled":true}, | ||
| {"id":5,"text":"The <b>30 SC Bonus</b> is reviewed manually by GambleCodez — no screenshots required.","enabled":true}, | ||
| {"id":6,"text":"Need help? Open <b>Help / Commands</b> from the main menu.","enabled":true}, | ||
| {"id":7,"text":"Join community hubs for updates: <a href=\"https://t.me/GambleCodezDrops\">Channel</a> and <a href=\"https://t.me/GambleCodezPrizeHub\">Group</a>.","enabled":true}, | ||
| {"id":8,"text":"Referral boosts can grant a <b>2× giveaway boost</b> for 7 days.","enabled":true}, | ||
| {"id":8,"text":"Referral boosts can grant a <b>2x giveaway boost</b> for 7 days.","enabled":true}, | ||
| {"id":9,"text":"All Discord actions open externally; this bot does not use Discord APIs.","enabled":true}, |
There was a problem hiding this comment.
suggestion (bug_risk): Hardcoding tooltip IDs here can diverge from IDs managed in index.js / generate_tooltips, leading to collisions or inconsistencies.
This script now embeds a static TOOLTIP_JSON with explicit ids, while generate_tooltips.sh and add_tooltip.sh derive IDs from DEFAULT_TIPS_LIST in index.js. If these paths coexist, IDs can collide and content can drift if this JSON isn’t kept in sync.
If DEFAULT_TIPS_LIST/generate_tooltips.sh is meant to be canonical, consider either removing/deprecating load_tooltips.sh, or refactoring it to invoke generate_tooltips.sh / shared extraction logic so tooltip definitions live in a single source of truth.
Suggested implementation:
# ── Tooltip content (HTML-sanitized: only <b>,<i>,<a href>,<code> allowed) ─
# Source of truth for tooltip IDs/text lives in generate_tooltips.sh / DEFAULT_TIPS_LIST (index.js).
# To avoid ID collisions or drift, derive TOOLTIP_JSON by invoking generate_tooltips.sh.
tmp_tooltips_json="$(mktemp)"
OUT="$tmp_tooltips_json" ./generate_tooltips.sh
TOOLTIP_JSON="$(cat "$tmp_tooltips_json")"
rm -f "$tmp_tooltips_json"
- Ensure
generate_tooltips.shis executable and available onPATHor in the same directory thatload_tooltips.shruns from (you may need to change./generate_tooltips.shto a fully-qualified path or"$SCRIPT_DIR/generate_tooltips.sh"based on the rest of the script). - If
generate_tooltips.shexpects additional environment variables beyondOUT, those should also be set here so the generated JSON matches what other callers use. - If
load_tooltips.shis sourced rather than executed, confirmmktempis available in the runtime environment, or replace it with your project’s standard temp-file helper if one exists elsewhere in the codebase.
| for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { | ||
| let entries; | ||
| try { | ||
| entries = fs.readdirSync(dir, { withFileTypes: true }); | ||
| } catch (_) { | ||
| return; // Permission denied or transient error — skip silently | ||
| } | ||
| for (const entry of entries) { | ||
| const fullPath = path.join(dir, entry.name); | ||
| let stats; | ||
| try { |
There was a problem hiding this comment.
suggestion (testing): Add a test for skipping unreadable directories in collectJsFiles
This new behavior (swallowing fs.readdirSync errors and skipping unreadable directories) isn’t covered by tests. Please add a unit test that mocks fs.readdirSync to throw (e.g., EACCES) for a specific subdirectory and verifies that the function:
- Does not throw,
- Still returns files from readable subdirectories,
- Omits files from the unreadable directory.
This will lock in the intended behavior and prevent regressions.
Nitpicks 🔍
|
| NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF | ||
| const fs = require('fs'); | ||
| const file = process.argv[1]; | ||
| const list = JSON.parse(fs.readFileSync(file, 'utf8')); | ||
| const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0); | ||
| const newId = maxId + 1; | ||
| list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true }); |
There was a problem hiding this comment.
Suggestion: The Node.js heredoc currently reads from the wrong CLI argument (process.argv[1] instead of the tooltips file argument) and inlines the tooltip text via a nested node -e command substitution, which both causes it to read a non-existent - file and makes any tooltip text containing quotes or special characters break the script or corrupt the JSON; passing the text via an environment variable and using the correct argv index fixes both issues. [logic error]
Severity Level: Critical 🚨
- ❌ Admin "Add Tooltip (Script)" cannot append any tooltips.
- ❌ Manual `./add_tooltip.sh` usage always fails with ENOENT.
- ⚠️ Custom texts containing quotes corrupt or abort JSON generation.| NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF | |
| const fs = require('fs'); | |
| const file = process.argv[1]; | |
| const list = JSON.parse(fs.readFileSync(file, 'utf8')); | |
| const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0); | |
| const newId = maxId + 1; | |
| list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true }); | |
| NEW_ID=$(TOOLTIP_TEXT="$TOOLTIP_TEXT" node - "$TOOLTIPS_FILE" <<EOF | |
| const fs = require('fs'); | |
| const file = process.argv[2]; | |
| const list = JSON.parse(fs.readFileSync(file, 'utf8')); | |
| const tooltipText = process.env.TOOLTIP_TEXT || ''; | |
| const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0); | |
| const newId = maxId + 1; | |
| list.push({ id: newId, text: tooltipText, enabled: true }); |
Steps of Reproduction ✅
1. From the repository root `/workspace/Runewager`, run `./add_tooltip.sh` (script defined
in `add_tooltip.sh:1-63`).
2. Script creates `data/tooltips.json` if missing and validates it via `node -e` at
`add_tooltip.sh:32-40`.
3. The Node heredoc at `add_tooltip.sh:44-54` runs `node - "$TOOLTIPS_FILE"`, so inside
Node `process.argv[1]` is `"-"` and `process.argv[2]` is the actual `tooltips.json` path.
4. Line `47` (`const file = process.argv[1];`) causes `fs.readFileSync('-','utf8')` to
execute; since no file named `-` exists, Node throws `ENOENT`, the `node` command exits
non‑zero, `set -e` aborts the script, and no tooltip is appended.
5. Additionally, invoke `./add_tooltip.sh --text "It's broken"`; at `add_tooltip.sh:51`
the nested `node -e "JSON.stringify('$TOOLTIP_TEXT')"` expansion injects an unescaped
single quote into JavaScript, causing a parse error in the inner Node process and again
aborting the script before updating `tooltips.json`.
6. In both cases, the admin-facing "Add Tooltip (Script)" flow described in
`add_tooltip.sh:3` will fail whenever it calls this script, preventing new tooltips from
being added.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** add_tooltip.sh
**Line:** 45:51
**Comment:**
*Logic Error: The Node.js heredoc currently reads from the wrong CLI argument (`process.argv[1]` instead of the tooltips file argument) and inlines the tooltip text via a nested `node -e` command substitution, which both causes it to read a non-existent `-` file and makes any tooltip text containing quotes or special characters break the script or corrupt the JSON; passing the text via an environment variable and using the correct argv index fixes both issues.
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.| TOOLTIP_JSON=$(node - <<'EOF' | ||
| const fs = require('fs'); | ||
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | ||
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | ||
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | ||
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | ||
| try { | ||
| // Use Function constructor for safe eval of the array literal | ||
| const list = (new Function('return ' + m[1]))(); | ||
| console.log(JSON.stringify(list, null, 2)); | ||
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | ||
| EOF | ||
| node "$APP_DIR/index.js" --version 2>/dev/null || true |
There was a problem hiding this comment.
Suggestion: The tooltip generation script never actually reads the intended index.js file and may also append unrelated node index.js --version output into the JSON, because the Node snippet is invoked as node - <<'EOF' and then uses process.argv[1] (which is '-' in this mode) as the path, and also runs a second node "$APP_DIR/index.js" --version inside the same command substitution; this causes the extraction to fail and always fall back to the placeholder data instead of the real DEFAULT_TIPS_LIST. [logic error]
Severity Level: Major ⚠️
- ⚠️ Helpful tooltips file always ends up with placeholder content.
- ⚠️ Bot loads placeholder tooltip instead of DEFAULT_TIPS_LIST array.
- ⚠️ Admins cannot deploy updated tooltip definitions via index.js.| TOOLTIP_JSON=$(node - <<'EOF' | |
| const fs = require('fs'); | |
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | |
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | |
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | |
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | |
| try { | |
| // Use Function constructor for safe eval of the array literal | |
| const list = (new Function('return ' + m[1]))(); | |
| console.log(JSON.stringify(list, null, 2)); | |
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | |
| EOF | |
| node "$APP_DIR/index.js" --version 2>/dev/null || true | |
| TOOLTIP_JSON=$(node - "$APP_DIR/index.js" <<'EOF' | |
| const fs = require('fs'); | |
| const srcPath = process.argv[2] || 'index.js'; | |
| const src = fs.readFileSync(srcPath, 'utf8'); | |
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | |
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | |
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | |
| try { | |
| // Use Function constructor for safe eval of the array literal | |
| const list = (new Function('return ' + m[1]))(); | |
| console.log(JSON.stringify(list, null, 2)); | |
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | |
| EOF |
Steps of Reproduction ✅
1. Trigger a deployment that runs the tooltip generator, e.g. execute the VPS deployment
script `deploy.sh` at `/workspace/Runewager/deploy.sh`, which at lines 217–221 sets
`TOOLTIP_SCRIPT="$PROJECT_DIR/generate_tooltips.sh"` and invokes
`RUNEWAGER_DIR="$PROJECT_DIR" bash "$TOOLTIP_SCRIPT"` before restarting the bot.
2. Inside `generate_tooltips.sh` at lines 36–47, the script runs `TOOLTIP_JSON=$(node -
<<'EOF' ...)` and the embedded Node snippet calls `fs.readFileSync(process.argv[1] ||
'index.js', 'utf8')`; because the `node` command is invoked as `node -`, `process.argv[1]`
is `'-'`, so Node attempts to read a non-existent file named `-` and exits with an error
without emitting JSON.
3. Still inside the same command substitution (lines 36–50), the next command `node
"$APP_DIR/index.js" --version 2>/dev/null || true` runs successfully and prints a version
string (for example `v3.0.0`) to stdout; because this is the last command in the subshell
its zero exit status causes the overall `$(...)` to succeed, and `TOOLTIP_JSON` is set to
the non‑JSON version string instead of a JSON array.
4. Later in `generate_tooltips.sh` at lines 62–71, the script writes `"$TOOLTIP_JSON"`
(the version string) to the temp file and validates it with `node -e "JSON.parse(...)"`;
JSON parsing fails, so the `|| { ... }` block at lines 66–71 replaces the temp file
contents with a hard-coded placeholder JSON array, and finally `mv "$TMP_FILE"
"$TOOLTIPS_FILE"` at line 73 writes this placeholder to
`/var/www/html/Runewager/data/tooltips.json`, which is exactly the path used by
`loadHelpfulMessages()` in `index.js` (lines 14–23 and the `systemTooltipsFile` constant
at line 337) to populate the in-bot Helpful Tooltips list.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** generate_tooltips.sh
**Line:** 37:49
**Comment:**
*Logic Error: The tooltip generation script never actually reads the intended `index.js` file and may also append unrelated `node index.js --version` output into the JSON, because the Node snippet is invoked as `node - <<'EOF'` and then uses `process.argv[1]` (which is `'-'` in this mode) as the path, and also runs a second `node "$APP_DIR/index.js" --version` inside the same command substitution; this causes the extraction to fail and always fall back to the placeholder data instead of the real `DEFAULT_TIPS_LIST`.
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. |
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (5)
generate_tooltips.sh (2)
41-41: Non-greedy regex may truncate nested arrays.The pattern
(\[[\s\S]+?\])uses a non-greedy quantifier (+?) which will match the shortest possible string ending with];. IfDEFAULT_TIPS_LISTcontains nested arrays or objects with]characters, the match may be incomplete.♻️ Consider greedy match with lookahead or balanced bracket parsing
A safer approach for complex arrays:
-const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); +// Match from opening bracket, find the balanced closing bracket +const start = src.indexOf('const DEFAULT_TIPS_LIST'); +if (start === -1) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } +const arrStart = src.indexOf('[', start); +let depth = 0, end = arrStart; +for (; end < src.length; end++) { + if (src[end] === '[') depth++; + else if (src[end] === ']' && --depth === 0) break; +} +const arrStr = src.slice(arrStart, end + 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generate_tooltips.sh` at line 41, The regex used to extract DEFAULT_TIPS_LIST (in the src.match call that assigns to m) is non-greedy and can truncate when the array contains nested brackets; replace this extraction with a robust parser: either change the regex to a greedy match with a lookahead that anchors on the closing sequence (e.g., match the full "const DEFAULT_TIPS_LIST = " followed by a greedy bracket block using a lookahead for the terminating semicolon) or implement a small bracket-balancing routine that scans src from the index of "const DEFAULT_TIPS_LIST" and finds the matching closing bracket before the semicolon; update the code that sets m and any downstream parsing to use the new extraction so nested arrays/objects are handled correctly.
66-71: Validation uses shell variable interpolation in Node code.Line 66 embeds
$TMP_FILEdirectly into the Node-escript. IfTMP_FILEcontains special characters (unlikely but possible with$$PID), this could break. Consider passing as an argument.♻️ Safer argument passing
-node -e "JSON.parse(require('fs').readFileSync('$TMP_FILE','utf8'))" 2>/dev/null || { +node -e "JSON.parse(require('fs').readFileSync(process.argv[1],'utf8'))" "$TMP_FILE" 2>/dev/null || {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generate_tooltips.sh` around lines 66 - 71, The node -e invocation embeds the shell variable $TMP_FILE directly into the JS code which risks breaking when TMP_FILE contains special chars; instead pass the filename as a separate argument and read it inside the one-liner via process.argv (so the JS uses JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')) rather than interpolating $TMP_FILE), and update the shell command to supply "$TMP_FILE" as an argument while keeping the existing error handling and placeholder write (references: TMP_FILE, node -e, JSON.parse, warn).prod-run.sh (1)
272-277: Consider notifying all admins, not only the first one.Failure alerts currently go to
ADMIN_IDS%%,*only, which can hide deploy issues from other operators.♻️ Proposed improvement
- if [[ -n "${BOT_TOKEN:-}" && -n "${ADMIN_IDS:-}" ]]; then - ADMIN_ID="${ADMIN_IDS%%,*}" - curl -s "https://api.telegram.org/bot${BOT_TOKEN}/sendMessage" \ - -d chat_id="$ADMIN_ID" \ - -d text="⚠️ generate_tooltips.sh failed during prod-run — tooltips may be stale." \ - >/dev/null 2>&1 || true - fi + if [[ -n "${BOT_TOKEN:-}" && -n "${ADMIN_IDS:-}" ]]; then + IFS=',' read -ra _IDS <<< "$ADMIN_IDS" + for ADMIN_ID in "${_IDS[@]}"; do + ADMIN_ID="${ADMIN_ID// /}" + [[ -z "$ADMIN_ID" ]] && continue + curl -s "https://api.telegram.org/bot${BOT_TOKEN}/sendMessage" \ + -d chat_id="$ADMIN_ID" \ + -d text="⚠️ generate_tooltips.sh failed during prod-run — tooltips may be stale." \ + >/dev/null 2>&1 || true + done + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prod-run.sh` around lines 272 - 277, The alert currently only messages the first admin by using ADMIN_ID="${ADMIN_IDS%%,*}"; change the logic to iterate over all comma-separated ADMIN_IDS and send the Telegram message to each one using BOT_TOKEN and the same sendMessage curl call; properly split/truncate whitespace from ADMIN_IDS (e.g., set IFS=',' and loop) and ensure the curl call is executed for every admin id and still silenced (>/dev/null 2>&1 || true) to preserve behavior.load_tooltips.sh (2)
79-83: Consider whether--pullfailure should be fatal.When a user explicitly passes
--pull, they may expect the operation to fail if the pull doesn't succeed. Currently, it continues with a warning, which could lead to writing outdated content. This behavior could be intentional for offline flexibility, but if so, consider documenting it in the usage comments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@load_tooltips.sh` around lines 79 - 83, The current git pull block governed by DO_PULL silently continues on failure (git -C "$REPO_DIR" pull origin main || warn ...); decide and implement one of two fixes: either make pull failure fatal by replacing the non-fatal warning with an error/exit (e.g., use error/die/exit 1 when git fails inside the DO_PULL branch) so explicit --pull enforces up-to-date state, or keep the non-fatal behavior but update the script usage/help text to document that --pull failures are non-fatal and the script will proceed with local state; update the code in the DO_PULL/git pull section and the usage/help comment accordingly, referencing DO_PULL, the git -C "$REPO_DIR" pull call, and the warn/info logging calls.
97-103: Minor edge case: missing trailing newline in .gitignore.If
.gitignoredoesn't end with a newline, the new entry would be appended to the last line instead of on its own line. This is uncommon but can happen with some editors.🔧 Ensure newline before appending
if ! grep -qxF "data/tooltips.json" "$GITIGNORE"; then + # Ensure file ends with newline before appending + [ -s "$GITIGNORE" ] && [ "$(tail -c1 "$GITIGNORE")" != "" ] && echo >> "$GITIGNORE" echo "data/tooltips.json" >> "$GITIGNORE" info "Added data/tooltips.json to .gitignore" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@load_tooltips.sh` around lines 97 - 103, The current append logic may join the new entry to the previous line if .gitignore lacks a trailing newline; before echoing "data/tooltips.json" into the file (the block using the GITIGNORE variable and info logging), check whether the file is non-empty and ends with a newline (e.g., inspect the last byte of "$GITIGNORE"), and if it does not, write a single newline first so the appended entry lands on its own line; keep the existing grep -qxF guard and info "Added data/tooltips.json to .gitignore" behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@add_tooltip.sh`:
- Around line 20-25: The argument parser loop incorrectly assumes a value
follows the --text flag (it assigns CUSTOM_TEXT="$2" and does shift 2) without
checking bounds; update the case branch that handles --text to first verify
there is at least one remaining argument (e.g. check $# -ge 2 or test that $2 is
non-empty and not another flag) before assigning CUSTOM_TEXT and shifting, and
handle the error path by printing a clear message and exiting; refer to the
while loop and the --text case handling (CUSTOM_TEXT, shift 2) to locate where
to add the guard.
- Around line 45-55: The heredoc that injects shell variables into the embedded
Node script (the NEW_ID block) is vulnerable to injection; change it to a quoted
heredoc (<<'EOF') and stop interpolating $TOOLTIP_TEXT and ${TMP_FILE} directly
into the JS source — instead export TOOLTIP_TEXT and TMP_FILE into the
environment and read them inside Node via process.env.TOOLTIP_TEXT and
process.env.TMP_FILE (use JSON.parse/JSON.stringify as needed) so the Node code
uses safe environment values rather than shell-expanded strings.
In `@deploy.sh`:
- Around line 195-202: The script currently swallows chown/chmod failures (lines
using chown/chmod with "|| true"), so deployment can report success while
permissions are incorrect; update the block that runs id "$APP_NAME" and the
subsequent chown/chmod operations to fail loudly: remove the "|| true"
suppressors, and after each critical chown/chmod (for "$PROJECT_DIR/data",
"$PROJECT_DIR/logs", "$PROJECT_DIR/.env", "$PROJECT_DIR/data/backups", and
specific files like "$PROJECT_DIR/data/sshv-sessions.json" and
"$PROJECT_DIR/data/admin-events.log") check the exit status and either log a
clear error via stderr and exit non‑zero or chain commands with && so any
failure aborts the script; ensure you still guard the .env actions with the
existing [[ -f "$PROJECT_DIR/.env" ]] check and preserve the id -u "$APP_NAME"
conditional but propagate failures instead of ignoring them.
In `@generate_tooltips.sh`:
- Around line 37-54: The command substitution that populates TOOLTIP_JSON
accidentally contains an orphaned line that runs node "$APP_DIR/index.js"
--version inside the $(...) block, causing the full app to execute and corrupt
the JSON; remove that orphaned line from the heredoc/block (or move it outside
the command substitution and run it separately if you need the version check),
ensuring the only commands inside the $(...) are the inline node script that
extracts DEFAULT_TIPS_LIST and the subsequent JSON-producing logic so
TOOLTIP_JSON contains only valid JSON.
In `@index.js`:
- Around line 2548-2550: The current high-risk exec(command, ...) call (assigned
to child) uses a shell string; replace it with execFile to eliminate shell
parsing: parse the incoming command string into a binary and argument array (or
better: require callers to supply a sanitized [executable, ...args] array) and
call execFile(executable, args, { cwd: session.cwd, timeout: 8000, maxBuffer: 2
* 1024 * 1024 }, async (err, stdout, stderr) => { ... }); ensure you preserve
the existing child variable, session.cwd, timeout and maxBuffer, remove
shell:true usage, and add validation/whitelisting of the executable name before
calling execFile to satisfy the hardening requirement.
- Around line 27-36: The startup env-var checks currently only print to console
(ADMIN_IDS, TELEGRAM_CHANNEL_ID, TELEGRAM_GROUP_ID) so administrators never
receive these non-fatal warnings; update the checks in index.js to also append
descriptive warning strings to the existing _startupWarnings collection (e.g.,
push messages when ADMIN_IDS is empty or TELEGRAM_CHANNEL_ID / TELEGRAM_GROUP_ID
are missing) while optionally preserving the console.warn/console.error calls,
ensuring each message references the specific variable name and consequence so
the startup-warning delivery path can forward them to admins.
- Around line 6921-6925: The fallback path construction in the execFile callback
uses an undefined symbol APP_DIR causing a ReferenceError; update the logFile
resolution in that callback (the execFile('journalctl' ... async (err, stdout)
=> { ... })) to use a defined fallback such as process.env.BOT_LOG_FILE ||
(process.env.APP_DIR || __dirname) + '/logs/bot.log' (or use path.join with
process.env.APP_DIR || __dirname) so that logFile is always defined when
journalctl is unavailable.
- Around line 560-571: The current logic in evaluatePendingActionTimeout treats
missing/invalid user.pendingAction.createdAt as expired and mutates
user.pendingAction; instead, do not clear or mutate user.pendingAction when
createdAt is missing—treat missing createdAt as not-timed-out so flows that omit
createdAt continue; add guards using Number.isFinite(created) and
Number.isFinite(now) but only treat as expired when created is finite and (now -
created) >= PENDING_ACTION_TIMEOUT_MS (use strict >=), and keep any
logging/expiredType determination without assigning null to user.pendingAction;
reference user.pendingAction, user.pendingAction.createdAt, ACTION_LABELS, and
PENDING_ACTION_TIMEOUT_MS when making the change.
- Around line 12431-12475: There are duplicate definitions of
buildGiveawayAnnouncementText and buildGiveawayAnnouncementKeyboard causing the
later declarations to override the earlier V3 versions; either remove the older
duplicates or rename the new helpers (e.g., buildGiveawayAnnouncementTextV3 and
buildGiveawayAnnouncementKeyboardV3) and update all call sites to the new names,
or consolidate the logic into a single implementation that accepts an
options/version flag inside buildGiveawayAnnouncementText and
buildGiveawayAnnouncementKeyboard to handle both behaviors; search for all
references to buildGiveawayAnnouncementText, buildGiveawayAnnouncementKeyboard,
gw_join_*, gw_details_*, and gw_elig_* to ensure call sites are updated
accordingly.
- Line 2541: In the if condition that tests the variable command for control
characters (the regex that checks for a null byte, backtick, and the substrings
"$(" and "${"), escape the backtick inside the character class so the regex no
longer contains an unescaped backtick; update that regex literal accordingly and
re-run the Biome linter to confirm the noControlCharactersInRegex error is
resolved.
In `@load_tooltips.sh`:
- Around line 85-95: The info logging currently embeds $OUT directly inside the
python3 -c string (in the info "Wrote ..." line), which risks command injection
if OUT contains quotes or metacharacters; change the invocation so the Python
snippet reads the filename from sys.argv (or from stdin) instead of being
interpolated into the string, and invoke python3 with "$OUT" as a separate
argument; update the info message to still print the count but call the modified
python3 invocation (refer to symbols TMP_OUT, OUT and the info logging line that
runs python3 -c).
In `@prod-run.sh`:
- Around line 199-205: The current permission/ownership commands for
SERVICE_USER/SERVICE_GROUP on LOG_DIR, PROJECT_DIR/data (and its backups) and
the chmod calls for MAIN_LOG, ERROR_LOG, ADMIN_EVENTS_LOG, SSHV_SESSIONS_FILE
are silenced with "|| true", which hides failures; replace the silent failures
by chaining and checking exit codes: run chown and chmod without "|| true",
capture their exit status, and on non-zero return log a clear error (including
the failing command and variables like "$SERVICE_USER", "$SERVICE_GROUP",
"$LOG_DIR", "$PROJECT_DIR", "$MAIN_LOG") to stderr or a bootstrap logger and
exit non-zero so deployment fails fast. Ensure the block still checks id -u
"$SERVICE_USER" before chown and preserves safe ordering (chown then chmod)
while returning/propagating failures to the caller.
In `@RUNEWAGER_FUNCTIONALITY_MAP.md`:
- Around line 14-15: The document uses mixed terminology: replace all remaining
occurrences of "Content drops" (e.g., instances referenced near the sections
currently at lines mentioning that phrase) with the new canonical term "Helpful
Tooltips" so RUNEWAGER_FUNCTIONALITY_MAP.md is consistent; update any headings,
bullet points, indexes or table-of-contents entries that reference "Content
drops" to "Helpful Tooltips" and verify internal references (links/anchors)
still match the new heading text (adjust anchor names if necessary).
In `@test/smoke.test.js`:
- Around line 214-216: The isCatchAllRegexPattern() helper is missing detection
for non-capturing lazy/greedy catch-all forms like (?:.*?) and its anchored form
^(?:.*?)$; update isCatchAllRegexPattern() to treat patterns matching (.+),
(.*), (?:.*), (?:.+), and also (?:.*?), (?:.+?), and their optional/anchored
variants (e.g., ^(?:.*?)$ after anchor-stripping) as catch-alls. Modify the
detection logic inside isCatchAllRegexPattern() to normalize/remove anchors then
test for both capturing and non-capturing groups with greedy and lazy
quantifiers (.* , .+ , .*? , .+?) and optional ? suffixes; then update the
corresponding expectations in test/smoke.test.js to include ^(?:.*?)$ (and
similar variants) so the tests assert these are treated as catch-all patterns.
- Around line 241-246: The regex used in the loop that fills constMap
(source.matchAll(...)) requires a semicolon or newline as a terminator so
assignments at EOF are missed; update that pattern to accept semicolon OR
newline OR end-of-file (e.g. replace the trailing class [;\n] with a group that
allows [;\n] or $ / end anchor) so const/let/var declarations without a trailing
semicolon (e.g. "const X = 'x'") are matched and still pushed into constMap in
extractCommandHandlerNames()/the constMap population loop.
---
Nitpick comments:
In `@generate_tooltips.sh`:
- Line 41: The regex used to extract DEFAULT_TIPS_LIST (in the src.match call
that assigns to m) is non-greedy and can truncate when the array contains nested
brackets; replace this extraction with a robust parser: either change the regex
to a greedy match with a lookahead that anchors on the closing sequence (e.g.,
match the full "const DEFAULT_TIPS_LIST = " followed by a greedy bracket block
using a lookahead for the terminating semicolon) or implement a small
bracket-balancing routine that scans src from the index of "const
DEFAULT_TIPS_LIST" and finds the matching closing bracket before the semicolon;
update the code that sets m and any downstream parsing to use the new extraction
so nested arrays/objects are handled correctly.
- Around line 66-71: The node -e invocation embeds the shell variable $TMP_FILE
directly into the JS code which risks breaking when TMP_FILE contains special
chars; instead pass the filename as a separate argument and read it inside the
one-liner via process.argv (so the JS uses
JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')) rather than
interpolating $TMP_FILE), and update the shell command to supply "$TMP_FILE" as
an argument while keeping the existing error handling and placeholder write
(references: TMP_FILE, node -e, JSON.parse, warn).
In `@load_tooltips.sh`:
- Around line 79-83: The current git pull block governed by DO_PULL silently
continues on failure (git -C "$REPO_DIR" pull origin main || warn ...); decide
and implement one of two fixes: either make pull failure fatal by replacing the
non-fatal warning with an error/exit (e.g., use error/die/exit 1 when git fails
inside the DO_PULL branch) so explicit --pull enforces up-to-date state, or keep
the non-fatal behavior but update the script usage/help text to document that
--pull failures are non-fatal and the script will proceed with local state;
update the code in the DO_PULL/git pull section and the usage/help comment
accordingly, referencing DO_PULL, the git -C "$REPO_DIR" pull call, and the
warn/info logging calls.
- Around line 97-103: The current append logic may join the new entry to the
previous line if .gitignore lacks a trailing newline; before echoing
"data/tooltips.json" into the file (the block using the GITIGNORE variable and
info logging), check whether the file is non-empty and ends with a newline
(e.g., inspect the last byte of "$GITIGNORE"), and if it does not, write a
single newline first so the appended entry lands on its own line; keep the
existing grep -qxF guard and info "Added data/tooltips.json to .gitignore"
behavior unchanged.
In `@prod-run.sh`:
- Around line 272-277: The alert currently only messages the first admin by
using ADMIN_ID="${ADMIN_IDS%%,*}"; change the logic to iterate over all
comma-separated ADMIN_IDS and send the Telegram message to each one using
BOT_TOKEN and the same sendMessage curl call; properly split/truncate whitespace
from ADMIN_IDS (e.g., set IFS=',' and loop) and ensure the curl call is executed
for every admin id and still silenced (>/dev/null 2>&1 || true) to preserve
behavior.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.gitignoreCHANGELOG.mdCLAUDE.mdRUNEWAGER_FUNCTIONALITY_MAP.mdadd_tooltip.shdeploy.shgenerate_tooltips.shindex.jsload_tooltips.shpackage.jsonprod-run.shrunewager.servicescripts/pre-deploy-checks.shtest/smoke.test.jstodolist.md
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| --text) CUSTOM_TEXT="$2"; shift 2 ;; | ||
| *) shift ;; | ||
| esac | ||
| done |
There was a problem hiding this comment.
Argument parsing missing bounds check for --text value.
If --text is the last argument without a value, $2 will be empty and shift 2 will fail under set -euo pipefail (or silently consume the next option in edge cases).
🛡️ Add bounds check
while [[ $# -gt 0 ]]; do
case "$1" in
- --text) CUSTOM_TEXT="$2"; shift 2 ;;
+ --text)
+ [[ $# -lt 2 ]] && { echo "ERROR: --text requires a value" >&2; exit 1; }
+ CUSTOM_TEXT="$2"; shift 2 ;;
*) shift ;;
esac
done📝 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.
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --text) CUSTOM_TEXT="$2"; shift 2 ;; | |
| *) shift ;; | |
| esac | |
| done | |
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --text) | |
| [[ $# -lt 2 ]] && { echo "ERROR: --text requires a value" >&2; exit 1; } | |
| CUSTOM_TEXT="$2"; shift 2 ;; | |
| *) shift ;; | |
| esac | |
| done |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@add_tooltip.sh` around lines 20 - 25, The argument parser loop incorrectly
assumes a value follows the --text flag (it assigns CUSTOM_TEXT="$2" and does
shift 2) without checking bounds; update the case branch that handles --text to
first verify there is at least one remaining argument (e.g. check $# -ge 2 or
test that $2 is non-empty and not another flag) before assigning CUSTOM_TEXT and
shifting, and handle the error path by printing a clear message and exiting;
refer to the while loop and the --text case handling (CUSTOM_TEXT, shift 2) to
locate where to add the guard.
| NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF | ||
| const fs = require('fs'); | ||
| const file = process.argv[1]; | ||
| const list = JSON.parse(fs.readFileSync(file, 'utf8')); | ||
| const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0); | ||
| const newId = maxId + 1; | ||
| list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true }); | ||
| fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2)); | ||
| console.log(newId); | ||
| EOF | ||
| ) |
There was a problem hiding this comment.
Shell variable interpolation in Node code is vulnerable to injection.
Line 51 embeds $TOOLTIP_TEXT directly in a single-quoted JavaScript string. If the tooltip text contains single quotes or $ characters, this will break or produce unexpected results. Line 52 has the same issue with ${TMP_FILE}.
🔒 Safer approach using environment variables
# Append new entry and get new ID using Node.js
-NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF
+NEW_ID=$(TOOLTIP_TEXT="$TOOLTIP_TEXT" TMP_FILE="$TMP_FILE" node - "$TOOLTIPS_FILE" <<'EOF'
const fs = require('fs');
const file = process.argv[1];
const list = JSON.parse(fs.readFileSync(file, 'utf8'));
const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0);
const newId = maxId + 1;
-list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true });
-fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2));
+list.push({ id: newId, text: process.env.TOOLTIP_TEXT, enabled: true });
+fs.writeFileSync(process.env.TMP_FILE, JSON.stringify(list, null, 2));
console.log(newId);
EOF
)Note: Changed <<EOF to <<'EOF' (quoted) to prevent shell expansion inside the heredoc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@add_tooltip.sh` around lines 45 - 55, The heredoc that injects shell
variables into the embedded Node script (the NEW_ID block) is vulnerable to
injection; change it to a quoted heredoc (<<'EOF') and stop interpolating
$TOOLTIP_TEXT and ${TMP_FILE} directly into the JS source — instead export
TOOLTIP_TEXT and TMP_FILE into the environment and read them inside Node via
process.env.TOOLTIP_TEXT and process.env.TMP_FILE (use JSON.parse/JSON.stringify
as needed) so the Node code uses safe environment values rather than
shell-expanded strings.
| if id -u "$APP_NAME" >/dev/null 2>&1; then | ||
| chown -R "$APP_NAME:$APP_NAME" "$PROJECT_DIR/data" "$PROJECT_DIR/logs" || true | ||
| [[ -f "$PROJECT_DIR/.env" ]] && chown "$APP_NAME:$APP_NAME" "$PROJECT_DIR/.env" || true | ||
| fi | ||
|
|
||
| chmod 0750 "$PROJECT_DIR/data" "$PROJECT_DIR/data/backups" "$PROJECT_DIR/logs" || true | ||
| chmod 0640 "$PROJECT_DIR/data/sshv-sessions.json" "$PROJECT_DIR/data/admin-events.log" || true | ||
| [[ -f "$PROJECT_DIR/.env" ]] && chmod 0600 "$PROJECT_DIR/.env" || true |
There was a problem hiding this comment.
Don’t silently ignore permission-hardening failures.
chown/chmod failures are fully swallowed, so deploy can report success while runtime later fails to write data//logs/.
🔧 Proposed fix
- if id -u "$APP_NAME" >/dev/null 2>&1; then
- chown -R "$APP_NAME:$APP_NAME" "$PROJECT_DIR/data" "$PROJECT_DIR/logs" || true
- [[ -f "$PROJECT_DIR/.env" ]] && chown "$APP_NAME:$APP_NAME" "$PROJECT_DIR/.env" || true
- fi
-
- chmod 0750 "$PROJECT_DIR/data" "$PROJECT_DIR/data/backups" "$PROJECT_DIR/logs" || true
- chmod 0640 "$PROJECT_DIR/data/sshv-sessions.json" "$PROJECT_DIR/data/admin-events.log" || true
- [[ -f "$PROJECT_DIR/.env" ]] && chmod 0600 "$PROJECT_DIR/.env" || true
+ if id -u "$APP_NAME" >/dev/null 2>&1; then
+ chown -R "$APP_NAME:$APP_NAME" "$PROJECT_DIR/data" "$PROJECT_DIR/logs" \
+ || warn "Failed to chown runtime directories"
+ if [[ -f "$PROJECT_DIR/.env" ]]; then
+ chown "$APP_NAME:$APP_NAME" "$PROJECT_DIR/.env" \
+ || warn "Failed to chown .env"
+ fi
+ fi
+
+ chmod 0750 "$PROJECT_DIR/data" "$PROJECT_DIR/data/backups" "$PROJECT_DIR/logs" \
+ || warn "Failed to chmod runtime directories"
+ chmod 0640 "$PROJECT_DIR/data/sshv-sessions.json" "$PROJECT_DIR/data/admin-events.log" \
+ || warn "Failed to chmod runtime files"
+ if [[ -f "$PROJECT_DIR/.env" ]]; then
+ chmod 0600 "$PROJECT_DIR/.env" || warn "Failed to chmod .env"
+ fiAs per coding guidelines: "**/*.sh: ... chain commands and check exit codes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy.sh` around lines 195 - 202, The script currently swallows chown/chmod
failures (lines using chown/chmod with "|| true"), so deployment can report
success while permissions are incorrect; update the block that runs id
"$APP_NAME" and the subsequent chown/chmod operations to fail loudly: remove the
"|| true" suppressors, and after each critical chown/chmod (for
"$PROJECT_DIR/data", "$PROJECT_DIR/logs", "$PROJECT_DIR/.env",
"$PROJECT_DIR/data/backups", and specific files like
"$PROJECT_DIR/data/sshv-sessions.json" and "$PROJECT_DIR/data/admin-events.log")
check the exit status and either log a clear error via stderr and exit non‑zero
or chain commands with && so any failure aborts the script; ensure you still
guard the .env actions with the existing [[ -f "$PROJECT_DIR/.env" ]] check and
preserve the id -u "$APP_NAME" conditional but propagate failures instead of
ignoring them.
| TOOLTIP_JSON=$(node - <<'EOF' | ||
| const fs = require('fs'); | ||
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | ||
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | ||
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | ||
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | ||
| try { | ||
| // Use Function constructor for safe eval of the array literal | ||
| const list = (new Function('return ' + m[1]))(); | ||
| console.log(JSON.stringify(list, null, 2)); | ||
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | ||
| EOF | ||
| node "$APP_DIR/index.js" --version 2>/dev/null || true | ||
| ) || { | ||
| # Fallback: emit a minimal valid tooltips.json with a placeholder | ||
| warn "Could not extract tooltips from index.js — writing placeholder." | ||
| TOOLTIP_JSON='[{"id":1,"text":"Helpful tooltip placeholder — configure via /tips in the bot admin.","enabled":true}]' | ||
| } |
There was a problem hiding this comment.
Orphaned command inside command substitution will cause issues.
Line 49 (node "$APP_DIR/index.js" --version ...) appears after the heredoc ends but is still within the $(...) command substitution. This line:
- Executes the entire
index.jsfile (not just parsing), which may start the bot or have side effects - Its output gets appended to
TOOLTIP_JSON, corrupting the JSON
🐛 Proposed fix — remove orphaned line
TOOLTIP_JSON=$(node - <<'EOF'
const fs = require('fs');
const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8');
// Execute just the DEFAULT_TIPS_LIST block and print it as JSON
const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/);
if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); }
try {
// Use Function constructor for safe eval of the array literal
const list = (new Function('return ' + m[1]))();
console.log(JSON.stringify(list, null, 2));
} catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); }
EOF
-node "$APP_DIR/index.js" --version 2>/dev/null || true
) || {📝 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.
| TOOLTIP_JSON=$(node - <<'EOF' | |
| const fs = require('fs'); | |
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | |
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | |
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | |
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | |
| try { | |
| // Use Function constructor for safe eval of the array literal | |
| const list = (new Function('return ' + m[1]))(); | |
| console.log(JSON.stringify(list, null, 2)); | |
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | |
| EOF | |
| node "$APP_DIR/index.js" --version 2>/dev/null || true | |
| ) || { | |
| # Fallback: emit a minimal valid tooltips.json with a placeholder | |
| warn "Could not extract tooltips from index.js — writing placeholder." | |
| TOOLTIP_JSON='[{"id":1,"text":"Helpful tooltip placeholder — configure via /tips in the bot admin.","enabled":true}]' | |
| } | |
| TOOLTIP_JSON=$(node - <<'EOF' | |
| const fs = require('fs'); | |
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | |
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | |
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | |
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | |
| try { | |
| // Use Function constructor for safe eval of the array literal | |
| const list = (new Function('return ' + m[1]))(); | |
| console.log(JSON.stringify(list, null, 2)); | |
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | |
| EOF | |
| ) || { | |
| # Fallback: emit a minimal valid tooltips.json with a placeholder | |
| warn "Could not extract tooltips from index.js — writing placeholder." | |
| TOOLTIP_JSON='[{"id":1,"text":"Helpful tooltip placeholder — configure via /tips in the bot admin.","enabled":true}]' | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@generate_tooltips.sh` around lines 37 - 54, The command substitution that
populates TOOLTIP_JSON accidentally contains an orphaned line that runs node
"$APP_DIR/index.js" --version inside the $(...) block, causing the full app to
execute and corrupt the JSON; remove that orphaned line from the heredoc/block
(or move it outside the command substitution and run it separately if you need
the version check), ensuring the only commands inside the $(...) are the inline
node script that extracts DEFAULT_TIPS_LIST and the subsequent JSON-producing
logic so TOOLTIP_JSON contains only valid JSON.
| // Startup env var validation — warn on missing optional-but-important vars | ||
| if (!ADMIN_IDS || ADMIN_IDS.length === 0) { | ||
| console.error('[WARN] ADMIN_IDS is empty — no admins configured. Admin commands will be inaccessible.'); | ||
| } | ||
| if (!process.env.TELEGRAM_CHANNEL_ID && !process.env.ANNOUNCE_CHANNEL) { | ||
| console.warn('[WARN] TELEGRAM_CHANNEL_ID not set — channel announcements will be disabled.'); | ||
| } | ||
| if (!process.env.TELEGRAM_GROUP_ID) { | ||
| console.warn('[WARN] TELEGRAM_GROUP_ID not set — group-linked Helpful Tooltips will be disabled.'); | ||
| } |
There was a problem hiding this comment.
Startup env warnings are not queued for admin delivery.
Lines 27-36 only log to console. These warnings won’t be sent through the startup warning delivery path (_startupWarnings), so admins can miss misconfiguration.
As per coding guidelines: Generate non-fatal startup warnings for missing ADMIN_IDS, TELEGRAM_CHANNEL_ID, TELEGRAM_GROUP_ID.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 27 - 36, The startup env-var checks currently only
print to console (ADMIN_IDS, TELEGRAM_CHANNEL_ID, TELEGRAM_GROUP_ID) so
administrators never receive these non-fatal warnings; update the checks in
index.js to also append descriptive warning strings to the existing
_startupWarnings collection (e.g., push messages when ADMIN_IDS is empty or
TELEGRAM_CHANNEL_ID / TELEGRAM_GROUP_ID are missing) while optionally preserving
the console.warn/console.error calls, ensuring each message references the
specific variable name and consequence so the startup-warning delivery path can
forward them to admins.
| # ── Validate JSON before writing ─────────────────────────────────────────── | ||
| TMP_OUT="$OUT.tmp.$$" | ||
| printf '%s\n' "$TOOLTIP_JSON" > "$TMP_OUT" | ||
| if ! python3 -m json.tool "$TMP_OUT" >/dev/null 2>&1; then | ||
| rm -f "$TMP_OUT" | ||
| error "Generated tooltip JSON failed validation — aborting write" | ||
| fi | ||
|
|
||
| # ── Atomic write ────────────────────────────────────────────────────────── | ||
| mv "$TMP_OUT" "$OUT" | ||
| info "Wrote $(python3 -c "import json; data=json.load(open('$OUT')); print(len(data))") tooltips to $OUT" |
There was a problem hiding this comment.
Atomic write correctly implemented, but line 95 has potential command injection.
The atomic write pattern (temp → validate → move) follows coding guidelines. However, line 95 embeds $OUT directly in a Python string literal. If the path contains quotes or shell metacharacters, this could break or allow injection.
🛡️ Safer approach using argument passing
-info "Wrote $(python3 -c "import json; data=json.load(open('$OUT')); print(len(data))") tooltips to $OUT"
+info "Wrote $(python3 -c "import json,sys; print(len(json.load(open(sys.argv[1]))))" "$OUT") tooltips to $OUT"📝 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.
| # ── Validate JSON before writing ─────────────────────────────────────────── | |
| TMP_OUT="$OUT.tmp.$$" | |
| printf '%s\n' "$TOOLTIP_JSON" > "$TMP_OUT" | |
| if ! python3 -m json.tool "$TMP_OUT" >/dev/null 2>&1; then | |
| rm -f "$TMP_OUT" | |
| error "Generated tooltip JSON failed validation — aborting write" | |
| fi | |
| # ── Atomic write ────────────────────────────────────────────────────────── | |
| mv "$TMP_OUT" "$OUT" | |
| info "Wrote $(python3 -c "import json; data=json.load(open('$OUT')); print(len(data))") tooltips to $OUT" | |
| # ── Validate JSON before writing ─────────────────────────────────────────── | |
| TMP_OUT="$OUT.tmp.$$" | |
| printf '%s\n' "$TOOLTIP_JSON" > "$TMP_OUT" | |
| if ! python3 -m json.tool "$TMP_OUT" >/dev/null 2>&1; then | |
| rm -f "$TMP_OUT" | |
| error "Generated tooltip JSON failed validation — aborting write" | |
| fi | |
| # ── Atomic write ────────────────────────────────────────────────────────── | |
| mv "$TMP_OUT" "$OUT" | |
| info "Wrote $(python3 -c "import json,sys; print(len(json.load(open(sys.argv[1]))))" "$OUT") tooltips to $OUT" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@load_tooltips.sh` around lines 85 - 95, The info logging currently embeds
$OUT directly inside the python3 -c string (in the info "Wrote ..." line), which
risks command injection if OUT contains quotes or metacharacters; change the
invocation so the Python snippet reads the filename from sys.argv (or from
stdin) instead of being interpolated into the string, and invoke python3 with
"$OUT" as a separate argument; update the info message to still print the count
but call the modified python3 invocation (refer to symbols TMP_OUT, OUT and the
info logging line that runs python3 -c).
| # Restrict directory and file permissions for runtime security | ||
| if id -u "$SERVICE_USER" >/dev/null 2>&1; then | ||
| chown -R "$SERVICE_USER:$SERVICE_GROUP" "$LOG_DIR" "$PROJECT_DIR/data" || true | ||
| fi | ||
| chmod 0750 "$PROJECT_DIR/data" "$PROJECT_DIR/data/backups" "$LOG_DIR" || true | ||
| chmod 0640 "$MAIN_LOG" "$ERROR_LOG" "$ADMIN_EVENTS_LOG" "$SSHV_SESSIONS_FILE" || true | ||
|
|
There was a problem hiding this comment.
Permission hardening failures are currently silent.
If ownership/permission updates fail here, subsequent runtime write failures are hard to diagnose.
🔧 Proposed fix
if id -u "$SERVICE_USER" >/dev/null 2>&1; then
- chown -R "$SERVICE_USER:$SERVICE_GROUP" "$LOG_DIR" "$PROJECT_DIR/data" || true
+ chown -R "$SERVICE_USER:$SERVICE_GROUP" "$LOG_DIR" "$PROJECT_DIR/data" \
+ || warn "Failed to chown runtime paths to ${SERVICE_USER}:${SERVICE_GROUP}"
fi
-chmod 0750 "$PROJECT_DIR/data" "$PROJECT_DIR/data/backups" "$LOG_DIR" || true
-chmod 0640 "$MAIN_LOG" "$ERROR_LOG" "$ADMIN_EVENTS_LOG" "$SSHV_SESSIONS_FILE" || true
+chmod 0750 "$PROJECT_DIR/data" "$PROJECT_DIR/data/backups" "$LOG_DIR" \
+ || warn "Failed to chmod runtime directories"
+chmod 0640 "$MAIN_LOG" "$ERROR_LOG" "$ADMIN_EVENTS_LOG" "$SSHV_SESSIONS_FILE" \
+ || warn "Failed to chmod runtime files"As per coding guidelines: "**/*.sh: ... chain commands and check exit codes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prod-run.sh` around lines 199 - 205, The current permission/ownership
commands for SERVICE_USER/SERVICE_GROUP on LOG_DIR, PROJECT_DIR/data (and its
backups) and the chmod calls for MAIN_LOG, ERROR_LOG, ADMIN_EVENTS_LOG,
SSHV_SESSIONS_FILE are silenced with "|| true", which hides failures; replace
the silent failures by chaining and checking exit codes: run chown and chmod
without "|| true", capture their exit status, and on non-zero return log a clear
error (including the failing command and variables like "$SERVICE_USER",
"$SERVICE_GROUP", "$LOG_DIR", "$PROJECT_DIR", "$MAIN_LOG") to stderr or a
bootstrap logger and exit non-zero so deployment fails fast. Ensure the block
still checks id -u "$SERVICE_USER" before chown and preserves safe ordering
(chown then chmod) while returning/propagating failures to the caller.
| - Helpful Tooltips (scheduled/random posts to configured target chat; formerly "Content Drops"). | ||
| - Admin operations (broadcasts, diagnostics, SSHV console, bug triage, backups). |
There was a problem hiding this comment.
Terminology migration is incomplete in the authoritative map.
Line 14 switches to “Helpful Tooltips,” but other sections in this map still use “Content drops” (for example Line 111 and Line 123), which makes the source-of-truth internally inconsistent.
Based on learnings: "Applies to RUNEWAGER_FUNCTIONALITY_MAP.md : Always treat RUNEWAGER_FUNCTIONALITY_MAP.md as the authoritative source of truth before and after code changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@RUNEWAGER_FUNCTIONALITY_MAP.md` around lines 14 - 15, The document uses mixed
terminology: replace all remaining occurrences of "Content drops" (e.g.,
instances referenced near the sections currently at lines mentioning that
phrase) with the new canonical term "Helpful Tooltips" so
RUNEWAGER_FUNCTIONALITY_MAP.md is consistent; update any headings, bullet
points, indexes or table-of-contents entries that reference "Content drops" to
"Helpful Tooltips" and verify internal references (links/anchors) still match
the new heading text (adjust anchor names if necessary).
| * Supports: `.*`, `^.*$`, `.+`, `^.+$`, `(.*)`, `(.+)`, `(.+)?`, `(?:.*)`, `(?:.+)`, | ||
| * `^(?:.*)$`, `(.|\n)*`, and whitespace-padded variants. | ||
| */ |
There was a problem hiding this comment.
Catch-all detection still misses ^(?:.*?)$.
After anchor stripping, ^(?:.*?)$ becomes (?:.*?), but that core is not included, so a generic catch-all can still count as coverage.
🔧 Proposed fix
- * `^(?:.*)$`, `(.|\n)*`, and whitespace-padded variants.
+ * `^(?:.*)$`, `^(?:.*?)$`, `(.|\n)*`, and whitespace-padded variants.
@@
const CATCH_ALL_CORES = new Set([
'.*', '.+', '(?:.*)', '(?:.+)',
+ '(?:.*?)',
'(.*)', '(.+)', '(.+)?',
'(.|\n)*', '(.|\n)+',
'(\\.|[\\s\\S])*',
]);
@@
- const catchAllCases = ['.*', '^.*$', '.+', '^.+$', '(.*)', '(.+)', '(?:.*)', '(?:.+)', '^(?:.*)$', '(.+)?'];
+ const catchAllCases = ['.*', '^.*$', '.+', '^.+$', '(.*)', '(.+)', '(?:.*)', '(?:.+)', '^(?:.*)$', '^(?:.*?)$', '(.+)?'];As per coding guidelines: "**/*.test.js: Expand isCatchAllRegexPattern() to detect (.+), (.), (?:.), and ^(?:.*?)$ patterns."
Also applies to: 223-231, 448-449
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/smoke.test.js` around lines 214 - 216, The isCatchAllRegexPattern()
helper is missing detection for non-capturing lazy/greedy catch-all forms like
(?:.*?) and its anchored form ^(?:.*?)$; update isCatchAllRegexPattern() to
treat patterns matching (.+), (.*), (?:.*), (?:.+), and also (?:.*?), (?:.+?),
and their optional/anchored variants (e.g., ^(?:.*?)$ after anchor-stripping) as
catch-alls. Modify the detection logic inside isCatchAllRegexPattern() to
normalize/remove anchors then test for both capturing and non-capturing groups
with greedy and lazy quantifiers (.* , .+ , .*? , .+?) and optional ? suffixes;
then update the corresponding expectations in test/smoke.test.js to include
^(?:.*?)$ (and similar variants) so the tests assert these are treated as
catch-all patterns.
| // Resolve simple constants/variables: const/let/var HELP = 'help'; — semicolon optional | ||
| const constMap = new Map(); | ||
| for (const m of source.matchAll(/\bconst\s+([A-Za-z_$][\w$]*)\s*=\s*(["'`])((?:\\.|(?!\2).)*)\2\s*;/g)) { | ||
| for (const m of source.matchAll(/\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*(["'`])((?:\\.|(?!\2).)*)\2\s*[;\n]/g)) { | ||
| const value = m[3]; | ||
| if (!value.includes('${')) constMap.set(m[1], value); | ||
| } |
There was a problem hiding this comment.
No-semicolon extraction is still incomplete at end-of-file.
The current pattern requires ; or newline terminator, so const X = 'x' at EOF can be skipped.
🔧 Proposed fix
- for (const m of source.matchAll(/\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*(["'`])((?:\\.|(?!\2).)*)\2\s*[;\n]/g)) {
+ for (const m of source.matchAll(/\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*(["'`])((?:\\.|(?!\2).)*)\2(?:\s*;)?(?=\s*(?:\/\/.*)?(?:\r?\n|$))/g)) {As per coding guidelines: "**/*.test.js: Support let/var and no-semicolon forms in extractCommandHandlerNames() for flexible parsing."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/smoke.test.js` around lines 241 - 246, The regex used in the loop that
fills constMap (source.matchAll(...)) requires a semicolon or newline as a
terminator so assignments at EOF are missed; update that pattern to accept
semicolon OR newline OR end-of-file (e.g. replace the trailing class [;\n] with
a group that allows [;\n] or $ / end anchor) so const/let/var declarations
without a trailing semicolon (e.g. "const X = 'x'") are matched and still pushed
into constMap in extractCommandHandlerNames()/the constMap population loop.
User description
Summary by Sourcery
Upgrade bot to v3.0 with hardened menu lifecycle, safer URL and command handling, enhanced giveaways and helpful tooltips systems, richer admin tooling, and tightened deployment/runtime scripts.
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores:
CodeAnt-AI Description
Make tooltips, menus, giveaways, and admin tooling safer and more reliable
What Changed
Impact
✅ Fewer orphaned/stale menus✅ Safer referral and external link handling✅ Clearer and more reliable giveaway announcements💡 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
New Features
Documentation
Chores