Merge pull request #15 from AEmotionStudio/sentinel/config-validation-8752764304139623309

🛡️ Sentinel: Add input validation for configuration settings

.jules/sentinel.md

@@ -4,3 +4,8 @@
 **Vulnerability:** Found multiple instances of `innerHTML` being used with unsanitized user inputs (Node titles, widget values) in `UIManager.ts`.
 **Learning:** This project uses direct DOM manipulation without a framework, making XSS a primary risk. Developers were manually building HTML strings.
 **Prevention:** Introduced `escapeHtml` utility. Any new code using `innerHTML` MUST sanitize inputs. Prefer `textContent` where possible, or use the `escapeHtml` helper.
+
+## 2026-01-07 - Unvalidated Configuration Inputs
+**Vulnerability:** `ConfigManager` loaded settings from user storage without validation, allowing invalid values (e.g., negative dimensions, non-hex colors) that could degrade stability or rendering.
+**Learning:** Configuration data from local storage/user settings must be treated as untrusted input.
+**Prevention:** Implemented strict input validation (clamping for numbers, regex for hex colors) in `ConfigManager.loadSettings`.

src/magnify-glass/ConfigManager.ts

@@ -4,7 +4,7 @@
  * Manages configuration and settings for the magnifying glass.
  */
 
-import { getSettingValue } from '../shared/utils';
+import { getSettingValue, clamp } from '../shared/utils';
 import { STORAGE_KEYS } from '../shared/constants';
 import type { GlassPosition, GlassShape, TextureFilter } from '../shared/constants';
 
@@ -203,15 +203,25 @@ export class ConfigManager {
      * Load settings from ComfyUI settings system.
      */
     loadSettings(): void {
-        this.zoomFactor = getSettingValue<number>("🔍MagnifyGlass.ZoomFactor", this.zoomFactor * 100) / 100;
-        this.glassSize = getSettingValue<number>("🔍MagnifyGlass.GlassSize", this.glassSize);
-        this.borderColor = getSettingValue<string>("🔍MagnifyGlass.BorderColor", this.borderColor);
-        this.borderWidth = getSettingValue<number>("🔍MagnifyGlass.BorderWidth", this.borderWidth);
+        const rawZoom = getSettingValue<number>("🔍MagnifyGlass.ZoomFactor", this.zoomFactor * 100);
+        this.zoomFactor = this.validateNumber(rawZoom, 10, 5000, 300) / 100;
+
+        const rawGlassSize = getSettingValue<number>("🔍MagnifyGlass.GlassSize", this.glassSize);
+        this.glassSize = this.validateNumber(rawGlassSize, 50, 2000, 300);
+
+        const rawBorderColor = getSettingValue<string>("🔍MagnifyGlass.BorderColor", this.borderColor);
+        this.borderColor = this.validateColor(rawBorderColor, "#6b7280");
+
+        const rawBorderWidth = getSettingValue<number>("🔍MagnifyGlass.BorderWidth", this.borderWidth);
+        this.borderWidth = this.validateNumber(rawBorderWidth, 0, 50, 1);
+
         this.activationKey = getSettingValue<string>("🔍MagnifyGlass.ActivationKey", this.activationKey);
         this.altRequired = getSettingValue<boolean>("🔍MagnifyGlass.AltRequired", this.altRequired);
         this.followCursor = getSettingValue<boolean>("🔍MagnifyGlass.FollowCursor", this.followCursor);
 
-        this.offsetStep = getSettingValue<number>("🔍MagnifyGlass.OffsetStep", this.offsetStep);
+        const rawOffsetStep = getSettingValue<number>("🔍MagnifyGlass.OffsetStep", this.offsetStep);
+        this.offsetStep = this.validateNumber(rawOffsetStep, 1, 100, 5);
+
         this.glassPosition = getSettingValue<string>("🔍MagnifyGlass.GlassPosition", this.glassPosition);
         this.resetKey = getSettingValue<string>("🔍MagnifyGlass.ResetKey", this.resetKey);
         this.glassShape = getSettingValue<string>("🔍MagnifyGlass.GlassShape", this.glassShape);
@@ -227,18 +237,50 @@ export class ConfigManager {
         this.accessibilityEnabled = getSettingValue<boolean>("🔍MagnifyGlass.AccessibilityEnabled", this.accessibilityEnabled);
         this.highContrastMode = getSettingValue<boolean>("🔍MagnifyGlass.HighContrastMode", this.highContrastMode);
         this.textGlowEnabled = getSettingValue<boolean>("🔍MagnifyGlass.TextGlowEnabled", this.textGlowEnabled);
-        this.textGlowColor = getSettingValue<string>("🔍MagnifyGlass.TextGlowColor", this.textGlowColor);
-        this.textGlowIntensity = getSettingValue<number>("🔍MagnifyGlass.TextGlowIntensity", this.textGlowIntensity);
-        this.fontScaleFactor = getSettingValue<number>("🔍MagnifyGlass.FontScaleFactor", this.fontScaleFactor);
+
+        const rawGlowColor = getSettingValue<string>("🔍MagnifyGlass.TextGlowColor", this.textGlowColor);
+        this.textGlowColor = this.validateColor(rawGlowColor, "#ffff00");
+
+        const rawGlowIntensity = getSettingValue<number>("🔍MagnifyGlass.TextGlowIntensity", this.textGlowIntensity);
+        this.textGlowIntensity = this.validateNumber(rawGlowIntensity, 1, 50, 5);
+
+        const rawFontScale = getSettingValue<number>("🔍MagnifyGlass.FontScaleFactor", this.fontScaleFactor);
+        this.fontScaleFactor = this.validateNumber(rawFontScale, 50, 500, 100);
+
         this.boldTextEnabled = getSettingValue<boolean>("🔍MagnifyGlass.BoldTextEnabled", this.boldTextEnabled);
         this.textOutlineEnabled = getSettingValue<boolean>("🔍MagnifyGlass.TextOutlineEnabled", this.textOutlineEnabled);
-        this.textOutlineColor = getSettingValue<string>("🔍MagnifyGlass.TextOutlineColor", this.textOutlineColor);
+
+        const rawOutlineColor = getSettingValue<string>("🔍MagnifyGlass.TextOutlineColor", this.textOutlineColor);
+        this.textOutlineColor = this.validateColor(rawOutlineColor, "#000000");
+
         this.nodeTitleEmphasis = getSettingValue<boolean>("🔍MagnifyGlass.NodeTitleEmphasis", this.nodeTitleEmphasis);
         this.invertColors = getSettingValue<boolean>("🔍MagnifyGlass.InvertColors", this.invertColors);
         this.grayscaleMode = getSettingValue<boolean>("🔍MagnifyGlass.GrayscaleMode", this.grayscaleMode);
         this.reduceMotion = getSettingValue<boolean>("🔍MagnifyGlass.ReduceMotion", this.reduceMotion);
     }
 
+    /**
+     * Validate numeric input with bounds checking.
+     */
+    private validateNumber(value: unknown, min: number, max: number, fallback: number): number {
+        const num = Number(value);
+        if (isNaN(num)) return fallback;
+        return clamp(num, min, max);
+    }
+
+    /**
+     * Validate color string (hex).
+     */
+    private validateColor(color: unknown, fallback: string): string {
+        if (!color || typeof color !== 'string') return fallback;
+        // Basic hex validation: #RGB, #RRGGBB, #RRGGBBAA
+        // We permit 3, 4, 6, or 8 hex digits
+        if (/^#([0-9A-Fa-f]{3}|[0-9A-Fa-f]{4}|[0-9A-Fa-f]{6}|[0-9A-Fa-f]{8})$/.test(color)) {
+            return color;
+        }
+        return fallback;
+    }
+
     /**
      * Load saved offsets from localStorage.
      */

tests/unit/ConfigManager.test.ts

@@ -6,6 +6,9 @@
 
 import { describe, it, expect, beforeEach, vi } from 'vitest';
 
+// Mock settings store
+const settingsStore: Record<string, any> = {};
+
 // Mock localStorage
 const localStorageMock = (() => {
     let store: Record<string, string> = {};
@@ -22,7 +25,7 @@ vi.mock('/scripts/app.js', () => ({
     app: {
         ui: {
             settings: {
-                getSettingValue: (key: string) => undefined
+                getSettingValue: (key: string) => settingsStore[key]
             }
         }
     }
@@ -41,6 +44,8 @@ describe('ConfigManager', () => {
 
     beforeEach(() => {
         localStorageMock.clear();
+        // Clear settings store
+        for (const key in settingsStore) delete settingsStore[key];
         configManager = new ConfigManager();
     });
 
@@ -62,6 +67,71 @@ describe('ConfigManager', () => {
         });
     });
 
+    describe('loadSettings Validation', () => {
+        it('should load valid settings correctly', () => {
+            settingsStore['🔍MagnifyGlass.ZoomFactor'] = 500;
+            settingsStore['🔍MagnifyGlass.GlassSize'] = 400;
+
+            configManager.loadSettings();
+
+            expect(configManager.zoomFactor).toBe(5);
+            expect(configManager.glassSize).toBe(400);
+        });
+
+        it('should clamp values exceeding maximum', () => {
+            settingsStore['🔍MagnifyGlass.ZoomFactor'] = 10000; // Way too high
+            settingsStore['🔍MagnifyGlass.GlassSize'] = 5000; // Way too big
+            settingsStore['🔍MagnifyGlass.BorderWidth'] = 100; // Too thick
+            settingsStore['🔍MagnifyGlass.OffsetStep'] = 200; // Too fast
+
+            configManager.loadSettings();
+
+            expect(configManager.zoomFactor).toBe(50); // Clamped to 5000 (50x)
+            expect(configManager.glassSize).toBe(2000); // Clamped to 2000
+            expect(configManager.borderWidth).toBe(50); // Clamped to 50
+            expect(configManager.offsetStep).toBe(100); // Clamped to 100
+        });
+
+        it('should clamp values below minimum', () => {
+            settingsStore['🔍MagnifyGlass.ZoomFactor'] = -100;
+            settingsStore['🔍MagnifyGlass.GlassSize'] = 10;
+            settingsStore['🔍MagnifyGlass.BorderWidth'] = -5;
+            settingsStore['🔍MagnifyGlass.OffsetStep'] = 0;
+
+            configManager.loadSettings();
+
+            expect(configManager.zoomFactor).toBe(0.1); // Clamped to 10 (0.1x)
+            expect(configManager.glassSize).toBe(50); // Clamped to 50
+            expect(configManager.borderWidth).toBe(0); // Clamped to 0
+            expect(configManager.offsetStep).toBe(1); // Clamped to 1
+        });
+
+        it('should validate hex colors', () => {
+            settingsStore['🔍MagnifyGlass.BorderColor'] = '#ff0000'; // Valid
+            configManager.loadSettings();
+            expect(configManager.borderColor).toBe('#ff0000');
+
+            settingsStore['🔍MagnifyGlass.BorderColor'] = 'invalid-color'; // Invalid
+            configManager.loadSettings();
+            expect(configManager.borderColor).toBe('#6b7280'); // Fallback to default (initialized in constructor)
+
+            settingsStore['🔍MagnifyGlass.BorderColor'] = '#123'; // Valid short hex
+            configManager.loadSettings();
+            expect(configManager.borderColor).toBe('#123');
+
+             settingsStore['🔍MagnifyGlass.BorderColor'] = '#12345678'; // Valid alpha hex
+            configManager.loadSettings();
+            expect(configManager.borderColor).toBe('#12345678');
+        });
+
+        it('should handle non-numeric inputs gracefully', () => {
+             settingsStore['🔍MagnifyGlass.GlassSize'] = "not a number";
+             configManager.loadSettings();
+             // Should fallback to default (300) or previous value
+             expect(configManager.glassSize).toBe(300);
+        });
+    });
+
     describe('loadSavedOffsets', () => {
         it('should load offsets from localStorage', () => {
             localStorageMock.setItem('comfyui_magnify_offset_x', '50');