diff --git a/backend/src/api/index.ts b/backend/src/api/index.ts index fc5d68501e..cad8e1ae90 100644 --- a/backend/src/api/index.ts +++ b/backend/src/api/index.ts @@ -33,7 +33,7 @@ import { tenantMiddleware } from '../middlewares/tenantMiddleware' import { createRateLimiter } from './apiRateLimiter' import authSocial from './auth/authSocial' import { publicRouter } from './public' -import { mountInteractivityRoute } from './slack' +import { mountEventsRoute, mountInteractivityRoute } from './slack' import WebSockets from './websockets' const serviceLogger = getServiceLogger() @@ -112,6 +112,7 @@ setImmediate(async () => { // Mounted before DB/Redis/OpenSearch middleware to protect Slack's 3s ack window. mountInteractivityRoute(app) + mountEventsRoute(app, redis) // Initializes and adds the database middleware. app.use(databaseMiddleware) diff --git a/backend/src/api/slack/eventDeduplication.ts b/backend/src/api/slack/eventDeduplication.ts new file mode 100644 index 0000000000..d27c53ad2f --- /dev/null +++ b/backend/src/api/slack/eventDeduplication.ts @@ -0,0 +1,12 @@ +import type { RedisClient } from '@crowd/redis' + +const EVENT_KEY_PREFIX = 'slack_event' +const EVENT_TTL_SECONDS = 15 * 60 + +export async function claimSlackEvent(redis: RedisClient, eventId: string): Promise { + const result = await redis.set(`${EVENT_KEY_PREFIX}:${eventId}`, '1', { + NX: true, + EX: EVENT_TTL_SECONDS, + }) + return result === 'OK' +} diff --git a/backend/src/api/slack/events.test.ts b/backend/src/api/slack/events.test.ts new file mode 100644 index 0000000000..d5ca0526c0 --- /dev/null +++ b/backend/src/api/slack/events.test.ts @@ -0,0 +1,106 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('./verifySignature', () => ({ verifySlackSignature: vi.fn(() => true) })) +vi.mock('@/services/slack/requestClassificationBot', () => ({ + runRequestClassificationBot: vi.fn(async () => undefined), +})) + +import { runRequestClassificationBot } from '@/services/slack/requestClassificationBot' + +import { createEventsHandler } from './events' +import { verifySlackSignature } from './verifySignature' + +const claimEvent = vi.fn(async () => true) +const events = createEventsHandler(claimEvent) + +function call(body: object, headers: Record = {}) { + const req = { + body, + headers, + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + } as any + const res = { sendStatus: vi.fn(), json: vi.fn() } as any + return events(req, res).then(() => ({ req, res })) +} + +const mention = { + type: 'event_callback', + event_id: 'Ev1', + event: { type: 'app_mention', text: '<@U0BOT> hi', channel: 'C1', ts: '100.1' }, +} + +describe('slack events', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(verifySlackSignature).mockReturnValue(true) + claimEvent.mockResolvedValue(true) + }) + + it('answers the url verification challenge', async () => { + const { res } = await call({ type: 'url_verification', challenge: 'abc' }) + + expect(res.json).toHaveBeenCalledWith({ challenge: 'abc' }) + }) + + it('acks and classifies a mention, replying in the message thread', async () => { + const { res } = await call(mention) + + expect(res.sendStatus).toHaveBeenCalledWith(200) + expect(runRequestClassificationBot).toHaveBeenCalledWith( + expect.objectContaining({ + text: '<@U0BOT> hi', + channelId: 'C1', + messageTs: '100.1', + threadTs: '100.1', + }), + ) + }) + + it('replies in the existing thread when the mention is inside one', async () => { + await call({ ...mention, event: { ...mention.event, thread_ts: '90.0' } }) + + expect(runRequestClassificationBot).toHaveBeenCalledWith( + expect.objectContaining({ threadTs: '90.0', messageTs: '100.1' }), + ) + }) + + it('processes a Slack retry when the event was not handled yet', async () => { + await call(mention, { 'x-slack-retry-num': '1' }) + + expect(runRequestClassificationBot).toHaveBeenCalledTimes(1) + }) + + it('ignores an event that was already handled', async () => { + claimEvent.mockResolvedValue(false) + + await call(mention, { 'x-slack-retry-num': '1' }) + + expect(claimEvent).toHaveBeenCalledWith('Ev1') + expect(runRequestClassificationBot).not.toHaveBeenCalled() + }) + + it('handles the event when deduplication is unavailable', async () => { + claimEvent.mockRejectedValue(new Error('redis down')) + + const { req } = await call(mention) + + expect(req.log.warn).toHaveBeenCalled() + expect(runRequestClassificationBot).toHaveBeenCalledTimes(1) + }) + + it('ignores bot messages and other event types', async () => { + await call({ ...mention, event: { ...mention.event, bot_id: 'B1' } }) + await call({ ...mention, event: { ...mention.event, type: 'message' } }) + + expect(runRequestClassificationBot).not.toHaveBeenCalled() + }) + + it('does nothing for unverified requests', async () => { + vi.mocked(verifySlackSignature).mockReturnValue(false) + + const { res } = await call(mention) + + expect(res.sendStatus).toHaveBeenCalledWith(200) + expect(runRequestClassificationBot).not.toHaveBeenCalled() + }) +}) diff --git a/backend/src/api/slack/events.ts b/backend/src/api/slack/events.ts new file mode 100644 index 0000000000..8798d89bec --- /dev/null +++ b/backend/src/api/slack/events.ts @@ -0,0 +1,94 @@ +import type { Request, Response } from 'express' +import { z } from 'zod' + +import { runRequestClassificationBot } from '@/services/slack/requestClassificationBot' +import { validateOrThrow } from '@/utils/validation' +import { getErrorMessage } from '@crowd/common' + +import { verifySlackSignature } from './verifySignature' + +const URL_VERIFICATION_TYPE = 'url_verification' +const EVENT_CALLBACK_TYPE = 'event_callback' +const APP_MENTION_EVENT_TYPE = 'app_mention' + +const payloadSchema = z.discriminatedUnion('type', [ + z.object({ type: z.literal(URL_VERIFICATION_TYPE), challenge: z.string() }), + z.object({ + type: z.literal(EVENT_CALLBACK_TYPE), + event_id: z.string(), + event: z.object({ + type: z.string(), + text: z.string().optional(), + channel: z.string().optional(), + ts: z.string().optional(), + thread_ts: z.string().optional(), + bot_id: z.string().optional(), + }), + }), +]) + +type EventPayload = z.infer + +async function isFirstDelivery( + eventId: string, + claimEvent: (eventId: string) => Promise, + req: Request, +): Promise { + try { + return await claimEvent(eventId) + } catch (err) { + req.log.warn({ error: getErrorMessage(err), eventId }, 'Could not deduplicate Slack event.') + return true + } +} + +function dispatchAppMention( + payload: Extract, + req: Request, +) { + const { event } = payload + const isUserMention = event.type === APP_MENTION_EVENT_TYPE && !event.bot_id + if (!isUserMention || !event.channel || !event.ts) { + return + } + + runRequestClassificationBot({ + text: event.text ?? '', + channelId: event.channel, + messageTs: event.ts, + threadTs: event.thread_ts ?? event.ts, + options: { log: req.log }, + }).catch((err) => req.log.error(err, 'Slack request bot failed unexpectedly!')) +} + +// Mounted ahead of responseHandlerMiddleware, so errors are handled here +// directly instead of via the global errorMiddleware. +export function createEventsHandler(claimEvent: (eventId: string) => Promise) { + return async (req: Request, res: Response) => { + if (!verifySlackSignature(req)) { + req.log.warn('Received unverified Slack event!') + res.sendStatus(200) + return + } + + try { + const payload = validateOrThrow(payloadSchema, req.body) + + if (payload.type === URL_VERIFICATION_TYPE) { + res.json({ challenge: payload.challenge }) + return + } + + res.sendStatus(200) + + if (!(await isFirstDelivery(payload.event_id, claimEvent, req))) { + return + } + + dispatchAppMention(payload, req) + } catch (err) { + req.log.error(err, 'Error processing Slack event!') + res.sendStatus(200) + } + } +} diff --git a/backend/src/api/slack/index.ts b/backend/src/api/slack/index.ts index 428b05c2f1..ec42a8abf8 100644 --- a/backend/src/api/slack/index.ts +++ b/backend/src/api/slack/index.ts @@ -1,11 +1,13 @@ import bodyParser from 'body-parser' import type { Application, NextFunction, Request, Response } from 'express' +import type { RedisClient } from '@crowd/redis' import { getSlackBotConfig } from '@crowd/slack' import { SLACK_CONFIG } from '../../conf/index' import { safeWrap } from '../../middlewares/errorMiddleware' import { createRateLimiter } from '../apiRateLimiter' +import { claimSlackEvent } from './eventDeduplication' // Mounted ahead of the shared rate limiter and tenant/segment middleware // to protect Slack's 3-second acknowledgement window; keeps its own limiter. @@ -38,6 +40,35 @@ export function mountInteractivityRoute(app: Application): void { ) } +export function mountEventsRoute(app: Application, redis: RedisClient): void { + if (!getSlackBotConfig().signingSecret) { + return + } + + const captureRawBody = (req: Request, _res: Response, buf: Buffer) => { + req.rawBody = buf + } + + const eventsRateLimiter = createRateLimiter({ + max: 200, + windowMs: 60 * 1000, + }) + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const handleParserError = (err: Error, req: Request, res: Response, _next: NextFunction) => { + req.log.error(err, 'Error parsing Slack event payload!') + res.sendStatus(200) + } + + app.post( + '/v1/slack/events', + eventsRateLimiter, + bodyParser.json({ limit: '5mb', verify: captureRawBody }), + handleParserError, + require('./events').createEventsHandler((eventId: string) => claimSlackEvent(redis, eventId)), + ) +} + export default (app) => { if ( SLACK_CONFIG.onboardingAppId && diff --git a/backend/src/services/slack/requestClassificationBot.run.test.ts b/backend/src/services/slack/requestClassificationBot.run.test.ts new file mode 100644 index 0000000000..7b0b584fc4 --- /dev/null +++ b/backend/src/services/slack/requestClassificationBot.run.test.ts @@ -0,0 +1,76 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@crowd/slack', () => ({ + getSlackPermalink: vi.fn(async () => 'https://slack.test/permalink'), + postSlackMessage: vi.fn(async () => ({ ok: true })), +})) +vi.mock('@crowd/project-onboarding/src/requestClassifierDeps', () => ({ + withRequestClassifierDeps: vi.fn(async () => ({ + resolution: { kind: 'lf_not_in_pcc', projectName: 'Acme' }, + node: 'lf_not_in_pcc_flag_human', + trace: { parsed: null, pccLookup: null, cdpLookup: null, failure: null }, + })), +})) +vi.mock('./slackBackground', () => ({ getBgQx: vi.fn(async () => ({})) })) + +import { getSlackPermalink, postSlackMessage } from '@crowd/slack' + +import { runRequestClassificationBot } from './requestClassificationBot' + +const log = { info: vi.fn(), warn: vi.fn(), error: vi.fn() } as any + +function run(overrides: Record = {}) { + return runRequestClassificationBot({ + text: '<@U0BOT> onboard Acme', + channelId: 'C1', + messageTs: '100.1', + threadTs: '90.0', + options: { log }, + ...overrides, + }) +} + +describe('runRequestClassificationBot', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(postSlackMessage).mockResolvedValue({ ok: true }) + }) + + it('links the mentioned message and replies in the thread', async () => { + await run() + + expect(getSlackPermalink).toHaveBeenCalledWith('C1', '100.1') + expect(postSlackMessage).toHaveBeenCalledWith( + expect.objectContaining({ channel: 'C1', thread_ts: '90.0' }), + ) + expect(log.warn).not.toHaveBeenCalled() + }) + + it('disables the Snowflake platform detection before loading the classifier deps', async () => { + delete process.env.SNOWFLAKE_DISABLE_PLATFORM_DETECTION + + await run() + + expect(process.env.SNOWFLAKE_DISABLE_PLATFORM_DETECTION).toBe('true') + }) + + it('warns when Slack does not deliver the reply', async () => { + vi.mocked(postSlackMessage).mockResolvedValue({ ok: false, error: 'channel_not_found' }) + + await run() + + expect(log.warn).toHaveBeenCalledWith( + expect.objectContaining({ channelId: 'C1', error: 'channel_not_found' }), + 'Slack bot reply was not delivered.', + ) + }) + + it('asks for the request details when only the mention is left', async () => { + await run({ text: '<@U0BOT>' }) + + expect(getSlackPermalink).not.toHaveBeenCalled() + expect(postSlackMessage).toHaveBeenCalledWith( + expect.objectContaining({ text: expect.stringContaining('Tell me about the project') }), + ) + }) +}) diff --git a/backend/src/services/slack/requestClassificationBot.test.ts b/backend/src/services/slack/requestClassificationBot.test.ts new file mode 100644 index 0000000000..8d39959120 --- /dev/null +++ b/backend/src/services/slack/requestClassificationBot.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@crowd/slack', () => ({ getSlackPermalink: vi.fn(), postSlackMessage: vi.fn() })) +vi.mock('@crowd/project-onboarding/src/requestClassifierDeps', () => ({ + withRequestClassifierDeps: vi.fn(), +})) +vi.mock('./slackBackground', () => ({ getBgQx: vi.fn() })) + +import { IRequestClassification } from '@crowd/project-onboarding' + +import { + buildClassificationReply, + hideFailureDetails, + toRequestText, +} from './requestClassificationBot' + +describe('toRequestText', () => { + it('removes the bot mention and keeps the rest of the message', () => { + expect(toRequestText('<@U0BOT> onboard Acme\nnot LF')).toBe('onboard Acme\nnot LF') + }) + + it('unwraps Slack links so the parser sees plain URLs', () => { + expect( + toRequestText( + '<@U0BOT> and ', + ), + ).toBe('https://github.com/acme/one and https://github.com/acme/two') + }) + + it('decodes the HTML entities Slack applies to the message text', () => { + expect(toRequestText('<@U0BOT> onboard R&D <internal>')).toBe( + 'onboard R&D ', + ) + }) + + it('returns an empty string when only the mention is left', () => { + expect(toRequestText('<@U0BOT> ')).toBe('') + }) +}) + +describe('buildClassificationReply', () => { + const classification: IRequestClassification = { + resolution: { kind: 'lf_not_in_pcc', projectName: 'Acme' }, + node: 'lf_not_in_pcc_flag_human', + trace: { + parsed: null, + pccLookup: null, + cdpLookup: null, + failure: null, + }, + } + + it('names the step reached and states that nothing was done', () => { + const { blocks } = buildClassificationReply(classification, 'https://slack.test/thread') + const text = JSON.stringify(blocks) + + expect(text).toContain('lf_not_in_pcc_flag_human') + expect(text).toContain('[DRY RUN]') + expect(text).toContain('https://slack.test/thread') + }) + + it('keeps every section under the Slack limit when the request lists many repositories', () => { + const repoUrls = Array.from({ length: 300 }, (_, i) => `https://github.com/acme/repo-${i}`) + const { blocks } = buildClassificationReply( + { + ...classification, + trace: { ...classification.trace, parsed: { githubRepoUrls: repoUrls } as any }, + }, + 'https://slack.test/thread', + ) + + const longest = Math.max(...blocks.map((block: any) => block.text?.text.length ?? 0)) + expect(longest).toBeLessThanOrEqual(3000) + }) + + it('adds a plain text fallback for clients that do not render blocks', () => { + const message = buildClassificationReply(classification, 'https://slack.test/thread') + + expect(message.text).toContain('lf_not_in_pcc_flag_human') + }) + + it('does not expose dependency errors or model output in the reply', () => { + const failed: IRequestClassification = { + resolution: { + kind: 'ambiguous', + reason: 'Classification failed: SQL compilation error at line 11', + candidates: [], + }, + node: 'ambiguous_human_review', + trace: { + ...classification.trace, + failure: { stage: 'resolve', reason: 'SQL compilation error at line 11' }, + }, + } + + const text = JSON.stringify( + buildClassificationReply(hideFailureDetails(failed), 'https://slack.test/thread'), + ) + + expect(text).not.toContain('SQL compilation error') + expect(text).toContain('could not be classified automatically') + }) +}) diff --git a/backend/src/services/slack/requestClassificationBot.ts b/backend/src/services/slack/requestClassificationBot.ts new file mode 100644 index 0000000000..42839182bd --- /dev/null +++ b/backend/src/services/slack/requestClassificationBot.ts @@ -0,0 +1,132 @@ +import { Message, Section, SlackMessageDto } from 'slack-block-builder' + +import { getErrorMessage } from '@crowd/common' +import { + IRequestClassification, + buildRequestClassificationAlert, + buildRequestClassificationAlertTitle, + classifyOnboardingRequest, +} from '@crowd/project-onboarding' +import { getSlackPermalink, postSlackMessage } from '@crowd/slack' + +import { IServiceOptions } from '../IServiceOptions' +import { getBgQx } from './slackBackground' + +const MENTION_PATTERN = /<@[A-Z0-9]+>/g +const LINK_WITH_LABEL_PATTERN = /<(https?:\/\/[^|>\s]+)\|[^>]*>/g +const LINK_PATTERN = /<(https?:\/\/[^>\s]+)>/g + +const MAX_SECTION_TEXT = 2900 + +const FAILURE_REASON = 'The request could not be classified automatically.' + +const HELP_TEXT = + 'Tell me about the project you want to onboard: its name, whether it is a Linux Foundation project and the GitHub repositories.' + +function truncateSectionText(text: string): string { + return text.length > MAX_SECTION_TEXT ? `${text.slice(0, MAX_SECTION_TEXT - 1)}…` : text +} + +async function loadRequestClassifierDeps() { + // TODO(CM-1841): drop once snowflake-sdk stops building a region-less STS client on import + process.env.SNOWFLAKE_DISABLE_PLATFORM_DETECTION ??= 'true' + return import('@crowd/project-onboarding/src/requestClassifierDeps') +} + +export function toRequestText(slackText: string): string { + return slackText + .replace(MENTION_PATTERN, '') + .replace(LINK_WITH_LABEL_PATTERN, '$1') + .replace(LINK_PATTERN, '$1') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/&/g, '&') + .trim() +} + +export function hideFailureDetails(classification: IRequestClassification): IRequestClassification { + if (!classification.trace.failure) { + return classification + } + + return { + ...classification, + resolution: { kind: 'ambiguous', reason: FAILURE_REASON, candidates: [] }, + } +} + +export function buildClassificationReply( + classification: IRequestClassification, + requestUrl: string, +): SlackMessageDto { + const alert = { + sourceUrl: requestUrl, + repoUrls: classification.trace.parsed?.githubRepoUrls ?? [], + resolution: classification.resolution, + dryRun: true, + } + const sections = buildRequestClassificationAlert(alert) + + return Message() + .text(`${buildRequestClassificationAlertTitle(alert)}. Step reached: ${classification.node}`) + .blocks( + Section({ text: `*${buildRequestClassificationAlertTitle(alert)}*` }), + ...sections.map(({ title, text }) => + Section({ text: truncateSectionText(title ? `*${title}*\n${text}` : text) }), + ), + Section({ text: `_Step reached: ${classification.node}_` }), + ) + .buildToObject() +} + +export async function runRequestClassificationBot({ + text, + channelId, + messageTs, + threadTs, + options, +}: { + text: string + channelId: string + messageTs: string + threadTs: string + options: Pick +}): Promise { + const { log } = options + const reply = async (message: SlackMessageDto | { text: string }) => { + const result = await postSlackMessage({ channel: channelId, thread_ts: threadTs, ...message }) + if (!result.ok) { + log.warn({ channelId, threadTs, error: result.error }, 'Slack bot reply was not delivered.') + } + } + + const requestText = toRequestText(text) + if (!requestText) { + await reply({ text: HELP_TEXT }) + return + } + + try { + const { withRequestClassifierDeps } = await loadRequestClassifierDeps() + const qx = await getBgQx() + const classification = await withRequestClassifierDeps(qx, (deps) => + classifyOnboardingRequest(requestText, deps), + ) + const requestUrl = (await getSlackPermalink(channelId, messageTs)) ?? '' + + log.info( + { + channelId, + threadTs, + node: classification.node, + kind: classification.resolution.kind, + failure: classification.trace.failure, + }, + 'Onboarding request classified from Slack.', + ) + await reply(buildClassificationReply(hideFailureDetails(classification), requestUrl)) + } catch (err) { + log.error({ error: getErrorMessage(err), channelId, threadTs }, 'Slack request failed.') + await reply({ text: ':no_entry: I could not process this request, please try again later.' }) + } +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 74143e26dc..5612fc1d7f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2804,6 +2804,9 @@ importers: '@crowd/logging': specifier: workspace:* version: link:../logging + '@crowd/slack': + specifier: workspace:* + version: link:../slack '@crowd/snowflake': specifier: workspace:* version: link:../snowflake diff --git a/services/apps/automatic_projects_discovery_worker/src/activities/activities.ts b/services/apps/automatic_projects_discovery_worker/src/activities/activities.ts index e3288d3ae8..b603fbc968 100644 --- a/services/apps/automatic_projects_discovery_worker/src/activities/activities.ts +++ b/services/apps/automatic_projects_discovery_worker/src/activities/activities.ts @@ -23,22 +23,19 @@ import { } from '@crowd/data-access-layer/src/project-catalog/types' import { pgpQx } from '@crowd/data-access-layer/src/queryExecutor' import { getServiceLogger } from '@crowd/logging' -import { countNodes } from '@crowd/project-onboarding' +import { + IRequestClassificationAlert, + buildRequestClassificationAlert, + buildRequestClassificationAlertTitle, + countNodes, +} from '@crowd/project-onboarding' import { withRequestClassifierDeps } from '@crowd/project-onboarding/src/requestClassifierDeps' import { SlackChannel, SlackPersona, sendSlackNotificationAsync } from '@crowd/slack' import { svc } from '../main' import { getAvailableSourceNames, getSource } from '../sources/registry' import { IDatasetDescriptor } from '../sources/types' -import { - IClassifiedRows, - IRequestClassificationAlert, - classifyDiscussions, -} from './requestClassification' -import { - buildRequestClassificationAlert, - buildRequestClassificationAlertTitle, -} from './requestClassificationAlert' +import { IClassifiedRows, classifyDiscussions } from './requestClassification' const log = getServiceLogger() diff --git a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassification.ts b/services/apps/automatic_projects_discovery_worker/src/activities/requestClassification.ts index 7f5522e4d4..f48e60d11a 100644 --- a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassification.ts +++ b/services/apps/automatic_projects_discovery_worker/src/activities/requestClassification.ts @@ -3,6 +3,7 @@ import { getServiceLogger } from '@crowd/logging' import { CdpIntegrationAction, ClassificationNode, + IRequestClassificationAlert, IRequestClassificationDeps, OnboardingResolution, buildClassificationLogEntry, @@ -11,13 +12,6 @@ import { const log = getServiceLogger() -export interface IRequestClassificationAlert { - sourceUrl: string - repoUrls: string[] - resolution: OnboardingResolution - dryRun: boolean -} - export interface IClassifiedRows { rows: IDbProjectCatalogCreate[] alerts: IRequestClassificationAlert[] diff --git a/services/libs/project-onboarding/package.json b/services/libs/project-onboarding/package.json index 8b1afb76c0..7196c04a21 100644 --- a/services/libs/project-onboarding/package.json +++ b/services/libs/project-onboarding/package.json @@ -7,6 +7,7 @@ "@crowd/common_services": "workspace:*", "@crowd/data-access-layer": "workspace:*", "@crowd/logging": "workspace:*", + "@crowd/slack": "workspace:*", "@crowd/snowflake": "workspace:*", "@crowd/types": "workspace:*" }, diff --git a/services/libs/project-onboarding/src/index.ts b/services/libs/project-onboarding/src/index.ts index 8ea01d9871..25379ed7f5 100644 --- a/services/libs/project-onboarding/src/index.ts +++ b/services/libs/project-onboarding/src/index.ts @@ -2,6 +2,7 @@ export * from './classificationTrace' export * from './classifyRequest' export * from './onboarder' export * from './pccLookup' +export * from './requestClassificationAlert' export * from './requestParser' export * from './requestResolver' export * from './types' diff --git a/services/libs/project-onboarding/src/pccLookup.test.ts b/services/libs/project-onboarding/src/pccLookup.test.ts index 351c120e96..336ec2e38d 100644 --- a/services/libs/project-onboarding/src/pccLookup.test.ts +++ b/services/libs/project-onboarding/src/pccLookup.test.ts @@ -29,16 +29,17 @@ describe('createPccCandidatesLookup', () => { }) it('excludes internal projects', () => { - expect(PCC_CANDIDATES_QUERY).toContain('NOT IS_INTERNAL_PROJECT') + expect(PCC_CANDIDATES_QUERY).toContain('NOT p.IS_INTERNAL_PROJECT') }) it('skips rows without a name or slug so null scores cannot outrank real matches', () => { - expect(PCC_CANDIDATES_QUERY).toContain('NAME IS NOT NULL') - expect(PCC_CANDIDATES_QUERY).toContain('SLUG IS NOT NULL') + expect(PCC_CANDIDATES_QUERY).toContain('p.NAME IS NOT NULL') + expect(PCC_CANDIDATES_QUERY).toContain('p.SLUG IS NOT NULL') }) it('flags leaf projects and orders ties deterministically', () => { expect(PCC_CANDIDATES_QUERY).toContain('AS IS_LEAF') - expect(PCC_CANDIDATES_QUERY).toContain('ORDER BY SCORE DESC, IS_LEAF DESC, PROJECT_ID') + expect(PCC_CANDIDATES_QUERY).not.toContain('NOT IN') + expect(PCC_CANDIDATES_QUERY).toContain('ORDER BY SCORE DESC, IS_LEAF DESC, p.PROJECT_ID') }) }) diff --git a/services/libs/project-onboarding/src/pccLookup.ts b/services/libs/project-onboarding/src/pccLookup.ts index 3bf5efb805..9bc4713ce3 100644 --- a/services/libs/project-onboarding/src/pccLookup.ts +++ b/services/libs/project-onboarding/src/pccLookup.ts @@ -5,23 +5,24 @@ const SNOWFLAKE_MAX_SCORE = 100 export const PCC_CANDIDATES_QUERY = ` SELECT - PROJECT_ID, - NAME, - SLUG, + p.PROJECT_ID, + p.NAME, + p.SLUG, GREATEST( - JAROWINKLER_SIMILARITY(LOWER(NAME), ?), - JAROWINKLER_SIMILARITY(LOWER(SLUG), ?) + JAROWINKLER_SIMILARITY(LOWER(p.NAME), ?), + JAROWINKLER_SIMILARITY(LOWER(p.SLUG), ?) ) AS SCORE, - PROJECT_ID NOT IN ( - SELECT DISTINCT PARENT_ID - FROM ANALYTICS.SILVER_DIM.PROJECTS - WHERE PARENT_ID IS NOT NULL - ) AS IS_LEAF - FROM ANALYTICS.SILVER_DIM.PROJECTS - WHERE NOT IS_INTERNAL_PROJECT - AND NAME IS NOT NULL - AND SLUG IS NOT NULL - ORDER BY SCORE DESC, IS_LEAF DESC, PROJECT_ID + parents.PARENT_ID IS NULL AS IS_LEAF + FROM ANALYTICS.SILVER_DIM.PROJECTS p + LEFT JOIN ( + SELECT DISTINCT PARENT_ID + FROM ANALYTICS.SILVER_DIM.PROJECTS + WHERE PARENT_ID IS NOT NULL + ) parents ON parents.PARENT_ID = p.PROJECT_ID + WHERE NOT p.IS_INTERNAL_PROJECT + AND p.NAME IS NOT NULL + AND p.SLUG IS NOT NULL + ORDER BY SCORE DESC, IS_LEAF DESC, p.PROJECT_ID LIMIT ${MAX_PCC_CANDIDATES} ` diff --git a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.test.ts b/services/libs/project-onboarding/src/requestClassificationAlert.test.ts similarity index 80% rename from services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.test.ts rename to services/libs/project-onboarding/src/requestClassificationAlert.test.ts index 8c68c5474e..d1368bad66 100644 --- a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.test.ts +++ b/services/libs/project-onboarding/src/requestClassificationAlert.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' -import { IRequestClassificationAlert } from './requestClassification' import { + IRequestClassificationAlert, buildRequestClassificationAlert, buildRequestClassificationAlertTitle, } from './requestClassificationAlert' @@ -99,4 +99,28 @@ describe('dry run alert body', () => { expect(sections.map((section) => section.title)).not.toContain('Dry run') }) + + it('escapes Slack control characters in untrusted values', () => { + const sections = buildRequestClassificationAlert( + alert({ + kind: 'ambiguous', + reason: 'ping & <@U1>', + candidates: [{ ...pccProject, name: '' }], + }), + ) + const text = JSON.stringify(sections) + + expect(text).toContain('ping <!channel> & <@U1>') + expect(text).toContain('<!here>') + expect(text).not.toContain('') + expect(text).not.toContain('') + }) + + it('keeps the discussion link intact', () => { + const [intro] = buildRequestClassificationAlert( + alert({ kind: 'lf_not_in_pcc', projectName: '' }), + ) + + expect(intro.text).toContain(' = { non_github_source: 'Onboarding request without GitHub repositories', @@ -18,9 +24,13 @@ const INTEGRATION_ACTION_LABELS = { human_review: 'Human review: GitHub v1 integration, migration to v2 needed', } as const +export function escapeSlackText(value: string): string { + return value.replace(/&/g, '&').replace(//g, '>') +} + function formatCandidate(candidate: IPccCandidate): string { const level = candidate.isLeaf ? 'project' : 'parent group' - return `• ${candidate.name} (${candidate.slug}), score ${candidate.score.toFixed(2)}, ${level}` + return `• ${escapeSlackText(candidate.name)} (${escapeSlackText(candidate.slug)}), score ${candidate.score.toFixed(2)}, ${level}` } function formatCandidates(candidates: IPccCandidate[]): string { @@ -30,11 +40,16 @@ function formatCandidates(candidates: IPccCandidate[]): string { function resolutionSections(resolution: OnboardingResolution): SlackMessageSection[] { switch (resolution.kind) { case 'non_github_source': - return [{ title: 'Repositories', text: resolution.nonGithubRepoUrls.join('\n') }] + return [ + { + title: 'Repositories', + text: resolution.nonGithubRepoUrls.map(escapeSlackText).join('\n'), + }, + ] case 'non_lf_new_project': return [{ title: 'Outcome', text: 'Would be onboarded as a non-LF project' }] case 'lf_not_in_pcc': - return [{ title: 'Project name', text: resolution.projectName }] + return [{ title: 'Project name', text: escapeSlackText(resolution.projectName) }] case 'lf_not_in_cdp': return [{ title: 'PCC project', text: formatCandidate(resolution.pccProject) }] case 'lf_in_cdp': @@ -42,13 +57,13 @@ function resolutionSections(resolution: OnboardingResolution): SlackMessageSecti { title: 'PCC project', text: formatCandidate(resolution.pccProject) }, { title: 'CDP segment', - text: `${resolution.segment.name}\nIntegration: ${resolution.segment.integration}`, + text: `${escapeSlackText(resolution.segment.name)}\nIntegration: ${resolution.segment.integration}`, }, { title: 'Proposed action', text: INTEGRATION_ACTION_LABELS[resolution.action] }, ] case 'ambiguous': return [ - { title: 'Reason', text: resolution.reason }, + { title: 'Reason', text: escapeSlackText(resolution.reason) }, ...(resolution.candidates.length > 0 ? [{ title: 'PCC candidates', text: formatCandidates(resolution.candidates) }] : []), @@ -71,7 +86,7 @@ export function buildRequestClassificationAlert( return [ { title: '', - text: [`Requested in: ${requestedIn}`, ...alert.repoUrls].join('\n'), + text: [`Requested in: ${requestedIn}`, ...alert.repoUrls.map(escapeSlackText)].join('\n'), }, ...resolutionSections(alert.resolution), ...(alert.dryRun diff --git a/services/libs/project-onboarding/tsconfig.json b/services/libs/project-onboarding/tsconfig.json index 16a548dcf8..6216af6521 100644 --- a/services/libs/project-onboarding/tsconfig.json +++ b/services/libs/project-onboarding/tsconfig.json @@ -14,6 +14,9 @@ { "path": "../logging" }, + { + "path": "../slack" + }, { "path": "../snowflake" },