From fe771ef87e6c96c3cca47aefd4c9202f4decb15a Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 16 Jan 2026 04:45:01 +0000 Subject: [PATCH] feat(security): implement strict input validation for InfoPanel settings Add validation for all InfoPanel settings loaded in StateManager to prevent injection of invalid values (e.g. out-of-bounds numbers, invalid colors). This mitigates risks of degraded stability or potential injection via manipulated settings storage. - Add validateNumber, validateColor, validateStringOption helpers - Update StateManager.loadSettings to validate all keys - Add unit tests for validation logic --- src/info-panel/StateManager.ts | 82 ++++++++++++- tests/unit/StateManagerValidation.test.ts | 136 ++++++++++++++++++++++ 2 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 tests/unit/StateManagerValidation.test.ts diff --git a/src/info-panel/StateManager.ts b/src/info-panel/StateManager.ts index 0612464..561e386 100644 --- a/src/info-panel/StateManager.ts +++ b/src/info-panel/StateManager.ts @@ -5,7 +5,7 @@ */ import type { GatheredInfo, SettingsChange } from '../types/comfyui'; -import { getSettingValue } from '../shared/utils'; +import { getSettingValue, clamp } from '../shared/utils'; import { DEFAULT_PANEL_SETTINGS } from '../shared/settings'; export interface InfoPanelStateData { @@ -304,17 +304,93 @@ export class StateManager { } loadSettings(): void { - Object.keys(DEFAULT_PANEL_SETTINGS).forEach(key => { + const defaults = DEFAULT_PANEL_SETTINGS; + + Object.keys(defaults).forEach(key => { // Skip the theme setting since we auto-detect it if (key !== "🔍MagnifyGlass.InfoPanelTheme") { - this.state.settings[key] = getSettingValue(key, (DEFAULT_PANEL_SETTINGS as any)[key]); + this.state.settings[key] = getSettingValue(key, (defaults as any)[key]); } }); + // Validate numeric settings + this.state.settings["🔍MagnifyGlass.InfoPanelWidth"] = this.validateNumber( + this.state.settings["🔍MagnifyGlass.InfoPanelWidth"], 200, 600, defaults["🔍MagnifyGlass.InfoPanelWidth"] as number + ); + + this.state.settings["🔍MagnifyGlass.InfoPanelMaxHeight"] = this.validateNumber( + this.state.settings["🔍MagnifyGlass.InfoPanelMaxHeight"], 200, 1500, defaults["🔍MagnifyGlass.InfoPanelMaxHeight"] as number + ); + + this.state.settings["🔍MagnifyGlass.InfoPanelOpacity"] = this.validateNumber( + this.state.settings["🔍MagnifyGlass.InfoPanelOpacity"], 10, 100, defaults["🔍MagnifyGlass.InfoPanelOpacity"] as number + ); + + this.state.settings["🔍MagnifyGlass.InfoPanelFontSize"] = this.validateNumber( + this.state.settings["🔍MagnifyGlass.InfoPanelFontSize"], 8, 24, defaults["🔍MagnifyGlass.InfoPanelFontSize"] as number + ); + + // Validate colors + this.state.settings["🔍MagnifyGlass.InfoPanelTextColor"] = this.validateColor( + this.state.settings["🔍MagnifyGlass.InfoPanelTextColor"], defaults["🔍MagnifyGlass.InfoPanelTextColor"] as string + ); + + this.state.settings["🔍MagnifyGlass.InfoPanelAccentColor"] = this.validateColor( + this.state.settings["🔍MagnifyGlass.InfoPanelAccentColor"], defaults["🔍MagnifyGlass.InfoPanelAccentColor"] as string + ); + + // Validate Enums + const validPanelPositions = ['Bottom', 'Top', 'Left', 'Right']; + this.state.settings["🔍MagnifyGlass.InfoPanelPosition"] = this.validateStringOption( + this.state.settings["🔍MagnifyGlass.InfoPanelPosition"], validPanelPositions, defaults["🔍MagnifyGlass.InfoPanelPosition"] as string + ); + + const validControlPositions = ['top-left', 'top-right', 'bottom-left', 'bottom-right', 'top', 'bottom', 'left', 'right']; + this.state.settings["🔍MagnifyGlass.ControlsPosition"] = this.validateStringOption( + this.state.settings["🔍MagnifyGlass.ControlsPosition"], validControlPositions, defaults["🔍MagnifyGlass.ControlsPosition"] as string + ); + + const validFonts = [ + 'System Default', 'Inter', 'Roboto', 'JetBrains Mono', 'Fira Code', + 'IBM Plex Sans', 'Space Grotesk', 'Lexend', 'Outfit', 'monospace', 'system-ui' + ]; + this.state.settings["🔍MagnifyGlass.InfoPanelFontFamily"] = this.validateStringOption( + this.state.settings["🔍MagnifyGlass.InfoPanelFontFamily"], validFonts, defaults["🔍MagnifyGlass.InfoPanelFontFamily"] as string + ); + // Set theme to auto-detected value this.state.settings["🔍MagnifyGlass.InfoPanelTheme"] = this.state.currentTheme; } + /** + * 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 + 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; + } + + /** + * Validate string against an allowlist. + */ + private validateStringOption(value: unknown, options: string[], fallback: string): string { + if (typeof value !== 'string') return fallback; + return options.includes(value) ? value : fallback; + } + updateSettings(): Record { const oldSettings = { ...this.state.settings }; this.loadSettings(); diff --git a/tests/unit/StateManagerValidation.test.ts b/tests/unit/StateManagerValidation.test.ts new file mode 100644 index 0000000..486c5e8 --- /dev/null +++ b/tests/unit/StateManagerValidation.test.ts @@ -0,0 +1,136 @@ + +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; + +// Mock localStorage +const localStorageMock = (() => { + let store: Record = {}; + return { + getItem: (key: string) => store[key] || null, + setItem: (key: string, value: string) => { store[key] = value; }, + removeItem: (key: string) => { delete store[key]; }, + clear: () => { store = {}; } + }; +})(); + +Object.defineProperty(globalThis, 'localStorage', { + value: localStorageMock +}); + +// Mock window +Object.defineProperty(globalThis, 'window', { + value: { + matchMedia: () => ({ + addListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + }), + infoPanelManager: null, + getComputedStyle: () => ({ + backgroundColor: 'rgb(30, 30, 30)' + }) + }, + writable: true +}); + +// Mock document +Object.defineProperty(globalThis, 'document', { + value: { + body: { + getAttribute: () => null, + className: '' + }, + documentElement: { + getAttribute: () => null, + className: '' + }, + querySelector: () => null, + querySelectorAll: () => [], + addEventListener: vi.fn() + }, + writable: true +}); + +// Mock getComputedStyle +(globalThis as any).getComputedStyle = () => ({ + backgroundColor: 'rgb(30, 30, 30)' +}); + +// Mock MutationObserver +(globalThis as any).MutationObserver = class { + observe = vi.fn(); + disconnect = vi.fn(); + takeRecords = vi.fn(); +}; + +// Mock utils +vi.mock('../../src/shared/utils', () => ({ + getSettingValue: vi.fn((key, def) => def), // Default implementation returns default + clamp: (v: number, min: number, max: number) => Math.min(Math.max(v, min), max) +})); + +import { StateManager } from '../../src/info-panel/StateManager'; +import { getSettingValue } from '../../src/shared/utils'; +import { DEFAULT_PANEL_SETTINGS } from '../../src/shared/settings'; + +describe('StateManager Settings Validation', () => { + let stateManager: StateManager; + + beforeEach(() => { + vi.clearAllMocks(); + // Reset getSettingValue to return default + (getSettingValue as any).mockImplementation((key: string, def: any) => def); + stateManager = new StateManager(); + }); + + afterEach(() => { + if (stateManager) { + stateManager.cleanup(); + } + }); + + it('should validate InfoPanelWidth (number validation)', () => { + // Mock invalid string input + (getSettingValue as any).mockImplementation((key: string, def: any) => { + if (key === '🔍MagnifyGlass.InfoPanelWidth') return 'invalid-width'; + return def; + }); + + stateManager.loadSettings(); + expect(stateManager.state.settings['🔍MagnifyGlass.InfoPanelWidth']).toBe(DEFAULT_PANEL_SETTINGS['🔍MagnifyGlass.InfoPanelWidth']); + + // Mock out of bounds input + (getSettingValue as any).mockImplementation((key: string, def: any) => { + if (key === '🔍MagnifyGlass.InfoPanelWidth') return 10000; + return def; + }); + stateManager.loadSettings(); + expect(stateManager.state.settings['🔍MagnifyGlass.InfoPanelWidth']).toBe(600); // Max is 600 + }); + + it('should validate InfoPanelOpacity (number validation)', () => { + (getSettingValue as any).mockImplementation((key: string, def: any) => { + if (key === '🔍MagnifyGlass.InfoPanelOpacity') return -50; + return def; + }); + stateManager.loadSettings(); + expect(stateManager.state.settings['🔍MagnifyGlass.InfoPanelOpacity']).toBe(10); // Min is 10 + }); + + it('should validate InfoPanelTextColor (color validation)', () => { + (getSettingValue as any).mockImplementation((key: string, def: any) => { + if (key === '🔍MagnifyGlass.InfoPanelTextColor') return 'not-a-color'; + return def; + }); + stateManager.loadSettings(); + expect(stateManager.state.settings['🔍MagnifyGlass.InfoPanelTextColor']).toBe('#6b7280'); // Default + }); + + it('should validate InfoPanelFontFamily (allowlist validation)', () => { + (getSettingValue as any).mockImplementation((key: string, def: any) => { + if (key === '🔍MagnifyGlass.InfoPanelFontFamily') return 'DangerousFont'; + return def; + }); + stateManager.loadSettings(); + expect(stateManager.state.settings['🔍MagnifyGlass.InfoPanelFontFamily']).toBe('System Default'); // Default + }); +});