From 2cd9853902684d87bd2cd9e4cc3377c5990804bb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 21 Mar 2026 22:42:26 +0000 Subject: [PATCH 1/3] Add comprehensive application audit report Detailed security, performance, code quality, and architecture audit covering all 3,830 lines of the Threads DraftCraft Chrome extension. https://claude.ai/code/session_01Na7TPMpUpohXy29azuD8fD --- AUDIT.md | 381 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 381 insertions(+) create mode 100644 AUDIT.md diff --git a/AUDIT.md b/AUDIT.md new file mode 100644 index 0000000..922d1a2 --- /dev/null +++ b/AUDIT.md @@ -0,0 +1,381 @@ +# Threads DraftCraft - Detaljan Audit Aplikacije + +**Datum:** 2026-03-21 +**Verzija:** 1.0.0 +**Tip aplikacije:** Chrome Extension (Manifest V3) +**Ukupno linija koda:** ~3,830 + +--- + +## 1. PREGLED ARHITEKTURE + +### Struktura +``` +background/background.js (433 LOC) - Service worker, lifecycle, messaging +content/content.js (1,351 LOC) - DOM manipulacija na threads.com +content/content.css (446 LOC) - Stilovi za content script +popup/popup.html (159 LOC) - Popup UI +popup/popup.js (611 LOC) - Popup kontroler +popup/popup.css (783 LOC) - Popup stilovi +manifest.json (47 LOC) - Extension konfiguracija +``` + +### Ocjena arhitekture: 7/10 +- Dobra separacija (background/content/popup) +- Class-based pristup je konzistentan +- Nema build sistema, lintinga, ni testova + +--- + +## 2. SIGURNOSNI PROBLEMI + +### 2.1 KRITIČNO: innerHTML koriscenje bez sanitizacije + +**Fajlovi:** `content/content.js:890`, `content/content.js:1258-1268`, `content/content.js:1309-1324`, `popup/popup.js:548-561` + +```javascript +// content.js:890 - Sort indicator +statusIndicator.innerHTML = `...${this.sortOrder}...`; + +// content.js:1258 - Time indicator fallback +timeIndicator.innerHTML = `
πŸ“… ${draft.scheduledTimeStr}
`; + +// content.js:1309 - Count badge +countBadge.innerHTML = `πŸ“Š ${this.drafts.length}`; + +// popup.js:548 - Info message +infoDiv.innerHTML = `
πŸ“Œ Navigate to Threads.com...
`; +``` + +**Rizik:** Dok su `this.sortOrder` i `this.drafts.length` kontrolisane vrijednosti, `draft.scheduledTimeStr` dolazi iz parsiranog DOM sadrzaja Threads.com stranice. Ako bi napadac kontrolisao sadrzaj koji se renderuje na Threads.com (npr. kroz draft tekst koji sadrzi ` diff --git a/popup/popup.js b/popup/popup.js index 2a00af2..85dba1e 100644 --- a/popup/popup.js +++ b/popup/popup.js @@ -21,7 +21,7 @@ class ThreadsDraftCraftPopup { * Initialize the popup */ async init() { - console.log('[Threads DraftCraft] Popup initialized'); + // Popup initialized // Load current settings await this.loadSettings(); @@ -55,7 +55,7 @@ class ThreadsDraftCraftPopup { this.settings = { ...this.settings, ...result }; } catch (error) { - console.error('[Threads DraftCraft] Failed to load settings:', error); + // Settings load failed this.showError('Failed to load settings'); } } @@ -66,9 +66,9 @@ class ThreadsDraftCraftPopup { async saveSettings() { try { await chrome.storage.sync.set(this.settings); - console.log('[Threads DraftCraft] Settings saved:', this.settings); + // Settings saved } catch (error) { - console.error('[Threads DraftCraft] Failed to save settings:', error); + // Settings save failed this.showError('Failed to save settings'); } } @@ -241,7 +241,7 @@ class ThreadsDraftCraftPopup { this.showNoStatsMessage(); } } catch (error) { - console.error('[Threads DraftCraft] Failed to load draft stats:', error); + // Draft stats load failed this.showNoStatsMessage(); } } @@ -261,7 +261,7 @@ class ThreadsDraftCraftPopup { } if (scheduledDrafts) { - scheduledDrafts.textContent = stats.totalDrafts || '0'; + scheduledDrafts.textContent = stats.scheduledDrafts || stats.totalDrafts || '0'; } if (stats.nextScheduled && nextScheduledContainer && nextDraftText && nextDraftTime) { @@ -275,140 +275,58 @@ class ThreadsDraftCraftPopup { /** - * Handle sort order change + * Generic helper: send a message to the active threads.com tab */ - async handleSortOrderChange(order) { - this.settings.sortOrder = order; - await this.saveSettings(); - - // Send message to content script + async _sendToActiveTab(message) { try { const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); if (tab && tab.url.includes('threads.com')) { - await chrome.tabs.sendMessage(tab.id, { - action: 'changeSortOrder', - sortOrder: order - }); + await chrome.tabs.sendMessage(tab.id, message); } } catch (error) { - console.error('[Threads DraftCraft] Failed to change sort order:', error); + // Tab might not have content script } - - this.showSuccess(`Sort order changed to ${order} first`); } /** - * Handle auto sort toggle + * Generic toggle handler to reduce duplication */ - async handleAutoSortToggle(enabled) { - this.settings.autoSort = enabled; + async _toggleSetting(settingKey, action, value, label) { + this.settings[settingKey] = value; await this.saveSettings(); - - // Send message to content script - try { - const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); - if (tab && tab.url.includes('threads.com')) { - await chrome.tabs.sendMessage(tab.id, { - action: 'toggleAutoSort', - enabled: enabled - }); - } - } catch (error) { - console.error('[Threads DraftCraft] Failed to toggle auto sort:', error); + await this._sendToActiveTab({ action, enabled: value }); + if (typeof value === 'boolean') { + this.showSuccess(value ? `${label} enabled` : `${label} disabled`); + } else { + this.showSuccess(`${label} changed to ${value} first`); } - - this.showSuccess(enabled ? 'Auto sort enabled' : 'Auto sort disabled'); } - /** - * Handle time indicators toggle - */ - async handleTimeIndicatorsToggle(enabled) { - this.settings.showTimeIndicators = enabled; + async handleSortOrderChange(order) { + this.settings.sortOrder = order; await this.saveSettings(); + await this._sendToActiveTab({ action: 'changeSortOrder', sortOrder: order }); + this.showSuccess(`Sort order changed to ${order} first`); + } - // Send message to content script - try { - const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); - if (tab && tab.url.includes('threads.com')) { - await chrome.tabs.sendMessage(tab.id, { - action: 'toggleTimeIndicators', - enabled: enabled - }); - } - } catch (error) { - console.error('[Threads DraftCraft] Failed to toggle time indicators:', error); - } + async handleAutoSortToggle(enabled) { + await this._toggleSetting('autoSort', 'toggleAutoSort', enabled, 'Auto sort'); + } - this.showSuccess(enabled ? 'Time indicators enabled' : 'Time indicators disabled'); + async handleTimeIndicatorsToggle(enabled) { + await this._toggleSetting('showTimeIndicators', 'toggleTimeIndicators', enabled, 'Time indicators'); } - /** - * Handle draft count toggle - */ async handleDraftCountToggle(enabled) { - this.settings.showDraftCount = enabled; - await this.saveSettings(); - - // Send message to content script - try { - const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); - if (tab && tab.url.includes('threads.com')) { - await chrome.tabs.sendMessage(tab.id, { - action: 'toggleDraftCount', - enabled: enabled - }); - } - } catch (error) { - console.error('[Threads DraftCraft] Failed to toggle draft count:', error); - } - - this.showSuccess(enabled ? 'Draft count enabled' : 'Draft count disabled'); + await this._toggleSetting('showDraftCount', 'toggleDraftCount', enabled, 'Draft count'); } - /** - * Handle sort indicator toggle - */ async handleSortIndicatorToggle(enabled) { - this.settings.showSortIndicator = enabled; - await this.saveSettings(); - - // Send message to content script - try { - const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); - if (tab && tab.url.includes('threads.com')) { - await chrome.tabs.sendMessage(tab.id, { - action: 'toggleSortIndicator', - enabled: enabled - }); - } - } catch (error) { - console.error('[Threads DraftCraft] Failed to toggle sort indicator:', error); - } - - this.showSuccess(enabled ? 'Sort indicator enabled' : 'Sort indicator disabled'); + await this._toggleSetting('showSortIndicator', 'toggleSortIndicator', enabled, 'Sort indicator'); } - /** - * Handle date divider toggle - */ async handleDateDividerToggle(enabled) { - this.settings.showDateDivider = enabled; - await this.saveSettings(); - - try { - const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); - if (tab && tab.url.includes('threads.com')) { - await chrome.tabs.sendMessage(tab.id, { - action: 'toggleDateDivider', - enabled: enabled - }); - } - } catch (error) { - console.error('[Threads DraftCraft] Failed to toggle date divider:', error); - } - - this.showSuccess(enabled ? 'Date divider enabled' : 'Date divider disabled'); + await this._toggleSetting('showDateDivider', 'toggleDateDivider', enabled, 'Date divider'); } /** @@ -429,7 +347,7 @@ class ThreadsDraftCraftPopup { await this.loadDraftStats(); this.showSuccess('Drafts refreshed'); } catch (error) { - console.error('[Threads DraftCraft] Failed to refresh drafts:', error); + // Refresh failed this.showError('Failed to refresh drafts'); } finally { this.showLoading(false); @@ -446,7 +364,7 @@ class ThreadsDraftCraftPopup { }); window.close(); } catch (error) { - console.error('[Threads DraftCraft] Failed to open Threads:', error); + // Open Threads failed this.showError('Failed to open Threads.com'); } } @@ -462,7 +380,7 @@ class ThreadsDraftCraftPopup { this.showThreadsNotActiveMessage(); } } catch (error) { - console.error('[Threads DraftCraft] Failed to check tab:', error); + // Tab check failed } } @@ -545,20 +463,10 @@ class ThreadsDraftCraftPopup { // Show info message const infoDiv = document.createElement('div'); infoDiv.className = 'info-message'; - infoDiv.innerHTML = ` -
- πŸ“Œ Navigate to Threads.com to see draft statistics -
- `; + const innerInfo = document.createElement('div'); + innerInfo.className = 'info-message-content'; + innerInfo.textContent = 'Navigate to Threads.com to see draft statistics'; + infoDiv.appendChild(innerInfo); const statsSection = document.querySelector('.stats-section'); if (statsSection && !document.querySelector('.info-message')) { From 2731ba34f2de644c3d7e62272128f71cd4309228 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 23 Mar 2026 02:21:43 +0000 Subject: [PATCH 3/3] Fix remaining issues from second audit pass Bugs fixed: - Add return true to message listener for async response support - Add ping handler for background script content script detection - Fix index=-1 producing wrong scheduling times (new _safeElementIndex helper) - Fix unstable sort for equal timestamps (tiebreaker by originalOrder) - Add missing showDateDivider to background.js getSettings() defaults - Add Date validity check (isNaN) after Date construction - Fix logError missing .catch() on promise chain, simplify with slice(-50) - Add _daysUntilDay input validation for out-of-range day indices Performance: - Combine 4 querySelectorAll calls into single selector in removeExistingEnhancements - Reuse getDateKey() in addDateDividers instead of inline string construction Code quality: - Remove dead code: unreachable m===60 check, commented-out console.log block - Simplify randomNonZeroMinute to one-liner Robustness: - Add document.contains() check before DOM operations in reorderDraftElements - Add ARIA labels to dynamically created time indicators - Add tabindex and aria-live to popup error/success messages for focus management - Add focus() call on error message display https://claude.ai/code/session_01Na7TPMpUpohXy29azuD8fD --- background/background.js | 35 +++++++-------- content/content.js | 97 ++++++++++++++++++++-------------------- popup/popup.html | 4 +- popup/popup.js | 9 ++-- 4 files changed, 71 insertions(+), 74 deletions(-) diff --git a/background/background.js b/background/background.js index 0a0785b..76eae13 100644 --- a/background/background.js +++ b/background/background.js @@ -283,7 +283,8 @@ class ThreadsDraftCraftBackground { autoSort: true, showTimeIndicators: true, showDraftCount: true, - showSortIndicator: true + showSortIndicator: true, + showDateDivider: true }); return result; @@ -373,27 +374,21 @@ class ThreadsDraftCraftBackground { logError(error, context) { bgLog(`ERROR: ${context}:`, error); - // Store error in local storage for debugging (optional) - try { - chrome.storage.local.get('errorLog').then((result) => { - const errorLog = result.errorLog || []; - errorLog.push({ - timestamp: Date.now(), - error: error.toString(), - context: context, - stack: error.stack - }); - - // Keep only last 50 errors - if (errorLog.length > 50) { - errorLog.splice(0, errorLog.length - 50); - } - - chrome.storage.local.set({ errorLog }); + // Store error in local storage for debugging + chrome.storage.local.get('errorLog').then((result) => { + const errorLog = result.errorLog || []; + errorLog.push({ + timestamp: Date.now(), + error: error.toString(), + context: context, + stack: error.stack }); - } catch (storageError) { + + // Keep only last 50 errors + chrome.storage.local.set({ errorLog: errorLog.slice(-50) }); + }).catch((storageError) => { bgLog('ERROR: Failed to log error:', storageError); - } + }); } /** diff --git a/content/content.js b/content/content.js index 1e5b679..51fbd8a 100644 --- a/content/content.js +++ b/content/content.js @@ -135,29 +135,32 @@ class ThreadsDraftCraft { chrome.runtime.onMessage.addListener((message, sender, sendResponse) => { if (message.action === 'changeSortOrder') { this.sortOrder = message.sortOrder; - this.processDrafts(null, true); // Force reprocessing + this.processDrafts(null, true); } else if (message.action === 'toggleAutoSort') { this.autoSort = message.enabled; - this.processDrafts(null, true); // Force reprocessing to apply/remove sorting immediately + this.processDrafts(null, true); } else if (message.action === 'toggleTimeIndicators') { this.showTimeIndicators = message.enabled; - this.processDrafts(null, true); // Force reprocessing to show/hide time indicators + this.processDrafts(null, true); } else if (message.action === 'toggleDraftCount') { this.showDraftCount = message.enabled; - this.processDrafts(null, true); // Force reprocessing to show/hide draft count + this.processDrafts(null, true); } else if (message.action === 'toggleSortIndicator') { this.showSortIndicator = message.enabled; - this.processDrafts(null, true); // Force reprocessing to show/hide sort indicator + this.processDrafts(null, true); } else if (message.action === 'toggleDateDivider') { this.showDateDivider = message.enabled; - this.processDrafts(null, true); // Reprocess to show/hide date dividers + this.processDrafts(null, true); } else if (message.action === 'getDraftStats') { sendResponse({ totalDrafts: this.drafts.length, scheduledDrafts: this.drafts.filter(d => d.scheduledTime).length, nextScheduled: this.getNextScheduledDraft() }); + } else if (message.action === 'ping') { + sendResponse({ success: true }); } + return true; // Keep message channel open for async responses }); } @@ -303,34 +306,18 @@ class ThreadsDraftCraft { const hasMultipleScheduledPosts = (textContent.match(/posting (today|tomorrow) at/g) || []).length > 1; // Only return true if we have clear drafts indicators and no edit indicators - const isDrafts = hasDraftsText && (hasDraftsIndicator || hasMultipleScheduledPosts); - - // if (isDrafts) { - // console.log('[Threads DraftCraft] Confirmed drafts dialog with indicators'); - // } - - return isDrafts; + return hasDraftsText && (hasDraftsIndicator || hasMultipleScheduledPosts); } /** * Remove existing extension enhancements from dialog */ removeExistingEnhancements(dialogElement) { - // Remove extension indicators - const indicators = dialogElement.querySelectorAll('.threads-draftcraft-indicator'); - indicators.forEach(indicator => indicator.remove()); - - // Remove draft count indicators - const countIndicators = dialogElement.querySelectorAll('.threads-draftcraft-count'); - countIndicators.forEach(indicator => indicator.remove()); - - // Remove time indicators and reset processed flags - const timeIndicators = dialogElement.querySelectorAll('.threads-draftcraft-time'); - timeIndicators.forEach(indicator => indicator.remove()); - - // Remove date dividers - const dateDividers = dialogElement.querySelectorAll('.threads-draftcraft-date-divider'); - dateDividers.forEach(divider => divider.remove()); + // Remove all extension-injected elements in a single DOM query + const extensionElements = dialogElement.querySelectorAll( + '.threads-draftcraft-indicator, .threads-draftcraft-count, .threads-draftcraft-time, .threads-draftcraft-date-divider, .threads-draftcraft-time-subtle, .threads-draftcraft-time-info, .threads-draftcraft-status, .threads-draftcraft-count-badge' + ); + extensionElements.forEach(el => el.remove()); // Reset all processed flags on draft elements const processedElements = dialogElement.querySelectorAll('[data-threads-draftcraft-time-added]'); @@ -637,6 +624,9 @@ class ThreadsDraftCraft { * If target is same as current day, returns 7 (next week) */ _daysUntilDay(targetDayIndex, currentDayIndex) { + if (targetDayIndex < 0 || targetDayIndex > 6 || currentDayIndex < 0 || currentDayIndex > 6) { + return 1; // Safe fallback: tomorrow + } let daysUntil = targetDayIndex - currentDayIndex; if (daysUntil <= 0) { daysUntil += 7; // Next week if day has passed or is today @@ -644,6 +634,15 @@ class ThreadsDraftCraft { return daysUntil; } + /** + * Get safe element index within its parent (returns 0 if not found or detached) + */ + _safeElementIndex(element) { + if (!element || !element.parentElement) return 0; + const idx = Array.from(element.parentElement.children).indexOf(element); + return idx >= 0 ? idx : 0; + } + extractScheduledTime(element) { const textContent = element.textContent.toLowerCase(); @@ -679,6 +678,8 @@ class ThreadsDraftCraft { const currentYear = new Date().getFullYear(); const scheduledDate = new Date(currentYear, monthIndex, dateNum, hour24, minutes, 0, 0); + if (isNaN(scheduledDate.getTime())) return null; + if (scheduledDate < new Date()) { scheduledDate.setFullYear(currentYear + 1); } @@ -728,7 +729,7 @@ class ThreadsDraftCraft { scheduledDate.setDate(now.getDate() + daysUntil); // Deterministic time based on element position (not random) - const index = Array.from(element.parentElement?.children || []).indexOf(element); + const index = this._safeElementIndex(element); const deterministicHour = BUSINESS_HOURS_START + ((index * 3) % (BUSINESS_HOURS_END - BUSINESS_HOURS_START)); scheduledDate.setHours(deterministicHour, 0, 0, 0); return scheduledDate; @@ -738,13 +739,13 @@ class ThreadsDraftCraft { // Check for "today" indicators without specific time - deterministic if (textContent.includes('posting today') || textContent.includes('today at')) { - const index = Array.from(element.parentElement?.children || []).indexOf(element); + const index = this._safeElementIndex(element); return new Date(Date.now() + (1 + index * 2) * 60 * 60 * 1000); } // Check for "tomorrow" indicators without specific time - deterministic if (textContent.includes('posting tomorrow') || textContent.includes('tomorrow at')) { - const index = Array.from(element.parentElement?.children || []).indexOf(element); + const index = this._safeElementIndex(element); return new Date(Date.now() + (24 + index * 2) * 60 * 60 * 1000); } @@ -760,8 +761,7 @@ class ThreadsDraftCraft { } // Final fallback: deterministic times based on position - const index = parseInt(element.getAttribute('data-draft-index')) || - Array.from(element.parentElement?.children || []).indexOf(element) || 0; + const index = parseInt(element.getAttribute('data-draft-index')) || this._safeElementIndex(element); return new Date(Date.now() + FALLBACK_MOCK_HOURS[index % FALLBACK_MOCK_HOURS.length] * 60 * 60 * 1000); } @@ -801,15 +801,16 @@ class ThreadsDraftCraft { */ sortDrafts() { this.drafts.sort((a, b) => { - if (!a.scheduledTime && !b.scheduledTime) return 0; + if (!a.scheduledTime && !b.scheduledTime) return a.originalOrder - b.originalOrder; if (!a.scheduledTime) return 1; if (!b.scheduledTime) return -1; - if (this.sortOrder === 'earliest') { - return a.scheduledTime - b.scheduledTime; - } else { - return b.scheduledTime - a.scheduledTime; - } + const timeDiff = this.sortOrder === 'earliest' + ? a.scheduledTime - b.scheduledTime + : b.scheduledTime - a.scheduledTime; + + // Stable sort: use originalOrder as tiebreaker for equal times + return timeDiff !== 0 ? timeDiff : a.originalOrder - b.originalOrder; }); } @@ -901,11 +902,13 @@ class ThreadsDraftCraft { // Find the container that holds all drafts const container = this.drafts[0].element.parentElement; - if (!container) return; + if (!container || !document.contains(container)) return; - // Reorder elements + // Reorder elements (only if still in DOM) this.drafts.forEach((draft) => { - container.appendChild(draft.element); + if (document.contains(draft.element)) { + container.appendChild(draft.element); + } }); } @@ -968,11 +971,7 @@ class ThreadsDraftCraft { }; // Minute generator: never return 00, randomize 1..59 - const randomNonZeroMinute = () => { - let m = Math.floor(Math.random()*59) + 1; // 1..59 - if (m === 60) m = 59; - return m; - }; + const randomNonZeroMinute = () => Math.floor(Math.random() * 59) + 1; const withRandomMinutes = (dt) => { const nd = new Date(dt); @@ -1075,7 +1074,7 @@ class ThreadsDraftCraft { this.drafts.forEach((d) => { const dt = d.scheduledTime instanceof Date ? d.scheduledTime : null; if (!dt) return; - const key = `${dt.getFullYear()}-${String(dt.getMonth() + 1).padStart(2, '0')}-${String(dt.getDate()).padStart(2, '0')}`; + const key = this.getDateKey(dt); dateCounts.set(key, (dateCounts.get(key) || 0) + 1); }); @@ -1212,6 +1211,7 @@ class ThreadsDraftCraft { const timeInfo = document.createElement('span'); timeInfo.className = 'threads-draftcraft-time-info'; timeInfo.textContent = escapeText(draft.scheduledTimeStr); + timeInfo.setAttribute('aria-label', `Scheduled ${draft.scheduledTimeStr}`); // Integrate the time info with the existing posting text postingTextNode.parentElement.appendChild(timeInfo); @@ -1219,6 +1219,7 @@ class ThreadsDraftCraft { // Fallback: if no "Posting" text found, add a subtle indicator at the top const timeIndicator = document.createElement('div'); timeIndicator.className = 'threads-draftcraft-time-subtle'; + timeIndicator.setAttribute('aria-label', `Scheduled ${draft.scheduledTimeStr}`); const innerDiv = document.createElement('div'); innerDiv.className = 'threads-draftcraft-time-subtle-inner'; diff --git a/popup/popup.html b/popup/popup.html index fa2499a..99d1218 100644 --- a/popup/popup.html +++ b/popup/popup.html @@ -140,14 +140,14 @@

πŸ”§ Actions

-