Update pr-validator (#7)

* Update tests * Switch from tabs to spaces * Fix async handling and new lines * Speed up blocklist search & make tests async * Make non-required messages debug messages * Clean up * Clean up code * Apply suggestions from code review Co-authored-by: H. Kamran <[email protected]> Signed-off-by: Carl <[email protected]> --------- Signed-off-by: Carl <[email protected]> Co-authored-by: H. Kamran <[email protected]>
2factorauth · Dec 19, 2024 · d82873c · d82873c
1 parent c68088e
commit d82873c
Show file tree

Hide file tree

Showing 10 changed files with 328 additions and 263 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -2,7 +2,8 @@
 root = true
 
 [*]
-indent_style = tab
+indent_style = space
+indent_size = 2
 end_of_line = lf
 charset = utf-8
 trim_trailing_whitespace = true

diff --git a/package.json b/package.json
@@ -9,13 +9,9 @@
 		"start": "wrangler dev",
 		"format": "prettier . --write"
 	},
-	"dependencies": {
-		"crypto-js": "^4.2.0",
-		"oauth-1.0a": "^2.2.6"
-	},
 	"devDependencies": {
 		"itty-router": "^3.0.12",
 		"prettier": "^3.3.2",
-		"wrangler": "^3.0.0"
+		"wrangler": "^3.97.0"
 	}
 }
diff --git a/src/logger.js b/src/logger.js
@@ -1,32 +1,57 @@
 class Logger {
-	constructor() {
-		this.messages = [];
-	}
+  constructor() {
+    this.messages = [];
+  }
 
-	addMessage(test, message) {
-		const msg = `${test}: ${message}`;
-		console.debug(msg);
-		this.messages.push(msg);
-	}
+  /**
+   * Add a general message to the log
+   * @param {string} test The test name
+   * @param {string} message The message to log
+   */
+  addDebug(test, message) {
+    const msg = `::debug title=${test}:: ${message}`;
+    console.debug(msg);
+    this.messages.push(msg);
+  }
 
-	/**
-	 * Add a warning to the log using [GitHub workflow commands](https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions)
-	 * @param {string} file The filename of the entry
-	 * @param {string} message The message to be written to the log
-	 */
-	addWarning(file, message) {
-		const msg = `::warning file=${file}:: ${message}`;
-		console.debug(msg);
-		this.messages.push(msg);
-	}
+  /**
+   * Add a warning to the log in GitHub workflow format
+   * @param {string} file The filename of the entry
+   * @param {string} message The warning message
+   * @param {string} [title="Warning"] A short description of the warning
+   */
+  addWarning(file, message, title = 'Warning') {
+    const msg = `::warning file=${file},title=${title}::${message}`;
+    console.warn(msg);
+    this.messages.push(msg);
+  }
 
-	getMessages() {
-		return this.messages;
-	}
+  /**
+   * Add an error to the log in GitHub workflow format
+   * @param {string} file The filename of the entry
+   * @param {string} message The error message
+   * @param {string} [title="Error"] A short description of the error
+   */
+  addError(file, message, title = 'Error') {
+    const msg = `::error file=${file},title=${title}::${message}`;
+    console.error(msg);
+    this.messages.push(msg);
+  }
 
-	clearMessages() {
-		this.messages = [];
-	}
+  /**
+   * Retrieve all logged messages
+   * @returns {string[]} An array of all logged messages
+   */
+  getMessages() {
+    return this.messages;
+  }
+
+  /**
+   * Clear all logged messages
+   */
+  clearMessages() {
+    this.messages = [];
+  }
 }
 
 // Create a singleton instance of Logger

diff --git a/src/passkeys.js b/src/passkeys.js
@@ -3,38 +3,39 @@ import Facebook from './tests/Facebook.js';
 import Blocklist from './tests/Blocklist.js';
 import logger from './logger';
 
-export default async function (req, env) {
-	const { pr, repo } = req.params;
-	const repository = `${env.OWNER}/${repo}`;
-
-	// Fetch all entries modified in the PR
-	const entries = await fetchEntries(repository, pr);
-
-	for (const entry of entries) {
-		try {
-			// Validate primary domain
-			await SimilarWeb(entry.domain, env);
-			await Blocklist(entry.domain);
-
-			// Validate any additional domains
-			for (const domain of entry['additional-domains'] || []) {
-				await SimilarWeb(domain, env, entry.file);
-				await Blocklist(domain);
-			}
-
-			// Validate Facebook contact if present
-			if (entry.contact?.facebook) await Facebook(entry.contact.facebook);
-		} catch (e) {
-			// Return an error response if validation fails
-			return new Response(`::error file=${entry.file}:: ${e.message}`, { status: 400 });
-		}
-	}
-
-	const messages = logger.getMessages().join('\n');
-	logger.clearMessages();
-
-	// Return a success response if no errors were thrown
-	return new Response(messages);
+export default async function(req, env) {
+  const { pr, repo } = req.params;
+  const repository = `${env.OWNER}/${repo}`;
+
+  // Fetch all entries modified in the PR
+  const entries = await fetchEntries(repository, pr);
+
+  for (const entry of entries) {
+    try {
+      // Validate primary domain
+      await SimilarWeb(entry.domain, env);
+      await Blocklist(entry.domain);
+
+      // Validate any additional domains
+      for (const domain of entry['additional-domains'] || []) {
+        await SimilarWeb(domain, env, entry.file);
+        await Blocklist(domain);
+      }
+
+      // Validate Facebook contact if present
+      if (entry.contact?.facebook) await Facebook(entry.contact.facebook);
+    } catch (e) {
+      // Return an error response if validation fails
+      return new Response(`::error file=${entry.file}:: ${e.message}`,
+        { status: 400 });
+    }
+  }
+
+  const messages = logger.getMessages().join('\n');
+  logger.clearMessages();
+
+  // Return a success response if no errors were thrown
+  return new Response(messages);
 }
 
 /**
@@ -45,38 +46,39 @@ export default async function (req, env) {
  * @returns {Promise<*[]>} Returns all modified entry files as an array.
  */
 async function fetchEntries(repo, pr) {
-	const data = await fetch(`https://api.github.com/repos/${repo}/pulls/${pr}/files`, {
-		headers: {
-			Accept: 'application/vnd.github.v3+json',
-			'User-Agent': '2factorauth/twofactorauth (+https://2fa.directory/bots)',
-		},
-	});
+  const data = await fetch(
+    `https://api.github.com/repos/${repo}/pulls/${pr}/files`, {
+      headers: {
+        Accept: 'application/vnd.github.v3+json',
+        'User-Agent': '2factorauth/twofactorauth (+https://2fa.directory/bots)'
+      }
+    });
 
-	if (!data.ok) throw new Error(await data.text());
+  if (!data.ok) throw new Error(await data.text());
 
-	const json = await data.json();
-	let files = [];
-	for (const i in json) {
-		const file = json[i];
+  const json = await data.json();
+  let files = [];
+  for (const i in json) {
+    const file = json[i];
 
-		// Ignore deleted files
-		if (file.status === 'removed') continue;
+    // Ignore deleted files
+    if (file.status === 'removed') continue;
 
-		const path = file.filename;
-		if (path.startsWith('entries/')) {
-			// Parse entry file as JSON
-			const f = await (await fetch(file.raw_url)).json();
+    const path = file.filename;
+    if (path.startsWith('entries/')) {
+      // Parse entry file as JSON
+      const f = await (await fetch(file.raw_url)).json();
 
-			// Get first object of entry file (f)
-			const data = f[Object.keys(f)[0]];
+      // Get first object of entry file (f)
+      const data = f[Object.keys(f)[0]];
 
-			// Append file path to object
-			data.file = file.filename;
-			data.domain = file.filename.replace(/.*\/|\.[^.]*$/g, '');
+      // Append file path to object
+      data.file = file.filename;
+      data.domain = file.filename.replace(/.*\/|\.[^.]*$/g, '');
 
-			files.push(data);
-		}
-	}
+      files.push(data);
+    }
+  }
 
-	return files;
+  return files;
 }
diff --git a/src/router.js b/src/router.js
@@ -3,21 +3,21 @@ import twofactorauth from './twofactorauth';
 import passkeys from './passkeys';
 
 export default {
-	async fetch(request, env) {
-		return router.handle(request, env);
-	},
+  async fetch(request, env) {
+    return router.handle(request, env);
+  }
 };
 
 const router = Router();
 
 router.get('/:repo/:pr/', async (req, env) => {
-	const { repo } = req.params;
-	switch (repo) {
-		case 'twofactorauth':
-			return twofactorauth(req, env);
-		case 'passkeys':
-			return passkeys(req, env);
-	}
+  const { repo } = req.params;
+  switch (repo) {
+    case 'twofactorauth':
+      return twofactorauth(req, env);
+    case 'passkeys':
+      return passkeys(req, env);
+  }
 });
 
 router.all('*', () => new Response('Not Found.', { status: 404 }));
diff --git a/src/tests/Blocklist.js b/src/tests/Blocklist.js
@@ -5,11 +5,10 @@ const test = 'Blocklist';
  * List of URLs for different blocklist categories.
  */
 const lists = {
-	malware: 'https://blocklistproject.github.io/Lists/alt-version/malware-nl.txt',
-	piracy: 'https://blocklistproject.github.io/Lists/alt-version/piracy-nl.txt',
-	porn: 'https://blocklistproject.github.io/Lists/alt-version/porn-nl.txt',
+  malware: 'https://blocklistproject.github.io/Lists/alt-version/malware-nl.txt',
+  piracy: 'https://blocklistproject.github.io/Lists/alt-version/piracy-nl.txt',
+  porn: 'https://blocklistproject.github.io/Lists/alt-version/porn-nl.txt',
 };
-const cache = {}; // To cache the fetched lists
 
 /**
  * Checks if a given domain is present in any of the blocklists.
@@ -19,46 +18,43 @@ const cache = {}; // To cache the fetched lists
  * @throws Will throw an error if the domain is found in any blocklist, specifying the category.
  */
 export default async function (domain) {
-	const listPromises = Object.entries(lists).map(async ([list, url]) => {
-		const domainSet = await fetchAndCacheList(url);
-		if (domainSet.has(domain)) {
-			throw new Error(`${domain} is categorized as a ${list} website.`);
-		}
-	});
-
-	await Promise.all(listPromises);
-	logger.addMessage(test, `${domain} not found in any list`);
+  await Promise.all(
+    Object.entries(lists).map(async ([list, url]) => {
+      const isBlocked = await isDomainBlocked(domain, url);
+      if (isBlocked) {
+        throw {
+          title: `${domain} labeled as a ${list} website`,
+          message: `According to [The Block List Project](https://github.com/blocklistproject/Lists), the site ${domain} hosts content marked as ${list}.\nSuch content is against our guidelines.`,
+        };
+      }
+    })
+  );
+  logger.addDebug(test, `${domain} not found in any list`);
 }
 
 /**
- * Fetches a blocklist from a given URL, parses it, and caches the result.
+ * Fetches a blocklist from a given URL, parses it into a Set, and checks if the domain exists.
  *
+ * @param {string} domain - The domain to find.
  * @param {string} url - The URL of the blocklist to fetch.
- * @returns {Promise<Set<string>>} A promise that resolves to a set of domains from the blocklist.
+ * @returns {Promise<boolean>} Returns true if a match is found, otherwise false.
  */
-async function fetchAndCacheList(url) {
-	if (!cache[url]) {
-		const res = await fetch(url, {
-			headers: {
-				'user-agent': '2factorauth/twofactorauth (+https://2fa.directory/bots)',
-			},
-			cf: {
-				cacheEverything: true,
-				cacheTtl: 24 * 60, // Cache 1 day
-			},
-		});
-		const text = await res.text();
-		const lines = text.split('\n');
-		const domainSet = new Set();
+async function isDomainBlocked (domain, url) {
+  const res = await fetch(url, {
+    headers: {
+      'user-agent': '2factorauth/twofactorauth (+https://2fa.directory/bots)',
+    },
+    cf: {
+      cacheEverything: true,
+      cacheTtl: 7 * 24 * 60, // Cache 1 week
+    },
+  });
 
-		lines.forEach((line) => {
-			const trimmedLine = line.trim();
-			if (trimmedLine && !trimmedLine.startsWith('#')) {
-				domainSet.add(trimmedLine);
-			}
-		});
+  if (!res.ok) {
+    logger.addDebug(test, `Unable to fetch blocklist ${url}. ${res.status}`);
+    return false; // Assume no match if the blocklist can't be fetched
+  }
 
-		cache[url] = domainSet;
-	}
-	return cache[url];
+  const blocklist = new Set((await res.text()).split('\n').map((d) => d.trim()));
+  return blocklist.has(domain);
 }
diff --git a/src/tests/Facebook.js b/src/tests/Facebook.js
@@ -1,14 +1,15 @@
-import logger from '../logger.js';
-
 export default async function Facebook(handle) {
-	let res = await fetch(`https://www.facebook.com/${handle}`, {
-		cf: {
-			cacheTtl: 7 * 24 * 60, // Cache 7 days
-			cacheEverything: true,
-		},
-	});
+  let res = await fetch(`https://www.facebook.com/${handle}`, {
+    cf: {
+      cacheTtl: 7 * 24 * 60, // Cache 7 days
+      cacheEverything: true
+    }
+  });
+
+  if (res.ok) return true;
 
-	if (!res.ok) throw new Error(`Failed to fetch Facebook page ${handle}`);
-	logger.addMessage('Facebook', `${handle} found.`);
-	return true;
+  throw {
+    title: 'Facebook handle not found',
+    message: `Failed to find the Facebook page https://facebook.com/${handle}\\nThe page might be private or not exist.`
+  };
 }