From dc603b8b49d619e33f11ad255325e2929f5f408f Mon Sep 17 00:00:00 2001 From: Lucas 'Paperboy' Rose-Winters Date: Mon, 20 Oct 2025 07:50:43 +1100 Subject: [PATCH] feat(inventory): granular item-level validation and auto-capitalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhances validation to clean corrupted items at load time while preserving valid ones, rather than discarding entire sections. Also auto-capitalizes first letter of items for consistency. New capability - Granular Item Cleaning: 1. **cleanItemString()** (src/utils/security.js): - Parses item string, removes bad items, re-serializes clean ones - Applies ALL parsing rules: markdown, sanitization, length limits - Used at load time to clean persisted data immediately - Returns "None" if no valid items remain 2. **Enhanced validateStoredInventory()**: - Now cleans items within each location - Only removes locations if ALL items are invalid - Example: "Home": "Sword, __proto__, Shield" → "Home": "Sword, Shield" - Example: "Bad": "__proto__, constructor" → location removed 3. **Enhanced validateInventoryStructure()** (src/core/persistence.js): - Cleans onPerson, stored, and assets at load time - Logs exactly what was cleaned for debugging - Auto-saves cleaned data back to storage Auto-Capitalization: - Added to cleanSingleItem() in itemParser.js - Capitalizes first letter of each item after all cleaning - Preserves rest of case: "iPhone" → "iPhone" (not "Iphone") - Examples: "sword" → "Sword", "3x potions" → "3x potions" Behavior examples: Before (threw away entire array): - "Home": "Sword, " + "A".repeat(600) + ", Shield" → Entire location lost After (granular cleaning): - "Home": "Sword, " + "A".repeat(600) + ", Shield" → "Home": "Sword, AAA...(500 chars), Shield" Before (kept corrupted data): - onPerson: "sword, __proto__, shield" → Stored as-is, filtered only at render After (cleaned at load): - onPerson: "Sword, Shield" → Cleaned and saved immediately, capitalized Benefits: - ✓ Preserves valid items when some are corrupted - ✓ Cleans data at source, not just at render - ✓ Detailed logging of what was cleaned - ✓ Consistent capitalization across all items - ✓ Single source of truth for "valid item" --- src/core/persistence.js | 18 +++++++++++++- src/utils/itemParser.js | 8 ++++++- src/utils/security.js | 52 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/core/persistence.js b/src/core/persistence.js index 30e04cd..75e53e7 100644 --- a/src/core/persistence.js +++ b/src/core/persistence.js @@ -15,7 +15,7 @@ import { FEATURE_FLAGS } from './state.js'; import { migrateInventory } from '../utils/migration.js'; -import { validateStoredInventory } from '../utils/security.js'; +import { validateStoredInventory, cleanItemString } from '../utils/security.js'; const extensionName = 'third-party/rpg-companion-sillytavern'; @@ -212,6 +212,14 @@ function validateInventoryStructure(inventory, source) { console.warn(`[RPG Companion] Invalid onPerson from ${source}, resetting to "None"`); inventory.onPerson = "None"; needsSave = true; + } else { + // Clean items in onPerson (removes corrupted/dangerous items) + const cleanedOnPerson = cleanItemString(inventory.onPerson); + if (cleanedOnPerson !== inventory.onPerson) { + console.warn(`[RPG Companion] Cleaned corrupted items from onPerson inventory (${source})`); + inventory.onPerson = cleanedOnPerson; + needsSave = true; + } } // Validate stored field (CRITICAL for Bug #3) @@ -234,6 +242,14 @@ function validateInventoryStructure(inventory, source) { console.warn(`[RPG Companion] Invalid assets from ${source}, resetting to "None"`); inventory.assets = "None"; needsSave = true; + } else { + // Clean items in assets (removes corrupted/dangerous items) + const cleanedAssets = cleanItemString(inventory.assets); + if (cleanedAssets !== inventory.assets) { + console.warn(`[RPG Companion] Cleaned corrupted items from assets inventory (${source})`); + inventory.assets = cleanedAssets; + needsSave = true; + } } // Persist repairs if needed diff --git a/src/utils/itemParser.js b/src/utils/itemParser.js index 4351312..11651cb 100644 --- a/src/utils/itemParser.js +++ b/src/utils/itemParser.js @@ -187,7 +187,7 @@ export function parseItems(itemString) { /** * Cleans a single item string (helper for parseItems) - * Removes list markers, wrapping quotes, and trims + * Removes list markers, wrapping quotes, trims, and capitalizes first letter * * @param {string} item - Single item string to clean * @returns {string|null} Cleaned item or null if empty/invalid @@ -222,6 +222,12 @@ function cleanSingleItem(item) { return null; } + // Capitalize first letter for consistency + // Preserves rest of string case (e.g., "iPhone" stays "iPhone", not "Iphone") + if (cleaned.length > 0) { + cleaned = cleaned.charAt(0).toUpperCase() + cleaned.slice(1); + } + return cleaned; } diff --git a/src/utils/security.js b/src/utils/security.js index 17f7ad5..a59712f 100644 --- a/src/utils/security.js +++ b/src/utils/security.js @@ -3,6 +3,8 @@ * Handles input sanitization and validation to prevent security vulnerabilities */ +import { parseItems, serializeItems } from './itemParser.js'; + /** * List of dangerous property names that could cause prototype pollution * or shadow critical object methods. @@ -99,6 +101,7 @@ export function sanitizeItemName(name) { /** * Validates and cleans a stored inventory object. * Ensures all keys are safe property names and all values are strings. + * Cleans items within each location (removes corrupted/dangerous items). * Prevents prototype pollution attacks via object keys. * * @param {Object} stored - Raw stored inventory object @@ -108,9 +111,15 @@ export function sanitizeItemName(name) { * validateStoredInventory({ "Home": "Sword, Shield" }) * // → { "Home": "Sword, Shield" } * + * validateStoredInventory({ "Home": "Sword, __proto__, Shield" }) + * // → { "Home": "Sword, Shield" } (dangerous item removed) + * * validateStoredInventory({ "__proto__": "malicious" }) * // → {} (dangerous key removed, logged) * + * validateStoredInventory({ "BadLocation": "__proto__, constructor" }) + * // → {} (location removed because all items were invalid, logged) + * * validateStoredInventory(null) * // → {} (invalid input, returns empty object) */ @@ -143,8 +152,15 @@ export function validateStoredInventory(stored) { continue; } - // Add to cleaned object - cleaned[sanitizedKey] = value; + // Clean items within this location (removes corrupted/dangerous items) + const cleanedValue = cleanItemString(value); + + // Only add location if it has valid items remaining + if (cleanedValue && cleanedValue !== 'None' && cleanedValue.toLowerCase() !== 'none') { + cleaned[sanitizedKey] = cleanedValue; + } else { + console.warn(`[RPG Companion] Location "${sanitizedKey}" had no valid items after cleaning, removing location`); + } } return cleaned; @@ -156,3 +172,35 @@ export function validateStoredInventory(stored) { * @constant {number} */ export const MAX_ITEMS_PER_SECTION = 500; + +/** + * Cleans an item string by parsing and re-serializing. + * Removes corrupted, dangerous, or invalid items while preserving valid ones. + * Applies ALL parsing rules: markdown stripping, sanitization, length limits, etc. + * + * This is used at LOAD time to clean persisted data immediately, not just at render time. + * + * @param {string} itemString - Raw item string (possibly corrupted) + * @returns {string} Clean item string with only valid items, or "None" if no valid items + * + * @example + * cleanItemString("Sword, Shield") // "Sword, Shield" (unchanged) + * cleanItemString("Sword, __proto__, Shield") // "Sword, Shield" (dangerous item removed) + * cleanItemString("A".repeat(600) + ", Sword") // "AAA... (truncated), Sword" + * cleanItemString("**Sword**, *Shield*") // "Sword, Shield" (markdown stripped) + * cleanItemString("__proto__, constructor") // "None" (all items invalid) + */ +export function cleanItemString(itemString) { + // Parse using robust parser (handles all edge cases, sanitizes each item) + // This applies: newlines→commas, markdown stripping, parenthesis-aware splitting, + // sanitizeItemName() validation, length limits, max items limit + const items = parseItems(itemString); + + // If no valid items remain after parsing/sanitization, return "None" + if (items.length === 0) { + return "None"; + } + + // Re-serialize clean items back to string format + return serializeItems(items); +}