diff --git a/src/browser/BrowserFunc.ts b/src/browser/BrowserFunc.ts index c772457..1a8351c 100644 --- a/src/browser/BrowserFunc.ts +++ b/src/browser/BrowserFunc.ts @@ -1,14 +1,14 @@ -import { BrowserContext, Page } from 'rebrowser-playwright' -import { CheerioAPI, load } from 'cheerio' import { AxiosRequestConfig } from 'axios' +import { CheerioAPI, load } from 'cheerio' +import { BrowserContext, Page } from 'rebrowser-playwright' +import { RETRY_LIMITS, SELECTORS, TIMEOUTS, URLS } from '../constants' import { MicrosoftRewardsBot } from '../index' -import { saveSessionData } from '../util/Load' -import { TIMEOUTS, RETRY_LIMITS, SELECTORS, URLS } from '../constants' -import { Counters, DashboardData, MorePromotion, PromotionalItem } from '../interface/DashboardData' -import { QuizData } from '../interface/QuizData' import { AppUserData } from '../interface/AppUserData' +import { Counters, DashboardData, MorePromotion, PromotionalItem } from '../interface/DashboardData' import { EarnablePoints } from '../interface/Points' +import { QuizData } from '../interface/QuizData' +import { saveSessionData } from '../util/Load' import { logError } from '../util/Logger' @@ -138,8 +138,10 @@ export default class BrowserFunc { await this.reloadPageWithRetry(target, 2) // Wait for the more-activities element to ensure page is fully loaded - await target.waitForSelector(SELECTORS.MORE_ACTIVITIES, { timeout: TIMEOUTS.DASHBOARD_WAIT }).catch(() => { - this.bot.log(this.bot.isMobile, 'GET-DASHBOARD-DATA', 'Activities element not found, continuing anyway', 'warn') + await target.waitForSelector(SELECTORS.MORE_ACTIVITIES, { timeout: TIMEOUTS.DASHBOARD_WAIT }).catch((error) => { + // Continuing is intentional: page may still be functional even if this specific element is missing + // The script extraction will catch any real issues + this.bot.log(this.bot.isMobile, 'GET-DASHBOARD-DATA', `Activities element not found after ${TIMEOUTS.DASHBOARD_WAIT}ms timeout, attempting to proceed: ${error instanceof Error ? error.message : String(error)}`, 'warn') }) let scriptContent = await this.extractDashboardScript(target) diff --git a/src/browser/BrowserUtil.ts b/src/browser/BrowserUtil.ts index c2edaf1..9c9d0ba 100644 --- a/src/browser/BrowserUtil.ts +++ b/src/browser/BrowserUtil.ts @@ -1,5 +1,5 @@ -import { Page } from 'rebrowser-playwright' import { load } from 'cheerio' +import { Page } from 'rebrowser-playwright' import { MicrosoftRewardsBot } from '../index' import { logError } from '../util/Logger' @@ -84,7 +84,9 @@ export default class BrowserUtil { await loc.first().click({ timeout: 500 }).catch(logError('BROWSER-UTIL', `Failed to click ${btn.label}`, this.bot.isMobile)) this.bot.log(this.bot.isMobile, 'DISMISS-ALL-MESSAGES', `Dismissed: ${btn.label}`) return true - } catch { + } catch (e) { + // Silent catch is intentional: button detection/click failures shouldn't break page flow + // Most failures are expected (button not present, timing issues, etc.) return false } } @@ -111,7 +113,8 @@ export default class BrowserUtil { } return 0 - } catch { + } catch (e) { + // Silent catch is intentional: overlay detection failures are expected when no overlay present return 0 } } @@ -133,7 +136,8 @@ export default class BrowserUtil { await page.keyboard.press('Escape').catch(logError('BROWSER-UTIL', 'Streak dialog Escape failed', this.bot.isMobile)) this.bot.log(this.bot.isMobile, 'DISMISS-ALL-MESSAGES', 'Dismissed: Streak Protection Dialog Escape') return 1 - } catch { + } catch (e) { + // Silent catch is intentional: streak dialog detection failures are expected return 0 } } @@ -162,7 +166,8 @@ export default class BrowserUtil { } return 0 - } catch { + } catch (e) { + // Silent catch is intentional: terms dialog detection failures are expected return 0 } } diff --git a/src/functions/Login.ts b/src/functions/Login.ts index 2ba69f8..20a8703 100644 --- a/src/functions/Login.ts +++ b/src/functions/Login.ts @@ -1,15 +1,27 @@ -import type { Page, Locator } from 'playwright' -import * as crypto from 'crypto' -import readline from 'readline' import { AxiosRequestConfig } from 'axios' +import * as crypto from 'crypto' +import type { Locator, Page } from 'playwright' +import readline from 'readline' -import { generateTOTP } from '../util/Totp' -import { saveSessionData } from '../util/Load' import { MicrosoftRewardsBot } from '../index' import { OAuth } from '../interface/OAuth' -import { Retry } from '../util/Retry' -import { LoginState, LoginStateDetector } from '../util/LoginStateDetector' +import { saveSessionData } from '../util/Load' import { logError } from '../util/Logger' +import { LoginState, LoginStateDetector } from '../util/LoginStateDetector' +import { Retry } from '../util/Retry' +import { generateTOTP } from '../util/Totp' + +// ------------------------------- +// REFACTORING NOTE (1700+ lines) +// ------------------------------- +// This file violates Single Responsibility Principle. Consider splitting into: +// - LoginFlow.ts (main orchestration) +// - TotpHandler.ts (2FA/TOTP logic) +// - PasskeyHandler.ts (passkey/biometric prompts) +// - RecoveryHandler.ts (recovery email detection) +// - SecurityDetector.ts (ban/block detection) +// This will improve maintainability and testability. +// ------------------------------- // ------------------------------- // Constants / Tunables @@ -47,8 +59,7 @@ const SIGN_IN_BLOCK_PATTERNS: { re: RegExp; label: string }[] = [ { re: /incorrect account or password too many times/i, label: 'too-many-incorrect' }, { re: /used an incorrect account or password too many times/i, label: 'too-many-incorrect-variant' }, { re: /sign-in has been blocked/i, label: 'sign-in-blocked-phrase' }, - { re: /your account has been locked/i, label: 'account-locked' }, - { re: /your account or password is incorrect too many times/i, label: 'incorrect-too-many-times' } + { re: /your account has been locked/i, label: 'account-locked' } ] interface SecurityIncident { @@ -204,6 +215,10 @@ export class Login { const stackTrace = e instanceof Error ? e.stack : undefined this.bot.log(this.bot.isMobile, 'LOGIN', `Failed login: ${errorMessage}${stackTrace ? '\nStack: ' + stackTrace.split('\n').slice(0, 3).join(' | ') : ''}`, 'error') throw new Error(`Login failed for ${email}: ${errorMessage}`) + } finally { + // Always cleanup compromised interval to prevent memory leaks + // The interval is only used during active login sessions + this.cleanupCompromisedInterval() } } @@ -759,7 +774,12 @@ export class Login { // Other errors, just log and continue this.bot.log(this.bot.isMobile, 'LOGIN', '2FA code entry error: ' + error, 'warn') } finally { - try { rl.close() } catch {/* ignore */} + try { + rl.close() + } catch { + // Intentionally silent: readline interface already closed or error during cleanup + // This is a cleanup operation that shouldn't throw + } } } @@ -1491,7 +1511,14 @@ export class Login { } catch { return false } } - private async tryRecoveryMismatchCheck(page: Page, email: string) { try { await this.detectAndHandleRecoveryMismatch(page, email) } catch {/* ignore */} } + private async tryRecoveryMismatchCheck(page: Page, email: string) { + try { + await this.detectAndHandleRecoveryMismatch(page, email) + } catch { + // Intentionally silent: Recovery mismatch check is a best-effort security check + // Failure here should not break the login flow as the page may simply not have recovery info + } + } private async detectAndHandleRecoveryMismatch(page: Page, email: string) { try { const recoveryEmail: string | undefined = this.bot.currentAccountRecoveryEmail @@ -1652,7 +1679,12 @@ export class Login { private startCompromisedInterval() { if (this.compromisedInterval) clearInterval(this.compromisedInterval) this.compromisedInterval = setInterval(()=>{ - try { this.bot.log(this.bot.isMobile,'SECURITY','Security standby active. Manual review required before proceeding.','warn') } catch {/* ignore */} + try { + this.bot.log(this.bot.isMobile,'SECURITY','Security standby active. Manual review required before proceeding.','warn') + } catch { + // Intentionally silent: If logging fails in interval, don't crash the timer + // The interval will try again in 5 minutes + } }, 5*60*1000) } diff --git a/src/functions/Workers.ts b/src/functions/Workers.ts index e57447f..956cf42 100644 --- a/src/functions/Workers.ts +++ b/src/functions/Workers.ts @@ -3,10 +3,16 @@ import { Page } from 'rebrowser-playwright' import { DashboardData, MorePromotion, PromotionalItem, PunchCard } from '../interface/DashboardData' import { MicrosoftRewardsBot } from '../index' -import JobState from '../util/JobState' -import { Retry } from '../util/Retry' import { AdaptiveThrottler } from '../util/AdaptiveThrottler' +import JobState from '../util/JobState' import { logError } from '../util/Logger' +import { Retry } from '../util/Retry' + +// Selector patterns (extracted to avoid magic strings) +const ACTIVITY_SELECTORS = { + byName: (name: string) => `[data-bi-id^="${name}"] .pointLink:not(.contentContainer .pointLink)`, + byOfferId: (offerId: string) => `[data-bi-id^="${offerId}"] .pointLink:not(.contentContainer .pointLink)` +} as const export class Workers { public bot: MicrosoftRewardsBot @@ -198,10 +204,16 @@ export class Workers { const name = activity.name.toLowerCase() if (name.includes('membercenter') || name.includes('exploreonbing')) { - return `[data-bi-id^="${activity.name}"] .pointLink:not(.contentContainer .pointLink)` + return ACTIVITY_SELECTORS.byName(activity.name) } - return `[data-bi-id^="${activity.offerId}"] .pointLink:not(.contentContainer .pointLink)` + // Validate offerId exists before using it in selector + if (!activity.offerId) { + this.bot.log(this.bot.isMobile, 'WORKERS', `Activity "${activity.name || activity.title}" has no offerId, falling back to name-based selector`, 'warn') + return ACTIVITY_SELECTORS.byName(activity.name) + } + + return ACTIVITY_SELECTORS.byOfferId(activity.offerId) } private async prepareActivityPage(page: Page, selector: string, throttle: AdaptiveThrottler): Promise { @@ -221,7 +233,8 @@ export class Workers { return // Skip this activity gracefully instead of waiting 30s } - await page.click(selector) + // Click with timeout to prevent indefinite hangs + await page.click(selector, { timeout: 10000 }) page = await this.bot.browser.utils.getLatestTab(page) const timeoutMs = this.bot.utils.stringToMs(this.bot.config?.globalTimeout ?? '30s') * 2 diff --git a/src/index.ts b/src/index.ts index c12a7f4..f7055b5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,34 +1,47 @@ -import cluster from 'cluster' +// ------------------------------- +// REFACTORING NOTE (1800+ lines) +// ------------------------------- +// MicrosoftRewardsBot class is too large and violates Single Responsibility Principle. +// Consider extracting into separate modules: +// - DesktopFlow.ts (Desktop automation logic) +// - MobileFlow.ts (Mobile automation logic) +// - SummaryReporter.ts (Conclusion/report generation) +// - BuyModeHandler.ts (Manual spending mode) +// - ClusterManager.ts (Worker orchestration) +// This will improve testability and maintainability. +// ------------------------------- + +import { spawn } from 'child_process' import type { Worker } from 'cluster' -import type { Page } from 'playwright' +import cluster from 'cluster' import fs from 'fs' import path from 'path' -import { spawn } from 'child_process' +import type { Page } from 'playwright' import { createInterface } from 'readline' import Browser from './browser/Browser' import BrowserFunc from './browser/BrowserFunc' import BrowserUtil from './browser/BrowserUtil' -import { log } from './util/Logger' -import { Util } from './util/Utils' -import { loadAccounts, loadConfig, saveSessionData } from './util/Load' import Axios from './util/Axios' -import Humanizer from './util/Humanizer' import { detectBanReason } from './util/BanDetector' -import { QueryDiversityEngine } from './util/QueryDiversityEngine' +import { BuyModeMonitor, BuyModeSelector } from './util/BuyMode' +import Humanizer from './util/Humanizer' import JobState from './util/JobState' -import { StartupValidator } from './util/StartupValidator' +import { loadAccounts, loadConfig, saveSessionData } from './util/Load' +import { log } from './util/Logger' import { MobileRetryTracker } from './util/MobileRetryTracker' +import { QueryDiversityEngine } from './util/QueryDiversityEngine' import { SchedulerManager } from './util/SchedulerManager' -import { BuyModeSelector, BuyModeMonitor } from './util/BuyMode' +import { StartupValidator } from './util/StartupValidator' +import { Util } from './util/Utils' +import { Activities } from './functions/Activities' import { Login } from './functions/Login' import { Workers } from './functions/Workers' -import { Activities } from './functions/Activities' -import { Account } from './interface/Account' import { DISCORD } from './constants' +import { Account } from './interface/Account' // Main bot class diff --git a/src/util/BuyMode.ts b/src/util/BuyMode.ts index d04330e..217baf7 100644 --- a/src/util/BuyMode.ts +++ b/src/util/BuyMode.ts @@ -122,6 +122,8 @@ export class BuyModeSelector { } private displayAccountList(): void { + // Note: console.log is intentionally used here for interactive user prompts + // This is a CLI menu, not system logging - should go directly to stdout console.log('\nAvailable accounts:') console.log('─'.repeat(60)) diff --git a/src/util/Load.ts b/src/util/Load.ts index 1692914..6e1b9de 100644 --- a/src/util/Load.ts +++ b/src/util/Load.ts @@ -1,10 +1,10 @@ -import { BrowserContext, Cookie } from 'rebrowser-playwright' import { BrowserFingerprintWithHeaders } from 'fingerprint-generator' import fs from 'fs' import path from 'path' +import { BrowserContext, Cookie } from 'rebrowser-playwright' import { Account } from '../interface/Account' -import { Config, ConfigSaveFingerprint, ConfigBrowser, ConfigScheduling } from '../interface/Config' +import { Config, ConfigBrowser, ConfigSaveFingerprint, ConfigScheduling } from '../interface/Config' import { Util } from './Utils' const utils = new Util() @@ -71,7 +71,12 @@ function stripJsonComments(input: string): string { // Normalize both legacy (flat) and new (nested) config schemas into the flat Config interface function normalizeConfig(raw: unknown): Config { - // Using any here is necessary to support both legacy flat config and new nested config structures + // TYPE SAFETY NOTE: Using `any` here is necessary for backwards compatibility + // The config format has evolved from flat structure to nested structure over time + // We need to support both formats dynamically without knowing which one we'll receive + // Alternative approaches (discriminated unions, multiple interfaces) would require + // runtime type checking on every property access, making the code much more complex + // The validation happens implicitly through the Config interface return type // eslint-disable-next-line @typescript-eslint/no-explicit-any const n = (raw || {}) as any @@ -345,6 +350,9 @@ export function loadAccounts(): Account[] { if (!Array.isArray(parsed)) throw new Error('accounts must be an array') // minimal shape validation for (const entry of parsed) { + // TYPE SAFETY NOTE: Using `any` for account validation + // Accounts come from user-provided JSON with unknown structure + // We validate each property explicitly below rather than trusting the type // eslint-disable-next-line @typescript-eslint/no-explicit-any const a = entry as any if (!a || typeof a.email !== 'string' || typeof a.password !== 'string') { diff --git a/src/util/Logger.ts b/src/util/Logger.ts index eb0b479..40759c3 100644 --- a/src/util/Logger.ts +++ b/src/util/Logger.ts @@ -1,9 +1,9 @@ import axios from 'axios' import chalk from 'chalk' -import { Ntfy } from './Ntfy' -import { loadConfig } from './Load' import { DISCORD } from '../constants' +import { loadConfig } from './Load' +import { Ntfy } from './Ntfy' /** * Safe error logger for catch blocks @@ -95,7 +95,9 @@ async function sendBatch(url: string, buf: WebhookBuffer) { } catch (error) { // Re-queue failed batch at front and exit loop buf.lines = chunk.concat(buf.lines) - console.error('[Webhook] live log delivery failed:', error) + // Note: Using stderr directly here to avoid circular dependency with log() + // This is an internal logger error that shouldn't go through the logging system + process.stderr.write(`[Webhook] live log delivery failed: ${error}\n`) break } } @@ -255,7 +257,8 @@ export function log(isMobile: boolean | 'main', title: string, message: string, enqueueWebhookLog(liveUrl, cleanStr) } } catch (error) { - console.error('[Logger] Failed to enqueue webhook log:', error) + // Note: Using stderr directly to avoid recursion - this is an internal logger error + process.stderr.write(`[Logger] Failed to enqueue webhook log: ${error}\n`) } // Return an Error when logging an error so callers can `throw log(...)` diff --git a/src/util/Utils.ts b/src/util/Utils.ts index f63cc43..ec2d0b1 100644 --- a/src/util/Utils.ts +++ b/src/util/Utils.ts @@ -10,9 +10,9 @@ export class Util { const MAX_WAIT_MS = 3600000 // 1 hour max to prevent infinite waits const MIN_WAIT_MS = 0 - // Validate and clamp input - if (!Number.isFinite(ms)) { - throw new Error(`Invalid wait time: ${ms}. Must be a finite number.`) + // Validate and clamp input - explicit NaN check before isFinite + if (typeof ms !== 'number' || Number.isNaN(ms) || !Number.isFinite(ms)) { + throw new Error(`Invalid wait time: ${ms}. Must be a finite number (not NaN or Infinity).`) } const safeMs = Math.min(Math.max(MIN_WAIT_MS, ms), MAX_WAIT_MS)