fix(config): add path validation to prevent directory traversal (Issue #12)

Added comprehensive validation for configuration path keys to prevent injection attacks.

Changes:
- Added validateKeyString() method to check original key before splitting
- Enhanced validatePath() to check individual path segments
- Validates against directory traversal: "..", "../", "..\\"
- Validates against path separators: "/" and "\\"
- Validates against null bytes: "\0"
- Validates against leading dots: "." prefix
- Validates against empty segments: "auth..apiKey"

Security Impact:
- Closes vulnerability allowing malicious config keys
- Prevents directory traversal attacks via config paths
- Prevents injection attacks via null bytes
- Prevents access to hidden files via leading dots

Testing:
- Added 7 comprehensive security tests covering all attack vectors
- All 1461 tests pass (up from 1454)
- Tests verify both rejection of malicious paths and acceptance of valid paths

Technical Details:
- Two-stage validation: original string + split segments
- validateKeyString() checks patterns lost in split (e.g., ".." in "../auth")
- validatePath() checks individual segments after split
- Proper error messages for each validation failure

Example Attack Prevented:
configService.set('../../../etc/passwd', 'data')

Location: src/storage/config.ts:72,389-406

Author: sjsyrek

CHANGELOG.md

@@ -85,6 +85,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Location: `src/services/watch.ts:172-186` (timer callback cleanup)
 
 ### Security
+- **Config Path Validation** - Prevents directory traversal and injection attacks (Issue #12)
+  - Added comprehensive validation for configuration path keys
+  - Rejects directory traversal patterns: "..", "../", "..\\"
+  - Rejects path separators: "/" and "\\" in path segments
+  - Rejects null bytes: "\0" (common injection technique)
+  - Rejects leading dots: paths starting with "." (hidden files/directories)
+  - Rejects empty path segments: "auth..apiKey" creates empty segment
+  - Validation happens in two stages: original string + split segments
+  - Added 7 comprehensive security tests covering all attack vectors
+  - **Impact**: Closes security vulnerability allowing malicious config keys
+  - **Example Attack Prevented**: `configService.set('../../../etc/passwd', 'data')`
+  - **Technical Detail**: validateKeyString() checks original key, validatePath() checks segments
+  - Location: `src/storage/config.ts:72,389-406` (validation methods)
+
 - **Symlink Path Validation** - Prevents directory traversal attacks (Issue #6)
   - Rejects symlinks at translate() entry point before processing files
   - Uses `fs.lstatSync()` to detect symlinks (doesn't follow symlinks)

src/storage/config.ts

@@ -65,8 +65,12 @@ export class ConfigService {
 
   /**
    * Set a configuration value by path
+   * Issue #12: Security validation on original key before splitting
    */
   set(key: string, value: unknown): void {
+    // Issue #12: Validate original key string before splitting
+    this.validateKeyString(key);
+
     const keys = key.split('.');
     this.validatePath(keys, value);
 
@@ -267,12 +271,46 @@ export class ConfigService {
 
   /**
    * Validate configuration path and value
+   * Issue #12: Security validation to prevent directory traversal and injection attacks
    */
   private validatePath(keys: string[], value: unknown): void {
     if (keys.length === 0) {
       throw new Error('Invalid path: empty');
     }
 
+    // Issue #12: Security validation - check each path segment for malicious patterns
+    for (const key of keys) {
+      // Check for null bytes first (common injection technique)
+      if (key.includes('\0')) {
+        throw new Error('Invalid path: contains null byte');
+      }
+
+      // Check for path separators (Unix and Windows)
+      if (key.includes('/') || key.includes('\\')) {
+        throw new Error('Invalid path: contains path separator');
+      }
+
+      // Check for directory traversal patterns
+      // Note: ".." in a path like "../auth" becomes ["", "", "auth"] after split
+      // So we check for ".." OR check if key is empty AND previous key was also empty
+      if (key === '..' || key.includes('..')) {
+        throw new Error('Invalid path: contains directory traversal');
+      }
+
+      // Check for leading dots (handles paths starting with "." like ".auth")
+      // Note: ".auth" becomes ["", "auth"] after split, so first element is empty
+      // We check if segment is exactly "." or starts with "." (for segments like ".hidden")
+      if (key === '.' || (key.startsWith('.') && key.length > 0)) {
+        throw new Error('Invalid path: segment starts with dot');
+      }
+
+      // Check for empty segments last (catch-all for other cases)
+      // This catches cases like "auth..apiKey" which splits to ["auth", "", "apiKey"]
+      if (key === '') {
+        throw new Error('Invalid path: empty segment');
+      }
+    }
+
     const path = keys.join('.');
 
     // Validate specific paths
@@ -343,4 +381,27 @@ export class ConfigService {
       throw new Error('Invalid output format');
     }
   }
+
+  /**
+   * Validate key string for security issues (Issue #12)
+   * Checks original key before splitting to catch patterns that would be lost in split
+   */
+  private validateKeyString(key: string): void {
+    // Check for directory traversal in original string
+    // This catches patterns like "../auth" or "auth/../key"
+    if (key.includes('../') || key.includes('..\\') || key.startsWith('..')) {
+      throw new Error('Invalid path: contains directory traversal');
+    }
+
+    // Check for leading dot (hidden files/directories)
+    // This catches ".auth.apiKey" before it's split into ["", "auth", "apiKey"]
+    if (key.startsWith('.')) {
+      throw new Error('Invalid path: segment starts with dot');
+    }
+
+    // Check for consecutive dots (empty segments)
+    if (key.includes('..')) {
+      throw new Error('Invalid path: contains directory traversal');
+    }
+  }
 }

tests/unit/config-service.test.ts

@@ -299,4 +299,50 @@ describe('ConfigService', () => {
       expect(Array.isArray(langs)).toBe(true);
     });
   });
+
+  describe('path security validation (Issue #12)', () => {
+    it('should reject paths with directory traversal (..)', () => {
+      expect(() => {
+        configService.set('../auth.apiKey', 'malicious');
+      }).toThrow('Invalid path: contains directory traversal');
+    });
+
+    it('should reject paths with forward slashes', () => {
+      expect(() => {
+        configService.set('auth/apiKey', 'malicious');
+      }).toThrow('Invalid path: contains path separator');
+    });
+
+    it('should reject paths with backslashes', () => {
+      expect(() => {
+        configService.set('auth\\apiKey', 'malicious');
+      }).toThrow('Invalid path: contains path separator');
+    });
+
+    it('should reject paths with null bytes', () => {
+      expect(() => {
+        configService.set('auth\0apiKey', 'malicious');
+      }).toThrow('Invalid path: contains null byte');
+    });
+
+    it('should reject empty path segments (consecutive dots)', () => {
+      // Note: ".." is caught by directory traversal check (more serious security issue)
+      expect(() => {
+        configService.set('auth..apiKey', 'malicious');
+      }).toThrow('Invalid path: contains directory traversal');
+    });
+
+    it('should reject paths with leading dots', () => {
+      expect(() => {
+        configService.set('.auth.apiKey', 'malicious');
+      }).toThrow('Invalid path: segment starts with dot');
+    });
+
+    it('should accept valid paths with dots in segments', () => {
+      // This should be allowed: segments themselves are just "auth" and "apiKey"
+      expect(() => {
+        configService.set('auth.apiKey', 'valid-key');
+      }).not.toThrow();
+    });
+  });
 });