diff --git a/docs/advanced/creating-detection-rules.md b/docs/advanced/creating-detection-rules.md index 7b548276..125d10ca 100644 --- a/docs/advanced/creating-detection-rules.md +++ b/docs/advanced/creating-detection-rules.md @@ -22,14 +22,20 @@ Rules are managed by the [`DetectionRulesManager`](https://github.com/CyberDrain * Load rules at extension startup * Check for updates based on the configured interval (default: 24 hours) * Cache rules locally in browser storage for offline use -* Fall back to local rules ([`rules/detection-rules.json`](https://github.com/CyberDrain/Check/blob/main/rules/detection-rules.json)) if remote fetch fails +* Keep using the last successfully downloaded rules if an update fails +* Fall back to local rules ([`rules/detection-rules.json`](https://github.com/CyberDrain/Check/blob/main/rules/detection-rules.json)) only if rules have never been downloaded **Update Process:** -1. Rules are fetched from the configured remote URL, with a fallback to the local file -2. New rules are cached locally and immediately applied -3. A message is sent to notify other extension components of the update -4. Open tabs require a reload to apply the new rules +1. Rules are fetched from the configured remote URL +2. A response is accepted only if it is a valid rules file (JSON object containing at least one known rules section). Error pages and other unexpected responses are rejected +3. New rules are cached locally and immediately applied +4. A message is sent to notify other extension components of the update +5. Open tabs require a reload to apply the new rules + +**When an update fails:** + +If the remote URL can't be reached or returns an invalid response (for example during a GitHub outage, rate limiting, or a network problem), Check keeps using the last rules it downloaded successfully, even if they are older than the update interval. It retries on the next refresh, waiting at least 5 minutes between attempts. The bundled rules are used only when no rules have been downloaded yet. ## Exclusions diff --git a/docs/settings/detection-rules.md b/docs/settings/detection-rules.md index 128dc4ab..40005bd1 100644 --- a/docs/settings/detection-rules.md +++ b/docs/settings/detection-rules.md @@ -82,6 +82,7 @@ Sometimes you need to update rules immediately: * Go to the Detection Rules section * Click "Update Rules Now" * Wait for the "Rules updated successfully" message + * If the update fails, you'll see an error message. Check keeps using your current rules, so protection continues while you fix the problem ## Understanding the Configuration Overview @@ -116,6 +117,8 @@ If some settings do not appear in your version, it means your organization's IT ### **Problem: Rules won't update** +If an update fails, Check keeps using the last rules it downloaded successfully and tries again later, so you stay protected with the most recent rules it has. + 1. Check your internet connection 2. Try clicking "Update Rules Now" again 3. If using a custom rules URL, verify that the URL is correct diff --git a/scripts/modules/detection-rules-manager.js b/scripts/modules/detection-rules-manager.js index 3825ede4..552f20e9 100644 --- a/scripts/modules/detection-rules-manager.js +++ b/scripts/modules/detection-rules-manager.js @@ -6,6 +6,45 @@ import { chrome, storage } from "../browser-polyfill.js"; import logger from "../utils/logger.js"; +// Top-level sections of a rules file. A downloaded file must contain at least +// one of them, so an error page or unrelated JSON response can't replace good +// rules. +const KNOWN_RULE_SECTIONS = [ + "trusted_login_patterns", + "microsoft_domain_patterns", + "exclusion_system", + "legitimate_discussion_domains", + "m365_detection_requirements", + "blocking_rules", + "allow_rules", + "aad_detection_elements", + "required_elements", + "rules", + "thresholds", + "phishing_indicators", + "legitimate_patterns", + "suspicious_behaviors", + "detection_settings", + "detection_logic", + "rogue_apps_detection", + "domain_squatting", +]; + +// After a failed fetch, wait this long before trying again so an outage or +// rate limit doesn't cause a request on every page load. +// The failure time is persisted so the delay survives service worker restarts. +const FAILED_FETCH_RETRY_DELAY = 5 * 60 * 1000; +const FAILED_FETCH_KEY = "detection_rules_last_failed_fetch"; + +function isValidDetectionRules(rules) { + if (!rules || typeof rules !== "object" || Array.isArray(rules)) { + return false; + } + return KNOWN_RULE_SECTIONS.some((section) => + Object.prototype.hasOwnProperty.call(rules, section) + ); +} + export class DetectionRulesManager { constructor(configManager = null, onRulesUpdated = null) { this.cachedRules = null; @@ -21,6 +60,7 @@ export class DetectionRulesManager { this.onRulesUpdated = onRulesUpdated; this._refreshInFlight = null; this._usingFallback = false; + this._lastFailedFetchAt = 0; } async initialize() { @@ -33,9 +73,27 @@ export class DetectionRulesManager { // Load cached rules first await this.loadFromCache(); + const stored = await storage.local + .get([FAILED_FETCH_KEY]) + .catch(() => ({})); + this._lastFailedFetchAt = stored?.[FAILED_FETCH_KEY] || 0; + + if (!this.cachedRules && this._isInFailureBackoff(Date.now())) { + const response = await fetch(this.fallbackUrl); + if (!response.ok) { + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + } + this.cachedRules = await response.json(); + this.lastUpdate = 0; + this._usingFallback = true; + } + // Check if we need to update const now = Date.now(); - if (now - this.lastUpdate > this.updateInterval) { + if ( + now - this.lastUpdate > this.updateInterval && + !this._isInFailureBackoff(now) + ) { // Update in background this.updateDetectionRules().catch((error) => { logger.warn( @@ -102,19 +160,15 @@ export class DetectionRulesManager { const result = await storage.local.get([this.cacheKey]); const cached = result?.[this.cacheKey]; - if (cached && cached.rules && cached.lastUpdate) { - // Check if cache is still valid - const now = Date.now(); - const cacheAge = now - cached.lastUpdate; - - if (cacheAge < this.updateInterval) { - this.cachedRules = cached.rules; - this.lastUpdate = cached.lastUpdate; - logger.log("Detection rules loaded from cache"); - return true; - } else { - logger.log("Cached detection rules expired, will fetch new ones"); - } + // The cache only holds rules from a successful remote fetch, so use it + // even when it's past the update interval: it's the newest copy we + // have. lastUpdate keeps its age, so a refresh is still triggered. + if (cached && cached.lastUpdate && isValidDetectionRules(cached.rules)) { + this.cachedRules = cached.rules; + this.lastUpdate = cached.lastUpdate; + this._usingFallback = false; + logger.log("Detection rules loaded from cache"); + return true; } return false; @@ -142,7 +196,12 @@ export class DetectionRulesManager { } } - async fetchDetectionRules() { + /** + * @param {object} [outcome] Receives `error` if the remote fetch failed and + * `usingBundled` if the bundled rules were returned, so a caller can tell + * how its own fetch went even if other fetches overlap with it. + */ + async fetchDetectionRules(outcome = {}) { let rules = null; // Try to fetch from remote URL first @@ -162,22 +221,44 @@ export class DetectionRulesManager { } rules = await response.json(); + if (!isValidDetectionRules(rules)) { + throw new Error("Response is not a valid detection rules file"); + } logger.log("Successfully fetched detection rules from remote URL"); // Persist only successful remote fetches so the cache always reflects // the true remote state - never poisoned by bundled fallback content. this._usingFallback = false; + if (this._lastFailedFetchAt) { + await this._setLastFailedFetchAt(0); + } await this.saveToCache(rules); return rules; } catch (error) { logger.warn("Failed to fetch rules from remote URL:", error.message); + outcome.error = error.message; + await this._setLastFailedFetchAt(Date.now()); + } + + // Keep the last successfully downloaded rules rather than falling back + // to the bundled ones, which may be several releases older. lastUpdate + // is left unchanged so the next refresh retries the remote URL. + const hasDownloadedRules = + (this.cachedRules && !this._usingFallback && this.lastUpdate > 0) || + (await this.loadFromCache()); + if (hasDownloadedRules) { + logger.warn( + "Keeping previously downloaded detection rules until the remote URL is reachable" + ); + return this.cachedRules; } } - // Remote fetch failed (or no remote URL configured): serve the bundled - // rules in-memory but DO NOT persist them. lastUpdate stays at 0 so the - // next getDetectionRules() call treats the cache as stale and retries the - // remote URL - the persistent cache remains a record of remote-only state. + // No previously downloaded rules (or no remote URL configured): serve the + // bundled rules in-memory but DO NOT persist them. lastUpdate stays at 0 so + // the next getDetectionRules() call treats the cache as stale and retries + // the remote URL - the persistent cache remains a record of remote-only + // state. try { logger.log( "Falling back to bundled detection rules (in-memory only; will retry remote on next refresh)" @@ -194,6 +275,7 @@ export class DetectionRulesManager { this.cachedRules = rules; this.lastUpdate = 0; // Force next access to re-attempt the remote fetch this._usingFallback = true; + outcome.usingBundled = true; return rules; } catch (error) { logger.error("Failed to load bundled detection rules:", error.message); @@ -201,9 +283,9 @@ export class DetectionRulesManager { } } - async updateDetectionRules() { + async updateDetectionRules(outcome = {}) { try { - const rules = await this.fetchDetectionRules(); + const rules = await this.fetchDetectionRules(outcome); // Notify the background-script wiring so dependent subsystems (e.g. the // domain-squatting detector) can re-initialize with the new rules. @@ -283,11 +365,19 @@ export class DetectionRulesManager { // Cache is past the configured refresh interval. Return the (still usable) // cached rules immediately so page detection isn't blocked, and kick off // a non-blocking remote refresh whose result lands on the NEXT request. - this._scheduleBackgroundRefresh( - `cache age ${Math.round(cacheAge / 60000)}m > interval ${Math.round( - this.updateInterval / 60000 - )}m` - ); + if (!this._isInFailureBackoff(now)) { + this._scheduleBackgroundRefresh( + `cache age ${Math.round(cacheAge / 60000)}m > interval ${Math.round( + this.updateInterval / 60000 + )}m` + ); + } + return this.cachedRules; + } + + // Running on bundled rules after a recent failure: don't block on another + // network attempt until the retry delay has passed. + if (this.cachedRules && this._isInFailureBackoff(now)) { return this.cachedRules; } @@ -304,10 +394,44 @@ export class DetectionRulesManager { } } + _isInFailureBackoff(now) { + const elapsed = now - this._lastFailedFetchAt; + return ( + this._lastFailedFetchAt > 0 && + elapsed >= 0 && + elapsed < Math.min(FAILED_FETCH_RETRY_DELAY, this.updateInterval) + ); + } + + async _setLastFailedFetchAt(timestamp) { + this._lastFailedFetchAt = timestamp; + try { + if (timestamp) { + await storage.local.set({ [FAILED_FETCH_KEY]: timestamp }); + } else { + await storage.local.remove([FAILED_FETCH_KEY]); + } + } catch (error) { + logger.warn("Failed to save detection rules retry state:", error.message); + } + } + async forceUpdate() { logger.log("Forcing detection rules update"); await this.reloadConfiguration(); - return await this.updateDetectionRules(); + const outcome = {}; + const rules = await this.updateDetectionRules(outcome); + + // Report a failed download instead of claiming success. The previous + // rules stay in use. + if (outcome.error) { + throw new Error( + `Could not download detection rules (${outcome.error}). Still using ${ + outcome.usingBundled ? "the bundled rules" : "the previously downloaded rules" + }.` + ); + } + return rules; } getCacheInfo() { diff --git a/tests/detection-rules-manager.test.js b/tests/detection-rules-manager.test.js new file mode 100644 index 00000000..99707813 --- /dev/null +++ b/tests/detection-rules-manager.test.js @@ -0,0 +1,197 @@ +import { test, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert'; +import { setupGlobalChrome } from './helpers/chrome-mock.js'; + +// browser-polyfill.js binds to the global chrome object when it is first +// imported, so install one mock for the whole file before importing. +const chromeMock = setupGlobalChrome(); +const { init: initLogger } = await import('../scripts/utils/logger.js'); +const { DetectionRulesManager } = await import('../scripts/modules/detection-rules-manager.js'); +initLogger({ enabled: false }); + +const HOUR = 60 * 60 * 1000; +const CACHE_KEY = 'detection_rules_cache'; +const BUNDLED_URL = chromeMock.runtime.getURL('rules/detection-rules.json'); + +const makeRules = (version) => ({ version, trusted_login_patterns: [], phishing_indicators: [] }); +const BUNDLED_RULES = makeRules('1.2.0'); +const DOWNLOADED_RULES = makeRules('1.2.5'); +const NEW_RULES = makeRules('1.3.0'); + +const jsonResponse = (body, status = 200) => ({ + ok: status === 200, + status, + statusText: status === 200 ? 'OK' : 'Error', + json: async () => JSON.parse(JSON.stringify(body)) +}); + +let remoteHandler; +let remoteCalls; + +async function seedCache(ageMs) { + await chromeMock.storage.local.set({ + [CACHE_KEY]: { rules: DOWNLOADED_RULES, lastUpdate: Date.now() - ageMs, source: 'remote' } + }); +} + +const storedCache = () => chromeMock.storage.local.getData()[CACHE_KEY]; + +async function startManager() { + const manager = new DetectionRulesManager({ getConfig: async () => ({}) }); + await manager.initialize(); + // Let the background refresh started by initialize() finish. + await new Promise((resolve) => setImmediate(resolve)); + return manager; +} + +beforeEach(() => { + chromeMock.reset(); + remoteCalls = 0; + remoteHandler = async () => jsonResponse(NEW_RULES); + global.fetch = async (url) => { + if (url === BUNDLED_URL) return jsonResponse(BUNDLED_RULES); + remoteCalls++; + return remoteHandler(); + }; +}); + +afterEach(() => { + delete global.fetch; +}); + +test('keeps an expired cache when the update fails', async (t) => { + const failures = { + 'network error': async () => { throw new TypeError('Failed to fetch'); }, + 'HTTP 429': async () => jsonResponse({ message: 'rate limited' }, 429), + 'non-JSON response': async () => ({ ok: true, status: 200, json: async () => { throw new SyntaxError('Unexpected token <'); } }), + 'JSON that is not a rules file': async () => jsonResponse({ message: 'Not Found' }) + }; + + for (const [name, handler] of Object.entries(failures)) { + await t.test(name, async () => { + chromeMock.reset(); + remoteHandler = handler; + await seedCache(48 * HOUR); + const cachedBefore = storedCache(); + + const manager = await startManager(); + + assert.strictEqual(remoteCalls, 1); + assert.strictEqual((await manager.getDetectionRules()).version, '1.2.5'); + assert.strictEqual(manager.getCacheInfo().usingFallback, false); + assert.deepStrictEqual(storedCache(), cachedBefore, 'cache must not be overwritten'); + }); + } +}); + +test('keeps downloaded rules when a refresh fails while running', async () => { + const manager = await startManager(); + assert.strictEqual(manager.cachedRules.version, '1.3.0'); + + manager.lastUpdate = Date.now() - 25 * HOUR; + remoteHandler = async () => jsonResponse({}, 503); + await manager.getDetectionRules(); + await manager._refreshInFlight; + + assert.strictEqual((await manager.getDetectionRules()).version, '1.3.0'); + assert.strictEqual(manager.getCacheInfo().usingFallback, false); +}); + +test('uses bundled rules only when nothing has been downloaded', async () => { + remoteHandler = async () => { throw new TypeError('Failed to fetch'); }; + const manager = await startManager(); + + assert.strictEqual((await manager.getDetectionRules()).version, '1.2.0'); + assert.strictEqual(manager.getCacheInfo().usingFallback, true); + assert.strictEqual(storedCache(), undefined, 'bundled rules must not be persisted'); +}); + +test('replaces expired rules when the update succeeds', async () => { + await seedCache(48 * HOUR); + const manager = await startManager(); + + assert.strictEqual(manager.cachedRules.version, '1.3.0'); + assert.strictEqual(storedCache().rules.version, '1.3.0'); +}); + +test('does not refetch a fresh cache', async () => { + await seedCache(HOUR); + const manager = await startManager(); + + assert.strictEqual((await manager.getDetectionRules()).version, '1.2.5'); + assert.strictEqual(remoteCalls, 0); +}); + +test('does not retry a failed fetch on every request', async () => { + remoteHandler = async () => jsonResponse({}, 429); + await seedCache(48 * HOUR); + const manager = await startManager(); + + for (let i = 0; i < 5; i++) { + await manager.getDetectionRules(); + await manager._refreshInFlight; + } + assert.strictEqual(remoteCalls, 1); + + // Retries once the retry delay has passed. + manager._lastFailedFetchAt = Date.now() - 10 * 60 * 1000; + await manager.getDetectionRules(); + await manager._refreshInFlight; + assert.strictEqual(remoteCalls, 2); +}); + +test('forceUpdate reports a failed download and keeps the previous rules', async () => { + remoteHandler = async () => jsonResponse({}, 503); + await seedCache(48 * HOUR); + const manager = await startManager(); + + await assert.rejects(manager.forceUpdate(), /Still using the previously downloaded rules/); + assert.strictEqual(manager.cachedRules.version, '1.2.5'); + + remoteHandler = async () => jsonResponse(NEW_RULES); + assert.strictEqual((await manager.forceUpdate()).version, '1.3.0'); +}); + +test('keeps the retry delay across service worker restarts', async () => { + remoteHandler = async () => jsonResponse({}, 429); + await seedCache(48 * HOUR); + await startManager(); + assert.strictEqual(remoteCalls, 1); + + // A restarted worker within the retry delay doesn't refetch. + const restarted = await startManager(); + assert.strictEqual(remoteCalls, 1); + assert.strictEqual((await restarted.getDetectionRules()).version, '1.2.5'); + await restarted._refreshInFlight; + assert.strictEqual(remoteCalls, 1); + + // Once the delay has passed, a restarted worker retries. + await chromeMock.storage.local.set({ + detection_rules_last_failed_fetch: Date.now() - 10 * 60 * 1000 + }); + remoteHandler = async () => jsonResponse(NEW_RULES); + const later = await startManager(); + assert.strictEqual(remoteCalls, 2); + assert.strictEqual(later.cachedRules.version, '1.3.0'); +}); + +test('forceUpdate reports its own result when another fetch overlaps', async () => { + await seedCache(HOUR); + const manager = await startManager(); + + const pending = []; + remoteHandler = () => new Promise((resolve) => pending.push(resolve)); + const tick = () => new Promise((resolve) => setImmediate(resolve)); + + const forced = manager.forceUpdate(); + while (pending.length < 1) await tick(); + const background = manager.updateDetectionRules(); + while (pending.length < 2) await tick(); + + // The overlapping fetch fails first, then the forced one succeeds. + pending[1](jsonResponse({}, 503)); + await background; + pending[0](jsonResponse(NEW_RULES)); + + assert.strictEqual((await forced).version, '1.3.0'); +}); diff --git a/tests/helpers/chrome-mock.js b/tests/helpers/chrome-mock.js index db01d3cc..a4282d48 100644 --- a/tests/helpers/chrome-mock.js +++ b/tests/helpers/chrome-mock.js @@ -16,12 +16,38 @@ export class ChromeMock { } } +// Real chrome.storage areas accept an optional callback as well as returning a +// promise. browser-polyfill.js uses the callback form, so support both. +function settle(promise, callback) { + if (typeof callback === 'function') { + promise.then((result) => callback(result)); + } + return promise; +} + class LocalStorageMock { constructor() { this.data = {}; } - get(keys) { + get(keys, callback) { + return settle(this._get(keys), callback); + } + + set(items, callback) { + return settle(this._set(items), callback); + } + + remove(keys, callback) { + return settle(this._remove(keys), callback); + } + + clear(callback) { + this.data = {}; + return settle(Promise.resolve(), callback); + } + + _get(keys) { return new Promise((resolve) => { if (typeof keys === 'string') { resolve({ [keys]: this.data[keys] }); @@ -41,14 +67,14 @@ class LocalStorageMock { }); } - set(items) { + _set(items) { return new Promise((resolve) => { Object.assign(this.data, items); resolve(); }); } - remove(keys) { + _remove(keys) { return new Promise((resolve) => { const keyArray = Array.isArray(keys) ? keys : [keys]; keyArray.forEach(key => delete this.data[key]); @@ -56,11 +82,6 @@ class LocalStorageMock { }); } - clear() { - this.data = {}; - return Promise.resolve(); - } - getData() { return { ...this.data }; } @@ -71,7 +92,11 @@ class ManagedStorageMock { this.data = {}; } - get(keys) { + get(keys, callback) { + return settle(this._get(keys), callback); + } + + _get(keys) { return new Promise((resolve) => { if (keys === null || keys === undefined) { resolve({ ...this.data }); @@ -105,7 +130,15 @@ class SessionStorageMock { this.data = {}; } - get(keys) { + get(keys, callback) { + return settle(this._get(keys), callback); + } + + set(items, callback) { + return settle(this._set(items), callback); + } + + _get(keys) { return new Promise((resolve) => { if (typeof keys === 'string') { resolve({ [keys]: this.data[keys] }); @@ -123,7 +156,7 @@ class SessionStorageMock { }); } - set(items) { + _set(items) { return new Promise((resolve) => { Object.assign(this.data, items); resolve(); @@ -144,6 +177,7 @@ class RuntimeMock { this.id = 'test-extension-id'; this.lastError = null; this.messageListeners = []; + this.sentMessages = []; } getManifest() { @@ -155,11 +189,12 @@ class RuntimeMock { } sendMessage(message, callback) { - setTimeout(() => { - if (callback) { - callback({ success: true }); - } - }, 0); + this.sentMessages.push(message); + const response = Promise.resolve({ success: true }); + if (callback) { + response.then((result) => setTimeout(() => callback(result), 0)); + } + return response; } onMessage = { @@ -171,6 +206,7 @@ class RuntimeMock { reset() { this.lastError = null; this.messageListeners = []; + this.sentMessages = []; } }