Claude/cpu guard script dn d8u - #131
Conversation
…eout Telegraf's long-polling uses getUpdates with timeout=50 (Telegram holds the connection for up to 50 seconds waiting for new updates). The callApi patch was wrapping ALL methods — including getUpdates — with globalThrottle, which has a hard 15-second race. This fired on every poll cycle, throwing 'rateLimiter: API call timed out after 15000ms', causing bot.launch() to reject and systemd to enter an infinite restart loop. Fix: add getUpdates (and Telegraf startup/lifecycle calls: getMe, deleteWebhook, setWebhook, getWebhookInfo, close, logOut) to the _MANAGED bypass set so they call the original callApi directly without the timeout wrapper. All 60 unit tests pass. Bot loads cleanly in CI mode. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…hod sets Two fixes: 1. index.js — ensureQaArtifacts(), persistQaProviderState(), ensureQaDirs(): All QA file writes are now wrapped in try/catch and treated as non-fatal. The bot service runs as the 'runewager' system user but qa/ is owned by root, so writeFileSync was throwing EACCES on every startup, which was caught by startBot()'s outer try/catch and called process.exit(1). 2. telegramSafe.js — callApi bypass list refactored (code review feedback): The inline _MANAGED set is replaced with three named, exported constants: - LONG_POLL_METHODS : getUpdates (must bypass 15 s timeout) - LIFECYCLE_METHODS : getMe, deleteWebhook, setWebhook, etc. - INDIVIDUALLY_PATCHED_METHODS : sendMessage, sendPhoto, etc. Combined into CALLAPI_BYPASS_METHODS used by the callApi patch. All sets are exported so test suites can assert bypass membership without requiring a live Telegraf instance. All 60 unit tests pass. Module loads cleanly in CI mode. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…e conflict Addresses code review feedback: index.js: - Added QA_FS_SUPPRESS constant (EACCES, EPERM, EROFS, EEXIST) — the set of expected error codes when qa/ is owned by root but the service runs as the runewager user. Defined once, shared by all three QA helpers. - ensureQaDirs: replace silent catch-all with per-directory loop; unexpected error codes (e.g. ENOENT, ENOMEM) are logged as warnings so they remain visible; permission codes are silently skipped. - persistQaProviderState: catch checks err.code — returns silently for permission errors, logs a warning for any other code (e.g. JSON failure). - ensureQaArtifacts: same pattern — unexpected errors are logged at warn level; permission errors (EACCES/EPERM/EROFS) are intentionally silent. telegramSafe.js: - Resolved merge conflict: kept the CALLAPI_BYPASS_METHODS module-level constants introduced in the previous commit; dropped the inline _MANAGED block that arrived from main during the merge. All 60 unit tests pass. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
External code or tests could accidentally mutate the exported Sets (LONG_POLL_METHODS, LIFECYCLE_METHODS, etc.) which would silently alter runtime bypass behavior in callApi. Export frozen array snapshots instead — callers can still iterate and use .includes() for membership checks; the internal module-level Sets remain mutable for runtime use by the callApi patch. All 60 unit tests pass. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…=false, UMask=0022
ProtectSystem=strict makes the ENTIRE filesystem read-only (including /var),
requiring every directory Node.js touches to be explicitly listed in
ReadWritePaths. This caused silent crashes under systemd while manual
`node index.js` worked fine (no sandbox).
Changes:
- ProtectSystem=strict → full
/usr, /boot, /etc stay read-only; /var (where the app lives) is fully
writable. No more brittle per-directory ReadWritePaths maintenance.
- ProtectHome=true → false
Service runs as root; /root is root's home. Node/npm use /root for
internal caches and temp files — blocking it caused subtle startup
failures that only appeared under systemd sandboxing.
- UMask=0077 → 0022
0077 (owner-only) made log files mode 600, unreadable by log-tailing
utilities running as non-root. 0022 gives the standard 644/755.
- ReadWritePaths kept as belt-and-suspenders documentation; functionally
redundant under ProtectSystem=full but makes intent explicit and guards
against future tightening.
- Expanded inline comments explain each directive's rationale so the next
operator understands the trade-off before changing settings.
All 60 unit tests pass.
https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…s catch blocks
EEXIST fix:
fs.mkdirSync with { recursive: true } never throws EEXIST for an existing
directory — it silently succeeds. The only way EEXIST reaches these catch
blocks is if a *file* exists at the expected directory path, which is a real
misconfiguration that will silently swallow all QA artifact writes. Removing
EEXIST from QA_FS_SUPPRESS lets that case surface as a logged warning.
Comment updated to explain the exclusion.
Non-Error guard fix:
JavaScript allows any value to be thrown, so a catch block's `e` is not
guaranteed to be an Error instance. All three QA fs catch blocks now use
optional chaining (e?.code, e?.message) and fall back to String(e) for the
message field, preventing secondary TypeErrors during error logging.
All 60 tests pass; node --check clean.
https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
|
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 GuideRefactors Telegram callApi rate-limiter bypass logic into reusable method sets and exports them for testing, and hardens QA artifact directory/file creation to treat permission-related filesystem errors as non-fatal while logging unexpected failures. Sequence diagram for Telegram callApi rate-limiter bypasssequenceDiagram
actor User
participant Bot
participant TelegrafTelegram as Telegraf_Telegram
participant globalThrottle
participant TelegramAPI
User->>Bot: triggerTelegramAction
Bot->>TelegrafTelegram: callApi(method, data, signal)
alt method in CALLAPI_BYPASS_METHODS
TelegrafTelegram->>TelegramAPI: _callApi(method, data, signal)
TelegramAPI-->>TelegrafTelegram: response
else method not in CALLAPI_BYPASS_METHODS
TelegrafTelegram->>globalThrottle: globalThrottle(_callApi, method, data, signal)
globalThrottle->>TelegramAPI: _callApi(method, data, signal)
TelegramAPI-->>globalThrottle: response or timeout
globalThrottle-->>TelegrafTelegram: response or error
end
TelegrafTelegram-->>Bot: response or error
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes introduce graceful error handling for QA file-system operations in index.js, update systemd service sandboxing and permissions in runewager.service, and refactor Telegram API rate-limiting bypass logic in telegramSafe.js with new public exports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider exporting or otherwise centralizing
QA_FS_SUPPRESSif other QA-related helpers may need to share the same error-suppression semantics, to avoid duplicating the set or having it drift from future usage. - In the QA filesystem error handling, you’re consistently logging only
codeandmessage; if deeper diagnosis is ever needed it might be worth including a minimal subset of additional context (e.g.stackbehind a debug flag) so unexpected failures are easier to trace without changing code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider exporting or otherwise centralizing `QA_FS_SUPPRESS` if other QA-related helpers may need to share the same error-suppression semantics, to avoid duplicating the set or having it drift from future usage.
- In the QA filesystem error handling, you’re consistently logging only `code` and `message`; if deeper diagnosis is ever needed it might be worth including a minimal subset of additional context (e.g. `stack` behind a debug flag) so unexpected failures are easier to trace without changing code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
index.js (1)
4923-4945: Make artifact writes independent to maximize best-effort output.Right now a single unexpected failure in
ensureQaArtifacts()prevents later artifact writes in the same block. Splitting writes into per-artifact guarded calls keeps observability more complete.♻️ Suggested refactor
+function tryWriteQaJson(filePath, payload, label) { + try { + fs.writeFileSync(filePath, JSON.stringify(payload, null, 2)); + } catch (e) { + if (!QA_FS_SUPPRESS.has(e?.code)) { + logEvent('warn', `${label}: unexpected error`, { code: e?.code, error: e?.message ?? String(e) }); + } + } +} + function ensureQaArtifacts() { - // QA artifacts are observability helpers — never fatal. - // The service user may lack write access to qa/ when the directory was - // created as root; that is expected and silently skipped. - // Other errors (bad path, JSON serialization failure) are logged as warnings - // so they remain visible without crashing the bot. - try { - ensureQaDirs(); - fs.writeFileSync(qaRepoInfoFile, JSON.stringify({ - repoPath: '/var/www/html/Runewager', - branch: process.env.GIT_BRANCH || 'main', - systemdService: 'runewager.service', - entryFile: 'index.js', - telegramDefault: true, - }, null, 2)); - fs.writeFileSync(qaCapabilitiesFile, JSON.stringify(generateQaCapabilities(), null, 2)); - persistQaProviderState(); - } catch (e) { - if (!QA_FS_SUPPRESS.has(e?.code)) { - logEvent('warn', 'ensureQaArtifacts: unexpected error writing QA artifacts', { code: e?.code, error: e?.message ?? String(e) }); - } - // Permission errors (EACCES/EPERM/EROFS) are intentionally silent — - // the bot runs fine without QA artifacts. - } + ensureQaDirs(); + tryWriteQaJson(qaRepoInfoFile, { + repoPath: '/var/www/html/Runewager', + branch: process.env.GIT_BRANCH || 'main', + systemdService: 'runewager.service', + entryFile: 'index.js', + telegramDefault: true, + }, 'ensureQaArtifacts: repo_info'); + tryWriteQaJson(qaCapabilitiesFile, generateQaCapabilities(), 'ensureQaArtifacts: capabilities'); + persistQaProviderState(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 4923 - 4945, A single failure inside the large try in ensureQaArtifacts() prevents subsequent artifact writes; split the block so each artifact write is performed in its own guarded try/catch (keep ensureQaDirs() run once beforehand or guard it separately), and apply the same QA_FS_SUPPRESS check and logEvent('warn', ...) behavior per write for qaRepoInfoFile, qaCapabilitiesFile, and persistQaProviderState() so one error (e.g., JSON serialization or bad path) does not stop the remaining artifacts from being written.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@runewager.service`:
- Around line 66-67: Change the systemd unit's ProtectHome setting to avoid
blanket /root write access: set ProtectHome=true and add explicit ReadWritePaths
entries for only the Node/npm cache paths (e.g., /root/.npm and /root/.cache) if
the sshv admin shell in index.js (sshv feature around the admin shell) must
remain writable; alternatively make the sshv feature optional/disabled by
default (toggle in index.js) and keep ProtectHome=true to block /root entirely.
Keep ProtectSystem=full as-is but consider ProtectSystem=strict later after
auditing Node.js write paths.
- Around line 62-70: Change the service UMask to 0077 and update file-creation
code to avoid relying on a world-readable umask: set UMask=0077 in the unit
file, then audit and modify file-writing calls (writeFileAtomic, saveJson,
adminLog and any backend log stream writers) to explicitly set permissive modes
only for log files that require group access (e.g., admin-events.log, backend
logs) via open/create flags or chmod/acl after write, and ensure sensitive
artifacts like runtime-state.json are created with restrictive mode (0600). Also
add or document an explicit mechanism to grant log-reader access (group
membership or ACL) for the log files instead of making all artifacts
world-readable.
---
Nitpick comments:
In `@index.js`:
- Around line 4923-4945: A single failure inside the large try in
ensureQaArtifacts() prevents subsequent artifact writes; split the block so each
artifact write is performed in its own guarded try/catch (keep ensureQaDirs()
run once beforehand or guard it separately), and apply the same QA_FS_SUPPRESS
check and logEvent('warn', ...) behavior per write for qaRepoInfoFile,
qaCapabilitiesFile, and persistQaProviderState() so one error (e.g., JSON
serialization or bad path) does not stop the remaining artifacts from being
written.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b04500e-e1a0-4ed4-b25c-1b98c9545638
📒 Files selected for processing (3)
index.jsrunewager.servicetelegramSafe.js
| # UMask=0022 — created files are world-readable (644) and | ||
| # directories are world-executable (755). Required so the log files written | ||
| # by root are readable by non-root log-tailing utilities. | ||
| # | ||
| ProtectSystem=full | ||
| ProtectHome=false | ||
| PrivateTmp=true | ||
| NoNewPrivileges=true | ||
| UMask=0077 | ||
| UMask=0022 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the repository structure to locate the relevant files
git ls-files | grep -E "(index\.js|backend\.js|runewager\.service)" | head -20Repository: gamblecodezcom/Runewager
Length of output: 105
🏁 Script executed:
# Check the size and content of index.js around the mentioned lines
wc -l index.js backend.js runewager.service 2>/dev/null || echo "Files not found at root, searching..."
fd -t f "index\.js$" -o -name "backend\.js$" -o -name "runewager\.service$"Repository: gamblecodezcom/Runewager
Length of output: 387
🏁 Script executed:
# Search for writeFileAtomic, saveJson, adminLog, persistRuntimeState function definitions
rg "writeFileAtomic|saveJson|adminLog|persistRuntimeState" -t js --max-count=50Repository: gamblecodezcom/Runewager
Length of output: 3804
🏁 Script executed:
# Check writeFileAtomic and saveJson implementations (lines 4798-4830)
sed -n '4790,4840p' index.jsRepository: gamblecodezcom/Runewager
Length of output: 2366
🏁 Script executed:
# Check adminLog implementation (lines 4533-4543)
sed -n '4530,4550p' index.jsRepository: gamblecodezcom/Runewager
Length of output: 822
🏁 Script executed:
# Check persistRuntimeState (lines 1361-1366)
sed -n '1355,1375p' index.jsRepository: gamblecodezcom/Runewager
Length of output: 813
🏁 Script executed:
# Check backend.js createWriteStream usage (lines 37-51)
sed -n '35,60p' backend.jsRepository: gamblecodezcom/Runewager
Length of output: 1208
🏁 Script executed:
# Check what's stored in runtimeStateFile and adminEventsLogFile
rg "runtimeStateFile|adminEventsLogFile" index.js | head -20Repository: gamblecodezcom/Runewager
Length of output: 637
🏁 Script executed:
# Find where these files are defined to understand their paths and sensitivity
rg "const.*StateFile\s*=|const.*LogFile\s*=" index.js | head -30Repository: gamblecodezcom/Runewager
Length of output: 350
🏁 Script executed:
# Check the runewager.service file to understand the full context of security settings
cat runewager.serviceRepository: gamblecodezcom/Runewager
Length of output: 3077
🏁 Script executed:
# Verify if there are any other file writes that might be affected
rg "fs\.(writeFileSync|appendFileSync|createWriteStream)" index.js backend.js | wc -lRepository: gamblecodezcom/Runewager
Length of output: 70
🏁 Script executed:
# Check if there are any explicit mode parameters being set anywhere
rg "fs\.(writeFileSync|appendFileSync|mkdirSync).*mode|fs\.writeFile.*\{.*mode" index.js backend.jsRepository: gamblecodezcom/Runewager
Length of output: 50
🏁 Script executed:
# Verify the data sensitivity by checking what's stored in runtime-state.json
rg "createRuntimeStateSnapshot" index.js -A 30 | head -50Repository: gamblecodezcom/Runewager
Length of output: 2188
UMask=0022 exposes sensitive operational data to local users.
Line 70 makes new files world-readable (644) by default. File write operations throughout the codebase—writeFileAtomic, saveJson, adminLog, and backend log streams—do not set explicit restrictive modes, so sensitive files become broadly readable on multi-user hosts. This includes runtime-state.json (user data, promo codes, claims), admin-events.log (admin event records), and bonus logs. With the service running as root, the blast radius is significant.
The service file comment justifies this for "non-root log-tailing utilities," but that rationale applies only to logs, not to all service-written artifacts. Hardening approach: use UMask=0077 and grant log reader access explicitly (group membership or ACL) rather than making all artifacts world-readable.
Proposed hardening diff
-# UMask=0022 — created files are world-readable (644) and
-# directories are world-executable (755). Required so the log files written
-# by root are readable by non-root log-tailing utilities.
+# UMask=0077 — default to owner-only file/dir permissions.
+# If non-root log readers are required, grant access explicitly (group/ACL)
+# instead of making all service-written artifacts world-readable.
@@
-UMask=0022
+UMask=0077📝 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.
| # UMask=0022 — created files are world-readable (644) and | |
| # directories are world-executable (755). Required so the log files written | |
| # by root are readable by non-root log-tailing utilities. | |
| # | |
| ProtectSystem=full | |
| ProtectHome=false | |
| PrivateTmp=true | |
| NoNewPrivileges=true | |
| UMask=0077 | |
| UMask=0022 | |
| # UMask=0077 — default to owner-only file/dir permissions. | |
| # If non-root log readers are required, grant access explicitly (group/ACL) | |
| # instead of making all service-written artifacts world-readable. | |
| # | |
| ProtectSystem=full | |
| ProtectHome=false | |
| PrivateTmp=true | |
| NoNewPrivileges=true | |
| UMask=0077 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@runewager.service` around lines 62 - 70, Change the service UMask to 0077 and
update file-creation code to avoid relying on a world-readable umask: set
UMask=0077 in the unit file, then audit and modify file-writing calls
(writeFileAtomic, saveJson, adminLog and any backend log stream writers) to
explicitly set permissive modes only for log files that require group access
(e.g., admin-events.log, backend logs) via open/create flags or chmod/acl after
write, and ensure sensitive artifacts like runtime-state.json are created with
restrictive mode (0600). Also add or document an explicit mechanism to grant
log-reader access (group membership or ACL) for the log files instead of making
all artifacts world-readable.
Merge conflicts (5 hunks in index.js): - Keep HEAD QA_FS_SUPPRESS without EEXIST (correct: recursive mkdirSync never throws EEXIST for dirs, so EEXIST means misconfiguration) - Keep HEAD optional-chaining (e?.code / e?.message ?? String(e)) throughout Review comment — centralize QA_FS_SUPPRESS: - Added authoritative JSDoc explaining the set is the single source of suppression semantics for all QA fs helpers; adding a code here propagates to every caller automatically Review comment — stack trace behind debug flag: - All QA logEvent warn calls now spread stack: e?.stack when LOG_LEVEL=debug Review comment — independent artifact writes in ensureQaArtifacts: - Extracted tryWriteQaJson() helper: each artifact write is now guarded independently so one serialization/path failure cannot block the others - ensureQaArtifacts() restructured to call tryWriteQaJson per file Review comment — runewager.service hardening: - ProtectHome=false → ProtectHome=true; added /root/.npm and /root/.cache to ReadWritePaths so Node/npm caches work without blanket /root access - UMask=0022 → UMask=0027 (files 640, dirs 750): blocks world-readable artifacts while keeping group-read for log-tailing utilities https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
runewager.service: - UMask=0022 → UMask=0077; files now default to 600, dirs to 700 - Updated comment documenting per-call mode strategy and noting that systemd StandardOutput/StandardError logs are opened by systemd (not the process) and thus unaffected by UMask; group access via usermod -aG or setfacl is the documented mechanism for those files index.js — explicit mode on every writeFileSync / appendFileSync: - writeFileAtomic (temp file): mode 0o600 — sensitive data (runtime-state.json) - adminLog → adminEventsLogFile: mode 0o640 — log file, group-readable - appendBonusAdminLog → bonusAdminLogFile: mode 0o640 — log file, group-readable - writeQaLog daily log files: mode 0o640 — observability, group-readable - persistQaProviderState → qaProviderStateFile: mode 0o600 — private state - tryWriteQaJson → QA artifact files: mode 0o600 — private artifacts - sshv editor save → session.editorMode.filePath: mode 0o600 — private draft https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
User description
Summary by Sourcery
Harden Telegram API rate-limiting exclusions and make QA artifact generation tolerant of permission-restricted environments.
Bug Fixes:
Enhancements:
CodeAnt-AI Description
Avoid Telegram long-poll failures and make QA artifact writes non-fatal
What Changed
Impact
✅ Stable bot startup under long-polling✅ Fewer service restarts due to Telegram timeouts✅ Bot continues running when QA files are unwritable💡 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
Release Notes
Bug Fixes
New Features