Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 46 additions & 15 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4829,10 +4829,19 @@ function saveJson(filePath, data) {
writeFileAtomic(filePath, JSON.stringify(data, null, 2));
}

// Expected error codes when the service user cannot write to qa/ (created as root).
const QA_FS_SUPPRESS = new Set(['EACCES', 'EPERM', 'EROFS', 'EEXIST']);
Comment on lines +4832 to +4833

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (bug_risk): Suppressing EEXIST for mkdir can hide a real configuration problem when a qa path is a file rather than a directory.

With fs.mkdirSync, EEXIST also covers the case where a file already exists at the target path (e.g. qa/ or qa/logs is a file, not a directory). In that case we likely do want to surface an error, since QA artifacts will never be written and this will be hard to debug. Consider either removing EEXIST from QA_FS_SUPPRESS, or only suppressing it after verifying the existing path is a directory (e.g. via fs.statSync).


function ensureQaDirs() {
fs.mkdirSync(qaContextDir, { recursive: true });
fs.mkdirSync(qaStateDir, { recursive: true });
fs.mkdirSync(qaLogsDir, { recursive: true });
for (const dir of [qaContextDir, qaStateDir, qaLogsDir]) {
try {
fs.mkdirSync(dir, { recursive: true });
} catch (e) {
if (!QA_FS_SUPPRESS.has(e.code)) {
logEvent('warn', 'ensureQaDirs: unexpected error creating QA directory', { dir, code: e.code, error: e.message });
}
}
}
}

function getQaLogDirForToday() {
Expand Down Expand Up @@ -4882,8 +4891,17 @@ function generateQaCapabilities() {
}

function persistQaProviderState() {
ensureQaDirs();
fs.writeFileSync(qaProviderStateFile, JSON.stringify(qaRuntime.providerStatus, null, 2));
try {
ensureQaDirs();
fs.writeFileSync(qaProviderStateFile, JSON.stringify(qaRuntime.providerStatus, null, 2));
} catch (e) {
if (QA_FS_SUPPRESS.has(e.code)) {
// Expected: service user lacks write permission on qa/ (created as root). Non-fatal.
return;
}
// Unexpected (e.g. JSON serialization failure, bad path) — log so it stays visible.
logEvent('warn', 'persistQaProviderState: unexpected error', { code: e.code, error: e.message });
}
}

function refreshQaProviderCooldowns() {
Expand All @@ -4899,16 +4917,29 @@ function refreshQaProviderCooldowns() {
}

function ensureQaArtifacts() {
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();
// 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 });
}
// Permission errors (EACCES/EPERM/EROFS) are intentionally silent —
// the bot runs fine without QA artifacts.
}
}

setInterval(refreshQaProviderCooldowns, qaRuntime.providerStatus.resetIntervalMs).unref();
Expand Down
32 changes: 28 additions & 4 deletions runewager.service
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,36 @@ TimeoutStopSec=30
# Logging: append mode — survives log rotation without restart
StandardOutput=append:/var/www/html/Runewager/logs/runewager.log
StandardError=append:/var/www/html/Runewager/logs/runewager-error.log
ReadWritePaths=/var/www/html/Runewager/logs /var/www/html/Runewager/data /var/www/html/Runewager/qa
ProtectSystem=strict
ProtectHome=true

# ── Filesystem sandboxing ────────────────────────────────────────────────────
#
# ProtectSystem=full — mounts /usr, /boot, /etc read-only; /var stays fully
# writable so the bot can read/write anywhere under /var/www/html/Runewager.
# (strict would make ALL paths read-only, requiring explicit ReadWritePaths
# for every directory Node touches — too brittle for a Node.js app.)
#
# ProtectHome=false — the service runs as root, whose home is /root.
# Node.js and npm use /root for caches and internal temp files; blocking
# /root causes subtle startup failures under strict sandboxing.
#
# PrivateTmp=true — the process gets its own /tmp namespace.
#
# NoNewPrivileges=true — process and children cannot gain new privileges.
#
# 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

# Explicit write grants under /var (belt-and-suspenders — /var is already
# writable under ProtectSystem=full, but listing these makes intent clear
# and ensures compatibility if ProtectSystem is ever tightened again).
ReadWritePaths=/var/www/html/Runewager/logs /var/www/html/Runewager/data /var/www/html/Runewager/qa

# File descriptor limit (for concurrent connections)
LimitNOFILE=65536
Expand Down
83 changes: 62 additions & 21 deletions telegramSafe.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,59 @@

const { enqueue, globalThrottle } = require('./rateLimiter');

// ─── callApi bypass lists ─────────────────────────────────────────────────────
//
// The callApi patch routes unknown methods through globalThrottle (which has a
// hard 15-second timeout). Methods listed here BYPASS that wrapper and call
// the original callApi directly.
//
// Two categories of bypass:
//
// LONG_POLL_METHODS — Telegram long-polling calls.
// getUpdates passes timeout=50 so Telegram holds the connection for up to
// 50 seconds. Applying a 15-second race on top kills every poll cycle.
//
// LIFECYCLE_METHODS — Telegraf startup / shutdown one-shots.
// Fast calls (<1 s), but must never be queued behind a getUpdates slot in
// the global rate-limit chain or they will be delayed by up to 50 seconds.
//
// Individually-patched methods (sendMessage, sendPhoto, etc.) are also in this
// set to prevent double-wrapping when Telegraf routes them through callApi.
//
// To add a future Telegraf lifecycle call: append to LIFECYCLE_METHODS below.
const LONG_POLL_METHODS = new Set([
'getUpdates',
]);

const LIFECYCLE_METHODS = new Set([
'getMe',
'deleteWebhook',
'setWebhook',
'getWebhookInfo',
'close',
'logOut',
]);

// Methods individually patched in init() — already wrapped; skip callApi re-wrap.
const INDIVIDUALLY_PATCHED_METHODS = new Set([
'sendMessage',
'editMessageText',
'answerCbQuery',
'answerCallbackQuery',
'sendPhoto',
'sendDocument',
'sendAnimation',
'sendSticker',
'forwardMessage',
]);

// Combined bypass set used in the callApi patch.
const CALLAPI_BYPASS_METHODS = new Set([
...LONG_POLL_METHODS,
...LIFECYCLE_METHODS,
...INDIVIDUALLY_PATCHED_METHODS,
]);

// ─── State ────────────────────────────────────────────────────────────────────

let _bot = null;
Expand Down Expand Up @@ -179,29 +232,11 @@ function init(bot) {
// method (sendMessage, etc.) calls this.callApi internally, we skip the extra
// layer and pass through to the original.
if (typeof tg.callApi === 'function') {
const _MANAGED = new Set([
// ── Individually-patched above — skip double-wrap ────────────────
'sendMessage', 'editMessageText',
'answerCbQuery', 'answerCallbackQuery',
'sendPhoto', 'sendDocument', 'sendAnimation', 'sendSticker', 'forwardMessage',
// ── Long-poll — MUST bypass the 15-second API_CALL_TIMEOUT_MS ────
// Telegraf passes timeout=50 to getUpdates so Telegram holds the
// connection open for up to 50 s. Wrapping it with globalThrottle's
// 15-second race fires on every poll cycle and kills the bot.
'getUpdates',
// ── Startup / lifecycle calls — fast one-shots ───────────────────
// Pass through directly so they are never blocked behind a queued
// getUpdates slot in the global rate-limit chain.
'getMe',
'deleteWebhook',
'setWebhook',
'getWebhookInfo',
'close',
'logOut',
]);
const _callApi = tg.callApi.bind(tg);
tg.callApi = (method, data, signal) => {
if (_MANAGED.has(method)) return _callApi(method, data, signal);
// Bypass rate-limiter for long-poll, lifecycle, and individually-patched
// methods — see CALLAPI_BYPASS_METHODS definition at top of file.
if (CALLAPI_BYPASS_METHODS.has(method)) return _callApi(method, data, signal);
return _withRetry(() => globalThrottle(() => _callApi(method, data, signal)));
};
}
Expand Down Expand Up @@ -291,4 +326,10 @@ module.exports = {
answerCallbackQuery,
sendPhoto,
sendDocument,
// Exported for testing — frozen arrays so callers can inspect which methods
// bypass rate-limiting without being able to mutate the live runtime Sets.
LONG_POLL_METHODS: Object.freeze([...LONG_POLL_METHODS]),
LIFECYCLE_METHODS: Object.freeze([...LIFECYCLE_METHODS]),
INDIVIDUALLY_PATCHED_METHODS: Object.freeze([...INDIVIDUALLY_PATCHED_METHODS]),
CALLAPI_BYPASS_METHODS: Object.freeze([...CALLAPI_BYPASS_METHODS]),
};