fix: checking for banned banned external links outside of external links

packages/moderation/src/data/nags/links.ts

@@ -3,8 +3,8 @@ import { defineMessage, formatProjectTypeSentence, useVIntl } from '@modrinth/ui
 import type { Nag, NagContext } from '../../types/nags'
 import { licenseRequiresSource, notSourceAsDistributed } from '../../utils'
 import {
-	getBlockedProjectContentLink,
+	getBlockedProjectExternalLink,
-	PROJECT_CONTENT_LINK_SHORTENERS,
+	PROJECT_LINK_SHORTENERS,
 } from '../../validators/project-links'
 
 export const commonLinkDomains = {
@@ -49,7 +49,7 @@ export const commonLinkDomains = {
 		'example.com',
 		't.me',
 	],
-	linkShorteners: PROJECT_CONTENT_LINK_SHORTENERS,
+	linkShorteners: PROJECT_LINK_SHORTENERS,
 }
 
 export function isCommonUrl(url: string | null, commonDomains: readonly string[]): boolean {
@@ -92,7 +92,7 @@ export function findBlockedProjectExternalLink(context: Pick<NagContext, 'projec
 
 	for (const url of urls) {
 		if (!url) continue
-		const blockedLink = getBlockedProjectContentLink(url)
+		const blockedLink = getBlockedProjectExternalLink(url)
 		if (blockedLink) return blockedLink
 	}
 

packages/moderation/src/locales/en-US/index.json

@@ -134,6 +134,9 @@
 	"nags.invalid-project-summary.title": {
 		"defaultMessage": "Fix the project summary"
 	},
+	"nags.link.description.invalid-url": {
+		"defaultMessage": "The description has an invalid link"
+	},
 	"nags.link.discord.channel": {
 		"defaultMessage": "This is a link to a Discord channel, not a server invite."
 	},
@@ -168,7 +171,7 @@
 		"defaultMessage": "The wiki is disabled on this repository."
 	},
 	"nags.link.invalid-url": {
-		"defaultMessage": "There's an invalid URL in the description."
+		"defaultMessage": "This URL is invalid"
 	},
 	"nags.link.license.url-mismatch": {
 		"defaultMessage": "This link points to the {detected} license, but your project is set to {selected}."

packages/moderation/src/validators/link-checks/index.ts

@@ -1,6 +1,9 @@
 import { computed, onScopeDispose, reactive, type Ref, watch } from 'vue'
 
-import { PROJECT_CONTENT_LINK_BLOCKLIST } from '../project-links/index.ts'
+import {
+	getBlockedProjectContentLink,
+	getBlockedProjectExternalLink,
+} from '../project-links/index.ts'
 
 interface MessageDescriptor {
 	id: string
@@ -104,11 +107,6 @@ function anchored(source: string): RegExp {
 	return new RegExp(`^${source}`, 'i')
 }
 
-function blacklist(label: string, ...domains: string[]): LinkCheckBuilder {
-	const pattern = domains.map((domain) => domain.replace(/\./g, '\\.')).join('|')
-	return check(new RegExp(`^(?:[^./:?#]+\\.)*(?:${pattern})(?=[:/?#]|$)`, 'i'), label)
-}
-
 function buildNode(when: LinkCheckMatcher, label?: string): LinkCheckBuilder {
 	const childNodes: LinkCheckNode[] = []
 	const forMatchers: FieldMatcher[] = []
@@ -278,7 +276,12 @@ const coreMessages = defineMessages({
 //TODO: we should probably just let you not provide https but backend currently requires it
 const invalidUrlMessage = defineMessage({
 	id: 'nags.link.invalid-url',
-	defaultMessage: "There's an invalid URL in the description.",
+	defaultMessage: 'This URL is invalid',
+})
+
+const invalidDescriptionUrlMessage = defineMessage({
+	id: 'nags.link.description.invalid-url',
+	defaultMessage: 'The description has an invalid link',
 })
 
 function validUrlPrefix(remaining: string): number | null {
@@ -337,6 +340,13 @@ async function checkLink(context: LinkCheckContext) {
 	const normalizedUrl = url.replace(/^(https:\/\/)www\./i, '$1')
 
 	cache.set(key, 'pending')
+	const blockedLink = context.generalContent
+		? getBlockedProjectContentLink(url)
+		: getBlockedProjectExternalLink(url)
+	if (blockedLink) {
+		cache.set(key, error(coreMessages.neverValid, { label: blockedLink.label }))
+		return
+	}
 
 	const found = await matchNode(rootNode, normalizedUrl, context, true)
 	if (!found) {
@@ -361,7 +371,12 @@ async function checkLink(context: LinkCheckContext) {
 		const build = matched.unrecognizedSeverity === 'warn' ? warn : error
 
 		if (matched.unrecognizedMessage && isLeaf) {
-			cache.set(key, build(matched.unrecognizedMessage, { label: matched.label }))
+			const message =
+				context.field === 'description' &&
+				matched.unrecognizedMessage.id === invalidUrlMessage.id
+					? invalidDescriptionUrlMessage
+					: matched.unrecognizedMessage
+			cache.set(key, build(message, { label: matched.label }))
 			return
 		}
 
@@ -830,8 +845,4 @@ checks.children(
 	),
 )
 
-checks.children(
-	...PROJECT_CONTENT_LINK_BLOCKLIST.map(({ label, domains }) => blacklist(label, ...domains)),
-)
-
 export { checkLink, getLinkCheckState, isLinkCheckPending, useLinkCheck }

packages/moderation/src/validators/link-checks/tests.ts

@@ -16,6 +16,22 @@ test('rejects invalid and insecure URLs', async () => {
 	assert.equal(getLinkCheckState(reserved)?.severity, 'error')
 })
 
+test('uses a description-specific message for invalid content links', async () => {
+	const context = {
+		field: 'description',
+		url: 'http://example.dev/project',
+		generalContent: true,
+	}
+
+	await checkLink(context)
+
+	assert.equal(getLinkCheckState(context)?.message?.id, 'nags.link.description.invalid-url')
+	assert.equal(
+		getLinkCheckState(context)?.message?.defaultMessage,
+		'The description has an invalid link',
+	)
+})
+
 test('matches recognized hosts case-insensitively', async () => {
 	const googleForm = { field: 'issues', url: 'https://DOCS.GOOGLE.COM/forms/d/e/example' }
 	const shortener = { field: 'source', url: 'https://BIT.LY/example' }
@@ -67,8 +83,12 @@ test('allows unrecognized valid links but keeps global restrictions in general c
 	assert.equal(getLinkCheckState(blocked)?.severity, 'error')
 })
 
-test('blocks subdomains of blocklisted hosts without blocking lookalike domains', async () => {
+test('applies the external-link blocklist only outside general content', async () => {
-	const blocked = {
+	const blockedExternalLink = {
+		field: 'site',
+		url: 'https://social.modrinth.com/project',
+	}
+	const allowedContentLink = {
 		field: 'description',
 		url: 'https://social.modrinth.com/project',
 		generalContent: true,
@@ -79,10 +99,12 @@ test('blocks subdomains of blocklisted hosts without blocking lookalike domains'
 		generalContent: true,
 	}
 
-	await checkLink(blocked)
+	await checkLink(blockedExternalLink)
+	await checkLink(allowedContentLink)
 	await checkLink(allowed)
 
-	assert.equal(getLinkCheckState(blocked)?.severity, 'error')
+	assert.equal(getLinkCheckState(blockedExternalLink)?.severity, 'error')
+	assert.equal(getLinkCheckState(allowedContentLink)?.severity, 'valid')
 	assert.equal(getLinkCheckState(allowed)?.severity, 'valid')
 })
 

packages/moderation/src/validators/project-fields/tests.ts

@@ -161,10 +161,7 @@ test('validates project summaries', () => {
 })
 
 test('rejects blocklisted links and IP addresses in summaries and descriptions', () => {
-	const blockedSummary = validateProjectSummary(
+	const blockedSummary = validateProjectSummary('Visit https://bit.ly/project', 'Title')
-		'Visit https://social.modrinth.com/project',
-		'Title',
-	)
 	assert.deepEqual(blockedSummary[0], {
 		code: 'text-banned-link',
 		severity: 'error',
@@ -173,8 +170,8 @@ test('rejects blocklisted links and IP addresses in summaries and descriptions',
 			defaultMessage: '“{url}” is not allowed in project summaries or descriptions.',
 		},
 		values: {
-			label: 'Modrinth',
+			label: 'URL shortener',
-			url: 'https://social.modrinth.com/project',
+			url: 'https://bit.ly/project',
 		},
 	})
 
@@ -189,7 +186,7 @@ test('rejects blocklisted links and IP addresses in summaries and descriptions',
 	assert.equal(blockedIp[0]?.values?.label, 'IP address')
 
 	const allowedDescription = validateProjectDescription(
-		`Read more at https://example.dev/project. ${'More details. '.repeat(20)}`,
+		`Read more at https://social.modrinth.com/project. ${'More details. '.repeat(20)}`,
 	)
 	assert.equal(
 		allowedDescription.some(({ code }) => code === 'text-banned-link'),

packages/moderation/src/validators/project-links/index.ts

@@ -1,9 +1,9 @@
-export interface ProjectContentLinkBlocklistEntry {
+export interface ProjectLinkBlocklistEntry {
 	label: string
 	domains: readonly string[]
 }
 
-export const PROJECT_CONTENT_LINK_SHORTENERS = [
+export const PROJECT_LINK_SHORTENERS = [
 	'bit.ly',
 	'adf.ly',
 	'tinyurl.com',
@@ -11,11 +11,17 @@ export const PROJECT_CONTENT_LINK_SHORTENERS = [
 	'is.gd',
 ] as const
 
-export const PROJECT_CONTENT_LINK_BLOCKLIST: readonly ProjectContentLinkBlocklistEntry[] = [
+const URL_SHORTENER_BLOCKLIST_ENTRY: ProjectLinkBlocklistEntry = {
-	{
+	label: 'URL shortener',
-		label: 'URL shortener',
+	domains: PROJECT_LINK_SHORTENERS,
-		domains: PROJECT_CONTENT_LINK_SHORTENERS,
+}
-	},
+
+export const PROJECT_CONTENT_LINK_BLOCKLIST: readonly ProjectLinkBlocklistEntry[] = [
+	URL_SHORTENER_BLOCKLIST_ENTRY,
+]
+
+export const PROJECT_EXTERNAL_LINK_BLOCKLIST: readonly ProjectLinkBlocklistEntry[] = [
+	URL_SHORTENER_BLOCKLIST_ENTRY,
 	{ label: 'Twitter', domains: ['twitter.com', 'x.com'] },
 	{ label: 'Instagram', domains: ['instagram.com'] },
 	{ label: 'Facebook', domains: ['facebook.com'] },
@@ -37,7 +43,7 @@ export const PROJECT_CONTENT_LINK_BLOCKLIST: readonly ProjectContentLinkBlocklis
 	},
 ]
 
-export interface BlockedProjectContentLink extends Record<string, unknown> {
+export interface BlockedProjectLink extends Record<string, unknown> {
 	label: string
 	url: string
 }
@@ -47,7 +53,10 @@ function isIpAddress(hostname: string) {
 	return /^\d{1,3}(?:\.\d{1,3}){3}$/.test(strippedHostname) || strippedHostname.includes(':')
 }
 
-export function getBlockedProjectContentLink(url: string): BlockedProjectContentLink | null {
+function getBlockedProjectLink(
+	url: string,
+	blocklist: readonly ProjectLinkBlocklistEntry[],
+): BlockedProjectLink | null {
 	let hostname: string
 	try {
 		hostname = new URL(url).hostname.toLowerCase().replace(/\.$/, '')
@@ -57,9 +66,17 @@ export function getBlockedProjectContentLink(url: string): BlockedProjectContent
 
 	if (isIpAddress(hostname)) return { label: 'IP address', url }
 
-	const entry = PROJECT_CONTENT_LINK_BLOCKLIST.find(({ domains }) =>
+	const entry = blocklist.find(({ domains }) =>
 		domains.some((domain) => hostname === domain || hostname.endsWith(`.${domain}`)),
 	)
 
 	return entry ? { label: entry.label, url } : null
 }
+
+export function getBlockedProjectContentLink(url: string): BlockedProjectLink | null {
+	return getBlockedProjectLink(url, PROJECT_CONTENT_LINK_BLOCKLIST)
+}
+
+export function getBlockedProjectExternalLink(url: string): BlockedProjectLink | null {
+	return getBlockedProjectLink(url, PROJECT_EXTERNAL_LINK_BLOCKLIST)
+}

packages/moderation/src/validators/project-links/tests.ts

@@ -1,7 +1,12 @@
 import assert from 'node:assert/strict'
 import test from 'node:test'
 
-import { getBlockedProjectContentLink, PROJECT_CONTENT_LINK_BLOCKLIST } from './index.ts'
+import {
+	getBlockedProjectContentLink,
+	getBlockedProjectExternalLink,
+	PROJECT_CONTENT_LINK_BLOCKLIST,
+	PROJECT_EXTERNAL_LINK_BLOCKLIST,
+} from './index.ts'
 
 test('blocks every configured project-content domain and its subdomains', () => {
 	for (const { label, domains } of PROJECT_CONTENT_LINK_BLOCKLIST) {
@@ -18,9 +23,31 @@ test('blocks every configured project-content domain and its subdomains', () =>
 	}
 })
 
+test('blocks every configured external-link domain and its subdomains', () => {
+	for (const { label, domains } of PROJECT_EXTERNAL_LINK_BLOCKLIST) {
+		for (const domain of domains) {
+			assert.deepEqual(getBlockedProjectExternalLink(`https://${domain}/project`), {
+				label,
+				url: `https://${domain}/project`,
+			})
+			assert.equal(
+				getBlockedProjectExternalLink(`https://subdomain.${domain}/project`)?.label,
+				label,
+			)
+		}
+	}
+})
+
+test('allows external-only blocklist entries in project content', () => {
+	assert.equal(getBlockedProjectContentLink('https://social.modrinth.com/project'), null)
+	assert.equal(getBlockedProjectExternalLink('https://social.modrinth.com/project')?.label, 'Modrinth')
+})
+
 test('blocks IP-address URLs without blocking domain lookalikes', () => {
 	assert.equal(getBlockedProjectContentLink('http://127.0.0.1:25565')?.label, 'IP address')
 	assert.equal(getBlockedProjectContentLink('https://[2001:db8::1]')?.label, 'IP address')
+	assert.equal(getBlockedProjectExternalLink('http://127.0.0.1:25565')?.label, 'IP address')
 	assert.equal(getBlockedProjectContentLink('https://modrinth.com.example.dev'), null)
+	assert.equal(getBlockedProjectExternalLink('https://modrinth.com.example.dev'), null)
 	assert.equal(getBlockedProjectContentLink('not a URL'), null)
 })

packages/moderation/src/validators/project-validation/tests.ts

@@ -119,7 +119,7 @@ test('treats version numbers and explicit summary links as errors', () => {
 
 test('rejects blocklisted links in summaries and descriptions', () => {
 	const summaryResult = validateProjectFields(
-		createProject({ summary: 'Visit modrinth.com for more information' }),
+		createProject({ summary: 'Visit bit.ly/project for more information' }),
 	)
 	assert.equal(summaryResult.valid, false)
 	assert.equal(summaryResult.failures[0]?.field, 'summary')