diff --git a/packages/moderation/src/data/nags/links.ts b/packages/moderation/src/data/nags/links.ts index 0a31d08125..25c7df34af 100644 --- a/packages/moderation/src/data/nags/links.ts +++ b/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, - PROJECT_CONTENT_LINK_SHORTENERS, + getBlockedProjectExternalLink, + 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 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 } diff --git a/packages/moderation/src/validators/link-checks/tests.ts b/packages/moderation/src/validators/link-checks/tests.ts index da2394c043..f03b54f04a 100644 --- a/packages/moderation/src/validators/link-checks/tests.ts +++ b/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 () => { - const blocked = { +test('applies the external-link blocklist only outside general content', async () => { + 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') }) diff --git a/packages/moderation/src/validators/project-fields/tests.ts b/packages/moderation/src/validators/project-fields/tests.ts index 3a6c062486..369022f501 100644 --- a/packages/moderation/src/validators/project-fields/tests.ts +++ b/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( - 'Visit https://social.modrinth.com/project', - 'Title', - ) + const blockedSummary = validateProjectSummary('Visit https://bit.ly/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', - url: 'https://social.modrinth.com/project', + label: 'URL shortener', + 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'), diff --git a/packages/moderation/src/validators/project-links/index.ts b/packages/moderation/src/validators/project-links/index.ts index ac63188b9e..7af2eec671 100644 --- a/packages/moderation/src/validators/project-links/index.ts +++ b/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[] = [ - { - label: 'URL shortener', - domains: PROJECT_CONTENT_LINK_SHORTENERS, - }, +const URL_SHORTENER_BLOCKLIST_ENTRY: ProjectLinkBlocklistEntry = { + label: 'URL shortener', + domains: PROJECT_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 { +export interface BlockedProjectLink extends Record { 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) +} diff --git a/packages/moderation/src/validators/project-links/tests.ts b/packages/moderation/src/validators/project-links/tests.ts index f1ac61c53c..5afab43700 100644 --- a/packages/moderation/src/validators/project-links/tests.ts +++ b/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) }) diff --git a/packages/moderation/src/validators/project-validation/tests.ts b/packages/moderation/src/validators/project-validation/tests.ts index f1d94719b0..dbb1a93914 100644 --- a/packages/moderation/src/validators/project-validation/tests.ts +++ b/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')