From 41d19ecb64dcda10ce9cacd46010fa1214791b26 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Fri, 27 Feb 2026 15:41:00 +0000
Subject: [PATCH] fix: address security and reliability issues found in code
 review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- strategy_registry: add isSafeStrategyName() to reject names with path
  traversal chars; add isPathWithinStrategiesDir() to confine file loads
  to var/strategies/; remove arbitrary direct-file-path resolution branch
- bot_runner: attach .catch() handlers to both the initial and recurring
  onTick() calls so async errors are logged instead of silently swallowed
- services: fix loose equality (== → ===) for mail port 465 check
- services: replace console.log with this.getLogger().info() for Telegram
  missing-token warning so the message goes through the structured logger
- services: surface config-load errors with console.warn including the
  file path and error details instead of silently falling back to {}

https://claude.ai/code/session_01MAw1iz4srnJs7oBbqFPwZg
---
 src/modules/services.ts                      | 10 ++++---
 src/modules/strategy/v2/strategy_registry.ts | 28 ++++++++++++++++----
 src/strategy/bot_runner.ts                   |  7 +++--
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/src/modules/services.ts b/src/modules/services.ts
index 5d421ffc..6755bc3f 100644
--- a/src/modules/services.ts
+++ b/src/modules/services.ts
@@ -212,10 +212,12 @@ const services: Services = {
     parameters.portOverride = portOverride;
 
     // Load config if exists, otherwise use empty config
+    const confPath = `${parameters.projectDir}/var/conf.json`;
     try {
-      config = JSON.parse(fs.readFileSync(`${parameters.projectDir}/var/conf.json`, 'utf8'));
-    } catch {
+      config = JSON.parse(fs.readFileSync(confPath, 'utf8'));
+    } catch (err) {
       config = {};
+      console.warn(`[boot] Config file not loaded from "${confPath}": ${err}. Starting with empty config.`);
     }
 
     this.getDatabase();
@@ -439,7 +441,7 @@ const services: Services = {
     return nodemailer.createTransport({
       host: config.notify?.mail?.server,
       port: config.notify?.mail?.port,
-      secure: config.notify?.mail?.port == 465,
+      secure: config.notify?.mail?.port === 465,
       auth: {
         user: config.notify?.mail?.username,
         pass: config.notify?.mail?.password
@@ -452,7 +454,7 @@ const services: Services = {
     const { token } = config.notify?.telegram || {};
 
     if (!token) {
-      console.log('Telegram: No api token given');
+      this.getLogger().info('Telegram: No api token given');
       return;
     }
 
diff --git a/src/modules/strategy/v2/strategy_registry.ts b/src/modules/strategy/v2/strategy_registry.ts
index 5be485c1..3a6e41be 100644
--- a/src/modules/strategy/v2/strategy_registry.ts
+++ b/src/modules/strategy/v2/strategy_registry.ts
@@ -49,22 +49,40 @@ export class StrategyRegistry {
 
   // ============== File Resolution ==============
 
+  /**
+   * Validate that a strategy name contains only safe characters.
+   * Rejects names with path traversal sequences or special characters.
+   */
+  private isSafeStrategyName(name: string): boolean {
+    return /^[a-zA-Z0-9_-]+$/.test(name);
+  }
+
+  /**
+   * Validate that a resolved file path is within the allowed strategies directory.
+   * Prevents path traversal attacks.
+   */
+  private isPathWithinStrategiesDir(filePath: string): boolean {
+    const strategiesDir = path.resolve(process.cwd(), 'var/strategies');
+    const resolved = path.resolve(filePath);
+    return resolved.startsWith(strategiesDir + path.sep) || resolved === strategiesDir;
+  }
+
   /**
    * Resolve strategy name/path to a file path
    */
   private resolveFilePath(strategy: string): string | null {
-    // Direct file path
-    if (fs.existsSync(strategy)) {
-      return strategy;
+    // Reject unsafe strategy names (path traversal, special chars)
+    if (!this.isSafeStrategyName(strategy)) {
+      return null;
     }
     // var/strategies/{name}/{name}.ts
     const varPath = path.join('var/strategies', strategy, `${strategy}.ts`);
-    if (fs.existsSync(varPath)) {
+    if (fs.existsSync(varPath) && this.isPathWithinStrategiesDir(varPath)) {
       return varPath;
     }
     // var/strategies/{name}.ts
     const flatPath = path.join('var/strategies', `${strategy}.ts`);
-    if (fs.existsSync(flatPath)) {
+    if (fs.existsSync(flatPath) && this.isPathWithinStrategiesDir(flatPath)) {
       return flatPath;
     }
     return null;
diff --git a/src/strategy/bot_runner.ts b/src/strategy/bot_runner.ts
index 73fb923b..1de35f25 100644
--- a/src/strategy/bot_runner.ts
+++ b/src/strategy/bot_runner.ts
@@ -59,8 +59,11 @@ export class BotRunner {
     const delay = nextBoundary - now + 8_000;
 
     setTimeout(() => {
-      this.onTick();
-      setInterval(() => this.onTick(), oneMinuteMs);
+      this.onTick().catch(err => this.logger.error(`BotRunner: unhandled tick error: ${err}`));
+      setInterval(
+        () => this.onTick().catch(err => this.logger.error(`BotRunner: unhandled tick error: ${err}`)),
+        oneMinuteMs
+      );
     }, delay);
 
     this.logger.info(`BotRunner: first tick in ${(delay / 1000).toFixed(1)}s`);