DollhouseMCP

# Vulnerabilities and Bugs Fix Plan for v1.9.10 *Date: September 27, 2025* ## Summary - **14 Vulnerabilities** (all BLOCKER severity) - **58 Bugs** (1 CRITICAL, 57 MAJOR) ## Priority 1: Vulnerabilities (14 total) ### 1. GitHub Token in Code (1 issue) - FALSE POSITIVE **File**: `run-sync-test-with-pat.sh:7` **Issue**: Expired token used as example (already documented as intentional) **Action**: Mark as won't fix in SonarCloud - this is a best practice example token ### 2. GitHub Actions Command Injection (13 issues) **Files**: Multiple workflow files - `readme-sync.yml:69` - `version-update.yml:43,44,61,144,146,148,158,159,160,165,167` - `branch-protection.yml:134` **Issue**: User-controlled data used directly in run blocks (potential command injection) **Action Required**: Use environment variables instead of direct interpolation #### Example Fix Pattern: ```yaml # VULNERABLE: - run: echo "Version is ${{ github.event.inputs.version }}" # SECURE: - run: echo "Version is $VERSION" env: VERSION: ${{ github.event.inputs.version }} ``` ## Priority 2: Bugs (58 total) ### Critical Bugs (1) **File**: `persona-lifecycle.test.ts:292` **Issue**: Unsafe usage of ThrowStatement **Action**: Review error handling in test file ### Major Bugs by Category #### 1. Control Characters (16 issues) **Pattern**: S6324 - Remove control characters **Files**: - `Memory.ts:60` (6 occurrences) - `submitToPortfolioTool.ts:734,735,796` - `searchUtils.ts:110` (2 occurrences) - `InputValidator.ts:14` (2 occurrences) **Fix**: Remove or escape control characters in strings #### 2. Regex Precedence (9 issues) **Pattern**: S5850 - Make regex operator precedence explicit **Files**: - `PortfolioDownloader.ts:110` - `YamlSecurityFormatting.test.ts:83` - `index.ts:3911` - `submitToPortfolioTool.ts:1957` - `filesystem.ts:38` - `InputValidator.ts:26` **Fix**: Add parentheses to clarify regex precedence ```javascript // Before: /^foo|bar$/ // After: /^(foo|bar)$/ ``` #### 3. Identical Conditions (4 issues) **Pattern**: S3923 - Identical code blocks **Files**: - `update-version.mjs:255` - `persona-lifecycle.test.ts:254` - `PersonaSubmitter.ts:180` **Fix**: Refactor to remove redundant conditions #### 4. Unreachable Code (2 issues) **Pattern**: S1763 **Files**: - `GitHubPortfolioIndexer.ts:331` - `UnifiedIndexManager.ts:1250` **Fix**: Remove unreachable code after return/throw statements #### 5. Other Issues - **Promise void return** (`GitHubRateLimiter.ts:147`) - **Missing reduce initial value** (`PerformanceMonitor.ts:264`) - **Identical sub-expressions** (`qa-performance-testing.js:138`) - **Unused object instantiation** (`Template.test.ts:38`) ## Fix Priority Order ### Phase 1: Quick Wins (30 mins) 1. Remove control characters (16 bugs) 2. Fix regex precedence with parentheses (9 bugs) 3. Remove unreachable code (2 bugs) 4. Add reduce() initial value (1 bug) ### Phase 2: GitHub Actions Security (1 hour) 1. Fix all 13 command injection vulnerabilities in workflows 2. Test workflows still function correctly ### Phase 3: Logic Fixes (30 mins) 1. Fix identical conditions (4 bugs) 2. Fix promise/void return issue 3. Fix test issues ## Verification Plan 1. Fix each category of issues 2. Run tests after each category 3. Verify SonarCloud analysis shows improvements 4. Document any intentional patterns that shouldn't be fixed ## Expected Outcome - 0 vulnerabilities (1 marked as won't fix, 13 fixed) - Significantly reduced bug count - Green quality gate maintained - More secure GitHub Actions workflows